sec(install): bound the cosign bootstrap fetch (size + timeout)

[aptracebloc]

What

Resolve-Cosign bootstrapped cosign with two raw Invoke-WebRequest calls — no size ceiling and no timeout. cosign is the supply-chain trust root, fetched before anything has authenticated it, so a MITM or a broken mirror could wedge the install on an unbounded, never-ending body.

Fix

Add a small bounded-download path and route both cosign fetches through it:

• Copy-StreamBounded — a pure function that streams the body in 64 KB chunks and throws the moment the bytes actually read would cross the cap. A server that under-declares or omits Content-Length cannot slip a runaway body past it.
• Save-BoundedFile — request glue: HttpWebRequest.Timeout (connect + headers) and ReadWriteTimeout (per-read stall, not a wall-clock cap — deliberately matching install.sh review chore: clear house-rules findings #426 so a slow-but-alive link still finishes), an early Content-Length refusal, then the streamed copy. Inherits the existing TLS 1.2 ServicePointManager floor.

Caps: cosign binary 100 MB (it's ~17 MB at the pinned version — headroom for a future build while still refusing a runaway); cosign_checksums.txt1 MB / 60 s (it's a few KB).

Invoke-WebRequest has -TimeoutSec but no portable max-size knob on the PS 5.1 floor this installer targets, hence the manual stream. Verification logic is untouched — the bootstrapped cosign is still checksum-verified and refused on mismatch; this only bounds the fetch.

Tests (pwsh 7.6.5)

• New hermetic §2b: Copy-StreamBounded passes an under-cap body through and throws over-cap (MemoryStream); Save-BoundedFile end-to-end over a file:// URI, under and over the cap.
• Resolve-Cosign mock re-pointed from shadowing Invoke-WebRequest to Save-BoundedFile.
• install-ps1-verify.sh §5b: asserts the raw cosign fetches are gone and both go through the capped helper.
• Behavioural tier 26/26, verify 8/8, shellcheck --severity=error + bash -n clean.

Follow-up (not in this PR)

The same unbounded-Invoke-WebRequest pattern remains for the CLI binary, SHA256SUMS, and .sig/.cert (those are checksum/signature-verified after download, so lower priority than the pre-auth cosign fetch). Tracked separately rather than widening this cosign-scoped change.

Fixes tracebloc/backend#2199 (cross-repo — keyword won't auto-close; the backend issue is closed by hand on merge).

— drafted with Claude Code

---

Note

Medium Risk
Changes the pre-authentication cosign download path in the installer; incorrect caps or timeout handling could break bootstrap on slow links or leave edge-case resource behavior, but the change is defensive hardening with tests.

Overview
The Windows installer no longer bootstraps cosign with unbounded Invoke-WebRequest calls. It adds Copy-StreamBounded (stream copy with a hard byte ceiling on bytes actually read) and Save-BoundedFile (timeouts on connect/headers and per-read stalls, optional Content-Length early reject, partial-file cleanup on failure, PS 5.1-safe Close()).

Resolve-Cosign now downloads cosign-windows-amd64.exe through Save-BoundedFile with a 300 MB cap and cosign_checksums.txt with 1 MB / 60 s. The progress message reflects the larger Windows cosign size (~180 MB). Checksum verification after download is unchanged.

Tests add hermetic coverage for the cap logic (including multi-chunk overflow) and file:// paths through Save-BoundedFile; Resolve-Cosign mocks shadow Save-BoundedFile instead of Invoke-WebRequest. install-ps1-verify.sh §5b guards against regressing to raw cosign fetches.

^{Reviewed by Cursor Bugbot for commit 3dca580. Bugbot is set up for automated code reviews on this repo. Configure here.}

[LukasWodka]

Reviewed 4528153a. The code is right — I read the ceiling logic closely because it's the pre-auth trust root, and it holds. One coverage gap worth a line of test, then this is good to go.

Copy-StreamBounded is correct. It counts bytes actually read and throws before writing the offending chunk, so $OutFile can never exceed the cap. The boundary is right too: a body of exactly MaxBytes passes (-gt, not -ge), which is the sane reading of a cap. And splitting it out as a pure function so the test can drive it with a MemoryStream — rather than restating the rule in a test — is the right shape.

Save-BoundedFile's layering is right, including the part that looks like a hole and isn't.$resp.ContentLength -gt $MaxBytes returns -1 for a server that declares nothing, so the early reject silently doesn't fire — and that's fine precisely because Copy-StreamBounded is the authority. The comment says so. Good.

The ReadWriteTimeout-as-stall-detector rather than wall-clock choice is right and matches install.sh — a slow-but-alive link on a 17 MB binary must be allowed to finish. Using the legacy WebRequest API is also correct for the PS 5.1 floor, unattractive as it is; Invoke-WebRequest genuinely has no portable size knob there. And the $req -is [HttpWebRequest] guard is what lets the tests run over file:// without a network.

Extracting the real functions from install.ps1 by AST and Invoke-Expression-ing them is the right call — the test drives production source, so it can't drift from a copy.

---

The gap: the multi-chunk streamed ceiling — the exact case the design names as the authority — has no coverage.

Both Save-BoundedFile tests bypass Copy-StreamBounded's loop, and I verified rather than inferred it:

response type : FileWebResponse
ContentLength : 3145728
early reject vs 1MB cap: True

FileWebResponse.ContentLength reports the real file size, so Save-BoundedFile refuses a file over the cap throws at the ContentLength early reject and never reaches the streamed copy. The under-cap case is 2048 bytes, so it doesn't iterate either.

And the MemoryStream over-cap test is 2048 bytes against a 65536-byte buffer — one Read, cap tripped on the first iteration. So $total accumulating across iterations is never exercised anywhere.

That's the one path that matters most: an HTTP server that omits Content-Length skips the early reject entirely, and then a 17 MB-sized body arriving in 64 KB chunks is only stopped by the accumulator. As written, a bug in the accumulation (resetting $total, comparing $n instead of $total) would still pass the whole suite, because every current case decides on the first read.

One-line fix — grow the existing over-cap body past a buffer:

$over= [System.IO.MemoryStream]::new([byte[]]::new(200*1024)) # 4 buffers$sink2= [System.IO.MemoryStream]::new()
$capped=$falsetry { Copy-StreamBounded-Source $over-Dest $sink2-MaxBytes (128*1024) } catch { $capped=$true }
is 'Copy-StreamBounded throws when the body exceeds the cap mid-stream'$capped$true
is 'Copy-StreamBounded stops at the cap rather than writing the whole body' ($sink2.Length-le (128*1024)) $true

The second assertion is the one that makes it bite — it pins that the ceiling actually bounded the output, not just that something threw. Worth mutation-proving with if ($n -gt $MaxBytes) in place of if ($total -gt $MaxBytes): that mutation passes today and should redden after.

---

Two minor notes, neither worth a round:

• A rejected download leaves its partial file behind.finally { $out.Dispose(); $in.Dispose() } closes the handles but doesn't delete $OutFile, so an over-cap fetch leaves up to MaxBytes on disk. Harmless here — Resolve-Cosign returns $null and checksum verification would reject it regardless — but a Remove-Item -Force -ErrorAction SilentlyContinue in that finally on the throw path would keep a 100 MB cap from also being a 100 MB litter budget.
• Scoping the follow-up (CLI binary, SHA256SUMS, .sig/.cert) out of this PR is the right call, and the reason given is the right one — those are verified after download, so the pre-auth cosign fetch genuinely is the priority.

Both Bugbot checks are still pending, so I'm not stamping ahead of the rollup either way. Add the test (or tell me why the single-read coverage is enough) and I'll approve on the next pass.

[LukasWodka]

Bugbot's bugbot / review came back red after my last comment with a High on the cap size. I measured it, and it's right — in fact it understates the problem. Switching to changes-requested; detail inline.

cosign-windows-amd64.exe at the pinned v2.4.1 is 186,998,456 bytes — 178.34 MiB, straight from the release API. The cap is 100MB (104,857,600 bytes). So this isn't "a legitimate fetch can trip the cap" — every bootstrap fetch trips it, and Resolve-Cosign returns $null on all of them. Any user without cosign already on PATH gets a fail-closed exit 1.

And a correction to my own last review: I said "the code is right". The ceiling logic is right, and I stand behind that analysis — I verified the mechanism and never checked the magnitude against the artifact it's sized for. That's the wrong half to skip on a change whose entire content is two constants and the machinery to enforce them.

Worth drawing the general lesson, because it's the interesting part: no test in this PR could have caught this. The behavioural tier drives MemoryStreams and file:// URIs, so 26/26 green says the ceiling works and nothing at all about whether the number is right. The cap is a fact about the outside world, and the suite is hermetic by design. That gap is worth a line in the PR description at least, so the next person doesn't read green as "the constants are checked".

The rest of the PR — Copy-StreamBounded, Save-BoundedFile, the layering, the ReadWriteTimeout choice — stands as reviewed. This is a constant, not a design problem.

My earlier note about the untested multi-chunk ceiling still applies too, and it becomes more relevant once the cap is above 178 MiB: at that size the streamed path is doing real work across thousands of iterations rather than deciding on the first read.

[LukasWodka]

Re-reviewed e9316737. Both findings closed. My change-request is lifted on the merits — the only thing left is the rollup.

The cap now matches reality, and I re-measured rather than reading the diff:

cap 300MB = 314,572,800 B
artifact = 186,998,456 B (cosign-windows-amd64.exe @ v2.4.1)
headroom = 1.68x fits: True

All three places the wrong number lived moved together — the user-facing (~180 MB), the comment, and the constant. Citing the exact byte count in the comment and noting that the Windows build is far larger than the other platforms' ~105 MB is worth more than the fix itself: it tells the next person why the number is what it is, which is the thing that would otherwise get re-guessed. And REVISIT this cap whenever $CosignVersion is bumped is the right way to handle a coupling that genuinely can't be machine-checked from inside this repo — name it at the point where it breaks rather than implying coverage that doesn't exist.

I also checked cosign_checksums.txt: 3,906 B against the 1 MB cap. Fine.

The multi-chunk ceiling test is now real, and the second assertion is what makes it real. Traced it: 200 KB body, 64 KB buffer, 128 KB cap — reads accumulate 65536 → 131072 → and the third read trips at 196608, leaving the sink at exactly 131072. Under the if ($n -gt $MaxBytes) mutation $n is always 65536, never exceeds the cap, nothing throws and the whole 200 KB lands — so $cappedMid goes false and the length assertion fails. Both bite, which is exactly the mutation a single-read test can't see.

That closes the gap I flagged: the streamed ceiling is the sole defence when a server omits Content-Length, and it now has coverage that exercises the accumulation rather than deciding on the first chunk.

---

Nothing outstanding from me; my thread is resolved. Seven checks are still running on this head (Test, four Build matrix legs, both Bugbot jobs), so I'm not stamping ahead of them — I'll approve next pass once they land.

[aptracebloc]

Addressed on e931673:

Bugbot (High) — cosign cap too small: confirmed and fixed. The pinned cosign-windows-amd64.exe at v2.4.1 is 186,998,456 B (~178 MB), not the "~17 MB" the comment claimed — the 100 MB cap would have tripped on every legitimate fetch and fail-closed the installer when cosign isn't already on PATH. Raised to 300 MB (headroom over ~178 MB while still refusing a runaway), corrected the size figure in both the Write-Host and the comment, and added a "REVISIT on $CosignVersion bump" note.

@LukasWodka — streamed-ceiling coverage: added your exact test — a 200 KB body (≈4 read buffers) over a 128 KB cap through Copy-StreamBounded, with the $sink.Length -le cap assertion that pins the output was bounded, not just that it threw. That's the one that mutation-proofs if ($n -gt $MaxBytes). Also took the minor note: Save-BoundedFile now removes the partial file on the throw path (handles disposed first so the delete succeeds on Windows). Functions tier is now 28/0. Re-requesting your review.

— drafted with Claude Code

[aptracebloc]

bugbot run

[aptracebloc]

bugbot run

[LukasWodka]

Re-checked e9316737. My own findings stay closed — the cap and the multi-chunk ceiling are both settled and I'm not reopening them. But bugbot / review went red with a new High, and I think it's right.

$resp.Dispose() at install.ps1:156 is not callable on Windows PowerShell 5.1.

System.Net.WebResponse on .NET Framework implements IDisposableexplicitly — void IDisposable.Dispose() — so it isn't a public member and PowerShell's method resolution can't reach it. You get Method invocation failed because [System.Net.HttpWebResponse] does not contain a method named 'Dispose', thrown from the outer finallyafter a successful fetch. Resolve-Cosign catches it, prints ⚠ couldn't download cosign, returns $null, and fail-closes.

On .NET Core the same type gained a public Dispose(). Which is exactly why nothing you have catches it — I confirmed the asymmetry on the half I can run:

runtime : .NET 9.0.6
response type : System.Net.FileWebResponse
public Dispose/Close here : Close, Dispose ← both public on .NET Core
Close() on pwsh7 : OK

I can't run .NET Framework here, so the 5.1 half is the documented framework difference rather than something I measured — but Close() is public on both, so it's the call that works either way:

} finally {
$resp.Close()
}

