review-bot/cmd/review-bot/validatedocmap_test.go

package main

import (
	"bytes"
	"os"
	"path/filepath"
	"strings"
	"testing"
)

// makeDocmapYAML writes a YAML string to a temp file and returns its path.
// The file is created in t.TempDir() — use makeDocmapInDir when the docmap
// must be located inside a specific repo-root directory.
func makeDocmapYAML(t *testing.T, content string) string {
	t.Helper()
	f, err := os.CreateTemp(t.TempDir(), "doc-map-*.yml")
	if err != nil {
		t.Fatalf("CreateTemp: %v", err)
	}
	defer f.Close()
	if _, err := f.WriteString(content); err != nil {
		t.Fatalf("WriteString: %v", err)
	}
	return f.Name()
}

// makeDocmapInDir writes a YAML string to a file inside dir and returns the
// file path. Use this instead of makeDocmapYAML when also passing --repo-root,
// because validateDocmapPath requires the docmap to be within the repo root.
func makeDocmapInDir(t *testing.T, dir, content string) string {
	t.Helper()
	if err := os.MkdirAll(filepath.Join(dir, ".review-bot"), 0o755); err != nil {
		t.Fatalf("MkdirAll: %v", err)
	}
	path := filepath.Join(dir, ".review-bot", "doc-map.yml")
	if err := os.WriteFile(path, []byte(content), 0o644); err != nil {
		t.Fatalf("WriteFile: %v", err)
	}
	return path
}

// makeDocFile creates a file (and any parent dirs) at the given path relative to dir.
func makeDocFile(t *testing.T, dir, rel string) {
	t.Helper()
	full := filepath.Join(dir, rel)
	if err := os.MkdirAll(filepath.Dir(full), 0o755); err != nil {
		t.Fatalf("MkdirAll: %v", err)
	}
	if err := os.WriteFile(full, []byte("# doc\n"), 0o644); err != nil {
		t.Fatalf("WriteFile: %v", err)
	}
}

// captureOutput redirects outWriter/errWriter to buffers for the duration of f.
func captureOutput(f func()) (stdout, stderr string) {
	var outBuf, errBuf bytes.Buffer
	origOut, origErr := outWriter, errWriter
	outWriter = &outBuf
	errWriter = &errBuf
	defer func() {
		outWriter = origOut
		errWriter = origErr
	}()
	f()
	return outBuf.String(), errBuf.String()
}

func TestRunValidateDocmap_Clean(t *testing.T) {
	dir := t.TempDir()
	makeDocFile(t, dir, "docs/foo.md")

	docmap := makeDocmapInDir(t, dir, `
mappings:
  - paths:
      - "lib/foo/**"
    docs:
      - docs/foo.md
`)

	// A covered file with all docs existing → clean.
	code, stdout, _ := stdinValidateDocmap(t,
		"lib/foo/bar.ex\n",
		[]string{"--docmap", docmap, "--repo-root", dir},
	)
	if code != 0 {
		t.Errorf("expected exit 0 for clean, got %d", code)
	}
	if !strings.Contains(stdout, "OK") {
		t.Errorf("expected 'OK' in stdout, got %q", stdout)
	}
}

func TestRunValidateDocmap_MissingDocmapFlag(t *testing.T) {
	var code int
	_, stderr := captureOutput(func() {
		code = runValidateDocmap([]string{})
	})
	if code != 2 {
		t.Errorf("expected exit 2 for missing --docmap, got %d", code)
	}
	if !strings.Contains(stderr, "--docmap") {
		t.Errorf("expected --docmap in stderr, got %q", stderr)
	}
}

func TestRunValidateDocmap_BadYAML(t *testing.T) {
	dir := t.TempDir()
	docmap := makeDocmapInDir(t, dir, "mappings: [{{invalid")
	var code int
	_, stderr := captureOutput(func() {
		code = runValidateDocmap([]string{"--docmap", docmap, "--repo-root", dir})
	})
	if code != 2 {
		t.Errorf("expected exit 2 for bad YAML, got %d", code)
	}
	if !strings.Contains(stderr, "failed to parse") {
		t.Errorf("expected parse error in stderr, got %q", stderr)
	}
}

