Skip to content

Authenticate headless runners with a machine credential - #495

Merged
realtonyyoung merged 4 commits into
mainfrom
claude-tyoung/ai990-machine-auth
Aug 8, 2026
Merged

Authenticate headless runners with a machine credential#495
realtonyyoung merged 4 commits into
mainfrom
claude-tyoung/ai990-machine-auth

Conversation

@realtonyyoung

Copy link
Copy Markdown
Collaborator

Supplies the half of machine credentials that was never built: a runner obtaining a token from one.

The gap

kcap machine create could provision a credential and register it, and the server would accept a machine's token. But nothing could get one. Verified by grep before writing a line:

  • No client_credentials grant anywhere in the CLI — zero hits.
  • No call to the token endpoint — zero hits.
  • No token-exchange endpoint on the tenant server or the auth proxy.
  • Nothing read KCAP_CLIENT_ID / KCAP_CLIENT_SECRET. Every occurrence was README.md, help-machine.txt, two Console.Error.WriteLineAsync lines, and a test asserting the help mentions them.

help-machine.txt step 3 says "On the runner, both variables must be in the environment" — promising behaviour that did not exist. The tests asserted the help mentions the variables, which is the same defect class as the dead-on-arrival kcap machine found the same day: testing the artifact I was asked to get right instead of the path that has to execute.

What this adds

MachineAuth — reads the two variables, resolves the token endpoint.

MachineTokenProvider — exchanges them via client_credentials, caches the bearer in memory only, single-flighted behind a semaphore so a burst of callers (hooks, watcher, MCP servers) mints once.

Never the token store: that would cost the property the design rests on — a machine's bearer existing only for the life of the process that needs it — and buy nothing, since client_credentials returns no refresh token. "Refresh" here means "mint another", which needs only the credential the runner already has.

One wiring pointCreateClientCoreAsync, the single place every authenticated CLI call resolves a token:

  • After the None check (a server needing no auth needs no credential).
  • Before the token-store paths, because on a runner those find nothing and advise kcap login, which a runner cannot do.
  • Gated on either variable being present, not both, so a half-configured runner is told which one is missing rather than getting that same wrong advice.
  • No UnauthorizedRetryHandler — it refreshes through TokenStore, and there is no refresh token. A 401 arrives as rejectedAccessToken; re-minting is the repair.

A runner needs no profile: that path already honours KCAP_URL, and /auth/config discovery is unauthenticated.

Two decisions worth scrutinising

