fix(preview): prevent path traversal in serve-preview file serving by davidfirst · Pull Request #10424 · teambit/bit

davidfirst · 2026-06-12T15:48:12Z

serveLocalPreview built the served file path as join(publicDir, filePath), where filePath is the trailing portion of the untrusted request URL (/:message/:componentId(*)), then called res.sendFile(file) with an absolute path and no { root }. Because path.join collapses .., a request like /msg/org/comp/../../../../etc/passwd (or percent-encoded) resolved outside publicDir and let sendFile return arbitrary files readable by the process.

Adds resolvePreviewFilePath(publicDir, filePath) which returns the path only when it stays within publicDir (else undefined → 403), used before the existing existsSync/sendFile. Allows legitimate nested files (assets/x.js), rejects .. escapes and sibling-prefix dirs. Unit-tested in resolve-preview-file-path.spec.ts.

Server bind address is intentionally left unchanged (same reasoning as the cloud login-listener: not narrowing a server's reachability here).

serveLocalPreview built the served file path as join(publicDir, filePath) where filePath comes from the untrusted request URL, then res.sendFile(file) with an absolute path and no { root }. Since path.join collapses '..', a request like /msg/org/comp/../../../../etc/passwd escaped publicDir and read arbitrary files. Add resolvePreviewFilePath(publicDir, filePath) which returns the path only when it stays within publicDir (else undefined -> 403), and use it before existsSync/sendFile. Unit-tested.

qodo-free-for-open-source-projects · 2026-06-12T15:48:20Z

Code Review by Qodo

🐞 Bugs (4) 📘 Rule violations (0)

1. Dot path sends directory 🐞 Bug ≡ Correctness ⭐ New

Description

resolvePreviewFilePath() allows filePath='.' to resolve to publicDir, and
serveLocalPreview() then calls existsSync(publicDir) and res.sendFile(publicDir), which
attempts to serve a directory and can bubble an error response. A request like
/:message/:componentId/. (or %2e) will hit this path.

Code

scopes/preview/preview/preview.main.runtime.ts[R409-415]

+        const file = resolvePreviewFilePath(publicDir, filePath);
+        if (!file) {
+          return res.status(403).send('Invalid file path.');
+        }
        if (!existsSync(file)) {
          return res.status(404).send('File not found.');
        }

Evidence
The server route sends sendFile() for any truthy filePath, and the new test suite explicitly
asserts that . resolves to publicDir, which is a directory. This combination makes
sendFile(publicDir) reachable.
scopes/preview/preview/preview.main.runtime.ts[402-420]
scopes/preview/preview/resolve-preview-file-path.spec.ts[36-38]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`resolvePreviewFilePath(publicDir, filePath)` currently treats `filePath='.'` as valid and returns `publicDir`. In `serveLocalPreview()`, this leads to `res.sendFile(publicDir)` for a directory path.

### Issue Context
This route chooses between `sendFile()` (when `filePath` is present) and `express.static(publicDir)` (when `filePath` is absent). Allowing `.` in the sendFile path bypasses the static middleware and tries to send a directory.

### Fix Focus Areas
- scopes/preview/preview/preview.main.runtime.ts[402-420]
- scopes/preview/preview/resolve-preview-file-path.ts[25-33]
- scopes/preview/preview/resolve-preview-file-path.spec.ts[36-38]

### Suggested fix
- In `resolvePreviewFilePath()`, return `undefined` when the resolved target equals the root directory (i.e. `resolved === root`) unless you explicitly intend to serve directories.
 - Alternatively, in `serveLocalPreview()`, detect the directory case (e.g. via `statSync(file).isDirectory()`) and delegate to `express.static(publicDir)` or return 404/403.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

2. Root prefix check breaks 🐞 Bug ≡ Correctness ⭐ New

Description

resolvePreviewFilePath() uses resolved.startsWith(root + sep) for containment; this fails when
root is already a filesystem root (e.g. / or C:\\) and is also case-sensitive on Windows
paths, causing legitimate in-root paths to be rejected on some configurations/platforms. The repo
already implements win32 case normalization in other path-inside logic, but this function doesn’t
apply it.

Code

scopes/preview/preview/resolve-preview-file-path.ts[R27-30]

+  const root = safeRealpath(publicDir) ?? resolve(publicDir);
+  const resolved = safeRealpath(file) ?? resolve(file);
+  if (resolved !== root && !resolved.startsWith(root + sep)) {
+    return undefined;

Evidence
The resolver’s containment decision is based on a startsWith(root + sep) string check. Separately,
other code in the repo explicitly normalizes case on win32 for path containment logic, highlighting
that win32 case sensitivity is a known concern here.
scopes/preview/preview/resolve-preview-file-path.ts[25-32]
scopes/toolbox/path/is-path-inside/is-path-inside.ts[11-15]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The containment check `resolved !== root && !resolved.startsWith(root + sep)` is fragile for root directories and doesn’t account for Windows case-insensitivity.

### Issue Context
This helper is intended to be security-sensitive and platform-agnostic. Relying on string prefix checks with a constructed trailing separator is error-prone for edge roots and case differences.

### Fix Focus Areas
- scopes/preview/preview/resolve-preview-file-path.ts[25-33]
- scopes/toolbox/path/is-path-inside/is-path-inside.ts[6-20]

### Suggested fix
- Replace the `startsWith(root + sep)` logic with a `path.relative(root, resolved)`-based check:
 - Compute `rel = relative(root, resolved)` and treat it as inside iff `rel === ''` or (`!rel.startsWith('..' + sep)` and `rel !== '..'` and `!isAbsolute(rel)`).
- On win32, normalize casing consistently before comparing (e.g. `root = root.toLowerCase(); resolved = resolved.toLowerCase();`).
- Keep `realpath` usage as-is for symlink resolution; just improve the containment predicate.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

3. Check/send race window 🐞 Bug ⛨ Security ⭐ New

Description

serveLocalPreview() validates the path then separately calls existsSync() and res.sendFile()
on the mutable filesystem path string, leaving a time-of-check/time-of-use window where the served
file can change after validation. This matters if anything else can write into the preview
publicDir concurrently (e.g. another local process, or the preview build output updating while
requests are in-flight).

Code

scopes/preview/preview/preview.main.runtime.ts[R409-415]

+        const file = resolvePreviewFilePath(publicDir, filePath);
+        if (!file) {
+          return res.status(403).send('Invalid file path.');
+        }
        if (!existsSync(file)) {
          return res.status(404).send('File not found.');
        }

Evidence

The handler performs validation, then a separate existence check, then serves by path later; there
is no subsequent re-validation between the checks and the actual send operation.

scopes/preview/preview/preview.main.runtime.ts[406-417]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The current flow validates a path, then performs separate filesystem operations (`existsSync`, `sendFile`) later. If the path target is replaced in-between, the request may serve a different file than the one validated.

### Issue Context
This is a hardening issue: exploitability depends on the ability to mutate `publicDir` during requests. Still, the code is in a security-sensitive area (path traversal prevention), so it’s worth tightening.

### Fix Focus Areas
- scopes/preview/preview/preview.main.runtime.ts[406-417]
- scopes/preview/preview/resolve-preview-file-path.ts[25-33]

### Suggested fix
- Avoid pre-checking with `existsSync()`; instead, call `res.sendFile()` with a callback and handle not-found/errors there.
- Re-run `resolvePreviewFilePath(publicDir, filePath)` immediately before sending (or modify it to also return the `resolved`/`realpath` value) and use that stable resolved path for the send operation.
- If you keep a pre-check, ensure you compare realpaths again right before sending to reduce the race window.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

View more (1)

4. ~~Symlink escape still possible~~ ✓ Resolved 🐞 Bug ⛨ Security

Description

resolvePreviewFilePath() only compares path.resolve() results, so a symlink within publicDir
that points outside can still pass the prefix check and be served by res.sendFile(). This can
still expose arbitrary files if symlinks exist in (or can be introduced into) the preview
publicDir tree.

Code

scopes/preview/preview/resolve-preview-file-path.ts[R13-19]

+  const file = join(publicDir, filePath);
+  const root = resolve(publicDir);
+  const resolved = resolve(file);
+  if (resolved !== root && !resolved.startsWith(root + sep)) {
+    return undefined;
+  }
+  return file;

Evidence

The helper only performs a path.resolve() prefix check and returns a path that is then used
directly for existsSync() and sendFile(), so any filesystem-level redirection (symlinks) under
publicDir is not constrained by the check. The repo already uses realpathSync() elsewhere when
the real on-disk target matters, indicating symlink-awareness is a known requirement in other
path-sensitive code paths.

scopes/preview/preview/resolve-preview-file-path.ts[12-19]
scopes/preview/preview/preview.main.runtime.ts[402-420]
scopes/harmony/aspect-loader/plugin.ts[31-35]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`resolvePreviewFilePath()` prevents `..` traversal by checking `path.resolve()` output stays within `publicDir`, but this check is not symlink-aware. If any segment under `publicDir` is a symlink to a location outside `publicDir`, the lexical check still passes while the filesystem access performed by `existsSync()`/`sendFile()` follows the symlink.
### Issue Context
This route serves files from a local preview directory based on URL-derived `filePath` and uses `existsSync(file)` followed by `res.sendFile(file)`.
### Fix Focus Areas
- scopes/preview/preview/resolve-preview-file-path.ts[12-19]
- scopes/preview/preview/preview.main.runtime.ts[406-417]
### Suggested fix
1. Keep the existing lexical containment check (fast reject for obvious `..` escapes).
2. After confirming existence, add a second containment check using `fs.realpathSync()` (or async equivalent) for both `publicDir` and the candidate file path, and reject if the *real* file path escapes the *real* publicDir.
Example approach in the route:
- `const rootReal = realpathSync(publicDir)`
- `const fileReal = realpathSync(file)` (after `existsSync(file)`)
- verify `fileReal === rootReal || fileReal.startsWith(rootReal + sep)` (or use `path.relative(rootReal, fileReal)` and ensure it does not start with `'..'` and is not absolute)
- `sendFile(fileReal)`
This closes the symlink-based escape vector while preserving current behavior for normal files.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

5. Symlink tests not portable 🐞 Bug ☼ Reliability ⭐ New

Description

resolve-preview-file-path.spec.ts uses fs.symlink() in its tests, which commonly fails on
Windows without elevated permissions/developer mode and can make the unit suite platform-dependent.
The repo already documents Windows symlink permission issues elsewhere, suggesting this is a known
portability concern.

Code

scopes/preview/preview/resolve-preview-file-path.spec.ts[R64-72]

+    it('rejects a symlink inside publicDir that points outside', async () => {
+      await fs.symlink(join(root, 'secret.txt'), join(realPublicDir, 'evil')); // public/evil -> ../secret.txt
+      expect(resolvePreviewFilePath(realPublicDir, 'evil')).to.equal(undefined);
+    });
+
+    it('allows a symlink inside publicDir that points to another file inside it', async () => {
+      await fs.symlink(join(realPublicDir, 'assets', 'main.js'), join(realPublicDir, 'link.js'));
+      expect(resolvePreviewFilePath(realPublicDir, 'link.js')).to.equal(join(realPublicDir, 'link.js'));
+    });

Evidence
The test suite creates symlinks directly. Elsewhere in the repo, code explicitly notes that Windows
often can’t create symlinks due to permissions and implements fallbacks, which indicates these tests
may be flaky on win32.
scopes/preview/preview/resolve-preview-file-path.spec.ts[40-72]
scopes/toolbox/fs/link-or-symlink/create-link-or-symlink.ts[10-13]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The new symlink tests rely on creating real symlinks. On Windows, symlink creation often fails (EPERM) unless the environment is configured appropriately.

### Issue Context
This is test-only but can cause CI failures or local test friction on Windows.

### Fix Focus Areas
- scopes/preview/preview/resolve-preview-file-path.spec.ts[40-72]
- scopes/toolbox/fs/link-or-symlink/create-link-or-symlink.ts[10-13]

### Suggested fix
- Skip or relax the symlink tests when `process.platform === 'win32'`, or when symlink creation throws EPERM.
- Alternative: wrap `fs.symlink(...)` in try/catch and `this.skip()` (or equivalent) if symlinks aren’t supported in the current environment.
- Keep the non-symlink traversal tests as they are (they don’t require filesystem symlink support).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

6. ~~Non-absolute path returned~~ ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

resolvePreviewFilePath() documentation says it returns an absolute path, but it returns
join(publicDir, filePath) (which can be relative if publicDir is relative), creating a
misleading contract for future callers. This can lead to incorrect usage in places that require
absolute paths (e.g. when passing paths to APIs that reject relative paths).

Code

scopes/preview/preview/resolve-preview-file-path.ts[R12-20]

+export function resolvePreviewFilePath(publicDir: string, filePath: string): string | undefined {
+  const file = join(publicDir, filePath);
+  const root = resolve(publicDir);
+  const resolved = resolve(file);
+  if (resolved !== root && !resolved.startsWith(root + sep)) {
+    return undefined;
+  }
+  return file;
+}

Evidence

The docstring explicitly promises an absolute path, but the implementation returns the non-absolute
file value instead of the computed absolute resolved value.

scopes/preview/preview/resolve-preview-file-path.ts[3-20]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`resolvePreviewFilePath()` claims it returns an **absolute** path, but it currently returns the `join(publicDir, filePath)` value (`file`) rather than the absolute `resolved` value it computed.
### Issue Context
This is primarily an API/contract mismatch: the helper computes `resolved` but does not return it.
### Fix Focus Areas
- scopes/preview/preview/resolve-preview-file-path.ts[3-20]
### Suggested fix
Choose one:
1) Make the implementation match the docs by returning `resolved` (and optionally compute `file` as `resolve(publicDir, filePath)` directly).
2) If returning the joined (possibly-relative) path is intentional, update the docstring to remove the “absolute path” claim.
If you change the return value, update the unit expectations accordingly.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Previous review results

