fix(webui): withhold the body of a diff entry outside the workspace by modacker · Pull Request #244 · mcode-webui/minimax-code

modacker · 2026-10-23T08:22:09Z

fix(webui): withhold the body of a diff entry outside the workspace

The vulnerability

A turn's diff lists the files a turn touched, and one of those files can be outside the
workspace. The runtime's capture layer records an out-of-workspace write as an absolute path, so
the entry reaches the client with that file's content attached — the diff text, the patch hunks,
the file's lines.

The client rendered it. Three surfaces put a diff body on screen:

the transcript's diff card (features/transcript/ui/DiffCard.tsx) — its own review panel,
the workspace turn-review panel (features/workspace/ui/WorkspacePanels.tsx),
the workspace review page (features/review/ui/ReviewSettingsPage.public.tsx).

So a turn that touched a file outside the workspace showed that file's content in the browser.

Why this is fixed in the WebUI layer

The fix that stopped the collection at the source was made upstream in
packages/local-runtime/src/turns/file-changes.ts (commit b462add2) and reverted in PR #240,
because it changed a package this fork may not change. The upstream fix was not wrong; it was in
the wrong layer for this repository, whose boundary is packages/webui alone.

The boundary is therefore drawn on the client side, which is the last point before the content
reaches a screen. This is a mitigation with a stated scope, not a replacement for the upstream
fix — see the limits below.

The implementation

A new projection module, packages/webui/src/client/projection/common/diff-workspace-boundary.ts,
owns the decision:

isWebuiDiffPathWithinWorkspace(workspaceDir, filePath) — the lexical comparison.
projectWebuiDiffFile(file, workspaceDir) / projectWebuiDiffFiles(files, workspaceDir) — the
projection. An entry outside the workspace keeps its name, status and added/deleted line
counts, and loses its body (diff and patch). The entry is not dropped, and the counts
are not changed.
WEBUI_DIFF_OUTSIDE_WORKSPACE_NOTICE — what the surface says instead: 「该文件在工作区之外，
内容未展示。」

The boundary is applied by the projection, not by the components: a surface renders the entries
it is given, and an entry for a file outside the workspace arrives without a body — there is
nothing for a renderer to leak even if it forgets to ask. The verdict travels with the entry
(withinWorkspace), so the surface that renders the body and the projection that withholds it
cannot disagree.

The comparison is lexical, on posix-style strings, the same normalization the message
file-reference reader uses (features/transcript/projection/message-file-reference.ts), extended to
resolve .. and to compare absolute paths segment by segment:

a relative path is under the root by construction, and is refused when it climbs out
(../outside.txt, src/../../outside.txt);
an absolute path is compared with the root segment by segment, so a sibling directory
whose name starts with the root's name (/work/project-backup against the root /work/project)
is outside — a string prefix would have called it inside;
both separator spellings and a Windows drive letter are accepted.

Applied at all three surfaces that show a diff body, plus the two places the entries travel:

Surface / path	File
the diff card's own review panel	`packages/webui/src/client/features/transcript/ui/DiffCard.tsx`
the workspace turn-review panel	`packages/webui/src/client/features/workspace/ui/WorkspacePanels.tsx`
the workspace review page	`packages/webui/src/client/features/review/ui/ReviewSettingsPage.public.tsx`
the view the diff card hands to the review panel	`packages/webui/src/client/features/transcript/ui/MessageItem.tsx`

The workspace review page also does not ask the runtime for the patch of a file outside the
workspace: the content is not going to be shown, so it does not have to travel.

The decision is recorded in ADR 0024
and its residual limits in packages/webui/docs/known-gaps.md §10.

What the tests cover

packages/webui/test/unit/diff-workspace-boundary.test.tsx (16 cases) asserts the boundary at
both levels, because either alone would pass with the other broken:

The projection — the body is blocked in the data:

a relative path inside the workspace → the body is kept;
an absolute path outside the workspace → the body and the patch object are removed;
a path that walks out with .. → the body is removed;
the entry, its name and its counts survive the removal;
an unknown workspace root → the body is kept (no false positive);
a sibling directory sharing the root's name → outside;
the list keeps every entry, in order.

The surfaces — the body is not rendered:

