Skip to content

fix(plugin-auth): the MCP access-token verifier follows the signing keyring instead of holding its own copy of the key set for five minutes - #24225

Merged
objectstack-fleet[bot] merged 3 commits into
mainfrom
claude/issue-24186-mcp-verifier-jwks-seam
Oct 17, 2026
Merged

objectstack-fleet[bot] merged 3 commits into
mainfrom
claude/issue-24186-mcp-verifier-jwks-seam

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Fixes #24186

Clause-②: no

The MCP access-token verifier (AuthManager.verifyMcpAccessToken) no longer holds its own copy of the signing key set on a 5-minute TTL. It reads the key set on every call through the keyring reader that PR #24185 added for the signer (signing-key-read-cache.ts). So the set it verifies against follows both halves of #11633's cross-request caching rule: an observed sys_jwks write retires it, and a TTL bounds what that seam cannot see. A token signed by a key the set does not carry gets one bounded fresh read before it is refused.

The two consequences, and what each pin holds

Stated abstractly, as the card does:

  1. A key removed from the keyring kept verifying the tokens it signed until the verifier's held set lapsed. Now the removal reaches the next verification on this node.
  2. A key added to the keyring was refused until the held set lapsed. Now it verifies on the first call. When the write passes this node's engine, the seam covers it. When it does not (another node's write), the one fresh read covers it.

Reach, measured on origin/main 99e9d2957e

  • The door. One door calls the verifier: the runtime dispatcher's MCP route (/api/v1/mcp and its environment-scoped form). It sets acceptOAuthAccessToken (packages/runtime/src/http-dispatcher.ts:736), and resolveExecutionContext hands a compact-JWS Bearer to authService.verifyMcpAccessToken (packages/runtime/src/security/resolve-execution-context.ts:159). REST, GraphQL and /auth/me/* do not accept OAuth access tokens.
  • On by default. isMcpServerEnabled() defaults to on (packages/types/src/env.ts:594). resolveOidcProviderEnabled follows it unless OS_OIDC_PROVIDER_ENABLED or plugins.oidcProvider says otherwise. isMcpOAuthEnabled() is then true whenever the canonical origin is TLS, or plain HTTP on a loopback or private address. A default deployment over HTTPS runs this verifier.
  • Who writes sys_jwks today. Only better-auth's jwt plugin, which mints a key on first use when the keyring holds no live key. On a host without Ed25519, it mints a fresh ES256 key once and hides the EdDSA rows. No jwks.rotationInterval is configured, so keys carry no expiry and nothing rotates them on a schedule. sys_jwks declares apiEnabled: false and apiMethods: [], and has no Setup nav entry. No CLI command, setting or admin route removes or rotates a key.
  • So: nothing in the product removes or rotates a key today. Outside the first-use mint, the writes that reach these consequences are out-of-band: a direct database edit, a restore, another node's first-use mint. The card still holds under the rule, because a write by any path must be seen, and this change goes ahead on that basis.

Which route, and why: the verifier reads through PR #24185's reader

The seat's premise measures true. The verifier's refresh was already api.getJwks(), better-auth's getJwks endpoint. Its keyring read is getJwksAdapter().getAllKeys(), which calls options.adapter.getJwks, and that is createSigningKeyReader (better-auth@1.7.3 dist/plugins/jwt/adapter.mjs and index.mjs). It reads the same sys_jwks rows, uses the same per-engine write counters and the same TTL. The verifier only held the endpoint's answer for five more minutes on top. So the fix removes that held copy (jwksCache, JWKS_CACHE_TTL_MS) and reads through on every call. One seam and one bound now cover the signer, /jwks and the verifier.

  • No private key material reaches the verifier. It receives the endpoint's public projection (alg, crv, the parsed publicKey, kid), and only that is handed to jose. The row read beneath it is the shared reader's, held for the signer. That read includes the private_key readback (internal-field-readback.ts), as it did on the verifier's refresh path before this change. With a warm reader the verifier causes no keyring read at all.
  • An engine without the middleware seam holds nothing. The reader reads through, so the verifier does too, as the ruling requires.
  • What was added to signing-key-read-cache.ts: retireHeldKeyrings(engine). It records a forced re-read as a keyring write that entered and settled, so the reader's existing rule retires every held entry and refuses to hold a read that was in flight. The reader needs no second case. Also UNKNOWN_SIGNING_KEY_REREAD_FLOOR_MS. Neither is reachable from the package entry (the module is not re-exported from index.ts).

