nit(#150): clarify why resolved != symlinkPath in InRepoSymlinkAllowed test

Add a comment explaining that validateDocmapPath calls EvalSymlinks
internally, so the returned path is always the fully-resolved real path
and can never equal the symlink entry itself.

Addresses sonnet bot NIT (review 4810) against d6bab7a9.

cmd/review-bot/validatedocmap.go

@@ -50,8 +50,8 @@ func validateDocmapPath(localPath, resolvedRoot string) (string, error) {
 		return "", fmt.Errorf("cannot resolve path (symlink): %w", err)
 	}
 
-	// Lstat the resolved path — EvalSymlinks guarantees resolvedPath is
+	// Lstat the resolved path for size and existence checks — EvalSymlinks
-	// symlink-free, so ModeSymlink can never be set here; this is unreachable.
+	// guarantees no symlink components remain, so ModeSymlink can never be set.
 	fi, err := os.Lstat(resolvedPath)
 	if err != nil {
 		return "", fmt.Errorf("cannot stat file: %w", err)
@@ -152,11 +152,49 @@ func runValidateDocmap(args []string) int {
 		return 2
 	}
 
-	// Parse docmap YAML using the resolved path — eliminates any TOCTOU race
+	// Open and read the docmap with a LimitedReader — closes the residual TOCTOU
-	// between validation and use.
+	// window between the Lstat size check in validateDocmapPath and the file open
-	cfg, err := review.ParseDocMapConfig(resolvedDocmap)
+	// here. The limit is maxDocmapBytes+1 so we can detect a file that grew past
+	// the cap after the stat without reading unbounded bytes.
+	//
+	// Defense-in-depth: stat the path immediately before and after open so we can
+	// detect a file swap between validateDocmapPath's validation and this open via
+	// os.SameFile. An attacker with workspace write access could otherwise replace
+	// the validated file with a symlink in the gap between validation and use.
+	preStat, err := os.Lstat(resolvedDocmap)
 	if err != nil {
-		fmt.Fprintf(errWriter, "Error: failed to parse docmap %q: %v\n", resolvedDocmap, err)
+		fmt.Fprintf(errWriter, "Error: failed to stat docmap before open %q: %v\n", *docmapFlag, err)
+		return 2
+	}
+	f, err := os.Open(resolvedDocmap)
+	if err != nil {
+		fmt.Fprintf(errWriter, "Error: failed to open docmap %q: %v\n", *docmapFlag, err)
+		return 2
+	}
+	defer func() { _ = f.Close() }()
+	// Verify we opened the same file that was validated — rejects a swap between
+	// the pre-open Lstat and the open call.
+	postStat, err := f.Stat()
+	if err != nil {
+		fmt.Fprintf(errWriter, "Error: failed to stat open docmap %q: %v\n", *docmapFlag, err)
+		return 2
+	}
+	if !os.SameFile(preStat, postStat) {
+		fmt.Fprintf(errWriter, "Error: --docmap %q changed between validation and open\n", *docmapFlag)
+		return 2
+	}
+	docmapData, err := io.ReadAll(io.LimitReader(f, maxDocmapBytes+1))
+	if err != nil {
+		fmt.Fprintf(errWriter, "Error: failed to read docmap %q: %v\n", *docmapFlag, err)
+		return 2
+	}
+	if int64(len(docmapData)) > maxDocmapBytes {
+		fmt.Fprintf(errWriter, "Error: --docmap %q exceeded %d-byte limit after open\n", *docmapFlag, maxDocmapBytes)
+		return 2
+	}
+	cfg, err := review.ParseDocMapConfigContent(string(docmapData), *docmapFlag)
+	if err != nil {
+		fmt.Fprintf(errWriter, "Error: failed to parse docmap %q: %v\n", *docmapFlag, err)
 		return 2
 	}
 

cmd/review-bot/validatedocmap_test.go

@@ -687,6 +687,9 @@ func TestValidateDocmapPath_InRepoSymlinkAllowed(t *testing.T) {
 		t.Fatalf("expected in-repo symlink to be accepted, got error: %v", err)
 	}
 	// The returned resolved path must be the real file (not the symlink entry).
+	// validateDocmapPath calls filepath.EvalSymlinks internally, so the returned
+	// path is always the fully-resolved real path — it can never equal the
+	// symlink entry itself.
 	if resolved == symlinkPath {
 		t.Errorf("expected resolved path to differ from symlink path")
 	}