feat(#143): fetch doc-map config from trusted VCS ref
CI / test (pull_request) Successful in 18s
CI / review (anthropic--claude-4.6-sonnet, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 25s
CI / review (gpt-5, security, ., rodin/security-patterns, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 1m5s
CI / review (gpt-5, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 1m17s

The doc-map YAML config was previously read from the local workspace
(the PR branch checkout). A malicious PR author could modify
.review-bot/doc-map.yml to map any path glob to sensitive design docs,
causing review-bot to fetch and inject those docs into the LLM prompt.

Fix: add --doc-map-trusted-ref (DOC_MAP_TRUSTED_REF) flag. When set to
a trusted ref (e.g. 'main'), the doc-map config is fetched from the VCS
API at that ref instead of from local workspace. A 404 from VCS is a
hard error (no silent fallback to local copy).

When unset, the local workspace is used with a security warning in the
logs pointing operators to the new flag.

Changes:
- review/docmap.go: add ParseDocMapConfigContent + parseDocMapBytes
  helper to parse from in-memory content (fetched via VCS API)
- cmd/review-bot/main.go: add --doc-map-trusted-ref flag; Step 6c
  branches on trusted-ref to fetch vs local-workspace load
- .gitea/actions/review/action.yml: add doc-map-trusted-ref input
- README.md: document new input
- CHANGELOG.md: security and feature entries

Tests:
- TestParseDocMapConfigContent_Valid/Empty/InvalidYAML/UnknownKeys
  in review/docmap_test.go

Coverage: 53.0% cmd/review-bot
This commit is contained in:
Rodin
2026-05-15 08:34:26 +00:00
parent 30fe48d265
commit a4f04897b6
6 changed files with 145 additions and 14 deletions
+60
View File
@@ -510,3 +510,63 @@ func TestFileCoveredByDocMap_EmptyConfig(t *testing.T) {
t.Error("expected false for empty config, got true")
}
}
// ============================================================
// ParseDocMapConfigContent
// ============================================================
func TestParseDocMapConfigContent_Valid(t *testing.T) {
content := `
mappings:
- paths:
- "lib/foo/**"
docs:
- docs/foo.md
`
cfg, err := ParseDocMapConfigContent(content, "owner/repo@main:.review-bot/doc-map.yml")
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if len(cfg.Mappings) != 1 {
t.Fatalf("expected 1 mapping, got %d", len(cfg.Mappings))
}
if len(cfg.Mappings[0].Docs) != 1 || cfg.Mappings[0].Docs[0] != "docs/foo.md" {
t.Errorf("unexpected mapping: %+v", cfg.Mappings[0])
}
}
func TestParseDocMapConfigContent_EmptyContent(t *testing.T) {
cfg, err := ParseDocMapConfigContent("", "test-source")
if err != nil {
t.Fatalf("unexpected error for empty content: %v", err)
}
if len(cfg.Mappings) != 0 {
t.Errorf("expected 0 mappings for empty content, got %d", len(cfg.Mappings))
}
}
func TestParseDocMapConfigContent_InvalidYAML(t *testing.T) {
_, err := ParseDocMapConfigContent("mappings: [{{invalid", "test-source")
if err == nil {
t.Fatal("expected error for invalid YAML, got nil")
}
}
func TestParseDocMapConfigContent_UnknownKeys(t *testing.T) {
content := `
mappings:
- paths:
- "lib/**"
docs:
- docs/foo.md
unknown_top_level_key: "should be warned but not fatal"
`
// Unknown top-level keys produce a warning but not an error.
cfg, err := ParseDocMapConfigContent(content, "test-source")
if err != nil {
t.Fatalf("unexpected error for unknown keys: %v", err)
}
if len(cfg.Mappings) == 0 {
t.Error("expected mappings to be parsed despite unknown key")
}
}