func TestRunValidateDocmap_StaleDocs(t *testing.T) {
	dir := t.TempDir()
	// docs/foo.md does NOT exist on disk.

	docmap := makeDocmapInDir(t, dir, `
mappings:
  - paths:
      - "lib/foo/**"
    docs:
      - docs/foo.md
`)

	var code int
	_, stderr := captureOutput(func() {
		code = runValidateDocmap([]string{
			"--docmap", docmap,
			"--repo-root", dir,
		})
	})
	if code != 1 {
		t.Errorf("expected exit 1 for stale docs, got %d", code)
	}
	if !strings.Contains(stderr, "docs/foo.md") {
		t.Errorf("expected stale path in stderr, got %q", stderr)
	}
	if !strings.Contains(stderr, "stale docmap") {
		t.Errorf("expected 'stale docmap' in stderr, got %q", stderr)
	}
}

// stdinValidateDocmap runs runValidateDocmap with a synthetic stdin.
//
// Implementation note: we write stdinContent to a temp file and point
// os.Stdin at it. The defer f.Close() fires after stdinValidateDocmap
// returns, which is after runValidateDocmap has finished reading stdin
// synchronously — so the file is not closed while still in use.
// Tests must not call t.Parallel() while sharing the global os.Stdin.
func stdinValidateDocmap(t *testing.T, stdinContent string, args []string) (code int, stdout, stderr string) {
	t.Helper()
	// Write stdin content to a temp file and redirect os.Stdin.
	f, err := os.CreateTemp(t.TempDir(), "stdin-*")
	if err != nil {
		t.Fatalf("CreateTemp for stdin: %v", err)
	}
	defer f.Close()
	if _, err := f.WriteString(stdinContent); err != nil {
		t.Fatalf("WriteString for stdin: %v", err)
	}
	if _, err := f.Seek(0, 0); err != nil {
		t.Fatalf("Seek for stdin: %v", err)
	}

	origStdin := os.Stdin
	os.Stdin = f
	defer func() { os.Stdin = origStdin }()

	stdout, stderr = captureOutput(func() {
		code = runValidateDocmap(args)
	})
	return
}

func TestRunValidateDocmap_UncoveredFile(t *testing.T) {
	dir := t.TempDir()
	makeDocFile(t, dir, "docs/foo.md")

	docmap := makeDocmapInDir(t, dir, `
mappings:
  - paths:
      - "lib/foo/**"
    docs:
      - docs/foo.md
`)

	code, _, stderr := stdinValidateDocmap(t,
		"lib/bar/uncovered.ex\n",
		[]string{"--docmap", docmap, "--repo-root", dir},
	)
	if code != 1 {
		t.Errorf("expected exit 1 for uncovered file, got %d", code)
	}
	if !strings.Contains(stderr, "lib/bar/uncovered.ex") {
		t.Errorf("expected uncovered file in stderr, got %q", stderr)
	}
	if !strings.Contains(stderr, "no docmap coverage") {
		t.Errorf("expected 'no docmap coverage' in stderr, got %q", stderr)
	}
}

func TestRunValidateDocmap_BothFailures(t *testing.T) {
	dir := t.TempDir()
	// docs/foo.md intentionally missing

	docmap := makeDocmapInDir(t, dir, `
mappings:
  - paths:
      - "lib/foo/**"
    docs:
      - docs/foo.md
`)

	code, _, stderr := stdinValidateDocmap(t,
		"lib/bar/uncovered.ex\n",
		[]string{"--docmap", docmap, "--repo-root", dir},
	)
	if code != 1 {
		t.Errorf("expected exit 1 for both failures, got %d", code)
	}
	if !strings.Contains(stderr, "no docmap coverage") {
		t.Errorf("expected coverage error in stderr, got %q", stderr)
	}
	if !strings.Contains(stderr, "stale docmap") {
		t.Errorf("expected stale-docs error in stderr, got %q", stderr)
	}
}

func TestRunValidateDocmap_EmptyStdin(t *testing.T) {
	dir := t.TempDir()
	makeDocFile(t, dir, "docs/foo.md")

	docmap := makeDocmapInDir(t, dir, `
mappings:
  - paths:
      - "lib/foo/**"
    docs:
      - docs/foo.md
`)

	code, stdout, _ := stdinValidateDocmap(t,
		"",
		[]string{"--docmap", docmap, "--repo-root", dir},
	)
	if code != 0 {
		t.Errorf("expected exit 0 for empty stdin, got %d", code)
	}
	if !strings.Contains(stdout, "OK") {
		t.Errorf("expected 'OK' in stdout, got %q", stdout)
	}
}

