From 4dc53c5798077d461b19b41462e80f4135efd4a5 Mon Sep 17 00:00:00 2001 From: "Claude (review-harness setup)" Date: Thu, 30 Apr 2026 01:24:02 -0500 Subject: [PATCH] =?UTF-8?q?Phase=20D=20=E2=80=94=20validator=20cross-check?= =?UTF-8?q?s=20LLM=20findings=20+=202=20close-out=20fixes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implements PROMPT.md / docs/REVIEW_PIPELINE.md Phase 3: "AI may suggest. Code validates." internal/validators/validate.go — 3 hard checks per the "Reject A Finding If" list: - file does not exist (with path-traversal guard against the LLM hallucinating ../../../etc/passwd) - cited evidence does not appear in the file (verbatim or trim-line-by-line — models often re-indent quotes when quoting code) - line hint exceeds file line count 3 soft checks documented as open (claim semantics, suggested-fix relevance, invented tests/commands — all need another LLM pass). internal/validators/validate_test.go — 9 tests including: - TestValidate_RejectsNonexistentFile (gate D1) - TestValidate_RejectsEvidenceNotInFile - TestValidate_RejectsLineHintBeyondFile - TestValidate_AcceptsRealFinding - TestValidate_AcceptsEvidenceWithDifferentLeadingWhitespace - TestValidate_RejectsEmptyEvidence - TestValidate_PassesThroughStaticFindings - TestValidate_RejectsPathEscapingRepo (path-traversal protection) - TestValidate_AcceptsRelativeRepoPath (the regression — see below) Pipeline phase 3 wired between LLM review (Phase C) and report gen (Phase 4). validated-findings.json contains the confirmed set; rejected-findings.json contains rejects with per-finding reason + detail. Receipt phase entry honest about output files + status. === Bug J caught === First Phase D run rejected EVERY real LLM finding as file_not_found because the path-traversal check compared a relative joined path (`tests/fixtures/insecure-repo/src/handler.go`) against an absolute repoAbs (`/home/profit/share/.../insecure-repo`), so HasPrefix always returned false. Both sides now resolved via filepath.Abs before comparison. Regression test TestValidate_AcceptsRelativeRepoPath locks this in — runs the validator against a relative repo path AND a relative chdir, the exact shape that hit the bug. J's framing was honest: "I don't know what the problem is, but you know what we're trying to accomplish." The fix-it-yourself signal let me trace through the rejection details + see the smoking gun in the detail string ("escapes repo root"). Without that prompt the 9 false rejections might have looked like real LLM bugs. === 2 close-out fixes === 1. .gitignore: changed `/reports/latest/` → `**/reports/latest/` (and same for `run-*`). Phase C committed 22 generated files from `tests/fixtures/*/reports/latest/` because the original pattern was anchored at the harness root only. Existing tracked files removed via git rm --cached; new pattern keeps fixture reports out of version control going forward. 2. pipeline.cleanOutputDir: pipeline now deletes the bounded list of known per-run files at the start of each run. Before this, a prior run's rejected-findings.json could linger when the current run had no rejections — confused J during the bug hunt above. cleanOutputDir is bounded (deletes only files we emit) so operator-owned adjacent files stay. Verified end-to-end: insecure-repo + --enable-llm → 25 confirmed findings (16 static + 9 LLM), 0 rejected. Co-Authored-By: Claude Opus 4.7 (1M context) --- .gitignore | 9 +- internal/pipeline/pipeline.go | 73 +++++- internal/validators/validate.go | 247 ++++++++++++++++++ internal/validators/validate_test.go | 225 ++++++++++++++++ .../reports/latest/acceptance-gates.md | 8 - .../reports/latest/claim-coverage-table.md | 8 - .../latest/llm-findings.normalized.json | 16 -- .../reports/latest/llm-findings.raw.json | 30 --- .../clean-repo/reports/latest/receipts.json | 70 ----- .../reports/latest/repo-intake.json | 42 --- .../reports/latest/risk-register.md | 5 - .../clean-repo/reports/latest/scrum-test.md | 69 ----- .../reports/latest/sprint-backlog.md | 26 -- .../reports/latest/static-findings.json | 16 -- .../reports/latest/acceptance-gates.md | 9 - .../reports/latest/claim-coverage-table.md | 23 -- .../latest/llm-findings.normalized.json | 147 ----------- .../reports/latest/llm-findings.raw.json | 151 ----------- .../reports/latest/receipts.json | 82 ------ .../reports/latest/repo-intake.json | 37 --- .../reports/latest/risk-register.md | 22 -- .../reports/latest/scrum-test.md | 96 ------- .../reports/latest/sprint-backlog.md | 28 -- .../reports/latest/static-findings.json | 242 ----------------- 24 files changed, 546 insertions(+), 1135 deletions(-) create mode 100644 internal/validators/validate.go create mode 100644 internal/validators/validate_test.go delete mode 100644 tests/fixtures/clean-repo/reports/latest/acceptance-gates.md delete mode 100644 tests/fixtures/clean-repo/reports/latest/claim-coverage-table.md delete mode 100644 tests/fixtures/clean-repo/reports/latest/llm-findings.normalized.json delete mode 100644 tests/fixtures/clean-repo/reports/latest/llm-findings.raw.json delete mode 100644 tests/fixtures/clean-repo/reports/latest/receipts.json delete mode 100644 tests/fixtures/clean-repo/reports/latest/repo-intake.json delete mode 100644 tests/fixtures/clean-repo/reports/latest/risk-register.md delete mode 100644 tests/fixtures/clean-repo/reports/latest/scrum-test.md delete mode 100644 tests/fixtures/clean-repo/reports/latest/sprint-backlog.md delete mode 100644 tests/fixtures/clean-repo/reports/latest/static-findings.json delete mode 100644 tests/fixtures/insecure-repo/reports/latest/acceptance-gates.md delete mode 100644 tests/fixtures/insecure-repo/reports/latest/claim-coverage-table.md delete mode 100644 tests/fixtures/insecure-repo/reports/latest/llm-findings.normalized.json delete mode 100644 tests/fixtures/insecure-repo/reports/latest/llm-findings.raw.json delete mode 100644 tests/fixtures/insecure-repo/reports/latest/receipts.json delete mode 100644 tests/fixtures/insecure-repo/reports/latest/repo-intake.json delete mode 100644 tests/fixtures/insecure-repo/reports/latest/risk-register.md delete mode 100644 tests/fixtures/insecure-repo/reports/latest/scrum-test.md delete mode 100644 tests/fixtures/insecure-repo/reports/latest/sprint-backlog.md delete mode 100644 tests/fixtures/insecure-repo/reports/latest/static-findings.json diff --git a/.gitignore b/.gitignore index f3f011f..3e6bfd4 100644 --- a/.gitignore +++ b/.gitignore @@ -2,9 +2,12 @@ /review-harness /bin/ -# Runtime artifacts (PROMPT.md: reports go here per-run; gitignored) -/reports/latest/ -/reports/run-*/ +# Runtime artifacts (PROMPT.md: reports go here per-run; gitignored). +# Pattern is double-star so fixtures (tests/fixtures/*/reports/latest) +# also stay untracked — Phase D shipped with a leak that committed +# 1100+ lines of generated json/md before this rule was tightened. +**/reports/latest/ +**/reports/run-*/ # Memory persistence (lives next to target repos, not this one) /.memory/ diff --git a/internal/pipeline/pipeline.go b/internal/pipeline/pipeline.go index 2c9d673..ac53782 100644 --- a/internal/pipeline/pipeline.go +++ b/internal/pipeline/pipeline.go @@ -20,6 +20,7 @@ import ( "local-review-harness/internal/llm" "local-review-harness/internal/reporters" "local-review-harness/internal/scanner" + "local-review-harness/internal/validators" ) // Inputs is the bag the CLI passes to the pipeline. @@ -51,6 +52,13 @@ func RunRepo(ctx context.Context, in Inputs) (*Result, error) { StartedAt: startedAt.Format(time.RFC3339Nano), } + // Clean output dir before each run so stale files from a prior + // run can't leak into the current report set. 2026-04-30 fix: + // before this, a previous run's rejected-findings.json could + // stick around when the current run had no rejections, confusing + // operators about which data was current. + cleanOutputDir(in.OutputDir) + // --- Phase 0: repo intake --- scan, err := scanner.Walk(in.RepoPath, true) scanPhase := reporters.PhaseReceipt{Name: "repo_intake", Status: "ok"} @@ -143,11 +151,38 @@ func RunRepo(ctx context.Context, in Inputs) (*Result, error) { } receipt.Phases = append(receipt.Phases, llmPhase) - // --- Phase 3: validation (Phase D — also deferred) --- - receipt.Phases = append(receipt.Phases, reporters.PhaseReceipt{ - Name: "validation", Status: "skipped", - Errors: []string{"Phase D not implemented in MVP — depends on Phase C"}, - }) + // --- Phase 3: validation (Phase D) --- + // Cross-checks every LLM-sourced finding against actual file + // content + path-traversal protection. Static findings pass + // through promoted-to-confirmed (their evidence is already + // grep-truthful by construction). Rejected findings land in + // rejected-findings.json with per-rejection reason for the + // audit trail. + validatePhase := reporters.PhaseReceipt{Name: "validation", Status: "ok"} + valOut := validators.Validate(in.RepoPath, findings) + findings = valOut.Validated // pipeline downstream only sees validated set + + if sha, err := reporters.WriteJSON(filepath.Join(in.OutputDir, "validated-findings.json"), reporters.StaticFindings{ + GeneratedAt: time.Now().UTC().Format(time.RFC3339Nano), + Findings: valOut.Validated, + Summary: reporters.SummarizeFindings(valOut.Validated), + }); err != nil { + validatePhase.Status = "failed" + validatePhase.Errors = append(validatePhase.Errors, "validated: "+err.Error()) + } else { + validatePhase.OutputFiles = append(validatePhase.OutputFiles, "validated-findings.json") + validatePhase.OutputHash = sha + } + if len(valOut.Rejected) > 0 { + // Rejected file is informational, not gate-blocking — the + // audit trail belongs in version control. + if _, err := reporters.WriteJSON(filepath.Join(in.OutputDir, "rejected-findings.json"), valOut); err == nil { + validatePhase.OutputFiles = append(validatePhase.OutputFiles, "rejected-findings.json") + res.OutputFiles = append(res.OutputFiles, "rejected-findings.json") + } + } + res.OutputFiles = append(res.OutputFiles, "validated-findings.json") + receipt.Phases = append(receipt.Phases, validatePhase) // --- Phase 4: report generation (markdown) --- if in.EmitScrum { @@ -252,6 +287,34 @@ func runLLMReview(ctx context.Context, scan *scanner.Result, in Inputs) ([]analy return findings, outputs, nil } +// cleanOutputDir removes only the files this pipeline emits. We don't +// nuke the dir because operators might keep adjacent files there +// (e.g. `.gitkeep`); we delete a bounded list so prior-run artifacts +// can't masquerade as current data, but operator-owned files stay. +func cleanOutputDir(dir string) { + if dir == "" { + return + } + known := []string{ + "repo-intake.json", + "static-findings.json", + "llm-findings.raw.json", + "llm-findings.normalized.json", + "validated-findings.json", + "rejected-findings.json", + "scrum-test.md", + "risk-register.md", + "claim-coverage-table.md", + "sprint-backlog.md", + "acceptance-gates.md", + "receipts.json", + "model-doctor.json", + } + for _, name := range known { + _ = os.Remove(filepath.Join(dir, name)) + } +} + func newRunID(t time.Time) string { var rb [4]byte _, _ = rand.Read(rb[:]) diff --git a/internal/validators/validate.go b/internal/validators/validate.go new file mode 100644 index 0000000..55bafcf --- /dev/null +++ b/internal/validators/validate.go @@ -0,0 +1,247 @@ +// Package validators cross-checks LLM-generated findings against +// real repository evidence. PROMPT.md / REVIEW_PIPELINE.md Phase 3: +// "AI may suggest. Code validates." Findings that pass validation +// move from status=suspected → status=confirmed; failures land in a +// separate rejected-findings.json with a per-rejection reason. +// +// V0 implements 3 hard checks per the PROMPT.md "Reject A Finding If" +// list: +// - file does not exist +// - cited evidence does not exist verbatim in the file +// - line hint is impossible (file has fewer lines than claimed) +// +// 3 softer checks from the same list are NOT v0 — documented as +// "open" so the audit trail is honest: +// - claim is unsupported (semantic, requires another LLM pass) +// - suggested fix targets unrelated code (semantic) +// - model invents tests/commands/files (covered by file-exists for +// files; tests/commands need a Phase D+1 fact-check) +package validators + +import ( + "fmt" + "os" + "path/filepath" + "regexp" + "strconv" + "strings" + + "local-review-harness/internal/analyzers" +) + +// Reason captures why a finding was rejected. Stable strings so +// reports + receipts can group/sort by reason. +type Reason string + +const ( + ReasonFileNotFound Reason = "file_not_found" + ReasonNoEvidence Reason = "evidence_not_in_file" + ReasonLineHintTooHigh Reason = "line_hint_exceeds_file_length" + ReasonEmptyEvidence Reason = "empty_evidence_field" +) + +// Result is the validator's output for one finding. +type Result struct { + Finding analyzers.Finding `json:"finding"` + Validated bool `json:"validated"` + RejectionReason Reason `json:"rejection_reason,omitempty"` + RejectionDetail string `json:"rejection_detail,omitempty"` +} + +// Outputs split the input list into validated + rejected. Only LLM +// findings (Source == SourceLLM) get validated — static findings +// already have grep-able evidence by construction. +type Outputs struct { + Validated []analyzers.Finding `json:"-"` // promoted to confirmed + Rejected []Result `json:"rejected"` + Pass []Result `json:"pass"` +} + +// Validate runs the 3 hard checks for every LLM finding. Static and +// validator-source findings pass through unchanged (they have their +// own evidence pipeline). Returns the validated set + the rejected +// set with per-rejection reason for the audit trail. +// +// repoPath is the absolute path the LLM was asked to review; finding +// File paths are joined under it. +func Validate(repoPath string, findings []analyzers.Finding) Outputs { + out := Outputs{} + contentCache := map[string]string{} // abs path → content (read once) + + for _, f := range findings { + if f.Source != analyzers.SourceLLM { + // Non-LLM findings carry their own evidence path; pass through + // unchanged. The pipeline still ships them as-is. + f.Status = analyzers.StatusConfirmed + out.Validated = append(out.Validated, f) + out.Pass = append(out.Pass, Result{Finding: f, Validated: true}) + continue + } + + res := check(repoPath, f, contentCache) + if res.Validated { + res.Finding.Status = analyzers.StatusConfirmed + out.Validated = append(out.Validated, res.Finding) + out.Pass = append(out.Pass, res) + } else { + res.Finding.Status = analyzers.StatusRejected + out.Rejected = append(out.Rejected, res) + } + } + return out +} + +// check is the per-finding validation logic. Stops at the first +// failure — operators only need to see one rejection reason. +func check(repoPath string, f analyzers.Finding, cache map[string]string) Result { + res := Result{Finding: f} + + // Empty evidence is unusable — the model didn't quote anything. + if strings.TrimSpace(f.Evidence) == "" { + res.RejectionReason = ReasonEmptyEvidence + res.RejectionDetail = "finding has no evidence quote — can't be validated" + return res + } + + // Resolve absolute path. The validator runs after the scanner has + // already classified the repo; we trust f.File is repo-relative. + // Both repoPath AND the joined target are converted to absolute + // before the path-traversal check — bug fixed 2026-04-30: prior + // version compared relative-abs to absolute-repoAbs and HasPrefix + // always failed, rejecting every real finding as file_not_found. + joined := f.File + if !filepath.IsAbs(joined) { + joined = filepath.Join(repoPath, f.File) + } + abs, err := filepath.Abs(joined) + if err != nil { + res.RejectionReason = ReasonFileNotFound + res.RejectionDetail = "abs(" + joined + "): " + err.Error() + return res + } + abs = filepath.Clean(abs) + + // Refuse to traverse outside the repo (path-traversal protection + // — the LLM might have hallucinated a "../../../etc/passwd" file). + repoAbs, err := filepath.Abs(repoPath) + if err != nil { + res.RejectionReason = ReasonFileNotFound + res.RejectionDetail = "abs(" + repoPath + "): " + err.Error() + return res + } + repoAbs = filepath.Clean(repoAbs) + if !strings.HasPrefix(abs, repoAbs+string(filepath.Separator)) && abs != repoAbs { + res.RejectionReason = ReasonFileNotFound + res.RejectionDetail = fmt.Sprintf("path %q escapes repo root %q (resolved: abs=%q repo_abs=%q)", f.File, repoPath, abs, repoAbs) + return res + } + + // Read once + cache. + content, ok := cache[abs] + if !ok { + b, err := os.ReadFile(abs) + if err != nil { + res.RejectionReason = ReasonFileNotFound + res.RejectionDetail = err.Error() + return res + } + content = string(b) + cache[abs] = content + } + + // Evidence presence check — the verbatim quote MUST appear in the + // file. Tolerate leading/trailing whitespace differences (models + // often re-indent quotes); compare on trim. Multi-line evidence + // is matched as-is (newlines preserved). + if !evidencePresent(content, f.Evidence) { + res.RejectionReason = ReasonNoEvidence + res.RejectionDetail = fmt.Sprintf("evidence %q not found in %s", abbrev(f.Evidence, 80), f.File) + return res + } + + // Line hint plausibility — parse "42" or "10-20" or "line 42"; + // reject if file has fewer lines than the highest cited number. + if hint := strings.TrimSpace(f.LineHint); hint != "" { + hi, ok := highestLine(hint) + if ok { + fileLines := strings.Count(content, "\n") + 1 + if hi > fileLines { + res.RejectionReason = ReasonLineHintTooHigh + res.RejectionDetail = fmt.Sprintf("line %d cited but file has only %d lines", hi, fileLines) + return res + } + } + } + + res.Validated = true + return res +} + +// evidencePresent returns true if the evidence appears verbatim in +// the file. Multi-line evidence is checked as-is; single-line evidence +// is also compared trimmed (models often add/drop leading whitespace +// when quoting code). +func evidencePresent(content, evidence string) bool { + if strings.Contains(content, evidence) { + return true + } + // Trim each line of the evidence + match line-by-line. Conservative: + // every evidence line must appear (in order) in the file's trimmed + // lines for the evidence to count as found. + evLines := strings.Split(strings.TrimSpace(evidence), "\n") + if len(evLines) == 0 { + return false + } + contentLines := strings.Split(content, "\n") + cursor := 0 + for _, ev := range evLines { + want := strings.TrimSpace(ev) + if want == "" { + continue + } + found := false + for cursor < len(contentLines) { + if strings.Contains(strings.TrimSpace(contentLines[cursor]), want) { + found = true + cursor++ + break + } + cursor++ + } + if !found { + return false + } + } + return true +} + +// highestLine extracts the largest line number cited in the hint. +// Accepts "42", "10-20" (returns 20), "line 42", "L42", "42:5". +// Returns (n, true) on parse; (0, false) if no number found. +var lineHintNumRe = regexp.MustCompile(`\d+`) + +func highestLine(hint string) (int, bool) { + matches := lineHintNumRe.FindAllString(hint, -1) + if len(matches) == 0 { + return 0, false + } + hi := 0 + for _, m := range matches { + n, err := strconv.Atoi(m) + if err != nil { + continue + } + if n > hi { + hi = n + } + } + return hi, hi > 0 +} + +func abbrev(s string, n int) string { + s = strings.TrimSpace(s) + if len(s) <= n { + return s + } + return s[:n] + "…" +} diff --git a/internal/validators/validate_test.go b/internal/validators/validate_test.go new file mode 100644 index 0000000..e393a20 --- /dev/null +++ b/internal/validators/validate_test.go @@ -0,0 +1,225 @@ +package validators + +import ( + "os" + "path/filepath" + "testing" + + "local-review-harness/internal/analyzers" +) + +// makeFile is a tiny helper for test setup: write content to a path +// under tmp + return the abs path. +func makeFile(t *testing.T, dir, name, content string) string { + t.Helper() + p := filepath.Join(dir, name) + if err := os.MkdirAll(filepath.Dir(p), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(p, []byte(content), 0o644); err != nil { + t.Fatal(err) + } + return p +} + +// llmFinding builds a minimal Source=LLM finding. +func llmFinding(file, lineHint, evidence string) analyzers.Finding { + return analyzers.Finding{ + Title: "test", + Severity: analyzers.SeverityHigh, + Status: analyzers.StatusSuspected, + File: file, + LineHint: lineHint, + Evidence: evidence, + Reason: "test reason", + Source: analyzers.SourceLLM, + } +} + +// === GATE D1 — synthetic LLM finding citing nonexistent file → must reject === + +func TestValidate_RejectsNonexistentFile(t *testing.T) { + repo := t.TempDir() + out := Validate(repo, []analyzers.Finding{ + llmFinding("does/not/exist.go", "42", "fmt.Println"), + }) + if len(out.Rejected) != 1 { + t.Fatalf("expected 1 rejected finding, got %d", len(out.Rejected)) + } + if out.Rejected[0].RejectionReason != ReasonFileNotFound { + t.Errorf("expected ReasonFileNotFound, got %q", out.Rejected[0].RejectionReason) + } +} + +func TestValidate_RejectsEvidenceNotInFile(t *testing.T) { + repo := t.TempDir() + makeFile(t, repo, "real.go", "package main\nfunc main() {}\n") + out := Validate(repo, []analyzers.Finding{ + llmFinding("real.go", "1", "this string does not exist in the file"), + }) + if len(out.Rejected) != 1 { + t.Fatalf("expected 1 rejected, got %d", len(out.Rejected)) + } + if out.Rejected[0].RejectionReason != ReasonNoEvidence { + t.Errorf("expected ReasonNoEvidence, got %q", out.Rejected[0].RejectionReason) + } +} + +func TestValidate_RejectsLineHintBeyondFile(t *testing.T) { + repo := t.TempDir() + makeFile(t, repo, "small.go", "line one\nline two\n") // 2 lines + out := Validate(repo, []analyzers.Finding{ + llmFinding("small.go", "100", "line one"), + }) + if len(out.Rejected) != 1 { + t.Fatalf("expected 1 rejected, got %d", len(out.Rejected)) + } + if out.Rejected[0].RejectionReason != ReasonLineHintTooHigh { + t.Errorf("expected ReasonLineHintTooHigh, got %q", out.Rejected[0].RejectionReason) + } +} + +func TestValidate_AcceptsRealFinding(t *testing.T) { + repo := t.TempDir() + makeFile(t, repo, "good.go", "package main\nfunc badPattern() {}\n") + out := Validate(repo, []analyzers.Finding{ + llmFinding("good.go", "2", "func badPattern()"), + }) + if len(out.Validated) != 1 { + t.Fatalf("expected 1 validated, got %d (rejected=%d)", len(out.Validated), len(out.Rejected)) + } + if out.Validated[0].Status != analyzers.StatusConfirmed { + t.Errorf("expected status=confirmed, got %q", out.Validated[0].Status) + } +} + +func TestValidate_AcceptsEvidenceWithDifferentLeadingWhitespace(t *testing.T) { + // Models often re-indent code when quoting; the validator's + // trim-line-by-line fallback should accept it. + repo := t.TempDir() + makeFile(t, repo, "indented.go", "package main\n\n\tfunc indented() {\n\t\treturn\n\t}\n") + out := Validate(repo, []analyzers.Finding{ + llmFinding("indented.go", "3", "func indented() {"), // model dropped leading tab + }) + if len(out.Validated) != 1 { + t.Fatalf("expected 1 validated; got rejected=%d (reason=%q)", + len(out.Rejected), + func() Reason { + if len(out.Rejected) > 0 { + return out.Rejected[0].RejectionReason + } + return "" + }(), + ) + } +} + +func TestValidate_RejectsEmptyEvidence(t *testing.T) { + repo := t.TempDir() + makeFile(t, repo, "any.go", "package main\n") + out := Validate(repo, []analyzers.Finding{ + llmFinding("any.go", "1", ""), + }) + if len(out.Rejected) != 1 || out.Rejected[0].RejectionReason != ReasonEmptyEvidence { + t.Errorf("empty-evidence finding should be rejected with ReasonEmptyEvidence; got %+v", out.Rejected) + } +} + +func TestValidate_PassesThroughStaticFindings(t *testing.T) { + // Static findings already have grep-able evidence by construction. + // Validator promotes them to confirmed without re-checking. + repo := t.TempDir() + staticF := analyzers.Finding{ + Title: "static finding", + Severity: analyzers.SeverityMedium, + Status: analyzers.StatusSuspected, + File: "anywhere.go", + Evidence: "anything", + Source: analyzers.SourceStatic, + } + out := Validate(repo, []analyzers.Finding{staticF}) + if len(out.Validated) != 1 { + t.Fatalf("static finding should pass through validated; got %d", len(out.Validated)) + } + if out.Validated[0].Status != analyzers.StatusConfirmed { + t.Errorf("static finding should be promoted to confirmed; got %q", out.Validated[0].Status) + } +} + +// TestValidate_AcceptsRelativeRepoPath locks in the fix for the +// 2026-04-30 bug where every real finding was rejected as +// file_not_found because the path-traversal check compared a +// relative joined path against an absolute repoAbs (HasPrefix +// always false). Caught by J running ./review-harness with a +// relative target path; gate D1 now exercises this path. +func TestValidate_AcceptsRelativeRepoPath(t *testing.T) { + repo := t.TempDir() + makeFile(t, repo, "src/handler.go", "package main\nfunc bad() {}\n") + + // Pass the repo as a RELATIVE path. Pre-fix this triggered the bug. + cwd, _ := os.Getwd() + defer os.Chdir(cwd) + parent := filepath.Dir(repo) + relRepo, err := filepath.Rel(parent, repo) + if err != nil { + t.Skip("can't compute relative path: " + err.Error()) + } + if err := os.Chdir(parent); err != nil { + t.Fatal(err) + } + + out := Validate(relRepo, []analyzers.Finding{ + llmFinding("src/handler.go", "2", "func bad()"), + }) + if len(out.Validated) != 1 { + t.Errorf("relative repo path should still validate; got rejected=%d (reasons: %v)", + len(out.Rejected), + func() []Reason { + rs := []Reason{} + for _, r := range out.Rejected { + rs = append(rs, r.RejectionReason) + } + return rs + }()) + } +} + +// === Path-traversal protection: hallucinated "../../../etc/passwd" must reject === + +func TestValidate_RejectsPathEscapingRepo(t *testing.T) { + repo := t.TempDir() + out := Validate(repo, []analyzers.Finding{ + llmFinding("../../../etc/passwd", "1", "root:x:0:0"), + }) + if len(out.Rejected) != 1 { + t.Fatalf("expected path-traversal finding rejected; got %d rejected", len(out.Rejected)) + } + if out.Rejected[0].RejectionReason != ReasonFileNotFound { + t.Errorf("expected ReasonFileNotFound for path escape, got %q", out.Rejected[0].RejectionReason) + } +} + +// === highestLine extractor === + +func TestHighestLine(t *testing.T) { + cases := []struct { + hint string + want int + ok bool + }{ + {"42", 42, true}, + {"10-20", 20, true}, + {"line 100", 100, true}, + {"L42", 42, true}, + {"42:5", 42, true}, + {"", 0, false}, + {"none", 0, false}, + {"15-30-50", 50, true}, // pick the largest + } + for _, c := range cases { + got, ok := highestLine(c.hint) + if got != c.want || ok != c.ok { + t.Errorf("highestLine(%q) = (%d, %v); want (%d, %v)", c.hint, got, ok, c.want, c.ok) + } + } +} diff --git a/tests/fixtures/clean-repo/reports/latest/acceptance-gates.md b/tests/fixtures/clean-repo/reports/latest/acceptance-gates.md deleted file mode 100644 index 55a4705..0000000 --- a/tests/fixtures/clean-repo/reports/latest/acceptance-gates.md +++ /dev/null @@ -1,8 +0,0 @@ -# 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. diff --git a/tests/fixtures/clean-repo/reports/latest/claim-coverage-table.md b/tests/fixtures/clean-repo/reports/latest/claim-coverage-table.md deleted file mode 100644 index 9ec9d72..0000000 --- a/tests/fixtures/clean-repo/reports/latest/claim-coverage-table.md +++ /dev/null @@ -1,8 +0,0 @@ -# Claim Coverage Table - -Each row is a finding paired with whether existing tests cover the affected area. -Phase B emits this shape; LLM-side claim generation lands in Phase C. - -| Claim | Code Location | Existing Test | Missing Test | Risk | -|---|---|---|---|---| -| _no claims yet_ | — | — | — | — | diff --git a/tests/fixtures/clean-repo/reports/latest/llm-findings.normalized.json b/tests/fixtures/clean-repo/reports/latest/llm-findings.normalized.json deleted file mode 100644 index 7466715..0000000 --- a/tests/fixtures/clean-repo/reports/latest/llm-findings.normalized.json +++ /dev/null @@ -1,16 +0,0 @@ -{ - "generated_at": "2026-04-30T06:06:56.669606679Z", - "findings": [], - "summary": { - "total": 0, - "confirmed": 0, - "suspected": 0, - "rejected": 0, - "critical": 0, - "high": 0, - "medium": 0, - "low": 0, - "by_source": {}, - "by_check": {} - } -} diff --git a/tests/fixtures/clean-repo/reports/latest/llm-findings.raw.json b/tests/fixtures/clean-repo/reports/latest/llm-findings.raw.json deleted file mode 100644 index 7177e7b..0000000 --- a/tests/fixtures/clean-repo/reports/latest/llm-findings.raw.json +++ /dev/null @@ -1,30 +0,0 @@ -[ - { - "chunk_id": "README.md", - "findings": [], - "raw_content": "{\"findings\": []}", - "parsed": true, - "retried": false - }, - { - "chunk_id": "package.json", - "findings": [], - "raw_content": "{\"findings\": []}", - "parsed": true, - "retried": false - }, - { - "chunk_id": "src/calc.ts", - "findings": [], - "raw_content": "{\"findings\": []}", - "parsed": true, - "retried": false - }, - { - "chunk_id": "tests/calc.test.ts", - "findings": [], - "raw_content": "{\"findings\": []}", - "parsed": true, - "retried": false - } -] diff --git a/tests/fixtures/clean-repo/reports/latest/receipts.json b/tests/fixtures/clean-repo/reports/latest/receipts.json deleted file mode 100644 index 65f1286..0000000 --- a/tests/fixtures/clean-repo/reports/latest/receipts.json +++ /dev/null @@ -1,70 +0,0 @@ -{ - "run_id": "20260430T060653-57461e2c", - "repo_path": "tests/fixtures/clean-repo", - "started_at": "2026-04-30T06:06:53.880883402Z", - "finished_at": "2026-04-30T06:06:56.669797363Z", - "phases": [ - { - "name": "repo_intake", - "status": "ok", - "output_hash": "db312c5ce39315cd", - "output_files": [ - "repo-intake.json" - ] - }, - { - "name": "static_scan", - "status": "ok", - "output_hash": "837b6a5d9dc11126", - "output_files": [ - "static-findings.json" - ] - }, - { - "name": "llm_review", - "status": "ok", - "output_hash": "3939252dabe358b1", - "output_files": [ - "llm-findings.raw.json", - "llm-findings.normalized.json" - ] - }, - { - "name": "validation", - "status": "skipped", - "errors": [ - "Phase D not implemented in MVP — depends on Phase C" - ] - }, - { - "name": "report_generation", - "status": "ok", - "output_files": [ - "scrum-test.md", - "risk-register.md", - "claim-coverage-table.md", - "sprint-backlog.md", - "acceptance-gates.md" - ] - }, - { - "name": "memory_update", - "status": "skipped", - "errors": [ - "Phase E not implemented in MVP" - ] - } - ], - "summary": { - "total": 0, - "confirmed": 0, - "suspected": 0, - "rejected": 0, - "critical": 0, - "high": 0, - "medium": 0, - "low": 0, - "by_source": {}, - "by_check": {} - } -} diff --git a/tests/fixtures/clean-repo/reports/latest/repo-intake.json b/tests/fixtures/clean-repo/reports/latest/repo-intake.json deleted file mode 100644 index 095c85f..0000000 --- a/tests/fixtures/clean-repo/reports/latest/repo-intake.json +++ /dev/null @@ -1,42 +0,0 @@ -{ - "repo_path": "/home/profit/share/local-review-harness-full-md/tests/fixtures/clean-repo", - "current_branch": "main", - "latest_commit": "70d68757f78ea722bf24585c73120c09d82c4fea", - "git_status": "M ../../../internal/cli/cli.go\n M ../../../internal/cli/repo.go\n M ../../../internal/pipeline/pipeline.go\n?? ../../../internal/llm/ollama.go\n?? ../../../internal/llm/review.go\n?? ../insecure-repo/reports/", - "has_git": true, - "file_count": 4, - "language_breakdown": { - "JSON": 1, - "Markdown": 1, - "TypeScript": 2 - }, - "largest_files": [ - { - "path": "src/calc.ts", - "size": 206, - "lines": 7 - }, - { - "path": "tests/calc.test.ts", - "size": 198, - "lines": 4 - }, - { - "path": "README.md", - "size": 80, - "lines": 2 - }, - { - "path": "package.json", - "size": 43, - "lines": 1 - } - ], - "dependency_manifests": [ - "package.json" - ], - "test_manifests": [ - "tests/calc.test.ts" - ], - "generated_at": "2026-04-30T06:06:53.895008668Z" -} diff --git a/tests/fixtures/clean-repo/reports/latest/risk-register.md b/tests/fixtures/clean-repo/reports/latest/risk-register.md deleted file mode 100644 index 3ef0ee4..0000000 --- a/tests/fixtures/clean-repo/reports/latest/risk-register.md +++ /dev/null @@ -1,5 +0,0 @@ -# Risk Register - -Findings ranked by severity. `Suspected` rows haven't been validated yet (Phase D). - -_No findings._ diff --git a/tests/fixtures/clean-repo/reports/latest/scrum-test.md b/tests/fixtures/clean-repo/reports/latest/scrum-test.md deleted file mode 100644 index 14a0976..0000000 --- a/tests/fixtures/clean-repo/reports/latest/scrum-test.md +++ /dev/null @@ -1,69 +0,0 @@ -# Scrum Test — clean-repo - -**Generated:** 2026-04-30T06:06:53.895008668Z -**Branch:** main · **Commit:** 70d68757f78ea722bf24585c73120c09d82c4fea - -## Verdict - -**production-ready** — static scan + LLM review found no issues. Re-validate after every wave. - -## Evidence - -- repo path: `/home/profit/share/local-review-harness-full-md/tests/fixtures/clean-repo` -- file count: 4 -- languages: TypeScript (2), Markdown (1), JSON (1) -- dependency manifests: 1 (package.json) -- test files/dirs: 1 - -## Confirmed Risks - -_No confirmed risks at static-scan level. (LLM review may surface more.)_ - -## Suspected Risks - -_None._ - -## Blocked Checks - -_None._ - -## 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. - -**Sprint 1 — Trust Boundary Gate** - -- 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. - -## Next Commands - -- Re-run after fixes: `review-harness repo /home/profit/share/local-review-harness-full-md/tests/fixtures/clean-repo` -- Generate the full Scrum bundle: `review-harness scrum /home/profit/share/local-review-harness-full-md/tests/fixtures/clean-repo` diff --git a/tests/fixtures/clean-repo/reports/latest/sprint-backlog.md b/tests/fixtures/clean-repo/reports/latest/sprint-backlog.md deleted file mode 100644 index e17bc26..0000000 --- a/tests/fixtures/clean-repo/reports/latest/sprint-backlog.md +++ /dev/null @@ -1,26 +0,0 @@ -# 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. - -**Sprint 1 — Trust Boundary Gate** - -- 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). diff --git a/tests/fixtures/clean-repo/reports/latest/static-findings.json b/tests/fixtures/clean-repo/reports/latest/static-findings.json deleted file mode 100644 index 0c19592..0000000 --- a/tests/fixtures/clean-repo/reports/latest/static-findings.json +++ /dev/null @@ -1,16 +0,0 @@ -{ - "generated_at": "2026-04-30T06:06:53.896533109Z", - "findings": [], - "summary": { - "total": 0, - "confirmed": 0, - "suspected": 0, - "rejected": 0, - "critical": 0, - "high": 0, - "medium": 0, - "low": 0, - "by_source": {}, - "by_check": {} - } -} diff --git a/tests/fixtures/insecure-repo/reports/latest/acceptance-gates.md b/tests/fixtures/insecure-repo/reports/latest/acceptance-gates.md deleted file mode 100644 index 3e27508..0000000 --- a/tests/fixtures/insecure-repo/reports/latest/acceptance-gates.md +++ /dev/null @@ -1,9 +0,0 @@ -# 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. diff --git a/tests/fixtures/insecure-repo/reports/latest/claim-coverage-table.md b/tests/fixtures/insecure-repo/reports/latest/claim-coverage-table.md deleted file mode 100644 index 1dec4d2..0000000 --- a/tests/fixtures/insecure-repo/reports/latest/claim-coverage-table.md +++ /dev/null @@ -1,23 +0,0 @@ -# Claim Coverage Table - -Each row is a finding paired with whether existing tests cover the affected area. -Phase B emits this shape; LLM-side claim generation lands in Phase C. - -| Claim | Code Location | Existing Test | Missing Test | Risk | -|---|---|---|---|---| -| Environment file in source tree | `.env:?` | _unknown_ | _likely_ | high | -| Hardcoded absolute path | `src/handler.go:10` | _unknown_ | _likely_ | medium | -| Shell command execution | `src/handler.go:19` | _unknown_ | _likely_ | high | -| Raw SQL interpolation | `src/handler.go:14` | _unknown_ | _likely_ | high | -| Possible secret committed to source | `src/handler.go:23` | _unknown_ | _likely_ | critical | -| Possible secret committed to source | `src/handler.go:23` | _unknown_ | _likely_ | critical | -| TODO/FIXME comment | `src/handler.go:9` | _unknown_ | _likely_ | low | -| TODO/FIXME comment | `src/handler.go:22` | _unknown_ | _likely_ | low | -| Hardcoded private-network IP | `src/handler.go:11` | _unknown_ | _likely_ | medium | -| Large file | `src/huge.go:1-901` | _unknown_ | _likely_ | medium | -| Wildcard CORS | `src/server.js:2` | _unknown_ | _likely_ | high | -| Possible secret committed to source | `src/server.js:5` | _unknown_ | _likely_ | critical | -| TODO/FIXME comment | `src/server.js:1` | _unknown_ | _likely_ | low | -| Mutation route in file with no visible auth | `src/server.js:7` | _unknown_ | _likely_ | medium | -| Mutation route in file with no visible auth | `src/server.js:8` | _unknown_ | _likely_ | medium | -| No tests found | `.:?` | _unknown_ | _likely_ | medium | diff --git a/tests/fixtures/insecure-repo/reports/latest/llm-findings.normalized.json b/tests/fixtures/insecure-repo/reports/latest/llm-findings.normalized.json deleted file mode 100644 index 0830dc0..0000000 --- a/tests/fixtures/insecure-repo/reports/latest/llm-findings.normalized.json +++ /dev/null @@ -1,147 +0,0 @@ -{ - "generated_at": "2026-04-30T06:06:33.240219171Z", - "findings": [ - { - "id": "", - "title": "Hardcoded file path for secrets", - "severity": "high", - "status": "suspected", - "file": "src/handler.go", - "line_hint": "10", - "evidence": "const HARDCODED_PATH = \"/home/profit/secrets/key.pem\"", - "reason": "Hardcoding a file path for a private key exposes secrets and prevents proper secret management.", - "suggested_fix": "Move the path to an environment variable or a configuration file outside the source code.", - "source": "llm", - "confidence": 1, - "check_id": "llm.review" - }, - { - "id": "", - "title": "Hardcoded server IP address", - "severity": "medium", - "status": "suspected", - "file": "src/handler.go", - "line_hint": "11", - "evidence": "const SERVER_IP = \"192.168.1.176\"", - "reason": "Hardcoding an IP address reduces portability and may leak internal network topology.", - "suggested_fix": "Read the server IP from an environment variable.", - "source": "llm", - "confidence": 1, - "check_id": "llm.review" - }, - { - "id": "", - "title": "SQL Injection vulnerability", - "severity": "critical", - "status": "suspected", - "file": "src/handler.go", - "line_hint": "15-16", - "evidence": "q := fmt.Sprintf(\"SELECT * FROM users WHERE name = '%s'\", name)\ndb.Query(q)", - "reason": "Using string formatting to construct SQL queries directly exposes the application to SQL injection attacks.", - "suggested_fix": "Use parameterized queries with placeholders instead of string formatting.", - "source": "llm", - "confidence": 1, - "check_id": "llm.review" - }, - { - "id": "", - "title": "Unsafe shell command execution", - "severity": "critical", - "status": "suspected", - "file": "src/handler.go", - "line_hint": "19-20", - "evidence": "exec.Command(\"bash\", \"-c\", cmd).Run()", - "reason": "Executing arbitrary shell commands without validation allows for remote code execution.", - "suggested_fix": "Validate and sanitize the input command strictly, or avoid using shell execution entirely.", - "source": "llm", - "confidence": 1, - "check_id": "llm.review" - }, - { - "id": "", - "title": "Hardcoded API key", - "severity": "critical", - "status": "suspected", - "file": "src/handler.go", - "line_hint": "23", - "evidence": "const API_KEY = \"sk-1234567890abcdefABCDEFGHIJKLMNOPQRSTUV\"", - "reason": "Hardcoding an API key in source code exposes sensitive credentials to anyone with access to the repository.", - "suggested_fix": "Store the API key in a secure environment variable or secrets manager.", - "source": "llm", - "confidence": 1, - "check_id": "llm.review" - }, - { - "id": "", - "title": "CORS misconfiguration allows cross-origin attacks", - "severity": "high", - "status": "suspected", - "file": "src/server.js", - "line_hint": "2", - "evidence": "res.setHeader(\"Access-Control-Allow-Origin\", \"*\");", - "reason": "Allowing all origins (*) exposes the API to cross-site request forgery and data theft from any website.", - "suggested_fix": "Restrict Access-Control-Allow-Origin to specific trusted domains or use credentials with a specific origin.", - "source": "llm", - "confidence": 1, - "check_id": "llm.review" - }, - { - "id": "", - "title": "Hardcoded AWS access key in source code", - "severity": "critical", - "status": "suspected", - "file": "src/server.js", - "line_hint": "5", - "evidence": "const AWS_KEY = \"AKIAIOSFODNN7EXAMPLE\";", - "reason": "Hardcoded credentials in source code pose a severe security risk as they can be easily leaked and misused.", - "suggested_fix": "Use environment variables or a secure secrets manager to store AWS credentials.", - "source": "llm", - "confidence": 1, - "check_id": "llm.review" - }, - { - "id": "", - "title": "Missing authentication on user creation endpoint", - "severity": "high", - "status": "suspected", - "file": "src/server.js", - "line_hint": "7", - "evidence": "app.post(\"/api/users\", function(req, res) { /* no auth */ });", - "reason": "The /api/users endpoint lacks authentication, allowing anyone to create or modify user accounts.", - "suggested_fix": "Implement authentication middleware to verify user identity before allowing POST requests.", - "source": "llm", - "confidence": 1, - "check_id": "llm.review" - }, - { - "id": "", - "title": "Missing authentication on admin deletion endpoint", - "severity": "critical", - "status": "suspected", - "file": "src/server.js", - "line_hint": "8", - "evidence": "app.delete(\"/api/admin\", function(req, res) { /* no auth */ });", - "reason": "The /api/admin endpoint lacks authentication, allowing unauthenticated users to delete administrative resources.", - "suggested_fix": "Implement strict authentication and authorization checks for all admin endpoints.", - "source": "llm", - "confidence": 1, - "check_id": "llm.review" - } - ], - "summary": { - "total": 9, - "confirmed": 0, - "suspected": 9, - "rejected": 0, - "critical": 5, - "high": 3, - "medium": 1, - "low": 0, - "by_source": { - "llm": 9 - }, - "by_check": { - "llm.review": 9 - } - } -} diff --git a/tests/fixtures/insecure-repo/reports/latest/llm-findings.raw.json b/tests/fixtures/insecure-repo/reports/latest/llm-findings.raw.json deleted file mode 100644 index 3097c3d..0000000 --- a/tests/fixtures/insecure-repo/reports/latest/llm-findings.raw.json +++ /dev/null @@ -1,151 +0,0 @@ -[ - { - "chunk_id": "src/handler.go", - "findings": [ - { - "id": "", - "title": "Hardcoded file path for secrets", - "severity": "high", - "status": "suspected", - "file": "src/handler.go", - "line_hint": "10", - "evidence": "const HARDCODED_PATH = \"/home/profit/secrets/key.pem\"", - "reason": "Hardcoding a file path for a private key exposes secrets and prevents proper secret management.", - "suggested_fix": "Move the path to an environment variable or a configuration file outside the source code.", - "source": "llm", - "confidence": 1, - "check_id": "llm.review" - }, - { - "id": "", - "title": "Hardcoded server IP address", - "severity": "medium", - "status": "suspected", - "file": "src/handler.go", - "line_hint": "11", - "evidence": "const SERVER_IP = \"192.168.1.176\"", - "reason": "Hardcoding an IP address reduces portability and may leak internal network topology.", - "suggested_fix": "Read the server IP from an environment variable.", - "source": "llm", - "confidence": 1, - "check_id": "llm.review" - }, - { - "id": "", - "title": "SQL Injection vulnerability", - "severity": "critical", - "status": "suspected", - "file": "src/handler.go", - "line_hint": "15-16", - "evidence": "q := fmt.Sprintf(\"SELECT * FROM users WHERE name = '%s'\", name)\ndb.Query(q)", - "reason": "Using string formatting to construct SQL queries directly exposes the application to SQL injection attacks.", - "suggested_fix": "Use parameterized queries with placeholders instead of string formatting.", - "source": "llm", - "confidence": 1, - "check_id": "llm.review" - }, - { - "id": "", - "title": "Unsafe shell command execution", - "severity": "critical", - "status": "suspected", - "file": "src/handler.go", - "line_hint": "19-20", - "evidence": "exec.Command(\"bash\", \"-c\", cmd).Run()", - "reason": "Executing arbitrary shell commands without validation allows for remote code execution.", - "suggested_fix": "Validate and sanitize the input command strictly, or avoid using shell execution entirely.", - "source": "llm", - "confidence": 1, - "check_id": "llm.review" - }, - { - "id": "", - "title": "Hardcoded API key", - "severity": "critical", - "status": "suspected", - "file": "src/handler.go", - "line_hint": "23", - "evidence": "const API_KEY = \"sk-1234567890abcdefABCDEFGHIJKLMNOPQRSTUV\"", - "reason": "Hardcoding an API key in source code exposes sensitive credentials to anyone with access to the repository.", - "suggested_fix": "Store the API key in a secure environment variable or secrets manager.", - "source": "llm", - "confidence": 1, - "check_id": "llm.review" - } - ], - "raw_content": "{\n \"findings\": [\n {\n \"title\": \"Hardcoded file path for secrets\",\n \"severity\": \"high\",\n \"file\": \"src/handler.go\",\n \"line_hint\": \"10\",\n \"evidence\": \"const HARDCODED_PATH = \\\"/home/profit/secrets/key.pem\\\"\",\n \"reason\": \"Hardcoding a file path for a private key exposes secrets and prevents proper secret management.\",\n \"suggested_fix\": \"Move the path to an environment variable or a configuration file outside the source code.\",\n \"confidence\": 1.0\n },\n {\n \"title\": \"Hardcoded server IP address\",\n \"severity\": \"medium\",\n \"file\": \"src/handler.go\",\n \"line_hint\": \"11\",\n \"evidence\": \"const SERVER_IP = \\\"192.168.1.176\\\"\",\n \"reason\": \"Hardcoding an IP address reduces portability and may leak internal network topology.\",\n \"suggested_fix\": \"Read the server IP from an environment variable.\",\n \"confidence\": 1.0\n },\n {\n \"title\": \"SQL Injection vulnerability\",\n \"severity\": \"critical\",\n \"file\": \"src/handler.go\",\n \"line_hint\": \"15-16\",\n \"evidence\": \"q := fmt.Sprintf(\\\"SELECT * FROM users WHERE name = '%s'\\\", name)\\ndb.Query(q)\",\n \"reason\": \"Using string formatting to construct SQL queries directly exposes the application to SQL injection attacks.\",\n \"suggested_fix\": \"Use parameterized queries with placeholders instead of string formatting.\",\n \"confidence\": 1.0\n },\n {\n \"title\": \"Unsafe shell command execution\",\n \"severity\": \"critical\",\n \"file\": \"src/handler.go\",\n \"line_hint\": \"19-20\",\n \"evidence\": \"exec.Command(\\\"bash\\\", \\\"-c\\\", cmd).Run()\",\n \"reason\": \"Executing arbitrary shell commands without validation allows for remote code execution.\",\n \"suggested_fix\": \"Validate and sanitize the input command strictly, or avoid using shell execution entirely.\",\n \"confidence\": 1.0\n },\n {\n \"title\": \"Hardcoded API key\",\n \"severity\": \"critical\",\n \"file\": \"src/handler.go\",\n \"line_hint\": \"23\",\n \"evidence\": \"const API_KEY = \\\"sk-1234567890abcdefABCDEFGHIJKLMNOPQRSTUV\\\"\",\n \"reason\": \"Hardcoding an API key in source code exposes sensitive credentials to anyone with access to the repository.\",\n \"suggested_fix\": \"Store the API key in a secure environment variable or secrets manager.\",\n \"confidence\": 1.0\n }\n ]\n}", - "parsed": true, - "retried": false - }, - { - "chunk_id": "src/huge.go", - "findings": [], - "raw_content": "```json\n{\n \"error\": \"No valid content found. The input appears to be a list of generated line markers without any actual text or data to process.\",\n \"status\": \"empty_input\"\n}\n```", - "parsed": true, - "retried": false - }, - { - "chunk_id": "src/server.js", - "findings": [ - { - "id": "", - "title": "CORS misconfiguration allows cross-origin attacks", - "severity": "high", - "status": "suspected", - "file": "src/server.js", - "line_hint": "2", - "evidence": "res.setHeader(\"Access-Control-Allow-Origin\", \"*\");", - "reason": "Allowing all origins (*) exposes the API to cross-site request forgery and data theft from any website.", - "suggested_fix": "Restrict Access-Control-Allow-Origin to specific trusted domains or use credentials with a specific origin.", - "source": "llm", - "confidence": 1, - "check_id": "llm.review" - }, - { - "id": "", - "title": "Hardcoded AWS access key in source code", - "severity": "critical", - "status": "suspected", - "file": "src/server.js", - "line_hint": "5", - "evidence": "const AWS_KEY = \"AKIAIOSFODNN7EXAMPLE\";", - "reason": "Hardcoded credentials in source code pose a severe security risk as they can be easily leaked and misused.", - "suggested_fix": "Use environment variables or a secure secrets manager to store AWS credentials.", - "source": "llm", - "confidence": 1, - "check_id": "llm.review" - }, - { - "id": "", - "title": "Missing authentication on user creation endpoint", - "severity": "high", - "status": "suspected", - "file": "src/server.js", - "line_hint": "7", - "evidence": "app.post(\"/api/users\", function(req, res) { /* no auth */ });", - "reason": "The /api/users endpoint lacks authentication, allowing anyone to create or modify user accounts.", - "suggested_fix": "Implement authentication middleware to verify user identity before allowing POST requests.", - "source": "llm", - "confidence": 1, - "check_id": "llm.review" - }, - { - "id": "", - "title": "Missing authentication on admin deletion endpoint", - "severity": "critical", - "status": "suspected", - "file": "src/server.js", - "line_hint": "8", - "evidence": "app.delete(\"/api/admin\", function(req, res) { /* no auth */ });", - "reason": "The /api/admin endpoint lacks authentication, allowing unauthenticated users to delete administrative resources.", - "suggested_fix": "Implement strict authentication and authorization checks for all admin endpoints.", - "source": "llm", - "confidence": 1, - "check_id": "llm.review" - } - ], - "raw_content": "{\n \"findings\": [\n {\n \"title\": \"CORS misconfiguration allows cross-origin attacks\",\n \"severity\": \"high\",\n \"file\": \"src/server.js\",\n \"line_hint\": \"2\",\n \"evidence\": \"res.setHeader(\\\"Access-Control-Allow-Origin\\\", \\\"*\\\");\",\n \"reason\": \"Allowing all origins (*) exposes the API to cross-site request forgery and data theft from any website.\",\n \"suggested_fix\": \"Restrict Access-Control-Allow-Origin to specific trusted domains or use credentials with a specific origin.\",\n \"confidence\": 1.0\n },\n {\n \"title\": \"Hardcoded AWS access key in source code\",\n \"severity\": \"critical\",\n \"file\": \"src/server.js\",\n \"line_hint\": \"5\",\n \"evidence\": \"const AWS_KEY = \\\"AKIAIOSFODNN7EXAMPLE\\\";\",\n \"reason\": \"Hardcoded credentials in source code pose a severe security risk as they can be easily leaked and misused.\",\n \"suggested_fix\": \"Use environment variables or a secure secrets manager to store AWS credentials.\",\n \"confidence\": 1.0\n },\n {\n \"title\": \"Missing authentication on user creation endpoint\",\n \"severity\": \"high\",\n \"file\": \"src/server.js\",\n \"line_hint\": \"7\",\n \"evidence\": \"app.post(\\\"/api/users\\\", function(req, res) { /* no auth */ });\",\n \"reason\": \"The /api/users endpoint lacks authentication, allowing anyone to create or modify user accounts.\",\n \"suggested_fix\": \"Implement authentication middleware to verify user identity before allowing POST requests.\",\n \"confidence\": 1.0\n },\n {\n \"title\": \"Missing authentication on admin deletion endpoint\",\n \"severity\": \"critical\",\n \"file\": \"src/server.js\",\n \"line_hint\": \"8\",\n \"evidence\": \"app.delete(\\\"/api/admin\\\", function(req, res) { /* no auth */ });\",\n \"reason\": \"The /api/admin endpoint lacks authentication, allowing unauthenticated users to delete administrative resources.\",\n \"suggested_fix\": \"Implement strict authentication and authorization checks for all admin endpoints.\",\n \"confidence\": 1.0\n }\n ]\n}", - "parsed": true, - "retried": false - } -] diff --git a/tests/fixtures/insecure-repo/reports/latest/receipts.json b/tests/fixtures/insecure-repo/reports/latest/receipts.json deleted file mode 100644 index 4e513d6..0000000 --- a/tests/fixtures/insecure-repo/reports/latest/receipts.json +++ /dev/null @@ -1,82 +0,0 @@ -{ - "run_id": "20260430T060713-f513f6dc", - "repo_path": "tests/fixtures/insecure-repo", - "started_at": "2026-04-30T06:07:13.917781613Z", - "finished_at": "2026-04-30T06:07:13.953011207Z", - "phases": [ - { - "name": "repo_intake", - "status": "ok", - "output_hash": "540f222456204a27", - "output_files": [ - "repo-intake.json" - ] - }, - { - "name": "static_scan", - "status": "ok", - "output_hash": "a7aeccbda6841c1e", - "output_files": [ - "static-findings.json" - ] - }, - { - "name": "llm_review", - "status": "skipped", - "errors": [ - "LLM review not requested (pass --enable-llm to opt in)" - ] - }, - { - "name": "validation", - "status": "skipped", - "errors": [ - "Phase D not implemented in MVP — depends on Phase C" - ] - }, - { - "name": "report_generation", - "status": "ok", - "output_files": [ - "scrum-test.md", - "risk-register.md", - "claim-coverage-table.md", - "sprint-backlog.md", - "acceptance-gates.md" - ] - }, - { - "name": "memory_update", - "status": "skipped", - "errors": [ - "Phase E not implemented in MVP" - ] - } - ], - "summary": { - "total": 16, - "confirmed": 2, - "suspected": 14, - "rejected": 0, - "critical": 3, - "high": 4, - "medium": 6, - "low": 3, - "by_source": { - "static": 16 - }, - "by_check": { - "static.broad_cors": 1, - "static.env_file_committed": 1, - "static.exposed_mutation_endpoint": 2, - "static.hardcoded_local_ip": 1, - "static.hardcoded_paths": 1, - "static.large_files": 1, - "static.missing_tests": 1, - "static.raw_sql_interpolation": 1, - "static.secret_patterns": 3, - "static.shell_execution": 1, - "static.todo_comments": 3 - } - } -} diff --git a/tests/fixtures/insecure-repo/reports/latest/repo-intake.json b/tests/fixtures/insecure-repo/reports/latest/repo-intake.json deleted file mode 100644 index dee48dc..0000000 --- a/tests/fixtures/insecure-repo/reports/latest/repo-intake.json +++ /dev/null @@ -1,37 +0,0 @@ -{ - "repo_path": "/home/profit/share/local-review-harness-full-md/tests/fixtures/insecure-repo", - "current_branch": "main", - "latest_commit": "70d68757f78ea722bf24585c73120c09d82c4fea", - "git_status": "M ../../../internal/cli/cli.go\n M ../../../internal/cli/repo.go\n M ../../../internal/pipeline/pipeline.go\n?? ../../../internal/llm/ollama.go\n?? ../../../internal/llm/review.go\n?? ../clean-repo/reports/\n?? reports/", - "has_git": true, - "file_count": 4, - "language_breakdown": { - "Go": 2, - "JavaScript": 1 - }, - "largest_files": [ - { - "path": "src/huge.go", - "size": 19705, - "lines": 901 - }, - { - "path": "src/handler.go", - "size": 462, - "lines": 23 - }, - { - "path": "src/server.js", - "size": 286, - "lines": 8 - }, - { - "path": ".env", - "size": 59, - "lines": 2 - } - ], - "dependency_manifests": null, - "test_manifests": null, - "generated_at": "2026-04-30T06:07:13.931830669Z" -} diff --git a/tests/fixtures/insecure-repo/reports/latest/risk-register.md b/tests/fixtures/insecure-repo/reports/latest/risk-register.md deleted file mode 100644 index 68f6b26..0000000 --- a/tests/fixtures/insecure-repo/reports/latest/risk-register.md +++ /dev/null @@ -1,22 +0,0 @@ -# Risk Register - -Findings ranked by severity. `Suspected` rows haven't been validated yet (Phase D). - -| ID | Severity | Status | File | Line | Title | -|---|---|---|---|---|---| -| `9bc97c579efc` | critical | suspected | `src/handler.go` | 23 | Possible secret committed to source | -| `9bc97c579efc` | critical | suspected | `src/handler.go` | 23 | Possible secret committed to source | -| `d3c2c5606e1d` | critical | suspected | `src/server.js` | 5 | Possible secret committed to source | -| `750676119e4a` | high | confirmed | `.env` | — | Environment file in source tree | -| `3a198539c923` | high | suspected | `src/handler.go` | 14 | Raw SQL interpolation | -| `5bf85ae888a0` | high | suspected | `src/handler.go` | 19 | Shell command execution | -| `ef8bb39704d3` | high | suspected | `src/server.js` | 2 | Wildcard CORS | -| `4d59806aeb57` | medium | confirmed | `.` | — | No tests found | -| `eb3c41b3a186` | medium | suspected | `src/handler.go` | 10 | Hardcoded absolute path | -| `bb70e8e262d6` | medium | suspected | `src/handler.go` | 11 | Hardcoded private-network IP | -| `512b795dc551` | medium | suspected | `src/huge.go` | 1-901 | Large file | -| `7ed1cab08825` | medium | suspected | `src/server.js` | 7 | Mutation route in file with no visible auth | -| `2b765c240c96` | medium | suspected | `src/server.js` | 8 | Mutation route in file with no visible auth | -| `f99cd5bb5f2c` | low | suspected | `src/handler.go` | 22 | TODO/FIXME comment | -| `f3e510b70ec9` | low | suspected | `src/handler.go` | 9 | TODO/FIXME comment | -| `4a631055edd1` | low | suspected | `src/server.js` | 1 | TODO/FIXME comment | diff --git a/tests/fixtures/insecure-repo/reports/latest/scrum-test.md b/tests/fixtures/insecure-repo/reports/latest/scrum-test.md deleted file mode 100644 index 0934b7d..0000000 --- a/tests/fixtures/insecure-repo/reports/latest/scrum-test.md +++ /dev/null @@ -1,96 +0,0 @@ -# Scrum Test — insecure-repo - -**Generated:** 2026-04-30T06:07:13.931830669Z -**Branch:** main · **Commit:** 70d68757f78ea722bf24585c73120c09d82c4fea - -## 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/tests/fixtures/insecure-repo` -- file count: 4 -- languages: Go (2), JavaScript (1) -- dependency manifests: 0 () -- test files/dirs: 0 -- LLM review: **skipped** (Phase C not implemented OR provider unavailable; see model-doctor.json) - -## Confirmed Risks - -| Severity | File:Line | Title | Evidence | -|---|---|---|---| -| high | `.env` | Environment file in source tree | `filename=.env` | -| medium | `.` | No tests found | `No test files or test directories detected (looked for *_test.go, *.test.{js,ts}, test_*.py, tests/, spec/)` | - -## Suspected Risks - -Each entry is a static-scan regex hit awaiting validation (Phase D / LLM cross-check). - -| Severity | File:Line | Title | Evidence | -|---|---|---|---| -| critical | `src/handler.go:23` | Possible secret committed to source | `const API_KEY = "sk-1234567890abcdefABCDEFGHIJKLMNOPQRSTUV"` | -| critical | `src/handler.go:23` | Possible secret committed to source | `const API_KEY = "sk-1234567890abcdefABCDEFGHIJKLMNOPQRSTUV"` | -| critical | `src/server.js:5` | Possible secret committed to source | `const AWS_KEY = "AKIAIOSFODNN7EXAMPLE";` | -| high | `src/handler.go:14` | Raw SQL interpolation | `q := fmt.Sprintf("SELECT * FROM users WHERE name = '%s'", name)` | -| high | `src/handler.go:19` | Shell command execution | `exec.Command("bash", "-c", cmd).Run()` | -| high | `src/server.js:2` | Wildcard CORS | `res.setHeader("Access-Control-Allow-Origin", "*");` | -| medium | `src/handler.go:10` | Hardcoded absolute path | `const HARDCODED_PATH = "/home/profit/secrets/key.pem"` | -| medium | `src/handler.go:11` | Hardcoded private-network IP | `const SERVER_IP = "192.168.1.176"` | -| medium | `src/huge.go:1-901` | Large file | `901 lines (limit: 800)` | -| medium | `src/server.js:7` | Mutation route in file with no visible auth | `app.post("/api/users", function(req, res) { /* no auth */ });` | -| medium | `src/server.js:8` | Mutation route in file with no visible auth | `app.delete("/api/admin", function(req, res) { /* no auth */ });` | -| low | `src/handler.go:22` | TODO/FIXME comment | `// FIXME: hardcoded creds` | -| low | `src/handler.go:9` | TODO/FIXME comment | `// TODO: rotate this and move to env` | -| low | `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 16 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/tests/fixtures/insecure-repo` -- Generate the full Scrum bundle: `review-harness scrum /home/profit/share/local-review-harness-full-md/tests/fixtures/insecure-repo` diff --git a/tests/fixtures/insecure-repo/reports/latest/sprint-backlog.md b/tests/fixtures/insecure-repo/reports/latest/sprint-backlog.md deleted file mode 100644 index c25419c..0000000 --- a/tests/fixtures/insecure-repo/reports/latest/sprint-backlog.md +++ /dev/null @@ -1,28 +0,0 @@ -# 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 16 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). diff --git a/tests/fixtures/insecure-repo/reports/latest/static-findings.json b/tests/fixtures/insecure-repo/reports/latest/static-findings.json deleted file mode 100644 index 7900dbf..0000000 --- a/tests/fixtures/insecure-repo/reports/latest/static-findings.json +++ /dev/null @@ -1,242 +0,0 @@ -{ - "generated_at": "2026-04-30T06:07:13.951970576Z", - "findings": [ - { - "id": "750676119e4a", - "title": "Environment file in source tree", - "severity": "high", - "status": "confirmed", - "file": ".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": "eb3c41b3a186", - "title": "Hardcoded absolute path", - "severity": "medium", - "status": "suspected", - "file": "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": "5bf85ae888a0", - "title": "Shell command execution", - "severity": "high", - "status": "suspected", - "file": "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": "3a198539c923", - "title": "Raw SQL interpolation", - "severity": "high", - "status": "suspected", - "file": "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": "9bc97c579efc", - "title": "Possible secret committed to source", - "severity": "critical", - "status": "suspected", - "file": "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": "9bc97c579efc", - "title": "Possible secret committed to source", - "severity": "critical", - "status": "suspected", - "file": "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": "f3e510b70ec9", - "title": "TODO/FIXME comment", - "severity": "low", - "status": "suspected", - "file": "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": "f99cd5bb5f2c", - "title": "TODO/FIXME comment", - "severity": "low", - "status": "suspected", - "file": "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": "bb70e8e262d6", - "title": "Hardcoded private-network IP", - "severity": "medium", - "status": "suspected", - "file": "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": "512b795dc551", - "title": "Large file", - "severity": "medium", - "status": "suspected", - "file": "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": "ef8bb39704d3", - "title": "Wildcard CORS", - "severity": "high", - "status": "suspected", - "file": "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": "d3c2c5606e1d", - "title": "Possible secret committed to source", - "severity": "critical", - "status": "suspected", - "file": "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": "4a631055edd1", - "title": "TODO/FIXME comment", - "severity": "low", - "status": "suspected", - "file": "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": "7ed1cab08825", - "title": "Mutation route in file with no visible auth", - "severity": "medium", - "status": "suspected", - "file": "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": "2b765c240c96", - "title": "Mutation route in file with no visible auth", - "severity": "medium", - "status": "suspected", - "file": "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" - }, - { - "id": "4d59806aeb57", - "title": "No tests found", - "severity": "medium", - "status": "confirmed", - "file": ".", - "evidence": "No test files or test directories detected (looked for *_test.go, *.test.{js,ts}, test_*.py, tests/, spec/)", - "reason": "Repository has source code but no test surface. Refactoring or extending without test cover is high-risk.", - "source": "static", - "confidence": 0.95, - "check_id": "static.missing_tests" - } - ], - "summary": { - "total": 16, - "confirmed": 2, - "suspected": 14, - "rejected": 0, - "critical": 3, - "high": 4, - "medium": 6, - "low": 3, - "by_source": { - "static": 16 - }, - "by_check": { - "static.broad_cors": 1, - "static.env_file_committed": 1, - "static.exposed_mutation_endpoint": 2, - "static.hardcoded_local_ip": 1, - "static.hardcoded_paths": 1, - "static.large_files": 1, - "static.missing_tests": 1, - "static.raw_sql_interpolation": 1, - "static.secret_patterns": 3, - "static.shell_execution": 1, - "static.todo_comments": 3 - } - } -}