Conversation
The archive allowlist accepted any path under a runtime's licenses/ directory, at any depth, with nothing tying those bytes to what the producer actually shipped. Narrow it to what upstream declares. The tar listing now accepts only a flat, safely named licenses/<name>, since validateArchiveEntries runs before extraction and cannot consult the runtime manifest. After extraction, every runtime file outside lib/, tools/, manifest.json, and README.md must appear in the runtime manifest's runtime.files map with a matching SHA-256. That check runs after the manifest digest is bound to product-manifest.json, so the declarations come from a manifest the product already vouches for. Runtimes that ship no extra files make no declaration and are left alone, so bundles predating license bundling still verify. Traversal, entry-type, checksum, tree-digest, and single-runtime checks are unchanged.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe change restricts native runtime license paths and verifies all additional extracted runtime files against declarations and SHA-256 digests in the runtime manifest. Tests cover valid, invalid, mismatched, and file-free runtimes. ChangesRuntime archive validation
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Sequence Diagram(s)sequenceDiagram
participant ComponentA
participant ComponentB
ComponentA->>ComponentB: observable interaction
Merge Risk: ⚪ Minimal · up to The archive validation changes do not show an actionable correctness or compatibility risk and are ready to merge. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 18.18% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 2 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
1 participant
Why
fb4e29bunblocked CUDA archive extraction by adding!relative.startsWith("licenses/")to the runtime entry allowlist. That accepts any path underlicenses/, at any depth, with nothing tying those bytes to what upstream actually shipped. An archive could carry arbitrary extra files there and still pass every other check.Upstream already declares this material.
package-native-runtime.shcopies the CUDA EULA tolicenses/NVIDIA-CUDA-LICENSE.txtand records it in the runtime manifest'sruntime.filesmap next to its SHA-256, alongside the libraries. So we can key the allowlist off the producer's own declaration instead of a prefix wildcard.What changed
Two phases, because
validateArchiveEntriesruns on the tar listing before anything is extracted and cannot read the manifest:licenses/<name>. No subdirectories, same character class the runtime id already uses.verifyDeclaredRuntimeFiles) requires every runtime file outsidelib/,tools/,manifest.json, andREADME.mdto appear inruntime.fileswith a matching SHA-256.The second gate runs after the runtime manifest digest is checked against
product-manifest.json, so the declarations come from a manifest the product already vouches for.This is the first thing in mesh-packaging that reads the runtime manifest's contents. Until now it was existence-checked and hashed, never parsed.
Compatibility
A runtime that ships no files outside the structural set makes no declaration and is never asked for one, so bundles predating license bundling (v0.76.2 and earlier) verify exactly as before. The test fixture keeps its
{}manifest to hold that path.Traversal, entry-type, checksum, tree-digest, and single-runtime checks are untouched.
QA
tests/upstream-archive.test.ts: 13/13. New coverage for the flat-and-safe listing gate (nested/EULA.txt,.hidden,-leading,spaced name.txtall rejected), a declared license accepted end-to-end throughverifyAndExtract, an undeclared file rejected, a digest-drifted file rejected, and the no-extra-files path.scripts/image-matrix.ts validate: 9 packaging variants, 5 npm lanes.tests/workflow-provenance.test.tsand reproduce identically on unmodifiedorigin/mainon macOS; the precheck lane runsubuntu-24.04, so I expect them green in CI. Flagging rather than fixing here.Summary by CodeRabbit
Security
licenses/.Compatibility