You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@magpie.apache.org by po...@apache.org on 2026/10/24 21:30:04 UTC
(magpie) branch main updated: fix(setup): drop the stale ~/.local/share/uv sandbox grant at session start (#1788)
This is an automated email from the ASF dual-hosted git repository.
potiuk pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/magpie.git
The following commit(s) were added to refs/heads/main by this push:
new d00a1e62e fix(setup): drop the stale ~/.local/share/uv sandbox grant at session start (#1788)
d00a1e62e is described below
commit d00a1e62e97a4c695cd8059db81439ddb17173f4
Author: Jarek Potiuk <ja...@potiuk.com>
AuthorDate: Sat Oct 24 23:29:57 2026 +0200
fix(setup): drop the stale ~/.local/share/uv sandbox grant at session start (#1788)
* fix(setup): drop the stale ~/.local/share/uv sandbox grant at session start
sandbox-add-project-root.sh removes the old ~/.local/share/uv write grant
(#1785) only when it runs, so a project nobody checks out or upgrades keeps
it, and sandboxed code there can still change the interpreters and tools uv
runs outside the sandbox. magpie-setup's session-start hook now removes
the exact entries the helper once added from the checkout's gitignored
.claude/settings.local.json (and the main checkout's, in a linked
worktree), and tells the user in a systemMessage. A broader grant, or one in
user-scope, committed or tracked settings, is reported for the user to
remove.
Generated-by: Claude (Opus 5.5)
* fix(setup): session-start cleanup runs no git and follows no symlink
Review follow-up; the hook runs outside the sandbox over a sandbox-writable
tree. It wrote through a fixed settings.local.json.tmp name a sandboxed
command could plant as a symlink; it now writes a fresh mkstemp file beside
the target and refuses when .claude or the file is a symlink. It ran
`git check-ignore`, which the repository's own sandbox-writable config can
turn into command execution (core.fsmonitor); no git runs now, and a tracked
settings.local.json is cleaned too, since dropping the grant only narrows the
sandbox. It followed a linked worktree's .git pointer to the main checkout,
letting sandboxed code choose the directory it rewrites; it now cleans only
the session's own checkout.
Generated-by: Claude (Opus 5.5)
* fix(setup): rewrite settings.local.json through one directory descriptor
Review follow-up. Checking for symlinks and then writing by path left a
window in which a sandboxed process still running from an earlier session
could swap .claude for a symlink. The cleanup now opens .claude once with
O_NOFOLLOW and does the read (O_NOFOLLOW), the fresh temporary file
(O_EXCL|O_NOFOLLOW, random name) and the rename relative to that
descriptor, so nothing that happens to the path meanwhile redirects it.
Generated-by: Claude (Opus 5.5)
* fix(setup): address CodeQL findings in the session-start cleanup
Builds the session-start message and context as lists instead of
conditional generator parts (CodeQL: potentially uninitialized local), and
replaces the silent except around the temporary-file cleanup with a
commented contextlib.suppress (CodeQL: empty except).
Generated-by: Claude (Opus 5.5)
---
docs/setup/sandbox-troubleshooting.md | 3 +-
docs/setup/secure-agent-setup.md | 2 +
plugins/magpie-setup/hooks/check-upgrade.sh | 9 +-
.../skills/setup/setup_preflight/isolated.py | 190 ++++++++++++++++++++-
tools/setup-preflight/tests/test_isolated.py | 147 ++++++++++++++++
5 files changed, 344 insertions(+), 7 deletions(-)
diff --git a/docs/setup/sandbox-troubleshooting.md b/docs/setup/sandbox-troubleshooting.md
index fd9807145..5dd1466a7 100644
--- a/docs/setup/sandbox-troubleshooting.md
+++ b/docs/setup/sandbox-troubleshooting.md
@@ -1117,7 +1117,8 @@ hooks, MCP servers, the excluded `uvx` / `uv run` commands and your own
terminal run outside the sandbox, so the sandbox may read it but not write
it ([Host-only uv state](secure-agent-setup.md#host-only-uv-state)). Older
versions of `sandbox-add-project-root.sh` granted the write; the current one
-removes it.
+removes it, and so does magpie-setup's session-start hook the next time a
+session opens in the project.
### Fix
diff --git a/docs/setup/secure-agent-setup.md b/docs/setup/secure-agent-setup.md
index 8c6d3da79..bff91fb8a 100644
--- a/docs/setup/secure-agent-setup.md
+++ b/docs/setup/secure-agent-setup.md
@@ -1041,6 +1041,8 @@ uv tool upgrade --all --reinstall
```
The old `~/.cache/uv/` stays as the sandbox's cache; the host never reads it again.
+
+Projects set up before the switch may still carry the old write grant. `sandbox-add-project-root.sh` removes it whenever it runs (on checkout, adopt and upgrade), and magpie-setup's session-start hook removes it from the checkout's `.claude/settings.local.json` the next time a session opens there, and says so. A linked worktree's main checkout is cleaned when a session opens in it: the worktree's `.git` file is sandbox-writable, so the hook never follows it. A broader grant, or one in us [...]
`sandbox-lint` fails a settings file that makes `~/.local/share/uv` writable or lacks the two `~/.config/uv` deny rules; `setup-isolated-setup-verify` checks the config file itself.
## Project-root coverage in the sandbox allowlists
diff --git a/plugins/magpie-setup/hooks/check-upgrade.sh b/plugins/magpie-setup/hooks/check-upgrade.sh
index 09b281880..adef0acab 100755
--- a/plugins/magpie-setup/hooks/check-upgrade.sh
+++ b/plugins/magpie-setup/hooks/check-upgrade.sh
@@ -17,9 +17,12 @@
#
# Deliberately DETECT-AND-PROMPT, not auto-run: a plugin hook cannot invoke
# a slash command, and Magpie never mutates an adopter repo without the
-# guided skill's confirmation. The hook is read-only apart from writing its
-# own version marker; it makes no network calls and touches nothing in the
-# adopter repo.
+# guided skill's confirmation. The hook makes no network calls and writes
+# only its own version marker and, through `setup_preflight`, one cleanup: it
+# drops the old `~/.local/share/uv` sandbox write grant from the checkout's
+# `.claude/settings.local.json`, the per-machine file the project-root helper
+# already maintains, and tells the user (apache/magpie#1785). It runs no git
+# command and follows no symlink to do it.
#
# Agents without a session hook (e.g. Gemini CLI) surface the same prompt via
# their extension context file (GEMINI.md) instead.
diff --git a/plugins/magpie-setup/skills/setup/setup_preflight/isolated.py b/plugins/magpie-setup/skills/setup/setup_preflight/isolated.py
index 820810786..57131f6b4 100644
--- a/plugins/magpie-setup/skills/setup/setup_preflight/isolated.py
+++ b/plugins/magpie-setup/skills/setup/setup_preflight/isolated.py
@@ -57,9 +57,12 @@ fingerprint into the stamp:
from __future__ import annotations
import argparse
+import contextlib
import hashlib
import json
+import os
import re
+import secrets
import sys
from datetime import date
from pathlib import Path
@@ -237,6 +240,176 @@ def _adopted_root(start: Path) -> Path | None:
return None
+#: The sandbox write grant earlier versions of `sandbox-add-project-root.sh`
+#: added. `~/.local/share/uv` holds the interpreters and tool venvs that hooks,
+#: MCP servers and the excluded `uvx` / `uv run` commands run outside the
+#: sandbox, so a sandboxed command able to write there plants code that runs
+#: unsandboxed (apache/magpie#1785). The helper removes it whenever it runs;
+#: the session start removes it from a project the helper has not run in since.
+_UV_DATA_DIR = ".local/share/uv"
+
+
+def _list_at(loaded: object, *keys: str) -> list[object]:
+ """`loaded[k1][k2]...` when every step is an object and the end a list, else `[]`."""
+ for key in keys:
+ if not isinstance(loaded, dict):
+ return []
+ loaded = loaded.get(key)
+ return loaded if isinstance(loaded, list) else []
+
+
+def _stale_uv_writes(home: Path) -> frozenset[str]:
+ absolute = str(home / _UV_DATA_DIR)
+ return frozenset({absolute, absolute + "/", "~/" + _UV_DATA_DIR, "~/" + _UV_DATA_DIR + "/"})
+
+
+def _covers_uv_data_dir(entry: str, home: Path, base: Path) -> bool:
+ """Whether a write-grant entry makes `~/.local/share/uv` (or a part of it) writable."""
+ text = entry.strip()
+ if text.startswith("//"):
+ text = text[1:]
+ if text == "~" or text.startswith("~/"):
+ text = str(home) + text[1:]
+ path = Path(text) if text.startswith("/") else base / text
+ target = home / _UV_DATA_DIR
+ for candidate in (path, target):
+ other = target if candidate is path else path
+ if candidate == other or other in candidate.parents:
+ return True
+ return False
+
+
+def _checkout(root: Path) -> Path:
+ """The session's own checkout: the nearest directory holding a `.git`.
+
+ Never the main checkout a linked worktree's `.git` file names: that file is
+ as writable from the sandbox as the worktree, so following it would let
+ sandboxed code pick the directory this unsandboxed hook rewrites. The main
+ checkout is cleaned when a session opens there.
+ """
+ for directory in (root, *root.parents):
+ if (directory / ".git").exists():
+ return directory
+ return root
+
+
+_NOFOLLOW = getattr(os, "O_NOFOLLOW", 0)
+_DIRECTORY = getattr(os, "O_DIRECTORY", 0)
+
+
+def _drop_from_local_settings(local: Path, stale: frozenset[str]) -> str | None:
+ """Remove `stale` from `local`'s allowWrite; `"removed"`, `"refused"`, or None if absent.
+
+ The project tree is sandbox-writable and this runs outside the sandbox, so
+ everything goes through one descriptor for `.claude`, opened without
+ following a symlink: the read, a fresh temporary file (O_EXCL, random name)
+ and the rename all stay in that directory, whatever happens to its path
+ meanwhile. `.claude` or the file being a symlink refuses the rewrite.
+ """
+ try:
+ dfd = os.open(local.parent, os.O_RDONLY | _DIRECTORY | _NOFOLLOW)
+ except OSError:
+ return None if not local.parent.is_symlink() else "refused"
+ try:
+ try:
+ fd = os.open(local.name, os.O_RDONLY | _NOFOLLOW, dir_fd=dfd)
+ except OSError:
+ return "refused" if os.path.islink(local) else None
+ with os.fdopen(fd, encoding="utf-8") as handle:
+ try:
+ loaded = json.load(handle)
+ except (json.JSONDecodeError, UnicodeDecodeError):
+ return None
+ writes = _list_at(loaded, "sandbox", "filesystem", "allowWrite")
+ if not isinstance(loaded, dict) or not any(w in stale for w in writes if isinstance(w, str)):
+ return None
+ loaded["sandbox"]["filesystem"]["allowWrite"] = [w for w in writes if w not in stale]
+ tmp_name = f".settings.local.{secrets.token_hex(8)}.tmp"
+ try:
+ tfd = os.open(tmp_name, os.O_WRONLY | os.O_CREAT | os.O_EXCL | _NOFOLLOW, 0o600, dir_fd=dfd)
+ except OSError:
+ return "refused"
+ try:
+ with os.fdopen(tfd, "w", encoding="utf-8") as handle:
+ handle.write(json.dumps(loaded, indent=2) + "\n")
+ os.replace(tmp_name, local.name, src_dir_fd=dfd, dst_dir_fd=dfd)
+ except OSError:
+ # Best effort: a temporary file left behind is inert, and the
+ # refusal is reported either way.
+ with contextlib.suppress(OSError):
+ os.unlink(tmp_name, dir_fd=dfd)
+ return "refused"
+ return "removed"
+ finally:
+ os.close(dfd)
+
+
+def drop_stale_uv_writes(root: Path, home: Path | None = None) -> list[str]:
+ """Remove the old `~/.local/share/uv` write grant; report what was removed or must be.
+
+ Only from the session checkout's `.claude/settings.local.json`, the
+ per-machine file the project-root helper maintains, and only the exact
+ entries it once added. Removing them only narrows the sandbox, so a tracked
+ copy of the file is safe to rewrite too: the change shows in `git diff`.
+ No git command runs here: the repository's own config is sandbox-writable.
+ A broader grant, or one in user-scope or committed settings, is reported for
+ the user to remove: those files are theirs, not the helper's. Hooks run
+ outside the sandbox, so every file here is readable.
+ """
+ home = home if home is not None else Path.home()
+ stale = _stale_uv_writes(home)
+ messages: list[str] = []
+ directory = _checkout(root)
+ local = directory / ".claude" / "settings.local.json"
+ outcome = _drop_from_local_settings(local, stale)
+ if outcome == "removed":
+ messages.append(f"removed the ~/.local/share/uv sandbox write grant from {local}")
+ elif outcome == "refused" and any(_covers_uv_data_dir(e, home, directory) for e in _grants_in(local)):
+ messages.append(
+ f"{local} grants sandbox writes to ~/.local/share/uv and could not be rewritten safely; remove it by hand"
+ )
+ for settings in (directory / ".claude" / "settings.json", local):
+ messages.extend(_report_covering_grants(settings, directory, home, stale))
+ user = home / ".claude"
+ for name in ("settings.json", "settings.local.json"):
+ messages.extend(_report_covering_grants(user / name, user, home, frozenset()))
+ return messages
+
+
+def _grants_in(settings: Path) -> list[str]:
+ try:
+ loaded = json.loads(settings.read_text(encoding="utf-8"))
+ except (OSError, json.JSONDecodeError, UnicodeDecodeError):
+ return []
+ return [e for e in _list_at(loaded, "sandbox", "filesystem", "allowWrite") if isinstance(e, str)]
+
+
+def _report_covering_grants(settings: Path, base: Path, home: Path, removable: frozenset[str]) -> list[str]:
+ try:
+ loaded = json.loads(settings.read_text(encoding="utf-8"))
+ except (OSError, json.JSONDecodeError):
+ return []
+ if not isinstance(loaded, dict):
+ return []
+ entries = [
+ *_list_at(loaded, "sandbox", "filesystem", "allowWrite"),
+ *_list_at(loaded, "permissions", "additionalDirectories"),
+ ]
+ left = [
+ e for e in entries if isinstance(e, str) and e not in removable and _covers_uv_data_dir(e, home, base)
+ ]
+ return [
+ f"{settings} makes ~/.local/share/uv writable to the sandbox ({e!r}); remove it by hand" for e in left
+ ]
+
+
+#: Shown when the session start removed or found a `~/.local/share/uv` grant.
+STALE_UV_HEADER = (
+ "Apache Magpie: ~/.local/share/uv holds the interpreters and tools uv runs outside the sandbox, "
+ "so the sandbox must not write there (apache/magpie#1785)."
+)
+
+
def session_start(root: Path, notice: str = "", home: Path | None = None) -> str:
"""What the SessionStart hook prints: its own notice, plus the warning while unconfigured.
@@ -246,14 +419,25 @@ def session_start(root: Path, notice: str = "", home: Path | None = None) -> str
settings are readable here and "not configured" means what it says.
Plain text keeps the hook's original output when there is nothing to
warn about.
+
+ Every session start also drops the old `~/.local/share/uv` write grant
+ (`drop_stale_uv_writes`), adopted or not, and tells the user what it did.
"""
+ stale = drop_stale_uv_writes(root.resolve(), home)
+ stale_message = "\n".join([STALE_UV_HEADER, *(f"- {m}" for m in stale)]) if stale else ""
project = _adopted_root(root.resolve())
- if project is None or warning_opted_out(project) or configured(project, home):
+ messages = [stale_message] if stale_message else []
+ contexts = [part for part in (notice, stale_message) if part]
+ if project is not None and not warning_opted_out(project) and not configured(project, home):
+ messages.append(UNCONFIGURED_WARNING)
+ contexts.append(UNCONFIGURED_CONTEXT)
+ if not messages:
return notice
- context = "\n\n".join(part for part in (notice, UNCONFIGURED_CONTEXT) if part)
+ message = "\n\n".join(messages)
+ context = "\n\n".join(contexts)
return json.dumps(
{
- "systemMessage": UNCONFIGURED_WARNING,
+ "systemMessage": message,
"hookSpecificOutput": {"hookEventName": "SessionStart", "additionalContext": context},
}
)
diff --git a/tools/setup-preflight/tests/test_isolated.py b/tools/setup-preflight/tests/test_isolated.py
index c3ff37895..673eb3d1a 100644
--- a/tools/setup-preflight/tests/test_isolated.py
+++ b/tools/setup-preflight/tests/test_isolated.py
@@ -20,6 +20,7 @@
from __future__ import annotations
import json
+import subprocess
from datetime import date
from pathlib import Path
@@ -300,3 +301,149 @@ def test_session_start_passes_the_notice_through_once_configured(project: Path)
(project / ".claude" / "settings.json").write_text('{"sandbox": {"enabled": true}}')
assert isolated.session_start(project, "plugin 1.0 is active") == "plugin 1.0 is active"
assert isolated.session_start(project) == ""
+
+
+# --- the session start drops the old ~/.local/share/uv write grant ---
+
+
+def git_repo(root: Path, ignore_local: bool = True) -> Path:
+ """A real repository, so `git check-ignore` can judge settings.local.json."""
+ subprocess.run(["git", "init", "-q", str(root)], check=True)
+ if ignore_local:
+ (root / ".gitignore").write_text("/.claude/settings.local.json\n")
+ (root / ".claude").mkdir(exist_ok=True)
+ return root
+
+
+def write_local(root: Path, allow_write: list[str]) -> Path:
+ local = root / ".claude" / "settings.local.json"
+ local.write_text(
+ json.dumps({"sandbox": {"enabled": True, "filesystem": {"allowWrite": allow_write}}, "keep": 1})
+ )
+ return local
+
+
+@pytest.mark.parametrize(
+ "spelling",
+ ["{home}/.local/share/uv", "{home}/.local/share/uv/", "~/.local/share/uv", "~/.local/share/uv/"],
+)
+def test_session_start_removes_the_stale_uv_write(tmp_path: Path, isolated_home: Path, spelling: str) -> None:
+ repo = git_repo(tmp_path / "repo")
+ local = write_local(repo, [str(repo), spelling.format(home=isolated_home), f"{isolated_home}/.cache"])
+ out = json.loads(isolated.session_start(repo, home=isolated_home))
+ data = json.loads(local.read_text())
+ assert data["sandbox"]["filesystem"]["allowWrite"] == [str(repo), f"{isolated_home}/.cache"]
+ assert data["keep"] == 1 and data["sandbox"]["enabled"] is True # nothing else touched
+ assert "removed the ~/.local/share/uv sandbox write grant" in out["systemMessage"]
+ assert isolated.STALE_UV_HEADER in out["hookSpecificOutput"]["additionalContext"]
+
+
+def test_the_next_session_start_is_quiet_again(tmp_path: Path, isolated_home: Path) -> None:
+ repo = git_repo(tmp_path / "repo")
+ write_local(repo, [f"{isolated_home}/.local/share/uv"])
+ isolated.session_start(repo, home=isolated_home)
+ assert isolated.session_start(repo, "plugin 1.0 is active", home=isolated_home) == "plugin 1.0 is active"
+
+
+def test_a_tracked_settings_local_is_cleaned_too(tmp_path: Path, isolated_home: Path) -> None:
+ """Dropping the grant only narrows the sandbox; the change shows in git diff."""
+ repo = git_repo(tmp_path / "repo", ignore_local=False)
+ local = write_local(repo, [f"{isolated_home}/.local/share/uv"])
+ out = json.loads(isolated.session_start(repo, home=isolated_home))
+ assert json.loads(local.read_text())["sandbox"]["filesystem"]["allowWrite"] == []
+ assert "removed the ~/.local/share/uv sandbox write grant" in out["systemMessage"]
+
+
+@pytest.mark.parametrize("entry", ["~/.local", "~/.local/share/uv/python", "~"])
+def test_a_broader_grant_is_reported_not_rewritten(tmp_path: Path, isolated_home: Path, entry: str) -> None:
+ repo = git_repo(tmp_path / "repo")
+ local = write_local(repo, [entry])
+ before = local.read_text()
+ out = json.loads(isolated.session_start(repo, home=isolated_home))
+ assert local.read_text() == before
+ assert f"({entry!r}); remove it by hand" in out["systemMessage"]
+
+
+def test_a_user_scope_grant_is_reported_not_rewritten(tmp_path: Path, isolated_home: Path) -> None:
+ repo = git_repo(tmp_path / "repo")
+ user = isolated_home / ".claude" / "settings.json"
+ user.parent.mkdir()
+ user.write_text(json.dumps({"sandbox": {"filesystem": {"allowWrite": ["~/.local/share/uv/"]}}}))
+ before = user.read_text()
+ out = json.loads(isolated.session_start(repo, home=isolated_home))
+ assert user.read_text() == before
+ assert str(user) in out["systemMessage"]
+
+
+def test_a_linked_worktree_never_reaches_into_the_main_checkout(tmp_path: Path, isolated_home: Path) -> None:
+ """The worktree's .git file is sandbox-writable; following it would let sandboxed code pick the target."""
+ main = git_repo(tmp_path / "main")
+ subprocess.run(["git", "-C", str(main), "add", ".gitignore"], check=True)
+ subprocess.run(
+ [
+ "git",
+ "-C",
+ str(main),
+ "-c",
+ "user.name=t",
+ "-c",
+ "user.email=t@t",
+ "-c",
+ "commit.gpgsign=false",
+ "commit",
+ "-q",
+ "-m",
+ "init",
+ ],
+ check=True,
+ )
+ worktree = tmp_path / "wt"
+ subprocess.run(["git", "-C", str(main), "worktree", "add", "-q", str(worktree)], check=True)
+ main_local = write_local(main, [f"{isolated_home}/.local/share/uv"])
+ before = main_local.read_text()
+ isolated.session_start(worktree, home=isolated_home)
+ assert main_local.read_text() == before
+
+
+def test_a_symlinked_settings_local_is_never_written_through(tmp_path: Path, isolated_home: Path) -> None:
+ repo = git_repo(tmp_path / "repo")
+ victim = tmp_path / "victim.json"
+ victim.write_text(
+ json.dumps({"sandbox": {"filesystem": {"allowWrite": [f"{isolated_home}/.local/share/uv"]}}})
+ )
+ before = victim.read_text()
+ (repo / ".claude" / "settings.local.json").symlink_to(victim)
+ isolated.session_start(repo, home=isolated_home)
+ assert victim.read_text() == before
+ assert (repo / ".claude" / "settings.local.json").is_symlink()
+
+
+def test_a_symlinked_claude_dir_is_never_written_through(tmp_path: Path, isolated_home: Path) -> None:
+ repo = git_repo(tmp_path / "repo")
+ (repo / ".claude").rmdir()
+ elsewhere = tmp_path / "elsewhere"
+ elsewhere.mkdir()
+ (repo / ".claude").symlink_to(elsewhere)
+ local = write_local(repo, [f"{isolated_home}/.local/share/uv"])
+ before = local.read_text()
+ out = json.loads(isolated.session_start(repo, home=isolated_home))
+ assert local.read_text() == before
+ assert "could not be rewritten safely" in out["systemMessage"]
+
+
+def test_a_planted_temp_file_symlink_is_never_followed(tmp_path: Path, isolated_home: Path) -> None:
+ """The old fixed `.tmp` name could be pre-planted as a link to, say, ~/.bashrc."""
+ repo = git_repo(tmp_path / "repo")
+ local = write_local(repo, [f"{isolated_home}/.local/share/uv"])
+ victim = tmp_path / "bashrc"
+ victim.write_text("untouched\n")
+ (repo / ".claude" / "settings.local.json.tmp").symlink_to(victim)
+ isolated.session_start(repo, home=isolated_home)
+ assert victim.read_text() == "untouched\n"
+ assert json.loads(local.read_text())["sandbox"]["filesystem"]["allowWrite"] == []
+
+
+def test_nothing_to_remove_keeps_the_plain_notice(tmp_path: Path, isolated_home: Path) -> None:
+ repo = git_repo(tmp_path / "repo")
+ write_local(repo, [str(repo), f"{isolated_home}/.cache"])
+ assert isolated.session_start(repo, "plugin 1.0 is active", home=isolated_home) == "plugin 1.0 is active"