From 6e86626cf771e89a369c7bcae314202ea376c41c Mon Sep 17 00:00:00 2001 From: fujibee Date: Mon, 17 Aug 2026 01:27:35 -0700 Subject: [PATCH 1/4] fix(remote): render embedded curl-config paths for the platform (#850) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `_remote_http_post_json` writes a curl `-K` config so the request body — which holds the token — never reaches curl's argv. Two of the values it writes are paths: `dump-header` and `data = "@..."`. On Windows/Git Bash those paths are unopenable. MSYS translates POSIX paths to Windows form for a native binary's ARGV, and not for the contents of a file that binary reads. So `/tmp/...` written into the config stays `/tmp/...`, and native curl has no such path. curl fails, and the caller reports the "000" it reports for every failure, with no indication that a path was the problem. `cygpath -m`, not `-w`: -m yields a Windows drive path with FORWARD slashes, and curl's config parser treats a backslash as an escape — so the -w form is re-mangled by curl itself. Measured on the machine, not reasoned about. Gated on `command -v cygpath`, not on an OS name. Where cygpath does not exist the helper returns its argument unchanged, so macOS and Linux take exactly the path they took before — verified directly rather than inferred from a green suite: the helper is a pass-through for /tmp, $TMPDIR and a /private/tmp path on this machine. --- scripts/remote.sh | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/scripts/remote.sh b/scripts/remote.sh index e17de5846..6f2d0b9b9 100644 --- a/scripts/remote.sh +++ b/scripts/remote.sh @@ -285,6 +285,17 @@ cmd_doctor() { # --- shared HTTP helpers (B1: never put secrets in curl's own argv/ps) --- +# _remote_curl_path — render for embedding INSIDE a curl -K config +# file. On Windows/Git Bash, MSYS translates POSIX paths to Windows form only for +# a native binary's argv, NOT for paths read from a config file's contents, so an +# embedded /tmp or /c/.. path is unopenable by native curl (→ curl fails → the +# caller's HTTP 000). cygpath -m yields a Windows drive path with FORWARD slashes; +# -w is wrong here because curl's config parser treats backslashes as escapes. +# Capability-gated on cygpath so macOS/Linux (no cygpath) are unchanged. +_remote_curl_path() { + if command -v cygpath >/dev/null 2>&1; then cygpath -m "$1"; else printf '%s' "$1"; fi +} + # _remote_http_post_json -> prints http_code # Posts as the request body via a curl -K config file, so the # body (which holds the token) never appears in curl's own argv/ps. The @@ -311,11 +322,11 @@ _remote_http_post_json() { printf 'request = "POST"\n' printf 'header = "Content-Type: application/json"\n' printf 'header = "Agmsg-Protocol-Version: 1"\n' - printf 'dump-header = "%s"\n' "$header_fifo" + printf 'dump-header = "%s"\n' "$(_remote_curl_path "$header_fifo")" printf 'connect-timeout = "10"\n' printf 'max-time = "15"\n' printf 'max-filesize = "2097152"\n' - printf 'data = "@%s"\n' "$body_file" + printf 'data = "@%s"\n' "$(_remote_curl_path "$body_file")" } > "$cfg" if curl_output=$(curl -sS -o "$out_file" -w '%{http_code}' -K "$cfg" 2>/dev/null); then : From 2f21bf42c4649511fdbc2dc3738e66321b63b759 Mon Sep 17 00:00:00 2001 From: fujibee Date: Mon, 17 Aug 2026 15:39:03 -0700 Subject: [PATCH 2/4] test(remote): bind the curl-config path rendering, both fields separately (#850) The stop condition was that reverting the production change leaves CI green. It does now: reverting the renderer turns two tests red, and reverting either single field turns exactly its own test red. The test drives the real _remote_http_post_json, sourced from remote.sh, with two stubs on PATH. What makes the stub worth anything is that it does not just record the config -- it OPENS WHAT THE CONFIG NAMES: cygpath -m yields forward slashes, -w yields backslashes. It must really produce both, or the test cannot tell the two apart, and telling them apart is the whole point of the fix naming -m. curl resolves each embedded path the way a native Windows binary resolves a Windows path back to the same file, and EXITS NON-ZERO when it cannot. A stub that accepts any string as a path would pass on a rendering no curl could open -- which is the defect itself. Four cases: no cygpath both fields byte-for-byte, so macOS and Linux are demonstrably unchanged rather than assumed unchanged cygpath, data rendered mixed cygpath, dump-header rendered mixed either way no backslash anywhere in the config The two field cases were one test until the matrix showed why they cannot be. Reverting the header field and reverting the data field both reddened the same combined assertion, which says 'something is untranslated' and points at neither. They are two effects of one line and regress apart: translate the body and not the header and curl opens the body, fails on the header, and the caller reports 000 -- which reads as a header problem. A fifth case is the control on the stub: replace _remote_curl_path after sourcing so it emits a path nothing can open, and the helper must return 000 -- the code the user actually saw. Without it, every assertion above could be passing on a string that no curl would accept. M0 no mutation 0 red M1 renderer back to passthrough 2 -- DATA and DUMP-HEADER M2 cygpath -m becomes -w 3 -- both fields and the slash assertion M3 header field left untranslated 1 -- DUMP-HEADER M4 data field left untranslated 1 -- DATA Production is untouched. Windows: still unverified here -- the cygpath branch is driven by a stub, and a stub cannot tell you what MSYS does. --- tests/test_remote_curl_config_paths.bats | 224 +++++++++++++++++++++++ 1 file changed, 224 insertions(+) create mode 100644 tests/test_remote_curl_config_paths.bats diff --git a/tests/test_remote_curl_config_paths.bats b/tests/test_remote_curl_config_paths.bats new file mode 100644 index 000000000..20e571622 --- /dev/null +++ b/tests/test_remote_curl_config_paths.bats @@ -0,0 +1,224 @@ +#!/usr/bin/env bats +# PATHS EMBEDDED IN A CURL CONFIG FILE ARE NOT TRANSLATED FOR YOU (#850). +# +# `_remote_http_post_json` hands curl its arguments in a `-K` config file so the +# request body -- which carries the token -- never appears in curl's argv. On +# Windows that has a consequence the POSIX side never sees: MSYS rewrites POSIX +# paths into Windows form for a native binary's ARGV, and does not touch the +# CONTENTS of a file that binary reads. So `data = "@/tmp/x"` reaches native +# curl as the literal string `/tmp/x`, which it cannot open. curl fails, and the +# caller reports HTTP 000 -- a network-shaped symptom for a path-shaped fault. +# +# WHAT THIS FILE DRIVES. The production `_remote_http_post_json`, sourced from +# `remote.sh`, with two stubs on PATH: +# +# cygpath present or absent, which is the capability the fix gates on +# curl captures the config it was given, and OPENS WHAT IT NAMES +# +# The second half of that stub is the point. A stub that only records the +# config would pass whatever the config said, including a path no curl could +# open -- which is the defect. This one resolves the path the way MSYS would, +# writes the headers, and FAILS if it cannot, so a wrong rendering shows up as +# the same 000 the user got rather than as a passing assertion about a string. + +load test_helper + +setup() { + setup_test_env + + STUB_BIN="$BATS_TEST_TMPDIR/bin" + mkdir -p "$STUB_BIN" + CFG_CAPTURE="$BATS_TEST_TMPDIR/captured-config" + export CFG_CAPTURE + + # The Windows drive prefix the fake cygpath maps onto. Any string works; what + # matters is that the two stubs agree, so the curl stub can undo it exactly + # the way a native binary on Windows resolves a real Windows path back to the + # same file. + FAKE_ROOT="C:/msys64" + export FAKE_ROOT + + cat > "$STUB_BIN/curl" <<'STUB' +#!/usr/bin/env bash +# A stand-in for native curl on Windows: it reads the config, and it can only +# open paths that a native binary could open. +set -u +cfg="" +out="" +prev="" +for arg in "$@"; do + case "$prev" in + -K) cfg="$arg" ;; + -o) out="$arg" ;; + esac + prev="$arg" +done +[ -n "$cfg" ] || { echo "STUB_CURL: no -K config" >&2; exit 2; } +cp "$cfg" "$CFG_CAPTURE" + +# Undo the fake cygpath mapping, which is what the real MSYS/Windows pair does: +# a Windows path names the same file the POSIX path did. A path in neither form +# is one this stub cannot open -- and neither could curl. +resolve() { + case "$1" in + "$FAKE_ROOT"/*) printf '%s' "/${1#"$FAKE_ROOT"/}" ;; + /*) printf '%s' "$1" ;; + *) return 1 ;; + esac +} + +# `data = "@"` must name a readable file, or curl has nothing to post. +data_field="$(sed -n 's/^data = "@\(.*\)"$/\1/p' "$cfg")" +if [ -n "$data_field" ]; then + if ! body="$(resolve "$data_field")" || [ ! -r "$body" ]; then + echo "STUB_CURL: cannot open data path: $data_field" >&2 + exit 26 + fi +fi + +# `dump-header = ""` must be openable for writing. On the real thing this +# is the fifo the caller is already reading; failing to open it is exactly the +# fault this fix exists for. +hdr_field="$(sed -n 's/^dump-header = "\(.*\)"$/\1/p' "$cfg")" +if [ -n "$hdr_field" ]; then + if ! hdr="$(resolve "$hdr_field")"; then + echo "STUB_CURL: cannot open dump-header path: $hdr_field" >&2 + exit 23 + fi + printf 'HTTP/1.1 200 OK\r\n\r\n' > "$hdr" || { + echo "STUB_CURL: dump-header not writable: $hdr_field" >&2 + exit 23 + } +fi + +[ -z "$out" ] || printf '{"ok":true}' > "$out" +printf '200' +STUB + chmod +x "$STUB_BIN/curl" + + cat > "$STUB_BIN/cygpath" <<'STUB' +#!/usr/bin/env bash +# Mixed (-m) gives forward slashes; Windows (-w) gives backslashes. The +# difference is the whole reason the fix names one of them, so the stub must +# actually produce both -- returning the same string for either would make the +# test unable to tell the two apart. +set -u +mode="$1"; path="$2" +case "$mode" in + -m) printf '%s%s' "$FAKE_ROOT" "$path" ;; + -w) printf '%s%s' "${FAKE_ROOT//\//\\}" "${path//\//\\}" ;; + *) echo "stub cygpath: unexpected mode $mode" >&2; exit 64 ;; +esac +STUB + chmod +x "$STUB_BIN/cygpath" +} + +teardown() { teardown_test_env; } + +# Runs the real helper with PATH arranged by the caller, and prints the http +# code it returned. `cygpath` is present only when asked for. +post_with() { + local with_cygpath="$1" body_file="$2" + local bin="$STUB_BIN" + if [ "$with_cygpath" = "no" ]; then + bin="$BATS_TEST_TMPDIR/bin-nocygpath" + mkdir -p "$bin" + ln -sf "$STUB_BIN/curl" "$bin/curl" + fi + run env PATH="$bin:$PATH" bash -c ' + set -uo pipefail + . '"$SCRIPTS"'/remote.sh 2>/dev/null + _remote_http_post_json "https://example.invalid/v1/x" "'"$body_file"'" \ + "'"$BATS_TEST_TMPDIR"'/out-body" "'"$BATS_TEST_TMPDIR"'/out-header" + ' +} + +@test "without cygpath the embedded paths are passed through byte for byte (#850)" { + # macOS and Linux have no cygpath, and this is the assertion that says the fix + # costs them nothing: the config must hold the exact strings the caller built. + body="$BATS_TEST_TMPDIR/body.json" + printf '{"t":"secret"}' > "$body" + + post_with no "$body" + [ "$status" -eq 0 ] + [ "$output" = "200" ] + + # The data path is one the test chose, so it can be compared exactly. + grep -q -F -- "data = \"@$body\"" "$CFG_CAPTURE" + + # The header path is generated inside the helper, so what is asserted is its + # shape: still POSIX, and untouched by any translation. + hdr="$(sed -n 's/^dump-header = "\(.*\)"$/\1/p' "$CFG_CAPTURE")" + [ -n "$hdr" ] + case "$hdr" in /*) : ;; *) echo "not a POSIX path: $hdr"; return 1 ;; esac + case "$hdr" in *"$FAKE_ROOT"*) echo "translated with no cygpath present: $hdr"; return 1 ;; esac +} + +@test "with cygpath the DATA path is rendered in mixed Windows form (#850)" { + # Two fields, two tests, because they are two effects of one line and can + # regress apart. Asserting both in one test says "something is untranslated" + # -- which is true of either, and points at neither. Reverting the header + # field alone must not be able to fail this one. + body="$BATS_TEST_TMPDIR/body.json" + printf '{"t":"secret"}' > "$body" + + post_with yes "$body" + [ "$status" -eq 0 ] + [ "$output" = "200" ] + + grep -q -F -- "data = \"@$FAKE_ROOT$body\"" "$CFG_CAPTURE" +} + +@test "with cygpath the DUMP-HEADER path is rendered in mixed Windows form (#850)" { + # The half that is easy to leave behind: the body path is the one a reader + # thinks of as "the file", and the header sink is generated inside the helper. + # Translate one and not the other and curl opens the body, fails on the + # header, and the caller reports 000 -- which reads as a header problem. + body="$BATS_TEST_TMPDIR/body.json" + printf '{"t":"secret"}' > "$body" + + post_with yes "$body" + [ "$status" -eq 0 ] + [ "$output" = "200" ] + + hdr="$(sed -n 's/^dump-header = "\(.*\)"$/\1/p' "$CFG_CAPTURE")" + case "$hdr" in "$FAKE_ROOT"/*) : ;; *) echo "header path not translated: $hdr"; return 1 ;; esac +} + +@test "the rendered paths carry forward slashes, never backslashes (#850)" { + # `cygpath -w` also produces a valid Windows path, and it is the wrong one: + # curl's config parser reads a backslash as an escape, so the path arrives + # corrupted. This is the assertion that separates -m from -w, and it is + # written against the config text rather than against the flag, because what + # breaks a user is the bytes curl parses. + body="$BATS_TEST_TMPDIR/body.json" + printf '{"t":"secret"}' > "$body" + + post_with yes "$body" + [ "$status" -eq 0 ] + + # No backslash anywhere in either embedded path. + ! grep -q '\\' "$CFG_CAPTURE" +} + +@test "a path curl cannot open surfaces as HTTP 000, the way the user saw it (#850)" { + # The negative control for the stub itself. If the stub accepted any string as + # a path, every assertion above would pass on a broken rendering -- so make + # the rendering broken on purpose and confirm the stub notices. + # + # `_remote_curl_path` is replaced AFTER sourcing, so the production helper is + # still the one under test; only its path renderer is made to produce + # something no curl could open. + body="$BATS_TEST_TMPDIR/body.json" + printf '{"t":"secret"}' > "$body" + + run env PATH="$STUB_BIN:$PATH" bash -c ' + set -uo pipefail + . '"$SCRIPTS"'/remote.sh 2>/dev/null + _remote_curl_path() { printf "Z:\\\\nowhere\\\\%s" "$1"; } + _remote_http_post_json "https://example.invalid/v1/x" "'"$body"'" \ + "'"$BATS_TEST_TMPDIR"'/out-body" "'"$BATS_TEST_TMPDIR"'/out-header" + ' + [ "$status" -eq 0 ] + [ "$output" = "000" ] +} From 432f2f3b37e4fe694467a8e8d762b6b412e4bdc3 Mon Sep 17 00:00:00 2001 From: fujibee Date: Mon, 17 Aug 2026 16:05:57 -0700 Subject: [PATCH 3/4] test(remote): make the stubs disagree the way the two platforms do (#850) Review found two holes, both in my instrument rather than in the fix. P1-a THE NEGATIVE ARM WAS NOT NEGATIVE. 'without cygpath' prepended a directory holding only a curl symlink and left the host PATH behind it, so production's 'command -v cygpath' would still have found a real one. It passed here because macOS has no cygpath -- the test inherited an absence instead of creating it, and on the one machine this fix is for it would have run the wrong arm. Now an allowlist sandbox, the shape test_helper already uses for path_without_python3. A subtraction cannot express this: on Git Bash cygpath lives in the same directory as mktemp. The first test asserts the sandbox produces BOTH answers, so the capability every other arm rests on is measured rather than assumed. P1-b THE STUB ACCEPTED THE BROKEN PATH. Its resolve() took any absolute POSIX path whatever the platform, so it could not fail on the untranslated /tmp/... that broke the user -- while the file header claimed it opened what curl would open and the commit message claimed it failed where curl would fail. M1 went red on string assertions alone. The claim and the stub now say the same thing. The stub is told which consumer it plays. native-windows opens only Windows paths; posix opens only POSIX ones. Neither accepts both, which is the actual shape of the defect: the same bytes are openable or not depending on who reads them. Both directions have a case, so 'refuses a POSIX path' cannot be read as 'refuses whatever the test needs'. The outcome assertion moved out of the two field tests, and this was forced by the matrix rather than chosen. With the stub now failing on either untranslated field, asserting 200 inside both field tests made both go red for either mutation -- destroying the per-field attribution that splitting them was for. The 200 has its own test, whose job is to fail for any of them. M0 no mutation 0 red M1 renderer back to passthrough 3 -- DATA, DUMP-HEADER, completes M2 cygpath -m becomes -w 4 -- those three and the slash assertion M3 header field untranslated 2 -- DUMP-HEADER and completes M4 data field untranslated 2 -- DATA and completes 7 ok / 0 not ok. Production untouched; the diff is this one test file. Windows: still unverified by me. The cygpath arm is driven by stubs, and a stub cannot tell you what MSYS does. What this closes is 'revert production and CI stays green', not 'it works on Windows'. --- tests/test_remote_curl_config_paths.bats | 233 +++++++++++++++-------- 1 file changed, 152 insertions(+), 81 deletions(-) diff --git a/tests/test_remote_curl_config_paths.bats b/tests/test_remote_curl_config_paths.bats index 20e571622..782714c84 100644 --- a/tests/test_remote_curl_config_paths.bats +++ b/tests/test_remote_curl_config_paths.bats @@ -9,43 +9,60 @@ # curl as the literal string `/tmp/x`, which it cannot open. curl fails, and the # caller reports HTTP 000 -- a network-shaped symptom for a path-shaped fault. # -# WHAT THIS FILE DRIVES. The production `_remote_http_post_json`, sourced from -# `remote.sh`, with two stubs on PATH: +# THE SAME BYTES, TWO CONSUMERS. That is what makes this hard to test honestly: +# `/tmp/x` is openable by a POSIX curl and not by a native Windows one, so a +# path string is only right or wrong RELATIVE TO WHO READS IT. The stubs here +# are therefore capability-paired, and each test states which pairing it runs: # -# cygpath present or absent, which is the capability the fix gates on -# curl captures the config it was given, and OPENS WHAT IT NAMES +# PATH an allowlist sandbox -- cygpath is ABSENT because this file did +# not link it, not because the host happens to lack one. Each arm +# asserts the capability it relies on before relying on it. +# curl told which consumer it is playing. As `native-windows` it +# refuses a POSIX path the way real curl.exe would; as `posix` it +# refuses a Windows path. Neither accepts both. +# cygpath -m yields forward slashes, -w yields backslashes -- really both, +# because telling them apart is the whole reason the fix names -m. # -# The second half of that stub is the point. A stub that only records the -# config would pass whatever the config said, including a path no curl could -# open -- which is the defect. This one resolves the path the way MSYS would, -# writes the headers, and FAILS if it cannot, so a wrong rendering shows up as -# the same 000 the user got rather than as a passing assertion about a string. +# An earlier version of this file got the second point wrong: its stub accepted +# any absolute POSIX path whatever the platform, so it could not have failed on +# the untranslated path that broke the user, and the file's own header claimed +# otherwise. Review caught it. What follows is written so that the claim and the +# stub say the same thing. load test_helper +# Externals `remote.sh` needs to source, plus those `_remote_http_post_json` +# and the curl stub call. Derived by sourcing under an empty PATH and adding +# what it asked for, then reading the function for the rest: mktemp, mkfifo, +# chmod, python3 (the bounded-copy reader), rm, rmdir. +# +# An allowlist rather than a subtraction, matching `path_without_python3` in +# test_helper. A subtraction cannot express "cygpath is absent" on a machine +# where cygpath lives in the same directory as mktemp, which is every Git Bash. +SANDBOX_TOOLS=(bash dirname mktemp mkfifo chmod rm rmdir sed cp cat grep python3 uname) + setup() { setup_test_env - STUB_BIN="$BATS_TEST_TMPDIR/bin" - mkdir -p "$STUB_BIN" CFG_CAPTURE="$BATS_TEST_TMPDIR/captured-config" export CFG_CAPTURE # The Windows drive prefix the fake cygpath maps onto. Any string works; what - # matters is that the two stubs agree, so the curl stub can undo it exactly - # the way a native binary on Windows resolves a real Windows path back to the - # same file. + # matters is that the stubs agree, so the curl stub can undo it exactly the + # way a native binary resolves a Windows path back to the same file. FAKE_ROOT="C:/msys64" export FAKE_ROOT - cat > "$STUB_BIN/curl" <<'STUB' + STUB_SRC="$BATS_TEST_TMPDIR/stubs" + mkdir -p "$STUB_SRC" + + cat > "$STUB_SRC/curl" <<'STUB' #!/usr/bin/env bash -# A stand-in for native curl on Windows: it reads the config, and it can only -# open paths that a native binary could open. +# Stands in for whichever curl the config is destined for. STUB_CURL_CONSUMER +# decides which one, and the two do NOT accept the same strings -- that +# asymmetry is the defect being tested, so a stub without it proves nothing. set -u -cfg="" -out="" -prev="" +cfg=""; out=""; prev="" for arg in "$@"; do case "$prev" in -K) cfg="$arg" ;; @@ -56,37 +73,38 @@ done [ -n "$cfg" ] || { echo "STUB_CURL: no -K config" >&2; exit 2; } cp "$cfg" "$CFG_CAPTURE" -# Undo the fake cygpath mapping, which is what the real MSYS/Windows pair does: -# a Windows path names the same file the POSIX path did. A path in neither form -# is one this stub cannot open -- and neither could curl. +# Resolve an embedded path to something this consumer can open, or fail. +# +# native-windows only a Windows path opens. A POSIX path is a literal +# filename with no such directory -- the reported bug. +# posix only a POSIX path opens. A Windows path is a filename +# containing a colon, which is not a path here. resolve() { - case "$1" in - "$FAKE_ROOT"/*) printf '%s' "/${1#"$FAKE_ROOT"/}" ;; - /*) printf '%s' "$1" ;; + case "$STUB_CURL_CONSUMER:$1" in + "native-windows:$FAKE_ROOT"/*) printf '%s' "/${1#"$FAKE_ROOT"/}" ;; + native-windows:*) return 1 ;; + posix:/*) printf '%s' "$1" ;; + posix:*) return 1 ;; *) return 1 ;; esac } -# `data = "@"` must name a readable file, or curl has nothing to post. data_field="$(sed -n 's/^data = "@\(.*\)"$/\1/p' "$cfg")" if [ -n "$data_field" ]; then if ! body="$(resolve "$data_field")" || [ ! -r "$body" ]; then - echo "STUB_CURL: cannot open data path: $data_field" >&2 + echo "STUB_CURL($STUB_CURL_CONSUMER): cannot open data path: $data_field" >&2 exit 26 fi fi -# `dump-header = ""` must be openable for writing. On the real thing this -# is the fifo the caller is already reading; failing to open it is exactly the -# fault this fix exists for. hdr_field="$(sed -n 's/^dump-header = "\(.*\)"$/\1/p' "$cfg")" if [ -n "$hdr_field" ]; then if ! hdr="$(resolve "$hdr_field")"; then - echo "STUB_CURL: cannot open dump-header path: $hdr_field" >&2 + echo "STUB_CURL($STUB_CURL_CONSUMER): cannot open dump-header path: $hdr_field" >&2 exit 23 fi printf 'HTTP/1.1 200 OK\r\n\r\n' > "$hdr" || { - echo "STUB_CURL: dump-header not writable: $hdr_field" >&2 + echo "STUB_CURL($STUB_CURL_CONSUMER): dump-header not writable: $hdr_field" >&2 exit 23 } fi @@ -94,14 +112,10 @@ fi [ -z "$out" ] || printf '{"ok":true}' > "$out" printf '200' STUB - chmod +x "$STUB_BIN/curl" + chmod +x "$STUB_SRC/curl" - cat > "$STUB_BIN/cygpath" <<'STUB' + cat > "$STUB_SRC/cygpath" <<'STUB' #!/usr/bin/env bash -# Mixed (-m) gives forward slashes; Windows (-w) gives backslashes. The -# difference is the whole reason the fix names one of them, so the stub must -# actually produce both -- returning the same string for either would make the -# test unable to tell the two apart. set -u mode="$1"; path="$2" case "$mode" in @@ -110,40 +124,70 @@ case "$mode" in *) echo "stub cygpath: unexpected mode $mode" >&2; exit 64 ;; esac STUB - chmod +x "$STUB_BIN/cygpath" + chmod +x "$STUB_SRC/cygpath" } teardown() { teardown_test_env; } -# Runs the real helper with PATH arranged by the caller, and prints the http -# code it returned. `cygpath` is present only when asked for. -post_with() { - local with_cygpath="$1" body_file="$2" - local bin="$STUB_BIN" - if [ "$with_cygpath" = "no" ]; then - bin="$BATS_TEST_TMPDIR/bin-nocygpath" - mkdir -p "$bin" - ln -sf "$STUB_BIN/curl" "$bin/curl" - fi - run env PATH="$bin:$PATH" bash -c ' +# A PATH holding the allowlist, the curl stub, and cygpath only when asked. +# Fails the test if the host is missing a tool: a silently short sandbox would +# make the helper fail for a reason that has nothing to do with the fix. +sandbox_path() { + local want_cygpath="$1" dir tool src + dir="$(mktemp -d "$BATS_TEST_TMPDIR/sandbox.XXXXXX")" + for tool in "${SANDBOX_TOOLS[@]}"; do + src="$(command -v "$tool")" || { echo "host lacks $tool" >&2; return 1; } + ln -s "$src" "$dir/$tool" + done + ln -s "$STUB_SRC/curl" "$dir/curl" + [ "$want_cygpath" = "yes" ] && ln -s "$STUB_SRC/cygpath" "$dir/cygpath" + printf '%s' "$dir" +} + +# Runs the real helper under a sandbox PATH, and prints the http code. The +# consumer the curl stub plays is named by the caller, so a test cannot +# accidentally get a curl that accepts whatever the config happens to say. +post_under() { + local bin="$1" consumer="$2" body_file="$3" extra="${4:-}" + run env PATH="$bin" STUB_CURL_CONSUMER="$consumer" CFG_CAPTURE="$CFG_CAPTURE" \ + FAKE_ROOT="$FAKE_ROOT" bash -c ' set -uo pipefail . '"$SCRIPTS"'/remote.sh 2>/dev/null + '"$extra"' _remote_http_post_json "https://example.invalid/v1/x" "'"$body_file"'" \ "'"$BATS_TEST_TMPDIR"'/out-body" "'"$BATS_TEST_TMPDIR"'/out-header" ' } +@test "the sandbox decides whether cygpath exists — both directions (#850)" { + # The control on the instrument. Every arm below rests on the capability + # being what this file says it is, and on the host that is true by accident: + # macOS has no cygpath at all. If the sandbox were not really removing it, + # the negative arm would pass on any machine and fail on a Windows runner -- + # which is the one machine this fix is for. + local without with + without="$(sandbox_path no)" + with="$(sandbox_path yes)" + + run env PATH="$without" bash -c 'command -v cygpath >/dev/null 2>&1 && echo PRESENT || echo ABSENT' + [ "$output" = "ABSENT" ] + + run env PATH="$with" bash -c 'command -v cygpath >/dev/null 2>&1 && echo PRESENT || echo ABSENT' + [ "$output" = "PRESENT" ] +} + @test "without cygpath the embedded paths are passed through byte for byte (#850)" { - # macOS and Linux have no cygpath, and this is the assertion that says the fix - # costs them nothing: the config must hold the exact strings the caller built. + # macOS and Linux, and the assertion that says the fix costs them nothing: + # the config must hold the exact strings the caller built. + local bin; bin="$(sandbox_path no)" body="$BATS_TEST_TMPDIR/body.json" printf '{"t":"secret"}' > "$body" - post_with no "$body" + post_under "$bin" posix "$body" [ "$status" -eq 0 ] [ "$output" = "200" ] - # The data path is one the test chose, so it can be compared exactly. + # The data path is one this test chose, so it can be compared exactly. grep -q -F -- "data = \"@$body\"" "$CFG_CAPTURE" # The header path is generated inside the helper, so what is asserted is its @@ -157,15 +201,19 @@ post_with() { @test "with cygpath the DATA path is rendered in mixed Windows form (#850)" { # Two fields, two tests, because they are two effects of one line and can # regress apart. Asserting both in one test says "something is untranslated" - # -- which is true of either, and points at neither. Reverting the header - # field alone must not be able to fail this one. + # -- true of either, pointing at neither. + local bin; bin="$(sandbox_path yes)" body="$BATS_TEST_TMPDIR/body.json" printf '{"t":"secret"}' > "$body" - post_with yes "$body" + post_under "$bin" native-windows "$body" [ "$status" -eq 0 ] - [ "$output" = "200" ] + # Deliberately NOT asserting the 200 here. The stub refuses to open either + # field's path when it is untranslated, so a request fails whichever half is + # wrong -- and asserting the outcome in both field tests made them both go + # red for either mutation, which is the attribution this split exists to get. + # The outcome has its own test below. grep -q -F -- "data = \"@$FAKE_ROOT$body\"" "$CFG_CAPTURE" } @@ -174,51 +222,74 @@ post_with() { # thinks of as "the file", and the header sink is generated inside the helper. # Translate one and not the other and curl opens the body, fails on the # header, and the caller reports 000 -- which reads as a header problem. + local bin; bin="$(sandbox_path yes)" body="$BATS_TEST_TMPDIR/body.json" printf '{"t":"secret"}' > "$body" - post_with yes "$body" + post_under "$bin" native-windows "$body" [ "$status" -eq 0 ] - [ "$output" = "200" ] + # Config bytes only, for the reason given in the DATA case above. hdr="$(sed -n 's/^dump-header = "\(.*\)"$/\1/p' "$CFG_CAPTURE")" case "$hdr" in "$FAKE_ROOT"/*) : ;; *) echo "header path not translated: $hdr"; return 1 ;; esac } +@test "with both fields rendered, the request actually completes (#850)" { + # The outcome the two field tests deliberately leave alone. A native curl has + # to be able to open BOTH paths for the call to return a code at all, so this + # is the one assertion that fails for either field -- and that is its job: + # whatever regresses, the user's request stops working. + local bin; bin="$(sandbox_path yes)" + body="$BATS_TEST_TMPDIR/body.json" + printf '{"t":"secret"}' > "$body" + + post_under "$bin" native-windows "$body" + [ "$status" -eq 0 ] + [ "$output" = "200" ] +} + @test "the rendered paths carry forward slashes, never backslashes (#850)" { # `cygpath -w` also produces a valid Windows path, and it is the wrong one: # curl's config parser reads a backslash as an escape, so the path arrives - # corrupted. This is the assertion that separates -m from -w, and it is - # written against the config text rather than against the flag, because what - # breaks a user is the bytes curl parses. + # corrupted. Written against the config text rather than against the flag, + # because what breaks a user is the bytes curl parses. + local bin; bin="$(sandbox_path yes)" body="$BATS_TEST_TMPDIR/body.json" printf '{"t":"secret"}' > "$body" - post_with yes "$body" + post_under "$bin" native-windows "$body" [ "$status" -eq 0 ] - # No backslash anywhere in either embedded path. - ! grep -q '\\' "$CFG_CAPTURE" + refute grep -q '\\' "$CFG_CAPTURE" } -@test "a path curl cannot open surfaces as HTTP 000, the way the user saw it (#850)" { - # The negative control for the stub itself. If the stub accepted any string as - # a path, every assertion above would pass on a broken rendering -- so make - # the rendering broken on purpose and confirm the stub notices. +@test "an untranslated POSIX path reaching native curl is the reported 000 (#850)" { + # The defect itself, and the control on the stub. `_remote_curl_path` is + # replaced AFTER sourcing, so the production helper is still the one under + # test -- only its renderer is reduced to the passthrough it was before the + # fix. The consumer is the native Windows one, which cannot open /tmp/... # - # `_remote_curl_path` is replaced AFTER sourcing, so the production helper is - # still the one under test; only its path renderer is made to produce - # something no curl could open. + # Without this case the assertions above could all be passing on a config no + # curl would accept, which is exactly how the previous version of this file + # was wrong. + local bin; bin="$(sandbox_path yes)" body="$BATS_TEST_TMPDIR/body.json" printf '{"t":"secret"}' > "$body" - run env PATH="$STUB_BIN:$PATH" bash -c ' - set -uo pipefail - . '"$SCRIPTS"'/remote.sh 2>/dev/null - _remote_curl_path() { printf "Z:\\\\nowhere\\\\%s" "$1"; } - _remote_http_post_json "https://example.invalid/v1/x" "'"$body"'" \ - "'"$BATS_TEST_TMPDIR"'/out-body" "'"$BATS_TEST_TMPDIR"'/out-header" - ' + post_under "$bin" native-windows "$body" '_remote_curl_path() { printf "%s" "$1"; }' + [ "$status" -eq 0 ] + [ "$output" = "000" ] +} + +@test "a Windows path reaching a POSIX curl is equally a 000 (#850)" { + # The mirror, so "native-windows refuses POSIX paths" is not read as "this + # stub refuses whatever the test needs it to". Each consumer refuses the + # other's form, and the fix is what puts the right form in front of each. + local bin; bin="$(sandbox_path no)" + body="$BATS_TEST_TMPDIR/body.json" + printf '{"t":"secret"}' > "$body" + + post_under "$bin" posix "$body" '_remote_curl_path() { printf "%s%s" "$FAKE_ROOT" "$1"; }' [ "$status" -eq 0 ] [ "$output" = "000" ] } From ab5e9b2289a7dc20fd1e8974a1c03bda9593ea26 Mon Sep 17 00:00:00 2001 From: fujibee Date: Mon, 17 Aug 2026 18:04:29 -0700 Subject: [PATCH 4/4] test(remote): read the http code off stdout alone, not off a merged stream (#850) CI caught this; my machine did not. Two cases went red on macOS CI at the head I had reported as green: not ok 351 an untranslated POSIX path reaching native curl is the reported 000 not ok 352 a Windows path reaching a POSIX curl is equally a 000 Both assert the helper returns "000". Both got it. What they compared it against was bats's $output, which MERGES stdout and stderr, and under load the runner produced: remote.sh: line 318: .../agmsg-header-pipe.JVbrl7/header: Interrupted system call 000 An exact comparison against a two-line string fails. The code was right; the instrument was reading a stream with something else on it. Not a platform difference -- same OS, same code, different timing. The fifo open in the copier was interrupted on a busy runner and bash reported it. My local runs never hit it, so the assertion looked exact when it was only usually-exact. The helper's stderr now goes to a file and $output holds the http code alone, which is the same separation the stderr tests (#853) make deliberately. There the separation IS the subject; here it was an accident I had not noticed depending on. 8 ok / 0 not ok locally, and the matrix is unchanged: M0 no mutation 0 red M1 renderer back to passthrough 3 -- DATA, DUMP-HEADER, completes M2 cygpath -m becomes -w 4 -- those three and the slash assertion M3 header field untranslated 2 -- DUMP-HEADER and completes M4 data field untranslated 2 -- DATA and completes Production untouched. Windows unverified by me. --- tests/test_remote_curl_config_paths.bats | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/tests/test_remote_curl_config_paths.bats b/tests/test_remote_curl_config_paths.bats index 782714c84..ef5b208f1 100644 --- a/tests/test_remote_curl_config_paths.bats +++ b/tests/test_remote_curl_config_paths.bats @@ -147,15 +147,26 @@ sandbox_path() { # Runs the real helper under a sandbox PATH, and prints the http code. The # consumer the curl stub plays is named by the caller, so a test cannot # accidentally get a curl that accepts whatever the config happens to say. +# STDERR GOES TO A FILE, NOT INTO $output. bats merges the two streams, and the +# helper's stderr is not always empty: under load on CI, bash reported +# "Interrupted system call" opening the header fifo, which turned an $output of +# "000" into a diagnostic line followed by "000". Two cases failed on macOS CI +# and passed here, on the same code -- the difference was timing, not platform. +# +# The http code is the only thing on stdout, so separating the streams is what +# makes an exact comparison meaningful. It also matches the harness in the +# stderr tests (#853), where the separation is the subject rather than an +# accident. post_under() { local bin="$1" consumer="$2" body_file="$3" extra="${4:-}" + ERR_FILE="$BATS_TEST_TMPDIR/helper-stderr" run env PATH="$bin" STUB_CURL_CONSUMER="$consumer" CFG_CAPTURE="$CFG_CAPTURE" \ FAKE_ROOT="$FAKE_ROOT" bash -c ' set -uo pipefail . '"$SCRIPTS"'/remote.sh 2>/dev/null '"$extra"' _remote_http_post_json "https://example.invalid/v1/x" "'"$body_file"'" \ - "'"$BATS_TEST_TMPDIR"'/out-body" "'"$BATS_TEST_TMPDIR"'/out-header" + "'"$BATS_TEST_TMPDIR"'/out-body" "'"$BATS_TEST_TMPDIR"'/out-header" 2>"'"$ERR_FILE"'" ' }