Repository navigation
feat(runner-release): add bounded release transport and a strict package reader - #43
Merged
namikmesic merged 11 commits intoSep 30, 2026
Merged
Conversation
…t package reader Two shared Node modules under src/runner-release, importable by the app's main process, the Puck server and the runner (never the renderer or the daemon), with no consumer wired yet. download.ts is the transport. readBoundedBody reads a response's exact bytes under a bound checked against Content-Length before the read and the running count during it, and refuses a content encoding. verifySignedRunnerReleases decodes a listing's signed records (at most 16, each manifest at most 256 KiB once decoded, the listing at most 4 MiB) and hands every manifest's exact bytes to the verifier; nothing re-serialised is verified. downloadRunnerPackage streams a package to a private 0600 file: under the official policy the URL is built from the repository's release prefix, the canonical version and the signed file name, every redirect is checked by hand (at most five, https only, exactly github.com and release-assets.githubusercontent.com, no userinfo, fragment, port or control character, the github.com path compared segment by segment), redirect bodies are cancelled, the request carries no token, the signed byte count is enforced while streaming under a 256 MiB cap with a 30 s header timeout per hop and a 15 minute whole-body deadline, and a short, long, mistimed, cancelled or mis-hashed body cancels the response and removes the file. The development policy is the caller's: http on the configured server's origin, no redirect. archive.ts reads a package into a fresh 0700 staging directory, accepting only the packager's fixed layout (nine regular files and bin/, exact modes): ustar headers with verified checksums and octal fields, no links, devices, PAX or GNU entries, no absolute or traversing paths, no setuid, setgid or sticky bits, each member once, a two-block trailer and nothing after it, a 512 MiB expansion cap, and VERSION equal to the expected version. Failures remove staging. System tar is never run, and the lint boundary now allows the built-ins these modules need while keeping child_process out of the directory. The harness gains the pure shapes: the release tag, the one mapping from Node's platform names to the manifest's (the runner's currentPlatform now delegates to it), the package layout, and the signed record with strict canonical base64 and its bounds. The app's and the runner's release listing readers now read through the bounded transport; other server requests are unchanged.
…ger at the shared layout readBoundedBody now cancels the response when it refuses a content encoding or an unreadable Content-Length, as the rest of the transport does. The packager's header names RUNNER_PACKAGE_ENTRIES as the one place its entry list must match.
A file handle's write may persist only a prefix without throwing, so a download or an unpack could report a verified package while the file on disk was shorter than the bytes hashed. writeAll loops until every byte of a chunk is written and treats a write of nothing as write-failed, at both sites, with the existing cleanup.
zlib's gunzip joins concatenated members and ignores trailing NULs, and the tar reader only ever saw inflated bytes, so a package followed by an empty member, preceded by one, split across two members, or followed by a NUL and garbage all passed. The reader now frames the gzip member itself: a fixed ten-byte header with no optional fields, one raw deflate stream fed only the bytes before the eight-byte trailer and required to consume all of them, and a trailer whose CRC-32 and size must match what was inflated. The streaming inflation cap is unchanged. Tests cover each of those inputs, trailer and header mutations, and that staging is removed.
…ackage header uname, gname, devmajor and devminor (bytes 265 to 344) were skipped, so a header with bytes after an owner name's terminator or with 0xff in a device field passed once its checksum was corrected. The owner names are now read like every other text field, and the device numbers must be NUL, as the packager writes them. Tests mutate each of the four fields with a corrected checksum and assert rejection with empty staging.
…during download The download suite's short-write test claimed to cover unpacking but only called downloadRunnerPackage. The archive suite now injects a file handle that persists half a write, or nothing, and asserts RunnerArchiveError write-failed with the staging parent left empty; the download test's title says what it covers.
… work has settled On a stream failure the reader destroyed its three streams and rejected at once, so the staging directory was removed while an entry's open could still be in the threadpool; that create could land between the removal's readdir and rmdir, the removal would throw ENOTEMPTY (an error that is not a RunnerArchiveError) and the directory survived. The sink now tracks the chunk it is consuming, its destruction waits for that work and closes the file it left open, an open that completes after destruction is closed at once, the rejection waits for the sink's 'close', and the removal retries. A test fails the read stream the moment the first entry's open begins and asserts that open completed and closed before the caller heard of the failure, with staging gone.
Flipping a byte near the end of an archive whose runtime payload was random could end the raw deflate stream early and leave bytes before the trailer, which the reader correctly rejects as trailing-data, but the assertion accepted only corrupt or truncated, so the case failed intermittently. The payload is now fixed pseudo-random bytes from a seeded xorshift, and the assertion accepts each of the three correct rejections; staging must still be gone.
namikmesic
force-pushed
the
fm/runner-distribution-bounded-transport-an-92
branch
from
September 30, 2026 23:10
6d5c97a to
82e9ed5
Compare
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Intent
Add the bounded, authenticated-package transport and the strict release-archive reader that later runner distribution slices will use: a dedicated reader for signed release metadata that bounds every response before parsing, a streamed tarball downloader that constructs official URLs from the known release prefix, validates every redirect against an allowlist, forwards no credentials, and enforces the signed byte count and absolute size and time limits; and a strict reader for the fixed runner package layout that validates the archive entry by entry into private staging without activating anything. No consumer is wired yet.
What Changed
/v1/runner/releases.Risk Assessment
✅ Low: The new downloader and archive reader fail closed on redirects, size, digest, gzip framing, and tar layout, and the only live caller change is the bounded release-listing read.
Testing
I stood up the real development puck server and drove the release listing through the Electron app client and the runner client, including an oversized body, a still-readable unrelated body, and a gzip listing. I then ran the real downloader and archive reader against that server, a redirecting host, GitHub, a file-size limit, and a sandbox that blocks
bin/. Ten of those checks passed. The official redirect onto the asset host stayed untested because GitHub returned 404 with no Location.Evidence: Development server release listing
Evidence: Live scenario results
Evidence: Official download request
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
✅ **Review** - passed
✅ No issues found.
✅ **Test** - passed
✅ No issues found.
npm run build:server, thennode .webpack/server/puck-server.jswithPUCK_DEVELOPMENT=trueandPUCK_RUNNER_DOWNLOADSset to a9.9.9/puck-runner-linux-x64-9.9.9.tar.gzGET /v1/runner/releaseson that server, then the same listing read by the app'sreleases()inside Electron and by puck-runnerServerApi.releases()a local server returning 4194305 listing bytes, and a larger non-listing body read byserverRequestandServerApi.registera gzip-encoded listing read by bothreleases()clientsdownloadRunnerPackageunder the development policy against the development server, a 302, a foreign origin, a URL with userinfo, and aContent-Encoding: gzipresponsedownloadRunnerPackageunder the official policy againsthttps://github.com/namikmesic/puck/releases/download/v0.1.0/puck-runner-macos-arm64-0.1.0.tar.gz, with outgoing headers taken from the undici requestulimit -f 4around an 8192-bytedownloadRunnerPackageandunpackRunnerPackageunpackRunnerPackageon a layout-matching archive, an archive with an extra entry, a version mismatch, and asandbox-execprofile that denies creatingbin/✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.