Skip to content

Deepak/fix issue 1031/prevent python symlink escape#1303

Open
deepakdp17 wants to merge 4 commits intojfrog:devfrom
deepakdp17:deepak/fix_issue_1031/prevent_python_symlink_escape
Open

Deepak/fix issue 1031/prevent python symlink escape#1303
deepakdp17 wants to merge 4 commits intojfrog:devfrom
deepakdp17:deepak/fix_issue_1031/prevent_python_symlink_escape

Conversation

@deepakdp17
Copy link
Copy Markdown

Summary

  • Resolves symlinks using filepath.EvalSymlinks before reading or writing requirements files, so a symlinked requirements.txt pointing outside the repository is rejected instead of silently followed
  • Adds a shared ValidateFileWithinDir utility that other package handlers can reuse
  • Hardens descriptor file discovery (GetAllDescriptorFilesFullPaths) to skip symlinks that resolve outside the working directory, covering NuGet, Pnpm, Conan, and others

Why

Frogbot's Python handler validated file paths using string prefix checks, but never resolved symlinks. In CI environments, a malicious symlink like requirements.txt -> /etc/shadow would pass the path check and still be read or overwritten. This is the issue described in #1031.

What changed

utils/pathutils.go — New ValidateFileWithinDir function that resolves symlinks on both the file and the allowed directory, then checks containment
utils/pathutils_test.go — 7 tests covering regular files, subdirectories, symlink escape, symlinks within the directory, path traversal, nonexistent files, and directory symlink escape
packagehandlers/pythonpackagehandler.go — tryReadRequirementFile and handlePip now validate paths through the new utility before any read or write
packagehandlers/commonpackagehandler.go — GetAllDescriptorFilesFullPaths validates each discovered file and logs a warning if it skips one

  • All tests passed. If this feature is not already covered by the tests, I added new tests.
  • Project compiles cleanly
  • No lint errors introduced
  • Existing TestUpdateDependency pip/poetry/pipenv cases should still pass (requires JFrog server credentials)
  • Manual test: create a symlinked requirements.txt pointing outside the repo and confirm Frogbot rejects it
  • This pull request is on the dev branch.
  • I used gofmt for formatting the code before submitting the pull request.

Fixes #1031

deepakdp17 added 4 commits May 8, 2026 10:12
Introduces a shared utility that resolves symlinks via
filepath.EvalSymlinks and verifies the real path stays within an
allowed directory boundary. Covers file symlinks, directory symlinks,
path traversal, and nonexistent file cases.

Ref: jfrog#1031
working directory are skipped with a warning log.
This hardens all package managers that use this
shared discovery path (NuGet, Pnpm, Conan, etc.).

Ref: jfrog#1031
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant