Skip to content

feat(auth): add headless jwt-bearer client (actual mint-token) - #803

Open
benw5483 wants to merge 2 commits into
mainfrom
feat/mint-token-jwt-bearer
Open

feat(auth): add headless jwt-bearer client (actual mint-token)#803
benw5483 wants to merge 2 commits into
mainfrom
feat/mint-token-jwt-bearer

Conversation

@benw5483

@benw5483 benw5483 commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds actual mint-token, a fully-headless RFC 7523 jwt-bearer client: it signs a short-lived service-account assertion with a registered private key and exchanges it for an access token — no browser, no human, no stored long-lived secret. This is the unattended-agent path that the existing enrollment commands (login, and auth create-token / login --device in #801 / #802) do not cover: those establish a human-delegated session, whereas an autonomous agent on a dev box or in CI holds its own key and self-issues an assertion.

The command mirrors the authorization server's jwt-bearer grant exactly:

  • Assertion header{ alg: RS256 | ES256, kid, typ: JWT }. HS* and none cannot be represented, let alone emitted — closing alg-confusion on the client the same way the server closes it.
  • Assertion claimsiss == sub == <service-account-id> (validated as a UUID before signing), aud defaulting to the issuer origin, a fresh unique jti per call, iat = now, and exp clamped to at most 300s after iat.
  • RequestPOST <issuer>/api/oauth/token with grant_type=urn:ietf:params:oauth:grant-type:jwt-bearer, assertion=<JWS>, and an optional space-delimited scope.

Headless usage

Every input comes from a flag, an environment variable, or a file:

# Key inline via env (preferred with a secret manager — keeps it off argv):
export ACTUAL_SERVICE_ACCOUNT_KEY="$(cat service-account.pem)"
TOKEN=$(actual mint-token \
  --service-account-id 3f8a1c2e-4b5d-4e6f-8a9b-0c1d2e3f4a5b \
  --kid my-registered-key \
  --scope adr:query --scope adr:review)
# $TOKEN is exactly the access token — nothing else on stdout.

--key <PATH> (or ACTUAL_SERVICE_ACCOUNT_KEY_FILE) reads the key from a file instead. The algorithm is inferred from the key (RSA → RS256, EC P-256 → ES256) unless --alg is given. --json swaps stdout for the full token response.

Output contract: the raw access token is the only thing on stdout, so TOKEN=$(actual mint-token …) captures exactly the token; all status goes to stderr. This matches the capture contract used by the other auth commands.

Scope decisions

A few calls the spec left open, surfaced here for review:

  1. New top-level mint-token command, not an auth subcommand. feat(auth): add actual auth create-token for non-interactive auth #801 and feat(auth): add actual login --device for browserless sign-in #802 are still open and both restructure the auth surface; adding a sibling auth subcommand now would collide with them. A standalone command keeps this change independent. It can fold under auth once those land and that surface settles.
  2. Token goes to stdout; persisting it is deferred. The load-bearing deliverable is the headless mint with the stdout-capture contract — an agent captures the token and uses it directly. Persisting into the existing on-disk credential store is deferred: that store models a browser-login user session (it requires a member id), which a service-account grant has no natural value for. A follow-up can add an opt-in service-account store if wanted.
  3. RS256 and ES256 both supported — the server accepts both, so the client does too, inferring from the key type.
  4. Key generation + registration is out of scope (assumes an already-registered key), as this change is scoped to the sign→mint core.

Test plan

Unit tests (src/auth/jwt_bearer.rs, src/cli/commands/mint_token.rs) and end-to-end binary tests (tests/mint_token_cli.rs):

  • A signed assertion verifies under the server's exact validation for both RS256 and ES256 (ephemeral keypair generated at test time; verified with the pinned algorithm, required audience, and exp), and carries iss == sub == UUID, an accepted aud, exp - iat <= 300, a header alg ∈ {RS256, ES256} + kid, and a fresh unique jti per call.
  • Forbidden algorithms (HS256, none, RS512, …) are refused; a non-UUID principal and a wrong-key-for-alg fail cleanly (no panic, no key material in the message).
  • The mint request carries the right wire shape (grant_type, assertion, scope); a server invalid_grant surfaces cleanly with no stack trace or secret leakage.
  • End-to-end against a mock token endpoint: the real binary signs, mints, and prints only the token to stdout (status on stderr); --json stays machine-parseable; a non-HTTPS non-loopback issuer is refused before anything is sent; a non-UUID principal exits non-zero with an empty stdout.
  • cargo fmt --check, cargo clippy -- -D warnings, cargo test, cargo build --release all green.

A live end-to-end mint against the running server is not included: the server-side grant is not yet reachable from this repo's test environment (it needs a full app + database stack and a pre-registered key). The client is instead proven to produce a spec-compliant, server-verifiable assertion (the unit tests verify the signature under the server's exact validation) and to capture the token correctly.

