-
Notifications
You must be signed in to change notification settings - Fork 10
fix(windows): fix Expand-Archive Windows Defender lock bug via native tar #131
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
PunchingElm
wants to merge
2
commits into
supermodeltools:main
from
PunchingElm:fix/windows-npm-install-lock
+188
−37
Closed
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,133 @@ | ||
| // Tests for the Windows zip extraction logic in install.js. | ||
| // Uses Node's built-in test runner (node:test, available since Node 18). | ||
|
|
||
| "use strict"; | ||
|
|
||
| const { test } = require("node:test"); | ||
| const assert = require("node:assert/strict"); | ||
| const fs = require("fs"); | ||
| const os = require("os"); | ||
| const path = require("path"); | ||
| const { execSync } = require("child_process"); | ||
| const { extractZip } = require("./install"); | ||
|
|
||
| // createTestZip builds a real .zip containing a single file named "supermodel.exe" | ||
| // using the system zip/tar command. Skips on platforms where neither is available. | ||
| function createTestZip(t) { | ||
| const src = fs.mkdtempSync(path.join(os.tmpdir(), "supermodel-test-src-")); | ||
| const binary = path.join(src, "supermodel.exe"); | ||
| fs.writeFileSync(binary, "fake binary"); | ||
|
|
||
| const archive = path.join(os.tmpdir(), `supermodel-test-${process.pid}.zip`); | ||
| try { | ||
| // Use system zip or tar to build the archive. | ||
| try { | ||
| execSync(`zip -j "${archive}" "${binary}"`, { stdio: "pipe" }); | ||
| } catch { | ||
| execSync(`tar -cf "${archive}" --format=zip -C "${src}" supermodel.exe`, { stdio: "pipe" }); | ||
| } | ||
| } catch { | ||
| fs.rmSync(src, { recursive: true, force: true }); | ||
| return null; // zip tooling not available — caller should skip | ||
| } | ||
| fs.rmSync(src, { recursive: true, force: true }); | ||
| return archive; | ||
| } | ||
|
|
||
| test("extractZip extracts via tar when tar succeeds", () => { | ||
| const archive = createTestZip(); | ||
| if (!archive) { | ||
| // Skip gracefully if zip tooling unavailable (e.g. minimal CI image). | ||
| console.log(" skipped: zip tooling not available"); | ||
| return; | ||
| } | ||
| try { | ||
| const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "supermodel-test-out-")); | ||
| try { | ||
| let called = null; | ||
| extractZip(archive, tmpDir, (cmd) => { | ||
| called = cmd; | ||
| // Only simulate tar; let the actual extraction happen via real execSync | ||
| // if this is the tar command. | ||
| if (cmd.startsWith("tar")) { | ||
| execSync(cmd, { stdio: "pipe" }); | ||
| } else { | ||
| throw new Error("should not reach PowerShell"); | ||
| } | ||
| }); | ||
| assert.ok(called.startsWith("tar"), "should have called tar first"); | ||
| const extracted = fs.readdirSync(tmpDir); | ||
| assert.ok(extracted.length > 0, "tmpDir should contain extracted files"); | ||
| } finally { | ||
| fs.rmSync(tmpDir, { recursive: true, force: true }); | ||
| } | ||
| } finally { | ||
| fs.unlinkSync(archive); | ||
| } | ||
| }); | ||
|
|
||
| test("extractZip falls back to PowerShell when tar fails", () => { | ||
| const commands = []; | ||
| // Simulate tar failing; PowerShell "succeeds" (no-op). | ||
| extractZip("/fake/archive.zip", "/fake/tmpdir", (cmd) => { | ||
| commands.push(cmd); | ||
| if (cmd.startsWith("tar")) throw new Error("tar not available"); | ||
| // PowerShell call — just record it, don't execute. | ||
| }); | ||
|
|
||
| assert.equal(commands.length, 2, "should have attempted tar then PowerShell"); | ||
| assert.ok(commands[0].startsWith("tar"), "first call should be tar"); | ||
| assert.ok(commands[1].includes("powershell"), "second call should be PowerShell"); | ||
| assert.ok(commands[1].includes("Expand-Archive"), "PowerShell command should use Expand-Archive"); | ||
| }); | ||
|
|
||
| test("extractZip PowerShell fallback includes retry loop", () => { | ||
| const commands = []; | ||
| extractZip("/fake/archive.zip", "/fake/tmpdir", (cmd) => { | ||
| commands.push(cmd); | ||
| if (cmd.startsWith("tar")) throw new Error("tar not available"); | ||
| }); | ||
|
|
||
| const psCmd = commands.find((c) => c.includes("powershell")); | ||
| assert.ok(psCmd, "PowerShell command should be present"); | ||
| assert.ok(psCmd.includes("$RetryCount"), "should include retry counter"); | ||
| assert.ok(psCmd.includes("Start-Sleep"), "should include sleep between retries"); | ||
| assert.ok(psCmd.includes("-lt 10"), "should retry up to 10 times"); | ||
| }); | ||
|
|
||
| test("extractZip uses tar when both succeed — tar wins", () => { | ||
| const commands = []; | ||
| extractZip("/fake/archive.zip", "/fake/tmpdir", (cmd) => { | ||
| commands.push(cmd); | ||
| // Both would succeed; tar is tried first and doesn't throw. | ||
| }); | ||
|
|
||
| assert.equal(commands.length, 1, "should only call tar when it succeeds"); | ||
| assert.ok(commands[0].startsWith("tar"), "the single call should be tar"); | ||
| }); | ||
|
|
||
| test("extractZip passes archive and tmpDir paths into tar command", () => { | ||
| const archive = "/tmp/test.zip"; | ||
| const tmpDir = "/tmp/extract-dir"; | ||
| let tarCmd = null; | ||
| extractZip(archive, tmpDir, (cmd) => { | ||
| tarCmd = cmd; | ||
| }); | ||
|
|
||
| assert.ok(tarCmd.includes(archive), "tar command should include archive path"); | ||
| assert.ok(tarCmd.includes(tmpDir), "tar command should include tmpDir path"); | ||
| }); | ||
|
|
||
| test("extractZip passes archive and tmpDir paths into PowerShell fallback", () => { | ||
| const archive = "/tmp/test.zip"; | ||
| const tmpDir = "/tmp/extract-dir"; | ||
| const commands = []; | ||
| extractZip(archive, tmpDir, (cmd) => { | ||
| commands.push(cmd); | ||
| if (cmd.startsWith("tar")) throw new Error("tar failed"); | ||
| }); | ||
|
|
||
| const psCmd = commands[1]; | ||
| assert.ok(psCmd.includes(archive), "PowerShell command should include archive path"); | ||
| assert.ok(psCmd.includes(tmpDir), "PowerShell command should include tmpDir path"); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🌐 Web query:
In PowerShell, how should apostrophes be escaped inside single-quoted string literals, and is-LiteralPathpreferred for paths passed toExpand-Archive?💡 Result:
In PowerShell, apostrophes (single quotes) inside single-quoted string literals are escaped by doubling them: use two consecutive single quotes (''') to represent a literal apostrophe. The backtick () escape character does not work in single-quoted strings, as they are treated literally with no substitutions or escapes except for the doubled single quote. For paths passed to Expand-Archive, -LiteralPath is preferred when the path contains special characters like [ or ] that could be interpreted as wildcards by the -Path parameter. -LiteralPath treats the path exactly as typed, without wildcard expansion. Official examples recommend -LiteralPath for such cases, e.g., Expand-Archive -LiteralPath 'C:\Archives\Draft[v1].zip' -DestinationPath C:\Reference.
Citations:
🏁 Script executed:
Repository: supermodeltools/cli
Length of output: 1852
Escape apostrophes in paths before passing to PowerShell.
Paths like
C:\Users\O'Brien\Downloads\archive.zipwould break the current code. When that path hits the single-quoted string (e.g.,'-Path '${archive}'), the apostrophe closes the string early, and PowerShell fails to expand the archive.The fix is straightforward: double any apostrophes before interpolating (PowerShell treats
''as a literal apostrophe in single-quoted strings). Also switch to-LiteralPathsince it's designed exactly for this—it treats the path literally without trying to interpret special characters like brackets as wildcards.Suggested fix
function extractZip(archive, tmpDir, execFn) { const exec = execFn || execSync; + const psEscape = (value) => String(value).replace(/'/g, "''"); try { exec(`tar -xf "${archive}" -C "${tmpDir}"`); } catch { + const archivePs = psEscape(archive); + const tmpDirPs = psEscape(tmpDir); const psCommand = `$RetryCount = 0; while ($RetryCount -lt 10) { try { Expand-Archive` + - ` -Force -Path '${archive}' -DestinationPath '${tmpDir}'; break }` + + ` -Force -LiteralPath '${archivePs}' -DestinationPath '${tmpDirPs}' -ErrorAction Stop; break }` + ` catch { Start-Sleep -Seconds 1; $RetryCount++ } }`; exec(`powershell -NoProfile -Command "${psCommand}"`); } }📝 Committable suggestion
🤖 Prompt for AI Agents