Extract Electron ASAR archives - #201
davidnewhall wants to merge 4 commits into
Conversation
Use golift.io/asar for the pickle/JSON index, then mkdir and create in-tree symlinks sequentially before copying packed and unpacked files. FileWorkers greater than 1 shares dispatchWorkers with ZIP and 7z. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Unpacked inputs can escape through symlinks, and DeleteOrig leaves the consumed sidecar directory behind.
Get a fresh assessment by requesting another Copilot review.
Review effort: Balanced
Findings: 1
Open (2)
What changed in this PR
Adds extraction support for Electron ASAR archives, including packed files, unpacked siblings, symlinks, and parallel workers.
Changes:
- Registers
.asararchives and adds the ASAR dependency. - Implements secure output paths, progress tracking, and parallel extraction.
- Adds ASAR extraction tests.
| File | Description |
|---|---|
asar.go |
Implements ASAR extraction. |
asar_test.go |
Tests packed, unpacked, symlink, and parallel cases. |
files.go |
Registers .asar support. |
start.go |
Documents ASAR worker support. |
go.mod, go.sum |
Adds the ASAR dependency. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| if !pathWithin(root, srcPath) { | ||
| return nil, fmt.Errorf("%s: %w: %s", name, ErrInvalidPath, srcPath) | ||
| } | ||
|
|
||
| src, err := os.Open(srcPath) |
| // The ones with double extensions that match a single (below) need to come first. | ||
| {Type: "7zip", Ext: ".7z", Fn: Extract7z}, | ||
| {Type: "7zip", Ext: ".7z.001", Fn: Extract7z}, | ||
| {Type: "asar", Ext: ".asar", Fn: ChngInt(ExtractASAR)}, |
![qwen-pr-bot[bot]](https://avatars.githubusercontent.com/in/4697433?s=60&v=4){kind=small}
| return nil, fmt.Errorf("%s: %w: %s", name, ErrInvalidPath, srcPath) | ||
| } | ||
|
|
||
| src, err := os.Open(srcPath) |
There was a problem hiding this comment.
Small FYI, not a blocker: this os.Open follows a symlink planted inside the {archive}.asar.unpacked sibling. I reproduced it — pointing an unpacked entry at a file outside the unpacked dir copies that file's bytes into the output. Not exploitable in practice: whoever ships the archive owns that directory and can already read whatever they link, and extraction never writes through the link, so there's no traversal or overwrite. It's just the one place in an otherwise symlink-paranoid codebase that reads through a link. A one-line comment noting the assumption (or routing this through the no-follow open helper like the write path does) would keep the posture consistent.
There was a problem hiding this comment.
Approving — first look, head b763900 vs base 8cc8e92a.
What this PR actually contains (three distinct changes, not just "ASAR support"):
.asarregistered in the extension table and routed to a newExtractASAR: dirs and package-relative symlinks first, then packed and unpacked file copies, parallel via the existingdispatchWorkerspool whenFileWorkers > 1.- A bump of
golift/asarso unpacked entries that also carry anoffsetare read from the{archive}.asar.unpackedsibling instead of being treated as packed archive bytes (the parser now checksunpackedbeforeoffset). - Doc-comment fixes in
files.go/start.golisting ASAR as a random-access format alongside ZIP and 7z.
I traced the data flow end to end rather than relying on the diff alone. asarProgress (header-declared totals, archive-file compressed size, entry count) feeds checkClaimedLimits, so a malicious header still fails the MaxBytes/MaxFiles/MaxRatio fast-fail; the runtime write path re-checks per byte. asarPrepareEntries does containment + mkdir + symlink before any writes, and both the entry-name and symlink-target containment checks engage after the asar library has already rejected any entry name containing /, \, . or .., so traversal isn't reachable through names at all. createASARSymlink computes a relative target from the symlink's own directory and createSymlink re-validates it lexically and by resolving planted links. For concurrency, File.Open() hands each worker its own SectionReader over the shared ReaderAt, so shared-offset entries (a.txt/b.txt both at offset 8) copy intact — that's the exact case the parallel test exercises. ExtractASAR follows the same shape as ExtractZIP, so the tracker/done()/error handling is house-standard, not a copy-paste hazard.
Executed validation at head b763900:
go test -race -run ASAR ./...→ okgo test -race -covermode=atomic ./...→ ok, 142s, 76.2% coverage (mirrors the CI gotest job)go vet ./...→ clean;golangci-lint run ./...(v2.13.0, repo config) → 0 issuesgo build ./...,go mod tidy→ clean, no changes wanted- Focused experiments for paths the tests skip: a nested symlink target (
dir/nested.txt) and an absolute one (/bin) both neutralize to relative links inside the output dir; the one genuine gap is the read through a link in the.unpackedsibling (inline note, non-blocking). - CI on this exact commit is green: golangci-lint (linux/windows/freebsd/darwin),
go test -race(ubuntu/macos/windows), Snyk.
One non-blocking note inline; nothing blocks approval.
…ith the archive.
os.Open followed links and special files out of {archive}.unpacked, and DeleteOrig only removed the archive path.
Co-authored-by: Cursor <cursoragent@cursor.com>
Summary
.asarand extract throughExtractASAR: directories and package-relative symlinks first, then packed/unpacked file copies.dispatchWorkerspool whenFileWorkers > 1. Unpacked members come from{archive}.asar.unpacked; a missing sibling is an error that names the entry.Test plan
go test -race -run ASAR ./...FileWorkers0/1) and parallel (FileWorkers4) both write shared-offset files intactErrInvalidPath{archive}.asar.unpackedmember fails without writing an empty stand-inMade with Cursor