feat(#143): fetch doc-map config from trusted VCS ref

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 b09f12b8ff
commit b01e3c487f
6 changed files with 137 additions and 9 deletions
+44 -7
View File
@@ -101,6 +101,7 @@ func main() {
aicoreResourceGroup := flag.String("aicore-resource-group", envOrDefault("AICORE_RESOURCE_GROUP", "default"), "SAP AI Core resource group (for provider=aicore)")
docMapFile := flag.String("doc-map", envOrDefault("DOC_MAP_FILE", ""), "Path to YAML file mapping source path globs to governing design docs")
docMapMaxBytes := flag.Int("doc-map-max-bytes", envOrDefaultInt("DOC_MAP_MAX_BYTES", review.DefaultDocMapMaxBytes), "Maximum bytes of injected doc content (default 102400)")
docMapTrustedRef := flag.String("doc-map-trusted-ref", envOrDefault("DOC_MAP_TRUSTED_REF", ""), "Git ref (e.g. main) to fetch the doc-map config from via VCS API instead of local workspace. Recommended to prevent PR branch from controlling which docs are injected.")
flag.Parse()
@@ -368,10 +369,45 @@ func main() {
// Step 6c: Load path-scoped design docs if doc-map specified
designDocs := ""
if *docMapFile != "" {
docMapCfg, err := review.ParseDocMapConfig(resolvedDocMapFile)
if err != nil {
slog.Error("failed to parse doc-map file", "file", *docMapFile, "error", err)
os.Exit(1)
var docMapCfg *review.DocMapConfig
if *docMapTrustedRef != "" {
// Fetch doc-map config from a trusted VCS ref (e.g. the default branch).
// This prevents a malicious PR from modifying the doc-map config to
// inject arbitrary docs into the LLM prompt.
slog.Info("doc-map: fetching config from trusted ref",
"path", *docMapFile,
"ref", *docMapTrustedRef)
content, fetchErr := vcs.GetFileContentRef(ctx, owner, repoName, *docMapFile, *docMapTrustedRef)
if fetchErr != nil {
slog.Error("doc-map: failed to fetch config from trusted ref",
"path", *docMapFile,
"ref", *docMapTrustedRef,
"error", fetchErr)
os.Exit(1)
}
source := fmt.Sprintf("%s/%s@%s:%s", owner, repoName, *docMapTrustedRef, *docMapFile)
var parseErr error
docMapCfg, parseErr = review.ParseDocMapConfigContent(content, source)
if parseErr != nil {
slog.Error("doc-map: failed to parse fetched config",
"source", source,
"error", parseErr)
os.Exit(1)
}
} else {
// Local workspace fallback — the doc-map is read from the PR branch checkout.
// SECURITY WARNING: a malicious PR can modify this file to inject arbitrary
// docs. Set --doc-map-trusted-ref (or DOC_MAP_TRUSTED_REF) to a trusted ref
// (e.g. "main") to fetch the config from the default branch instead.
slog.Warn("doc-map: loading config from local workspace (PR branch) — " +
"set --doc-map-trusted-ref to fetch from a trusted ref for security")
var parseErr error
docMapCfg, parseErr = review.ParseDocMapConfig(resolvedDocMapFile)
if parseErr != nil {
slog.Error("failed to parse doc-map file", "file", *docMapFile, "error", parseErr)
os.Exit(1)
}
}
// Collect changed file paths from the PR for intersection.
@@ -385,10 +421,11 @@ func main() {
if len(matchedDocs) > 0 {
docMapOpts := review.DocMapOptions{MaxBytes: *docMapMaxBytes}
designDocs, err = review.LoadMatchingDocs(ctx, vcs, owner, repoName, matchedDocs, docMapOpts)
if err != nil {
var loadErr error
designDocs, loadErr = review.LoadMatchingDocs(ctx, vcs, owner, repoName, matchedDocs, docMapOpts)
if loadErr != nil {
// Non-fatal: individual missing files are already warned; log and continue.
slog.Warn("doc-map: partial failure loading docs", "error", err)
slog.Warn("doc-map: partial failure loading docs", "error", loadErr)
}
if designDocs != "" {
slog.Info("doc-map: injected design docs", "matched", len(matchedDocs), "bytes", len(designDocs))