Security notes

  • Transport — reuses the existing HTTPS-only guard; a non-HTTPS, non-loopback issuer is rejected before any assertion leaves the process.
  • No secret logging — the minted token's Debug impl redacts the token; nothing logs key or token material; the private key is read from a file or env, never a CLI arg.
  • One-shot assertions — a fresh jti per call and a short (default 60s, ≤300s) lifetime bound the replay/leak window; the server anti-replays on the jti.
  • No symmetric/unsigned algorithms — the client can only emit RS256 or ES256.

Test plan for a reviewer

  • cargo test (unit + tests/mint_token_cli.rs)
  • cargo clippy -- -D warnings and cargo fmt --check
  • actual mint-token --help reads clearly for a headless caller

Generated by the operator's software factory.
• On behalf of: @benw5483

@benw5483
benw5483 force-pushed the feat/mint-token-jwt-bearer branch from 27951c2 to 21f8500 Compare July 6, 2026 14:24
@benw5483
benw5483 marked this pull request as ready for review July 6, 2026 17:27
@davidmiuraactualai

Copy link
Copy Markdown

Actual Adversarial Review

Key findings


🛑 APR-001 P1 Parse the issuer before allowing plaintext loopback transport

Location: src/cli/commands/mint_token.rs:68 at 21f8500712b06c641ee591d0daed98730e7ae193
Risk: A signed JWT bearer assertion can be sent over plaintext to an attacker-controlled hostname and replayed for an access token during its validity window.

Evidence and required correction

Issue: The new command signs its assertion before calling the shared transport guard. That guard decides whether HTTP is loopback with string prefixes in src/auth/oauth.rs:146, so http://localhost.evil.invalid, http://localhost@evil.invalid, and http://127.0.0.1.evil.invalid are all treated as local. The command can also set --aud to the real authorization-server identity, so an endpoint controlled by that host receives an assertion the authorization server can accept.

Evidence:

  • Code: the new command invokes oauth::build_http_client(&base_url) only after build_and_sign_assertion; the predicate uses starts_with rather than parsed URL scheme and host identity.
  • ADR: docs/adr/8d11aed5-d4a3-4ef6-937a-085f723bd997-adopt-secure-secrets-management-in-ci-cd-pipeline.md requires secret-handling paths to prevent credential exposure.
  • Public: RFC 7523 section 2.1 defines the signed assertion as the JWT-bearer authorization grant submitted to the token endpoint; section 3 requires the authorization server to accept a valid assertion whose audience identifies that server within its expiration window.
  • Verification: ACTUAL_SERVICE_ACCOUNT_KEY=<ephemeral P-256 PEM> cargo run --quiet -- mint-token ... --issuer http://localhost.evil.invalid --aud https://app.actual.ai signed the assertion and reached Token mint request failed: error sending request, rather than returning the expected HTTPS configuration error. cargo test auth::oauth::tests::test_build_http_client --lib -- --nocapture passed, but the current test only checks http://example.com and does not exercise prefix lookalikes.

Required correction: Parse the issuer as a URL before signing or dispatching. Permit plaintext only when the parsed scheme is HTTP and the parsed host is exactly an intended loopback host or loopback IP address; require HTTPS otherwise. Reject malformed URLs, user-info host confusion, and lookalike hostnames before an assertion can leave the process. Add coverage for the rejected lookalikes and the allowed loopback forms.

🤖 Corrective action — 🛑 APR-001 P1 — Option A: parsed loopback allowlist

Tradeoff: This preserves HTTP support for local development while making the exception depend on URL semantics rather than a textual prefix.

Fix APR-001 P1, Parse the issuer before allowing plaintext loopback transport, in actual-software/actual-cli at src/auth/oauth.rs:145 and src/cli/commands/mint_token.rs:68 on 21f8500712b06c641ee591d0daed98730e7ae193.

Problem: actual mint-token signs a JWT bearer assertion, then the shared HTTP guard accepts http://localhost.evil.invalid and related prefix lookalikes because it uses starts_with. With --aud set to the real authorization-server identity, an attacker controlling that hostname receives a bearer assertion that can be exchanged during its validity window.
Required invariant: no signed assertion or OAuth credential is sent over plaintext HTTP unless the parsed issuer host is an exact intended loopback host or loopback IP address.
Constraints: Follow AGENTS.md and src/AGENTS.md, including the secure-secrets ADR and comprehensive unit-testing requirement; honor .gitignore; preserve valid HTTPS issuers and supported local HTTP test endpoints.
Implement: parse and validate the issuer URL in the shared transport guard. Require HTTPS except for HTTP with an exact loopback hostname or loopback IP. Reject prefix lookalikes, user-info host confusion, malformed URLs, and non-loopback hosts before creating a client that can send the assertion.
Verify: Add unit coverage that rejects http://localhost.evil.invalid, http://localhost@evil.invalid, http://127.0.0.1.evil.invalid, and an IPv6 lookalike, while allowing http://localhost:<port>, http://127.0.0.1:<port>, and http://[::1]:<port>. Add a mint-token regression that fails with the HTTPS configuration error before any request. Run cargo test --workspace --features integration, cargo clippy -- -D warnings, and cargo fmt --check.
Do not: weaken HTTPS enforcement, send a real assertion to a non-loopback HTTP endpoint, log assertion or key material, or modify unrelated command behavior.

