diff --git a/scripts/gateway-pre-start.sh b/scripts/gateway-pre-start.sh index 6cf12e8c..776fbeb2 100755 --- a/scripts/gateway-pre-start.sh +++ b/scripts/gateway-pre-start.sh @@ -74,6 +74,10 @@ fi # already matches the target state. The CLI calls below (gateway # restart + MCP server) are guarded by their own idempotency checks. export CLAWBOX_HOSTNAME="$CONFIGURED_HOSTNAME" +# Let the inline Python import gateway_origins.py (origin validation for the +# operator-configurable extra trusted origins — issue #232). +CLAWBOX_SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +export CLAWBOX_SCRIPT_DIR # Serialize the LAN_IPS bash array into an env var Python can parse — # newline-separated is bash-safe (IPv4s contain no newlines). if [ ${#LAN_IPS[@]} -gt 0 ]; then @@ -86,6 +90,15 @@ export CLAWBOX_LAN_IPS python3 - "$OPENCLAW_CONFIG" <<'PY' import json, os, sys, tempfile, secrets +# Origin validation for operator-configured extra trusted origins (issue #232). +# Imported from scripts/gateway_origins.py so the logic stays unit-testable; +# fall back to defaults-only if the module can't be found (never block boot). +sys.path.insert(0, os.environ.get("CLAWBOX_SCRIPT_DIR", "")) +try: + from gateway_origins import merge_extra_origins +except Exception: + merge_extra_origins = None + # Gateway auth token gates LAN access to the agent's privileged tools # (run_command / file_write / system_power). Earlier builds wrote the public # literal "clawbox" — documented in the open-source history — so any device @@ -268,11 +281,25 @@ def set_if(obj, key, value): set_if(control_ui, "allowInsecureAuth", True) set_if(control_ui, "dangerouslyDisableDeviceAuth", True) +# Union in operator-configured extra trusted origins (VPN / MagicDNS names, +# a stable HTTPS origin) from controlUi.extraAllowedOrigins so deployments +# don't hand-patch this generated list. Each entry is validated as a full +# origin; invalid ones are reported here and dropped, never written to the +# active allowlist. Missing/None config = defaults only (unchanged behavior). +if merge_extra_origins is not None: + effective_origins, invalid_extra = merge_extra_origins( + allowed_origins, control_ui.get("extraAllowedOrigins") + ) + for bad in invalid_extra: + print(" WARN: ignoring invalid controlUi.extraAllowedOrigins entry: " + repr(bad)) +else: + effective_origins = allowed_origins + # Compare allowedOrigins as sets since ordering shouldn't force a # rewrite — the gateway doesn't care about the order, and the LAN IP # enumeration can reorder entries between boots. -if set(control_ui.get("allowedOrigins", []) or []) != set(allowed_origins): - control_ui["allowedOrigins"] = allowed_origins +if set(control_ui.get("allowedOrigins", []) or []) != set(effective_origins): + control_ui["allowedOrigins"] = effective_origins changed = True # Normalize bind to "lan" if missing or set to something the gateway diff --git a/scripts/gateway_origins.py b/scripts/gateway_origins.py new file mode 100644 index 00000000..d28d87c7 --- /dev/null +++ b/scripts/gateway_origins.py @@ -0,0 +1,100 @@ +"""Trusted-origin helpers for the setup/control UI allowlist. + +Kept as an importable module (rather than inline in gateway-pre-start.sh's +heredoc) so the validation is unit-testable. Operators can add extra trusted +origins — VPN / MagicDNS hostnames, a stable HTTPS origin — via the +`controlUi.extraAllowedOrigins` key in openclaw.json instead of patching the +generated allowlist by hand (which drifts across updates). See issue #232. + +Design: an explicit allowlist only. Entries must be full http/https origins +(scheme://host[:port]); bare hostnames, wildcards, paths, and credentials are +rejected so a configured value can never quietly widen cross-origin access. +""" +from urllib.parse import urlsplit + + +def normalize_origin(value): + """Return the normalized origin (``scheme://host[:port]``, lowercased + scheme + host, no trailing slash) for a well-formed http/https origin, or + ``None`` if ``value`` is not a valid full origin.""" + if not isinstance(value, str): + return None + v = value.strip() + if not v or "*" in v: + return None + try: + parts = urlsplit(v) + port = parts.port # the property raises ValueError on a bad port + except ValueError: + return None + if parts.scheme not in ("http", "https"): + return None + host = parts.hostname + if not host: + return None + # No path/query/fragment and no embedded credentials — an origin is just + # scheme + host + optional port. + if parts.path not in ("", "/") or parts.query or parts.fragment: + return None + if parts.username or parts.password: + return None + origin = parts.scheme + "://" + host.lower() + if port is not None: + origin += ":" + str(port) + return origin + + +def merge_extra_origins(defaults, configured): + """Union the validated ``configured`` origins into ``defaults``, preserving + order and dropping duplicates. Returns ``(merged, invalid)`` where + ``invalid`` lists the rejected entries so the caller can report them. + ``configured`` of ``None`` leaves the defaults untouched (default behavior); + a non-list ``configured`` is reported as invalid and ignored.""" + merged = list(defaults) + seen = set(merged) + invalid = [] + if configured is None: + return merged, invalid + if not isinstance(configured, list): + return merged, [configured] + for entry in configured: + norm = normalize_origin(entry) + if norm is None: + invalid.append(entry) + elif norm not in seen: + merged.append(norm) + seen.add(norm) + return merged, invalid + + +def _selftest(): + # Valid origins (scheme/host lowercased, trailing slash + default handling). + assert normalize_origin("https://box.example.com") == "https://box.example.com" + assert normalize_origin("http://100.64.0.1:8443") == "http://100.64.0.1:8443" + assert normalize_origin("HTTP://Box.Local") == "http://box.local" + assert normalize_origin("https://host/") == "https://host" + # Invalid: bare host, wrong scheme, wildcard, path/query, credentials, junk. + for bad in ["box.example.com", "ftp://host", "https://*.example.com", + "https://host/path", "https://host?q=1", "https://host#f", + "https://user:pw@host", "https://", "", " ", + "http://host:notaport", 42, None, ["x"]]: + assert normalize_origin(bad) is None, bad + # Default behavior: no config leaves defaults untouched. + d = ["http://localhost", "http://clawbox.local"] + assert merge_extra_origins(d, None) == (d, []) + # Valid extra union'd in; an existing/duplicate entry is not re-added. + merged, invalid = merge_extra_origins(d, ["https://vpn.example.ts.net", "http://localhost"]) + assert merged == d + ["https://vpn.example.ts.net"], merged + assert invalid == [] + # Invalid entry reported, never added to the active list. + merged, invalid = merge_extra_origins(d, ["not-an-origin", "https://ok.example.com"]) + assert merged == d + ["https://ok.example.com"], merged + assert invalid == ["not-an-origin"], invalid + # A non-list config value is reported and ignored. + merged, invalid = merge_extra_origins(d, "https://single") + assert merged == d and invalid == ["https://single"] + print("gateway_origins self-test: OK") + + +if __name__ == "__main__": + _selftest() diff --git a/src/tests/unit/gateway-origins.test.ts b/src/tests/unit/gateway-origins.test.ts new file mode 100644 index 00000000..81cd7a35 --- /dev/null +++ b/src/tests/unit/gateway-origins.test.ts @@ -0,0 +1,52 @@ +import { describe, it, expect } from "vitest"; +import { execFileSync } from "node:child_process"; + +// The setup/control-UI origin validation runs in Python inside +// gateway-pre-start.sh (issue #232). It lives in an importable module, +// scripts/gateway_origins.py, so it can be exercised here without booting the +// gateway. The `test` CI job runs on ubuntu-latest, which ships python3. +function pythonBin(): string { + for (const bin of ["python3", "python"]) { + try { + execFileSync(bin, ["--version"], { stdio: "ignore" }); + return bin; + } catch { + /* try the next candidate */ + } + } + throw new Error("python3 not found on the test runner"); +} + +describe("gateway_origins.py (configurable extra trusted origins)", () => { + const py = pythonBin(); + + it("passes the module self-test (valid, invalid, and default cases)", () => { + const out = execFileSync(py, ["scripts/gateway_origins.py"], { encoding: "utf8" }); + expect(out).toContain("self-test: OK"); + }); + + it("accepts a full https origin and rejects a bare hostname", () => { + const script = [ + "import sys; sys.path.insert(0, 'scripts')", + "from gateway_origins import normalize_origin as n", + "print(n('https://vpn.example.ts.net'))", + "print(n('vpn.example.ts.net'))", + ].join("; "); + const [valid, invalid] = execFileSync(py, ["-c", script], { encoding: "utf8" }).trim().split(/\r?\n/); + expect(valid).toBe("https://vpn.example.ts.net"); + expect(invalid).toBe("None"); + }); + + it("merges a configured origin into the defaults (default behavior unchanged when unset)", () => { + const script = [ + "import sys; sys.path.insert(0, 'scripts')", + "from gateway_origins import merge_extra_origins as m", + "d = ['http://localhost']", + "print(m(d, None)[0] == d)", + "print(m(d, ['https://ok.example.com'])[0] == d + ['https://ok.example.com'])", + ].join("; "); + const [unchanged, merged] = execFileSync(py, ["-c", script], { encoding: "utf8" }).trim().split(/\r?\n/); + expect(unchanged).toBe("True"); + expect(merged).toBe("True"); + }); +});