From 4917f52de2c200328f953f13a27f46632fa5bdbe Mon Sep 17 00:00:00 2001 From: Nikolai Emil Damm Date: Tue, 3 Nov 2026 09:43:00 +0100 Subject: [PATCH 1/3] fix(gateway): bind denied-peer probe to verified transport --- .github/workflows/ci.yaml | 4 +- .../workflows/probe-gateway-denied-peer.yaml | 16 ++- scripts/gateway-receiver-binding/main.go | 2 +- scripts/probe-gateway-denied-peer.sh | 29 +++- .../tests/gateway-denied-peer-wire/main.go | 126 ++++++++++++++++++ .../tests/test-probe-gateway-denied-peer.sh | 72 +++++++++- 6 files changed, 235 insertions(+), 14 deletions(-) create mode 100644 scripts/tests/gateway-denied-peer-wire/main.go diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 9ea4d89486..4b0158c875 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -399,6 +399,7 @@ jobs: # up, and that no address reaches the public log. - 'scripts/probe-gateway-denied-peer.sh' - 'scripts/tests/test-probe-gateway-denied-peer.sh' + - 'scripts/tests/gateway-denied-peer-wire/**' - '.github/workflows/probe-gateway-denied-peer.yaml' # The #3144 admission probe, its test and its workflow are one # contract: the test pins that every write the probe sends is a @@ -2500,7 +2501,8 @@ jobs: - name: 🚧 Validate Gateway denied-peer probe if: needs.changes.outputs.k8s == 'true' # The probe needs prod, runs only on dispatch and writes while it runs. Its - # test fakes kubectl and pins each verdict against a one-fixture control. + # test pins verdicts and cleanup, then executes both rendered transport modes with + # real curl and synthetic loopback HTTP/TLS peers. run: | shellcheck scripts/probe-gateway-denied-peer.sh scripts/tests/test-probe-gateway-denied-peer.sh bash scripts/tests/test-probe-gateway-denied-peer.sh diff --git a/.github/workflows/probe-gateway-denied-peer.yaml b/.github/workflows/probe-gateway-denied-peer.yaml index 31f92ae0e1..642fd8bc17 100644 --- a/.github/workflows/probe-gateway-denied-peer.yaml +++ b/.github/workflows/probe-gateway-denied-peer.yaml @@ -76,6 +76,7 @@ jobs: # The allowed half: the fence is in place and the load balancer's traffic is still answered. - name: Verify the selected fence and the public routes + id: binding shell: bash env: HCLOUD_TOKEN: ${{ secrets.HCLOUD_TOKEN }} @@ -84,14 +85,19 @@ jobs: verified="$(go run ./scripts/gateway-receiver-binding verify --context admin@prod)" printf '%s\n' "${verified}" # Without a selected fence, an answered connection would say nothing about the fence. - grep -Eq '(^| )fence_selected=true( |$)' <<<"${verified}" || { printf 'The fence is not selected; nothing was probed.\n' >&2; exit 1; } + pattern='^GATEWAY_RECEIVER_SOURCE=VERIFIED public_probes=4 fence_selected=true proxy_protocol=(true|false)$' + [[ "${verified}" =~ ${pattern} ]] || { printf 'The fenced transport profile is unverified; nothing was probed.\n' >&2; exit 1; } + transport=plain + [[ "${BASH_REMATCH[1]}" == false ]] || transport=proxy-v1 + printf 'transport=%s\n' "${transport}" >>"${GITHUB_OUTPUT}" - name: Probe with a pod as the peer id: probe shell: bash env: RUN_ID: ${{ github.run_id }} - run: ./scripts/probe-gateway-denied-peer.sh --context admin@prod --run-id "${RUN_ID}" + TRANSPORT: ${{ steps.binding.outputs.transport }} + run: ./scripts/probe-gateway-denied-peer.sh --context admin@prod --run-id "${RUN_ID}" --transport "${TRANSPORT}" # A cancelled job can end the step before the script's own cleanup runs. - name: Remove anything this run left behind @@ -99,4 +105,8 @@ jobs: shell: bash env: RUN_ID: ${{ github.run_id }} - run: kubectl --context admin@prod --request-timeout=30s --namespace whoami delete pods,ciliumnetworkpolicies --selector "platform.devantler.tech/gateway-peer-probe=${RUN_ID}" --ignore-not-found >/dev/null 2>&1 + run: | + set -euo pipefail + kubectl --context admin@prod --request-timeout=30s --namespace whoami delete pods,ciliumnetworkpolicies --selector "platform.devantler.tech/gateway-peer-probe=${RUN_ID}" --ignore-not-found --wait=true --timeout=25s >/dev/null 2>&1 + remaining="$(kubectl --context admin@prod --request-timeout=30s --namespace whoami get pods,ciliumnetworkpolicies --selector "platform.devantler.tech/gateway-peer-probe=${RUN_ID}" -o name 2>/dev/null)" + [[ -z "${remaining}" ]] || { printf 'Run-owned cleanup is unverified.\n' >&2; exit 1; } diff --git a/scripts/gateway-receiver-binding/main.go b/scripts/gateway-receiver-binding/main.go index ceffa6fa3c..e005d3ca70 100644 --- a/scripts/gateway-receiver-binding/main.go +++ b/scripts/gateway-receiver-binding/main.go @@ -225,7 +225,7 @@ func main() { fmt.Fprintf(os.Stderr, "GATEWAY_RECEIVER_SOURCE=UNVERIFIED stage=%s\n", err.Error()) os.Exit(2) } - fmt.Printf("GATEWAY_RECEIVER_SOURCE=VERIFIED public_probes=4 fence_selected=%t\n", profile.Fence) + fmt.Printf("GATEWAY_RECEIVER_SOURCE=VERIFIED public_probes=4 fence_selected=%t proxy_protocol=%t\n", profile.Fence, profile.Proxy) return } if err := prepare(ctx, kubeContext, outputPath); err != nil { diff --git a/scripts/probe-gateway-denied-peer.sh b/scripts/probe-gateway-denied-peer.sh index c9e6ce1434..9c89ee2dff 100755 --- a/scripts/probe-gateway-denied-peer.sh +++ b/scripts/probe-gateway-denied-peer.sh @@ -3,7 +3,7 @@ # # THE QUESTION (devantler-tech/platform#5448, stage 3). Every TLS chain on the Gateway's listener # accepts only the load balancer's private address. So far only allowed traffic has been observed, -# which cannot show the fence refuses anything. This must be answered before any PROXY change, +# which cannot show the fence refuses anything. Both listener transport modes must be testable, # because PROXY metadata names a client the listener must not take from an unbound peer. # # HOW. One short-lived pod connects straight to the gateway Service, so its physical peer is the @@ -11,6 +11,8 @@ # control plain HTTP to an unrouted hostname. It must get the platform's HTTP-to-HTTPS redirect # (301) before and after the TLS attempts, which proves this pod reaches this Envoy and # that the plain chain, which carries no fence, still answers. +# In proxy-v1 mode every request sends a valid PROXY preamble before HTTP or TLS; +# otherwise the parser's rejection could be mistaken for the physical-source fence. # tls HTTPS with the SNI of a public route. Certificate checks are off on purpose: a finished # handshake is the failure this looks for, whatever certificate came back. # refused curl 35 or 56 with the TLS handshake never finished (time_appconnect is 0) @@ -68,15 +70,16 @@ readonly settle_seconds=20 readonly poll_seconds="${PROBE_POLL_SECONDS:-2}" usage() { - printf 'Usage: %s --context --run-id [--attempts N]\n' "$(basename "$0")" >&2 + printf 'Usage: %s --context --run-id --transport [--attempts N]\n' "$(basename "$0")" >&2 } context='' run_id='' attempts='10' +transport='' while [[ "$#" -gt 0 ]]; do case "$1" in - --context | --run-id | --attempts) + --context | --run-id | --attempts | --transport) if [[ "$#" -lt 2 || -z "$2" ]]; then usage exit 1 @@ -85,6 +88,7 @@ while [[ "$#" -gt 0 ]]; do --context) context="$2" ;; --run-id) run_id="$2" ;; --attempts) attempts="$2" ;; + --transport) transport="$2" ;; esac shift 2 ;; @@ -94,7 +98,7 @@ while [[ "$#" -gt 0 ]]; do ;; esac done -if [[ -z "${context}" ]] || [[ ! "${run_id}" =~ ^[1-9][0-9]{0,19}$ ]]; then +if [[ -z "${context}" ]] || [[ ! "${run_id}" =~ ^[1-9][0-9]{0,19}$ ]] || [[ "${transport}" != plain && "${transport}" != proxy-v1 ]]; then usage exit 1 fi @@ -102,6 +106,11 @@ if [[ ! "${attempts}" =~ ^[1-9][0-9]?$ ]] || ((attempts > 30)); then printf 'Refusing to run: --attempts must be a whole number from 1 to 30.\n' >&2 exit 1 fi +curl_transport_option='' +if [[ "${transport}" == proxy-v1 ]]; then + curl_transport_option='--haproxy-protocol' +fi +readonly curl_transport_option # Worst case: warm-up, the settle wait, every attempt timing out, the closing control, and pull. readonly pod_deadline_seconds=$(((warmup_attempts + attempts + 1) * attempt_timeout + warmup_attempts + settle_seconds + 120)) @@ -147,6 +156,7 @@ fi # shellcheck disable=SC2317,SC2329 cleanup() { local rc=$? + local remaining trap - EXIT INT TERM if ! kc -n "${probe_namespace}" delete pods,ciliumnetworkpolicies -l "${probe_label}=${run_id}" --ignore-not-found --wait=true --timeout=25s >/dev/null; then printf 'CLEANUP: FAILED, delete objects labelled %s=%s by hand\n' "${probe_label}" "${run_id}" @@ -154,6 +164,13 @@ cleanup() { summary "**Cleanup failed:** delete objects labelled \`${probe_label}=${run_id}\` by hand." exit 4 fi + remaining="$(kc -n "${probe_namespace}" get pods,ciliumnetworkpolicies -l "${probe_label}=${run_id}" -o name)" || remaining='unknown' + if [[ -n "${remaining}" ]]; then + printf 'CLEANUP: FAILED, absence of run-owned objects was not verified\n' + summary "" + summary "**Cleanup failed:** absence of run-owned objects was not verified." + exit 4 + fi printf 'CLEANUP: done\n' exit "${rc}" } @@ -239,7 +256,7 @@ spec: - | set -u control() { - curl -s -o /dev/null --max-time "\$TIMEOUT" -H "Host: \$CONTROL_HOST" -w '%{http_code}' "http://\$GATEWAY_IP/" 2>/dev/null || true + curl -s ${curl_transport_option} -o /dev/null --max-time "\$TIMEOUT" -H "Host: \$CONTROL_HOST" -w '%{http_code}' "http://\$GATEWAY_IP/" 2>/dev/null || true } i=0 first='' @@ -255,7 +272,7 @@ spec: i=0 while [ "\$i" -lt "\$ATTEMPTS" ]; do i=\$((i + 1)) - code="\$(curl -sk -o /dev/null --max-time "\$TIMEOUT" --resolve "\$TLS_HOST:443:\$GATEWAY_IP" -w '%{http_code} %{time_appconnect}' "https://\$TLS_HOST/" 2>/dev/null)" + code="\$(curl -sk ${curl_transport_option} -o /dev/null --max-time "\$TIMEOUT" --resolve "\$TLS_HOST:443:\$GATEWAY_IP" -w '%{http_code} %{time_appconnect}' "https://\$TLS_HOST/" 2>/dev/null)" printf 'PROBE tls %s %s\\n' "\$?" "\$code" done printf 'PROBE control-after %s\\n' "\$(control)" diff --git a/scripts/tests/gateway-denied-peer-wire/main.go b/scripts/tests/gateway-denied-peer-wire/main.go new file mode 100644 index 0000000000..dc84344488 --- /dev/null +++ b/scripts/tests/gateway-denied-peer-wire/main.go @@ -0,0 +1,126 @@ +// Exercise the rendered diagnostic with real curl and synthetic loopback peers. +package main + +import ( + "bufio" + "bytes" + "context" + "fmt" + "io" + "net" + "os" + "os/exec" + "path/filepath" + "strings" + "time" +) + +func serve(listener net.Listener, proxy, secure bool, results chan<- error) { + for { + conn, err := listener.Accept() + if err != nil { + return + } + func() { + defer func() { _ = conn.Close() }() + _ = conn.SetDeadline(time.Now().Add(2 * time.Second)) + reader := bufio.NewReader(conn) + if proxy { + header, err := reader.ReadString('\n') + if err != nil || !strings.HasPrefix(header, "PROXY TCP4 ") || !strings.HasSuffix(header, "\r\n") || len(strings.Fields(header)) != 6 { + results <- fmt.Errorf("missing valid PROXY preamble") + return + } + } + if secure { + header := make([]byte, 5) + if _, err := io.ReadFull(reader, header); err != nil || header[0] != 22 || header[1] != 3 { + results <- fmt.Errorf("missing TLS ClientHello after transport framing") + return + } + // Deliberately refuse before the handshake, just as a denied physical peer is closed. + results <- nil + return + } + line, err := reader.ReadString('\n') + if err != nil || line != "GET / HTTP/1.1\r\n" { + results <- fmt.Errorf("missing HTTP request after transport framing") + return + } + _, err = io.WriteString(conn, "HTTP/1.1 301 Moved Permanently\r\nContent-Length: 0\r\nConnection: close\r\n\r\n") + results <- err + }() + } +} + +func run() error { + if len(os.Args) != 3 || os.Args[2] != "plain" && os.Args[2] != "proxy-v1" { + return fmt.Errorf("arguments") + } + httpPeer, err := net.Listen("tcp4", "127.0.0.1:0") + if err != nil { + return err + } + defer func() { _ = httpPeer.Close() }() + tlsPeer, err := net.Listen("tcp4", "127.0.0.1:0") + if err != nil { + return err + } + defer func() { _ = tlsPeer.Close() }() + results := make(chan error, 32) + go serve(httpPeer, os.Args[2] == "proxy-v1", false, results) + go serve(tlsPeer, os.Args[2] == "proxy-v1", true, results) + curl, err := exec.LookPath("curl") + if err != nil { + return err + } + work, err := os.MkdirTemp("", "gateway-denied-wire-") + if err != nil { + return err + } + defer func() { _ = os.RemoveAll(work) }() + wrapper := "#!/bin/sh\nexec \"$WIRE_CURL\" --noproxy '*' --connect-to \"$GATEWAY_IP:80:$WIRE_HTTP\" --connect-to \"$TLS_HOST:443:$WIRE_TLS\" \"$@\"\n" + if err := os.WriteFile(filepath.Join(work, "curl"), []byte(wrapper), 0700); err != nil { + return err + } + ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) + defer cancel() + command := exec.CommandContext(ctx, "sh", os.Args[1]) + command.Env = append(os.Environ(), "PATH="+work+":"+os.Getenv("PATH"), "WIRE_CURL="+curl, + "WIRE_HTTP="+httpPeer.Addr().String(), "WIRE_TLS="+tlsPeer.Addr().String(), + "ATTEMPTS=2", "WARMUP_ATTEMPTS=1", "TIMEOUT=1", "SETTLE_SECONDS=0", + "GATEWAY_IP=192.0.2.1", "TLS_HOST=example.test", "CONTROL_HOST=x.invalid") + output, err := command.CombinedOutput() + if err != nil || !bytes.Contains(output, []byte("PROBE control-before 301\n")) || !bytes.Contains(output, []byte("PROBE control-after 301\n")) { + return fmt.Errorf("native controls failed: %v: %s", err, output) + } + for _, line := range strings.Split(strings.TrimSpace(string(output)), "\n") { + if strings.HasPrefix(line, "PROBE tls ") && !strings.HasPrefix(line, "PROBE tls 35 000 0.000000") && !strings.HasPrefix(line, "PROBE tls 56 000 0.000000") { + return fmt.Errorf("native TLS refusal: %s", line) + } + } + for i := 0; i < 4; i++ { + select { + case err := <-results: + if err != nil { + return err + } + default: + return fmt.Errorf("native request count below four") + } + } + select { + case <-results: + return fmt.Errorf("unexpected extra request") + default: + } + fmt.Printf("GATEWAY_DIAGNOSTIC_WIRE=PASS transport=%s requests=4\n", os.Args[2]) + return nil +} + +func main() { + if err := run(); err != nil { + fmt.Fprintln(os.Stderr, err) + os.Exit(1) + } +} diff --git a/scripts/tests/test-probe-gateway-denied-peer.sh b/scripts/tests/test-probe-gateway-denied-peer.sh index 31d0d579c2..089c029abd 100755 --- a/scripts/tests/test-probe-gateway-denied-peer.sh +++ b/scripts/tests/test-probe-gateway-denied-peer.sh @@ -43,6 +43,11 @@ case "${args}" in exit 0 ;; *' get pods,ciliumnetworkpolicies '*) + [[ -f "${FIXTURES}/cleanup-read-fails" && -f "${FIXTURES}/applied" ]] && exit 1 + if [[ -f "${FIXTURES}/delete-keeps-objects" && -f "${FIXTURES}/applied" ]]; then + printf 'pod/gateway-peer-probe-12345\n' + exit 0 + fi cat "${FIXTURES}/leftovers" 2>/dev/null || true exit 0 ;; @@ -84,10 +89,10 @@ fixture() { # Runs the probe against a fixture set; expects the exit code and the verdict line. expect() { - local name="$1" want_rc="$2" want_verdict="$3" rc=0 + local name="$1" want_rc="$2" want_verdict="$3" transport="${4:-plain}" rc=0 local dir="${work}/${name}" PATH="${work}/bin:${PATH}" FIXTURES="${dir}" PROBE_POLL_SECONDS=0 GITHUB_STEP_SUMMARY="${dir}/summary" \ - bash "${script}" --context admin@prod --run-id 12345 --attempts 3 >"${dir}/out" 2>&1 || rc=$? + bash "${script}" --context admin@prod --run-id 12345 --attempts 3 --transport "${transport}" >"${dir}/out" 2>&1 || rc=$? [[ "${rc}" == "${want_rc}" ]] || fail "${name}: exit ${rc}, want ${want_rc}: $(tr '\n' '|' <"${dir}/out")" if [[ -n "${want_verdict}" ]]; then grep -q "^VERDICT: ${want_verdict}\$" "${dir}/out" || fail "${name}: no VERDICT ${want_verdict}: $(tr '\n' '|' <"${dir}/out")" @@ -100,6 +105,8 @@ expect() { # The positive case, and one changed fixture for each thing it depends on. fixture refused 35 000 301 301 3 expect refused 0 REFUSED +fixture proxy-refused 35 000 301 301 3 +expect proxy-refused 0 REFUSED proxy-v1 fixture reset 56 000 301 301 5 expect reset 0 REFUSED fixture accepted 0 200 301 301 0 3 0.094778 @@ -157,9 +164,16 @@ fixture cleanup-fails 35 000 301 301 3 touch "${work}/cleanup-fails/delete-fails" expect cleanup-fails 4 '' grep -q '^CLEANUP: FAILED' "${work}/cleanup-fails/out" || fail 'cleanup-fails: not reported' +fixture cleanup-retained 35 000 301 301 3 +touch "${work}/cleanup-retained/delete-keeps-objects" +expect cleanup-retained 4 '' +grep -q '^CLEANUP: FAILED' "${work}/cleanup-retained/out" || fail 'cleanup-retained: deletion success concealed retained resources' +fixture cleanup-unknown 35 000 301 301 3 +touch "${work}/cleanup-unknown/cleanup-read-fails" +expect cleanup-unknown 4 '' # Usage errors read and write nothing. -for bad in '--run-id 012' '--run-id abc' '--run-id 12345 --attempts 0' '--run-id 12345 --attempts 31' '--run-id 12345 --attempts 08'; do +for bad in '--run-id 012 --transport plain' '--run-id abc --transport plain' '--run-id 12345 --attempts 0 --transport plain' '--run-id 12345 --attempts 31 --transport plain' '--run-id 12345 --attempts 08 --transport plain' '--run-id 12345 --transport guessed' '--run-id 12345'; do dir="${work}/usage" rm -rf "${dir}" mkdir -p "${dir}" @@ -216,6 +230,58 @@ pod_out="$(PATH="${work}/pod/bin:${PATH}" ATTEMPTS=2 WARMUP_ATTEMPTS=1 TIMEOUT=1 want_pod="$(printf 'PROBE control-before 301\nPROBE-READY\nPROBE tls 35 000 0.000000\nPROBE tls 35 000 0.000000\nPROBE control-after 301\nPROBE-DONE')" [[ "${pod_out}" == "${want_pod}" ]] || fail "the pod script printed: $(tr '\n' '|' <<<"${pod_out}")" +# Execute the rendered shell with real curl and loopback listeners. A wrapper changes only the +# destination socket; the listeners inspect the actual bytes before HTTP or the TLS ClientHello. +for transport in plain proxy-v1; do + fixture_name=refused + [[ "${transport}" == proxy-v1 ]] && fixture_name=proxy-refused + if [[ ! -f "${work}/${fixture_name}/applied" ]]; then + fail "wire ${transport}: no rendered pod" + continue + fi + sed -n '/^ - |$/,/^ securityContext:$/p' "${work}/${fixture_name}/applied" | sed '1d;$d' | sed 's/^ //' >"${work}/pod/wire.sh" + go run "${repo}/scripts/tests/gateway-denied-peer-wire/main.go" "${work}/pod/wire.sh" "${transport}" || fail "wire ${transport}: native framing failed" + if [[ "${transport}" == proxy-v1 ]]; then + sed 's/curl -sk --haproxy-protocol /curl -sk /' "${work}/pod/wire.sh" >"${work}/pod/tls-framing-mutant.sh" + if go run "${repo}/scripts/tests/gateway-denied-peer-wire/main.go" "${work}/pod/tls-framing-mutant.sh" proxy-v1 >/dev/null 2>&1; then + fail 'wire proxy-v1: missing TLS framing was not detected' + fi + fi +done + +# Run the workflow's actual receipt parser. A stale success line or an unknown mode must never +# select a transport, even when it contains a plausible fenced=true substring. +# shellcheck disable=SC2016 # extract literal workflow shell, including its command substitution +sed -n '/verified="$(go run /,/^ - name: Probe /p' "${workflow}" | sed '$d;s/^ //' >"${work}/binding.sh" +cat >"${work}/bin/go" <<'FAKE' +#!/usr/bin/env bash +printf '%s\n' "${BINDING_RECEIPT}" +FAKE +chmod +x "${work}/bin/go" +for mode in true false; do + output="${work}/binding-${mode}" + if ! PATH="${work}/bin:${PATH}" BINDING_RECEIPT="GATEWAY_RECEIVER_SOURCE=VERIFIED public_probes=4 fence_selected=true proxy_protocol=${mode}" GITHUB_OUTPUT="${output}" bash "${work}/binding.sh" >/dev/null; then + fail "workflow: verified ${mode} profile refused" + fi + want=plain + [[ "${mode}" == false ]] || want=proxy-v1 + [[ "$(cat "${output}" 2>/dev/null)" == "transport=${want}" ]] || fail "workflow: wrong ${mode} transport" +done +for receipt in \ + 'GATEWAY_RECEIVER_SOURCE=VERIFIED public_probes=4 fence_selected=true' \ + 'GATEWAY_RECEIVER_SOURCE=VERIFIED public_probes=4 fence_selected=false proxy_protocol=false' \ + 'GATEWAY_RECEIVER_SOURCE=VERIFIED public_probes=4 fence_selected=true proxy_protocol=unknown' \ + 'GATEWAY_RECEIVER_SOURCE=UNVERIFIED public_probes=4 fence_selected=true proxy_protocol=true' \ + "$(printf 'extra\nGATEWAY_RECEIVER_SOURCE=VERIFIED public_probes=4 fence_selected=true proxy_protocol=true')"; do + output="${work}/binding-invalid" + rm -f "${output}" + if PATH="${work}/bin:${PATH}" BINDING_RECEIPT="${receipt}" GITHUB_OUTPUT="${output}" bash "${work}/binding.sh" >/dev/null 2>&1; then + fail 'workflow: an unverified receipt selected a transport' + fi + [[ ! -e "${output}" ]] || fail 'workflow: invalid receipt emitted a step output' +done +rm "${work}/bin/go" + # The workflow: dormant, main only, confirmed, serialised with deploys. grep -qE '^ workflow_dispatch:$' "${workflow}" || fail 'workflow: not dispatch-only' if grep -qE '^ (schedule|pull_request|push|merge_group):' "${workflow}"; then From 57c42c48c1bed26a4e239c9a1405e1aec52027f1 Mon Sep 17 00:00:00 2001 From: Nikolai Emil Damm Date: Tue, 3 Nov 2026 09:59:30 +0100 Subject: [PATCH 2/3] test(gateway): bound native wire receipt synchronization --- scripts/tests/gateway-denied-peer-wire/main.go | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/scripts/tests/gateway-denied-peer-wire/main.go b/scripts/tests/gateway-denied-peer-wire/main.go index dc84344488..598e09f4e1 100644 --- a/scripts/tests/gateway-denied-peer-wire/main.go +++ b/scripts/tests/gateway-denied-peer-wire/main.go @@ -94,18 +94,25 @@ func run() error { if err != nil || !bytes.Contains(output, []byte("PROBE control-before 301\n")) || !bytes.Contains(output, []byte("PROBE control-after 301\n")) { return fmt.Errorf("native controls failed: %v: %s", err, output) } + attempts := 0 for _, line := range strings.Split(strings.TrimSpace(string(output)), "\n") { - if strings.HasPrefix(line, "PROBE tls ") && !strings.HasPrefix(line, "PROBE tls 35 000 0.000000") && !strings.HasPrefix(line, "PROBE tls 56 000 0.000000") { - return fmt.Errorf("native TLS refusal: %s", line) + if strings.HasPrefix(line, "PROBE tls ") { + attempts++ + if !strings.HasPrefix(line, "PROBE tls 35 000 0.000000") && !strings.HasPrefix(line, "PROBE tls 56 000 0.000000") { + return fmt.Errorf("native TLS refusal: %s", line) + } } } + if attempts != 2 { + return fmt.Errorf("native TLS attempt count is %d, want two", attempts) + } for i := 0; i < 4; i++ { select { case err := <-results: if err != nil { return err } - default: + case <-ctx.Done(): return fmt.Errorf("native request count below four") } } From 293a1cfcb88b15136792cb1cef35913cedaa6c98 Mon Sep 17 00:00:00 2001 From: Nikolai Emil Damm Date: Tue, 3 Nov 2026 10:24:21 +0100 Subject: [PATCH 3/3] fix(email): restrict backup cleanup to its parent shell --- scripts/refresh-email-recovery-backup.sh | 4 ++ .../backup_refresh_test.go | 38 +++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/scripts/refresh-email-recovery-backup.sh b/scripts/refresh-email-recovery-backup.sh index 389e01a61c..7f94f6a54e 100755 --- a/scripts/refresh-email-recovery-backup.sh +++ b/scripts/refresh-email-recovery-backup.sh @@ -5,8 +5,12 @@ umask 077 stage=dispatch work='' watchers=() +readonly owner_bashpid="$BASHPID" fail() { printf 'EMAIL_BACKUP_REFRESH=UNVERIFIED stage=%s\n' "$stage" >&2; exit 2; } cleanup() { + # A terminated watch may inherit this EXIT handler before Bash resets it. + # Only the parent owns the workspace and the watch processes. + [[ "$BASHPID" == "$owner_bashpid" ]] || return 0 for pid in "${watchers[@]}"; do kill "$pid" 2>/dev/null || true; done for pid in "${watchers[@]}"; do wait "$pid" 2>/dev/null || true; done if [[ -n "$work" ]]; then rm -rf "$work"; fi diff --git a/scripts/tests/vault-email-credentials/backup_refresh_test.go b/scripts/tests/vault-email-credentials/backup_refresh_test.go index b44106f9cb..7994ab7873 100644 --- a/scripts/tests/vault-email-credentials/backup_refresh_test.go +++ b/scripts/tests/vault-email-credentials/backup_refresh_test.go @@ -10,6 +10,44 @@ import ( "testing" ) +func TestBackupRefreshCleanupBelongsToTheParentShell(t *testing.T) { + raw, err := os.ReadFile(filepath.Join(repositoryRoot(), "scripts/refresh-email-recovery-backup.sh")) + if err != nil { + t.Fatal(err) + } + prefix, _, ok := strings.Cut(string(raw), "\n[[ ${BASH_VERSINFO") + if !ok { + t.Fatal("missing dispatch boundary") + } + // Bash can terminate an asynchronous child before it resets its inherited EXIT + // handler. Reinstall that native handler in a real child to select the otherwise + // scheduling-dependent window, then exercise the actual cleanup function. + command := exec.Command("bash", "-c", prefix+` +work="$FIXTURE/work" +mkdir "$work" +sleep 60 & +watchers=("$!") +watch_pid="${watchers[0]}" +inherited=$(trap -p EXIT) +( eval "$inherited"; exit 0 ) +[[ -d "$work" ]] || { printf 'child removed parent workspace\n'; exit 1; } +kill -0 "$watch_pid" || { printf 'child stopped parent watch\n'; exit 1; } +cleanup +watchers=() +trap - EXIT +[[ ! -e "$work" ]] || { printf 'parent retained its workspace\n'; exit 1; } +if kill -0 "$watch_pid" 2>/dev/null; then + printf 'parent retained its watch\n'; exit 1 +fi +printf 'BACKUP_CLEANUP_OWNER=PASS child=REFUSED parent=VERIFIED\n' +`) + command.Env = append(os.Environ(), "FIXTURE="+t.TempDir()) + output, err := command.CombinedOutput() + if err != nil || !strings.Contains(string(output), "BACKUP_CLEANUP_OWNER=PASS child=REFUSED parent=VERIFIED") { + t.Fatalf("cleanup ownership: %v: %s", err, output) + } +} + func TestBackupRefreshRequiresTheOwnedCompletedSnapshotAndMirror(t *testing.T) { for _, scenario := range []string{"complete", "owned-unfinished", "owned-complete", "foreign-unfinished", "jobs-read-failed", "jobs-partial", "active", "image", "pvc", "changed", "create-unknown", "failed", "replacement", "pod-missing"} { t.Run(scenario, func(t *testing.T) {