From fd6a55925f98d65129a6e2604640fe7815b5f6b9 Mon Sep 17 00:00:00 2001 From: Dawson Toth Date: Fri, 14 Aug 2026 14:19:31 -0400 Subject: [PATCH 1/4] feat(security): narrow a minted token to a subset of its user's operations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stacked on feat/oidc-trusted-publishing. Explores the per-policy operation scoping Kris asked about — with the constraint that makes it safe, which is the reason it is a separate PR rather than part of #2173. An OIDC trust policy may carry `operations`. The exchanged token then carries that list as a claim, and verifyPerms intersects it with the user's role. One Harper user can back several workflows, each holding a credential narrower than the user itself. It can only ever subtract. Two things make that true, and both are the whole point: 1. The check is the FIRST authorization step in verifyPerms. Both the super_user bypass and the `operations` gate-2 grant return null early, so a narrowing check after either would be bypassable by exactly the identities it most needs to constrain. Tested directly: a super_user token scoped to get_status cannot insert. 2. The scope is never merged into role.permission.operations. That field is not purely narrowing — gate 2 treats an explicit listing of an SU-only operation as a deliberate grant — so merging into it could widen instead of narrow. It travels on the user as `tokenOperations` and is intersected separately. Absent claim means today's behavior exactly, so every existing token and every unscoped policy is unaffected. Operation names are validated at write time against OPERATIONS_ENUM (groups expanded first): a typo would otherwise fail closed at request time, in CI, with nothing to point at. Co-Authored-By: Claude Opus 5 --- json/systemSchema.json | 3 + security/authn/oidc/tokenExchange.ts | 7 +- security/authn/oidc/trustPolicyOperations.ts | 20 +++++ security/authn/oidc/types.ts | 5 ++ security/tokenAuthentication.ts | 18 +++- .../security/authn/oidc/tokenExchange.test.js | 18 ++++ .../authn/oidc/trustPolicyOperations.test.js | 24 ++++++ .../security/tokenOperationScope.test.js | 84 +++++++++++++++++++ utility/operation_authorization.ts | 23 +++++ 9 files changed, 199 insertions(+), 3 deletions(-) create mode 100644 unitTests/security/tokenOperationScope.test.js diff --git a/json/systemSchema.json b/json/systemSchema.json index 812c6ea12d..a3161f100a 100644 --- a/json/systemSchema.json +++ b/json/systemSchema.json @@ -494,6 +494,9 @@ { "attribute": "user" }, + { + "attribute": "operations" + }, { "attribute": "enabled" }, diff --git a/security/authn/oidc/tokenExchange.ts b/security/authn/oidc/tokenExchange.ts index 3aa7fcd9f7..99974608a7 100644 --- a/security/authn/oidc/tokenExchange.ts +++ b/security/authn/oidc/tokenExchange.ts @@ -203,9 +203,14 @@ export async function exchangeOidcToken(req: any) { await recordTokenUse(fingerprint, claims, policy.id); const operationToken = await createOperationToken( - { username: user.username, super_user: user.role?.permission?.super_user === true }, + { + username: user.username, + super_user: user.role?.permission?.super_user === true, + operations: policy.operations, + }, EXCHANGED_TOKEN_LIFETIME_SECONDS ); + if (policy.operations?.length) audit.scoped_operations = policy.operations; logger.info?.(`OIDC exchange: policy '${policy.id}' authenticated '${user.username}' for ${audit.principal}`); auditExchange(req, username, AUTH_AUDIT_STATUS.SUCCESS, audit); diff --git a/security/authn/oidc/trustPolicyOperations.ts b/security/authn/oidc/trustPolicyOperations.ts index ff090a5fea..c33d144cdd 100644 --- a/security/authn/oidc/trustPolicyOperations.ts +++ b/security/authn/oidc/trustPolicyOperations.ts @@ -19,6 +19,7 @@ import { getUsersWithRolesCache } from '../../user.ts'; import { validateClaimConstraintShape } from './claims.ts'; import { normalizeIssuer } from './jwks.ts'; import { profileForIssuer } from './providers/index.ts'; +import { expandOperationsPerms } from '../../../utility/operationPermissions.ts'; import type { OidcTrustPolicy } from './types.ts'; const { HTTP_STATUS_CODES } = hdbErrors; @@ -44,6 +45,21 @@ function validate(validation: any): void { if (validation) throw new ClientError(validation.message); } +/** + * A typo in an operation name would otherwise fail closed at request time, in CI, with nothing to + * point at — so it is caught here, where the reader is the administrator who wrote it. Group names + * are accepted: expandOperationsPerms resolves them, and a name that expands to only itself and is + * not a known operation is the typo we are looking for. + */ +function assertOperationsAreKnown(operations: string[]): void { + const known = new Set(Object.values(terms.OPERATIONS_ENUM)); + for (const name of expandOperationsPerms(operations)) { + if (!known.has(name)) { + throw new ClientError(`operations contains '${name}', which is not a Harper operation`); + } + } +} + function trustTable() { const table = (databases as any).system?.[OIDC_TRUST_TABLE]; if (!table) { @@ -66,6 +82,7 @@ function toRecord(row: any): OidcTrustPolicy & Record { audience: row.audience, claims: row.claims ?? {}, user: row.user, + operations: row.operations ?? null, enabled: row.enabled !== false, description: row.description ?? null, updated_by: row.updated_by ?? null, @@ -112,6 +129,7 @@ export async function addOidcTrust(req: any) { audience: Joi.string().min(1).max(512).required(), claims: Joi.object().min(1).required(), user: Joi.string().min(1).max(512).required(), + operations: Joi.array().items(Joi.string().min(1)).min(1).max(100).unique(), enabled: Joi.boolean(), description: Joi.string().allow('').max(1024), }).unknown(true) @@ -123,6 +141,7 @@ export async function addOidcTrust(req: any) { // generic profile rather than a permissive default. Each throws ClientError naming the problem. const profile = profileForIssuer(issuer); profile.assertAudienceIsSpecific(req.audience); + if (req.operations) assertOperationsAreKnown(req.operations); validateClaimConstraintShape(req.claims); profile.assertPolicyIsSpecific(req.claims); @@ -146,6 +165,7 @@ export async function addOidcTrust(req: any) { audience: req.audience, claims: req.claims, user: req.user, + operations: req.operations ?? null, enabled: req.enabled !== false, description: req.description ?? null, updated_by: req.hdb_user?.username ?? null, diff --git a/security/authn/oidc/types.ts b/security/authn/oidc/types.ts index 450d08b4f1..26891667f4 100644 --- a/security/authn/oidc/types.ts +++ b/security/authn/oidc/types.ts @@ -19,6 +19,11 @@ export interface OidcTrustPolicy { claims: Record; /** The exchanged token authenticates as this user, whose role is the least-privilege boundary. */ user: string; + /** + * Optional narrowing of that boundary: the minted token may perform only these operations, even + * where the role allows more. Never widens — an operation the role forbids stays forbidden. + */ + operations?: string[]; /** Defaults to true; false keeps the policy for reference without honoring it. */ enabled?: boolean; description?: string; diff --git a/security/tokenAuthentication.ts b/security/tokenAuthentication.ts index c724d97d9e..404a586d36 100644 --- a/security/tokenAuthentication.ts +++ b/security/tokenAuthentication.ts @@ -257,12 +257,20 @@ export async function refreshOperationToken(tokenObj: TokenObject): Promise { const keys: JWTRSAKeys = await getJWTRSAKeys(); + const payload: { username: string; super_user: boolean; operations?: string[] } = { + username: user.username, + super_user: user.super_user, + }; + // A narrowing scope, never a grant: verifyPerms intersects it with the user's role. Absent means + // the role governs alone, which is every token minted before this existed. + if (user.operations?.length) payload.operations = user.operations; + return jwt.sign( - { username: user.username, super_user: user.super_user }, + payload, { key: keys.privateKey, passphrase: keys.passphrase } satisfies Secret, { expiresIn, @@ -308,6 +316,12 @@ async function validateToken(token: string, tokenType: string): Promise { throw new Error('Invalid token'); } + // Surfaced on the user rather than merged into role.permission.operations: that field is not + // purely narrowing (verifyPerms gate 2 treats an explicit SU-only listing as a grant), so + // merging a token scope into it could widen rather than narrow. verifyPerms intersects this + // separately, ahead of every bypass. + if (Array.isArray(tokenVerified.operations)) user.tokenOperations = tokenVerified.operations; + return user; } catch (err) { logger.warn(err); diff --git a/unitTests/security/authn/oidc/tokenExchange.test.js b/unitTests/security/authn/oidc/tokenExchange.test.js index 84d21e5007..a32ba8196b 100644 --- a/unitTests/security/authn/oidc/tokenExchange.test.js +++ b/unitTests/security/authn/oidc/tokenExchange.test.js @@ -438,6 +438,24 @@ describe('exchangeOidcToken', () => { }); }); + // End to end: the policy's scope reaches the minted token, so verifyPerms can narrow on it. + it('carries the policy operation scope into the minted token', async () => { + await addPolicy({ operations: ['deploy_component'] }); + const result = await exchangeOidcToken({ operation: 'exchange_oidc_token', token: identityToken() }); + + const payload = JSON.parse(Buffer.from(result.operation_token.split('.')[1], 'base64url').toString('utf8')); + assert.deepStrictEqual(payload.operations, ['deploy_component']); + assert.strictEqual(payload.username, 'ci-deploy'); + }); + + it('omits the claim entirely when the policy does not scope', async () => { + await addPolicy(); + const result = await exchangeOidcToken({ operation: 'exchange_oidc_token', token: identityToken() }); + + const payload = JSON.parse(Buffer.from(result.operation_token.split('.')[1], 'base64url').toString('utf8')); + assert.strictEqual(payload.operations, undefined, 'an unscoped token must look exactly as it did before'); + }); + it('rejects malformed input', async () => { await addPolicy(); for (const token of ['not-a-jwt', 'a.b.c']) { diff --git a/unitTests/security/authn/oidc/trustPolicyOperations.test.js b/unitTests/security/authn/oidc/trustPolicyOperations.test.js index b2e5be36ad..bea08de145 100644 --- a/unitTests/security/authn/oidc/trustPolicyOperations.test.js +++ b/unitTests/security/authn/oidc/trustPolicyOperations.test.js @@ -188,6 +188,30 @@ describe('oidc trustPolicyOperations', () => { assert.strictEqual(installed.mock.rows.get('my-app-prod').user, 'admin'); }); + it('stores an operation scope', async () => { + await addOidcTrust(su('add_oidc_trust', validPolicy({ operations: ['deploy_component'] }))); + assert.deepStrictEqual(installed.mock.rows.get('my-app-prod').operations, ['deploy_component']); + }); + + it('accepts an operation group', async () => { + await addOidcTrust(su('add_oidc_trust', validPolicy({ operations: ['read_only'] }))); + assert.deepStrictEqual(installed.mock.rows.get('my-app-prod').operations, ['read_only']); + }); + + // A typo would otherwise fail closed at request time, in CI, with nothing to point at. + it('rejects an operation name that is not a Harper operation', async () => { + await assert.rejects( + () => addOidcTrust(su('add_oidc_trust', validPolicy({ operations: ['deploy_compnent'] }))), + /not a Harper operation/ + ); + assert.strictEqual(installed.mock.rows.size, 0, 'expected nothing stored'); + }); + + it('leaves operations null when the policy does not scope', async () => { + await addOidcTrust(su('add_oidc_trust', validPolicy())); + assert.strictEqual(installed.mock.rows.get('my-app-prod').operations, null); + }); + it('rejects a malformed id', async () => { for (const id of ['', 'has spaces', 'has/slash', 'x'.repeat(129)]) { await assert.rejects(() => addOidcTrust(su('add_oidc_trust', validPolicy({ id })))); diff --git a/unitTests/security/tokenOperationScope.test.js b/unitTests/security/tokenOperationScope.test.js new file mode 100644 index 0000000000..5cf89c3604 --- /dev/null +++ b/unitTests/security/tokenOperationScope.test.js @@ -0,0 +1,84 @@ +'use strict'; + +// The narrowing contract for a token-scoped operation allowlist: it may only ever subtract from what +// the user's role allows, and it must not be bypassable by any of verifyPerms' early-return paths. + +const assert = require('node:assert'); +const testUtils = require('../testUtils.js'); +testUtils.preTestPrep(); + +const opAuth = require('#src/utility/operation_authorization'); + +// `insertData` is the internal function name for the `insert` operation; verifyPerms resolves the +// api_name via the permission registry, which is what a scope is written against. +const INSERT_FN = 'insertData'; + +function requestAs(permission, tokenOperations) { + const hdb_user = { username: 'ci-deploy', role: { role: 'r', permission } }; + if (tokenOperations !== undefined) hdb_user.tokenOperations = tokenOperations; + return { operation: 'insert', schema: 'data', table: 'dog', hdb_user, records: [] }; +} + +/** verifyPerms returns null when allowed, or a response object describing the denial. */ +function isAllowed(result) { + return result === null || result === undefined; +} + +describe('token-scoped operation narrowing', () => { + it('allows an operation inside the scope', () => { + const result = opAuth.verifyPerms(requestAs({ super_user: true }, ['insert']), INSERT_FN); + assert.ok(isAllowed(result), 'expected the in-scope operation to be permitted'); + }); + + it('denies an operation outside the scope', () => { + const result = opAuth.verifyPerms(requestAs({ super_user: true }, ['get_status']), INSERT_FN); + assert.ok(!isAllowed(result), 'expected the out-of-scope operation to be denied'); + }); + + // The whole point: a super_user returns null early in verifyPerms, so a narrowing check placed + // after that bypass would do nothing for exactly the identity that most needs constraining. + it('constrains a super_user', () => { + assert.ok(!isAllowed(opAuth.verifyPerms(requestAs({ super_user: true }, ['get_status']), INSERT_FN))); + }); + + // Gate 2 treats an explicit SU-only listing in role.permission.operations as a deliberate grant + // and returns null. The scope must still win. + it('constrains an operation the role granted through its own operations allowlist', () => { + const permission = { super_user: false, operations: ['insert'] }; + assert.ok(!isAllowed(opAuth.verifyPerms(requestAs(permission, ['get_status']), INSERT_FN))); + }); + + // Narrowing only: naming an operation the role forbids must not grant it. + it('does not grant an operation the role forbids', () => { + const permission = { super_user: false, operations: ['get_status'] }; + const result = opAuth.verifyPerms(requestAs(permission, ['insert']), INSERT_FN); + assert.ok(!isAllowed(result), 'a scope must never widen the role'); + }); + + it('is inert when the token carries no scope', () => { + assert.ok(isAllowed(opAuth.verifyPerms(requestAs({ super_user: true }), INSERT_FN))); + }); + + // A group name must be expanded rather than compared literally, or naming one would deny + // everything. `read_only` deliberately excludes insert, so it also proves the expansion narrows. + it('expands operation groups', () => { + const withGroup = requestAs({ super_user: true }, ['read_only']); + const result = opAuth.verifyPerms(withGroup, INSERT_FN); + + assert.ok(withGroup.hdb_user._expandedTokenOperations.size > 1, 'expected the group to expand'); + assert.ok(withGroup.hdb_user._expandedTokenOperations.has('search_by_value'), 'expected group members'); + assert.ok(!isAllowed(result), 'read_only must not admit insert'); + }); + + it('memoizes the expansion on the user', () => { + const request = requestAs({ super_user: true }, ['insert']); + opAuth.verifyPerms(request, INSERT_FN); + const first = request.hdb_user._expandedTokenOperations; + opAuth.verifyPerms(request, INSERT_FN); + assert.strictEqual(request.hdb_user._expandedTokenOperations, first, 'expected one expansion'); + }); + + it('denies everything when the scope is empty', () => { + assert.ok(!isAllowed(opAuth.verifyPerms(requestAs({ super_user: true }, []), INSERT_FN))); + }); +}); diff --git a/utility/operation_authorization.ts b/utility/operation_authorization.ts index 8c3f09a8b8..64dc444520 100644 --- a/utility/operation_authorization.ts +++ b/utility/operation_authorization.ts @@ -582,6 +582,29 @@ export function verifyPerms(requestJson: any, operation: any, _options?: any) { const permsResponse = new PermissionResponseObject(); + // Token-scoped narrowing: a minted operation token may carry a subset of what its user's role + // allows, so one credential can be handed out with less authority than the user has (an OIDC + // trust policy uses this to scope a single workflow). + // + // Deliberately the FIRST authorization check in this function. Both the super_user bypass and the + // `operations` gate-2 grant below `return null` early, so a narrowing check placed after either + // would be silently bypassable by exactly the identities it most needs to constrain. + // + // It can only subtract. The scope is never merged into `permission.operations`, because that field + // is not purely narrowing — gate 2 treats an explicit listing of an SU-only operation as a + // deliberate grant, so merging into it could widen instead. + const tokenOperations = requestJson.hdb_user?.tokenOperations; + if (tokenOperations !== undefined) { + const scopedOps = + requestJson.hdb_user._expandedTokenOperations ?? + (requestJson.hdb_user._expandedTokenOperations = expandOperationsPerms(tokenOperations)); + const opApiName = requiredPermissions.get(op)?.api_name ?? op; + if (!scopedOps.has(opApiName)) { + harperLogger.info(`Operation '${opApiName}' is outside the scope of the presented token`); + return permsResponse.handleUnauthorizedItem(HDB_ERROR_MSGS.OP_NOT_IN_OPERATIONS(opApiName)); + } + } + if ( commonUtils.isEmptyOrZeroLength(requestJson.hdb_user?.role) || commonUtils.isEmptyOrZeroLength(requestJson.hdb_user?.role?.permission) From 47d222c43e50fb5b1a93bf7827db6672d232e957 Mon Sep 17 00:00:00 2001 From: Dawson Toth Date: Fri, 14 Aug 2026 14:41:58 -0400 Subject: [PATCH 2/4] fix(security): carry an empty operation scope instead of dropping it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review feedback from gemini-code-assist on #2174; all four points were correct. The one that mattered: `createOperationToken` gated the claim on `user.operations?.length`, so an EMPTY scope — meaning "no operations" — was omitted from the payload entirely. The minted token then looked unscoped, verifyPerms skipped narrowing, and the holder got everything its role allowed. A security control failing open, and in the one direction that matters. add_oidc_trust rejects an empty array (Joi .min(1)), so this is not reachable through the documented API. It is reachable by a row arriving through replication from a peer, which is the same path matchTrustPolicyClaims already backstops against — a control must fail closed regardless of how the input got there. Also: - verifyPerms used `!== undefined` where an unscoped policy stores `operations: null`; expanding null would throw rather than fall through to the role. Now `!= null`, which is also the repo's documented idiom (.gemini/styleguide.md). - Operation-name validation delegates to validateOperations instead of a local OPERATIONS_ENUM check. That helper also accepts operations registered at runtime via server.registerOperation, which the local check would have rejected — so a policy could not scope to a dynamically registered op. Three tests added, one per failure mode. The empty-scope test is a round trip through createOperationToken rather than a verifyPerms unit check: the existing suite passed `[]` straight to verifyPerms and denied correctly, which is exactly why it missed a mint path that never emitted the claim. Co-Authored-By: Claude Opus 5 --- security/authn/oidc/trustPolicyOperations.ts | 18 +++---- security/tokenAuthentication.ts | 7 ++- .../authn/oidc/trustPolicyOperations.test.js | 15 ++++++ .../security/tokenOperationScope.test.js | 5 ++ .../tokenOperationScopeMinting.test.js | 51 +++++++++++++++++++ utility/operation_authorization.ts | 4 +- 6 files changed, 88 insertions(+), 12 deletions(-) create mode 100644 unitTests/security/tokenOperationScopeMinting.test.js diff --git a/security/authn/oidc/trustPolicyOperations.ts b/security/authn/oidc/trustPolicyOperations.ts index c33d144cdd..83e6b593a4 100644 --- a/security/authn/oidc/trustPolicyOperations.ts +++ b/security/authn/oidc/trustPolicyOperations.ts @@ -19,7 +19,7 @@ import { getUsersWithRolesCache } from '../../user.ts'; import { validateClaimConstraintShape } from './claims.ts'; import { normalizeIssuer } from './jwks.ts'; import { profileForIssuer } from './providers/index.ts'; -import { expandOperationsPerms } from '../../../utility/operationPermissions.ts'; +import { validateOperations } from '../../../utility/operationPermissions.ts'; import type { OidcTrustPolicy } from './types.ts'; const { HTTP_STATUS_CODES } = hdbErrors; @@ -46,17 +46,15 @@ function validate(validation: any): void { } /** - * A typo in an operation name would otherwise fail closed at request time, in CI, with nothing to - * point at — so it is caught here, where the reader is the administrator who wrote it. Group names - * are accepted: expandOperationsPerms resolves them, and a name that expands to only itself and is - * not a known operation is the typo we are looking for. + * A typo would otherwise fail closed at request time, in CI, with nothing to point at — so it is + * caught here, where the reader is the administrator who wrote it. Delegates to the same helper + * add_role/alter_role use, which accepts group names and operations registered at runtime via + * server.registerOperation; a local OPERATIONS_ENUM check would reject those. */ function assertOperationsAreKnown(operations: string[]): void { - const known = new Set(Object.values(terms.OPERATIONS_ENUM)); - for (const name of expandOperationsPerms(operations)) { - if (!known.has(name)) { - throw new ClientError(`operations contains '${name}', which is not a Harper operation`); - } + const invalidOperation = validateOperations(operations); + if (invalidOperation != null) { + throw new ClientError(`operations contains '${invalidOperation}', which is not a Harper operation`); } } diff --git a/security/tokenAuthentication.ts b/security/tokenAuthentication.ts index 404a586d36..eba13def6f 100644 --- a/security/tokenAuthentication.ts +++ b/security/tokenAuthentication.ts @@ -267,7 +267,12 @@ export async function createOperationToken( }; // A narrowing scope, never a grant: verifyPerms intersects it with the user's role. Absent means // the role governs alone, which is every token minted before this existed. - if (user.operations?.length) payload.operations = user.operations; + // + // `!= null` rather than a truthiness check on purpose: an EMPTY scope means "no operations", and + // a length check would drop it from the payload, leaving the token unscoped — a security control + // failing open. add_oidc_trust rejects an empty array, but a row can reach the table by + // replication from a peer, so this must not depend on that. + if (user.operations != null) payload.operations = user.operations; return jwt.sign( payload, diff --git a/unitTests/security/authn/oidc/trustPolicyOperations.test.js b/unitTests/security/authn/oidc/trustPolicyOperations.test.js index bea08de145..5615a2f8cf 100644 --- a/unitTests/security/authn/oidc/trustPolicyOperations.test.js +++ b/unitTests/security/authn/oidc/trustPolicyOperations.test.js @@ -17,6 +17,7 @@ const { const { databases } = require('#src/resources/databases'); const { setUsersWithRolesCache } = require('#src/security/user'); const terms = require('#src/utility/hdbTerms'); +const opAuth = require('#src/utility/operation_authorization'); const OIDC_TRUST_TABLE = terms.SYSTEM_TABLE_NAMES.OIDC_TRUST_TABLE_NAME; const ISSUER = 'https://token.actions.githubusercontent.com'; @@ -207,6 +208,20 @@ describe('oidc trustPolicyOperations', () => { assert.strictEqual(installed.mock.rows.size, 0, 'expected nothing stored'); }); + // Operations registered at runtime via server.registerOperation are grantable in a role's + // allowlist, so a policy must be able to scope to them too — this is why validation delegates + // to validateOperations rather than checking OPERATIONS_ENUM locally. + it('accepts a dynamically registered operation', async () => { + const dynamicOp = 'test_dynamic_scope_op'; + opAuth.registerOperationPermission(dynamicOp, { requiresSu: true }); + try { + await addOidcTrust(su('add_oidc_trust', validPolicy({ operations: [dynamicOp] }))); + assert.deepStrictEqual(installed.mock.rows.get('my-app-prod').operations, [dynamicOp]); + } finally { + opAuth.unregisterOperationPermission(dynamicOp); + } + }); + it('leaves operations null when the policy does not scope', async () => { await addOidcTrust(su('add_oidc_trust', validPolicy())); assert.strictEqual(installed.mock.rows.get('my-app-prod').operations, null); diff --git a/unitTests/security/tokenOperationScope.test.js b/unitTests/security/tokenOperationScope.test.js index 5cf89c3604..3ea3154662 100644 --- a/unitTests/security/tokenOperationScope.test.js +++ b/unitTests/security/tokenOperationScope.test.js @@ -78,6 +78,11 @@ describe('token-scoped operation narrowing', () => { assert.strictEqual(request.hdb_user._expandedTokenOperations, first, 'expected one expansion'); }); + // A null scope is what an unscoped policy stores; it must fall through to the role, not throw. + it('falls through to the role when the scope is null', () => { + assert.ok(isAllowed(opAuth.verifyPerms(requestAs({ super_user: true }, null), INSERT_FN))); + }); + it('denies everything when the scope is empty', () => { assert.ok(!isAllowed(opAuth.verifyPerms(requestAs({ super_user: true }, []), INSERT_FN))); }); diff --git a/unitTests/security/tokenOperationScopeMinting.test.js b/unitTests/security/tokenOperationScopeMinting.test.js new file mode 100644 index 0000000000..f61ab70870 --- /dev/null +++ b/unitTests/security/tokenOperationScopeMinting.test.js @@ -0,0 +1,51 @@ +'use strict'; + +// The mint side of the narrowing contract. Kept separate from tokenOperationScope.test.js because +// this needs real JWT signing keys, and the point of these cases is the *round trip*: a scope that +// survives verifyPerms in isolation is worthless if createOperationToken drops it on the way out. + +const assert = require('node:assert'); +const testUtils = require('../testUtils.js'); +testUtils.preTestPrep(); + +const { createOperationToken, clearJWTRSAKeysCache } = require('#src/security/tokenAuthentication'); + +function payloadOf(token) { + return JSON.parse(Buffer.from(token.split('.')[1], 'base64url').toString('utf8')); +} + +describe('operation scope on a minted token', () => { + const user = { username: 'ci-deploy', super_user: false }; + let removeJwtKeys; + + before(() => { + // The keys land in a directory another suite asserts is empty, so they must come back out — + // see testUtils.installTestJwtKeys. + removeJwtKeys = testUtils.installTestJwtKeys(); + clearJWTRSAKeysCache(); + }); + + after(() => { + removeJwtKeys(); + clearJWTRSAKeysCache(); + }); + + it('carries a scope', async () => { + const token = await createOperationToken({ ...user, operations: ['deploy_component'] }, 3600); + assert.deepStrictEqual(payloadOf(token).operations, ['deploy_component']); + }); + + // The bug this exists to prevent: an empty scope means "no operations". A truthiness check on + // length drops the claim, which leaves the token UNSCOPED — a security control failing open. + it('carries an empty scope rather than dropping it', async () => { + const token = await createOperationToken({ ...user, operations: [] }, 3600); + assert.deepStrictEqual(payloadOf(token).operations, [], 'an empty scope must not become an absent scope'); + }); + + it('omits the claim when there is no scope', async () => { + for (const operations of [undefined, null]) { + const token = await createOperationToken({ ...user, operations }, 3600); + assert.strictEqual(payloadOf(token).operations, undefined); + } + }); +}); diff --git a/utility/operation_authorization.ts b/utility/operation_authorization.ts index 64dc444520..c6dba50ab4 100644 --- a/utility/operation_authorization.ts +++ b/utility/operation_authorization.ts @@ -593,8 +593,10 @@ export function verifyPerms(requestJson: any, operation: any, _options?: any) { // It can only subtract. The scope is never merged into `permission.operations`, because that field // is not purely narrowing — gate 2 treats an explicit listing of an SU-only operation as a // deliberate grant, so merging into it could widen instead. + // `!= null`, not `!== undefined`: an unscoped policy stores `operations: null`, and expanding a + // null would throw rather than fall through to the role. const tokenOperations = requestJson.hdb_user?.tokenOperations; - if (tokenOperations !== undefined) { + if (tokenOperations != null) { const scopedOps = requestJson.hdb_user._expandedTokenOperations ?? (requestJson.hdb_user._expandedTokenOperations = expandOperationsPerms(tokenOperations)); From 2e4f9c5d956d47ffdeeb30dd5aa6c412db1b5b9d Mon Sep 17 00:00:00 2001 From: Dawson Toth Date: Fri, 14 Aug 2026 15:00:46 -0400 Subject: [PATCH 3/4] fix(security): apply the token operation scope to the SQL path too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Caught in review by claude[bot] on #2174, and it falsified the PR's central claim. chooseOperation dispatches `operation === 'sql'` to verifyPermsAST, in a branch mutually exclusive with the verifyPerms call — and the narrowing gate lived only in verifyPerms. So a token scoped to, say, `operations: ['get_status']` could send {"operation":"sql","sql":"DELETE FROM ..."} and run arbitrary SQL against whatever its role could reach. verifyPermsAST also returns null unconditionally for a super_user, so the identity most needing the constraint was the least constrained. The gate is now a shared tokenScopeDenial() called first by BOTH entry points, rather than a second copy in verifyPermsAST. The lesson of the bug is that a check living inside one of two mutually exclusive branches is one refactor away from being skipped, so the comment enumerates all three early-return paths that bypass it if it ever moves. On the AST path the scope is checked against `sql` — the operation the caller actually invoked — because verifyPermsAST's `operation` parameter is the statement variant (select/insert/...), not the API name. It runs ahead of AST parsing as well as the super_user bypass: an out-of-scope request should not get its SQL parsed at all. Five tests on the SQL path. Verified they fail without the fix (2 failing) and pass with it, so they pin the hole rather than describing it. Co-Authored-By: Claude Opus 5 --- .../security/tokenOperationScope.test.js | 43 ++++++++++++ utility/operation_authorization.ts | 67 ++++++++++++------- 2 files changed, 86 insertions(+), 24 deletions(-) diff --git a/unitTests/security/tokenOperationScope.test.js b/unitTests/security/tokenOperationScope.test.js index 3ea3154662..7830e5d7eb 100644 --- a/unitTests/security/tokenOperationScope.test.js +++ b/unitTests/security/tokenOperationScope.test.js @@ -8,6 +8,7 @@ const testUtils = require('../testUtils.js'); testUtils.preTestPrep(); const opAuth = require('#src/utility/operation_authorization'); +const sql = require('#src/sqlTranslator/index'); // `insertData` is the internal function name for the `insert` operation; verifyPerms resolves the // api_name via the permission registry, which is what a scope is written against. @@ -87,3 +88,45 @@ describe('token-scoped operation narrowing', () => { assert.ok(!isAllowed(opAuth.verifyPerms(requestAs({ super_user: true }, []), INSERT_FN))); }); }); + +// `sql` dispatches to verifyPermsAST, in a branch mutually exclusive with the verifyPerms call in +// chooseOperation. A gate in only one of them lets a token scoped to e.g. get_status run arbitrary +// SQL against whatever its role can reach — which would falsify the whole "can only subtract" claim. +describe('token-scoped narrowing on the SQL path', () => { + function userWithScope(permission, tokenOperations) { + const user = { username: 'ci-deploy', role: { role: 'r', permission } }; + if (tokenOperations !== undefined) user.tokenOperations = tokenOperations; + return user; + } + + function checkSql(statement, user) { + const parsed = sql.convertSQLToAST(statement); + return sql.checkASTPermissions({ operation: 'sql', sql: statement, hdb_user: user }, parsed); + } + + it('denies SQL when the scope does not include it', () => { + const denial = checkSql('SELECT * FROM data.dog', userWithScope({ super_user: true }, ['get_status'])); + assert.ok(denial, 'a token scoped away from sql must not be able to run SQL'); + }); + + // The dangerous case: verifyPermsAST returns null unconditionally for a super_user. + it('denies a super_user whose scope excludes SQL', () => { + const denial = checkSql('DELETE FROM data.dog', userWithScope({ super_user: true }, ['deploy_component'])); + assert.ok(denial, 'the super_user bypass must not outrank the token scope'); + }); + + it('allows SQL when the scope includes it', () => { + const denial = checkSql('SELECT * FROM data.dog', userWithScope({ super_user: true }, ['sql'])); + assert.strictEqual(denial, null, 'an in-scope sql statement should reach the normal perms checks'); + }); + + it('allows SQL through a group that contains it', () => { + const denial = checkSql('SELECT * FROM data.dog', userWithScope({ super_user: true }, ['read_only'])); + assert.strictEqual(denial, null); + }); + + it('is inert for a token with no scope', () => { + const denial = checkSql('SELECT * FROM data.dog', userWithScope({ super_user: true })); + assert.strictEqual(denial, null); + }); +}); diff --git a/utility/operation_authorization.ts b/utility/operation_authorization.ts index c6dba50ab4..bb7d48598e 100644 --- a/utility/operation_authorization.ts +++ b/utility/operation_authorization.ts @@ -442,6 +442,40 @@ module.exports = { * @param operation - The operation specified in the call. * @returns {null | PermissionResponseObject} - null if permissions match, errors returned in the PermissionResponseObject */ +/** + * Token-scoped narrowing: a minted operation token may carry a subset of what its user's role allows, + * so one credential can be handed out with less authority than the user has (an OIDC trust policy + * uses this to scope a single workflow). Returns a denial, or undefined when the scope permits — or + * when there is no scope, which is every token minted before this existed. + * + * Shared by verifyPerms AND verifyPermsAST, and called first in both. Three ways this gets bypassed + * if it moves: + * + * 1. `sql` dispatches to verifyPermsAST, in a branch mutually exclusive with the verifyPerms call + * (server/serverHelpers/serverUtilities.ts). A gate in only one of them means a token scoped to + * `get_status` can still run arbitrary SQL against whatever its role can reach. + * 2. Both functions `return null` early for a super_user — the identity that most needs constraining. + * 3. verifyPerms' `operations` gate 2 also returns null, treating an explicit listing of an SU-only + * operation as a deliberate grant. + * + * It can only subtract. The scope is deliberately NOT merged into `permission.operations`, because + * that field is not purely narrowing (see gate 2) — merging into it could widen instead. + */ +function tokenScopeDenial(userObject: any, opApiName: string) { + // `!= null`, not `!== undefined`: an unscoped policy stores `operations: null`, and expanding a + // null would throw rather than fall through to the role. + const tokenOperations = userObject?.tokenOperations; + if (tokenOperations == null) return undefined; + + const scopedOps = + userObject._expandedTokenOperations ?? + (userObject._expandedTokenOperations = expandOperationsPerms(tokenOperations)); + if (scopedOps.has(opApiName)) return undefined; + + harperLogger.info(`Operation '${opApiName}' is outside the scope of the presented token`); + return new PermissionResponseObject().handleUnauthorizedItem(HDB_ERROR_MSGS.OP_NOT_IN_OPERATIONS(opApiName)); +} + export function verifyPermsAST(ast, userObject, operation) { //TODO - update these validation checks to use validate.js if (commonUtils.isEmptyOrZeroLength(ast)) { @@ -456,6 +490,13 @@ export function verifyPermsAST(ast, userObject, operation) { harperLogger.info('verify_perms_ast has a null operation parameter'); throw handleHDBError(new Error()); } + + // `operation` here is the SQL statement variant (select/insert/...), not the API operation, so the + // scope is checked against `sql` — the operation the caller actually invoked. Ahead of the AST + // parsing below as well as the super_user bypass: a denied scope should not parse attacker SQL. + const scopeDenial = tokenScopeDenial(userObject, terms.OPERATIONS_ENUM.SQL); + if (scopeDenial) return scopeDenial; + try { const bucketModule = require('../sqlTranslator/sql_statement_bucket'); const bucket = bucketModule.default || bucketModule; @@ -582,30 +623,8 @@ export function verifyPerms(requestJson: any, operation: any, _options?: any) { const permsResponse = new PermissionResponseObject(); - // Token-scoped narrowing: a minted operation token may carry a subset of what its user's role - // allows, so one credential can be handed out with less authority than the user has (an OIDC - // trust policy uses this to scope a single workflow). - // - // Deliberately the FIRST authorization check in this function. Both the super_user bypass and the - // `operations` gate-2 grant below `return null` early, so a narrowing check placed after either - // would be silently bypassable by exactly the identities it most needs to constrain. - // - // It can only subtract. The scope is never merged into `permission.operations`, because that field - // is not purely narrowing — gate 2 treats an explicit listing of an SU-only operation as a - // deliberate grant, so merging into it could widen instead. - // `!= null`, not `!== undefined`: an unscoped policy stores `operations: null`, and expanding a - // null would throw rather than fall through to the role. - const tokenOperations = requestJson.hdb_user?.tokenOperations; - if (tokenOperations != null) { - const scopedOps = - requestJson.hdb_user._expandedTokenOperations ?? - (requestJson.hdb_user._expandedTokenOperations = expandOperationsPerms(tokenOperations)); - const opApiName = requiredPermissions.get(op)?.api_name ?? op; - if (!scopedOps.has(opApiName)) { - harperLogger.info(`Operation '${opApiName}' is outside the scope of the presented token`); - return permsResponse.handleUnauthorizedItem(HDB_ERROR_MSGS.OP_NOT_IN_OPERATIONS(opApiName)); - } - } + const scopeDenial = tokenScopeDenial(requestJson.hdb_user, requiredPermissions.get(op)?.api_name ?? op); + if (scopeDenial) return scopeDenial; if ( commonUtils.isEmptyOrZeroLength(requestJson.hdb_user?.role) || From 6038dadcb93d2af68c07b71777674e569371aa86 Mon Sep 17 00:00:00 2001 From: Dawson Toth Date: Mon, 17 Aug 2026 15:40:56 -0400 Subject: [PATCH 4/4] fix(security): carry the token operation scope across credential minting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A cross-model review (codex + gemini) found a fourth bypass of the token operation scope, the same authz-escape class as the earlier SQL-path hole: the scope is only enforced inside verifyPerms/verifyPermsAST, but three operations PRODUCE a new credential or principal and dropped it. - create_authentication_tokens (the headline path): it is in NO_AUTH_OPERATIONS, so verifyPerms — and the scope gate inside it — never runs. A token scoped to e.g. deploy_component could call it with no username/password and receive fresh, UNSCOPED operation + refresh tokens for its own user: full-role escalation. - refresh_operation_token: dropped the operations claim when re-signing. - impersonation: enforceDowngrade bounds the impersonated role's perms but shed the token scope, so a scoped super_user token could drop the scope by impersonating. Fix: the scope carries forward on all three surfaces, so a scoped credential can only ever mint/become an equally-scoped one — the same "can only subtract" invariant, extended to the paths that leave verifyPerms. createTokens and refreshOperationToken copy the caller's scope into the minted payload; applyImpersonation copies it onto the new principal. Also moved the api_name resolution into tokenScopeDenial so the unscoped default path (every non-scoped request) does no registry lookup before its `== null` return, and updated the helper's comment to enumerate this fourth bypass class alongside the three in-function ones. Tests: createTokens carries the scope into both minted tokens and stays unscoped for an unscoped caller (reusing the existing mocked suite); refresh_operation_token preserves the scope through a real validate->decode->sign round trip; impersonation carries it onto the impersonated user. 253 passing across the affected suites. Adjudication of the rest of that review is in the PR description. Two flagged "blockers" were false positives (an undefined `op` — `op` is declared and the scope tests exercise that exact line; and a non-existent alter_oidc_trust operation). Co-Authored-By: Claude Opus 4.8 --- security/impersonation.ts | 6 ++ security/tokenAuthentication.ts | 18 +++- unitTests/security/impersonation.test.js | 19 ++++ .../security/tokenAuthentication.test.js | 29 ++++++ unitTests/security/tokenScopeRefresh.test.js | 88 +++++++++++++++++++ utility/operation_authorization.ts | 17 +++- 6 files changed, 172 insertions(+), 5 deletions(-) create mode 100644 unitTests/security/tokenScopeRefresh.test.js diff --git a/security/impersonation.ts b/security/impersonation.ts index 4d2ea3e14e..de55d31785 100644 --- a/security/impersonation.ts +++ b/security/impersonation.ts @@ -38,6 +38,12 @@ export async function applyImpersonation(authenticatedUser: User, payload: Imper // Enforce downgrade: never allow escalation enforceDowngrade(impersonatedUser); + // A token's operation scope (#2174) constrains the credential regardless of which principal it + // acts as, so it survives impersonation. enforceDowngrade only bounds the impersonated role's + // permissions; without carrying the scope, a scoped super_user token would shed it by impersonating. + const inheritedScope = (authenticatedUser as any).tokenOperations; + if (Array.isArray(inheritedScope)) (impersonatedUser as any).tokenOperations = inheritedScope; + // Tag for audit trail impersonatedUser._impersonated = true; impersonatedUser._impersonatedBy = authenticatedUser.username; diff --git a/security/tokenAuthentication.ts b/security/tokenAuthentication.ts index eba13def6f..7556ca82d3 100644 --- a/security/tokenAuthentication.ts +++ b/security/tokenAuthentication.ts @@ -162,9 +162,18 @@ export async function createTokens(authObj: AuthObject): Promise { username: string; super_user: boolean; role?: any; + operations?: string[]; } = { username: authObj.username, super_user: superUser }; if (authObj.role) payload.role = authObj.role; + // A scoped credential can only mint an equally-scoped one (#2174). create_authentication_tokens is + // NO_AUTH, so verifyPerms — and the token-scope gate inside it — never runs here; if the caller + // authenticated with a scoped operation token, the operation AND refresh tokens it mints inherit + // that scope. Without this, a token scoped to e.g. deploy_component escalates to unscoped, full-role + // credentials simply by calling create_authentication_tokens with no username/password. + const inheritedScope = (authObj.hdb_user as any)?.tokenOperations; + if (Array.isArray(inheritedScope)) payload.operations = inheritedScope; + const keys: JWTRSAKeys = await getJWTRSAKeys(); if (authObj.purpose === 'login') { @@ -233,8 +242,15 @@ export async function refreshOperationToken(tokenObj: TokenObject): Promise { assert.strictEqual(modeC.role.id, '_impersonated_ctx_user'); }); }); + + describe('token operation scope survives impersonation (#2174)', () => { + // A scoped operation token constrains the credential regardless of which principal it acts as, + // so impersonating must not shed the scope — otherwise a scoped super_user token escalates by + // impersonating a broader (still-downgraded) role. + const INLINE_ROLE = { role: { permission: { super_user: false, dev: { tables: {} } } } }; + + it('carries tokenOperations onto the impersonated user', async () => { + const su = makeSuperUser(); + su.tokenOperations = ['deploy_component']; + const impersonated = await applyImpersonation(su, INLINE_ROLE); + assert.deepStrictEqual(impersonated.tokenOperations, ['deploy_component']); + }); + + it('adds no scope when the authenticating token was unscoped', async () => { + const impersonated = await applyImpersonation(makeSuperUser(), INLINE_ROLE); + assert.strictEqual(impersonated.tokenOperations, undefined); + }); + }); }); diff --git a/unitTests/security/tokenAuthentication.test.js b/unitTests/security/tokenAuthentication.test.js index 9cafb18a9a..5bce6755e7 100644 --- a/unitTests/security/tokenAuthentication.test.js +++ b/unitTests/security/tokenAuthentication.test.js @@ -441,6 +441,35 @@ describe('test createTokens', () => { rw_get_tokens(); }); + // #2174: create_authentication_tokens is NO_AUTH, so verifyPerms — and the token-scope gate + // inside it — never runs here. A caller authenticated with a scoped operation token must not be + // able to mint UNSCOPED credentials; the minted operation and refresh tokens inherit the scope. + it('carries an inherited token scope into the minted operation and refresh tokens', async () => { + let rw = token_auth.__set__( + 'getJWTRSAKeys', + async () => new JWTRSAKeys(PUBLIC_KEY_VALUE, PRIVATE_KEY_VALUE, PASSPHRASE_VALUE) + ); + // No username/password: the caller is the already-authenticated bearer, whose hdb_user carries + // the scope — exactly the shape a scoped OIDC token presents to create_authentication_tokens. + let result = await token_auth.createTokens({ + hdb_user: { username: 'HDB_USER', tokenOperations: ['deploy_component'] }, + }); + assert.deepStrictEqual(jwt.decode(result.operation_token).operations, ['deploy_component']); + assert.deepStrictEqual(jwt.decode(result.refresh_token).operations, ['deploy_component']); + rw(); + }); + + it('mints unscoped credentials when the caller is unscoped', async () => { + let rw = token_auth.__set__( + 'getJWTRSAKeys', + async () => new JWTRSAKeys(PUBLIC_KEY_VALUE, PRIVATE_KEY_VALUE, PASSPHRASE_VALUE) + ); + let result = await token_auth.createTokens({ username: 'HDB_USER', password: 'pass' }); + assert.strictEqual(jwt.decode(result.operation_token).operations, undefined); + assert.strictEqual(jwt.decode(result.refresh_token).operations, undefined); + rw(); + }); + it('test update failed', async () => { update_stub.callsFake(async (_update_object) => { throw Error('update failed'); diff --git a/unitTests/security/tokenScopeRefresh.test.js b/unitTests/security/tokenScopeRefresh.test.js new file mode 100644 index 0000000000..548e88b983 --- /dev/null +++ b/unitTests/security/tokenScopeRefresh.test.js @@ -0,0 +1,88 @@ +'use strict'; + +// A scoped credential must only ever produce an equally-scoped one (#2174). refresh_operation_token +// mints a fresh operation token from a refresh token; if it dropped the `operations` claim, a scoped +// refresh credential would refresh back to the full role. This drives the real validate→decode→sign +// path with the test signing keys rather than asserting the payload in isolation. + +const assert = require('node:assert'); +const testUtils = require('../testUtils.js'); +testUtils.preTestPrep(); + +const fs = require('node:fs'); +const path = require('node:path'); +const jwt = require('jsonwebtoken'); +const { refreshOperationToken, clearJWTRSAKeysCache } = require('#src/security/tokenAuthentication'); +const password = require('#src/utility/password'); +const { setUsersWithRolesCache } = require('#src/security/user'); +const env = require('#src/utility/environment/environmentManager'); +const terms = require('#src/utility/hdbTerms'); + +const USERNAME = 'ci-deploy'; + +function payloadOf(token) { + return JSON.parse(Buffer.from(token.split('.')[1], 'base64url').toString('utf8')); +} + +describe('refresh_operation_token operation scope', () => { + let removeJwtKeys; + let privateKey; + let passphrase; + + before(async () => { + removeJwtKeys = testUtils.installTestJwtKeys(); + clearJWTRSAKeysCache(); + // Sign the refresh token with the exact keys refreshOperationToken will verify against. + const keysDir = path.join(env.getHdbBasePath(), terms.LICENSE_KEY_DIR_NAME); + privateKey = fs.readFileSync(path.join(keysDir, terms.JWT_ENUM.JWT_PRIVATE_KEY_NAME)); + passphrase = fs.readFileSync(path.join(keysDir, terms.JWT_ENUM.JWT_PASSPHRASE_NAME), 'utf8'); + }); + + after(() => { + removeJwtKeys(); + clearJWTRSAKeysCache(); + }); + + // Mints a `refresh`-subject token and seeds the users cache so validateRefreshToken accepts it + // (it matches the token against the SHA-256 hash stored on the user). + async function seedRefreshToken(claims) { + const refreshToken = jwt.sign( + claims, + { key: privateKey, passphrase }, + { + algorithm: 'RS256', + subject: 'refresh', + expiresIn: '30d', + } + ); + const users = new Map([ + [ + USERNAME, + { + username: USERNAME, + active: true, + refresh_token: password.hash(refreshToken, password.HASH_FUNCTION.SHA256), + role: { role: 'deployer', permission: { super_user: false } }, + }, + ], + ]); + await setUsersWithRolesCache(users); + return refreshToken; + } + + it('carries the scope from the refresh token into the minted operation token', async () => { + const refreshToken = await seedRefreshToken({ + username: USERNAME, + super_user: false, + operations: ['deploy_component'], + }); + const { operation_token } = await refreshOperationToken({ refresh_token: refreshToken }); + assert.deepStrictEqual(payloadOf(operation_token).operations, ['deploy_component']); + }); + + it('mints an unscoped operation token from an unscoped refresh token', async () => { + const refreshToken = await seedRefreshToken({ username: USERNAME, super_user: false }); + const { operation_token } = await refreshOperationToken({ refresh_token: refreshToken }); + assert.strictEqual(payloadOf(operation_token).operations, undefined); + }); +}); diff --git a/utility/operation_authorization.ts b/utility/operation_authorization.ts index bb7d48598e..cd217309ec 100644 --- a/utility/operation_authorization.ts +++ b/utility/operation_authorization.ts @@ -448,8 +448,8 @@ module.exports = { * uses this to scope a single workflow). Returns a denial, or undefined when the scope permits — or * when there is no scope, which is every token minted before this existed. * - * Shared by verifyPerms AND verifyPermsAST, and called first in both. Three ways this gets bypassed - * if it moves: + * Shared by verifyPerms AND verifyPermsAST, and called first in both. Four ways this gets bypassed + * if it moves or is incomplete: * * 1. `sql` dispatches to verifyPermsAST, in a branch mutually exclusive with the verifyPerms call * (server/serverHelpers/serverUtilities.ts). A gate in only one of them means a token scoped to @@ -457,11 +457,19 @@ module.exports = { * 2. Both functions `return null` early for a super_user — the identity that most needs constraining. * 3. verifyPerms' `operations` gate 2 also returns null, treating an explicit listing of an SU-only * operation as a deliberate grant. + * 4. Operations that never reach verifyPerms at all: create_authentication_tokens (NO_AUTH), + * refresh_operation_token, and impersonation each PRODUCE a new credential or principal, so the + * scope has to be carried forward there too or a scoped token mints an unscoped one. That carry- + * forward lives in tokenAuthentication.ts and impersonation.ts, not here. * * It can only subtract. The scope is deliberately NOT merged into `permission.operations`, because * that field is not purely narrowing (see gate 2) — merging into it could widen instead. + * + * `op` is the raw operation (a handler function name, or the literal `sql`); its snake_case api_name + * is resolved here rather than by the caller, so the unscoped default path — every request that is + * not a scoped token — does no registry lookup and no allocation before the `== null` return. */ -function tokenScopeDenial(userObject: any, opApiName: string) { +function tokenScopeDenial(userObject: any, op: string) { // `!= null`, not `!== undefined`: an unscoped policy stores `operations: null`, and expanding a // null would throw rather than fall through to the role. const tokenOperations = userObject?.tokenOperations; @@ -470,6 +478,7 @@ function tokenScopeDenial(userObject: any, opApiName: string) { const scopedOps = userObject._expandedTokenOperations ?? (userObject._expandedTokenOperations = expandOperationsPerms(tokenOperations)); + const opApiName = requiredPermissions.get(op)?.api_name ?? op; if (scopedOps.has(opApiName)) return undefined; harperLogger.info(`Operation '${opApiName}' is outside the scope of the presented token`); @@ -623,7 +632,7 @@ export function verifyPerms(requestJson: any, operation: any, _options?: any) { const permsResponse = new PermissionResponseObject(); - const scopeDenial = tokenScopeDenial(requestJson.hdb_user, requiredPermissions.get(op)?.api_name ?? op); + const scopeDenial = tokenScopeDenial(requestJson.hdb_user, op); if (scopeDenial) return scopeDenial; if (