diff --git a/cmd/review-bot/validatedocmap.go b/cmd/review-bot/validatedocmap.go index 2c1f364..8fc19bb 100644 --- a/cmd/review-bot/validatedocmap.go +++ b/cmd/review-bot/validatedocmap.go @@ -156,12 +156,33 @@ func runValidateDocmap(args []string) int { // window between the Lstat size check in validateDocmapPath and the file open // 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 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 f.Close() // nolint:errcheck + 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)