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

[coderdailyone]

Summary

DockerSandboxSession.persist_workspace() stages a copy of the workspace with cp -R, has the daemon archive it, and normalizes the member prefix in Python (strip_tar_member_prefix()). hydrate_workspace() then extracts with the strict policy that refuses FIFO/device members and absolute symlink targets. A staged Docker workspace legitimately carries both, so the snapshot was produced and then could never be restored:

workspace content	result on main
a FIFO left behind by a dev server	persist_workspace() fails: UnsafeTarMemberError: unsupported member type
ln -s /workspace/sub/data.txt link (absolute link to a file inside the workspace)	persist succeeds; hydrate_workspace() refuses the archive with absolute symlink target not allowed

strip_tar_member_prefix() now drops FIFO and character/block device members, and, when the caller passes the workspace root (relativize_symlinks_under=; Docker passes its root), rebases an absolute symlink target under that root onto the link's own directory. Only the root prefix is replaced and the remaining components are kept verbatim: with alias -> sub/deep, /workspace/alias/../data.txt names sub/data.txt (the kernel resolves the .. against the alias target), so the restored link is alias/../data.txt, never a lexically collapsed data.txt. A long target's PAX linkpath record is dropped so the rewritten linkname is what gets written. Absolute targets outside the workspace are left unchanged for hydrate's existing policy (#3094); without the argument, symlink targets are untouched.

Hardlink handling from earlier revisions is removed: cp -R copies hardlinked files independently, so hardlink members do not reach this path.

Test plan

• tests/sandbox/test_docker.py::test_docker_persist_and_hydrate_keep_absolute_workspace_symlinks_resolving: a host-backed Docker session persists a workspace with alias -> sub/deep, abs_alias -> /workspace/alias/../data.txt (where data.txt = "wrong" and sub/data.txt = "right") and sub/abs_up -> /workspace/sub/data.txt through the real staging + normalization path (the fake cp -R now preserves symlinks like the real one), hydrates into a second session (the fake tar -x extracts on the host), and asserts the restored links are alias/../data.txt / ../sub/data.txt and both read "right". On main hydrate refuses the archive.
• tests/sandbox/test_tar_utils.py: the workspace/… fixture covers the FIFO, sub/abs_up, a //workspace/... target, the alias/.. case, a >100-byte PAX linkpath target and an external target; the rewrite test asserts each member, the strict-hydrate test extracts and reads abs_alias as "right", and test_strip_tar_member_prefix_still_rejects_hardlink_members pins that hardlink members are still rejected.
• tests/sandbox/test_tar_utils.py, test_extract.py, test_docker.py: 196 + 110 passed. ruff format/ruff check clean, mypy and pyright clean on the changed files (Linux, Python 3.10). The repository-wide mypy src reports pre-existing Python 3.10 errors in archive_ops.py / run_loop.py unrelated to this change, so the full-stack checkbox is left unchecked.

Issue number

None (Docker counterpart of #4831).

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)
• If using Codex, I've run /review before submitting this PR

🤖 Generated with Claude Code

https://claude.ai/code/session_01DFSMrLZZ3oq1dKmJFAofz6

[sylvesterkaczmarek]

This changes persist_workspace() from streaming the Docker archive to fully spooling it to a local tempfile before producing another tempfile. For large workspaces, peak host temp usage becomes roughly the input archive plus the rewritten archive (and hardlink expansion can make the output larger), so a valid workspace can now fail solely because /tmp lacks roughly 2x its size. Could hardlink payload lookup be made bounded without buffering the entire archive, or should this enforce an explicit archive/temp-space limit?

[coderdailyone]

Good point, thanks — the spool was the easy way to get the earlier member back, but it doubled the temp footprint. Reworked in the latest revision: the source archive is streamed once again (r|*), and a hardlink member's payload is read back from the rewritten archive that is being written (the regular members' data offsets are recorded by original member name as they are emitted; a small reader seeks to the payload and restores the writer's position around each read). Peak temp usage is back to one archive, the same as before this PR; hardlink expansion only grows the output by the duplicated payloads, which is inherent to storing them as regular files. A hardlink whose target is not a regular file already in the archive is rejected with a clear UnsafeTarMemberError. Existing and new strip_tar_member_prefix tests pass unchanged.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a64ccf615f

ℹ️ 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]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: dce44cfdb8

ℹ️ 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".

[seratch]

The FIFO and internal absolute-symlink cases warrant a focused fix, but Docker stages the workspace with cp -R, which already copies hardlinked files independently on the normal GNU coreutils path. Please remove the hardlink expansion machinery and narrow the patch to the remaining snapshot failures.

The symlink rewrite also needs to preserve path semantics. With alias -> sub/deep, /workspace/alias/../data.txt refers to sub/data.txt, but the current normalization rewrites it to the workspace-root data.txt. Please rebase internal targets without collapsing their components, and add Docker-session persist/hydrate coverage that exercises staging and verifies restored contents, including this symlink case.

[coderdailyone]

Thanks for the review. Both points addressed in fa4a43a6:

• Hardlinks: removed the expansion machinery (payload read-back, spooling). The function is back to the original single-pass stream; hardlink members are rejected as before (test_strip_tar_member_prefix_still_rejects_hardlink_members pins it). The PR is now only the FIFO/device drop and the in-workspace absolute symlink rebase.
• Symlink semantics: the rewrite no longer normalizes. Only the root prefix is replaced by the climb out of the link's own archive directory and the remaining components are kept verbatim, so /workspace/alias/../data.txt with alias -> sub/deep becomes alias/../data.txt and still resolves to sub/data.txt after restore.
• Docker-session coverage: test_docker_persist_and_hydrate_keep_absolute_workspace_symlinks_resolving runs a host-backed Docker session through staging (the fake cp -R now preserves symlinks, as the real one does) and archive normalization, hydrates into a second session, and asserts the restored links are alias/../data.txt / ../sub/data.txt and read the sub/data.txt contents ("right", not the root-level "wrong"). On main hydrate refuses the archive.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fa4a43a6e5

ℹ️ 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]

Keep duplicate separators relative after removing the root

When a root-level symlink targets /workspace//a.txt, Linux resolves it inside the workspace, but this slice leaves rest as /a.txt; with an empty climb, the rewritten target remains absolute. persist_workspace() therefore returns the archive, while the strict hydrate_workspace() rejects it with absolute symlink target not allowed. Remove all separators immediately after the matched root boundary, while continuing to preserve subsequent path components, so this valid in-workspace link remains restorable.

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

Useful? React with 👍 / 👎.

[coderdailyone]

Fixed in 266eab83: the whole separator run after the matched root is consumed, so /workspace//a.txt rebases to a.txt (with an empty climb it is no longer left absolute) while later components such as .. stay untouched. The fixture gained a double_sep member and the strict-hydrate round trip covers it.