fix(sandbox): strip forward-proxy fields when rewriting to https.request

nemoclaw-blueprint/scripts/http-proxy-fix.js

@@ -54,6 +54,66 @@
   }
   if (!proxyHost) return;
 
+  // Strip headers that were meaningful for the proxy hop only. Once we
+  // re-issue against the target via https.request, the original Host
+  // points at the proxy and the hop-by-hop headers (RFC 7230 §6.1) leak
+  // upstream — they describe the connection between the caller and the
+  // proxy, not the rewritten connection to the target.
+  //
+  // RFC 7230 §6.1 hop-by-hop set (request direction):
+  //   Connection, Keep-Alive, Proxy-Authorization, TE, Trailer,
+  //   Transfer-Encoding, Upgrade.
+  // Also stripped: Host (points at the proxy); Proxy-Connection (de
+  // facto deprecated header still emitted by some clients); and
+  // Proxy-Authenticate (response-only per RFC 7235 §4.3, included
+  // belt-and-suspenders for clients that echo response headers into
+  // retry-request options). Plus: per RFC 7230 §6.1, any token named in
+  // the Connection header is itself hop-by-hop and must be stripped.
+  var STATIC_HOP_BY_HOP = [
+    'host',
+    'connection',
+    'keep-alive',
+    'proxy-authenticate',
+    'proxy-authorization',
+    'proxy-connection',
+    'te',
+    'trailer',
+    'transfer-encoding',
+    'upgrade',
+  ];
+
+  function sanitizeHeaders(headers) {
+    if (!headers || typeof headers !== 'object') return undefined;
+    // Collect tokens named in the Connection header — those become
+    // hop-by-hop transitively per RFC 7230 §6.1.
+    var dynamic = new Set();
+    for (var k in headers) {
+      if (
+        !Object.prototype.hasOwnProperty.call(headers, k) ||
+        String(k).toLowerCase() !== 'connection'
+      ) {
+        continue;
+      }
+      var raw = headers[k];
+      var listed = Array.isArray(raw) ? raw.join(',') : raw;
+      if (typeof listed === 'string') {
+        listed.split(',').forEach(function (token) {
+          var t = token.trim().toLowerCase();
+          if (t) dynamic.add(t);
+        });
+      }
+    }
+    var staticSet = new Set(STATIC_HOP_BY_HOP);
+    var out = {};
+    for (var key in headers) {
+      if (!Object.prototype.hasOwnProperty.call(headers, key)) continue;
+      var lower = String(key).toLowerCase();
+      if (staticSet.has(lower) || dynamic.has(lower)) continue;
+      out[key] = headers[key];
+    }
+    return out;
+  }
+
   http.request = function (options, callback) {
     if (typeof options === 'string' || !options) {
       return origRequest.apply(http, arguments);
@@ -70,18 +130,52 @@
         return origRequest.apply(http, arguments);
       }
       var https = require('https');
-      // Clone caller's options and overwrite only the proxy-specific
-      // routing fields. Preserves signal (AbortController), lookup,
-      // TLS fields (ca/cert/key/rejectUnauthorized), auth, timeout,
-      // and any other per-request setting the caller supplied.
+      // Clone caller's options and overwrite proxy-specific routing
+      // fields. Strip fields that were set up for the proxy hop and
+      // would misbehave on the rewritten https.request to the target:
+      //   - agent: a forward-proxy http.Agent cannot speak TLS. Leaving
+      //     it attached caused upstreams like deepinfra to surface as
+      //     "LLM request failed: network connection error" while other
+      //     upstreams that don't end up on this code path still worked.
+      //     On Node 22 https.request throws a synchronous TypeError; on
+      //     Node 18/20 it falls through and the TLS handshake fails.
+      //   - auth: basic-auth meant for the proxy hop. Leaving it on
+      //     would Basic-auth the target server with proxy credentials.
+      //   - servername / checkServerIdentity: TLS SNI + cert validation
+      //     pre-computed for the proxy hop. Wrong cert chain and wrong
+      //     SNI must not survive into the rewrite — drop them so Node
+      //     re-derives from the new `hostname`.
+      //   - socketPath: Unix-socket proxies exist (e.g. cntlm-style
+      //     local proxies). Routing TLS bytes into the proxy's Unix
+      //     socket would defeat the entire rewrite.
+      //   - localAddress / lookup / family / hints: source-binding and
+      //     DNS hints picked for reachability to the proxy. The
+      //     rewritten target may not be reachable from the same NIC or
+      //     DNS family.
+      //   - Host / hop-by-hop headers (RFC 7230 §6.1): stripped via
+      //     sanitizeHeaders so Node regenerates Host from `host`/`port`
+      //     to point at the real target.
+      // Signal (AbortController) and TLS material (ca/cert/key/
+      // rejectUnauthorized), timeout, body, and target-intent headers
+      // (Authorization, Content-Type, …) are preserved.
       var rewritten = Object.assign({}, options, {
         method: options.method || 'GET',
         hostname: target.hostname,
         host: target.hostname,
         port: target.port || 443,
         path: target.pathname + target.search,
         protocol: 'https:',
+        headers: sanitizeHeaders(options.headers),
       });
+      delete rewritten.agent;
+      delete rewritten.auth;
+      delete rewritten.servername;
+      delete rewritten.checkServerIdentity;
+      delete rewritten.socketPath;
+      delete rewritten.localAddress;
+      delete rewritten.lookup;
+      delete rewritten.family;
+      delete rewritten.hints;
       return https.request(rewritten, callback);
     }
     return origRequest.apply(http, arguments);

