review-bot/PLAN-143.md

PLAN-143: Load doc-map config from trusted (default) branch

Issue: #143
Status: Planning
Branch: TBD (issue-143)

---

Problem Statement

The --doc-map flag reads the doc-map YAML config from the local GITHUB_WORKSPACE checkout, which is the PR branch in CI. A malicious PR author can:

1. Modify .review-bot/doc-map.yml in their branch to map any path glob to sensitive docs
2. review-bot reads the PR-branch doc-map config
3. Docs from the default branch are fetched and injected into the LLM prompt
4. Via prompt injection in those docs, the attacker could exfiltrate content

The config is the trust boundary. The data fetched (design docs) already comes from the default branch via VCS API. The config is what needs to be pinned to the default branch.

Constraints

• Must not break existing callers (backward compatibility)
• Should have a clearly named flag/env var
• Fall back to local workspace if no trusted ref configured (for users not yet migrated)
• The gargoyle workflow (.github/workflows/review.yml) will need updating

Proposed Approach

Option A: Fetch via VCS API from default branch (preferred)

Add a new flag --doc-map-trusted-ref (default: "" = use local workspace).

When --doc-map-trusted-ref is set:

1. Use the VCS API to fetch the file at --doc-map path from the specified ref
2. Parse the fetched content as YAML
3. Use this config (not the local workspace copy)

When --doc-map-trusted-ref is empty:

• Current behavior (local workspace) with a deprecation warning

This follows the same pattern as patterns-repo which fetches from VCS.

Option B: Auto-detect and always use default branch

Always fetch doc-map from the default branch via VCS API, ignoring local workspace. Simpler API but breaks local testing (where there's no VCS to fetch from).

Recommendation

Option A — explicit --doc-map-trusted-ref flag. The gargoyle workflow would set:

doc-map-trusted-ref: "main"

This is explicit and allows local testing to continue using local workspace.

Implementation Plan

Phase 1: VCS API fetch for doc-map config

Files to change:

• cmd/review-bot/main.go — add --doc-map-trusted-ref flag, conditional fetch logic
• review/docmap.go — add FetchDocMapConfig(vcs, owner, repo, ref, path string) (*DocMapConfig, error)
• action.yml — add doc-map-trusted-ref input
• README.md — document new flag

Logic:

if *docMapTrustedRef != "" {
    // Fetch from VCS (trusted branch) — secure
    content, err := vcs.GetFileContent(ctx, owner, repoName, *docMapTrustedRef, resolvedDocMap)
    ...
    docMapCfg, err = review.ParseDocMapConfigContent(content)
} else {
    // Local workspace (backward compat with deprecation warning)
    slog.Warn("doc-map loaded from local workspace (PR branch) — consider --doc-map-trusted-ref for security")
    docMapCfg, err = review.ParseDocMapConfig(resolvedDocMap)
}

Phase 2: Tests

• TestFetchDocMapConfig_Success: mock VCS returns valid YAML → parses correctly
• TestFetchDocMapConfig_NotFound: VCS returns 404 → clear error
• TestMainSubprocess_DocMapTrustedRef: subprocess test for the new flag

Phase 3: Gargoyle workflow update

Update .github/workflows/review.yml in gargoyle to add doc-map-trusted-ref: main.

State/Data Model

New flag: --doc-map-trusted-ref / DOC_MAP_TRUSTED_REF env var

• Type: string
• Default: "" (local workspace)
• Example value: "main", "master", HEAD

Error Cases

• VCS returns 404 for doc-map path at trusted ref → error + exit (not silent)
• VCS returns 404 but local copy exists → do NOT fall back (could be attack path)
• Parse error on fetched content → error + exit

Edge Cases

• What if the doc-map doesn't exist at the trusted ref? → log error, exit (don't silently continue)
• What if trusted-ref is a commit SHA? → should work via VCS GetFileContent
• What if the user sets trusted-ref to the PR branch? → Works, but defeats the purpose. Not our problem to prevent.

Open Questions

• Should we warn when --doc-map is set without --doc-map-trusted-ref? → Yes, deprecation warning pointing to docs
• Should we add --doc-map-trusted-ref to the validate-docmap subcommand? → No, that subcommand operates on local files only; it's a developer tool

Acceptance Criteria

• --doc-map-trusted-ref flag added to action.yml and cmd/review-bot/main.go
• When set, doc-map config fetched from VCS at the specified ref (not local workspace)
• When unset, local workspace used with deprecation warning in logs
• 404 from VCS is a hard error (no silent fallback to local copy)
• Tests cover: fetch success, fetch 404, parse error
• Gargoyle .github/workflows/review.yml updated to use doc-map-trusted-ref: main
• README updated
• CHANGELOG updated
• make precommit passes