fix: skip symlinks in file listing and archive generation

[hobostay]

Summary

• Adds lstat checks to skip symbolic links in collectFiles() and collectArchiveEntries()

Problem

Both collectFiles (used by GET /api/projects/:id/files) and collectArchiveEntries (used by project archive ZIP downloads) use Dirent.isDirectory() which follows symlinks. A symlink placed inside a project directory that points to an arbitrary location (e.g. /etc) would be followed, leaking files outside the project tree in file listings and archive downloads.

Fix

Use lstat to detect symbolic links before recursing or including entries. Symlinks are silently skipped.

Test plan

• Create a symlink inside a project directory: ln -s /etc/project-secrets .od/projects/<id>/leak
• GET /api/projects/<id>/files should NOT include files from the symlink target
• Archive download should NOT include files from the symlink target

🤖 Generated with Claude Code

[lefarcen]

Hi @hobostay! 🎉

Thanks for the contribution — this is a critical security fix for symlink directory traversal.

I will run a deep review and get back to you within 24h.

Thanks for making open-design better!
— open-design team

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 92681b316e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Re-throw unexpected lstat failures

The new catch { continue; } around lstat(full) silently drops entries on any filesystem error (for example EACCES, EIO, or transient I/O failures), which can produce incomplete /files responses and ZIP exports without signaling an error to callers. This is a regression from prior behavior where unexpected file-access errors surfaced, allowing clients to detect failure instead of receiving partial data; only expected race conditions like ENOENT should be skipped.

Useful? React with 👍 / 👎.

[mrcfps]

@hobostay thanks for tightening the project file/archive traversal path. I found one remaining symlink-root escape in the archive flow that keeps the security fix incomplete for targeted ZIP downloads.

_{Generated by Looper 0.5.4 · runner=reviewer · agent=opencode}

[mrcfps]

collectArchiveEntries() now checks each child with lstat(full), but this still misses the case where the archive root itself is a symlink. buildProjectArchive() accepts a root parameter, resolves it under the project, then stat(archiveRoot) follows symlinks before calling this walker. If root is leak and .od/projects/<id>/leak -> /etc, readdir(dir) has already crossed into /etc, and lstat(path.join(dir, child)) reports the external child (for example /etc/passwd) rather than the parent leak symlink. That means a targeted archive download can still include files outside the project tree, so the security fix does not fully satisfy the PR goal.

Please reject a symlink archive root before the initial stat/readdir (for example lstat(archiveRoot) and fail/skip if isSymbolicLink()), and consider having collectArchiveEntries() validate the current directory before reading it so every recursive entry point enforces the same invariant. A fixture covering buildProjectArchive(projectRoot, id, 'leak') where leak points outside the project would lock this down. 🔒

[lefarcen]

Hey @hobostay, thank you for catching this critical symlink traversal vulnerability! 🔒

The fix correctly blocks symlinks in the recursive walkers ( and ), but there are three additional attack surfaces that still follow symlinks:

🔒 P1 Issues (must fix before merge)

1. Archive root itself can be a symlink

Location: apps/daemon/src/projects.ts:106 in buildProjectArchive()

stat(archiveRoot) follows leak -> /etc, accepts it as a directory, then collectArchiveEntries() walks the target's real children and zips them.

Attack: /api/projects/:id/archive?root=leak where leak -> /etc

Fix: Use lstat(archiveRoot) before recursing and reject isSymbolicLink() roots. Add a test for this case — should reject with 400.

---

2. Direct file reads still follow symlinks

Location: apps/daemon/src/projects.ts:298 in readProjectFile()

readProjectFile() only does lexical resolveSafe(), then readFile() and stat() follow leak -> /etc/passwd.

Attack:

• /api/projects/:id/raw/leak
• /api/projects/:id/files/leak

These can still leak files even though listings now hide the symlink.

Fix: Centralize "safe regular project file" resolution that lstats each segment, rejects symlinks, and opens the final file with no-follow semantics where possible (e.g., fs.openSync with O_NOFOLLOW flag on systems that support it).

---

3. TOCTOU race in archive packing

Location: apps/daemon/src/projects.ts:131 in archive packing logic

collectArchiveEntries() lstats a path, stores fullPath, then later readFile(entry.fullPath) follows whatever is there at read time. A symlink swap between collection and packing can escape the project.

