feat(#114): CI grounding lint — plan paths + PR-body refs#121
Merged
Conversation
Four phases: Phase 0 — plan-grounding lint (Check A, blocking); Phase 1 — PR-body refs lint (Check B, advisory); Phase 2 — CI integration; Phase 3 — DEV_CYCLE.md docs + CHANGELOG. Six open questions surfaced for audit: - Q1: CI-only for v1; pre-commit hook deferred (no .pre-commit-config infrastructure in repo yet). - Q2: dynamic discovery of registered packages via ls + __init__.py presence (no hardcoded list). - Q3: Check B advisory (warn-only, never blocks merge). - Q4: standardised keyword set: Closes/Fixes/Resolves + Refs + Related to + See. - Q5: scripts/ for dev-utility (Check A); .github/scripts/ for CI-only (Check B). Mirrors existing precedent. - Q6: Check A is fast pre-audit catch; doesn't replace audit's deeper grounding pass. Risk grade L1 (pure checker scripts + advisory CI workflow; no production code paths, no schema, no contracts). Branched off BicameralAI/dev tip 2e9a842 post-#117. SG-PLAN-GROUNDING-DRIFT mitigated: ran `ls -d */` and confirmed every package + workflow path before submission. The plan that builds the lint that prevents this very pattern. Self-test exit criterion #5: when the lint lands, this very plan file must lint clean (zero diagnostics on plan-114-grounding-lint.md).
…rom-env Audit found: - F-1 (BLOCKING, OWASP A03): pr-body-refs-lint.yml used `echo "$PR_BODY" > /tmp/pr-body.md` which lets Bash double-quote interpolation expand $(cmd) substitutions in user-controlled PR body text — arbitrary code execution in CI. - F-2: 6th test needed for the env-var read path. Remediation: - Phase 1 main() signature gains `--from-env <NAME>` mutually exclusive with `--body <file>`. Direct os.environ read; no shell interpreter in the path. - Phase 2 workflow drops the echo line: `run: python ...py --from-env PR_BODY`. - Phase 1 test count 5 → 6 (added test_main_reads_from_env_var verifying the security-critical invocation matches file-mode output). - Razor estimates bumped: lint_pr_body_refs.py 100 → 110 LOC, test_lint_pr_body_refs.py 90 → 100 LOC. - Inline security note in the workflow YAML section explaining why we don't use the echo pattern. The plan that builds the lint that prevents one class of carelessness (filesystem-grounding drift) had a different class of carelessness (shell injection) — the audit caught both classes. Re-audit pending.
GATE TRIBUNAL entry covering both audit iterations: - v1 (a5e6a05): VETO on F-1 OWASP A03 — `echo "$PR_BODY"` in workflow shell exposes command-substitution injection. - v2 (4ea06be): PASS — workflow command now passes PR_BODY via direct os.environ read in Python, eliminating the shell intermediate. Chain hash 850ec57f extends from #18 (#48 SEAL eacc6f89) on dev. Notable: the plan that builds the lint for one class of carelessness (filesystem-grounding drift) had a different class of carelessness (OWASP A03). Audit caught both; QOR defense-in-depth working as designed. Plan PASS at 4ea06be; chain to /qor-implement.
Phase 0 — plan-grounding lint (Check A, blocking): - scripts/lint_plan_grounding.py (212 LOC, 8 tests): walks plan-*.md for backtick-wrapped path tokens, verifies each resolves on the working tree. Markers (**new**, (planned)/(future)/(v2)/ (nonexistent)/(example)) exempt the line. Skips HTML comments, blockquotes, multi-word tokens, glob patterns. - .github/workflows/lint-and-typecheck.yml: new step lints only plan-*.md files modified in the current PR (git diff against base ref). Blocks merge if any modified plan has unresolved paths. Phase 1 — PR-body refs lint (Check B, advisory): - .github/scripts/lint_pr_body_refs.py (162 LOC, 6 tests): warns on bare #NUMBER mentions outside Closes/Refs/etc keywords or Linked-issues section. SECURITY: --from-env PR_BODY reads via os.environ directly (no shell interpolation; OWASP A03 mitigation per #114 audit v1). - .github/workflows/pr-body-refs-lint.yml: advisory workflow on pull_request: [opened, edited, reopened]. continue-on-error: true. Phase 2 — CI integration (above). Phase 3 — docs: - docs/DEV_CYCLE.md §2.1: plan-grounding lint callout - docs/DEV_CYCLE.md §4.3: PR-body keyword discipline (Closes/Refs/ Linked-issues section) - CHANGELOG.md [Unreleased] entry Validation: - 29/29 tests green (8 + 6 + 15 regression on #117/#113) - Self-test: scripts/lint_plan_grounding.py against plan-114-grounding-lint.md exits 0 (clean) — the plan that builds the lint passes the lint - ruff check + format: clean on all 4 new files - mypy: clean on both new modules - Razor: scripts/lint_plan_grounding.py 212 LOC; .github/scripts/ lint_pr_body_refs.py 162 LOC; entry funcs ≤30 LOC; helpers ≤25. Plan: plan-114-grounding-lint.md (audit v1 VETO at a5e6a05 on OWASP A03; v2 PASS at 4ea06be after --from-env remediation; chain bf890347 → 850ec57f). Implementation chains to seal in /qor-substantiate. Closes #114
Reality matches Promise. All 7 planned files exist; 14/14 new tests green; ruff/format/mypy clean; self-test passes (the plan that builds the lint cleanly passes the lint). Plan: plan-114-grounding-lint.md (v2 PASS @ 4ea06be) Audit: META_LEDGER #19 (chain hash 850ec57f) Merkle seal: a19a04debe5f8f38aab182263e94819d50743849a26cdb8cc4aa3279a81be265 Closes the SG-PLAN-GROUNDING-DRIFT loop after 5 instances tracked. Defense-in-depth: author-time `ls -d */` mitigation + CI lint durable countermeasure now both in place. Capability shortfalls: gate artifacts, reliability sweep, version bump all skipped (qor/ runtime helpers absent on this branch). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Knapp-Kevin
added
enhancement
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Bicameral drift report — skippedNo To enable: add a |
This was referenced Apr 30, 2026
Resolves long-standing conflicts between PR #121 (CI grounding lint for plan files + PR-body refs, closes #114) and dev's progression since the PR was opened on 2026-04-29: - CHANGELOG.md: kept #114 grounding-lint entry alongside dev's #173 (preflight graph expansion) and #175 (Step 5.6 disambiguation) Unreleased entries; collapsed duplicate "### Changed" header. - docs/META_LEDGER.md: took dev side wholesale. PR #121's branch inserted Entry #19 (audit PASS) and #20 (substantiation seal) for #114, but dev's chain has progressed to 41 entries since (most recent: v0-release-blockers SEAL at 7cc405fc). Re-anchoring the PR's mid-chain entries would require recomputing every subsequent chain hash — not safely automatable. The audit/seal history for #114 is preserved in git history via commits 11685e5 and 218254e. - docs/SYSTEM_STATE.md: same — took dev side wholesale. - plan-114-grounding-lint.md: line 61 referenced `.github/scripts/post_drift_comment.py` as a precedent example; dev deleted that file. Marked as `(nonexistent)` exemption per the lint's own exemption vocabulary. Self-test: `python scripts/lint_plan_grounding.py plan-114-grounding-lint.md` exits 0 (the plan that builds the lint passes the lint). 14/14 lint unit tests pass; ruff/mypy clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
scripts/lint_plan_grounding.py(212 LOC, blocking) — fails CI if aplan-*.mdfile references a filesystem path that doesn't exist on the working tree, unless the path is marked**new**/(planned)/(future)/(v2)/(nonexistent)/(example). Exemption markers cover the legitimate cases where a plan deliberately proposes a not-yet-existent path..github/scripts/lint_pr_body_refs.py(162 LOC, advisory) — warns on bare#NUMBERmentions in PR bodies that aren't wrapped withCloses/Fixes/Refskeywords or placed under a## Linked issuesheading. Hardened against OWASP A03 via directos.environread (no shell interpolation of user-controlled PR body).ls -d */discipline already in use.Linked issues
Closes #114
Refs #117 (branch is off
BicameralAI/devpost-#117 pre-push-hook merge — this PR continues the same drift-defense workstream)Plan / Audit / Seal
plan-114-grounding-lint.md(v2, content hashsha256:1447b2ad…), committed at4ea06be.850ec57f…— verdict PASS (post-remediation).a5e6a05was VETO on F-1 (OWASP A03:echo "$PR_BODY" > /tmp/pr-body.mdexposed Bash command-substitution injection in CI).4ea06beremediated by switching topython ...py --from-env PR_BODY(directos.environread, no shell interpreter in the path).a19a04de…— Reality matches Promise.Risk grade: L1 (pure checker scripts + advisory CI workflow; zero production code paths, zero schema migrations, zero MCP tool changes, zero contract changes).
Test plan
pytest -q tests/test_lint_plan_grounding.py tests/test_lint_pr_body_refs.py— 14/14 new testsruff check scripts/lint_plan_grounding.py .github/scripts/lint_pr_body_refs.py tests/test_lint_*.py— cleanruff format --check scripts/lint_plan_grounding.py .github/scripts/lint_pr_body_refs.py tests/test_lint_*.py— cleanmypy scripts/lint_plan_grounding.py .github/scripts/lint_pr_body_refs.py— cleanpython scripts/lint_plan_grounding.py plan-114-grounding-lint.md→ exit 0 (the plan that builds the lint passes the lint).lint-and-typecheck.ymlruns the new Plan-grounding step against this PR's modified plan file.pr-body-refs-lint.ymlruns Check B against this very PR body and emits zero warnings (this body usesCloses #114+Refs #117+ a## Linked issuessection — self-validating).Phase breakdown
--body <file>(local/tests) +--from-env <NAME>(CI security path); always returns 0 (advisory).git diff --name-only) — historical plans don't block unrelated PRs. PR-body workflow usespermissions: pull-requests: read+pull_request(fork-safe) +continue-on-error: true.DEV_CYCLE.md§2.1 (Check A callout) + §4.3 (Check B PR-body keyword discipline);CHANGELOG.md[Unreleased]entry.Security note
The
--from-envinvocation pattern is load-bearing. An earlier draft of this plan usedecho "$PR_BODY" > /tmp/pr-body.mdin the workflow, which is vulnerable to OWASP A03 command-substitution injection — Bash double-quotes expand$(cmd), and PR body is contributor-editable. The audit caught this at v1 (VETO). The shipped pattern readsPR_BODYdirectly viaos.environin Python, with no shell interpreter in the path. The historical mistake is documented inline in the workflow YAML and module docstring so the lesson stays load-bearing.Followups (not in this PR)
bicameral-mcp link_commit HEADis not a registered subcommand; the post-commit hook silently no-ops. Worth its own issue.CLAUDE.mdreferencespilot/mcp/skills/which doesn't exist in this repo. Stale doc.