feat(philosophy): expand to 12-principle QWED constitution + CI boundary gate#202
Conversation
…y gate - Expand QWED_RULES.md from 7 to 12 principles (Deterministic Decisions, Explicit Boundaries, Approved Paths Only, Vulnerability Family Thinking, Existing Issues Must Survive New Boundaries, etc.) - Update CONTRIBUTING.md with Approved Paths table - Update PR template with expanded enforcement checklist - Update copilot-instructions.md and .coderabbit.yaml with new rules - Add scripts/check_boundary.py (AST-based CI gate for eval/exec/parse_expr) - Add boundary-check step to CI workflow, exempt safe_parser.py and safe_evaluator.py
|
Warning Review limit reached
More reviews will be available in 17 minutes and 7 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughExpands QWED enforcement rules into 12 structured Core Principles and two new Forbidden Suggestions in ChangesQWED Enforcement Hardening
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Greptile SummaryThis PR expands QWED_RULES.md from 7 to 12 principles and adds a new AST-based CI boundary gate (
Confidence Score: 3/5Not safe to merge as-is: the CI security gate has a meaningful detection gap and compiled Windows binaries inside test_venv/ should not land on main. The AST gate's FORBIDDEN_CALLS set only covers fully-qualified attribute-style calls. A contributor who writes scripts/check_boundary.py (from-import bypass gap), test_venv/ (entire directory should be removed), fix_cache.py and update_cache.py (accidental dev artifacts). Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[CI trigger] --> B[checkout]
B --> C[Setup Python 3.11]
C --> D[Boundary Check check_boundary.py]
D --> E{rglob src py files}
E --> F[AST parse each file]
F --> G{Call node type?}
G -->|ast.Name| H[bare name]
G -->|ast.Attribute| I[dotted name]
G -->|chained call| J[empty - skipped]
H --> K{in FORBIDDEN_CALLS or eval/exec leaf?}
I --> K
K -->|yes and not in wrapper| M[FAIL - exit 1]
K -->|no| N[pass]
J --> N
N --> O[from subprocess import run produces bare run not in FORBIDDEN_CALLS]
O --> N
M --> P[CI blocked]
N --> Q[Continue - install deps and tests]
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
.github/copilot-instructions.md (1)
10-12: 💤 Low valueConsider rewording to reduce repetitive "Do not" sentence openings.
Lines 10–12 begin three consecutive bullets with "Do not," which may reduce readability. You could vary the phrasing (e.g., "Reject suggestions that…" or restructure one as a positive directive) while preserving the enforcement semantics.
♻️ Example rewording (optional)
- Do not suggest fallback execution paths. - Do not suggest graceful degradation that continues past failed verification. - Do not suggest retries that weaken enforcement. - Do not trust model output as proof of correctness. - - Do not use scoring, confidence thresholds, or heuristics for decisions. - - Do not suggest `eval` / `exec` / `parse_expr` outside approved wrappers. + - Reject scoring, confidence thresholds, or heuristics for enforcement decisions. + - Do not suggest `eval` / `exec` / `parse_expr` outside approved wrappers. - Prefer fail-closed behavior over convenience or availability.Or combine them differently—the key is varying the sentence structure to improve flow.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/copilot-instructions.md around lines 10 - 12, The three consecutive bullets (lines 10-12) in the copilot-instructions.md file all begin with "Do not," creating repetitive sentence structure that reduces readability. Vary the phrasing of these three bullets while maintaining their enforcement meaning—for example, restructure one as a positive directive like "Reject suggestions that...", use alternative negations like "Never...", or restructure them to improve overall flow and sentence variety. The semantic content about distrusting model output, avoiding scoring/heuristics, and rejecting eval/exec/parse_expr usage should remain intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/check_boundary.py`:
- Around line 48-49: The wrapper allowlist keying mechanism uses filepath.name
(basename only) which implicitly trusts any file with that name regardless of
its location, weakening boundary enforcement. Replace the basename-only approach
with the full or relative file path to ensure trust is explicitly tied to
specific approved paths rather than just filenames. This change is needed at the
location shown (around line 48-49) where filename is assigned from
filepath.name, and also at lines 63-67 where this filename variable is
subsequently used in the allowlist lookup logic.
- Around line 45-46: The except SyntaxError block at lines 45-46 currently
silently returns errors when a file has syntax issues, allowing malformed files
to be skipped instead of blocking CI execution. To implement fail-closed
behavior as required by coding guidelines, replace the silent return with logic
that raises or propagates the SyntaxError exception (or logs it as a fatal error
that causes the script to exit with a non-zero status), ensuring that parse
failures block CI rather than being ignored.
- Around line 56-67: The dangerous-call matching in the conditional blocks for
"parse_expr", "eval", and "exec" is incomplete and bypassable. Update the checks
to handle both bare function names and qualified forms (e.g., builtins.eval,
module.parse_expr) by extracting the function name from qualified calls.
Additionally, add comprehensive checks for the full vulnerability family
including os.system and subprocess.Popen, which are documented as CI-gated but
currently not enforced. Create a deterministic matching mechanism that covers
all declared forbidden families rather than just partial symbol variants,
ensuring that no dangerous call can bypass the checks through qualification or
aliasing.
---
Nitpick comments:
In @.github/copilot-instructions.md:
- Around line 10-12: The three consecutive bullets (lines 10-12) in the
copilot-instructions.md file all begin with "Do not," creating repetitive
sentence structure that reduces readability. Vary the phrasing of these three
bullets while maintaining their enforcement meaning—for example, restructure one
as a positive directive like "Reject suggestions that...", use alternative
negations like "Never...", or restructure them to improve overall flow and
sentence variety. The semantic content about distrusting model output, avoiding
scoring/heuristics, and rejecting eval/exec/parse_expr usage should remain
intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ecc5fd3a-4050-4946-a79e-00b7ee36c817
📒 Files selected for processing (7)
.coderabbit.yaml.github/copilot-instructions.md.github/pull_request_template.md.github/workflows/ci.ymlCONTRIBUTING.mdQWED_RULES.mdscripts/check_boundary.py
- Use full relative path for wrapper exemption, not basename-only - Normalize paths with as_posix() for cross-platform consistency - Catch eval/exec as bare calls and builtins.eval, not session.exec() - Add os.system, subprocess.*, popen to FORBIDDEN_CALLS - Exempt pre-existing subprocess.run sites as known tech debt - Fail closed on SyntaxError instead of silent skip - Move boundary check after setup-python in CI
|
1 participant
Description
Expands QWED_RULES.md from 7 to 12 principles, updates all governance files (CONTRIBUTING.md, PR template, copilot-instructions, coderabbit), and adds an AST-based CI boundary check gate.
Changes:
QWED_RULES.mdCONTRIBUTING.md.github/pull_request_template.md.github/copilot-instructions.md.coderabbit.yaml.github/workflows/ci.ymlBoundary Check (QWED Rules)stepscripts/check_boundary.pySummary by CodeRabbit
Documentation
Chores