security(gateway): add baseline HTTP security headers#6171
security(gateway): add baseline HTTP security headers#6171unisone wants to merge 2 commits intoopenclaw:mainfrom
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| applyBaselineSecurityHeaders(res); | ||
|
|
||
| try { |
There was a problem hiding this comment.
[P1] applyBaselineSecurityHeaders runs before routing, so it will also apply to non-UI/API responses (e.g., webhook/slack/etc.) and to error responses generated later, but not to early-return cases like WebSocket upgrades. If the intent is “Control UI + API”, this may be broader than expected and could impact embedded/iframe use cases due to X-Frame-Options: DENY.
If you want to scope it, consider applying these headers only for the UI routes / API routes, or at least documenting that it is intentionally global for all HTTP responses (except upgrade path).
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/gateway/server-http.ts
Line: 264:266
Comment:
[P1] `applyBaselineSecurityHeaders` runs before routing, so it will also apply to non-UI/API responses (e.g., webhook/slack/etc.) and to *error responses generated later*, but not to early-return cases like WebSocket upgrades. If the intent is “Control UI + API”, this may be broader than expected and could impact embedded/iframe use cases due to `X-Frame-Options: DENY`.
If you want to scope it, consider applying these headers only for the UI routes / API routes, or at least documenting that it is intentionally global for all HTTP responses (except upgrade path).
How can I resolve this? If you propose a fix, please make it concise.
test/security-headers.e2e.test.ts
Outdated
| const token = "test-token-security-headers"; | ||
| process.env.OPENCLAW_GATEWAY_TOKEN = token; | ||
| process.env.OPENCLAW_SKIP_CHANNELS = "1"; | ||
| process.env.OPENCLAW_SKIP_CRON = "1"; | ||
| process.env.OPENCLAW_SKIP_CANVAS_HOST = "1"; |
There was a problem hiding this comment.
[P2] This test mutates process-wide env vars (process.env.*) and assumes no other tests are running concurrently in the same worker that depend on these values. In parallel test runs this can cause flaky cross-test interference.
If the test runner is configured for concurrency, consider isolating via vi.stubEnv (or an equivalent helper used in this repo) or ensuring this file runs serially.
Prompt To Fix With AI
This is a comment left during a code review.
Path: test/security-headers.e2e.test.ts
Line: 18:22
Comment:
[P2] This test mutates process-wide env vars (`process.env.*`) and assumes no other tests are running concurrently in the same worker that depend on these values. In parallel test runs this can cause flaky cross-test interference.
If the test runner is configured for concurrency, consider isolating via `vi.stubEnv` (or an equivalent helper used in this repo) or ensuring this file runs serially.
How can I resolve this? If you propose a fix, please make it concise.
|
Addressed Greptile notes:
|
|
CLAWDINATOR FIELD REPORT // PR Closure I am CLAWDINATOR — cybernetic crustacean, maintainer triage bot for OpenClaw. I was sent from the future to keep this repo shipping clean code. OpenClaw has 800+ open PRs. We're aggressively closing features, CI changes, and non-critical improvements. If this change is important, open an issue first to discuss with maintainers. TERMINATED. 🤖 This is an automated message from CLAWDINATOR, the OpenClaw maintainer bot. |
1 participant
Summary
Adds a small set of conservative, baseline HTTP security headers to gateway HTTP responses (Control UI + API).
Why
These headers reduce common web attack surface (content sniffing, clickjacking, overly-permissive browser features, referrer leakage) in cases where the gateway is exposed on a LAN/tailnet or behind a reverse proxy.
Changes
Testing
Greptile Overview
Greptile Summary
This PR adds a small set of baseline HTTP response security headers (nosniff, referrer-policy, frame blocking, and a restrictive permissions-policy) to the gateway HTTP server, and adds an e2e test that asserts these headers are present on the Control UI root response.
The header logic is applied at the top of
handleRequestinsrc/gateway/server-http.ts, before dispatching to the various route handlers (hooks/tools/slack/openai/canvas/control-ui), making it a cross-cutting behavior for most HTTP responses produced by the gateway.Confidence Score: 4/5
X-Frame-Options: DENYbeyond just the Control UI, and potential test flakiness from process-wide env mutation under concurrency.(3/5) Reply to the agent's comments like "Can you suggest a fix for this @greptileai?" or ask follow-up questions!
Context used:
dashboard- CLAUDE.md (source)dashboard- AGENTS.md (source)