-
Notifications
You must be signed in to change notification settings - Fork 9
docs(roles): state the limits of the operations allowlist #631
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
c4c697f
ac0376c
4a0675e
f5e6ad7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -79,6 +79,40 @@ The `operations` field in a permission object restricts which Operations API cal | |
|
|
||
| Operations normally restricted to `super_user` can be selectively granted by including them in the list. If `operations` is not set, the role can call any non-`super_user` operation, subject to table CRUD permissions. | ||
|
|
||
| <VersionBadge type="changed" version="v5.3.0" /> | ||
|
|
||
| An operation the list omits is denied whatever else the role carries — the list is checked ahead of every other permission on the role. Earlier v5 releases let table DDL and SQL around it; both now go through it. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Confirmed, and it reproduces exactly as you describe. Both helpers short-circuit on // components/mcp/toolRegistry.ts:413 — canRoleInvokeOperation
if (perm.structure_user && SCHEMA_STRUCTURE_OPERATIONS.has(operation)) return true;
if (Array.isArray(perm.operations) && perm.operations.includes(operation)) return true;
One correction to the framing: this does not conflict with the invariant on this page. The sentence here is that an omitted operation is denied — which is what happens, as your own "rejects every invocation" confirms. The promise it does contradict is on the other page — On the fix itself: that is a core change in 🤖 Addressed by Claude Code
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @kriszyp Would you consider this to block this PR? |
||
|
|
||
| So grant by listing, and build a role up rather than trying to narrow `super_user` — `add_role` and `alter_role` reject `super_user` or `cluster_user` set to `true` alongside any other key. A role that maintains one database's tables and queries them with SQL: | ||
|
|
||
| ```json | ||
| { | ||
| "operation": "add_role", | ||
| "role": "orders_maintainer", | ||
| "permission": { | ||
| "operations": ["sql", "create_table", "drop_table"], | ||
| "structure_user": ["orders_db"], | ||
| "orders_db": { | ||
| "tables": { | ||
| "orders": { | ||
| "read": true, | ||
| "insert": true, | ||
| "update": false, | ||
| "delete": false, | ||
| "attribute_permissions": [] | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| `sql` has to be listed or the role cannot run SQL at all. Listing it grants the interface, not the data: the statement is still checked against the table permissions above, so this role can `SELECT` and `INSERT` on `orders` and nothing else. That check is what separates the `read_only` and `standard_user` groups below, which both include `sql`. | ||
|
|
||
| `create_table` and `drop_table` have to be listed too, and `structure_user` then limits them to `orders_db`. `create_database` and `drop_database` additionally require `structure_user: true`. | ||
|
|
||
| The value must be an array of strings; a non-array value is rejected on write. A role that already holds one — a pre-5.0 role that granted a database named `operations`, before the key became reserved — can stop the instance loading its user cache ([harper#2194](https://github.com/HarperFast/harper/issues/2194)). | ||
|
|
||
| **Permission Groups** | ||
|
|
||
| Groups expand to a predefined set of operations and can be mixed with individual operation names: | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The first affected release is
v5.2.5, notv5.3.0. The tagged core sources establish that v5.2.4 lacks both the DDL/SQL allowlist enforcement and v5.2.5 contains them. Keeping this badge, with the corresponding warning atrelease-notes/v5-lincoln/5.3.md:35, leaves 5.2.4 -> 5.2.5 upgrades without an alert to listsqland DDL operations before access is revoked. Please badge thisv5.2.5and relocate the release-note warning to the 5.2.5 notes.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not seeing this —
v5.2.4andv5.2.5are byte-identical on both files that would carry the enforcement:And the enforcement is absent from the whole 5.2 line, not just 5.2.4. At
v5.2.13:verifyOperationsAllowlistdoes not exist at any 5.2.x tag (v5.2.3throughv5.2.13all absent).sql.checkASTPermissions(json, parsedSqlObject)with no allowlist consultation —expandOperationsPermsappears zero times inserverUtilities.ts.verifyPerms, thestructure_usercarve-out is still at line 604 and theoperationsgate at line 628, so DDL continues to bypass the list.The change is harper#2176, merged to
main2026-08-21 and tagged only inv5.3.0-alpha.1. I checked for a back-port by file content rather than ancestry, since a cherry-pick would defeatgit tag --contains— that is the failure mode your reading would fit, but the files show no cherry-pick happened.So a 5.2.4 to 5.2.5 upgrade revokes nothing, and a warning in the 5.2.5 notes would tell operators to change roles for a tightening their release does not contain. Keeping the badge at
v5.3.0. Happy to look again if you have a specific 5.2.5 behavior in mind that I am reading past.🤖 Addressed by Claude Code