Skip to content

Add scoped authentication tokens (inline role on create_authentication_tokens) - #2176

Open
kriszyp wants to merge 15 commits into
mainfrom
kris/create-token-scoped-role
Open

Add scoped authentication tokens (inline role on create_authentication_tokens)#2176
kriszyp wants to merge 15 commits into
mainfrom
kris/create-token-scoped-role

Conversation

@kriszyp

@kriszyp kriszyp commented Aug 14, 2026

Copy link
Copy Markdown
Member

create_authentication_tokens now accepts role as an inline role-shaped object (e.g. { permission: { operations: ['read_only'] } }) and mints a single scoped-operation JWT that embeds the (downgraded, deep-validated) permission set. The bearer needs no pre-existing hdb_user or hdb_role row. Minting is super_user-gated (or trusted internal dispatch); no refresh token is issued and no user record is touched. validateOperationToken accepts the new subject and builds a synthetic user from the embedded role. A string role keeps its legacy meaning (stamped verbatim for component-defined tokens, still rejected by the operations API).

username is attribution only: any string that does not name an existing user (collisions are rejected at mint — see decision 5), defaulting to scoped:<minter>. It appears in audit logs and user_info for the bearer's requests.

Example:

{
  "operation": "create_authentication_tokens",
  "username": "reporting-service",
  "role": { "permission": { "operations": ["read_only"] } },
  "expires_in": "7d"
}

The cross-model design and implementation reviews surfaced four pre-existing defects this PR also fixes:

  1. SQL bypassed the operations allowlist: chooseOperation never calls verifyPerms for sql, so the allowlist was unenforced there. The gate is now a shared exported helper (verifyOperationsAllowlist) called from both verifyPerms and the SQL branch. Note operation_authorization.ts has a legacy module.exports = {...} that clobbers the ESM exports at runtime — new exports must be added to that list too.
  2. Permissions-translator memo aliasing: getRolePermissions memoizes translated permissions by role name. Impersonation used a constant _impersonated name + per-request Date.now(), so two same-millisecond inline impersonations with different permissions could alias one cache slot; Modes B/C could write downgraded copies under the persisted role's name. All synthetic roles now get content-derived identities (syntheticRoleName), and synthetic translations live in a 256-entry LRU so distinct permission sets can't grow the memo unboundedly.
  3. operations allowlist gate ordering: the gate ran after the structure_user early-returns, so an inline role combining structure_user: true with a restrictive allowlist could reach unlisted schema DDL. The deny gate now runs before every ambient-privilege early return; the SU-grant bypass stays where it was. Persisted roles cannot combine super_user with other permission keys (validateNoSUPerms), so this is fail-closed only for inline/legacy anomalies.
  4. Expired Bearer tokens acted as anonymous: the Bearer catch in auth.ts swallowed non-invalid token failures, caching an undefined user — an expired token continued as anonymous (or a TypeError with success audit logging on). Now rethrown as a 401, and cached Bearer identities are evicted exactly at token exp instead of the auth-cache TTL — which matters doubly for scoped tokens, where expiry is the only revocation mechanism.