func TestRunValidateDocmap_BlankLinesSkipped(t *testing.T) {
	dir := t.TempDir()
	makeDocFile(t, dir, "docs/foo.md")

	docmap := makeDocmapInDir(t, dir, `
mappings:
  - paths:
      - "lib/foo/**"
    docs:
      - docs/foo.md
`)

	// stdin with only blank lines → effectively empty, should be clean
	code, stdout, _ := stdinValidateDocmap(t,
		"\n  \n\n",
		[]string{"--docmap", docmap, "--repo-root", dir},
	)
	if code != 0 {
		t.Errorf("expected exit 0 for blank-only stdin, got %d", code)
	}
	if !strings.Contains(stdout, "OK") {
		t.Errorf("expected 'OK' in stdout for blank-only stdin, got %q", stdout)
	}
}

func TestRunValidateDocmap_DuplicateDocsDeduped(t *testing.T) {
	dir := t.TempDir()
	// docs/shared.md intentionally missing — but it appears in TWO mappings.
	// Should appear only once in stale list.

	docmap := makeDocmapInDir(t, dir, `
mappings:
  - paths:
      - "lib/foo/**"
    docs:
      - docs/shared.md
  - paths:
      - "lib/bar/**"
    docs:
      - docs/shared.md
`)

	code, _, stderr := stdinValidateDocmap(t,
		"",
		[]string{"--docmap", docmap, "--repo-root", dir},
	)
	if code != 1 {
		t.Errorf("expected exit 1 for stale doc, got %d", code)
	}
	count := strings.Count(stderr, "docs/shared.md")
	if count != 1 {
		t.Errorf("expected docs/shared.md to appear exactly once in stderr (deduplicated), got %d occurrences: %q", count, stderr)
	}
}

// TestCheckStaleDocs_PathTraversal verifies that checkStaleDocs rejects
// traversal and absolute paths without touching the host filesystem.
func TestCheckStaleDocs_PathTraversal(t *testing.T) {
	dir := t.TempDir()

	// Baseline: a valid doc that exists.
	makeDocFile(t, dir, "docs/valid.md")

	tests := []struct {
		name      string
		docPath   string
		wantStale bool
	}{
		{"dot-dot traversal", "../../etc/passwd", true},
		{"dot-dot single", "../outside", true},
		{"absolute path", "/etc/passwd", true},
		{"valid present path", "docs/valid.md", false},
		{"valid missing path", "docs/missing.md", true},
	}

	for _, tc := range tests {
		t.Run(tc.name, func(t *testing.T) {
			docmap := makeDocmapInDir(t, dir, `
mappings:
  - paths:
      - "lib/**"
    docs:
      - `+tc.docPath+`
`)
			code, _, stderr := stdinValidateDocmap(t,
				"",
				[]string{"--docmap", docmap, "--repo-root", dir},
			)

			if tc.wantStale {
				if code != 1 {
					t.Errorf("path %q: expected exit 1 (stale/invalid), got %d; stderr: %q", tc.docPath, code, stderr)
				}
			} else {
				if code != 0 {
					t.Errorf("path %q: expected exit 0 (valid), got %d; stderr: %q", tc.docPath, code, stderr)
				}
			}
		})
	}
}

// TestCheckStaleDocs_SymlinkOutside verifies that a symlink under repoRoot
// pointing outside the repo is treated as stale (not followed).
func TestCheckStaleDocs_SymlinkOutside(t *testing.T) {
	dir := t.TempDir()

	// Create a symlink inside repoRoot pointing to a file outside the repo.
	// We point at /etc/hostname (exists on Linux CI) but the test does not
	// depend on that file existing — Lstat must reject the symlink itself.
	linkPath := filepath.Join(dir, "docs", "secret.md")
	if err := os.MkdirAll(filepath.Dir(linkPath), 0o755); err != nil {
		t.Fatalf("MkdirAll: %v", err)
	}
	if err := os.Symlink("/etc/hostname", linkPath); err != nil {
		t.Fatalf("Symlink: %v", err)
	}

	docmap := makeDocmapInDir(t, dir, `
mappings:
  - paths:
      - "lib/**"
    docs:
      - docs/secret.md
`)

	code, _, stderr := stdinValidateDocmap(t,
		"",
		[]string{"--docmap", docmap, "--repo-root", dir},
	)
	if code != 1 {
		t.Errorf("expected exit 1 for symlink doc, got %d; stderr: %q", code, stderr)
	}
	if !strings.Contains(stderr, "docs/secret.md") {
		t.Errorf("expected stale path in stderr, got %q", stderr)
	}
}

