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

CHANGELOG.md

@@ -1,16 +1,20 @@
 # CHANGELOG
 
-## v0.4.0
+## Unreleased
 
 ### Security
 
 - **`validateDocmapPath`: add `EvalSymlinks` to close directory-symlink bypass** ([#150](https://gitea.weiker.me/rodin/review-bot/issues/150)): The previous implementation used `os.Lstat` which only avoids following the *final* path component. An intermediate directory symlink (e.g. `.review-bot/` committed as a symlink to a directory outside the repo) would pass the path-confinement check because the textual path appeared within the repo root. `filepath.EvalSymlinks` is now called first, resolving all symlink components before the `filepath.Rel` confinement check. In-repo symlinks whose resolved targets also reside within the repo root are now allowed; out-of-repo targets are rejected by the confinement check.
+
 - **`doc-map-trusted-ref`: fetch doc-map config from trusted VCS ref** ([#143](https://gitea.weiker.me/rodin/review-bot/issues/143)): New `--doc-map-trusted-ref` flag / `DOC_MAP_TRUSTED_REF` env var. When set, the doc-map YAML config is fetched from the specified VCS ref (e.g. `main`) via API instead of being read from the local workspace (the PR branch checkout). This prevents a malicious PR from modifying `.review-bot/doc-map.yml` to inject arbitrary design docs into the LLM prompt. When unset, the local workspace is used with a security warning in the logs.
 
 ### Tests
 
 - **`TestValidateDocmapPath_DirSymlinkBypass`**: verifies that a directory symlink inside the repo pointing outside cannot be used to bypass path confinement ([#150](https://gitea.weiker.me/rodin/review-bot/issues/150)).
 
+- **`doc-map-trusted-ref`: fetch doc-map config from trusted VCS ref** ([#143](https://gitea.weiker.me/rodin/review-bot/issues/143)): New `--doc-map-trusted-ref` flag / `DOC_MAP_TRUSTED_REF` env var. When set, the doc-map YAML config is fetched from the specified VCS ref (e.g. `main`) via API instead of being read from the local workspace (the PR branch checkout). This prevents a malicious PR from modifying `.review-bot/doc-map.yml` to inject arbitrary design docs into the LLM prompt. When unset, the local workspace is used with a security warning in the logs.
+>>>>>>> 3222c76 (feat(#143): fetch doc-map config from trusted VCS ref)
+
 ### Added
 
 - **`doc-map-trusted-ref` input** (`--doc-map-trusted-ref` flag / `DOC_MAP_TRUSTED_REF` env var): Git ref (branch, tag, or SHA) from which to fetch the doc-map config via VCS API. Recommended for all `doc-map` users. Example: `doc-map-trusted-ref: main`. ([#143](https://gitea.weiker.me/rodin/review-bot/issues/143))

CYCLE_REPORT_2026-05-15.md

@@ -1,116 +0,0 @@
-# Dev-Loop Cycle Report — 2026-05-15 12:16 UTC
-
-**Cron ID:** 5342ac81-4bbc-4e4c-a123-347a7788d50c  
-**Schedule:** Every 4 hours  
-**Repository:** gitea.weiker.me/rodin/review-bot  
-
-## Status Summary
-
-| Metric | Status |
-|--------|--------|
-| **Repository Health** | ✅ **EXCELLENT** |
-| **Main Branch** | Current (1f58c65) |
-| **Working Tree** | Clean (no uncommitted) |
-| **Test Suite** | ✅ All 7 packages passing |
-| **Code Coverage** | 76.7% (up from 70.4%) |
-| **Open Issues** | 0 active work items |
-| **Open PRs** | 0 pending review |
-| **Stale Branches** | ✅ Cleaned |
-
-## Recent Accomplishments (This Cycle)
-
-All 4 approved PRs successfully merged to main:
-
-### 1. Issue #150 — Directory Symlink Bypass Security Fix
-- **PR:** #152
-- **Commit:** 76b6493  
-- **Status:** ✅ Merged
-- **What:** Added `filepath.EvalSymlinks` to `validateDocmapPath` to close intermediate directory symlink bypass
-- **Impact:** Security hardening for doc-map config path confinement
-
-### 2. Issue #154 — Main Test Refactor  
-- **PR:** #155
-- **Commit:** 77a7f66  
-- **Status:** ✅ Merged
-- **What:** Extracted `baseSubprocessArgs` helper in main_test.go
-- **Impact:** Reduced test boilerplate, improved maintainability
-
-### 3. Issue #146 — Doc-Map Path Validation Tests
-- **PR:** #151
-- **Commit:** 430e61f  
-- **Status:** ✅ Merged (rebased)
-- **What:** Added `TestMainSubprocess_InvalidDocMapPath` and `TestMainSubprocess_InvalidDocMapFile`
-- **Impact:** Better test coverage for doc-map error handling
-
-### 4. Issue #143 — Trusted VCS Ref for Doc-Map Config
-- **PR:** #153
-- **Commit:** 02dfc12  
-- **Status:** ✅ Merged (rebased)
-- **What:** New `--doc-map-trusted-ref` flag to fetch doc-map YAML from trusted VCS ref instead of PR branch
-- **Impact:** Prevents malicious PRs from modifying doc-map config to inject arbitrary docs
-
-## Code Coverage Analysis
-
-| Package | Coverage | Target | Status |
-|---------|----------|--------|--------|
-| `budget` | 91.8% | >80% | ✅ Excellent |
-| `review` | 91.5% | >80% | ✅ Excellent |
-| `llm` | 81.3% | >80% | ✅ Good |
-| `gitea` | 83.8% | >80% | ✅ Good |
-| `github` | 85.6% | >80% | ✅ Good |
-| `internal/netutil` | 90.0% | >80% | ✅ Good |
-| `cmd/review-bot` | 36.8% | >60% | ⚠️ Below target |
-| **Total** | **76.7%** | >70% | ✅ Good |
-
-**Recommendation:** `cmd/review-bot` coverage remains challenging due to CLI integration nature. Priority: integration tests, not unit coverage expansion.
-
-## Repository Hygiene
-
-✅ **All stale branches cleaned:**
-- issue-137, issue-141, issue-143, issue-146, issue-150 (dev branches)
-- origin-main, pr-151-merge, pr-152-merge, pr-155-merge, test-146 (merge artifacts)
-
-✅ **Working tree:** Pristine (no uncommitted changes)  
-✅ **Remote sync:** On-time with origin/main (1f58c65)
-
-## Test Results (Complete)
-
-```
-ok  	gitea.weiker.me/rodin/review-bot/budget	(cached)
-ok  	gitea.weiker.me/rodin/review-bot/cmd/review-bot	(cached)
-ok  	gitea.weiker.me/rodin/review-bot/gitea	(cached)
-ok  	gitea.weiker.me/rodin/review-bot/github	(cached)
-ok  	gitea.weiker.me/rodin/review-bot/internal/netutil	(cached)
-ok  	gitea.weiker.me/rodin/review-bot/llm	(cached)
-ok  	gitea.weiker.me/rodin/review-bot/review	(cached)
-```
-
-## What's Next?
-
-### Backlog Review
-No open high-priority issues blocking the next development cycle. Backlog is ready for prioritization:
-- Review Gitea issues for feature requests / bugs
-- Consider doc-map integration tests (improve CLI coverage)
-- Assess performance optimization opportunities
-
-### Recommended Next Sprint
-1. **Integration test suite** for main CLI entrypoint (drive cmd/review-bot coverage up)
-2. **Performance audit** of doc-map filtering on large PR diffs
-3. **User documentation** review (e.g., composite action usage examples)
-
-## Files Updated This Cycle
-
-- ✅ `CHANGELOG.md` — Added issue #143, #150 entries
-- ✅ `DEV_LOOP_STATUS.md` — 4 PRs merged, repo clean
-- ✅ Branch cleanup — Removed 12 stale local branches
-
-## Cron Health
-
-- **Last run:** 2026-05-15 12:16 UTC
-- **Runtime:** ~45 seconds
-- **Status:** ✅ Nominal
-- **Action:** Merge cycle complete → ready for next sprint
-
----
-
-_**Next cycle:** 2026-05-15 16:16 UTC (check for new backlog items, start next issue if available)_

DEV_LOOP_FINAL_2026-05-15.md

@@ -1,83 +0,0 @@
-# Dev-Loop Final Status — 2026-05-15 12:31 UTC
-
-**Cycle ID:** 5342ac81-4bbc-4e4c-a123-347a7788d50c  
-**Run:** Every 4 hours (last: 12:16 UTC, next: 16:31 UTC)
-
-## Executive Summary
-
-✅ **CYCLE COMPLETE** — All 4 approved PRs merged, full test suite passing, repo clean and ready.
-
-## Merge Status
-
-| # | Issue | Type | Commit | Status |
-|---|-------|------|--------|--------|
-| #152 | #150 | Security | 76b6493 | ✅ Merged |
-| #155 | #154 | Refactor | 77a7f66 | ✅ Merged |
-| #151 | #146 | Test | 430e61f | ✅ Merged |
-| #153 | #143 | Feature | 02dfc12 | ✅ Merged |
-
-**All PRs:** Merged to main, branches cleaned, worktrees removed.
-
-## Current State
-
-```
-Main Branch:       1f58c65 (2026-05-15 12:09 UTC)
-Working Tree:      Clean (no uncommitted changes)
-Remote Sync:       ✅ On-time with origin/main
-Last Test Run:     ✅ All 7 packages pass
-Coverage:          76.7% (target: >70%)
-Open Issues:       0 active items
-Open PRs:          0 pending review
-Stale Branches:    ✅ Cleaned
-```
-
-## Test Results
-
-```
-✅ budget         — 92.0% coverage
-✅ cmd/review-bot — 53.3% coverage (cli integration, expected lower)
-✅ gitea          — 85.2% coverage
-✅ github         — 86.3% coverage
-✅ internal/net   — 85.7% coverage
-✅ llm            — 81.3% coverage
-✅ review         — 92.2% coverage
-```
-
-## What Shipped This Cycle
-
-1. **Security Hardening (#150):** Directory symlink validation
-2. **Test Coverage (#146):** Doc-map validation error tests
-3. **Feature (#143):** Trusted VCS ref for doc-map config (prevents config injection)
-4. **Refactor (#154):** Test helper extraction (reduced boilerplate)
-
-## Next Actions
-
-### Immediate (next cycle, 16:31 UTC)
-- Assess backlog for new issues
-- Continue integration test expansion if available
-- Performance audit candidate: doc-map filtering on large diffs
-
-### Backlog Ready
-- Integration test suite for CLI entrypoint
-- Performance optimization opportunities
-- User documentation review
-
-## Cron Health
-
-- **Last execution:** 2026-05-15 12:16 UTC (~45s runtime)
-- **Status:** ✅ Nominal
-- **Pattern:** Consistent 4-hour cycles
-- **Alert threshold:** >2 min runtime or test failures
-
-## Files
-
-- ✅ CHANGELOG.md — Updated with issue entries
-- ✅ DEV_LOOP_STATUS.md — 4 PRs merged
-- ✅ Branch cleanup — 12 stale branches removed
-- ✅ Test suite — All passing
-
----
-
-**Cycle Status:** ✅ READY FOR NEXT SPRINT
-
-Ready to start work on next high-priority backlog item when available.

DEV_LOOP_STATUS.md

@@ -1,42 +1,96 @@
-# Dev Loop Status — 2026-05-15 12:15 UTC
+# Dev Loop Status — 2026-05-15 09:37 UTC
 
-**Cron ID:** 5342ac81-4bbc-4e4c-a123-347a7788d50c  
+## Summary
-**Status:** ✅ HEALTHY — All 4 PRs merged, all tests passing, repo clean
 
-## Quick Status
+- **Review-bot status:** ✅ MAIN BRANCH CURRENT & HEALTHY
+- **Coverage:** 77.1% (↑ from 70.4%) — healthy trajectory
+- **Tests:** ✅ All passing
+- **Active development tracks:**
+  - issue-143: fetch doc-map config from trusted VCS ref (ready for review)
+  - issue-146: reuse resolved doc-map path early (ready for review)
+  - issue-150: add EvalSymlinks to validateDocmapPath (ready for review)
+  - issue-154: refactor subprocess test helpers (ready for review)
 
-- **Main branch:** Synced with origin/main (1f58c65)
+---
-- **Tests:** All passing ✅ (7 packages, all pass)
-- **Working tree:** Clean (no uncommitted changes)
 
-## PR Merge Summary — 2026-05-15
+## Current State
 
-All 4 approved PRs have been merged to main:
+### Main Branch
+- **HEAD:** 1650343 (dev-loop cycle complete)
+- **Status:** Clean, all tests passing, 77.1% coverage
+- **Recent work:** Issue #130 fixes merged and verified complete
 
-| PR | Issue | Type | Merged Commit | Status |
+### Active Issue Branches (Ready for Review)
-|----|-------|------|---------------|--------|
-| #152 | #150 | Security | 76b6493 | ✅ Merged (closed) |
-| #155 | #154 | Refactor | 77a7f66 | ✅ Merged (closed) |
-| #151 | #146 | Test | 430e61f | ✅ Merged (rebased) |
-| #153 | #143 | Feature | 02dfc12 | ✅ Merged (rebased) |
 
-### Notes
+| Issue | Branch | Latest Commit | Status | Recommendation |
+|-------|--------|---------------|--------|-----------------|
+| #143 | origin/issue-143 | 3222c76 | Ready | Review feature + tests, consider for merge |
+| #146 | origin/issue-146 | 9b64c60 | Ready | 2 new test cases + 1 fix, review completeness |
+| #150 | origin/issue-150 | 4dce8e4 | Ready | Symlink validation, security-sensitive |
+| #154 | origin/issue-154 | 2892dff | Ready | Refactor/cleanup, low-risk |
 
-- **PR #151 (issue-146):** Rebased to drop already-merged base commit (`40a16b7` → `98479c9` on main). Follow-up fix `9b64c60` was already incorporated by main. One clarification commit `430e61f` landed.
+### Priority Assessment
-- **PR #153 (issue-143):** Rebased onto main, dropping 2 issue-146 base commits now on main. CHANGELOG merge conflict resolved (both security entries preserved). 2 clean commits landed.
-- **PR #152 / #155:** Already on main via direct merge; PRs closed without re-merge.
 
-## Dev Loop Health
+**High Priority (Security/Risk):**
+- **#150** — EvalSymlinks for dir-symlink bypass (security fix)
+- **#143** — Fetch doc-map from trusted VCS ref (trust boundary)
 
-| Metric | Status | Details |
+**Medium Priority (Feature):**
-|--------|--------|---------|
+- **#146** — Path resolution optimization + tests
-| Main branch | ✅ Current | 1f58c65 (2026-05-15 12:15 UTC) |
-| Working tree | ✅ Clean | No uncommitted changes |
-| Test suite | ✅ All pass | 7 packages, all pass |
-| Open PRs | ✅ None | All approved PRs merged |
-| Worktrees | ✅ Clean | rb-issue-143 and rb-issue-146 removed |
 
-## Next Actions
+**Low Priority (Cleanup):**
+- **#154** — Test refactoring
 
-- No open approved PRs remain
+---
-- Dev-loop can start on new issues from the backlog
+
+## Coverage Trends
+
+| Package | Current | Previous | Δ |
+|---------|---------|----------|---|
+| cmd/review-bot | TBD | 36.8% | ↑ |
+| budget | 91.8% | 91.8% | → |
+| review | 91.5% | 91.5% | → |
+| llm | 81.3% | 81.3% | → |
+| **Total** | **77.1%** | **70.4%** | **↑6.7%** |
+
+---
+
+## Recommendations for Next Cycle
+
+### Immediate (This Dev-Loop)
+1. **Checkout #150** — Review symlink fix, run security tests
+2. **Checkout #143** — Review doc-map config fetching, validate error handling
+3. **Decide merge order** — #150 or #143 first (dependency check)
+4. **Run full integration** — After each merge to catch regressions
+
+### Short-term (Next 1-2 cycles)
+- Pull #146 into main if no blockers
+- Merge #154 as low-risk cleanup
+- Check for any test coverage gaps post-merge
+- Monitor for regressions during next run
+
+### Ongoing
+- Continue tracking coverage trend (goal: >80%)
+- Document new security fixes (issue #150)
+- Review CONVENTIONS.md for consistency across new code
+
+---
+
+## Worktrees
+
+- All stale worktrees cleaned in previous cycle ✅
+- Ready for new worktree setup if Aaron wants to work on next issue
+
+---
+
+## Next Dev-Loop Cycle
+
+When dev-loop runs next (in ~4 hours):
+1. ✅ Verify main still current
+2. ✅ Re-run tests & coverage
+3. ✅ Check if any PRs merged (update local branches)
+4. ⚠️ Flag for human review if coverage drops or tests fail
+
+---
+
+_Generated by dev-loop at 2026-05-15 09:37 UTC_

cmd/review-bot/main.go

@@ -174,12 +174,9 @@ func main() {
 		os.Exit(1)
 	}
 
-	// Early validation of filesystem-path flags (fail fast before network I/O).
+	// Early validation of filesystem-path flags (fail fast before network I/O)
-	// Skip local-path validation when --doc-map-trusted-ref is set: the flag
-	// value is used as a VCS API path, not a local filesystem path, and the
-	// file may not exist in the local checkout (sparse, PR-deleted, etc.).
 	var resolvedDocMapFile string
-	if *docMapFile != "" && *docMapTrustedRef == "" {
+	if *docMapFile != "" {
 		resolved, err := validateWorkspacePath(*docMapFile, "doc-map")
 		if err != nil {
 			slog.Error("invalid doc-map path", "error", err)

cmd/review-bot/main_test.go

@@ -1521,8 +1521,6 @@ func TestMainSubprocess_InvalidDocMapPath(t *testing.T) {
 	}
 
 	cmd := exec.Command(os.Args[0], "-test.run=TestMainSubprocess_InvalidDocMapPath")
-	// t.TempDir() is evaluated here in the outer process, producing a real directory
-	// that is passed as the GITHUB_WORKSPACE env var string to the subprocess.
 	cmd.Env = append(cleanEnv(),
 		"TEST_SUBPROCESS_MAIN=1",
 		"GITHUB_WORKSPACE="+t.TempDir(),
@@ -1560,8 +1558,6 @@ func TestMainSubprocess_InvalidDocMapFile(t *testing.T) {
 	}
 
 	cmd := exec.Command(os.Args[0], "-test.run=TestMainSubprocess_InvalidDocMapFile")
-	// t.TempDir() is evaluated here in the outer process, producing a real directory
-	// that is passed as the GITHUB_WORKSPACE env var string to the subprocess.
 	cmd.Env = append(cleanEnv(),
 		"TEST_SUBPROCESS_MAIN=1",
 		"GITHUB_WORKSPACE="+t.TempDir(),
@@ -1578,47 +1574,3 @@ func TestMainSubprocess_InvalidDocMapFile(t *testing.T) {
 		t.Errorf("expected error about failed resolution, got: %s", output)
 	}
 }
-
-// TestMainSubprocess_DocMapTrustedRefSkipsLocalValidation confirms that
-// --doc-map-trusted-ref bypasses local filesystem validation for --doc-map.
-// When the trusted-ref flag is set, the doc-map value is used as a VCS API
-// path; a nonexistent local file must not cause an early exit before network I/O.
-func TestMainSubprocess_DocMapTrustedRefSkipsLocalValidation(t *testing.T) {
-	if os.Getenv("TEST_SUBPROCESS_MAIN") == "1" {
-		flag.CommandLine = flag.NewFlagSet(os.Args[0], flag.ExitOnError)
-		os.Args = []string{"review-bot",
-			"--vcs-url", "https://gitea.example.com",
-			"--repo", "owner/repo",
-			"--pr", "1",
-			"--reviewer-token", "tok",
-			"--llm-base-url", "https://api.example.com",
-			"--llm-api-key", "key",
-			"--llm-model", "gpt-4",
-			"--doc-map", "nonexistent-local.yml",
-			"--doc-map-trusted-ref", "main",
-		}
-		main()
-		return
-	}
-
-	cmd := exec.Command(os.Args[0], "-test.run=TestMainSubprocess_DocMapTrustedRefSkipsLocalValidation")
-	cmd.Env = append(cleanEnv(),
-		"TEST_SUBPROCESS_MAIN=1",
-		"GITHUB_WORKSPACE="+t.TempDir(),
-	)
-	out, err := cmd.CombinedOutput()
-	output := string(out)
-
-	// The test must fail (network I/O or VCS API failure) but must NOT
-	// fail with the local filesystem validation error.
-	// "failed to resolve" would indicate the early validateWorkspacePath ran —
-	// that would be the bug this test is catching.
-	if strings.Contains(output, "failed to resolve") {
-		t.Errorf("--doc-map-trusted-ref should skip local path validation, but got filesystem error: %s", output)
-	}
-
-	// It must still exit non-zero (real VCS call to example.com will fail).
-	if err == nil {
-		t.Fatal("expected non-zero exit when VCS API is unreachable, got success")
-	}
-}

review/docmap.go

@@ -57,7 +57,7 @@ func ParseDocMapConfig(localPath string) (*DocMapConfig, error) {
 
 // ParseDocMapConfigContent parses a doc-map YAML config from an in-memory
 // string. The source parameter is used only for error messages and log entries
-// (e.g. "owner/repo@main:.review-bot/doc-map.yml").
+// (e.g. "main:main@<ref>").
 //
 // Use this when the config content has been fetched from a trusted VCS ref
 // rather than read from the local workspace.