The token endpoint is hardcoded (https://signin.kcap.ai/oauth2/token) with a KCAP_WORKOS_TOKEN_URL override — exactly as AuthProxyEndpoint hardcodes the proxy with KCAP_AUTH_PROXY_URL. It is one value for the whole fleet (a single WorkOS environment and application), and it cannot be derived from the tenant's /auth/config, because the field that would carry it — authkit_domain — is blank on every tenant, so deriving it would yield a broken URL everywhere. Verified live: that host answers this grant with an OAuth2 credential rejection, while api.workos.com/oauth2/token 404s.

A rejection never echoes the response body. A token endpoint's error body is attacker-influenced and can reflect the request — which contains the secret. The status is the diagnostic; the body isn't worth the risk. There's a test for that, using a stub that reflects the credential back.

Tests

11 tests, going through the real HTTP exchange against WireMock rather than asserting shapes around it:

  • Minting posts the correct form and returns the token — the wire format is asserted, since that's what would be silently wrong.
  • A cached token is reused with no second request.
  • A rejected token forces a fresh mint.
  • A rejected credential reports a problem that does not contain the secret.
  • Success with no access_token is a failure, not an empty bearer.
  • Credential reading: both present; each half missing, naming the missing one; neither.
  • End-to-end: a constructed client actually carries the minted bearer with no profile present. This is the test whose absence let the previous slice ship non-functional.
  • A half-configured runner reports NotAuthenticated rather than a silently unauthenticated client.

Mutation-verified: disabling the branch (Intended => false) fails 4 tests including the end-to-end one, proving the branch is reached rather than merely present.

MachineTokenResponse is registered in CapacitorJsonContext and read through it explicitly, so the AOT build has no reflection path.

Not done yet, deliberately

The live end-to-end on a real tenant — create a machine, record a session as it, confirm ownership/attribution/visibility, confirm no seat consumed, revoke, confirm the coded refusal. That runs from a dev build before this is tagged. I am not calling this feature done until it has, because a green build and a clean review have already hidden a dead version of it once.

Part of AI-990.

A machine credential could be provisioned and registered, and the server would
accept a machine's token — but nothing could OBTAIN one. There was no
client_credentials grant anywhere in the CLI, no call to the token endpoint, and
nothing read KCAP_CLIENT_ID/KCAP_CLIENT_SECRET: every occurrence was README, help
text, two Console.Error lines, or a test asserting the help mentioned them. The
help even promised "on the runner, both variables must be in the environment".
This is that missing half.

MachineAuth reads the two variables and resolves the token endpoint. MachineToken-
Provider exchanges them for a bearer via client_credentials and caches it IN
MEMORY ONLY — never the token store, which would cost the property the design
rests on (a machine's bearer exists only for the life of the process that needs
it) and buy nothing, since client_credentials returns no refresh token and
"refresh" here means "mint another" from a credential the runner already has.

One wiring point: CreateClientCoreAsync, the single place every authenticated CLI
call resolves a token. Placed after the None check and before the token-store
paths, because on a runner those find nothing and advise `kcap login` — which a
runner cannot do. No UnauthorizedRetryHandler, since it refreshes through
TokenStore; a 401 arrives as rejectedAccessToken and re-minting is the repair.

Gated on either variable being present, not both, so a half-configured runner is
told which one is missing instead of getting that same wrong advice.

The token endpoint is hardcoded with a KCAP_WORKOS_TOKEN_URL override, exactly as
AuthProxyEndpoint hardcodes the proxy: it is one value for the whole fleet, and it
CANNOT be derived from the tenant's /auth/config because the field that would
carry it, authkit_domain, is blank on every tenant. Verified live: that host
answers this grant with an OAuth2 credential rejection; api.workos.com/oauth2/token
404s.

A rejection never echoes the response body. A token endpoint's error body is
attacker-influenced and can reflect the request, which contains the secret.

Tests go through the real HTTP exchange against WireMock, not shapes around it —
including the wire format, which is what would be silently wrong. The end-to-end
test asserts a constructed client actually carries the minted bearer with no
profile present; its absence is why the previous slice of this feature shipped
completely non-functional with a clean build and a clean review. Mutation-verified:
disabling the branch fails 4 tests including that one. 11/11.

Part of AI-990.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@linear-code

linear-code Bot commented Aug 8, 2026

Copy link
Copy Markdown

AI-990

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Authenticate headless runners using machine credentials

✨ Enhancement 🧪 Tests 🕐 40+ Minutes

Grey Divider

AI Description

• Add machine-credential auth via KCAP_CLIENT_ID/KCAP_CLIENT_SECRET using OAuth client_credentials.
• Wire machine auth into CreateClientCoreAsync before token-store login/refresh paths.
• Add in-memory token caching with single-flight minting and WireMock end-to-end tests.
Diagram

graph TD
  cli["CLI / headless runner"] --> core["HttpClientExtensions.CreateClientCoreAsync"] --> disc["GET /auth/config"] --> gate{"Machine env vars?"}
  gate -- "Yes" --> mint["MachineTokenProvider (client_credentials)"] --> workos{{"WorkOS token endpoint"}} --> client["HttpClient w/ Bearer"]
  gate -- "No" --> store[("TokenStore (disk)")] --> client

  subgraph Legend
    direction LR
    _d{"Decision"} ~~~ _db[("Disk store")] ~~~ _ext{{"External"}}
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Derive token URL from tenant discovery (/auth/config)
  • ➕ Avoids hardcoding the WorkOS domain and reduces need for a special override env var
  • ➕ Allows multi-environment / multi-region setups without rebuilding the CLI
  • ➖ Current tenants reportedly return blank authkit_domain, so derivation may be broken today
  • ➖ Requires server-side contract changes and rollout coordination
2. Exchange machine credentials through the existing auth proxy
  • ➕ Keeps token-minting behind a single controlled endpoint (consistent with other auth flows)
  • ➕ Could centralize provider-specific behavior and simplify future provider changes
  • ➖ Adds an extra hop and couples headless auth availability to proxy uptime/configuration
  • ➖ May require new proxy endpoints and operational changes
3. Persist machine tokens in the existing TokenStore (cross-process cache)
  • ➕ Reduces token endpoint calls across many short-lived CLI processes
  • ➕ Makes behavior more consistent with interactive auth paths
  • ➖ Weakens the stated security/property goal (token only lives for process lifetime)
  • ➖ client_credentials has no refresh token; persistence mainly adds secret-at-rest risk without strong benefit

Recommendation: The PR’s approach (direct client_credentials mint with in-memory-only caching, integrated at CreateClientCoreAsync) is a good fit for headless runners: it avoids profile/token-store assumptions, gives actionable errors for partial env config, and keeps secrets off disk. The main decision to revisit later is token endpoint hardcoding; if/when /auth/config reliably exposes an authkit domain, switching to derived discovery would reduce fleet-wide coupling while retaining the current override mechanism for tests.

Files changed (5) +518 / -1

Enhancement (4) +261 / -1
MachineAuth.csAdd env-based machine credential reader and token URL resolver +71/-0

Add env-based machine credential reader and token URL resolver

• Introduces MachineCredential and MachineAuth helpers to read KCAP_CLIENT_ID/KCAP_CLIENT_SECRET from the environment and validate partial configuration. Defines a default WorkOS token endpoint with an internal KCAP_WORKOS_TOKEN_URL override and an 'Intended' gate for runner detection.

src/Capacitor.Cli.Core/Auth/MachineAuth.cs

MachineTokenProvider.csImplement client_credentials token minting with in-memory single-flight cache +138/-0

Implement client_credentials token minting with in-memory single-flight cache

• Adds a WorkOS client_credentials exchange that posts form-encoded client_id/client_secret and parses access_token/expires_in. Caches the bearer in static memory with an expiry margin, supports forced re-mint when a rejected token is reported, and avoids leaking secrets in error output.

src/Capacitor.Cli.Core/Auth/MachineTokenProvider.cs

HttpClientExtensions.csWire machine auth into authenticated client construction and messaging +51/-1

Wire machine auth into authenticated client construction and messaging

• Extends CreateClientCoreAsync to prefer machine-credential auth (when intended) after provider discovery and before TokenStore resolution. Adds MachineAuthProblem state to surface actionable machine-auth failures and updates stderr messaging to avoid advising 'kcap login' on headless runners.

src/Capacitor.Cli.Core/HttpClientExtensions.cs

Models.csRegister MachineTokenResponse for source-generated JSON serialization +1/-0

Register MachineTokenResponse for source-generated JSON serialization

• Adds MachineTokenResponse to CapacitorJsonContext serialization metadata so MachineTokenProvider can parse token responses via ReadFromJsonAsync with the generated context.

src/Capacitor.Cli.Core/Models.cs

Tests (1) +257 / -0
MachineAuthTests.csAdd end-to-end unit tests for machine auth exchange and wiring +257/-0

Add end-to-end unit tests for machine auth exchange and wiring

• Introduces WireMock-backed tests that exercise real HTTP POST wire format to the token endpoint, in-memory caching behavior, secret-safe error reporting, and the CreateClientCoreAsync wiring that attaches the Bearer header without relying on profiles/token store. Uses NotInParallel and resets process-wide env vars and caches between tests.

test/Capacitor.Cli.Tests.Unit/MachineAuthTests.cs

@qodo-code-review

qodo-code-review Bot commented Aug 8, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Action required

1. No automatic 401 remint ✓ Resolved 🐞 Bug ≡ Correctness
Description
The machine-auth branch returns a client without any 401 retry mechanism, so a rejected machine
token won’t be re-minted unless the caller explicitly reconstructs the client with
rejectedAccessToken. Many existing call paths use CreateAuthenticatedClientAsync (no
rejectedAccessToken propagation), so a revoked/invalid cached machine token can cause repeated 401s
until expiry.
Code

src/Capacitor.Cli.Core/HttpClientExtensions.cs[R111-114]

+            var machineClient = NewClient();
+            machineClient.DefaultRequestHeaders.Authorization = new("Bearer", machineToken);
+
+            return (machineClient, AuthStatus.Ok, null);
Evidence
Machine-auth client creation attaches a bearer token but never installs a 401 retry handler, unlike
token-store auth. The re-mint pathway depends on callers supplying rejectedAccessToken, which most
command code paths don’t do.

src/Capacitor.Cli.Core/HttpClientExtensions.cs[57-142]
src/Capacitor.Cli.Core/Auth/UnauthorizedRetryHandler.cs[19-52]
src/Capacitor.Cli/Commands/ClaudeHookCommand.cs[35-41]
src/Capacitor.Cli/Commands/MachineCommand.cs[337-342]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
Machine auth can mint and cache a bearer token, but the returned HttpClient has no automatic 401 recovery. `MachineTokenProvider` supports re-minting via a `rejectedToken` signal, yet most commands create clients via `CreateAuthenticatedClientAsync()` and never feed a rejected token back into `CreateClientCoreAsync`, causing repeated 401s after server-side invalidation/revocation.

### Issue Context
- `CreateClientCoreAsync` installs `UnauthorizedRetryHandler` for token-store auth when `autoRetryUnauthorized` is enabled, but machine-auth always returns a plain client with a fixed Authorization header.
- Only a small subset of hook/memory paths ever call `CreateClientWithAuthStatusAsync(..., rejectedAccessToken: ...)`.

### Fix Focus Areas
- Implement a new `DelegatingHandler` analogous to `UnauthorizedRetryHandler`, but for machine auth. On 401:
 - retry at most once
 - call `MachineTokenProvider.GetTokenAsync(credential, rejectedToken: appliedToken, ct)` to force cache eviction + re-mint
 - update the request Authorization header and resend
- In `HttpClientExtensions.CreateClientCoreAsync`, when `MachineAuth.Intended` is true:
 - honor `autoRetryUnauthorized` by installing the new handler
 - avoid relying on callers to propagate `rejectedAccessToken`

- src/Capacitor.Cli.Core/HttpClientExtensions.cs[57-142]
- src/Capacitor.Cli.Core/Auth/UnauthorizedRetryHandler.cs[19-52]
- src/Capacitor.Cli.Core/Auth/MachineTokenProvider.cs[60-96]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

2. Unsanitized token URL errors ✓ Resolved 🐞 Bug ⛨ Security
Description
MachineTokenProvider interpolates MachineAuth.TokenUrl and exception messages directly into Problem
strings, which can leak URL userinfo (if KCAP_WORKOS_TOKEN_URL contains credentials) and allow
control-character/log injection into stderr.
Code

src/Capacitor.Cli.Core/Auth/MachineTokenProvider.cs[R120-123]

+                return (null, 0, $"the machine credential was rejected by {MachineAuth.TokenUrl} "
+                              + $"(HTTP {(int)response.StatusCode}). Check {MachineAuth.ClientIdVar}/"
+                              + $"{MachineAuth.ClientSecretVar}, or re-issue with 'kcap machine create'.");
+            }
Evidence
MachineTokenProvider builds error strings containing TokenUrl and raw exception messages, while
HttpClientExtensions explicitly sanitizes URLs and strips control characters for similar stderr
diagnostics.

