Repository navigation
Commit 9040945
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)
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](https://claude.ai/code/session_01Nk17Z26X5DB59iQztDBADH)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent df000d4 commit 9040945
4 files changed
Lines changed: 472 additions & 16 deletions
File tree
- .changeset
- packages/plugins/plugin-auth/src
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
| 236 | + | |
| 237 | + | |
| 238 | + | |
| 239 | + | |
| 240 | + | |
| 241 | + | |
| 242 | + | |
| 243 | + | |
| 244 | + | |
| 245 | + | |
| 246 | + | |
| 247 | + | |
| 248 | + | |
| 249 | + | |
| 250 | + | |
| 251 | + | |
| 252 | + | |
| 253 | + | |
| 254 | + | |
| 255 | + | |
| 256 | + | |
| 257 | + | |
| 258 | + | |
| 259 | + | |
| 260 | + | |
| 261 | + | |
| 262 | + | |
| 263 | + | |
| 264 | + | |
| 265 | + | |
| 266 | + | |
| 267 | + | |
| 268 | + | |
| 269 | + | |
| 270 | + | |
| 271 | + | |
| 272 | + | |
| 273 | + | |
| 274 | + | |
| 275 | + | |
| 276 | + | |
| 277 | + | |
| 278 | + | |
| 279 | + | |
| 280 | + | |
| 281 | + | |
| 282 | + | |
| 283 | + | |
| 284 | + | |
| 285 | + | |
| 286 | + | |
| 287 | + | |
| 288 | + | |
| 289 | + | |
| 290 | + | |
| 291 | + | |
| 292 | + | |
| 293 | + | |
| 294 | + | |
| 295 | + | |
| 296 | + | |
| 297 | + | |
| 298 | + | |
| 299 | + | |
| 300 | + | |
| 301 | + | |
| 302 | + | |
| 303 | + | |
| 304 | + | |
| 305 | + | |
| 306 | + | |
| 307 | + | |
| 308 | + | |
| 309 | + | |
| 310 | + | |
| 311 | + | |
| 312 | + | |
| 313 | + | |
| 314 | + | |
| 315 | + | |
| 316 | + | |
| 317 | + | |
| 318 | + | |
| 319 | + | |
| 320 | + | |
0 commit comments