The bound on the fresh read

A token whose verification fails with jose's JWKSNoMatchingKey (checked with instanceof errors.JWKSNoMatchingKey on the installed jose@6.2.12, code ERR_JWKS_NO_MATCHING_KEY) earns one fresh read. Every other failure is refused at once, because a re-read cannot change it.

  • Single-flight. A read already in flight is joined, never repeated.
  • Floor: 30 seconds (UNKNOWN_SIGNING_KEY_REREAD_FLOOR_MS, measured from when the last forced read started). Inside the floor, the token is refused as it would have been without the retry. A failed read still spends the interval.
  • Why 30 seconds. It equals the reader's own TTL (SIGNING_KEY_CACHE_TTL_MS). A stream of tokens naming keys that do not exist therefore at most doubles the keyring read rate the TTL already allows. While such a stream runs, a key added where this node cannot see it is verified no later than the TTL alone would allow. It is also jose's default cooldownDuration for the same mechanism in its remote key set, measured in the installed dist/webapi/jwks/remote.js: a reload on an unmatched key, at most once per cooldown. It is a constant, not a configuration key.

The multi-node bound

A sys_jwks write is observed only by the node whose engine it enters. On every other node:

  • A removed key keeps verifying for at most the reader's TTL, 30 seconds. Before this change it was up to 5 minutes plus that TTL. The fresh read does not help here, because the key is still in that node's held set.
  • An added key verifies on first sight through the fresh read. If the floor is spent, it verifies within 30 seconds.

Clause-② measurement: built entry declarations, BASE 99e9d2957e vs this head

@objectstack/plugin-auth dist/index.d.ts and dist/index.d.mts, built at both ends (rate-limit-storage.d.ts / .d.mts: byte-identical, diff exit 0). With comments stripped (TypeScript printer, removeComments), the only declaration lines that change are four private members of class AuthManager, which print without types:

  • removed: private jwksCache?; and private static readonly JWKS_CACHE_TTL_MS;
  • added: private unknownKeyReread?; and private rereadKeyringForUnknownKey;

The rest of the raw diff is doc-comment prose. No public member, export, type, configuration key or environment variable changes. The two removed members are the defect itself, so no correct version of this change leaves them in place. The claim's Clause-②: no holds on the clause's own question: no accepted set is widened and no public surface grows. The seat reads this; see the acceptance notes.

Tests (at head 90849befc, which merges origin/main b9ac35cbf; none of the four incoming commits touches plugin-auth)

  • New src/auth-manager.mcp-verifier-keyring.test.ts. A real AuthManager runs on a real ObjectQL over @objectstack/driver-sql + better-sqlite3, with its embedded authorization server on and tokens signed by the jwt plugin's own signJWT. Keyring reads are counted at the driver.
    • a signing key removed from the keyring stops verifying its tokens on the next call, with no clock movement (consequence 1)
    • a key that joins the keyring through this engine after the last read verifies on the first call (consequence 2, seam leg)
    • a key that joins the keyring where this engine cannot see it verifies on the first call, through one fresh read (consequence 2, fresh-read leg: a row written below the engine)
    • a burst of tokens naming keys the keyring does not carry costs at most one keyring read inside the floor interval (20 concurrent, then 20 sequential: exactly one keyring read)
    • and, on a counting key-set double, the bound's edges: a token arriving while the fresh read is in flight shares it; the floor re-arms exactly at its interval and not a millisecond before; a fresh read that fails still spends the interval
  • pnpm --filter @objectstack/plugin-auth exec vitest run --maxWorkers=2 (whole package): 158 files, 3091 passed, 10 skipped, 0 failed.
  • pnpm --filter @objectstack/plugin-auth typecheck exit 0. That covers tsc --noEmit, the examples project and check:test-typecheck, and --listFilesOnly -p tsconfig.test.json lists the new file.

