Skip to content

fix(client): restore system tar for extraction - #448

Merged
alecthomas merged 1 commit into
mainfrom
aat/restore-system-tar
Nov 13, 2026
Merged

alecthomas merged 1 commit into
mainfrom
aat/restore-system-tar

Conversation

@alecthomas

Copy link
Copy Markdown
Collaborator

#418 replaced the zstd | tar -xpf pipeline in client.Extract with an in-process Go extractor that validates entry paths, symlink targets and hard links. That hardening only matters if clients can be handed a malicious archive, but clients only extract archives that cachew service accounts wrote, so there is no untrusted input to defend against. The extractor did have real costs:

  • It was about 35% slower on trees of many small files. Extracting the Go source tree (15.6k files, 271 MB) went from 5.6s to 7.6s median on an M3 Max. A CPU profile showed roughly 43% of extract time in Lstat, because every file re-checks each parent directory twice before it is written. Trees of a few large files were faster with Go, but those are not the common case.
  • It rejected any absolute or out-of-tree symlink, so legitimate trees such as Python virtualenvs could no longer be restored with cachew restore.

This restores the pre-#418 Extract and documents that its input must be trusted, so cachew must only allow writes from trusted sources. The Go-extractor-specific tests are removed, and a new test checks that absolute and ../ symlinks survive a save and restore. The rest of #418 is unchanged: request-path validation, per-strategy path checks, GitHub URL escaping, and the Archive include-path check.

The benchmark was run on macOS with bsdtar, not on Linux with GNU tar.

🤖 Generated with Claude Code

#418 replaced `tar -xpf` with an in-process extractor that validates
entry paths and symlink targets. Clients only extract archives that
cachew service accounts wrote, so there is no untrusted input to
defend against, and the extractor had real costs:

- ~35% slower on trees of many small files (Go source tree: 5.6s ->
  7.6s), mostly from repeated Lstat calls on every parent directory.
- It rejected legitimate absolute or out-of-tree symlinks, so trees
  such as Python virtualenvs could no longer be restored.

Extract now documents that cachew must only accept writes from trusted
sources. Request-path validation and GitHub URL escaping from #418 are
unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@alecthomas
alecthomas marked this pull request as ready for review November 13, 2026 00:41
@alecthomas
alecthomas requested a review from a team as a code owner November 13, 2026 00:41
@alecthomas
alecthomas requested review from jrobotham-square and removed request for a team November 13, 2026 00:41
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 13, 2026 •

Copy link
Copy Markdown
Codex Review Summary: ✅ Completed

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-13T00:43:19.039469Z 02440aa Draft marked ready
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@alecthomas
alecthomas merged commit 3cc9a30 into main Nov 13, 2026
7 checks passed
@alecthomas
alecthomas deleted the aat/restore-system-tar branch November 13, 2026 00:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants