Repository navigation
fix: make MCP credential failures visible and diagnosable - #9
Merged
Merged
Conversation
Two MCP servers had been dead for days without anyone noticing. The credentials themselves need rotating by a human, but the reason the outage stayed invisible - and the reason it misdiagnosed once someone did look - is fixable here. Auth host resolution resolveHostForAuth (auth token, auth logout) and auth status ignored NULLIFY_HOST, unlike resolveHost used by all eleven other command sites. That is exactly the container and MCP setup this repo ships: credentials seeded from a secret store, host supplied purely through the environment. The result was 'auth status' reporting "Not configured" on a host every other command resolved fine, hiding the real cause - a refresh token rejected with 401. Adds lookupHostForAuth with flag > env > config precedence, reporting absence to the caller instead of exiting so auth status can print guidance. auth login now consults it too, rather than dropping to an interactive prompt in a container that has no terminal. Half-configured CLI nullify-serve.sh seeded credentials.json but never config.json, so anyone debugging inside the container hit "Not configured" with valid credentials sitting right there. It now seeds both. Silent secret resolution with-op-env.sh exec'd the server even when requested vars were still unset. It now names the missing vars, the vault and item it consulted, and warns that the server will likely expose no working tools. Also warns when the target command never appears on PATH. Preflight Adds .claude/mcp/preflight.sh, run report-only from the session-start hook. It probes each server for the two failure modes that matter: dying at startup, and starting with a full tool list while every call is unauthorized. The second is the dangerous one - coverage looks complete while the data is silently absent - so servers offering an authorization probe get one rather than being judged on tool count. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mcq6LfAVPzKZL5rpvww13R
The whoami tool falls back to the interactive device flow when the API key is rejected, so it blocked until the 120s timeout and the preflight downgraded a definite "key rejected" to an inconclusive "did not answer". That is the wrong way round: an expired key is exactly the case this check exists to catch. Probing the portal's /auth/me directly answers the same question in under a second and distinguishes 401/403 from an unreachable host. Whole preflight now completes in ~11s instead of timing out. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mcq6LfAVPzKZL5rpvww13R
Seeded OAuth credentials are the wrong model for a container that is rebuilt
every session. The refresh token has to be minted through a browser, any
token the CLI refreshes is written to a filesystem that is discarded on exit,
and so the copy in the secret store only ever ages. When it finally expires
the server exits during startup and shows up as a server with no tools.
The CLI has always accepted a long-lived NULLIFY_TOKEN, which outranks stored
credentials and has no refresh cycle at all - but nothing in the cloud wiring
resolved it, so putting one in the secret store had no effect. Now:
- .mcp.json requests NULLIFY_TOKEN and NULLIFY_CREDENTIALS_JSON as
alternatives, and nullify-serve.sh uses whichever arrived, preferring the
token and logging which model is live.
- The session-start hook resolves NULLIFY_TOKEN first and only seeds the
credentials file when no token is configured, rather than skipping the
var deliberately as before.
- with-op-env.sh gained optional vars, marked with a "?" prefix. They are
still resolved, but their absence is not reported - without this, offering
two alternative credentials guarantees a spurious warning about whichever
one is unused.
- preflight names the credential model in use, so a failure points at the
secret that actually needs attention.
Verified end to end: with NULLIFY_TOKEN set the server completes a handshake
and lists 27 tools instead of dying on the refresh.
Also fixes a latent bug that may be the cause of the current outage.
refreshToken() re-saved the same refresh token and never looked for a new one,
even though the same handler already falls back to a Set-Cookie for the access
token. Against a provider that rotates refresh tokens on use, the replacement
is dropped and the next refresh fails with a 401 - which is exactly the
symptom seen here. A rotated token is now read from the JSON body (either
spelling) or a Set-Cookie and persisted; absent means non-rotating, so the
existing token carries forward.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Mcq6LfAVPzKZL5rpvww13R
Two corrections, both cases where the preflight confidently reported the wrong thing. latent-defense was never broken. /auth/me serves device-flow sessions only and answers 401 to a perfectly valid API key, so checking it produced a false failure - and the no-key control returned the same 401, which made the wrong conclusion look confirmed. The whoami tool is worse: it calls /auth/me, reads the 401 as unauthenticated, then blocks on the interactive device flow. /api/infra/records discriminates honestly - 200 with a valid key, 401 with a missing or bogus one - and the server reports 130 tools against a graph of 31,111 nodes and 21 attack paths. NULLIFY_TOKEN now gets validated before use. A JWT carries its own verdict, so read it rather than waiting for the API to answer 403 on every call. Two mistakes are worth naming explicitly: an expired token, and the "access_token" field copied out of a credentials.json, which on this provider is an ID token (token_use=id) that authenticates the process but is rejected for API calls. Opaque non-JWT tokens are left alone. Covered: expired ID token, in-window ID token, valid access token, and an opaque token that must not trip either guard. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mcq6LfAVPzKZL5rpvww13R
Upstream documents the actual contract: GET /auth/refresh_token returns no token in its JSON body at all (RefreshTokenOutput has no token field), and the access token arrives solely as a Set-Cookie. This provider does not rotate refresh tokens, so the rotation handling added earlier was guarding against a case that cannot occur, and it would conflict with adopting the upstream fix. The 401 was a refresh token reaching its own expiry. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mcq6LfAVPzKZL5rpvww13R
An MCP server almost never fails loudly. It either dies during startup and appears in the client as a server with no tools, or it starts, advertises its whole tool set, and answers every call with an empty result because its credential is rejected. Both read as "nothing to report" rather than "outage", and the second is worse: work gets published against data that was never there. Preflight now takes its server list from .mcp.json rather than a list kept by hand, so a newly added server cannot quietly become a blind spot - it is reported WARN "no probe defined" until someone gives it one. Each server is started through its real .mcp.json command, so the wrapper and argument wiring are under test too, not just the binary. Authorization probes move into .claude/mcp/health-probes.json, with three kinds: an HTTP call against a data endpoint, JWT inspection for expiry and token type, and a presence check for servers with no cheap authenticated endpoint. The file documents why latent-defense is probed on /api/infra/records and explicitly not on /auth/me, which serves login sessions and answers 401 to a valid API key - the cause of a six-day false outage. Results are written to .claude/mcp/health.json (gitignored, per-session) so a later session can read coverage instead of assuming it, and the session-start hook now prints an unmissable banner when anything is unusable. CLAUDE.md tells whoever comes next to read that file before reporting on security data, and that an empty result from a failing server means absent data, never "nothing found" - the loop that failed the first time round. Also adds `make preflight`, and records upstream's refresh-token contract and the one fix (Nullify-Platform#165) this fork still lacks. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mcq6LfAVPzKZL5rpvww13R
Our transport had no 401 handling at all. It refreshed on a five-minute TTL and, when the provider failed, fell back to the stale cached token - so against a credential revoked mid-life every single request failed, forever, with no attempt to fetch a replacement. That is the shape of the outage we just spent a week diagnosing. This ports the behaviour of upstream Nullify-Platform#165 onto our simpler transport rather than adopting the file wholesale, because this fork branched at Nullify-Platform#138 and the upstream version depends on Nullify-Platform#151/Nullify-Platform#158 that we do not carry. - refreshProvider is a second, distinct provider used only after a 401. tokenProvider returns the stored token untouched while it still looks valid, so reusing it for the retry would replay the rejected string - the exact bug Nullify-Platform#165 was written to fix. - auth.ForceRefreshToken refreshes regardless of the recorded expiry, and reports ErrNotRefreshable when the credentials carry no refresh token. - lib.RefreshNullifyToken reports client.ErrTokenNotRefreshable for sources that yield a fixed string (--nullify-token, NULLIFY_TOKEN), so the transport surfaces the 401 instead of retrying something that cannot change. - Separate backoff clocks for the TTL and 401 paths: one provider failing says nothing about the other, and without them a rejected credential puts a blocking refresh in front of every request. - A refresh that returns nothing, or returns the token just rejected, leaves the cache alone rather than overwriting a working value with junk. - Request bodies are buffered so a retried POST carries its payload. Tested: retry-and-succeed, fixed-source surfaces the 401, same-token and empty returns do not loop or clobber the cache, body replay, backoff suppression, concurrent rotation, non-401 passthrough, and provider-failure backoff. Upstream's own note on Nullify-Platform#165 was that this code went broken from February to August precisely because it had no tests. CLAUDE.md records what remains unported, including Nullify-Platform#159 - the GitHub PAT is still sent in a URL query string. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mcq6LfAVPzKZL5rpvww13R
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two MCP servers had been dead for days without anyone noticing. The
credentials themselves need rotating by a human, but the reason the
outage stayed invisible - and the reason it misdiagnosed once someone
did look - is fixable here.
Auth host resolution
resolveHostForAuth (auth token, auth logout) and auth status ignored
NULLIFY_HOST, unlike resolveHost used by all eleven other command
sites. That is exactly the container and MCP setup this repo ships:
credentials seeded from a secret store, host supplied purely through
the environment. The result was 'auth status' reporting "Not
configured" on a host every other command resolved fine, hiding the
real cause - a refresh token rejected with 401.
Adds lookupHostForAuth with flag > env > config precedence, reporting
absence to the caller instead of exiting so auth status can print
guidance. auth login now consults it too, rather than dropping to an
interactive prompt in a container that has no terminal.
Half-configured CLI
nullify-serve.sh seeded credentials.json but never config.json, so
anyone debugging inside the container hit "Not configured" with valid
credentials sitting right there. It now seeds both.
Silent secret resolution
with-op-env.sh exec'd the server even when requested vars were still
unset. It now names the missing vars, the vault and item it consulted,
and warns that the server will likely expose no working tools. Also
warns when the target command never appears on PATH.
Preflight
Adds .claude/mcp/preflight.sh, run report-only from the session-start
hook. It probes each server for the two failure modes that matter:
dying at startup, and starting with a full tool list while every call
is unauthorized. The second is the dangerous one - coverage looks
complete while the data is silently absent - so servers offering an
authorization probe get one rather than being judged on tool count.
Co-Authored-By: Claude Opus 5 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_01Mcq6LfAVPzKZL5rpvww13R