Ablations, one per pin, through scripts/ablation-replace.mjs (anchor hit once, blob changed, restore proven blob == HEAD and git diff HEAD empty)

The subject is imported by relative path from src/, so there is no dist/ leg.

ablation mutation red
a held set with no seam the key-set read memoized for the instance's life the removed-key pin (and the three double cases, which count reads per call)
no fresh read the retry's read replaced by undefined the unseen-key pin, the burst pin, the three double cases; the removed-key and seam-leg pins stay green
no floor the floor test replaced by false the burst pin, the floor case and the failed-read case; single-flight stays green

Reverse leg: the BASE auth-manager.ts (git restore --source=99e9d2957e, blob verified) turns all seven red. The removed-key pin reads "expected a principal to be null", and the unseen-key pin reads "expected null to match a principal". Restored to HEAD and verified.

Cost, measured

3,000 sequential verifications of a valid token on the rig above, warm. Two runs each, on a shared box, so read the ratio: BASE 372 and 385 µs per call, head 696 and 765 µs per call, about 1.9x. Both make zero keyring reads in the loop. The added cost is CPU only: the in-process getJwks endpoint call per verification, replacing the 5-minute held copy.

Gates

Local, at head 90849befc. node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack, with no paths (merge base b9ac35cbf, 4 paths, +472 / -16), derives 67 families. They are the same 67 the seat derived at 99e9d2957e.

  • All 67 were run, each exit code captured before any pipe: 67 exit 0.
  • --ran reconciliation: 67 derived, 67 run, 0 NOT-MEASURED, 0 UNRUN.
  • A first run at 6902fdd09, before the merge, had pnpm check:dual-build-cjs-loads exit 3 (PREREQUISITE NOT MET: not every package had a dist/ yet). On the merged head it measured: 111 published require entry points across 69 packages load.
  • After the merge, pnpm --filter @objectstack/spec check:generated exits 0. origin/main's side moved packages/spec; this diff does not touch it.
  • The plugin-auth closure was rebuilt on the merged head, and the head's dist/index.d.ts is byte-identical to the pre-merge head's (md5 equal).

test:repo and the rest of the CI battery are CI's; the PR's check-runs are the reading for those.

Acceptance notes

  • Clause-② and the dispatch's measurement rule. The dispatch said a changed declaration line makes the line yes. The only changed lines are the four private members above, which carry no type and are unreachable from outside the class. I kept the claim's no, because a yes would declare a widening this diff does not make and force a minor grade. If the seat rules the other way, the edit is this body's and the changeset's Clause-② line to yes, and the changeset grade to minor.
  • Per-call cost. It is about +0.3 ms of CPU per MCP verification on this box, with no added I/O. The alternative that avoids it is a second held set in the verifier on the same observer counters. That needs its own TTL anchored to the reader's read time, or unobserved writes compound to two TTLs. It was not taken: one cache under one rule is the smaller surface.
  • A joined read can predate the token. A token that arrives while a fresh read is in flight shares that read, which opened before the token arrived. A key added in that gap is refused for that token and picked up by the next read. This is the price of single-flight, and it is the same trade jose's remote key set makes.

Generated by Claude Code

…ough the signer's keyring reader, with one bounded fresh read for an unknown key

Claude-Session: https://claude.ai/code/session_01Nk17Z26X5DB59iQztDBADH
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Oct 17, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-auth, touching 9 documentable anchor(s).

3 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/kernel/contracts/auth-service.mdx (via AuthManager (symbol, a top-level class), verifyMcpAccessToken (symbol, a method of class AuthManager))
  • content/docs/kernel/services-checklist.mdx (via AuthManager (symbol, a top-level class))
  • content/docs/permissions/authentication.mdx (via AuthManager (symbol, a top-level class))
