Description
Summary
safeReadFile can incorrectly reject a symlink that resolves to a regular file inside the project root. This happens when the path used for the project root itself contains a symlink, such as /tmp on macOS (/tmp -> /private/tmp) or a symlinked developer workspace.
The symlink target is canonicalized with filepath.EvalSymlinks, but the project root is only converted to an absolute path. The subsequent containment check therefore compares paths from different namespaces.
Reproduction
On macOS:
mkdir -p /tmp/brief-symlink-repro
printf 'module example.com/repro\n\ngo 1.24\n' > /tmp/brief-symlink-repro/real_go.mod
ln -s real_go.mod /tmp/brief-symlink-repro/go.mod
brief --json /tmp/brief-symlink-repro
The project and package manager are detected from the filename, but the manifests field does not contain go.mod. Replacing the symlink with a regular go.mod causes the manifest to be parsed normally.
Root cause
In detect/detect.go, the target is resolved before comparison:
target, err := filepath.EvalSymlinks(path)
// ...
absRoot, _ := filepath.Abs(e.Root)
if !strings.HasPrefix(target, absRoot+string(filepath.Separator)) {
return nil, fmt.Errorf("symlink escapes project root: %s -> %s", file, target)
}
For a project under /tmp, these values can be:
target: /private/tmp/brief-symlink-repro/real_go.mod
absRoot: /tmp/brief-symlink-repro
The target is internal, but the prefix comparison fails.
Expected behavior
- Symlinks resolving to regular files inside the canonical project root should be readable.
- Symlinks resolving outside the project root must continue to be rejected.
- Errors from resolving or canonicalizing paths should not be ignored.
Proposed implementation
Canonicalize the absolute project root with filepath.EvalSymlinks before checking containment. Prefer filepath.Rel over a string-prefix comparison so containment is evaluated using path semantics:
absRoot, err := filepath.Abs(e.Root)
if err != nil {
return nil, err
}
resolvedRoot, err := filepath.EvalSymlinks(absRoot)
if err != nil {
return nil, err
}
rel, err := filepath.Rel(resolvedRoot, target)
if err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) || filepath.IsAbs(rel) {
return nil, fmt.Errorf("symlink escapes project root: %s -> %s", file, target)
}
Suggested tests
- Accept an internal file symlink when the path used for
Engine.Root contains a symlink.
- Continue rejecting a symlink whose target is outside the project root.
- Preserve regular-file handling.
Description
Summary
safeReadFilecan incorrectly reject a symlink that resolves to a regular file inside the project root. This happens when the path used for the project root itself contains a symlink, such as/tmpon macOS (/tmp -> /private/tmp) or a symlinked developer workspace.The symlink target is canonicalized with
filepath.EvalSymlinks, but the project root is only converted to an absolute path. The subsequent containment check therefore compares paths from different namespaces.Reproduction
On macOS:
The project and package manager are detected from the filename, but the
manifestsfield does not containgo.mod. Replacing the symlink with a regulargo.modcauses the manifest to be parsed normally.Root cause
In
detect/detect.go, the target is resolved before comparison:For a project under
/tmp, these values can be:The target is internal, but the prefix comparison fails.
Expected behavior
Proposed implementation
Canonicalize the absolute project root with
filepath.EvalSymlinksbefore checking containment. Preferfilepath.Relover a string-prefix comparison so containment is evaluated using path semantics:Suggested tests
Engine.Rootcontains a symlink.