Copy into a coding agent to run.

🧭 ADR option — APR-001: Service-account assertion transport boundary · Policy: JWT-bearer assertions and OAuth credentials may use HTTP only after parsed URL validation establishes an exact loopback destination. — Create · Update

Architecture intent

🧭 Suggested ADR: Service-account assertion transport boundary — Create · Update

  • APR-001 Policy: JWT-bearer assertions and OAuth credentials may use HTTP only after parsed URL validation establishes an exact loopback destination.

Review metadata

Important

One finding remains unresolved at head 21f8500712b06c641ee591d0daed98730e7ae193. This is the canonical remediation record for the PR.

Compared: upstream main at cfcae4cd84645417e7d8042517a69cbd55d22404 … PR head 21f8500712b06c641ee591d0daed98730e7ae193
CI: Build, Test, Lint, Coverage Enforcement, and CodeQL succeeded for this head; Live E2E was skipped. Local targeted guard tests passed, while the manual prefix-lookalike invocation reproduced the guard bypass.

@benw5483
benw5483 requested a review from wattswolf July 27, 2026 15:21
austinborn
austinborn previously approved these changes Jul 27, 2026

@austinborn austinborn left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the full diff at head 21f85007, focused on assertion forgery, replay, and audience handling.

No blocking findings. Approving.

What holds up

Algorithm confusion is closed at the type level, which is stronger than the usual runtime check. AssertionAlgorithm has exactly two variants, so HS256 and none aren't merely rejected at parse time, they're unrepresentable. No code path can emit a symmetric or unsigned alg.

Replay and lifetime look right. Every call mints a fresh jti from Uuid::new_v4(), exp - iat is clamped to the server's 300-second ceiling with a 60-second default well under it, and iss == sub == service_account_id matches the server's contract.

is_uuid initially looked like a reimplementation of something the already-present uuid crate does. It isn't. Uuid::parse_str accepts any version and variant, while this enforces version 1-5 and variant 8/9/a/b specifically to mirror the server's isUuid, so the client accepts a principal id exactly when the server would. The doc comment says as much. Worth flagging anyway, so nobody "simplifies" it later.

Secret handling matches the module's standard: MintedToken hand-writes a redacting Debug, mint_error surfaces only OAuth error JSON, key-load failures never echo PEM content, and write_token_output keeps stdout to the token alone with all status on stderr.

Non-blocking

1. An explicit --aud combined with a redirected issuer would sign a correctly-audienced assertion and POST it to the wrong host. resolve_audience defaults aud to the resolved issuer, and that default is the safe case: point --issuer at an attacker and the assertion carries aud: https://attacker, which the real server rejects, so what leaks is useless. Passing --aud https://app.actual.ai while ACTUAL_AUTH_URL points somewhere else breaks that coupling. The attacker then holds a valid, real-audience assertion and roughly 60 seconds to replay it for a token carrying the principal's full scope whitelist.

I don't think this is blocking, and I'd rather be explicit about why than leave it implied. It needs a second misconfiguration stacked on the redirect, and anyone who can set ACTUAL_AUTH_URL in an agent's environment can usually read ACTUAL_SERVICE_ACCOUNT_KEY out of that same environment, so the marginal escalation is small. Closing it is cheap, though: warn, or refuse, when an explicit aud is neither the resolved issuer nor <issuer>/api/oauth/token. That's defense in depth for the unattended path, which is where a poisoned env var is most plausible in the first place.

2. resolve_lifetime's comment describes a sentinel that clap never produces. It treats 0 as "the unset sentinel from the default", but assertion_ttl_seconds is default_value_t = 60, so 0 only shows up when someone passes --assertion-ttl-seconds 0 explicitly. That input becomes 60 rather than clamping to 1. Defensible, but it isn't what the comment describes.

3. The key file is read with no permission check. load_key_pem reads whatever --key points at. Given that token_store goes out of its way to write 0600, a warning when a private key is group- or world-readable would round out the story.


Posted by the operator's software factory.
• City: factory-main · Agent: local-core.builder-1
• On behalf of: @austinborn