Fix: Revalidate and open each file with no-follow immediately before reading, or return/open file handles instead of unchecked paths.

---

⚠️ P3 Issue (nice to have)

Error handling in lstat guards

Location: apps/daemon/src/projects.ts:56 and :280 (the new try/catch blocks)

The new lstat guards silently skip every lstat failure, not just vanished files or symlinks. That can turn EPERM/EACCES/transient filesystem errors into incomplete successful listings/archives.

Suggestion: Silent skip for symlinks is reasonable; for errors, catch ENOENT specifically for races and otherwise throw or log:

try {
  const ls = await lstat(full);
  if (ls.isSymbolicLink()) continue;
} catch (err) {
  if (err.code === 'ENOENT') continue; // file vanished, skip
  throw err; // propagate permission/IO errors
}

---

📋 Test Coverage

Location: apps/daemon/tests/project-archive.test.ts

The security fix is untested. Existing archive tests cover filtering dotfiles and sidecars, but not symlinks.

Add tests:

• Create a symlink to an external temp directory/file
• Assert listing omits it
• Assert full archive omits it
• Assert scoped archive rejects it (root symlink case)
• Assert direct reads reject it (once fixed)

---

Once these are addressed, this will be a solid security hardening. Thanks again for finding this! 🙏

[lefarcen]

Hey @hobostay, thank you for catching this critical symlink traversal vulnerability! 🔒

The fix correctly blocks symlinks in the recursive walkers (collectFiles and collectArchiveEntries), but there are three additional attack surfaces that still follow symlinks:

🔒 P1 Issues (must fix before merge)

1. Archive root itself can be a symlink

Location: apps/daemon/src/projects.ts:106 in buildProjectArchive()

stat(archiveRoot) follows leak -> /etc, accepts it as a directory, then collectArchiveEntries() walks the target's real children and zips them.

Attack: /api/projects/:id/archive?root=leak where leak -> /etc

Fix: Use lstat(archiveRoot) before recursing and reject isSymbolicLink() roots. Add a test for this case — should reject with 400.

---

2. Direct file reads still follow symlinks

Location: apps/daemon/src/projects.ts:298 in readProjectFile()

readProjectFile() only does lexical resolveSafe(), then readFile() and stat() follow leak -> /etc/passwd.

Attack:

• /api/projects/:id/raw/leak
• /api/projects/:id/files/leak

These can still leak files even though listings now hide the symlink.

Fix: Centralize "safe regular project file" resolution that lstats each segment, rejects symlinks, and opens the final file with no-follow semantics where possible (e.g., fs.openSync with O_NOFOLLOW flag on systems that support it).

---

3. TOCTOU race in archive packing

Location: apps/daemon/src/projects.ts:131 in archive packing logic

collectArchiveEntries() lstats a path, stores fullPath, then later readFile(entry.fullPath) follows whatever is there at read time. A symlink swap between collection and packing can escape the project.

Fix: Revalidate and open each file with no-follow immediately before reading, or return/open file handles instead of unchecked paths.

---

⚠️ P3 Issue (nice to have)

Error handling in lstat guards

Location: apps/daemon/src/projects.ts:56 and :280 (the new try/catch blocks)

The new lstat guards silently skip every lstat failure, not just vanished files or symlinks. That can turn EPERM/EACCES/transient filesystem errors into incomplete successful listings/archives.

Suggestion: Silent skip for symlinks is reasonable; for errors, catch ENOENT specifically for races and otherwise throw or log:

try {
  const ls = await lstat(full);
  if (ls.isSymbolicLink()) continue;
} catch (err) {
  if (err.code === 'ENOENT') continue; // file vanished, skip
  throw err; // propagate permission/IO errors
}

---

📋 Test Coverage

Location: apps/daemon/tests/project-archive.test.ts

The security fix is untested. Existing archive tests cover filtering dotfiles and sidecars, but not symlinks.

Add tests:

• Create a symlink to an external temp directory/file
• Assert listing omits it
• Assert full archive omits it
• Assert scoped archive rejects it (root symlink case)
• Assert direct reads reject it (once fixed)

---

Once these are addressed, this will be a solid security hardening. Thanks again for finding this! 🙏