fix(lib): pin TLS on vendor installer fetches; drop apt-key - #47
Merged
Conversation
Found in a security audit of the finished work. install.sh verifies
install-hooks.sh before executing it — and install-hooks.sh then downloaded six
files from resq-software/crates@master, a mutable branch, with no verification,
chmod +x'd them, and pointed core.hooksPath at them. Those run on every commit
and push. The chain of custody terminated one link too early.
It was the default path, not an edge case: main() runs post_clone_setup before
install_resq_cli, so a fresh machine never has resq on PATH at that point and
always took the raw-fetch branch. And RESQ_CRATES_REF is an environment
variable, so the source of those executables was caller-controlled.
Now pinned to a commit with per-hook SHA-256 digests, failing closed. Hooks are
staged in a temp dir and published only after all six verify, so a mismatch on
the last file cannot leave the first five installed and active. Overriding the
ref still works but requires RESQ_ALLOW_UNVERIFIED=1 — the same contract
install-resq.sh already used. The fetch also pins --proto '=https' --tlsv1.2,
which it did not before.
install-hooks.ps1 had the identical flaw and gets the identical treatment.
required.yml now re-checks the pins against the live endpoint. Pins rot
silently: if the templates change, the installers would refuse good hooks and
every onboard would fail with a checksum mismatch. Better a red check than a
broken install.
Verified: correct pin installs six executable hooks and sets core.hooksPath;
an overridden ref without opt-in installs nothing; a tampered digest installs
nothing. bin/stamp.sh propagated the new installer digests into install.{sh,ps1}
on its own, which is what that machinery is for.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Second half of the security audit. install.sh pins --proto '=https' --tlsv1.2 for its own vendor installer; scripts/lib did not, so the two halves of the repo disagreed about how much to trust the network. Two of these were more than inconsistency: nix.sh had no -f. Without it curl emits the response body on a 4xx/5xx and that HTML is piped straight into sh. -L meant a redirect chain, unconstrained. bun.sh invoked `irm bun.sh/install.ps1` with no scheme, so the request could begin as plaintext http — for a response piped directly into iex. docker.sh used `apt-key add`, which installs into the global trusted set: the Docker key could then sign packages for every repository on the system. It is deprecated for that reason. Replaced with a keyring file scoped by signed-by, and an explicit sources.list.d entry rather than add-apt-repository. misc.sh's release lookup gets -f and the same proto flags. It reads JSON rather than feeding a shell, but without -f an error body is piped into the parser and a GitHub error message can be mistaken for a tag name. All six executed curl invocations under scripts/lib now carry --proto '=https' --proto-redir '=https' --tlsv1.2. The PowerShell mirrors match. These remain unverified third-party installers — the vendors publish no digests to check against — so this constrains the transport, not the content. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
get-resq-software | 4a2b696 | Aug 11 2026, 08:22 AM |
|
Warning Review limit reached
Next review available in: 2 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
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 |
resq-sw
approved these changes
Aug 11, 2026
Merged
WomB0ComB0
added a commit
that referenced
this pull request
Aug 11, 2026
Ships the security fixes from #46 and #47, which are on main but not yet reaching anyone: get.resq.software serves what PINS says, and PINS still points at v0.4.0. Until this releases and the pin bump merges, every `curl -fsSL https://get.resq.software | sh` still installs the hook installer that fetched executable git hooks from a mutable branch without verifying them. main install-hooks.sh 3b3e67197ffe... pinned + verified served (v0.4.0) 24bd874dd27f... unverified Merging this is the release: release.yml validates VERSION, creates the tag, publishes the Release and SHA256SUMS, and opens the pin-bump PR. Merging that is what changes the served bytes. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
WomB0ComB0
added a commit
that referenced
this pull request
Aug 11, 2026
…po (#50) * fix(lib): verify the Nix installer; install Docker from its signed repo Items 1 and 2 from the vendor-installer discussion. #47 pinned the transport for these; this verifies the content, where the vendors make that possible. Nix. install.determinate.systems/nix is a redirect to a versioned URL — /nix/tag/v3.21.9 — which returns identical bytes across fetches. So it can be pinned and digest-checked, and now is: downloaded to a temp dir, SHA-256 compared, executed only on match, in all four places that installed Nix (install.sh, install.ps1, and both libs). scripts/lib/nix.sh additionally switches from nixos.org's rolling installer to the same Determinate one; two different installers for one tool in a single repo was its own problem. Docker. scripts/lib/docker.ps1 still piped get.docker.com to sh while docker.sh installed from Docker's apt repository, which apt verifies by GPG signature on every package. Only one of them was verified. get.docker.com is rolling, unsigned and unversioned — there is nothing to pin it against, so the fix is to stop using it rather than to constrain how it is fetched. Non-apt Linux under PowerShell now reports what to run instead of silently taking the weaker path. required.yml re-checks the Nix pin against the live URL and asserts all four copies agree, for the same reason it checks the hook digests: a stale pin would otherwise fail every user's install rather than one CI run. What this does not do: Determinate publish no digest of their own, so this pins what we execute, not what they wrote. It closes "silently different tomorrow", not "already compromised when we pinned it". Bun is unchanged — its install script is not covered by the signed SHASUMS256.txt, and fixing that properly means replacing the script with a download-and-verify against the signed manifest, which is a separate change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: handle installer failures; correct Debian vs Ubuntu docker repo Review findings on #50, all valid. Silent failures. Verifying a download says the script is authentic, not that it worked. install.sh set NIX_JUST_INSTALLED after the installer ran regardless of its exit status, so a failed install printed "Nix was just installed" and a PATH hint for a Nix that was not there. The two PowerShell paths were worse: a native command's failure does not throw, so Install-Nix and Install-Docker fell through to their success returns and setup.ps1 carried on. All three now check. Debian. Both docker paths hardcoded linux/ubuntu with `lsb_release -cs`. On Debian that yields e.g. "bookworm", which the ubuntu suite does not publish, so apt errors or silently finds no candidate. Now reads ID/ID_LIKE from /etc/os-release and picks the matching suite, per Docker's own documentation. Derivatives resolve through ID_LIKE with UBUNTU_CODENAME/DEBIAN_CODENAME, which matters for Kali: its VERSION_CODENAME is "kali-rolling", not a Docker suite. Exercised across ubuntu/debian/mint/pop/kali/raspbian, with fedora correctly rejected to a manual install. The Nix pin check covered install.sh and both libs but not install.ps1, which declares its own $NixInstallUrl and $NixInstallerSha256. It could have drifted while the job passed — meaning Windows users would get a different installer than everyone else, which is the failure the check exists to prevent. Now all four are compared. Two findings not taken, with reasons in the review thread: the RETURN traps do fire (verified on normal return, early failure return, and under set -e), and the missing Apache headers are a pre-existing repo-wide gap — all 20 files in scripts/lib lack one, so fixing two of them makes the directory less consistent, not more. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(lib): confine the installer cleanup trap to a subshell The review was right and I tested the wrong thing first. I checked whether a RETURN trap set inside a function fires; the finding was that it does not stay inside that function. Verified, all three claims: without set -T scoped to the function with set -T PERSISTS - fired on an unrelated later return nested functions silently replaced the caller's own RETURN trap under set -Tu "unbound variable", status 1, once the local is gone These are library functions meant to be sourced, so a caller enabling functrace is a realistic way to break them, and the failure would surface far from here. Download, verify and run now happen in a subshell with an EXIT trap, which cannot escape and takes the temp directory with it. Exit status still propagates, so a failed install is still reported. Removed the vestigial `if true` the restructure left behind, along with its unreachable else. scripts/lib/bun.sh has the same pattern and is fixed on the #51 branch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(ci): use an explicit if instead of A && B || C SC2015, which actionlint fails the build on. The behaviour was correct here — C should run when either test fails — but the pattern also runs C when A succeeds and B fails, which is the trap shellcheck is pointing at. An explicit if says what is meant without relying on the reader knowing that. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Merged
WomB0ComB0
added a commit
that referenced
this pull request
Aug 11, 2026
Ships the security audit: #46, #47, #50 and #51. All four are on main and none are reaching anyone — the endpoint serves what PINS says, and PINS still points at v0.4.1, which predates every one of them. install.sh on main f38612c071bcd6d4... served (v0.4.1) 639b4167471082c6... What this releases: #46 install-hooks.{sh,ps1} fetched six executables from a mutable branch with no verification, chmod +x'd them, and pointed core.hooksPath at them — on every fresh onboard, since resq is installed after hooks run. Now pinned and digest-checked, failing closed. #47 TLS pinning across scripts/lib; nix.sh piped 4xx bodies into sh; bun.sh fetched schemelessly; docker.sh used apt-key, which grants the Docker key authority over every repository on the system. #50 Nix installer pinned to a versioned URL and verified; Docker installed from its GPG-signed apt repo rather than get.docker.com; installer failures no longer report success; Debian and Ubuntu suites resolved correctly. #51 Bun's installer pinned to a tagged copy and verified. Merging this is the release. Merging the pin-bump PR that follows is what changes the served bytes. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
2 participants
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.
Second half of the audit, kept separate from #46 so that one stays reviewable as a security fix on its own.
install.shpins--proto '=https' --tlsv1.2for its own vendor installer.scripts/libdid not — so the two halves of the repo disagreed about how much to trust the network.Two were more than inconsistency
nix.shhad no-f. Without it, curl emits the response body on a 4xx/5xx and that HTML is piped straight intosh. With-Lthere was also an unconstrained redirect chain.bun.shusedirm bun.sh/install.ps1— no scheme. A bare host lets the request begin as plaintext HTTP, for a response piped directly intoiex.apt-key
docker.shusedapt-key add, which installs into the global trusted set: the Docker key could then sign packages for every repository on the system. Deprecated for exactly that reason.Replaced with a keyring file scoped by
signed-by, plus an explicitsources.list.dentry instead ofadd-apt-repository:Also
misc.sh's release lookup gets-fand the same flags. It reads JSON rather than feeding a shell, but without-fan error body is piped into the parser and a GitHub error message can be mistaken for a tag name.Result
All six executed
curlinvocations underscripts/libnow carry--proto '=https' --proto-redir '=https' --tlsv1.2; the PowerShell mirrors match. Remaining unpinned hits in the tree are comments and user-facing usage text, not executed code.What this does not do: these are still unverified third-party installers. The vendors publish no digests to check against, so this constrains the transport, not the content. Unlike our own artifacts — which are pinned and hash-verified end to end — a compromise of
bun.shorget.docker.comwould still land. Worth being clear about, since the rest of this work sets a higher bar.shellcheck clean, all libs parse,
stamp --checkin sync, 48/48 worker tests.🤖 Generated with Claude Code