From b109baf0c23c67b07a773023f1bc89a4bd4a4d4c Mon Sep 17 00:00:00 2001 From: zephyrq-z <122677241+zephyrq-z@users.noreply.github.com> Date: Sat, 27 Jun 2026 11:06:38 +0800 Subject: [PATCH] Feature/rule link to files (#226) * feat: support inline content and file path resolution for rule field - rule field auto-detects: .md/.txt/.markdown ending = file path, otherwise inline - file paths: project-relative first, then as absolute path - safety: stat before read (512KB cap), extension whitelist, symlink resolution - tightened heuristic: values with spaces treated as inline to avoid false positives - guard against empty repoDir to avoid CWD-relative resolution - 5-language README docs updated with file path usage and first-match-wins behavior - 15 new unit tests covering all resolution branches Closes #67 Supersedes #87 * update readme * feat: update rule resolution logic to clear rules for missing or invalid files * feat: update rule field description to clarify file path detection criteria * feat: enhance rule file resolution to block path traversal and improve validation * fix: clean up tryReadRuleFile and add missing blank line - Add blank line between matchProjectRuleEntry and allowedRuleExts (Issue 1) - Remove dead code '|| repoDir == ' in tryReadRuleFile (Issue 2) - Remove unnecessary warning when repoDir is empty but path is absolute (Issue 3) --- README.ja-JP.md | 39 ++ README.ko-KR.md | 39 ++ README.md | 39 ++ README.ru-RU.md | 39 ++ README.zh-CN.md | 39 ++ internal/config/rules/system_rules.go | 113 ++++++ internal/config/rules/system_rules_test.go | 403 +++++++++++++++++++++ 7 files changed, 711 insertions(+) diff --git a/README.ja-JP.md b/README.ja-JP.md index 8998dd9..016b3cb 100644 --- a/README.ja-JP.md +++ b/README.ja-JP.md @@ -565,6 +565,45 @@ OCRは4層の優先度チェーンを使ってレビュールールを解決し - 各層の中では、ルールは宣言順に評価されます — 最初にマッチしたものが採用されます。 - ルールファイルが存在しない場合は、何も出力せずスキップされます。 +**`rule` フィールドはインラインコンテンツとファイルパスの両方をサポートします。** システムは次の順序で自動判別します： + +1. 値に改行が含まれる → **インラインコンテンツ**（複数行ルールがファイルパスと見なされることはありません）。 +2. 値が単一行で、スペースを含まず、`.md` / `.txt` / `.markdown` で終わる → **ファイルパス**。 + - 絶対パス（`/` で始まる）はそのまま使用されます。 + - 相対パスはプロジェクトルートで解決されます。パストラバーサル（例: `../../etc/passwd.md`）はブロックされます。見つからない場合は `[WARN]` を出力し、ルールはクリアされます（インラインへのフォールバックなし）。 + - ファイルはバリデーションを通過する必要があります：ホワイトリスト拡張子、≤ 512 KB、シンボリックリンク解決後のターゲットもホワイトリスト拡張子であること。バリデーションに失敗した場合、ルールはクリアされます。 +3. それ以外 → **インラインコンテンツ**。 + +```json +{ + "rules": [ + { + "path": "**/*mapper*.xml", + "rule": "docs/sql-rules.md" + }, + { + "path": "**/*.java", + "rule": "Always check for null safety and resource leaks" + }, + { + "path": "**/*.go", + "rule": "shared/go-concurrency.md" + }, + { + "path": "**/*.py", + "rule": "/Users/me/team-rules/python.md" + } + ] +} +``` + +- `docs/sql-rules.md` — 相対パス、`/docs/sql-rules.md` から読み込み。 +- `Always check for null safety…` — インライン文字列、そのまま使用。 +- `shared/go-concurrency.md` — 相対パス、同様に解決。 +- `/Users/me/team-rules/python.md` — 絶対パス、そのまま使用。 + +> 絶対パスはプロジェクト外のファイルにアクセスできますが、これは意図的な設計です。`rule.json` はメンテナが作成する信頼された入力のためです。共有ルールを共通パス（例：`/opt/company-rules/`）に置くことで、各プロジェクトへのコピーが不要になります。 + ### パスフィルタリング ルールファイルでは `include` と `exclude` フィールドも使用でき、どのファイルをレビュー対象にするかを制御できます： diff --git a/README.ko-KR.md b/README.ko-KR.md index c0d4558..5ce4dbb 100644 --- a/README.ko-KR.md +++ b/README.ko-KR.md @@ -565,6 +565,45 @@ OCR은 네 계층의 priority chain으로 review rule을 해석합니다. 각 - 각 계층 안에서는 rule이 선언 순서대로 평가되며 첫 번째 match가 선택됩니다. - rule file이 없으면 조용히 건너뜁니다. +**`rule` 필드는 인라인 콘텐츠와 파일 경로를 모두 지원합니다.** 시스템이 다음 순서로 자동 판별합니다: + +1. 값에 줄바꿈이 포함된 경우 → **인라인 콘텐츠** (여러 줄 규칙은 파일 경로로 간주되지 않습니다). +2. 값이 한 줄이고 공백이 없으며 `.md` / `.txt` / `.markdown`으로 끝나는 경우 → **파일 경로**. + - 절대 경로(`/`로 시작)는 그대로 사용됩니다. + - 상대 경로는 프로젝트 루트에서 확인합니다. 경로 탐색(예: `../../etc/passwd.md`)은 차단됩니다. 없으면 `[WARN]`을 출력하고 규칙이 지워집니다 (인라인으로 폴백 없음). + - 파일은 유효성 검사를 통과해야 합니다: 허용된 확장자, ≤ 512 KB, 심볼릭 링크 해석 후 대상도 허용된 확장자여야 합니다. 검증 실패 시 규칙이 지워집니다. +3. 그 외의 경우 → **인라인 콘텐츠**. + +```json +{ + "rules": [ + { + "path": "**/*mapper*.xml", + "rule": "docs/sql-rules.md" + }, + { + "path": "**/*.java", + "rule": "Always check for null safety and resource leaks" + }, + { + "path": "**/*.go", + "rule": "shared/go-concurrency.md" + }, + { + "path": "**/*.py", + "rule": "/Users/me/team-rules/python.md" + } + ] +} +``` + +- `docs/sql-rules.md` — 상대 경로, `/docs/sql-rules.md`에서 로드. +- `Always check for null safety…` — 인라인 문자열, 그대로 사용. +- `shared/go-concurrency.md` — 상대 경로, 동일하게 해결. +- `/Users/me/team-rules/python.md` — 절대 경로, 그대로 사용. + +> 절대 경로는 프로젝트 외부 파일에 접근할 수 있으며, 이는 의도된 설계입니다. `rule.json`은 프로젝트 메인테이너가 작성하는 신뢰된 입력입니다. 팀은 공유 규칙을 공통 경로(예: `/opt/company-rules/`)에 두어 각 프로젝트에 복사할 필요가 없습니다. + ## Configuration Reference Config file: `~/.opencodereview/config.json` diff --git a/README.md b/README.md index b37e65c..a5b8946 100644 --- a/README.md +++ b/README.md @@ -570,6 +570,45 @@ Layers 1–3 share the same JSON format: - Within each layer, rules are evaluated in declaration order — the first match wins. - If a rule file does not exist, it is silently skipped. +**The `rule` field supports both inline content and file paths.** The system auto-detects which one you mean: + +1. If the value contains newlines → **inline content** (multi-line rules are never file paths). +2. If the value is a single line, contains no spaces, and ends with `.md` / `.txt` / `.markdown` → **file path**. + - Absolute paths (starting with `/`) are used directly. + - Relative paths are resolved against the project root. Path traversal (e.g. `../../etc/passwd.md`) is blocked. If not found, a `[WARN]` is emitted and the rule is cleared (no fallback to inline). + - The file must pass validation: whitelisted extension, ≤ 512 KB, and resolved symlink target must also be a whitelisted extension. If validation fails, the rule is cleared. +3. Otherwise → **inline content**. + +```json +{ + "rules": [ + { + "path": "**/*mapper*.xml", + "rule": "docs/sql-rules.md" + }, + { + "path": "**/*.java", + "rule": "Always check for null safety and resource leaks" + }, + { + "path": "**/*.go", + "rule": "shared/go-concurrency.md" + }, + { + "path": "**/*.py", + "rule": "/Users/me/team-rules/python.md" + } + ] +} +``` + +- `docs/sql-rules.md` — relative path, resolved from `/docs/sql-rules.md`. +- `Always check for null safety…` — inline string, used directly. +- `shared/go-concurrency.md` — relative path, same resolution. +- `/Users/me/team-rules/python.md` — absolute path, used directly. + +> Absolute paths can access files outside the project directory — this is intentional. `rule.json` is authored by project maintainers, i.e. trusted input. Teams can store shared rules at a common path (e.g. `/opt/company-rules/`) instead of copying them into every project. + ### Path Filtering Rule files also support `include` and `exclude` fields to control which files enter the review scope: diff --git a/README.ru-RU.md b/README.ru-RU.md index 522aabd..8f74bf9 100644 --- a/README.ru-RU.md +++ b/README.ru-RU.md @@ -567,6 +567,45 @@ OCR разрешает правила ревью по цепочке приор - Внутри каждого уровня правила проверяются в порядке объявления — побеждает первое совпадение. - Если файл правил не существует, он молча пропускается. +**Поле `rule` поддерживает как встроенный текст, так и пути к файлам.** Система определяет тип автоматически: + +1. Если значение содержит переносы строк → **встроенный текст** (многострочные правила никогда не считаются путями). +2. Если значение — одна строка, без пробелов, и заканчивается на `.md` / `.txt` / `.markdown` → **путь к файлу**. + - Абсолютные пути (начинающиеся с `/`) используются напрямую. + - Относительные пути проверяются в корне проекта. Выход за пределы директории (например, `../../etc/passwd.md`) блокируется. Если не найдены — выводится `[WARN]` и правило очищается (без fallback на inline). + - Файл должен пройти проверку: допустимое расширение, ≤ 512 KB, цель симлинка также должна иметь допустимое расширение. При ошибке проверки правило очищается. +3. Иначе → **встроенный текст**. + +```json +{ + "rules": [ + { + "path": "**/*mapper*.xml", + "rule": "docs/sql-rules.md" + }, + { + "path": "**/*.java", + "rule": "Always check for null safety and resource leaks" + }, + { + "path": "**/*.go", + "rule": "shared/go-concurrency.md" + }, + { + "path": "**/*.py", + "rule": "/Users/me/team-rules/python.md" + } + ] +} +``` + +- `docs/sql-rules.md` — относительный путь, загружается из `/docs/sql-rules.md`. +- `Always check for null safety…` — встроенная строка, используется напрямую. +- `shared/go-concurrency.md` — относительный путь, аналогично. +- `/Users/me/team-rules/python.md` — абсолютный путь, используется напрямую. + +> Абсолютные пути могут указывать на файлы вне директории проекта — это сделано намеренно. `rule.json` пишут мейнтейнеры проекта, это доверенный ввод. Команды могут хранить общие правила по единому пути (например, `/opt/company-rules/`) и не копировать их в каждый проект. + ### Фильтрация путей Файлы правил также поддерживают поля `include` и `exclude`, управляющие тем, какие файлы попадают в область ревью: diff --git a/README.zh-CN.md b/README.zh-CN.md index ba594d4..1d37593 100644 --- a/README.zh-CN.md +++ b/README.zh-CN.md @@ -555,6 +555,45 @@ OCR 通过四层优先级链解析评审规则。每层采用首次匹配原则 - 在每一层内，规则按声明顺序评估 —— 首次匹配生效。 - 如果规则文件不存在，将被静默跳过。 +**`rule` 字段同时支持内联内容和文件路径。**系统按以下顺序自动判断： + +1. 如果值包含换行 → **内联内容**（多行规则永远不会被当作文件路径）。 +2. 如果值是单行、不含空格、且以 `.md` / `.txt` / `.markdown` 结尾 → **文件路径**。 + - 绝对路径（以 `/` 开头）直接使用。 + - 相对路径在项目根目录下查找，路径穿越（如 `../../etc/passwd.md`）会被拦截。找不到则 `[WARN]` 并清空该规则（不会回退为内联）。 + - 文件需通过安全校验：白名单扩展名、≤ 512 KB、symlink 解析后目标也必须是白名单扩展名。校验失败则清空该规则。 +3. 否则 → **内联内容**。 + +```json +{ + "rules": [ + { + "path": "**/*mapper*.xml", + "rule": "docs/sql-rules.md" + }, + { + "path": "**/*.java", + "rule": "始终检查空值安全和资源泄漏" + }, + { + "path": "**/*.go", + "rule": "shared/go-concurrency.md" + }, + { + "path": "**/*.py", + "rule": "/Users/me/team-rules/python.md" + } + ] +} +``` + +- `docs/sql-rules.md` — 相对路径，从 `/docs/sql-rules.md` 加载。 +- `始终检查空值安全…` — 内联字符串，直接使用。 +- `shared/go-concurrency.md` — 相对路径，同上。 +- `/Users/me/team-rules/python.md` — 绝对路径，直接使用。 + +> 绝对路径可以访问项目目录之外的文件，这是有意为之的设计——`rule.json` 由项目维护者编写，属于受信输入。团队可将共享规则放在统一路径下（如 `/opt/company-rules/`），无需在各项目中复制。 + ### 路径过滤 规则文件同时支持 `include` 和 `exclude` 字段，用于控制哪些文件进入审查范围： diff --git a/internal/config/rules/system_rules.go b/internal/config/rules/system_rules.go index a752861..2da4d14 100644 --- a/internal/config/rules/system_rules.go +++ b/internal/config/rules/system_rules.go @@ -327,6 +327,7 @@ func loadGlobalRule() (*ProjectRule, error) { if err := json.Unmarshal(data, &pr); err != nil { return nil, fmt.Errorf("unmarshal global rule: %w", err) } + resolveRuleEntries(pr.Rules, filepath.Dir(path)) return &pr, nil } @@ -339,6 +340,7 @@ func loadRuleFile(path string) (*ProjectRule, error) { if err := json.Unmarshal(data, &pr); err != nil { return nil, fmt.Errorf("unmarshal rule file %s: %w", path, err) } + resolveRuleEntries(pr.Rules, filepath.Dir(path)) return &pr, nil } @@ -355,6 +357,7 @@ func loadProjectRule(repoDir string) (*ProjectRule, error) { if err := json.Unmarshal(data, &pr); err != nil { return nil, fmt.Errorf("unmarshal project rule: %w", err) } + resolveRuleEntries(pr.Rules, repoDir) return &pr, nil } @@ -438,3 +441,113 @@ func matchProjectRuleEntry(pr *ProjectRule, path string) *ProjectRuleEntry { } return nil } + +// allowedRuleExts is the set of file extensions permitted for rule file references. +var allowedRuleExts = map[string]bool{".md": true, ".txt": true, ".markdown": true} + +// looksLikeFilePath returns true when s is likely a file path (not inline content). +// Heuristic: multi-line text is always inline; single-line text without spaces +// ending in .md/.txt/.markdown is treated as a file path. Values containing spaces +// (e.g. "Follow rules from team.md") are treated as inline to avoid false positives. +func looksLikeFilePath(s string) bool { + if strings.Contains(s, "\n") { + return false + } + if strings.Contains(s, " ") { + return false + } + return allowedRuleExts[strings.ToLower(filepath.Ext(s))] +} + +// resolveRuleEntries scans each entry's Rule field. When the value looks like a file +// path, it reads the file content and replaces the Rule. Absolute paths are used +// directly; relative paths are resolved against repoDir only. Multi-line and short +// inline rules are left unchanged. If the file cannot be read, the Rule is cleared +// (set to empty) and a warning is emitted. +func resolveRuleEntries(entries []ProjectRuleEntry, repoDir string) { + for i := range entries { + e := &entries[i] + if strings.TrimSpace(e.Rule) == "" || !looksLikeFilePath(e.Rule) { + continue + } + if content := tryReadRuleFile(e.Rule, repoDir); content != nil { + e.Rule = *content + } else { + e.Rule = "" + } + } +} + +// tryReadRuleFile attempts to read a rule file. Absolute paths are used directly. +// Relative paths are resolved against repoDir and validated to stay within repoDir. +// Returns nil when the file cannot be read safely or does not exist. +func tryReadRuleFile(rule string, repoDir string) *string { + if repoDir == "" { + if !filepath.IsAbs(rule) { + fmt.Fprintf(os.Stderr, "[ocr] WARNING: cannot resolve relative rule path %q without a repo dir\n", rule) + return nil + } + } + if filepath.IsAbs(rule) { + content, err := readRuleFileSafe(rule) + if err == nil { + return &content + } + if os.IsNotExist(err) { + fmt.Fprintf(os.Stderr, "[ocr] WARNING: rule file not found: %s\n", rule) + } else { + fmt.Fprintf(os.Stderr, "[ocr] WARNING: cannot read rule file %s: %v\n", rule, err) + } + return nil + } + + // Relative path: resolve against repoDir, validate no traversal. + resolved := filepath.Clean(filepath.Join(repoDir, rule)) + cleanRepo := filepath.Clean(repoDir) + if !strings.HasPrefix(resolved, cleanRepo+string(os.PathSeparator)) { + fmt.Fprintf(os.Stderr, "[ocr] WARNING: rule file path escapes repo dir: %s\n", rule) + return nil + } + + content, err := readRuleFileSafe(resolved) + if err == nil { + return &content + } + if os.IsNotExist(err) { + fmt.Fprintf(os.Stderr, "[ocr] WARNING: rule file not found: %s\n", rule) + } else { + fmt.Fprintf(os.Stderr, "[ocr] WARNING: cannot read rule file %s: %v\n", resolved, err) + } + return nil +} + +// readRuleFileSafe reads and validates a rule file. It enforces extension whitelist +// (.md / .txt / .markdown), a 512 KB size cap, and resolves symlinks before checking +// the path. Symlinks are resolved first, then size is checked via Stat before reading. +// Returns the trimmed content on success. +func readRuleFileSafe(path string) (string, error) { + resolved, err := filepath.EvalSymlinks(path) + if err != nil { + return "", err + } + + if !allowedRuleExts[strings.ToLower(filepath.Ext(resolved))] { + return "", fmt.Errorf("unsupported extension %q, only .md/.txt/.markdown allowed", filepath.Ext(resolved)) + } + + const maxSize = 512 * 1024 + info, err := os.Stat(resolved) + if err != nil { + return "", err + } + if info.Size() > maxSize { + return "", fmt.Errorf("file too large (%d bytes, max %d)", info.Size(), maxSize) + } + + content, err := os.ReadFile(resolved) + if err != nil { + return "", err + } + + return strings.TrimRight(string(content), "\n"), nil +} diff --git a/internal/config/rules/system_rules_test.go b/internal/config/rules/system_rules_test.go index 2d22a25..8d18fdf 100644 --- a/internal/config/rules/system_rules_test.go +++ b/internal/config/rules/system_rules_test.go @@ -945,3 +945,406 @@ func TestNewResolver_BraceExpansionInProjectRule(t *testing.T) { }) } } + +// ── resolveRuleEntries tests ── + +func TestResolveRuleEntries_BasicFile(t *testing.T) { + dir := t.TempDir() + ruleFile := filepath.Join(dir, "sql-rules.md") + if err := os.WriteFile(ruleFile, []byte("Check for SQL injection\n"), 0o644); err != nil { + t.Fatal(err) + } + + entries := []ProjectRuleEntry{ + {Path: "**/*.xml", Rule: "sql-rules.md"}, + {Path: "**/*.go", Rule: "Always check for nil"}, + } + resolveRuleEntries(entries, dir) + + if entries[0].Rule != "Check for SQL injection" { + t.Errorf("expected file content, got %q", entries[0].Rule) + } + if entries[1].Rule != "Always check for nil" { + t.Errorf("inline rule should not change, got %q", entries[1].Rule) + } +} + +func TestResolveRuleEntries_MultiLineInline(t *testing.T) { + dir := t.TempDir() + // Create a file with the same name as the inline rule to make sure + // multi-line detection prevents file lookup. + if err := os.WriteFile(filepath.Join(dir, "security.md"), []byte("file content"), 0o644); err != nil { + t.Fatal(err) + } + + entries := []ProjectRuleEntry{ + {Path: "**/*.ts", Rule: "security.md\nBut this is multi-line\nso it should stay inline"}, + } + resolveRuleEntries(entries, dir) + + if entries[0].Rule != "security.md\nBut this is multi-line\nso it should stay inline" { + t.Errorf("multi-line rule should stay inline, got %q", entries[0].Rule) + } +} + +func TestResolveRuleEntries_MissingFile(t *testing.T) { + dir := t.TempDir() + + entries := []ProjectRuleEntry{ + {Path: "**/*.xml", Rule: "nonexistent.md"}, + } + resolveRuleEntries(entries, dir) + + // Missing file should clear the rule. + if entries[0].Rule != "" { + t.Errorf("missing file should clear rule, got %q", entries[0].Rule) + } +} + +func TestResolveRuleEntries_AbsolutePath(t *testing.T) { + dir := t.TempDir() + ruleFile := filepath.Join(dir, "my-rule.md") + if err := os.WriteFile(ruleFile, []byte("absolute rule content"), 0o644); err != nil { + t.Fatal(err) + } + + // Use an absolute path pointing to a file in a different directory. + entries := []ProjectRuleEntry{ + {Path: "**/*.go", Rule: ruleFile}, + } + resolveRuleEntries(entries, "/some/other/repo") + + if entries[0].Rule != "absolute rule content" { + t.Errorf("expected absolute file content, got %q", entries[0].Rule) + } +} + +func TestResolveRuleEntries_TooLarge(t *testing.T) { + dir := t.TempDir() + big := make([]byte, 513*1024) + for i := range big { + big[i] = 'a' + } + bigFile := filepath.Join(dir, "big.md") + if err := os.WriteFile(bigFile, big, 0o644); err != nil { + t.Fatal(err) + } + + entries := []ProjectRuleEntry{ + {Path: "**/*.go", Rule: "big.md"}, + } + resolveRuleEntries(entries, dir) + + if entries[0].Rule != "" { + t.Errorf("oversized file should clear rule, got %q", entries[0].Rule) + } +} + +func TestResolveRuleEntries_RelativePath(t *testing.T) { + repoDir := t.TempDir() + if err := os.WriteFile(filepath.Join(repoDir, "shared.md"), []byte("repo-level"), 0o644); err != nil { + t.Fatal(err) + } + + entries := []ProjectRuleEntry{ + {Path: "**/*.go", Rule: "shared.md"}, + } + resolveRuleEntries(entries, repoDir) + + if entries[0].Rule != "repo-level" { + t.Errorf("repo-level should win, got %q", entries[0].Rule) + } +} + +func TestResolveRuleEntries_EmptyRule(t *testing.T) { + entries := []ProjectRuleEntry{ + {Path: "**/*.go", Rule: ""}, + {Path: "**/*.ts", Rule: " "}, + {Path: "**/*.java", Rule: "\t\n"}, + } + resolveRuleEntries(entries, "/tmp") + + if entries[0].Rule != "" { + t.Errorf("empty rule should stay empty, got %q", entries[0].Rule) + } + if entries[1].Rule != " " { + t.Errorf("whitespace-only rule should stay unchanged, got %q", entries[1].Rule) + } + if entries[2].Rule != "\t\n" { + t.Errorf("whitespace+newline rule should stay unchanged, got %q", entries[2].Rule) + } +} + +func TestResolveRuleEntries_SymlinkSafety(t *testing.T) { + dir := t.TempDir() + sensitiveFile := filepath.Join(dir, "secret.json") + if err := os.WriteFile(sensitiveFile, []byte("SECRET"), 0o644); err != nil { + t.Fatal(err) + } + + // Create a symlink with .md extension pointing to a .json file. + // The extension check on the resolved path should reject .json. + symlinkPath := filepath.Join(dir, "evil.md") + if err := os.Symlink(sensitiveFile, symlinkPath); err != nil { + t.Fatal(err) + } + + entries := []ProjectRuleEntry{ + {Path: "**/*.go", Rule: "evil.md"}, + } + resolveRuleEntries(entries, dir) + // The symlink target is .json, which is not in the whitelist. + // The rule should be cleared. + if entries[0].Rule != "" { + t.Errorf("symlink to non-whitelisted file should clear rule, got %q", entries[0].Rule) + } +} + +func TestResolveRuleEntries_TxtExtension(t *testing.T) { + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, "rules.txt"), []byte("rule from txt"), 0o644); err != nil { + t.Fatal(err) + } + + entries := []ProjectRuleEntry{ + {Path: "**/*.go", Rule: "rules.txt"}, + } + resolveRuleEntries(entries, dir) + + if entries[0].Rule != "rule from txt" { + t.Errorf(".txt should be accepted, got %q", entries[0].Rule) + } +} + +func TestResolveRuleEntries_MarkdownExtension(t *testing.T) { + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, "rules.markdown"), []byte("rule from markdown"), 0o644); err != nil { + t.Fatal(err) + } + + entries := []ProjectRuleEntry{ + {Path: "**/*.go", Rule: "rules.markdown"}, + } + resolveRuleEntries(entries, dir) + + if entries[0].Rule != "rule from markdown" { + t.Errorf(".markdown should be accepted, got %q", entries[0].Rule) + } +} + +func TestResolveRuleEntries_SubdirectoryPath(t *testing.T) { + dir := t.TempDir() + docsDir := filepath.Join(dir, "docs") + if err := os.MkdirAll(docsDir, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(docsDir, "my-rule.md"), []byte("nested rule"), 0o644); err != nil { + t.Fatal(err) + } + + entries := []ProjectRuleEntry{ + {Path: "**/*.go", Rule: "docs/my-rule.md"}, + } + resolveRuleEntries(entries, dir) + + if entries[0].Rule != "nested rule" { + t.Errorf("subdirectory path should work, got %q", entries[0].Rule) + } +} + +// ── looksLikeFilePath tests ── + +func TestLooksLikeFilePath_InlineContent(t *testing.T) { + tests := []string{ + "Check for null pointers", + "Always validate input", + "security", + "xss", + } + for _, s := range tests { + if looksLikeFilePath(s) { + t.Errorf("looksLikeFilePath(%q) should be false", s) + } + } +} + +func TestLooksLikeFilePath_MultiLine(t *testing.T) { + s := "line1\nline2\nline3" + if looksLikeFilePath(s) { + t.Errorf("multi-line should be false") + } +} + +func TestLooksLikeFilePath_FileExtensions(t *testing.T) { + tests := []string{ + "rules.md", + "doc.txt", + "doc.markdown", + "DOC.MD", + "path/to/file.md", + } + for _, s := range tests { + if !looksLikeFilePath(s) { + t.Errorf("looksLikeFilePath(%q) should be true", s) + } + } +} + +func TestLooksLikeFilePath_WithSpaces(t *testing.T) { + // Values containing spaces are inline, not file paths. + tests := []string{ + "Follow rules from team.md", + "Ensure output is in .md", + "use .txt format", + } + for _, s := range tests { + if looksLikeFilePath(s) { + t.Errorf("looksLikeFilePath(%q) should be false (contains space)", s) + } + } +} + +func TestLooksLikeFilePath_PathWithoutExtension(t *testing.T) { + // Paths without .md/.txt/.markdown are NOT treated as file paths. + tests := []string{ + "docs/security", + "shared/rules/go", + "Use HTTP/2 for all requests", + } + for _, s := range tests { + if looksLikeFilePath(s) { + t.Errorf("looksLikeFilePath(%q) should be false (no .md/.txt/.markdown)", s) + } + } +} + +// ── readRuleFileSafe tests ── + +func TestReadRuleFileSafe_NormalFile(t *testing.T) { + dir := t.TempDir() + f := filepath.Join(dir, "test.md") + if err := os.WriteFile(f, []byte("hello world\n"), 0o644); err != nil { + t.Fatal(err) + } + + content, err := readRuleFileSafe(f) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if content != "hello world" { + t.Errorf("expected 'hello world', got %q", content) + } +} + +func TestReadRuleFileSafe_UnsupportedExt(t *testing.T) { + dir := t.TempDir() + f := filepath.Join(dir, "test.json") + if err := os.WriteFile(f, []byte("{}"), 0o644); err != nil { + t.Fatal(err) + } + + _, err := readRuleFileSafe(f) + if err == nil { + t.Fatal("expected error for .json") + } +} + +func TestReadRuleFileSafe_TooLarge(t *testing.T) { + dir := t.TempDir() + f := filepath.Join(dir, "big.md") + big := make([]byte, 513*1024) + if err := os.WriteFile(f, big, 0o644); err != nil { + t.Fatal(err) + } + + _, err := readRuleFileSafe(f) + if err == nil { + t.Fatal("expected error for oversized file") + } +} + +func TestReadRuleFileSafe_Missing(t *testing.T) { + _, err := readRuleFileSafe("/nonexistent/path.md") + if err == nil { + t.Fatal("expected error for missing file") + } +} + +// ── path traversal tests ── + +func TestResolveRuleEntries_PathTraversalBlocked(t *testing.T) { + dir := t.TempDir() + // Create a file outside the repo dir to prove it is NOT read. + outside := filepath.Join(t.TempDir(), "outside.md") + if err := os.WriteFile(outside, []byte("should not be read\n"), 0o644); err != nil { + t.Fatal(err) + } + + entries := []ProjectRuleEntry{ + {Path: "**/*.go", Rule: outside}, // absolute path to outside — allowed + {Path: "**/*.ts", Rule: "../outside.md"}, // relative traversal — blocked + } + resolveRuleEntries(entries, dir) + + // Absolute path to outside is allowed (explicit design choice). + if entries[0].Rule != "should not be read" { + t.Errorf("absolute path to outside should be allowed, got %q", entries[0].Rule) + } + // Relative traversal should be blocked and rule cleared. + if entries[1].Rule != "" { + t.Errorf("relative traversal should be blocked, got %q", entries[1].Rule) + } +} + +func TestResolveRuleEntries_EmptyRepoDirRelative(t *testing.T) { + // When repoDir is empty and rule is relative, it should be rejected. + entries := []ProjectRuleEntry{ + {Path: "**/*.go", Rule: "rules.md"}, + } + resolveRuleEntries(entries, "") + + if entries[0].Rule != "" { + t.Errorf("relative path with empty repoDir should be rejected, got %q", entries[0].Rule) + } +} + +func TestResolveRuleEntries_EmptyRepoDirAbsolute(t *testing.T) { + dir := t.TempDir() + absFile := filepath.Join(dir, "abs.md") + if err := os.WriteFile(absFile, []byte("absolute content\n"), 0o644); err != nil { + t.Fatal(err) + } + + entries := []ProjectRuleEntry{ + {Path: "**/*.go", Rule: absFile}, + } + resolveRuleEntries(entries, "") + + if entries[0].Rule != "absolute content" { + t.Errorf("absolute path with empty repoDir should work, got %q", entries[0].Rule) + } +} + +func TestResolveRuleEntries_GlobalRuleFileResolution(t *testing.T) { + // Simulate loadGlobalRule: repoDir = filepath.Dir(~/.opencodereview/rule.json) + homeDir := t.TempDir() + t.Setenv("HOME", homeDir) + + globalRuleDir := filepath.Join(homeDir, ".opencodereview") + if err := os.MkdirAll(globalRuleDir, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(globalRuleDir, "reusable.md"), []byte("global reusable rule\n"), 0o644); err != nil { + t.Fatal(err) + } + + entries := []ProjectRuleEntry{ + {Path: "**/*.go", Rule: "reusable.md"}, + } + // repoDir = ~/.opencodereview (where rule.json lives) + resolveRuleEntries(entries, globalRuleDir) + + if entries[0].Rule != "global reusable rule" { + t.Errorf("global rule file should be resolved, got %q", entries[0].Rule) + } +}