Skip to content

agent: Re-enable sandbox - #61711

Merged
cameron1024 merged 20 commits into
mainfrom
sandbox-persist-canonical
Jul 29, 2026
Merged

agent: Re-enable sandbox#61711
cameron1024 merged 20 commits into
mainfrom
sandbox-persist-canonical

Conversation

@cameron1024

Copy link
Copy Markdown
Contributor

Fixes the bug that made us remove the sandbox.

The bug in question was very dumb:

  • there is sophisticated machinery for detecting whether a user-granted writable path is swapped out for a symlink in the timing gap between approval and sandbox creation
  • there was no equivalent machinery to do the same for the (much larger) gap between a user persisting an approval (either for the current thread or permanently via settings)
  • The fix is essentially to store canonical (i.e. absolute and symlink-free at all depths) paths as the source of truth, but retain the raw path for display purposes
  • On WSL, there is extra care needed becasue of the bidirectional mounting (i.e. /mnt/c/... and \\wsl.localhost\Ubuntu\...). In particular, /mnt/c/... paths, since their inodes do not necessarily pin NTFS file references, weaken the sandbox guarantees, and so we need some extra UI to call this out and docs etc...

This also does not remove the feature flag, but just toggles it to "enabled_for_all"


Release Notes:

  • N/A or Added/Fixed/Improved ...

cameron1024 and others added 8 commits July 22, 2026 20:13
Co-authored-by: Richard Feldman <oss@rtfeldman.com>
Co-authored-by: Jakub Konka <kubkon@jakubkonka.com>
Co-authored-by: Richard Feldman <oss@rtfeldman.com>
Co-authored-by: Jakub Konka <kubkon@jakubkonka.com>
@cla-bot cla-bot Bot added the cla-signed The user has signed the Contributor License Agreement label Jul 27, 2026
@zed-community-bot zed-community-bot Bot added the staff Pull requests authored by a current member of Zed staff label Jul 27, 2026
@cameron1024
cameron1024 force-pushed the sandbox-persist-canonical branch from 8322c76 to 6273a91 Compare July 28, 2026 10:33
@cameron1024
cameron1024 force-pushed the sandbox-persist-canonical branch from d07cc2d to 8894aa6 Compare July 28, 2026 11:53
@cameron1024
cameron1024 enabled auto-merge July 29, 2026 12:54
@cameron1024
cameron1024 added this pull request to the merge queue Jul 29, 2026
Merged via the queue into main with commit 12a19dc Jul 29, 2026
36 checks passed
@cameron1024
cameron1024 deleted the sandbox-persist-canonical branch July 29, 2026 13:06
AtlantisPleb pushed a commit to OpenAgentsInc/omega that referenced this pull request Jul 30, 2026
Fixes the bug that made us remove the sandbox.

The bug in question was very dumb:
- there is sophisticated machinery for detecting whether a user-granted
writable path is swapped out for a symlink in the timing gap between
approval and sandbox creation
- there was no equivalent machinery to do the same for the (much larger)
gap between a user *persisting an approval* (either for the current
thread or permanently via settings)
- The fix is essentially to store canonical (i.e. absolute and
symlink-free at all depths) paths as the source of truth, but retain the
raw path for display purposes
- On WSL, there is extra care needed becasue of the bidirectional
mounting (i.e. `/mnt/c/...` and `\\wsl.localhost\Ubuntu\...`). In
particular, `/mnt/c/...` paths, since their inodes do not necessarily
pin NTFS file references, weaken the sandbox guarantees, and so we need
some extra UI to call this out and docs etc...

This also does not remove the feature flag, but just toggles it to
"enabled_for_all"

---

Release Notes:

- N/A or Added/Fixed/Improved ...

---------

Co-authored-by: Richard Feldman <oss@rtfeldman.com>
Co-authored-by: Jakub Konka <kubkon@jakubkonka.com>
Co-authored-by: Danilo Leal <daniloleal09@gmail.com>
Co-authored-by: zed-zippy[bot] <234243425+zed-zippy[bot]@users.noreply.github.com>
(cherry picked from commit 12a19dc)
AtlantisPleb added a commit to OpenAgentsInc/omega that referenced this pull request Jul 30, 2026
jolutz pushed a commit to jolutz/zed that referenced this pull request Aug 8, 2026
Fixes the bug that made us remove the sandbox.

The bug in question was very dumb:
- there is sophisticated machinery for detecting whether a user-granted
writable path is swapped out for a symlink in the timing gap between
approval and sandbox creation
- there was no equivalent machinery to do the same for the (much larger)
gap between a user *persisting an approval* (either for the current
thread or permanently via settings)
- The fix is essentially to store canonical (i.e. absolute and
symlink-free at all depths) paths as the source of truth, but retain the
raw path for display purposes
- On WSL, there is extra care needed becasue of the bidirectional
mounting (i.e. `/mnt/c/...` and `\\wsl.localhost\Ubuntu\...`). In
particular, `/mnt/c/...` paths, since their inodes do not necessarily
pin NTFS file references, weaken the sandbox guarantees, and so we need
some extra UI to call this out and docs etc...

This also does not remove the feature flag, but just toggles it to
"enabled_for_all"

---

Release Notes:

- N/A or Added/Fixed/Improved ...

---------

Co-authored-by: Richard Feldman <oss@rtfeldman.com>
Co-authored-by: Jakub Konka <kubkon@jakubkonka.com>
Co-authored-by: Danilo Leal <daniloleal09@gmail.com>
Co-authored-by: zed-zippy[bot] <234243425+zed-zippy[bot]@users.noreply.github.com>
playdohface pushed a commit to playdohface/zed that referenced this pull request Aug 29, 2026
Fixes the bug that made us remove the sandbox.

The bug in question was very dumb:
- there is sophisticated machinery for detecting whether a user-granted
writable path is swapped out for a symlink in the timing gap between
approval and sandbox creation
- there was no equivalent machinery to do the same for the (much larger)
gap between a user *persisting an approval* (either for the current
thread or permanently via settings)
- The fix is essentially to store canonical (i.e. absolute and
symlink-free at all depths) paths as the source of truth, but retain the
raw path for display purposes
- On WSL, there is extra care needed becasue of the bidirectional
mounting (i.e. `/mnt/c/...` and `\\wsl.localhost\Ubuntu\...`). In
particular, `/mnt/c/...` paths, since their inodes do not necessarily
pin NTFS file references, weaken the sandbox guarantees, and so we need
some extra UI to call this out and docs etc...

This also does not remove the feature flag, but just toggles it to
"enabled_for_all"

---

Release Notes:

- N/A or Added/Fixed/Improved ...

---------

Co-authored-by: Richard Feldman <oss@rtfeldman.com>
Co-authored-by: Jakub Konka <kubkon@jakubkonka.com>
Co-authored-by: Danilo Leal <daniloleal09@gmail.com>
Co-authored-by: zed-zippy[bot] <234243425+zed-zippy[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla-signed The user has signed the Contributor License Agreement staff Pull requests authored by a current member of Zed staff

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants