fix(sandbox): make UnixLocal persist_workspace archives restorable by hydrate_workspace

[coderdailyone]

Summary

This pull request fixes UnixLocalSandboxSession.persist_workspace() so the archive it produces can actually be restored by hydrate_workspace().

Bug

persist_workspace() archives the workspace with tarfile.add() as-is, while hydrate_workspace() extracts with the strict policy (safe_extract_tarfile(..., allow_external_symlink_targets=False)) that refuses hardlink members, FIFOs / device nodes, and absolute symlink targets. Ordinary local workspaces contain all three, so the snapshot is taken successfully and then can never be restored. Reproduced on main with a real UnixLocalSandboxClient session (persist into one session, hydrate into a fresh one):

workspace content	hydrate result on main
a.txt + ln a.txt b.txt (what uv / pnpm do for every installed package)	WorkspaceArchiveWriteError — hardlink member not allowed, member ./b.txt
a FIFO left behind by a dev server (mkfifo pipe)	unsupported member type, member ./pipe
ln -s "$PWD/a.txt" abs_inside (absolute link to a file inside the workspace)	absolute symlink target not allowed: /tmp/sandbox-local-…/a.txt
python3 -m venv .venv	absolute symlink target not allowed: /usr/bin/python3

The last row is the intentional #3094 policy and is not changed here. The first three are not "external" in any sense, and the absolute-internal link is additionally wrong after restore even if it were accepted, because UnixLocal creates a fresh /tmp/sandbox-local-* root per session.

Fix

Add a filter step to the tarfile.add() call in persist_workspace() (_restorable_tar_member):

• hardlink members become regular file members (TarFile.add then reads the payload), so the restored workspace has two files with the same content;
• FIFOs and character/block device members are dropped (sockets already are, by tarfile);
• an absolute symlink target under the workspace root (checked against both the configured root and its resolved form, for hosts where /tmp is itself a symlink) has only its root prefix replaced by the climb out of the link's own archive directory, e.g. /tmp/sandbox-local-x/a.txt from sub/abs_up becomes ../a.txt; the remaining components are kept verbatim, so <root>/current/../config stays current/../config and still resolves through the current symlink after restore;
• everything else, including relative symlinks and absolute targets outside the workspace, is left untouched.

No public signatures change and hydrate_workspace() is not modified.

Test plan

• tests/sandbox/test_unix_local.py::TestUnixLocalPersistWorkspaceRestorable inspects the emitted members and round-trips the archive into a new root through hydrate_workspace(); both tests fail on main and pass with this change.
• tests/sandbox/test_unix_local.py, test_tar_utils.py, test_extract.py, ruff, mypy, and pyright on the changed files pass locally (Linux, Python 3.10).

Verification stack: make format/ruff check clean, mypy and pyright clean on the changed files, focused suites plus tests/sandbox pass on Linux/Python 3.10. The repository-wide mypy src reports pre-existing errors on Python 3.10 (archive_ops.py SpooledTemporaryFile typing, run_loop.py BaseExceptionGroup export) that are unrelated to this PR, so the full-stack checkbox is left unchecked.

Issue number

None (found while auditing the UnixLocal / Docker sandbox file operations).

Checks

• I've added new tests, if relevant
• I've run the verification steps from .agents/skills/code-change-verification individually (format, lint, typecheck on changed files, tests)
• I've confirmed all verification steps pass (see Test plan: pre-existing Python 3.10 mypy errors outside this change)
• If using Codex, I've run /review before submitting this PR

🤖 Generated with Claude Code

https://claude.ai/code/session_01BN4v25msJgrjNgac97g1Az

[sylvesterkaczmarek]

Normalising at archive creation is the right boundary because hydrate already has a strict, security-relevant contract. Converting hardlinks to payload-bearing regular members and relativising only absolute targets proven inside the workspace preserves that contract without weakening extraction.

[seratch]

Thanks for the contribution. Fixing archive creation is the right direction, and the hardlink restore failure warrants a focused fix.

Before merging, please correct the absolute-symlink rewrite so it preserves path semantics. With current -> releases/v1, a link to <root>/current/../config refers to <root>/releases/config, but normpath() collapses it to <root>/config. The restored link can therefore silently read a different file.

Please preserve the target components when replacing the workspace-root prefix, avoiding normalization that collapses .. across symlink components. Add a persist/hydrate regression with different contents at those two paths and assert that the restored link reads the original contents.

[coderdailyone]

Thanks — you're right that normpath() changed the meaning. Fixed in 1e1806d4: the rewrite now only replaces the workspace-root prefix with the climb out of the link's own archive directory and keeps the target's components verbatim, so <root>/current/../config becomes current/../config (or ../../current/../config from releases/v1/) and the kernel still resolves the .. against the current symlink's target after restore. Regression test_rebased_symlink_keeps_parent_steps_after_symlink_components sets up current -> releases/v1, config = "wrong", releases/config = "right", persists and hydrates into a new root, and asserts both restored links read "right"; on the previous revision the link was rewritten to config.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1e1806d47c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Consume duplicate separators at the root boundary

When an internal absolute symlink target contains two separators immediately after the workspace root (for example, ln -s "$PWD//a.txt" link), this slice consumes only one separator, leaving rest as /a.txt. _rebase_symlink_target() therefore emits an absolute target, and hydrate_workspace() rejects the persisted archive because external absolute symlinks are disallowed. Consume the full separator run at this boundary while preserving meaningful components such as ...

AGENTS.md reference: AGENTS.md:L205-L205

Useful? React with 👍 / 👎.

[coderdailyone]

Fixed in 7133097a: after matching the root prefix the whole separator run is consumed (rest = target[len(prefix):].lstrip("/")), so <root>//a.txt rebases to a.txt while later components such as .. stay untouched. The persist fixture gained a double_sep link and the strict round-trip covers it.