Skip to content

fix: contain static assets across symlinks#5

Merged
smiggleworth merged 2 commits into
mainfrom
fix/security-asset-symlink-containment
Jul 26, 2026
Merged

fix: contain static assets across symlinks#5
smiggleworth merged 2 commits into
mainfrom
fix/security-asset-symlink-containment

Conversation

@smiggleworth

Copy link
Copy Markdown
Contributor

Closes #4. Canonicalizes asset roots and candidates before enforcing containment and adds an out-of-root symlink regression.

Copilot AI review requested due to automatic review settings July 26, 2026 16:53

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR addresses a static-asset path traversal issue where lexical path containment checks could be bypassed via in-root symlinks that resolve outside assets.root. It does this by canonicalizing both the configured asset root and the requested asset candidate (post-symlink resolution) before enforcing containment, and adds a regression test.

Changes:

  • Canonicalize assets.root using realpath() and enforce containment against the canonicalized root.
  • Canonicalize each asset request candidate via realpath() before stat() and serving.
  • Add a non-Windows regression test ensuring symlinks cannot escape the configured asset root.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/serve.ts Canonicalizes asset root and resolved candidate paths before enforcing containment and serving static assets.
tests/node.test.ts Adds a regression test for an in-root symlink pointing outside the asset root.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/serve.ts
@smiggleworth
smiggleworth merged commit b1d9278 into main Jul 26, 2026
3 checks passed
@smiggleworth
smiggleworth deleted the fix/security-asset-symlink-containment branch July 26, 2026 17:02
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.

Security: contain static assets across symlinks

2 participants