Review updated until commit ad7980b

Results up to commit 78d43e4

🐞 Bugs (2) 📘 Rule violations (0)

1. Symlink escape still possible 🐞 Bug ⛨ Security

Description

resolvePreviewFilePath() only compares path.resolve() results, so a symlink within publicDir
that points outside can still pass the prefix check and be served by res.sendFile(). This can
still expose arbitrary files if symlinks exist in (or can be introduced into) the preview
publicDir tree.

Code

scopes/preview/preview/resolve-preview-file-path.ts[R13-19]

+  const file = join(publicDir, filePath);
+  const root = resolve(publicDir);
+  const resolved = resolve(file);
+  if (resolved !== root && !resolved.startsWith(root + sep)) {
+    return undefined;
+  }
+  return file;

Evidence

The helper only performs a path.resolve() prefix check and returns a path that is then used
directly for existsSync() and sendFile(), so any filesystem-level redirection (symlinks) under
publicDir is not constrained by the check. The repo already uses realpathSync() elsewhere when
the real on-disk target matters, indicating symlink-awareness is a known requirement in other
path-sensitive code paths.

scopes/preview/preview/resolve-preview-file-path.ts[12-19]
scopes/preview/preview/preview.main.runtime.ts[402-420]
scopes/harmony/aspect-loader/plugin.ts[31-35]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`resolvePreviewFilePath()` prevents `..` traversal by checking `path.resolve()` output stays within `publicDir`, but this check is not symlink-aware. If any segment under `publicDir` is a symlink to a location outside `publicDir`, the lexical check still passes while the filesystem access performed by `existsSync()`/`sendFile()` follows the symlink.
### Issue Context
This route serves files from a local preview directory based on URL-derived `filePath` and uses `existsSync(file)` followed by `res.sendFile(file)`.
### Fix Focus Areas
- scopes/preview/preview/resolve-preview-file-path.ts[12-19]
- scopes/preview/preview/preview.main.runtime.ts[406-417]
### Suggested fix
1. Keep the existing lexical containment check (fast reject for obvious `..` escapes).
2. After confirming existence, add a second containment check using `fs.realpathSync()` (or async equivalent) for both `publicDir` and the candidate file path, and reject if the *real* file path escapes the *real* publicDir.
Example approach in the route:
- `const rootReal = realpathSync(publicDir)`
- `const fileReal = realpathSync(file)` (after `existsSync(file)`)
- verify `fileReal === rootReal || fileReal.startsWith(rootReal + sep)` (or use `path.relative(rootReal, fileReal)` and ensure it does not start with `'..'` and is not absolute)
- `sendFile(fileReal)`
This closes the symlink-based escape vector while preserving current behavior for normal files.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