For the human reviewer

  1. Scoped tokens are irrevocable until expiry. They are not tied to a user row, so drop_user/alter_user/onInvalidatedUser cannot revoke them — only JWT key rotation (which kills all tokens, cluster-wide since keys are cloned) or expiry. This matches the impersonation trust model, but it is the PR's biggest policy call: a token-id + revocation registry would be a follow-up design, and customer expectations set here are hard to walk back. Each mint logs minter, attribution, and the permission content hash so outstanding grants can be correlated.
  2. The operations allowlist gates the operations API (including sql) — not the app/REST port. REST/GraphQL/MQTT authorize on the embedded table CRUD permissions only (same as persisted roles today). This is now stated in DESIGN.md, the MCP description, and the docs: a token meant to be read-only on REST must carry restrictive table perms. Extending the allowlist to the Resource path would be a semantic change to what operations means and is deliberately not done here.
  3. expires_in is uncapped, consistent with normal operation tokens. Given (1), an SU can deliberately mint a very long-lived credential. A hard cap was rejected to keep parity; revisit if (1) gets a revocation story.
  4. Silent downgrade of super_user/cluster_user in the inline role (forced false at mint and re-forced at validation) mirrors impersonation rather than rejecting the request.
    Behavioral change (deliberate): the auth.ts fix rethrows an expired/failed Bearer as a 401 instead of falling through to an undefined (anonymous) user — that fall-through was also the source of a potential newUser.username deref/500 under success-audit logging. Net effect: a request carrying an expired Bearer now 401s rather than proceeding anonymously, including on otherwise-unauthenticated operations. Fail-closed is the right default for expired credentials (and required for scoped tokens, where expiry is the only revocation); it's reversible with a narrow NO_AUTH exemption if a stale-header refresh flow needs it.
  5. Attribution usernames must not collide with real users (rejected at mint; default scoped:<minter>). Reason: paths that rehydrate a user by name would substitute the real principal's full permissions for the token's — or kill a valid session on the non-existent name. All three by-name sites are guarded by the same _scopedToken short-circuit: the MQTT last-will replay (DurableSubscriptionsSession.ts — wills persist the scoped role/marker/expiry; replay skips by-name lookup and drops expired-token wills), the live-subscription stale-auth recheck from Live subscriptions (SSE/MQTT/WS) continue delivering events after drop_user / role revocation — stale-auth leak #1414 (Resource.ts registerLiveSubscriptionForContext keeps the embedded role as the identity), and the MCP list_changed session refresh (components/mcp/listChanged.ts refreshSessionUser). A scoped bearer also cannot self-mint standing tokens (the passwordless createTokens path rejects a _scopedToken requester). Residual: only some new unguarded by-name site — which must check _scopedToken (stated in DESIGN.md); short expiries mitigate.
    Related open point (deliberate, adjudicated minor): request-time audit fields (updated_by, denial logs) show the attribution label without a scoped marker or minter. Collision rejection guarantees the label can't name a real principal, so it can't impersonate in the trail; threading _mintedBy through every audit surface is a follow-up.
  6. Scoped-token will expiry (DurableSubscriptionsSession.ts): both the abnormal-disconnect publish and worker-0 restart replay refuse to publish a scoped will past its authExpiresAt, via the shared isWillFromExpiredScopedToken(will) (keyed off the persisted will principal so the two paths can't diverge). Known coverage gap (accepted): there is no automated test that the live disconnect path refuses an expired scoped will. A scoped-token MQTT-will registration is not a first-class exercised path (MQTT CONNECT authenticates by username/password, so this is reachable only via the WS-Bearer handshake), and the module cannot be unit-tested locally (its imports open the system DB). The guard is defense-in-depth sharing the same helper as the restart-replay path; if scoped-token MQTT wills become a supported flow, an integration test should be added.
  7. Mint-time deep validation binds the token to schemas that exist on the minting node (addRoleValidation checks database/table existence). A token referencing a table that only exists on a peer is rejected at mint. Defensible, but couples issuance to local schema convergence; relaxing later is easy, tightening later breaks callers.
  8. Synthetic-role translations live in a 256-entry LRU keyed off the _ name prefix. Beyond 256 concurrently live distinct permission sets, requests degrade to per-request translation (a deliberate cliff, documented in DESIGN.md; the constant is trivially raised). A persisted role named with a leading _ lands in the LRU too — correct, just evictable.
  9. Aliased operation names: the allowlist gate resolves ops to their canonical api_name (create_schemacreate_database), so grants must use canonical names. Pre-existing gate behavior, now also load-bearing for scoped tokens.
  10. Mixed-version clusters: older nodes reject scoped-operation tokens with a 401 (fail-closed). Deploy all nodes before handing out scoped tokens.

Verification

  • Unit: 247 passing across the touched suites (security/tokenAuthentication, security/impersonation, security/auth, security/auth-fastify, security/permissionsTranslator, utility/operation_authorization, server/serverHelpers/serverHandlers, components/mcp/listChanged). The new scoped-token suite is stub-free per AGENTS.md (real key files + clearJWTRSAKeysCache, real users cache, direct jwt.sign for the legacy-token case). Coverage: SU gate, trusted-dispatch mint, downgrade at mint and validation, attribution-collision rejection, scoped self-mint block, password/purpose incompatibilities, unknown-op and malformed-shape rejection, signed-token size cap, synthetic-identity isolation, expired/tampered/wrong-subject rejection, legacy string-role rejection unchanged, allowlist-vs-structure_user/super_user ordering, MCP scoped-session-not-re-resolved.
  • Fails-on-base: new tests applied to a base-ref worktree — 14 tokenAuthentication, 7 impersonation, and both gate-ordering tests fail on base and pass on this branch.
  • Integration (end-to-end, real Harper): integrationTests/apiTests/token-auth.test.mjs 18/18 — mint for a non-existent username; listed read allowed; user_info reports the attribution name; unlisted insert denied despite a table-level grant; SU-only ops denied; an explicitly-listed SU-only op allowed (gate-2 delegation); unknown op rejected at mint; SQL SELECT allowed when sql is listed and denied when not (despite table read perms); expired token rejected 401 through the cache. integrationTests/security/subscription-revocation.test.ts 7/7 — including a scoped-token subscription that survives an unrelated recheck AND a real colliding no-read user (proving non-substitution) and then terminates at token expiry.
  • Full test:unit:main/test:unit:resources gates: blocked locally by the known shared-DB lock contention on this machine; deferring to CI for the full-suite gates.
  • npm run typecheck, npm run build, oxlint (lint:required), prettier on changed files: clean.

Companion docs PR: HarperFast/documentation#627 (scoped-tokens section in JWT authentication + operations reference).

Review coverage

Generated by Claude (Fable). Reviewed across a plan pass and multiple full/delta implementation rounds via the pre-push CLI (Codex graded + Gemini + Harper-domain adjudication; Cursor-Grok contributed from round 3 on, cursor-composer pruned). Convergence: the final rounds adjudicated minor with no surviving majors. Key rounds:

  • Plan review (Codex gpt-5.6-sol, design note before implementation): raised the memo-aliasing and gate-ordering blockers.
  • Round 1 (full, at 2314334): three majors — SQL allowlist bypass, expired-token swallow, unbounded synthetic memo — all fixed, plus size-cap/async-test/comment findings. Gemini's "authExpiresAt never populated" blocker adjudicated factually wrong.
  • Round 2 (full, at c8735e6): two majors — attribution-username collision enabling by-name rehydration escalation (fixed: mint-time rejection + replay-site guard), and the allowlist not gating the REST port (addressed as explicit documented scope, decision 2). Also the AGENTS.md sinon/rewire violation (fixed: stub-free rewrite). Gemini's "eviction is dead code" major adjudicated factually wrong.
  • Round 3 (delta, at 9dec475, cursor-grok also succeeded this round): one major — the last-will record persisted only { username }, so the round-2 replay guard never saw the scoped marker (fixed: the will now persists the token's role/marker/expiry and replay drops expired-token wills). The audit-marker minor is recorded as decision 5's open point; the test suite now uses a plainly-required module instance.
  • Round 4 (delta, at a158328): one major — the live-subscription stale-auth recheck (Live subscriptions (SSE/MQTT/WS) continue delivering events after drop_user / role revocation — stale-auth leak #1414 machinery) re-resolved scoped bearers by name, killing valid scoped subscriptions and re-opening the collision escalation (fixed: the embedded role is the recheck identity; expiry already handled via authExpiresAt). Also: will-replay rehydration moved inside per-will error handling, stale JSDoc fixed. The audit-marker minor remains decision 5's documented open point.
  • Round 5 (delta, at 5819953): one major — a scoped bearer could use the passwordless self-mint path of create_authentication_tokens, and after a colliding real user was created its unverified attribution name resolved to that user, issuing standing operation/refresh tokens (fixed: scoped bearers are blocked from self-mint). Added a scoped-token live-subscription integration test and a self-mint unit test for the two prior fixes' coverage.
  • Round 6 (full, at 6bc7738 — cursor-grok succeeded): its adjudicated severity was minor, but cursor-grok surfaced a third by-name rehydration site the DESIGN.md completeness claim had missed (components/mcp/listChanged.ts refreshSessionUser, MCP list_changed) plus a will-expiry asymmetry — both fixed, with the collision test tightened to create a real colliding user and an MCP unit test added.
  • Round 7 (delta, cursor-grok+domain): cursor-grok caught that the round-6 abnormal-disconnect expiry check keyed off this.user rather than the persisted will — fixed by keying both will paths off a shared isWillFromExpiredScopedToken(will) helper.
  • Round 8 (delta, cursor-grok+domain): cursor-grok reported no blockers and no concerns; verdict COMMENTS. Remaining item is the will-disconnect coverage gap documented in reviewer note 6.
  • Final full-lens passes (graded/codex + gemini + domain, at HEAD): reviewed the will-code fixes; findings were minor/nit and are addressed (a scoped will now publishes under its own embedded role, not a later same-clientId session's principal; restart replay tolerates a single will failure without aborting; the persisted will strips runtime-only state and fails closed on absent expiry) or carried as the accepted decisions below.

cursor-grok found real defects in three consecutive rounds (the MCP rehydration site, the will-expiry asymmetry, and the persisted-will divergence) — a high-value lens on this change, not a formality.

Accepted-by-design / documented (not defects): expiry-only revocation (decision 1); the operations allowlist scoped to the operations API + SQL, not REST (decision 2); attribution appearing in audit fields as a label that can't name a real principal (reviewer note 5); and no automated test for the live-disconnect scoped-will path (reviewer note 6, with the reason).

🤖 Generated with Claude Code

Human-Review-Need: 4 @ 0dace55

kriszyp and others added 13 commits August 14, 2026 11:51
create_authentication_tokens now accepts a role-shaped object (e.g.
{ permission: { operations: ['read_only'] } }) and mints a single
'scoped-operation' JWT that embeds the downgraded permission set, so a
limited credential can be issued without a pre-existing hdb_user or
hdb_role and with an arbitrary attribution username. Minting is
super_user-gated (or trusted internal dispatch); no refresh token is
issued and no user record is touched.

validateOperationToken accepts the new subject and builds a synthetic
user from the embedded role. Includes three defense-in-depth fixes
surfaced during design review:
- synthetic (scoped/impersonated) roles get content-derived identities
  so the permissions-translation memo can never alias two different
  inline permission sets (or poison a persisted role's entry)
- the role operations allowlist gate now runs before the ambient
  privilege early-returns (structure_user etc), so an allowlisted role
  cannot reach unlisted schema operations
- cached Bearer identities are evicted exactly at token expiry instead
  of at the auth-cache TTL

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ESIGN.md

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…, bounded synthetic memo

- enforce the role operations allowlist on the SQL path (chooseOperation
  never reaches verifyPerms for sql); exported verifyOperationsAllowlist
  and added it to the legacy module.exports list that clobbers the ESM
  exports at runtime
- rethrow non-invalid-token Bearer failures in auth.ts so an expired
  token is a 401 credential rejection instead of caching an undefined
  user (anonymous or TypeError under success audit logging)
- bound synthetic-role permission translations in an LRU (256 entries)
  so content-derived role names cannot grow rolePermsMap unboundedly
- measure the scoped-token size cap on the signed token, log the
  permission content hash at mint, fix the async-escaped handler test,
  trim narration comments
- integration: prove SQL allowlist enforcement and exact expiry through
  the authorization cache end-to-end

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…stub-free tests

- reject an attribution username that names an existing hdb_user (default
  is now scoped:<minter>): by-name rehydration paths (MQTT last-will
  replay) would otherwise substitute the real principal's permissions;
  the replay site additionally skips rehydration for _scopedToken users
- document that the operations allowlist gates the operations API + SQL
  only — application/REST surfaces authorize on table CRUD permissions
  (DESIGN.md, MCP description, docs)
- rewrite the scoped-token unit suite stub-free per AGENTS.md (real key
  files + clearJWTRSAKeysCache + real users cache; legacy token built
  with jwt.sign directly); document the synthetic-LRU cliff and the
  '_'-prefix convention in DESIGN.md; trim narration comments

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…require test module

- last-will records for scoped-token bearers now persist the token's
  role, marker, and expiry; replay uses the embedded role instead of
  rehydrating by name, and drops wills whose token has expired
- the scoped-token unit suite uses a plainly-required module instance
  rather than the file's rewire-loaded one

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- the live-subscription recheck (stale-auth revocation, #1414) no longer
  re-resolves a scoped-token bearer by name: the embedded role is the
  identity, expiry is the revocation; by-name lookup either killed a
  valid scoped subscription (no hdb_user row) or, after a colliding user
  was created, escalated the subscription to that user's permissions
- will replay wraps rehydration in the per-will error handling so one
  failed lookup cannot abort recovery of the remaining wills
- fix stale JSDoc default-attribution claim; drop a narrating test comment

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ression coverage

- a scoped-token bearer can no longer use the passwordless self-mint path
  of create_authentication_tokens: its unverified attribution name would
  otherwise resolve to (and issue standing operation/refresh tokens for) a
  real user created with that name after the scoped token was minted
- integration: a scoped-token live subscription survives an unrelated
  user-change recheck (proving it is not re-resolved by name) and still
  terminates at token expiry
- unit: scoped bearer self-mint is rejected

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… symmetry

cursor-grok found a third by-name rehydration site the DESIGN.md
completeness claim missed, plus a restart-vs-live will-expiry asymmetry:

- components/mcp/listChanged.ts refreshSessionUser no longer re-resolves a
  scoped-token session by name (would advertise a later-created colliding
  user's tool/resource surface). AuthedUser gains the _scopedToken marker
- the live abnormal-disconnect will publish now honors authExpiresAt for
  scoped tokens, matching restart replay (was: fires after expiry)
- DESIGN.md lists all three guarded by-name sites + the self-mint block
- tests: MCP scoped-session-not-re-resolved unit test; the scoped
  subscription integration test now creates a REAL colliding user with a
  no-read role, so continued delivery proves non-substitution; widened
  its expiry margin to remove the load-sensitive timing window

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…cipal

cursor-grok found the round-6 abnormal-disconnect expiry check keyed off
this.user, not the persisted will: a later same-clientId session with no
will leaves the prior scoped will row in place, and that session's
this.user may be a different non-scoped principal — so the leftover
scoped will would publish past expiry while restart replay drops it.

Both paths now share isWillFromExpiredScopedToken(will), keyed off the
persisted will principal, so they can't diverge again.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Final codex+gemini+domain pass on the round-7 will code (adjudicated minor):

- a scoped will now publishes under its OWN embedded role, not this.user:
  a later same-clientId session (a different, possibly more privileged
  principal) can read back the prior session's leftover will row and must
  not gain that principal's permissions. Non-scoped wills unchanged
- await the will-row deletes in restart replay (symmetry; caught by the
  worker unhandledRejection guard either way)
- verifyOperationsAllowlist treats a null operations list like undefined
  (the SQL call site would otherwise inherit the old gate's TypeError)
- restore the verifyPerms JSDoc that the extracted helper had orphaned
- scoped subscription test waits on delivery readiness instead of fixed
  sleeps; trim review-history-narrating comments

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…afe will

- restart replay: a single will's publish/delete failure no longer aborts
  replay of the rest (my prior await-for-symmetry change had let one
  rejection break the loop); the row stays and retries next restart
- isWillFromExpiredScopedToken fails closed — a scoped will with no
  recorded expiry is dropped, not published
- persist only durable permission fields on a scoped will, stripping the
  runtime-only _expandedOperations Set (rebuilt on read; not storage-safe)
- scoped subscription test anchors the expiry wait on mint time so the
  variable recheck phase can't race it
- add e2e coverage that a scoped token can invoke an explicitly-listed
  super_user-only op (gate-2 delegation)

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

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request implements scoped tokens and synthetic-role identities to allow minting JWTs with inline, downgraded permission sets that do not require persistent user records. It updates token authentication, session management, live subscriptions, and permission translation to support these scoped tokens safely without risk of privilege substitution or unbounded cache growth. The reviewer pointed out a potential issue in syntheticRoleName where non-deterministic key ordering in JSON.stringify could cause cache misses, and suggested sorting the permission object keys recursively before hashing.

Comment thread security/impersonation.ts
@claude

claude Bot commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Reviewed; no blockers found.

kriszyp and others added 2 commits August 14, 2026 14:36
Addresses gemini review on #2176: JSON.stringify key order is not
deterministic, so two structurally-identical permission sets differing
only in key order got distinct synthetic role names — never an aliasing
bug (distinct name = distinct slot), but it wastes entries in the bounded
synthetic-role LRU. Hash a key-sorted canonical form so identical sets
share one slot, easing the documented LRU-cliff pressure.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@kriszyp
kriszyp marked this pull request as ready for review August 14, 2026 22:24
@kriszyp
kriszyp requested review from DavidCockerill and removed request for dawsontoth August 14, 2026 22:24

@DavidCockerill DavidCockerill left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving at 68a87f5. The trust boundary here is well built, and every escalation path I went looking for was already closed with a comment naming the exact risk — which is not the usual experience.

One significant comment on the mint path (tokenAuthentication.ts:282), operational rather than a hole: expires_in is unbounded on a credential that has no revocation path. The docblock states the consequence but nothing enforces it. The cap is about four lines and needs no new dependency; real revocation is the expensive half and I am not asking for it.

Verified and dropped — seven candidates chased rather than sent:

  • Self-mint escalation. A scoped token's username is unverified attribution, so I expected the bearer to call create_authentication_tokens and have that name resolve to whatever real user later took it. Blocked at :155-160, with a comment naming exactly that.
  • Validation resolving the username to a real account. It does not — sub === 'scoped-operation' routes to buildUserFromScopedToken(tokenVerified) off the embedded payload; only the non-scoped path calls findAndValidateUser.
  • Subject not pinned in jwt.verify for the OPERATION type. Safe: the scoped case returns first, sub !== tokenType throws after, and a component-defined string-role token is caught by the tokenVerified.role check.
  • The legacy module.exports clobber your body warns about — a missing entry would leave the new SQL gate undefined at runtime and silently non-functional. verifyOperationsAllowlist is present at operation_authorization.ts:409 and called at :599, ahead of the ambient-privilege checks.
  • The syntheticRoleName memo-key flag from @gemini-code-assist. Already handled, and past what was asked: the key is a sha256 of canonicalized JSON with object keys sorted (impersonation.ts:24-41), so structurally-identical permission sets share a slot. The docblock is also right that key-order variance was never an aliasing bug, only cache pressure.
  • A scoped token's MQTT last will republishing after expiry. Closed, and fails closed — isWillFromExpiredScopedToken treats a scoped will with no authExpiresAt as expired, the replay path refuses to rehydrate the principal by name, and _expandedOperations is stripped so it cannot fail to round-trip.
  • MAX_SCOPED_TOKEN_LENGTH checked after signing. Deliberate — it measures the real signed token, which is what the Authorization header carries.

The best thing in this PR is incidental to it: chooseOperation never called verifyPerms for sql, so an operations allowlist was entirely unenforced through that door — a read_only role could reach unlisted operations via SQL. Routing both callers through one exported helper is the right shape.

Coverage, so the approve is honest: I read the mint path, the validation/subject routing, the self-mint guard, the SQL gate and its module.exports dependency, the synthetic-role memo key, and the durable-subscription/last-will path. I did not read closely the three components/mcp/* files, resources/Resource.ts, serverHandlers.js / serverUtilities.ts, or permissionsTranslator.js — further from the escalation surface, but that is where a second finding would be if one exists.

— DAIvid (Claude Opus 5)

Comment on lines +282 to +286
if (operationToken.length > MAX_SCOPED_TOKEN_LENGTH) {
throw new ClientError(
`the minted token exceeds ${MAX_SCOPED_TOKEN_LENGTH} bytes and would not fit in an Authorization header; reduce the role permission size`
);
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Significant — expires_in is unbounded on a credential with no revocation path.

The schema at :140 accepts any string or number and it flows straight into jwt.sign here, so expires_in: '3650d' mints a ten-year token. The docblock above already states the consequence — “no refresh token is issued and no user record is touched, so the token is irrevocable until expiry — size expires_in accordingly” — but that is guidance, not a limit.

Trigger: any super_user mint, or a mistyped value, with a large expires_in. There is no per-token kill switch, no stored record to delete and no inventory to enumerate, so the only remedy is rotating the JWT RSA keypair — which invalidates every token for every user on the cluster.

In plain terms: if one of these tokens leaks, you cannot cancel just that token. The choice is waiting for it to expire, possibly years, or signing everyone out of the whole cluster at once.

To be clear, this is not privilege escalation — minting is super_user-gated and a super_user could do worse directly. It is blast radius, and the kind of invariant worth enforcing rather than documenting.

A lifetime cap is the cheap 90%; real revocation needs a token store and is a much bigger change. jwt.sign has already resolved the duration, so no ms parsing and no new import are needed (JwtPayload is imported at :1):

Suggested change
if (operationToken.length > MAX_SCOPED_TOKEN_LENGTH) {
throw new ClientError(
`the minted token exceeds ${MAX_SCOPED_TOKEN_LENGTH} bytes and would not fit in an Authorization header; reduce the role permission size`
);
}
if (operationToken.length > MAX_SCOPED_TOKEN_LENGTH) {
throw new ClientError(
`the minted token exceeds ${MAX_SCOPED_TOKEN_LENGTH} bytes and would not fit in an Authorization header; reduce the role permission size`
);
}
// A scoped token has no revocation path, so cap the lifetime rather than only documenting it. jwt.sign
// has already resolved `expires_in`, so read the computed claims back instead of parsing the duration.
const { exp, iat } = jwt.decode(operationToken) as JwtPayload;
if (exp && iat && exp - iat > MAX_SCOPED_TOKEN_LIFETIME_SECONDS) {
throw new ClientError(
`scoped token lifetime exceeds the ${MAX_SCOPED_TOKEN_LIFETIME_SECONDS}s maximum; a scoped token cannot be revoked before it expires`
);
}

One caveat, since I have not compiled this: MAX_SCOPED_TOKEN_LIFETIME_SECONDS needs declaring alongside MAX_SCOPED_TOKEN_LENGTH at :253, and a GitHub suggestion has to be contiguous so it could not be included above. Worth hanging it off a config param — hdbTerms.ts:454-455 already has the pattern for AUTHENTICATION_OPERATIONTOKENTIMEOUT / REFRESHTOKENTIMEOUT — so a deployment can tighten it.

— DAIvid (Claude Opus 5)

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.

2 participants