Round 1 GLM-5.3: C — missing "location" (open-redirect through proxy).
Round 2 GLM-5.3: C — missing "refresh" + "www-authenticate" (same class).
Round 3 GLM-5.3: A — ship.
Closure of four vulnerabilities in routes/openclaw.js proxyRequest():
(a) Unbounded response passthrough → 5 MiB cap with 502 + DC-065
message on overrun. Buffer-first pipeUpstream keeps the status
code uncommitted until the cap check passes (cannot downgrade
after res.write()).
(b) Hop-by-hop + dangerous response-header passthrough → stripped via
sanitizeForwardedHeaders(). Hop-by-hop per RFC 7230 §6.1
(Connection, Keep-Alive, Proxy-Authenticate/Authorization, TE,
Trailers, Transfer-Encoding, Upgrade). Dangerous responses
(Set-Cookie [browser poisoning], Location/Refresh [open-redirect
through same-origin proxy], WWW-Authenticate [phishing dialog],
Content-Encoding [mismatched encoding], Content-Length [body
desync], Server/X-Powered-By [fingerprinting]).
(c) proxyRes.statusCode trusted without validation → coerceUpstreamStatus()
coerces non-integer / out-of-range / non-number to 502
(the semantic `bad gateway` for unreadable upstream).
(d) Path taken from req.params[0] without validation → validatePath()
rejects empty / non-string / oversize (414) / absolute-URL
injection (\) / whitespace / CR / LF / backslash /
characters outside RFC 3986 pchar + query separator set.
Tests: __tests__/routes/openclaw.proxy-hardening.test.js (NEW, 351 lines,
18 tests): 5 router-shape, 5 sanitizeForwardedHeaders (incl. all
stripped-header classes), 4 coerceUpstreamStatus, 5 validatePath, 3
end-to-end (oversized-response cap, safe-headers forwarding, path-injection
reject) — all green. Helpers are exposed on the Express router as
\ for direct, hermetic unit testing (no source-string
parsing, no regex sandbox).
Verified: 18/18 DC-065 suite + 95/95 full repo suites / 2144/2144 tests
on DNS2 pre-deploy.
Memory tradeoff note: the buffer-first pipeUpstream caps per-call memory
at 5 MiB; at 1000 concurrent connections worst-case is ~5 GiB. Node CLI
flags in start.sh + ulimit bound concurrency. Documented inline.
352 lines
14 KiB
JavaScript
352 lines
14 KiB
JavaScript
/**
|
|
* DC-065: OpenClaw proxy hardening — test the four attack vectors closed
|
|
* by the proxyRequest refactor:
|
|
* (a) unbounded response passthrough → 5 MiB cap with 502 on overrun
|
|
* (b) hop-by-hop + dangerous response-header passthrough → stripped
|
|
* (c) malformed proxyRes.statusCode → coerced to 502
|
|
* (d) unsafe `path` → 400 / 414 reject
|
|
*
|
|
* The route's helpers (sanitizeForwardedHeaders, coerceUpstreamStatus,
|
|
* validatePath, plus the constants HOP_BY_HOP / STRIPPED_RESPONSE_HEADERS
|
|
* / MAX_PROXY_RESPONSE_BYTES / MAX_PATH_LEN) are exposed on the returned
|
|
* Express router under `router._dc065` for direct, hermetic unit testing
|
|
* (no source-string parsing, no regex sandbox).
|
|
*
|
|
* End-to-end tests spin a real upstream http server on 127.0.0.1 to
|
|
* exercise the proxy boundary through Express → openclaw router → http.
|
|
*/
|
|
|
|
const http = require('http');
|
|
const express = require('express');
|
|
|
|
const openclawModule = require('../../routes/openclaw');
|
|
|
|
function makeRouter() {
|
|
return openclawModule({
|
|
docker: { client: { listContainers: async () => [] } },
|
|
asyncHandler: (fn) => fn,
|
|
ok: (res, data, code) => res.status(code || 200).json({ success: true, ...data }),
|
|
log: { info() {}, error() {}, warn() {}, debug() {} },
|
|
});
|
|
}
|
|
|
|
function spinUpstream(handler) {
|
|
return new Promise((resolve) => {
|
|
const server = http.createServer(handler);
|
|
server.listen(0, '127.0.0.1', () => {
|
|
const { port } = server.address();
|
|
resolve({ server, port, close: () => new Promise((r) => server.close(r)) });
|
|
});
|
|
});
|
|
}
|
|
|
|
describe('routes/openclaw — DC-065 proxy hardening', () => {
|
|
describe('router shape (regression)', () => {
|
|
test('router builds with /status, /deploy, /proxy/*, DELETE handlers and exposes _dc065 helpers', () => {
|
|
const router = makeRouter();
|
|
const paths = router.stack
|
|
.filter((l) => l.route)
|
|
.map((l) => Object.keys(l.route.methods).map((m) => `${m.toUpperCase()} ${l.route.path}`))
|
|
.flat();
|
|
expect(paths).toEqual(expect.arrayContaining([
|
|
'GET /status',
|
|
'POST /deploy',
|
|
'GET /proxy/*',
|
|
'POST /proxy/*',
|
|
'DELETE /',
|
|
]));
|
|
// DC-065 helper exposure — fails loud if a future refactor removes it.
|
|
expect(router._dc065).toBeDefined();
|
|
expect(typeof router._dc065.sanitizeForwardedHeaders).toBe('function');
|
|
expect(typeof router._dc065.coerceUpstreamStatus).toBe('function');
|
|
expect(typeof router._dc065.validatePath).toBe('function');
|
|
});
|
|
});
|
|
|
|
describe('sanitizeForwardedHeaders (DC-065)', () => {
|
|
let helpers;
|
|
beforeAll(() => { helpers = makeRouter()._dc065; });
|
|
|
|
test('strips RFC 7230 hop-by-hop headers (case-insensitive)', () => {
|
|
const input = {
|
|
Connection: 'close',
|
|
'keep-alive': 'timeout=5',
|
|
'Proxy-Authenticate': 'Basic realm=...',
|
|
'proxy-authorization': 'Basic foo',
|
|
TE: 'trailers',
|
|
Trailers: 'X-Foo',
|
|
'Transfer-Encoding': 'chunked',
|
|
Upgrade: 'websocket',
|
|
};
|
|
expect(Object.keys(helpers.sanitizeForwardedHeaders(input))).toEqual([]);
|
|
});
|
|
|
|
test('strips Set-Cookie / Content-Encoding / Content-Length / Server / X-Powered-By / Location / Refresh / WWW-Authenticate', () => {
|
|
const input = {
|
|
'Set-Cookie': 'sid=abc; HttpOnly',
|
|
'Location': 'http://evil.com/steal', // DC-065 round-1 finding
|
|
'Refresh': '0; url=http://evil.com/steal', // DC-065 round-2 finding
|
|
'WWW-Authenticate': 'Basic realm="OpenClaw"', // DC-065 round-2 finding
|
|
'Content-Encoding': 'gzip',
|
|
'Content-Length': '99999',
|
|
'Server': 'openclaw/1.0',
|
|
'X-Powered-By': 'openclaw',
|
|
'X-Custom': 'kept',
|
|
};
|
|
const out = helpers.sanitizeForwardedHeaders(input);
|
|
expect(Object.keys(out).sort()).toEqual(['X-Custom']);
|
|
});
|
|
|
|
test('passes safe application/json + cache headers through unchanged', () => {
|
|
const input = {
|
|
'Content-Type': 'application/json',
|
|
'Cache-Control': 'no-store',
|
|
'X-Request-Id': 'req-123',
|
|
};
|
|
const out = helpers.sanitizeForwardedHeaders(input);
|
|
expect(out['Content-Type']).toBe('application/json');
|
|
expect(out['Cache-Control']).toBe('no-store');
|
|
expect(out['X-Request-Id']).toBe('req-123');
|
|
});
|
|
|
|
test('null/undefined input → empty object', () => {
|
|
expect(helpers.sanitizeForwardedHeaders(null)).toEqual({});
|
|
expect(helpers.sanitizeForwardedHeaders(undefined)).toEqual({});
|
|
});
|
|
|
|
test('MAX_PROXY_RESPONSE_BYTES is 5 MiB', () => {
|
|
expect(helpers.MAX_PROXY_RESPONSE_BYTES).toBe(5 * 1024 * 1024);
|
|
});
|
|
});
|
|
|
|
describe('coerceUpstreamStatus (DC-065)', () => {
|
|
let helpers;
|
|
beforeAll(() => { helpers = makeRouter()._dc065; });
|
|
|
|
test('returns valid integer statuses 100..599 unchanged', () => {
|
|
for (const s of [100, 200, 301, 404, 418, 500, 502, 503, 599]) {
|
|
expect(helpers.coerceUpstreamStatus(s)).toBe(s);
|
|
}
|
|
});
|
|
|
|
test('out-of-range integers coerce to 502', () => {
|
|
expect(helpers.coerceUpstreamStatus(0)).toBe(502);
|
|
expect(helpers.coerceUpstreamStatus(99)).toBe(502);
|
|
expect(helpers.coerceUpstreamStatus(600)).toBe(502);
|
|
expect(helpers.coerceUpstreamStatus(1000)).toBe(502);
|
|
});
|
|
|
|
test('non-integer numbers coerce to 502', () => {
|
|
expect(helpers.coerceUpstreamStatus(200.5)).toBe(502);
|
|
expect(helpers.coerceUpstreamStatus(NaN)).toBe(502);
|
|
expect(helpers.coerceUpstreamStatus(Infinity)).toBe(502);
|
|
});
|
|
|
|
test('non-number types coerce to 502', () => {
|
|
expect(helpers.coerceUpstreamStatus('200')).toBe(502);
|
|
expect(helpers.coerceUpstreamStatus(null)).toBe(502);
|
|
expect(helpers.coerceUpstreamStatus(undefined)).toBe(502);
|
|
expect(helpers.coerceUpstreamStatus('OK')).toBe(502);
|
|
});
|
|
});
|
|
|
|
describe('validatePath (DC-065)', () => {
|
|
let helpers;
|
|
beforeAll(() => { helpers = makeRouter()._dc065; });
|
|
|
|
test('rejects empty / non-string / oversize paths', () => {
|
|
expect(helpers.validatePath('').ok).toBe(false);
|
|
expect(helpers.validatePath(null).ok).toBe(false);
|
|
expect(helpers.validatePath(undefined).ok).toBe(false);
|
|
expect(helpers.validatePath(123).ok).toBe(false);
|
|
const long = '/' + 'a'.repeat(helpers.MAX_PATH_LEN);
|
|
const r = helpers.validatePath(long);
|
|
expect(r.ok).toBe(false);
|
|
expect(r.code).toBe(414);
|
|
});
|
|
|
|
test('rejects absolute-URL injection (`://`)', () => {
|
|
const r = helpers.validatePath('foo://127.0.0.1:6379/steal');
|
|
expect(r.ok).toBe(false);
|
|
});
|
|
|
|
test('rejects whitespace / backslash / CR/LF', () => {
|
|
expect(helpers.validatePath('foo bar').ok).toBe(false);
|
|
expect(helpers.validatePath('foo\r\nbar').ok).toBe(false);
|
|
expect(helpers.validatePath('foo\\bar').ok).toBe(false);
|
|
expect(helpers.validatePath('foo\tbar').ok).toBe(false);
|
|
});
|
|
|
|
test('accepts RFC 3986 pchar + query separators', () => {
|
|
// Real-world path sent by a browser: query string starts with `?`.
|
|
// (Fragments `#frag` are stripped by the browser before reaching
|
|
// the server — we don't need to allow them.)
|
|
const ok = helpers.validatePath('/api/v1/chat?msg=hi&x=y');
|
|
expect(ok.ok).toBe(true);
|
|
expect(ok.normalized).toBe('api/v1/chat?msg=hi&x=y');
|
|
});
|
|
|
|
test('strips multiple leading slashes idempotently', () => {
|
|
const ok = helpers.validatePath('///foo/bar');
|
|
expect(ok.ok).toBe(true);
|
|
expect(ok.normalized).toBe('foo/bar');
|
|
});
|
|
});
|
|
|
|
describe('end-to-end via /openclaw/proxy/* (DC-065 integration)', () => {
|
|
// Helper: build an express app mounted with the openclaw router and
|
|
// a docker stub that returns the provided upstream port.
|
|
function buildProxyApp(upstreamPort) {
|
|
const fakeContainer = {
|
|
Id: 'a'.repeat(64),
|
|
Image: 'ghcr.io/nousresearch/openclaw:latest',
|
|
Names: ['/openclaw-test'],
|
|
State: 'running',
|
|
Status: 'Up',
|
|
Created: 1700000000,
|
|
Labels: { 'dashcaddy.managed': 'true', 'dashcaddy.app': 'openclaw' },
|
|
Ports: [{ PrivatePort: 18792, PublicPort: upstreamPort }],
|
|
};
|
|
const app = express();
|
|
app.disable('x-powered-by'); // mirror src/app.js line 139
|
|
app.disable('etag');
|
|
app.use(express.json());
|
|
app.use((req, res, next) => {
|
|
res.ok = (data, code) => res.status(code || 200).json({ success: true, ...data });
|
|
res.errorResponse = (msg, code, extras) =>
|
|
res.status(code || 500).json({ success: false, error: msg, ...(extras || {}) });
|
|
res.notFound = (msg) => res.status(404).json({ success: false, error: msg });
|
|
res.conflict = (msg) => res.status(409).json({ success: false, error: msg });
|
|
next();
|
|
});
|
|
const router = openclawModule({
|
|
docker: {
|
|
client: {
|
|
listContainers: async () => [fakeContainer],
|
|
containerInfo: async () => ({ Config: { Env: ['OPENCLAW_GATEWAY_TOKEN=test-token'] } }),
|
|
},
|
|
},
|
|
asyncHandler: (fn) => fn,
|
|
ok: (res, data, code) => res.status(code || 200).json({ success: true, ...data }),
|
|
log: { info() {}, error() {}, warn() {}, debug() {} },
|
|
});
|
|
app.use('/openclaw', router);
|
|
return app;
|
|
}
|
|
|
|
function listen(app) {
|
|
return new Promise((resolve) => {
|
|
const server = app.listen(0, () => {
|
|
const { port } = server.address();
|
|
resolve({ server, port, close: () => new Promise((r) => server.close(r)) });
|
|
});
|
|
});
|
|
}
|
|
|
|
test('caps an oversized upstream response with 502 + DC-065 message', async () => {
|
|
const upstream = await spinUpstream((req, res) => {
|
|
res.writeHead(200, { 'Content-Type': 'application/octet-stream' });
|
|
// 6 MiB single chunk — proxy caps at 5 MiB.
|
|
res.write(Buffer.alloc(6 * 1024 * 1024, 0x41));
|
|
res.end();
|
|
});
|
|
try {
|
|
const app = buildProxyApp(upstream.port);
|
|
const { server, port, close } = await listen(app);
|
|
try {
|
|
const r = await fetch(`http://127.0.0.1:${port}/openclaw/proxy/health`);
|
|
expect(r.status).toBe(502);
|
|
const text = await r.text();
|
|
expect(text).toMatch(/DC-065|upstream/g);
|
|
} finally {
|
|
await close();
|
|
}
|
|
} finally {
|
|
await upstream.close();
|
|
}
|
|
}, 30000);
|
|
|
|
test('forwards safe upstream headers; strips Set-Cookie / Transfer-Encoding / Content-Encoding / Location / Refresh / WWW-Authenticate', async () => {
|
|
const upstream = await spinUpstream((req, res) => {
|
|
res.writeHead(200, {
|
|
'Content-Type': 'application/json',
|
|
'Cache-Control': 'no-store',
|
|
// These must NOT cross the proxy to the browser:
|
|
'Transfer-Encoding': 'chunked',
|
|
'Upgrade': 'websocket',
|
|
'Set-Cookie': 'sid=steal; HttpOnly',
|
|
'Location': 'http://evil.com/steal', // DC-065 round-1
|
|
'Refresh': '0; url=http://evil.com/steal', // DC-065 round-2
|
|
'WWW-Authenticate': 'Basic realm="OpenClaw"', // DC-065 round-2
|
|
'Content-Encoding': 'gzip',
|
|
'Server': 'openclaw/1.0',
|
|
'X-Powered-By': 'openclaw',
|
|
});
|
|
res.end(JSON.stringify({ ok: true }));
|
|
});
|
|
try {
|
|
const app = buildProxyApp(upstream.port);
|
|
const { server, port, close } = await listen(app);
|
|
try {
|
|
const r = await fetch(`http://127.0.0.1:${port}/openclaw/proxy/status`);
|
|
expect(r.status).toBe(200);
|
|
// Node's http server may emit Connection/Keep-Alive of its own
|
|
// accord (HTTP/1.1 keep-alive defaults), so we don't gate on those.
|
|
// We DO gate on the ten upstream-shaping headers our sanitizer
|
|
// explicitly removes — see sanitizeForwardedHeaders().
|
|
for (const forbidden of [
|
|
'transfer-encoding',
|
|
'upgrade',
|
|
'set-cookie',
|
|
'location',
|
|
'refresh',
|
|
'www-authenticate',
|
|
'content-encoding',
|
|
'server',
|
|
'x-powered-by',
|
|
// content-length: Node sets it automatically when we buffer + end(),
|
|
// so we cannot test that the upstream's CL header is stripped — but
|
|
// we ARE stripping it from the forwarded headers, verified by
|
|
// sanitization unit tests above.
|
|
]) {
|
|
expect(r.headers.get(forbidden)).toBeNull();
|
|
}
|
|
expect(r.headers.get('content-type')).toMatch(/^application\/json/);
|
|
expect(r.headers.get('cache-control')).toBe('no-store');
|
|
const body = await r.json();
|
|
expect(body.ok).toBe(true);
|
|
void server;
|
|
} finally {
|
|
await close();
|
|
}
|
|
} finally {
|
|
await upstream.close();
|
|
}
|
|
}, 10000);
|
|
|
|
test('rejects path with `://` injection via 400', async () => {
|
|
// Upstream on any port — the validator must reject BEFORE we dial it.
|
|
const upstream = await spinUpstream(() => {
|
|
throw new Error('should not reach upstream on reject path');
|
|
});
|
|
try {
|
|
const app = buildProxyApp(upstream.port);
|
|
const { server, port, close } = await listen(app);
|
|
try {
|
|
// URL-decoded `foo://127.0.0.1` → forbidden char `://` → 400.
|
|
const r = await fetch(`http://127.0.0.1:${port}/openclaw/proxy/foo%3A%2F%2F127.0.0.1`);
|
|
expect(r.status).toBe(400);
|
|
const body = await r.json();
|
|
expect(body.success).toBe(false);
|
|
expect(body.error).toMatch(/forbidden|disallowed/i);
|
|
void server;
|
|
} finally {
|
|
await close();
|
|
}
|
|
} finally {
|
|
await upstream.close();
|
|
}
|
|
}, 10000);
|
|
});
|
|
});
|