Global git.auto_close authorizes the unattended GitLab publish and merge #131

New issue

john · 2026-08-15T14:33:35Z

john commented

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

Source: deep dive F1; strict re-review C-04 (independent convergence)
Verdict: AGREE - high, conditional on GitLab

Claim as reviewed

A user-global git.auto_close=true can authorize the unattended GitLab publication and merge path even though the stated policy and the git-publish seam require repository-local authorization.

At HEAD `f354864` (v3.3.0)

cadence-core/bin/git-publish.mjs:65-72 (repoAutoClose) reads .planning/config.json directly and never the merged value, enforcing D-08. cadence-core/config.schema.json:47 states the same contract.

skills/cad-land/SKILL.md:23-27 reads git.auto_close through config.mjs get, which returns the merged global-plus-repo value, and a true value enters the no-prompt branch at :69-77.

GitHub and Forgejo then publish through the repo-authorized seam and would stop on ok:false. GitLab does not: skills/cad-land/SKILL.md:100-124 states "On GitLab glab mr create publishes the source branch itself, so no seam call is needed there", and the workflow proceeds to glab mr merge.

cadence-core/bin/land-cleanup.mjs:126-147 records the discrepancy in the source: "on GitHub the chain then died at the publish seam; on GitLab nothing gates it at all".

Note

The prior narrowing (0b1c322, reverted) aligned the two seams' values and broke the skipped-ask / halt pairing that land-cleanup.mjs gate depends on. The fix that does not touch that pairing is to resolve two distinct booleans: autoCloseRequested from the merged config (presentation, and what the gate reads), autoCloseAuthorized from the repo layer only, required before any unattended external mutation on every host including GitLab.

**Source:** deep dive F1; strict re-review C-04 (independent convergence) **Verdict:** AGREE - high, conditional on GitLab ### Claim as reviewed A user-global `git.auto_close=true` can authorize the unattended GitLab publication and merge path even though the stated policy and the git-publish seam require repository-local authorization. ### At HEAD f354864 (v3.3.0) `cadence-core/bin/git-publish.mjs:65-72` (`repoAutoClose`) reads `.planning/config.json` directly and never the merged value, enforcing D-08. `cadence-core/config.schema.json:47` states the same contract. `skills/cad-land/SKILL.md:23-27` reads `git.auto_close` through `config.mjs get`, which returns the merged global-plus-repo value, and a true value enters the no-prompt branch at `:69-77`. GitHub and Forgejo then publish through the repo-authorized seam and would stop on `ok:false`. GitLab does not: `skills/cad-land/SKILL.md:100-124` states "On GitLab `glab mr create` publishes the source branch itself, so no seam call is needed there", and the workflow proceeds to `glab mr merge`. `cadence-core/bin/land-cleanup.mjs:126-147` records the discrepancy in the source: "on GitHub the chain then died at the publish seam; on GitLab nothing gates it at all". ### Note The prior narrowing (0b1c322, reverted) aligned the two seams' values and broke the skipped-ask / halt pairing that `land-cleanup.mjs gate` depends on. The fix that does not touch that pairing is to resolve two distinct booleans: `autoCloseRequested` from the merged config (presentation, and what the gate reads), `autoCloseAuthorized` from the repo layer only, required before any unattended external mutation on every host including GitLab.

john added the

bug

security

external-review

labels

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

john added this to the v3.5.1 milestone

2026-08-15 15:04:36 +00:00

john commented

2026-08-15 15:04:37 +00:00

Do not re-do the reverted fix

The obvious change - narrow the land gate to layers.repo so both seams read one value - was already made and reverted, and cadence-core/bin/land-cleanup.mjs:126-147 records why in the source:

Narrowing this to layers.repo (0b1c322, reverted here) aligned the two seams' VALUES and broke that pairing: with a global-only auto_close the prose still entered the unattended chain and still suppressed triage while this gate believed no chain was running. On GitHub the chain then died at the publish seam; on GitLab nothing gates it at all.

The gate and the skipped publish-ask are a matched pair and must read the same value. That is not the bug.

The fix that does not touch the pairing

Two named booleans, resolved separately:

autoCloseRequested - merged config. Drives presentation and the land-cleanup.mjs gate halt, exactly as today. Unchanged behaviour.
autoCloseAuthorized - repository layer only. Required before any unattended external mutation, on every host.

skills/cad-land/SKILL.md step 3b may enter the unattended chain on requested, but must not publish or merge on any host without authorized. Today GitHub and Forgejo get that for free because they route through git-publish.mjs:65-72; GitLab does not, because glab mr create publishes the source branch itself (SKILL.md:100-124).

Vocabulary collision with #121

#121 lands first at v3.4.0 and rewrites /cad-land step 1. Its naming trap section warns that git.auto_close already means "merge the branch unattended" and that issue-closing vocabulary must not collide with it in this file. Adding autoCloseRequested/autoCloseAuthorized puts more auto_close-shaped names in the same prose. Read that section before naming anything here.

Verification — live GitLab repo

John is creating a throwaway GitLab account and repo for this. That matters because the defect is in the wiring, not in a decision function: a unit test over the resolver can pass while glab mr create still publishes.

Two cases, both walked against the live repo, neither automatable in CI:

The defect. Global config git.auto_close: true; repo .planning/config.json without it; GitLab origin. Run /cad-land. Expected after the fix: the publish ask is NOT skipped, or the unattended arm refuses before glab mr create. Today: it publishes and merges.
The sanctioned path. Repo-layer git.auto_close: true. Run /cad-land. Expected: unchanged - publishes, opens the MR, merges.

