Compare commits

..

3 Commits

Author SHA1 Message Date
Rodin d6bab7a9cf fix(#150): close residual TOCTOU with LimitedReader at docmap open
PR Ready Gate / clear-labels (pull_request) Successful in 2s
CI / test (pull_request) Successful in 17s
CI / review (anthropic--claude-4.6-sonnet, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 43s
CI / review (gpt-5, security, ., rodin/security-patterns, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 1m16s
CI / review (gpt-5, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 1m23s
2026-05-15 16:11:15 -07:00
Rodin 4359518e50 nit(#150): report original --docmap flag value in parse error, not resolved path 2026-05-15 16:10:42 -07:00
Rodin 6e11107c77 nit(#150): fix misleading 'this is unreachable' in Lstat comment 2026-05-15 16:10:27 -07:00
+23 -6
View File
@@ -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
// symlink-free, so ModeSymlink can never be set here; this is unreachable.
// Lstat the resolved path for size and existence checks — EvalSymlinks
// 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,28 @@ func runValidateDocmap(args []string) int {
return 2
}
// Parse docmap YAML using the resolved path — eliminates any TOCTOU race
// between validation and use.
cfg, err := review.ParseDocMapConfig(resolvedDocmap)
// Open and read the docmap with a LimitedReader — closes the residual TOCTOU
// 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.
f, err := os.Open(resolvedDocmap)
if err != nil {
fmt.Fprintf(errWriter, "Error: failed to parse docmap %q: %v\n", resolvedDocmap, err)
fmt.Fprintf(errWriter, "Error: failed to open docmap %q: %v\n", *docmapFlag, err)
return 2
}
defer f.Close() // nolint:errcheck
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
}