packaging: add portable content bundles - #574
Conversation
Add hash-addressed, inert content bundles stored under table/content/, with secure extraction, validation, consent, and LRU cache management. Bundles are deterministic: files are canonically ordered and hashed so identical trees deduplicate regardless of source location, timestamps, or platform. Writers reject absolute/unsafe paths and case collisions; readers verify every file's declared size and SHA-256 while streaming into an atomically published cache directory under persistentDataPath. Split-role IPackagedContentResolver: PackageWriter ingests directories via IPackagedContentSource at write time, while PackageReader and RuntimePackageReader inject a read-only resolver into IPackagedContentConsumer components after a table is restored. Cover round-tripping, malformed manifests, path traversal, and package limits with tests.
Greptile SummaryAdds deterministic, hash-addressed content bundles to
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| VisualPinball.Unity/VisualPinball.Unity/Packaging/Content/PackagedContentResolver.cs | Implements verified streaming extraction, heartbeat-protected temporary publication, and bounded cache cleanup; the previously reported live-extraction cleanup path is addressed. |
| VisualPinball.Unity/VisualPinball.Unity/Packaging/Content/PackagedContentWriter.cs | Implements deterministic directory preparation and verified package writing, with the remaining portable-filesystem TOCTOU limitation explicitly accepted by the author. |
| VisualPinball.Unity/VisualPinball.Unity/Packaging/Content/PackagedContentValidator.cs | Defines reference, manifest, package-integrity, and authoring-policy validation for version-one content bundles. |
| VisualPinball.Unity/VisualPinball.Unity.Editor/Packaging/PackageWriter.cs | Collects content sources before component serialization and writes prepared bundles in stable content-ID order. |
| VisualPinball.Unity/VisualPinball.Unity/Packaging/RuntimePackageReader.cs | Exposes and injects the read-only packaged-content resolver during runtime restoration. |
| VisualPinball.Unity/VisualPinball.Unity.Test/Packaging/PackagedContentTests.cs | Covers deterministic round trips, validation failures, path safety, cancellation, cleanup, and cache eviction. |
Sequence Diagram
sequenceDiagram
participant Source as Content Source
participant Writer as Package Writer
participant Package as .vpe Package
participant Reader as Package Reader
participant Resolver as Content Resolver
participant Cache as Verified Cache
Source->>Writer: PreparePackagedContent
Writer->>Writer: Hash and validate directory
Writer->>Package: Write manifest and payload
Reader->>Resolver: Inject read-only resolver
Resolver->>Package: Read manifest and files
Resolver->>Resolver: Verify paths, sizes, and hashes
Resolver->>Cache: Atomically publish completed bundle
Resolver-->>Reader: Return extracted directory
Reviews (3): Last reviewed commit: "packaging: heartbeat live extractions, b..." | Re-trigger Greptile
Address review feedback on the content bundle subsystem. Startup temp-directory cleanup only reaps orphans older than one hour. Resolvers share the default cache root, so a blanket wipe of *.tmp-* could delete the extraction another concurrently constructed resolver is still writing. An in-progress extraction keeps its directory young, so an age guard reaps crashed-run orphans without touching live work. Re-check for a reparse point before measuring and hashing each source file. Enumeration skips links, but a source tree mutated mid-export could swap a regular file for a symlink pointing outside sourceRoot before it is hashed; the captured hash then also guards the later write.
| // before it is measured and hashed, which would otherwise package external bytes. The | ||
| // hash captured here also guards the later write: bytes that change afterwards fail the | ||
| // verification in Write(). | ||
| if (IsLink(file.FullPath)) { |
There was a problem hiding this comment.
Link check leaves containment race
If another process can mutate the selected source tree, replacing an enumerated file with a symbolic link after IsLink returns but before hashing causes the writer to hash and later package bytes outside sourceRoot; leaving the link stable makes write-time hash verification accept the external payload. How this was verified: the pathname is independently reopened for hashing and writing after the sole reparse-point check, with no no-follow handle or stable identity check.
There was a problem hiding this comment.
Hardened in 0cc1092. The measure-and-hash read is now bracketed by reparse-point checks on both sides (no longer a sole check), and size + hash come from a single handle so the path is resolved once. A link seen at either boundary is rejected before any bytes are packaged. The only residual is the sub-syscall window inside the read itself, which can't be closed without no-follow open semantics that netstandard2.1 doesn't expose; the writer only ever ingests the author's own local tree at editor export time, so this is outside the design's trust boundary (extraction from untrusted packages is the defended side).
Follow-up to review on the content bundle subsystem. An extraction that spends a long time writing a single large file never creates another directory entry, so its temp directory's write time would go stale and a concurrently constructed resolver could reap it mid-write. The extraction now refreshes its temp directory's write time every few minutes, keeping it well under the one-hour stale cutoff. Bracket the source-file measure-and-hash with reparse-point checks on both sides instead of one, and read size and hash from a single handle so the path is resolved once. A link present at either boundary is rejected before its bytes are packaged. The residual is the sub-syscall window inside the read, which cannot be closed without no-follow open semantics that netstandard2.1 does not expose; the writer operates on the author's own local tree at export time.
Summary
Adds hash-addressed, inert content bundles to the
.vpepackage format, stored undertable/content/<content-id>/. This is the packaging foundation that consumer components (e.g. a web-show host or an MPF machine directory) build on to ship a directory tree inside a table and resolve it back out at runtime, without VPE ever executing the payload.This is the packaged-content subsystem in isolation. It was originally developed together with the Light Show Studio work; this branch carries only the portable-content half so it can be reviewed and merged on its own.
What it adds
Runtime (
VisualPinball.Unity/Packaging/Content/)PackagedContent.cs—PackagedContentRefand the split-roleIPackagedContentResolver, plusIPackagedContentSource(contributes a directory at write time) andIPackagedContentConsumer(receives a read-only resolver after restore).PackagedContentWriter.cs— deterministic directory ingestion: canonical ordering, per-file SHA-256, case-collision and unsafe-path rejection, overflow-checked totals, verify-on-write.PackagedContentResolver.cs— streaming extraction intopersistentDataPath/ContentCache/<hash>/, per-file size + hash verification, atomic publish via temp-dir rename with a.completemarker, and an LRU cache cap (4 GiB default).PackagedContentValidator.cs— normativeValidatePackage()integrity gate plus authoringLintManifest()(entry-point policy, forbidden-executable policy, duplicate-payload warnings).PackagedContentPath.cs— path-traversal-safe relative-path validation and containment checks.PackagedContentConsent.cs— per-content-hash user consent, keyed so changed content re-prompts.Editor / packaging wiring
PackageWriterimplementsIPackagedContentResolver, drivesIPackagedContentSourcecomponents, and writes bundles.PackageReader/RuntimePackageReaderexpose a read-onlyContentResolverand inject it intoIPackagedContentConsumercomponents after a table is restored.PackageApigains thecontentfolder constant;PackagedContentRefDrawerrenders the ref in the inspector.Docs & tests
FORMAT.md/README.mdspecify the normativevpe-contentv1 layout, canonical hashing, extraction rules, and an author checklist.PackagedContentTestscover round-tripping through both the editor and runtime resolvers, malformed manifests, path traversal, and package limits.Format
Bundles are byte-deterministic:
<content-id>is the first 16 hex chars of the canonical SHA-256 over each file's ordinal path + NUL + raw file hash + LF, so identical trees deduplicate independent of source path, timestamps, or platform. Readers accept exactlyvpe-contentversion 1 and reject unknown versions before extraction.Notes for review
content/folder is only written when a component implementsIPackagedContentSource, and only read when one implementsIPackagedContentConsumer.PackageWriter.cshunks were 3-way merged onto currentmaster(the file has diverged since the original commit); all other files applied cleanly.