diff --git a/.github/scripts/download-r2-object.sh b/.github/scripts/download-r2-object.sh index 4bf11b24..45a07bd0 100755 --- a/.github/scripts/download-r2-object.sh +++ b/.github/scripts/download-r2-object.sh @@ -32,6 +32,29 @@ if [[ "${object_path}" =~ [[:cntrl:]] ]]; then echo "download-r2-object: object path must not contain control characters" >&2 exit 2 fi +# Nothing here percent-encodes. The same raw key bytes are signed into the +# SigV4 canonical request AND handed to curl, so the script is only correct +# while identity encoding is the right encoding. For the keys the workflows +# actually fetch ([A-Za-z0-9._/-]) it is. Outside that set it is not, and each +# way it breaks is quiet: +# +# ' ' curl exit 3, "URL rejected: Malformed input" +# '#' curl truncates the URL at the fragment -> wrong key, or 403 +# '?' the tail becomes a query string, but the canonical query +# string is signed as EMPTY -> 403 SignatureDoesNotMatch +# non-ASCII curl percent-encodes it on the wire AFTER it was signed raw +# -> 403 SignatureDoesNotMatch against a healthy bucket +# +# The fix is NOT to add an RFC 3986 encoder: the aws-CLI branch below encodes +# by botocore's rules, and a second encoder here that disagreed with it would +# be a fresh divergence between the two branches for the same key. Enforce the +# assumption instead, so an unsupported key is a clear refusal before any +# request rather than a 403 that reads like a credentials fault. +object_path_pattern='^[A-Za-z0-9._/-]+$' +if [[ ! "${object_path}" =~ ${object_path_pattern} ]]; then + echo "download-r2-object: object path must match [A-Za-z0-9._/-]+ because the key bytes are signed and sent verbatim with no percent-encoding: ${object_path}" >&2 + exit 2 +fi : "${R2_ACCESS_KEY_ID:?R2_ACCESS_KEY_ID is required}" : "${R2_BUCKET_ENDPOINT:?R2_BUCKET_ENDPOINT is required}" @@ -113,31 +136,8 @@ url="https://${host}${request_path}" region="${R2_REGION:-auto}" service="s3" -amz_date="$(date -u +%Y%m%dT%H%M%SZ)" -date_stamp="${amz_date:0:8}" payload_hash="$(printf '' | shasum -a 256 | awk '{print $1}')" signed_headers="host;x-amz-content-sha256;x-amz-date" -# SigV4 CanonicalRequest is -# METHOD \n URI \n QUERY \n CanonicalHeaders \n SignedHeaders \n PayloadHash -# and CanonicalHeaders is itself "name:value\n" per header -- so a BLANK LINE -# separates the last header from SignedHeaders. This used to spell that -# terminating newline inside canonical_headers' own printf, where command -# substitution ATE it: `$(...)` strips every trailing newline, so the canonical -# request went on the wire one line short, hashed differently from what R2 -# computed for the same request, and R2 answered HTTP 403 SignatureDoesNotMatch -# with a well-formed signature. Never reached on the serial box because its -# runner PATH has the aws CLI, which takes download_with_aws_cli() instead; -# M5-C's runner PATH is /usr/bin:/bin:/usr/sbin:/sbin, so it is the first box to -# execute this signer at all (2026-07-30). -# -# Both newlines are therefore spelled in the canonical_request format string -# ("...%s\n\n%s..."), where nothing can strip them: one terminates the last -# header line, one is the blank separator. -canonical_headers="$(printf 'host:%s\nx-amz-content-sha256:%s\nx-amz-date:%s' "${host}" "${payload_hash}" "${amz_date}")" -canonical_request="$(printf 'GET\n%s\n\n%s\n\n%s\n%s' "${request_path}" "${canonical_headers}" "${signed_headers}" "${payload_hash}")" -credential_scope="${date_stamp}/${region}/${service}/aws4_request" -canonical_request_hash="$(printf '%s' "${canonical_request}" | shasum -a 256 | awk '{print $1}')" -string_to_sign="$(printf 'AWS4-HMAC-SHA256\n%s\n%s\n%s' "${amz_date}" "${credential_scope}" "${canonical_request_hash}")" hmac_hex() { local key_opt="$1" @@ -147,37 +147,87 @@ hmac_hex() { | xxd -p -c 256 } -k_date="$(hmac_hex "key:AWS4${R2_SECRET_ACCESS_KEY}" "${date_stamp}")" -k_region="$(hmac_hex "hexkey:${k_date}" "${region}")" -k_service="$(hmac_hex "hexkey:${k_region}" "${service}")" -k_signing="$(hmac_hex "hexkey:${k_service}" "aws4_request")" -# Signed through hmac_hex, exactly like the four key-derivation steps above. -# It used to run its own openssl pipeline WITHOUT -binary and parse the text -# form with `awk '{print $2}'`, which silently depends on the openssl -# implementation's output format: +# Produce a COMPLETE, self-consistent SigV4 signature for one network attempt: +# amz_date, the canonical request built from it, and the Authorization header +# that covers both. Every retry calls this again. # -# OpenSSL 3.x "SHA2-256(stdin)= " -> $2 is the hex -# LibreSSL 3.3 "" -> $2 is EMPTY, the hex is $1 -# -# macOS ships LibreSSL as /usr/bin/openssl, so on a box without Homebrew -# OpenSSL ahead of it the signature came out EMPTY and R2 answered -# "InvalidArgument: Signature element value should not be blank" -- a 400 that -# looks like a credentials problem and is not one. Diagnosed on M5-C -# (LibreSSL 3.3.6) 2026-07-30. -# -# That diagnosis originally added "the serial box worked only because OpenSSL 3 -# was first on its PATH." That is FALSE, and believing it sends the next -# debugger to audit PATH ordering on a box that never runs this code. The -# serial box has the aws CLI on its runner PATH, so download_with_aws_cli() -# below returns 0 and the signer is never reached: every successful hidden- -# golden fetch in either repo announced "using AWS CLI S3 path-style download", -# never "using signed HTTPS download". Whichever openssl it ships is -# irrelevant. Treat this signed path as covered ONLY by the box that lacks aws. -# -# -binary sidesteps the text format entirely, so there is no field to index. -signature="$(hmac_hex "hexkey:${k_signing}" "${string_to_sign}")" +# It used to run once, before curl, and curl owned the retry (`--retry 5 +# --retry-all-errors`). curl replays the argv it was handed, and the +# Authorization / x-amz-date headers are already fixed in that argv -- measured +# against real R2, all six attempts carried the SAME x-amz-date. A SigV4 +# signature is only accepted inside R2's ~15 minute clock-skew window, so a run +# that spends long enough in retries turns a transient 503 into a +# permanent-looking 403 RequestTimeTooSkewed. curl cannot fix that: only the +# caller can re-sign. Nothing that varies per attempt may be hoisted out of +# this function. +sign_request() { + amz_date="$(date -u +%Y%m%dT%H%M%SZ)" + date_stamp="${amz_date:0:8}" + # SigV4 CanonicalRequest is + # METHOD \n URI \n QUERY \n CanonicalHeaders \n SignedHeaders \n PayloadHash + # and CanonicalHeaders is itself "name:value\n" per header -- so a BLANK LINE + # separates the last header from SignedHeaders. This used to spell that + # terminating newline inside canonical_headers' own printf, where command + # substitution ATE it: `$(...)` strips every trailing newline, so the canonical + # request went on the wire one line short, hashed differently from what R2 + # computed for the same request, and R2 answered HTTP 403 SignatureDoesNotMatch + # with a well-formed signature. Never reached on the serial box because its + # runner PATH has the aws CLI, which takes download_with_aws_cli() instead; + # M5-C's runner PATH is /usr/bin:/bin:/usr/sbin:/sbin, so it is the first box to + # execute this signer at all (2026-07-30). + # + # Both newlines are therefore spelled in the canonical_request format string + # ("...%s\n\n%s..."), where nothing can strip them: one terminates the last + # header line, one is the blank separator. + canonical_headers="$(printf 'host:%s\nx-amz-content-sha256:%s\nx-amz-date:%s' "${host}" "${payload_hash}" "${amz_date}")" + canonical_request="$(printf 'GET\n%s\n\n%s\n\n%s\n%s' "${request_path}" "${canonical_headers}" "${signed_headers}" "${payload_hash}")" + credential_scope="${date_stamp}/${region}/${service}/aws4_request" + canonical_request_hash="$(printf '%s' "${canonical_request}" | shasum -a 256 | awk '{print $1}')" + string_to_sign="$(printf 'AWS4-HMAC-SHA256\n%s\n%s\n%s' "${amz_date}" "${credential_scope}" "${canonical_request_hash}")" + k_date="$(hmac_hex "key:AWS4${R2_SECRET_ACCESS_KEY}" "${date_stamp}")" + k_region="$(hmac_hex "hexkey:${k_date}" "${region}")" + k_service="$(hmac_hex "hexkey:${k_region}" "${service}")" + k_signing="$(hmac_hex "hexkey:${k_service}" "aws4_request")" + # Signed through hmac_hex, exactly like the four key-derivation steps above. + # It used to run its own openssl pipeline WITHOUT -binary and parse the text + # form with `awk '{print $2}'`, which silently depends on the openssl + # implementation's output format: + # + # OpenSSL 3.x "SHA2-256(stdin)= " -> $2 is the hex + # LibreSSL 3.3 "" -> $2 is EMPTY, the hex is $1 + # + # macOS ships LibreSSL as /usr/bin/openssl, so on a box without Homebrew + # OpenSSL ahead of it the signature came out EMPTY and R2 answered + # "InvalidArgument: Signature element value should not be blank" -- a 400 that + # looks like a credentials problem and is not one. Diagnosed on M5-C + # (LibreSSL 3.3.6) 2026-07-30. + # + # That diagnosis originally added "the serial box worked only because OpenSSL 3 + # was first on its PATH." That is FALSE, and believing it sends the next + # debugger to audit PATH ordering on a box that never runs this code. The + # serial box has the aws CLI on its runner PATH, so download_with_aws_cli() + # below returns 0 and the signer is never reached: every successful hidden- + # golden fetch in either repo announced "using AWS CLI S3 path-style download", + # never "using signed HTTPS download". Whichever openssl it ships is + # irrelevant. R2_FORCE_SIGNED=1 now gives this path an execution environment + # that is not a production run; use it, and keep using it. + # + # -binary sidesteps the text format entirely, so there is no field to index. + signature="$(hmac_hex "hexkey:${k_signing}" "${string_to_sign}")" + authorization="AWS4-HMAC-SHA256 Credential=${R2_ACCESS_KEY_ID}/${credential_scope}, SignedHeaders=${signed_headers}, Signature=${signature}" +} + +# Attempt budget. Six attempts is what `--retry 5` gave, kept so the change is +# to WHO retries, not to how many times. The delay is overridable so tests can +# exercise the loop's shape without its wall clock. +max_attempts=6 +retry_delay_seconds="${R2_RETRY_DELAY_SECONDS:-2}" +retry_delay_pattern='^[0-9]+$' +if [[ ! "${retry_delay_seconds}" =~ ${retry_delay_pattern} ]]; then + echo "download-r2-object: R2_RETRY_DELAY_SECONDS must be a non-negative integer" >&2 + exit 2 +fi -authorization="AWS4-HMAC-SHA256 Credential=${R2_ACCESS_KEY_ID}/${credential_scope}, SignedHeaders=${signed_headers}, Signature=${signature}" tmp_path="${output_path}.tmp" umask 077 @@ -216,7 +266,21 @@ download_with_aws_cli() { --no-progress } -if download_with_aws_cli; then +# Which implementation actually performs a transfer is NOT fixed: +# +# * `aws` absent (or the endpoint has no bucket path) -> signed path; +# * `aws` present and succeeds -> aws path; +# * `aws` present but FAILS (missing plugin, bad config, transient error) +# -> download_with_aws_cli returns non-zero and this script falls +# through to the signed path SILENTLY. +# +# So a box can take either branch on different runs of the same workflow, and +# the only record of which one ran is the banner each branch prints. That is +# also how the signer stayed unexecuted long enough to ship two signing bugs: +# it is PREFERRED against, never selected. R2_FORCE_SIGNED=1 skips the +# fallback so the signed path can be exercised deliberately -- by the tests in +# Tests/MLXFastTests/R2RequestExecutionTests.swift, and by hand on any box. +if [[ "${R2_FORCE_SIGNED:-0}" != "1" ]] && download_with_aws_cli; then chmod 600 "${tmp_path}" mv "${tmp_path}" "${output_path}" trap - EXIT @@ -226,22 +290,57 @@ fi echo "download-r2-object: using signed HTTPS download" curl_rc=0 -http_status="$( - curl \ - --fail-with-body \ - --silent \ - --show-error \ - --location \ - --retry 5 \ - --retry-all-errors \ - --retry-delay 2 \ - --write-out '%{http_code}' \ - -H "Authorization: ${authorization}" \ - -H "x-amz-content-sha256: ${payload_hash}" \ - -H "x-amz-date: ${amz_date}" \ - --output "${tmp_path}" \ - "${url}" -)" || curl_rc=$? +http_status="" +attempt=1 +while :; do + # Truncate before every attempt. curl truncates --output only once it OPENS + # that file, which it never does when the attempt dies at connect time or + # times out -- so without this a failed attempt's document survives + # under, or ahead of, the bytes the next attempt writes. The caller verifies + # a pinned sha256 and would report a golden mismatch for a transport + # artefact. + : > "${tmp_path}" + # Fresh amz_date, canonical request and signature for THIS attempt. + sign_request + curl_rc=0 + # --connect-timeout / --max-time are what make an attempt terminate at all. + # There was no bound: a peer that accepts the connection and then sends + # nothing triggers neither a retry nor a timeout, so the transfer "succeeds" + # with an empty body, or the job hangs to its 30-minute limit with no + # diagnosis. --retry/--retry-all-errors/--retry-delay are deliberately gone: + # a curl-level retry replays fixed argv and therefore cannot re-sign. + # --location is deliberately gone too: on a cross-host redirect curl DROPS + # the custom Authorization header but FORWARDS x-amz-date and + # x-amz-content-sha256, so the follow-up arrives unsigned with signing + # headers attached, and on a same-host redirect it re-sends a signature + # bound to the OLD path. R2 path-style issues no legitimate redirects, so + # following one can only turn a loud failure into a confusing one. + http_status="$( + curl \ + --fail-with-body \ + --silent \ + --show-error \ + --connect-timeout 30 \ + --max-time 600 \ + --write-out '%{http_code}' \ + -H "Authorization: ${authorization}" \ + -H "x-amz-content-sha256: ${payload_hash}" \ + -H "x-amz-date: ${amz_date}" \ + --output "${tmp_path}" \ + "${url}" + )" || curl_rc=$? + if [[ "${curl_rc}" -eq 0 ]]; then + break + fi + if [[ "${attempt}" -ge "${max_attempts}" ]]; then + break + fi + echo "download-r2-object: attempt ${attempt}/${max_attempts} failed (curl exit ${curl_rc}, HTTP ${http_status:-none}); re-signing and retrying" >&2 + attempt=$((attempt + 1)) + if [[ "${retry_delay_seconds}" -gt 0 ]]; then + sleep "${retry_delay_seconds}" + fi +done if [[ "${curl_rc}" -ne 0 ]]; then # --fail-with-body keeps --fail's retry/exit semantics but preserves the # response body for diagnostics. The body is only PROVABLY an R2/S3 error diff --git a/.github/scripts/upload-r2-object.sh b/.github/scripts/upload-r2-object.sh index aec728ad..0b892653 100755 --- a/.github/scripts/upload-r2-object.sh +++ b/.github/scripts/upload-r2-object.sh @@ -32,6 +32,29 @@ if [[ "${object_path}" =~ [[:cntrl:]] ]]; then echo "upload-r2-object: object path must not contain control characters" >&2 exit 2 fi +# Nothing here percent-encodes. The same raw key bytes are signed into the +# SigV4 canonical request AND handed to curl, so the script is only correct +# while identity encoding is the right encoding. For the keys the workflows +# actually use ([A-Za-z0-9._/-]) it is. Outside that set it is not, and each +# way it breaks is quiet: +# +# ' ' curl exit 3, "URL rejected: Malformed input" +# '#' curl truncates the URL at the fragment -> wrong key, or 403 +# '?' the tail becomes a query string, but the canonical query +# string is signed as EMPTY -> 403 SignatureDoesNotMatch +# non-ASCII curl percent-encodes it on the wire AFTER it was signed raw +# -> 403 SignatureDoesNotMatch against a healthy bucket +# +# The fix is NOT to add an RFC 3986 encoder: the aws-CLI branch below encodes +# by botocore's rules, and a second encoder here that disagreed with it would +# be a fresh divergence between the two branches for the same key. Enforce the +# assumption instead, so an unsupported key is a clear refusal before any +# request rather than a 403 that reads like a credentials fault. +object_path_pattern='^[A-Za-z0-9._/-]+$' +if [[ ! "${object_path}" =~ ${object_path_pattern} ]]; then + echo "upload-r2-object: object path must match [A-Za-z0-9._/-]+ because the key bytes are signed and sent verbatim with no percent-encoding: ${object_path}" >&2 + exit 2 +fi : "${R2_ACCESS_KEY_ID:?R2_ACCESS_KEY_ID is required}" : "${R2_BUCKET_ENDPOINT:?R2_BUCKET_ENDPOINT is required}" @@ -118,29 +141,23 @@ url="https://${host}${request_path}" region="${R2_REGION:-auto}" service="s3" -amz_date="$(date -u +%Y%m%dT%H%M%SZ)" -date_stamp="${amz_date:0:8}" -payload_hash="$(shasum -a 256 "${input_path}" | awk '{print $1}')" -signed_headers="host;x-amz-content-sha256;x-amz-date" -# SigV4 CanonicalRequest is -# METHOD \n URI \n QUERY \n CanonicalHeaders \n SignedHeaders \n PayloadHash -# and CanonicalHeaders is itself "name:value\n" per header -- so a BLANK LINE -# separates the last header from SignedHeaders. This used to spell that -# terminating newline inside canonical_headers' own printf, where command -# substitution ATE it (`$(...)` strips every trailing newline), producing a -# canonical request one line short of the one R2 computes and a 403 -# SignatureDoesNotMatch with a well-formed signature. See the twin comment in -# download-r2-object.sh; that script is where the fault was observed, and this -# one had the identical construction. +# Hash the file's CONTENT through stdin, never `shasum -a 256 "${path}"`. # -# Both newlines are therefore spelled in the canonical_request format string -# ("...%s\n\n%s..."), where nothing can strip them: one terminates the last -# header line, one is the blank separator. -canonical_headers="$(printf 'host:%s\nx-amz-content-sha256:%s\nx-amz-date:%s' "${host}" "${payload_hash}" "${amz_date}")" -canonical_request="$(printf 'PUT\n%s\n\n%s\n\n%s\n%s' "${request_path}" "${canonical_headers}" "${signed_headers}" "${payload_hash}")" -credential_scope="${date_stamp}/${region}/${service}/aws4_request" -canonical_request_hash="$(printf '%s' "${canonical_request}" | shasum -a 256 | awk '{print $1}')" -string_to_sign="$(printf 'AWS4-HMAC-SHA256\n%s\n%s\n%s' "${amz_date}" "${credential_scope}" "${canonical_request_hash}")" +# perl's shasum (macOS ships it) escapes the FILENAME in its output and marks +# the line by prefixing the digest with a backslash whenever the name contains +# a backslash or a newline: +# +# $ shasum -a 256 'back\slash.json' | awk '{print $1}' +# \b7367c22dfc669fdf6f9fcdb91112e6aee109312a2fe68a1508b00dba48cc9cb (65 chars) +# +# That 65-character non-hex value then lands in BOTH the signed canonical +# request and the x-amz-content-sha256 header, so it is self-consistent and +# still rejected -- R2 compares it against the body it received and answers +# 400/403. A path starting with '-' is worse: shasum parses it as options and +# dies ("Unknown option: d"). Reading from stdin removes the filename from the +# output entirely, so the digest is the content's and nothing else. +payload_hash="$(shasum -a 256 < "${input_path}" | awk '{print $1}')" +signed_headers="host;x-amz-content-sha256;x-amz-date" hmac_hex() { local key_opt="$1" @@ -150,35 +167,82 @@ hmac_hex() { | xxd -p -c 256 } -k_date="$(hmac_hex "key:AWS4${R2_SECRET_ACCESS_KEY}" "${date_stamp}")" -k_region="$(hmac_hex "hexkey:${k_date}" "${region}")" -k_service="$(hmac_hex "hexkey:${k_region}" "${service}")" -k_signing="$(hmac_hex "hexkey:${k_service}" "aws4_request")" -# Signed through hmac_hex, exactly like the four key-derivation steps above. -# It used to run its own openssl pipeline WITHOUT -binary and parse the text -# form with `awk '{print $2}'`, which silently depends on the openssl -# implementation's output format: -# -# OpenSSL 3.x "SHA2-256(stdin)= " -> $2 is the hex -# LibreSSL 3.3 "" -> $2 is EMPTY, the hex is $1 -# -# macOS ships LibreSSL as /usr/bin/openssl, so on a box without Homebrew -# OpenSSL ahead of it the signature came out EMPTY and R2 answered -# "InvalidArgument: Signature element value should not be blank" -- a 400 that -# looks like a credentials problem and is not one. Diagnosed on M5-C -# (LibreSSL 3.3.6) 2026-07-30. +# Produce a COMPLETE, self-consistent SigV4 signature for one network attempt: +# amz_date, the canonical request built from it, and the Authorization header +# that covers both. Every retry calls this again. # -# That diagnosis originally added "the serial box worked only because OpenSSL 3 -# was first on its PATH." That is FALSE, and believing it sends the next -# debugger to audit PATH ordering on a box that never runs this code. The -# serial box has the aws CLI on its runner PATH, so upload_with_aws_cli() below -# returns 0 and the signer is never reached. Whichever openssl it ships is -# irrelevant. Treat this signed path as covered ONLY by the box that lacks aws. -# -# -binary sidesteps the text format entirely, so there is no field to index. -signature="$(hmac_hex "hexkey:${k_signing}" "${string_to_sign}")" +# It used to run once, before curl, and curl owned the retry (`--retry 5 +# --retry-all-errors`). curl replays the argv it was handed, and the +# Authorization / x-amz-date headers are already fixed in that argv -- measured +# against real R2 on the download twin, all six attempts carried the SAME +# x-amz-date. A SigV4 signature is only accepted inside R2's ~15 minute +# clock-skew window, so a run that spends long enough in retries turns a +# transient 503 into a permanent-looking 403 RequestTimeTooSkewed. curl cannot +# fix that: only the caller can re-sign. Nothing that varies per attempt may be +# hoisted out of this function. +sign_request() { + amz_date="$(date -u +%Y%m%dT%H%M%SZ)" + date_stamp="${amz_date:0:8}" + # SigV4 CanonicalRequest is + # METHOD \n URI \n QUERY \n CanonicalHeaders \n SignedHeaders \n PayloadHash + # and CanonicalHeaders is itself "name:value\n" per header -- so a BLANK LINE + # separates the last header from SignedHeaders. This used to spell that + # terminating newline inside canonical_headers' own printf, where command + # substitution ATE it (`$(...)` strips every trailing newline), producing a + # canonical request one line short of the one R2 computes and a 403 + # SignatureDoesNotMatch with a well-formed signature. See the twin comment in + # download-r2-object.sh; that script is where the fault was observed, and this + # one had the identical construction. + # + # Both newlines are therefore spelled in the canonical_request format string + # ("...%s\n\n%s..."), where nothing can strip them: one terminates the last + # header line, one is the blank separator. + canonical_headers="$(printf 'host:%s\nx-amz-content-sha256:%s\nx-amz-date:%s' "${host}" "${payload_hash}" "${amz_date}")" + canonical_request="$(printf 'PUT\n%s\n\n%s\n\n%s\n%s' "${request_path}" "${canonical_headers}" "${signed_headers}" "${payload_hash}")" + credential_scope="${date_stamp}/${region}/${service}/aws4_request" + canonical_request_hash="$(printf '%s' "${canonical_request}" | shasum -a 256 | awk '{print $1}')" + string_to_sign="$(printf 'AWS4-HMAC-SHA256\n%s\n%s\n%s' "${amz_date}" "${credential_scope}" "${canonical_request_hash}")" + k_date="$(hmac_hex "key:AWS4${R2_SECRET_ACCESS_KEY}" "${date_stamp}")" + k_region="$(hmac_hex "hexkey:${k_date}" "${region}")" + k_service="$(hmac_hex "hexkey:${k_region}" "${service}")" + k_signing="$(hmac_hex "hexkey:${k_service}" "aws4_request")" + # Signed through hmac_hex, exactly like the four key-derivation steps above. + # It used to run its own openssl pipeline WITHOUT -binary and parse the text + # form with `awk '{print $2}'`, which silently depends on the openssl + # implementation's output format: + # + # OpenSSL 3.x "SHA2-256(stdin)= " -> $2 is the hex + # LibreSSL 3.3 "" -> $2 is EMPTY, the hex is $1 + # + # macOS ships LibreSSL as /usr/bin/openssl, so on a box without Homebrew + # OpenSSL ahead of it the signature came out EMPTY and R2 answered + # "InvalidArgument: Signature element value should not be blank" -- a 400 that + # looks like a credentials problem and is not one. Diagnosed on M5-C + # (LibreSSL 3.3.6) 2026-07-30. + # + # That diagnosis originally added "the serial box worked only because OpenSSL 3 + # was first on its PATH." That is FALSE, and believing it sends the next + # debugger to audit PATH ordering on a box that never runs this code. The + # serial box has the aws CLI on its runner PATH, so upload_with_aws_cli() below + # returns 0 and the signer is never reached. Whichever openssl it ships is + # irrelevant. R2_FORCE_SIGNED=1 now gives this path an execution environment + # that is not a production run; use it, and keep using it. + # + # -binary sidesteps the text format entirely, so there is no field to index. + signature="$(hmac_hex "hexkey:${k_signing}" "${string_to_sign}")" + authorization="AWS4-HMAC-SHA256 Credential=${R2_ACCESS_KEY_ID}/${credential_scope}, SignedHeaders=${signed_headers}, Signature=${signature}" +} -authorization="AWS4-HMAC-SHA256 Credential=${R2_ACCESS_KEY_ID}/${credential_scope}, SignedHeaders=${signed_headers}, Signature=${signature}" +# Attempt budget. Six attempts is what `--retry 5` gave, kept so the change is +# to WHO retries, not to how many times. The delay is overridable so tests can +# exercise the loop's shape without its wall clock. +max_attempts=6 +retry_delay_seconds="${R2_RETRY_DELAY_SECONDS:-2}" +retry_delay_pattern='^[0-9]+$' +if [[ ! "${retry_delay_seconds}" =~ ${retry_delay_pattern} ]]; then + echo "upload-r2-object: R2_RETRY_DELAY_SECONDS must be a non-negative integer" >&2 + exit 2 +fi upload_with_aws_cli() { local trimmed_path="${base_path#/}" @@ -212,32 +276,79 @@ upload_with_aws_cli() { --no-progress } -if upload_with_aws_cli; then +# Which implementation actually performs a transfer is NOT fixed: +# +# * `aws` absent (or the endpoint has no bucket path) -> signed path; +# * `aws` present and succeeds -> aws path; +# * `aws` present but FAILS (missing plugin, bad config, transient error) +# -> upload_with_aws_cli returns non-zero and this script falls through +# to the signed path SILENTLY. +# +# So a box can take either branch on different runs of the same workflow, and +# the only record of which one ran is the banner each branch prints. That is +# also how the signer stayed unexecuted long enough to ship two signing bugs: +# it is PREFERRED against, never selected. R2_FORCE_SIGNED=1 skips the +# fallback so the signed path can be exercised deliberately -- by the tests in +# Tests/MLXFastTests/R2RequestExecutionTests.swift, and by hand on any box. +if [[ "${R2_FORCE_SIGNED:-0}" != "1" ]] && upload_with_aws_cli; then echo "upload-r2-object: wrote ${object_path}" exit 0 fi echo "upload-r2-object: using signed HTTPS upload" response_body_path="$(mktemp)" +trap 'rm -f "${response_body_path}"' EXIT curl_rc=0 -http_status="$( - curl \ - --fail-with-body \ - --silent \ - --show-error \ - --location \ - --retry 5 \ - --retry-all-errors \ - --retry-delay 2 \ - --write-out '%{http_code}' \ - --request PUT \ - -H "Authorization: ${authorization}" \ - -H "x-amz-content-sha256: ${payload_hash}" \ - -H "x-amz-date: ${amz_date}" \ - --upload-file "${input_path}" \ - --output "${response_body_path}" \ - "${url}" -)" || curl_rc=$? +http_status="" +attempt=1 +while :; do + # Truncate before every attempt: curl opens --output lazily, so an attempt + # that dies at connect time or times out leaves the previous attempt's error + # document in place to be misreported as this one's. + : > "${response_body_path}" + # Fresh amz_date, canonical request and signature for THIS attempt. + sign_request + curl_rc=0 + # --connect-timeout / --max-time are what make an attempt terminate at all. + # There was no bound: a peer that accepts the connection and then sends + # nothing triggers neither a retry nor a timeout, so the job hangs to its + # own limit with no diagnosis. --retry/--retry-all-errors/--retry-delay are + # deliberately gone: a curl-level retry replays fixed argv and therefore + # cannot re-sign. --location is deliberately gone too: on a cross-host + # redirect curl DROPS the custom Authorization header but FORWARDS + # x-amz-date and x-amz-content-sha256, so the follow-up arrives unsigned + # with signing headers attached -- and for a PUT it would also re-send the + # whole body somewhere it was never signed for. On a same-host redirect it + # re-sends a signature bound to the OLD path. R2 path-style issues no + # legitimate redirects. + http_status="$( + curl \ + --fail-with-body \ + --silent \ + --show-error \ + --connect-timeout 30 \ + --max-time 600 \ + --write-out '%{http_code}' \ + --request PUT \ + -H "Authorization: ${authorization}" \ + -H "x-amz-content-sha256: ${payload_hash}" \ + -H "x-amz-date: ${amz_date}" \ + --upload-file "${input_path}" \ + --output "${response_body_path}" \ + "${url}" + )" || curl_rc=$? + if [[ "${curl_rc}" -eq 0 ]]; then + break + fi + if [[ "${attempt}" -ge "${max_attempts}" ]]; then + break + fi + echo "upload-r2-object: attempt ${attempt}/${max_attempts} failed (curl exit ${curl_rc}, HTTP ${http_status:-none}); re-signing and retrying" >&2 + attempt=$((attempt + 1)) + if [[ "${retry_delay_seconds}" -gt 0 ]]; then + sleep "${retry_delay_seconds}" + fi +done if [[ "${curl_rc}" -ne 0 ]]; then # A PUT's response body is server-authored (R2 never echoes the uploaded # bytes back), so printing it cannot leak the artifact -- but keep the diff --git a/Tests/MLXFastTests/BenchmarkScriptTests.swift b/Tests/MLXFastTests/BenchmarkScriptTests.swift index 8deb8d5e..b446a879 100644 --- a/Tests/MLXFastTests/BenchmarkScriptTests.swift +++ b/Tests/MLXFastTests/BenchmarkScriptTests.swift @@ -2382,6 +2382,10 @@ func downloadR2ObjectWithholdsBodyUnlessServerErrorStatus() throws { "R2_SECRET_ACCESS_KEY": "testsecret", "R2_BUCKET_ENDPOINT": "https://r2.example.test", "PATH": shimDir + ":" + inheritedPath, + // The script retries in bash now (re-signing each attempt; see + // R2RequestExecutionTests), so a failing case walks the full attempt + // budget. Keep the attempt count and drop only the delay between them. + "R2_RETRY_DELAY_SECONDS": "0", ] // 200-then-truncated (curl exit 18 after all retries): the temp file is diff --git a/Tests/MLXFastTests/R2RequestExecutionTests.swift b/Tests/MLXFastTests/R2RequestExecutionTests.swift new file mode 100644 index 00000000..8c79e8ff --- /dev/null +++ b/Tests/MLXFastTests/R2RequestExecutionTests.swift @@ -0,0 +1,863 @@ +import Foundation +import Testing + +/// Executes `.github/scripts/{download,upload}-r2-object.sh` end to end. +/// +/// Every other test of these scripts reads them as text or splices a few of +/// their lines into a fresh shell. That is how two signing bugs shipped: the +/// signed path is only ever *taken* when `aws` is missing from PATH, so before +/// M5-C (runner PATH `/usr/bin:/bin:/usr/sbin:/sbin`) it had literally never +/// run anywhere, and its first execution was a production hidden-golden fetch. +/// +/// These tests run the real scripts, unmodified, with `R2_FORCE_SIGNED=1` and +/// with stub `curl`/`date`/`aws` binaries first on PATH. Nothing about the +/// script is faked: argument validation, the SigV4 signer, the retry loop, the +/// temp-file handling and the exit codes are the shipped ones. What is +/// observed is the argv and headers curl was actually handed, attempt by +/// attempt. +/// +/// The stub PATH is deliberately `/usr/bin:/bin:/usr/sbin:/sbin` plus the stub +/// directory — the same minimal PATH M5-C's runner has, so `shasum`, `openssl` +/// (LibreSSL), `xxd` and friends resolve exactly as they do there. +@Suite("R2 request execution") +struct R2RequestExecutionTests { + // MARK: - Harness + + /// One network attempt as the stub curl saw it. + struct Attempt { + var index: Int + /// Bytes already in the `--output` file when the attempt began. The + /// script is required to truncate between attempts, so this must be 0. + var preexistingOutputBytes: Int + var argv: [String] + + func headerValue(_ name: String) -> String? { + let prefix = "\(name): " + for (i, arg) in argv.enumerated() where arg == "-H" || arg == "--header" { + guard i + 1 < argv.count else { continue } + if argv[i + 1].hasPrefix(prefix) { + return String(argv[i + 1].dropFirst(prefix.count)) + } + } + return nil + } + + var authorizationSignature: String? { + guard let auth = headerValue("Authorization"), + let range = auth.range(of: "Signature=") + else { return nil } + return String(auth[range.upperBound...]) + } + + var url: String? { argv.last } + + func flagValue(_ flag: String) -> String? { + guard let i = argv.firstIndex(of: flag), i + 1 < argv.count else { return nil } + return argv[i + 1] + } + } + + struct Run { + var status: Int32 + var stdout: String + var stderr: String + var attempts: [Attempt] + var awsInvocations: [String] + var outputContents: String? + } + + /// A stub `curl`. + /// + /// It models the two behaviours of real curl this concern turns on: + /// + /// * `--retry N` is honoured *inside one process*, replaying the argv it + /// was given. That is precisely the defect — a curl-level retry cannot + /// re-sign, because the Authorization and x-amz-date headers are fixed + /// in argv before curl starts. So the pre-fix script logs N+1 attempts + /// with identical headers, exactly as measured against real R2. + /// * `--max-time` is what converts a stalled-but-open connection into a + /// failure. Without it, the `stall` script below returns HTTP 200 and + /// exit 0 having written nothing — the hang/empty-body outcome the fix + /// exists to prevent. + /// + /// It deliberately does NOT truncate the `--output` file (it appends), so + /// that whether the file is empty at the start of an attempt is a property + /// of the *script*, which is the thing under test. Real curl truncates only + /// once it opens the file, which it never does on a connect-time failure. + private static let curlStub = #""" + #!/bin/bash + set -u + log="${R2_STUB_CURL_LOG}" + counter="${R2_STUB_CURL_COUNTER}" + + out="" + retries=0 + args=("$@") + for ((i = 0; i < ${#args[@]}; i++)); do + case "${args[i]}" in + --output) out="${args[i+1]}" ;; + --retry) retries="${args[i+1]}" ;; + esac + done + + have_max_time=0 + for a in "$@"; do + if [[ "${a}" == "--max-time" ]]; then have_max_time=1; fi + done + + IFS=' ' read -r -a plan <<< "${R2_STUB_PLAN:-200:0}" + + rc=0 + for ((k = 0; k <= retries; k++)); do + n=$(( $(cat "${counter}" 2>/dev/null || echo 0) + 1 )) + printf '%s' "${n}" > "${counter}" + + pre=0 + if [[ -n "${out}" && -e "${out}" ]]; then + pre="$(wc -c < "${out}" | tr -d '[:space:]')" + fi + + { + printf 'ATTEMPT %s\n' "${n}" + printf 'PREEXISTING %s\n' "${pre}" + for a in "$@"; do printf 'ARG %s\n' "${a}"; done + printf 'END\n' + } >> "${log}" + + idx=$(( n - 1 )) + if (( idx >= ${#plan[@]} )); then idx=$(( ${#plan[@]} - 1 )); fi + entry="${plan[idx]}" + + if [[ "${entry}" == "stall" ]]; then + if (( have_max_time )); then + status="000"; rc=28 + else + # Connection alive, no bytes, no timeout: curl reports success + # and the caller is handed an EMPTY file. + status="200"; rc=0 + fi + else + status="${entry%%:*}" + rc="${entry##*:}" + if [[ "${rc}" == "0" ]]; then + [[ -n "${out}" ]] && printf '%s' "${R2_STUB_SUCCESS_BODY:-OK}" >> "${out}" + elif [[ "${status}" != "000" ]]; then + [[ -n "${out}" ]] && printf 'SlowDownstub error body for attempt %s, padded to be conspicuous if it survives into a later attempt' "${n}" >> "${out}" + fi + fi + + if [[ "${rc}" == "0" ]]; then break; fi + done + + printf '%s' "${status}" + exit "${rc}" + """# + + /// A stub `date` that advances 7 minutes per SigV4 clock read. + /// + /// Two attempts therefore straddle R2's ~15-minute clock-skew window, which + /// is the failure a reused signature produces (403 RequestTimeTooSkewed). + /// Anything that is not the SigV4 format string is delegated to /bin/date. + private static let dateStub = #""" + #!/bin/bash + set -u + if [[ "${1:-}" == "-u" && "${2:-}" == "+%Y%m%dT%H%M%SZ" ]]; then + counter="${R2_STUB_DATE_COUNTER}" + n=$(( $(cat "${counter}" 2>/dev/null || echo 0) + 1 )) + printf '%s' "${n}" > "${counter}" + printf '20260730T00%02d00Z' $(( (n - 1) * 7 )) + exit 0 + fi + exec /bin/date "$@" + """# + + /// A stub `aws` that succeeds, so taking the fallback branch is observable. + private static let awsStub = #""" + #!/bin/bash + set -u + { printf 'aws'; for a in "$@"; do printf ' %s' "${a}"; done; printf '\n'; } \ + >> "${R2_STUB_AWS_LOG}" + next_is_dest=0 + for a in "$@"; do + if (( next_is_dest )); then + next_is_dest=0 + case "${a}" in -*) ;; *) printf 'aws-stub-object' > "${a}" ;; esac + fi + case "${a}" in s3://*) next_is_dest=1 ;; esac + done + exit "${R2_STUB_AWS_EXIT:-0}" + """# + + private static let repositoryRoot = FileManager.default.currentDirectoryPath + + private static func write(_ contents: String, to url: URL) throws { + try contents.write(to: url, atomically: true, encoding: .utf8) + try FileManager.default.setAttributes( + [.posixPermissions: 0o755], ofItemAtPath: url.path) + } + + /// Run one of the real scripts with the stubs in front of a minimal PATH. + /// + /// - Parameters: + /// - script: repo-relative script path, executed as itself. + /// - arguments: the script's two positional arguments. + /// - plan: per-attempt stub behaviour, e.g. `"503:22 200:0"` or `"stall"`. + /// - stubAWS: install a succeeding `aws` stub on PATH. + /// - forceSigned: set `R2_FORCE_SIGNED=1`. + /// - readOutputAt: repo/temp path whose contents to return afterwards. + @discardableResult + private static func runScript( + _ script: String, + arguments: [String], + workingDirectory: URL, + plan: String = "200:0", + successBody: String = "the-object-bytes", + stubAWS: Bool = false, + forceSigned: Bool = true, + extraEnvironment: [String: String] = [:], + readOutputAt outputPath: String? = nil + ) throws -> Run { + let stubs = workingDirectory.appendingPathComponent("__stubs") + try FileManager.default.createDirectory(at: stubs, withIntermediateDirectories: true) + try write(curlStub, to: stubs.appendingPathComponent("curl")) + try write(dateStub, to: stubs.appendingPathComponent("date")) + if stubAWS { + try write(awsStub, to: stubs.appendingPathComponent("aws")) + } + + let curlLog = workingDirectory.appendingPathComponent("curl.log") + let awsLog = workingDirectory.appendingPathComponent("aws.log") + + var environment: [String: String] = [ + "PATH": "\(stubs.path):/usr/bin:/bin:/usr/sbin:/sbin", + "HOME": workingDirectory.path, + "R2_ACCESS_KEY_ID": "AKIAIOSFODNN7EXAMPLE", + "R2_SECRET_ACCESS_KEY": "wJalrXUtnFEMI/K7MDENG+bPxRfiCYEXAMPLEKEY", + "R2_BUCKET_ENDPOINT": "https://acct.r2.cloudflarestorage.com/mybucket", + // Keep the loop's shape but not its wall clock. + "R2_RETRY_DELAY_SECONDS": "0", + "R2_STUB_PLAN": plan, + "R2_STUB_SUCCESS_BODY": successBody, + "R2_STUB_CURL_LOG": curlLog.path, + "R2_STUB_CURL_COUNTER": workingDirectory.appendingPathComponent("curl.n").path, + "R2_STUB_DATE_COUNTER": workingDirectory.appendingPathComponent("date.n").path, + "R2_STUB_AWS_LOG": awsLog.path, + ] + if forceSigned { environment["R2_FORCE_SIGNED"] = "1" } + for (key, value) in extraEnvironment { environment[key] = value } + + let process = Process() + process.executableURL = URL(fileURLWithPath: "/bin/bash") + process.arguments = ["\(repositoryRoot)/\(script)"] + arguments + process.currentDirectoryURL = workingDirectory + process.environment = environment + let out = Pipe() + let err = Pipe() + process.standardOutput = out + process.standardError = err + try process.run() + let stdoutData = out.fileHandleForReading.readDataToEndOfFile() + let stderrData = err.fileHandleForReading.readDataToEndOfFile() + process.waitUntilExit() + + return Run( + status: process.terminationStatus, + stdout: String(decoding: stdoutData, as: UTF8.self), + stderr: String(decoding: stderrData, as: UTF8.self), + attempts: parseAttempts(at: curlLog), + awsInvocations: (try? String(contentsOf: awsLog, encoding: .utf8))? + .split(separator: "\n").map(String.init) ?? [], + outputContents: outputPath.flatMap { + let url = + $0.hasPrefix("/") + ? URL(fileURLWithPath: $0) + : workingDirectory.appendingPathComponent($0) + return try? String(contentsOf: url, encoding: .utf8) + } + ) + } + + private static func parseAttempts(at url: URL) -> [Attempt] { + guard let text = try? String(contentsOf: url, encoding: .utf8) else { return [] } + var attempts: [Attempt] = [] + var current: Attempt? + for line in text.split(separator: "\n", omittingEmptySubsequences: false) { + if line.hasPrefix("ATTEMPT ") { + current = Attempt( + index: Int(line.dropFirst("ATTEMPT ".count)) ?? -1, + preexistingOutputBytes: 0, argv: []) + } else if line.hasPrefix("PREEXISTING ") { + current?.preexistingOutputBytes = Int(line.dropFirst("PREEXISTING ".count)) ?? -1 + } else if line.hasPrefix("ARG ") { + current?.argv.append(String(line.dropFirst("ARG ".count))) + } else if line == "END", let attempt = current { + attempts.append(attempt) + current = nil + } + } + return attempts + } + + private static func temporaryDirectory() throws -> URL { + let url = URL(fileURLWithPath: NSTemporaryDirectory()) + .appendingPathComponent("r2-exec-\(UUID().uuidString)") + try FileManager.default.createDirectory(at: url, withIntermediateDirectories: true) + return url + } + + private static let downloadScript = ".github/scripts/download-r2-object.sh" + private static let uploadScript = ".github/scripts/upload-r2-object.sh" + + // MARK: - F2: every attempt is signed afresh + + /// Six attempts, six DIFFERENT x-amz-date values and six DIFFERENT + /// signatures. + /// + /// The retry used to be curl's own (`--retry 5 --retry-all-errors`), and + /// curl replays the argv it was handed — measured against real R2, all six + /// attempts carried the same x-amz-date, because `amz_date` was computed + /// once before curl started. A signature is only valid inside R2's + /// ~15-minute clock-skew window, so a run that spends long enough in + /// retries turns a transient 503 into a permanent-looking 403 + /// RequestTimeTooSkewed. The stub curl honours `--retry` in-process for + /// exactly this reason, so the pre-fix script still produces six logged + /// attempts here — with identical headers. + @Test + func everyRetryIsSignedAfresh() throws { + let workspace = try Self.temporaryDirectory() + let run = try Self.runScript( + Self.downloadScript, + arguments: ["correctness_prompts/golden.json", "out/golden.json"], + workingDirectory: workspace, + plan: "503:22 503:22 503:22 503:22 503:22 200:0", + readOutputAt: "out/golden.json" + ) + + #expect( + run.status == 0, + "download should have recovered on the sixth attempt: \(run.stderr)") + #expect( + run.attempts.count == 6, + """ + expected 6 network attempts, saw \(run.attempts.count). Each must be a \ + SEPARATE curl invocation so the script can re-sign between them; curl's \ + own --retry cannot, because the headers are already fixed in its argv. + """ + ) + + let dates = run.attempts.compactMap { $0.headerValue("x-amz-date") } + #expect(dates.count == run.attempts.count, "every attempt must send x-amz-date") + #expect( + Set(dates).count == run.attempts.count, + """ + attempts reused an x-amz-date: \(dates). A retried request must carry a \ + FRESH date or it can fall outside R2's ~15 min skew window and be \ + rejected 403 RequestTimeTooSkewed no matter how healthy the bucket is. + """ + ) + + let signatures = run.attempts.compactMap { $0.authorizationSignature } + #expect(signatures.count == run.attempts.count, "every attempt must be signed") + #expect( + Set(signatures).count == run.attempts.count, + """ + attempts reused a SigV4 signature: \(signatures). The signature covers \ + x-amz-date, so a fresh date with a stale signature is just as dead as a \ + stale date. + """ + ) + for signature in signatures { + #expect( + signature.count == 64 && signature.allSatisfy { $0.isHexDigit }, + "signature \(signature) is not a 64-hex SigV4 signature") + } + + #expect( + run.outputContents == "the-object-bytes", + "final file should hold only the successful attempt's body, got \(run.outputContents ?? "")" + ) + } + + /// The same property for the upload script. + @Test + func everyUploadRetryIsSignedAfresh() throws { + let workspace = try Self.temporaryDirectory() + let payload = workspace.appendingPathComponent("payload.json") + try "{\"score\":1}".write(to: payload, atomically: true, encoding: .utf8) + + let run = try Self.runScript( + Self.uploadScript, + arguments: ["scores/run.json", payload.path], + workingDirectory: workspace, + plan: "500:22 500:22 200:0" + ) + + #expect(run.status == 0, "upload should have recovered: \(run.stderr)") + #expect(run.attempts.count == 3, "expected 3 separate curl invocations") + let dates = run.attempts.compactMap { $0.headerValue("x-amz-date") } + let signatures = run.attempts.compactMap { $0.authorizationSignature } + #expect(Set(dates).count == 3, "upload attempts reused an x-amz-date: \(dates)") + #expect( + Set(signatures).count == 3, + "upload attempts reused a signature: \(signatures)") + } + + // MARK: - F2: timeouts + + /// A stalled attempt must fail and be retried, not hang. + /// + /// Without `--max-time`, a connection that stays open and delivers nothing + /// triggers neither curl's retry nor any timeout: the transfer "succeeds" + /// with an empty body and hangs the job to its 30-minute limit in the worst + /// case. The stub reproduces that: on a `stall` attempt it returns HTTP 200 + /// / exit 0 having written nothing *unless* `--max-time` is present, in + /// which case it reports exit 28 like real curl. + @Test + func aStalledAttemptTimesOutAndIsRetriedRatherThanSucceedingEmpty() throws { + let workspace = try Self.temporaryDirectory() + let run = try Self.runScript( + Self.downloadScript, + arguments: ["correctness_prompts/golden.json", "out/golden.json"], + workingDirectory: workspace, + plan: "stall stall 200:0", + readOutputAt: "out/golden.json" + ) + + #expect(run.status == 0, "download should have recovered: \(run.stderr)") + #expect( + run.outputContents == "the-object-bytes", + """ + the script accepted a stalled transfer: wrote \ + \(run.outputContents.map { "\($0.count) byte(s)" } ?? "no file") instead of \ + the object. A stalled 200 delivers no bytes and, without --max-time, \ + reports success — the caller then fails a pinned-sha256 check for a \ + reason that has nothing to do with the object. + """ + ) + #expect( + run.attempts.count == 3, + "expected the two stalled attempts to be abandoned and retried, saw \(run.attempts.count)" + ) + } + + /// Both scripts must bound connect and total transfer time on every attempt. + @Test + func everyAttemptBoundsConnectAndTotalTime() throws { + for (script, arguments, needsPayload) in [ + (Self.downloadScript, ["correctness_prompts/g.json", "out/g.json"], false), + (Self.uploadScript, ["scores/run.json", "payload.json"], true), + ] as [(String, [String], Bool)] { + let workspace = try Self.temporaryDirectory() + if needsPayload { + try "x".write( + to: workspace.appendingPathComponent("payload.json"), + atomically: true, encoding: .utf8) + } + let run = try Self.runScript( + script, arguments: arguments, workingDirectory: workspace, + plan: "503:22 200:0") + + #expect(run.status == 0, "\(script) failed: \(run.stderr)") + #expect(!run.attempts.isEmpty, "\(script) never invoked curl") + for attempt in run.attempts { + #expect( + attempt.flagValue("--connect-timeout") == "30", + """ + \(script) attempt \(attempt.index) has no --connect-timeout 30. \ + A dead peer then blocks on the OS default, and later retries can \ + fall outside R2's clock-skew window. + """ + ) + #expect( + attempt.flagValue("--max-time") == "600", + """ + \(script) attempt \(attempt.index) has no --max-time 600. A \ + connection that stalls mid-body then never returns, and the CI \ + job dies on its own limit with no diagnosis. + """ + ) + } + } + } + + /// The retry must not be delegated back to curl. + @Test + func noAttemptDelegatesRetryToCurl() throws { + let workspace = try Self.temporaryDirectory() + let run = try Self.runScript( + Self.downloadScript, + arguments: ["correctness_prompts/g.json", "out/g.json"], + workingDirectory: workspace, + plan: "503:22 200:0") + + #expect(!run.attempts.isEmpty) + for attempt in run.attempts { + #expect( + !attempt.argv.contains("--retry") && !attempt.argv.contains("--retry-all-errors"), + """ + attempt \(attempt.index) hands the retry to curl. curl replays its \ + argv, so the Authorization and x-amz-date it retries with are the \ + ones computed before it started: a retry that cannot re-sign. + """ + ) + } + } + + // MARK: - F2: the output file is truncated between attempts + + /// A failed attempt's error document must not survive into the next one. + @Test + func theOutputFileIsTruncatedBetweenAttempts() throws { + let workspace = try Self.temporaryDirectory() + let run = try Self.runScript( + Self.downloadScript, + arguments: ["correctness_prompts/golden.json", "out/golden.json"], + workingDirectory: workspace, + plan: "503:22 503:22 200:0", + readOutputAt: "out/golden.json" + ) + + #expect(run.status == 0, "download should have recovered: \(run.stderr)") + for attempt in run.attempts { + #expect( + attempt.preexistingOutputBytes == 0, + """ + attempt \(attempt.index) began with \(attempt.preexistingOutputBytes) \ + byte(s) already in the --output file. curl truncates only once it \ + opens that file, which it never does when the attempt dies at connect \ + time, so a previous attempt's body can sit under — or ahead \ + of — the real object. The caller verifies a pinned sha256 and would \ + report a golden mismatch for a transport artefact. + """ + ) + } + #expect( + run.outputContents == "the-object-bytes", + """ + the downloaded file is not exactly the object: \ + \(run.outputContents ?? "") + """ + ) + } + + // MARK: - F3: the object key charset is enforced + + /// Keys this script cannot sign correctly must be refused before any + /// request is made, with a message that says why. + @Test + func unsignableObjectKeysAreRefusedBeforeAnyRequest() throws { + let rejected = [ + "correctness_prompts/has space.json", + "correctness_prompts/frag#ment.json", + "correctness_prompts/query?x=1.json", + "correctness_prompts/caf\u{00e9}.json", + "correctness_prompts/star*.json", + "correctness_prompts/semi;colon.json", + ] + + for (script, secondArgument, needsPayload) in [ + (Self.downloadScript, "out/o.json", false), + (Self.uploadScript, "payload.json", true), + ] as [(String, String, Bool)] { + for key in rejected { + let workspace = try Self.temporaryDirectory() + if needsPayload { + try "x".write( + to: workspace.appendingPathComponent("payload.json"), + atomically: true, encoding: .utf8) + } + let run = try Self.runScript( + script, arguments: [key, secondArgument], + workingDirectory: workspace) + + #expect( + run.status == 2, + """ + \(script) accepted the unsignable key \(key) (exit \(run.status)). \ + Nothing here percent-encodes, so the key bytes are signed verbatim \ + and handed to curl verbatim: a space is curl exit 3, '#' truncates \ + at the fragment, '?' starts a query string, and non-ASCII is \ + percent-encoded on the wire after being signed raw — the last three \ + arrive as 403 SignatureDoesNotMatch against a healthy bucket. + """ + ) + #expect( + run.stderr.contains("[A-Za-z0-9._/-]"), + """ + \(script) rejected \(key) without naming the enforced charset. \ + stderr was: \(run.stderr) + """ + ) + #expect( + run.attempts.isEmpty, + "\(script) issued \(run.attempts.count) request(s) for \(key) anyway") + } + } + } + + /// The invariant must not cost the keys the workflows actually fetch. + /// + /// Read out of the ranked workflows rather than restated, so a new object + /// key that the charset would reject fails here instead of on the runner. + @Test + func theObjectKeysTheWorkflowsUseAreStillAccepted() throws { + var keys: Set = [] + for workflow in [ + ".github/workflows/benchmark.yml", ".github/workflows/dflash-benchmark.yml", + ] { + let text = try String(contentsOfFile: workflow, encoding: .utf8) + for line in text.split(separator: "\n") { + let trimmed = line.trimmingCharacters(in: .whitespaces) + guard let colon = trimmed.range(of: ": "), + trimmed[..= 3, + "found only \(keys.count) literal *_R2_PATH keys in the workflows; retarget this test" + ) + + for key in keys.sorted() { + let workspace = try Self.temporaryDirectory() + let run = try Self.runScript( + Self.downloadScript, arguments: [key, "out/o.json"], + workingDirectory: workspace, readOutputAt: "out/o.json") + #expect( + run.status == 0, + """ + the object-key charset now rejects a key the ranked workflow really \ + fetches: \(key). stderr: \(run.stderr) + """ + ) + #expect(run.outputContents == "the-object-bytes") + } + } + + // MARK: - F4: the payload hash survives exotic input paths + + /// `x-amz-content-sha256` must be the digest of the file's CONTENT, whatever + /// the file is called. + /// + /// `shasum -a 256 "${path}"` prefixes the digest with a backslash and + /// escapes the name when the path holds a backslash or a newline, so the + /// signed hash becomes a 65-character non-hex string; a path starting with + /// '-' is parsed as an option and yields nothing at all. + @Test + func thePayloadHashIsTheContentDigestForExoticInputPaths() throws { + let body = "payload-content" + // sha256("payload-content") + let expected = "b7367c22dfc669fdf6f9fcdb91112e6aee109312a2fe68a1508b00dba48cc9cb" + + for name in ["back\\slash.json", "new\nline.json", "-dash.json", "plain.json"] { + let workspace = try Self.temporaryDirectory() + try body.write( + to: workspace.appendingPathComponent(name), atomically: true, encoding: .utf8) + + // Relative, so a leading '-' really is exposed to argument parsing. + let run = try Self.runScript( + Self.uploadScript, arguments: ["scores/run.json", name], + workingDirectory: workspace) + + #expect( + run.status == 0, + """ + upload of a file named \(name.debugDescription) failed (exit \ + \(run.status)): \(run.stderr) + """ + ) + guard let attempt = run.attempts.first else { + Issue.record("no request was made for \(name.debugDescription)") + continue + } + #expect( + attempt.headerValue("x-amz-content-sha256") == expected, + """ + x-amz-content-sha256 for \(name.debugDescription) was \ + \(attempt.headerValue("x-amz-content-sha256") ?? ""), expected \ + \(expected) — the digest of the file's bytes. The same value is signed \ + into the canonical request, so a mangled one is a 403 \ + SignatureDoesNotMatch that reads as a credentials fault. + """ + ) + #expect( + attempt.headerValue("Authorization")?.contains("Signature=") == true, + "the request for \(name.debugDescription) was not signed") + } + } + + // MARK: - F5: no redirect following + + /// `--location` must be gone from both scripts. + /// + /// Measured against real curl 8.7.1 (macOS system curl, the one the runners + /// use) with two local servers and `-H Authorization: … -H x-amz-date: … + /// -H x-amz-content-sha256: …`: + /// + /// cross-host hop 1 (127.0.0.1) authorization PRESENT + /// cross-host hop 2 (localhost, 302) authorization ABSENT + /// x-amz-date PRESENT + /// x-amz-content-sha256 PRESENT + /// same-host hop 2 (/other-path) authorization PRESENT (signature + /// still bound to /start) + /// + /// So a cross-host redirect delivers an UNSIGNED request that still carries + /// the signing headers, and a same-host redirect delivers a signature for + /// the wrong path. R2 path-style issues no legitimate redirects, so + /// following one can only turn a loud failure into a confusing one — and, + /// on a cross-host hop, ship the request somewhere it was never signed for. + /// + /// This asserts on the argv curl was actually handed by the real script, + /// not on the script's text. + @Test + func noAttemptFollowsRedirects() throws { + for (script, arguments, needsPayload) in [ + (Self.downloadScript, ["correctness_prompts/g.json", "out/g.json"], false), + (Self.uploadScript, ["scores/run.json", "payload.json"], true), + ] as [(String, [String], Bool)] { + let workspace = try Self.temporaryDirectory() + if needsPayload { + try "x".write( + to: workspace.appendingPathComponent("payload.json"), + atomically: true, encoding: .utf8) + } + let run = try Self.runScript( + script, arguments: arguments, workingDirectory: workspace) + + #expect(!run.attempts.isEmpty, "\(script) never invoked curl") + for attempt in run.attempts { + #expect( + !attempt.argv.contains("--location") && !attempt.argv.contains("-L"), + """ + \(script) attempt \(attempt.index) follows redirects. curl drops \ + the Authorization header across hosts but keeps x-amz-date and \ + x-amz-content-sha256, so the follow-up request arrives unsigned \ + with signing headers attached — a 400/403 pointing at the wrong \ + cause. + """ + ) + } + } + } + + // MARK: - F6: the signed path has an execution environment + + /// `R2_FORCE_SIGNED=1` must bypass the aws CLI even when `aws` works. + /// + /// Without this the signer is reachable only by *not* having `aws` + /// installed, which is why it went unexecuted until a production run on + /// M5-C. Everything above depends on this knob. + @Test + func forceSignedBypassesAWorkingAWSCLI() throws { + for (script, arguments, needsPayload, banner) in [ + ( + Self.downloadScript, ["correctness_prompts/g.json", "out/g.json"], false, + "download-r2-object: using signed HTTPS download" + ), + ( + Self.uploadScript, ["scores/run.json", "payload.json"], true, + "upload-r2-object: using signed HTTPS upload" + ), + ] as [(String, [String], Bool, String)] { + // Baseline: `aws` present and working -> the fallback is taken and + // the signer never runs. This is the production reality on M5-A. + let fallbackWorkspace = try Self.temporaryDirectory() + if needsPayload { + try "x".write( + to: fallbackWorkspace.appendingPathComponent("payload.json"), + atomically: true, encoding: .utf8) + } + let fallback = try Self.runScript( + script, arguments: arguments, workingDirectory: fallbackWorkspace, + stubAWS: true, forceSigned: false) + #expect(fallback.status == 0, "\(script) aws fallback failed: \(fallback.stderr)") + #expect( + !fallback.awsInvocations.isEmpty, + "\(script) did not use the aws CLI even though it was on PATH") + #expect( + fallback.attempts.isEmpty, + """ + \(script) reached the signer with a working aws CLI on PATH. If that \ + is now the default, the premise of this test has changed — but note \ + the whole point: the signer historically never ran, so it was never \ + tested. + """ + ) + + // With the knob: same PATH, same working `aws`, signer runs anyway. + let signedWorkspace = try Self.temporaryDirectory() + if needsPayload { + try "x".write( + to: signedWorkspace.appendingPathComponent("payload.json"), + atomically: true, encoding: .utf8) + } + let signed = try Self.runScript( + script, arguments: arguments, workingDirectory: signedWorkspace, + stubAWS: true, forceSigned: true) + #expect(signed.status == 0, "\(script) signed path failed: \(signed.stderr)") + #expect( + signed.awsInvocations.isEmpty, + """ + R2_FORCE_SIGNED=1 did not skip the aws CLI for \(script); it ran \ + \(signed.awsInvocations.count) time(s). Without a way to force the \ + signed path, the SigV4 signer can only be exercised by uninstalling \ + aws — i.e. never, until a production run does it by accident. + """ + ) + #expect( + !signed.attempts.isEmpty, + "R2_FORCE_SIGNED=1 did not reach the signed HTTPS path for \(script)") + #expect( + signed.stdout.contains(banner), + "\(script) did not announce the signed path; stdout: \(signed.stdout)") + } + } + + /// The request the signer actually emits must be the one it signed. + /// + /// A whole-script check that the URL, the three signed headers and the + /// SignedHeaders list agree — the pieces the extraction-based tests in + /// R2SignatureTests verify in isolation, here observed on the wire. + @Test + func theEmittedRequestMatchesWhatWasSigned() throws { + let workspace = try Self.temporaryDirectory() + let run = try Self.runScript( + Self.downloadScript, + arguments: ["correctness_prompts/laguna/golden.json", "out/g.json"], + workingDirectory: workspace, readOutputAt: "out/g.json") + + #expect(run.status == 0, "download failed: \(run.stderr)") + let attempt = try #require(run.attempts.first) + #expect( + attempt.url + == "https://acct.r2.cloudflarestorage.com/mybucket/correctness_prompts/laguna/golden.json" + ) + // Empty-payload digest: the GET body is empty and must be signed as such. + #expect( + attempt.headerValue("x-amz-content-sha256") + == "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855") + let authorization = try #require(attempt.headerValue("Authorization")) + #expect(authorization.hasPrefix("AWS4-HMAC-SHA256 Credential=AKIAIOSFODNN7EXAMPLE/")) + #expect( + authorization.contains("SignedHeaders=host;x-amz-content-sha256;x-amz-date"), + """ + SignedHeaders must list exactly the headers sent, sorted. Authorization \ + was: \(authorization) + """ + ) + let amzDate = try #require(attempt.headerValue("x-amz-date")) + #expect( + authorization.contains("/\(amzDate.prefix(8))/auto/s3/aws4_request"), + """ + the credential scope's date must be the x-amz-date's day. A mismatch is \ + 403 SignatureDoesNotMatch. Authorization was: \(authorization) + """ + ) + #expect(run.outputContents == "the-object-bytes") + } +} diff --git a/Tests/MLXFastTests/R2SignatureTests.swift b/Tests/MLXFastTests/R2SignatureTests.swift index af90b3b7..67cb7c09 100644 --- a/Tests/MLXFastTests/R2SignatureTests.swift +++ b/Tests/MLXFastTests/R2SignatureTests.swift @@ -33,6 +33,60 @@ struct R2SignatureTests { ".github/scripts/upload-r2-object.sh", ] + /// Lift the named shell assignments out of a script, de-indented, in source + /// order, so a test can re-run the SHIPPED construction rather than a + /// restatement of it. + /// + /// De-indenting matters: the signing steps live inside `sign_request()` + /// now (they must be recomputed per network attempt, see + /// R2RequestExecutionTests), so they are no longer at column 0. + private static func assignments(in text: String, named names: [String]) -> String { + text + .split(separator: "\n", omittingEmptySubsequences: false) + .map { $0.trimmingCharacters(in: .whitespaces) } + .filter { line in names.contains { line.hasPrefix("\($0)=") } } + .joined(separator: "\n") + } + + /// The script's own `hmac_hex()` definition, extracted verbatim. + private static func hmacDefinition(in text: String, path: String) throws -> String { + let start = try #require( + text.range(of: "hmac_hex() {"), + "\(path) no longer defines hmac_hex()" + ) + let tail = text[start.lowerBound...] + let end = try #require( + tail.range(of: "\n}"), + "\(path) hmac_hex() definition is unterminated" + ) + return String(tail[.. ShellResult { + let process = Process() + process.executableURL = URL(fileURLWithPath: "/bin/bash") + process.arguments = ["-c", program] + let out = Pipe() + let err = Pipe() + process.standardOutput = out + process.standardError = err + try process.run() + let outData = out.fileHandleForReading.readDataToEndOfFile() + let errData = err.fileHandleForReading.readDataToEndOfFile() + process.waitUntilExit() + return ShellResult( + status: process.terminationStatus, + stdout: String(decoding: outData, as: UTF8.self), + stderr: String(decoding: errData, as: UTF8.self) + ) + } + /// Every `openssl dgst` must use `-binary`, so no output-format field index /// exists to get wrong. @Test @@ -95,16 +149,7 @@ struct R2SignatureTests { // Extract the real hmac_hex definition rather than restating it, so // this test tracks the script instead of a copy of it. - let start = try #require( - text.range(of: "hmac_hex() {"), - "\(path) no longer defines hmac_hex()" - ) - let tail = text[start.lowerBound...] - let end = try #require( - tail.range(of: "\n}"), - "\(path) hmac_hex() definition is unterminated" - ) - let definition = String(tail[..= 2, "unexpected harness output: \(result.stdout)") + guard head.count >= 2 else { continue } + + #expect( + String(head[0]) == expectedHash, + "\(path) hashed the AWS docs canonical request to \(head[0]), expected \(expectedHash)" + ) + + #expect( + stringToSign == expectedStringToSign, + """ + \(path) built a string-to-sign the AWS documentation does not \ + recognise. + + got: + \(stringToSign.debugDescription) + + AWS docs worked example: + \(expectedStringToSign.debugDescription) + """ + ) + + // Say WHICH line moved; a signature mismatch alone does not. + let lines = stringToSign.split(separator: "\n", omittingEmptySubsequences: false) + #expect( + lines.count == 4, + """ + \(path) string-to-sign has \(lines.count) lines, expected 4 \ + (algorithm, x-amz-date, credential scope, canonical-request HASH). + """ + ) + if lines.count == 4 { + #expect(lines[0] == "AWS4-HMAC-SHA256", "\(path) line 1 must be the algorithm") + #expect( + lines[1] == "20150830T123600Z", + "\(path) line 2 must be the full x-amz-date, not the date stamp") + #expect( + lines[2] == "20150830/us-east-1/iam/aws4_request", + "\(path) line 3 must be the credential scope") + #expect( + lines[3] == expectedHash, + """ + \(path) line 4 must be the HASH of the canonical request, not the \ + canonical request itself. + """ + ) + } + + #expect( + String(head[1]) == expectedSignature, + """ + \(path) produced signature \(head[1]) for the AWS documentation's \ + worked example, expected \(expectedSignature). + """ + ) + } + } + + /// One pinned end-to-end signature per script, composing every step. + /// + /// The other tests each cover a link: canonical request (vs botocore), + /// string-to-sign (vs the AWS docs), HMAC derivation (vs a pinned digest). + /// None composes them, so a mismatch BETWEEN links — `string_to_sign` fed a + /// stale `canonical_request_hash`, `credential_scope` built from a + /// different region than the one keyed into `k_region`, `authorization` + /// advertising SignedHeaders the canonical request did not use — passes + /// every one of them. + /// + /// This runs the shipped chain start to finish over R2-shaped inputs and + /// pins the final Signature. The intermediate canonical-request hashes are + /// the same botocore-verified values pinned above, so the end-to-end pin + /// inherits that independent verification rather than restating the script. + @Test + func theWholeSigningChainReproducesOnePinnedEndToEndSignature() throws { + let cases: + [( + path: String, requestPath: String, payloadHash: String, amzDate: String, + canonicalRequestHash: String, signature: String + )] = [ + ( + ".github/scripts/download-r2-object.sh", + "/mybucket/correctness_prompts/x.json", + "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", + "20260731T000829Z", + "18ec091efbf67625090e1e4b4faa2909228244578b5802a1b8222ad60a7096ad", + "01223ee753030a5657b87ba4ea367e8e3d637ba79daf09687433c00fefcefbd8" + ), + ( + ".github/scripts/upload-r2-object.sh", + "/mybucket/up/x.json", + "e38af85860a1452206b018e69c01595e89ce0626bd0068d69ea1b270e993cd41", + "20260731T001036Z", + "48c8f7da8097d5493dcb5140f1f66b1fd9c609d67a70c68637127dc60ab6651a", + "22f0a1d8193784980b800a5da2d86ffcd2188239b1f3c7c84b17e4725d810c75" + ), + ] + + for testCase in cases { + let text = try String(contentsOfFile: testCase.path, encoding: .utf8) + let chain = Self.assignments( + in: text, + named: [ + "canonical_headers", "canonical_request", "credential_scope", + "canonical_request_hash", "string_to_sign", "k_date", "k_region", + "k_service", "k_signing", "signature", "authorization", + ] + ) + #expect( + chain.contains("authorization="), + "\(testCase.path) no longer assembles the Authorization header") + + let program = """ + set -euo pipefail + \(try Self.hmacDefinition(in: text, path: testCase.path)) + host='acct.r2.cloudflarestorage.com' + region='auto' + service='s3' + request_path='\(testCase.requestPath)' + payload_hash='\(testCase.payloadHash)' + amz_date='\(testCase.amzDate)' + date_stamp="${amz_date:0:8}" + signed_headers='host;x-amz-content-sha256;x-amz-date' + R2_ACCESS_KEY_ID='AKIAIOSFODNN7EXAMPLE' + R2_SECRET_ACCESS_KEY='wJalrXUtnFEMI/K7MDENG+bPxRfiCYEXAMPLEKEY' + \(chain) + printf '%s\\n%s\\n%s\\n' \ + "${canonical_request_hash}" "${signature}" "${authorization}" + """ + + let result = try Self.runBash(program) + #expect( + result.status == 0, + "\(testCase.path) signing chain failed to run: \(result.stderr)") + let lines = result.stdout.split(separator: "\n", omittingEmptySubsequences: false) + #expect(lines.count >= 3, "unexpected harness output: \(result.stdout)") + guard lines.count >= 3 else { continue } + + #expect( + String(lines[0]) == testCase.canonicalRequestHash, + """ + \(testCase.path) canonical-request hash \(lines[0]), expected \ + \(testCase.canonicalRequestHash) (botocore's value for the same request). + """ + ) + #expect( + String(lines[1]) == testCase.signature, + """ + \(testCase.path) end-to-end signature \(lines[1]), expected \ + \(testCase.signature). Every link is pinned separately and they all \ + still pass when the chain is wired up wrong, so this is the assertion \ + that catches a step feeding the wrong value to the next one. + """ + ) + #expect( + String(lines[2]) == """ + AWS4-HMAC-SHA256 Credential=AKIAIOSFODNN7EXAMPLE/\ + \(testCase.amzDate.prefix(8))/auto/s3/aws4_request, \ + SignedHeaders=host;x-amz-content-sha256;x-amz-date, \ + Signature=\(testCase.signature) + """, + """ + \(testCase.path) Authorization header is not the one the signature \ + belongs to: \(lines[2]) + """ + ) + } + } }