@wattswolf wattswolf left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Solid, well-tested jwt-bearer client, but the loopback transport check lets a signed assertion go out over plaintext to an attacker-controlled host. Fix the guard before merge.

  • The HTTPS bypass keys off base_url.starts_with("http://localhost") / "http://127.0.0.1", so http://localhost.evil.invalid, http://localhost@evil.invalid, and http://127.0.0.1.evil.invalid all read as loopback and get plaintext. With --aud pointed at the real server, the minted assertion is a replayable bearer sent in the clear — src/auth/oauth.rs:146. Parse the URL and match the exact host (localhost, 127.0.0.1, ::1), not a prefix.
  • Same guard is reused by mint_token via build_http_client, so this new signed-assertion path inherits the weakness with no local override — src/cli/commands/mint_token.rs:68.

@wattswolf wattswolf left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The client implementation looks strong, but I’d hold approval until the integration evidence is complete.

The JWT handling is well designed: RS256/ES256-only signing, bounded claims and lifetime, fresh jti values, redacted debugging, token-only stdout, and substantial unit/E2E coverage. I also checked the #804-then-#803 merge order, and the hardened transport guard is preserved without conflict.

The remaining gaps are integration-related:

  • The current head, 7debf81, predates #804, so there isn’t yet a real post-#804 SHA with green CI.

  • The matching server grant in sprintreview#3267 is still open and conflicting on top of prerequisite #3259.

  • Because the server route has not landed, the complete client/server mint flow could not be verified live.

  • Before approval, I’d like to see #804 land, this PR refreshed onto the resulting main, CI green on the new head, and the #3259/#3267 server rollout confirmed.

One non-blocking hardening suggestion: consider preventing an explicit --aud from differing from --issuer, which would reduce the chance of a valid short-lived assertion being sent to the wrong HTTPS endpoint.

benw5483 and others added 2 commits July 30, 2026 11:08
Add `actual mint-token`, a fully-headless RFC 7523 jwt-bearer client: it
signs a short-lived service-account assertion with a registered private key
(RS256 or ES256) and exchanges it for an access token at the OAuth token
endpoint — no browser, no human, no stored long-lived secret. This is the
unattended-agent path the existing enrollment commands do not cover.

- New `src/auth/jwt_bearer.rs`: build and sign the assertion, mint the token,
  and the stdout-only-token output contract.
- New `src/cli/commands/mint_token.rs`: the headless command handler.
- Reuse the existing HTTPS transport guard in `src/auth/oauth.rs`.
- Only RS256/ES256 can be emitted; HS*/none are refused. The minted token is
  redacted in Debug and printed only to stdout; status goes to stderr.

Verified with unit and end-to-end tests: a signed assertion verifies under the
server's exact validation for both algorithms, forbidden algorithms and
malformed inputs fail cleanly, and the real binary mints against a mock token
endpoint printing only the token to stdout.

Generated by the operator's software factory.
On behalf of: @benw5483
Co-Authored-By: Actual Factory Bot <factory-bot@actual.ai.invalid>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The Coverage Enforcement gate was red on three files, not the single
lib.rs line the first review flagged: lib.rs (the MintToken dispatch
arm), auth/jwt_bearer.rs (15 lines), and cli/commands/mint_token.rs
(55 lines, essentially the whole exec() body).

Root cause: tests/mint_token_cli.rs — the end-to-end client tests that
drive exec() and the mint round-trip — was never added to the coverage
workflow's --test list, so it never ran under instrumentation. The
binary exits via process::exit, which flushes the llvm-cov profile, so
these subprocess tests do contribute coverage once they run. Register
the test file in coverage.yml, as that file's own reminder requires.

That covers exec()'s orchestration. The remaining gaps are branches on
their own lines that no end-to-end test reaches, so cover them the way
the surrounding code already does — with small, unit-testable seams:

- Extract resolve_lifetime() next to resolve_scope/resolve_audience/
  resolve_algorithm, and unit-test the ttl==0 default path.
- Split unix_seconds(SystemTime) out of now_unix(), so the
  clock-before-epoch error path is testable with an injected time
  (the same pattern build_and_sign_assertion already uses for now).

Add unit tests for the jwt_bearer.rs error branches that had no
coverage: non-UTF-8 and PKCS#1/SEC1/PKCS#8 key-header inference, the
is_uuid dash-position and non-hex-digit rejections, the empty-audience
guard, and the mint_error fallback for a non-OAuth error body. Add an
in-process run() dispatch test for the MintToken arm in lib.rs,
matching the sibling per-command dispatch tests.

No production behavior changes; the two extractions are pure
refactors and the 100% coverage gate is left intact.

Generated by the operator's software factory.
On behalf of: @benw5483
Co-Authored-By: <operator-factory-bot> <factory-bot@actual.ai.invalid>
@benw5483
benw5483 force-pushed the feat/mint-token-jwt-bearer branch from 7debf81 to 616a6c6 Compare July 30, 2026 15:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants