Skip to content

Commit cc73885

Browse files
authored
Strip cookie from web socket requests (#8111)
This matches what we already do for regular HTTP requests.
1 parent c587540 commit cc73885

2 files changed

Lines changed: 28 additions & 8 deletions

File tree

‎src/node/proxy.ts‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import * as cookie from "cookie"
22
import type { Request } from "express"
3+
import http from "http"
34
import proxyServer from "http-proxy"
45
import { getCookieSessionName, HttpCode } from "../common/http"
56

@@ -26,7 +27,7 @@ function identity<T>(val: T): T {
2627

2728
// Strip the code-server cookie if it exists to avoid transmitting the cookie
2829
// to potentially malicious local ports.
29-
proxy.on("proxyReq", (preq, req) => {
30+
function stripCookie(preq: http.ClientRequest, req: http.IncomingMessage) {
3031
if (req.headers.cookie) {
3132
const cookieSessionName = getCookieSessionName((req as Request).args["cookie-suffix"])
3233
// Encoding and decoding are no-ops; we just want to remove the token
@@ -45,7 +46,10 @@ proxy.on("proxyReq", (preq, req) => {
4546
),
4647
)
4748
}
48-
})
49+
}
50+
51+
proxy.on("proxyReq", stripCookie)
52+
proxy.on("proxyReqWs", stripCookie)
4953

5054
// Intercept the response to rewrite absolute redirects against the base path.
5155
// Is disabled when the request has no base path which means /absproxy is in use.

‎test/unit/node/proxy.test.ts‎

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -303,6 +303,13 @@ describe("proxy", () => {
303303
res.end(req.headers.cookie)
304304
})
305305

306+
wsRouter.ws("/wsup/cookies", async (req) => {
307+
wss.handleUpgrade(req, req.ws, req.head, (ws) => {
308+
ws.on("message", () => ws.send(req.headers.cookie || ""))
309+
req.ws.resume()
310+
})
311+
})
312+
306313
const cookies = [
307314
"cookie2=hello",
308315
// Cookies should pass through unchanged, neither encoded nor decoded.
@@ -316,13 +323,22 @@ describe("proxy", () => {
316323
for (let i = 0; i <= cookies.length; ++i) {
317324
const left = cookies.slice(0, i)
318325
const right = cookies.slice(i)
319-
const resp = await codeServer.fetch(proxyPath + "/cookies", {
320-
headers: {
321-
cookie: [...left, `code-server-session=${token}`, ...right].join("; "),
322-
},
323-
})
326+
const expected = cookies.join("; ")
327+
const headers = {
328+
cookie: [...left, `code-server-session=${token}`, ...right].join("; "),
329+
}
330+
// Regular HTTP request.
331+
const resp = await codeServer.fetch(proxyPath + "/cookies", { headers })
324332
expect(resp.status).toBe(200)
325-
expect(await resp.text()).toBe(cookies.join("; "))
333+
expect(await resp.text()).toBe(expected)
334+
// Web socket request.
335+
const ws = await codeServer.wsWait(proxyPath + "/cookies", { headers })
336+
const got = await new Promise((resolve) => {
337+
ws.send("init")
338+
ws.on("message", (msg) => resolve(msg))
339+
})
340+
ws.terminate()
341+
expect(got).toBe(expected)
326342
}
327343
})
328344

0 commit comments

Comments
 (0)