diff --git a/.changeset/kind-deer-rest.md b/.changeset/kind-deer-rest.md new file mode 100644 index 000000000..716c7b71b --- /dev/null +++ b/.changeset/kind-deer-rest.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 1633 +--- +**`/gsd:secure-phase` now honors the configured ASVS level and block threshold** — the security auditor previously received unsubstituted `{SECURITY_ASVS}` / `{SECURITY_BLOCK_ON}` placeholder text because secure-phase.md never assigned those variables. It now resolves `workflow.security_asvs_level` and `workflow.security_block_on` from config (`--raw`) before the auditor handoff. (#1625) diff --git a/gsd-core/workflows/secure-phase.md b/gsd-core/workflows/secure-phase.md index 90a6d20cf..264ecccd5 100644 --- a/gsd-core/workflows/secure-phase.md +++ b/gsd-core/workflows/secure-phase.md @@ -27,6 +27,8 @@ Parse: `phase_dir`, `phase_number`, `phase_name`, `phase_slug`, `padded_phase`. ```bash AUDITOR_MODEL=$(gsd_run query resolve-model gsd-security-auditor --raw) VERIFY_POST_HOOKS_JSON=$(gsd_run loop render-hooks verify:post --raw) +SECURITY_ASVS=$(gsd_run query config-get workflow.security_asvs_level --raw 2>/dev/null || echo "1") +SECURITY_BLOCK_ON=$(gsd_run query config-get workflow.security_block_on --raw 2>/dev/null || echo "high") ``` Resolve active step hooks from `VERIFY_POST_HOOKS_JSON` where `kind == "step"` and `ref.skill == "secure-phase"`. @@ -95,6 +97,8 @@ Call AskUserQuestion with threat table and options: - `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. +Substitute `{SECURITY_ASVS}` with the value of `$SECURITY_ASVS` and `{SECURITY_BLOCK_ON}` with the value of `$SECURITY_BLOCK_ON` resolved in Step 0 via `config-get`. + Print: `◆ Spawning security auditor... (runs in a subagent — no output until it returns, ~1–5 min; expected, not a freeze)` ``` diff --git a/tests/secure-phase.test.cjs b/tests/secure-phase.test.cjs index 2038f092b..e4f7f84ea 100644 --- a/tests/secure-phase.test.cjs +++ b/tests/secure-phase.test.cjs @@ -451,3 +451,80 @@ describe('SECURE: threat-model-anchored behaviour', () => { ); }); }); + +// ─── 8. Regression: security config variables resolved before use (#1625) ──── +// allow-test-rule: runtime-contract-is-the-product — secure-phase.md prose is the executed contract (#1625) + +describe('SECURE: security config variables resolved before use (#1625)', () => { + const wfPath = path.join(WORKFLOWS_DIR, 'secure-phase.md'); + + test('SECURITY_ASVS is assigned (not only used as placeholder)', () => { + const content = fs.readFileSync(wfPath, 'utf-8'); + assert.ok( + content.includes('SECURITY_ASVS='), + 'SECURITY_ASVS must be assigned via config-get in the workflow, not only appear as {SECURITY_ASVS} placeholder' + ); + }); + + test('SECURITY_BLOCK_ON is assigned (not only used as placeholder)', () => { + const content = fs.readFileSync(wfPath, 'utf-8'); + assert.ok( + content.includes('SECURITY_BLOCK_ON='), + 'SECURITY_BLOCK_ON must be assigned via config-get in the workflow, not only appear as {SECURITY_BLOCK_ON} placeholder' + ); + }); + + test('SECURITY_ASVS assignment appears before the auditor injection line', () => { + const content = fs.readFileSync(wfPath, 'utf-8'); + const assignIdx = content.indexOf('SECURITY_ASVS='); + const configInjIdx = content.indexOf('block_on: {SECURITY_BLOCK_ON}'); + assert.ok(assignIdx > -1, 'SECURITY_ASVS= must exist in the file'); + assert.ok(configInjIdx > -1, 'block_on: {SECURITY_BLOCK_ON} injection line must exist'); + assert.ok( + assignIdx < configInjIdx, + 'SECURITY_ASVS must be assigned before the auditor injection line that references {SECURITY_BLOCK_ON}' + ); + }); + + test('SECURITY_BLOCK_ON assignment appears before the auditor injection line', () => { + const content = fs.readFileSync(wfPath, 'utf-8'); + const assignIdx = content.indexOf('SECURITY_BLOCK_ON='); + const configInjIdx = content.indexOf('block_on: {SECURITY_BLOCK_ON}'); + assert.ok(assignIdx > -1, 'SECURITY_BLOCK_ON= must exist in the file'); + assert.ok(configInjIdx > -1, 'block_on: {SECURITY_BLOCK_ON} injection line must exist'); + assert.ok( + assignIdx < configInjIdx, + 'SECURITY_BLOCK_ON must be assigned before the auditor injection line that references it' + ); + }); + + test('security config resolved via config-get with correct keys and defaults', () => { + const content = fs.readFileSync(wfPath, 'utf-8'); + assert.ok( + content.includes('config-get workflow.security_asvs_level'), + 'must resolve SECURITY_ASVS via config-get workflow.security_asvs_level' + ); + assert.ok( + content.includes('config-get workflow.security_block_on'), + 'must resolve SECURITY_BLOCK_ON via config-get workflow.security_block_on' + ); + assert.ok( + content.includes('echo "1"') && content.includes('echo "high"'), + 'config-get resolution must include the registry default fallbacks (1, high) so an unset/failed lookup still yields a valid value' + ); + }); + + test('security config-get uses --raw so the injected string value is unquoted', () => { + const content = fs.readFileSync(wfPath, 'utf-8'); + // Without --raw, config-get returns JSON ("high" with quotes), which would + // corrupt the auditor block to `block_on: "high"`. --raw yields bare `high`. + assert.ok( + /config-get workflow\.security_block_on --raw/.test(content), + 'SECURITY_BLOCK_ON must be resolved with --raw (config-get returns a quoted "high" without it)' + ); + assert.ok( + /config-get workflow\.security_asvs_level --raw/.test(content), + 'SECURITY_ASVS must be resolved with --raw for consistency' + ); + }); +}); diff --git a/tests/workflow-size-baseline.json b/tests/workflow-size-baseline.json index d878bca24..4cf4f368f 100644 --- a/tests/workflow-size-baseline.json +++ b/tests/workflow-size-baseline.json @@ -65,7 +65,7 @@ "resume-project.md": 17226, "review.md": 39404, "scan.md": 7688, - "secure-phase.md": 12282, + "secure-phase.md": 12656, "session-report.md": 4044, "settings-advanced.md": 39666, "settings-integrations.md": 15848,