-
Notifications
You must be signed in to change notification settings - Fork 4
feat(roles): surface operation-level grants (permission.operations) in the roles/users UI #1628
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
Merged
Merged
Changes from all commits
Commits
Show all changes
22 commits
Select commit
Hold shift + click to select a range
cba1593
feat(roles): surface operation-level grants (permission.operations) i…
dawsontoth f0a2697
refactor(roles): address Gemini review on the operations lens
dawsontoth bb2933f
fix(roles): address Codex cross-model review of the operations editor
dawsontoth 24006ab
refactor(roles): hoist per-database permission lookup and harden the …
dawsontoth 7eff23f
fix(roles): stop claiming the operations allowlist restricts elevated…
dawsontoth bbde522
fix(roles): correct allowlist semantics per review — save conflict, s…
dawsontoth dc2c6a5
fix(roles): tighten allowlist edges from Kris's and Chris's review
dawsontoth 993e175
fix(roles): make the operations verdict version-aware and cover the _…
dawsontoth 5c71e54
fix(roles): qualify the empty-allowlist sentence for structure_user r…
dawsontoth 61d068b
fix(roles): unify the operations predicates and stop guessing while t…
dawsontoth b7c01cf
fix(roles): keep table-permission checks version-blind, and test the …
dawsontoth 6fe0c48
fix(roles): stop advising a "fix" that breaks the role, and test the …
dawsontoth e8cea62
fix(roles): share the collision notice so the editor stops giving des…
dawsontoth 2f9f747
fix(roles): state the real consequence of an unusable operations value
dawsontoth 9bd8ab9
fix(roles): separate the fatal operations values from the merely inva…
dawsontoth 5afabca
fix(roles): correct the allowlist remedy, the gate claim, and the flo…
dawsontoth b53ce8e
docs(roles): record the operations allowlist semantics in AGENTS.md
dawsontoth 701cecf
fix(roles): correct the floor gate I added, and reserve the outage wa…
dawsontoth bcfb362
fix(roles): block saves and assignments that would widen or break access
dawsontoth 186300a
fix(roles): make the inert-grant notion one predicate, and stop contr…
dawsontoth 7a15831
fix(roles): drop catchup from the inert set, and guard the branches t…
dawsontoth 6da21ff
fix(roles): make the collision remedy data-safe and stop counting ung…
dawsontoth File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
67 changes: 67 additions & 0 deletions
67
src/features/instance/config/roles/defaultCalculator.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,67 @@ | ||
| import { calculateDefaultPermissions } from '@/features/instance/config/roles/defaultCalculator'; | ||
| import { InstanceDatabaseMap, LocalRolePermission } from '@/integrations/api/api.patch'; | ||
| import { orderPermissionKeys, withOperations } from '@/integrations/api/localRolePermission'; | ||
| import { describe, expect, it } from 'vitest'; | ||
|
|
||
| // JSON.parse, not an object literal: a literal `__proto__` key sets the prototype instead of | ||
| // creating an own property, so the fixture itself has to come from parsed JSON. | ||
| const databaseMap = (json: string) => JSON.parse(json) as InstanceDatabaseMap; | ||
| const oneTable = '{"dog":{"attributes":[{"attribute":"id"}]}}'; | ||
|
|
||
| describe('calculateDefaultPermissions', () => { | ||
| it('keeps a database named __proto__ as an own, serializable key', () => { | ||
| // Plain assignment would hit the prototype setter: the database would vanish from the | ||
| // template and from JSON.stringify, so the role could never be granted it. | ||
| const result = calculateDefaultPermissions({ | ||
| instanceDatabaseMap: databaseMap(`{"__proto__":{"dog":{"attributes":[{"attribute":"id"}]}}}`), | ||
| currentRolePermissions: {}, | ||
| version: '5.2.2', | ||
| showAttributes: false, | ||
| }); | ||
|
|
||
| expect(Object.hasOwn(result, '__proto__')).toBe(true); | ||
| // Read the own property explicitly: `parsed.__proto__` would resolve the prototype instead. | ||
| const serialized = JSON.parse(JSON.stringify(result)) as object; | ||
| const record = Object.getOwnPropertyDescriptor(serialized, '__proto__')?.value as | ||
| | { tables: Record<string, unknown> } | ||
| | undefined; | ||
| expect(record?.tables).toHaveProperty('dog'); | ||
| // The round trip the editor actually performs must preserve it too. | ||
| const roundTripped = JSON.parse(JSON.stringify(withOperations(orderPermissionKeys(result), ['sql']))); | ||
| expect(Object.hasOwn(roundTripped, '__proto__')).toBe(true); | ||
| }); | ||
|
|
||
| it('skips a database named like a reserved key when the instance reserves it', () => { | ||
| const map = databaseMap(`{"operations":{"dog":{"attributes":[{"attribute":"id"}]}}}`); | ||
| // 5.2 reserves `operations` for the allowlist, so writing the database there would clobber it. | ||
| expect( | ||
| calculateDefaultPermissions({ | ||
| instanceDatabaseMap: map, | ||
| currentRolePermissions: { operations: ['sql'] }, | ||
| version: '5.2.2', | ||
| showAttributes: false, | ||
| }).operations, | ||
| ).toEqual(['sql']); | ||
|
|
||
| // Below the allowlist floor the same name is just a database, so it gets a permission record. | ||
| const legacy = calculateDefaultPermissions({ | ||
| instanceDatabaseMap: map, | ||
| currentRolePermissions: {}, | ||
| version: '4.7.3', | ||
| showAttributes: false, | ||
| }).operations as unknown as { tables: Record<string, unknown> }; | ||
| expect(legacy.tables).toHaveProperty('dog'); | ||
| }); | ||
|
|
||
| it('preserves an existing allowlist while rebuilding table permissions', () => { | ||
| const permission: LocalRolePermission = { operations: ['read_only'] }; | ||
| const result = calculateDefaultPermissions({ | ||
| instanceDatabaseMap: databaseMap(`{"data":${oneTable}}`), | ||
| currentRolePermissions: permission, | ||
| version: '5.2.2', | ||
| showAttributes: false, | ||
| }); | ||
| expect(result.operations).toEqual(['read_only']); | ||
| expect(result.data).toBeDefined(); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.