Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 55 additions & 0 deletions scripts/regressions/get-conditional-missing-secure-origin-gate.sh
Original file line number Diff line number Diff line change
@@ -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"
31 changes: 31 additions & 0 deletions src/net/client.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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();
Expand All @@ -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));
Expand Down
Loading