From 17454feae24c96a48e83c23453b6ac740696dde1 Mon Sep 17 00:00:00 2001 From: Balaji Ganesan Date: Sat, 29 Aug 2026 08:28:03 -0700 Subject: [PATCH 1/2] fix(container-cache): ignore malformed Range headers instead of returning 416 The NGC CLI computes a range of first-byte-pos 0 through last-byte-pos size-1. For a zero-length file that is 0 through -1, so it sends "Range: bytes=0--1". RFC 7233 section 2.1 defines last-byte-pos as 1*DIGIT, which admits no sign, so the byte-range-set does not parse. nginx rejects it at parse time and answers 416. That is conformant: section 3.1 says a server SHOULD send 416 when ranges are invalid. The NGC origin instead ignores the bad header and returns 200 with the full representation. The result is that a client which downloads successfully from the origin fails behind this cache, and any model containing a zero-length file cannot be pulled through proxy-cache at all. Since we are a transparent intermediary in front of a client we do not control, match the origin rather than being stricter than it. A new set_by_lua_file sanitizer clears a "bytes=" header that fails to parse and exposes $safe_range, which the ngc, hf, nucleus and relay blocks now use in place of $http_range for proxy_set_header Range, proxy_cache_key and $cc_hash_key. The sanitizer must run ahead of every consumer because nginx caches $http_range once evaluated; a `set` that reads it first would pin the malformed value into the cache key. Scope is deliberately narrow. Only the "bytes" unit is considered, since nginx already ignores units it does not understand. Only headers that fail to parse are cleared: "bytes=100-50" parses but is unsatisfiable and keeps nginx's conformant 416. Verified against OpenResty that bytes=0--1 returns 416 on a 1000-byte file as well, so this is a Range parse failure rather than zero-length handling, and the fix belongs in Range parsing generally. Co-Authored-By: Balaji Ganesan --- .../helm/container-cache/deploy/files/hf.conf | 19 ++- .../deploy/files/lua/safe-range.lua | 78 +++++++++ .../container-cache/deploy/files/ngc.conf | 13 +- .../deploy/files/proxy-common.conf | 2 +- .../deploy/files/proxy-nucleus-cache.conf | 15 +- .../deploy/templates/configmap.yaml | 2 + .../deploy/templates/statefulset.yaml | 2 + .../tests/range-sanitize-runtime-test.sh | 150 ++++++++++++++++++ .../tests/range-validator-test.lua | 83 ++++++++++ .../tests/range-validator-test.sh | 26 +++ .../tests/render-consistent-hash-test.sh | 2 +- .../tests/render-range-sanitize-test.sh | 84 ++++++++++ 12 files changed, 462 insertions(+), 14 deletions(-) create mode 100644 deploy/helm/container-cache/deploy/files/lua/safe-range.lua create mode 100755 deploy/helm/container-cache/tests/range-sanitize-runtime-test.sh create mode 100644 deploy/helm/container-cache/tests/range-validator-test.lua create mode 100755 deploy/helm/container-cache/tests/range-validator-test.sh create mode 100755 deploy/helm/container-cache/tests/render-range-sanitize-test.sh diff --git a/deploy/helm/container-cache/deploy/files/hf.conf b/deploy/helm/container-cache/deploy/files/hf.conf index f4b2a4873..9d0228d41 100644 --- a/deploy/helm/container-cache/deploy/files/hf.conf +++ b/deploy/helm/container-cache/deploy/files/hf.conf @@ -164,8 +164,13 @@ proxy_ssl_protocols TLSv1.2 TLSv1.3; proxy_pass_request_headers on; + # Neutralise a malformed "bytes=" Range (see lua/safe-range.lua). + # The range filter runs regardless of caching, so this bypass block + # needs it too. + set_by_lua_file $safe_range /etc/nginx/conf.d/lua/safe-range.lua; + # Forward the client Range untouched so chunk reads work as-is. - proxy_set_header Range $http_range; + proxy_set_header Range $safe_range; # Initialize upstream_last_modified to prevent warnings set $upstream_last_modified ""; @@ -216,9 +221,15 @@ location / { proxy_pass_request_headers on; + + # Neutralise a malformed "bytes=" Range before anything reads + # $http_range (see lua/safe-range.lua). nginx caches $http_range once + # evaluated, so this must stay ahead of the directives below. + set_by_lua_file $safe_range /etc/nginx/conf.d/lua/safe-range.lua; + # Use Range header for file content - enables partial downloads - proxy_set_header Range $http_range; - proxy_cache_key $request_method|$uri|$arg_versionId|$http_range; + proxy_set_header Range $safe_range; + proxy_cache_key $request_method|$uri|$arg_versionId|$safe_range; # HF-OPTIMIZED: Buffers sized for 10MB median object size # Buffer profile inherited from proxy-common.conf (aligned fleet-wide). @@ -229,7 +240,7 @@ # Consistent-hash routing to the owner pod (see lua/cc-route.lua). # Route on the same key nginx caches on so each byte-range chunk maps # to its own owner and a large blob spreads across the tier. - set $cc_hash_key "$request_method|$uri|$arg_versionId|$http_range"; + set $cc_hash_key "$request_method|$uri|$arg_versionId|$safe_range"; set $cc_replicas {{ int $.Values.replicaCount }}; rewrite_by_lua_file /etc/nginx/conf.d/lua/cc-route.lua; {{- end }} diff --git a/deploy/helm/container-cache/deploy/files/lua/safe-range.lua b/deploy/helm/container-cache/deploy/files/lua/safe-range.lua new file mode 100644 index 000000000..520035bdc --- /dev/null +++ b/deploy/helm/container-cache/deploy/files/lua/safe-range.lua @@ -0,0 +1,78 @@ +-- SPDX-FileCopyrightText: Copyright (c) 2023-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +-- SPDX-License-Identifier: Apache-2.0 +-- +-- Licensed under the Apache License, Version 2.0 (the "License"); +-- you may not use this file except in compliance with the License. +-- You may obtain a copy of the License at +-- +-- http://www.apache.org/licenses/LICENSE-2.0 +-- +-- Unless required by applicable law or agreed to in writing, software +-- distributed under the License is distributed on an "AS IS" BASIS, +-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +-- See the License for the specific language governing permissions and +-- limitations under the License. +--[[ +Neutralises a Range header that names the "bytes" unit but does not parse. + +Why this exists. The NGC CLI computes a range of first-byte-pos 0 through +last-byte-pos size-1. For a zero-length file that is 0 through -1, so it sends: + + Range: bytes=0--1 + +RFC 7233 section 2.1 defines last-byte-pos as 1*DIGIT, which admits no sign, so +the byte-range-set does not parse. nginx rejects it at parse time and answers +416. That is conformant: section 3.1 says a server SHOULD send 416 when ranges +are invalid. The NGC origin instead ignores the bad header and returns 200 with +the full representation, which is lenient but permitted. + +The result is that a client which downloads successfully from the origin fails +behind this cache. Since we are a transparent intermediary in front of a client +we do not control, we match the origin rather than being stricter than it. + +Scope, deliberately narrow: + - Only a header naming the "bytes" unit is considered. nginx already ignores + units it does not understand, so those are left untouched. + - Only a header that fails to PARSE is cleared. A header that parses but is + unsatisfiable or semantically invalid, such as "bytes=100-50", keeps + nginx's 416. That behaviour is correct and the origin rejects it too. + +Used via set_by_lua_file so it runs in configuration order alongside the +surrounding `set` directives. That ordering matters: $http_range is cached by +nginx once evaluated, so any `set` that reads it before this runs would pin the +malformed value into the cache key. Callers must use the returned $safe_range, +not $http_range, in proxy_cache_key, proxy_set_header Range and $cc_hash_key. + +Returns the original value when it is valid or absent, otherwise an empty +string, having also removed the header so nginx's range filter does not see it. +]] + +local range = ngx.var.http_range +if not range or range == "" then + return "" +end + +local range_set = range:match("^[Bb][Yy][Tt][Ee][Ss]=(.*)$") +if not range_set then + return range +end + +local valid = range_set ~= "" +if valid then + -- byte-range-spec = first-byte-pos "-" [ last-byte-pos ] + -- suffix-byte-range-spec = "-" suffix-length + for spec in (range_set .. ","):gmatch("([^,]*),") do + spec = spec:gsub("^%s*(.-)%s*$", "%1") + if not (spec:match("^%d+%-%d*$") or spec:match("^%-%d+$")) then + valid = false + break + end + end +end + +if valid then + return range +end + +ngx.req.clear_header("Range") +return "" diff --git a/deploy/helm/container-cache/deploy/files/ngc.conf b/deploy/helm/container-cache/deploy/files/ngc.conf index 3f481b112..6443a7b16 100644 --- a/deploy/helm/container-cache/deploy/files/ngc.conf +++ b/deploy/helm/container-cache/deploy/files/ngc.conf @@ -47,6 +47,13 @@ } location / { + # Neutralise a malformed "bytes=" Range before anything reads + # $http_range. nginx would answer 416; the origin ignores it and + # serves 200. See lua/safe-range.lua. Must stay ahead of every + # directive below that consumes the range, because nginx caches + # $http_range once evaluated. + set_by_lua_file $safe_range /etc/nginx/conf.d/lua/safe-range.lua; + # Initialize upstream_last_modified to prevent warnings set $upstream_last_modified ""; @@ -67,14 +74,14 @@ # Consistent-hash routing to the owner pod (see lua/cc-route.lua). # Route on the same key nginx caches on so each byte-range chunk maps # to its own owner and a large blob spreads across the tier. - set $cc_hash_key "$request_method|$uri|$arg_versionId|$http_range"; + set $cc_hash_key "$request_method|$uri|$arg_versionId|$safe_range"; set $cc_replicas {{ int $.Values.replicaCount }}; rewrite_by_lua_file /etc/nginx/conf.d/lua/cc-route.lua; {{- end }} proxy_pass_request_headers on; - proxy_set_header Range $http_range; - proxy_cache_key $request_method|$uri|$arg_versionId|$http_range; + proxy_set_header Range $safe_range; + proxy_cache_key $request_method|$uri|$arg_versionId|$safe_range; access_by_lua_file /etc/nginx/conf.d/lua/lua-access.lua; proxy_set_header Host $host; proxy_pass $scheme://$host; diff --git a/deploy/helm/container-cache/deploy/files/proxy-common.conf b/deploy/helm/container-cache/deploy/files/proxy-common.conf index 5487cfa0a..04394e44b 100644 --- a/deploy/helm/container-cache/deploy/files/proxy-common.conf +++ b/deploy/helm/container-cache/deploy/files/proxy-common.conf @@ -254,7 +254,7 @@ proxy_http_version 1.1; proxy_pass_request_headers on; proxy_set_header Host $host; - proxy_set_header Range $http_range; + proxy_set_header Range $safe_range; proxy_set_header X-NVCF-CC-Relayed "1"; # Declaring any proxy_set_header here cancels inheritance of the # server-level set, which includes `Connection ""`. Without repeating diff --git a/deploy/helm/container-cache/deploy/files/proxy-nucleus-cache.conf b/deploy/helm/container-cache/deploy/files/proxy-nucleus-cache.conf index 93ecc85a6..d7b34691f 100644 --- a/deploy/helm/container-cache/deploy/files/proxy-nucleus-cache.conf +++ b/deploy/helm/container-cache/deploy/files/proxy-nucleus-cache.conf @@ -32,8 +32,11 @@ # DNS resolver for Nucleus requests - moderate caching for internal service resolver nvcf-unbound.dns-proxy.svc.cluster.local valid=900s; # 15min cache - internal services less stable than CDNs proxy_set_header Host $http_host; - # Client library knows what slices to use and we trust it with http_range - proxy_set_header Range $http_range; + # Neutralise a malformed "bytes=" Range (see lua/safe-range.lua); + # any range that parses is still passed through untouched. + set_by_lua_file $safe_range /etc/nginx/conf.d/lua/safe-range.lua; + # Client library knows what slices to use and we trust it with the range + proxy_set_header Range $safe_range; # Required headers for Nucleus LFT proxy_set_header X-OV-URI $http_x_ov_uri; proxy_set_header Omniverse-Proxy-Scheme $http_omniverse_proxy_scheme; @@ -45,7 +48,7 @@ proxy_socket_keepalive on; # Cache key includes NCA ID and OVC 1 cache parameters - proxy_cache_key $http_nvcf_nca_id|$request_method|$http_omniverse_content_uid|$http_range; + proxy_cache_key $http_nvcf_nca_id|$request_method|$http_omniverse_content_uid|$safe_range; proxy_pass https://$host; # From OVC 1 @@ -65,12 +68,14 @@ # Use Host header proxy_set_header Host $http_host; + # Neutralise a malformed "bytes=" Range (see lua/safe-range.lua). + set_by_lua_file $safe_range /etc/nginx/conf.d/lua/safe-range.lua; # Use Range Header - proxy_set_header Range $http_range; + proxy_set_header Range $safe_range; proxy_pass_request_headers on; # Cache key includes NCA ID and generic uri with optional omniverse content uid and range - proxy_cache_key $http_nvcf_nca_id|$request_method|$host|$request_uri|$http_omniverse_content_uid|$http_range; + proxy_cache_key $http_nvcf_nca_id|$request_method|$host|$request_uri|$http_omniverse_content_uid|$safe_range; proxy_pass https://$http_host; } } diff --git a/deploy/helm/container-cache/deploy/templates/configmap.yaml b/deploy/helm/container-cache/deploy/templates/configmap.yaml index 076c947c1..0c1fa2e03 100644 --- a/deploy/helm/container-cache/deploy/templates/configmap.yaml +++ b/deploy/helm/container-cache/deploy/templates/configmap.yaml @@ -31,6 +31,8 @@ data: {{ .Files.Get "files/lua/lua-gen-key.lua" | nindent 4 }} lua-ssl.lua: |- {{ .Files.Get "files/lua/lua-ssl.lua" | nindent 4 }} + safe-range.lua: |- +{{ .Files.Get "files/lua/safe-range.lua" | nindent 4 }} {{- if (.Values.consistentHashRouting).enabled }} cc-route.lua: |- {{ .Files.Get "files/lua/cc-route.lua" | nindent 4 }} diff --git a/deploy/helm/container-cache/deploy/templates/statefulset.yaml b/deploy/helm/container-cache/deploy/templates/statefulset.yaml index ef3bbeeb0..c8237c66b 100644 --- a/deploy/helm/container-cache/deploy/templates/statefulset.yaml +++ b/deploy/helm/container-cache/deploy/templates/statefulset.yaml @@ -260,6 +260,8 @@ spec: path: lua-gen-key.lua - key: lua-ssl.lua path: lua-ssl.lua + - key: safe-range.lua + path: safe-range.lua {{- if (.Values.consistentHashRouting).enabled }} - key: cc-route.lua path: cc-route.lua diff --git a/deploy/helm/container-cache/tests/range-sanitize-runtime-test.sh b/deploy/helm/container-cache/tests/range-sanitize-runtime-test.sh new file mode 100755 index 000000000..57dbd3868 --- /dev/null +++ b/deploy/helm/container-cache/tests/range-sanitize-runtime-test.sh @@ -0,0 +1,150 @@ +#!/usr/bin/env bash +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 +# +# Behavioural test for files/lua/safe-range.lua. Runs the real sanitizer inside +# OpenResty against a mock origin that tolerates a malformed Range the way the +# NGC CDN does, and asserts we match the origin instead of answering 416. +# +# bash tests/range-sanitize-runtime-test.sh +# +# Requires docker and python3. Skips cleanly when docker is unavailable. +set -euo pipefail +CHART_DIR="$(cd "$(dirname "$0")/.." && pwd)/deploy" +LUA_SRC="$CHART_DIR/files/lua/safe-range.lua" +IMAGE="${OPENRESTY_IMAGE:-openresty/openresty:alpine}" +PORT_PROXY=18190 +PORT_ORIGIN=18191 + +command -v docker >/dev/null 2>&1 || { echo "SKIP: docker not available"; exit 0; } +docker info >/dev/null 2>&1 || { echo "SKIP: docker daemon not reachable"; exit 0; } +[ -f "$LUA_SRC" ] || { echo "FAIL: $LUA_SRC missing" >&2; exit 1; } + +TMP="$(mktemp -d)" +CONT="ccrange-$$" +cleanup() { + docker rm -f "$CONT" >/dev/null 2>&1 || true + [ -n "${ORIGIN_PID:-}" ] && kill "$ORIGIN_PID" 2>/dev/null || true + rm -rf "$TMP" +} +trap cleanup EXIT +fail() { echo "FAIL: $*" >&2; exit 1; } + +mkdir -p "$TMP/lua" +cp "$LUA_SRC" "$TMP/lua/safe-range.lua" + +cat > "$TMP/origin.py" < "$TMP/nginx.conf" </dev/null + +for _ in $(seq 1 25); do + curl -sf -o /dev/null "http://127.0.0.1:$PORT_PROXY/nonempty" 2>/dev/null && break + sleep 0.4 +done +curl -sf -o /dev/null "http://127.0.0.1:$PORT_PROXY/nonempty" \ + || { docker logs "$CONT" 2>&1 | tail -20; fail "proxy did not come up"; } + +probe() { # path, range -> "code size" + if [ "$2" = "-" ]; then + curl -sS -o /dev/null -w '%{http_code} %{size_download}' "http://127.0.0.1:$PORT_PROXY$1" + else + curl -sS -o /dev/null -w '%{http_code} %{size_download}' -H "Range: $2" "http://127.0.0.1:$PORT_PROXY$1" + fi +} +expect() { # path, range, want, why + got="$(probe "$1" "$2")" + [ "$got" = "$3" ] || fail "$1 with Range '$2': expected [$3], got [$got] -- $4" + printf ' ok %-12s %-14s -> %s\n' "$1" "$2" "$got" +} + +echo "1. a malformed range is neutralised, matching the origin's 200" +expect /empty "bytes=0--1" "200 0" "the reported NGC CLI failure: zero-length file" +expect /nonempty "bytes=0--1" "200 1000" "the fault is Range parsing, not zero length" + +echo "2. valid ranges are untouched" +expect /nonempty "bytes=0-0" "206 1" "single byte range must still work" +expect /nonempty "bytes=0-99" "206 100" "bounded range must still work" +expect /nonempty "bytes=-100" "206 100" "suffix range must still work" +expect /nonempty "bytes=900-" "206 100" "open-ended range must still work" + +echo "2b. what the origin actually receives (the sanitizer's real contract)" +saw() { # path, range -> the Range header the origin observed + curl -sS -o /dev/null -D- -H "Range: $2" "http://127.0.0.1:$PORT_PROXY$1?cb=$RANDOM" \ + | tr -d '\r' | awk -F': ' 'tolower($1)=="x-origin-saw-range"{print $2; exit}' +} +forwards() { # path, range, expected-at-origin, why + got="$(saw "$1" "$2")" + [ "$got" = "$3" ] || fail "$1 with Range '$2': origin saw '$got', expected '$3' -- $4" + printf ' ok origin saw %-18s for %s\n' "'$got'" "'$2'" +} +forwards /nonempty "bytes=0-9,20-29" "bytes=0-9,20-29" "multi-range must reach the origin unchanged" +forwards /nonempty "bytes=0-99" "bytes=0-99" "valid range must reach the origin unchanged" +forwards /nonempty "bytes=0--1" "none" "malformed range must be stripped before the origin" +forwards /empty "bytes=0--1" "none" "malformed range stripped on the zero-length file too" + +echo "3. a parseable but unsatisfiable range keeps nginx's conformant 416" +expect /nonempty "bytes=100-50" "416 203" "RFC 7233 3.1 says SHOULD 416; do not paper over it" +expect /empty "bytes=0-0" "200 0" "zero-length with a valid range is not an error" + +echo "4. no range is unaffected" +expect /empty "-" "200 0" +expect /nonempty "-" "200 1000" + +echo "5. a sanitized request shares the cache entry with an unranged one" +docker exec "$CONT" sh -c 'rm -rf /cache/*' >/dev/null 2>&1 || true +docker restart "$CONT" >/dev/null +for _ in $(seq 1 25); do curl -sf -o /dev/null "http://127.0.0.1:$PORT_PROXY/nonempty" 2>/dev/null && break; sleep 0.4; done +curl -sS -o /dev/null "http://127.0.0.1:$PORT_PROXY/nonempty" +hdrs="$(curl -sS -o /dev/null -D- -H 'Range: bytes=0--1' "http://127.0.0.1:$PORT_PROXY/nonempty")" +echo "$hdrs" | grep -qi 'X-Cache: HIT' \ + || fail "malformed-range request did not reuse the unranged cache entry (duplicate entry)" +echo "$hdrs" | grep -qi 'X-Key: GET|/nonempty|$' \ + || echo "$hdrs" | grep -qiE 'X-Key: GET\|/nonempty\|[[:space:]]*$' \ + || fail "cache key still carries the malformed range" +echo " ok shares the cache key with the unranged request" + +echo "PASS: safe-range.lua behaves correctly against a tolerant origin" diff --git a/deploy/helm/container-cache/tests/range-validator-test.lua b/deploy/helm/container-cache/tests/range-validator-test.lua new file mode 100644 index 000000000..036ff8daf --- /dev/null +++ b/deploy/helm/container-cache/tests/range-validator-test.lua @@ -0,0 +1,83 @@ +-- SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +-- SPDX-License-Identifier: Apache-2.0 +-- +-- Unit tests for files/lua/safe-range.lua. Loads the real file with a stubbed +-- `ngx` so the shipped code is exercised exactly as nginx runs it, with no +-- test-only branches in production code. +-- +-- luajit tests/range-validator-test.lua [path/to/safe-range.lua] + +local lua_path = arg and arg[1] or "deploy/files/lua/safe-range.lua" + +local function run(input) + local cleared = false + _G.ngx = { + var = { http_range = input }, + req = { clear_header = function(name) + if name == "Range" then cleared = true end + end }, + } + local chunk = assert(loadfile(lua_path)) + local returned = chunk() + return returned, cleared +end + +-- input, expected return, expected "was the header cleared", why it matters +local cases = { + -- The reported failure: NGC CLI computes last-byte-pos as 0 - 1. + { "bytes=0--1", "", true, "the reported NGC CLI failure" }, + -- Valid forms must survive untouched, or we break every real download. + { "bytes=0-0", "bytes=0-0", false, "single byte" }, + { "bytes=0-99", "bytes=0-99", false, "bounded range" }, + { "bytes=900-", "bytes=900-", false, "open-ended range" }, + { "bytes=-100", "bytes=-100", false, "suffix range" }, + { "bytes=0-9,20-29", "bytes=0-9,20-29", false, "multi-range" }, + { "bytes=0-9, 20-29", "bytes=0-9, 20-29", false, "multi-range with OWS" }, + { "bytes=536870911-1073741823", "bytes=536870911-1073741823", false, "large offsets" }, + -- Parseable but unsatisfiable: nginx's 416 is conformant, leave it alone. + { "bytes=100-50", "bytes=100-50", false, "unsatisfiable stays a 416" }, + -- Other malformed shapes. + { "bytes=abc", "", true, "non-numeric" }, + { "bytes=", "", true, "empty range-set" }, + { "bytes=-", "", true, "bare hyphen" }, + { "bytes=0-1-2", "", true, "too many hyphens" }, + { "bytes=0-9,", "", true, "trailing empty spec" }, + { "bytes=--1", "", true, "no first-byte-pos" }, + -- Unknown units are nginx's business; it already ignores them. + { "items=0-1", "items=0-1", false, "unknown unit passes through" }, + -- Absent header. + { "", "", false, "empty header" }, +} + +local failures = 0 +for _, c in ipairs(cases) do + local input, want_ret, want_cleared, why = c[1], c[2], c[3], c[4] + local got_ret, got_cleared = run(input) + if got_ret ~= want_ret or got_cleared ~= want_cleared then + failures = failures + 1 + print(string.format( + "FAIL %-30s returned %-22s cleared=%-5s (want %-22s cleared=%-5s) -- %s", + "'" .. input .. "'", "'" .. tostring(got_ret) .. "'", tostring(got_cleared), + "'" .. want_ret .. "'", tostring(want_cleared), why)) + else + print(string.format("ok %-30s -> %s", "'" .. input .. "'", + got_cleared and "cleared" or "kept")) + end +end + +-- nil header (no Range at all) must not error. +do + local ok, err = pcall(run, nil) + if not ok then + failures = failures + 1 + print("FAIL absent Range header raised: " .. tostring(err)) + else + print("ok absent Range header handled") + end +end + +if failures > 0 then + print(string.format("\nFAILED: %d case(s)", failures)) + os.exit(1) +end +print("\nPASS: safe-range.lua validator") diff --git a/deploy/helm/container-cache/tests/range-validator-test.sh b/deploy/helm/container-cache/tests/range-validator-test.sh new file mode 100755 index 000000000..e69608f69 --- /dev/null +++ b/deploy/helm/container-cache/tests/range-validator-test.sh @@ -0,0 +1,26 @@ +#!/usr/bin/env bash +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 +# +# Runs the safe-range.lua unit tests. Prefers a local luajit/lua, otherwise +# borrows the one in the OpenResty image. Skips only if neither is available. +# +# bash tests/range-validator-test.sh +set -euo pipefail +DIR="$(cd "$(dirname "$0")/.." && pwd)" +LUA_FILE="deploy/files/lua/safe-range.lua" +TEST_FILE="tests/range-validator-test.lua" +cd "$DIR" + +for bin in luajit lua5.1 lua; do + if command -v "$bin" >/dev/null 2>&1; then + exec "$bin" "$TEST_FILE" "$LUA_FILE" + fi +done + +if command -v docker >/dev/null 2>&1 && docker info >/dev/null 2>&1; then + exec docker run --rm -v "$DIR:/w" -w /w "${OPENRESTY_IMAGE:-openresty/openresty:alpine}" \ + /usr/local/openresty/luajit/bin/luajit "$TEST_FILE" "$LUA_FILE" +fi + +echo "SKIP: no lua interpreter and no docker available" diff --git a/deploy/helm/container-cache/tests/render-consistent-hash-test.sh b/deploy/helm/container-cache/tests/render-consistent-hash-test.sh index 0050879ff..b43231f6a 100755 --- a/deploy/helm/container-cache/tests/render-consistent-hash-test.sh +++ b/deploy/helm/container-cache/tests/render-consistent-hash-test.sh @@ -48,7 +48,7 @@ echo "5. enabled: N peer Services + N owner upstreams on the listener port (1412 [ "$(count 'port: 14128' "$TMP/on.yaml")" = 3 ] || fail "each peer Service must expose port 14128" echo "6. enabled: routing key == proxy_cache_key; marker emitted AND inbound marker rejected" -grep -q 'set $cc_hash_key "$request_method|$uri|$arg_versionId|$http_range"' "$TMP/on.yaml" || fail "routing key must equal the proxy_cache_key" +grep -q 'set $cc_hash_key "$request_method|$uri|$arg_versionId|$safe_range"' "$TMP/on.yaml" || fail "routing key must equal the proxy_cache_key" grep -q 'proxy_set_header X-NVCF-CC-Relayed "1"' "$TMP/on.yaml" || fail "relay hop must emit the one-hop marker" grep -q 'ngx.req.get_headers()\["X-NVCF-CC-Relayed"\]' "$TMP/on.yaml" || fail "cc-route.lua must reject an inbound relay marker (serve locally, prevents relay loops)" diff --git a/deploy/helm/container-cache/tests/render-range-sanitize-test.sh b/deploy/helm/container-cache/tests/render-range-sanitize-test.sh new file mode 100755 index 000000000..a9224240c --- /dev/null +++ b/deploy/helm/container-cache/tests/render-range-sanitize-test.sh @@ -0,0 +1,84 @@ +#!/usr/bin/env bash +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 +# +# Rendered-output regression tests for malformed-Range sanitization. Run from +# the chart subtree: +# bash tests/render-range-sanitize-test.sh +# +# Background: the NGC CLI sends "Range: bytes=0--1" for a zero-length file. +# nginx answers 416; the origin ignores the bad header and answers 200. We +# match the origin. See files/lua/safe-range.lua. +set -euo pipefail +CHART_DIR="$(cd "$(dirname "$0")/.." && pwd)/deploy" +TMP="$(mktemp -d)" +trap 'rm -rf "$TMP"' EXIT +fail() { echo "FAIL: $*" >&2; exit 1; } +count() { grep -Ec "$1" "$2" || true; } + +helm template t "$CHART_DIR" > "$TMP/off.yaml" 2>/dev/null +helm template t "$CHART_DIR" --set consistentHashRouting.enabled=true --set replicaCount=3 > "$TMP/on.yaml" 2>/dev/null + +echo "1. the sanitizer ships in the ConfigMap and is projected into the lua volume" +for f in "$TMP/off.yaml" "$TMP/on.yaml"; do + grep -q 'safe-range.lua:' "$f" || fail "safe-range.lua missing from the ConfigMap" + grep -q 'key: safe-range.lua' "$f" || fail "safe-range.lua not projected into the lua volume" +done + +echo "2. it is unconditional (present whether or not hash routing is enabled)" +[ "$(count 'safe-range.lua:' "$TMP/off.yaml")" -ge 1 ] || fail "sanitizer must not be gated on consistentHashRouting" + +echo "3. every range-consuming directive uses \$safe_range, never \$http_range" +for f in "$TMP/off.yaml" "$TMP/on.yaml"; do + [ "$(count 'proxy_set_header Range \$http_range' "$f")" = 0 ] \ + || fail "a proxy_set_header Range still forwards the raw \$http_range" + [ "$(count 'proxy_cache_key .*\$http_range' "$f")" = 0 ] \ + || fail "a proxy_cache_key still keys on the raw \$http_range" + [ "$(count 'set \$cc_hash_key .*\$http_range' "$f")" = 0 ] \ + || fail "a routing key still hashes the raw \$http_range" +done + +echo "4. no nginx directive still consumes the raw \$http_range" +# Only nginx directives are checked: a rendered directive is a line ending in +# ";". That deliberately excludes the log formats (we log what the client +# actually sent), comments, and lua reading ngx.var.http_range -- which covers +# both the sanitizer itself and the S3 slice block's $request_range. S3 is out +# of scope here: it has its own `slice` interaction and no reported failures. +python3 - "$TMP/on.yaml" <<'PY2' || fail "an nginx directive still consumes the raw \$http_range" +import sys +bad=[f' line {n}: {l.strip()[:90]}' + for n,l in enumerate(open(sys.argv[1]),1) + if '$http_range' in l and l.rstrip().endswith(';')] +if bad: + print('\n'.join(bad)); sys.exit(1) +PY2 + +echo "5. within each block, the sanitizer is set before \$safe_range is read" +# nginx caches $http_range once evaluated and $safe_range is empty until set, +# so a consumer placed above the sanitizer would silently drop the range. +python3 - "$TMP/on.yaml" <<'PY2' || fail "a \$safe_range consumer is ordered above its sanitizer" +import re,sys +text=open(sys.argv[1]).read() +bad=[] +for m in re.finditer(r'set_by_lua_file\s+\$safe_range', text): + # Walk back to the nearest enclosing "location ... {" and check the span + # between it and the sanitizer for any read of $safe_range. + head = text[:m.start()] + loc = head.rfind('location') + if loc == -1: + bad.append('sanitizer outside any location block'); continue + span = head[loc:] + for consumer in ('proxy_cache_key', 'proxy_set_header Range', 'set $cc_hash_key'): + if consumer in span and '$safe_range' in span[span.find(consumer):]: + bad.append(f'{consumer!r} reads $safe_range before it is set') +if bad: + print('\n'.join(' '+b for b in dict.fromkeys(bad))); sys.exit(1) +PY2 + +echo "6. routing key still equals the cache key (invariant preserved, new variable)" +grep -q 'set $cc_hash_key "$request_method|$uri|$arg_versionId|$safe_range"' "$TMP/on.yaml" \ + || fail "routing key must equal the proxy_cache_key" +grep -q 'proxy_cache_key $request_method|$uri|$arg_versionId|$safe_range' "$TMP/on.yaml" \ + || fail "cache key must use the sanitized range" + +echo "PASS: all malformed-Range sanitization render assertions hold" From 9d90b818992b1e13d4e7af188e16d3d8d83f4462 Mon Sep 17 00:00:00 2001 From: Balaji Ganesan Date: Sat, 29 Aug 2026 09:39:27 -0700 Subject: [PATCH 2/2] test(container-cache): assert only the status for the 416 case The unsatisfiable-range assertion pinned both status and body size as one value. The body of a 416 is nginx's default error page, whose size varies between builds: 194 bytes in production, 203 in the OpenResty test image. With OPENRESTY_IMAGE overridable, that would fail on a correct 416. Add expect_status for status-only assertions and use it there. The remaining exact-size assertions stay, because those sizes are the contract under test: the full representation or the requested byte range. Co-Authored-By: Balaji Ganesan --- .../tests/range-sanitize-runtime-test.sh | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/deploy/helm/container-cache/tests/range-sanitize-runtime-test.sh b/deploy/helm/container-cache/tests/range-sanitize-runtime-test.sh index 57dbd3868..fe7d3c18a 100755 --- a/deploy/helm/container-cache/tests/range-sanitize-runtime-test.sh +++ b/deploy/helm/container-cache/tests/range-sanitize-runtime-test.sh @@ -95,11 +95,20 @@ probe() { # path, range -> "code size" curl -sS -o /dev/null -w '%{http_code} %{size_download}' -H "Range: $2" "http://127.0.0.1:$PORT_PROXY$1" fi } -expect() { # path, range, want, why +expect() { # path, range, want "code size", why got="$(probe "$1" "$2")" [ "$got" = "$3" ] || fail "$1 with Range '$2': expected [$3], got [$got] -- $4" printf ' ok %-12s %-14s -> %s\n' "$1" "$2" "$got" } +expect_status() { # path, range, want-code, why + # Status only. The body of an error response is nginx's default error page, + # whose size varies between builds (194 bytes in production, 203 in the + # OpenResty test image), so asserting it would break under OPENRESTY_IMAGE. + got="$(probe "$1" "$2")" + code="${got%% *}" + [ "$code" = "$3" ] || fail "$1 with Range '$2': expected status $3, got [$got] -- $4" + printf ' ok %-12s %-14s -> %s (body size not asserted)\n' "$1" "$2" "$code" +} echo "1. a malformed range is neutralised, matching the origin's 200" expect /empty "bytes=0--1" "200 0" "the reported NGC CLI failure: zero-length file" @@ -127,7 +136,7 @@ forwards /nonempty "bytes=0--1" "none" "malformed range must be forwards /empty "bytes=0--1" "none" "malformed range stripped on the zero-length file too" echo "3. a parseable but unsatisfiable range keeps nginx's conformant 416" -expect /nonempty "bytes=100-50" "416 203" "RFC 7233 3.1 says SHOULD 416; do not paper over it" +expect_status /nonempty "bytes=100-50" "416" "RFC 7233 3.1 says SHOULD 416; do not paper over it" expect /empty "bytes=0-0" "200 0" "zero-length with a valid range is not an error" echo "4. no range is unaffected"