Skip to content

safeReadFile rejects internal symlinks when the project root path contains symlinks #175

Description

@abhinavgautam01

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.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workinghelp wantedExtra attention is needed

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions