feat: OIDC trusted publishing — deploy from CI with no stored credential

[dawsontoth]

Closes #2171. Now includes the token operation-scoping work formerly in #2174 (merged into this branch 2026-08-17, so this reviews as one whole rather than a stack).

Two things predate parts of this description: (a) the module moved security/oidcTrust/ → security/authn/oidc/ with the core made issuer-agnostic behind provider profiles (providers/githubActions.ts, providers/generic.ts); (b) the AuthAuditLog item in "Open / not done" is now done — exchanges emit AuthAuditLog on success and failure via the baseRequest serverHandlers injects for NO_AUTH_OPERATIONS. The security shape below still holds.

Folded in from #2174 — token operation scoping (has an open design question)

An OIDC trust policy may carry an optional operations list; the exchanged token carries it as a claim, and verifyPerms/verifyPermsAST intersect it with the user's role, so one Harper user can back several workflows each holding a credential narrower than the user itself. Absent claim = today's behavior exactly.

Open design question, unresolved — do not merge to main before it is settled: @kriszyp asked for per-policy operation trust; @heskew reviewed and preferred it dropped ("one authorization system; least privilege lives in the role"). Folding #2174 in here does not decide that — it only stops juggling a stack. If the answer is "drop it," the four token-scoping commits revert cleanly off the top.

The invariant is "can only ever subtract," and getting there took finding six bypasses (across three review rounds plus a local cross-model pass), which is the strongest argument for reviewing this part closely — or for dropping the feature per @heskew, since the attack surface is this wide:

1. The scope gate is the first authorization step in both verifyPerms and verifyPermsAST, ahead of the super_user bypass and the operations gate-2 grant, which both return null early.
2. It is never merged into role.permission.operations — that field is not purely narrowing (gate 2 treats an explicit SU-only listing as a grant), so merging could widen. It travels separately as tokenOperations.
3. It is enforced on the SQL path (verifyPermsAST), mutually exclusive with verifyPerms — a gate in only one let a get_status-scoped token run arbitrary SQL until fixed. It also gates on the API operation the caller sent (requestJson.operation), not the handler name — the handler-name form both denied the feature's own deploy_component and let a read-scoped token ride an export_local/export_to_s3 job (SQL and NoSQL variants) to exfiltrate data.
4. It carries forward across credential-minting — create_authentication_tokens (NO_AUTH, so it skips verifyPerms entirely), refresh_operation_token, and impersonation each produce a new credential/principal and would otherwise drop the scope, escalating a scoped token to unscoped. Fixed in 6038dadcb; found by the cross-model review.
5. A scoped credential cannot mint a login token — create_authentication_tokens with purpose: 'login' (also NO_AUTH) trades for a cookie session, and a session is username-only by construction (session-restore reloads the full user), so the scope cannot survive it. Denied rather than carried. Fixed in 31da888c9.

A cross-model review of the combined branch is running; findings and adjudication will be added below when it completes.

What this does

Lets a CI runner authenticate to Harper with no stored credential. It presents an OIDC identity token minted by its provider; if that token verifies against a stored trust policy, Harper returns a one-hour operation token for the user the policy names.

Today, after #1876, a deploy workflow carries a 30-day refresh token — full user authentication, one per user (#2018), expiring on a schedule nobody tracks. After this:

permissions:
  id-token: write
  contents: read
environment: production
steps:
  - run: harper deploy by_ref=true
    env:
      HARPER_CLI_TARGET: ${{ vars.HARPER_CLI_TARGET }}   # a var, not a secret

Configured once against the instance, and revoked with drop_oidc_trust:

harper add_oidc_trust id=my-app-prod issuer=https://token.actions.githubusercontent.com \
  audience=https://my-instance.harperdb.io:9925/ user=ci-deploy \
  claims='{"repository_id":"67890","workflow_ref":"HarperFast/my-app/.github/workflows/deploy.yml@refs/heads/main","environment":"production"}'