2. Non-absolute path returned 🐞 Bug ⚙ Maintainability ⭐ New

Description

resolvePreviewFilePath() documentation says it returns an absolute path, but it returns
join(publicDir, filePath) (which can be relative if publicDir is relative), creating a
misleading contract for future callers. This can lead to incorrect usage in places that require
absolute paths (e.g. when passing paths to APIs that reject relative paths).

Code

scopes/preview/preview/resolve-preview-file-path.ts[R12-20]

+export function resolvePreviewFilePath(publicDir: string, filePath: string): string | undefined {
+  const file = join(publicDir, filePath);
+  const root = resolve(publicDir);
+  const resolved = resolve(file);
+  if (resolved !== root && !resolved.startsWith(root + sep)) {
+    return undefined;
+  }
+  return file;
+}

Evidence

The docstring explicitly promises an absolute path, but the implementation returns the non-absolute
file value instead of the computed absolute resolved value.

scopes/preview/preview/resolve-preview-file-path.ts[3-20]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`resolvePreviewFilePath()` claims it returns an **absolute** path, but it currently returns the `join(publicDir, filePath)` value (`file`) rather than the absolute `resolved` value it computed.

### Issue Context
This is primarily an API/contract mismatch: the helper computes `resolved` but does not return it.

### Fix Focus Areas
- scopes/preview/preview/resolve-preview-file-path.ts[3-20]

