Skip to content

feat(rt): add os/unzip - #688

Merged
nnunley merged 1 commit into
nooga:mainfrom
abogoyavlensky:feat-os-unzip
Aug 7, 2026
Merged

feat(rt): add os/unzip#688
nnunley merged 1 commit into
nooga:mainfrom
abogoyavlensky:feat-os-unzip

Conversation

@abogoyavlensky

Copy link
Copy Markdown
Contributor

Resolves: #687

(os/unzip zip-path dest-dir) extracts a zip archive into dest-dir and returns it. Motivation: dependency tooling built on let-go currently shells out to unzip to expand jars, and this removes that host-binary requirement.

Extraction is confined by os.Root, which opens each path component under the destination and refuses any that escapes. That covers both the lexical case (../evil.txt) and the symbolic one (link/x where dest/link already points elsewhere), and unlike a check-then-write guard it leaves no window for a process sharing the destination to swap a validated directory for a symlink after the check. Absolute entry names are refused rather than silently re-rooted.

Safety over fidelity otherwise: symlink, device, fifo and socket entries are skipped rather than recreated. Permissions follow the unzip(1) contract — as recorded, masked by the umask — with a 0644 fallback for DOS/FAT entries that record no unix mode. A failed extraction is never worse than no extraction: a directory entry colliding with an existing file is an error rather than a swallowed EEXIST, and each entry is opened before its destination is cleared.

Tests: pkg/rt/os_unzip_test.go builds archives programmatically, including the adversarial ones, plus a test/os_unzip_test.lg smoke test.

(os/unzip zip-path dest-dir) extracts a zip archive into dest-dir and
returns dest-dir. The destination is created if missing; existing files
are overwritten.

Motivation: dependency tooling built on let-go currently shells out to
`unzip` to expand jars. A native extractor removes that host-binary
requirement.

Extraction is confined by os.Root, which opens each path component under
the destination and refuses any that escapes. That covers both the
lexical case ("../evil.txt") and the symbolic one ("link/x" where
dest/link already points elsewhere), and unlike a check-then-write guard
it leaves no window for a process sharing the destination to swap a
validated directory for a symlink after the check. Absolute entry names
are refused outright rather than silently re-rooted inside dest.

Safety is favoured over fidelity besides, since the archives in the
motivating use case arrive over the network: symlink entries are skipped
rather than recreated, as are devices, fifos and sockets. A non-regular
file already sitting at a target path is removed rather than written
through.

Permissions follow the unzip(1) contract — as recorded in the entry,
masked by the process umask. Entries written by DOS/FAT-style tools
record no unix mode and archive/zip synthesizes 0666 for them, so those
fall back to 0644 rather than landing world-writable. Directory entries
keep their recorded mode with owner rwx forced on, so a read-only
directory listed before its contents cannot make the rest of the archive
unextractable.

Two failure modes are handled so that a failed extraction is never worse
than no extraction: a directory entry colliding with an existing regular
file is an error rather than a silently swallowed EEXIST, and each entry
is opened before its destination is cleared, so an unreadable entry
cannot destroy the file it was meant to replace.

Covered by pkg/rt/os_unzip_test.go (archives built programmatically,
including the adversarial ones) plus a test/os_unzip_test.lg smoke test
whose fixture is embedded as base64 to keep test/ free of binary blobs.

@mparrett mparrett left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving. Built it and ran the suite: 22 unzip subtests pass, and it builds for js/wasm, plan9/amd64, linux/amd64, and wasip1/wasm. The windows/amd64 failure is pre-existing in term.go and reproduces on the merge base.

Routing every write through an os.Root confined to the destination closes the check-then-write race that a lexical containment guard cannot, and the tests cover zip-slip, absolute entry names, and escape through a pre-existing symlink. Stripping setuid via .Perm() and separating a recorded mode from archive/zip's synthesized 0666 are both easy to miss.

No findings.

Not a change request: io.Copy is unbounded, so a crafted archive can expand without limit. That matches io/slurp and http/get, and there is no in-tree caller yet, so it isn't a defect here. It becomes worth revisiting when something starts unzipping artifacts fetched over the network, where an optional caller-supplied byte limit — the shape net/read! already uses — would fit better than a hidden default.

@mparrett mparrett added the perf Run the perf A/B benchmark check label Aug 7, 2026
@nnunley
nnunley merged commit 40ab67a into nooga:main Aug 7, 2026
24 checks passed
@mparrett

mparrett commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator

Now these have landed: between them they cover half of what let-go needs for Grenadine's host contract.

Checked all 17 slots against lg on current main. Most already map onto os, io, http, and edn: :http-get, :read-edn, :getenv, :home-dir, :run-process, :mkdirs, :exists, :find-files, :read-bytes / :write-bytes, plus the os/stat fields behind :directory and :regular-file. With hash/sha1 and os/unzip merged, that leaves:

  • :atomic-move — no rename in os or clojure.core
  • :delete-treedelete-file only, so no recursive form
  • :canonical-path / :absolute-path — no path or filepath namespace
  • :bytes->utf8 / :byte-count — depends on how bytes get represented, so more a design question than a missing function