scripts/nemoclaw-start.sh

@@ -1091,6 +1091,66 @@ if [ "${NODE_USE_ENV_PROXY:-}" = "1" ]; then
   }
   if (!proxyHost) return;
 
+  // Strip headers that were meaningful for the proxy hop only. Once we
+  // re-issue against the target via https.request, the original Host
+  // points at the proxy and the hop-by-hop headers (RFC 7230 §6.1) leak
+  // upstream — they describe the connection between the caller and the
+  // proxy, not the rewritten connection to the target.
+  //
+  // RFC 7230 §6.1 hop-by-hop set (request direction):
+  //   Connection, Keep-Alive, Proxy-Authorization, TE, Trailer,
+  //   Transfer-Encoding, Upgrade.
+  // Also stripped: Host (points at the proxy); Proxy-Connection (de
+  // facto deprecated header still emitted by some clients); and
+  // Proxy-Authenticate (response-only per RFC 7235 §4.3, included
+  // belt-and-suspenders for clients that echo response headers into
+  // retry-request options). Plus: per RFC 7230 §6.1, any token named in
+  // the Connection header is itself hop-by-hop and must be stripped.
+  var STATIC_HOP_BY_HOP = [
+    'host',
+    'connection',
+    'keep-alive',
+    'proxy-authenticate',
+    'proxy-authorization',
+    'proxy-connection',
+    'te',
+    'trailer',
+    'transfer-encoding',
+    'upgrade',
+  ];
+
+  function sanitizeHeaders(headers) {
+    if (!headers || typeof headers !== 'object') return undefined;
+    // Collect tokens named in the Connection header — those become
+    // hop-by-hop transitively per RFC 7230 §6.1.
+    var dynamic = new Set();
+    for (var k in headers) {
+      if (
+        !Object.prototype.hasOwnProperty.call(headers, k) ||
+        String(k).toLowerCase() !== 'connection'
+      ) {
+        continue;
+      }
+      var raw = headers[k];
+      var listed = Array.isArray(raw) ? raw.join(',') : raw;
+      if (typeof listed === 'string') {
+        listed.split(',').forEach(function (token) {
+          var t = token.trim().toLowerCase();
+          if (t) dynamic.add(t);
+        });
+      }
+    }
+    var staticSet = new Set(STATIC_HOP_BY_HOP);
+    var out = {};
+    for (var key in headers) {
+      if (!Object.prototype.hasOwnProperty.call(headers, key)) continue;
+      var lower = String(key).toLowerCase();
+      if (staticSet.has(lower) || dynamic.has(lower)) continue;
+      out[key] = headers[key];
+    }
+    return out;
+  }
+
   http.request = function (options, callback) {
     if (typeof options === 'string' || !options) {
       return origRequest.apply(http, arguments);
@@ -1107,18 +1167,52 @@ if [ "${NODE_USE_ENV_PROXY:-}" = "1" ]; then
         return origRequest.apply(http, arguments);
       }
       var https = require('https');
-      // Clone caller's options and overwrite only the proxy-specific
-      // routing fields. Preserves signal (AbortController), lookup,
-      // TLS fields (ca/cert/key/rejectUnauthorized), auth, timeout,
-      // and any other per-request setting the caller supplied.
+      // Clone caller's options and overwrite proxy-specific routing
+      // fields. Strip fields that were set up for the proxy hop and
+      // would misbehave on the rewritten https.request to the target:
+      //   - agent: a forward-proxy http.Agent cannot speak TLS. Leaving
+      //     it attached caused upstreams like deepinfra to surface as
+      //     "LLM request failed: network connection error" while other
+      //     upstreams that don't end up on this code path still worked.
+      //     On Node 22 https.request throws a synchronous TypeError; on
+      //     Node 18/20 it falls through and the TLS handshake fails.
+      //   - auth: basic-auth meant for the proxy hop. Leaving it on
+      //     would Basic-auth the target server with proxy credentials.
+      //   - servername / checkServerIdentity: TLS SNI + cert validation
+      //     pre-computed for the proxy hop. Wrong cert chain and wrong
+      //     SNI must not survive into the rewrite — drop them so Node
+      //     re-derives from the new `hostname`.
+      //   - socketPath: Unix-socket proxies exist (e.g. cntlm-style
+      //     local proxies). Routing TLS bytes into the proxy's Unix
+      //     socket would defeat the entire rewrite.
+      //   - localAddress / lookup / family / hints: source-binding and
+      //     DNS hints picked for reachability to the proxy. The
+      //     rewritten target may not be reachable from the same NIC or
+      //     DNS family.
+      //   - Host / hop-by-hop headers (RFC 7230 §6.1): stripped via
+      //     sanitizeHeaders so Node regenerates Host from `host`/`port`
+      //     to point at the real target.
+      // Signal (AbortController) and TLS material (ca/cert/key/
+      // rejectUnauthorized), timeout, body, and target-intent headers
+      // (Authorization, Content-Type, …) are preserved.
       var rewritten = Object.assign({}, options, {
         method: options.method || 'GET',
         hostname: target.hostname,
         host: target.hostname,
         port: target.port || 443,
         path: target.pathname + target.search,
         protocol: 'https:',
+        headers: sanitizeHeaders(options.headers),
       });
+      delete rewritten.agent;
+      delete rewritten.auth;
+      delete rewritten.servername;
+      delete rewritten.checkServerIdentity;
+      delete rewritten.socketPath;
+      delete rewritten.localAddress;
+      delete rewritten.lookup;
+      delete rewritten.family;
+      delete rewritten.hints;
       return https.request(rewritten, callback);
     }
     return origRequest.apply(http, arguments);