### Suggested fix
Choose one:
1) Make the implementation match the docs by returning `resolved` (and optionally compute `file` as `resolve(publicDir, filePath)` directly).
2) If returning the joined (possibly-relative) path is intentional, update the docstring to remove the “absolute path” claim.

If you change the return value, update the unit expectations accordingly.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Results up to commit N/A

New Review Started

This review has been superseded by a new analysis

qodo-free-for-open-source-projects · 2026-06-12T15:49:04Z

PR Summary by Qodo

Fix serve-preview path traversal by validating request-derived file paths
🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

Walkthroughs

Description

• Block path traversal in serve-preview by rejecting file paths escaping the public directory.
• Add a dedicated resolver utility to safely validate request-derived file paths.
• Cover normal, nested, traversal, and prefix-sibling edge cases with unit tests.

Diagram

graph TD
  A(["HTTP client"]) --> B["PreviewMain: serveLocalPreview"] --> C["resolvePreviewFilePath()"]
  C -->|"valid"| D{"Exists on disk?"} -->|"yes"| E["res.sendFile()"]
  D -->|"no"| F["404 File not found"]
  C -->|"invalid (escapes root)"| G["403 Invalid file path"]

  subgraph Legend
    direction LR
    _actor(["Actor"]) ~~~ _proc["Process"] ~~~ _dec{"Decision"}
  end