Same exchange npm, PyPI, and AWS STS AssumeRoleWithWebIdentity use.

Where to put attention

The ref-gate requirement is stricter than npm's model, and it is the one thing most likely to be wrong. validateTrustPolicyClaims rejects a policy that pins repository + workflow without also pinning workflow_ref, ref, or environment — because otherwise anyone who can push a branch can add the trusted workflow to it and mint a token. npm accepts that shape and mitigates with environment protection instead. The practical cost: a tag-triggered release cannot pin an unknowable tag, so it must name an environment. ref_type: tag is deliberately not accepted as a gate, since anyone with push access can create a tag. If that blocks a real workflow shape, it is a one-line change in security/oidcTrust/claims.ts.

I dropped operations from the policy shape that #2171 described. An operation allowlist on the policy would be a second authorization system running beside roles — two places to disagree about what a CI identity may do. Least privilege is now the role of the user the policy names. Workflow config is unchanged.

Replay protection is honest about its race. hdb_oidc_token_use records issuer|jti with expiresAt past the token's own expiry. The get-then-put is not atomic; Harper's OCC may serialize it, but the code does not depend on that. The reasoning (in tokenExchange.ts) is that a concurrent replay is not a privilege escalation — whoever holds the token could obtain one operation token anyway — and what this does stop is the realistic leak-then-reuse case. Worth a second opinion on whether that trade is acceptable, since it is the one place I chose not to reach for a stronger primitive.

Cross-node replay depends on replication timing. The table replicates like other system tables, but replication is asynchronous, so two simultaneous replays against different nodes can both land.