the diff card renders the body for a file inside the workspace, and does not render it for a file
outside — the notice is shown instead, and the file stays in the list with its counts;
the diff card still renders the body when the workspace root is unknown;
the workspace review panel renders the body inside the workspace and the notice outside, and
keeps the file's name and counts either way.

A separate assertion pins the known limit rather than leaving it to be discovered: a symlink
inside the workspace that points outside it is not detectable — the path is a relative one and
stays contained.

The frozen path-metadata oracle
(packages/webui/test/gates/fixtures/path-metadata-f00.json) records the new module's
classification, and release/public-source.json and test/vitest-suites.json are regenerated (not
hand-edited) for the new files.

Capability limits — stated, not papered over

The comparison is lexical. It reads the path as written; it does not resolve it.
A symlink out of the workspace is not detectable. A symlink inside the workspace that points
outside it resolves outside, and the browser has no filesystem to see that: the entry's path is a
relative one, so it stays contained and its body is shown. Telling the two apart needs
realpath on every target, with careful handling of files that do not exist yet — which is
runtime-side work. This is the one case the fix does not cover.
An unknown workspace root is not a boundary. When the client does not know the root, the
entries are shown as they are. A check that hid everything it could not place would hide ordinary
content as well, and "the diff shows nothing" is a silent, confusing failure. The check may
therefore say "inside" only when it can compare.
The content still travels. Nothing here stops the runtime from sending an
out-of-workspace file's patch to the browser; it stops the browser from showing it. The client
stops the content from being displayed; only the upstream change (reverted in PR revert(upstream-packages): keep the webui branch's local changes inside packages/webui #240) stops it
from being collected.
getSessionDiff is not consumed by any surface today. When one is, it goes through the same
projection.

Scope

Only packages/webui/ is touched, plus the two generated manifests the repository requires
(release/public-source.json, test/vitest-suites.json). No upstream package is changed.

Gates run

pnpm --filter @mavis/webui typecheck:server / typecheck:client / typecheck:test — pass.
node scripts/run-vitest-suite.mjs webui — 209 files pass, 3297 tests pass.
One pre-existing failure remains: packages/webui/test/integration/agent-transcript.test.ts
("Timed out waiting for held cancellation response"). It fails identically on the base commit
with this change stashed — it needs a live model provider.
node scripts/check-webui-dependency.mjs — pass (0 violations, 0 cycles, 0 role violations).
node scripts/build-webui.mjs then node scripts/check-webui-boundary.mjs — pass.
node scripts/source-inventory.mjs --write then node scripts/source-inventory.mjs — pass.
pnpm test:webui-browser — not run: no usable Chromium on this machine. CI runs it.

Gates run

pnpm --filter @mavis/webui typecheck:server / typecheck:client / typecheck:test — pass.
node scripts/run-vitest-suite.mjs webui — 209 files pass, 3297 tests pass.
node scripts/check-webui-dependency.mjs — pass (0 violations, 0 cycles, 0 role violations).
node scripts/build-webui.mjs then node scripts/check-webui-boundary.mjs — pass.
node scripts/source-inventory.mjs --write then node scripts/source-inventory.mjs — pass.
pnpm test:webui-browser — not run: no usable Chromium on this machine. CI runs it.

A third commit: the transcript case was racing three writes it reads

The first CI run of this pull request turned agent-transcript.test.ts red on macOS. That file
is not touched by this pull request, and the failure is not this pull request's: with this
branch's changes stashed, the same case fails the same way on the base commit.

What the case was doing wrong is worth fixing here, because the fix is in this repository's
package and the case is this repository's own gate:

the turn before the cancellation step is over when its answer is on screen, but the runtime
retires the turn a moment later, and the case sent the next message into that gap — a session
whose turn has not been retired queues the message instead of starting a turn, so it never
reaches the model;
the plan-entry request was read out of the frame capture once, right after the answer that
answers it had rendered;
the runtime's event watch was torn down the moment the turn went inactive, which is exactly
when the runtime reports the failure, so an event still in flight was lost and the comparison
that follows had nothing on the runtime side to compare the client's event against.

All three now wait for the write they read, and the assertions themselves are unchanged. The
turn id agreement is still checked, against every error each side reported, because the turn
retries the failing model call and the runtime therefore reports the failure more than once.

CI on this pull request

The checks on this pull request are the ones on its current head — see the checks tab. The
transcript case runs on all three platforms; the change above removes the races it was reading
through rather than loosening any of its expectations, and no timeout a case owns was raised.

A turn's diff can name a file outside the workspace. The runtime's capture layer records an out-of-workspace write as an absolute path, so the entry reaches the client with that file's content attached — and the client rendered it: the transcript's diff card, the workspace turn-review panel and the workspace review page each put the patch text on screen. The fix that stopped the collection at the source was made upstream in `packages/local-runtime/src/turns/file-changes.ts` and reverted in PR #240, because it changed a package this fork may not change. The boundary is therefore drawn on the client side, which is the last point before the content reaches a screen. A diff's file entries are now projected through that boundary before they reach a surface. An entry whose path resolves outside the workspace keeps its name, its status and its line counts, and loses its body: the diff text, the patch hunks, the file's content. The surface says what is withheld instead of showing it. The entry itself stays in the list — "which file was touched" is the part a person can still act on. The comparison is lexical, on posix-style strings, the same normalization the message file-reference reader uses. An absolute path is compared with the root segment by segment, so a sibling directory that shares the root's name is outside; a relative path is refused when it climbs out with `..`. Two limits are stated rather than papered over, and ADR 0024 and the tests say so out loud: * A symlink inside the workspace that points outside it stays contained here. The entry's path is a relative one and the browser has no filesystem to resolve it against; telling the two apart needs `realpath` on the runtime side. * An unknown workspace root is not a boundary. The entries are shown as they are, because a check that hid everything it could not place would hide ordinary content too. The upstream half of this is still open: nothing here stops the runtime from sending an out-of-workspace file's patch to the browser, only from the browser showing it. The client stops the content from being displayed; only the upstream change stops it from being collected.

…it reads The case reads three things that are written by someone else — the runtime's active turn, the client's captured request frames, and the runtime's event stream — and it read all three once, at a moment that is not the moment the write lands. The turn before the cancellation step is over when its answer is on screen, but the runtime retires the turn a moment later, and the case sent the next message into that gap. A session whose turn has not been retired does not start a turn: the message is queued instead, so it never reaches the model and the case waits for an event that will not come. The case now waits for the turn to be retired, the way it already does before its other sends. The plan-entry request was read out of the frame capture once, right after the answer that answers it had rendered. The capture is now polled, the way the persisted history next to it is already polled. The runtime's event watch was torn down the moment the turn went inactive — which is exactly when the runtime reports the failure — so an event still in flight when the watch was aborted was lost, and the comparison that follows had nothing on the runtime side to compare the client's event against. The watch now stays open until both sides carry the error. The turn retries the failing model call, so the runtime reports the failure more than once and the first report is not necessarily the one the client saw; the pairing now looks at every error each side reported, and the two views still have to name the same turn. How it was verified: the case was run repeatedly on the same machine. Before the change it failed on the cancellation gate in about two runs out of five, and the model server never received the message at all — the runtime never started a turn for it. The same case fails the same way on the trunk without this branch, so the flake is not this branch's; the change removes the races the case was reading through and leaves the assertions as they were.

modacker force-pushed the fix/diff-workspace-boundary branch from 0777100 to bf70d86 Compare October 23, 2026 11:43

modacker force-pushed the fix/diff-workspace-boundary branch from bf70d86 to a2f4294 Compare October 23, 2026 13:34

modacker merged commit a8fb9d8 into webui Oct 23, 2026
8 checks passed

modacker deleted the fix/diff-workspace-boundary branch October 23, 2026 14:33

Navigation Menu

Repository navigation

fix(webui): withhold the body of a diff entry outside the workspace - #244

fix(webui): withhold the body of a diff entry outside the workspace#244
modacker merged 2 commits into
webuifrom
fix/diff-workspace-boundary

modacker commented Oct 23, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

modacker commented Oct 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

fix(webui): withhold the body of a diff entry outside the workspace

The vulnerability

Why this is fixed in the WebUI layer

The implementation

What the tests cover

Capability limits — stated, not papered over

Scope

Gates run

Gates run

A third commit: the transcript case was racing three writes it reads

CI on this pull request

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

modacker commented Oct 23, 2026 •

edited

Loading