fix(cmd): refuse to arm APPROVE on an empty binding require

cmd/assent/run.go

@@ -522,6 +522,9 @@ func selectBinding(rb *policy.RulesetBinding) (*policy.Binding, error) {
 //     an obligation that does not apply and could APPROVE, so an opaque/empty diff
 //     short-circuits to the fail-safe REVIEW here, matching the skeleton's
 //     failSafe(), never a silent APPROVE.
+//   - empty require (RVW-S01 / D-184): a binding that declares no required
+//     obligations makes the obligation layer vacuous, so a decidable subject is
+//     refused fail-closed rather than APPROVEd on no positive vouch.
 //
 // Otherwise the frozen engine decides over the decoded EvaluationInput with
 // forge-resolved ApprovalEvidence (E4-S06) and the pack phase ceiling. Every
@@ -537,6 +540,17 @@ func decide(subject, subjectClass string, cs change.ChangeSet, mp *policy.MergeP
 	case cs.Opaque || len(cs.Changes) == 0:
 		res = undecidableReview(subject)
 	default:
+		// RVW-S01 (D-184): an empty/absent require[] declares no required
+		// obligations, so the obligation layer is vacuous — the run would APPROVE a
+		// governed change no rule positively vouches. Fail CLOSED before any forge
+		// write (the seam note at internal/core/decision/record.go assigns this duty
+		// to the CLI wiring; GUIDELINES §Safety-1: every change must be positively
+		// vouched). Reached only for a non-reserved, decidable subject: a reserved
+		// `.assent/**` subject BLOCKs above, and an opaque/empty changeset REVIEWs
+		// above, so neither depends on require[].
+		if len(bind.Require) == 0 {
+			return aggregate.Result{}, fmt.Errorf("ruleset-binding binding (class=%q, environment=%q) declares no required obligations (require is empty) — refusing to arm APPROVE; add at least one obligation to require[] (GUIDELINES §Safety-1, D-184)", bind.Class, bind.Environment)
+		}
 		in := buildEvaluationInput(cs, mrFrom(info, mrAuthor), bind.Require)
 		if len(facts) > 0 {
 			in.Facts = facts

cmd/assent/run_test.go

@@ -641,6 +641,18 @@ const rulesetBindingLabelGuard = `{
   ]
 }`
 
+// rulesetBindingEmptyRequire is schema-valid (require is not required, no
+// minItems) but declares no required obligations — the RVW-S01 vacuous shape. The
+// pre-guard engine decides APPROVE with zero findings on a change its block rules
+// do not fire on; the run-path guard must refuse before any forge write.
+const rulesetBindingEmptyRequire = `{
+  "apiVersion": "assent.dev/v1alpha1",
+  "kind": "RulesetBinding",
+  "bindings": [
+    { "class": "topic-registry", "environment": "prod", "packs": ["topic-safety"], "risk": { "threshold": 10 }, "require": [] }
+  ]
+}`
+
 // configOwnerFailOpen configures the controlling `owner` provider failure:open —
 // a posture ValidateProviderPosture must REJECT (a controlling fact must fail closed).
 const configOwnerFailOpen = `apiVersion: assent.dev/v1alpha1
@@ -742,6 +754,34 @@ func TestRunApproveArmedMerges(t *testing.T) {
 	}
 }
 
+// RVW-S01 polarity: an empty-require binding is refused fail-closed, never
+// APPROVEd. The partitions increase (12 -> 24) is the exact change
+// TestRunApproveArmedMerges shows deciding APPROVE under a non-empty require; with
+// require: [] the obligation layer is vacuous and the pre-guard engine would
+// APPROVE with zero findings and (armed) approve+merge. The guard must stop the
+// run before any forge write and emit no APPROVE record.
+func TestRunEmptyRequireNeverApproves(t *testing.T) {
+	f := newFakeGitLab(t)
+	f.rulesetBinding = rulesetBindingEmptyRequire
+	f.baseFile = "partitions: 12\n"
+	f.headFile = "partitions: 24\n"
+
+	var out bytes.Buffer
+	code := runRun(runArgs("--arm"), env("tok"), fixedClock(), &out, &out, f.factory())
+	if code == 0 {
+		t.Fatalf("empty require must fail closed (non-zero exit), got 0\n%s", out.String())
+	}
+	if f.approvals != 0 || f.merges != 0 || f.discussionsPosted != 0 {
+		t.Errorf("empty require must write NOTHING to the forge: approvals=%d merges=%d discussions=%d", f.approvals, f.merges, f.discussionsPosted)
+	}
+	if strings.Contains(out.String(), `"decision":"APPROVE"`) {
+		t.Errorf("empty require must never emit an APPROVE record:\n%s", out.String())
+	}
+	if !strings.Contains(out.String(), "require is empty") {
+		t.Errorf("refusal must be contributor-readable and name the empty require:\n%s", out.String())
+	}
+}
+
 // APPROVE with forge probe refusing: same increase → APPROVE decision, but
 // forge-probed ArmEligible=false → Reconcile refuses (ErrArmingRefused) → ZERO
 // approve/merge writes. --arm alone cannot override (E4-S06 / D-034).

docs/adr/0009-execution-modes.md

@@ -53,8 +53,9 @@ Shipping order: `run`/`--dry-run`/`explain` in v1; `serve` in v1.x once the CI p
 > zero risk."* **There is no zero-risk adoption path via a mode switch.** The only way to run
 > `assent` without approve/merge writes is to leave one of the forge-probed arming
 > preconditions unmet; `docs/usage/cli.md` §*How to keep assent advisory* states this, and
-> explains why a pack's `spec.phase` must not be used as a substitute (with no `require:`
-> obligations declared, an `observe` or `off` ceiling turns a BLOCK into an approve+merge).
+> explains why a pack's `spec.phase` must not be used as a substitute (a binding with no
+> `require:` obligations declared is refused on the run path, failing closed before any forge
+> write, so an `observe` or `off` ceiling can no longer turn a BLOCK into an approve+merge).
 >
 > This gap is *why* the CLI reference previously recommended `phase: observe` as the advisory
 > lever: the sanctioned mechanism was never built, so the docs invented one. Same open options

docs/decisions/decisions.md

@@ -187,3 +187,4 @@ project/process decisions.
 | D-181 | 2026-09-10 | **The changelog drift gate tolerates exactly one drift: HEAD is itself a release tag and `CHANGELOG.md` lacks only that tag's section. Closes the lockout window D-180 left open.** Finding (D-180's own review, recorded there as the open F2 proposal): between `git push origin vX.Y.Z` and the stamp commit, any `verify` run that checks out the tagged SHA — a re-run, or the weekly schedule on an unchanged `main` — rendered the new section, found it missing and went red; `release.yaml`'s verify-green tag gate (REQ-AUD-S03-01) requires EVERY `verify` run on the SHA to be green, so one such red locked the tag out of its release job, re-runs and `workflow_dispatch` rebuilds. The only defence was a written rule ("never re-run verify on the tagged SHA; stamp before Monday 06:00 UTC"). **Decision:** `verify-changelog.sh`, after the normal diff fails, collects the `v[0-9]*` tags pointing AT HEAD (never tags further back) and renders the committed form again through `render-changelog.sh` with `--ignore-tags` for exactly those tags — the same bytes the tagged commit was green with before the tag existed. Equal → exit 0 with a notice naming the stamp commit to land (a `::warning::` annotation under `GITHUB_ACTIONS`); otherwise the original diff and red. `render-changelog.sh` forwards extra arguments to git-cliff for this; it stays the one definition of the committed form. **Still red:** a hand edit or a history-re-rendering `cliff.toml` change on the tagged HEAD, an unstamped tag with ANY commit on top (so the stamp must be the next commit on `main`), and an older unstamped tag behind a newer tagged HEAD. **Every other check on the tagged commit follows the same allowance (review F1):** CI runs the whole of `task check` in `release-exitgate` (`hack/audit/exitgate_test.sh`), so a red anywhere in it on the tag SHA locks the tag out exactly as the drift gate did. Measured on a freshly tagged clone, stage by stage, the only other red was `changelog_gate_test.sh` — §1 (newest section = newest tag) and the §10 sandbox baseline, whose overlay commit sat on top of the tag. §1 now reads `verify-changelog.sh`'s own allowance from its output (not a second implementation) and warns instead of failing on that commit; the §10 sandbox drops HEAD tags whose section is missing, starting from the pre-tag state. The stamp therefore cannot be forgotten because the first commit on top of an unstamped tag is red everywhere, not because the tagged commit is. **Empty releases (review N2):** a tag whose commits are all cliff-skipped (e.g. a tooling-only `:memo: chore(release):` patch) renders no section, so there is nothing to stamp; §1 accepts it when `verify-changelog.sh` is plainly green, which means the file is byte-identical to the released-only render of every tag and therefore cannot hide a missing stamp. §10e's overlay commit uses a rendering subject for the same reason — with a skipped one, v98.0.0 was an empty release right after every stamp commit and `main` went red (review N1). **Proof:** §10c at both polarities (tagged HEAD green; hand edit, re-render, commit on top, older unstamped tag behind a tagged HEAD red; a release and its -rc on one commit green with the hint naming the release; an older `v99x0x0` that `v99.0.0` matches as a bare regex red), and §10e runs this whole script on a tagged-but-unstamped clone (green) and with a commit on top (red), and grades the stamp commit and an empty release through §1's function. Six mutants each red the suite: no allowance; an allowance that skips the pre-tag comparison; one keyed on the newest reachable tag instead of HEAD's; no regex escaping; §1 without the exception; the §10 sandbox without the rewind. Amends REQ-E9-S03-03, REQ-AUD-S02-01 and AUD-S02 criterion #2 in their specs; `hack/release/README.md` "Cutting a release" drops the never-re-run and before-Monday rules. |
 | D-182 | 2026-09-25 | **`mr.labels` is WIRED through the live `assent run` path rather than retired by a lint hard error. The frozen predicate-scope contract keeps its label field, and the fail-open is closed by making the forge fetch honest.** The six-leg review found `mr.labels` schema-carried (`evaluation-input.schema.json`), engine-bound (`aggregate/evaluate.go` binds `labels`) and harness-tested (`internal/adoptertest/mr_test.go`) but never fetched on the live path: `cmd/assent/run.go:581` `mrFrom` built `aggregate.MR` with no `Labels`, and the GitLab adapter's two MR decoders (`GetMR` in `internal/forge/gitlab/gitlab.go` and `mrWithAuthor` in `snapshot.go`) ignored the API's `labels` array. **Consequence, and why it is a safety defect not a missing feature:** every label predicate in production evaluated against an empty list, so a NEGATIVE guard (`!('security-hold' in mr.labels)`) was vacuously true and a `security-hold` MR could APPROVE and merge. **Decision — the alternative is recorded and rejected:** a lint hard error rejecting any `mr.labels` reference plus a `predicate-scope.md` amendment would have made the contract truthful by deleting a capability adopters were already sold; wiring keeps the contract and costs two JSON decodes. **Implementation:** `forge.MRInfo` gains `Labels`; `GetMR` and `mrWithAuthor` decode the string array (absent field → nil, never an error); `Snapshot` surfaces them on `forge.MRHeads.Labels`; `mrFrom` copies `info.Labels` into `aggregate.MR.Labels`. **Proven, not asserted:** a run-path test through the real GitLab adapter over `httptest` (`fakeGitLab`) shows a labelled MR reaching the engine (a label-proven obligation APPROVEs with the label, REVIEWs without it) and the negative-guard polarity failing CLOSED (the guarded label yields BLOCK with zero approve/merge writes). Spec: `openspec/specs/p5-e4-gitlab-forge/spec.md` REQ-E4-S02-05 + REQ-E4-S06-08. Revert: drop `Labels` from `MRInfo`/`MRHeads`, the two decodes and the `mrFrom` copy — reopens the empty-label fail-open for every negative guard. |
 | D-183 | 2026-09-25 | **The six-review round's first fix slice lands: `assent run` reads every judged byte at the pinned commit SHA, not the mutable branch name (U-04 item 1).** Origin: the 2026-09-25 six-leg review (`data/assent-unify/report.md` §4.1 U-04, §6 lane B1; `data/assent-diff-qfn/report.md` QFN-01) at HEAD `b8123cc`. **The defect, stated precisely:** `orchestrate` pins `info.SourceSHA`/`info.TargetSHA` from `GetMR` (`run.go:177`), and the approval and `merge?sha=` CAS are pinned to those commits — but the six content reads on the live path resolved a mutable branch name (`run.go:203,211,230,249,270` = `info.TargetBranch`; `:274` = `info.SourceBranch`). The invariant the merge leans on, *final head == pin ⇒ final head == what was judged*, is therefore false: a contributor with push rights on the MR branch pushes benign X2 (judged), then force-pushes back to pinned X1 before `Reconcile`'s re-read, the SHA-guard passes (head == pin), and the forge merges X1 — bytes never evaluated. The `DecisionRecord` pins `policySha/sourceSha/targetSha` but no content digest, so the record cannot distinguish the states either; ADR-0015 §2's "Merge is SHA-guarded (no TOCTOU)" heading describes the *branch-moved* guard, not a bytes-equal-to-pin guarantee. **The change:** pass `info.TargetSHA` at `:203`, `:211`, `:230`, `:249`, `:270` and `info.SourceSHA` at `:274`. The GitLab adapter already accepts a commit SHA as `?ref=` (`gitlab.go:474`, the same URL-escaped parameter), so this is one argument at six sites — no adapter API change, no new discriminator, no reordering. Target-side reads are pinned too (judgment call (a)): not strictly required for the source-side attack, but it removes the maintainer-push-window ambiguity on the policy/config/pack loads and leaves one uniform rule rather than an asymmetry a later reader must re-justify. **The tests are FLIPPED, not merely updated (judgment call (b)):** the run-path fake's file router now serves the pinned SHAs and errors on any other ref, and `TestFileAtRef` asserts the SHA it sends — so a regression to `info.TargetBranch`/`info.SourceBranch` reddens the existing suite rather than passing against a fake that still accepts branch names. `run_self_vouch_test.go:70`'s policy-load ref assertion moves from `f.target` to `f.targetTip` for the same reason. **The move-and-restore case is in two places, deliberately (judgment call (c)):** the `internal/forge/conformance` case `sha-guard-source-moved-and-restored` (with a new `Fixture.MoveSourceHead` seam and a `catalog.yaml` row) drives `forge.Reconcile` only, so it proves the CAS **merges once the head is back at the pin** — i.e. the CAS alone cannot catch a move-and-restore, which is exactly why the read must be pinned; the run-path case `TestRunJudgesPinnedSHAWhenBranchMoves` proves the read itself follows the pin (branch points at benign bytes, pin at violating bytes, decision follows the pin). **Strictly stronger pinning, no behaviour change beyond it:** the same bytes are read when the branch has not moved. **Explicitly NOT in this lane:** U-04 items 2–4 (SHA-bound `/repository/compare` enumeration re-fold — shares its mechanism with the D-139 trio, lane B5; the `CI_MERGE_REQUEST_SOURCE_BRANCH_SHA` belt; the additive `headContentSha` pin, lane B9), the `--checkout` tree↔SHA binding (U-08/QFN-04, lane B5), the `forgePort` shape (E10), and the `docs/usage/cli.md` "runs without `--checkout` are not exposed" claim (its enumeration half is item 2). Spec: `openspec/specs/p5-rev1-pinned-sha/spec.md`. **Revert:** restore `info.TargetBranch`/`info.SourceBranch` at the six sites and un-flip the fake routers — which re-opens the move-and-restore merge of un-evaluated bytes on the checkout-less path the docs call safe. |
+| D-184 | 2026-09-25 | **RVW-S01 — an empty/absent binding `require:` must never arm APPROVE: the run path refuses it, `assent lint` hard-errors `binding-require-empty`, and this row supersedes D-021's "vacuously covered" clause. The frozen schema's `minItems: 1` is deferred.** **Finding (reproduced on the run path):** a schema-valid `RulesetBinding` binding with `require: []` (or no `require:` key) decides **APPROVE with zero findings** for any governed change its block rules do not fire on. The obligation layer iterates `bind.Require`, so an empty list makes it vacuous; with the forge-probed arming preconditions met the run then approves and merges. The schema accepts it (`require` is not in the binding's `required` list and carries no `minItems`; `schemas/policy/v1alpha1/ruleset-binding.schema.json:52-57`) and lint passed it (`checkObligationCoverage` iterates the same empty list), while two in-repo authorities say the opposite: `GUIDELINES.md` §Safety-1 ("Every change must be positively vouched") and `internal/core/decision/record.go`'s own seam note (S03 review F3: the CLI wiring "must guarantee require is non-empty before an APPROVE is armed"). D-021 added `require` as optional and recorded "absent/empty ⇒ no required obligations (vacuously covered)" — a true statement about the schema's shape that was read as a safety property, and was never one. **Decision:** the invariant wins. (a) **Run path** — `decide`'s decidable, non-reserved arm returns a hard error when the covering binding's `require` is empty; the run exits `1` with a contributor-readable message naming the binding `(class, environment)`, and **zero forge writes** happen (the guard sits before `buildEvaluationInput`, so no record is emitted either). It is deliberately NOT placed in `selectBinding`: that function is shared with `assent compare`, whose whole purpose is to compare permissive candidates, and refusing to compare an unsafe candidate would defeat it. (b) **Lint** — new hard error `binding-require-empty`, located to the binding, with a `good`/`bad` fixture pair registered in the E3-S08 `hardErrorCorpus`. (c) **Reserved-class exemption, stated because it is a real carve-out:** the reserved `assent-policy` meta-class is block-by-default (ADR-0015 §1) and its safety comes from GUARD 1 (any `.assent/**` edit BLOCKs), not from obligation coverage — and it *cannot* carry a non-empty `require` at all, because any `prove` rule in a reserved-bound pack routes the policy class to a vouch/APPROVE-arming disposition the reserved-class check forbids. The lint check therefore skips `class == assent-policy`; the run-path guard still refuses an empty require whenever the SUBJECT is not the reserved class. (d) **Reconciliation** — the schema description's "Absent or empty ⇒ no required obligations (vacuously covered)" and D-021's identical clause are superseded as *safety* statements by this row; D-021 is left byte-unchanged (supersede, don't edit); D-134's ARM-08 run-path consequence ("with no `require:` on the binding it turns a BLOCK into APPROVE+merge") is superseded the same way — the engine still decides APPROVE for an empty `require`, but `decide` (a) refuses before it, so that approve+merge path is gone. The schema's `minItems: 1` is **deferred to the next schema change window**: `schemas/**/*.json` is under the ref-relative freeze guard (D-132) against the released `v0.1.0` tag, which permits exactly one diff (the D-120 `toolDigest` description) and no other, so editing the frozen file in this lane would red `hack/audit/exitgate_test.sh`. Until that window, (a)+(b) are the enforcement. **Not in scope, deliberately:** the unmatched-edit fail-open (a rule that names an obligation but whose `match` selects nothing still marks it covered) shares the `cover()` invariant but is its own register row and is gated on a held captain decision; `assent test` does not arm APPROVE and is unchanged; `assent compare` is unchanged per (a). **Scope note:** `examples/lint-fixtures/{fail-open,reserved-class}/` were touched — `fail-open`'s bindings gained `require: [reviewed]` (the rule already proved it), and `reserved-class/good` is covered by the exemption; both were previously "clean" only because the check did not exist. **Revert:** delete `checkBindingRequireEmpty` and its corpus row, drop the `decide` guard, restore the two `fail-open` bindings, and the vacuous APPROVE returns. |