From 2207bf406ec93926c32a2d8084103526c7900b96 Mon Sep 17 00:00:00 2001 From: Maximilian Roos Date: Sat, 19 Sep 2026 12:28:35 -0700 Subject: [PATCH 1/3] fix(sandbox): make /tmp writable inside the sandbox MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SRT read-only-binds the whole root and rw-binds only what tend lists in allowWrite, which never included /tmp. Every tool that hard-codes a path under it therefore needed a workaround of its own: a TMPDIR override, a zsh TMPPREFIX override, skill text warning sessions off it, and most recently NuGet's build mutex, which no environment variable moves. /tmp is writable now. Tend's two runner-owned containers — the per-run SRT runtime and the disposable event checkout — move to /var/tmp, which is 1777 like /tmp but outside the write policy, so they stay as unwritable as they were; the Claude auto-memory working copy moves with them, since the dispose step runs before the save that reads it. After the reap that step also removes the top-level /tmp entries the sandbox uid owns, so nothing sandbox-written is left where a later runner step or a setup: action's POST step would read it. The TMPPREFIX override goes with the constraint that produced it, along with its reserved sandbox_env key and the repo-wide ban on bare /tmp paths in shipped instructions. Co-Authored-By: Claude Opus 5 (1M context) --- .claude/skills/running-tend/SKILL.md | 2 +- .github/workflows/ci.yaml | 5 +- claude/action.yaml | 6 +- docs/security-model.md | 18 +++-- docs/tend.example.yaml | 2 +- generator/src/tend/config.py | 1 - generator/tests/test_repo_pins.py | 23 ------- .../install-tend/references/tend.example.yaml | 2 +- .../tend-ci-runner/skills/run-tend/SKILL.md | 2 +- proxy/setup_sandbox.py | 5 -- proxy/test-setup-sandbox.sh | 32 ++++++++- proxy/test_setup_sandbox.py | 14 ---- shared/steps/dispose_sandbox_resources.py | 44 +++++++++--- shared/steps/install-sandbox-runtime.sh | 4 +- shared/steps/prepare_agent_workspace.py | 6 +- shared/steps/sandbox_runtime.mjs | 17 ++++- .../steps/test_dispose_sandbox_resources.py | 67 ++++++++++++++----- shared/steps/test_prepare_agent_workspace.py | 2 +- 18 files changed, 164 insertions(+), 88 deletions(-) diff --git a/.claude/skills/running-tend/SKILL.md b/.claude/skills/running-tend/SKILL.md index b194187ea..000cc9690 100644 --- a/.claude/skills/running-tend/SKILL.md +++ b/.claude/skills/running-tend/SKILL.md @@ -73,7 +73,7 @@ Step 1 anchor instead, so its spend covers the same band that step censuses. ## Session Log Paths -Artifact paths: `-tmp-tend-agent-workspace-*-checkout/.jsonl` +Artifact paths: `-var-tmp-tend-agent-workspace-*-checkout/.jsonl` `review-reviewers` runs produce one session log per matrix repo in `.github/workflows/review-reviewers.yaml`. diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 7ef1cec39..1ebb19ae1 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -96,7 +96,8 @@ jobs: # remains on 8899 — so a later step must expect that state. This job exercises # the real boundary: shared paths cross, runner-home paths do not, same-name # system fallbacks are blocked, and Tend preserves a shared uv. It then drives - # sandbox_setup and both harness adapters inside their real SRT lifecycle. + # sandbox_setup and both harness adapters inside their real SRT lifecycle, and + # ends on the dispose step that clears what the sandbox wrote to /tmp. test-sandbox: runs-on: ubuntu-24.04 steps: @@ -120,6 +121,8 @@ jobs: run: bash proxy/test-setup-sandbox.sh verify-srt - name: Refuse unsafe sandbox configuration run: bash proxy/test-setup-sandbox.sh verify-refusals + - name: Dispose the sandbox's scratch + run: bash proxy/test-setup-sandbox.sh verify-dispose - name: Clean up sandbox fixtures if: always() run: bash proxy/test-setup-sandbox.sh cleanup diff --git a/claude/action.yaml b/claude/action.yaml index 394f4ea17..32487cdbe 100644 --- a/claude/action.yaml +++ b/claude/action.yaml @@ -363,14 +363,16 @@ runs: # This trusted step restores before Claude starts, signs the baseline with a # per-run key that never crosses into the sandbox env, then hands only the # memory directory to the sandbox uid. Failure leaves Claude running without - # shared memory. + # shared memory. The directory goes in /var/tmp because it ends up + # sandbox-owned, and the dispose step — which runs before the save below — + # clears that uid's /tmp entries. - name: Restore experimental memory Gist id: auto_memory if: inputs.memory_gist == 'true' continue-on-error: true shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0} run: | - memory_dir=$(/usr/bin/mktemp -d /tmp/tend-auto-memory.XXXXXXXXXX) + memory_dir=$(/usr/bin/mktemp -d /var/tmp/tend-auto-memory.XXXXXXXXXX) key_file="" cleanup_restore() { /usr/bin/sudo /usr/bin/rm -rf -- "$memory_dir" diff --git a/docs/security-model.md b/docs/security-model.md index 50e8ccece..156538ac1 100644 --- a/docs/security-model.md +++ b/docs/security-model.md @@ -307,7 +307,7 @@ The harness has four states and only four transitions: Each transition is a bottleneck with one job: -- **Content ingress** creates a full remote clone inside a dedicated `/tmp` +- **Content ingress** creates a full remote clone inside a dedicated `/var/tmp` container with no hardlinks or object-store alternates and with runner/system Git config and attributes disabled. It selects the exact base, PR merge/head, or open-PR head ref, pins startup configuration to the exact chosen base @@ -316,7 +316,13 @@ Each transition is a bottleneck with one job: `RUNNER_TEMP` remains unreadable except for the exact read-only event payload named by `GITHUB_EVENT_PATH`. SRT, the Codex binaries, and the immutable agent environment live in one dedicated runner-owned, sandbox-readable runtime - directory; the sandbox cannot write it. + directory; the sandbox cannot write it. Both containers sit under `/var/tmp` + rather than `/tmp`, which is ordinary writable scratch inside the sandbox + because tooling hard-codes paths there with no way to redirect them. What the + sandbox may write follows the policy rather than the directory: `/tmp`, the + agent's own checkout and home, and, where the memory experiment is on, the + one `/var/tmp` directory that experiment owns. `/tmp` is sticky, so the + sandbox can add entries beside the runner's but cannot unlink or rename one. - **Launch and lifetime** invokes the consumer's `sandbox_setup:` and the whole Claude or Codex turn as one command under the pinned Anthropic Sandbox Runtime. Tend supplies absolute `node`, `bwrap`, `socat`, `rg`, and seccomp @@ -398,8 +404,12 @@ non-sudo sandbox user in the same SRT process lifetime as the agent. After SRT exits, the trusted supervisor kills and verifies the complete sandbox UID process tree, then copies only size-bounded fixed outputs. The next fixed -action step deletes the dedicated `/tmp/tend-agent-workspace-*` container; no -post-sandbox step executes a file from that checkout. +action step deletes the dedicated `/var/tmp/tend-agent-workspace-*` container +and every top-level `/tmp` entry the sandbox UID owns, so no post-sandbox step +executes a file from that checkout and the agent's scratch is gone before a +later step or a `setup:` action's POST step could read it. A run whose reap +failed keeps both: the job has already failed, and the live writer is the +reason not to delete underneath it. **Credential isolation.** Both harness actions run the agent as a separate non-sudo `tend-sandbox` user, sharing the GitHub proxy machinery under the diff --git a/docs/tend.example.yaml b/docs/tend.example.yaml index 2feaedf3a..216d5126d 100644 --- a/docs/tend.example.yaml +++ b/docs/tend.example.yaml @@ -228,7 +228,7 @@ bot_name: my-project-bot # `OPENAI_API_KEY`, `CODEX_API_KEY`, `CODEX_AUTH_JSON`, `CODEX_HOME` # - the disposable checkout identity — `GITHUB_WORKSPACE` # - prompt suppression — `CLAUDE_CODE_REMOTE` -# - the sandbox's own locations — `HOME`, `PATH`, `TMPDIR`, `TMPPREFIX`, +# - the sandbox's own locations — `HOME`, `PATH`, `TMPDIR`, # `XDG_CONFIG_HOME`, `XDG_CACHE_HOME`, `XDG_DATA_HOME`, `XDG_STATE_HOME` # # sandbox_env: diff --git a/generator/src/tend/config.py b/generator/src/tend/config.py index 0a744bdb2..418bb4fe3 100644 --- a/generator/src/tend/config.py +++ b/generator/src/tend/config.py @@ -127,7 +127,6 @@ "CODEX_AUTH_JSON", "CODEX_HOME", "TMPDIR", - "TMPPREFIX", } diff --git a/generator/tests/test_repo_pins.py b/generator/tests/test_repo_pins.py index 3ac66b6db..2b9c7a599 100644 --- a/generator/tests/test_repo_pins.py +++ b/generator/tests/test_repo_pins.py @@ -736,29 +736,6 @@ def test_codex_refresher_keeps_the_secret_writer_pat_out_of_the_model_step() -> assert publish["env"]["CODEX_OUTCOME"] == "${{ steps.codex.outcome }}" -def test_bundled_runner_instructions_have_no_unscoped_tmp_paths() -> None: - """`/tmp` is not writable in the sandbox; `$TMPDIR` is. - - A bare `/tmp` anywhere the runner reads — instructions, script, helper — sends - the session to a path that fails on write, so the ban is repo-wide rather - than a rule any one file states. A line that says `/tmp` is read-only is - that rule, not an instance of the failure, so it is exempt. - """ - runner = REPO_ROOT / "plugins" / "tend-ci-runner" - unscoped_tmp = re.compile(r"(? None: """`cd -` cannot bring a session back to where a recipe started. diff --git a/plugins/install-tend/skills/install-tend/references/tend.example.yaml b/plugins/install-tend/skills/install-tend/references/tend.example.yaml index 2feaedf3a..216d5126d 100644 --- a/plugins/install-tend/skills/install-tend/references/tend.example.yaml +++ b/plugins/install-tend/skills/install-tend/references/tend.example.yaml @@ -228,7 +228,7 @@ bot_name: my-project-bot # `OPENAI_API_KEY`, `CODEX_API_KEY`, `CODEX_AUTH_JSON`, `CODEX_HOME` # - the disposable checkout identity — `GITHUB_WORKSPACE` # - prompt suppression — `CLAUDE_CODE_REMOTE` -# - the sandbox's own locations — `HOME`, `PATH`, `TMPDIR`, `TMPPREFIX`, +# - the sandbox's own locations — `HOME`, `PATH`, `TMPDIR`, # `XDG_CONFIG_HOME`, `XDG_CACHE_HOME`, `XDG_DATA_HOME`, `XDG_STATE_HOME` # # sandbox_env: diff --git a/plugins/tend-ci-runner/skills/run-tend/SKILL.md b/plugins/tend-ci-runner/skills/run-tend/SKILL.md index 321f0eb52..8bf8a0ae9 100644 --- a/plugins/tend-ci-runner/skills/run-tend/SKILL.md +++ b/plugins/tend-ci-runner/skills/run-tend/SKILL.md @@ -25,7 +25,7 @@ Nearly every task takes some of these, so work out which apply before you start: ## Temporary Files -Tend sets `$TMPDIR` to `/home/tend-sandbox/tmp`, the writable scratch directory; writes land only there and in the checkout, which sits under a `/tmp` container that is itself read-only, so a scratch path written by hand under `/tmp` fails with `Read-only file system` even though the mode reads `drwxrwxrwt`. A compound command without `set -e` runs on past that failure. Shell commands expand `$TMPDIR`; file-writing tools need the absolute path. +Write scratch files under `$TMPDIR`, which Tend sets to `/home/tend-sandbox/tmp`. Shell commands expand `$TMPDIR`; file-writing tools need the absolute path. ## Conduct diff --git a/proxy/setup_sandbox.py b/proxy/setup_sandbox.py index 16a95a28b..a06c5ec6e 100644 --- a/proxy/setup_sandbox.py +++ b/proxy/setup_sandbox.py @@ -69,7 +69,6 @@ "CODEX_AUTH_JSON", "CODEX_HOME", "TMPDIR", - "TMPPREFIX", } BLOCKED_COMMAND = """#!/bin/sh printf "tend: %s came from the runner home and is unavailable; install it into ~/.local/bin with sandbox_setup, or point sandbox_path at a copy outside the runner home\n" "${0##*/}" >&2 @@ -312,10 +311,6 @@ def base_agent_env( "GITHUB_WORKSPACE": str(workspace), "CLAUDE_CODE_REMOTE": "1", "TMPDIR": str(AGENT_TMP_DIR), - # zsh names here-document temp files from TMPPREFIX, not TMPDIR, and - # its default sits in the read-only root /tmp. A prefix needs only its - # directory to exist, which AGENT_TMP_DIR already does. - "TMPPREFIX": str(AGENT_TMP_DIR / "zsh"), } if anthropic_dummy: values[anthropic_dummy[0]] = anthropic_dummy[1] diff --git a/proxy/test-setup-sandbox.sh b/proxy/test-setup-sandbox.sh index 0837093f5..870b939a7 100755 --- a/proxy/test-setup-sandbox.sh +++ b/proxy/test-setup-sandbox.sh @@ -24,7 +24,9 @@ plant() { bin="$HOME/.cargo-install/tend-probe/bin" seeded="$HOME/.tend-seeded/bin" shared="/opt/tend-sandbox-test-$GITHUB_RUN_ID/bin" - TEND_AGENT_CONTAINER=$(mktemp -d /tmp/tend-agent-workspace-test.XXXXXX) + # /var/tmp, where prepare_agent_workspace puts the real container, so the + # assertions below read the same boundary the action builds. + TEND_AGENT_CONTAINER=$(mktemp -d /var/tmp/tend-agent-workspace-test.XXXXXX) TEND_AGENT_WORKSPACE="$TEND_AGENT_CONTAINER/checkout" TEND_RUNNER_WORKSPACE="$GITHUB_WORKSPACE" TEND_TEST_ACTION_PATH="$TEND_AGENT_CONTAINER/action" @@ -329,6 +331,10 @@ PY dummy_token=$(sed -n 's/^GITHUB_TOKEN=//p' "$AGENT_ENV_FILE") test -n "$dummy_token" + # The /tmp and /var/tmp probes below both aim at 1777 directories, where only + # SRT's bind can deny the write. Aimed at the 755 runtime or workspace + # container instead, the second would pass on file permissions alone and hold + # whether or not the boundary was still there. setup_commands=$(printf '%s\n' \ 'touch "$TEND_RUNNER_WORKSPACE/.tend-srt-wrote-here" 2>/dev/null || true' \ 'printf "%s\n" "$HTTP_PROXY" > .tend-setup-proxy' \ @@ -337,7 +343,8 @@ PY 'chmod +x ~/.local/bin/tend-probe' \ 'tend-probe > .tend-setup-tool' \ 'test -z "${GITHUB_ENV:-}"' \ - 'if touch /tmp/tend-unscoped 2>/dev/null; then exit 91; fi' \ + 'touch /tmp/tend-sandbox-scratch' \ + 'if touch /var/tmp/tend-unscoped 2>/dev/null; then exit 91; fi' \ 'touch "$TMPDIR/tend-scratch-probe"' \ "test \"\$GITHUB_TOKEN\" = \"$dummy_token\"") @@ -431,6 +438,24 @@ PY echo "[test-setup-sandbox] complete Claude and Codex SRT lifecycles verified" } +# The real dispose step, against the real filesystem: /tmp is the sandbox's to +# write and the runner's to keep, so ownership is all that separates what goes +# from what stays. +verify_dispose() { + local keep + keep="/tmp/tend-runner-keeps-$GITHUB_RUN_ID" + touch "$keep" + test "$(stat -c %U /tmp/tend-sandbox-scratch)" = "$SANDBOX" + PATH=/usr/sbin:/usr/bin:/sbin:/bin /usr/bin/python3 -E -s \ + shared/steps/dispose_sandbox_resources.py + test ! -e /tmp/tend-sandbox-scratch + test ! -e "$TEND_AGENT_CONTAINER" + test ! -e "$TEND_RUNTIME_ROOT" + test -f "$keep" + rm -f "$keep" + echo "[test-setup-sandbox] sandbox scratch disposed, runner entries kept" +} + cleanup() { local shared shared="/opt/tend-sandbox-test-$GITHUB_RUN_ID/bin" @@ -455,9 +480,10 @@ case "${1:-}" in verify) verify ;; verify-refusals) verify_refusals ;; verify-srt) verify_srt ;; + verify-dispose) verify_dispose ;; cleanup) cleanup ;; *) - echo "usage: $0 {plant|setup|install-agent-uv|verify|verify-refusals|verify-srt|cleanup}" >&2 + echo "usage: $0 {plant|setup|install-agent-uv|verify|verify-refusals|verify-srt|verify-dispose|cleanup}" >&2 exit 2 ;; esac diff --git a/proxy/test_setup_sandbox.py b/proxy/test_setup_sandbox.py index f14067805..ff8d15b03 100644 --- a/proxy/test_setup_sandbox.py +++ b/proxy/test_setup_sandbox.py @@ -117,20 +117,6 @@ def test_every_fixed_agent_assignment_is_reserved() -> None: assert f"TMPDIR={setup_sandbox.AGENT_TMP_DIR}" in assignments -def test_agent_environment_keeps_zsh_heredoc_scratch_writable() -> None: - """zsh names here-document temp files from TMPPREFIX, which ignores TMPDIR. - - Left at its `/tmp/zsh` default every heredoc fails against the sandbox's - read-only root `/tmp` — after the shell has already truncated the - redirection target to zero bytes. - """ - assignments = setup_sandbox.base_agent_env( - "/usr/bin", None, workspace=Path("/agent") - ) - - assert f"TMPPREFIX={setup_sandbox.AGENT_TMP_DIR / 'zsh'}" in assignments - - def test_github_only_agent_environment_has_no_model_credential() -> None: assignments = setup_sandbox.base_agent_env( "/usr/bin", None, workspace=Path("/agent") diff --git a/shared/steps/dispose_sandbox_resources.py b/shared/steps/dispose_sandbox_resources.py index acae4ece1..34e342f96 100644 --- a/shared/steps/dispose_sandbox_resources.py +++ b/shared/steps/dispose_sandbox_resources.py @@ -2,15 +2,24 @@ # requires-python = ">=3.12" # dependencies = [] # /// -"""Delete the event checkout and per-run runtime after the sandbox is reaped.""" +"""Delete what the sandbox ran from and what it left behind, after the reap. + +The event checkout and the per-run runtime are named containers under +``/var/tmp``; ``/tmp`` is scratch the sandbox shares with the runner, so what +the sandbox uid owns at the top level of ``/tmp`` goes too, before a later +runner step or a ``setup:`` action's POST step reads it. +""" from __future__ import annotations import os +import pwd import re import subprocess from pathlib import Path +CONTAINER_PARENT = Path("/var/tmp") +SANDBOX_SCRATCH = Path("/tmp") WORKSPACE_CONTAINER = re.compile(r"tend-agent-workspace-[A-Za-z0-9._-]+\Z") RUNTIME_CONTAINER = re.compile(r"tend-runtime\.[A-Za-z0-9]+\Z") @@ -21,26 +30,44 @@ def fail(message: str) -> int: def workspace_container(workspace: Path) -> Path: - """Return the dedicated /tmp container or reject a broader target.""" + """Return the dedicated /var/tmp container or reject a broader target.""" if workspace.name != "checkout": raise ValueError("TEND_AGENT_WORKSPACE must end in /checkout") container = workspace.parent - if container.parent != Path("/tmp") or not WORKSPACE_CONTAINER.fullmatch( + if container.parent != CONTAINER_PARENT or not WORKSPACE_CONTAINER.fullmatch( container.name ): raise ValueError( - "TEND_AGENT_WORKSPACE must be inside a tend-agent-workspace-* /tmp container" + "TEND_AGENT_WORKSPACE must be inside a " + "tend-agent-workspace-* /var/tmp container" ) return container def runtime_container(runtime: Path) -> Path: """Accept only the mktemp shape used by install-sandbox-runtime.sh.""" - if runtime.parent != Path("/tmp") or not RUNTIME_CONTAINER.fullmatch(runtime.name): - raise ValueError("TEND_RUNTIME_ROOT must be a tend-runtime.* /tmp directory") + if runtime.parent != CONTAINER_PARENT or not RUNTIME_CONTAINER.fullmatch( + runtime.name + ): + raise ValueError( + "TEND_RUNTIME_ROOT must be a tend-runtime.* /var/tmp directory" + ) return runtime +def scratch_entries(sandbox: str) -> list[Path]: + """Return the top-level /tmp entries the sandbox uid owns. + + Ownership is the whole test: /tmp is sticky, so an entry the sandbox uid + owns is one the sandbox created, and a runner-owned entry beside it — a + `setup:` action's state, whatever the runner image keeps here — stays. + """ + uid = pwd.getpwnam(sandbox).pw_uid + return sorted( + entry for entry in SANDBOX_SCRATCH.iterdir() if entry.lstat().st_uid == uid + ) + + def targets() -> list[Path]: """Resolve every resource that this run got far enough to create.""" resources: list[Path] = [] @@ -56,8 +83,6 @@ def targets() -> list[Path]: def main() -> int: try: resources = targets() - if not resources: - return 0 sandbox = os.environ.get("SANDBOX", "") if sandbox: live = subprocess.run( @@ -72,6 +97,9 @@ def main() -> int: ) if live.returncode != 1: return fail("could not verify that sandbox processes were reaped") + resources.extend(scratch_entries(sandbox)) + if not resources: + return 0 subprocess.run( [ "/usr/bin/sudo", diff --git a/shared/steps/install-sandbox-runtime.sh b/shared/steps/install-sandbox-runtime.sh index 576a0f51b..502f1286b 100644 --- a/shared/steps/install-sandbox-runtime.sh +++ b/shared/steps/install-sandbox-runtime.sh @@ -136,7 +136,9 @@ if [ "${#stale[@]}" -gt 0 ]; then /usr/bin/sudo /usr/bin/apt-get "${apt_options[@]}" install -y --allow-downgrades "${stale[@]}" fi -runtime_root=$(/usr/bin/mktemp -d /tmp/tend-runtime.XXXXXX) +# /var/tmp rather than /tmp: both are 1777, but the sandbox may write /tmp, and +# the runtime it executes from has to stay read-only to it. +runtime_root=$(/usr/bin/mktemp -d /var/tmp/tend-runtime.XXXXXX) # Publish the cleanup target before any fallible install work. The later # always() cleanup can then remove a partial runtime too. echo "TEND_RUNTIME_ROOT=$runtime_root" >> "$GITHUB_ENV" diff --git a/shared/steps/prepare_agent_workspace.py b/shared/steps/prepare_agent_workspace.py index a3dc2e2b1..6da013bd8 100644 --- a/shared/steps/prepare_agent_workspace.py +++ b/shared/steps/prepare_agent_workspace.py @@ -5,7 +5,7 @@ """Create the only repository tree the agent is allowed to access. The Actions checkout is trusted orchestration state. This step clones the -same repository into a dedicated ``/tmp`` container without local object +same repository into a dedicated ``/var/tmp`` container without local object sharing, selects the event topology with runner/system Git configuration disabled, removes every temporary credential, and exports the resulting path as ``TEND_AGENT_WORKSPACE``. The dedicated parent holds that clone alone and @@ -336,8 +336,10 @@ def main() -> int: ) if mode not in {"base", "review", "mention"}: raise ValueError(f"unsupported checkout mode: {mode}") + # /var/tmp rather than /tmp: both are 1777, but the sandbox may write + # /tmp, and only the clone inside this container is its to write. workspace_container = Path( - tempfile.mkdtemp(prefix="tend-agent-workspace-", dir="/tmp") + tempfile.mkdtemp(prefix="tend-agent-workspace-", dir="/var/tmp") ) destination = workspace_container / "checkout" if destination == runner_workspace or destination.is_relative_to( diff --git a/shared/steps/sandbox_runtime.mjs b/shared/steps/sandbox_runtime.mjs index 44d5b8a8f..978fbaaf2 100644 --- a/shared/steps/sandbox_runtime.mjs +++ b/shared/steps/sandbox_runtime.mjs @@ -49,9 +49,11 @@ async function main() { } const { SandboxManager } = await import(`file://${entry}`); - // SRT otherwise replaces TMPDIR with its default /tmp/claude path when - // filesystem isolation is enabled. Keep its generated child environment on - // Tend's already-writable scratch directory. + // With filesystem isolation on, SRT sets TMPDIR in the child environment to + // CLAUDE_CODE_TMPDIR or its own /tmp/claude default, whichever it finds, + // overriding the agent environment file. Tend's scratch directory is the one + // the supervisor reads the step summary from and the one the shipped skills + // name, so hand SRT that path rather than letting it choose. process.env.CLAUDE_CODE_TMPDIR = agentTmpDir; const command = `/usr/bin/python3 -E -s ${quote(lifecycle)}`; const config = { @@ -79,6 +81,15 @@ async function main() { allowWrite: [ agentWorkspace, agentHome, + // /tmp is ordinary scratch inside the sandbox. Tooling hard-codes + // paths under it with no environment variable to move them — NuGet's + // build mutex and zsh's here-documents among them — so a read-only + // /tmp buys a per-tool workaround every time one surfaces. Tend's own + // runtime and checkout containers sit under /var/tmp, which this list + // does not cover and the sandbox therefore cannot write, and + // dispose_sandbox_resources.py removes what the sandbox uid leaves + // here before any later runner step reads /tmp. + "/tmp", ...(autoMemory ? [autoMemory] : []), ], denyWrite: [], diff --git a/shared/steps/test_dispose_sandbox_resources.py b/shared/steps/test_dispose_sandbox_resources.py index 12f30f9df..5d0c72750 100644 --- a/shared/steps/test_dispose_sandbox_resources.py +++ b/shared/steps/test_dispose_sandbox_resources.py @@ -1,7 +1,9 @@ from __future__ import annotations +import os import subprocess from pathlib import Path +from types import SimpleNamespace import dispose_sandbox_resources as dispose import pytest @@ -11,39 +13,71 @@ def result(args: list[str], returncode: int = 0) -> subprocess.CompletedProcess[ return subprocess.CompletedProcess(args, returncode) +def scratch(monkeypatch: pytest.MonkeyPatch, directory: Path, uid: int) -> None: + """Point the /tmp sweep at a directory this suite owns.""" + monkeypatch.setattr(dispose, "SANDBOX_SCRATCH", directory) + monkeypatch.setattr( + dispose.pwd, "getpwnam", lambda _name: SimpleNamespace(pw_uid=uid) + ) + + def test_accepts_only_the_dedicated_workspace_shape() -> None: - expected = Path("/tmp/tend-agent-workspace-abc123") + expected = Path("/var/tmp/tend-agent-workspace-abc123") assert dispose.workspace_container(expected / "checkout") == expected for path in ( - Path("/tmp/tend-agent-workspace-abc123"), - Path("/tmp/unrelated/checkout"), - Path("/var/tmp/tend-agent-workspace-abc123/checkout"), + Path("/var/tmp/tend-agent-workspace-abc123"), + Path("/var/tmp/unrelated/checkout"), + Path("/tmp/tend-agent-workspace-abc123/checkout"), ): with pytest.raises(ValueError): dispose.workspace_container(path) def test_accepts_only_the_dedicated_runtime_shape() -> None: - expected = Path("/tmp/tend-runtime.aB123z") + expected = Path("/var/tmp/tend-runtime.aB123z") assert dispose.runtime_container(expected) == expected for path in ( - Path("/tmp/tend-runtime"), - Path("/tmp/tend-runtime.bad/name"), - Path("/var/tmp/tend-runtime.abc123"), + Path("/var/tmp/tend-runtime"), + Path("/var/tmp/tend-runtime.bad/name"), + Path("/tmp/tend-runtime.abc123"), ): with pytest.raises(ValueError): dispose.runtime_container(path) +def test_the_scratch_sweep_selects_on_the_owning_uid( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + """Every entry here belongs to the suite, so only the uid can be varied. + + `verify_dispose` in `proxy/test-setup-sandbox.sh` is where a runner-owned + entry sits beside a sandbox-owned one; chowning a fixture needs the root + that test has and this one does not. + """ + (tmp_path / "tend-agent-scratch").mkdir() + (tmp_path / "pytest-of-runner").mkdir() + + scratch(monkeypatch, tmp_path, os.getuid()) + assert dispose.scratch_entries("tend-sandbox") == [ + tmp_path / "pytest-of-runner", + tmp_path / "tend-agent-scratch", + ] + + scratch(monkeypatch, tmp_path, os.getuid() + 1) + assert dispose.scratch_entries("tend-sandbox") == [] + + def test_disposes_only_after_the_sandbox_uid_is_empty( - monkeypatch: pytest.MonkeyPatch, + monkeypatch: pytest.MonkeyPatch, tmp_path: Path ) -> None: - workspace = Path("/tmp/tend-agent-workspace-abc123/checkout") + workspace = Path("/var/tmp/tend-agent-workspace-abc123/checkout") monkeypatch.setenv("TEND_AGENT_WORKSPACE", str(workspace)) - monkeypatch.setenv("TEND_RUNTIME_ROOT", "/tmp/tend-runtime.r1a2b3") + monkeypatch.setenv("TEND_RUNTIME_ROOT", "/var/tmp/tend-runtime.r1a2b3") monkeypatch.setenv("SANDBOX", "tend-sandbox") + (tmp_path / "nuget-mutex").touch() + scratch(monkeypatch, tmp_path, os.getuid()) calls: list[list[str]] = [] def run(args: list[str], **_kwargs: object) -> subprocess.CompletedProcess[str]: @@ -61,8 +95,9 @@ def run(args: list[str], **_kwargs: object) -> subprocess.CompletedProcess[str]: "/usr/bin/rm", "-rf", "--", - "/tmp/tend-agent-workspace-abc123", - "/tmp/tend-runtime.r1a2b3", + "/var/tmp/tend-agent-workspace-abc123", + "/var/tmp/tend-runtime.r1a2b3", + str(tmp_path / "nuget-mutex"), ], ] @@ -71,7 +106,7 @@ def test_refuses_to_dispose_while_a_sandbox_process_lives( monkeypatch: pytest.MonkeyPatch, ) -> None: monkeypatch.setenv( - "TEND_AGENT_WORKSPACE", "/tmp/tend-agent-workspace-abc123/checkout" + "TEND_AGENT_WORKSPACE", "/var/tmp/tend-agent-workspace-abc123/checkout" ) monkeypatch.setenv("SANDBOX", "tend-sandbox") calls: list[list[str]] = [] @@ -90,7 +125,7 @@ def test_disposes_partial_runtime_when_workspace_was_never_prepared( monkeypatch: pytest.MonkeyPatch, ) -> None: monkeypatch.delenv("TEND_AGENT_WORKSPACE", raising=False) - monkeypatch.setenv("TEND_RUNTIME_ROOT", "/tmp/tend-runtime.abc123") + monkeypatch.setenv("TEND_RUNTIME_ROOT", "/var/tmp/tend-runtime.abc123") monkeypatch.delenv("SANDBOX", raising=False) calls: list[list[str]] = [] @@ -108,6 +143,6 @@ def run(args: list[str], **_kwargs: object) -> subprocess.CompletedProcess[str]: "/usr/bin/rm", "-rf", "--", - "/tmp/tend-runtime.abc123", + "/var/tmp/tend-runtime.abc123", ] ] diff --git a/shared/steps/test_prepare_agent_workspace.py b/shared/steps/test_prepare_agent_workspace.py index dd3f0f2df..ae1da2394 100644 --- a/shared/steps/test_prepare_agent_workspace.py +++ b/shared/steps/test_prepare_agent_workspace.py @@ -295,7 +295,7 @@ def refuse(path: str, _token: str) -> dict[str, object]: raise urllib.error.HTTPError(path, 404, "Not Found", {}, None) clone_workspace = prepare.clone_workspace - # The step roots the agent workspace at the runner's own /tmp, which a + # The step roots the agent workspace at the runner's own /var/tmp, which a # suite run inside a tend session cannot write to. Keep the real call and # move only its parent, so the assertions below still read a real clone. mkdtemp = prepare.tempfile.mkdtemp From 6492e62f2dff4994afad7db5fc2659f8b85580bd Mon Sep 17 00:00:00 2001 From: Maximilian Roos Date: Sat, 19 Sep 2026 12:47:07 -0700 Subject: [PATCH 2/3] test(sandbox): probe the sticky bit that a writable /tmp rests on The security model now says the sandbox can add entries to /tmp but cannot unlink or rename a runner-owned one beside them. That sticky bit is all that protects runner state in the shared directory while the agent runs, and nothing exercised it: the dispose test plants its runner-owned file after the sandbox is already gone, so it covers the sweep's ownership filter instead. The file is planted before the lifecycle now, the sandbox tries to unlink it from inside, and dispose still has to leave it. A lost fixture fails the probe the same way a lost sticky bit does, since rm -f returns 0 on a path that is not there. Co-Authored-By: Claude Opus 5 (1M context) --- proxy/test-setup-sandbox.sh | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/proxy/test-setup-sandbox.sh b/proxy/test-setup-sandbox.sh index 870b939a7..593dbbfcc 100755 --- a/proxy/test-setup-sandbox.sh +++ b/proxy/test-setup-sandbox.sh @@ -233,7 +233,7 @@ verify_refusals() { verify_srt() { local claude_argv claude_env claude_stub codex_argv codex_env codex_stub dummy_token local github_output private_action probe_info probe_pid probe_port rc runner_summary - local setup_commands setup_proxy stream_json tool_root + local runner_owned setup_commands setup_proxy stream_json tool_root github_output="$RUNNER_TEMP/srt-github-output" runner_summary="$RUNNER_TEMP/srt-step-summary" probe_info="$RUNNER_TEMP/srt-network-probe" @@ -331,10 +331,14 @@ PY dummy_token=$(sed -n 's/^GITHUB_TOKEN=//p' "$AGENT_ENV_FILE") test -n "$dummy_token" - # The /tmp and /var/tmp probes below both aim at 1777 directories, where only - # SRT's bind can deny the write. Aimed at the 755 runtime or workspace - # container instead, the second would pass on file permissions alone and hold - # whether or not the boundary was still there. + # What the runner keeps in the shared /tmp while the agent runs, protected by + # nothing but the sticky bit. + runner_owned="/tmp/tend-runner-owned-$GITHUB_RUN_ID" + touch "$runner_owned" + # The three /tmp and /var/tmp probes below all aim at 1777 directories, where + # only SRT's bind and the sticky bit can deny them. Aimed at the 755 runtime + # or workspace container instead, the write probe would be refused on file + # permissions alone and would hold whether or not the boundary was there. setup_commands=$(printf '%s\n' \ 'touch "$TEND_RUNNER_WORKSPACE/.tend-srt-wrote-here" 2>/dev/null || true' \ 'printf "%s\n" "$HTTP_PROXY" > .tend-setup-proxy' \ @@ -345,6 +349,7 @@ PY 'test -z "${GITHUB_ENV:-}"' \ 'touch /tmp/tend-sandbox-scratch' \ 'if touch /var/tmp/tend-unscoped 2>/dev/null; then exit 91; fi' \ + "if rm -f '$runner_owned' 2>/dev/null; then exit 92; fi" \ 'touch "$TMPDIR/tend-scratch-probe"' \ "test \"\$GITHUB_TOKEN\" = \"$dummy_token\"") @@ -440,19 +445,16 @@ PY # The real dispose step, against the real filesystem: /tmp is the sandbox's to # write and the runner's to keep, so ownership is all that separates what goes -# from what stays. +# from what stays. The runner-owned file the sandbox failed to unlink in +# verify_srt is the one that has to survive this too. verify_dispose() { - local keep - keep="/tmp/tend-runner-keeps-$GITHUB_RUN_ID" - touch "$keep" test "$(stat -c %U /tmp/tend-sandbox-scratch)" = "$SANDBOX" PATH=/usr/sbin:/usr/bin:/sbin:/bin /usr/bin/python3 -E -s \ shared/steps/dispose_sandbox_resources.py test ! -e /tmp/tend-sandbox-scratch test ! -e "$TEND_AGENT_CONTAINER" test ! -e "$TEND_RUNTIME_ROOT" - test -f "$keep" - rm -f "$keep" + test -f "/tmp/tend-runner-owned-$GITHUB_RUN_ID" echo "[test-setup-sandbox] sandbox scratch disposed, runner entries kept" } @@ -466,6 +468,7 @@ cleanup() { if [ -n "${TEND_AGENT_CONTAINER:-}" ]; then /usr/bin/sudo rmdir -- "$TEND_AGENT_CONTAINER" 2>/dev/null || true fi + /usr/bin/sudo rm -f "/tmp/tend-runner-owned-$GITHUB_RUN_ID" /usr/bin/sudo rm -f /usr/local/bin/tend-probe "$shared/tend-shared" "$shared/uv" /usr/bin/sudo rmdir "$shared" "${shared%/bin}" 2>/dev/null || true /usr/bin/sudo rm -f /etc/skel/.tend-seeded/bin/tend-seeded From 7a3c0f680d5f41336e18d81c6a0b36ec1d59a1b2 Mon Sep 17 00:00:00 2001 From: Maximilian Roos Date: Sat, 19 Sep 2026 13:05:30 -0700 Subject: [PATCH 3/3] fix(sandbox): survive a /tmp entry that vanishes mid-sweep MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The sweep lstats every top-level entry of a directory the whole runner shares, so a temp file another process removes between the listing and the stat raised FileNotFoundError and failed the step. The dispose step runs under always(), after the agent has already posted its work, and a failure there files a tend-outage issue — over someone else's temp file. An entry that is already gone is the outcome the sweep wants, so skip it. The security model now also says where ownership stops separating the sandbox's scratch from the runner's: at /tmp's top level. What the agent wrote inside a directory the runner or a setup: action already owned there survives the sweep, and walking those trees to find it would be a larger mechanism than the one this protects. Co-Authored-By: Claude Opus 5 (1M context) --- docs/security-model.md | 8 ++++--- shared/steps/dispose_sandbox_resources.py | 15 ++++++++++--- .../steps/test_dispose_sandbox_resources.py | 22 +++++++++++++++++++ 3 files changed, 39 insertions(+), 6 deletions(-) diff --git a/docs/security-model.md b/docs/security-model.md index 156538ac1..89e0dd1ba 100644 --- a/docs/security-model.md +++ b/docs/security-model.md @@ -406,9 +406,11 @@ After SRT exits, the trusted supervisor kills and verifies the complete sandbox UID process tree, then copies only size-bounded fixed outputs. The next fixed action step deletes the dedicated `/var/tmp/tend-agent-workspace-*` container and every top-level `/tmp` entry the sandbox UID owns, so no post-sandbox step -executes a file from that checkout and the agent's scratch is gone before a -later step or a `setup:` action's POST step could read it. A run whose reap -failed keeps both: the job has already failed, and the live writer is the +executes a file from that checkout and the agent's own scratch is gone before a +later step or a `setup:` action's POST step could read it. Ownership separates +the two at `/tmp`'s top level alone: what the agent wrote inside a directory +the runner or a `setup:` action already owned there stays. A run whose reap +failed keeps everything: the job has already failed, and the live writer is the reason not to delete underneath it. **Credential isolation.** Both harness actions run the agent as a separate diff --git a/shared/steps/dispose_sandbox_resources.py b/shared/steps/dispose_sandbox_resources.py index 34e342f96..426075ad9 100644 --- a/shared/steps/dispose_sandbox_resources.py +++ b/shared/steps/dispose_sandbox_resources.py @@ -63,9 +63,18 @@ def scratch_entries(sandbox: str) -> list[Path]: `setup:` action's state, whatever the runner image keeps here — stays. """ uid = pwd.getpwnam(sandbox).pw_uid - return sorted( - entry for entry in SANDBOX_SCRATCH.iterdir() if entry.lstat().st_uid == uid - ) + owned = [] + for entry in sorted(SANDBOX_SCRATCH.iterdir()): + try: + owner = entry.lstat().st_uid + except FileNotFoundError: + # Someone else's temp file, removed between the listing and the + # stat. Already gone is what this step wants; /tmp is shared, so + # reading it races anything else still running on the runner. + continue + if owner == uid: + owned.append(entry) + return owned def targets() -> list[Path]: diff --git a/shared/steps/test_dispose_sandbox_resources.py b/shared/steps/test_dispose_sandbox_resources.py index 5d0c72750..135067fe0 100644 --- a/shared/steps/test_dispose_sandbox_resources.py +++ b/shared/steps/test_dispose_sandbox_resources.py @@ -69,6 +69,28 @@ def test_the_scratch_sweep_selects_on_the_owning_uid( assert dispose.scratch_entries("tend-sandbox") == [] +def test_an_entry_that_vanishes_mid_sweep_is_not_an_error( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + """/tmp is shared, so the listing can name a file another process removes. + + The stand-in supplies a listing that is already stale, which is what the + real `iterdir` hands back when a runner process deletes its own temp file + between the listing and the stat. + """ + (tmp_path / "tend-agent-scratch").touch() + scratch(monkeypatch, tmp_path, os.getuid()) + monkeypatch.setattr( + dispose, + "SANDBOX_SCRATCH", + SimpleNamespace( + iterdir=lambda: [tmp_path / "vanished", tmp_path / "tend-agent-scratch"] + ), + ) + + assert dispose.scratch_entries("tend-sandbox") == [tmp_path / "tend-agent-scratch"] + + def test_disposes_only_after_the_sandbox_uid_is_empty( monkeypatch: pytest.MonkeyPatch, tmp_path: Path ) -> None: