Policy: add gateway exposure checks#81981
Conversation
|
Codex review: needs real behavior proof before merge. Latest ClawSweeper review: 2026-05-22 21:05 UTC / May 22, 2026, 5:05 PM ET. Workflow note: Future ClawSweeper reviews update this same comment in place. How this review workflow works
Summary Reproducibility: not applicable. as a feature PR; there is no current-main bug to reproduce, and the review path is source/proof inspection of the new Policy behavior. PR rating Rank-up moves:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. Real behavior proof Risk before merge
Maintainer options:
Next step before merge Security Review detailsBest possible solution: Land this only after maintainer approval of the Gateway exposure semantics, current-head redacted CLI proof, and explicit acceptance of the Policy attestation/hash churn for Gateway policy adopters. Do we have a high-confidence way to reproduce the issue? Not applicable as a feature PR; there is no current-main bug to reproduce, and the review path is source/proof inspection of the new Policy behavior. Is this the best way to solve the issue? Yes, conditionally: keeping this as read-only config conformance in the bundled Policy plugin matches the existing Policy architecture, but merge should wait for maintainer acceptance of the semantics and current-head real CLI proof. Label justifications:
Acceptance criteria:
What I checked:
Likely related people:
Codex review notes: model gpt-5.5, reasoning high; reviewed against bb5010b89a5a. |
0e55ab0 to
dd4691b
Compare
|
Restacked this branch onto the updated
I also updated the PR body with the required Real behavior proof fields. |
|
No dependency changes detected. Learn more about Socket for GitHub. 👍 No dependency changes detected in pull request |
e91651b to
d4fa5c5
Compare
3232b3d to
d67f0c3
Compare
6729aaf to
bdd19c2
Compare
bdd19c2 to
04e8ff0
Compare
galiniliev
left a comment
There was a problem hiding this comment.
Reviewed the policy evidence paths against current gateway runtime contracts. The focused policy tests pass locally, but I found two false-positive evidence cases that should be fixed before this becomes a strict policy gate.
| nonLoopback: | ||
| bind === undefined | ||
| ? !tailscaleForcesLoopback | ||
| : bind === "custom" |
There was a problem hiding this comment.
[P2] Don't report invalid custom bind as non-loopback exposure
When gateway.bind is "custom" and gateway.customBindHost is missing or blank, this marks the bind as nonLoopback: true, so strict policy emits policy/gateway-non-loopback-bind. Startup rejects that config before listening (gateway.bind=custom requires gateway.customBindHost), so the policy evidence/attestation says exposed when the actual runtime state is invalid and not listening. Please separate invalid custom-bind config from actual non-loopback exposure, or only set nonLoopback after a concrete non-loopback host exists.
There was a problem hiding this comment.
Addressed in signed commit b52241b5675d06034e4940aadf4109b2072fffe2.
gateway.bind=custom without a nonblank customBindHost now reports no non-loopback exposure evidence. Host-specific exposure is only emitted from gateway.customBindHost when a concrete host exists, and the custom-host loopback check matches runtime's valid IPv4-only custom bind contract. Added regressions for blank custom host and for localhost / ::1 falling back to LAN exposure.
| value: "remote", | ||
| }); | ||
| } | ||
| if (remote.enabled === true) { |
There was a problem hiding this comment.
[P1] Do not treat gateway.remote.enabled as active exposure
gateway.remote.enabled does not enable a runtime path today: gateway callers gate remote behavior on config.gateway?.mode === "remote", and a repo-wide search finds no code reading this field. With this evidence, a config that merely contains gateway.remote.enabled: true fails gateway.remote.allow=false even though no remote mode or URL is active. Please drop this evidence or tie it to an actually-used runtime contract so strict policy does not reject inert config.
There was a problem hiding this comment.
Addressed in signed commit b52241b5675d06034e4940aadf4109b2072fffe2.
Dropped gateway.remote.enabled as exposure evidence. Remote policy now keys off the runtime contract: gateway.mode === "remote", with gateway.remote.url evidence only when remote mode is active. Added regressions proving inert remote.enabled / remote.url outside remote mode do not fail strict policy.
Proof run on the pushed branch:
git diff --checknode scripts/run-vitest.mjs extensions/policy/src/doctor/register.test.ts extensions/policy/src/cli.test.ts --run --reporter=dot-> 117 tests passednode node_modules/@typescript/native-preview/bin/tsgo.js -p test/tsconfig/tsconfig.extensions.test.json --incremental --tsBuildInfoFile .artifacts/tsgo-cache/extensions-test-policy-81981-tilde-fix.tsbuildinfo/mnt/c/src/claws-hapi/.agents/skills/autoreview/scripts/autoreview --mode branch --reviewer codex --fallback-reviewer none-> no accepted/actionable findings
04e8ff0 to
b52241b
Compare
galiniliev
left a comment
There was a problem hiding this comment.
I found one remaining runtime-contract mismatch in the gateway exposure evidence. The earlier blank-custom-bind and inactive-remote cases look fixed on this revision.
| kind: "bind", | ||
| source: "oc://openclaw.config/gateway/customBindHost", | ||
| value: customBindHost, | ||
| nonLoopback: !isRuntimeLoopbackCustomBindHost(customBindHost), |
There was a problem hiding this comment.
[P2] Don’t classify invalid custom hosts as active exposure
This now skips a blank customBindHost, but non-IPv4 values such as localhost or ::1 still produce nonLoopback: true evidence and the new tests expect that as a LAN fallback. Gateway startup no longer allows that fallback: resolveGatewayRuntimeConfig rejects gateway.bind=custom unless customBindHost is a valid IPv4 and the resolved bind host matches it (src/gateway/server-runtime-config.ts:69-82). As a result, strict policy can report policy/gateway-non-loopback-bind for a gateway that would fail before listening, which is the same false-positive shape this revision fixed for blank custom hosts. Please gate this evidence on the same valid-IPv4 runtime contract, or surface it as invalid config rather than active non-loopback exposure.
There was a problem hiding this comment.
Fixed in f559951 after rebasing this stack on the updated #81974 head.
Policy evidence now treats custom bind exposure the same way gateway startup does: a customBindHost only counts as active non-loopback exposure when it is a valid canonical IPv4 and not 127/8. Invalid custom hosts such as localhost, ::1, and non-canonical IPv4 strings no longer produce policy/gateway-non-loopback-bind evidence for a gateway that would fail before listening. A valid LAN IPv4 still reports the exposure.
Proof:
- git diff --check
- node scripts/run-vitest.mjs extensions/policy/src/doctor/register.test.ts extensions/policy/src/cli.test.ts --run --reporter=dot
- node node_modules/@typescript/native-preview/bin/tsgo.js -p test/tsconfig/tsconfig.extensions.test.json --incremental --tsBuildInfoFile .artifacts/tsgo-cache/extensions-test-policy-81981-custom-bind.tsbuildinfo
- /mnt/c/src/claws-hapi/.agents/skills/autoreview/scripts/autoreview --mode local -> clean, no accepted/actionable findings
f559951 to
494972e
Compare
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
Summary
Adds a Policy 1.0 Gateway exposure conformance category with config-only evidence and doctor/lint findings.
Logical base: #81974, now merged into
main. This PR intentionally does not include runtime audit, gateway protocol, approval metadata, Swift protocol, or before-tool-call changes.gateway.http.requireUrlAllowlists.Policy syntax
{ "gateway": { "exposure": { "allowNonLoopbackBind": false, "allowTailscaleFunnel": false, }, "auth": { "requireAuth": true, "requireExplicitRateLimit": true, }, "controlUi": { "allowInsecure": false, }, "remote": { "allow": false, }, "http": { "denyEndpoints": ["chatCompletions", "responses"], "requireUrlAllowlists": true, }, }, }Safety
This is config-level conformance only.
doctor --lintand policy checks read existing OpenClaw config and report findings. This PR does not launch a Gateway, mutate Gateway config, change Gateway runtime behavior, or add runtime enforcement.Real behavior proof
Behavior addressed: adds policy conformance evidence and findings for Gateway exposure settings, including unsafe remote/custom binding, disabled auth, missing rate-limit posture, insecure Control UI toggles, remote gateway mode, enabled HTTP endpoints, and unrestricted URL-fetch posture.
Real environment tested: WSL Ubuntu checkout; PR head
e7af8ffabd9f71c8d7ee84cc0749b97f1e5c7856is signed and GitHub-verified.Exact steps or command run after this patch:
pnpm docs:listgit diff --checknode scripts/run-vitest.mjs extensions/policy/src/doctor/register.test.ts extensions/policy/src/cli.test.ts -- --reporter=dotpnpm deps:shrinkwrap:check/mnt/c/src/claws-hapi/.agents/skills/autoreview/scripts/autoreview --mode branch --base origin/main --reviewer codex --fallback-reviewer none/mnt/c/src/claws-hapi/.agents/skills/autoreview/scripts/autoreview --mode local --reviewer codex --fallback-reviewer noneEvidence after fix: docs index sanity completed; focused policy doctor/CLI tests passed 2 files and 117 tests after the ClawSweeper custom-bind fix; shrinkwrap check reported all package shrinkwraps current; branch and local autoreview both reported no accepted/actionable findings.
Observed result after fix: the rebased branch contains only Gateway policy/docs changes on top of current
main, with no lockfile or shrinkwrap changes. Policy tests cover the Gateway conformance findings and the custom-bind/runtime-validation alignment cases, including blank custom bind hosts not counting as active non-loopback exposure.What was not tested: no live Gateway was launched. The proof verifies config-level policy doctor/lint behavior against config and policy inputs.
Related