diff --git a/.changeset/4683-threat-id-uniqueness.md b/.changeset/4683-threat-id-uniqueness.md new file mode 100644 index 000000000..c5ebe6b8a --- /dev/null +++ b/.changeset/4683-threat-id-uniqueness.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 4828 +--- +**Gap-closure plans can no longer silently reuse threat IDs that earlier plans in the same phase already assigned** — a `--gaps` re-plan numbered its `` registers from `T-{phase}-01` again, so the new plans claimed IDs that earlier plans had already given to different threats, and nothing detected it: `/gsd-secure-phase` builds `SECURITY.md` rows and `VALIDATION.md` carries a Threat Ref column keyed on that ID, leaving every consumer ambiguous. `init execute-phase` and `init plan-phase` now report cross-plan duplicates (`threat_id_duplicates` / `threat_id_duplicate_count` — register rows only, never prose; the reserved `T-{phase}-SC` row is exempt since every plan keeps it; superseded plans don't hold IDs against their replacements), execute-phase hard-stops on a non-empty list before dispatching any executor, and the planner (agent template + `planner-gap-closure.md` §9) is instructed to continue numbering after the phase's highest in-use `T-{phase}-NN`. (#4683) diff --git a/agents/gsd-planner.md b/agents/gsd-planner.md index 005ca4c37..a8da07a56 100644 --- a/agents/gsd-planner.md +++ b/agents/gsd-planner.md @@ -372,6 +372,8 @@ Output: [Artifacts created] ## STRIDE Threat Register +Threat IDs are unique within a phase. Before numbering, read the `` blocks of the phase's existing PLAN files and continue after their highest `T-{phase}-NN` — if the phase already uses `T-47-19`, new plans number from `T-47-20`, never from `T-47-01` again (#4683: a reused ID names two different threats, which makes `SECURITY.md` rows and `VALIDATION.md`'s Threat Ref column ambiguous). `T-{phase}-SC` is the exception: reserved, and every plan keeps it. + | Threat ID | Category | Component | Severity | Disposition | Mitigation Plan | |-----------|----------|-----------|----------|-------------|-----------------| | T-{phase}-01 | {S/T/R/I/D/E} | {function/endpoint/file} | {critical\|high\|medium\|low} | mitigate | {specific mitigation action} | diff --git a/docs/INVENTORY-MANIFEST.json b/docs/INVENTORY-MANIFEST.json index 646ed0ffc..fcb1ec693 100644 --- a/docs/INVENTORY-MANIFEST.json +++ b/docs/INVENTORY-MANIFEST.json @@ -666,7 +666,9 @@ "execute-phase/steps/sequential-root-pin.md", "execute-phase/steps/stale-reverification.md", "execute-phase/steps/tdd-applicability-resolution.md", + "execute-phase/steps/threat-id-gate.md", "execute-phase/steps/wave-post-gate-hooks.md", + "execute-phase/steps/worktree-base-check.md", "execute-phase/steps/worktree-recovery-policy.md", "new-milestone/steps/project-md-milestone-write.md", "new-milestone/steps/reset-phase-safety.md", diff --git a/gsd-core/references/planner-gap-closure.md b/gsd-core/references/planner-gap-closure.md index b1f9f2432..de1091c96 100644 --- a/gsd-core/references/planner-gap-closure.md +++ b/gsd-core/references/planner-gap-closure.md @@ -60,3 +60,5 @@ autonomous: true gap_closure: true # Flag for tracking --- ``` + +**9. Number threat IDs after the phase's existing registers** (#4683): when `security_enforcement` is on, read the `` blocks of the phase's existing PLAN files and continue after their highest `T-{phase}-NN` — do not renumber from `T-{phase}-01`. Every ID already names a specific threat in an earlier plan; reusing it makes `SECURITY.md` rows and `VALIDATION.md`'s Threat Ref column ambiguous. `T-{phase}-SC` is reserved and every plan keeps it. The execute-phase init reports any cross-plan duplicates and hard-stops on them (`threat_id_duplicates`), so a collision surfaces here as a failed plan check. diff --git a/gsd-core/workflows/execute-phase.md b/gsd-core/workflows/execute-phase.md index bba0312aa..864dccd17 100644 --- a/gsd-core/workflows/execute-phase.md +++ b/gsd-core/workflows/execute-phase.md @@ -92,7 +92,9 @@ if [[ "$INIT" == @file:* ]]; then INIT=$(cat "${INIT#@file:}"); fi AGENT_SKILLS=$(gsd_run query agent-skills gsd-executor) ``` -Parse JSON for: `executor_model`, `verifier_model`, `commit_docs`, `parallelization`, `branching_strategy`, `branch_name`, `phase_found`, `phase_dir`, `phase_number`, `phase_name`, `phase_slug`, `plans`, `incomplete_plans`, `plan_count`, `incomplete_count`, `state_exists`, `roadmap_exists`, `phase_req_ids`, `response_language`, `requirements_path`, `section_manifest`. +Parse JSON for: `executor_model`, `verifier_model`, `commit_docs`, `parallelization`, `branching_strategy`, `branch_name`, `phase_found`, `phase_dir`, `phase_number`, `phase_name`, `phase_slug`, `plans`, `incomplete_plans`, `plan_count`, `incomplete_count`, `state_exists`, `roadmap_exists`, `phase_req_ids`, `response_language`, `requirements_path`, `section_manifest`, `threat_id_duplicate_count`. + +**Threat-ID gate (#4683):** if `threat_id_duplicate_count` is non-zero, read and execute `execute-phase/steps/threat-id-gate.md` BEFORE any dispatch — it is a hard stop (the full duplicate list is in `threat_id_duplicates`). `section_manifest` (#2932) gates the three `steps/*.md` reads below: read a step file only when its `id` is in `section_manifest.included` (equivalently, its path is in `section_manifest.read`); skip it — without reading — when its `id` is in `section_manifest.excluded`. When `section_manifest` is `null` (degraded: manifest artifact missing/unreadable), read all three unconditionally — the safe superset. @@ -133,7 +135,7 @@ fi When `USE_WORKTREES` is `false`, `ISOLATION` is forced to `none`: executors run sequentially on the main working tree. The per-plan decision below has no effect when worktrees are project-disabled. -`USE_WORKTREES` and `ISOLATION` are also reset for the run when `worktree base-check` detects the orchestrator HEAD has diverged from the worktree fork base (#683 — e.g. an unmerged milestone branch). This runs for **any** isolated run, not only Claude: fork-base divergence is a property of the repository, so it degrades a GSD-created worktree exactly as a harness-created one. The auto-degrade prints a one-line warning to stderr and falls through to the sequential path so executors do not hit the exit-42 worktree-branch-check halt. Setting `worktree.baseRef:"head"` restores parallel execution only where GSD itself creates the worktrees (orchestrator-managed runtimes — Codex, OpenCode, Kimi, Kimi Code); harness-isolated runtimes (Claude Code, Cursor) do not read the setting (#48, verified 5/5; upstream claude-code#44965), so there the check compares against the real fork base and parallel execution returns once HEAD is merged/pushed so `origin/HEAD` matches it (#3659). The `worktree-branch-check` exit-42 guard inside each executor remains in place as a backstop. +`USE_WORKTREES` and `ISOLATION` are also reset for the run when `worktree base-check` detects the orchestrator HEAD has diverged from the worktree fork base (#683) — read and follow `execute-phase/steps/worktree-base-check.md` for the degrade semantics and the `worktree.baseRef:"head"` escape hatch (#3659). Read context window size for adaptive prompt enrichment: diff --git a/gsd-core/workflows/execute-phase/steps/threat-id-gate.md b/gsd-core/workflows/execute-phase/steps/threat-id-gate.md new file mode 100644 index 000000000..e6dbf1e30 --- /dev/null +++ b/gsd-core/workflows/execute-phase/steps/threat-id-gate.md @@ -0,0 +1,28 @@ +Apply response_language to all user-facing prose — narration between tool calls, status updates, progress notes, and findings included; preserve code, paths, and identifiers. + + +Cross-plan threat-ID duplicates — a `T-{phase}-NN` ID claimed by more than one live PLAN file in +this phase (#4683). The reserved `T-{phase}-SC` supply-chain row is never listed: every plan keeps +it by design. + +This is a hard stop BEFORE any dispatch — do not spawn executors, do not update state, do not +write artifacts. A reused ID names two different threats, so `SECURITY.md` rows and +`VALIDATION.md`'s Threat Ref column are ambiguous until it is fixed; executing the phase would +silently mark the wrong threat. + +Report the full `threat_id_duplicates` list verbatim — each ID with its claiming plan files: + +``` +### GSD ► THREAT ID DUPLICATES — EXECUTION BLOCKED (#4683) + +{for each {id, plans}: "{id} — claimed by {plans.join(', ')}"} + +Threat IDs must be unique within a phase. Renumber the newer plans' registers to continue +after the phase's highest in-use `T-{phase}-NN` (see @~/.claude/gsd-core/references/planner-gap-closure.md §9), +then re-run /gsd:execute-phase {phase}. +``` + +Do not offer to execute anyway. The only forward paths are renumbering the colliding registers +or (if a plan was superseded after the check) re-running init so the stale plan drops out of the +scan — superseded plans never hold IDs against their replacements (#2349). + diff --git a/gsd-core/workflows/execute-phase/steps/worktree-base-check.md b/gsd-core/workflows/execute-phase/steps/worktree-base-check.md new file mode 100644 index 000000000..2fc01bb25 --- /dev/null +++ b/gsd-core/workflows/execute-phase/steps/worktree-base-check.md @@ -0,0 +1,16 @@ +Apply response_language to all user-facing prose — narration between tool calls, status updates, progress notes, and findings included; preserve code, paths, and identifiers. + + +`USE_WORKTREES` and `ISOLATION` are also reset for the run when `worktree base-check` detects the +orchestrator HEAD has diverged from the worktree fork base (#683 — e.g. an unmerged milestone +branch). This runs for **any** isolated run, not only Claude: fork-base divergence is a property +of the repository, so it degrades a GSD-created worktree exactly as a harness-created one. The +auto-degrade prints a one-line warning to stderr and falls through to the sequential path so +executors do not hit the exit-42 worktree-branch-check halt. Setting `worktree.baseRef:"head"` +restores parallel execution only where GSD itself creates the worktrees (orchestrator-managed +runtimes — Codex, OpenCode, Kimi, Kimi Code); harness-isolated runtimes (Claude Code, Cursor) do +not read the setting (#48, verified 5/5; upstream claude-code#44965), so there the check compares +against the real fork base and parallel execution returns once HEAD is merged/pushed so +`origin/HEAD` matches it (#3659). The `worktree-branch-check` exit-42 guard inside each executor +remains in place as a backstop. + diff --git a/gsd-core/workflows/plan-phase.md b/gsd-core/workflows/plan-phase.md index 10182477c..82ef758f4 100644 --- a/gsd-core/workflows/plan-phase.md +++ b/gsd-core/workflows/plan-phase.md @@ -100,7 +100,7 @@ When `CONTEXT_WINDOW >= 500000`, the planner prompt includes the 3 most recent p **#2401 — `prior_verify_commands` is NOT part of that enrichment and is never gated on `CONTEXT_WINDOW`.** It is a handful of one-line `` commands harvested from the nearest prior phase that had any; the payload is tiny and its absence at 200k is exactly what made the planner re-invent a verify command and author an unrunnable path. Surface it at every context window. -Parse JSON for: `researcher_model`, `planner_model`, `checker_model`, `research_enabled`, `plan_checker_enabled`, `nyquist_validation_enabled`, `commit_docs`, `text_mode`, `phase_found`, `phase_dir`, `phase_number`, `phase_name`, `phase_slug`, `padded_phase`, `has_research`, `has_context`, `has_reviews`, `has_plans`, `plan_count`, `phase_status` (#3569), `planning_exists`, `roadmap_exists`, `phase_req_ids`, `response_language`, `granularity`, `prior_verify_commands` (#2401 — array of `{phase, plan, task, command}`, possibly empty; emitted at every context window). +Parse JSON for: `researcher_model`, `planner_model`, `checker_model`, `research_enabled`, `plan_checker_enabled`, `nyquist_validation_enabled`, `commit_docs`, `text_mode`, `phase_found`, `phase_dir`, `phase_number`, `phase_name`, `phase_slug`, `padded_phase`, `has_research`, `has_context`, `has_reviews`, `has_plans`, `plan_count`, `phase_status` (#3569), `planning_exists`, `roadmap_exists`, `phase_req_ids`, `response_language`, `granularity`, `prior_verify_commands` (#2401 — array of `{phase, plan, task, command}`, possibly empty; emitted at every context window), `threat_id_duplicates` + `threat_id_duplicate_count` (#4683 — cross-plan threat-ID collisions; consumed at step 5.55). **#2517:** omit the `model=` param from an `Agent()` call when its `researcher`/`planner`/`checker`_model is `"inherit"` or empty — passing `model=""` 404s on non-Claude runtimes; omitting inherits the orchestrator model (mirrors execute-phase). @@ -467,9 +467,11 @@ PLAN_PRE_HOOKS_JSON=$(gsd_run loop render-hooks plan:pre --raw) Resolve active contribution hooks from `PLAN_PRE_HOOKS_JSON` where `kind == "contribution"` and `capId == "security"`. +**Threat-ID uniqueness (#4683 — applies whether or not the security hook is active):** if the init payload's `threat_id_duplicate_count` is non-zero, init reports `threat_id_duplicates` — each `T-{phase}-NN` ID claimed by more than one live PLAN file in this phase. Surface the list to the planner spawn prompt in step 8 — "these threat IDs are already claimed by earlier plans in this phase: {list}; number new registers continuing after the phase's highest in-use `T-{phase}-NN`". Regardless of the count, include the numbering rule in the planner spawn prompt whenever this phase already has PLAN files: threat IDs are unique within a phase, and new registers continue after the highest in-use `T-{phase}-NN` — the count only reports an EXISTING collision, it cannot prevent the first one. The reserved `T-{phase}-SC` row is never listed. execute-phase hard-stops on a non-empty list regardless of what happened here. + **If no active security contribution hook exists:** Skip to step 5.6. -**If an active security contribution hook exists:** Read `SECURITY_ASVS` from the active hook's `configValues.security_asvs_level` (default: `1`) and `SECURITY_BLOCK` from `configValues.security_block_on` (default: `"high"`). These values are resolved by the capability registry from user config using the same four-level precedence as hook activation — no inline `config-get` is needed. +If an active security contribution hook exists, read `SECURITY_ASVS` from the hook's `configValues.security_asvs_level` (default: `1`) and `SECURITY_BLOCK` from `configValues.security_block_on` (default: `"high"`). These values are resolved by the capability registry from user config using the same four-level precedence as hook activation — no inline `config-get` is needed. Display banner: diff --git a/scripts/lib/macos-conformance-tier.generated.cjs b/scripts/lib/macos-conformance-tier.generated.cjs index 612a8c0b3..f7ccfc424 100644 --- a/scripts/lib/macos-conformance-tier.generated.cjs +++ b/scripts/lib/macos-conformance-tier.generated.cjs @@ -144,6 +144,7 @@ module.exports = { "tests/phase-locator.test.cjs", "tests/phase.test.cjs", "tests/plan-count-single-owner.test.cjs", + "tests/plan-document.test.cjs", "tests/plan-phase-stall-detection.test.cjs", "tests/plan-review-convergence.test.cjs", "tests/planning-inspect.test.cjs", diff --git a/src/init.cts b/src/init.cts index 902be1e2d..5545526e0 100644 --- a/src/init.cts +++ b/src/init.cts @@ -39,6 +39,8 @@ type Scope = planningScopeMod.Scope; import { maskIfSecret } from './secrets.cjs'; // eslint-disable-next-line @typescript-eslint/no-require-imports -- plan-scan.cjs is an export= CommonJS module import scanPhasePlans = require('./plan-scan.cjs'); +// eslint-disable-next-line @typescript-eslint/no-require-imports -- plan-document.cjs is an export= CommonJS module +import planDocument = require('./plan-document.cjs'); import { stateExtractField } from './state-document.cjs'; import { formatGsdSlash, resolveRuntime } from './runtime-slash.cjs'; import { resolveReportedRuntime } from './host-runtime-detection.cjs'; @@ -894,6 +896,50 @@ function milestoneRecord(cwd: string): Record { return (getMilestoneInfo(cwd).value ?? {}) as unknown as Record; } +/** + * #4683 — threat IDs claimed by more than one of the phase's live PLAN files. + * `scanPhasePlans().planFiles` is the right input set twice over: it excludes + * derivative files (OUTLINE / PLAN-REVIEW / pre-bounce) AND `status: superseded` + * plans (#2349) — a superseded plan's IDs were deliberately reassigned to its + * replacement, so they must not hold against it. The per-document row parse is + * planDocument.extractThreatRegisterIds; only `` register rows + * count, and the reserved `T-{phase}-SC` shape is excluded there by grammar + * (every plan keeps that row by design). A missing/unreadable phase directory + * degrades to "no duplicates" — planning a brand-new phase has nothing to + * collide with. + */ +function findDuplicateThreatIds(cwd: string, phaseDirRel: string | null | undefined): Array<{ id: string; plans: string[] }> { + if (!phaseDirRel) return []; + const phaseDir = path.join(cwd, phaseDirRel); + let planFiles: string[]; + try { + planFiles = scanPhasePlans(phaseDir).planFiles; + } catch { + return []; + } + const owners = new Map(); + for (const planFile of planFiles) { + let content: string; + try { + content = fs.readFileSync(path.join(phaseDir, planFile), 'utf-8'); + } catch { + continue; + } + for (const id of planDocument.extractThreatRegisterIds(content)) { + const claimed = owners.get(id); + if (claimed) { + if (!claimed.includes(planFile)) claimed.push(planFile); + } else { + owners.set(id, [planFile]); + } + } + } + return [...owners.entries()] + .filter(([, claims]) => claims.length > 1) + .map(([id, plans]) => ({ id, plans: [...plans].sort() })) + .sort((a, b) => (a.id < b.id ? -1 : a.id > b.id ? 1 : 0)); +} + function cmdInitExecutePhase( cwd: string, phase: string, @@ -957,6 +1003,9 @@ function cmdInitExecutePhase( const roadmapPath = path.join(planningDir(cwd), 'ROADMAP.md'); const requirementsPath = path.join(planningDir(cwd), 'REQUIREMENTS.md'); + // #4683: computed once — see the threat_id_duplicates fields in the payload. + const threatIdDuplicates = findDuplicateThreatIds(cwd, phaseInfo?.['directory'] as string | undefined); + const result: Record = { executor_model: resolveModelInternal(cwd, 'gsd-executor'), verifier_model: resolveModelInternal(cwd, 'gsd-verifier'), @@ -998,6 +1047,13 @@ function cmdInitExecutePhase( plan_count: (phaseInfo?.['plans'] as unknown[] | undefined)?.length || 0, incomplete_count: (phaseInfo?.['incomplete_plans'] as unknown[] | undefined)?.length || 0, + // #4683: cross-plan threat-ID collisions (gap-closure plans renumbering + // from T-{phase}-01 again). execute-phase.md hard-stops on a non-empty + // list BEFORE any dispatch — SECURITY.md rows and VALIDATION.md's Threat + // Ref column key on this ID, so a reused ID is ambiguous downstream. + threat_id_duplicates: threatIdDuplicates, + threat_id_duplicate_count: threatIdDuplicates.length, + // #2830: the halt-aware view, forwarded from the shared computation in // phase-locator. Additive — `incomplete_plans`/`incomplete_count` above keep // their exact name, type and semantics. Without this passthrough the shared @@ -1142,6 +1198,9 @@ function cmdInitPlanPhase( const granularityOverride = options['granularity'] as string | undefined; assertValidGranularityOverride(granularityOverride, error); + + // #4683: computed once — see the threat_id_duplicates fields in the payload. + const threatIdDuplicatesPlan = findDuplicateThreatIds(cwd, phaseDirPlan); const granularity = resolveGranularityInternal(cwd, 'planning', granularityOverride || undefined); // #3188: see cmdInitExecutePhase — null when absent, parity with the @@ -1198,6 +1257,12 @@ function cmdInitPlanPhase( has_plans: ((phaseInfo?.['plans'] as unknown[] | undefined)?.length || 0) > 0, plan_count: (phaseInfo?.['plans'] as unknown[] | undefined)?.length || 0, + // #4683: the same cross-plan threat-ID duplicate list execute-phase gates + // on, surfaced at PLAN time so the checker/reviewer catches the collision + // before the plans are approved — not just before execution. + threat_id_duplicates: threatIdDuplicatesPlan, + threat_id_duplicate_count: threatIdDuplicatesPlan.length, + planning_exists: fs.existsSync(planningDir(cwd)), roadmap_exists: fs.existsSync(path.join(planningDir(cwd), 'ROADMAP.md')), diff --git a/src/plan-document.cts b/src/plan-document.cts index 664fe9db4..974ba0045 100644 --- a/src/plan-document.cts +++ b/src/plan-document.cts @@ -357,7 +357,7 @@ function parsePlanDocument(content: string, planPath = ''): PlanDocument { }; } -const planDocument = { TASK_KIND, parsePlanDocument, planIdFromFile }; +const planDocument = { TASK_KIND, parsePlanDocument, planIdFromFile, extractThreatRegisterIds }; // Required to merge the compile-time-only types onto the `export =` runtime // value; there is no ES-module-syntax way to export a type alongside a CJS @@ -368,3 +368,52 @@ declare namespace planDocument { } export = planDocument; + +/** + * #4683 — first-cell IDs of the STRIDE register rows inside every + * `` block. The register is a markdown table (the + * `` template in agents/gsd-planner.md): one row per threat, + * first cell `T-{phase}-NN` — decimal phases included — or the reserved + * `T-{phase}-SC` supply-chain row. Only digit-suffixed IDs match: `-SC` is + * deliberately shared by EVERY plan in a phase (planner rule "Keep + * `T-{phase}-SC` in ``"), so it can never be a uniqueness + * violation. IDs in prose or non-threat tables never count; only register + * rows inside a threat_model block do. One entry per matched row, in document + * order — deciding that the same ID in two plans is a collision is the + * aggregator's question (init.cts), not the per-document parser's. + * + * Knowingly unmatched residual classes (#4683 review, accepted): lowercase + * `t-47-01`, letter suffixes (`T-47-05A`), annotated first cells + * (`| T-47-06 (revised) |`), IDs in non-first cells, and an unterminated + * `` block all yield no claim. All are off-template shapes — the + * planner template fixes the row grammar — so the residual risk is silent + * under-detection, never a false hard-stop. + */ +const THREAT_MODEL_BLOCK_RE = /([\s\S]*?)<\/threat_model>/gi; +const THREAT_REGISTER_ROW_RE = /^[^\S\n]*\|[^\S\n]*(T-\d+(?:\.\d+)?-\d+)[^\S\n]*\|/; + +function extractThreatRegisterIds(content: string): string[] { + // Fenced code blocks are prose, not registers (#4683 review MAJOR): a plan + // that QUOTES an existing register — exactly what the gap-closure flow tells + // the planner to read — must not have its quoted IDs counted as claims, or + // the execute-phase gate would hard-stop a correct phase. Same line-toggling + // idiom as the deferred-scope scan in phase.cts. + const lines: string[] = []; + let inFence = false; + for (const line of content.split(/\r?\n/)) { + if (/^\s*(?:```|~~~)/.test(line)) { + inFence = !inFence; + lines.push(''); + continue; + } + lines.push(inFence ? '' : line); + } + const ids: string[] = []; + for (const blockMatch of lines.join('\n').matchAll(THREAT_MODEL_BLOCK_RE)) { + for (const line of blockMatch[1].split('\n')) { + const row = line.match(THREAT_REGISTER_ROW_RE); + if (row) ids.push(row[1]); + } + } + return ids; +} diff --git a/tests/fixtures/compact-content-benchmark-baseline.json b/tests/fixtures/compact-content-benchmark-baseline.json index 09f80c519..ba29cdac1 100644 --- a/tests/fixtures/compact-content-benchmark-baseline.json +++ b/tests/fixtures/compact-content-benchmark-baseline.json @@ -18,9 +18,9 @@ "reductionPct": 16.51 }, "execute-phase": { - "offTokens": 26297, - "onTokens": 24007, - "reductionPct": 8.71 + "offTokens": 26184, + "onTokens": 23894, + "reductionPct": 8.75 }, "new-project": { "offTokens": 14308, @@ -28,9 +28,9 @@ "reductionPct": 13.59 }, "plan-phase": { - "offTokens": 27641, - "onTokens": 24351, - "reductionPct": 11.9 + "offTokens": 27878, + "onTokens": 24588, + "reductionPct": 11.8 }, "verify-work": { "offTokens": 13384, @@ -39,8 +39,8 @@ } }, "aggregate": { - "offTokens": 109060, - "onTokens": 92373, - "reductionPct": 15.3 + "offTokens": 109184, + "onTokens": 92497, + "reductionPct": 15.28 } } diff --git a/tests/fixtures/install-tree/antigravity.json b/tests/fixtures/install-tree/antigravity.json index 5e2861a3b..7ccd8cf56 100644 --- a/tests/fixtures/install-tree/antigravity.json +++ b/tests/fixtures/install-tree/antigravity.json @@ -455,7 +455,9 @@ "gsd-core/workflows/execute-phase/steps/sequential-root-pin.md", "gsd-core/workflows/execute-phase/steps/stale-reverification.md", "gsd-core/workflows/execute-phase/steps/tdd-applicability-resolution.md", + "gsd-core/workflows/execute-phase/steps/threat-id-gate.md", "gsd-core/workflows/execute-phase/steps/wave-post-gate-hooks.md", + "gsd-core/workflows/execute-phase/steps/worktree-base-check.md", "gsd-core/workflows/execute-phase/steps/worktree-recovery-policy.md", "gsd-core/workflows/execute-plan.md", "gsd-core/workflows/explore.md", diff --git a/tests/fixtures/install-tree/augment.json b/tests/fixtures/install-tree/augment.json index 3f9eb2fea..8b9d86388 100644 --- a/tests/fixtures/install-tree/augment.json +++ b/tests/fixtures/install-tree/augment.json @@ -527,7 +527,9 @@ "gsd-core/workflows/execute-phase/steps/sequential-root-pin.md", "gsd-core/workflows/execute-phase/steps/stale-reverification.md", "gsd-core/workflows/execute-phase/steps/tdd-applicability-resolution.md", + "gsd-core/workflows/execute-phase/steps/threat-id-gate.md", "gsd-core/workflows/execute-phase/steps/wave-post-gate-hooks.md", + "gsd-core/workflows/execute-phase/steps/worktree-base-check.md", "gsd-core/workflows/execute-phase/steps/worktree-recovery-policy.md", "gsd-core/workflows/execute-plan.md", "gsd-core/workflows/explore.md", diff --git a/tests/fixtures/install-tree/claude-local.json b/tests/fixtures/install-tree/claude-local.json index c159b2c27..ed43854d1 100644 --- a/tests/fixtures/install-tree/claude-local.json +++ b/tests/fixtures/install-tree/claude-local.json @@ -391,7 +391,9 @@ "gsd-core/workflows/execute-phase/steps/sequential-root-pin.md", "gsd-core/workflows/execute-phase/steps/stale-reverification.md", "gsd-core/workflows/execute-phase/steps/tdd-applicability-resolution.md", + "gsd-core/workflows/execute-phase/steps/threat-id-gate.md", "gsd-core/workflows/execute-phase/steps/wave-post-gate-hooks.md", + "gsd-core/workflows/execute-phase/steps/worktree-base-check.md", "gsd-core/workflows/execute-phase/steps/worktree-recovery-policy.md", "gsd-core/workflows/execute-plan.md", "gsd-core/workflows/explore.md", diff --git a/tests/fixtures/install-tree/claude.json b/tests/fixtures/install-tree/claude.json index 367d39b00..a14d356db 100644 --- a/tests/fixtures/install-tree/claude.json +++ b/tests/fixtures/install-tree/claude.json @@ -455,7 +455,9 @@ "gsd-core/workflows/execute-phase/steps/sequential-root-pin.md", "gsd-core/workflows/execute-phase/steps/stale-reverification.md", "gsd-core/workflows/execute-phase/steps/tdd-applicability-resolution.md", + "gsd-core/workflows/execute-phase/steps/threat-id-gate.md", "gsd-core/workflows/execute-phase/steps/wave-post-gate-hooks.md", + "gsd-core/workflows/execute-phase/steps/worktree-base-check.md", "gsd-core/workflows/execute-phase/steps/worktree-recovery-policy.md", "gsd-core/workflows/execute-plan.md", "gsd-core/workflows/explore.md", diff --git a/tests/fixtures/install-tree/cline.json b/tests/fixtures/install-tree/cline.json index 723ed06ac..1099af2d9 100644 --- a/tests/fixtures/install-tree/cline.json +++ b/tests/fixtures/install-tree/cline.json @@ -457,7 +457,9 @@ "gsd-core/workflows/execute-phase/steps/sequential-root-pin.md", "gsd-core/workflows/execute-phase/steps/stale-reverification.md", "gsd-core/workflows/execute-phase/steps/tdd-applicability-resolution.md", + "gsd-core/workflows/execute-phase/steps/threat-id-gate.md", "gsd-core/workflows/execute-phase/steps/wave-post-gate-hooks.md", + "gsd-core/workflows/execute-phase/steps/worktree-base-check.md", "gsd-core/workflows/execute-phase/steps/worktree-recovery-policy.md", "gsd-core/workflows/execute-plan.md", "gsd-core/workflows/explore.md", diff --git a/tests/fixtures/install-tree/codebuddy.json b/tests/fixtures/install-tree/codebuddy.json index 2a806c4b4..baf3d5eed 100644 --- a/tests/fixtures/install-tree/codebuddy.json +++ b/tests/fixtures/install-tree/codebuddy.json @@ -527,7 +527,9 @@ "gsd-core/workflows/execute-phase/steps/sequential-root-pin.md", "gsd-core/workflows/execute-phase/steps/stale-reverification.md", "gsd-core/workflows/execute-phase/steps/tdd-applicability-resolution.md", + "gsd-core/workflows/execute-phase/steps/threat-id-gate.md", "gsd-core/workflows/execute-phase/steps/wave-post-gate-hooks.md", + "gsd-core/workflows/execute-phase/steps/worktree-base-check.md", "gsd-core/workflows/execute-phase/steps/worktree-recovery-policy.md", "gsd-core/workflows/execute-plan.md", "gsd-core/workflows/explore.md", diff --git a/tests/fixtures/install-tree/codex.json b/tests/fixtures/install-tree/codex.json index 85d68f767..05e5b5dbd 100644 --- a/tests/fixtures/install-tree/codex.json +++ b/tests/fixtures/install-tree/codex.json @@ -491,7 +491,9 @@ "gsd-core/workflows/execute-phase/steps/sequential-root-pin.md", "gsd-core/workflows/execute-phase/steps/stale-reverification.md", "gsd-core/workflows/execute-phase/steps/tdd-applicability-resolution.md", + "gsd-core/workflows/execute-phase/steps/threat-id-gate.md", "gsd-core/workflows/execute-phase/steps/wave-post-gate-hooks.md", + "gsd-core/workflows/execute-phase/steps/worktree-base-check.md", "gsd-core/workflows/execute-phase/steps/worktree-recovery-policy.md", "gsd-core/workflows/execute-plan.md", "gsd-core/workflows/explore.md", diff --git a/tests/fixtures/install-tree/copilot.json b/tests/fixtures/install-tree/copilot.json index 451c467fb..5e040b96c 100644 --- a/tests/fixtures/install-tree/copilot.json +++ b/tests/fixtures/install-tree/copilot.json @@ -456,7 +456,9 @@ "gsd-core/workflows/execute-phase/steps/sequential-root-pin.md", "gsd-core/workflows/execute-phase/steps/stale-reverification.md", "gsd-core/workflows/execute-phase/steps/tdd-applicability-resolution.md", + "gsd-core/workflows/execute-phase/steps/threat-id-gate.md", "gsd-core/workflows/execute-phase/steps/wave-post-gate-hooks.md", + "gsd-core/workflows/execute-phase/steps/worktree-base-check.md", "gsd-core/workflows/execute-phase/steps/worktree-recovery-policy.md", "gsd-core/workflows/execute-plan.md", "gsd-core/workflows/explore.md", diff --git a/tests/fixtures/install-tree/cursor.json b/tests/fixtures/install-tree/cursor.json index 15b8db94a..a5726be4d 100644 --- a/tests/fixtures/install-tree/cursor.json +++ b/tests/fixtures/install-tree/cursor.json @@ -455,7 +455,9 @@ "gsd-core/workflows/execute-phase/steps/sequential-root-pin.md", "gsd-core/workflows/execute-phase/steps/stale-reverification.md", "gsd-core/workflows/execute-phase/steps/tdd-applicability-resolution.md", + "gsd-core/workflows/execute-phase/steps/threat-id-gate.md", "gsd-core/workflows/execute-phase/steps/wave-post-gate-hooks.md", + "gsd-core/workflows/execute-phase/steps/worktree-base-check.md", "gsd-core/workflows/execute-phase/steps/worktree-recovery-policy.md", "gsd-core/workflows/execute-plan.md", "gsd-core/workflows/explore.md", diff --git a/tests/fixtures/install-tree/hermes.json b/tests/fixtures/install-tree/hermes.json index 54d8d7346..32bec5ffd 100644 --- a/tests/fixtures/install-tree/hermes.json +++ b/tests/fixtures/install-tree/hermes.json @@ -455,7 +455,9 @@ "gsd-core/workflows/execute-phase/steps/sequential-root-pin.md", "gsd-core/workflows/execute-phase/steps/stale-reverification.md", "gsd-core/workflows/execute-phase/steps/tdd-applicability-resolution.md", + "gsd-core/workflows/execute-phase/steps/threat-id-gate.md", "gsd-core/workflows/execute-phase/steps/wave-post-gate-hooks.md", + "gsd-core/workflows/execute-phase/steps/worktree-base-check.md", "gsd-core/workflows/execute-phase/steps/worktree-recovery-policy.md", "gsd-core/workflows/execute-plan.md", "gsd-core/workflows/explore.md", diff --git a/tests/fixtures/install-tree/kilo.json b/tests/fixtures/install-tree/kilo.json index 5b7ed0724..f490e3ed6 100644 --- a/tests/fixtures/install-tree/kilo.json +++ b/tests/fixtures/install-tree/kilo.json @@ -527,7 +527,9 @@ "gsd-core/workflows/execute-phase/steps/sequential-root-pin.md", "gsd-core/workflows/execute-phase/steps/stale-reverification.md", "gsd-core/workflows/execute-phase/steps/tdd-applicability-resolution.md", + "gsd-core/workflows/execute-phase/steps/threat-id-gate.md", "gsd-core/workflows/execute-phase/steps/wave-post-gate-hooks.md", + "gsd-core/workflows/execute-phase/steps/worktree-base-check.md", "gsd-core/workflows/execute-phase/steps/worktree-recovery-policy.md", "gsd-core/workflows/execute-plan.md", "gsd-core/workflows/explore.md", diff --git a/tests/fixtures/install-tree/kimi-code.json b/tests/fixtures/install-tree/kimi-code.json index b30956d6d..eaa409aa1 100644 --- a/tests/fixtures/install-tree/kimi-code.json +++ b/tests/fixtures/install-tree/kimi-code.json @@ -456,7 +456,9 @@ "gsd-core/workflows/execute-phase/steps/sequential-root-pin.md", "gsd-core/workflows/execute-phase/steps/stale-reverification.md", "gsd-core/workflows/execute-phase/steps/tdd-applicability-resolution.md", + "gsd-core/workflows/execute-phase/steps/threat-id-gate.md", "gsd-core/workflows/execute-phase/steps/wave-post-gate-hooks.md", + "gsd-core/workflows/execute-phase/steps/worktree-base-check.md", "gsd-core/workflows/execute-phase/steps/worktree-recovery-policy.md", "gsd-core/workflows/execute-plan.md", "gsd-core/workflows/explore.md", diff --git a/tests/fixtures/install-tree/kimi.json b/tests/fixtures/install-tree/kimi.json index 6ba2149df..86c24e84c 100644 --- a/tests/fixtures/install-tree/kimi.json +++ b/tests/fixtures/install-tree/kimi.json @@ -463,7 +463,9 @@ "gsd-core/workflows/execute-phase/steps/sequential-root-pin.md", "gsd-core/workflows/execute-phase/steps/stale-reverification.md", "gsd-core/workflows/execute-phase/steps/tdd-applicability-resolution.md", + "gsd-core/workflows/execute-phase/steps/threat-id-gate.md", "gsd-core/workflows/execute-phase/steps/wave-post-gate-hooks.md", + "gsd-core/workflows/execute-phase/steps/worktree-base-check.md", "gsd-core/workflows/execute-phase/steps/worktree-recovery-policy.md", "gsd-core/workflows/execute-plan.md", "gsd-core/workflows/explore.md", diff --git a/tests/fixtures/install-tree/opencode.json b/tests/fixtures/install-tree/opencode.json index 7cac397a2..fda72eb19 100644 --- a/tests/fixtures/install-tree/opencode.json +++ b/tests/fixtures/install-tree/opencode.json @@ -527,7 +527,9 @@ "gsd-core/workflows/execute-phase/steps/sequential-root-pin.md", "gsd-core/workflows/execute-phase/steps/stale-reverification.md", "gsd-core/workflows/execute-phase/steps/tdd-applicability-resolution.md", + "gsd-core/workflows/execute-phase/steps/threat-id-gate.md", "gsd-core/workflows/execute-phase/steps/wave-post-gate-hooks.md", + "gsd-core/workflows/execute-phase/steps/worktree-base-check.md", "gsd-core/workflows/execute-phase/steps/worktree-recovery-policy.md", "gsd-core/workflows/execute-plan.md", "gsd-core/workflows/explore.md", diff --git a/tests/fixtures/install-tree/pi.json b/tests/fixtures/install-tree/pi.json index 53a9c4e8e..e94a789e5 100644 --- a/tests/fixtures/install-tree/pi.json +++ b/tests/fixtures/install-tree/pi.json @@ -257,7 +257,9 @@ "gsd-core/workflows/execute-phase/steps/sequential-root-pin.md", "gsd-core/workflows/execute-phase/steps/stale-reverification.md", "gsd-core/workflows/execute-phase/steps/tdd-applicability-resolution.md", + "gsd-core/workflows/execute-phase/steps/threat-id-gate.md", "gsd-core/workflows/execute-phase/steps/wave-post-gate-hooks.md", + "gsd-core/workflows/execute-phase/steps/worktree-base-check.md", "gsd-core/workflows/execute-phase/steps/worktree-recovery-policy.md", "gsd-core/workflows/execute-plan.md", "gsd-core/workflows/explore.md", diff --git a/tests/fixtures/install-tree/qwen.json b/tests/fixtures/install-tree/qwen.json index 83bc000da..120f45621 100644 --- a/tests/fixtures/install-tree/qwen.json +++ b/tests/fixtures/install-tree/qwen.json @@ -455,7 +455,9 @@ "gsd-core/workflows/execute-phase/steps/sequential-root-pin.md", "gsd-core/workflows/execute-phase/steps/stale-reverification.md", "gsd-core/workflows/execute-phase/steps/tdd-applicability-resolution.md", + "gsd-core/workflows/execute-phase/steps/threat-id-gate.md", "gsd-core/workflows/execute-phase/steps/wave-post-gate-hooks.md", + "gsd-core/workflows/execute-phase/steps/worktree-base-check.md", "gsd-core/workflows/execute-phase/steps/worktree-recovery-policy.md", "gsd-core/workflows/execute-plan.md", "gsd-core/workflows/explore.md", diff --git a/tests/fixtures/install-tree/trae.json b/tests/fixtures/install-tree/trae.json index 39fdd07c3..5e6ffc526 100644 --- a/tests/fixtures/install-tree/trae.json +++ b/tests/fixtures/install-tree/trae.json @@ -455,7 +455,9 @@ "gsd-core/workflows/execute-phase/steps/sequential-root-pin.md", "gsd-core/workflows/execute-phase/steps/stale-reverification.md", "gsd-core/workflows/execute-phase/steps/tdd-applicability-resolution.md", + "gsd-core/workflows/execute-phase/steps/threat-id-gate.md", "gsd-core/workflows/execute-phase/steps/wave-post-gate-hooks.md", + "gsd-core/workflows/execute-phase/steps/worktree-base-check.md", "gsd-core/workflows/execute-phase/steps/worktree-recovery-policy.md", "gsd-core/workflows/execute-plan.md", "gsd-core/workflows/explore.md", diff --git a/tests/fixtures/install-tree/windsurf.json b/tests/fixtures/install-tree/windsurf.json index 16953c416..09d5101ad 100644 --- a/tests/fixtures/install-tree/windsurf.json +++ b/tests/fixtures/install-tree/windsurf.json @@ -383,7 +383,9 @@ "gsd-core/workflows/execute-phase/steps/sequential-root-pin.md", "gsd-core/workflows/execute-phase/steps/stale-reverification.md", "gsd-core/workflows/execute-phase/steps/tdd-applicability-resolution.md", + "gsd-core/workflows/execute-phase/steps/threat-id-gate.md", "gsd-core/workflows/execute-phase/steps/wave-post-gate-hooks.md", + "gsd-core/workflows/execute-phase/steps/worktree-base-check.md", "gsd-core/workflows/execute-phase/steps/worktree-recovery-policy.md", "gsd-core/workflows/execute-plan.md", "gsd-core/workflows/explore.md", diff --git a/tests/fixtures/install-tree/zcode.json b/tests/fixtures/install-tree/zcode.json index 0edb96446..f327b6d7e 100644 --- a/tests/fixtures/install-tree/zcode.json +++ b/tests/fixtures/install-tree/zcode.json @@ -527,7 +527,9 @@ "gsd-core/workflows/execute-phase/steps/sequential-root-pin.md", "gsd-core/workflows/execute-phase/steps/stale-reverification.md", "gsd-core/workflows/execute-phase/steps/tdd-applicability-resolution.md", + "gsd-core/workflows/execute-phase/steps/threat-id-gate.md", "gsd-core/workflows/execute-phase/steps/wave-post-gate-hooks.md", + "gsd-core/workflows/execute-phase/steps/worktree-base-check.md", "gsd-core/workflows/execute-phase/steps/worktree-recovery-policy.md", "gsd-core/workflows/execute-plan.md", "gsd-core/workflows/explore.md", diff --git a/tests/init.test.cjs b/tests/init.test.cjs index 0a549d581..93a6c720e 100644 --- a/tests/init.test.cjs +++ b/tests/init.test.cjs @@ -5340,3 +5340,197 @@ describe('init plan-phase — wrapped Goal/Requirements fields (#4731)', () => { ); }); }); + +// ── #4683 — gap-closure plans reused threat IDs that earlier plans in the ──── +// same phase had already assigned to different threats. Nothing detected it: +// SECURITY.md rows and VALIDATION.md's Threat Ref column key on the ID, so a +// reused ID makes every downstream consumer ambiguous. The init payloads now +// carry the cross-plan duplicate list (T-{phase}-NN shapes; the reserved +// T-{phase}-SC supply-chain row is deliberately shared and never flagged), and +// execute-phase.md hard-stops on a non-empty list before any dispatch. +describe('#4683 — cross-plan threat-ID duplicate detection', () => { + let tmpDir; + + beforeEach(() => { + tmpDir = fs.realpathSync(createFixture()); + }); + afterEach(() => cleanup(tmpDir)); + + function threatPlan({ ids, gapClosure = false, superseded = false, withSc = true }) { + const rows = ids.map((id) => `| ${id} | Tampering | component | medium | mitigate | fix it |`); + return [ + ...(superseded ? ['---', 'status: superseded', '---', ''] : []), + '# Plan', + '', + ...(gapClosure ? ['gap_closure: true', ''] : []), + '', + '| Threat ID | Category | Component | Severity | Disposition | Mitigation |', + '|-----------|----------|-----------|----------|-------------|------------|', + ...(withSc ? ['| T-47-SC | Tampering | npm installs | high | mitigate | legitimacy gate |'] : []), + ...rows, + '', + '', + ].join('\n'); + } + + test('init execute-phase reports threat IDs reused across plans (#4683)', () => { + seedPhase(tmpDir, '47-security', { + // Earlier plans own T-47-01..09 / 10..14 / 15..19. + '47-03-PLAN.md': threatPlan({ ids: ['T-47-01', 'T-47-02', 'T-47-03'] }), + '47-04-PLAN.md': threatPlan({ ids: ['T-47-10', 'T-47-11', 'T-47-15'] }), + '47-05-PLAN.md': threatPlan({ ids: ['T-47-19'] }), + // Gap-closure plans renumber from 01 again — the bug: every ID below is + // already claimed by an earlier plan for a DIFFERENT threat. + '47-06-PLAN.md': threatPlan({ ids: ['T-47-10', 'T-47-11', 'T-47-19'], gapClosure: true }), + '47-07-PLAN.md': threatPlan({ ids: ['T-47-15'], gapClosure: true }), + }); + writePlanningDocs(tmpDir); + + const result = runGsdTools('init execute-phase 47', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + const output = JSON.parse(result.output); + + const duplicates = output.threat_id_duplicates; + assert.ok(Array.isArray(duplicates), 'threat_id_duplicates must be an array'); + const byId = Object.fromEntries(duplicates.map((d) => [d.id, d.plans])); + for (const id of ['T-47-10', 'T-47-11', 'T-47-15', 'T-47-19']) { + assert.ok(byId[id], `reused ID ${id} must be reported, got: ${JSON.stringify(duplicates)}`); + assert.ok(byId[id].length >= 2, `${id} must name at least the two plans claiming it`); + } + assert.strictEqual(byId['T-47-15'][0], '47-04-PLAN.md'); + assert.strictEqual(byId['T-47-15'][1], '47-07-PLAN.md'); + // Only genuinely reused IDs — the unique ones stay out. + assert.ok(!byId['T-47-01'] && !byId['T-47-02'] && !byId['T-47-03'], 'uniquely-claimed IDs must not be reported'); + assert.strictEqual(output.threat_id_duplicate_count, 4, + `count must match the duplicate list, got ${output.threat_id_duplicate_count} for ${JSON.stringify(duplicates)}`); + // The reserved supply-chain ID is shared BY DESIGN (every plan keeps it). + assert.ok(!byId['T-47-SC'], 'T-47-SC is reserved and deliberately shared — never a duplicate'); + }); + + test('init execute-phase reports no duplicates for unique registers (#4683)', () => { + seedPhase(tmpDir, '47-security', { + '47-03-PLAN.md': threatPlan({ ids: ['T-47-01', 'T-47-02'] }), + '47-04-PLAN.md': threatPlan({ ids: ['T-47-03', 'T-47-04'] }), + }); + writePlanningDocs(tmpDir); + + const result = runGsdTools('init execute-phase 47', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + const output = JSON.parse(result.output); + assert.deepEqual(output.threat_id_duplicates, []); + assert.strictEqual(output.threat_id_duplicate_count, 0); + }); + + test('superseded plans do not hold threat IDs against their replacements (#4683)', () => { + seedPhase(tmpDir, '47-security', { + // Deliberately retired: its IDs moved to the replacing plan. + '47-03-PLAN.md': threatPlan({ ids: ['T-47-01'], superseded: true }), + '47-05-PLAN.md': threatPlan({ ids: ['T-47-01'] }), + }); + writePlanningDocs(tmpDir); + + const result = runGsdTools('init execute-phase 47', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + const output = JSON.parse(result.output); + assert.deepEqual(output.threat_id_duplicates, [], + 'a superseded plan\'s IDs were deliberately reassigned — not a collision'); + assert.strictEqual(output.threat_id_duplicate_count, 0); + }); + + test('init plan-phase surfaces the same duplicate list (#4683)', () => { + seedPhase(tmpDir, '47-security', { + '47-03-PLAN.md': threatPlan({ ids: ['T-47-01'] }), + '47-04-PLAN.md': threatPlan({ ids: ['T-47-01'] }), + }); + writePlanningDocs(tmpDir); + + const result = runGsdTools('init plan-phase 47', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + const output = JSON.parse(result.output); + assert.strictEqual(output.threat_id_duplicate_count, 1); + assert.deepEqual(output.threat_id_duplicates, [{ id: 'T-47-01', plans: ['47-03-PLAN.md', '47-04-PLAN.md'] }]); + }); + + test('IDs outside a block never count (#4683)', () => { + seedPhase(tmpDir, '47-security', { + '47-03-PLAN.md': '# Plan\n\nSee T-47-01 in SECURITY.md. | T-47-02 | not a register |\n', + '47-04-PLAN.md': '# Plan\n\nSee T-47-01 again.\n', + }); + writePlanningDocs(tmpDir); + + const result = runGsdTools('init execute-phase 47', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + const output = JSON.parse(result.output); + assert.deepEqual(output.threat_id_duplicates, [], + 'prose mentions of an ID are not register rows — only tables count'); + assert.strictEqual(output.threat_id_duplicate_count, 0); + }); +}); + +// ── #4683 review repairs ───────────────────────────────────────────────────── +describe('#4683 review repairs — fence blindness and deterministic ordering', () => { + let tmpDir; + + beforeEach(() => { + tmpDir = fs.realpathSync(createFixture()); + }); + afterEach(() => cleanup(tmpDir)); + + test('a register QUOTED inside a code fence is not a claim (#4683 review MAJOR)', () => { + seedPhase(tmpDir, '47-security', { + '47-03-PLAN.md': [ + '# Plan', '', + '', + '| T-47-01 | Tampering | component | high | mitigate | fix |', + '', '', + 'The register we are extending (quoted verbatim):', '', + '```markdown', + '', + '| Threat ID | Category | Component | Severity | Disposition | Mitigation |', + '|-----------|----------|-----------|----------|-------------|------------|', + '| T-47-01 | Tampering | component | high | mitigate | fix |', + '', + '```', '', + ].join('\n'), + '47-04-PLAN.md': [ + '# Plan', '', + '', + '| T-47-02 | Repudiation | component | low | accept | rationale |', + '', '', + 'Reference copy of phase 47-03\'s register:', '', + '~~~', + '', + '| T-47-01 | Tampering | component | high | mitigate | fix |', + '', + '~~~', '', + ].join('\n'), + }); + writePlanningDocs(tmpDir); + + const result = runGsdTools('init execute-phase 47', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + const output = JSON.parse(result.output); + assert.deepEqual(output.threat_id_duplicates, [], + 'quoted registers live in fenced code blocks — prose, not claims; only live blocks count'); + assert.strictEqual(output.threat_id_duplicate_count, 0); + }); + + test('duplicate entries list claiming plans in deterministic sorted order (#4683 review MINOR)', () => { + seedPhase(tmpDir, '47-security', { + '47-07-PLAN.md': [ + '# Plan', '', '', '| T-47-15 | DoS | component | medium | mitigate | fix |', '', '', + ].join('\n'), + '47-04-PLAN.md': [ + '# Plan', '', '', '| T-47-15 | Repudiation | component | high | mitigate | fix |', '', '', + ].join('\n'), + }); + writePlanningDocs(tmpDir); + + const result = runGsdTools('init execute-phase 47', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + const output = JSON.parse(result.output); + assert.deepEqual(output.threat_id_duplicates, [ + { id: 'T-47-15', plans: ['47-04-PLAN.md', '47-07-PLAN.md'] }, + ], 'claiming-plan lists must be sorted, never readdir order'); + }); +}); diff --git a/tests/plan-document.test.cjs b/tests/plan-document.test.cjs index fd74e14f6..2d944970d 100644 --- a/tests/plan-document.test.cjs +++ b/tests/plan-document.test.cjs @@ -199,3 +199,102 @@ Some body text. assert.equal(t.trackerId, 'beads:GSD-7'); }); }); + +describe('plan-document: extractThreatRegisterIds (#4683)', () => { + const { extractThreatRegisterIds } = require('../gsd-core/bin/lib/plan-document.cjs'); + + const register = (rows) => [ + '', + '| Threat ID | Category | Component | Severity | Disposition | Mitigation |', + '|-----------|----------|-----------|----------|-------------|------------|', + ...rows, + '', + ].join('\n'); + + test('extracts first-cell IDs in document order', () => { + const ids = extractThreatRegisterIds(register([ + '| T-47-01 | Tampering | c | high | mitigate | fix |', + '| T-47-02 | Repudiation | c | low | accept | rationale |', + '| T-47-19 | DoS | c | medium | mitigate | fix |', + ])); + assert.deepEqual(ids, ['T-47-01', 'T-47-02', 'T-47-19']); + }); + + test('the reserved -SC supply-chain row never matches', () => { + const ids = extractThreatRegisterIds(register([ + '| T-47-SC | Tampering | npm installs | high | mitigate | gate |', + '| T-47-01 | Tampering | c | high | mitigate | fix |', + ])); + assert.deepEqual(ids, ['T-47-01']); + }); + + test('decimal phases match (T-4.1-05)', () => { + const ids = extractThreatRegisterIds(register(['| T-4.1-05 | Tampering | c | low | accept | r |'])); + assert.deepEqual(ids, ['T-4.1-05']); + }); + + test('rows outside a threat_model block never count', () => { + const ids = extractThreatRegisterIds([ + '# Plan', + '', + 'See T-47-01 in SECURITY.md. | T-47-02 | not a register |', + '', + register(['| T-47-03 | Tampering | c | high | mitigate | fix |']), + ].join('\n')); + assert.deepEqual(ids, ['T-47-03']); + }); + + test('a register quoted inside a backtick fence is prose, not a claim', () => { + const quoted = ['```markdown', register(['| T-47-01 | Tampering | c | high | mitigate | fix |']), '```'].join('\n'); + const live = register(['| T-47-02 | Repudiation | c | low | accept | r |']); + assert.deepEqual(extractThreatRegisterIds(`${quoted}\n\n${live}`), ['T-47-02']); + }); + + test('a tilde fence is stripped the same way', () => { + const quoted = ['~~~', register(['| T-47-01 | Tampering | c | high | mitigate | fix |']), '~~~'].join('\n'); + const live = register(['| T-47-02 | Repudiation | c | low | accept | r |']); + assert.deepEqual(extractThreatRegisterIds(`${quoted}\n\n${live}`), ['T-47-02']); + }); + + test('an unclosed fence suppresses everything after it', () => { + const doc = [register(['| T-47-01 | Tampering | c | high | mitigate | fix |']), '```', register(['| T-47-02 | DoS | c | low | accept | r |'])].join('\n'); + assert.deepEqual(extractThreatRegisterIds(doc), ['T-47-01']); + }); + + test('block tags are case-insensitive', () => { + const ids = extractThreatRegisterIds([ + '', '| T-47-05 | Tampering | c | high | mitigate | fix |', '', + ].join('\n')); + assert.deepEqual(ids, ['T-47-05']); + }); + + test('multiple blocks yield IDs across both, in order', () => { + const doc = [ + register(['| T-47-01 | Tampering | c | high | mitigate | fix |']), + '', + register(['| T-47-02 | Repudiation | c | low | accept | r |']), + ].join('\n'); + assert.deepEqual(extractThreatRegisterIds(doc), ['T-47-01', 'T-47-02']); + }); + + test('CRLF rows are read the same as LF', () => { + const ids = extractThreatRegisterIds(register(['| T-47-07 | Tampering | c | high | mitigate | fix |']).replace(/\n/g, '\r\n')); + assert.deepEqual(ids, ['T-47-07']); + }); + + test('indented rows still match on the first cell', () => { + const ids = extractThreatRegisterIds([ + '', ' | T-47-09 | Tampering | c | high | mitigate | fix |', '', + ].join('\n')); + assert.deepEqual(ids, ['T-47-09']); + }); + + test('annotated first cells are knowingly unmatched (accepted residual)', () => { + const ids = extractThreatRegisterIds(register(['| T-47-06 (revised) | Tampering | c | high | mitigate | fix |'])); + assert.deepEqual(ids, []); + }); + + test('content without any threat_model block yields an empty list', () => { + assert.deepEqual(extractThreatRegisterIds('# Plan\n\nDo things.\n'), []); + }); +});