src/Capacitor.Cli.Core/Auth/MachineTokenProvider.cs[116-133]
src/Capacitor.Cli.Core/HttpClientExtensions.cs[360-380]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
Machine-auth failure messages include `MachineAuth.TokenUrl` and `ex.Message` without sanitization. Elsewhere in the CLI, URL output is sanitized and control characters are stripped specifically to avoid leaking userinfo credentials and to prevent output injection.

### Issue Context
These strings are surfaced to stderr (e.g., via `HttpClientExtensions.CreateAuthenticatedClientAsync`) and can land in CI logs/harness transcripts.

### Fix Focus Areas
- Sanitize displayed URLs (e.g., strip userinfo, strip control chars; possibly display only scheme+host).
- Strip control characters from exception messages before storing/printing.
- Consider reusing `UnusableUrlDiagnostic.Sanitize` and `HttpClientExtensions.StripControlCharacters`-equivalent logic (or refactor into a shared helper) for consistent safety.

- src/Capacitor.Cli.Core/Auth/MachineTokenProvider.cs[116-133]
- src/Capacitor.Cli.Core/HttpClientExtensions.cs[360-380]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Token cache not credential-scoped ✓ Resolved 🐞 Bug ≡ Correctness
Description
MachineTokenProvider caches a single static token/expiry for the entire process without keying by
MachineCredential (or token endpoint), so changing env vars or using multiple machine credentials in
one process can attach the wrong bearer.
Code

src/Capacitor.Cli.Core/Auth/MachineTokenProvider.cs[R38-40]

+    static string?         cachedToken;
+    static DateTimeOffset  cachedExpiry;
+
Evidence
The provider stores a single process-wide cached token and returns it without checking the provided
credential; meanwhile the credential is re-read from environment (and can therefore change
independently of the cache).

src/Capacitor.Cli.Core/Auth/MachineTokenProvider.cs[36-92]
src/Capacitor.Cli.Core/Auth/MachineAuth.cs[50-66]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`MachineTokenProvider` maintains a single global `cachedToken`/`cachedExpiry` regardless of which `MachineCredential` (and which `MachineAuth.TokenUrl`) was used to mint it. If credentials rotate or multiple credentials are used in the same process, the provider can return a bearer minted for a different credential.

### Issue Context
`GetTokenAsync` accepts a `MachineCredential`, but the cache lookup does not verify that the cached token corresponds to that credential or the current token endpoint.

### Fix Focus Areas
- Scope the cache by `(tokenUrl, clientId)` at minimum; ideally `(tokenUrl, clientId, clientSecret hash)` or clear cache when any component differs.
- Alternatively, make `MachineTokenProvider` instance-based (non-static) and wire it as a per-process singleton with explicit keying.

- src/Capacitor.Cli.Core/Auth/MachineTokenProvider.cs[36-92]
- src/Capacitor.Cli.Core/Auth/MachineAuth.cs[50-66]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Informational

