Skip to content

ci(audit): spec-drift validator walks all workflow files + GOV-009 declares Snyk rules#114

Merged
rolandpg merged 1 commit into
masterfrom
fix/audit-spec-drift-all-workflows
Apr 25, 2026
Merged

ci(audit): spec-drift validator walks all workflow files + GOV-009 declares Snyk rules#114
rolandpg merged 1 commit into
masterfrom
fix/audit-spec-drift-all-workflows

Conversation

@rolandpg
Copy link
Copy Markdown
Owner

Summary

Closes the validator-broadening follow-up flagged in PR #102 and `tasks/compliance-audit-2026-04-25.md` under H-5.

The spec-drift validator only inspected `.github/workflows/ci.yml`. Snyk's SCA + SAST gates live in `.github/workflows/snyk-security.yml` (correctly — Snyk gets its own workflow with its own permission scope and SARIF upload), so the corresponding GOV-009 rules had to be left as comments to avoid breaking `test_ci_step_references_exist_in_workflow`.

Changes

  • `tests/test_governance_spec_drift.py` — `_extract_gov_labels_from_ci()` and the `ci_step` membership check now read every `.yml`/`.yaml` under `.github/workflows/`.
  • `governance/controls.yaml` GOV-009 — adds two declared rules:
    • `snyk_sca` → step name `"Snyk Open Source test (SCA)"`
    • `snyk_sast` → step name `"Snyk Code test (SAST)"`

Test plan

  • All 7 `test_governance_spec_drift` tests pass
  • CI green (the broadened validator runs as part of CI)

🤖 Generated with Claude Code

Until now, `tests/test_governance_spec_drift.py` only inspected
`.github/workflows/ci.yml`. That meant any control whose CI step lived
in another workflow (e.g., `snyk-security.yml`) had to either duplicate
the step into ci.yml or be left undeclared in `governance/controls.yaml`
to avoid tripping `test_ci_step_references_exist_in_workflow`.

