cli: address #1673 review — refresh fallback, 0600 write, distinct code

- Refresh failure now falls back to injecting a still-present stored access
  token instead of hard-exiting 3, so a transient auth-server hiccup doesn't
  regress a previously-working invocation (review #1). Only a failure with no
  access token to fall back on surfaces as exit 3.
- Persist rotated tokens via the shared writeStoreFile (0o600 + mkdir -p)
  instead of a raw writeFile, so the secrets file keeps owner-only perms
  identical to the web backend (review #2).
- Missing clientInformation now throws the distinct no_client_information
  envelope code rather than reusing no_stored_token (review #5a).
- Docs: --wait-for-auth no longer claims parity with --use-stored-auth (it
  injects the freshly-landed token without refreshing); noted the
  always-refresh rotation/concurrency caveat (review #3, #4).
- Tests: discovery-returns-undefined metadata path, no_client_information
  code, and an e2e refresh-failure → access-token fallback.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BrihGWcrM9JGRyu41nzZYw
This commit is contained in:
cliffhall 2026-07-11 23:21:59 -04:00
commit 59f3bd92a4

View file

@ -194,13 +194,15 @@ For the common case where OAuth was already completed in the **web inspector on
| Option | Description |
| ----------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| `--use-stored-auth` | Read the stored auth for `--server-url` and inject `Authorization: Bearer`. When a `refresh_token` is stored, the CLI runs the OAuth refresh grant first and injects the **fresh** access token (persisting the rotation); otherwise it injects the stored access token. Exits `3` (`no_stored_token`) — listing the stored keys — when nothing matches. Requires `--server-url`. |
| `--wait-for-auth <sec>` | Poll the OAuth state file (500 ms interval) until a token for `--server-url` appears, then proceed as if `--use-stored-auth` were set. Times out at `<sec>` with exit `3` (`auth_wait_timeout`). Use after handing off to a human to complete OAuth in a browser. |
| `--wait-for-auth <sec>` | Poll the OAuth state file (500 ms interval) until an access token for `--server-url` appears, then inject it. Times out at `<sec>` with exit `3` (`auth_wait_timeout`). Use after handing off to a human to complete OAuth in a browser. Unlike `--use-stored-auth`, this injects the freshly-landed access token directly (a token that just completed the browser flow is not expired), so it does **not** run the refresh grant. |
| `--list-stored-auth` | Print `{ oauthStatePath, storedServerUrls }` (the server keys that currently have a token) and exit. No server connection is made. |
| `--print-handoff` | Print a JSON handoff block (`deepLink`, `portForwardCmd`, `oauthStatePath`, `apiToken`) for `--server-url` and exit — everything a script/remote VM needs to drive the browser-side OAuth dance. Requires `--server-url`. |
**State-file resolution** follows `MCP_INSPECTOR_OAUTH_STATE_PATH` → `<MCP_STORAGE_DIR>/oauth.json` → `~/.mcp-inspector/storage/oauth.json` — the same precedence the rest of the Inspector uses, so the CLI and web backend agree on the file. Server keys are canonicalised with `new URL().href` (the scheme the web store writes), so a trailing-slash or case mismatch between the URL a human opened and the one the agent passed still resolves.
**Token refresh (#1665).** When the stored server state carries a `refresh_token` (plus the `clientInformation` the web inspector persists after a completed flow), `--use-stored-auth` runs the SDK's OAuth `refresh_token` grant to mint a fresh access token before connecting, then writes the rotated tokens back to the state file so web and CLI stay consistent. The auth-server metadata is reused from the stored state, or discovered from `--server-url` when absent. This covers both an absent and an expired stored access token — the persisted blob carries no expiry, so the refresh token is treated as the durable credential. A failed refresh (revoked/expired refresh token) exits `3` (`auth_required`, envelope `refresh_failed`); re-complete the flow in the web inspector and retry. Without a stored `refresh_token` the access token is injected as-is, and a stale one surfaces as an HTTP `401` → exit `3`.
**Token refresh (#1665).** When the stored server state carries a `refresh_token` (plus the `clientInformation` the web inspector persists after a completed flow), `--use-stored-auth` runs the SDK's OAuth `refresh_token` grant to mint a fresh access token before connecting, then writes the rotated tokens back to the state file (owner-only `0o600`, via the shared store writer) so web and CLI stay consistent. The auth-server metadata is reused from the stored state, or discovered from `--server-url` when absent. This covers both an absent and an expired stored access token — the persisted blob carries no expiry, so the refresh token is treated as the durable credential. If the refresh fails (revoked token, transient auth-server error) **and** a stored access token is also present, the CLI falls back to injecting that token rather than hard-failing; only when there is nothing to fall back on does it exit `3` (`auth_required`, envelope `refresh_failed`). Without a stored `refresh_token` the access token is injected as-is, and a stale one surfaces as an HTTP `401` → exit `3`.
Because there is no stored expiry, a `refresh_token` is refreshed on every `--use-stored-auth` run. Two consequences with rotating refresh tokens: two concurrent invocations against the same state file race the single-use token (one wins), and a crash between a successful grant and the write-back leaves the rotated token unsaved. Both are narrow; re-authorize in the web inspector to recover.
**Short-circuit modes.** `--list-stored-auth` and `--print-handoff` each print their output and exit without connecting to a server; they ignore the method/target flags. They are mutually exclusive — if both are passed, `--list-stored-auth` takes precedence.