Repository navigation
fix(agent-tools): keep desktop read/write/edit inside the workspace - #238
Merged
Merged
Conversation
What was broken: the desktop fs tools resolved their target lexically (path.resolve against the workspace root) and handed that spelling to pi, which opens it as-is. A symlink inside the workspace therefore turned any read, write or edit into an operation outside the workspace, and the permission layer could not see it, because it compares paths lexically too: a symlinked path produced no prompt at all. A dangling symlink was worse - a write through it created the file it pointed at, outside. Why this fix: the desktop fs tools now run their target through a real-path fence. The target's real location - following symlinks, and following dangling ones by hand the way the turn-diff capture fence does (#232) - has to sit inside the workspace. The fence only validates: pi still receives the lexical absolute path, byte for byte, because the ledger key has to stay aligned with pi's own resolveToCwd output. Scope: the fence governs the paths the workspace claims. A target that is already outside the workspace lexically keeps its existing approval flow in the permission layer; turning that approval into a hard failure would break cross-workspace reads that users approve today, and that change belongs to the permission layer, not here. The same dangling-symlink blind spot is closed in the desktop and cloud path guards (a dangling link was treated as a missing file and compared lexically) and in the local-runtime-v2 write path (a realpath failure fell back to the lexical spelling, i.e. a path inside the workspace). How it was verified: with a real probe against this same base commit, the symlink read / write / edit, the directory-symlink read, the dangling symlink write and the local-runtime-v2 dangling write path all escaped before and are refused now, while in-workspace symlinks, a symlinked workspace root and ordinary reads / writes / edits keep working. The new suites (23 tests) cover the same ground.
modacker
added a commit
that referenced
this pull request
Oct 23, 2026
…de packages/webui (#240) The webui branch is a downstream client of MiniMax-AI/minimax-code. Changes to the upstream packages do not flow back upstream and turn into unreviewed divergence at the next upstream sync, so they do not belong here. This removes the upstream-package half of the work merged today: - packages/local-runtime/src/turns/file-changes.ts (#232, #234) - packages/agent-tools/** (#238) - packages/local-runtime-v2/src/service/workspace/operations/workspace-path.ts (#238) - packages/local-runtime-v2/src/application/conversation/conversation-application.ts and .../stream/session-stream-service.ts (#237) Everything under packages/webui stays: the queue-drain client half, the regenerated visual baselines, the browser test budgets and the CJK font gate. The inventories follow the files: eight removed paths leave release/public-source.json and six removed suites leave test/vitest-suites.json. Co-authored-by: probe <probe@local>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What broke
The desktop fs tools —
read,writeandedit— resolved their target lexically (path.resolveagainst the workspace root) and handed that spelling to pi, which opens the path as-is.A symlink inside the workspace therefore turned any of the three into an operation outside the workspace:
readthrough a symlink returned the content of a file outside it.writethrough a symlinked directory created a file outside it.editthrough a symlink rewrote a file outside it.writethrough it created the file it pointed at, outside the workspace.The permission layer could not see any of it, because it compares paths lexically too. A symlinked path produced no prompt at all — not even the
askthe layer raises for a lexically-outside path.What this changes
The three tools now run their target through a real-path fence: the target's real location — following symlinks, and following dangling ones by hand — has to sit inside the workspace.
packages/agent-tools/src/shared/workspace-real-path.ts(new) holds the resolution:realpath, then the nearest existing ancestor with the missing segments re-attached, then the dangling-symlink branch thatrealpathcannot handle (it fails on a dangling link exactly as it fails on a missing file, but a write through such a link creates the file it points at). A path that cannot be resolved at all — a symlink loop, an unreadable path — is refused rather than guessed at.packages/agent-tools/src/desktop/workspace-fence.ts(new) compares the target's real path with the workspace root's real path. The comparison ispath.relative-based and therefore case-insensitive on Windows, where the filesystem is.packages/agent-tools/src/desktop/local-pi-tools.tscalls the fence in all three tools and returns the violation as a tool error, so the model is told which path it asked for and where that path really points.The fence only validates. pi still receives the lexical absolute path, byte for byte: the ledger key has to stay aligned with pi's own
resolveToCwdoutput.Scope
The fence governs the paths the workspace claims — the ones that are inside it as a string. A target that is already outside lexically (
/etc/passwd,~/notes,../../elsewhere) keeps the permission layer's existing approval flow, where the user is asked before the tool runs. Turning that approval into a hard failure would break the cross-workspace reads people approve today, and that change belongs to the permission layer, not here. The hole this closes is the one where the permission layer does not even get to ask.The same blind spot, closed in three other places
packages/agent-tools/src/desktop/path-guard.tstreated a dangling symlink as a missing file:realpathfailed, the basename was re-attached to the parent, and the resulting lexical path was compared against the root — so a dangling link pointing outside resolved to a path inside the workspace and was allowed. It now follows the link the same way.packages/agent-tools/src/cloud/matrix-tools/path-guard.tsis the deliberate cloud twin of that file (see its own header note), so it gets the same branch and the same comment.packages/local-runtime-v2/src/service/workspace/operations/workspace-path.tsfell back to the lexical spelling whenrealpathfailed, i.e. it returned a path inside the workspace for a dangling link pointing outside. Its write path now resolves the link instead.How it was checked
A probe run against the commit this branch was cut from and against the fix, with the same attacks:
readthrough a symlink to an outside filewritethrough a symlinked directoryeditthrough a symlinkwritethrough a dangling symlinkreadof an ordinary file in the workspacereadthrough a symlink to another file in the workspaceThe new suites cover the same ground — 23 tests across
packages/agent-tools/src/desktop/path-guard.test.ts(7),packages/agent-tools/src/desktop/workspace-fence.test.ts(10) andpackages/local-runtime-v2/src/service/workspace/operations/workspace-path.test.ts(6). The branch was rebased onto the currentwebuiafterwards, and those 23 tests pass on the rebased branch (3 files, 23 passed).tsc -p tsconfig.standalone.json --noEmitpasses, the build andcheck:standalonepass, andnode scripts/source-inventory.mjspasses with the manifest regenerated rather than hand-edited.Not addressed here
O_NOFOLLOW/openat2(RESOLVE_BENEATH)), which is a larger change than this refusal.realpathcannot tell a hardlink from a regular file: a hardlink inside the workspace that also points at a file outside it passes every path check, and writing to it changes the outside file. The path-based boundary does not cover it.