Repository navigation
fix(roles): the operations editor knows what Harper 5.3's allowlist grants - #1884
Conversation
![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 version-aware operations allowlist handling to differentiate between Harper versions before and after 5.3.0. It replaces the boolean support check with an AllowlistVersion object containing both support status and a version band (pre-5.3 or 5.3), updating the catalog, context, and UI components to accurately reflect which operations are delegable or inert based on the instance version. Feedback on these changes suggests optimizing the useMemo hook in useAllowlistVersion to depend on the version and a boolean flag instead of the entire registrationInfo object, preventing unnecessary re-renders when the query refetches.
| return useMemo( | ||
| () => | ||
| registrationInfo === undefined | ||
| ? undefined | ||
| : { supported: supportsOperationsAllowlist(version), band: resolveGrantBand(version ?? '') }, | ||
| [registrationInfo, version], | ||
| ); |
There was a problem hiding this comment.
The useMemo hook currently depends on the entire registrationInfo object. Since registrationInfo is returned by useQuery, its reference can change on refetches (even if the version remains the same) due to other dynamic fields or query updates. This causes useAllowlistVersion to return a new object reference, triggering unnecessary re-renders for all consumers of AllowlistSupportedContext (such as every row in the roles table).
Optimizing the dependency array to only track version and whether registrationInfo is defined will stabilize the returned object's identity.
const hasData = registrationInfo !== undefined;
return useMemo(
() =>
hasData
? { supported: supportsOperationsAllowlist(version), band: resolveGrantBand(version ?? '') }
: undefined,
[hasData, version],
);There was a problem hiding this comment.
Declining: the premise doesn't hold here. TanStack Query's default structural sharing keeps data's reference across a refetch whose result is deep-equal (replaceData → replaceEqualDeep, @tanstack/query-core@5.104.1 build/modern/utils.js:171-183), nothing in src/ overrides structuralSharing, and registration_info returns only { version, deprecated: true } (harper resources/registrationDeprecated.ts:3-8). So registrationInfo changes identity only when its content does, and the memo's output would have to change then anyway.
🤖 Generated by Claude Code (Claude Opus 5.5); posted via @dawsontoth.
…rants Harper 5.3.0-beta.3 names every authorization entry (harper feb8e8378, a6844db7a), so deploy_component and 19 other operations that the editor called inert on every 5.x are granted by an allowlist from then on. The editor would have told someone their deploy-only CI role could not deploy. The catalog now records, per entry, where listing a name starts or stops matching the gate, and callers resolve the instance version into a band once: - the 20 newly named operations need 5.3 (badged "needs Harper 5.3" below it, real super_user delegations from it); - catchup matched through the old fallback and is named null from 5.3; - get_backup and read_transaction_log are never granted, so they are no longer offered; - delete_records_before is an alias of delete_files_before (it dispatches to that handler), and drop_custom_function_project is not an alias of drop_component (it has its own handler and grant name); - add_oidc_trust, list_oidc_trust and drop_oidc_trust are catalogued as super_user only; their handlers enforce it. Refs #1788 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The "needs Harper 5.3" badge also showed on 5.3.0-alpha.1 through beta.2, which run 5.3 but predate the gate change. The badge now says "needs newer Harper" and its title names the exact first build. Also corrects AGENTS.md's put note, which said registration_info is gate-inert: from 5.3 an allowlist that lists it grants it. Closes #1877 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…the allowlist From Harper 5.3.0-alpha.1 (harper f8a5aa90a), verifyPerms runs a role's operations allowlist ahead of its super_user and structure_user returns, and chooseOperation runs it on `sql`. The editor and the assignment summary still said a structure user reaches DDL regardless of the list and that the list does not restrict SQL, and the effective count left `sql` out. GrantBand gains a `5.3-early` band (5.3.0-alpha.1 to beta.2) between `pre-5.3` and `5.3`, so the gate-order change and the beta.3 naming change are both resolved from one band. The structure-user, SQL and empty-list copy, the `sql` chip and the effective count follow it. The permission hooks keep their behavior; their comments and AGENTS.md now say why on each version, and #1894 tracks per-operation checks for 5.3. Closes #1885 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…change Nothing outside the catalog reads ALLOWLIST_GATE_RUNS_FIRST_FROM, and the GateChange doc still said "before 5.3" for a change that landed in 5.3.0-beta.3, which the new 5.3-early band makes ambiguous. Refs #1885 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ist gates its DDL On the 5.3 bands the role summary said only that the structure-user DDL needs listing, dropping the database scope the pre-5.3 copy and the editor's gated note both keep. verifyPerms still applies the structure_user array after gate 1 (harper utility/operation_authorization.ts:810-824), so a listed DDL operation reaches only those databases. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…wlist preparePermissionForSave's doc still described the pre-5.3 gate order as universal; from 5.3.0-alpha.1 Harper gates the structure-user DDL on the list too. Also drops a PR-number reference from a test comment in favour of the boundary it means. Comment-only; raised by the pre-push domain adjudication. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
883ff75 to
bc31bfe
Compare
Ports the Jobs tab and the CSV import job toast onto stage, which now has #1868's structured job messages and #1884's grant bands. - waitForJob keeps both behaviors: every poll still reaches onUpdate, the ERROR one included, and a structured job message is read as text (errorText) for the rejection and for the import's success message. A test pins the two together. - #1884 removed isGrantGateInert. canListJobs now takes the instance version and honours a listed search_jobs_by_start_date only where gateVerdict(name, resolveGrantBand(version)) === 'grants', so from 5.3.0-beta.3 on a 5.3 allowlist role can list jobs and a 5.2 one cannot. JobsTab reads the version (already loaded by the instance route) only for a role that lists the grant. If a refetch of it fails, the request is tried instead of refused outright, and a 403 shows the same explanation. - The uploaded-CSV path's progress wiring gets a test; only the URL load had one. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 participant
The role editor now knows which operations a role's
operationsallowlist grants on each Harper version. From Harper 5.3.0-beta.3,deploy_componentand 19 other operations are granted by an allowlist. The editor, the roles table and the user-assignment summary called them inert on every 5.x, so they told an admin that a deploy-only CI role couldn't deploy.Closes #1877. Refs #1788 (this is the roles part of it; the trust-policy page and the CLI tab are #1886, also off
stageand independent of this one; the deploy panel is not built).⊙ Problem
GATE_INERT_OPERATIONSwas one fixed list. Harper 5.3.0-beta.3 gave every authorization entry anapi_name, in harperfeb8e8378("Let a role's operations allowlist grant deploy_component and 19 more") anda6844db7a. Both are in every tag from v5.3.0-beta.3 on (git tag --contains). So on 5.3:GRANTED_BY_NAMEinunitTests/utility/operation_authorization.test.jsat v5.3.0.catchupchanges the other way. Before 5.3 it matched because it was registered under its wire name. From 5.3 itsapi_nameisnull, so no allowlist can grant it.get_backupandread_transaction_logare never granted on any version. Harper keeps themnullon purpose:utility/operation_authorization.tsat v5.3.0, the comments on thegetBackupandreadTransactionLogentries.delete_records_beforerunsdelete_files_before's handler (server/serverHelpers/serverUtilities.ts,DELETE_RECORDS_BEFORE→deleteFilesBefore). So it is an alias, granted by listingdelete_files_before.drop_custom_function_projectwas catalogued as an alias ofdrop_component, but it has its own handler (components/operations.js:308) and its own grant name.add_oidc_trust,list_oidc_trustanddrop_oidc_trust(5.3.0-alpha.1+) were not in the catalog. Their handlers require super_user (requireSuperUserinsecurity/authn/oidc/trustPolicyOperations.ts), so no allowlist can delegate them.exchange_oidc_tokenskips permission checks, likelogin(serverUtilities.tsat v5.3.0, ~403-411).💡 Solution
Each catalog entry now records where its gate behaviour changes (
gate: 'from-5.3' | 'until-5.3' | 'never'). Callers turn the instance version into aGrantBandonce, instead of parsing SemVer for every operation (the planning review asked for this), andgateVerdict(name, band)gives the answer.The operations a version predates are still offered and marked, as before, so a role is ready for an upgrade.
⚖️ Alternatives
catchup,delete_records_beforeanddrop_custom_function_projectwrong on 5.3.🔧 Changes
operationsCatalog.ts):GateChange,resolveGrantBandandgateVerdictreplaceGATE_INERT_OPERATIONSandisGrantGateInert.isInertGrantandexpandEffectiveOperationstake the band, andgetGrantableOperationsdrops never-granted names.gatevalues are on the entries themselves, for exampledeploy_component,catchupandget_backup.delete_records_beforeanddrop_custom_function_project.exchange_oidc_tokenexcluded likelogin.useAllowlistVersion()returns{ supported, band }, memoized.useOperationsAllowlistSupported()still works for callers that only need the boolean.OperationsAllowlistEditor.tsx): the badge and its title, the never-granted chip title, and the check on the custom-grant path.putnote no longer callsregistration_infogate-inert, which Role editor still calls 23 operation grants inert on Harper 5.3+, where Harper honours them #1877 asked to re-check.✅ Verification
operationsCatalog.test.ts: the band at 5.3.0-alpha.1, beta.1, beta.2, beta.3, beta.4, 5.3.0, 5.3.2, 5.4, 6.0 and unreadable versions. The exact 20needs-5.3names, which match Harper'sGRANTED_BY_NAME. Theneverset on each side. Both sides ofcatchup. The aliases. The OIDC operations never offered.OperationsAllowlistEditor.test.tsx: the badge on both sides;catchupon 5.3; never-granted names refused through the custom-grant path; the OIDC operations not offered.RoleOperationsSummary.test.tsx: the same deploy role counts 2 operations on 5.3 and 1 before it.pre-5.3failed 4 tests. Removing theneverfilter from the picker failed 3. Removing the custom-grantnevercheck failed 1.{ operations: ['deploy_component', 'get_job'] }ranget_joband was refuseddeploy_componentwith "Operation 'deployComponent' is not permitted". That is the pre-5.3 side as the catalog describes it. No 5.3.0-beta.3+ build was available locally, so the 5.3 side rests on Harper's own v5.3.0 unit test.npx tsc -bexit 0.wasAReleasedBeforeBmay be strict", because it isb >= a(src/lib/string/wasAReleasedBeforeB.ts) and the beta.3 case is tested; and two micro-allocation notes on code paths this PR does not change. Coverage gap: the Cursor legs failed on the 1Password SSH agent (no-receipt), and the Harper domain adjudicator failed auth (an expired Claude OAuth session), so no adjudication ran.🤖 Generated with Claude Code
Related PRs: #1880 overlaps (calls
isGrantGateInert, which this removes), #1886 independent (the other half of #1788)Complexity: medium
Review-Coverage: authored=claude; ran=codex,gemini; blocked=cursor-composer(no-receipt),domain(auth); declined=cursor-grok,cursor-kimi,cursor-muse; rounds=2; full=1 @ ae81c42
Review-Attention: read ~6m (raised: degraded review) @ ae81c42