// TestCheckStaleDocs_SymlinkInsideRepo verifies that a symlink pointing to
// another file *within* the repo is also treated as stale. We refuse all
// symlinks regardless of target to keep the check simple and safe.
func TestCheckStaleDocs_SymlinkInsideRepo(t *testing.T) {
	dir := t.TempDir()

	// Real doc file.
	makeDocFile(t, dir, "docs/real.md")

	// Symlink inside repo pointing at the real file.
	linkPath := filepath.Join(dir, "docs", "link.md")
	if err := os.Symlink(filepath.Join(dir, "docs", "real.md"), linkPath); err != nil {
		t.Fatalf("Symlink: %v", err)
	}

	docmap := makeDocmapInDir(t, dir, `
mappings:
  - paths:
      - "lib/**"
    docs:
      - docs/link.md
`)

	code, _, stderr := stdinValidateDocmap(t,
		"",
		[]string{"--docmap", docmap, "--repo-root", dir},
	)
	if code != 1 {
		t.Errorf("expected exit 1 for symlink doc (even intra-repo), got %d; stderr: %q", code, stderr)
	}
}

// TestRunValidateDocmap_SymlinkRepoRoot verifies that a --repo-root that is
// itself a symlink to a valid directory resolves correctly.
func TestRunValidateDocmap_SymlinkRepoRoot(t *testing.T) {
	realDir := t.TempDir()
	makeDocFile(t, realDir, "docs/foo.md")

	// Create a symlink pointing at realDir.
	symlinkDir := filepath.Join(t.TempDir(), "link-root")
	if err := os.Symlink(realDir, symlinkDir); err != nil {
		t.Fatalf("Symlink: %v", err)
	}

	// Place the docmap inside realDir so it passes the confinement check.
	// (symlinkDir resolves to realDir, so files inside realDir are also inside
	// the resolved repo-root.)
	docmap := makeDocmapInDir(t, realDir, `
mappings:
  - paths:
      - "lib/**"
    docs:
      - docs/foo.md
`)

	// Using the symlinked repo-root: the real doc exists → should be clean.
	code, stdout, stderr := stdinValidateDocmap(t,
		"lib/foo.go\n",
		[]string{"--docmap", docmap, "--repo-root", symlinkDir},
	)
	if code != 0 {
		t.Errorf("expected exit 0 for symlinked repo-root with existing doc, got %d; stderr: %q", code, stderr)
	}
	if !strings.Contains(stdout, "OK") {
		t.Errorf("expected 'OK' in stdout, got %q", stdout)
	}
}

// TestValidateDocmapPath_Symlink verifies that --docmap pointing at a symlink
// whose resolved target is outside --repo-root is rejected (prevents reading
// arbitrary host files via PR-controlled symlinks).
//
// Note: after the EvalSymlinks fix (issue #150), in-repo symlinks whose
// targets also reside within the repo root are now allowed — the confinement
// check is applied to the resolved path, not the symlink entry itself. The
// security invariant is: the resolved destination must be within the root.
func TestValidateDocmapPath_Symlink(t *testing.T) {
	dir := t.TempDir()
	outside := t.TempDir()

	// Create a docmap file OUTSIDE the repo root to serve as the symlink
	// target. EvalSymlinks will resolve to this path, which the Rel check
	// must then reject.
	if err := os.MkdirAll(filepath.Join(outside, ".review-bot"), 0o755); err != nil {
		t.Fatalf("MkdirAll: %v", err)
	}
	outsideDocmap := filepath.Join(outside, ".review-bot", "doc-map.yml")
	if err := os.WriteFile(outsideDocmap, []byte("mappings: []\n"), 0o644); err != nil {
		t.Fatalf("WriteFile: %v", err)
	}

	// Create a symlink inside dir pointing to the file outside the repo.
	if err := os.MkdirAll(filepath.Join(dir, ".review-bot"), 0o755); err != nil {
		t.Fatalf("MkdirAll: %v", err)
	}
	symlinkPath := filepath.Join(dir, ".review-bot", "doc-map-link.yml")
	if err := os.Symlink(outsideDocmap, symlinkPath); err != nil {
		t.Fatalf("Symlink: %v", err)
	}

	code, _, stderr := stdinValidateDocmap(t,
		"",
		[]string{"--docmap", symlinkPath, "--repo-root", dir},
	)
	if code != 2 {
		t.Errorf("expected exit 2 for out-of-repo symlink docmap, got %d; stderr: %q", code, stderr)
	}
	if !strings.Contains(stderr, "invalid") && !strings.Contains(stderr, "repo-root") {
		t.Errorf("expected confinement rejection in stderr, got %q", stderr)
	}
}