Are you working on any of these? If not I'd pick up the first two, which look like small additions in the same shape as os/unzip. Asking first since #690 and #688 landing together read like a batch, and you may have these queued already.

The complete contract is src/grenadine/host/jvm.clj in clojurestar/grenadine, with glojure.clj beside it as the non-JVM implementation. let-go already passes Grenadine's portable resolver suite (52 tests, 180 assertions); the effect host is what's missing.

@abogoyavlensky

Copy link
Copy Markdown
Contributor Author

@mparrett No, I'm not working on any of these, please, feel free to pick it up.

mparrett added a commit that referenced this pull request Aug 10, 2026
Grenadine's host contract needs an atomic move and a recursive delete, and
neither had a home. `delete-file` removes a single entry and fails on a
non-empty directory, so callers open-code a walk; nothing at all wrapped
rename(2), so publishing a file meant spit and a window where a reader
could see it half-written.

os/rename returns the destination and fails across filesystems rather than
falling back to copy-then-delete. That fallback is the thing callers reach
for this instead of, so substituting it silently would drop the only
property separating it from spit.

os/delete-tree removes a path and everything under it, and succeeds when
the path is already absent — the post-state asked for is the one that
holds. That diverges from delete-file, which throws, so it is pinned by a
test in both suites. An empty path is refused: os.RemoveAll treats "" as a
silent no-op, which hides the unset variable that produced it.

Both land in the os namespace rather than core, following os/unzip (#688).
Core is the always-loaded surface and there is active work to shrink it;
these are host effects and belong beside the other ones.

The TinyGo os namespace is a deliberate three-function subset and gains
neither, matching how os/unzip was added.

Requested in #688; abogoyavlensky confirmed they were not working on these.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
mparrett added a commit that referenced this pull request Aug 13, 2026
Grenadine's host contract needs an atomic move and a recursive delete, and
neither had a home. `delete-file` removes a single entry and fails on a
non-empty directory, so callers open-code a walk; nothing at all wrapped
rename(2), so publishing a file meant spit and a window where a reader
could see it half-written.

os/rename returns the destination and fails across filesystems rather than
falling back to copy-then-delete. That fallback is the thing callers reach
for this instead of, so substituting it silently would drop the only
property separating it from spit.

os/delete-tree removes a path and everything under it, and succeeds when
the path is already absent — the post-state asked for is the one that
holds. That diverges from delete-file, which throws, so it is pinned by a
test in both suites. An empty path is refused: os.RemoveAll treats "" as a
silent no-op, which hides the unset variable that produced it.

Both land in the os namespace rather than core, following os/unzip (#688).
Core is the always-loaded surface and there is active work to shrink it;
these are host effects and belong beside the other ones.

The TinyGo os namespace is a deliberate three-function subset and gains
neither, matching how os/unzip was added.

Requested in #688; abogoyavlensky confirmed they were not working on these.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
mparrett added a commit that referenced this pull request Aug 14, 2026
Grenadine's host contract needs an atomic move and a recursive delete, and
neither had a home. `delete-file` removes a single entry and fails on a
non-empty directory, so callers open-code a walk; nothing at all wrapped
rename(2), so publishing a file meant spit and a window where a reader
could see it half-written.

os/rename returns the destination and fails across filesystems rather than
falling back to copy-then-delete. That fallback is the thing callers reach
for this instead of, so substituting it silently would drop the only
property separating it from spit.

os/delete-tree removes a path and everything under it, and succeeds when
the path is already absent — the post-state asked for is the one that
holds. That diverges from delete-file, which throws, so it is pinned by a
test in both suites. An empty path is refused: os.RemoveAll treats "" as a
silent no-op, which hides the unset variable that produced it.

Both land in the os namespace rather than core, following os/unzip (#688).
Core is the always-loaded surface and there is active work to shrink it;
these are host effects and belong beside the other ones.

The TinyGo os namespace is a deliberate three-function subset and gains
neither, matching how os/unzip was added.

Requested in #688; abogoyavlensky confirmed they were not working on these.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
mparrett added a commit that referenced this pull request Aug 16, 2026
Grenadine's host contract needs an atomic move and a recursive delete, and
neither had a home. `delete-file` removes a single entry and fails on a
non-empty directory, so callers open-code a walk; nothing at all wrapped
rename(2), so publishing a file meant spit and a window where a reader
could see it half-written.

os/rename returns the destination and fails across filesystems rather than
falling back to copy-then-delete. That fallback is the thing callers reach
for this instead of, so substituting it silently would drop the only
property separating it from spit.

os/delete-tree removes a path and everything under it, and succeeds when
the path is already absent — the post-state asked for is the one that
holds. That diverges from delete-file, which throws, so it is pinned by a
test in both suites. An empty path is refused: os.RemoveAll treats "" as a
silent no-op, which hides the unset variable that produced it.

Both land in the os namespace rather than core, following os/unzip (#688).
Core is the always-loaded surface and there is active work to shrink it;
these are host effects and belong beside the other ones.

The TinyGo os namespace is a deliberate three-function subset and gains
neither, matching how os/unzip was added.

Requested in #688; abogoyavlensky confirmed they were not working on these.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

perf Run the perf A/B benchmark check

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add native os/unzip for archive extraction

3 participants