fix(#3120): add register_authored_at_plan_time guard — prevent rubber-stamping legacy phases (#3142)

* fix(#3120): add register_authored_at_plan_time guard to secure-phase

Root cause: Step 3 short-circuit used threats_open: 0 as the sole
condition to skip directly to Step 6 (write clean SECURITY.md). It
did not distinguish empty-by-all-mitigated from empty-by-no-planning.
Legacy phases authored before <threat_model> blocks were canonical
received a rubber-stamped clean SECURITY.md with no audit performed.

Fix:
  Step 2c: track register_authored_at_plan_time (true iff >=1 PLAN
           file contained a parseable <threat_model> block)
  Step 3:  two-condition short-circuit:
           - threats_open:0 AND register_authored_at_plan_time:true
             -> skip to Step 6 (legitimate, all mitigated)
           - threats_open:0 AND register_authored_at_plan_time:false
             -> retroactive-STRIDE mode in Step 5 (build register
                from implementation, then verify)
  Step 5:  auditor constraint varies by mode:
           planned     -> Verify mitigations exist, do not scan
           retroactive -> Build STRIDE register first, then verify

Suite: 7039/7039. Closes #3120.

* fix(lint+changeset): allow-test-rule, drop dead regex branches, fix pr field to 3142

.changeset/fix-3120-secure-phase-empty-register.md

@@ -0,0 +1,5 @@
+---
+type: Fixed
+pr: 3142
+---
+**`secure-phase` no longer rubber-stamps SECURITY.md for legacy phases with no `<threat_model>` blocks** — Step 3's short-circuit previously exited to Step 6 (write clean SECURITY.md) whenever `threats_open: 0`, regardless of whether zero threats meant "all mitigated" or "none were ever written". Legacy phases authored before `<threat_model>` blocks became canonical now trigger **retroactive-STRIDE mode** in Step 5: the auditor builds a register from implementation files before verifying mitigations. Step 2c now tracks `register_authored_at_plan_time` and Step 3 gates the skip on both `threats_open: 0 AND register_authored_at_plan_time: true`. Closes #3120.

get-shit-done/workflows/secure-phase.md

@@ -58,6 +58,8 @@ Read SUMMARY.md — extract `## Threat Flags` entries.
 
 Per threat: `{ threat_id, category, component, disposition, mitigation_pattern, files_to_check }`
 
+Also set `register_authored_at_plan_time: true` if **at least one** PLAN file contained a parseable `<threat_model>` block; `false` if no PLAN files had any `<threat_model>` block (legacy phase authored before formal threat modelling was standard).
+
 ## 3. Threat Classification
 
 Classify each threat:
@@ -69,7 +71,10 @@ Classify each threat:
 
 Build: `{ threat_id, category, component, disposition, status, evidence }`
 
-If `threats_open: 0` → skip to Step 6 directly.
+**Short-circuit rule:**
+- If `threats_open: 0 AND register_authored_at_plan_time: true` → skip to Step 6 directly. All plan-time threats are verified CLOSED.
+- If `threats_open: 0 AND register_authored_at_plan_time: false` → **do NOT skip**. Empty-by-no-planning must not rubber-stamp a clean SECURITY.md. Proceed to Step 5 in **retroactive-STRIDE mode** — the auditor builds a register from implementation files first, then verifies mitigations.
+- If `threats_open > 0` → proceed to Step 4 (present threat plan to user).
 
 ## 4. Present Threat Plan
 
@@ -82,6 +87,11 @@ Call AskUserQuestion with threat table and options:
 
 ## 5. Spawn gsd-security-auditor
 
+**Auditor constraint — varies by register origin:**
+
+- `register_authored_at_plan_time: true` — **Verify mitigations exist** — do not scan for new threats. The register is complete; verify each threat's mitigation is present in the implementation.
+- `register_authored_at_plan_time: false` (retroactive-STRIDE mode) — **Retroactive-STRIDE: build a STRIDE register from implementation files first, then verify mitigations.** The phase was authored before formal threat modelling; the auditor must construct the register from scratch before verifying.
+
 ```
 Task(
   prompt="Read ~/.claude/agents/gsd-security-auditor.md for instructions.\n\n" +
@@ -158,7 +168,8 @@ Display `/clear` reminder.
 - [ ] Input state detected (A/B/C) — state C exits cleanly
 - [ ] PLAN.md threat model parsed, register built
 - [ ] SUMMARY.md threat flags incorporated
-- [ ] threats_open: 0 → skip directly to Step 6
+- [ ] threats_open: 0 AND register_authored_at_plan_time: true → skip directly to Step 6
+- [ ] threats_open: 0 AND register_authored_at_plan_time: false → retroactive-STRIDE mode (Step 5), not skipped
 - [ ] User gate with threat table presented
 - [ ] Auditor spawned with complete context
 - [ ] All three return formats (SECURED/OPEN_THREATS/ESCALATE) handled

tests/bug-3120-secure-phase-empty-register.test.cjs

@@ -0,0 +1,58 @@
+'use strict';
+// allow-test-rule: reads product workflow markdown (secure-phase.md) to verify structural guard contract — not a source-grep test
+
+// Regression guard for bug #3120.
+//
+// secure-phase.md Step 3 short-circuited to Step 6 (write SECURITY.md)
+// whenever threats_open: 0, without distinguishing between:
+//   Case A: All plan-time threat_model threats are CLOSED (legitimate skip)
+//   Case B: No threat_model blocks were written at plan time (legacy phases)
+//          → rubber-stamps a clean SECURITY.md with zero audit performed
+//
+// Fix: Step 2c tracks `register_authored_at_plan_time` (true iff ≥1 PLAN
+// file contained a parseable <threat_model> block). Step 3 now requires BOTH
+// threats_open: 0 AND register_authored_at_plan_time to skip. If only
+// threats_open: 0 and NOT register_authored_at_plan_time, Step 5 runs in
+// retroactive-STRIDE mode.
+
+const { test, describe } = require('node:test');
+const assert = require('node:assert/strict');
+const fs = require('node:fs');
+const path = require('node:path');
+
+const ROOT = path.join(__dirname, '..');
+const src = fs.readFileSync(
+  path.join(ROOT, 'get-shit-done', 'workflows', 'secure-phase.md'),
+  'utf8',
+);
+
+describe('bug #3120: secure-phase short-circuit guards', () => {
+  test('Step 2c tracks register_authored_at_plan_time', () => {
+    assert.ok(
+      src.includes('register_authored_at_plan_time'),
+      'secure-phase.md does not track register_authored_at_plan_time in Step 2c',
+    );
+  });
+
+  test('Step 3 short-circuit requires both conditions', () => {
+    assert.ok(
+      src.includes('threats_open: 0 AND register_authored_at_plan_time'),
+      'Step 3 short-circuit does not gate on both threats_open:0 AND register_authored_at_plan_time',
+    );
+  });
+
+  test('retroactive-STRIDE mode is documented for legacy phases', () => {
+    assert.ok(
+      src.includes('retroactive') || src.includes('Retroactive'),
+      'secure-phase.md does not document retroactive-STRIDE mode for legacy phases (no <threat_model> blocks)',
+    );
+  });
+
+  test('Step 5 auditor constraint varies by mode', () => {
+    assert.ok(
+      (src.includes('Verify mitigations') || src.includes('verify mitigations')) &&
+      (src.includes('Retroactive') || src.includes('retroactive')),
+      'Step 5 does not distinguish planned vs retroactive-STRIDE auditor constraint',
+    );
+  });
+});