Uh oh!
There was an error while loading. Please reload this page.
sec(install): bound the remaining post-cosign install.ps1 fetches (backend#2544) - #588
Conversation
…ckend#2544) cli#582 (backend#2199) bounded only the cosign bootstrap fetch. The same unbounded Invoke-WebRequest pattern (no size ceiling, no timeout) remained for the four artifacts fetched after it — the CLI binary, SHA256SUMS, and the .sig/.cert. They are SHA256- and cosign-verified after download, but an unbounded body can still hang the install or exhaust disk BEFORE that check runs: the same DoS #2199 closed. Route all four through the existing Save-BoundedFile helper with ceilings sized against the real published assets (binary ~50 MB -> 200 MB; the three metadata files <4 KB -> 1 MB / 60 s each). The mandatory binary + SHA256SUMS fetches are wrapped so a cap-trip / network failure fails with a clean Fail() message instead of a raw PowerShell error record. Tests: both tiers of the install.ps1 suite gain call-site assertions - the string tier (install-ps1-verify.sh) and an AST tier (install-ps1-functions .tests.ps1) pinning "no artifact is fetched to disk with a raw Invoke-WebRequest -OutFile" and "every Save-BoundedFile call carries -MaxBytes". Mutation-tested: reverting any fetch to a raw Invoke-WebRequest fails the suite. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
LukasWodka
left a comment
There was a problem hiding this comment.
I measured every cap against the live release rather than reading the table, and all four are exact. That's the check that matters here, because on #582 the cap shipped below the real artifact size — so "the numbers are facts about the outside world" isn't a rhetorical caveat, it's the thing that went wrong last time.
tracebloc-v0.10.13-windows-amd64.exe 49,933,824 B (47.62 MB) cap 200MB → 4.0× headroom
tracebloc-v0.10.13-windows-amd64.exe.cert 3,248 B cap 1MB
tracebloc-v0.10.13-windows-amd64.exe.sig 96 B cap 1MB
SHA256SUMS 786 B cap 1MB
Every figure in your table matches byte-for-byte, the ~4× is precise rather than approximate, and 200 MB clears the largest asset in the release, not just the one this script fetches — I checked, windows-amd64.exe is the biggest at 47.62 MB, so the cap holds if the fetched platform ever changes.
The Fail() wrapping is the right asymmetry. The two mandatory fetches get a clean one-line failure because there's no fallback; the .sig/.cert land in the existing fail-closed catch that already distinguishes "missing" from TRACEBLOC_ALLOW_UNVERIFIED=1. Routing them into a new catch would have quietly bypassed that distinction — leaving them where they are is the smaller and better change.
And reusing Save-BoundedFile rather than adding a second download path keeps one place where the ceiling logic lives. #582 already covers the helper behaviourally, so call-site assertions are the correct addition here, not a re-test of the byte ceiling.
One straggler, and I'd like to know whether it's deliberate rather than assert it's a defect.
After this PR, install.ps1 has exactly one raw Invoke-WebRequest left — Resolve-Tag at line 279:
$resp=Invoke-WebRequest`-Uri "https://github.com/$script:GitHubRepo/releases/latest"`-MaximumRedirection 0-UseBasicParsing -ErrorAction SilentlyContinueHalf your stated risk doesn't apply to it: there's no -OutFile, so nothing reaches disk and the exhaustion half is moot. But the stall half does — and it carries no explicit -TimeoutSec, so it falls back to the platform default (100 s; confirmed against HttpClient's default on pwsh 7.5.2) rather than the 60 s the rest of the script now uses. It's also on the default path, since it runs whenever the version is latest.
So it's bounded, just loosely and implicitly. Given the PR is titled "the remaining post-cosign fetches", it's worth one line either way — a -TimeoutSec 60 for consistency, or a note that a header-only redirect probe is deliberately out of scope. My read is the latter is defensible; I'd just rather it be written down than left for the next person to re-derive.
Two checks still pending, so no verdict this pass. Nothing else outstanding.
…(backend#2544) The one remaining plain Invoke-WebRequest — Resolve-Tag's header-only /releases/latest redirect probe — carried no explicit -TimeoutSec, so it inherited the ~100 s platform default instead of the 60 s the rest of the script now uses. It reads only the Location header (no -OutFile, no body), so there is no size ceiling to add; pinning -TimeoutSec 60 bounds the stall half consistently. On timeout the WebException is caught, $resp is $null, and Resolve-Tag falls through to a clean Fail. Addresses LukasWodka's review on #588. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
aptracebloc
commented
Aug 26, 2026
Thanks for measuring the caps against the live release rather than the table — and good catch on the straggler. Took the Suite still green (31 / 10), |
aptracebloc
commented
Aug 26, 2026
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit d65c996. Configure here.
LukasWodka
commented
Aug 26, 2026
Bugbot passed, and with it as the only context the aggregate rollup computes That's why I checked whether this was org-wide and it isn't — other open PRs are carrying 9–19 contexts and Actions is servicing other repos normally. So this looks like a dropped push event on this commit specifically. Re-firing is the fix. Toggling draft ( Flagging it rather than silently waiting, because "green rollup + zero runs" is indistinguishable from "all green" in every summary view — the board, the PR header, and my own scan's |
LukasWodka
commented
Aug 26, 2026
Correction to my previous comment — I was wrong, and the advice in it would have made things worse. I said this head had zero workflow runs and suggested re-firing them via a draft toggle. Both parts are wrong. The runs exist and have existed since before I looked: My error was mechanical: I queried The real situation is an Actions backlog, not a dropped event. Sampling the org right now: All on What stands from the original comment: the rollup genuinely does read Nothing to do but wait for the queue to drain. Sorry for the noise. |
LukasWodka
commented
Aug 27, 2026
Overnight settles this: these runs are wedged, not queued behind capacity. I've gone back and forth on this PR twice, so here is the measurement rather than another opinion. Your head's runs, still Meanwhile the same repo ran and completed fresh work this morning: So Actions is healthy for new runs while a cohort created in a ~15-minute window yesterday afternoon never started. The visible consequence is the part worth knowing: with only the Correcting myself. Yesterday afternoon I told you there were zero runs and to re-fire; that was my error (abbreviated SHA to Nothing here is about your diff. Everything I raised on the code is settled: on |
The pull_request workflows for the prior head were created during a GitHub Actions runner outage (2026-08-26) and stuck in `queued`; the Build workflow was never dispatched for that commit. Actions is healthy again, so this empty commit fires a fresh `synchronize` to run the required checks. No content change — squash-merge erases this commit. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
LukasWodka
left a comment
There was a problem hiding this comment.
Approving 685bfbd6 — 25 passing, 3 path-skipped, nothing pending or failing, no open threads, mergeable=MERGEABLE.
My one finding from yesterday is addressed, and the answer is better than the fix I was angling for. I'd flagged the raw Invoke-WebRequest left in Resolve-Tag. Converting it to Save-BoundedFile would have been wrong, and the comment now says why: it is a header-only redirect probe (-MaximumRedirection 0 reads the Location header, never a body to disk), so the size ceiling has nothing to bound. The stall half does apply, and that is now pinned:
-TimeoutSec 60# rather than inherit the ~100 s platform defaultwith the failure path spelled out — on timeout the WebException is caught, $resp is $null, and Resolve-Tag falls through to a clean Fail, never a hang. Splitting "bounded size" from "bounded stall" and applying only the half that exists is the right distinction.
The class is fully covered — every fetch is now one or the other:
202 cosign binary Save-BoundedFile 300MB
203 cosign_checksums.txt Save-BoundedFile 1MB 60s
341 CLI binary Save-BoundedFile 200MB
344 SHA256SUMS Save-BoundedFile 1MB 60s
437 <binary>.sig Save-BoundedFile 1MB 60s
438 <binary>.cert Save-BoundedFile 1MB 60s
288 redirect probe plain IWR, -MaximumRedirection 0, -TimeoutSec 60 (documented)
Six artifact fetches bounded, one documented exception. Nothing unbounded remains.
Save-BoundedFile itself is defence-in-depth rather than a single check, which is what makes the caps meaningful: it refuses on the server-declared ContentLength first (cheap), and enforces during the copy (if ($total -gt $MaxBytes) { throw }). A server that lies about or omits Content-Length cannot walk past the cap — only the second check would catch that, and it's there. The partial-file cleanup on rejection is a nice touch too: a refused fetch doesn't leave up to MaxBytes of litter behind.
On the caps themselves, measured against the live release when I first looked: windows-amd64.exe is 49,933,824 B against the 200 MB ceiling — 4× headroom, so it won't trip on ordinary growth — and SHA256SUMS (786 B), .sig (96 B) and .cert (3,248 B) sit far under their 1 MB caps. Sized to catch a hostile or broken server, not to police normal drift.
One process note, since this PR spent the day looking broken through no fault of its own: it was stuck at contexts=1 because its checks were orphaned in yesterday's 15:00–15:32 UTC window along with 54 others across nine repos. 685bfbd6 re-triggering the workflows is what cleared it — the code has not changed since my review.
Value: a compromised or malfunctioning release host can no longer hand the Windows installer an unbounded stream or hang it indefinitely, at the point where it is fetching the very artifacts it is about to trust.
aptracebloc
commented
Aug 27, 2026
CI is fully green now — 25/25 checks pass (Build ×8, Test, Lint, Installer (shell), Bugbot, all quality gates), Heads-up on the delay: the run you saw pending last review got stuck in |
Uh oh!
There was an error while loading. Please reload this page.
Closes tracebloc/backend#2544 — https://github.com/tracebloc/backend/issues/2544
Follow-up to cli#582 / backend#2199, which bounded only the cosign bootstrap fetch (the pre-auth trust root). The same unbounded
Invoke-WebRequestpattern — no size ceiling, no timeout — remained for the four artifacts fetched after it:$baseUrl/$binaryFileSHA256SUMS.sig/.certThese are SHA256- and cosign-verified after download, so a corrupted/oversized body is rejected before use. But an unbounded body can still hang the install or exhaust disk before verification runs — the same DoS #2199 closed. #582 explicitly scoped this out ("those are verified after download, so the pre-auth cosign fetch genuinely is the priority").
Change
Route all four through the existing
Save-BoundedFilehelper (size ceiling + stall timeout) — no new download machinery:SHA256SUMS.sig.certThe binary cap is 200 MB (~4× today's ~50 MB) with a
REVISITnote; the three metadata files reuse the tightcosign_checksums.txtprofile (1 MB / 60 s). The mandatory binary +SHA256SUMSfetches are wrapped so a cap-trip / network failure exits via the cleanFail()one-liner rather than a raw PowerShell error record; the.sig/.certfetches already sit in the existing fail-closed catch.Same caveat as #582 — the caps are facts about the outside world
The suite is hermetic (MemoryStream /
file://), so green says the ceiling works, nothing about whether the numbers are right. I measured every cap against the live release API (sizes above) rather than inferring them.Save-BoundedFile/Copy-StreamBoundedthemselves are unchanged and already behaviourally covered by #582.Tests
Both tiers gain call-site assertions (the helper's byte ceiling is already tested):
install-ps1-verify.sh(string tier): no artifact is fetched to disk with a rawInvoke-WebRequest -OutFile; each of the four goes throughSave-BoundedFile … -MaxBytes.install-ps1-functions.tests.ps1(AST tier): the same invariant asserted structurally — zeroInvoke-WebRequest -OutFilenodes, ≥ 6Save-BoundedFilecall sites, every one carrying-MaxBytes. Robust to reformatting, unlike the grep.Mutation-tested: reverting any fetch to a raw
Invoke-WebRequestreddens both tiers (verified locally). Full suite green —install-ps1-functions: 31 passed,install-ps1-verify: 10 passed.🤖 Generated with Claude Code
Note
Low Risk
Installer-only download hardening with unchanged verification logic; risk is mainly mis-sized caps causing legitimate installs to fail, not security regression.
Overview
Extends backend#2544 by routing the four release artifact downloads (CLI binary,
SHA256SUMS,.sig,.cert) through the existingSave-BoundedFilehelper instead of rawInvoke-WebRequest -OutFile, matching the cosign bootstrap hardening from backend#2199. Caps are 200 MB for the binary and 1 MB / 60 s for the checksum and signature files; mandatory binary + sums fetches now fail viaFail()on bounded-fetch errors.The
Resolve-TagGitHub redirect probe stays a header-onlyInvoke-WebRequestbut addsTimeoutSec 60so it cannot hang on a stalled connection.install-ps1-verify.shandinstall-ps1-functions.tests.ps1add structural checks: no disk writes viaInvoke-WebRequest -OutFile, and everySave-BoundedFilecall includes an explicitMaxBytes.Reviewed by Cursor Bugbot for commit 685bfbd. Bugbot is set up for automated code reviews on this repo. Configure here.