Skip to content

feat(files): Files pane with tree, preview and add-to-prompt (#1506 H1) - #1508

Merged
currentbits merged 5 commits into
mainfrom
coder/1506-h1-files-pane-c88282
Oct 6, 2026
Merged

currentbits merged 5 commits into
mainfrom
coder/1506-h1-files-pane-c88282

Conversation

@currentbits

Copy link
Copy Markdown
Owner

Item H1 of #1506. This ships the declared but unshipped files pane (in the Views menu).

What changed

Security: files:tree and files:read go through resolveUnderRoot, plus a new realpath check. The existing check alone would have let a symlink inside the checkout read outside it. Tests cover ../, outside absolute paths, and symlinked files and dirs.

Verified independently, on main merged with this branch and #1507:

  • typecheck exits 0 and vite build passes.
  • test:renderer passes 2841 of 2841.
  • test:electron passes 4242 of 4242.
  • Screenshots reviewed: selection, chip, markdown, filter, image, too-large.

Merge note: the branch includes a merge of main resolving prop conflicts with #1505 in App.tsx and ThreadView.tsx (kept both sides).

🤖 Generated with Claude Code

Willem and others added 5 commits October 6, 2026 16:51
Lazy per-directory listing that drops gitignored entries unless asked,
a whole-tree list for the filter, and a read-only preview with a 1 MB
cap, binary sniff and image data URLs. Both reuse resolveUnderRoot and
add a realpath check so a symlink in the checkout cannot read outside
it. Remote projects are refused.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Files pane picks reuse the review-comment chip. An `excerpt` flag keeps
them in the prompt without a note (blank diff comments are still
dropped), and the chip shows an 'Add a note' hint.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A lazy tree of the thread checkout: directories load on expand, a
toggle shows gitignored entries, a fuzzy filter searches the whole
tree, and arrows/Enter/Home/End drive it. The preview shows highlighted
read-only text (lazy highlighter), rendered markdown with a source
toggle, images, and too-large/binary states. Click and shift-click line
numbers, then Add to prompt to send the range as a composer chip.
Open in uses the editor the worktree menu remembers.

Loaded with lazyNamed, so only a small stub lands in the main chunk.
Remote projects say Files is not available.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
# Conflicts:
#	src/App.tsx
#	src/components/ThreadView.tsx
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-06T15:18:30.126676Z 8c6445e PR opened
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8c6445e996

ℹ️ 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".

Comment thread electron/ipc-files.js
if (rel.startsWith("..") || path.isAbsolute(rel)) {
throw new Error("Path is outside the thread workspace");
}
return { abs: resolved, rel: path.relative(root, resolved) };

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Open the verified path without a symlink race

When another process is modifying the checkout, it can replace a workspace symlink after realpath(resolved) passes but before readPreview calls stat/readFile: this function returns the unchecked lexical resolved path rather than an already-open, verified file handle. A sandboxed workspace process can therefore race a preview request and make the Electron main process read a file outside the workspace, defeating the newly added realpath guard; open the target without following a replacement symlink and validate/read through that same handle.

Useful? React with 👍 / 👎.

Comment thread electron/ipc-files.js
Comment on lines +210 to +213
const all = await fs.promises.readdir(root, {
recursive: true,
withFileTypes: true,
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Bound the fallback traversal before materializing entries

When the Files filter is used in a non-git Scratch workspace, readdir({ recursive: true }) walks and materializes the entire directory tree before the 20,000-file slice is applied. A Scratch folder containing a large generated dependency or build tree can therefore consume unbounded main-process memory and block the IPC request despite the documented whole-tree cap; use an incremental walk that stops once the cap is reached.

Useful? React with 👍 / 👎.

Comment thread electron/ipc-files.js
Comment on lines +131 to +132
async function resolveRealUnderRoot(root, raw) {
const resolved = resolveUnderRoot(root, raw);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Allow in-workspace names beginning with two dots

For a legitimate workspace file or directory named ..foo, the inherited resolveUnderRoot check treats path.relative(root, resolved) === "..foo" as an escape because it tests startsWith(".."). The tree lists such an entry, but this added preview/directory-resolution path then rejects it as outside the workspace; the containment check should distinguish the .. path segment from ordinary names beginning with two dots.

Useful? React with 👍 / 👎.

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.

1 participant