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

@@ -5,12 +5,16 @@
 ### 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))

DEV_LOOP_STATUS.md

@@ -1,68 +1,96 @@
-# Dev Loop Status — 2026-05-15 11:58 UTC
+# Dev Loop Status — 2026-05-15 09:37 UTC
 
-**Cron ID:** 5342ac81-4bbc-4e4c-a123-347a7788d50c  
+## Summary
-**Status:** ✅ HEALTHY — All tests passing, repo clean, ready for review & merge
 
-## 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 (d855064)
+---
-- **Tests:** All passing ✅ (7 packages, 80+ test cases, race detector clean)
-- **Test coverage:** **77.1%** overall
-  - budget: 92.0%
-  - review: 92.0%
-  - gitea: 85.2%
-  - github: 86.3%
-  - llm: 81.3%
-  - netutil: 85.7%
-  - cmd/review-bot: 54.3%
-- **Working tree:** Clean (no uncommitted changes)
 
-## PR Status & Recommended Actions
+## Current State
 
-### Ready to Merge (3 PRs)
+### Main Branch
-These have `ready` label, passing tests, and are self-reviewed. Recommend merging in order:
+- **HEAD:** 1650343 (dev-loop cycle complete)
+- **Status:** Clean, all tests passing, 77.1% coverage
+- **Recent work:** Issue #130 fixes merged and verified complete
 
-| Order | PR | Issue | Type | Size | Status |
+### Active Issue Branches (Ready for Review)
-|-------|----|----|------|------|--------|
-| 1️⃣ | #155 | #154 | Refactor | M | ✅ Ready |
-| 2️⃣ | #152 | #150 | Security | S | ✅ Ready |
-| 3️⃣ | #151 | #146 | Test | S | ✅ Ready |
 
-**Merge strategy:** Sequential. All currently passing; no blocking dependencies.
+| 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 |
 
-### Awaiting AI-Review (2 PRs)
+### Priority Assessment
-These have passing tests and self-review but need ai-review before marking ready:
 
-| PR | Issue | Type | Size | Notes |
+**High Priority (Security/Risk):**
-|----|-------|------|------|-------|
+- **#150** — EvalSymlinks for dir-symlink bypass (security fix)
-| #156 | #141 | Feature | M | `validate-docmap` subcommand |
+- **#143** — Fetch doc-map from trusted VCS ref (trust boundary)
-| #153 | #143 | Feature | M | Fetch doc-map from VCS |
 
-## Dev Loop Health
+**Medium Priority (Feature):**
+- **#146** — Path resolution optimization + tests
 
-| Metric | Status | Details |
+**Low Priority (Cleanup):**
-|--------|--------|---------|
+- **#154** — Test refactoring
-| Main branch | ✅ Current | d855064 (2026-05-15 11:44 UTC) |
-| Working tree | ✅ Clean | Ready for fetch/merge |
-| Test suite | ✅ All pass | 7 packages, 80+ cases, ~2s runtime |
-| Race detector | ✅ Clean | No race conditions detected |
-| Coverage | ✅ 77.1% | Stable, no regressions |
-| Remotes | ✅ Current | origin/main up-to-date |
 
-## Recommendations
+---
 
-1. **[IMMEDIATE] Merge 3 ready PRs** (#155 → #152 → #151)
+## Coverage Trends
-   - All provide foundational support for downstream features
-   - Safe to merge in sequence; no cross-PR dependencies
-   - Post-merge: dev-loop can run verification cycle
 
-2. **Schedule AI-review for #156 and #153**
+| Package | Current | Previous | Δ |
-   - Both feature-complete and test-passing
+|---------|---------|----------|---|
-   - Waiting on code quality & design review
+| 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%** |
 
-## Cycle Complete ✅
+---
 
-Next dev-loop cycle will:
+## Recommendations for Next Cycle
-- Verify post-merge state
+
-- Update coverage tracking
+### Immediate (This Dev-Loop)
-- Monitor awaiting-review PRs for AI review status
+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.