diff --git a/README.ja-JP.md b/README.ja-JP.md index 3d7d799..4f4ec9f 100644 --- a/README.ja-JP.md +++ b/README.ja-JP.md @@ -359,9 +359,32 @@ OCRは4層の優先度チェーンを使ってレビュールールを解決し } ``` +複雑なルールを別ファイルで管理したい場合は、トップレベルの `use_file_path` フラグを有効にします： + +```json +{ + "use_file_path": true, + "rules": [ + { + "path": "**/*mapper*.xml", + "rule": "docs/sql-rules.md" + }, + { + "path": "web/**/*.ts", + "rule": "docs/frontend-rules.md" + } + ] +} +``` + +- `use_file_path` はトップレベルのフラグです。`true` に設定すると、すべての `rule` フィールドは実際のルールテキストを含む外部 `.md` または `.txt` ファイルへの相対パスとして扱われます。パスは現在の `rule.json` を含むディレクトリからの相対パスです。 +- `false` または省略した場合、`rule` フィールドはインライン文字列ルールとして使用されます（上記の最初の例のように）。 +- 外部ファイルの内容は `rule` フィールドの値を上書きします。 +- セキュリティ上の理由から、参照されるファイルはベースディレクトリの外側に配置できません（`../` によるパストラバーサルは禁止）。また、ファイルサイズは100KBを超えてはなりません。 +- 各層内でルールは宣言順に評価され、最初にマッチしたものが採用されます。 +- ルールファイルが存在しない場合は警告を出力してスキップされます。 + - `path`は`**`による再帰マッチと`{java,kt}`のブレース展開をサポートします。 -- 各層の中では、ルールは宣言順に評価されます — 最初にマッチしたものが採用されます。 -- ルールファイルが存在しない場合は、何も出力せずスキップされます。 ### パスフィルタリング diff --git a/README.ko-KR.md b/README.ko-KR.md index 9870de3..246c4d3 100644 --- a/README.ko-KR.md +++ b/README.ko-KR.md @@ -359,9 +359,32 @@ OCR은 네 계층의 priority chain으로 review rule을 해석합니다. 각 } ``` -- `path`는 `**` recursive matching과 `{java,kt}` brace expansion을 지원합니다. +복잡한 rule을 별도 파일로 관리하고 싶을 때는 최상위 `use_file_path` flag를 활성화합니다: + +```json +{ + "use_file_path": true, + "rules": [ + { + "path": "**/*mapper*.xml", + "rule": "docs/sql-rules.md" + }, + { + "path": "web/**/*.ts", + "rule": "docs/frontend-rules.md" + } + ] +} +``` + +- `use_file_path`는 최상위 flag입니다. `true`로 설정하면 모든 `rule` field는 실제 rule text가 포함된 외부 `.md` 또는 `.txt` file에 대한 상대 경로로 취급됩니다. 경로는 현재 `rule.json`이 있는 directory 기준 상대 경로입니다. +- `false`로 설정하거나 생략하면 `rule` field는 inline string rule로 사용됩니다 (위 첫 번째 예시처럼). +- 외부 file의 내용이 `rule` field 값을 덮어씁니다. +- 보안상 이유로 참조되는 file은 base directory 밖에 위치할 수 없으며 (`../` path traversal 금지), file 크기는 100KB를 초과할 수 없습니다. - 각 계층 안에서는 rule이 선언 순서대로 평가되며 첫 번째 match가 선택됩니다. -- rule file이 없으면 조용히 건너뜁니다. +- Rule file이 없으면 warning을 출력하고 건너뜁니다. + +- `path`는 `**` recursive matching과 `{java,kt}` brace expansion을 지원합니다. ## Configuration Reference diff --git a/README.md b/README.md index 4df7c9c..5376c0d 100644 --- a/README.md +++ b/README.md @@ -359,9 +359,30 @@ Layers 1–3 share the same JSON format: } ``` -- `path` supports `**` recursive matching and `{java,kt}` brace expansion. +When complex rules are better kept in separate files, enable the top-level `use_file_path` flag: + +```json +{ + "use_file_path": true, + "rules": [ + { + "path": "**/*mapper*.xml", + "rule": "docs/sql-rules.md" + }, + { + "path": "web/**/*.ts", + "rule": "docs/frontend-rules.md" + } + ] +} +``` + +- `use_file_path` is a top-level toggle. When set to `true`, all `rule` fields are treated as relative paths to external `.md` or `.txt` files whose content is the actual rule text. Paths are relative to the directory containing the current `rule.json`. +- When set to `false` or omitted, `rule` fields are used as inline string rules (as in the first example above). +- The content of external files will overwrite the `rule` field values. +- For security reasons, referenced files cannot be outside their base directory (no `../` path traversal), and file sizes must not exceed 100KB. - Within each layer, rules are evaluated in declaration order — the first match wins. -- If a rule file does not exist, it is silently skipped. +- Missing rule files will generate a warning and be skipped. ### Path Filtering diff --git a/README.zh-CN.md b/README.zh-CN.md index 82e9162..717b135 100644 --- a/README.zh-CN.md +++ b/README.zh-CN.md @@ -343,15 +343,36 @@ OCR 通过四层优先级链解析评审规则。每层采用首次匹配原则 }, { "path": "**/*mapper*.xml", - "rule": "检查 SQL 注入风险、参数错误和缺少闭合标签" + "rule": "检查 SQL 是否存在注入风险、参数错误及缺失闭合标签" } ] } ``` -- `path` 支持 `**` 递归匹配和 `{java,kt}` 大括号展开。 +当需要使用外部文件来存放较复杂的规则时，可设置顶层 `use_file_path` 开关： + +```json +{ + "use_file_path": true, + "rules": [ + { + "path": "**/*mapper*.xml", + "rule": "docs/sql-rules.md" + }, + { + "path": "web/**/*.ts", + "rule": "docs/frontend-rules.md" + } + ] +} +``` + +- `use_file_path` 是顶层开关。设为 `true` 时，所有 `rule` 字段均被视为指向外部 `.md` 或 `.txt` 文件的相对路径，文件内容即为实际的规则文本。路径相对于当前 `rule.json` 所在目录。 +- 设为 `false` 或省略时，`rule` 字段作为内联字符串规则使用（即上方第一个示例）。 +- 外部文件的内容将直接覆盖 `rule` 字段的值。 +- 出于安全考虑，引用的文件不能位于所在目录之外（禁止 `../` 路径穿越），且文件大小不能超过 100KB。 - 在每一层内，规则按声明顺序评估 —— 首次匹配生效。 -- 如果规则文件不存在，将被静默跳过。 +- 如果规则文件不存在，将生成警告并被跳过。 ### 路径过滤 diff --git a/internal/config/rules/system_rules.go b/internal/config/rules/system_rules.go index d3740ff..489213c 100644 --- a/internal/config/rules/system_rules.go +++ b/internal/config/rules/system_rules.go @@ -182,9 +182,10 @@ type ProjectRuleEntry struct { // ProjectRule holds rules loaded from /.opencodereview/rule.json. type ProjectRule struct { - Rules []ProjectRuleEntry `json:"rules"` - Include []string `json:"include,omitempty"` - Exclude []string `json:"exclude,omitempty"` + UseFilePath bool `json:"use_file_path,omitempty"` + Rules []ProjectRuleEntry `json:"rules"` + Include []string `json:"include,omitempty"` + Exclude []string `json:"exclude,omitempty"` } // FileFilter holds the merged user-configured include/exclude glob patterns @@ -302,12 +303,103 @@ func buildFileFilter(layers ...*ProjectRule) *FileFilter { return nil } +// resolveRuleFiles reads external rule files if UseFilePath is true and replaces the Rule field with the file content. +func resolveRuleFiles(pr *ProjectRule, baseDir string) { + if pr == nil || baseDir == "" { + return + } + + // Early exit if UseFilePath is not enabled at the top level + if !pr.UseFilePath { + return + } + + absBase, err := filepath.Abs(baseDir) + if err != nil { + fmt.Fprintf(os.Stderr, "[WARN] Failed to get absolute path for base dir %s: %v\n", baseDir, err) + return + } + // Resolve symlinks on base directory for consistent prefix matching + // (e.g. on macOS /var -> /private/var) + if resolved, err := filepath.EvalSymlinks(absBase); err == nil { + absBase = resolved + } + // Ensure base directory path ends with separator for proper prefix matching + if !strings.HasSuffix(absBase, string(filepath.Separator)) { + absBase += string(filepath.Separator) + } + + for i := range pr.Rules { + entry := &pr.Rules[i] + + // Warn if Rule path is empty + if entry.Rule == "" { + fmt.Fprintf(os.Stderr, "[WARN] Rule entry for path %q has use_file_path=true but empty rule path, skipping\n", entry.Path) + continue + } + + filePath := entry.Rule + absFile, err := filepath.Abs(filepath.Join(baseDir, filePath)) + if err != nil { + fmt.Fprintf(os.Stderr, "[WARN] Failed to get absolute path for rule file %s: %v\n", filePath, err) + continue + } + + // File size check and existence check (before symlink resolution) + info, err := os.Stat(absFile) + if err != nil { + if os.IsNotExist(err) { + fmt.Fprintf(os.Stderr, "[WARN] Rule file not found: %s\n", absFile) + } else { + fmt.Fprintf(os.Stderr, "[WARN] Failed to stat rule file %s: %v\n", absFile, err) + } + continue + } + if info.Size() > 100*1024 { + fmt.Fprintf(os.Stderr, "[WARN] Rule file %s is too large (%d bytes, max 100KB)\n", absFile, info.Size()) + continue + } + + // Security: resolve symlinks to prevent directory traversal via symlink + resolvedFile, err := filepath.EvalSymlinks(absFile) + if err != nil { + fmt.Fprintf(os.Stderr, "[WARN] Failed to resolve symlinks for rule file %s: %v\n", filePath, err) + continue + } + absFile = resolvedFile + + // Security check: prevent directory traversal + if !strings.HasPrefix(absFile+string(filepath.Separator), absBase) && absFile != strings.TrimSuffix(absBase, string(filepath.Separator)) { + fmt.Fprintf(os.Stderr, "[WARN] Security violation: rule file %s is outside the base directory %s\n", filePath, baseDir) + continue + } + + // Extension check + ext := strings.ToLower(filepath.Ext(absFile)) + if ext != ".md" && ext != ".txt" { + fmt.Fprintf(os.Stderr, "[WARN] Unsupported rule file extension %s for %s (only .md and .txt are allowed)\n", ext, filePath) + continue + } + + // Read file content + content, err := os.ReadFile(absFile) + if err != nil { + fmt.Fprintf(os.Stderr, "[WARN] Failed to read rule file %s: %v\n", absFile, err) + continue + } + + // Overwrite the Rule field with the file content + entry.Rule = strings.TrimRight(string(content), "\n") + } +} + func loadGlobalRule() (*ProjectRule, error) { home, err := os.UserHomeDir() if err != nil { return nil, nil } - path := filepath.Join(home, ".opencodereview", "rule.json") + baseDir := filepath.Join(home, ".opencodereview") + path := filepath.Join(baseDir, "rule.json") data, err := os.ReadFile(path) if err != nil { if os.IsNotExist(err) { @@ -319,6 +411,7 @@ func loadGlobalRule() (*ProjectRule, error) { if err := json.Unmarshal(data, &pr); err != nil { return nil, fmt.Errorf("unmarshal global rule: %w", err) } + resolveRuleFiles(&pr, baseDir) return &pr, nil } @@ -331,11 +424,13 @@ 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) } + resolveRuleFiles(&pr, filepath.Dir(path)) return &pr, nil } func loadProjectRule(repoDir string) (*ProjectRule, error) { - path := filepath.Join(repoDir, ".opencodereview", "rule.json") + baseDir := filepath.Join(repoDir, ".opencodereview") + path := filepath.Join(baseDir, "rule.json") data, err := os.ReadFile(path) if err != nil { if os.IsNotExist(err) { @@ -347,6 +442,7 @@ func loadProjectRule(repoDir string) (*ProjectRule, error) { if err := json.Unmarshal(data, &pr); err != nil { return nil, fmt.Errorf("unmarshal project rule: %w", err) } + resolveRuleFiles(&pr, baseDir) return &pr, nil } diff --git a/internal/config/rules/system_rules_test.go b/internal/config/rules/system_rules_test.go index 00e45dc..01ef2db 100644 --- a/internal/config/rules/system_rules_test.go +++ b/internal/config/rules/system_rules_test.go @@ -670,3 +670,83 @@ func TestNewResolver_BraceExpansionInProjectRule(t *testing.T) { }) } } + +func TestResolveRuleFiles_Basic(t *testing.T) { + dir := t.TempDir() + mdContent := "# Rule from file\nCheck for memory leaks." + err := os.WriteFile(filepath.Join(dir, "rule.md"), []byte(mdContent), 0644) + if err != nil { + t.Fatal(err) + } + + pr := &ProjectRule{ + UseFilePath: true, + Rules: []ProjectRuleEntry{ + {Path: "*.go", Rule: "rule.md"}, + }, + } + resolveRuleFiles(pr, dir) + + if pr.Rules[0].Rule != mdContent { + t.Errorf("expected file content only, got %q", pr.Rules[0].Rule) + } +} + +func TestResolveRuleFiles_Security(t *testing.T) { + dir := t.TempDir() + pr := &ProjectRule{ + UseFilePath: true, + Rules: []ProjectRuleEntry{ + {Path: "*.go", Rule: "../outside.md"}, + }, + } + resolveRuleFiles(pr, dir) + if pr.Rules[0].Rule != "../outside.md" { + t.Errorf("expected rule to remain unchanged due to security violation, got %q", pr.Rules[0].Rule) + } +} + +func TestResolveRuleFiles_UnsupportedExtension(t *testing.T) { + dir := t.TempDir() + os.WriteFile(filepath.Join(dir, "rule.json"), []byte("{}"), 0644) + pr := &ProjectRule{ + UseFilePath: true, + Rules: []ProjectRuleEntry{ + {Path: "*.go", Rule: "rule.json"}, + }, + } + resolveRuleFiles(pr, dir) + if pr.Rules[0].Rule != "rule.json" { + t.Errorf("expected rule to remain unchanged due to unsupported extension, got %q", pr.Rules[0].Rule) + } +} + +func TestResolveRuleFiles_TooLarge(t *testing.T) { + dir := t.TempDir() + largeContent := strings.Repeat("a", 101*1024) + os.WriteFile(filepath.Join(dir, "large.md"), []byte(largeContent), 0644) + pr := &ProjectRule{ + UseFilePath: true, + Rules: []ProjectRuleEntry{ + {Path: "*.go", Rule: "large.md"}, + }, + } + resolveRuleFiles(pr, dir) + if pr.Rules[0].Rule != "large.md" { + t.Errorf("expected rule to remain unchanged due to large file, got %q", pr.Rules[0].Rule) + } +} + +func TestResolveRuleFiles_MissingFile(t *testing.T) { + dir := t.TempDir() + pr := &ProjectRule{ + UseFilePath: true, + Rules: []ProjectRuleEntry{ + {Path: "*.go", Rule: "missing.md"}, + }, + } + resolveRuleFiles(pr, dir) + if pr.Rules[0].Rule != "missing.md" { + t.Errorf("expected original rule to be kept, got %q", pr.Rules[0].Rule) + } +}