Repository navigation
feat(config): list, disable and drop OIDC trust policies - #1886
dawsontoth wants to merge 3 commits into
Conversation
Harper 5.3 lets a CI workflow sign in with its OIDC identity token under a trust policy, and Studio had no way to see those policies. Config now has a Trust Policies page for super users on 5.3+, built on list_oidc_trust and drop_oidc_trust: what each policy trusts, the user it signs in as, what its token may run, and Harper's invalid_reason when it refuses every token. Disable and Enable re-save the policy through add_oidc_trust, which replaces the whole row. The policy is read again first and the write is refused if it changed since the page loaded it, so a stale copy cannot undo another administrator's edit. A malformed scope is never dropped on re-save, since that would widen the token. Refs #1788 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
New Application → CLI still passed --deploymentUsername and told people to save their cluster password in .env. create-harper dropped both in 1.7.0: it takes the cluster URL, offers to run harper login, and keeps the token in the home directory. The steps now say that, and the deploy step says it uses that login. Refs #1788 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
From review of the trust policies page: - Drop re-reads the policy too, and refuses if it changed since the page loaded it, so a confirmation can't drop a policy recreated under the same id. - The stale check compares the whole row, keys sorted, rather than a fixed field list, and a re-save sends back every stored field, including any a newer Harper adds, so toggling never erases one. - Actions stay disabled while the list refetches after a write. - An unreadable Harper version says so instead of loading forever. - The drop confirmation names the policy. Refs #1788 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
![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 support for OIDC trust policies in Harper 5.3+, allowing external CI workflows to authenticate using OIDC identity tokens. It adds a new "Trust Policies" configuration page to list, enable, disable, and drop policies, along with corresponding API integrations, routes, and comprehensive tests. Additionally, the CLI setup steps are refactored to align with the new harper login flow, removing the username and password from the project's local environment configuration. Feedback on the changes highlights a potential issue in the stableJson helper function used for detecting stale writes, where omitted properties and properties explicitly set to null are treated differently, potentially triggering false-positive out-of-date errors. A robust serialization fix has been suggested to resolve this.
| function stableJson(value: unknown): string { | ||
| if (Array.isArray(value)) { | ||
| return `[${value.map(stableJson).join(',')}]`; | ||
| } | ||
| if (value !== null && typeof value === 'object') { | ||
| const record = value as Record<string, unknown>; | ||
| return `{${ | ||
| Object.keys(record).sort().map((key) => `${JSON.stringify(key)}:${stableJson(record[key])}`).join(',') | ||
| }}`; | ||
| } | ||
| return JSON.stringify(value) ?? 'null'; | ||
| } |
There was a problem hiding this comment.
The 'stableJson' function is used to compare the loaded policy with the current policy from the server to detect stale writes. However, optional fields like 'description' or 'updated_by' can be 'null' or omitted ('undefined') depending on how they are returned by the server or normalized by the client. The current implementation of 'stableJson' treats '{}' and '{ description: null }' as different strings, which will trigger false-positive 'changed since this page loaded it' errors and block users from updating or dropping policies. Filtering out 'null' and 'undefined' values from the object keys before sorting and serializing will make the comparison robust and prevent these false-positives.
| function stableJson(value: unknown): string { | |
| if (Array.isArray(value)) { | |
| return `[${value.map(stableJson).join(',')}]`; | |
| } | |
| if (value !== null && typeof value === 'object') { | |
| const record = value as Record<string, unknown>; | |
| return `{${ | |
| Object.keys(record).sort().map((key) => `${JSON.stringify(key)}:${stableJson(record[key])}`).join(',') | |
| }}`; | |
| } | |
| return JSON.stringify(value) ?? 'null'; | |
| } | |
| function stableJson(value: unknown): string { | |
| if (Array.isArray(value)) { | |
| return '[' + value.map(stableJson).join(',') + ']'; | |
| } | |
| if (value !== null && typeof value === 'object') { | |
| const record = value as Record<string, unknown>; | |
| const keys = Object.keys(record).filter((key) => record[key] != null); | |
| return '{' + | |
| keys.sort().map((key) => JSON.stringify(key) + ':' + stableJson(record[key])).join(',') | |
| + '}'; | |
| } | |
| return JSON.stringify(value) ?? 'null'; | |
| } |
Coverage Report
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
1 participant
Config has a new Trust Policies page for super users on Harper 5.3+. It lists the cluster's OIDC trust policies, and each one can be disabled, enabled or dropped. A policy is what lets a GitHub Actions run deploy without a stored secret, by signing in with the run's OIDC identity token. New Application → CLI is also updated to create-harper's current flow:
harper login, with no username flag and no password in.env.Refs #1788. This is part of it. The roles-catalog part is #1884, off
stageand independent of this one. The "Deploy from GitHub" setup panel is not built; see the first decision below.⊙ Problem
Harper 5.3 added
add_oidc_trust/list_oidc_trust/drop_oidc_trust(first tag v5.3.0-alpha.1). Studio couldn't show what a cluster trusts, couldn't show that a policy can never match, and couldn't revoke one. Separately, the CLI tab told people to pass--deploymentUsernameand save their cluster password in.env. create-harper removed both in v1.7.0 (d793dbc, "feat: Use harper login"). It now takes the cluster URL, offers to runharper login, and the login lives in~/.harperdb/credentials.json(harperbin/cliCredentials.ts).💡 Solution
security/authn/oidc/types.ts. When Harper reportsinvalid_reason, the page shows it and offers only Drop, because re-saving would fail the same check. Rows are rendered as text whatever their shape, because Harper reports malformed values as stored.add_oidc_trust, which replaces the whole row. The request sends back every stored field, including ones a newer Harper may add. It leaves out the fields Harper sets itself and nulls, which the validator rejects. A malformed scope is refused rather than dropped, since dropping it would store the policy unscoped. The button's title says a token already issued lasts out its hour (EXCHANGED_TOKEN_LIFETIME_SECONDS = 3600,tokenExchange.ts).buildCLISteps): only--deploymentURLis passed. The notes now explain taking theharper loginoffer and that the deploy uses that login.harper deployreadsHARPER_CLI_TARGETfrom.envand the saved tokens for it (bin/cliOperations.tsresolveTarget).⚖️ Alternatives
__updatedtime__? Rejected: comparing the whole row also catches a row replaced under the same id with an equal timestamp, at the cost of one sort.🔧 Changes
src/integrations/api/instance/oidcTrust/oidcTrust.ts: the list query, the re-save request, the stale-row check, the toggle and drop mutations, and the version floor.src/features/instance/config/trustPolicies/index.tsx: the page, the policy card, the drop confirmation (ConfirmDeletionModalnaming the policy), and toasts on success. Failures toast through the mutation cache, and the list refetches either way.src/features/instance/config/routes.tsandsrc/features/instance/config/index.tsx: theconfig/trust-policiesroute and its nav entry.getParentRouteandaddChildrenstay in lockstep.src/features/instance/applications/components/NewApplication/useCLISteps.tsx: a purebuildCLIStepswith the updated steps.useCLIStepsonly resolves the target.✅ Verification
oidcTrust.test.ts: the version floor; the re-save request (every field, no nulls, a future field carried, a malformed scope refused); the stale check (changed time, changed field, reordered claims accepted, dropped row); the drop refused on a replaced row.trustPolicies/index.test.tsx: no list query for a non-super user, on 5.2, or with an unreadable version; the rendered fields; the unscoped copy; an invalid policy offered only Drop; a malformed row rendered as text; toggle and confirmed drop delegating the loaded row; actions disabled while refetching; the empty and error states.useCLISteps.test.ts: no--deploymentUsernameor.envpassword copy; the login copy; no URL flag when there is no target.ROOTPATHandHOME. The file was deleted afterwards. Results:add_oidc_trust→ list showed the policy, enabled, with noinvalid_reason.setTrustPolicyEnabled(false)→enabled: false, every other field unchanged.enabled: true.operations: null.invalid_reason: "names inactive user 'smoke-ci-deploy'".drop_oidc_trust→ gone.npx tsc -bexit 0.instanceParamsunused" inuseCLISteps(it supplies Local Studio's target); the remaining comment nits (the comments state the super-user enforcement, the replication scope and the version floor).no-receipt), and the Harper domain adjudicator failed auth (an expired Claude OAuth session).🤖 Generated with Claude Code
Related PRs: #1884 independent (roles-catalog half of #1788, also off
stage)Complexity: medium
Review-Coverage: authored=claude; ran=gemini,codex; blocked=cursor-grok(no-receipt),cursor-composer(no-receipt),domain(auth); declined=cursor-kimi,cursor-muse; rounds=2; full=1 @ 48cb0d7
Review-Attention: study ~9m (raised: degraded review) @ 48cb0d7