diff --git a/cmd/review-bot/validatedocmap.go b/cmd/review-bot/validatedocmap.go index e29fa25..d8f2547 100644 --- a/cmd/review-bot/validatedocmap.go +++ b/cmd/review-bot/validatedocmap.go @@ -23,8 +23,9 @@ const maxDocmapBytes int64 = 10 * 1024 * 1024 // 10 MB // 1. The path resolves to a regular file within resolvedRoot (path // confinement): prevents a PR-controlled --docmap from reading arbitrary // host files via absolute paths or ".." traversal. -// 2. The path is not a symlink: prevents denial-of-service via /dev/zero or -// information disclosure via symlinks that point outside the workspace. +// 2. The resolved path is within resolvedRoot: in-repo file-level symlinks +// are allowed when their resolved target is still inside the root; +// symlinks that escape the root are rejected by the confinement check. // 3. The file does not exceed maxDocmapBytes: prevents memory exhaustion // from an oversized but legitimately committed doc-map file. // @@ -49,18 +50,13 @@ func validateDocmapPath(localPath, resolvedRoot string) error { return fmt.Errorf("cannot resolve path (symlink): %w", err) } - // Lstat the resolved path — at this point resolvedPath is symlink-free, so - // ModeSymlink will never be set. We keep the check as defense-in-depth. + // Lstat the resolved path — EvalSymlinks guarantees resolvedPath is + // symlink-free, so ModeSymlink can never be set here; this is unreachable. fi, err := os.Lstat(resolvedPath) if err != nil { return fmt.Errorf("cannot stat file: %w", err) } - // Defense-in-depth: reject any remaining symlink indicator. - if fi.Mode()&os.ModeSymlink != 0 { - return fmt.Errorf("symlinks are not allowed") - } - // Reject anything that is not a regular file (directories, FIFOs, device // nodes, etc.) — ParseDocMapConfig expects a plain YAML file and would // produce a confusing error on non-regular entries. @@ -144,7 +140,8 @@ func runValidateDocmap(args []string) int { // may reference a PR-controlled file (e.g. .review-bot/doc-map.yml). // Validate that it: // 1. Resolves within resolvedRoot (prevent reading arbitrary host files). - // 2. Is not a symlink (prevent /dev/zero or symlink-based host probing). + // 2. Resolved target stays within the root (in-repo symlinks are allowed + // if they resolve to a path inside the root). // 3. Does not exceed maxDocmapBytes (prevent memory exhaustion from an // oversized committed file). if err := validateDocmapPath(*docmapFlag, resolvedRoot); err != nil {