[shadaj]

Previously both tools did sandbox_dir.join(&args.path) and wrote there with no
validation. Path::join replaces the base for absolute paths, .. components
were not checked, and (most importantly) symlinks were followed — so a symlink
committed by an earlier execute_command (e.g. ln -s ~/.ssh/authorized_keys evil)
would ride along into every subsequent workspace and let create_file evil or
edit_file evil read/write arbitrary files with the server's permissions.
create_file's exists() guard also returned false for dangling symlinks, so
fs::write would happily create the link's target.

Reads (read_file) are intentionally left unrestricted to match
execute_command, which may read anywhere. Writes outside the root are
rejected in every mode, including Direct.

resolve_write_path(root, path, display) -> ResolvedWritePath

New helper that resolves the path the way the kernel would and verifies the
result is under the canonicalized sandbox root:

• Walks components left to right. Whenever the prefix built so far exists on
  disk (via lstat, so dangling symlinks count as existing) it is
  canonicalized — i.e. realpath, which follows every symlink in every
  component recursively, so no hand-rolled symlink loop is needed. A dangling
  symlink makes canonicalize fail and is rejected.
• .. pops the last component. This is safe because an existing prefix is
  already canonical (symlink-free) and a not-yet-existing tail will be created
  by us as real directories. Doing it lexically on the original path would be
  wrong (link/../x with link -> /outside/dir resolves to /outside/x), and
  walking up with Path::parent from the "deepest existing ancestor" would be
  wrong too (a/../../outside/x with a missing passes that check but
  create_dir_all then mkdirs through .. for real — a unit test caught this
  in an earlier iteration).
• Absolute paths need no special-casing: the resolved result is simply
  compared against the root, so an absolute path into the sandbox is allowed
  and one outside it is rejected.
• Returns both the canonical absolute path (all I/O uses this, so what was
  checked is what gets written) and the root-relative path (for display).

Direct-mode approval prompt shows the resolved target

The approval prompt previously showed the raw args.path, so a symlink or
.. could disguise what the user was approving. Resolution now happens
before the prompt, inside the with_sandbox closure, and the prompt shows
the resolved repo-relative path (e.g. real/new.txt instead of
inner/new.txt when inner -> real). Paths resolving outside the repo are
rejected with no prompt at all. The write-orig grant shortcut and grant
choices are unchanged.

create_file

• Calls resolve_write_path before create_dir_all, which would otherwise
  mkdir through a symlinked or .. component.
• Replaces the exists() check + fs::write with
  OpenOptions::create_new(true) (O_CREAT|O_EXCL). POSIX guarantees O_EXCL
  does not follow a symlink in the final component, even a dangling one, and
  the check-and-create is atomic. This matters because the local backend caches
  sandbox directories per group, so a background execute_command can race a
  file tool in the same directory.

edit_file

• Resolves via resolve_write_path first and reads/writes the resolved path.

Tests

• 17 unit tests in sandbox-core covering: nested nonexistent dirs, .. that
  stays inside, .. escapes (including after a missing component and
  saturating past /), absolute paths inside and outside, symlinked dir/file
  outside, symlink chains (a -> b -> outside), relative symlink targets,
  .. after a symlink, dangling symlinks (final and intermediate), legit
  symlinks that stay inside, a symlinked root (macOS /var -> /private/var),
  prefix-sibling false positives (root-other vs root), and that the
  relative path is normalized / shows the real target behind a symlink.
• New e2e test crates/sandbox-local/tests/write_escape.rs that commits
  symlinks into a real jj repo and drives create_file/edit_file through
  the server in both jj and Direct modes, asserting rejections, that nothing
  lands outside, that legitimate writes still work, and — in Direct mode —
  that the UserChoice prompt text contains the resolved target (not the
  symlink or .. form), that "No" leaves the file untouched, and that
  out-of-repo writes produce a ToolResult rejection with no prompt. Verified
  the escape tests fail when the guard is disabled.

./check.bash passes (fmt, clippy -D warnings, all tests, doctests). The
optional Playwright step fails only because ~/.cargo/registry is read-only
in the sandbox, unrelated to this change.

[cloudflare-workers-and-pages]

Deploying infinity with    Cloudflare Pages

Latest commit:	8e217f7
Status:	✅  Deploy successful!
Preview URL:	https://edef6138.infinity-dc7.pages.dev
Branch Preview URL:	https://sandbox-064642ae-c9cf-4503-b.infinity-dc7.pages.dev

View logs

[akainth015]

Change overall LGTM. Hurts a bit that it's big but a lot of it is tests.

[akainth015]

I assume we'd have a permissions prompt if the tool call wants to write outside the sandbox?