Implement lfx api command for raw authenticated API calls

[emsearcy]

Summary

Implements lfx api <path> for making raw authenticated HTTP requests against LFX platform APIs (similar to gh api), per the plan in LFXV2-2517.

• -X/--method: GET, POST, PUT, or DELETE (default GET; auto-promotes to POST when a body is explicitly supplied via --field/--raw-field/--input and no --method was passed, matching gh api/curl convention). PATCH is intentionally excluded — the LFX API has no PATCH endpoints, favoring PUT with ETag/If-Match concurrency control.
• Auto-injects Authorization: Bearer <token> via a new resolveAccessToken helper, refactored out of newAuthTokenCommand so auth token and api share one code path (including audience resolution as the default API base URL).
• -H/--header (repeatable), --input <file>|-, -F/--field (gh-style type coercion) / --raw-field (string-only, mutually exclusive with --input), and automatic stdin body detection when piped with no explicit body flag.
• -q/--query <gjson-expr> to filter the response body before output (new github.com/tidwall/gjson dependency).
• --hostname to override the resolved API base URL (advanced/debugging escape hatch), rejected unless HTTPS (or loopback) via apiRequireHTTPS, and redirects are never auto-followed so a 3xx response can't bypass that check or silently leak the bearer token.
• Non-2xx responses print HTTP <status> to stderr and exit 1.
• --insecure-storage/--backend credential-store selectors were moved from auth to the root lfx command so lfx api can share them too; verified they still work in any position in the command line (urfave/cli v3 resolves persistent flags across the whole chain), so no README changes were needed there.

Once this lands, the parent epic LFXV2-2509 can be closed out — this was its last remaining open child story.

Testing

• make check (fmt, vet, lint, revive) and make test all pass.
• Added internal/commands/api_test.go covering coerceFieldValue, apiJoinURL, apiHasExplicitBody, and apiRequestBody (field/raw-field merging, --input from file, piped-stdin auto-detection, stdin Stat() failure, and the mutual-exclusion/malformed-flag error paths). End-to-end coverage of runAPI itself (method promotion, headers, query filtering, non-2xx handling) is tracked separately in #7.
• Manually smoke-tested against the real prod API using a live login: lfx api '/my-grants?v=1&object_type=projects' returned {"grants":[]} with exit 0.

Assisted-by: github-copilot:claude-sonnet-5

🤖 Generated with GitHub Copilot (via OpenCode)

[Copilot]

Pull request overview

Implements authenticated raw API requests through lfx api.

Changes:

• Adds request methods, bodies, headers, response querying, and error handling.
• Shares access-token resolution with lfx auth token.
• Adds helper tests, documentation, and GJSON dependencies.

Reviewed changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
README.md	Documents API usage.
internal/credstore/credstore.go	Removes a stale ticket reference from a comment.
internal/commands/auth.go	Extracts shared token resolution.
internal/commands/api.go	Implements the API command.
internal/commands/api_test.go	Tests API helpers and body handling.
go.mod	Adds GJSON dependencies.
go.sum	Records dependency checksums.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[emsearcy]

Addressed the Copilot review feedback in b970ee0:

• Moved --insecure-storage/--backend to the root lfx command (new exported commands.CredentialStoreFlags) so lfx api can select a credential-store backend the same way lfx auth subcommands do.
• lfx api now rejects extra positional arguments instead of silently ignoring them.
• Fixed a spurious blank Error: line on non-2xx responses by routing the status through cli.Exit's message instead of writing directly to stderr.
• Replaced strconv.ParseFloat-based --field number coercion with a JSON-number-syntax regex + json.Number, so NaN/Inf/hex floats stay strings and large integers (>2^53) don't silently lose precision.

The remaining test-coverage gap (no test exercises runAPI itself) is tracked separately in #7, left unprescribed on approach.

Assisted-by: github-copilot:claude-sonnet-5

[Copilot]

Pull request overview

