From b7de9c54d333278384ca94dd61a9bbb397ea7721 Mon Sep 17 00:00:00 2001 From: Julie Yaunches Date: Tue, 31 Mar 2026 20:27:36 -0400 Subject: [PATCH 1/4] fix(security): harden gateway auth defaults and restrict auto-pair (#117) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #117 — supersedes PR #123 which targeted stale code paths. Dockerfile: secure-by-default controlUi config - dangerouslyDisableDeviceAuth now defaults to false (was hardcoded True) - allowInsecureAuth derived from CHAT_UI_URL scheme (false for https) - New build-arg NEMOCLAW_DISABLE_DEVICE_AUTH=0 as opt-in escape hatch - Env var promoted safely via ENV (no ARG interpolation into Python) nemoclaw-start.sh: auto-pair client whitelisting - Only approve devices with known clientId (openclaw-control-ui) or clientMode (webchat); reject and log unknown clients - Validate device is a dict before accessing fields - Log client identity on approval for audit trail Tests (13 new) - Dockerfile: no hardcoded True for auth settings, env var derivation, ARG default, ENV promotion before RUN layer - Start script: whitelist definitions, rejection logic, dict validation, client identity logging, no unconditional approval pattern --- Dockerfile | 12 +++- scripts/nemoclaw-start.sh | 18 +++-- test/nemoclaw-start.test.js | 35 ++++++++++ test/security-c2-dockerfile-injection.test.js | 66 +++++++++++++++++++ 4 files changed, 124 insertions(+), 7 deletions(-) diff --git a/Dockerfile b/Dockerfile index d62ee0558..7f88d2f7a 100644 --- a/Dockerfile +++ b/Dockerfile @@ -54,6 +54,9 @@ ARG CHAT_UI_URL=http://127.0.0.1:18789 ARG NEMOCLAW_INFERENCE_BASE_URL=https://inference.local/v1 ARG NEMOCLAW_INFERENCE_API=openai-completions ARG NEMOCLAW_INFERENCE_COMPAT_B64=e30= +# Set to "1" to disable device-pairing auth (development/headless only). +# Default: "0" (device auth enabled — secure by default). +ARG NEMOCLAW_DISABLE_DEVICE_AUTH=0 # Unique per build to ensure each image gets a fresh auth token. # Pass --build-arg NEMOCLAW_BUILD_ID=$(date +%s) to bust the cache. ARG NEMOCLAW_BUILD_ID=default @@ -67,7 +70,8 @@ ENV NEMOCLAW_MODEL=${NEMOCLAW_MODEL} \ CHAT_UI_URL=${CHAT_UI_URL} \ NEMOCLAW_INFERENCE_BASE_URL=${NEMOCLAW_INFERENCE_BASE_URL} \ NEMOCLAW_INFERENCE_API=${NEMOCLAW_INFERENCE_API} \ - NEMOCLAW_INFERENCE_COMPAT_B64=${NEMOCLAW_INFERENCE_COMPAT_B64} + NEMOCLAW_INFERENCE_COMPAT_B64=${NEMOCLAW_INFERENCE_COMPAT_B64} \ + NEMOCLAW_DISABLE_DEVICE_AUTH=${NEMOCLAW_DISABLE_DEVICE_AUTH} WORKDIR /sandbox USER sandbox @@ -91,6 +95,8 @@ parsed = urlparse(chat_ui_url); \ chat_origin = f'{parsed.scheme}://{parsed.netloc}' if parsed.scheme and parsed.netloc else 'http://127.0.0.1:18789'; \ origins = ['http://127.0.0.1:18789']; \ origins = list(dict.fromkeys(origins + [chat_origin])); \ +disable_device_auth = os.environ.get('NEMOCLAW_DISABLE_DEVICE_AUTH', '') == '1'; \ +allow_insecure = parsed.scheme != 'https'; \ providers = { \ provider_key: { \ 'baseUrl': inference_base_url, \ @@ -106,8 +112,8 @@ config = { \ 'gateway': { \ 'mode': 'local', \ 'controlUi': { \ - 'allowInsecureAuth': True, \ - 'dangerouslyDisableDeviceAuth': True, \ + 'allowInsecureAuth': allow_insecure, \ + 'dangerouslyDisableDeviceAuth': disable_device_auth, \ 'allowedOrigins': origins, \ }, \ 'trustedProxies': ['127.0.0.1', '::1'], \ diff --git a/scripts/nemoclaw-start.sh b/scripts/nemoclaw-start.sh index fbdebda20..ce5234ab8 100755 --- a/scripts/nemoclaw-start.sh +++ b/scripts/nemoclaw-start.sh @@ -10,8 +10,9 @@ # The config hash is verified at startup to detect tampering. # # Optional env: -# NVIDIA_API_KEY API key for NVIDIA-hosted inference -# CHAT_UI_URL Browser origin that will access the forwarded dashboard +# NVIDIA_API_KEY API key for NVIDIA-hosted inference +# CHAT_UI_URL Browser origin that will access the forwarded dashboard +# NEMOCLAW_DISABLE_DEVICE_AUTH Set to "1" to skip device-pairing auth (development only) set -euo pipefail @@ -175,14 +176,23 @@ while time.time() < DEADLINE: if pending: QUIET_POLLS = 0 + ALLOWED_CLIENTS = {'openclaw-control-ui'} + ALLOWED_MODES = {'webchat'} for device in pending: - request_id = (device or {}).get('requestId') + if not isinstance(device, dict): + continue + client_id = device.get('clientId', '') + client_mode = device.get('clientMode', '') + if client_id not in ALLOWED_CLIENTS and client_mode not in ALLOWED_MODES: + print(f'[auto-pair] rejected unknown client={client_id} mode={client_mode}') + continue + request_id = device.get('requestId') if not request_id: continue arc, aout, aerr = run(OPENCLAW, 'devices', 'approve', request_id, '--json') if arc == 0: APPROVED += 1 - print(f'[auto-pair] approved request={request_id}') + print(f'[auto-pair] approved request={request_id} client={client_id}') elif aout or aerr: print(f'[auto-pair] approve failed request={request_id}: {(aerr or aout)[:400]}') time.sleep(1) diff --git a/test/nemoclaw-start.test.js b/test/nemoclaw-start.test.js index d54ac9709..5d851b73f 100644 --- a/test/nemoclaw-start.test.js +++ b/test/nemoclaw-start.test.js @@ -59,3 +59,38 @@ describe("nemoclaw-start non-root fallback", () => { } }); }); + +describe("nemoclaw-start auto-pair client whitelisting (#117)", () => { + const src = fs.readFileSync(START_SCRIPT, "utf-8"); + + it("defines ALLOWED_CLIENTS whitelist containing openclaw-control-ui", () => { + expect(src).toMatch(/ALLOWED_CLIENTS\s*=\s*\{.*'openclaw-control-ui'.*\}/); + }); + + it("defines ALLOWED_MODES whitelist containing webchat", () => { + expect(src).toMatch(/ALLOWED_MODES\s*=\s*\{.*'webchat'.*\}/); + }); + + it("rejects devices not in the whitelist", () => { + expect(src).toMatch(/client_id not in ALLOWED_CLIENTS and client_mode not in ALLOWED_MODES/); + expect(src).toMatch(/\[auto-pair\] rejected unknown client=/); + }); + + it("validates device is a dict before accessing fields", () => { + expect(src).toMatch(/if not isinstance\(device, dict\)/); + }); + + it("logs client identity on approval", () => { + expect(src).toMatch(/\[auto-pair\] approved request=\{request_id\} client=\{client_id\}/); + }); + + it("does not unconditionally approve all pending devices", () => { + // The old pattern: `(device or {}).get('requestId')` — approve everything + // Must NOT be present in the auto-pair block + expect(src).not.toMatch(/\(device or \{\}\)\.get\('requestId'\)/); + }); + + it("documents NEMOCLAW_DISABLE_DEVICE_AUTH in the script header", () => { + expect(src).toMatch(/NEMOCLAW_DISABLE_DEVICE_AUTH/); + }); +}); diff --git a/test/security-c2-dockerfile-injection.test.js b/test/security-c2-dockerfile-injection.test.js index 00a9c51c7..fba233f13 100644 --- a/test/security-c2-dockerfile-injection.test.js +++ b/test/security-c2-dockerfile-injection.test.js @@ -299,3 +299,69 @@ describe("C-2 regression: Dockerfile must not interpolate build-args into Python expect(hasEnvRead).toBeTruthy(); }); }); + +// ═══════════════════════════════════════════════════════════════════ +// 4. Gateway auth hardening — no hardcoded insecure defaults (#117) +// ═══════════════════════════════════════════════════════════════════ +describe("Gateway auth hardening: Dockerfile must not hardcode insecure auth defaults", () => { + it("dangerouslyDisableDeviceAuth is not hardcoded to True", () => { + const src = fs.readFileSync(DOCKERFILE, "utf-8"); + // Must not contain a literal `'dangerouslyDisableDeviceAuth': True` + expect(src).not.toMatch(/'dangerouslyDisableDeviceAuth':\s*True/); + }); + + it("allowInsecureAuth is not hardcoded to True", () => { + const src = fs.readFileSync(DOCKERFILE, "utf-8"); + // Must not contain a literal `'allowInsecureAuth': True` + expect(src).not.toMatch(/'allowInsecureAuth':\s*True/); + }); + + it("dangerouslyDisableDeviceAuth is derived from NEMOCLAW_DISABLE_DEVICE_AUTH env var", () => { + const src = fs.readFileSync(DOCKERFILE, "utf-8"); + // The Python config generation must read the env var + expect(src).toMatch(/os\.environ\.get\(['"]NEMOCLAW_DISABLE_DEVICE_AUTH['"]/); + // And use the derived variable in the config dict + expect(src).toMatch(/'dangerouslyDisableDeviceAuth':\s*disable_device_auth/); + }); + + it("allowInsecureAuth is derived from URL scheme", () => { + const src = fs.readFileSync(DOCKERFILE, "utf-8"); + // Must derive insecure auth from the parsed URL scheme + expect(src).toMatch(/allow_insecure\s*=\s*parsed\.scheme\s*!=\s*'https'/); + // And use the derived variable in the config dict + expect(src).toMatch(/'allowInsecureAuth':\s*allow_insecure/); + }); + + it("NEMOCLAW_DISABLE_DEVICE_AUTH defaults to '0' (secure by default)", () => { + const src = fs.readFileSync(DOCKERFILE, "utf-8"); + expect(src).toMatch(/ARG\s+NEMOCLAW_DISABLE_DEVICE_AUTH=0/); + }); + + it("NEMOCLAW_DISABLE_DEVICE_AUTH is promoted to ENV before the Python RUN layer", () => { + const src = fs.readFileSync(DOCKERFILE, "utf-8"); + const lines = src.split("\n"); + let promoted = false; + let inEnvBlock = false; + for (let i = 0; i < lines.length; i++) { + const line = lines[i]; + if (/^\s*FROM\b/.test(line)) { + promoted = false; + inEnvBlock = false; + } + if (/^\s*ENV\b/.test(line)) { + inEnvBlock = true; + } + if (inEnvBlock && /NEMOCLAW_DISABLE_DEVICE_AUTH[=\s]/.test(line)) { + promoted = true; + } + if (inEnvBlock && !/\\\s*$/.test(line)) { + inEnvBlock = false; + } + if (/^\s*RUN\b.*python3\s+-c\b/.test(line)) { + expect(promoted).toBeTruthy(); + return; + } + } + expect(promoted).toBeTruthy(); + }); +}); From 09ebafcfac71fc3ecf7f2af75dea609216974b40 Mon Sep 17 00:00:00 2001 From: Julie Yaunches Date: Tue, 31 Mar 2026 21:39:07 -0400 Subject: [PATCH 2/4] fix(security): address review feedback from PR #123 - allowInsecureAuth: use explicit http allowlist (parsed.scheme == 'http') instead of https denylist (parsed.scheme != 'https') so malformed or unknown schemes default to secure (CodeRabbit finding on #123) - Add HANDLED set to track rejected/approved requestIds so rejected devices are not reprocessed every poll cycle (CodeRabbit finding on #123) - Document that clientId/clientMode are client-supplied and spoofable; the allowlist is defense-in-depth, not a trust boundary (@cv review on #123) - Reference PR #690 for the more comprehensive auto-pair fix (one-shot exit, timeout reduction, token cleanup) --- Dockerfile | 2 +- scripts/nemoclaw-start.sh | 13 ++++++++++--- test/nemoclaw-start.test.js | 6 ++++++ test/security-c2-dockerfile-injection.test.js | 8 +++++--- 4 files changed, 22 insertions(+), 7 deletions(-) diff --git a/Dockerfile b/Dockerfile index 7f88d2f7a..2c8e59491 100644 --- a/Dockerfile +++ b/Dockerfile @@ -96,7 +96,7 @@ chat_origin = f'{parsed.scheme}://{parsed.netloc}' if parsed.scheme and parsed.n origins = ['http://127.0.0.1:18789']; \ origins = list(dict.fromkeys(origins + [chat_origin])); \ disable_device_auth = os.environ.get('NEMOCLAW_DISABLE_DEVICE_AUTH', '') == '1'; \ -allow_insecure = parsed.scheme != 'https'; \ +allow_insecure = parsed.scheme == 'http'; \ providers = { \ provider_key: { \ 'baseUrl': inference_base_url, \ diff --git a/scripts/nemoclaw-start.sh b/scripts/nemoclaw-start.sh index ce5234ab8..a685f3403 100755 --- a/scripts/nemoclaw-start.sh +++ b/scripts/nemoclaw-start.sh @@ -154,6 +154,7 @@ OPENCLAW = os.environ.get('OPENCLAW_BIN', 'openclaw') DEADLINE = time.time() + 600 QUIET_POLLS = 0 APPROVED = 0 +HANDLED = set() # Track rejected/approved requestIds to avoid reprocessing def run(*args): proc = subprocess.run(args, capture_output=True, text=True) @@ -176,20 +177,26 @@ while time.time() < DEADLINE: if pending: QUIET_POLLS = 0 + # SECURITY NOTE: clientId/clientMode are client-supplied and spoofable + # (the gateway stores connectParams.client.id verbatim). This allowlist + # is defense-in-depth, not a trust boundary. PR #690 adds one-shot exit, + # timeout reduction, and token cleanup for a more comprehensive fix. ALLOWED_CLIENTS = {'openclaw-control-ui'} ALLOWED_MODES = {'webchat'} for device in pending: if not isinstance(device, dict): continue + request_id = device.get('requestId') + if not request_id or request_id in HANDLED: + continue client_id = device.get('clientId', '') client_mode = device.get('clientMode', '') if client_id not in ALLOWED_CLIENTS and client_mode not in ALLOWED_MODES: + HANDLED.add(request_id) print(f'[auto-pair] rejected unknown client={client_id} mode={client_mode}') continue - request_id = device.get('requestId') - if not request_id: - continue arc, aout, aerr = run(OPENCLAW, 'devices', 'approve', request_id, '--json') + HANDLED.add(request_id) if arc == 0: APPROVED += 1 print(f'[auto-pair] approved request={request_id} client={client_id}') diff --git a/test/nemoclaw-start.test.js b/test/nemoclaw-start.test.js index 5d851b73f..758e153aa 100644 --- a/test/nemoclaw-start.test.js +++ b/test/nemoclaw-start.test.js @@ -90,6 +90,12 @@ describe("nemoclaw-start auto-pair client whitelisting (#117)", () => { expect(src).not.toMatch(/\(device or \{\}\)\.get\('requestId'\)/); }); + it("tracks handled requests to avoid reprocessing rejected devices", () => { + expect(src).toMatch(/HANDLED\s*=\s*set\(\)/); + expect(src).toMatch(/request_id in HANDLED/); + expect(src).toMatch(/HANDLED\.add\(request_id\)/); + }); + it("documents NEMOCLAW_DISABLE_DEVICE_AUTH in the script header", () => { expect(src).toMatch(/NEMOCLAW_DISABLE_DEVICE_AUTH/); }); diff --git a/test/security-c2-dockerfile-injection.test.js b/test/security-c2-dockerfile-injection.test.js index fba233f13..5689ce8d5 100644 --- a/test/security-c2-dockerfile-injection.test.js +++ b/test/security-c2-dockerfile-injection.test.js @@ -324,10 +324,12 @@ describe("Gateway auth hardening: Dockerfile must not hardcode insecure auth def expect(src).toMatch(/'dangerouslyDisableDeviceAuth':\s*disable_device_auth/); }); - it("allowInsecureAuth is derived from URL scheme", () => { + it("allowInsecureAuth is derived from URL scheme (explicit http allowlist)", () => { const src = fs.readFileSync(DOCKERFILE, "utf-8"); - // Must derive insecure auth from the parsed URL scheme - expect(src).toMatch(/allow_insecure\s*=\s*parsed\.scheme\s*!=\s*'https'/); + // Must use explicit 'http' allowlist — not `!= 'https'` which would allow + // insecure auth for malformed or unknown schemes (CodeRabbit review on #123) + expect(src).toMatch(/allow_insecure\s*=\s*parsed\.scheme\s*==\s*'http'/); + expect(src).not.toMatch(/allow_insecure\s*=\s*parsed\.scheme\s*!=\s*'https'/); // And use the derived variable in the config dict expect(src).toMatch(/'allowInsecureAuth':\s*allow_insecure/); }); From 20b30cd9429f264bdf45764cafc769f9d83f1d11 Mon Sep 17 00:00:00 2001 From: Julie Yaunches Date: Tue, 31 Mar 2026 22:05:38 -0400 Subject: [PATCH 3/4] fix(security): address self-review warnings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Move ALLOWED_CLIENTS/ALLOWED_MODES above the while loop — these are constants, not per-iteration state. Avoids reconstructing sets every poll cycle. - Document NEMOCLAW_DISABLE_DEVICE_AUTH as build-time only in the script header. Setting it at runtime has no effect because openclaw.json is baked at image build and verified by hash at startup. - Add tests: constants defined before loop, header mentions build-time. --- scripts/nemoclaw-start.sh | 16 +++++++++------- test/nemoclaw-start.test.js | 24 ++++++++++++++++++++++-- 2 files changed, 31 insertions(+), 9 deletions(-) diff --git a/scripts/nemoclaw-start.sh b/scripts/nemoclaw-start.sh index a685f3403..27fc64a8c 100755 --- a/scripts/nemoclaw-start.sh +++ b/scripts/nemoclaw-start.sh @@ -12,7 +12,9 @@ # Optional env: # NVIDIA_API_KEY API key for NVIDIA-hosted inference # CHAT_UI_URL Browser origin that will access the forwarded dashboard -# NEMOCLAW_DISABLE_DEVICE_AUTH Set to "1" to skip device-pairing auth (development only) +# NEMOCLAW_DISABLE_DEVICE_AUTH Build-time only. Set to "1" to skip device-pairing auth +# (development/headless). Has no runtime effect — openclaw.json +# is baked at image build and verified by hash at startup. set -euo pipefail @@ -155,6 +157,12 @@ DEADLINE = time.time() + 600 QUIET_POLLS = 0 APPROVED = 0 HANDLED = set() # Track rejected/approved requestIds to avoid reprocessing +# SECURITY NOTE: clientId/clientMode are client-supplied and spoofable +# (the gateway stores connectParams.client.id verbatim). This allowlist +# is defense-in-depth, not a trust boundary. PR #690 adds one-shot exit, +# timeout reduction, and token cleanup for a more comprehensive fix. +ALLOWED_CLIENTS = {'openclaw-control-ui'} +ALLOWED_MODES = {'webchat'} def run(*args): proc = subprocess.run(args, capture_output=True, text=True) @@ -177,12 +185,6 @@ while time.time() < DEADLINE: if pending: QUIET_POLLS = 0 - # SECURITY NOTE: clientId/clientMode are client-supplied and spoofable - # (the gateway stores connectParams.client.id verbatim). This allowlist - # is defense-in-depth, not a trust boundary. PR #690 adds one-shot exit, - # timeout reduction, and token cleanup for a more comprehensive fix. - ALLOWED_CLIENTS = {'openclaw-control-ui'} - ALLOWED_MODES = {'webchat'} for device in pending: if not isinstance(device, dict): continue diff --git a/test/nemoclaw-start.test.js b/test/nemoclaw-start.test.js index 758e153aa..6889f514a 100644 --- a/test/nemoclaw-start.test.js +++ b/test/nemoclaw-start.test.js @@ -96,7 +96,27 @@ describe("nemoclaw-start auto-pair client whitelisting (#117)", () => { expect(src).toMatch(/HANDLED\.add\(request_id\)/); }); - it("documents NEMOCLAW_DISABLE_DEVICE_AUTH in the script header", () => { - expect(src).toMatch(/NEMOCLAW_DISABLE_DEVICE_AUTH/); + it("documents NEMOCLAW_DISABLE_DEVICE_AUTH as a build-time setting in the script header", () => { + // Must mention it's build-time only — setting at runtime has no effect + // because openclaw.json is baked and immutable + const header = src.split("set -euo pipefail")[0]; + expect(header).toMatch(/NEMOCLAW_DISABLE_DEVICE_AUTH/); + expect(header).toMatch(/build[- ]time/i); + }); + + it("defines ALLOWED_CLIENTS and ALLOWED_MODES outside the poll loop", () => { + // These are constants — they should be defined once alongside HANDLED, + // not reconstructed inside the `if pending:` block every poll cycle + const autoPairBlock = src.match(/PYAUTOPAIR[\s\S]*?PYAUTOPAIR/); + expect(autoPairBlock).toBeTruthy(); + const pyCode = autoPairBlock[0]; + + // ALLOWED_CLIENTS/ALLOWED_MODES should appear BEFORE the `while` loop, + // at the same level as HANDLED, APPROVED, etc. + const allowedClientsPos = pyCode.indexOf("ALLOWED_CLIENTS"); + const whilePos = pyCode.indexOf("while time.time()"); + expect(allowedClientsPos).toBeGreaterThan(-1); + expect(whilePos).toBeGreaterThan(-1); + expect(allowedClientsPos).toBeLessThan(whilePos); }); }); From d25a13a422c51da0bf7be8de6ca56cbc85a1a0ea Mon Sep 17 00:00:00 2001 From: Julie Yaunches Date: Wed, 1 Apr 2026 15:09:01 -0400 Subject: [PATCH 4/4] fix(security): pass NEMOCLAW_DISABLE_DEVICE_AUTH=1 in onboard and Brev flows The secure-by-default flip (NEMOCLAW_DISABLE_DEVICE_AUTH=0) broke two automated paths that expect immediate dashboard access without device pairing: - bin/lib/onboard.js: patchStagedDockerfile() now sets the ARG to 1 - scripts/setup.sh: sed-patches the staged Dockerfile before build Addresses review feedback from @ericksoa on #1217. --- bin/lib/onboard.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/bin/lib/onboard.js b/bin/lib/onboard.js index cc853fc10..293537aa5 100644 --- a/bin/lib/onboard.js +++ b/bin/lib/onboard.js @@ -915,6 +915,12 @@ function patchStagedDockerfile( /^ARG NEMOCLAW_BUILD_ID=.*$/m, `ARG NEMOCLAW_BUILD_ID=${buildId}`, ); + // Onboard flow expects immediate dashboard access without device pairing, + // so disable device auth for images built during onboard (see #1217). + dockerfile = dockerfile.replace( + /^ARG NEMOCLAW_DISABLE_DEVICE_AUTH=.*$/m, + `ARG NEMOCLAW_DISABLE_DEVICE_AUTH=1`, + ); fs.writeFileSync(dockerfilePath, dockerfile); }