The inner finally is fine as-is: $out and $in are Streams, and Stream.Dispose() is public on both frameworks. Only WebResponse has the explicit implementation.

---

Worth naming the pattern, because this is the second one on this PR.

First the cap was calibrated to a size nobody had checked; now an API call that doesn't exist on the target runtime. Neither is a logic error, and neither could be caught by the suite as built — the tests are hermetic (so a constant about the outside world is invisible) and they run on pwsh 7 (so a .NET Framework surface difference is invisible). Both gaps are structural, not oversights.

Which is worth a line somewhere durable, since install.ps1 names PS 5.1 as its floor: anything touching a .NET type's method surface needs a real 5.1 run, or a check that the member is public on the floor. The CLI repo already has a Windows leg in its matrix; if it could execute even a smoke path of Resolve-Cosign under powershell.exe rather than pwsh, this class stops being invisible. That's larger than this PR — but this PR is the second data point, and two is usually where it's worth acting.

My change-request stands on Bugbot's finding, not on anything of mine. One line and this is done.

[LukasWodka]

Re-reviewed 3dca5807. Close() is the right call and the comment is the right comment. Naming the mechanism — explicit IDisposable implementation on .NET Framework, public on both runtimes for Close() — plus the line about pwsh-7 tests not being able to see the 5.1 break means the next person editing this function knows why it isn't the obvious Dispose(). That's the part that stops it being re-broken.

