feat: implement secrets injection proxy for secure gateway mode#7600
feat: implement secrets injection proxy for secure gateway mode#7600robertchang-ga wants to merge 3 commits into
Conversation
- Add secrets-proxy.ts: HTTP proxy with placeholder replacement, allowlist, 10MB body limit, 5min timeout - Add secrets-proxy-allowlist.ts: Domain allowlist with persistence to ~/.openclaw/allowlist.json - Add gateway-container.ts: Docker container lifecycle, env filtering, port mapping - Add secure-fetch.ts: Fetch wrapper routing requests through proxy via X-Target-URL header - Add allowlist-cli.ts: CLI commands for gateway allowlist management - Modify run.ts: Add --secure flag with proxy/container orchestration - Modify index.ts: Install secure fetch wrapper on startup - Modify model-auth.ts, live-auth-keys.ts: Return placeholders in secure mode - Add SecretsProxyConfig to types.gateway.ts - Update SECURITY.MD with implementation status
openclaw-barnacle
Bot
added
cli
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const response = await request(targetUrl, { | ||
| method: (req.method as any) || "POST", | ||
| headers, | ||
| body: modifiedBody, | ||
| }); |
There was a problem hiding this comment.
[P0] The proxy always forwards a string body, even for GET/HEAD requests (and even when the original request had no body). Some upstreams will reject this or behave unexpectedly, and it can break signature/auth schemes. This should preserve the original method/body semantics (e.g., omit body for GET/HEAD, and avoid forcing utf8 decoding for non-text payloads).
This also affects secure-fetch because it routes all fetches through this proxy.
Also appears in: src/security/secure-fetch.ts:54-57 (it forwards whatever init was, but doesn't handle input being a Request with its own body/headers).
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/security/secrets-proxy.ts
Line: 107:111
Comment:
[P0] The proxy always forwards a string body, even for GET/HEAD requests (and even when the original request had no body). Some upstreams will reject this or behave unexpectedly, and it can break signature/auth schemes. This should preserve the original method/body semantics (e.g., omit body for GET/HEAD, and avoid forcing utf8 decoding for non-text payloads).
This also affects `secure-fetch` because it routes all fetches through this proxy.
Also appears in: src/security/secure-fetch.ts:54-57 (it forwards whatever `init` was, but doesn't handle `input` being a Request with its own body/headers).
How can I resolve this? If you propose a fix, please make it concise.| // Build new headers with X-Target-URL | ||
| const headers = new Headers(init?.headers); | ||
| headers.set("X-Target-URL", targetUrl); | ||
|
|
||
| // Route through proxy | ||
| return originalFetch(PROXY_URL, { | ||
| ...init, | ||
| headers, | ||
| }); |
There was a problem hiding this comment.
[P0] When fetch(input) is called with a Request object and no init, this wrapper drops the original request's method/headers/body because it only uses init and ignores input's fields. That will silently turn many non-GET requests into a proxy request without the intended payload/auth headers.
This wrapper likely needs to derive method/headers/body from the Request when init is undefined, or re-create a Request to the proxy that copies the original request details plus X-Target-URL.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/security/secure-fetch.ts
Line: 49:57
Comment:
[P0] When `fetch(input)` is called with a `Request` object and no `init`, this wrapper drops the original request's method/headers/body because it only uses `init` and ignores `input`'s fields. That will silently turn many non-GET requests into a proxy request without the intended payload/auth headers.
This wrapper likely needs to derive `method/headers/body` from the `Request` when `init` is undefined, or re-create a `Request` to the proxy that copies the original request details plus `X-Target-URL`.
How can I resolve this? If you propose a fix, please make it concise.| try { | ||
| if (opts.secure) { | ||
| gatewayLog.info("Starting in SECURE mode (Docker + Secrets Proxy)"); | ||
|
|
||
| // Set secure mode environment variable for this process | ||
| process.env.OPENCLAW_SECURE_MODE = "1"; | ||
|
|
||
| const proxyPort = 8080; | ||
| const proxyUrl = `http://host.docker.internal:${proxyPort}`; | ||
|
|
||
| const proxyServer = await startSecretsProxy({ port: proxyPort }); | ||
| gatewayLog.info(`Secrets proxy started on port ${proxyPort}`); | ||
|
|
||
| const containerName = await startGatewayContainer({ | ||
| proxyUrl, | ||
| env: process.env, | ||
| }); | ||
| gatewayLog.info(`Gateway container started: ${containerName}`); | ||
|
|
||
| // Set up graceful shutdown handlers | ||
| const shutdown = async () => { | ||
| gatewayLog.info("Shutting down secure gateway..."); | ||
| try { | ||
| await stopGatewayContainer(); | ||
| gatewayLog.info("Gateway container stopped"); | ||
| } catch (err) { | ||
| gatewayLog.error(`Error stopping container: ${String(err)}`); | ||
| } | ||
| try { | ||
| proxyServer.close(); | ||
| gatewayLog.info("Secrets proxy stopped"); | ||
| } catch (err) { | ||
| gatewayLog.error(`Error stopping proxy: ${String(err)}`); | ||
| } | ||
| defaultRuntime.exit(0); | ||
| }; | ||
|
|
||
| process.on("SIGINT", shutdown); | ||
| process.on("SIGTERM", shutdown); | ||
|
|
||
| gatewayLog.info("Secure mode running. Press Ctrl+C to stop."); | ||
|
|
||
| // Keep process alive | ||
| await new Promise(() => {}); | ||
| return; |
There was a problem hiding this comment.
[P1] In --secure mode this command sets OPENCLAW_SECURE_MODE=1 and then blocks forever (await new Promise(() => {})). That means none of the normal gateway startup path (port checks, auth checks, bind mode, etc.) is used, and the CLI can never return normally.
This is fine if it's intended as a long-running supervisor, but it also means errors after starting the container/proxy are only visible via logs, and callers/scripts can’t easily detect readiness/failure. Consider reusing runGatewayLoop semantics (or at least waiting for container health/readiness and returning a nonzero exit on failure).
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/cli/gateway-cli/run.ts
Line: 263:307
Comment:
[P1] In `--secure` mode this command sets `OPENCLAW_SECURE_MODE=1` and then blocks forever (`await new Promise(() => {})`). That means none of the normal gateway startup path (port checks, auth checks, bind mode, etc.) is used, and the CLI can never return normally.
This is fine if it's intended as a long-running supervisor, but it also means errors after starting the container/proxy are only visible via logs, and callers/scripts can’t easily detect readiness/failure. Consider reusing `runGatewayLoop` semantics (or at least waiting for container health/readiness and returning a nonzero exit on failure).
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise.| for (const [key, value] of Object.entries(filteredEnv)) { | ||
| args.push("-e", `${key}=${value}`); | ||
| } |
There was a problem hiding this comment.
[P1] filterSecretEnv only strips variables by suffix (*_API_KEY, *_TOKEN, *_SECRET). Many common secrets won’t match these suffixes (e.g., AWS_ACCESS_KEY_ID, AWS_SECRET_ACCESS_KEY, DATABASE_URL, PASSWORD, COOKIE, etc.), so they will still be passed into the container via -e ${key}=${value}. That undermines the "zero secrets in container env" goal.
At minimum, AWS creds in particular are a gap here because they’re explicitly supported elsewhere in auth flow.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/security/gateway-container.ts
Line: 62:64
Comment:
[P1] `filterSecretEnv` only strips variables by suffix (`*_API_KEY`, `*_TOKEN`, `*_SECRET`). Many common secrets won’t match these suffixes (e.g., `AWS_ACCESS_KEY_ID`, `AWS_SECRET_ACCESS_KEY`, `DATABASE_URL`, `PASSWORD`, `COOKIE`, etc.), so they will still be passed into the container via `-e ${key}=${value}`. That undermines the "zero secrets in container env" goal.
At minimum, AWS creds in particular are a gap here because they’re explicitly supported elsewhere in auth flow.
How can I resolve this? If you propose a fix, please make it concise.| res.statusCode = response.statusCode; | ||
| for (const [key, value] of Object.entries(response.headers)) { | ||
| if (value) { | ||
| res.setHeader(key, value); | ||
| } | ||
| } |
There was a problem hiding this comment.
[P1] Proxy response header forwarding may be incorrect: undici response headers can be string | string[], but res.setHeader(key, value) expects string | string[] | number. Here the if (value) guard will pass for arrays, but also passes for non-string truthy values, and will skip legitimate empty-string headers. This can cause runtime type issues or subtly drop headers.
Consider explicitly handling string[] and string and avoiding filtering out valid empty values.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/security/secrets-proxy.ts
Line: 113:118
Comment:
[P1] Proxy response header forwarding may be incorrect: `undici` response headers can be `string | string[]`, but `res.setHeader(key, value)` expects `string | string[] | number`. Here the `if (value)` guard will pass for arrays, but also passes for non-string truthy values, and will skip legitimate empty-string headers. This can cause runtime type issues or subtly drop headers.
Consider explicitly handling `string[]` and `string` and avoiding filtering out valid empty values.
How can I resolve this? If you propose a fix, please make it concise.P0 Fixes: - secure-fetch: Properly extract method/headers/body from Request objects - secrets-proxy: Only send body for methods that should have one (not GET/HEAD) - secrets-proxy: Handle binary payloads properly, don't force utf8 P1 Fixes: - gateway-container: Comprehensive secret filtering (AWS, DB, cloud provider creds) - gateway-container: Add bind mount support for tools/skills - gateway-container: Add health check utility functions - secrets-proxy: Properly handle string[] response headers - secrets-proxy: Add timeout to DoS placeholder limits - run.ts: Add container health checks with timeout and periodic monitoring - run.ts: Add proper error handling for proxy/container startup failures
…mory YouTube Ultimate v2.0.0: - Added yt-dlp integration for video/audio downloads - formats command to list available qualities - 4K/1440p support, FLAC/WAV/OPUS audio - Cookie support for age-restricted content - Robust error handling WhatsApp Full History Sync: - New config option: syncFullHistory (opt-in) - Handles messaging-history.set events from Baileys - Enables reading all past messages (one-time sync at pairing) LanceDB Hybrid Memory: - Cherry-picked openclaw#7695 + openclaw#7636 for better recall - Hybrid search combining vector + keyword Also includes: - openclaw#7747 Zero-latency hot-reload - openclaw#7635 Browser cookies action - openclaw#7600 Secrets injection proxy Trust-first fork: Removing restrictions, enabling full AI access.
1 participant
This PR implements a secrets injection proxy that allows the gateway to run inside a Docker container with zero access to sensitive credentials.
Changes
New Files
Modified Files
Usage
```bash
Run in secure mode
openclaw gateway run --secure
Manage allowlist
openclaw gateway allowlist list
openclaw gateway allowlist add custom-api.example.com
```
Additional Security
Greptile Overview
Greptile Summary
This PR adds a “secure gateway mode” that runs the gateway in a Docker container with a host-side secrets-injection HTTP proxy. It introduces a global
fetchwrapper (enabled viaOPENCLAW_SECURE_MODE) that routes outbound HTTP calls through the proxy using anX-Target-URLheader, plus a persisted domain allowlist and CLI commands to manage it. Authentication helpers are updated to return{{ENV_VAR}}placeholders in secure mode so the proxy can inject secrets at request time.Overall, the change is coherent (container + proxy + placeholder auth), but there are a couple of correctness gaps in request forwarding (not preserving
Requestmethod/headers/body and always sending a string body) and a security goal gap in the env filtering passed into the container.Confidence Score: 2/5
fetchand proxying all outbound requests; the current wrapper drops data when called with aRequestand the proxy forces utf8 body forwarding, both of which can break real provider calls. Additionally, the container env filter does not cover common secret env vars (notably AWS), undermining the stated security objective.(3/5) Reply to the agent's comments like "Can you suggest a fix for this @greptileai?" or ask follow-up questions!