docs(roles): state the limits of the operations allowlist - #631
dawsontoth wants to merge 3 commits into
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 updates the documentation in reference/users-and-roles/overview.md to clarify the scoping behavior and limitations of the operations field in permission objects. The review feedback suggests clarifying the explanation of how super_user roles interact with the allowlist to avoid misleading phrasing, and recommends using a VersionBadge component to properly document the behavior change introduced in version 5.0.0.
🚀 Preview DeploymentYour preview deployment is ready! 🔗 Preview URL: https://preview.harper-documentation.harperfabric.com/pr-631 This preview will update automatically when you push new commits. |
harper#2176 moved gate one ahead of every other privilege check on the role, so the two limits this page originally described no longer hold: the structure_user carve-out and the SQL path both consulted the allowlist as of that change. Documents the 5.3.0 semantics instead — structure_user narrows which databases listed DDL reaches rather than widening which operations are reachable, and sql must be listed for a role to run SQL at all. The one limit that survives is that super_user/cluster_user roles cannot carry an allowlist: add_role and alter_role reject either flag set to true alongside other keys, so a 'restricted super user' is a validation error rather than a narrowed administrator. Drops the earlier claim about what authorization does with such a role, which described a state validation will not store and which #2176 inverted. Also notes that a non-array value can wedge the user-cache load (harper#2194), and adds the 5.3 release note, since the tightening revokes access from roles that relied on either bypass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
64c8bcb to
c4c697f
Compare
🚀 Preview DeploymentYour preview deployment is ready! 🔗 Preview URL: https://preview.harper-documentation.harperfabric.com/pr-631 This preview will update automatically when you push new commits. |
Matches the shape reference/database/compaction.md uses for the same kind of note: the badge on its own line, then a sentence naming what changed and an "earlier v5 releases" clause for the prior behavior. The badge was sitting mid-sentence immediately before a literal "Before v5.3.0", which rendered as "Changed in: v5.3.0 Before v5.3.0 ...". Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
🚀 Preview DeploymentYour preview deployment is ready! 🔗 Preview URL: https://preview.harper-documentation.harperfabric.com/pr-631 This preview will update automatically when you push new commits. |
The three bullets explained gate ordering and which path bypassed what — the mechanism, not the task. A developer arriving here is writing a role and wants to know what to put in the list. Replaces them with one role that exercises all three limits (sql listed, DDL listed, structure_user scoping the DDL to one database) and two sentences of annotation. The super_user limit becomes the reason to build a role up rather than a bullet about validation. Same for the release note: what broke and the two cases to audit, rather than a narrative of the ordering change. Verified while rewriting that listing `sql` alone admits any statement variant on a direct SQL call — the allowlist checks `sql`, and the statement is then authorized against table permissions (harper DESIGN.md:2536), so the example can show SELECT/INSERT reaching only what the table grants. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
🚀 Preview DeploymentYour preview deployment is ready! 🔗 Preview URL: https://preview.harper-documentation.harperfabric.com/pr-631 This preview will update automatically when you push new commits. |
dawsontoth
added
waiting-for-companion
|
Ready for review — but please do not merge this yet, even with an approval.
Please also do not arm auto-merge. Happy to flip it back to draft if that is a safer place to park it while it waits. Worth a careful look at two things:
🤖 Generated with Claude Code |
1 participant
Documents the limits of a role's
permission.operationsallowlist onreference/users-and-roles/overview.md, plus the 5.3 release note for the tightening that produced them.Companion to HarperFast/studio#1628 (merged), which surfaces
permission.operationsin the Studio roles UI. Building that UI meant establishing the field's real semantics against the Harper source, and several of them contradict a natural reading of this page.reference/is the published/reference/v5site, and this documents behavior that is merged toharpermainbut present only inv5.3.0-alpha.1. Latest stable isv5.2.13. Held as a draft until 5.3.0 is tagged; the companion-check gate is not the right mechanism here, because the companion PR has already merged and the marker would pass immediately.Why this was rewritten
The original version of this PR described 5.2.x behavior. Three days after it was opened, harper#2176 moved allowlist gate one ahead of every other privilege check on the role and gave the SQL path its own call into it. That invalidated the two substantive limits the page had claimed:
structure_userbypasses the allowlist for DDLstructure_userthen scopes it to its databasessqlis authorized on its own path and never consults the allowlistsqlmust be listed for a role to run SQLThe third claim survives unchanged:
super_user/cluster_userroles cannot carry an allowlist, becausevalidateNoSUPermsrejects either flag set totruealongside other keys. The page's earlier sentence about what authorization then does with such a role is dropped — it described a state validation will not store, and #2176 inverted it anyway.What changes
reference/users-and-roles/overview.md— the three limits, with<VersionBadge type="changed" version="v5.3.0" />on the ordering change that produces two of them. Also states that the value must be an array, and points at harper#2194 for the non-array case, where a role carrying one can stop the instance loading its user cache.release-notes/v5-lincoln/5.3.md— a### Operation Allowlist Enforcementnote under## Security. The tightening revokes access from any role that relied on either bypass, so it owes an upgrade caveat: audit roles combiningoperationswithstructure_user, and roles whose list omitssql.Verification
Re-derived from the source at
v5.2.13andorigin/main(5.3.0-alpha.1), not from PR descriptions:utility/operation_authorization.ts—verifyOperationsAllowlist(new in #2176) runs after the empty-role check and before every privilege early-return; its own comment names the ordering requirement. Gate 2 (the SU-only-op grant) stays below the ambient privilege checks.server/serverHelpers/serverUtilities.ts:362— the SQL path callsverifyOperationsAllowlist(json, json.operation)directly, ahead ofcheckASTPermissions.validation/role_validation.ts—validateNoSUPermsgates onsuper_user === true/cluster_user === truewith more than one permission key (hence "set totrue" in the text;super_user: falsealongside an allowlist is fine).OPERATIONS_MUST_BE_ARRAYrejects a non-array. Unchanged betweenv5.2.13andmain.api_namemappings (CREATE_TABLE,DROP_DATABASE, …) are registered, so gate one resolves the camelCase handler names to the snake_case names an allowlist is written in.package.json: #2176 is merged,git tag --containsreturns onlyv5.3.0-alpha.1, androle_validation.ts/operationPermissions.tsconfirm the shippedv5.2.13behavior differs. Ships inv5.3.0.npm run format:checkclean;npm run buildsucceeds (407 documents), which also validates the new cross-link and the badge.Review coverage
Written by an LLM (originally Claude Fable 5; re-verified and rewritten by Claude Opus 5). Prose-only, two files. The underlying 5.2.x findings were reviewed by @cb1kenobi and @kriszyp on studio#1628; the 5.3.0 semantics here are newly derived and have not been reviewed by a human.
Both Gemini review comments are addressed — see the replies inline.
🤖 Generated with Claude Code