I also see the partial-file cleanup went in on the same pass:

if (-not$ok) { Remove-Item-LiteralPath $OutFile-Force -ErrorAction SilentlyContinue }

That was a throwaway note from my first review and I didn't expect it back — good. A 300 MB cap that also had to be a 300 MB litter budget was a small wart.

So the full set is closed: the cap measured and corrected, the multi-chunk ceiling covered by a test that bites, the PS 5.1 API surface, and the partial file. My change-request is lifted on the merits and no threads are open.

Seven checks are still running (Test, Lint, govulncheck, three Build legs, both Bugbot jobs), so I'm not stamping ahead of the rollup — I'll approve next pass once they land. Nothing needed from you.

[cursor]

✅ 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 3dca580. Configure here.}

[LukasWodka]

Approving 3dca5807 — 25 checks pass, zero open threads, no conflict.

Four rounds, and the two that mattered were both things no test here could have caught:

The cap.100MB against an artifact that is 186,998,456 B — so every bootstrap fetch would have tripped it and fail-closed, on any machine without cosign already on PATH. The suite is hermetic by design, so it exercised the ceiling perfectly and could say nothing about whether the number was right. Now measured, with the byte count and the "Windows build is far larger than the other platforms'" note in the comment, plus REVISIT this cap whenever $CosignVersion is bumped — which is the honest way to handle a coupling that can't be machine-checked from inside this repo.

The API surface.$resp.Dispose() isn't callable on Windows PowerShell 5.1, the stated floor, because .NET Framework implements WebResponse.IDisposable explicitly. It would have thrown after a successful fetch and been read as a download failure. Invisible to a pwsh-7 suite. Close() is public on both, and the comment says why it isn't the obvious Dispose() — which is what stops it being re-broken.

The multi-chunk ceiling test now bites where it matters: 200 KB body, 64 KB buffer, 128 KB cap, so the throw comes from $total accumulating across reads rather than from the first chunk — the exact path that is the only defence when a server omits Content-Length. The length assertion is the half that mutation-proofs if ($n -gt $MaxBytes).

And the core design was right from the first version: counting bytes actually read rather than trusting Content-Length, ReadWriteTimeout as a stall detector rather than a wall-clock cap so a slow-but-alive link on a 180 MB binary still finishes, and splitting the ceiling into a pure function the tests can drive without a network.

Worth carrying forward: two defects on one PR were structurally invisible to the suite — one because it's hermetic, one because it runs on the wrong runtime. If the Windows matrix leg could execute even a smoke path of Resolve-Cosign under powershell.exe rather than pwsh, the second class stops being invisible. Larger than this PR, but two data points is usually where it's worth acting.