Feature/trusted proxy loopback#63379
Conversation
Allows loopback addresses (127.0.0.1, ::1) in trusted-proxy mode when explicitly enabled via config. Useful for same-pod or same-host deployments where agent and gateway share the same network namespace. - Add allowLoopback?: boolean to GatewayTrustedProxyConfig type - Add Zod validation for allowLoopback field - Modify authorizeTrustedProxy to check allowLoopback before rejecting - Add unit tests for allowLoopback behavior
* main: (522 commits) fix(browser): re-check interaction-driven navigations (openclaw#63226) test: reuse verbose directive reply imports test: reuse exec directive reply imports fix(browser): harden browser control override loading (openclaw#62663) Matrix: report startup failures as errors auth: persist explicit profile upserts directly test(doctor): mock memory-core runtime seam auth: avoid external cli sync on profile upsert feat: parallelize character eval runs fix: load QA live provider overrides build: stage nostr runtime dependencies fix(dotenv): block workspace runtime env vars (openclaw#62660) build: narrow plugin SDK declaration build test: harden Parallels macOS smoke fallback fix(memory): accept embedded dreaming heartbeat tokens test: harden provider mock isolation docs(config): tighten wording in reference test: reuse followup runner imports test: reuse image generate tool imports Align remote node exec event system messages with untrusted handling (openclaw#62659) ...
Greptile SummaryThis PR adds two opt-in fields —
Confidence Score: 4/5Not safe to merge as-is: the One P1 defect: src/gateway/auth.ts (requiredHeaders enforcement order) and src/gateway/auth.test.ts (test at line 957 is inconsistent with the implementation)
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7614779cc8
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if ((!userHeaderValue || userHeaderValue.trim() === "") && isLoopbackAddress(remoteAddr) && trustedProxyConfig.allowLoopback && trustedProxyConfig.loopbackUser) { | ||
| userHeaderValue = trustedProxyConfig.loopbackUser; |
There was a problem hiding this comment.
Move loopbackUser fallback before required-header validation
With allowLoopback: true and loopbackUser configured, headerless loopback clients are still rejected whenever requiredHeaders is non-empty, because the requiredHeaders loop runs earlier and returns trusted_proxy_missing_header_* before this fallback can execute. In practice this keeps internal loopback callers (browser tool/sub-agents/CLI) broken in trusted-proxy setups that require headers like x-forwarded-proto.
Useful? React with 👍 / 👎.
| @@ -399,9 +401,14 @@ function authorizeTrustedProxy(params: { | |||
| } | |||
| } | |||
|
|
|||
| const userHeaderValue = headerValue( | |||
| let userHeaderValue = headerValue( | |||
| req.headers[normalizeLowercaseStringOrEmpty(trustedProxyConfig.userHeader)], | |||
| ); | |||
| // Internal loopback connections (browser tool, sub-agents, CLI) don't carry | |||
| // proxy headers. Use the configured loopbackUser identity when available. | |||
| if ((!userHeaderValue || userHeaderValue.trim() === "") && isLoopbackAddress(remoteAddr) && trustedProxyConfig.allowLoopback && trustedProxyConfig.loopbackUser) { | |||
| userHeaderValue = trustedProxyConfig.loopbackUser; | |||
| } | |||
There was a problem hiding this comment.
requiredHeaders blocks loopbackUser injection
The requiredHeaders loop runs before the loopbackUser fallback (line 409), so any request that lacks a required header fails with trusted_proxy_missing_header_<header> before the loopback identity injection is ever reached. The primary use-case — a browser-tool or sub-agent connecting from 127.0.0.1 with no proxy headers while requiredHeaders: ["x-forwarded-proto"] is set — will always fail on the header check rather than falling through to loopbackUser.
The PR description explicitly says "requiredHeaders SKIP" for the loopback+loopbackUser path, and the test at line 957 expects res.ok === true with headers: { host: "localhost" } while trustedProxyConfig contains requiredHeaders: ["x-forwarded-proto"] — that test will fail with trusted_proxy_missing_header_x-forwarded-proto against the current code.
// suggested fix: skip requiredHeaders for loopback+loopbackUser connections
const isLoopback = isLoopbackAddress(remoteAddr);
const isLoopbackInternal = isLoopback && trustedProxyConfig.allowLoopback && trustedProxyConfig.loopbackUser;
if (!isLoopbackInternal) {
const requiredHeaders = trustedProxyConfig.requiredHeaders ?? [];
for (const header of requiredHeaders) {
const value = headerValue(req.headers[normalizeLowercaseStringOrEmpty(header)]);
if (!value || value.trim() === "") {
return { reason: `trusted_proxy_missing_header_${header}` };
}
}
}Prompt To Fix With AI
This is a comment left during a code review.
Path: src/gateway/auth.ts
Line: 396-411
Comment:
**`requiredHeaders` blocks `loopbackUser` injection**
The `requiredHeaders` loop runs before the `loopbackUser` fallback (line 409), so any request that lacks a required header fails with `trusted_proxy_missing_header_<header>` before the loopback identity injection is ever reached. The primary use-case — a browser-tool or sub-agent connecting from `127.0.0.1` with no proxy headers while `requiredHeaders: ["x-forwarded-proto"]` is set — will always fail on the header check rather than falling through to `loopbackUser`.
The PR description explicitly says "requiredHeaders SKIP" for the loopback+loopbackUser path, and the test at line 957 expects `res.ok === true` with `headers: { host: "localhost" }` while `trustedProxyConfig` contains `requiredHeaders: ["x-forwarded-proto"]` — that test will fail with `trusted_proxy_missing_header_x-forwarded-proto` against the current code.
```ts
// suggested fix: skip requiredHeaders for loopback+loopbackUser connections
const isLoopback = isLoopbackAddress(remoteAddr);
const isLoopbackInternal = isLoopback && trustedProxyConfig.allowLoopback && trustedProxyConfig.loopbackUser;
if (!isLoopbackInternal) {
const requiredHeaders = trustedProxyConfig.requiredHeaders ?? [];
for (const header of requiredHeaders) {
const value = headerValue(req.headers[normalizeLowercaseStringOrEmpty(header)]);
if (!value || value.trim() === "") {
return { reason: `trusted_proxy_missing_header_${header}` };
}
}
}
```
How can I resolve this? If you propose a fix, please make it concise.…nternal connections When auth.mode=trusted-proxy, internal subsystems (browser tool, sub-agents, CLI) connect via loopback without proxy headers, causing trusted_proxy_loopback_source and trusted_proxy_user_missing rejections. Add trustedProxy.allowLoopback to bypass the hard loopback rejection, trustedProxy.loopbackUser to auto-inject an operator identity for headerless loopback connections, and skip requiredHeaders checks for internal loopback connections using loopbackUser. Real proxy headers are always preferred. Closes openclaw#43300, closes openclaw#26007 Refs openclaw#4944, openclaw#16299, openclaw#50628
7614779 to
d73d8e5
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d73d8e5655
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const isInternalBackend = | ||
| (params.clientMode === GATEWAY_CLIENT_MODES.BACKEND || | ||
| params.clientMode === GATEWAY_CLIENT_MODES.PROBE) && | ||
| remoteAddr != null && | ||
| isLoopbackAddress(remoteAddr); |
There was a problem hiding this comment.
Keep backend/probe loopback on trusted-proxy path
This new isInternalBackend gate skips authorizeTrustedProxy() for loopback clients in BACKEND/PROBE mode, but execution then stays in auth.mode === "trusted-proxy" and eventually returns generic unauthorized (token/password branches are never reachable in this mode). Internal GatewayClient callers default to BACKEND mode (src/gateway/call.ts), so the advertised allowLoopback/loopbackUser fix still rejects browser/sub-agent/probe self-connections in production trusted-proxy deployments.
Useful? React with 👍 / 👎.
| const isInternalLoopback = | ||
| isLoopbackAddress(remoteAddr) && | ||
| trustedProxyConfig.allowLoopback && | ||
| trustedProxyConfig.loopbackUser; |
There was a problem hiding this comment.
Restrict required-header bypass to true loopback fallback
isInternalLoopback is true for any loopback request whenever allowLoopback and loopbackUser are set, so requiredHeaders are skipped even when a real proxied userHeader is present. In same-host sidecar setups this effectively disables required-header enforcement for normal proxied traffic, weakening the trusted-proxy hardening and allowing missing-header requests to authenticate if they include a user header.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cf6efce4fc
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| trustedProxyConfig.loopbackUser; | ||
| if (!isInternalLoopback) { | ||
| for (const h of requiredHeaders) { | ||
| if (!headerValue(req.headers[normalizeLowercaseStringOrEmpty(h)])) { |
There was a problem hiding this comment.
Reject whitespace-only trusted-proxy required headers
The new required-header check now only verifies header presence (headerValue(...)) and no longer trims values before validation, so a header like x-forwarded-proto: " " is treated as valid. In trusted-proxy mode this weakens the requiredHeaders hardening and can allow requests to pass provenance checks with effectively empty forwarded metadata as long as a user header is present.
Useful? React with 👍 / 👎.
| if (bindMode === "lan" || bindMode === "custom") { | ||
| // In K8s/container environments prefer POD_IP when available. | ||
| const podIp = process.env["POD_IP"]; | ||
| if (podIp) { | ||
| return `${scheme}://${podIp}:${port}`; |
There was a problem hiding this comment.
Honor customBindHost in internal gateway URL resolution
resolveInternalGatewayUrl() groups bind === "custom" with LAN resolution and never reads gateway.customBindHost, so it can return POD_IP or the primary LAN IP even when the gateway is bound to a different custom address. Any caller that follows the new deprecation guidance and uses this helper can fail to connect in custom-bind deployments (for example when customBindHost is 127.0.0.1 or another non-primary interface).
Useful? React with 👍 / 👎.
cf6efce to
12e0bf9
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 12e0bf997c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (bindMode === "lan" || bindMode === "custom") { | ||
| // In K8s/container environments prefer POD_IP when available. | ||
| const podIp = process.env["POD_IP"]; | ||
| if (podIp) { | ||
| return `${scheme}://${podIp}:${port}`; |
There was a problem hiding this comment.
Return loopback URL when local TLS is disabled
resolveInternalGatewayUrl() now returns ws://<POD_IP|LAN_IP> for bind: "lan"|"custom", but GatewayClient rejects plaintext ws:// to non-loopback hosts by default (src/gateway/client.ts security check), so callers that follow the new deprecation guidance and pass this helper result into callGateway/callGatewayTool will fail before auth unless they set the insecure override env var. Keep loopback for non-TLS local URLs (or otherwise ensure a secure URL path) so the new helper is usable in default local-trust deployments.
Useful? React with 👍 / 👎.
| if (!headerValue(req.headers[normalizeLowercaseStringOrEmpty(h)])) { | ||
| return { reason: "trusted_proxy_required_header_missing" }; |
There was a problem hiding this comment.
Preserve per-header trusted-proxy failure reason
The required-header loop now emits the generic trusted_proxy_required_header_missing reason instead of trusted_proxy_missing_header_<header>, which breaks existing expectations in this repo (for example src/gateway/auth.test.ts still asserts the header-specific reason) and removes actionable detail from failures. If this reason shape is being changed intentionally, all in-repo consumers/tests should be updated in the same change; otherwise keep the previous per-header reason format.
Useful? React with 👍 / 👎.
|
Thanks @mrosmarin. I landed the narrower same-host reverse-proxy part in #73266: I am closing this PR as superseded because the broader |
Summary
gateway.auth.mode=trusted-proxywithgateway.bind=lan, internal subsystems (browser tool, sub-agents, CLI) connect via loopback (ws://127.0.0.1:18789) and are hard-rejected bytrusted_proxy_loopback_sourcebeforetrustedProxiesis consulted. Even with127.0.0.1intrustedProxies, loopback connections are always rejected.sessions_spawn, Discord exec approvals, and all internalGatewayClientconnections are completely broken in Kubernetes/Docker deployments using trusted-proxy auth. This blocks the most common containerized deployment pattern (reverse proxy sidecar + trusted-proxy mode).trustedProxy.allowLoopbackto bypass the hard loopback rejection,trustedProxy.loopbackUserto auto-inject an operator identity for headerless loopback connections, and skiprequiredHeaderschecks for internal loopback connections usingloopbackUser. Real proxy headers are always preferred when present.GatewayClientURL resolution, browser tool code, or any other internal subsystem. This fix is entirely in the auth layer — internal clients still connect via loopback, but the auth layer now knows how to handle them.Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
Root Cause (if applicable)
authorizeTrustedProxy()insrc/gateway/auth.tsunconditionally rejects all loopback connections withtrusted_proxy_loopback_sourcebefore consulting thetrustedProxieslist. This was introduced as a security hardening measure to prevent sidecars from forging identity headers on loopback, but it also blocks the gateway's own internal subsystems (browser tool, sub-agents, CLI) which legitimately connect from loopback without proxy headers.bind: lan+trustedProxies: ["${POD_IP}/32"]pattern works for the external proxy but breaks internal self-connections.Regression Test Plan (if applicable)
src/gateway/auth.test.tsallowLoopback: true(with proxy headers)loopbackUserwhenuserHeaderis missing on loopbackuserHeaderwhenloopbackUseris not set (fail-closed)userHeaderoverloopbackUseron loopbackallowLoopback: truetrusted_proxy_loopback_sourcetest retained — confirms default behavior unchanged.User-visible / Behavior Changes
gateway.auth.trustedProxy:allowLoopback: boolean(default:false) — allow loopback addresses in trusted-proxy modeloopbackUser: string(optional) — identity to assign to headerless loopback connectionsDiagram (if applicable)
Security Impact (required)
allowLoopbackrelaxes the loopback rejection for trusted-proxy modeallowLoopbackis opt-in (defaultfalse), so existing deployments are unaffected. When enabled, loopback connections are still subject to all other trusted-proxy checks (user header validation,allowUsersgating).loopbackUseronly fires when the user header is absent AND the connection is from loopback ANDallowLoopbackis true — it cannot be triggered from non-loopback sources. Real proxy headers always take precedence overloopbackUser.Repro + Verification
Environment
ghcr.io/openclaw/openclaw:2026.4.2in K8s StatefulSetSteps
auth.mode=trusted-proxy,bind=lan, oauth2-proxy sidecartrusted_proxy_loopback_sourcerejectionExpected
Actual (before fix)
[ws] unauthorized conn=... remote=127.0.0.1 reason=trusted_proxy_loopback_source[tools] browser failed: gateway closed (1008): unauthorizedEvidence
Human Verification (required)
pnpm vitest run src/gateway/auth.test.tsgreen.Review Conversations
Compatibility / Migration
allowLoopback: false,loopbackUser: undefined). No behavior change without explicit opt-in.gateway.auth.trustedProxy.Risks and Mitigations
allowLoopbackwithoutloopbackUserand expects headerless internal connections to work — they'll gettrusted_proxy_user_missing.loopbackUserexplains the dependency. Could add a startup warning whenallowLoopback=trueandloopbackUseris unset.loopbackUseridentity could be used to bypassallowUsersrestrictions if the operator forgets to add it to the list.allowUserscheck runs after user resolution — ifloopbackUseris not inallowUsers, the connection is still rejected.