docs(security): OIDC trusted publishing (v5.3.0), ops-table anchor fixes, badge guidance - #637
Conversation
Extracts from #599 everything whose upstream is settled, now that harper#1849 has been **closed** — with #2135 and #2301, which were stacked on it — leaving no two-phase surface in main at all. **OIDC trusted publishing** (harper#2173, merged 2026-08-21, awaiting v5.3.0). Unlike the two-phase work this is merged, so the API is fixed and only the release is pending: - `add_oidc_trust` / `list_oidc_trust` / `drop_oidc_trust` (super_user) and the unauthenticated `exchange_oidc_token`, with the replace-not-merge semantics. - The GitHub Actions policy-specificity rules as a table — pin the repository, pin the workflow, gate the ref — with what each leaves open unsatisfied, since the ref gate is stricter than npm's model and is the rule most likely to refuse a real workflow. - The `operations` scope, including that it covers the Operations API and SQL but **not** an application's REST/GraphQL path, so it bounds what CI can administer rather than what data it reaches (harper#2201 tracks that gap). - Single-use tokens with an honest account of the cross-node replay race; one rejection message for every failure with the reason in the `oidc-trust` log; `drop_oidc_trust` not revoking already-issued tokens. - The CLI exchange as precedence entry 7 — ranked below every configured credential, so adding `id-token: write` to a workflow that still sets a token secret does not silently change identity. **Ops-table anchors.** Six user/role rows and seven SSH rows pointed at `#certificate-management`, which documents neither. Repointed to `#users--roles` and `#components`; the five genuine certificate rows are untouched. A pre-existing docs bug with no upstream dependency, which is why it should not have been sitting behind a feature branch. **`AGENTS.md`** gains the rule that produced most of this stack's version churn: derive a badge from core's git tags, not from a feature branch's `package.json`, and re-check every refresh pass. Deliberately left on #599: the deployment status groupings. They read as generic tracking corrections, but they enumerate `staged`, `staging`, `activating`, `reverting`, `extracting` and `loading` — and main's code defines only `pending`, `installing`, `replicating`, `restarting`, `success`, `failed` and `rolled_back`. Extracting them would document statuses that do not exist.
harper#1849 is closed, and so are #2135 and #2301 which were stacked on it — #2135's base was `claude/deploy-component-two-phase-94969a`, not `main`. None of `revert_component`, `two_phase`, `activateStagedApplication`, `deployComponentTwoPhase` or `stagingRetention` exists in `main`. So this PR's subject is not merely unmerged, it is abandoned in that form. There was no reason for the content whose upstream *is* settled to wait with it. Extracted to #637: - **OIDC trusted publishing** — harper#2173 is merged (2026-08-21), awaiting v5.3.0. The API is fixed, which is the difference that matters: the risk here was never "unreleased", it was "changing underneath the docs". - **The ops-table anchor fix** — 13 rows pointing at `#certificate-management`, which documents neither user/role nor SSH operations. A pre-existing docs bug with no upstream dependency, which should never have been behind a feature branch. - **The `AGENTS.md` badge guidance.** What stays is two-phase deploy, `revert_component`, deploy modes, activation failures, the deployment status groupings, retention, and the 5.3 Deployment release notes. Kept here deliberately, despite looking extractable: the status groupings and retention text. `list_deployments` and `get_deployment` are shipped, so the corrections read as generic — but they enumerate `staged`, `staging`, `activating`, `reverting`, `extracting` and `loading`, while `main` defines only `pending`, `installing`, `replicating`, `restarting`, `success`, `failed` and `rolled_back`. Extracting them would publish statuses that do not exist. Same for retention, which mixes the shipped payload bound with the unshipped staging one.
🚀 Preview DeploymentYour preview deployment is ready! 🔗 Preview URL: https://preview.harper-documentation.harperfabric.com/pr-637 This preview will update automatically when you push new commits. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces comprehensive documentation for OIDC Trusted Publishing (Workload Identity) in version 5.3.0, detailing how CI runners can authenticate without stored credentials using trust policies. It updates CLI authentication precedence, adds API reference documentation for new OIDC operations (such as add_oidc_trust and exchange_oidc_token), categorizes CLI commands, and adds release notes. The review feedback focuses on minor readability and grammatical enhancements, such as adding commas and splitting a run-on sentence to highlight a security warning regarding loopback targets.
Four from gemini on #637, all correct. Three are commas after introductory phrases ("In a reusable workflow,", "if minting then fails,", "In the CLI,"). The fourth is worth more than a comma: the sentence about a rejected OIDC exchange ran two consequences together with a semicolon, so the loopback case — where a credential-less request is authorized as superuser and the operation succeeds with no identity check at all — was buried mid-sentence behind the payload-credential case. Now its own sentence.
🚀 Preview DeploymentYour preview deployment is ready! 🔗 Preview URL: https://preview.harper-documentation.harperfabric.com/pr-637 This preview will update automatically when you push new commits. |
…kthrough Three review comments from @dawsontoth. **The YAML block is now labelled GitHub Actions.** `permissions: id-token: write` is Actions-specific syntax, not a portable convention, and the page presented it as though it were generic. It now says so, notes that other CI systems express the same idea differently, and that Harper detects Actions only — with an unrecognized runtime falling through rather than erroring. **Cut the combination-case analysis.** Two paragraphs explained what happens when OIDC is configured *alongside* a refresh token or payload credentials, including which one wins and the one case where the exchange takes over. Accurate, but the wrong thing to hand a first-time reader: it teaches the mixed configuration instead of the good one. Replaced with what someone setting this up needs — with a trust policy you need nothing else, remove a token if you have one, and a pointer to Authentication Precedence for anyone genuinely in a mixed setup. The rejected-exchange paragraph got the same treatment: it keeps the point that matters (a rejection does not reliably halt the command) and drops the payload-credential branching that only applies off the happy path. **Added a CLI walkthrough for `add_oidc_trust`** — `harper login` once, then run the operation against the cluster, then `list_oidc_trust` to confirm. Includes the detail that makes it work in a shell: `claims` must be quoted as one argument because the CLI parses each value as JSON, which is what produces the nested object the operation wants. Verified against `buildRequest` in `bin/cliOperations.ts`, where every value except `ref` is JSON-parsed. Also gives the explicit `target=` + `auth_username=` form for anyone who would rather not store a login token.
🚀 Preview DeploymentYour preview deployment is ready! 🔗 Preview URL: https://preview.harper-documentation.harperfabric.com/pr-637 This preview will update automatically when you push new commits. |
Per @cb1kenobi: "every rejection returns the same message" was an overclaim. `exchangeOidcToken` validates the request shape with Joi and throws ClientError(validation.message) before entering the try block that produces the uniform rejectToken 401s, so a missing or oversized `token` field fails with its own message. Now scoped to a well-formed token, with a note that the schema rejection reveals nothing about a policy — which is the property the uniformity exists to protect.
🚀 Preview DeploymentYour preview deployment is ready! 🔗 Preview URL: https://preview.harper-documentation.harperfabric.com/pr-637 This preview will update automatically when you push new commits. |
The guidance this PR adds is what produced a false Critical on this PR: a review reported git tag --contains returning v5.2.5 for the OIDC merge commit, when four checks against a tag object verified identical to origin say it is not contained there at all. So the check needs the step that distinguishes those cases. Fetch tags, then confirm git rev-parse <tag> matches git ls-remote for the same name — --contains is only as good as the local tag it compares against, and a divergent tag of the same name is confidently wrong in either direction. Also records the better decisive check for a disputed badge: whether the feature files exist at the tag (git ls-tree / git show <tag>:<file>). That cannot be confounded by ancestry or by which tag a clone holds, and it is what settled this one.
🚀 Preview DeploymentYour preview deployment is ready! 🔗 Preview URL: https://preview.harper-documentation.harperfabric.com/pr-637 This preview will update automatically when you push new commits. |
kriszyp
left a comment
kriszyp
left a comment
There was a problem hiding this comment.
Great stuff!
🤖 Reviewed with Codex
| "audience": "https://my-instance.harperdb.io:9925/", | ||
| "user": "ci-deploy", | ||
| "operations": ["deploy_component", "get_deployment", "restart_service"], | ||
| "claims": { "repository_id": "67890", "environment": "production" } |
There was a problem hiding this comment.
This example cannot pass GitHub Actions policy validation: it pins the repository and gates the ref through environment, but it does not pin a workflow. Please add one of workflow_ref, workflow_path, job_workflow_ref, or job_workflow_path to claims so the copyable add_oidc_trust example succeeds.
There was a problem hiding this comment.
Correct, and this is the one that stings — fixed in a7c885ac by adding workflow_ref to that example.
The page states the three structural requirements (pin the repository, pin the workflow, gate the ref) a few paragraphs above, and I still wrote a copyable example violating one of them. assertPolicyIsSpecific would have refused it. The lesson I am taking: check examples against the rules the same page states, not against what I meant them to show — I was focused on demonstrating operations and stopped thinking about claims.
🤖 Addressed by Claude Code
| | ------------- | -------------------------------------------------------------------------------------------------------------------------- | | ||
| | `id` | **Required.** Policy identifier, 1–128 characters of letters, numbers, `_`, `-`, and `.`. | | ||
| | `issuer` | **Required.** The token issuer (`iss`) this policy trusts. | | ||
| | `audience` | **Required.** The audience the token must be addressed to. Should identify **this instance**; enforced for GitHub Actions. | |
There was a problem hiding this comment.
Please document that every HTTP(S) audience—not only GitHub Actions audiences—must include an explicit port and trailing slash. For example, a generic issuer policy using the plausible https://my-instance.example.com value is rejected at write time; the accepted form is https://my-instance.example.com:9925/. This also qualifies the later statement that generic audiences are not checked for specificity.
There was a problem hiding this comment.
Verified and documented in a7c885ac. assertAudienceIsCanonical is general rather than GitHub-specific: any http(s) audience needs an explicit port and a trailing slash, so https://my-instance.example.com is rejected at write time and https://my-instance.example.com:9925/ is the accepted form. Non-http(s) audiences (an api:// identifier, a bare GUID) pass untouched.
Documented ahead of the specificity discussion with the reason, since that is what makes it stick: the audience is matched byte-for-byte at verification and the CLI asks for its token against the normalized target, which supplies both the port and the slash — so a policy missing them could never authenticate rather than merely being untidy.
And you are right that it qualifies the later line. "For an issuer with no registered profile the audience is not checked for specificity" now says explicitly that the canonical-form rule still applies — as written it implied generic audiences were unvalidated, which is wrong in the way most likely to waste someone an afternoon.
🤖 Addressed by Claude Code
| 5. `HARPER_CLI_OPERATION_TOKEN` and `HARPER_CLI_REFRESH_TOKEN` environment variables, or their legacy `CLI_TARGET_` equivalents — see [Token credentials for CI/CD](#token-credentials-for-cicd) | ||
| 6. A token saved by `harper login` | ||
| 7. `username=` and `password=` operation parameters (legacy fallback) | ||
| 7. A [workload identity token](#workload-identity-oidc) exchanged with the runtime's OIDC provider (v5.3.0) |
There was a problem hiding this comment.
Could we also update the recommendation table immediately below this list? It still tells every CI/CD pipeline to use HARPER_CLI_REFRESH_TOKEN, which conflicts with this new no-stored-credential happy path and steers supported runners toward a secret they need to rotate. Consider recommending workload identity when available, with a refresh token as the fallback.
There was a problem hiding this comment.
Yes — fixed in a7c885ac. The row now reads "Workload identity where the runtime supports it; otherwise HARPER_CLI_REFRESH_TOKEN".
Worth saying how it got that way, since it is a process failure rather than a judgment one. I had made exactly this edit on the earlier branch. When the OIDC content was extracted onto this one I re-applied the precedence entry and the Workload identity section but not the table row, so the page ended up recommending a rotatable secret three paragraphs above telling readers they need no stored credential. Re-applying a section by hand loses the edits that were scattered outside it.
🤖 Addressed by Claude Code
| { "operation": "list_oidc_trust" } | ||
| ``` | ||
|
|
||
| Returns `{ "policies": [ ... ] }`. Each entry carries `id`, `issuer`, `audience`, `claims`, `user`, `operations` (`null` when unscoped), `enabled`, `description`, `updated_by`, and timestamps. |
There was a problem hiding this comment.
The response can also include invalid_reason when a stored policy is malformed or when its user was later deleted or deactivated. Please document this conditional field; it is especially important because list_oidc_trust is the prescribed diagnostic path for an opaque exchange rejection.
There was a problem hiding this comment.
Documented in a7c885ac. Confirmed in trustPolicyOperations.ts: invalid_reason is set when the named user does not exist or is inactive, and included in the returned record when present.
Your framing is why it got more than a table row. The docs send readers to list_oidc_trust precisely because the exchange refuses to say what failed — so a conditional field that explains "this policy cannot authenticate anyone" is the highest-value thing on that response, not a footnote. It now says to check it first, and why: a policy you expected to match that carries an invalid_reason is the fastest answer available when the server will not give you one.
🤖 Addressed by Claude Code
Four from gemini on #637, all correct. Three are commas after introductory phrases ("In a reusable workflow,", "if minting then fails,", "In the CLI,"). The fourth is worth more than a comma: the sentence about a rejected OIDC exchange ran two consequences together with a semicolon, so the loopback case — where a credential-less request is authorized as superuser and the operation succeeds with no identity check at all — was buried mid-sentence behind the payload-credential case. Now its own sentence.
🧹 Preview CleanupThe preview deployment for this PR has been removed. |
4 participants
Extracts from #599 the content whose upstream is settled, now that its gate has gone away rather than landed.
harper#1849 is closed. So are #2135 and #2301, which were stacked on it — #2135's base was
claude/deploy-component-two-phase-94969a, notmain. None ofrevert_component,two_phase,activateStagedApplication,deployComponentTwoPhaseorstagingRetentionexists anywhere inmain, and harper is at v5.2.5. The two-phase deploy docs are therefore waiting on work that was abandoned, not delayed, and there is no reason for the rest of #599 to wait with them.What this carries
OIDC trusted publishing — harper#2173, merged 2026-08-21, in no tag yet. Unlike the two-phase work this is merged, so the API is fixed and only the release is outstanding; badged
v5.3.0, confirmed by the5-3-0.tsupgrade directive onmain.Ops-table anchors — a pre-existing docs bug: six user/role rows and seven SSH rows pointed at
#certificate-management, which documents neither.AGENTS.mdbadge guidance — derive the version from core's git tags, not a feature branch'spackage.json.For the human reviewer
reference/is the live site, so merging this documents a feature nobody can use until 5.3.0 ships. I judged it worth it because the API is settled — the risk that made docs(deploy): two-phase deploy, revert, by-reference deploys, and OIDC trusted publishing #599 dangerous was #1849 changing under the docs, and that risk is absent here. The alternative is holding this until 5.3.0 cuts. This is the call I'd most like ruled on.operationsscope is documented with a gap that is someone else's bug. It narrows the minted token on the Operations API and SQL paths but not on an application's REST/GraphQL resources, so it bounds what a CI credential can administer, not what data it can reach. harper#2201 tracks that. Documenting it as-is means the docs describe a partial control; softening it would misrepresent what the flag does.drop_oidc_trustdoes not revoke issued tokens, and the docs say so with the remediation (deactivate or re-role the user). That is a real operational sharp edge rather than a doc nicety; worth confirming the guidance matches what you'd want an operator doing during an incident.Deliberately left on #599
The deployment status groupings look like generic tracking corrections —
list_deploymentsandget_deploymentare shipped inmain— but they enumeratestaged,staging,activating,reverting,extractingandloading, whilemain's code defines onlypending,installing,replicating,restarting,success,failedandrolled_back. Extracting them would publish statuses that do not exist. Same for the payload/staging retention text, which mixes a shipped bound with an unshipped one.Verification
Docs-only; per
AGENTS.mdthe build is the end-to-end check.npm run format:checkclean,npm run buildsucceeds with the two anchors that pre-exist onmainand no others.Claims verified against harper
feat/oidc-trusted-publishingat its merge state, not the PR description — which had drifted, describing anissuer|jtireplay key and an unimplemented audit-log path, both since changed:security/authn/oidc/trustPolicyOperations.tsjob_workflow_refexcluded from the ref gate;pull_request_targetvetoproviders/githubActions.tssubproviders/generic.tstokenExchange.tsoperationsscope enforced atverifyPerms/verifyPermsAST, not the resource pathsecurity/authn/oidc/types.tsbin/workloadIdentity.ts,bin/cliOperations.tsnpm run buildDescription drafted by Claude Code (Opus 5).