feat(installer): pin and hash-verify the curl|sh distribution - #37
Merged
Conversation
get.resq.software served raw.githubusercontent.com/.../main — a mutable ref, piped straight into a shell, with nothing verifying the bytes. Any push to main reached every developer's machine immediately, and there was no way to state what a given install actually ran. Worker (worker/): - fetches by 40-char commit SHA, never a branch or tag - SHA-256 verifies every byte against digests baked in at deploy, and 502s having served nothing on mismatch — no degraded mode - adds /v<version>/ immutable URLs, /SHA256SUMS, /manifest.json - GET/HEAD only, nosniff/CSP/CORP/HSTS, ETag, x-resq-sha256 - shell-route error bodies are inert shell that exits 1, so a dropped `curl -f` cannot execute an English error message - 40 tests, including fail-closed and path-traversal cases install.sh / install.ps1: - `landing` went private but was still menu option 7, so that choice failed at clone time. Replaced three lists that could disagree with one REPOS table driving the menu, validation and the summary - hook install no longer curls a mutable main ref into a shell: pinned to the script's own version and SHA-256 checked before it executes - menu re-prompts instead of aborting, takes a name or a number, grows past 9 - install_resq_cli's trap clobbered any earlier trap and survived its own early returns; replaced with one script-wide cleanup - adds umask 077, an up-front curl check, --help, --version Version has one authored source (VERSION). bin/stamp.sh propagates it and the hook digests into both installers; CI verifies by regeneration, so a stamped value cannot be silently forgotten. Stamp, merge, then tag. Release is automatic from a tag and needs no Cloudflare credential: digests are computed from tag history, checked against live GitHub, published as a Release plus SHA256SUMS, then proposed as a pin-bump PR. Cloudflare Workers Builds deploys on merge. .github/CODEOWNERS is the file GitHub actually reads (it wins over the root copy), so ownership moves there and now covers worker/ and bin/. The root copy becomes a signpost rather than a set of silently ignored rules. 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 | dede732 | Aug 10 2026, 09:19 PM |
📝 WalkthroughWalkthroughThe project version changes to 0.4.0. Both installers gain pinned, hash-verified hook downloads and canonical repository metadata. A Worker serves verified artifacts. Release automation generates pins, publishes releases, updates pins, and validates deployments. ChangesRelease artifact delivery
Repository ownership controls
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant ReleaseTag
participant ReleaseWorkflow
participant GenPins
participant Worker
participant GitHubRelease
ReleaseTag->>ReleaseWorkflow: Trigger on v* tag
ReleaseWorkflow->>GenPins: Generate and verify release pins
ReleaseWorkflow->>Worker: Run Worker tests and verify served bytes
ReleaseWorkflow->>GitHubRelease: Publish SHA256SUMS and release notes
ReleaseWorkflow->>GenPins: Regenerate pins on main
GenPins-->>ReleaseWorkflow: Report pin changes
ReleaseWorkflow->>GitHubRelease: Open pin-update pull request when changed
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
Contributor
Audit Results: PASSI have audited the changes in this PR and found them to be of high quality with strong security considerations. Key Improvements:
Minor Observations:
Overall, this is a solid security enhancement. Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "localhost"See Network Configuration for more information.
|
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (9)
install.sh (3)
158-179: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winRemove the destination file when curl fails.
curl -fcan still create$_fv_destbefore it aborts on an HTTP error, and it does not remove the file unless--remove-on-erroris given. The mismatch path removes the file, but the curl path does not. The fallback download overwrites it, so no unverified bytes run today. Keep the invariant explicit anyway.🛡️ Proposed fix
- curl -fsSL --proto '=https' --tlsv1.2 "$_fv_url" -o "$_fv_dest" || return 1 + if ! curl -fsSL --proto '=https' --tlsv1.2 "$_fv_url" -o "$_fv_dest"; then + rm -f "$_fv_dest" + return 1 + fi🤖 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 `@install.sh` around lines 158 - 179, Update fetch_verified so a failed curl download removes $_fv_dest before returning failure. Preserve the existing checksum and verification behavior for successful downloads.
427-437: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueTrim the interactive input.
resolve_choicereceives the raw line. A value such as3matches the*[!0-9]*branch, falls through to a name lookup, and re-prompts.install.ps1trims the same input at line 305, so the two installers behave differently.♻️ Proposed change
- REPO="$(resolve_choice "$_cr_choice")" + _cr_choice="$(printf '%s' "$_cr_choice" | tr -d ' \t\r')" + REPO="$(resolve_choice "$_cr_choice")"🤖 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 `@install.sh` around lines 427 - 437, Trim leading and trailing whitespace from the value read into _cr_choice before passing it to resolve_choice, matching the input normalization used by install.ps1. Keep the existing invalid-choice warning and re-prompt behavior unchanged.
148-153: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRename the new helpers to verb_noun.
sha256_of,fetch_verified,repo_field,repo_exists, andrepo_namesare noun-first or verb-adjective.check_curlfollows the convention. Suggested names:hash_file,fetch_verified_file(ordownload_verified),get_repo_field,has_repo,list_repo_names.As per coding guidelines: "Use verb_noun naming convention for shell script functions (e.g., detect_platform, install_gh)".
Also applies to: 186-201, 717-720
🤖 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 `@install.sh` around lines 148 - 153, Rename the listed shell helpers to verb_noun names: sha256_of to hash_file, fetch_verified to fetch_verified_file or download_verified, repo_field to get_repo_field, repo_exists to has_repo, and repo_names to list_repo_names. Update every definition and call site consistently, including the additional affected sections, while preserving behavior.Source: Coding guidelines
.github/workflows/required.yml (1)
88-92: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePin the Node version for the worker job.
The job uses whatever Node the runner image ships. The test relies on
crypto.subtle,Request,Response, and a globalcachesoverride, so a runner image change can alter the result without a repository change. Addactions/setup-nodewith an explicit version. Apply the same pin to theWorker test suitestep in.github/workflows/release.yml.🤖 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 88 - 92, Update the workflow job containing the “Worker test suite” step to run actions/setup-node with an explicit Node version before executing tests, and apply the identical pinned version to the corresponding “Worker test suite” step in the release workflow. Preserve the existing test commands and setup behavior.worker/src/index.js (2)
312-312: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueSort
versionsnumerically.
Object.keys(config.releases).sort()is lexicographic, so0.10.0orders before0.9.0. The field is metadata only, but a client that reads the last entry gets the wrong answer. Use a numeric comparison on the dotted segments.🤖 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 `@worker/src/index.js` at line 312, Update the versions construction to sort release keys by numeric comparison of their dotted version segments rather than default lexicographic ordering, ensuring entries such as 0.9.0 precede 0.10.0 while preserving the existing versions metadata field.
120-133: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winValidate the commit of every release, not only the latest.
pinschecksreleases[parsed.latest].commitagainst/^[0-9a-f]{40}$/. A request for any other version usesrelease.commitunchecked, andverifiedFetchinterpolates it into the upstream URL at line 263. A value containing/would resolve to a different ref or path.RESQ_PINSis deploy-controlled and the digest check still fails closed, so this is hardening rather than an open hole. Validating the whole override keeps the "structurally impossible" property the file claims.🛡️ Proposed fix
- const rel = parsed?.releases?.[parsed?.latest]; - if (!rel || !/^[0-9a-f]{40}$/.test(rel.commit ?? "")) return PINS; - if (!rel.artifacts || typeof rel.artifacts !== "object") return PINS; + const all = Object.values(parsed?.releases ?? {}); + if (!all.length || !parsed?.releases?.[parsed?.latest]) return PINS; + for (const rel of all) { + if (!rel || !/^[0-9a-f]{40}$/.test(rel.commit ?? "")) return PINS; + if (!rel.artifacts || typeof rel.artifacts !== "object") return PINS; + } return parsed;🤖 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 `@worker/src/index.js` around lines 120 - 133, Update pins to validate every release in parsed.releases, ensuring each release has a 40-character hexadecimal commit before returning the override. Preserve the existing latest-release and artifacts validation, and continue returning PINS whenever any release is malformed or invalid.worker/test/index.test.mjs (1)
1-3: 🩺 Stability & Availability | 🔵 TrivialConsider a retry for the live upstream.
The suite gates every pull request through required.yml and reaches raw.githubusercontent.com on each artifact request. A transient upstream error fails the required check for changes unrelated to the Worker. One retry per request, or a separate non-gating job for the network assertions, keeps the property under test while removing the shared failure mode.
🤖 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 `@worker/test/index.test.mjs` around lines 1 - 3, Update the live upstream request flow in the smoke tests to retry each raw.githubusercontent.com artifact request once after a transient failure, while preserving the existing digest-validation assertions and required-check behavior.bin/gen-pins.sh (1)
36-40: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReport a fetch failure as a fetch failure.
Two related gaps in the same helper chain:
- Line 39 assumes
shasumexists whensha256sumdoes not. Add an explicit check so the script reports the missing tool, matchingbin/stamp.shline 36.- POSIX
shhas nopipefail, so line 86 reports the status ofsha256, notcurl. A failed download yields the digest of empty output and prints MISMATCH instead of FETCH FAIL. The run still fails closed, so this is a diagnostics problem only.♻️ Proposed change
-else +elif command -v shasum >/dev/null 2>&1; then sha256() { shasum -a 256 | cut -d' ' -f1; } +else + die "neither sha256sum nor shasum is available" fi- if actual="$(curl -fsSL --proto '=https' --tlsv1.2 "$RAW_BASE/$commit/$path" | sha256)"; then + body="$(mktemp)" + if curl -fsSL --proto '=https' --tlsv1.2 "$RAW_BASE/$commit/$path" -o "$body"; then + actual="$(sha256 < "$body")"Also applies to: 86-96
🤖 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 `@bin/gen-pins.sh` around lines 36 - 40, Update the sha256 helper selection in the script to explicitly verify that shasum is available when sha256sum is absent, and report the missing-tool error consistently with bin/stamp.sh. In the fetch-and-verify flow around the curl invocation and digest comparison, preserve curl’s exit status separately from the piped sha256 command so failed downloads are reported as FETCH FAIL rather than MISMATCH.install.ps1 (1)
40-43: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUpdate the header usage text to the pinned endpoint.
Lines 15-18 still document
irm https://github.com/@raw/resq-software/dev/main/install.ps1 | iex. That is the mutable path this change replaces.install.shnow advertises$DIST_BASEin itsusageoutput. Align the PowerShell header with$DistBase.🤖 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 `@install.ps1` around lines 40 - 43, Update the PowerShell usage text in the header to advertise the pinned $DistBase endpoint instead of the mutable raw.githubusercontent.com URL, matching the install.sh usage output while leaving the installer behavior unchanged.
🤖 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.
Inline comments:
In @.github/CODEOWNERS:
- Line 21: Update the CODEOWNERS precedence comment above the path patterns to
state that GitHub uses the last matching pattern, replacing the inaccurate “most
specific match wins” guidance.
In @.github/workflows/release.yml:
- Around line 200-203: Update the release workflow’s push-based workaround near
the documented release/pins-* handling so it uses credentials that trigger
downstream workflows rather than GITHUB_TOKEN/github.token. Align the
implementation with the credential strategy required by required.yml, and revise
the adjacent comment to accurately describe the trigger behavior.
- Around line 50-54: Update the verify job’s actions/checkout step to set
persist-credentials to false, keeping the existing full-history fetch
configuration unchanged. Do not modify the separate checkout in the bump job,
which must retain credentials for pushing.
- Around line 206-209: Update the release workflow’s branch handling around git
checkout and push to fetch origin "$branch" before the forced push, allowing the
fetch to fail without stopping the workflow. Keep the existing git push
--force-with-lease origin "$branch" command unchanged.
In @.github/workflows/required.yml:
- Line 83: Update the worker job’s actions/checkout step to set
persist-credentials to false, ensuring the checkout token is not retained in
.git/config before worker tests make outbound requests.
- Around line 13-19: The required workflow is not reliably triggered for
pin-bump PRs created with GITHUB_TOKEN. Update .github/workflows/release.yml
lines 200-209 to use a PAT/App-based push or dispatch
.github/workflows/required.yml explicitly with gh workflow run after pushing;
alternatively, relax the required check for release/pins-* while retaining
CODEOWNERS review. Adjust .github/workflows/required.yml lines 13-19
consistently with the chosen approach.
In `@bin/gen-pins.sh`:
- Around line 113-114: Update the occurrence-count assignment in the
pin-generation script so a zero-match grep result is converted to a successful
command status, allowing the subsequent exact-count validation and die message
to execute. Preserve the existing count comparison and error reporting for
counts other than one.
In `@bin/stamp.sh`:
- Around line 59-64: Strengthen the VERSION validation in the case check so it
accepts only a dotted numeric version with nonempty numeric components,
rejecting values such as 1..2, 1.2.3., and .... Preserve trimming of surrounding
whitespace and the existing die message for invalid versions.
In `@install.ps1`:
- Around line 406-415: Remove the `$reRun = "irm $url | iex"` assignment in the
checksum-success branch of the install flow. Replace the rerun guidance with a
verified local script path or the supported CLI command, ensuring the catch
block no longer suggests downloading and executing the URL directly.
In `@worker/test/index.test.mjs`:
- Around line 56-58: Guard the content-type lookup in the GET / assertions so a
missing header produces a failed check rather than throwing. Update the check
around `r.headers.get("content-type")` to safely handle null or undefined while
preserving the existing x-shellscript validation and allowing subsequent tests
to run.
---
Nitpick comments:
In @.github/workflows/required.yml:
- Around line 88-92: Update the workflow job containing the “Worker test suite”
step to run actions/setup-node with an explicit Node version before executing
tests, and apply the identical pinned version to the corresponding “Worker test
suite” step in the release workflow. Preserve the existing test commands and
setup behavior.
In `@bin/gen-pins.sh`:
- Around line 36-40: Update the sha256 helper selection in the script to
explicitly verify that shasum is available when sha256sum is absent, and report
the missing-tool error consistently with bin/stamp.sh. In the fetch-and-verify
flow around the curl invocation and digest comparison, preserve curl’s exit
status separately from the piped sha256 command so failed downloads are reported
as FETCH FAIL rather than MISMATCH.
In `@install.ps1`:
- Around line 40-43: Update the PowerShell usage text in the header to advertise
the pinned $DistBase endpoint instead of the mutable raw.githubusercontent.com
URL, matching the install.sh usage output while leaving the installer behavior
unchanged.
In `@install.sh`:
- Around line 158-179: Update fetch_verified so a failed curl download removes
$_fv_dest before returning failure. Preserve the existing checksum and
verification behavior for successful downloads.
- Around line 427-437: Trim leading and trailing whitespace from the value read
into _cr_choice before passing it to resolve_choice, matching the input
normalization used by install.ps1. Keep the existing invalid-choice warning and
re-prompt behavior unchanged.
- Around line 148-153: Rename the listed shell helpers to verb_noun names:
sha256_of to hash_file, fetch_verified to fetch_verified_file or
download_verified, repo_field to get_repo_field, repo_exists to has_repo, and
repo_names to list_repo_names. Update every definition and call site
consistently, including the additional affected sections, while preserving
behavior.
In `@worker/src/index.js`:
- Line 312: Update the versions construction to sort release keys by numeric
comparison of their dotted version segments rather than default lexicographic
ordering, ensuring entries such as 0.9.0 precede 0.10.0 while preserving the
existing versions metadata field.
- Around line 120-133: Update pins to validate every release in parsed.releases,
ensuring each release has a 40-character hexadecimal commit before returning the
override. Preserve the existing latest-release and artifacts validation, and
continue returning PINS whenever any release is malformed or invalid.
In `@worker/test/index.test.mjs`:
- Around line 1-3: Update the live upstream request flow in the smoke tests to
retry each raw.githubusercontent.com artifact request once after a transient
failure, while preserving the existing digest-validation assertions and
required-check behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 0346778a-9c78-43b4-9b89-3a7d76174839
📒 Files selected for processing (12)
.github/CODEOWNERS.github/workflows/release.yml.github/workflows/required.ymlCODEOWNERSVERSIONbin/gen-pins.shbin/stamp.shinstall.ps1install.shworker/src/index.jsworker/test/index.test.mjsworker/wrangler.jsonc
repo-drift.yml re-derives "public, non-fork, excluding .github" from the GitHub API and diffs it against what both installers offer. `landing` went private while still being menu option 7 and nothing noticed, because the list was hand-maintained in three places. Runs on PRs touching the installers and weekly — a repository changing visibility produces no commit, so a push-triggered check alone would never see it. worker-live.yml asserts that what get.resq.software serves matches what main declares. release.yml only verified digests before publishing, leaving every later failure invisible: a deploy that silently did not run, a deploy of the wrong directory, or an endpoint failing closed on stale pins. It runs main's Worker in-process to learn the expected manifest — rather than parsing PINS out of the source, so the check cannot drift from the implementation — then fetches the real script and hashes what actually came back. Both extractors are guarded against silently matching nothing: an empty list would agree with an empty list and pass while checking nothing. The install.ps1 extractor is scoped to the $Repos block, since matching Name = '...' across the whole file also picks up $binName values like 'resq' and 'resq.exe'. Docs now describe what actually happens. README pointed people at raw.githubusercontent.com/.../main — the unpinned path this work exists to replace — and documented RESQ_DEV_REF for pinning a revision, which no script has ever implemented, so it silently did nothing. Version-locked URLs replace it. `landing` is gone from both tables. AGENTS.md gains the layout entries for VERSION, bin/ and worker/, a note that .github/CODEOWNERS is the file GitHub actually reads, and the release procedure: stamp, merge, then tag. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Member
Author
|
Follow-ups from the description are now included in this PR:
Expect Worth confirming before merge: the Workers Builds trigger has root directory |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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.
Inline comments:
In @.github/workflows/repo-drift.yml:
- Around line 56-59: Update the repository enumeration command in the workflow
to match the documented membership rule by removing the --no-archived filter, so
all public, non-fork repositories except .github are included in the comparison.
In @.github/workflows/worker-live.yml:
- Around line 102-123: Extend the verification flow after the manifest
comparison to iterate every route exposed by local.artifacts, rather than
fetching only install.sh. For each artifact, request its published route,
require a successful response, hash the response bytes, and validate both the
x-resq-sha256 header and computed digest against that artifact’s local sha256
metadata; retain the existing installer-content check specifically for
install.sh.
In `@AGENTS.md`:
- Around line 72-75: Update the documentation near the Worker verification
behavior to replace “502s having served nothing on mismatch” with wording that
accurately states the Worker returns a failure response without serving
installer bytes, while preserving the existing SHA-256 verification and
no-degraded-mode details.
In `@README.md`:
- Around line 27-34: Update the README installation verification commands so
both install.sh and SHA256SUMS are downloaded from the same immutable,
version-pinned release path rather than separate latest endpoints. Preserve the
checksum comparison and subsequent installer execution flow while ensuring the
pinned version is reused consistently for both files.
- Around line 20-23: Update the latest-endpoint explanation in README.md to
state that the served commit changes only after the reviewed PINS update in the
pin-bump PR merges and deploys, rather than implying that tagging alone changes
the endpoint. Preserve the existing pinned-commit verification and mismatch
behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f900a247-4b3e-412d-a354-dd32c1d5a6be
📒 Files selected for processing (4)
.github/workflows/repo-drift.yml.github/workflows/worker-live.ymlAGENTS.mdREADME.md
actionlint failed the build on two shellcheck SC2016 findings ("expressions
don't expand in single quotes"). The escapes were intentional — the patterns
need a literal dollar to match `REPOS="$(cat` and `$Repos = @(` — but
shellcheck cannot tell intent from mistake here, and actionlint treats
info-level findings as failures.
A [$] bracket expression matches exactly the same character without tripping
the check, so the fix removes the warning rather than suppressing it.
Verified against the live org: install.sh, install.ps1 and the GitHub API all
yield the same 9 repositories, confirming the extractors still work and that
the fork (ardupilot) and .github are correctly excluded.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The bot PR could never have merged. The design relied on required.yml running on push for release/pins-*, but GitHub suppresses run-triggering events from GITHUB_TOKEN for push exactly as it does for pull_request — so no `required` check would ever have appeared and every pin bump would have stalled. release.yml now dispatches required.yml explicitly after pushing; workflow_dispatch is one of the two documented exceptions, so this stays free of a PAT or App secret. required.yml grows the matching trigger. Also in that job: --force-with-lease compared against a remote-tracking ref that was never fetched (checkout only brought down main), so any re-run of a release would have been rejected. bin/gen-pins.sh had an unreachable guard. `grep -c` exits 1 on zero matches, the command substitution inherits it, and set -e killed the script one line before the die that explains what went wrong — precisely in the case the guard was written for. Verified: it now reports "found 0" instead of exiting silently. install.ps1 printed `irm $url | iex` as the recovery hint after a successful verification, handing back the unverified pipe-to-execute path this change exists to remove. persist-credentials: false on every read-only checkout. Every pre-existing workflow in this repo already did this; the new ones did not, so this was a regression rather than a style question. The bump job keeps its credential because it genuinely pushes, and now says so. Corrections to claims that were simply wrong: - CODEOWNERS: GitHub applies the LAST matching pattern, not the most specific. The ordering was already correct; the comment explaining why was not. - README: tagging does not change what the endpoint serves. The merged pin bump does. Verification now uses one immutable release for both the installer and SHA256SUMS, since separate latest-endpoint fetches can straddle a deploy. - AGENTS: a failed verification does send a body — an inert shell snippet that exits 1. "Serves nothing" was wrong about the mechanism that makes dropping `curl -f` safe. - The membership rule omitted non-archived while the query passed --no-archived, so the check enforced something other than what it documented. worker-live now verifies every published artifact route, not just install.sh: a partial deploy could leave the installer correct and hooks.sh stale, and both get executed. stamp.sh rejects 1..2, 1.2 and 1.2.3.4, each of which became a URL path segment that would 404 on a user's machine. The test suite guards a header read that would have crashed the run instead of failing an assertion. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
README.md (1)
68-69: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPass provisioning variables to the installer shell.
Assignments before the pipeline apply to
curl, not to theshprocess that runs the installer. ThereforeYES=1does not suppress prompts, andREPOandRESQ_DIRare ignored.Proposed fix
- curl -fsSL https://get.resq.software | sh + curl -fsSL https://get.resq.software | \ + env REPO=npm YES=1 RESQ_DIR=/srv/work sh -REPO=npm YES=1 curl -fsSL https://get.resq.software/v0.4.0/install.sh | sh +curl -fsSL https://get.resq.software/v0.4.0/install.sh | \ + env REPO=npm YES=1 shAlso applies to: 75-75
🤖 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 `@README.md` around lines 68 - 69, Update the README installer command so YES, REPO, and RESQ_DIR are passed to the sh process executing the downloaded script rather than assigned to curl; preserve the existing curl pipeline and ensure all provisioning variables reach the installer.
♻️ Duplicate comments (1)
.github/workflows/release.yml (1)
216-221: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winFetch the remote-tracking ref explicitly before using the lease.
git fetch origin "$branch"uses a source-only refspec. It can store the result only inFETCH_HEADand does not guaranteerefs/remotes/origin/$branchexists. The following--force-with-leasecan then reject a re-run when the bump branch already exists. Fetch intorefs/remotes/origin/$branchexplicitly. (git-scm.com)Proposed fix
- git fetch origin "$branch" || echo "no existing $branch on origin" + git fetch origin \ + "+refs/heads/$branch:refs/remotes/origin/$branch" \ + || echo "no existing $branch on origin" git push --force-with-lease origin "$branch"🤖 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/release.yml around lines 216 - 221, Update the fetch command in the release workflow’s branch push step to map the remote branch explicitly to refs/remotes/origin/$branch before running git push --force-with-lease. Preserve the existing fallback behavior for branches that do not yet exist, and leave the push command unchanged.
🤖 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.
Outside diff comments:
In `@README.md`:
- Around line 68-69: Update the README installer command so YES, REPO, and
RESQ_DIR are passed to the sh process executing the downloaded script rather
than assigned to curl; preserve the existing curl pipeline and ensure all
provisioning variables reach the installer.
---
Duplicate comments:
In @.github/workflows/release.yml:
- Around line 216-221: Update the fetch command in the release workflow’s branch
push step to map the remote branch explicitly to refs/remotes/origin/$branch
before running git push --force-with-lease. Preserve the existing fallback
behavior for branches that do not yet exist, and leave the push command
unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7ae135bb-b80b-42c3-85ae-934214b15661
📒 Files selected for processing (12)
.github/CODEOWNERS.github/workflows/release.yml.github/workflows/repo-drift.yml.github/workflows/required.yml.github/workflows/worker-live.ymlAGENTS.mdREADME.mdbin/gen-pins.shbin/stamp.shinstall.ps1install.shworker/test/index.test.mjs
🚧 Files skipped from review as they are similar to previous changes (10)
- .github/workflows/repo-drift.yml
- .github/workflows/required.yml
- .github/CODEOWNERS
- install.sh
- bin/gen-pins.sh
- install.ps1
- worker/test/index.test.mjs
- bin/stamp.sh
- .github/workflows/worker-live.yml
- AGENTS.md
This was referenced Aug 11, 2026
Merged
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.
Why
get.resq.softwareservedraw.githubusercontent.com/.../main— a mutable ref, piped into a shell, with nothing verifying the bytes. Every push tomainreached developers' machines immediately, and there was no way to say what a given install actually ran.Separately,
landingwent private while still being offered as menu option 7, so one of nine choices failed at clone time.Worker
/v<version>/…immutable URLs,/SHA256SUMS,/manifest.json,x-resq-sha256, ETag,nosniff/CSP/CORP/HSTS, GET+HEAD only.curl -fcan't execute an English sentence.Installers
landingREPO=landingrejectedREPOStablecurl .../**main**/install-hooks.sh | shtrapVersion
VERSIONis the only authored copy.bin/stamp.shpropagates it plus both hook digests into the installers; CI verifies by regeneration, so a stamped value added later cannot be forgotten — forgetting it is a diff. Order is stamp → merge → tag.Release
Tag-triggered and needs no Cloudflare credential. Digests computed from tag history → checked against live GitHub → Release +
SHA256SUMSpublished → pin-bump PR opened. Workers Builds deploys on merge.Ownership
.github/CODEOWNERSis the file GitHub actually reads — it takes precedence over the root copy, which meant rules written at the root protected nothing. Ownership moved there and now coversworker/andbin/; the root copy is now a signpost.Verification
shellcheck -S errorclean across all shell scriptsinstall.shtable tests, incl. 7 pattern-injection probesbin/stamp.sh --checkin syncNot verified locally:
pwshis unavailable on my machine, soinstall.ps1has not been parse-checked. ThepowershellCI job is the first real check on it — please look there before approving.Follow-ups (not in this PR)
README.mdstill listslandingand still documents the unpinnedraw.githubusercontentinstall URLAGENTS.mdlayout table lacksworker/,bin/,VERSIONrepo-drift.ymlandworker-live.ymlnot yet writtenworker), then theproductionenvironment can be deleted🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
0.4.0support across installation workflows.Bug Fixes
Chores