fix(hooks): pin and verify the git hooks install-hooks fetches - #46
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>
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
get-resq-software | 5ca758e | Aug 11 2026, 07:25 AM |
📝 WalkthroughWalkthroughThe hook installers now fetch from pinned commits, verify six hook templates with SHA-256, stage files before publication, and reject unverified references by default. CI validates commit and digest alignment across both installers. Wrapper installer checksums were updated. ChangesHook integrity
Sequence Diagram(s)sequenceDiagram
participant Installer
participant CratesRepository
participant StagingDirectory
participant HashUtility
participant GitHooksDirectory
Installer->>CratesRepository: Download hooks from pinned commit
Installer->>StagingDirectory: Store hooks temporarily
Installer->>HashUtility: Compute and compare SHA-256 digests
HashUtility-->>Installer: Return verification results
Installer->>GitHooksDirectory: Publish verified hooks
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
.github/workflows/required.yml (2)
78-84: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd retries to the pinned-hook fetch.
This step gates the
requiredjob. A single transient failure fromraw.githubusercontent.commarks the pin as unfetchable and blocks the PR. Add bounded retries so network noise does not read as a digest problem.♻️ Proposed retry flags
- if ! actual="$(curl -fsSL --proto '=https' --tlsv1.2 "$base/$h" | sha256sum | cut -d' ' -f1)"; then + if ! actual="$(curl -fsSL --proto '=https' --tlsv1.2 \ + --retry 3 --retry-delay 2 --retry-all-errors \ + "$base/$h" | sha256sum | cut -d' ' -f1)"; then🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/required.yml around lines 78 - 84, Update the pinned-hook fetch loop in the required workflow to use bounded curl retries for transient raw.githubusercontent.com failures, while retaining the existing fail-fast options, TLS constraint, digest computation, and rc handling. Apply the retry configuration to the curl invocation fetching each hook listed in the loop.
85-93: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winBind each digest to its hook name, not just to the file.
grep -q "$actual" "$f"asserts only that the digest appears somewhere in the installer. Two digests swapped between hook entries still satisfy every iteration, because both values are present in both files. The installers compare per hook, so a swap makes each fresh install fail with a checksum mismatch. That is the exact failure this step states it prevents.Both installers keep the hook name and its digest on the same line, so a single line-scoped check covers both formats without a second parser.
♻️ Proposed line-scoped assertion
for f in scripts/install-hooks.sh scripts/install-hooks.ps1; do - if ! grep -q "$actual" "$f"; then + # Both installers keep name and digest on one line: + # sh : pre-commit) echo "<digest>" + # ps1: 'pre-commit' = '<digest>' + # The delimiters stop `commit-msg` matching the + # `prepare-commit-msg` entry. + hit="$(grep -F "$actual" "$f" || true)" + case "$hit" in + *" $h)"*|*"'$h'"*) ;; + *) + echo "::error::$f has no digest matching $h at $sh_commit (expected $actual)" + rc=1 ;; + esac - echo "::error::$f has no digest matching $h at $sh_commit (expected $actual)" - rc=1 - fi doneThe repository rule forbids
|| truefor error suppression. If you keep that rule strict here, replace the substitution with an explicit branch:if hit="$(grep -F "$actual" "$f")"; then case "$hit" in *" $h)"*|*"'$h'"*) ;; *) echo "::error::$f binds $actual to the wrong hook (expected $h)"; rc=1 ;; esac else echo "::error::$f has no digest matching $h at $sh_commit (expected $actual)" rc=1 fiAs per coding guidelines: "Do not use
|| trueto suppress errors — handle errors explicitly or document why ignoring an error is acceptable".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/required.yml around lines 85 - 93, Update the digest validation loop in the workflow to bind each matched digest to its expected hook name, rather than checking only whether the digest appears anywhere in the installer. Use an explicit grep success/failure branch and validate the matching line contains the expected hook identifier for both installer formats; preserve the existing error reporting and rc=1 behavior, without using “|| true”.Source: Coding guidelines
scripts/install-hooks.ps1 (1)
97-107: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueEnforce TLS 1.2 for the PowerShell download.
Set
[Net.ServicePointManager]::SecurityProtocol = [Net.SecurityProtocolType]::Tls12beforeInvoke-WebRequest. Do not use-bor; it preserves older protocols and does not enforce the TLS 1.2 floor.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/install-hooks.ps1` around lines 97 - 107, Set [Net.ServicePointManager]::SecurityProtocol to [Net.SecurityProtocolType]::Tls12 immediately before the Invoke-WebRequest call in the hook download loop, enforcing TLS 1.2 without using -bor or retaining older protocols.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In @.github/workflows/required.yml:
- Around line 78-84: Update the pinned-hook fetch loop in the required workflow
to use bounded curl retries for transient raw.githubusercontent.com failures,
while retaining the existing fail-fast options, TLS constraint, digest
computation, and rc handling. Apply the retry configuration to the curl
invocation fetching each hook listed in the loop.
- Around line 85-93: Update the digest validation loop in the workflow to bind
each matched digest to its expected hook name, rather than checking only whether
the digest appears anywhere in the installer. Use an explicit grep
success/failure branch and validate the matching line contains the expected hook
identifier for both installer formats; preserve the existing error reporting and
rc=1 behavior, without using “|| true”.
In `@scripts/install-hooks.ps1`:
- Around line 97-107: Set [Net.ServicePointManager]::SecurityProtocol to
[Net.SecurityProtocolType]::Tls12 immediately before the Invoke-WebRequest call
in the hook download loop, enforcing TLS 1.2 without using -bor or retaining
older protocols.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 500ccc91-9e2d-49c2-b631-225d0f36fc68
📒 Files selected for processing (5)
.github/workflows/required.ymlinstall.ps1install.shscripts/install-hooks.ps1scripts/install-hooks.sh
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>
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
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.
Found while auditing the finished work. This is the one finding I'd call urgent — everything else in the audit is hardening.
The chain terminated one link too early
install.shverifiesinstall-hooks.shbefore executing it.install-hooks.shthen did this:Six executables from a mutable branch, made executable, wired into git — running on every commit and push.
Two things make it worse than it reads:
main()runspost_clone_setup(step 22) beforeinstall_resq_cli(step 23), so a fresh machine has noresqon PATH and always took this branch.RESQ_CRATES_REFis an environment variable, so the source of those executables was caller-controlled.This is the same class of bug as the one fixed at
install.sh:354. I fixed the caller and never followed the chain down.Fix
Pinned commit + per-hook SHA-256, failing closed. Hooks stage in a temp dir and publish only after all six verify, so a mismatch on the last cannot leave the first five installed and active. Overriding the ref still works but needs
RESQ_ALLOW_UNVERIFIED=1— the contractinstall-resq.shalready used. The fetch now also pins--proto '=https' --tlsv1.2, which it did not.install-hooks.ps1had the identical flaw and gets identical treatment.required.ymlre-checks the pins against the live endpoint, because pins rot silently: if the templates change, the installers would refuse good hooks and every onboard would fail on a checksum mismatch. A red check beats a broken install.Verified
core.hooksPathset, "verified against pinned commit"bin/stamp.shpropagated the new installer digests intoinstall.{sh,ps1}by itself — that machinery earning its keep.Not fixed here
The rest of the audit, deliberately left for a separate change:
scripts/lib/{nix,bun,docker}.{sh,ps1}pipe vendor installers to a shell without--proto '=https' --tlsv1.2(whichinstall.shdoes pin), anddocker.sh:17usesapt-key add -, which trusts the key for every repo on the system. Third-party installers, so options are limited — but the TLS pinning is free and currently inconsistent.🤖 Generated with Claude Code
Summary by CodeRabbit
Security
Reliability