Split the work accordingly: a regression test over the two-boolean resolver, and a /cad-verify UAT item for the live walk. The live walk performs a real merge on a real remote, so keep it to the throwaway repo.

## Do not re-do the reverted fix The obvious change - narrow the land gate to `layers.repo` so both seams read one value - **was already made and reverted**, and `cadence-core/bin/land-cleanup.mjs:126-147` records why in the source: > Narrowing this to `layers.repo` (0b1c322, reverted here) aligned the two seams' VALUES and broke that pairing: with a global-only auto_close the prose still entered the unattended chain and still suppressed triage while this gate believed no chain was running. On GitHub the chain then died at the publish seam; on GitLab nothing gates it at all. The gate and the skipped publish-ask are a matched pair and must read the same value. That is not the bug. ## The fix that does not touch the pairing Two named booleans, resolved separately: - `autoCloseRequested` - merged config. Drives presentation and the `land-cleanup.mjs gate` halt, exactly as today. Unchanged behaviour. - `autoCloseAuthorized` - repository layer only. Required before any unattended external mutation, on every host. `skills/cad-land/SKILL.md` step 3b may enter the unattended chain on *requested*, but must not publish or merge on any host without *authorized*. Today GitHub and Forgejo get that for free because they route through `git-publish.mjs:65-72`; GitLab does not, because `glab mr create` publishes the source branch itself (`SKILL.md:100-124`). ## Vocabulary collision with #121 #121 lands first at v3.4.0 and rewrites `/cad-land` step 1. Its **naming trap** section warns that `git.auto_close` already means "merge the branch unattended" and that issue-closing vocabulary must not collide with it in this file. Adding `autoCloseRequested`/`autoCloseAuthorized` puts more `auto_close`-shaped names in the same prose. Read that section before naming anything here. ## Verification — live GitLab repo John is creating a throwaway GitLab account and repo for this. That matters because the defect is in the wiring, not in a decision function: a unit test over the resolver can pass while `glab mr create` still publishes. Two cases, both walked against the live repo, neither automatable in CI: 1. **The defect.** Global config `git.auto_close: true`; repo `.planning/config.json` without it; GitLab origin. Run `/cad-land`. Expected after the fix: the publish ask is NOT skipped, or the unattended arm refuses before `glab mr create`. Today: it publishes and merges. 2. **The sanctioned path.** Repo-layer `git.auto_close: true`. Run `/cad-land`. Expected: unchanged - publishes, opens the MR, merges. Split the work accordingly: a regression test over the two-boolean resolver, and a `/cad-verify` UAT item for the live walk. The live walk performs a real merge on a real remote, so keep it to the throwaway repo.

john commented

2026-08-15 15:12:13 +00:00

Better fix than the two hand-rolled booleans above - the provenance mechanism already exists and this path just does not use it.

cadence-core/bin/lib/config-merge.mjs:129-135 returns layers.repo and layers.global alongside the merged config, for exactly this reason, in its own words: 'the merge LOSES provenance, and a key whose schema src is repo has to know which file carried it. Without it a caller can only read the merged value, which is how a risk.override.<surface> written once in the user-global file waived a risk floor in every repository on the machine.'

git.auto_close is already declared src: repo at config.schema.json:47. So the schema states the constraint and the merge exposes the evidence for it. What breaks the chain is that skills/cad-land/SKILL.md:23-27 reads through config.mjs get, which returns the merged value and drops layers entirely.

Compare review.key_file (src: global), which IS enforced - a repo layer setting it is stripped at the merge and named in warnings.

So evaluate this before writing autoCloseRequested/autoCloseAuthorized by hand: can config.mjs get surface the repo-layer value for an src: repo key - a flag, a second field, whatever fits the envelope - so the skill branches on provenance that already exists? That is a smaller change, it generalizes to every src: repo key at once, and it does not add auto_close-shaped vocabulary to the file #121's naming-trap section warns about.

Better fix than the two hand-rolled booleans above - the provenance mechanism already exists and this path just does not use it. cadence-core/bin/lib/config-merge.mjs:129-135 returns `layers.repo` and `layers.global` alongside the merged config, for exactly this reason, in its own words: 'the merge LOSES provenance, and a key whose schema `src` is `repo` has to know which file carried it. Without it a caller can only read the merged value, which is how a `risk.override.<surface>` written once in the user-global file waived a risk floor in every repository on the machine.' git.auto_close is already declared `src: repo` at config.schema.json:47. So the schema states the constraint and the merge exposes the evidence for it. What breaks the chain is that skills/cad-land/SKILL.md:23-27 reads through `config.mjs get`, which returns the merged value and drops `layers` entirely. Compare review.key_file (`src: global`), which IS enforced - a repo layer setting it is stripped at the merge and named in warnings. So evaluate this before writing autoCloseRequested/autoCloseAuthorized by hand: can `config.mjs get` surface the repo-layer value for an `src: repo` key - a flag, a second field, whatever fits the envelope - so the skill branches on provenance that already exists? That is a smaller change, it generalizes to every `src: repo` key at once, and it does not add auto_close-shaped vocabulary to the file #121's naming-trap section warns about.

john referenced this issue

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

Split target selection from effect authorization in config reads #146

john referenced this issue from a pull request that will close it,

2026-08-16 02:08:16 +00:00

v3.5.1 - authorization the repo grants, not the user #188

john closed this issue

2026-08-16 02:08:43 +00:00

Sign in to join this conversation.