4. Global auth problem races ✓ Resolved 🐞 Bug ☼ Reliability
Description
MachineAuthProblem and MachineTokenProvider.Problem are process-wide mutable statics; concurrent
client constructions can overwrite them and cause unrelated commands to print the wrong machine-auth
failure reason.
Code

src/Capacitor.Cli.Core/HttpClientExtensions.cs[R185-190]

+    /// <summary>
+    /// Why machine auth failed on the last client construction, when it did. Lets the interactive
+    /// wrapper print an actionable message rather than advising `kcap login` on a runner that has no
+    /// browser and no profile.
+    /// </summary>
+    internal static string? MachineAuthProblem { get; private set; }
Evidence
The machine-auth branch sets a static MachineAuthProblem during client creation and
CreateAuthenticatedClientAsync later reads that static to choose messaging; MachineTokenProvider
similarly stores the last failure in a static Problem property.

src/Capacitor.Cli.Core/HttpClientExtensions.cs[92-115]
src/Capacitor.Cli.Core/HttpClientExtensions.cs[160-171]
src/Capacitor.Cli.Core/HttpClientExtensions.cs[185-191]
src/Capacitor.Cli.Core/Auth/MachineTokenProvider.cs[78-101]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
Authentication failure diagnostics are stored in shared static properties (`MachineAuthProblem`, `MachineTokenProvider.Problem`) and read later for messaging. In concurrent scenarios (daemon/watcher/MCP/multiple tasks), another client construction can overwrite these values before the original caller prints them.

