fix(core): evaluate ignore files named with dot prefixes - #5458
Conversation
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
No issues found. LGTM! ✅
The fix correctly replaces the naive relativePath.startsWith('..') with isPathWithinRoot(), which properly distinguishes path traversal (../..secret.log) from dot-prefixed filenames (..secret.log). Tsc, eslint, and tests all pass. The removed redundant startsWith(projectRoot) guard in gitIgnoreParser is safe — isPathWithinRoot covers all those cases.
— DeepSeek/deepseek-v4-pro via Qwen Code /review
|
@qwen-code /triage |
|
Thanks for the PR, @tt-a1i! Template: headings deviate from the PR template ("Summary" / "Validation" instead of "What this PR does" / "Reviewer Test Plan", missing "Why it's needed", "Risk & Scope", and "Linked Issues" sections). Not blocking here since the content is present and the change is small — but please use the standard template for future PRs. Direction: clear-cut bugfix. The ignore parsers incorrectly reject valid filenames starting with Approach: minimal and correct. The fix replaces the naive Moving on to code review. 🔍 中文说明感谢贡献，@tt-a1i！ 模板： 标题与 PR 模板 不一致（用了 "Summary" / "Validation" 而非标准标题，缺少 "Why it's needed"、"Risk & Scope"、"Linked Issues" 章节）。鉴于内容齐全且改动较小，此次不阻塞——但后续 PR 请使用标准模板。 方向： 明确的 bugfix。忽略解析器用 方案： 最小且正确。用已有的 进入代码审查 🔍 — Qwen Code · qwen3.7-max |
Code ReviewIndependent proposal: replace No correctness issues, no security concerns, no AGENTS.md violations. The shared helper is already used elsewhere in Real-Scenario TestingRan a direct parser test script (creates a temp project with Before (main branch — bug present)After (PR #5458 applied)Unit tests (with patch)All checks pass. The fix correctly resolves the bug without regressing traversal-path rejection. 中文说明代码审查独立方案：用已有的 无正确性问题、无安全隐患、无 AGENTS.md 违规。该辅助函数已在 真实场景测试运行了直接的解析器测试脚本（创建临时项目， 修复前（main 分支）： 修复后（PR #5458）： 单元测试： 25 个测试全部通过。 — Qwen Code · qwen3.7-max |
|
This is a textbook bugfix. The root cause is clear ( The only note is the template deviation (non-standard headings) — mentioned in Stage 1, not a blocker. Approving. ✅ 中文说明这是一个教科书级的 bugfix。根因清晰（ 唯一的小问题是 PR 模板标题不一致（Stage 1 已提及），不构成阻塞。 批准 ✅ — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
✅ Local verification (real build + tests via tmux) — recommend mergeI verified this PR end-to-end on a real checkout (not just re-reading the diff). Setup: an isolated Env: Node What the fix doesBoth parsers rejected out-of-root paths with Results
Decisive evidence
Correctness notes
One note on typecheckA single local VerdictCorrect, minimal, and surgical. Fixes #5457 with no regression to the path-boundary checks, and the differential E2E confirms the actual leak is closed. LGTM — good to merge. 🇨🇳 中文版（点击展开）✅ 本地真实构建 + 测试验证（tmux）— 建议合并我在真实检出环境中对本 PR 做了端到端验证（不只是看 diff）。方式：在 PR head 环境： Node 修复内容： 两个解析器原先用 结果：
关键证据：
正确性补充：
关于类型检查： 本地出现一个 结论： 改动正确、最小、外科手术式，修复 #5457 且未对路径边界检查造成回归，差分 E2E 也证实泄漏已被关闭。LGTM，建议合并。 |
✅ Runtime E2E verification — ignore files named with dot prefixesI built and ran the real The bug & the fix
// before — '..secret.log'.startsWith('..') === true → treated as "outside root", never evaluated
if (relativePath === '' || relativePath.startsWith('..')) return false;
// after — proper boundary check (startsWith('../') + !== '..' + !isAbsolute)
if (relativePath === '' || !isPathWithinRoot(resolved, this.projectRoot)) return false;So a 1) Real CLI E2E —
|
| Arm | list_directory result |
Files handed to the model | ..secret.log |
|---|---|---|---|
pre-PR startsWith('..') |
Listed 4 item(s) (1 git-ignored) |
..secret.log, .gitignore, normal.txt, tracked.txt |
❌ leaked |
PR isPathWithinRoot |
Listed 3 item(s) (2 git-ignored) |
.gitignore, normal.txt, tracked.txt |
✅ filtered |
git status --ignored confirms ..secret.log is genuinely git-ignored — so the pre-PR listing was wrong.
2) Unit tests + revert-fix A/B
- The PR's parser suites are green: 25 passed (both
gitIgnoreParser+qwenIgnoreParser), including the 4 new cases. - Revert-only-the-source-check (keep the PR's new tests) → the two
should still evaluate files whose names start with two dotstests fail withexpected false to be true. So the new tests genuinely pin the bug, and the fix is what makes them pass. Restoring the fix → 25/25 green again.
3) Live run (tmux)
The real interactive TUI (mock-backed, --yolo) executed list_directory live and completed the turn; a terminal A/B then printed:
ARM: PR/NEW (isPathWithinRoot) list_directory result : Listed 3 item(s) (2 git-ignored) ..secret.log: ABSENT (ignored) ✓
ARM: pre-PR/OLD (startsWith(..)) list_directory result : Listed 4 item(s) (1 git-ignored) ..secret.log: PRESENT (leaked) ✗
No regression
A genuinely out-of-root path (../..secret.log) is still correctly not evaluated (isPathWithinRoot → false), covered by the PR's new should not evaluate paths outside the project root test. The ..-fix only un-blocks in-root names.
Verdict
The fix resolves a real ignore-filtering leak for ..-prefixed filenames across both parsers, verified through the actual CLI tool path; tests pin it; no regression on out-of-root paths. LGTM — good to merge. 👍
🇨🇳 中文版（点击展开）
✅ 运行时端到端验证 —— 以点号前缀命名的 ignore 文件
我从本 PR（584caecb，Node v22）构建并运行了真实的 qwen CLI，在运行时层面确认修复（不只是单测）：一个被 gitignore 的文件 ..secret.log（双点前缀）在旧代码下会穿透文件过滤工具泄漏出来，修复后被正确过滤。作为 merge 参考。
Bug 与修复
isIgnored() 会对任何"项目相对路径 startsWith('..')"的路径提前返回 —— 本意是跳过根目录之外的路径，但它也误命中了根目录内真正以 ..something 命名的文件：
// 改前 —— '..secret.log'.startsWith('..') === true → 被当成"根目录外"，根本没参与判断
if (relativePath === '' || relativePath.startsWith('..')) return false;
// 改后 —— 正确的边界判断（startsWith('../') + !== '..' + !isAbsolute）
if (relativePath === '' || !isPathWithinRoot(resolved, this.projectRoot)) return false;于是针对 .. 前缀名字的 .gitignore / .qwenignore 规则被悄悄忽略，该文件会通过每一个尊重 ignore 的工具（list_directory、glob、ripgrep、read_many_files）暴露给模型。本 PR 同时删掉了 gitIgnoreParser 里一段冗余且粗糙的 absoluteFilePath.startsWith(projectRoot) 前缀判断（它有兄弟目录误判，例如 /proj vs /proj-evil）。
1）真实 CLI 端到端 —— 在 .gitignore 含 ..secret.log 的工作区上 list_directory
驱动真实 CLI 的 list_directory 工具（先 readdir，再做 gitignore 过滤），目标是一个含 ..secret.log、normal.txt、tracked.txt、.gitignore 的 git 仓库。同一套装置，只替换编译后的 parser 逻辑（dist 级 A/B）：
| 分支 | list_directory 结果 |
交给模型的文件 | ..secret.log |
|---|---|---|---|
改前 startsWith('..') |
Listed 4 item(s) (1 git-ignored) |
..secret.log、.gitignore、normal.txt、tracked.txt |
❌ 泄漏 |
本 PR isPathWithinRoot |
Listed 3 item(s) (2 git-ignored) |
.gitignore、normal.txt、tracked.txt |
✅ 被过滤 |
git status --ignored 确认 ..secret.log 确实被 git 忽略 —— 所以改前的列表是错的。
2）单测 + 撤销修复 A/B
- PR 的 parser 套件全绿：25 passed（
gitIgnoreParser+qwenIgnoreParser），含 4 个新增用例。 - 只把源码里的判断撤回旧逻辑（保留 PR 的新测试）→ 两个
should still evaluate files whose names start with two dots测试失败，报expected false to be true。说明新测试确实钉住了这个 bug，而修复正是让它们通过的原因。恢复修复 → 重新 25/25 全绿。
3）实时运行（tmux）
真实交互式 TUI（mock 后端，--yolo）实时执行了 list_directory 并完成该轮；随后终端里的 A/B 打印：
ARM: PR/NEW (isPathWithinRoot) list_directory result : Listed 3 item(s) (2 git-ignored) ..secret.log: ABSENT (ignored) ✓
ARM: pre-PR/OLD (startsWith(..)) list_directory result : Listed 4 item(s) (1 git-ignored) ..secret.log: PRESENT (leaked) ✗
无回归
真正在根目录之外的路径（../..secret.log）仍然被正确地不参与判断（isPathWithinRoot → false），由 PR 新增的 should not evaluate paths outside the project root 测试覆盖。这个 .. 修复只放开了根目录内的名字。
结论
该修复解决了两个 parser 上针对 .. 前缀文件名的真实 ignore 过滤泄漏，并通过真实 CLI 工具路径验证；测试钉死了它；对根目录外路径无回归。LGTM —— 可以合并。 👍
3 participants
Summary
.gitignoreand.qwenignoreparsers..../..secret.logoutside the ignore checksFixes #5457
Validation
npx vitest run packages/core/src/utils/gitIgnoreParser.test.ts --testNamePattern "two dots"npx vitest run packages/core/src/utils/qwenIgnoreParser.test.ts --testNamePattern "two dots"npx vitest run packages/core/src/utils/gitIgnoreParser.test.ts packages/core/src/utils/qwenIgnoreParser.test.tsnpx eslint packages/core/src/utils/gitIgnoreParser.ts packages/core/src/utils/gitIgnoreParser.test.ts packages/core/src/utils/qwenIgnoreParser.ts packages/core/src/utils/qwenIgnoreParser.test.tsnpx prettier --check packages/core/src/utils/gitIgnoreParser.ts packages/core/src/utils/gitIgnoreParser.test.ts packages/core/src/utils/qwenIgnoreParser.ts packages/core/src/utils/qwenIgnoreParser.test.ts && git diff --checknpm run typecheck --workspace=packages/coreAI Assistance Disclosure
I used Codex to review the changes, sanity-check the implementation against existing patterns, and help spot potential edge cases.