docs: add PRD for SAST pipeline v0.2 stabilization - #50
Merged
Conversation
Captures the design alignment reached through a relentless architecture interview covering all 5 areas: pipeline stages, static analysis to exploit mapping, sandbox isolation, incomplete logic, and module depth. Documents 13 confirmed gaps in 3 tiers (security, correctness, capability), 16 implementation decisions with rationale, and testing boundaries per module. Accompanies the 12 GitHub issues (#38–#49) filed against each gap. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR adds a new product requirements document for the ShieldClaw SAST pipeline v0.2 stabilization effort. It is intended to capture agreed architecture, gaps, and implementation decisions for the current verification pipeline and to serve as the documentation anchor behind the linked follow-up issues.
Changes:
- Adds
docs/prd-sast-pipeline-v02.mdas a full PRD for the 7-stage SAST pipeline - Documents confirmed gaps across security, correctness, and capability areas
- Records implementation decisions, testing boundaries, and out-of-scope items for upcoming work
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Three fixes, two root causes:
1. Integration job crash -- pytest-timeout missing from requirements-dev.txt
The CI workflow passes --timeout=300 to pytest but pytest-timeout was
not listed as a dev dependency, causing the integration job to exit
immediately with 'unrecognized arguments'.
Fix: add pytest-timeout>=0.5 to requirements-dev.txt.
2. Unit job failure -- test_full_stack_detonate_and_teardown missing marker
The test lives in test_docker_orchestrator_integration.py but had no
@pytest.mark.integration decorator, so the unit suite (-m 'not integration')
collected it. On ubuntu-latest, Docker is available and the compose
fixture exists, so both @pytest.mark.skipif guards passed -- then
_probe_attacker_image() failed because the attacker image is only built
in the integration job.
Fix: add @pytest.mark.integration to the test.
3. Pre-existing bug -- ContextAggregator inherited outer GIT_DIR
When run inside a git worktree, _git_diff_head_minus_one() inherited
GIT_DIR from the environment and read the wrong repository instead of
the one at root. This caused test_git_diff_head_minus_one to fail
('empty diff') and test_git_diff_requires_repository to get the wrong
error message on Windows.
Fix: strip GIT_* env vars from the aggregator's git subprocess so it
always reads the repository at root. Strip the same vars from the
test's run_git helper for consistency.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Address Copilot review comments on prd-sast-pipeline-v02.md: 1. Finding lifecycle states (line 68): POC_GENERATED and DETONATED are never written by the orchestrator. Actual states are INGESTED -> TRIAGED -> SCORED -> APPROVED -> VERDICTED (REJECTED for denied approval). Also documents the broken async HITL path: shieldclaw approve queries AWAITING_APPROVAL but nothing writes that state; the SCORED -> AWAITING_APPROVAL transition is missing. 2. Source excerpt (line 75): No excerpt is stored at ingest time. The Finding dataclass and SQLite schema have no excerpt field. The excerpt is reconstructed from disk on-the-fly by _extract_source_lines() at scoring and PoC generation time, meaning resumed scans read the current file, not the file as it was when Semgrep ran. 3. Async approval flow (line 105): Corrected to show the async path is currently broken (AWAITING_APPROVAL never set), not merely a two-process handoff. Both the missing state transition and the not-yet-built interactive mode are now tied to issue #48. 4. Patch generation rationale (line 231): Corrects 'mentioned in README' to 'specified in ADR-009 as v0.3 work (triple-verification patch loop)'. The README has no mention of patch generation. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
blondres04
enabled auto-merge (squash)
May 4, 2026 01:13
Three root causes: 1. Inverted assertion in test_pypi_unreachable_with_sealed_network The assertion checked for 'requests' or 'WARNING' in stdout, but when the network is correctly sealed the script prints 'PyPI unreachable (expected): ConnectionError' — neither string is present. Fix: check for 'unreachable' (sealed path) or 'WARNING' (open path). 2. Base images not pre-pulled before integration tests docker compose up blocked pulling nginx:alpine (concurrency test) and postgres:15-alpine (full-stack + e2e tests) at test time, eating into the startup deadline and causing timeout failures. Fix: add 'docker pull nginx:alpine postgres:15-alpine' CI step after the attacker image build and before pytest runs. 3. Default _START_WAIT_SECONDS too short for CI The DockerOrchestrator default of 60 s is not enough for Flask + Postgres stacks: postgres:15-alpine init + Flask healthcheck start_period (20 s) + retries can exceed 60 s even with pre-pulled images. The e2e test uses the default, so it always timed out. Fix: raise default from 60 s to 120 s. The full-stack test already used 120 s explicitly; this brings the default in line. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This was referenced May 4, 2026
…able timeout) Unify resolve_compose_start_wait_seconds to read SHIELDCLAW_COMPOSE_START_TIMEOUT (same env var as main's _compose_start_timeout) so both code paths honour the same CI override. Update unit test to use the unified env var name. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
detonate() now returns DetonationOutcome, not a bare int. Co-Authored-By: Claude Sonnet 4.6 <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
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
docs/prd-sast-pipeline-v02.md— a serialization of design alignment reached through a full architecture interview of the ShieldClaw codebaseWhat changed
A single new document:
shield-claw/docs/prd-sast-pipeline-v02.mdThe PRD was produced by walking every branch of the design tree across 5 focus areas:
Why
Accompanies the 12 issues filed in this session (#38–#49), each of which is a directly-grabbable vertical slice derived from the gaps documented here. The PRD is the source of truth for the decisions behind those issues.
What a reviewer should know
observer/base.py→observer/__init__.pyconsolidation noted in section 5 is tracked in Surface observer failures in report output #44.intelligence/and is explicitly deferred.Test plan