Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

feat: support reading external files as Code Review rules via use_file_path #87

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
zephyrq-z wants to merge 4 commits into alibaba:main
base: main
Choose a base branch
Loading
from zephyrq-z:feature/rule
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 25 additions & 2 deletions README.ja-JP.md
View file Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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}`のブレース展開をサポートします。
- 各層の中では、ルールは宣言順に評価されます — 最初にマッチしたものが採用されます。
- ルールファイルが存在しない場合は、何も出力せずスキップされます。

### パスフィルタリング

Expand Down
27 changes: 25 additions & 2 deletions README.ko-KR.md
View file Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
25 changes: 23 additions & 2 deletions README.md
View file Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
27 changes: 24 additions & 3 deletions README.zh-CN.md
View file Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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。
- 在每一层内,规则按声明顺序评估 —— 首次匹配生效。
- 如果规则文件不存在,将被静默跳过。
- 如果规则文件不存在,将生成警告并被跳过。

### 路径过滤

Expand Down
106 changes: 101 additions & 5 deletions internal/config/rules/system_rules.go
View file Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,9 +182,10 @@ type ProjectRuleEntry struct {

// ProjectRule holds rules loaded from <repoDir>/.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
Expand Down Expand Up @@ -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) {
Expand All @@ -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
}

Expand All @@ -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) {
Expand All @@ -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
}

Expand Down
80 changes: 80 additions & 0 deletions internal/config/rules/system_rules_test.go
View file Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

AltStyle によって変換されたページ (->オリジナル) /