diff --git a/internal/app/batch_test.go b/internal/app/batch_test.go index 0614882..e77f97f 100644 --- a/internal/app/batch_test.go +++ b/internal/app/batch_test.go @@ -5,6 +5,7 @@ import ( "encoding/json" "errors" "os" + "path/filepath" "strings" "testing" @@ -13,10 +14,18 @@ import ( ) // writeTempLogFile creates a temporary log file with the given content and -// returns its path. Cleaned up automatically via t.TempDir. +// returns its path. func writeTempLogFile(t *testing.T, content string) string { t.Helper() - dir := t.TempDir() + // Batch validates source paths relative to the current working directory, so + // test fixtures are created under "." to stay within the allowed root. + dir, err := os.MkdirTemp(".", "batch-test-*") + if err != nil { + t.Fatalf("create temp dir: %v", err) + } + t.Cleanup(func() { + _ = os.RemoveAll(dir) + }) f, err := os.CreateTemp(dir, "batch-*.log") if err != nil { t.Fatalf("create temp log: %v", err) @@ -414,3 +423,72 @@ func TestBatchMarkdownSourceListTruncatedAboveThree(t *testing.T) { t.Errorf("expected '+1 more' truncation in markdown output, got %q", out) } } + +func TestResolvePathWithinRootAllowsPathInRoot(t *testing.T) { + root := t.TempDir() + got, err := resolvePathWithinRoot("logs/build.log", root) + if err != nil { + t.Fatalf("resolvePathWithinRoot returned error: %v", err) + } + want := filepath.Join(root, "logs", "build.log") + if got != want { + t.Fatalf("resolvePathWithinRoot path = %q, want %q", got, want) + } +} + +func TestResolvePathWithinRootRejectsPathTraversal(t *testing.T) { + root := t.TempDir() + _, err := resolvePathWithinRoot("../outside.log", root) + if err == nil { + t.Fatal("expected path traversal error, got nil") + } + if !strings.Contains(err.Error(), "escapes allowed root") { + t.Fatalf("expected path escape error, got %v", err) + } +} + +func TestResolvePathWithinRootAllowsAbsolutePathInRoot(t *testing.T) { + root := t.TempDir() + inside := filepath.Join(root, "inside.log") + got, err := resolvePathWithinRoot(inside, root) + if err != nil { + t.Fatalf("resolvePathWithinRoot returned error: %v", err) + } + if got != inside { + t.Fatalf("resolvePathWithinRoot path = %q, want %q", got, inside) + } +} + +func TestResolvePathWithinRootRejectsAbsolutePathOutsideRoot(t *testing.T) { + root := t.TempDir() + outside := filepath.Join(filepath.Dir(root), "outside.log") + _, err := resolvePathWithinRoot(outside, root) + if err == nil { + t.Fatal("expected path traversal error, got nil") + } + if !strings.Contains(err.Error(), "escapes allowed root") { + t.Fatalf("expected path escape error, got %v", err) + } +} + +func TestResolvePathWithinRootRejectsSymlinkEscape(t *testing.T) { + root := t.TempDir() + outsideDir := t.TempDir() + outsideTarget := filepath.Join(outsideDir, "outside.log") + if err := os.WriteFile(outsideTarget, []byte("outside"), 0o644); err != nil { + t.Fatalf("write outside target: %v", err) + } + + linkPath := filepath.Join(root, "linked.log") + if err := os.Symlink(outsideTarget, linkPath); err != nil { + t.Fatalf("create symlink: %v", err) + } + + _, err := resolvePathWithinRoot(linkPath, root) + if err == nil { + t.Fatal("expected symlink escape error, got nil") + } + if !strings.Contains(err.Error(), "escapes allowed root") { + t.Fatalf("expected path escape error, got %v", err) + } +} diff --git a/internal/app/service.go b/internal/app/service.go index 4877acc..4cfed64 100644 --- a/internal/app/service.go +++ b/internal/app/service.go @@ -31,6 +31,14 @@ import ( // Service owns app-level orchestration for CLI commands. type Service struct{} +const ( + defaultFixtureBaselineMinTop1 = 0.65 + defaultFixtureBaselineMinTop3 = 0.85 + defaultFixtureBaselineMaxUnmatched = 0.15 + defaultFixtureBaselineMaxFalsePositive = 0.35 + defaultFixtureBaselineMaxWeakMatch = 0.15 +) + var ErrGuardFindings = errors.New("guard findings emitted") // ErrSilentFailure is returned by Analyze when --fail-on-silent is set and a @@ -360,7 +368,8 @@ func analyzeLog(r io.Reader, source string, opts AnalyzeOptions, surface string, a.Source = source } if a != nil || errors.Is(err, engine.ErrNoMatch) { - a, prepErr := prepareAnalysisWithStore(a, string(data), "log", surface, opts, persist) + var prepErr error + a, prepErr = prepareAnalysisWithStore(a, string(data), "log", surface, opts, persist) if prepErr != nil { return nil, prepErr } @@ -581,7 +590,13 @@ func (Service) FixturesStats(root string, class fixtures.Class, opts fixtures.Ev report.AppliedBaselinePath = baselinePath } if updateBaseline { - thresholds := fixtures.Thresholds{MinTop1: 0.65, MinTop3: 0.85, MaxUnmatched: 0.15, MaxFalsePositive: 0.35, MaxWeakMatch: 0.15} + thresholds := fixtures.Thresholds{ + MinTop1: defaultFixtureBaselineMinTop1, + MinTop3: defaultFixtureBaselineMinTop3, + MaxUnmatched: defaultFixtureBaselineMaxUnmatched, + MaxFalsePositive: defaultFixtureBaselineMaxFalsePositive, + MaxWeakMatch: defaultFixtureBaselineMaxWeakMatch, + } if err := fixtures.WriteBaseline(baselinePath, report.Baseline(thresholds)); err != nil { return err } @@ -621,8 +636,18 @@ func (Service) Batch(sources []string, opts AnalyzeOptions, w io.Writer) error { } patternMap := map[string]*model.BatchPattern{} + rootAbs, err := os.Getwd() + if err != nil { + return fmt.Errorf("resolve working directory: %w", err) + } + for _, src := range sources { - f, err := os.Open(src) // #nosec G304 -- src comes from CLI args, not untrusted input + validatedPath, err := resolvePathWithinRoot(src, rootAbs) + if err != nil { + return fmt.Errorf("invalid source path %s: %w", src, err) + } + + f, err := os.Open(validatedPath) if err != nil { return fmt.Errorf("open %s: %w", src, err) } @@ -703,6 +728,81 @@ func (Service) Batch(sources []string, opts AnalyzeOptions, w io.Writer) error { return nil } +// resolvePathWithinRoot normalizes src and ensures the resulting absolute path +// remains inside rootAbs. It rejects empty paths and traversal attempts (for +// both relative and absolute inputs) that escape the allowed root. +func resolvePathWithinRoot(src, rootAbs string) (string, error) { + if src == "" { + return "", errors.New("path is empty") + } + + cleaned := filepath.Clean(src) + var candidate string + if filepath.IsAbs(cleaned) { + candidate = cleaned + } else { + candidate = filepath.Join(rootAbs, cleaned) + } + + candidateAbs, err := filepath.Abs(candidate) + if err != nil { + return "", fmt.Errorf("resolve absolute path: %w", err) + } + + rootEval, err := filepath.EvalSymlinks(rootAbs) + if err != nil { + return "", fmt.Errorf("resolve root symlinks: %w", err) + } + candidateEval, err := resolvePathForContainment(candidateAbs) + if err != nil { + return "", fmt.Errorf("resolve candidate symlinks: %w", err) + } + + rel, err := filepath.Rel(rootEval, candidateEval) + if err != nil { + return "", fmt.Errorf("compute relative path: %w", err) + } + if rel == ".." || strings.HasPrefix(rel, ".."+string(os.PathSeparator)) { + return "", fmt.Errorf("path escapes allowed root") + } + + return candidateAbs, nil +} + +func resolvePathForContainment(candidateAbs string) (string, error) { + candidateEval, err := filepath.EvalSymlinks(candidateAbs) + if err == nil { + return candidateEval, nil + } + if !errors.Is(err, os.ErrNotExist) { + return "", err + } + + ancestor := filepath.Dir(candidateAbs) + for { + if _, statErr := os.Lstat(ancestor); statErr == nil { + ancestorEval, evalErr := filepath.EvalSymlinks(ancestor) + if evalErr != nil { + return "", evalErr + } + tail, relErr := filepath.Rel(ancestor, candidateAbs) + if relErr != nil { + return "", relErr + } + return filepath.Join(ancestorEval, tail), nil + } else if !errors.Is(statErr, os.ErrNotExist) { + return "", statErr + } + + next := filepath.Dir(ancestor) + // filepath.Dir returns the same value at filesystem root. + if next == ancestor { + return candidateAbs, nil + } + ancestor = next + } +} + func formatBatchText(r *model.BatchResult) string { var b strings.Builder fileWord := "files" diff --git a/internal/repo/signals_test.go b/internal/repo/signals_test.go index 6d70ee1..ce95d3d 100644 --- a/internal/repo/signals_test.go +++ b/internal/repo/signals_test.go @@ -1,6 +1,7 @@ package repo import ( + "fmt" "testing" ) @@ -114,7 +115,7 @@ func TestDeriveSignals_largeCommit(t *testing.T) { // Build a commit with LargeCommitFileThreshold files. largeFiles := make([]string, LargeCommitFileThreshold) for i := range largeFiles { - largeFiles[i] = "src/file" + string(rune('a'+i)) + ".go" + largeFiles[i] = fmt.Sprintf("src/file%d.go", i) } commits := []Commit{ {Hash: "aaa1111", Subject: "chore: big refactor", Files: largeFiles}, @@ -271,7 +272,7 @@ func TestDeriveSignals_topAuthorsLimit(t *testing.T) { } commits := make([]Commit, len(authors)) for i, a := range authors { - commits[i] = Commit{Hash: "h" + string(rune('0'+i)), Subject: "change", Author: a, Files: []string{"file.go"}} + commits[i] = Commit{Hash: fmt.Sprintf("h%d", i), Subject: "change", Author: a, Files: []string{"file.go"}} } sigs := DeriveSignals(commits) @@ -316,7 +317,7 @@ func TestCorrelate_newSignalFields(t *testing.T) { // Build commits that trigger all three new context fields. largeFiles := make([]string, LargeCommitFileThreshold) for i := range largeFiles { - largeFiles[i] = "src/module" + string(rune('a'+i)) + ".go" + largeFiles[i] = fmt.Sprintf("src/module%d.go", i) } commits := []Commit{ { diff --git a/internal/store/store_helpers_test.go b/internal/store/store_helpers_test.go index 6547d63..6db1679 100644 --- a/internal/store/store_helpers_test.go +++ b/internal/store/store_helpers_test.go @@ -21,8 +21,9 @@ func TestNullableBoolNilReturnsNil(t *testing.T) { func TestNullableBoolTrueReturnsOne(t *testing.T) { v := true got := nullableBool(&v) - if got != 1 { - t.Errorf("expected 1, got %v", got) + want := 1 + if got != want { + t.Errorf("expected %v, got %v", want, got) } } diff --git a/llms.txt b/llms.txt index 151fdc9..57e1baf 100644 --- a/llms.txt +++ b/llms.txt @@ -22,8 +22,8 @@ Coding agents: read `AGENTS.md` and `SYSTEM.md` before making changes. Run `make ## Failure catalog -- [docs/failures/llms.txt](docs/failures/llms.txt): Machine-readable index of all 173 failure playbooks, grouped by category, with one-line descriptions and direct links to each page -- [docs/failures/catalog/README.md](docs/failures/catalog/README.md): Human-readable catalog index — all 173 playbooks with diagnosis and fix links +- [docs/failures/llms.txt](docs/failures/llms.txt): Machine-readable index of all failure playbooks, grouped by category, with one-line descriptions and direct links to each page +- [docs/failures/catalog/README.md](docs/failures/catalog/README.md): Human-readable catalog index of bundled playbooks with diagnosis and fix links ## Optional diff --git a/scripts/cli-smoke.sh b/scripts/cli-smoke.sh index 6b5b5e4..f45eb5e 100755 --- a/scripts/cli-smoke.sh +++ b/scripts/cli-smoke.sh @@ -5,6 +5,7 @@ set -eu ROOT_DIR="${ROOT_DIR:-$(CDPATH= cd -- "$(dirname "$0")/.." && pwd)}" BINARY="${BINARY:-$ROOT_DIR/bin/faultline}" PLAYBOOK_DIR="${FAULTLINE_PLAYBOOK_DIR:-$ROOT_DIR/playbooks/bundled}" +STARTER_PLAYBOOK_COUNT="${STARTER_PLAYBOOK_COUNT:-173}" TMP_DIR="$(mktemp -d)" cleanup() { @@ -63,7 +64,9 @@ run_compare "runtime-mismatch.expected.md" "$ROOT_DIR/examples/runtime-mismatch. cat "$ROOT_DIR/examples/missing-executable.log" | \ FAULTLINE_PLAYBOOK_DIR="$PLAYBOOK_DIR" "$BINARY" analyze --json --no-history --git=false >"$TMP_DIR/missing.analysis.json" -grep -F '"pack_provenance":[{"name":"starter","playbook_count":173}]' "$TMP_DIR/missing.analysis.json" >/dev/null +jq -e --argjson expected_count "$STARTER_PLAYBOOK_COUNT" \ + '.pack_provenance | length == 1 and .[0].name == "starter" and .[0].playbook_count == $expected_count' \ + "$TMP_DIR/missing.analysis.json" >/dev/null cat "$ROOT_DIR/examples/runtime-mismatch.log" | \ FAULTLINE_PLAYBOOK_DIR="$PLAYBOOK_DIR" "$BINARY" analyze --json --no-history --git=false >"$TMP_DIR/runtime.analysis.json" @@ -152,7 +155,9 @@ HOME="$PACK_HOME" FAULTLINE_PLAYBOOK_DIR="$PLAYBOOK_DIR" "$BINARY" packs install printf '%s\n' "example cache prime missing" | \ HOME="$PACK_HOME" FAULTLINE_PLAYBOOK_DIR="$PLAYBOOK_DIR" "$BINARY" analyze --json --no-history --git=false >"$TMP_DIR/extra-pack.analysis.json" grep -F '"failure_id":"example-cache-prime-missing"' "$TMP_DIR/extra-pack.analysis.json" >/dev/null -grep -F '"pack_provenance":[{"name":"starter","playbook_count":173},{"name":"example-pack","version":"0.0.0+local"' "$TMP_DIR/extra-pack.analysis.json" >/dev/null +jq -e --argjson expected_count "$STARTER_PLAYBOOK_COUNT" \ + '.pack_provenance | length == 2 and .[0].name == "starter" and .[0].playbook_count == $expected_count and .[1].name == "example-pack" and .[1].version == "0.0.0+local"' \ + "$TMP_DIR/extra-pack.analysis.json" >/dev/null FAULTLINE_PLAYBOOK_DIR="$PLAYBOOK_DIR" "$BINARY" coverage >"$TMP_DIR/coverage.txt" grep -F "Playbook coverage report" "$TMP_DIR/coverage.txt" >/dev/null