// TestValidateDocmapPath_OutsideRepoRoot verifies that --docmap pointing
// outside --repo-root is rejected (prevents reading arbitrary host files).
func TestValidateDocmapPath_OutsideRepoRoot(t *testing.T) {
	repoDir := t.TempDir()

	// Create a docmap in a separate temp dir (outside the repo root).
	outside := makeDocmapYAML(t, `
mappings:
  - paths:
      - "lib/**"
    docs:
      - docs/foo.md
`)

	code, _, stderr := stdinValidateDocmap(t,
		"",
		[]string{"--docmap", outside, "--repo-root", repoDir},
	)
	if code != 2 {
		t.Errorf("expected exit 2 for docmap outside repo-root, got %d; stderr: %q", code, stderr)
	}
	if !strings.Contains(stderr, "invalid") && !strings.Contains(stderr, "repo-root") {
		t.Errorf("expected confinement rejection in stderr, got %q", stderr)
	}
}

// TestValidateDocmapPath_SizeLimit verifies that --docmap files exceeding
// maxDocmapBytes are rejected before reading (prevents memory exhaustion).
func TestValidateDocmapPath_SizeLimit(t *testing.T) {
	dir := t.TempDir()

	// Write a file larger than maxDocmapBytes.
	bigPath := filepath.Join(dir, ".review-bot", "big-doc-map.yml")
	if err := os.MkdirAll(filepath.Dir(bigPath), 0o755); err != nil {
		t.Fatalf("MkdirAll: %v", err)
	}
	// Exceed the limit by one byte.
	bigContent := make([]byte, maxDocmapBytes+1)
	if err := os.WriteFile(bigPath, bigContent, 0o644); err != nil {
		t.Fatalf("WriteFile: %v", err)
	}

	code, _, stderr := stdinValidateDocmap(t,
		"",
		[]string{"--docmap", bigPath, "--repo-root", dir},
	)
	if code != 2 {
		t.Errorf("expected exit 2 for oversized docmap, got %d; stderr: %q", code, stderr)
	}
	if !strings.Contains(stderr, "limit") && !strings.Contains(stderr, "size") && !strings.Contains(stderr, "invalid") {
		t.Errorf("expected size limit error in stderr, got %q", stderr)
	}
}

// TestValidateDocmapPath_DirSymlinkBypass verifies that a directory-symlink
// inside the repo pointing outside cannot be used to read arbitrary host files.
//
// Attack vector: a PR commits .review-bot/ as a directory symlink targeting a
// directory outside the repo. The textual path of the docmap file is inside
// the repo root, so the old Rel-only check passed — but the actual file is
// outside. This is closed by calling EvalSymlinks on the full path before the
// confinement check.
func TestValidateDocmapPath_DirSymlinkBypass(t *testing.T) {
	repoDir := t.TempDir()
	outsideDir := t.TempDir()

	// Secret file outside the repo.
	secretPath := filepath.Join(outsideDir, "secret.yml")
	if err := os.WriteFile(secretPath, []byte("mappings: []\n"), 0o644); err != nil {
		t.Fatalf("WriteFile: %v", err)
	}

	// Create .review-bot/ as a directory symlink pointing outside the repo.
	reviewBotDir := filepath.Join(repoDir, ".review-bot")
	if err := os.Symlink(outsideDir, reviewBotDir); err != nil {
		t.Skipf("cannot create dir symlink (platform may not support it): %v", err)
	}

	// Textually inside repo — .review-bot/secret.yml — but resolves outside.
	attackPath := filepath.Join(repoDir, ".review-bot", "secret.yml")

	// Resolve repoDir to a symlink-free path, as runValidateDocmap does.
	resolvedRoot, err := filepath.EvalSymlinks(repoDir)
	if err != nil {
		t.Fatalf("EvalSymlinks(repoDir): %v", err)
	}

	if err := validateDocmapPath(attackPath, resolvedRoot); err == nil {
		t.Error("expected rejection of dir-symlink bypass, got nil error")
	}
}