Repository navigation
fix(renderer): make copied runner install commands fail closed - #39
Merged
Merged
Conversation
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
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
Make the Add runner panel's copied installation commands safe. Today the Download block interpolates the server URL, file name, checksum and token without shell quoting, and a failed checksum does not stop the following tar, so a hostile value runs as shell and a bad download still extracts. Generate a self-contained fail-closed block instead: every dynamic value quoted for a POSIX shell and rejected if it carries control characters, prerequisites checked first, the download and checksum done in a private staging directory next to the destination with the archive extracted there, and the finished directory renamed into a previously absent
puck-runnerdirectory only after every step succeeds, with staging removed on any failure. Usesha256sumon Linux andshasum -a 256on macOS. The Configure and Run blocks each enter the completedpuck-runnerdirectory in a guarded subshell and verify the installation is complete before running, instead of assuming the earlier block changed the caller's directory. This works with today's development packages and claims no new authenticity.What Changed
curl,tar, andsha256sum(Linux) orshasum(macOS) are present before downloading.sha256sumorshasum -a 256, and extracted in a private staging directory next to the destination. It is renamed topuck-runneronly after the shipped files and version match, and staging is removed if any step fails../puck-runnerhas the expected version and an executableconfig.sh, then enter that directory in a subshell. When a package or registration value is not safe to paste, the panel shows an error and no commands.Risk Assessment
✅ Low: The copied install commands are a bounded fail-closed rewrite: dynamic values are POSIX-quoted and control-character rejected, checksum failure stops before extract, and staging is published to an absent puck-runner only after the checks succeed.
Testing
I generated the Add runner copy-paste blocks with the panel's command generator and ran them in /bin/sh against a local server and a tarball that uses the development package's config.sh, run.sh, and svc.sh. On macOS the Download block published a private puck-runner directory only after shasum -a 256 checked out, left the caller's directory unchanged, and removed staging when the checksum, the download, the redirect, or the package was bad. Configure and Run entered that directory in a subshell and refused an incomplete install; quotes and command substitutions stayed literal. The Linux block does call sha256sum, but this Mac's sha256sum is Darwin 1.0 and rejects the GNU --status flag, so a successful Linux checksum was not observed. The Add runner panel was not opened, because it requires a signed-in Puck session, so there is no screenshot of the settings UI.
Evidence: Live shell transcript of the copied install blocks
Evidence: macOS Download block that was executed
Evidence: config.sh received the hostile URL and token as single arguments inside puck-runner
cwd=.../puck-runner arg=.../bin/puck-runner.cjs arg=config arg=--url arg=http://127.0.0.1:54703/a';touch arg=--token arg=PRT_' "; touch .../INJECTED-ce741ceb; # PUCK_RUNNER_INSTALLEvidence: Configure and Run, including the Linux sudo attempt
configure, run.sh, and svc.sh ran from puck-runner via the development shell scripts. linux sudo attempt stderr: sudo: a terminal is required to read the password; either use the -S option to read from standard input or configure an askpass helper sudo: a password is requiredEvidence: Linux block invoked Darwin sha256sum, which rejected --status, and published nothing
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.
node --experimental-strip-types --disable-warning=MODULE_TYPELESS_PACKAGE_JSON ~/.no-mistakes/evidence/01M3SMZMNR4YN7MJEHQ82YK9H0/drive-runner-commands.mjs✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.