feat(extensions): add host-mediated workspace document reads and writes - #674
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Greptile SummaryThe PR adds a public
Confidence Score: 4/5The PR should not merge until host-mediated writes cannot escape through symlinks or target a path rewritten only by a presentation transform. The new write path can replace an external symlink target despite lexical root confinement, and it trusts mutable post-transform file paths rather than an authoritative mapping to the reviewed working-tree document. Files Needing Attention: src/ui/lib/extensionWorkspace.ts, src/ui/App.tsx, src/extensions/apply.ts
|
| Filename | Overview |
|---|---|
| src/ui/lib/extensionWorkspace.ts | Introduces the central read/write policy, but lexical confinement permits symlink escapes and post-transform paths are treated as authoritative write destinations. |
| src/ui/App.tsx | Wires workspace controls into command contexts, consent dialogs, filesystem writes, and reloads; the final write inherits unsafe target decisions from the policy. |
| src/extension-api/types.ts | Adds the documented public workspace request, result, and command-context types without importing internal modules. |
| src/ui/lib/extensionWorkspace.test.ts | Covers review-kind, malformed-input, file-state, side, and lexical traversal cases, but lacks symlink and transformed-path authority cases. |
| src/ui/AppHost.workspace.test.tsx | Exercises real reads, accepted and declined writes, reload behavior, and unavailable review kinds, without covering the two unsafe target-resolution mechanisms. |
Sequence Diagram
sequenceDiagram
participant E as Extension command
participant W as ctx.workspace
participant P as Workspace policy
participant D as Attributed dialog queue
participant FS as Filesystem
participant H as AppHost reload
E->>W: readDocument(fileId, side)
W->>P: resolve reviewed source fetcher
P-->>W: bound read or null
W-->>E: text or null
E->>W: writeDocument(fileId, text)
W->>P: resolve working-tree target
P-->>W: target or unavailable
W->>D: confirm extension and path
D-->>W: accepted or cancelled
W->>FS: writeFile(target, text)
FS-->>W: success or failure
W-->>E: write result
W-->>H: soft reload after success
Prompt To Fix All With AI
### Issue 1
src/ui/lib/extensionWorkspace.ts:160-161
**Symlinks bypass workspace confinement**
When a reviewed path is a symlink to a file outside the review root, the lexical `resolve`/`relative` check accepts it and `writeFile` follows the symlink, causing confirmation for an in-repository path to overwrite an external file.
**How this was verified:** The target path is never canonicalized or rejected as a symlink before it reaches `writeFile`.
### Issue 2
src/ui/lib/extensionWorkspace.ts:141-160
**Transformed paths redirect workspace writes**
When a changeset transform preserves a file's id and render metadata but rewrites its path, workspace target resolution treats that presentation path as authoritative and writes it, causing a different working-tree file to be overwritten than the document represented by the reviewed source.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: "chore: add changeset for workspace reads..." | Re-trigger Greptile
| const absolutePath = resolve(root, path); | ||
| const relativePath = relative(root, absolutePath); |
There was a problem hiding this comment.
Symlinks bypass workspace confinement
When a reviewed path is a symlink to a file outside the review root, the lexical resolve/relative check accepts it and writeFile follows the symlink, causing confirmation for an in-repository path to overwrite an external file.
How this was verified: The target path is never canonicalized or rejected as a symlink before it reaches writeFile.
Context Used: security.mdc Cursor rule (source)
Knowledge Base Used: Extension System: API, Loading, and Host Integration
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/ui/lib/extensionWorkspace.ts
Line: 160-161
Comment:
**Symlinks bypass workspace confinement**
When a reviewed path is a symlink to a file outside the review root, the lexical `resolve`/`relative` check accepts it and `writeFile` follows the symlink, causing confirmation for an in-repository path to overwrite an external file.
**How this was verified:** The target path is never canonicalized or rejected as a symlink before it reaches `writeFile`.
**Context Used:** security.mdc Cursor rule ([source](https://github.com/modem-dev/modem/blob/main/.cursor/rules/security.mdc))
**Knowledge Base Used:** [Extension System: API, Loading, and Host Integration](https://app.greptile.com/modem/-/custom-context/knowledge-base/modem-dev/hunk/-/docs/extension-system.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| const file = files.find((candidate) => candidate.id === fileId); | ||
| if (!file) { | ||
| return { writable: false, detail: `No reviewed file has the id "${fileId}".` }; | ||
| } | ||
|
|
||
| const path = normalizeDiffPath(file.path) ?? file.path; | ||
|
|
||
| if (readMetadataChangeType(file.metadata) === "deleted") { | ||
| return { writable: false, detail: `${path} was deleted in this review; it has no new side.` }; | ||
| } | ||
|
|
||
| if (file.isBinary) { | ||
| return { writable: false, detail: `${path} is binary; workspace writes are text-only.` }; | ||
| } | ||
|
|
||
| if (file.isTooLarge) { | ||
| return { writable: false, detail: `${path} was skipped as too large to load.` }; | ||
| } | ||
|
|
||
| const absolutePath = resolve(root, path); |
There was a problem hiding this comment.
Transformed paths redirect workspace writes
When a changeset transform preserves a file's id and render metadata but rewrites its path, workspace target resolution treats that presentation path as authoritative and writes it, causing a different working-tree file to be overwritten than the document represented by the reviewed source.
Knowledge Base Used: Extension System: API, Loading, and Host Integration
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/ui/lib/extensionWorkspace.ts
Line: 141-160
Comment:
**Transformed paths redirect workspace writes**
When a changeset transform preserves a file's id and render metadata but rewrites its path, workspace target resolution treats that presentation path as authoritative and writes it, causing a different working-tree file to be overwritten than the document represented by the reviewed source.
**Knowledge Base Used:** [Extension System: API, Loading, and Host Integration](https://app.greptile.com/modem/-/custom-context/knowledge-base/modem-dev/hunk/-/docs/extension-system.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.591e440 to
9575563
Compare
356e70a to
5cec20e
Compare
9575563 to
466361a
Compare
5cec20e to
41d71cc
Compare
466361a to
7425c13
Compare
Extension isolation is crash containment, not a sandbox, so nothing stopped an extension from reaching node:fs directly; there was just no supported contract. ctx.workspace.writeDocument gives writes an explicit one: targets are named by reviewed-file id only, available exclusively for working-tree reviews, confirmed through the attributed extension dialog queue, confined to the review root, and followed by the same soft reload the refresh key runs so the review reflects the write. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015aJpBUupsP9L7Wtd7MEzmU
An extension composing replacement text had no mediated way to read the current document from a command handler; reads existed only inside file-view layout requests. ctx.workspace.readDocument closes the loop (read, transform, writeDocument) through the same one-policy module. Reads never prompt and work in every review kind: they expose exactly what the review already shows, unlike writes, which stay working-tree only and confirmed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015aJpBUupsP9L7Wtd7MEzmU
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015aJpBUupsP9L7Wtd7MEzmU
…le sessions The write path's root confinement was purely lexical, but git tracks symlinks-to-files as ordinary reviewable entries, so a confirmed write through a reviewed link would follow it anywhere on disk while the dialog named only the innocent repo-relative path. writeDocument now runs a filesystem verification before ever prompting: lstat refuses links and non-files, a deleted target is not recreated, and the realpath'd parent must stay inside the realpath'd root. The policy module stays lexical and fs-free; both confinements share one containment predicate so they cannot drift. Writes also now require a session that can reload. A review started with --agent-context - can never refresh, which silently broke the documented promise that a successful write reloads the review; the reload predicate moved to src/core/inputReload.ts so the fs-free policy can share it with App and startup. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015aJpBUupsP9L7Wtd7MEzmU
4d9adcb to
b2edc6f
Compare
113a946 to
59b651f
Compare
Summary
Adds a host-mediated workspace API to extension command contexts:
readDocument(fileId, side)canWriteDocument(fileId)writeDocument({ fileId, text })Extensions could already access
node:fs; this API instead makes file access supported, attributable, consented, and synchronized with the review. It provides the foundation for inline diff editing.Behavior
"unavailable"reason.{ ok: false, reason: "cancelled" }.Validation
Coverage includes policy and request validation, real-filesystem confinement and symlink cases, consent-window target changes, and App-level Git flows for writes, cancellation, read-only reviews, symlink refusal, and read-transform-write behavior.
Typecheck, unit tests, PTY integration, TTY smoke tests, lint, package checks, documentation checks, and CI pass. Documentation and a changeset are included.
This PR description was generated by Pi using gpt-5.6-sol