diff --git a/docs/SELF_REVIEW_2026-04-30.findings.json b/docs/SELF_REVIEW_2026-04-30.findings.json new file mode 100644 index 0000000..40cb024 --- /dev/null +++ b/docs/SELF_REVIEW_2026-04-30.findings.json @@ -0,0 +1,401 @@ +{ + "generated_at": "2026-04-30T05:55:10.696108956Z", + "findings": [ + { + "id": "5d2d6990d7f5", + "title": "Shell command execution", + "severity": "high", + "status": "suspected", + "file": "PROMPT.md", + "line_hint": "419", + "evidence": "- `exec(`", + "reason": "Direct subprocess/shell invocation. Confirm inputs are sanitized; prefer typed APIs over string-built commands.", + "source": "static", + "confidence": 0.6, + "check_id": "static.shell_execution" + }, + { + "id": "ade02702c129", + "title": "Shell command execution", + "severity": "high", + "status": "suspected", + "file": "PROMPT.md", + "line_hint": "420", + "evidence": "- `spawn(`", + "reason": "Direct subprocess/shell invocation. Confirm inputs are sanitized; prefer typed APIs over string-built commands.", + "source": "static", + "confidence": 0.6, + "check_id": "static.shell_execution" + }, + { + "id": "a15313bf5fac", + "title": "Shell command execution", + "severity": "high", + "status": "suspected", + "file": "PROMPT.md", + "line_hint": "421", + "evidence": "- `Command::new`", + "reason": "Direct subprocess/shell invocation. Confirm inputs are sanitized; prefer typed APIs over string-built commands.", + "source": "static", + "confidence": 0.6, + "check_id": "static.shell_execution" + }, + { + "id": "08acbf12529d", + "title": "Raw SQL interpolation", + "severity": "high", + "status": "suspected", + "file": "PROMPT.md", + "line_hint": "423", + "evidence": "- `format!(\"SELECT`", + "reason": "SQL assembled via string formatting/concatenation rather than parameterized query. Verify inputs aren't user-controlled.", + "suggested_fix": "Use parameterized queries / prepared statements; pass values via driver placeholders, not string interpolation.", + "source": "static", + "confidence": 0.6, + "check_id": "static.raw_sql_interpolation" + }, + { + "id": "6a7008d0004a", + "title": "Wildcard CORS", + "severity": "high", + "status": "suspected", + "file": "PROMPT.md", + "line_hint": "429", + "evidence": "- `Access-Control-Allow-Origin: *`", + "reason": "Access-Control-Allow-Origin: * permits cross-origin reads from any domain. Narrow to an explicit allowlist unless this endpoint is intentionally public.", + "source": "static", + "confidence": 0.85, + "check_id": "static.broad_cors" + }, + { + "id": "7708bce04aa7", + "title": "Shell command execution", + "severity": "high", + "status": "suspected", + "file": "internal/analyzers/checks.go", + "line_hint": "89", + "evidence": "// === 2. shell execution (exec, spawn, Command::new, subprocess) ===", + "reason": "Direct subprocess/shell invocation. Confirm inputs are sanitized; prefer typed APIs over string-built commands.", + "source": "static", + "confidence": 0.6, + "check_id": "static.shell_execution" + }, + { + "id": "0b78dca77fb3", + "title": "Shell command execution", + "severity": "high", + "status": "suspected", + "file": "internal/analyzers/checks.go", + "line_hint": "101", + "evidence": "`Command::new|` + // Rust", + "reason": "Direct subprocess/shell invocation. Confirm inputs are sanitized; prefer typed APIs over string-built commands.", + "source": "static", + "confidence": 0.6, + "check_id": "static.shell_execution" + }, + { + "id": "3729ed9168dd", + "title": "Raw SQL interpolation", + "severity": "high", + "status": "suspected", + "file": "internal/analyzers/checks.go", + "line_hint": "140", + "evidence": "rawSQLFmtRe = regexp.MustCompile(`(?i)(?:format!|fmt\\.Sprintf|String::from|f\"|f')[^\\n]{0,80}?(?:SELECT|INSERT|UPDATE|DELETE|DROP)\\b`)", + "reason": "SQL assembled via string formatting/concatenation rather than parameterized query. Verify inputs aren't user-controlled.", + "suggested_fix": "Use parameterized queries / prepared statements; pass values via driver placeholders, not string interpolation.", + "source": "static", + "confidence": 0.6, + "check_id": "static.raw_sql_interpolation" + }, + { + "id": "4ed58fa58f04", + "title": "Raw SQL interpolation", + "severity": "high", + "status": "suspected", + "file": "internal/analyzers/checks.go", + "line_hint": "143", + "evidence": "rawSQLConcatRe = regexp.MustCompile(`(?i)(?:SELECT|INSERT|UPDATE|DELETE)\\b[^\\n]{0,40}(?:\\+\\s*\\w|%s|%v|\\$\\{|` + \"`\" + `\\$\\{)`)", + "reason": "SQL assembled via string formatting/concatenation rather than parameterized query. Verify inputs aren't user-controlled.", + "suggested_fix": "Use parameterized queries / prepared statements; pass values via driver placeholders, not string interpolation.", + "source": "static", + "confidence": 0.6, + "check_id": "static.raw_sql_interpolation" + }, + { + "id": "174c7276677f", + "title": "Wildcard CORS", + "severity": "high", + "status": "suspected", + "file": "internal/analyzers/checks.go", + "line_hint": "177", + "evidence": "// styles: Express's res.setHeader(\"Access-Control-Allow-Origin\", \"*\"),", + "reason": "Access-Control-Allow-Origin: * permits cross-origin reads from any domain. Narrow to an explicit allowlist unless this endpoint is intentionally public.", + "source": "static", + "confidence": 0.85, + "check_id": "static.broad_cors" + }, + { + "id": "e9beee52079c", + "title": "Wildcard CORS", + "severity": "high", + "status": "suspected", + "file": "internal/analyzers/checks.go", + "line_hint": "180", + "evidence": "var corsAnyRe = regexp.MustCompile(`Access-Control-Allow-Origin[^\\n]{0,40}\\*`)", + "reason": "Access-Control-Allow-Origin: * permits cross-origin reads from any domain. Narrow to an explicit allowlist unless this endpoint is intentionally public.", + "source": "static", + "confidence": 0.85, + "check_id": "static.broad_cors" + }, + { + "id": "14fa8051e73c", + "title": "Wildcard CORS", + "severity": "high", + "status": "suspected", + "file": "internal/analyzers/checks.go", + "line_hint": "194", + "evidence": "Reason: \"Access-Control-Allow-Origin: * permits cross-origin reads from any domain. Narrow to an explicit allowlist unless this endpoint is intentionally public.\",", + "reason": "Access-Control-Allow-Origin: * permits cross-origin reads from any domain. Narrow to an explicit allowlist unless this endpoint is intentionally public.", + "source": "static", + "confidence": 0.85, + "check_id": "static.broad_cors" + }, + { + "id": "5ccf0d1f8491", + "title": "Large file", + "severity": "medium", + "status": "suspected", + "file": "review-harness", + "line_hint": "1-18860", + "evidence": "18860 lines (limit: 800)", + "reason": "File exceeds the configured size threshold. Long files are a refactor target — split by responsibility.", + "source": "static", + "confidence": 1, + "check_id": "static.large_files" + }, + { + "id": "5703eb1ba5fe", + "title": "Environment file in source tree", + "severity": "high", + "status": "confirmed", + "file": "tests/fixtures/insecure-repo/.env", + "evidence": "filename=.env", + "reason": ".env files commonly hold real secrets and should not be tracked. If this is a sample, rename to .env.example with placeholder values.", + "suggested_fix": "Rename to .env.example with placeholders; add .env to .gitignore; rotate any committed secrets.", + "source": "static", + "confidence": 0.9, + "check_id": "static.env_file_committed" + }, + { + "id": "144fc5f2981e", + "title": "Hardcoded absolute path", + "severity": "medium", + "status": "suspected", + "file": "tests/fixtures/insecure-repo/src/handler.go", + "line_hint": "10", + "evidence": "const HARDCODED_PATH = \"/home/profit/secrets/key.pem\"", + "reason": "Absolute path encoded in source — couples the binary to one filesystem layout. Move to config or env var.", + "source": "static", + "confidence": 0.7, + "check_id": "static.hardcoded_paths" + }, + { + "id": "fe1d54ec5045", + "title": "Shell command execution", + "severity": "high", + "status": "suspected", + "file": "tests/fixtures/insecure-repo/src/handler.go", + "line_hint": "19", + "evidence": "exec.Command(\"bash\", \"-c\", cmd).Run()", + "reason": "Direct subprocess/shell invocation. Confirm inputs are sanitized; prefer typed APIs over string-built commands.", + "source": "static", + "confidence": 0.6, + "check_id": "static.shell_execution" + }, + { + "id": "e00ebc0ac661", + "title": "Raw SQL interpolation", + "severity": "high", + "status": "suspected", + "file": "tests/fixtures/insecure-repo/src/handler.go", + "line_hint": "14", + "evidence": "q := fmt.Sprintf(\"SELECT * FROM users WHERE name = '%s'\", name)", + "reason": "SQL assembled via string formatting/concatenation rather than parameterized query. Verify inputs aren't user-controlled.", + "suggested_fix": "Use parameterized queries / prepared statements; pass values via driver placeholders, not string interpolation.", + "source": "static", + "confidence": 0.6, + "check_id": "static.raw_sql_interpolation" + }, + { + "id": "66b494571e04", + "title": "Possible secret committed to source", + "severity": "critical", + "status": "suspected", + "file": "tests/fixtures/insecure-repo/src/handler.go", + "line_hint": "23", + "evidence": "const API_KEY = \"sk-1234567890abcdefABCDEFGHIJKLMNOPQRSTUV\"", + "reason": "OpenAI/OpenRouter-shaped key detected. If real, rotate immediately and move to a secret store.", + "suggested_fix": "Move secret to env var / secret manager; commit the .env.example with a placeholder; rotate the leaked credential.", + "source": "static", + "confidence": 0.75, + "check_id": "static.secret_patterns" + }, + { + "id": "66b494571e04", + "title": "Possible secret committed to source", + "severity": "critical", + "status": "suspected", + "file": "tests/fixtures/insecure-repo/src/handler.go", + "line_hint": "23", + "evidence": "const API_KEY = \"sk-1234567890abcdefABCDEFGHIJKLMNOPQRSTUV\"", + "reason": "Hardcoded credential pattern detected. If real, rotate immediately and move to a secret store.", + "suggested_fix": "Move secret to env var / secret manager; commit the .env.example with a placeholder; rotate the leaked credential.", + "source": "static", + "confidence": 0.75, + "check_id": "static.secret_patterns" + }, + { + "id": "5b886708c3b5", + "title": "TODO/FIXME comment", + "severity": "low", + "status": "suspected", + "file": "tests/fixtures/insecure-repo/src/handler.go", + "line_hint": "9", + "evidence": "// TODO: rotate this and move to env", + "reason": "Inline marker for deferred work. Audit whether the deferred concern is now blocking.", + "source": "static", + "confidence": 0.95, + "check_id": "static.todo_comments" + }, + { + "id": "1e61962f010a", + "title": "TODO/FIXME comment", + "severity": "low", + "status": "suspected", + "file": "tests/fixtures/insecure-repo/src/handler.go", + "line_hint": "22", + "evidence": "// FIXME: hardcoded creds", + "reason": "Inline marker for deferred work. Audit whether the deferred concern is now blocking.", + "source": "static", + "confidence": 0.95, + "check_id": "static.todo_comments" + }, + { + "id": "2c5c4350c263", + "title": "Hardcoded private-network IP", + "severity": "medium", + "status": "suspected", + "file": "tests/fixtures/insecure-repo/src/handler.go", + "line_hint": "11", + "evidence": "const SERVER_IP = \"192.168.1.176\"", + "reason": "RFC 1918 / link-local IP literal in source. Move to config so the binary isn't tied to one network.", + "source": "static", + "confidence": 0.7, + "check_id": "static.hardcoded_local_ip" + }, + { + "id": "3756dd35d39d", + "title": "Large file", + "severity": "medium", + "status": "suspected", + "file": "tests/fixtures/insecure-repo/src/huge.go", + "line_hint": "1-901", + "evidence": "901 lines (limit: 800)", + "reason": "File exceeds the configured size threshold. Long files are a refactor target — split by responsibility.", + "source": "static", + "confidence": 1, + "check_id": "static.large_files" + }, + { + "id": "6b5c4b19d770", + "title": "Wildcard CORS", + "severity": "high", + "status": "suspected", + "file": "tests/fixtures/insecure-repo/src/server.js", + "line_hint": "2", + "evidence": "res.setHeader(\"Access-Control-Allow-Origin\", \"*\");", + "reason": "Access-Control-Allow-Origin: * permits cross-origin reads from any domain. Narrow to an explicit allowlist unless this endpoint is intentionally public.", + "source": "static", + "confidence": 0.85, + "check_id": "static.broad_cors" + }, + { + "id": "0df161625ead", + "title": "Possible secret committed to source", + "severity": "critical", + "status": "suspected", + "file": "tests/fixtures/insecure-repo/src/server.js", + "line_hint": "5", + "evidence": "const AWS_KEY = \"AKIAIOSFODNN7EXAMPLE\";", + "reason": "AWS access key ID detected. If real, rotate immediately and move to a secret store.", + "suggested_fix": "Move secret to env var / secret manager; commit the .env.example with a placeholder; rotate the leaked credential.", + "source": "static", + "confidence": 0.75, + "check_id": "static.secret_patterns" + }, + { + "id": "6dd9b338f734", + "title": "TODO/FIXME comment", + "severity": "low", + "status": "suspected", + "file": "tests/fixtures/insecure-repo/src/server.js", + "line_hint": "1", + "evidence": "// HACK: open CORS for now", + "reason": "Inline marker for deferred work. Audit whether the deferred concern is now blocking.", + "source": "static", + "confidence": 0.95, + "check_id": "static.todo_comments" + }, + { + "id": "4a3098b9af2c", + "title": "Mutation route in file with no visible auth", + "severity": "medium", + "status": "suspected", + "file": "tests/fixtures/insecure-repo/src/server.js", + "line_hint": "7", + "evidence": "app.post(\"/api/users\", function(req, res) { /* no auth */ });", + "reason": "POST/PUT/DELETE/PATCH route registered in a file with no visible auth middleware. May still be auth'd at a higher layer — confirm.", + "source": "static", + "confidence": 0.4, + "check_id": "static.exposed_mutation_endpoint" + }, + { + "id": "44d91c0668c0", + "title": "Mutation route in file with no visible auth", + "severity": "medium", + "status": "suspected", + "file": "tests/fixtures/insecure-repo/src/server.js", + "line_hint": "8", + "evidence": "app.delete(\"/api/admin\", function(req, res) { /* no auth */ });", + "reason": "POST/PUT/DELETE/PATCH route registered in a file with no visible auth middleware. May still be auth'd at a higher layer — confirm.", + "source": "static", + "confidence": 0.4, + "check_id": "static.exposed_mutation_endpoint" + } + ], + "summary": { + "total": 28, + "confirmed": 1, + "suspected": 27, + "rejected": 0, + "critical": 3, + "high": 16, + "medium": 6, + "low": 3, + "by_source": { + "static": 28 + }, + "by_check": { + "static.broad_cors": 5, + "static.env_file_committed": 1, + "static.exposed_mutation_endpoint": 2, + "static.hardcoded_local_ip": 1, + "static.hardcoded_paths": 1, + "static.large_files": 2, + "static.raw_sql_interpolation": 4, + "static.secret_patterns": 3, + "static.shell_execution": 6, + "static.todo_comments": 3 + } + } +} diff --git a/docs/SELF_REVIEW_2026-04-30.md b/docs/SELF_REVIEW_2026-04-30.md new file mode 100644 index 0000000..94a7e35 --- /dev/null +++ b/docs/SELF_REVIEW_2026-04-30.md @@ -0,0 +1,115 @@ +# Scrum Test — local-review-harness-full-md +> **Self-review snapshot** — captured 2026-04-30 against commit `f3ee472` (Phase A + B MVP). +> Re-run with `./review-harness scrum .` for live numbers. +> +> The 3 critical findings below are from `tests/fixtures/insecure-repo/` — planted on +> purpose to prove the secret-pattern + env-file analyzers fire. Real codebase secrets +> would land in the same shape; operators dismiss fixture content as known-intentional. + + +**Generated:** 2026-04-30T05:55:10.657498039Z +**Branch:** (no git) · **Commit:** — + +## Verdict + +**blocked** — critical-severity finding present. See Confirmed Risks; rotate any leaked credentials, then re-run. + +## Evidence + +- repo path: `/home/profit/share/local-review-harness-full-md` +- file count: 36 +- languages: Go (18), Markdown (7), TypeScript (2), YAML (2), JSON (1) +- dependency manifests: 3 (go.mod, go.sum, tests/fixtures/clean-repo/package.json) +- test files/dirs: 9 +- LLM review: **skipped** (Phase C not implemented OR provider unavailable; see model-doctor.json) + +## Confirmed Risks + +| Severity | File:Line | Title | Evidence | +|---|---|---|---| +| high | `tests/fixtures/insecure-repo/.env` | Environment file in source tree | `filename=.env` | + +## Suspected Risks + +Each entry is a static-scan regex hit awaiting validation (Phase D / LLM cross-check). + +| Severity | File:Line | Title | Evidence | +|---|---|---|---| +| critical | `tests/fixtures/insecure-repo/src/handler.go:23` | Possible secret committed to source | `const API_KEY = "sk-1234567890abcdefABCDEFGHIJKLMNOPQRSTUV"` | +| critical | `tests/fixtures/insecure-repo/src/handler.go:23` | Possible secret committed to source | `const API_KEY = "sk-1234567890abcdefABCDEFGHIJKLMNOPQRSTUV"` | +| critical | `tests/fixtures/insecure-repo/src/server.js:5` | Possible secret committed to source | `const AWS_KEY = "AKIAIOSFODNN7EXAMPLE";` | +| high | `PROMPT.md:419` | Shell command execution | `- `exec(`` | +| high | `PROMPT.md:420` | Shell command execution | `- `spawn(`` | +| high | `PROMPT.md:421` | Shell command execution | `- `Command::new`` | +| high | `PROMPT.md:423` | Raw SQL interpolation | `- `format!("SELECT`` | +| high | `PROMPT.md:429` | Wildcard CORS | `- `Access-Control-Allow-Origin: *`` | +| high | `internal/analyzers/checks.go:101` | Shell command execution | ``Command::new\|` + // Rust` | +| high | `internal/analyzers/checks.go:140` | Raw SQL interpolation | `rawSQLFmtRe = regexp.MustCompile(`(?i)(?:format!\|fmt\.Sprintf\|String::from\|f"\|f')[^\n]{0,80}?(?:SELECT\|INSERT\|UPDA…` | +| high | `internal/analyzers/checks.go:143` | Raw SQL interpolation | `rawSQLConcatRe = regexp.MustCompile(`(?i)(?:SELECT\|INSERT\|UPDATE\|DELETE)\b[^\n]{0,40}(?:\+\s*\w\|%s\|%v\|\$\{\|` + "`…` | +| high | `internal/analyzers/checks.go:177` | Wildcard CORS | `// styles: Express's res.setHeader("Access-Control-Allow-Origin", "*"),` | +| high | `internal/analyzers/checks.go:180` | Wildcard CORS | `var corsAnyRe = regexp.MustCompile(`Access-Control-Allow-Origin[^\n]{0,40}\*`)` | +| high | `internal/analyzers/checks.go:194` | Wildcard CORS | `Reason: "Access-Control-Allow-Origin: * permits cross-origin reads from any domain. Narrow to an explicit allowlist un…` | +| high | `internal/analyzers/checks.go:89` | Shell command execution | `// === 2. shell execution (exec, spawn, Command::new, subprocess) ===` | +| high | `tests/fixtures/insecure-repo/src/handler.go:14` | Raw SQL interpolation | `q := fmt.Sprintf("SELECT * FROM users WHERE name = '%s'", name)` | +| high | `tests/fixtures/insecure-repo/src/handler.go:19` | Shell command execution | `exec.Command("bash", "-c", cmd).Run()` | +| high | `tests/fixtures/insecure-repo/src/server.js:2` | Wildcard CORS | `res.setHeader("Access-Control-Allow-Origin", "*");` | +| medium | `review-harness:1-18860` | Large file | `18860 lines (limit: 800)` | +| medium | `tests/fixtures/insecure-repo/src/handler.go:10` | Hardcoded absolute path | `const HARDCODED_PATH = "/home/profit/secrets/key.pem"` | +| medium | `tests/fixtures/insecure-repo/src/handler.go:11` | Hardcoded private-network IP | `const SERVER_IP = "192.168.1.176"` | +| medium | `tests/fixtures/insecure-repo/src/huge.go:1-901` | Large file | `901 lines (limit: 800)` | +| medium | `tests/fixtures/insecure-repo/src/server.js:7` | Mutation route in file with no visible auth | `app.post("/api/users", function(req, res) { /* no auth */ });` | +| medium | `tests/fixtures/insecure-repo/src/server.js:8` | Mutation route in file with no visible auth | `app.delete("/api/admin", function(req, res) { /* no auth */ });` | +| low | `tests/fixtures/insecure-repo/src/handler.go:22` | TODO/FIXME comment | `// FIXME: hardcoded creds` | +| low | `tests/fixtures/insecure-repo/src/handler.go:9` | TODO/FIXME comment | `// TODO: rotate this and move to env` | +| low | `tests/fixtures/insecure-repo/src/server.js:1` | TODO/FIXME comment | `// HACK: open CORS for now` | + +## Blocked Checks + +- LLM review (Phase 2 in REVIEW_PIPELINE.md). Reason: provider unavailable or stub. Next command: `review-harness model doctor` + +## Sprint Backlog + +**Sprint 0 — Reproducibility Gate** + +- Wire `just verify` (or equivalent) to run the static checks before every commit/PR. +- Add a CI step that fails on `critical` findings. +- Triage the 28 findings emitted by this run; mark each as accepted / blocking / dismiss-with-reason. + +**Sprint 1 — Trust Boundary Gate** + +- Resolve every `critical` and `high` finding before non-loopback deploy. +- Confirm auth posture for any mutation endpoint flagged as exposed. +- Replace raw SQL interpolation with parameterized queries. + +**Sprint 2 — Memory Correctness Gate** + +- (Phase E) Wire append-only `.memory/` writes for known-risks + fixed-patterns. +- Add a regression test that re-runs the harness and asserts no regression in confirmed-finding count. + +**Sprint 3 — Agent Loop Reality Gate** + +- (Phase C) Wire local-Ollama LLM review. +- (Phase D) Validator pass cross-checks every LLM finding against repo evidence. + +**Sprint 4 — Deployment Gate** + +- Ship the harness as a single static binary (`go build -o review-harness`). +- Document operator runbook (model setup, profile editing, output retention). + +## Acceptance Gates + +Each gate must be testable. Format: command + verifiable post-condition. + +1. **Reproducibility:** `review-harness repo .` exits 0; `reports/latest/repo-intake.json` exists with non-zero `file_count`. +2. **No false positives on a clean fixture:** `review-harness repo tests/fixtures/clean-repo` produces zero `confirmed` findings. +3. **Every documented static check fires on the insecure fixture:** `jq '[.findings[] | .check_id] | unique | length' reports/latest/static-findings.json` ≥ 8. +4. **Receipts are honest about degraded phases:** `jq '[.phases[] | select(.status == "degraded")]' reports/latest/receipts.json` lists every skipped/stubbed phase. +5. **Critical findings block production deploy:** at least one critical finding is currently present; resolve before deploy. + +## Next Commands + +1. Open the risk register: `cat reports/latest/risk-register.md` +2. Triage every `critical` finding; rotate any leaked credentials immediately. +- Probe the model provider: `review-harness model doctor` +- Re-run after fixes: `review-harness repo /home/profit/share/local-review-harness-full-md` +- Generate the full Scrum bundle: `review-harness scrum /home/profit/share/local-review-harness-full-md`