Split target selection from effect authorization in config reads #146

New issue

Closed

opened 2026-08-15 14:33:45 +00:00 by john · 1 comment

john commented

2026-08-15 14:33:45 +00:00

Owner

Source: deep dive, broader design B2
Verdict: AGREE - proposal

Claim as reviewed

Merged configuration may select behaviour, but external mutation should require an authorization value from a clearly named layer. Security-sensitive scalar reads should return provenance rather than reducing to a bare boolean.

At HEAD `f354864` (v3.3.0)

The git.auto_close GitLab issue in this batch is the concrete instance. cadence-core/bin/git-publish.mjs:65-72 already does this by hand for one key; nothing generalizes it.

Note

Scope check before planning: how many keys actually need layer-aware reads. If the answer is one, the general mechanism is not worth building and the specific fix in the auto_close issue is the whole job.

**Source:** deep dive, broader design B2 **Verdict:** AGREE - proposal ### Claim as reviewed Merged configuration may select behaviour, but external mutation should require an authorization value from a clearly named layer. Security-sensitive scalar reads should return provenance rather than reducing to a bare boolean. ### At HEAD f354864 (v3.3.0) The `git.auto_close` GitLab issue in this batch is the concrete instance. `cadence-core/bin/git-publish.mjs:65-72` already does this by hand for one key; nothing generalizes it. ### Note Scope check before planning: how many keys actually need layer-aware reads. If the answer is one, the general mechanism is not worth building and the specific fix in the auto_close issue is the whole job.

john added the

proposal

external-review

labels

2026-08-15 14:33:45 +00:00

john commented

2026-08-15 15:14:18 +00:00

Author

Owner

Closing - answered before it was planned.

The general mechanism this proposal asks for already exists. cadence-core/bin/lib/config-merge.mjs:129-135 returns layers.repo and layers.global beside the merged config specifically so a key declared src: repo can be checked against the file that carried it, and its comment names the bug that motivated it (a global-file risk.override waiving a risk floor in every repo on the machine). review.key_file demonstrates the enforced form for src: global.

There is also only one instance. Surveying the src: repo keys - git.auto_close, git.on_protected, git.integration_branch, git.auto_branch - only auto_close gates an EXTERNAL, irreversible mutation. The rest gate local operations where a merged read is correct.

So this is not an abstraction worth extracting; it is one call path that skips a mechanism already built for it. That work is folded into #131 (v3.5.1), which now carries the concrete recommendation: surface the repo-layer value for an src: repo key through config.mjs get, rather than hand-rolling a second boolean in the skill.

Reopen if a second external-mutation key ever appears.

Closing - answered before it was planned. The general mechanism this proposal asks for already exists. cadence-core/bin/lib/config-merge.mjs:129-135 returns layers.repo and layers.global beside the merged config specifically so a key declared `src: repo` can be checked against the file that carried it, and its comment names the bug that motivated it (a global-file risk.override waiving a risk floor in every repo on the machine). review.key_file demonstrates the enforced form for `src: global`. There is also only one instance. Surveying the `src: repo` keys - git.auto_close, git.on_protected, git.integration_branch, git.auto_branch - only auto_close gates an EXTERNAL, irreversible mutation. The rest gate local operations where a merged read is correct. So this is not an abstraction worth extracting; it is one call path that skips a mechanism already built for it. That work is folded into #131 (v3.5.1), which now carries the concrete recommendation: surface the repo-layer value for an `src: repo` key through config.mjs get, rather than hand-rolling a second boolean in the skill. Reopen if a second external-mutation key ever appears.

john closed this issue

2026-08-15 15:14:18 +00:00