Copilot reviewed 8 out of 9 changed files in this pull request and generated 3 comments.

[emsearcy]

Addressed a follow-up round of Copilot review feedback in 5be4fb3:

• Response body is now written verbatim via os.Stdout.Write instead of fmt.Println, which appended an extra newline byte not present in the original response and corrupted raw/binary output.
• POST/PUT requests with a body now default Content-Type: application/json unless overridden via -H (LFX APIs are overwhelmingly JSON), so the README's --input/stdin example no longer sends a body with no Content-Type.
• AGENTS.md's "Current State" section no longer describes lfx api as an unimplemented stub.

Assisted-by: github-copilot:claude-sonnet-5

[Copilot]

Pull request overview

Copilot reviewed 9 out of 10 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

Copilot reviewed 9 out of 10 changed files in this pull request and generated 2 comments.

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

internal/commands/api.go:223

• Checking the parsed value does not distinguish an omitted flag from --input=. In the latter case the command neither auto-promotes to POST nor reports the empty filename; it silently falls through to implicit stdin handling (or sends an empty GET). Use cmd.IsSet(apiInputFlagName) consistently when deciding whether --input was supplied, so the empty value reaches os.ReadFile("") and produces an error.

func apiHasExplicitBody(cmd *cli.Command) bool {
	return cmd.String(apiInputFlagName) != "" ||

[Copilot]

Pull request overview

Copilot reviewed 9 out of 10 changed files in this pull request and generated no new comments.

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

internal/commands/api.go:233

• An explicitly supplied empty --input= is treated as though the flag were absent. This also bypasses the --input/field mutual-exclusion check and may consume implicit stdin, so an unset script variable can produce a different request instead of failing. Use IsSet to recognize the flag and reject an empty value before selecting the body source.

	return cmd.String(apiInputFlagName) != "" ||

[andrest50]

Solid implementation — the security layering (HTTPS enforcement + redirect blocking together), the resolveAccessToken refactor, and the json.Number precision fix are all well-considered. Tests follow established patterns. Approving; inline comments below are a mix of one thing to verify before merging and a few low-effort follow-ups.

No HTTP-level test for runAPI itself — pure helpers are well covered, but auth injection, Content-Type defaulting, redirect blocking, and non-2xx exit aren't exercised by CI. An httptest.NewTLSServer-based test for the happy path + a 4xx case would close that gap (can land in a follow-up).

[emsearcy]

AI-assisted summary: Addressed the latest round of review feedback (3833f79):

• resp.Body.Close() error is now checked (fixes a real errcheck finding surfaced by an updated local golangci-lint).
• apiHasExplicitBody uses cmd.IsSet instead of a string-empty check.
• apiJoinURL now uses url.JoinPath instead of manual concatenation — confirmed Go's net/url/net/http don't normalize ../ segments on their own, so this was a real (if narrow) hardening gap, not just style; url.JoinPath also fixes missing percent-encoding of path segments.
• Renamed a local url variable in runAPI that shadowed the net/url package import.
• Documented (rather than changed) two intentional trade-offs flagged as nits: Header.Set vs Add for repeated -H, and the lack of a trailing newline on -q output (matches gh api -q).
• Verified the flag-ordering concern on --insecure-storage/--backend moving to root lfx: all positions (lfx --insecure-storage auth login, lfx auth --insecure-storage login, lfx auth login --insecure-storage) work correctly against the built binary, since urfave/cli v3 resolves persistent flags across the whole command chain. No regression, no README update needed.

make check (fmt/vet/lint/revive) and make test pass. PR description updated to reflect make lint now passing and to fold in the flag-position note.

[emsearcy]

AI-assisted: @dealako — good catch, addressed in 67af917: added a one-line note to --hostname's usage text that it sends your bearer token to whatever HTTPS host you name, so it's an advanced escape hatch rather than a validated safety net. Thanks also to both you and @andrest50 for independently verifying the flag-position behavior — matches what I found.

[Copilot]

Pull request overview

Copilot reviewed 9 out of 10 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

Copilot reviewed 9 out of 10 changed files in this pull request and generated no new comments.

Suppressed comments (2)

internal/commands/api.go:337

• url.JoinPath treats the second argument entirely as path data, so the documented /my-grants?v=1&object_type=projects path becomes /my-grants%3Fv=1&object_type=projects; the request does not send those query parameters. Parse the argument, join only its escaped path, and then restore its query components (with a query-bearing test).

	return url.JoinPath(base, path)

internal/commands/api.go:247

• An explicit empty --input= is counted as a body by apiHasExplicitBody (and promotes the request to POST), but these input != "" checks then treat it as omitted. This can read piped stdin or permit combination with fields instead of rejecting the invalid filename; use IsSet consistently and reject an empty value.

	if input != "" && (len(fields) > 0 || len(rawFields) > 0) {
		return nil, "", fmt.Errorf("--%s cannot be combined with --%s or --%s", apiInputFlagName, apiFieldFlagName, apiRawFieldFlagName)
	}

	if input != "" {

[emsearcy]

AI-assisted: Update to my earlier reply — went further than a usage-text note. --hostname now requires a development-environment login (lfx auth login --env=development); it's rejected outright for prod/staging logins, regardless of scheme or loopback. See bfb23c5.

[Copilot]

Pull request overview

Copilot reviewed 9 out of 10 changed files in this pull request and generated no new comments.

Suppressed comments (3)

Previously missed (1) — in code that hasn't changed since the last review.

internal/commands/api.go:362

• URL schemes are case-insensitive, but this exact comparison rejects valid secure URLs such as HTTPS://api.example.com. Use a case-insensitive comparison so the HTTPS guard enforces the protocol rather than its spelling.

	if parsed.Scheme == "https" {

internal/commands/api.go:349

• url.JoinPath treats the entire second argument as a path element, so a documented call such as /my-grants?v=1&object_type=projects becomes /my-grants%3Fv=1&object_type=projects; the server receives no query parameters. Parse the supplied argument first, join only its path component, then copy its RawQuery/ForceQuery onto the joined URL, and add the README example as a test case.

	return url.JoinPath(base, path)

internal/commands/api.go:251

• An explicitly empty --input= is silently treated as though the flag were absent, even though apiHasExplicitBody sees it and promotes the request to POST. In scripts, an unset filename variable can therefore send an empty body or consume inherited piped stdin unexpectedly. Track cmd.IsSet("input"), reject an empty value, and use that same boolean for mutual exclusion and input handling.

	input := cmd.String(apiInputFlagName)

[Copilot]

Pull request overview

Copilot reviewed 9 out of 10 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

Copilot reviewed 9 out of 10 changed files in this pull request and generated no new comments.

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

internal/commands/api.go:175

• The fallback cannot distinguish an absent Content-Type from an explicit empty override. -H 'Content-Type:' leaves Header.Get empty, so this branch immediately reinstates application/json, despite the header-handling contract above saying that this value can override the default. Check whether the header key exists rather than checking its value.

	if len(body) > 0 && req.Header.Get("Content-Type") == "" && (method == http.MethodPost || method == http.MethodPut) {

internal/commands/api.go:136

• audience is loaded from the mutable plaintext state.json and loadDeviceStateForBackend validates only its environment/IdP metadata, not this value. Changing just Audience to an attacker-controlled HTTPS URL now causes a valid cached or refreshed bearer token to be sent there; before this PR, credstore.DeviceState.Audience was documented as display-only. Bind the request destination to trusted credential/token data (for example, persist the audience with the protected credentials and compare it, or validate the token's aud claim) before using the state value as a URL.

	if baseURL == "" {
		baseURL = audience

[Copilot]

Pull request overview

Copilot reviewed 10 out of 11 changed files in this pull request and generated no new comments.

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

internal/commands/api.go:270

• Reading stdin fully before constructing the request makes memory usage proportional to the upload size; the file branch below has the same issue via os.ReadFile. A large --input upload can therefore exhaust memory before any bytes are sent. Return a streaming reader (and close files after the request) for raw input, while retaining buffered JSON only for field-generated bodies.

		if input == "-" {
			data, err := io.ReadAll(os.Stdin)

internal/commands/api.go:380

• The loopback exemption ignores the scheme and compares the DNS hostname case-sensitively. This accepts unsupported non-HTTP URLs such as ftp://localhost, while rejecting the valid loopback URL http://LOCALHOST. Require an HTTP scheme for the exemption and normalize the hostname before matching it.

	switch parsed.Hostname() {
	case "localhost", "127.0.0.1", "::1":
		return nil

[dealako]

Hi @emsearcy 👋 — follow-up review of the six commits since my last round (2183d19..3cb2cbd). You closed out every item, and a couple of the fixes went deeper than what was asked. Ran the three-subagent scan again scoped to just the new diff.

👏 Nice work this round:

• --hostname token exposure — genuinely fixed, not papered over. Gating it to development-environment logins keeps prod/staging tokens (the ones with real authority) off arbitrary hosts entirely. I traced the env value: it comes from resolveAccessToken only after loadDeviceStateForBackend runs it through resolveEnvironment and confirms the resolved IdP domain matches the persisted state.IDPDomain, so a tampered/empty environment fails the call before the gate. No bypass. The issuer-validation note (iss checked, not just aud, each env a distinct issuer) is a nice defense-in-depth argument on top.
• url.JoinPath tried, then correctly reverted. You took the suggestion, found it silently dropped query strings and decoded %2F slashes (two real correctness regressions), and reverted to verbatim concatenation with tests locking in query/fragment/encoded-slash preservation. That's the right call — verbatim path passthrough is what this command wants.
• Scheme check hardened with strings.EqualFold; empty --input= now rejected explicitly and folded into the mutual-exclusion/IsSet logic; url→endpoint rename; and the -H Set semantics documented as a deliberate override-friendly choice.

Revision tracking (prior round):

• ✅ --hostname sends token to any HTTPS host — resolved (dev-env gate, bfb23c5/3cb2cbd)
• ✅ url shadows net/url import — resolved (renamed to endpoint, 3833f79)
• ✅ Repeated -H overwrites — resolved as documented intentional behavior (3833f79); the override-the-auto-set-header rationale is sound
• ⚠️ runAPI has no direct httptest.Server coverage — still open, deferred to lfx api: no test coverage for runAPI's request/response handling #7 (unchanged this round; not a merge blocker)

Issue count (all open items):

• 🔴 Blocking: 0
• 🟡 Minor: 1 — runAPI end-to-end test coverage, tracked in lfx api: no test coverage for runAPI's request/response handling #7 (redirect guard, non-2xx exit, -H parsing, --query, and now the new --hostname env gate all still only covered indirectly)
• ⚪ Nit: 0

Reconciliation: the two new Copilot comments this cycle (query-string loss in the url.JoinPath attempt; the env-gate not keeping prod-audience tokens away) are both already addressed in your follow-up commits — agree with the resolutions. No new bot findings outstanding.

Zero new findings from my scan. Build and tests green, branch mergeable. The one remaining minor is tracked and accepted as out of scope for this PR.

✅ Approved

[dealako]

Follow-up round: all four prior items addressed. The --hostname token-exposure concern is resolved by the development-environment gate (verified the env value is trustworthy end-to-end via loadDeviceStateForBackend), the url→endpoint rename lands, the repeated--H Set behavior is documented as intentional, and the url.JoinPath experiment was correctly reverted after it introduced query-string-loss and encoded-slash regressions. Only remaining item is runAPI end-to-end test coverage, tracked in #7 and out of scope here. No new findings; build and tests green; branch mergeable.

✅ Approved