createOperationToken is new in tokenAuthentication.ts. createTokens could not be reused: it overwrites hdb_user.refresh_token as a side effect, so minting for CI would silently revoke whatever credential that user already held (#2018) — the exact problem this removes. The new function does no user lookup and no write; callers establish entitlement themselves.

CLI precedence. The exchange ranks below every configured credential, not above. Adding id-token: write to a workflow that still sets HARPER_CLI_REFRESH_TOKEN must not silently change which identity deploys. Tested both directions in unitTests/bin/cliOperations.test.js.

This PR now also changes how every operation body is logged. Review flagged that token was missing from the operations-log redaction list, so exchange_oidc_token wrote the raw CI identity JWT at INFO on every call — before the handler runs, so rejected attempts logged an unspent credential. Fixing it meant touching processLocalTransaction, which is shared by every operation:

• token and refresh_token added to the list. Both also cover pre-existing exposure — token on login (feat(cli): token env vars for CI/CD auth #1876), refresh_token on refresh_operation_token (a 30-day credential).
• The inline rest-destructure became redactForOperationLog + an exported UNLOGGABLE_OPERATION_FIELDS. Not tidying: operationLog is built from mainLogger at module load, so the logged body can't be intercepted afterwards, and the existing redaction test's assertions sit behind if (info_log_stub.called) — never true in the unit environment, so it has been passing vacuously. The extraction makes the contract testable without logger plumbing.

Behaviorally identical for every other operation, but it is a shared path and deserves a look.

Other decisions worth a look

• Every rejection returns one message. The exchange endpoint is unauthenticated, so a caller told which check failed can enumerate a policy one claim at a time. Reasons go to the oidc-trust logger.
• Audience must be instance-specific. add_oidc_trust refuses https://github.com/<owner> — the provider default, shared by every repository under an owner, which would make the audience check meaningless.
• Claims are matched discretely, not via sub. Since 2026-07-15 GitHub emits immutable subjects (repo:org@12345/repo@67890:...) for new repos, so sub parsing would have to handle two shapes forever. repository_id + repository_owner_id are also immune to org-name recycling.
• workflow_path is derived from workflow_ref by splitting on @refs/, so a tag release can pin the workflow file without knowing the tag.
• JWKS rate-limit clock lives outside the cache entry. A successful fetch replaces the entry, and a rate limit that resets whenever it fires is not a rate limit. Keeping it separate also means a genuine key rotation is picked up on first use rather than after the window — I got this wrong first and the tests caught it.
• Trust-policy ops join the secrets store in MCP DEFAULT_EXCLUDED. list_oidc_trust matches the list_* glob, and the policy set names exactly which repository and workflow are worth compromising.

Deep review (Harper-domain pass, 2026-08-18)

A single-pass Harper-domain deep-review (the lens the earlier cross-model run's domain leg failed to execute) found four issues; all fixed in 922d260a6 except where noted:

1. Scoped token could mint a long-lived credential. Fix was incomplete — see round 2, finding 1. create_authentication_tokens is NO_AUTH, so the scope gate never runs. The 922d260a6 fix denied a scoped caller, which missed the common case: an unscoped policy. Superseded by the provenance guard in 2a5a509ba.
2. Scope enforced only on operations/SQL, not REST/GraphQL — reviewers please note. The scope lives in the verifyPerms/verifyPermsAST gate; the application resource path authorizes via table-level checkPermission and does not consult it. So a scoped operation token used as a Bearer against REST/GraphQL keeps the role's full CRUD. I narrowed the types.ts guarantee to say so; full resource-path enforcement is a deliberate follow-up on the same surface as CORE-3061 (GraphQL bypasses ops allowlist), not done here. Until then, point a policy's user at a role that is itself least-privilege.
3. Replay table now audit: true. hdb_oidc_token_use was created without an explicit audit flag, so with logging.auditLog:false its rows never replicate — silently dropping cross-node replay protection. Fixed to match its sibling hdb_oidc_trust.
4. Exchange re-validates policy specificity. add_oidc_trust enforces the repo+workflow+ref pins, but the exchange only backstopped empty-claims / pull_request_target. A row arriving via replication from an older node or a restored backup could be under-specified; findMatchingPolicy now re-runs the specificity checks and skips (logs) any row that fails.

The pass also traced and cleared: token verification (alg/kid/aud/iss/exp confusion), JWKS SSRF, the replay fingerprint, the credential-minting carry-forwards, stale-scope cache leaks, body-controlled auth, trust-policy CRUD authz, MCP exclusion, the upgrade-directive version tag, and the expiresAt TTL.

Review coverage

Model	Pass	Findings	Disposition
Codex (KrAIs)	Cross-model review of 2b27671584ad	8 (1 blocker)	6 fixed, 1 documented + spun off, 1 deferred to the open design question
Claude (claude[bot])	Independent cross-model review of the same commit	4	All 4 overlap Codex's; same fixes
Claude (deep-review skill)	Harper-domain single pass, earlier	4	3 fixed in 922d260a6; finding 1's fix was incomplete and is superseded below
Barber AI (cb1kenobi)	Cross-model review of 2a5a509ba	16 across two passes	15 fixed; 1 filed as #2201
Claude (claude[bot])	Follow-up pass on 2a5a509ba	1 blocker	Fixed — a real fail-open my own earlier analysis got wrong

The two cross-model reviews were run independently and converged on four findings (1, 3, 4, 6 below), which is the main reason I treated those as real rather than speculative. Every claim was verified against the code before acting — one was verified and then declined, see 7.

Cross-model review round 2 — fixes in 2a5a509ba

1. Unscoped exchanged token could still mint standing credentials. (both models — the blocker.) Confirmed: the previous guard tested hasOperationScope(...), but a trust policy carries operations only when the operator opts in, so the ordinary exchanged token is unscoped and the guard was false for exactly the headline example. It reached the standing path with a caller-controlled expires_in and a 30-day refresh token. Mint provenance is now a signed claim on every token createOperationToken produces, lifted onto the principal in validateToken, and refused ahead of the user lookup — so a refused request costs no read and writes nothing. applyImpersonation carries it forward, since it returns a new principal and would otherwise launder the marker.
2. read_only scope could run write SQL. (Codex.) Confirmed: the group expands to include sql, and verifyPermsAST returns null for a super_user before any table check — so DELETE passed. A write statement now additionally requires its matching data operation in scope. I took this over tracking which group admitted sql: it needs no provenance through expansion, and it is what already separates read_only from standard_user. A bare ['sql'] therefore admits SELECT only.
3. job_workflow_ref accepted as a caller-ref gate. (both models.) Confirmed. It names the reusable workflow that ran, not the caller — its @ref is constant however it is invoked, so it admitted any branch of any caller repo referencing that workflow. Removed from the ref row; still valid as a workflow pin.
4. iat optional, exp unbounded against the clock. (both models.) Confirmed both halves: the ceiling was skipped entirely when iat was absent, and jsonwebtoken never rejects a future iat, so a pair shifted equally far forward kept a small delta. iat is now required, a future iat is rejected outside tolerance, and exp is bounded against the verification clock independently.
5. New rewire mutations in tests. (Codex.) Correct — AGENTS.md prohibits new sinon/rewire. Removed all six; the denial cases no longer need stubbing now that the guard runs before any I/O, and the two positive-path cases were already covered by pre-existing tests (lines 428 and 486 on main).
6. DESIGN.md contradicted the implemented allowlist. (both models.) Correct. Rewritten to describe the narrowing-only scope, its enforcement surface, the SQL rule from 2, and the open design question. The REST/GraphQL boundary now appears there too, not only on the types.ts field.
7. Grantable-operation registry is process-local — documented, not fixed here. (Codex.) Verified and confirmed as a mechanism, but the framing needed correcting: it is not introduced by this PR and not OIDC-specific. validateOperations is consulted on main by add_role, alter_role, and impersonation validation identically, and the OPERATION_REGISTERED bridge carries only name→thread routing. It fails closed — a rejected policy, never a widened one — so this is a DX bug, not a bypass. I corrected my overclaiming comment and filed the bridge fix separately rather than riding a shared-subsystem ITC change on an auth PR.
8. Scope not enforced on REST/GraphQL. (Codex.) Verified and confirmed as a real mechanism — four checkpoints, none consulting the scope. Deliberately not fixed here: it is the open design question below, and closing it means either enforcing at the Resource boundary with a verb mapping or making these credentials operations-API-only. Both are calls for a human. I made the caveat discoverable (DESIGN.md, not just types.ts) instead of quietly changing the model.

Cross-model review round 3 — fixes in 8799d450b, 11f2280c6, 76718e6b3, 5178cc799

A third model (Barber AI) plus a follow-up Claude pass. Every finding was verified against the code before acting, and two of them corrected earlier conclusions of mine. All review threads are resolved.

The two that were genuinely dangerous:

1. Replay protection was bypassable (8799d450b). The fingerprint hashed the whole token, justified in my own comment by "a replayed token is byte-identical by definition". That premise is false: the signature segment is covered by nothing, and base64url decoding ignores the surplus low bits of its final character. I reproduced it — 15 alternate spellings of an RS256 signature all verify, each hashing differently, so one leaked identity token bought 16 operation tokens. ES* malleability (s → n−s) is a second vector. Now keyed on the signed input (header.payload). The regression test was confirmed to fail against the old fingerprint before being kept.

2. A computed SQL permission denial was being discarded (5178cc799). processAST's guard was permissionsCheck && permissionsCheck.length > 0, but PermissionResponseObject has no length — so undefined > 0 was always false and the statement ran anyway. This only bites where processAST is the first checker: an export job, which in the job worker has no outer gate at all. I had earlier concluded this path failed closed; it did not, and the follow-up review was right to push back.

Also fixed: an issuer whose iss ends in / (Azure AD v1) could never authenticate; a natural audience value silently produced a policy that could never match (rejected at write time now, with a test pinning the check to the CLI's real normalizeTarget output); the replay table now has the three standard bootstrap touchpoints, which matters because a table auto-provisioned by replication is created without the audit flag — and auditing is the replication feed, so a peer-provisioned copy would have been silently non-replicating; "enabled": "false" stored an enabled policy (Joi coerces, validateBySchema discards the coerced value) — now .strict(); export jobs carry the real top-level operation to the inner scope check; the expanded-scope memo is guarded with instanceof Set (msgpackr returns a Set as an Array across the hdb_job boundary); plus audit client-IP, non-throwing audit emit, logged JWKS failures, uniform rejection when the trust table is missing, issuer filtering pushed into the scan, the CLI exchange timeout, blank-CI-secret identity switching, the JWKS cache-clear race, and two stale JSDoc symbols.

Claim matching was already exact, but nothing pinned it — a startsWith mutation left all 195 OIDC tests green. Prefix matching on repository/sub is the classic trusted-publishing escalation (HarperFast/my-app admitting HarperFast/my-app-evil), so there are now negative cases in both argument orders, verified against that exact mutation.

Filed rather than fixed: #2201 — token scope is not enforced on the REST/GraphQL resource path. Both remedies change user-visible behavior and it is entangled with the open design question below, so it belongs to a human. The boundary is documented in types.ts and DESIGN.md meanwhile.

Open / not done

• Docs companion: HarperFast/documentation#599. Covers the four operations, the GitHub Actions policy-specificity rules (including the pull_request_target veto and why ref_type: tag is not a ref gate), the generic-issuer sub requirement, and the CLI credential source, plus a 5.3 release-notes entry. Written against this branch rather than this description, so it already reflects the issuer-agnostic split noted at the top. It is a draft gated on this PR via a companion marker.
• The exchange does not write to AuthAuditLog. Resolved — exchanges now emit AuthAuditLog on success and failure, same stream and switches as Basic/Bearer/mTLS, using the baseRequest serverHandlers injects for NO_AUTH_OPERATIONS.
• GitHub Actions only. GitLab and CircleCI expose the same idea through different env vars; detection is explicit rather than guessed, so an unrecognized runner just falls through.
• No integration test. Coverage is 117 new unit tests (100 in unitTests/security/oidcTrust/, 12 in ciIdentityToken.test.js, 5 added to cliOperations.test.js), including an end-to-end exchange with a real signed token, a real JWKS fetch, and real token minting — only the network and the two system tables are displaced. The touched suites run 310 green in total.

Unrelated pre-existing failure

unitTests/server/serverHelpers/uwsServer.test.js — "rejects a body over maxBodyBytes with 413" fails with EPIPE, consistently across three runs. I stashed this branch and rebuilt to confirm it fails identically on clean main. Not from this PR, but nobody appears to be tracking it.

---

🤖 Generated by Claude Code. LLM-authored; deserves a human read — especially the token operation-scoping section (open design question) and the auth hot-path changes in verifyPerms/verifyPermsAST.

[gemini-code-assist]

Code Review

This pull request introduces OIDC trusted publishing, allowing CI runners (such as GitHub Actions) to authenticate with HarperDB without stored credentials by exchanging an OIDC identity token for a short-lived operation token. The implementation includes token verification, JWKS retrieval, claim validation, trust policy management operations, and database upgrade directives. Feedback on the changes suggests registering the in-flight promise in inFlightLoads before attaching the .finally() callback in jwks.ts to avoid potential race conditions with synchronously resolving promises.

[claude]

Reviewed the one new commit since the last review (5da87ee13: threads an optional now clock seam through getSigningKey to test the stale-key grace ceiling, mirroring the existing clockTimestamp pattern in verifyIdentityToken). Production behavior unchanged (default Date.now()); tests correctly pin both sides of the STALE_KEY_GRACE_MS boundary. No blockers.

[heskew]

[human edit] love this 🙌

---

This is a higher-level review — design and direction, not a line-by-line pass; that can come separately.

The hardening story holds where it counts — I traced the JWKS layer, verification, the exchange, and the wiring (alg allowlist at both decode and verify, discovery issuer echo-check, bounded fetches, rate-limited unknown-kid refetch, uniform rejections, replay recorded before mint). On the two calls you flagged for second opinions:

• Dropping the per-policy operations allowlist — agreed, keep it dropped. One authorization system; least privilege lives in the role of the user the policy names, and the super_user warning covers the sharp edge.
• The replay race — acceptable as analyzed. A concurrent replay yields nothing the token holder couldn't get anyway; leak-then-reuse is the case worth stopping, and it's stopped.

The one structural ask: make the core issuer-agnostic, and confine GitHub to a provider profile. We want this layer to seed a newer authn core — cloud/workload identity next (K8s service accounts, GCP/Azure, SPIFFE), and eventually the oauth component consuming core's verifier instead of shipping its own. The PR is closer to that than it reads: hdb_oidc_trust, the operation names, the policy schema, jwks.ts, and verifyIdentityToken are already generic. The GitHub-shaped parts are concentrated, so this is mostly moves:

1. Provider profile extraction. Move the pin-set rules in claims.ts (REPOSITORY_PIN_CLAIMS / WORKFLOW_PIN_CLAIMS / REF_GATE_CLAIMS and the three-pin requirement in validateTrustPolicyClaims), the workflow_path derivation, the SHARED_DEFAULT_AUDIENCE regex, and describeRun into a providers/githubActions.ts behind a small interface (validate policy claims / reject shared default audiences / normalize claims / describe principal), resolved by normalized issuer. Nothing else in the module should say GitHub. Your ref-gate rule — the part you called most likely to be wrong — is right for GitHub, and profile-scoping it means it never constrains other issuers. While it's all in one place: consider having the GitHub profile default-deny pull_request_target via event_name unless a policy opts in, which closes that Auth: GitHub OIDC trusted publishing — deploy from CI with no stored credential #2171 callout cheaply.
2. Generic fallback profile: require sub to be constrained. For issuers with no registered profile, the policy must pin sub. That makes workload identity work immediately with zero provider code — K8s SA tokens (system:serviceaccount:<ns>:<name>), GCP service accounts, and SPIFFE SVIDs all have stable canonical subjects. GitHub needs a profile precisely because its sub is the one claim you shouldn't pin.
3. Replay key: hash of the token rather than issuer|jti. Not every issuer emits jti (Azure uses uti; some omit it). Keying hdb_oidc_token_use on SHA-256 of the raw token is strictly more general with identical semantics — a replayed token is byte-identical — and removes the hard jti requirement from verifyIdentityToken.
4. Placement: security/authn/oidc/ rather than security/oidcTrust/. We're going to grow an authn namespace out of this (verifier strategies for the legacy Basic/mTLS paths, named credentials (Auth: support multiple named, independently revocable refresh credentials per user #2018) on top of createOperationToken, collapsing the duplicated no-auth registration this PR had to touch in two places). Cheap rename now, painful later.
5. CLI: structure ciIdentityToken.ts as a provider list — GitHub Actions is entry one; a K8s entry later is just reading the projected token file. Naming it workload identity rather than CI identity matches where this goes.

None of these touch the hardening. One more: agree with your own instinct on AuthAuditLog — a credential issued to an external principal belongs in the auth audit trail, so I'd pull that TODO into this PR rather than after.

---

🤖 Drafted by Claude on behalf of @heskew

[dawsontoth]

All five structural asks are in e8b3d238c, plus the AuthAuditLog one. No hardening changed.

1. Provider profile extraction. providers/githubActions.ts now owns the three pin requirements, workflow_path derivation, the shared-default audience regex, and principal description, behind IdentityProviderProfile resolved by normalized issuer. Nothing else in the module says GitHub — claims.ts keeps only issuer-agnostic matching plus constraint-shape validation, and validateTrustPolicyClaims split into that plus profile.assertPolicyIsSpecific. The ref-gate rule is now GitHub-scoped, so it cannot constrain another issuer.

Also added vetoClaims on the profile, which default-denies pull_request_target unless a policy constrains event_name. That closes the fork callout cheaply, as you suggested.

2. Generic fallback requiring sub. providers/generic.ts. The exchange suite now runs a second issuer (https://kubernetes.default.svc) with a service-account-shaped subject end to end, so the zero-provider-code claim is tested rather than asserted.

3. Replay on token hash. SHA-256 of the raw token, base64url. verifyIdentityToken no longer requires jti — that requirement is gone, not relaxed. Hashed rather than stored so the table never holds a credential, with a test asserting nothing JWT-shaped survives into the row.

4. security/authn/oidc/. Done, tests moved alongside.

5. CLI provider list. bin/ciIdentityToken.ts → bin/workloadIdentity.ts, WorkloadIdentityProvider[] with GitHub Actions as entry one. The module comment spells out what a Kubernetes entry is, since that is the next one.

AuthAuditLog — pulled in rather than deferred. You were both right that it belongs here; what I had missed is that serverHandlers already injects baseRequest for NO_AUTH_OPERATIONS, so ip/method/path were available all along. Emitted on failure as well as success, through the same stream and the same logging.auditAuthEvents switches as Basic/Bearer/mTLS, carrying auth_strategy: oidc, the policy id, the principal, and a truncated token fingerprint.

One note on operations scoping: you and Kris landed on opposite sides — he asked for per-policy operation trust for flexibility, you said keep it dropped. I have gone with your call here and captured the reasoning in DESIGN.md, including the part that makes a naive implementation actively unsafe: permission.operations is not purely narrowing, since gate 2 in operation_authorization.ts treats an explicit listing of an SU-only operation as a deliberate grant. Any future version has to be a post-role intersection, never an injection into that list. Dawson has asked for a stacked PR exploring it, so that discussion can happen there against real code rather than blocking this one.

438 tests green across the touched suites.

🤖 Addressed by Claude Code

[kriszyp]

Automated gate — not yet queued for human review.

This PR's AI review found issues, and the PR description reports no cross-model reviews.
Per team policy, a substantive PR with AI-review findings is queued for human review only after at least 2 cross-model reviews have been run, their findings addressed, and the coverage reported in the PR description (## Review coverage naming each model — see harper-engineering-guidelines).
The findings below count as one of the two: address them, run a second outside-model review, update the description, and the gate lifts automatically on the next pass.

---

TL;DR

This PR adds issuer-agnostic OIDC trusted publishing, trust-policy administration, GitHub Actions CLI exchange, and optional operation-scoped tokens.
Verification, caching, replay protection, upgrade handling, and focused tests are substantial.
However, the short-lived credential and least-authority guarantees have concrete bypasses, plus GitHub and token-lifetime edge cases.
These affect the feature’s core security claims; hold the merge until they are resolved.

verdict: BLOCK
merge: squash
Human-Review-Need: 4 @ 2b27671

Findings

blocker — security/authn/oidc/tokenExchange.ts:205 — the one-hour OIDC credential can mint caller-extended operation tokens, 30-day refresh tokens, and login sessions
major — utility/operation_authorization.ts:475 — token operation scope is bypassed by native Resource APIs
major — utility/operation_authorization.ts:510 — the read_only scope admits write SQL whenever the role permits it
major — security/authn/oidc/providers/githubActions.ts:53 — job_workflow_ref does not constrain the caller branch in reusable workflows
major — security/authn/oidc/identityToken.ts:101 — an absent or future iat defeats the advertised identity-token lifetime ceiling
minor — security/authn/oidc/trustPolicyOperations.ts:55 — main-thread policy validation rejects worker-registered operations it claims to support
minor — DESIGN.md:212 — the design document forbids the per-policy allowlist the PR exposes
minor — unitTests/security/tokenAuthentication.test.js:448 — four new tests violate the repository’s no-new-rewire invariant

Change tour

• The verification core validates and matches claim constraints, retrieves bounded OIDC discovery/JWKS responses with caching, verifies JWTs, and isolates issuer behavior in profiles: security/authn/oidc/claims.ts:24-134, security/authn/oidc/jwks.ts:56-211, security/authn/oidc/identityToken.ts:64-107, security/authn/oidc/providers/generic.ts, security/authn/oidc/providers/githubActions.ts, security/authn/oidc/providers/index.ts.
• Trust-policy types, CRUD operations, system schema, operation constants, and the 5.3 upgrade directive establish persistent administration: security/authn/oidc/types.ts:13-30, security/authn/oidc/trustPolicyOperations.ts:97-198, json/systemSchema.json:476-516, utility/hdbTerms.ts:214, utility/hdbTerms.ts:352-355, upgrade/directives/5-3-0.ts:18-74, upgrade/directives/directivesController.ts.
• Token exchange performs policy selection, replay recording, audit logging, and token minting; server routing exposes it as an authentication operation while redaction and MCP exclusions keep identity material away from logs/tools: security/authn/oidc/tokenExchange.ts:58-227, server/serverHelpers/serverHandlers.js:40-47, server/serverHelpers/serverUtilities.ts:85-117, server/serverHelpers/serverUtilities.ts:268-288, server/serverHelpers/serverUtilities.ts:606-619, components/mcp/tools/operations.ts:148-164.
• Token scope is carried through minting, refresh, validation, impersonation, and SQL dispatch before centralized operation authorization: security/operationScope.ts:18-33, security/tokenAuthentication.ts:162-175, security/tokenAuthentication.ts:244-304, security/tokenAuthentication.ts:323-347, security/impersonation.ts:39-45, sqlTranslator/index.ts:76-87, utility/operation_authorization.ts:468-511.
• The CLI requests GitHub’s ambient workload identity and exchanges it only after explicit credentials and stored tokens are unavailable: bin/workloadIdentity.ts:31-119, bin/cliOperations.ts:766-831.
• Design notes and focused suites cover the subsystem, scope propagation, CLI, MCP, and server plumbing: DESIGN.md:193-214, unitTests/security/authn/oidc/tokenExchange.test.js, unitTests/security/authn/oidc/trustPolicyOperations.test.js, unitTests/security/authn/oidc/verifyIdentityToken.test.js, unitTests/security/tokenOperationScope.test.js, unitTests/security/tokenOperationScopeMinting.test.js, unitTests/security/tokenScopeRefresh.test.js, unitTests/bin/workloadIdentity.test.js, unitTests/components/mcp/tools/operations.test.js, unitTests/server/serverHelpers/serverUtilities.test.js.

Verification

git diff --check origin/main...HEAD passed. Targeted tests could not run because this checkout has no local Mocha installation and restricted network access prevented npx from fetching it (EAI_AGAIN).

Review coverage

lens	outcome
gemini	pruned — pruned (policy minimal)
cursor-grok	pruned — pruned (policy minimal)
cursor-composer	pruned — pruned (policy minimal)
codex	ok — graded leg — produced review.md + comments.json
domain	pruned — pruned (policy minimal)

---

Pre-push review of feat/oidc-trusted-publishing (2b27671) vs origin/main by codex.
Review emphasis: Dispatch-configured.

— codex review, submitted by the dispatch review gate

[kriszyp]

Great PR!
_{🤖 Reviewed with Codex}

[kriszyp]

clearJwksCache() clears failedRefreshAt at security/authn/oidc/jwks.ts:72, but a refresh already in flight can fail afterward and execute this set, recreating the backoff. The next call then exits at security/authn/oidc/jwks.ts:233-235 without fetching, so an operator’s forced reread remains blocked for 30 seconds during an outage/recovery race. The existing cacheGeneration guard already prevents orphaned successful loads from repopulating the key cache; please apply the same generation or ownership rule to failure state and add a delayed-failure → clear → recovered-issuer regression test.

— KrAIs (Codex)