feat(security): harness pattern extraction — prompt injection defense - #81
Merged
Conversation
…tion defense Add three architectural patterns from the vulnerability-discovery harness as zero-dependency, pure Python modules: - UntrustedData: runtime-nonce wrapping for external data entering LLM prompts (ReadFile and FetchURL tool outputs) - Boundary artifacts: CodingArtifact, VerificationResult, VulnerabilityArtifact, AuditVerdict dataclasses enforcing coder↔verifier information barrier - Planner subagent: read-only recon decomposition for parallel worker seeding Modified tools wrap all external content paths: - ReadFile: directory listings, _read_forward, _read_tail - FetchURL: markdown, trafilatura extraction, service path Updated coder.yaml and verifier.yaml with artifact contract/receipt instructions. Registered planner subagent in agent.yaml. 86 tests (8 trust + 12 artifacts + 11 wrapping integration + 8 helper + 47 updated existing) all pass. make check-pythinker-code clean.
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds an UntrustedData envelope for external tool outputs, frozen artifact dataclasses for coder→verifier handoff, a read-only planner subagent and prompt contracts, applies wrapping to ReadFile/FetchURL, and migrates/extends tests and snapshots to validate the new wire formats. ChangesPrompt Safety & Data Contracts
Agent Configuration & Tool Integration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
tests/utils/test_artifacts.py (1)
1-167:⚠️ Potential issue | 🟠 Major | ⚡ Quick winTest coverage is comprehensive, but depends on fixing
artifacts.py.The test suite correctly validates round-trips, defaults, immutability, field names, and formatting. However,
test_coding_artifact_default_edge_cases()(lines 34-40) andtest_verification_result_defaults()(lines 84-90) will fail until thedefault_factory=list[str]syntax errors insrc/pythinker_code/utils/artifacts.pyare corrected todefault_factory=list.Once
artifacts.pyis fixed, these tests provide solid schema validation.🤖 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 `@tests/utils/test_artifacts.py` around lines 1 - 167, The issue is invalid use of PEP 695-style generics in dataclass default_factory; in artifacts.py replace default_factory=list[str] with default_factory=list for the fields that should default to empty lists (specifically the CodingArtifact.edge_cases_claimed field and the VerificationResult.discovered_gaps field) so the dataclass definitions (CodingArtifact, VerificationResult) use default_factory=list and keep their type annotations as list[str] (or List[str]) for correct typing while ensuring the defaults are valid at runtime.src/pythinker_code/agents/default/planner.yaml (1)
24-44:⚠️ Potential issue | 🟠 MajorAdd focused tests for
planner.yaml(planner subagent) spec invariants.
tests/core/test_agent_spec.pysnapshots default subagents but doesn’t assert theplannerentry fromsrc/pythinker_code/agents/default/agent.yaml/src/pythinker_code/agents/default/planner.yaml. Add focused assertions forplanner’swhen_to_use,allowed_tools/exclude_tools, and theROLE_ADDITIONALcontract that it must emit a<recon_seeds>JSON block (no trailing extra content).🤖 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 `@src/pythinker_code/agents/default/planner.yaml` around lines 24 - 44, Add focused unit assertions in tests/core/test_agent_spec.py that validate the planner subagent spec: load the planner entry from the agent specs (the planner.yaml content) and assert its when_to_use string equals the expected paragraph, assert allowed_tools contains the exact list ["pythinker_code.tools.shell:Shell","pythinker_code.tools.file:ReadFile","pythinker_code.tools.file:Glob","pythinker_code.tools.file:Grep","pythinker_code.tools.file:SmartSearch"] and exclude_tools contains the listed exclusions, and add a contract test for ROLE_ADDITIONAL that validates outputs must start with a JSON object keyed by "recon_seeds" with no trailing non-JSON content (i.e., emit only the <recon_seeds> JSON block). Use the planner subagent spec keys when_to_use, allowed_tools, exclude_tools and the ROLE_ADDITIONAL contract name to locate the checks.Source: Coding guidelines
🤖 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 `@src/pythinker_code/agents/default/coder.yaml`:
- Around line 35-47: Update the artifact contract text in coder.yaml to
explicitly mark optional fields: state that "edge_cases_claimed" is optional
(and any other fields with defaults) so models don't feel required to emit them;
reference the CodingArtifact dataclass in artifacts.py and ensure the prompt
language clarifies which JSON keys are mandatory vs optional (e.g., list
required keys like "files_changed", "test_command", "expected_behavior" and note
"edge_cases_claimed" is optional/default-empty). Make the change in the block
containing the <coding_artifact> example and keep the final-output format and
prohibition on logs/reasoning unchanged.
In `@src/pythinker_code/utils/artifacts.py`:
- Line 58: The dataclass field discovered_gaps uses an invalid default_factory
(default_factory=list[str]) which will raise TypeError because list[str] is not
callable; change the field to use default_factory=list (e.g., discovered_gaps:
list[str] = field(default_factory=list)) so the factory is a callable and the
type hint remains list[str]; apply the same fix used for the other field
referenced on line 28.
In `@tests/tools/test_untrusted_wrapping.py`:
- Around line 203-231: In
test_fetchurl_injection_payload_in_html_does_not_escape_wrapper, replace the
conditional that skips assertions when extraction fails with explicit assertions
that extraction succeeded: assert not result.is_error and
isinstance(result.output, str) immediately after obtaining result (before
computing opening_count/closing_count), so failures in FetchURL extraction are
flagged; then proceed to compute opening_count and closing_count from
result.output and assert the wrapper integrity as before.
In `@tests/utils/test_trust.py`:
- Around line 32-35: Rename the unused local variable in function _body from
prefix to _ to follow the discard convention and silence the linter;
specifically change the unpacking line "prefix, _, rest =
rendered.partition(...)" to use "_" for the first element so it becomes "_, _,
rest = rendered.partition(...)" and leave the rest of _body unchanged.
---
Outside diff comments:
In `@src/pythinker_code/agents/default/planner.yaml`:
- Around line 24-44: Add focused unit assertions in
tests/core/test_agent_spec.py that validate the planner subagent spec: load the
planner entry from the agent specs (the planner.yaml content) and assert its
when_to_use string equals the expected paragraph, assert allowed_tools contains
the exact list
["pythinker_code.tools.shell:Shell","pythinker_code.tools.file:ReadFile","pythinker_code.tools.file:Glob","pythinker_code.tools.file:Grep","pythinker_code.tools.file:SmartSearch"]
and exclude_tools contains the listed exclusions, and add a contract test for
ROLE_ADDITIONAL that validates outputs must start with a JSON object keyed by
"recon_seeds" with no trailing non-JSON content (i.e., emit only the
<recon_seeds> JSON block). Use the planner subagent spec keys when_to_use,
allowed_tools, exclude_tools and the ROLE_ADDITIONAL contract name to locate the
checks.
In `@tests/utils/test_artifacts.py`:
- Around line 1-167: The issue is invalid use of PEP 695-style generics in
dataclass default_factory; in artifacts.py replace default_factory=list[str]
with default_factory=list for the fields that should default to empty lists
(specifically the CodingArtifact.edge_cases_claimed field and the
VerificationResult.discovered_gaps field) so the dataclass definitions
(CodingArtifact, VerificationResult) use default_factory=list and keep their
type annotations as list[str] (or List[str]) for correct typing while ensuring
the defaults are valid at runtime.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: 50cd1b5b-7ba3-4d62-b531-3a46fa742306
📒 Files selected for processing (15)
src/pythinker_code/agents/default/agent.yamlsrc/pythinker_code/agents/default/coder.yamlsrc/pythinker_code/agents/default/planner.yamlsrc/pythinker_code/agents/default/verifier.yamlsrc/pythinker_code/tools/file/read.pysrc/pythinker_code/tools/web/fetch.pysrc/pythinker_code/utils/artifacts.pysrc/pythinker_code/utils/trust.pytests/tools/_untrusted.pytests/tools/test_fetch_url.pytests/tools/test_read_file.pytests/tools/test_untrusted_helper.pytests/tools/test_untrusted_wrapping.pytests/utils/test_artifacts.pytests/utils/test_trust.py
- Add planner to test snapshots in test_agent_spec.py, test_default_agent.py, and test_pyinstaller_utils.py - Update coder ROLE_ADDITIONAL snapshot to include artifact contract block - Fix _bypass_ssrf_validation fixture scope (non-autouse, explicit parameter) - Make FetchURL injection test unconditional (assert not is_error) - Remove redundant _prefix variable in test_trust._body() - Mark edge_cases_claimed as optional in coder.yaml artifact contract - Clarify planner.yaml final response contract (seeds-only, no preamble) - Add Unreleased CHANGELOG entry for the feature and sync docs copy
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Add explicit assertions for planner's when_to_use, allowed_tools, exclude_tools, and ROLE_ADDITIONAL recon_seeds contract invariants in test_load_default_agent_spec.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@tests/core/test_agent_spec.py`:
- Around line 441-473: The planner subagent test is missing the same property
assertions as the other subagents; add assertions checking
subagent_specs["planner"].name, subagent_specs["planner"].system_prompt_path
(should equal DEFAULT_AGENT_FILE.parent / "system.md"),
subagent_specs["planner"].model (snapshot(None)), and
subagent_specs["planner"].tools (a snapshot of the final tool list after
applying allowed_tools and exclude_tools) so the planner validates name,
system_prompt_path, system_prompt_args, when_to_use, model, allowed_tools,
exclude_tools, tools, and subagents consistently with the other subagent blocks;
place these checks alongside the existing planner assertions and compute the
expected tools snapshot by taking the combined tool set that results from
allowed_tools minus exclude_tools (and including any default/global tools used
elsewhere in the test suite).
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: c98ba43a-732e-4547-a970-efae07444885
📒 Files selected for processing (1)
tests/core/test_agent_spec.py
…agent blocks Add name, system_prompt_path, system_prompt_args, model, and tools snapshot assertions for the planner subagent, consistent with how coder, explore, and plan are tested. Replaces the loose string-in checks with a full snapshot of ROLE_ADDITIONAL so the recon_seeds contract is pinned exactly.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@tests/core/test_agent_spec.py`:
- Around line 443-466: Add lightweight semantic invariant assertions after the
existing snapshot assertion in the test for
subagent_specs["planner"].system_prompt_args (ROLE_ADDITIONAL): check that the
ROLE_ADDITIONAL text contains the <recon_seeds> tag, enforces that the final
response must contain only seeds (presence of "ONLY" or phrasing like "no
preamble"), and asserts presence of words requiring distinct/non-overlapping
seeds (e.g., "distinct" and "non-overlapping"); keep the full snapshot assertion
but supplement it with these targeted asserts to guard the protocol contract
without making the test brittle.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: 6970ecd2-e719-4dbd-bf3d-eae8cde45508
📒 Files selected for processing (1)
tests/core/test_agent_spec.py
7 tasks
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
Implements three architectural patterns from
docs/superpowers/specs/2026-06-06-harness-pattern-extraction-design.mdas zero-dependency, pure Python modules:1. Prompt injection defense (
utils/trust.py)UntrustedDatafrozen dataclass wraps external data in<untrusted_data id="NONCE">tags with closing-tag HTML escapingReadFile(3 paths) andFetchURL(3 paths)2. Adversarial verification boundary (
utils/artifacts.py)CodingArtifact/VerificationResult— generic coder↔verifier pairVulnerabilityArtifact/AuditVerdict— security-specific finder↔audit verifier paircoder.yamlandverifier.yaml3. Recon-first parallel seeding (
agents/default/planner.yaml)agent.yamlalongside existing subagentsChanges
src/pythinker_code/utils/trust.pyUntrustedDatawithrender_for_prompt()src/pythinker_code/utils/artifacts.pysrc/pythinker_code/agents/default/planner.yamlsrc/pythinker_code/tools/file/read.pyToolOkoutput pathssrc/pythinker_code/tools/web/fetch.pybuilder.write()pathssrc/pythinker_code/agents/default/coder.yamlsrc/pythinker_code/agents/default/verifier.yamlsrc/pythinker_code/agents/default/agent.yamlTesting
make check-pythinker-codeclean (ruff + pyright + ty)Security properties
UntrustedDatanonce wrapping atFetchURLUntrustedDatanonce wrapping atReadFileSummary by CodeRabbit
New Features
Improvements
Tests