From c9d21600a5c25d3dd98f8552dc12282603cc047f Mon Sep 17 00:00:00 2001 From: indaco Date: Fri, 21 Aug 2026 12:31:55 +0200 Subject: [PATCH] fix(net): refuse a cleartext origin on a conditional GET --- ...-conditional-missing-secure-origin-gate.sh | 55 +++++++++++++++++++ src/net/client.zig | 31 +++++++++++ 2 files changed, 86 insertions(+) create mode 100755 scripts/regressions/get-conditional-missing-secure-origin-gate.sh diff --git a/scripts/regressions/get-conditional-missing-secure-origin-gate.sh b/scripts/regressions/get-conditional-missing-secure-origin-gate.sh new file mode 100755 index 00000000..0248143f --- /dev/null +++ b/scripts/regressions/get-conditional-missing-secure-origin-gate.sh @@ -0,0 +1,55 @@ +#!/usr/bin/env bash +# Regression: a conditional GET must refuse a cleartext origin. +# +# The bug: every url entry point on `HttpClient` gated the caller-supplied URL +# with `requireSecureOrigin` except `getConditional`, which went straight from +# the offline check to the transport. The redirect loop below it already +# refuses an https-to-http hop, so the gap was strictly the initial scheme, +# and since `getConditional` exists to carry caller-supplied headers, that +# cleartext request could carry an `Authorization` header. +# +# The fix adds the guard and pins the entry-point list at comptime, so a +# seventh entry point cannot arrive ungated. +# +# Presenting a real https origin needs a TLS fixture, so the guard is judged +# through the colocated inline unit tests (`lib_tests`). This script builds and +# runs only that binary: no network, and no state beyond the zig cache. + +set -euo pipefail + +ROOT=$(cd "$(dirname "$0")/../.." && pwd) +cd "$ROOT" + +SRC="src/net/client.zig" + +# The source-shape greps are the load-bearing half: drop the guard *and* its +# assertion and the unit binary goes green vacuously. +body=$(awk '/pub fn getConditional\(/,/^ }$/' "$SRC") +if ! grep -Fq -- 'requireSecureOrigin(url, .transport_only)' <<<"$body"; then + echo "FAIL: getConditional has no secure-origin gate" >&2 + exit 1 +fi +if ! grep -Fqs -- 'expectError(error.InsecureUrlScheme, http.getConditional(' "$SRC"; then + echo "FAIL: the entry-point sweep no longer covers getConditional" >&2 + exit 1 +fi +if ! grep -Fqs -- 'entry_points' "$SRC"; then + echo "FAIL: the comptime entry-point roster is missing from $SRC" >&2 + exit 1 +fi + +BIN="$ROOT/zig-out/test-bin/lib_tests" +# Always rebuild - a prebuilt lib_tests could predate the fix. +if ! zig build test-bin >/dev/null 2>&1; then + echo "FAIL: could not build the unit test binary (zig build test-bin)" >&2 + exit 1 +fi + +OUT=$("$BIN" 2>&1) && STATUS=0 || STATUS=$? +if [[ "$STATUS" -ne 0 ]]; then + echo "FAIL: the unit suite is red - a cleartext origin may have reached a conditional GET" >&2 + printf '%s\n' "$OUT" | grep -iE "failed|leaked|panic" >&2 || true + exit 1 +fi + +echo "PASS: getConditional refuses a cleartext origin" diff --git a/src/net/client.zig b/src/net/client.zig index 8871d84b..03f7c211 100644 --- a/src/net/client.zig +++ b/src/net/client.zig @@ -654,6 +654,9 @@ pub const HttpClient = struct { extra_headers: []const std.http.Header, ) !ConditionalResponse { if (self.offline) return error.OfflineRequired; + // Metadata no digest covers it, and the caller may pass an + // `Authorization` header - cleartext is never acceptable here. + try requireSecureOrigin(url, .transport_only); // Stack-buffered header list: today's only callers add at most // `If-None-Match` + `Authorization`. Bump if a real caller exceeds. @@ -1663,6 +1666,33 @@ test "every url entry point refuses a cleartext origin before dialling out" { // A guard added to some of the entry points and not the rest is the whole // failure mode: `headResolved` takes a cask url too, and the type it hands // back decides whether the artifact is fed to `sudo installer`. + + // An assertion only covers the entry points someone remembered to list, so + // the roster is pinned at comptime: a new `pub fn (*HttpClient, []const u8, + // ...)` breaks the build until it is added here and asserted below. Decl + // enumeration sees only `pub`, and the match is on that exact shape - an + // entry point taking its url any other way still needs the guard by hand. + const entry_points = [_][]const u8{ + "get", + "getConditional", + "getWithHeaders", + "getToWriter", + "head", + "headResolved", + }; + comptime { + for (@typeInfo(HttpClient).@"struct".decls) |decl| { + const field = @field(HttpClient, decl.name); + const info = @typeInfo(@TypeOf(field)); + if (info != .@"fn") continue; + const params = info.@"fn".params; + if (params.len < 2) continue; + if (params[0].type != *HttpClient or params[1].type != []const u8) continue; + for (entry_points) |name| { + if (std.mem.eql(u8, name, decl.name)) break; + } else @compileError("url entry point missing from the cleartext sweep: " ++ decl.name); + } + } const a = std.testing.allocator; var http = HttpClient.init(std.Options.debug_io, .empty, a); defer http.deinit(); @@ -1672,6 +1702,7 @@ test "every url entry point refuses a cleartext origin before dialling out" { defer sink.deinit(); try std.testing.expectError(error.InsecureUrlScheme, http.get(url)); + try std.testing.expectError(error.InsecureUrlScheme, http.getConditional(url, null, &.{})); try std.testing.expectError(error.InsecureUrlScheme, http.getWithHeaders(url, &.{}, null, .transport_only)); try std.testing.expectError(error.InsecureUrlScheme, http.getToWriter(url, &.{}, &sink.writer, null)); try std.testing.expectError(error.InsecureUrlScheme, http.head(url));