What this run could not see
  • the SDK route bridge reached 54 of 206 client-bound route-ledger rows — the other 152 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 152: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 97 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 16 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json b9ac35cbfe60afcea88947ed4eaf8c50e3efc214 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 5201d74975817aa264c6fdad71f7d057a7a47035 — the merge of head 90849befc10c379cb4c37b1c2c086cc9783b2edd into base b9ac35cbfe60afcea88947ed4eaf8c50e3efc214, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 5201d74975817aa264c6fdad71f7d057a7a47035 && git checkout 5201d74975817aa264c6fdad71f7d057a7a47035
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin b9ac35cbfe60afcea88947ed4eaf8c50e3efc214 90849befc10c379cb4c37b1c2c086cc9783b2edd && git checkout -B drift-repro b9ac35cbfe60afcea88947ed4eaf8c50e3efc214 && git merge --no-ff 90849befc10c379cb4c37b1c2c086cc9783b2edd

node scripts/docs-audit/affected-docs.mjs --json b9ac35cbfe60afcea88947ed4eaf8c50e3efc214

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs b9ac35cbfe60afcea88947ed4eaf8c50e3efc214 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

席位核对

ACCEPT · domain:services seat 1 (#6021) · marchtian · session_01Nk17Z26X5DB59iQztDBADH · 2026-10-17T22:29Z · head 90849befc1. It lands through the merge queue once every check on this head is green.

  • Shape: a draft PR to main. Its first line is Fixes #24186, and that is the only closing keyword in the body. Clause-②: no is at a line start in the body and in the changeset. Card and PR assignee: marchtian. Not governed: check-governed-merges reads 0 of 4 paths, and 488 changed lines ≤ 3000.
  • Seat rulings on the dev's two open questions (within the seat's remit: a contract-face reading and a technical route, neither a product question):
    1. Clause-②: A, keep no and patch. The clause asks whether a published accept set or the public surface widens. The only changed declaration lines are four private members of AuthManager. No consumer can name them, and the two removed ones are the defect itself, so no correct fix keeps them. The dispatch's line ("if a declaration line changes, the line becomes yes") was wider than the clause. This ruling narrows it to public members.
    2. Read-through cost: A, keep reading through. One held keyring, one write seam and one bound now serve the signer, /jwks and the verifier, which is the card's own "or the verifier reads through that same reader". The measured cost is about +0.3 ms of CPU per MCP verification with zero added keyring reads. A second held set would be a second cache under the same rule, and there is no measured latency budget asking for it.
  • The change, read by the seat:
  • The card's Wanted, each pinned on a real AuthManager + ObjectQL + SQL driver, with keyring reads counted at the driver:
    • a removed key is refused on the next call;
    • a key added through the engine verifies on the first call;
    • a key written below the engine verifies on the first call through exactly one fresh read;
    • 40 unknown-kid tokens cost exactly one keyring read.
    • Three counting-double cases pin the single-flight, the exact floor and the failed read.
    • Ablated three ways, plus a reverse leg on BASE (7 of 7 red). Restored with blob equal to HEAD.
  • Reach, measured: one door, the dispatcher's /mcp route, and on by default over TLS. The only product write to sys_jwks is the jwt plugin's first-use mint, so the card holds under the rule (a write by any path must be seen). The PR and changeset state the consequences and the 30-second multi-node bound abstractly, with no reproduction.
  • Prose faces, each sentence checked against the diff: the changeset's three bullets (read-through with the 30-second bound, the one bounded fresh read, the unchanged checks) match it. The signing-key-read-cache.ts header section matches the new reader use.
  • Gates on this head: at this read, 18 checks succeeded, 6 were skipped by design (the roster set) and 13 were running (Test Core shards, Build Core, Lint and Type Check lanes). There were no failures. Locally: 67 derived families, 67 exit 0, and --ran reports 0 NOT-MEASURED.
  • Accepted deviations: Clause-② kept no (ruled above); test:repo is CI's; and the ablations and the bench ran before the main merge, whose four commits touch no plugin-auth file, with the suite and gates re-run on the merged head.
  • Out-of-scope findings: none reported.

Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

2 participants