The H-5 audit fix (PR #102) ran into exactly this. Snyk's SCA + SAST
gates were activated in `snyk-security.yml` but the corresponding
GOV-009 rules were left as comments in controls.yaml because declaring
them would fail the validator. Honest accounting required either
moving Snyk into ci.yml (architecturally wrong) or extending the
validator (this PR).

Changes:
- `_extract_gov_labels_from_ci()` and the `ci_step` membership check
  now read every `*.yml`/`*.yaml` under `.github/workflows/`.
- GOV-009 in controls.yaml gains two new rules: `snyk_sca` (SCA test)
  and `snyk_sast` (Code test), each with their actual workflow
  step names. The "tracked as a follow-up" comment is removed.

`CI_WORKFLOW` is retained as an alias for ci.yml in case external
callers reference it.

Closes the loose end called out in `tasks/compliance-audit-2026-04-25.md`
under H-5 (the validator-broadening follow-up).
Copilot AI review requested due to automatic review settings April 25, 2026 05:17
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR broadens the governance spec-drift validator so it validates ci_step references (and GOV label presence) across all GitHub Actions workflow files, enabling GOV-009 to formally declare Snyk SCA/SAST rules that live in a separate workflow.

Changes:

  • Update spec-drift tests to concatenate and scan every *.yml/*.yaml under .github/workflows/ (instead of only ci.yml).
  • Add GOV-009 manifest rules for Snyk Open Source (SCA) and Snyk Code (SAST) step names.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
tests/test_governance_spec_drift.py Aggregate all workflow YAML text for GOV label extraction and ci_step existence checks.
governance/controls.yaml Declare GOV-009 rules for Snyk SCA/SAST steps in addition to pip-audit.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 46 to +48
def _extract_gov_labels_from_ci():
"""Extract all GOV-XXX labels from CI workflow step names."""
assert CI_WORKFLOW.exists(), f"CI workflow not found: {CI_WORKFLOW}"
ci_text = CI_WORKFLOW.read_text()
# Match step names like "GOV-003 — ..." or "GOV-012 — ..."
return set(re.findall(r"GOV-\d{3}", ci_text))
"""Extract all GOV-XXX labels from any workflow step name."""
return set(re.findall(r"GOV-\d{3}", _read_all_workflow_text()))
Copy link

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

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

_extract_gov_labels_from_ci() now scans all workflow files, so the function name (and related wording like "...from_ci") is misleading. Consider renaming it (and any call sites) to reflect that it extracts GOV labels from all workflows; also update the phantom-controls failure message later in this file which still tells people to remove labels from ci.yml, even though the label could be in another workflow now.

Copilot uses AI. Check for mistakes.
Comment thread governance/controls.yaml
# the validator to walk all workflows is tracked as a separate
# follow-up.
- id: snyk_sca
description: "SCA (Software Composition Analysis) scan; HIGH/CRITICAL fails the gate"
Copy link

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

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

This description reads like the Snyk SCA job always enforces the HIGH/CRITICAL gate, but the workflow step is conditional on SNYK_TOKEN being configured (it’s skipped otherwise). To keep the governance manifest accurate, consider mentioning that it gates only when the token is present (with pip-audit providing the always-on path).

Suggested change
description: "SCA (Software Composition Analysis) scan; HIGH/CRITICAL fails the gate"
description: "SCA (Software Composition Analysis) scan; when SNYK_TOKEN is configured, HIGH/CRITICAL fails the gate. pip-audit provides the always-on dependency audit path."

Copilot uses AI. Check for mistakes.
Comment thread governance/controls.yaml
Comment on lines +51 to +52
description: "SAST (Static Application Security Testing); HIGH/CRITICAL fails the gate"
ci_step: "Snyk Code test (SAST)"
Copy link

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

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

Same as above for SAST: the workflow runs snyk code test only when SNYK_TOKEN is configured, so the manifest description should clarify the conditional enforcement to avoid overstating the guarantee of this gate.

Suggested change
description: "SAST (Static Application Security Testing); HIGH/CRITICAL fails the gate"
ci_step: "Snyk Code test (SAST)"
description: "SAST (Static Application Security Testing) scan when SNYK_TOKEN is configured; HIGH/CRITICAL fails the gate when this scan runs"
ci_step: "Snyk Code test (SAST, when SNYK_TOKEN is configured)"

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown

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

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: 6edc32b2b8

ℹ️ 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".

"""Every ci_step referenced in the manifest must exist in some workflow."""
manifest = _load_manifest()
ci_text = CI_WORKFLOW.read_text()
all_workflows = _read_all_workflow_text()
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Limit ci_step checks to CI-triggered workflows

test_ci_step_references_exist_in_workflow now accepts a ci_step as valid if it appears in any workflow file, which weakens the guardrail from “enforced in CI” to “text exists somewhere under .github/workflows.” Because this repo already has non-CI workflows (for example, publish.yml runs only on release), a control can now pass drift validation even if its step is no longer executed on PR/push CI paths, silently masking lost enforcement.

Useful? React with 👍 / 👎.

Comment thread governance/controls.yaml
Comment on lines +47 to +49
description: "SCA (Software Composition Analysis) scan; HIGH/CRITICAL fails the gate"
ci_step: "Snyk Open Source test (SCA)"
tool: "snyk test --severity-threshold=high"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Declare Snyk controls as conditional or enforce token presence

The new GOV-009 Snyk rules are declared as CI-enforced gates, but the referenced steps in .github/workflows/snyk-security.yml run only when steps.check_token.outputs.has_token == 'true'; on contexts without secrets (notably fork PRs), both checks are skipped. That makes the manifest overstate enforcement for these controls and can produce false compliance signals unless the workflow fails when the token is missing or the rules are explicitly marked conditional.

Useful? React with 👍 / 👎.

@rolandpg rolandpg merged commit e89c3d5 into master Apr 25, 2026
16 checks passed
@rolandpg rolandpg deleted the fix/audit-spec-drift-all-workflows branch April 25, 2026 05:23
rolandpg added a commit that referenced this pull request Apr 25, 2026
… work

The [Unreleased] section was empty since v2.4.3 cut. Captures
everything merged since: RFC-011 local LLM backend (#104), RFC-012
LiteLLM (#108), the H-1 Ruff ratchet (#106 #107 #109 #111 #113),
the L-4 CI shell-precedence fix (#112), the spec-drift validator
broadening + GOV-009 Snyk declarations (#114), and a CONTRIBUTING.md
accuracy pass (#115).

Adds a compliance-audit closure table mirroring the running scoreboard
in TODO.md, scoped to what shipped — outstanding items (H-3 mypy,
H-4 GOV-006, M-2 RFC template, M-4 lock file) listed below the table
as remaining work for v2.5.x.

Targets v2.5.0 release.
rolandpg added a commit that referenced this pull request Apr 25, 2026
… work (#116)

The [Unreleased] section was empty since v2.4.3 cut. Captures
everything merged since: RFC-011 local LLM backend (#104), RFC-012
LiteLLM (#108), the H-1 Ruff ratchet (#106 #107 #109 #111 #113),
the L-4 CI shell-precedence fix (#112), the spec-drift validator
broadening + GOV-009 Snyk declarations (#114), and a CONTRIBUTING.md
accuracy pass (#115).

Adds a compliance-audit closure table mirroring the running scoreboard
in TODO.md, scoped to what shipped — outstanding items (H-3 mypy,
H-4 GOV-006, M-2 RFC template, M-4 lock file) listed below the table
as remaining work for v2.5.x.

Targets v2.5.0 release.
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.

2 participants