Skip to content

Conversation

@liuxiaopai-ai
Copy link
Contributor

Summary

The file tree API (/api/files) and file preview API (/api/files/preview) have ineffective path safety checks that could allow reading arbitrary files on the host system.

Problem

/api/files (file tree)

// Current code — always returns true (comparing a path to itself)
if (!isPathSafe(resolvedDir, resolvedDir)) { ... }

/api/files/preview (file preview)

// Current code — always returns true (a file is always inside its parent)
const dir = path.dirname(resolvedPath);
if (!isPathSafe(dir, resolvedPath)) { ... }

This means requests like GET /api/files/preview?path=/etc/passwd will succeed and return the file contents.

Fix

  • Added a baseDir query parameter to both APIs. The frontend can pass the session's workingDirectory as the trust boundary.
  • When baseDir is provided, the requested path is validated to be within it using the existing isPathSafe() function.
  • For /api/files/preview without baseDir, falls back to restricting access to the user's home directory (prevents reading system files like /etc/passwd).
  • Backward compatible: existing callers without baseDir continue to work, with the home directory fallback providing a reasonable safety net.

Security Note

CodePilot runs locally so the attack surface is limited, but proper validation prevents accidental exposure when the dev server is bound to non-localhost interfaces or accessed via browser extensions.

Tests

Added 13 unit tests (src/__tests__/unit/files-security.test.ts) covering:

  • ✅ Paths within project directory (allowed)
  • ✅ Paths outside project directory (blocked)
  • ✅ Path traversal via ../ (blocked)
  • ✅ Directory prefix attacks (/project vs /project-evil) (blocked)
  • ✅ Symlink escape attempts (blocked)
  • ✅ System file access (/etc/passwd) (blocked)
  • ✅ Edge cases (root path, same path)

All 13 tests pass:

# tests 13
# pass 13
# fail 0

Run with: npx tsx --test src/__tests__/unit/files-security.test.ts

Copy link
Owner

@op7418 op7418 left a comment

Choose a reason for hiding this comment

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

Review: 需要修改后合并

安全修复方向正确,isPathSafe 逻辑修复有价值。但有两个问题需要解决:

1. 无 baseDir 时跳过校验(严重)

/api/files/route.ts 没有 baseDir 参数时完全不做校验(没有 else 分支),旧版前端调用时仍然没有路径校验。与 /api/files/preview 的 fallback 到 home 目录策略不一致。

建议:无 baseDir 时 fallback 到 os.homedir(),与 preview API 保持一致。

2. baseDir 来自客户端可被伪造(中等)

baseDir 通过 URL 查询参数传入,攻击者可设置 baseDir=/ 绕过所有限制。

建议:如果要做安全修复就应做彻底。理想情况下 baseDir 应从服务端 session 获取。至少应添加白名单校验(如限制在 home 目录子路径下)。

3. 测试清理(轻微)

it('cleanup test fixtures', ...) 应用 afterAll() hook 代替。


代码质量和测试覆盖都很好,修复上述问题后即可合并。

The file tree API (/api/files) and file preview API (/api/files/preview)
had ineffective path safety checks that could allow reading arbitrary
files on the host system.

Issues fixed:
- /api/files: isPathSafe(resolvedDir, resolvedDir) always returns true
  since it compares the directory against itself
- /api/files/preview: isPathSafe(dirname, filePath) always returns true
  since a file is always inside its own parent directory

Changes:
- Add baseDir parameter to both APIs so the frontend can pass the
  session's working directory as a trust boundary
- When baseDir is provided, validate that the requested path is within it
- For /api/files/preview without baseDir, fall back to restricting access
  to the user's home directory (prevents /etc/passwd etc.)
- Add comprehensive unit tests covering path traversal, prefix attacks,
  symlink escapes, and edge cases (13 tests, all passing)

Security: This is a defense-in-depth measure. CodePilot runs locally so
the attack surface is limited, but proper validation prevents accidental
exposure when the dev server is bound to non-localhost interfaces.
… afterAll

Address all 3 points from @op7418's review:

1. No baseDir fallback (severe): Both /api/files and /api/files/preview
   now fall back to os.homedir() when no baseDir is provided, preventing
   access to arbitrary system directories/files.

2. baseDir bypass via baseDir=/ (medium): Both APIs now validate that
   baseDir itself is within the user's home directory. Setting baseDir=/
   will be rejected with 403.

3. Test cleanup (minor): Replaced 'it(cleanup...)' with proper after()
   hook. Added 6 new tests for baseDir validation (18 total, all pass).
@liuxiaopai-ai liuxiaopai-ai force-pushed the fix/file-api-path-traversal branch from b9134bb to 0f05e4f Compare February 8, 2026 13:36
@liuxiaopai-ai
Copy link
Contributor Author

已按 review 修改并 rebase 到最新 main:

修改内容:

  1. ✅ 无 baseDir 时 fallback 到 homedir
    /api/files/route.ts 现在与 /api/files/preview 一致,无 baseDir 时限制在用户 home 目录下,防止扫描任意系统目录。

  2. ✅ baseDir 白名单校验
    两个 API 都新增了 baseDir 本身必须在 home 目录下的校验。baseDir=/ 会返回 403,彻底防止绕过。

  3. ✅ 测试清理改用 after() hook
    替换了 it("cleanup...")after()。新增 6 个 baseDir 校验测试,共 18 个测试全部通过。

@op7418 op7418 merged commit 487c0a5 into op7418:main Feb 9, 2026
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