Skip to content

fix: stop deploy packer from following out-of-root symlinks - #257

Merged
tlowrimore-heroku merged 1 commit into
mainfrom
fix/w-23510157-symlink-escape
Sep 3, 2026
Merged

fix: stop deploy packer from following out-of-root symlinks#257
tlowrimore-heroku merged 1 commit into
mainfrom
fix/w-23510157-symlink-escape

Conversation

@tlowrimore-heroku

@tlowrimore-heroku tlowrimore-heroku commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Summary

deploy_to_heroku could read files from outside the folder you asked it to deploy and upload them to Heroku inside your source tarball.

When packaging a workspace, the tool lists files with git ls-files and then calls stat() and readFile() on each one. Both of those follow symlinks. So if a repository contains a symlink that points somewhere outside the selected root (for example, a link named config.txt that actually points to /proc/self/environ), the packer would quietly read that external file and pack its contents into the tarball sent to Heroku. Build code in the repo could then read those bytes.

This means a repository you cloned from someone else could pull files off your machine the moment you deploy it — without any prompt-injection or special access. It was reported through the bug bounty program (W-23510157, CWE-59).

The fix

getSourceFilePaths() and walkDirectory() now:

  1. Use lstat() to notice when a path is a symlink.
  2. Resolve the symlink's real target with realpath() and check whether it stays inside the workspace root.
  3. Skip any symlink whose target escapes the root — its contents are never read or packed.

Symlinks that point to files inside the root are still followed, so normal deploys are unaffected.

Type of Change

Breaking Changes (major semver update)

  • Add a ! after your change type to denote a change that breaks current behavior

Feature Additions (minor semver update)

  • feat: Introduces a new feature to the codebase

Patch Updates (patch semver update)

  • fix: Bug fix
  • deps: Dependency upgrade
  • revert: Revert a previous commit
  • chore: Change that does not affect production code
  • refactor: Refactoring existing code without changing behavior
  • test: Add/update/remove tests

Testing

Notes:

Added regression tests in src/utils/tarball.spec.ts. They fail against the old code (the out-of-root file lands in the tarball) and pass with this fix. They cover both code paths that walk the workspace: the git ls-files path and the plain directory-walk fallback. A test also confirms that a legitimate symlink pointing inside the root is still included.

Steps:

  1. npm test — full suite passes (207 tests).
  2. Or: npx mocha src/utils/tarball.spec.ts --grep "symlink handling" to run just the new tests.

Screenshots (if applicable)

N/A

Related Issues

GUS work item: W-23510157

When deploy_to_heroku packaged a workspace, it listed files via git and then called stat() and readFile() on each one. Both follow symlinks, so a repository containing a symlink that points outside the selected root would have its target read and uploaded inside the source tarball. This let a malicious repo pull files from the deploying developer's machine into the build.

getSourceFilePaths() and walkDirectory() now detect symlinks with lstat() and skip any whose resolved (realpath) target escapes the workspace root. Symlinks that stay inside the root are still followed, so existing behavior is preserved.

Adds regression tests covering both the git ls-files path and the directory-walk path.

W-23510157
@tlowrimore-heroku
tlowrimore-heroku requested a review from a team as a code owner September 2, 2026 21:10

@jdodson jdodson left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved.

  • Visually looked over the PR and lined it up with the description
  • Ran a ChatGPT review and it found a thing, it's saying it found a possible edge case in your solution below.

High — Intermediate symlinks bypass containment
tarball.ts:91 checks only when the final path component is a symlink. If Git tracks leak/credentials but leak becomes a symlink to an external directory, lstat("leak/credentials") reports a regular file and its external contents are packed. I reproduced this behavior.

Validate escapesRoot(realRoot, fullPath) for every candidate, not only leaf symlinks, before adding or traversing it.

@tlowrimore-heroku
tlowrimore-heroku merged commit 75ec26a into main Sep 3, 2026
8 checks passed
@tlowrimore-heroku
tlowrimore-heroku deleted the fix/w-23510157-symlink-escape branch September 3, 2026 18:51
tlowrimore-heroku pushed a commit that referenced this pull request Sep 3, 2026
🤖 I have created a release *beep* *boop*
---


##
[1.2.7](mcp-server-v1.2.6...mcp-server-v1.2.7)
(2026-09-03)


### Bug Fixes

* stop deploy packer from following out-of-root symlinks
([#257](#257))
([75ec26a](75ec26a))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: heroku-devtools-release-workflows[bot] <261039447+heroku-devtools-release-workflows[bot]@users.noreply.github.com>
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.

2 participants