High-Level Assessment

The following are alternative approaches to this PR:

1. Use Express sendFile with { root: publicDir }

➕ Delegates path normalization and root confinement to Express
➕ Reduces custom security-critical logic
➖ May require refactoring current absolute-path usage and error handling
➖ Behavior can vary across Express versions and options (e.g., dotfiles)

2. Serve preview assets via serve-static middleware scoped to publicDir

➕ Battle-tested static serving with built-in traversal protections
➕ Simplifies handler logic for files vs. routes
➖ Harder to integrate if the route structure needs special matching/authorization
➖ May require careful ordering with existing preview routes

Recommendation: The PR’s approach (resolve + enforce resolved path within publicDir) is solid and easy to audit, and it explicitly handles the sibling-prefix edge case (e.g., public vs public-evil). If future refactors touch this area, consider switching to sendFile({ root }) or serve-static to further reduce custom path-handling surface area, but the current fix is appropriate and well-tested.

File Changes

Bug fix (2)

preview.main.runtime.ts Validate request-derived preview file paths before sendFile +7/-1
Validate request-derived preview file paths before sendFile
• Replaces direct join(publicDir, filePath) usage with resolvePreviewFilePath() to ensure the requested file stays within publicDir. Returns 403 for invalid/traversing paths before checking existence or serving the file.
scopes/preview/preview/preview.main.runtime.ts

resolve-preview-file-path.ts Add safe resolver to prevent publicDir escape via .. traversal +20/-0
Add safe resolver to prevent publicDir escape via .. traversal
• Introduces resolvePreviewFilePath(publicDir, filePath) which joins and resolves paths, then verifies the resolved target is either exactly publicDir or a descendant. Returns undefined when the path would escape, enabling callers to reject the request.
scopes/preview/preview/resolve-preview-file-path.ts

Tests (1)

resolve-preview-file-path.spec.ts Unit tests for traversal and sibling-prefix path validation +37/-0
Unit tests for traversal and sibling-prefix path validation
• Adds coverage for normal and nested files, parent traversal (including embedded traversal), separator-based '..' cases, and a sibling directory sharing a prefix with publicDir. Also verifies '.' resolves back to publicDir.
scopes/preview/preview/resolve-preview-file-path.spec.ts

qodo-free-for-open-source-projects · 2026-06-12T15:49:52Z

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0)

1. Symlink escape still possible 🐞 Bug ⛨ Security

Description

resolvePreviewFilePath() only compares path.resolve() results, so a symlink within publicDir
that points outside can still pass the prefix check and be served by res.sendFile(). This can
still expose arbitrary files if symlinks exist in (or can be introduced into) the preview
publicDir tree.

Code

scopes/preview/preview/resolve-preview-file-path.ts[R13-19]

+  const file = join(publicDir, filePath);
+  const root = resolve(publicDir);
+  const resolved = resolve(file);
+  if (resolved !== root && !resolved.startsWith(root + sep)) {
+    return undefined;
+  }
+  return file;

Evidence

The helper only performs a path.resolve() prefix check and returns a path that is then used
directly for existsSync() and sendFile(), so any filesystem-level redirection (symlinks) under
publicDir is not constrained by the check. The repo already uses realpathSync() elsewhere when
the real on-disk target matters, indicating symlink-awareness is a known requirement in other
path-sensitive code paths.

scopes/preview/preview/resolve-preview-file-path.ts[12-19]
scopes/preview/preview/preview.main.runtime.ts[402-420]
scopes/harmony/aspect-loader/plugin.ts[31-35]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`resolvePreviewFilePath()` prevents `..` traversal by checking `path.resolve()` output stays within `publicDir`, but this check is not symlink-aware. If any segment under `publicDir` is a symlink to a location outside `publicDir`, the lexical check still passes while the filesystem access performed by `existsSync()`/`sendFile()` follows the symlink.

### Issue Context
This route serves files from a local preview directory based on URL-derived `filePath` and uses `existsSync(file)` followed by `res.sendFile(file)`.

### Fix Focus Areas
- scopes/preview/preview/resolve-preview-file-path.ts[12-19]
- scopes/preview/preview/preview.main.runtime.ts[406-417]