test/e2e/e2e-cloud-experimental/features/skill/verify-sandbox-skill-via-agent.sh

@@ -39,9 +39,21 @@ info() { printf '%s\n' "verify-sandbox-skill-via-agent: INFO: $*"; }
 [ -n "$SANDBOX_NAME" ] || die "set SANDBOX_NAME (or NEMOCLAW_SANDBOX_NAME)"
 [ -n "${NVIDIA_API_KEY:-}" ] || die "set NVIDIA_API_KEY (needed for inference inside sandbox)"
 
-DEFAULT_PROMPT="Use the OpenClaw managed skill named '${SKILL_ID}'. Read its SKILL.md. Reply with ONLY this exact verification token string and nothing else: ${VERIFY_TOKEN}"
+# Do NOT include ${VERIFY_TOKEN} in the prompt itself. The token must come
+# from the agent reading the skill's SKILL.md — that is the entire point of
+# this test. Embedding it in the prompt makes the downstream grep match any
+# error path that echoes the prompt back (e.g. the openclaw 4.9 SSRF
+# regression in NemoClaw #2490 was masked by exactly this antipattern in
+# TC-SBX-02). Override SKILL_VERIFY_PROMPT only if you know what you're
+# doing — overrides that re-introduce the literal token defeat the test.
+DEFAULT_PROMPT="Use the OpenClaw managed skill named '${SKILL_ID}'. Read its SKILL.md and reply with ONLY the agent verification token defined in that file. No quotes, no extra words."
 PROMPT="${SKILL_VERIFY_PROMPT:-$DEFAULT_PROMPT}"
 
+# Guard against an override that accidentally smuggles the token back in.
+if printf '%s' "$PROMPT" | grep -Fq "$VERIFY_TOKEN"; then
+  die "SKILL_VERIFY_PROMPT must not contain VERIFY_TOKEN ('${VERIFY_TOKEN}'); the agent must read it from SKILL.md so a prompt-echo error path cannot satisfy the assertion"
+fi
+
 command -v openshell >/dev/null 2>&1 || die "openshell not on PATH"
 command -v base64 >/dev/null 2>&1 || die "base64 not on PATH"
 
@@ -84,6 +96,14 @@ printf '\n%s\n' "--- agent stdout/stderr (trimmed for display) ---"
 printf '%s' "$raw_out" | tail -c 12000
 printf '\n%s\n' "--- end ---"
 
+# Fail closed on provider/transport errors so a coincidental token match
+# (e.g. someone overrode SKILL_VERIFY_PROMPT to embed the token, or the
+# token leaked into a stack trace via the skill manifest path) cannot mask
+# an SSRF block, transport reset, or gateway error. See NemoClaw #2490.
+if printf '%s' "$raw_out" | grep -qiE "SsrFBlockedError|Blocked hostname|Blocked: resolves to|transport error|provider error|ECONNREFUSED|EAI_AGAIN|gateway unavailable"; then
+  die "agent failed before completing turn — provider/transport error in output (exit ${agent_rc}). Session: ${SESSION_ID}"
+fi
+
 # Collapse newlines so a model-wrapped token (e.g. "SKILL_SMOKE_VER\nIFY_K9X2") still matches.
 collapsed_out=$(printf '%s' "$raw_out" | tr -d '\n\r')
 if printf '%s' "$collapsed_out" | grep -Fq "$VERIFY_TOKEN"; then

