Skip to content

fix(atelet): prevent path traversal in OCI tar extraction#101

Open
Kyosuke Konishi (konippi) wants to merge 1 commit into
agent-substrate:mainfrom
konippi:fix/atelet-untar-path-traversal
Open

fix(atelet): prevent path traversal in OCI tar extraction#101
Kyosuke Konishi (konippi) wants to merge 1 commit into
agent-substrate:mainfrom
konippi:fix/atelet-untar-path-traversal

Conversation

@konippi
Copy link
Copy Markdown

Resolves the three TODO comments in cmd/atelet/oci.go that called for a constrained filesystem to prevent path traversal, symlink escape, and hardlink escape during OCI tar extraction.

Uses os.Root (Go 1.24+) to confine all file operations to the rootfs directory. Adds validateTarName() as defence in depth.

  • Tests pass
  • Appropriate changes to documentation are included in the PR

@konippi
Copy link
Copy Markdown
Author

e2e-test failure is unrelated to this change — TestDemo3 timed out waiting for ResumeGoldenActor (gVisor restore) after 90s. The atelet image built and rolled out successfully; run-tests (unit tests + verify) passed. This looks like a flaky timeout in the kind CI environment. Happy to rebase and re-run if needed.

@BenTheElder
Copy link
Copy Markdown
Collaborator

Hmm, I haven't seen this flake yet and we've tested quite a few PRs in this environment. Will rerun it.

@BenTheElder
Copy link
Copy Markdown
Collaborator

These tests are also possible to run locally.

@BenTheElder
Copy link
Copy Markdown
Collaborator

The repeated e2e seems likely to be related, flakes have not been observed with kind on other PRs and it has failed twice in a row here. You can test the counter demo locally by following the README.

@konippi Kyosuke Konishi (konippi) force-pushed the fix/atelet-untar-path-traversal branch from 452278e to 927b237 Compare May 28, 2026 07:41
@konippi Kyosuke Konishi (konippi) force-pushed the fix/atelet-untar-path-traversal branch from 927b237 to 3b201ee Compare May 28, 2026 07:49
@konippi
Copy link
Copy Markdown
Author

Kyosuke Konishi (konippi) commented May 28, 2026

Benjamin Elder (@BenTheElder)
Thanks for the re-run and the nudge to test locally — I was able to reproduce and found the root cause.

mutate.Extract produces entries with absolute paths (e.g. /ko-app/counter), and my validateTarName was rejecting them via filepath.IsLocal. The old code handled this implicitly through filepath.Join. Fixed by stripping the leading / before validation. Verified locally on kind.

@a4-a4s1
Copy link
Copy Markdown

a4-a4s1 Bot commented May 28, 2026

Heads-up on a parallel PR that touches the same file: #96 (WIP, dims) modifies cmd/atelet/oci.go to add gpu *ateletpb.GpuSpec to prepareOCIDirectory's signature and adds a ~168-line GPU/CUDA helper block after untar. It doesn't touch untar itself, but the signature change will likely conflict with the resolve here.

Two observations for the maintainers:

Q: any preference on merge order? Landing this PR first anchors the constrained-filesystem invariant for #96 to build on; the reverse means re-porting os.Root semantics into the GPU-aware code path. Both are dims's-queue calls, but flagging in case the overlap wasn't already on radar.

[🤖a4s1]

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