### Issue Context
This is primarily a diagnostic integrity issue (misattributed error messages), not token correctness.

### Fix Focus Areas
- Return the problem string as part of the client construction result instead of using global static state (e.g., extend the tuple to include `string? authProblem`).
- Avoid static `MachineTokenProvider.Problem`; return failure text directly from `GetTokenAsync` (or as an out param/record result).

- src/Capacitor.Cli.Core/HttpClientExtensions.cs[92-115]
- src/Capacitor.Cli.Core/HttpClientExtensions.cs[160-171]
- src/Capacitor.Cli.Core/HttpClientExtensions.cs[185-191]
- src/Capacitor.Cli.Core/Auth/MachineTokenProvider.cs[78-101]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


To customize comments, go to the Qodo configuration screen, or learn more in the docs.

Qodo Logo

Comment thread src/Capacitor.Cli.Core/HttpClientExtensions.cs Outdated
Comment thread src/Capacitor.Cli.Core/Auth/MachineTokenProvider.cs Outdated
Comment thread src/Capacitor.Cli.Core/Auth/MachineTokenProvider.cs Outdated
Comment thread src/Capacitor.Cli.Core/HttpClientExtensions.cs Outdated
realtonyyoung and others added 3 commits August 8, 2026 01:10
Two medium findings, both real, fixed at the root rather than patched.

The failure reason was a process-wide mutable static (MachineAuthProblem, and
MachineTokenProvider.Problem). Two concurrent failures would race, so a caller
could report the other one's reason — and a success clearing the field could
erase a concurrent failure's. The daemon makes exactly these concurrent calls and
is a primary consumer of machine credentials, so it was reachable. Both statics
are gone: GetTokenAsync returns a MachineTokenResult, and CreateClientCoreAsync
threads the reason out through its return tuple. That also removes the
memory-ordering question the reviewer raised separately — a SemaphoreSlim release
is not a barrier in the C# memory model, so a static written inside the gate and
read after it was not guaranteed visible on ARM64. Nothing crosses the gate now.

KCAP_WORKOS_TOKEN_URL was a redirect primitive: the REQUEST direction carries the
secret, so anyone able to set one environment variable — a ConfigMap rather than a
Secret, a CI "variable" rather than a "secret" — could point the mint at their own
host and harvest it. The no-echo rule only ever protected the response direction.
Now refused unless https or loopback.

The reviewer proposed requiring https outright and assumed the tests already used
it; they use WireMock over http, so that rule would have broken them and invited
someone to weaken it later. https-except-loopback is the same carve-out OAuth
redirect-URI rules make, for the same reason: a credential cannot leave the
machine over 127.0.0.1. It reports rather than throws, matching this path's
contract, and it refuses rather than falling back to the default — a silent
fallback would send the real credential to the real endpoint while the developer
believed they were pointed at a stub.

