mirror of
https://github.com/coder/code-server.git
synced 2026-09-02 08:30:17 +02:00
Fix mistakenly encoding cookies to proxy
We use the cookie parser to remove the code-server token but by default it encodes using encodeURIComponent, which encodes more than is strictly necessary and can break proxied applications. Now we pass the cookies through unchanged (other than removing the code-server token). Fixes #7927.
This commit is contained in:
@@ -22,6 +22,12 @@ Code v99.99.999
|
|||||||
|
|
||||||
## Unreleased
|
## Unreleased
|
||||||
|
|
||||||
|
### Fixed
|
||||||
|
|
||||||
|
- When proxying, cookies were being decoded and re-encoded, which could cause
|
||||||
|
issues for applications that encode differently. Cookies are now passed
|
||||||
|
through unchanged (aside from stripping out code-server's session token).
|
||||||
|
|
||||||
## [4.131.0](https://github.com/coder/code-server/releases/tag/v4.131.0) - 2026-07-30
|
## [4.131.0](https://github.com/coder/code-server/releases/tag/v4.131.0) - 2026-07-30
|
||||||
|
|
||||||
Code v1.131.0
|
Code v1.131.0
|
||||||
|
|||||||
@@ -20,17 +20,31 @@ proxy.on("error", (error, _, res) => {
|
|||||||
}
|
}
|
||||||
})
|
})
|
||||||
|
|
||||||
|
function identity<T>(val: T): T {
|
||||||
|
return val
|
||||||
|
}
|
||||||
|
|
||||||
// Strip the code-server cookie if it exists to avoid transmitting the cookie
|
// Strip the code-server cookie if it exists to avoid transmitting the cookie
|
||||||
// to potentially malicious local ports.
|
// to potentially malicious local ports.
|
||||||
proxy.on("proxyReq", (preq, req) => {
|
proxy.on("proxyReq", (preq, req) => {
|
||||||
const cookieSessionName = getCookieSessionName((req as Request).args["cookie-suffix"])
|
if (req.headers.cookie) {
|
||||||
preq.setHeader(
|
const cookieSessionName = getCookieSessionName((req as Request).args["cookie-suffix"])
|
||||||
"Cookie",
|
// Encoding and decoding are no-ops; we just want to remove the token
|
||||||
cookie.stringifyCookie({
|
// without changing anything else about the cookies because not all
|
||||||
...(req as Request).cookies,
|
// applications encode/decode the same way `cookie` here does.
|
||||||
[cookieSessionName]: undefined,
|
preq.setHeader(
|
||||||
}),
|
"Cookie",
|
||||||
)
|
cookie.stringifyCookie(
|
||||||
|
{
|
||||||
|
...cookie.parseCookie(req.headers.cookie, { decode: identity }),
|
||||||
|
[cookieSessionName]: undefined,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
encode: identity,
|
||||||
|
},
|
||||||
|
),
|
||||||
|
)
|
||||||
|
}
|
||||||
})
|
})
|
||||||
|
|
||||||
// Intercept the response to rewrite absolute redirects against the base path.
|
// Intercept the response to rewrite absolute redirects against the base path.
|
||||||
|
|||||||
@@ -298,25 +298,44 @@ describe("proxy", () => {
|
|||||||
process.env.HASHED_PASSWORD = token
|
process.env.HASHED_PASSWORD = token
|
||||||
codeServer = await integration.setup(["--auth=password"])
|
codeServer = await integration.setup(["--auth=password"])
|
||||||
|
|
||||||
// Set up a listener that just prints the cookies it got.
|
|
||||||
e.get("/wsup/cookies", (req, res) => {
|
e.get("/wsup/cookies", (req, res) => {
|
||||||
res.writeHead(HttpCode.Ok, { "Content-Type": "text/plain" })
|
res.writeHead(HttpCode.Ok, { "Content-Type": "text/plain" })
|
||||||
res.end(req.headers.cookie)
|
res.end(req.headers.cookie)
|
||||||
})
|
})
|
||||||
|
|
||||||
// Send the token along with other cookies which should be preserved.
|
const cookies = [
|
||||||
// Encode one to make sure they are being re-encoded properly.
|
"cookie2=hello",
|
||||||
const value = "hello=there"
|
// Cookies should pass through unchanged, neither encoded nor decoded.
|
||||||
const encodedValue = encodeURIComponent(value)
|
`cookie1=${encodeURIComponent("hello=there")}`,
|
||||||
const resp = await codeServer.fetch(proxyPath + "/cookies", {
|
"cookie3=foo|bar",
|
||||||
headers: {
|
`cookie4=${encodeURIComponent("foo|bar")}`,
|
||||||
cookie: `cookie1=${encodedValue}; code-server-session=${token}; cookie2=hello;`,
|
`cookie5=${encodeURIComponent("bar;baz")}`,
|
||||||
},
|
]
|
||||||
|
|
||||||
|
// Test each slot to ensure the token is found anywhere.
|
||||||
|
for (let i = 0; i <= cookies.length; ++i) {
|
||||||
|
const left = cookies.slice(0, i)
|
||||||
|
const right = cookies.slice(i)
|
||||||
|
const resp = await codeServer.fetch(proxyPath + "/cookies", {
|
||||||
|
headers: {
|
||||||
|
cookie: [...left, `code-server-session=${token}`, ...right].join("; "),
|
||||||
|
},
|
||||||
|
})
|
||||||
|
expect(resp.status).toBe(200)
|
||||||
|
expect(await resp.text()).toBe(cookies.join("; "))
|
||||||
|
}
|
||||||
|
})
|
||||||
|
|
||||||
|
it("should proxy when no cookies", async () => {
|
||||||
|
codeServer = await integration.setup(["--auth=none"])
|
||||||
|
|
||||||
|
e.get("/wsup/cookies", (req, res) => {
|
||||||
|
res.writeHead(HttpCode.Ok, { "Content-Type": "text/plain" })
|
||||||
|
res.end(req.headers.cookie || "no cookies")
|
||||||
})
|
})
|
||||||
|
|
||||||
// The proxied listener should not have printed the code-server token.
|
const resp = await codeServer.fetch(proxyPath + "/cookies")
|
||||||
expect(resp.status).toBe(200)
|
expect(resp.status).toBe(200)
|
||||||
const text = await resp.text()
|
expect(await resp.text()).toBe("no cookies")
|
||||||
expect(text).toBe(`cookie1=${encodedValue}; cookie2=hello`)
|
|
||||||
})
|
})
|
||||||
})
|
})
|
||||||
|
|||||||
Reference in New Issue
Block a user