Fail closed on invalid Linux release package roots - #53
Conversation
📝 WalkthroughWalkthroughLinux packaging now fails closed when run from a nested checkout, when ChangesLinux packaging validation
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant packageLinuxHost as package-linux-host.mjs
participant gitCheckout as Git checkout
participant sourceArchive as Source archive
packageLinuxHost->>gitCheckout: Verify repository root and HEAD version
packageLinuxHost->>sourceArchive: Create and extract archive
packageLinuxHost->>sourceArchive: Validate archive contents
packageLinuxHost->>packageLinuxHost: Continue or abort packaging
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/package-linux-host.mjs`:
- Around line 36-40: The Linux packaging validation must require complete
package.json equality with Git HEAD, not just matching version values. In
scripts/package-linux-host.mjs, replace or supplement the headVersion check with
a git diff --quiet HEAD -- package.json validation while preserving failure
handling. In test/connectors.mjs, add an executable fixture that modifies a
non-version package.json field and asserts packaging fails.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 00b0261a-067c-4796-98c6-d738aacaf0d8
📒 Files selected for processing (3)
CHANGELOG.mdscripts/package-linux-host.mjstest/connectors.mjs
| const headPackage = spawnSync("git", ["show", "HEAD:package.json"], { cwd: root, encoding: "utf8" }); | ||
| let headVersion = ""; | ||
| try { headVersion = String(JSON.parse(String(headPackage.stdout || "{}")).version || "").trim(); } catch { } | ||
| if (headPackage.status !== 0 || headVersion !== version) { | ||
| throw new Error("Linux packaging version does not match package.json at Git HEAD"); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Require and test complete package.json equality with HEAD. The implementation compares only version, and the test validates only that limited check. A same-version manifest edit can pass packaging.
scripts/package-linux-host.mjs#L36-L40: reject a working-treepackage.jsonthat differs fromHEAD, such as withgit diff --quiet HEAD -- package.json.test/connectors.mjs#L20-L22: add an executable fixture that changes a non-version manifest field and asserts that packaging fails.
📍 Affects 2 files
scripts/package-linux-host.mjs#L36-L40(this comment)test/connectors.mjs#L20-L22
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/package-linux-host.mjs` around lines 36 - 40, The Linux packaging
validation must require complete package.json equality with Git HEAD, not just
matching version values. In scripts/package-linux-host.mjs, replace or
supplement the headVersion check with a git diff --quiet HEAD -- package.json
validation while preserving failure handling. In test/connectors.mjs, add an
executable fixture that modifies a non-version package.json field and asserts
packaging fails.
Outcome
Prevents an extracted source copy nested below another Git checkout from silently producing a Linux archive containing only injected runtime assets. Packaging now requires its own exact Git root, matching
HEAD:package.json, a non-empty Git archive, and the staged versioned package contract.Verification
npm run typechecknpm run buildnpm test(116 tests, zero failures; two browser checks skipped without local Chrome)git diff --checkSummary by CodeRabbit
Bug Fixes
Documentation