fix(#1625): resolve security config in secure-phase.md before auditor handoff (#1633)

This commit is contained in:
Tom Boucher
2026-06-23 17:24:59 -04:00
committed by GitHub
parent 01ef08ed2a
commit 94be6d5b60
4 changed files with 87 additions and 1 deletions

View File

@@ -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)

View File

@@ -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)`
```

View File

@@ -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 <config> 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 <config> injection line that references {SECURITY_BLOCK_ON}'
);
});
test('SECURITY_BLOCK_ON assignment appears before the auditor <config> 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 <config> 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 <config> 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'
);
});
});

View File

@@ -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,