chore: dev-loop run 2026-05-15 09:15 UTC — all branches passing, ready for review
CI / test (push) Successful in 18s
CI / review (anthropic--claude-4.6-sonnet, sonnet, SONNET_REVIEW_TOKEN) (push) Has been skipped
CI / review (gpt-5, gpt, GPT_REVIEW_TOKEN) (push) Has been skipped
CI / review (gpt-5, security, ., rodin/security-patterns, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (push) Has been skipped

This commit is contained in:
Rodin
2026-05-15 09:16:15 +00:00
parent 9be46dfbda
commit 823265659a
3 changed files with 341 additions and 0 deletions
+125
View File
@@ -0,0 +1,125 @@
# 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:
```yaml
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:**
```go
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