### Suggested fix
1. Keep the existing lexical containment check (fast reject for obvious `..` escapes).
2. After confirming existence, add a second containment check using `fs.realpathSync()` (or async equivalent) for both `publicDir` and the candidate file path, and reject if the *real* file path escapes the *real* publicDir.

Example approach in the route:
- `const rootReal = realpathSync(publicDir)`
- `const fileReal = realpathSync(file)` (after `existsSync(file)`)
- verify `fileReal === rootReal || fileReal.startsWith(rootReal + sep)` (or use `path.relative(rootReal, fileReal)` and ensure it does not start with `'..'` and is not absolute)
- `sendFile(fileReal)`

This closes the symlink-based escape vector while preserving current behavior for normal files.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

qodo-free-for-open-source-projects · 2026-06-12T17:21:50Z

Code review by qodo was updated up to the latest commit 78d43e4

resolvePreviewFilePath now realpath()s both publicDir and the target so a symlink inside publicDir cannot redirect the served file outside it (lexical path.resolve alone doesn't follow symlinks). Falls back to the lexical resolve for not-yet-existent targets. Adds symlink unit coverage. Addresses Qodo review.

qodo-free-for-open-source-projects · 2026-06-12T17:26:21Z

Code review by qodo was updated up to the latest commit ad7980b

davidfirst · 2026-06-12T17:26:34Z

Re: Qodo's "Symlink escape still possible" finding — fixed in ad7980b. resolvePreviewFilePath now realpath()s both publicDir and the target (with a lexical fallback for not-yet-existent files), so a symlink inside publicDir pointing outside is resolved and rejected rather than served. Added unit coverage: symlink-inside→outside is rejected, symlink-inside→inside is allowed, and the existing lexical .. cases still pass.

Context on severity: in serve-preview's threat model the HTTP/LAN client only controls the request path and can't plant a symlink in publicDir (it's build-generated output), so this was defense-in-depth — but a path-confinement helper should be symlink-safe regardless, so the fix is worth it.

Merge branch 'master' into fix/serve-preview-path-traversal

047ceb3

Merge branch 'master' into fix/serve-preview-path-traversal

78d43e4

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(preview): prevent path traversal in serve-preview file serving#10424

fix(preview): prevent path traversal in serve-preview file serving#10424
davidfirst wants to merge 4 commits into
masterfrom
fix/serve-preview-path-traversal

davidfirst commented Jun 12, 2026

Uh oh!

qodo-free-for-open-source-projects Bot commented Jun 12, 2026 •

edited

Loading

New Review Started

Uh oh!

qodo-free-for-open-source-projects Bot commented Jun 12, 2026

Uh oh!

qodo-free-for-open-source-projects Bot commented Jun 12, 2026 •

edited

Loading

Uh oh!

qodo-free-for-open-source-projects Bot commented Jun 12, 2026 •

edited

Loading

Uh oh!

qodo-free-for-open-source-projects Bot commented Jun 12, 2026 •

edited

Loading

Uh oh!

davidfirst commented Jun 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

davidfirst commented Jun 12, 2026

Uh oh!

qodo-free-for-open-source-projects Bot commented Jun 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Previous review results

New Review Started

Uh oh!

qodo-free-for-open-source-projects Bot commented Jun 12, 2026

PR Summary by Qodo

Walkthroughs

File Changes

Uh oh!

qodo-free-for-open-source-projects Bot commented Jun 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Uh oh!

qodo-free-for-open-source-projects Bot commented Jun 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

qodo-free-for-open-source-projects Bot commented Jun 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

davidfirst commented Jun 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

qodo-free-for-open-source-projects Bot commented Jun 12, 2026 •

edited

Loading

qodo-free-for-open-source-projects Bot commented Jun 12, 2026 •

edited

Loading

qodo-free-for-open-source-projects Bot commented Jun 12, 2026 •

edited

Loading

qodo-free-for-open-source-projects Bot commented Jun 12, 2026 •

edited

Loading