test/e2e/test-full-e2e.sh

@@ -288,8 +288,13 @@ else
   fail "[LIVE] Direct API: empty response from curl"
 fi
 
-# ── Test 4b: Inference through the sandbox (THE definitive test) ──
-info "[LIVE] Sandbox inference test → user → sandbox → gateway → NVIDIA API..."
+# ── Test 4b: OpenShell DNS+proxy can route inference.local from the sandbox ──
+# This is a routing-layer check, not an openclaw check. The HTTP request is
+# made by `curl` from inside the sandbox; nothing in this path exercises
+# openclaw's HTTP client or its SSRF guard. See Phase 4c for the openclaw-
+# mediated assertion. (NemoClaw #2490 / openclaw 2026.4.9 SSRF regression
+# was invisible to this step because curl bypasses openclaw entirely.)
+info "[ROUTING] inference.local DNS + OpenShell proxy reachable from sandbox..."
 ssh_config="$(mktemp)"
 sandbox_response=""
 
@@ -349,10 +354,64 @@ for pong_attempt in 1 2 3; do
   rm -f "$ssh_config"
 done
 if $pong_ok; then
-  pass "[LIVE] Sandbox inference: model responded with PONG through sandbox"
-  info "Full path proven: user → sandbox → openshell gateway → NVIDIA Endpoints → response"
+  pass "[ROUTING] inference.local: OpenShell routed curl to NVIDIA Endpoints and returned PONG"
+  info "Routing path proven: sandbox curl → DNS forwarder → gateway proxy → NVIDIA Endpoints (does not exercise openclaw HTTP client; see Phase 4c)"
 else
-  fail "[LIVE] Sandbox inference: expected PONG after 3 attempts, got: ${sandbox_content:0:200}"
+  fail "[ROUTING] inference.local: expected PONG after 3 attempts, got: ${sandbox_content:0:200}"
+fi
+
+# ── Test 4c: openclaw-mediated turn against inference.local ──
+# This is the only assertion in this file that proves openclaw can complete
+# a turn against inference.local. Prior to this step, every "[LIVE] inference"
+# label in the suite was actually a [ROUTING] check via curl (see 4b above).
+#
+# Properties of this assertion that prevent the false-positive class that
+# masked the openclaw 2026.4.9 SSRF regression:
+#   * Uses `openclaw agent --json`. With --json the CLI calls
+#     routeLogsToStderr() (openclaw/src/commands/agent-via-gateway.ts:57),
+#     so stdout is a clean JSON envelope; prompt-echo on stderr cannot
+#     pollute the assertion.
+#   * Asserts on the model's reply text inside `result.payloads[].text`,
+#     not on the merged stdout/stderr.
+#   * The expected token (the integer 42) is not a literal substring of the
+#     prompt, so an error path that quoted the prompt back cannot satisfy
+#     the grep.
+info "[LIVE] openclaw agent → openclaw HTTP client → inference.local..."
+ssh_config="$(mktemp)"
+agent_response=""
+
+if openshell sandbox ssh-config "$SANDBOX_NAME" >"$ssh_config" 2>/dev/null; then
+  agent_session_id="e2e-live-$(date +%s)-$$"
+  # 2>/dev/null discards stderr (progress + log lines) so stdout is JSON-only.
+  agent_response=$($TIMEOUT_CMD ssh -F "$ssh_config" \
+    -o StrictHostKeyChecking=no \
+    -o UserKnownHostsFile=/dev/null \
+    -o ConnectTimeout=10 \
+    -o LogLevel=ERROR \
+    "openshell-${SANDBOX_NAME}" \
+    "openclaw agent --agent main --json --session-id '${agent_session_id}' -m 'What is 6 multiplied by 7? Reply with only the integer, no extra words.'" \
+    2>/dev/null) || true
+fi
+rm -f "$ssh_config"
+
+agent_reply=$(echo "$agent_response" | python3 -c "
+import json, sys
+try:
+    doc = json.load(sys.stdin)
+except Exception:
+    sys.exit(0)
+result = doc.get('result') or {}
+parts = []
+for p in result.get('payloads') or []:
+    if isinstance(p, dict) and isinstance(p.get('text'), str):
+        parts.append(p['text'])
+print('\n'.join(parts))
+" 2>/dev/null) || true
+
+if grep -qE "(^|[^0-9])42([^0-9]|$)" <<<"$agent_reply"; then
+  pass "[LIVE] openclaw agent: model answered 6×7=42 through openclaw → inference.local"
+else
+  fail "[LIVE] openclaw agent: expected '42' in agent reply, got: ${agent_reply:0:200}"
 fi
 
 # ══════════════════════════════════════════════════════════════════