Skip to content

fix(policy): fail closed when set_check extraction breaks - #379

Merged
brenpike merged 4 commits into
mainfrom
bugfix/373-set-check-extractor-fail-closed
Sep 25, 2026
Merged

brenpike merged 4 commits into
mainfrom
bugfix/373-set-check-extractor-fail-closed

Conversation

@brenpike

Copy link
Copy Markdown
Owner

Summary

tools/policy_check.sh test_set_check could not tell a broken regex extraction apart from a genuine zero-match result: both collapsed to an empty capture {}. A zero-count subset fixture (notably tests/policy/safety-destructive-fix-gate-no-second-copy.json) could therefore pass vacuously if its extractor ever broke. This PR makes the check fail closed.

Closes #373.

What changed

  • Regex passed as data. The fixture regex now reaches perl through the RE environment variable and is compiled with qr//. It is never spliced into program source, so an unescaped / can no longer break the program.
  • Same engine validates and executes. The compile check now uses perl qr// instead of grep -P, a different engine.
  • Exit status checked separately. The extractor's exit status and the jq aggregation's exit status are each checked, and each has its own finding. A match with no group-1 capture now fails. {} can only come from a successful extraction that found nothing.
  • perl is a hard dependency. The grep -oP / sed -E fallback is deleted. It could not parse the PCRE (?:...) syntax the fixtures use, and it returned zero captures silently. If perl is missing, the check now fails loudly and names perl. perl is an essential package on the ubuntu-latest CI runner, and the script already depends on grep -P elsewhere with no fallback.
  • Canary coverage. The in-script set_check SAFETY-CANARY adds three assertions: a non-compiling regex fails, a regex that matches but has no capture group fails, and a regex containing an unescaped / passes and captures a/b.
  • Stale citation replaced. tests/policy/README.md line 10 cited tools/policy_check.sh:1473, which was already stale. It now points at the SAFETY_FIXTURES discovery block by name. This is backlog item 6 of the prompt-audit follow-ups, folded in because this PR already edits the README.

Files by plan step

  • STEP-001: tests/policy/fixtures/set-check-zero-match-canary.md (slash marker line, prose describing the five canary branches)
  • STEP-002: tools/policy_check.sh (test_set_check rewrite, canary assertions 3-5)
  • STEP-003: tests/policy/README.md (new section 8 "Runtime dependencies", line 10 pointer)

Validation

  • bash tools/validate.sh --changed escalated to the full suite plus --self-test because tools/** changed. Exit 0, all suites pass.
  • policy_check.sh --strict: Checks passed 72/73, 0 new findings (unchanged baseline).
  • An old-vs-new harness confirmed that the no-capture and non-compiling cases previously passed vacuously and now fail. The legitimate zero-match case passes in both.
  • A mutation test (reverting to || extract_out="") makes the canary fail, so the canary catches the regression it guards.
  • All 9 existing set_check fixtures keep identical verdicts.
  • Local Codex adversarial review: clean on the first iteration, zero findings.

Versioning

No bump. Only tools/ and tests/ changed; no plugin/ file is touched.

Unresolved

None. The script-wide discovery gaps in other find sites remain tracked in #377.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a2f8628e4c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread tools/policy_check.sh
…ch count

The perl compile oracle only proved a regex compiles; a capture-free regex
that also matches nothing in the extraction loop never runs the
`defined($1) or die` guard, so it vacuously passed as a clean zero-match
result. Add an independent capture-group count check (via a synthetic
always-matching alternation against @+) plus a canary covering exactly
that combination.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 84ea548aec

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread tools/policy_check.sh Outdated
@brenpike
brenpike merged commit 7a873ea into main Sep 25, 2026
1 check passed
@brenpike
brenpike deleted the bugfix/373-set-check-extractor-fail-closed branch September 25, 2026 14:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

policy_check set_check: extractor failures read as zero matches (fail-open)

1 participant