Also: the token cache is keyed on client id AND token URL, so a second credential
cannot receive the first one's token; a comment records why Intended must not be
widened, since it diverts an interactive user off their own profile; the mint's
Content-Type is now asserted; the end-to-end asserts a mint actually happened,
not just that a header ended up set; and the help text states that a runner needs
no login and that the exchange needs egress to WorkOS.

Four new tests: plaintext non-loopback refused with NO request leaving the process,
loopback allowed, malformed URL refused, and a different credential minting its own
token. 15/15 here, 20/20 on the help-text tests after editing the help.

Part of AI-990.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
TryResolveTokenUrl admitted a URL if it was https OR loopback, but Uri.IsLoopback
is host-only — so ftp://127.0.0.1 or ws://localhost passed, being loopback but not
a credential-safe POST target. The remote-http exfiltration vector was already
blocked (not loopback, not https); this closes the odd-scheme loopback gap. Now:
https anywhere, or http on loopback.

New parameterised test refuses ftp/ws loopback URLs with zero requests leaving
the process. 17/17.

Part of AI-990.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Qodo re-scored after the review-flow fixes: its cache-scoping and static-race
findings resolved, two remained. Both real; neither the flow surfaced.

1. No automatic 401 re-mint. The machine client got no retry handler, unlike the
token-store path, and the common CreateAuthenticatedClientAsync does not thread
rejectedAccessToken — so a token revoked mid-life (unexpired by the local clock,
so the proactive renew margin never fires) produced repeated 401s until the cache
aged out. New MachineUnauthorizedRetryHandler mirrors UnauthorizedRetryHandler: on
a 401 it re-mints via the credential (client_credentials needs no refresh token)
and resends once. Installed on the same autoRetryUnauthorized terms, so the MCP
servers' own 401 loops are not double-retried. Test drives a real request through
the constructed client — 401 the first token, re-mint, 200 — asserting exactly one
extra mint.

2. Unsanitised token URL in Problem strings. The URL and the caught exception
message reach stderr, so both now go through the same helpers
HttpClientExtensions.RenderUnreachableError uses — UnusableUrlDiagnostic.Sanitize
drops userinfo (a KCAP_WORKOS_TOKEN_URL of https://id:secret@host would otherwise
print the secret) and StripControlCharacters (made internal) removes control chars
so a crafted value cannot inject lines into stderr. Test proves userinfo does not
survive into the Problem.

19/19.

Part of AI-990.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@realtonyyoung

Copy link
Copy Markdown
Collaborator Author

Both open Qodo findings addressed in 196b01f, with tests.

1. No automatic 401 re-mint — real, and the important one. The machine client installed no retry handler (unlike the token-store path), and the common CreateAuthenticatedClientAsync doesn't thread rejectedAccessToken, so a token revoked mid-life 401'd repeatedly until the cache aged out. New MachineUnauthorizedRetryHandler mirrors UnauthorizedRetryHandler: on a 401 it re-mints from the credential (no refresh token needed for client_credentials) and resends once, installed on the same autoRetryUnauthorized terms so the MCP servers' own loops aren't double-retried. Test drives a real request through the constructed client — 401 → re-mint → 200 — asserting exactly one extra mint.

2. Unsanitized token URL in error strings — the URL and caught exception message reach stderr, so both now go through the same helpers RenderUnreachableError uses: UnusableUrlDiagnostic.Sanitize (drops user:pass@ — a KCAP_WORKOS_TOKEN_URL of https://id:secret@host would otherwise print the secret) and StripControlCharacters (made internal). Test proves userinfo doesn't survive into the Problem.

Findings 3 (cache scoping) and 4 (static race) were already resolved by the review-flow round that removed the static entirely and keyed the cache on (ClientId, TokenUrl).

🤖 Addressed by Claude Code

@realtonyyoung
realtonyyoung merged commit fe175e6 into main Aug 8, 2026
6 checks passed
@realtonyyoung
realtonyyoung deleted the claude-tyoung/ai990-machine-auth branch August 8, 2026 05:49
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.

1 participant