fix: preserve permission ordering by accepting a layered array#23214
Conversation
asuffield
force-pushed
the
fix-permissions
branch
3 times, most recently
from
ca34b07 to
687dd4e
Compare
…erve permissions objects as layers, convert them individually to rulesets, and merge the rulesets (anomalyco#16157)
asuffield
force-pushed
the
fix-permissions
branch
from
687dd4e to
809e1e5
Compare
Replace the internal permission_layers config field with a union on permission itself (single object or array of layered configs). Add ConfigPermission.toLayers to normalise at consumption sites. Schema has no decode transform, so user files round-trip through Config.update / updateGlobal without their permission section being rewritten into array form.
|
slight change, instead of a separate config key merging them together w/ a union |
rekram1-node
changed the title
|
/review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| @@ -708,11 +720,12 @@ export const layer = Layer.effect( | |||
| } | |||
There was a problem hiding this comment.
Suggestion: normalize the parsed env value with ConfigPermission.toLayers here too. If OPENCODE_PERMISSION uses the new array syntax, this appends the whole array as a single layer, so later consumers pass that array into Permission.fromConfig and interpret numeric array indexes as permission names.
| // This preserves the ordering semantics: later rules override earlier rules. | ||
| // Each layer keeps the raw shape the user wrote on disk; consumers should use | ||
| // ConfigPermission.toLayers to normalise. | ||
| if (source.permission) { |
There was a problem hiding this comment.
Suggestion: consider using this layered merge in loadGlobal as well. Right now config.json, opencode.json, and opencode.jsonc are still combined with mergeConfig, so permission blocks split across global config files will still deep-merge into a single object before reaching this helper and can lose the user-written rule ordering this PR is trying to preserve.
2 participants
Issue for this PR
Closes #16157
Type of change
What does this PR do?
Merging permission objects with deep-merge silently discarded user-written rule ordering, so wildcards and specific patterns evaluated in unpredictable order across config sources.
Instead of merging objects, this preserves each config source's permission block as its own layer, converts each to a ruleset, and merges the rulesets. Later layers win, matching the existing intra-object ordering semantics.
New config syntax
permissionnow accepts either a single object (unchanged) or an array of layered configs:{ "permission": [ // Layer 1: safety defaults { "bash": { "rm -rf *": "deny" } }, // Layer 2: deny everything else that runs through bash { "bash": "deny" }, // Layer 3: punch a hole through layer 2 { "bash": { "echo *": "allow" } } ] }Each entry is one layer. Later layers override earlier rules (
findLastsemantics, same as inside a single object). Single-object configs work as before — they are treated as a one-element layer internally.How did you verify your code works?
Added tests covering:
Config.update/updateGlobalpreserving the user's on-disk shapeChecklist