Skip to content

Commit d477972

Browse files
code-ashercode-asher
authoredMar 3, 2023
Add origin checks to web sockets (#6048)
* Move splitOnFirstEquals to util I will be making use of this to parse the forwarded header. * Type splitOnFirstEquals with two items Also add some test cases. * Check origin header on web sockets * Update changelog with origin check * Fix web sockets not closing with error code
·
v4.104.34.101.0-rc.1
1 parent a47cd81 commit d477972

File tree

17 files changed

+353
-101
lines changed

17 files changed

+353
-101
lines changed
 

‎CHANGELOG.md‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,18 @@ Code v99.99.999
2020
2121
-->
2222

23+
## Unreleased
24+
25+
Code v1.75.1
26+
27+
### Security
28+
29+
Add an origin check to web sockets to prevent a cross-site hijacking attack that
30+
affects those who use older or niche browsers that do not support SameSite
31+
cookies and those who access code-server under a shared domain with other users
32+
on separate sub-domains. The check requires the host header to be set so if you
33+
use a reverse proxy ensure it forwards that information.
34+
2335
## [4.10.0](https://github.com/coder/code-server/releases/tag/v4.10.0) - 2023-02-15
2436

2537
Code v1.75.1

‎src/common/http.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ export enum HttpCode {
44
NotFound = 404,
55
BadRequest = 400,
66
Unauthorized = 401,
7+
Forbidden = 403,
78
LargePayload = 413,
89
ServerError = 500,
910
}

‎src/node/cli.ts‎

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,15 @@ import { promises as fs } from "fs"
33
import { load } from "js-yaml"
44
import * as os from "os"
55
import * as path from "path"
6-
import { canConnect, generateCertificate, generatePassword, humanPath, paths, isNodeJSErrnoException } from "./util"
6+
import {
7+
canConnect,
8+
generateCertificate,
9+
generatePassword,
10+
humanPath,
11+
paths,
12+
isNodeJSErrnoException,
13+
splitOnFirstEquals,
14+
} from "./util"
715

816
const DEFAULT_SOCKET_PATH = path.join(os.tmpdir(), "vscode-ipc")
917

@@ -292,19 +300,6 @@ export const optionDescriptions = (opts: Partial<Options<Required<UserProvidedAr
292300
})
293301
}
294302

295-
export function splitOnFirstEquals(str: string): string[] {
296-
// we use regex instead of "=" to ensure we split at the first
297-
// "=" and return the following substring with it
298-
// important for the hashed-password which looks like this
299-
// $argon2i$v=19$m=4096,t=3,p=1$0qR/o+0t00hsbJFQCKSfdQ$oFcM4rL6o+B7oxpuA4qlXubypbBPsf+8L531U7P9HYY
300-
// 2 means return two items
301-
// Source: https://stackoverflow.com/a/4607799/3015595
302-
// We use the ? to say the the substr after the = is optional
303-
const split = str.split(/=(.+)?/, 2)
304-
305-
return split
306-
}
307-
308303
/**
309304
* Parse arguments into UserProvidedArgs. This should not go beyond checking
310305
* that arguments are valid types and have values when required.

‎src/node/http.ts‎

Lines changed: 74 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,15 @@ import { version as codeServerVersion } from "./constants"
1212
import { Heart } from "./heart"
1313
import { CoderSettings, SettingsProvider } from "./settings"
1414
import { UpdateProvider } from "./update"
15-
import { getPasswordMethod, IsCookieValidArgs, isCookieValid, sanitizeString, escapeHtml, escapeJSON } from "./util"
15+
import {
16+
getPasswordMethod,
17+
IsCookieValidArgs,
18+
isCookieValid,
19+
sanitizeString,
20+
escapeHtml,
21+
escapeJSON,
22+
splitOnFirstEquals,
23+
} from "./util"
1624

1725
/**
1826
* Base options included on every page.
@@ -308,3 +316,68 @@ export const getCookieOptions = (req: express.Request): express.CookieOptions =>
308316
export const self = (req: express.Request): string => {
309317
return normalize(`${req.baseUrl}${req.originalUrl.endsWith("/") ? "/" : ""}`, true)
310318
}
319+
320+
function getFirstHeader(req: http.IncomingMessage, headerName: string): string | undefined {
321+
const val = req.headers[headerName]
322+
return Array.isArray(val) ? val[0] : val
323+
}
324+
325+
/**
326+
* Throw an error if origin checks fail. Call `next` if provided.
327+
*/
328+
export function ensureOrigin(req: express.Request, _?: express.Response, next?: express.NextFunction): void {
329+
if (!authenticateOrigin(req)) {
330+
throw new HttpError("Forbidden", HttpCode.Forbidden)
331+
}
332+
if (next) {
333+
next()
334+
}
335+
}
336+
337+
/**
338+
* Authenticate the request origin against the host.
339+
*/
340+
export function authenticateOrigin(req: express.Request): boolean {
341+
// A missing origin probably means the source is non-browser. Not sure we
342+
// have a use case for this but let it through.
343+
const originRaw = getFirstHeader(req, "origin")
344+
if (!originRaw) {
345+
return true
346+
}
347+
348+
let origin: string
349+
try {
350+
origin = new URL(originRaw).host.trim().toLowerCase()
351+
} catch (error) {
352+
return false // Malformed URL.
353+
}
354+
355+
// Honor Forwarded if present.
356+
const forwardedRaw = getFirstHeader(req, "forwarded")
357+
if (forwardedRaw) {
358+
const parts = forwardedRaw.split(/[;,]/)
359+
for (let i = 0; i < parts.length; ++i) {
360+
const [key, value] = splitOnFirstEquals(parts[i])
361+
if (key.trim().toLowerCase() === "host" && value) {
362+
return origin === value.trim().toLowerCase()
363+
}
364+
}
365+
}
366+
367+
// Honor X-Forwarded-Host if present.
368+
const xHost = getFirstHeader(req, "x-forwarded-host")
369+
if (xHost) {
370+
return origin === xHost.trim().toLowerCase()
371+
}
372+
373+
// A missing host likely means the reverse proxy has not been configured to
374+
// forward the host which means we cannot perform the check. Emit a warning
375+
// so an admin can fix the issue.
376+
const host = getFirstHeader(req, "host")
377+
if (!host) {
378+
logger.warn(`no host headers found; blocking request to ${req.originalUrl}`)
379+
return false
380+
}
381+
382+
return origin === host.trim().toLowerCase()
383+
}

‎src/node/routes/domainProxy.ts‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { Request, Router } from "express"
22
import { HttpCode, HttpError } from "../../common/http"
3-
import { authenticated, ensureAuthenticated, redirect, self } from "../http"
3+
import { authenticated, ensureAuthenticated, ensureOrigin, redirect, self } from "../http"
44
import { proxy } from "../proxy"
55
import { Router as WsRouter } from "../wsRouter"
66

@@ -78,10 +78,8 @@ wsRouter.ws("*", async (req, _, next) => {
7878
if (!port) {
7979
return next()
8080
}
81-
82-
// Must be authenticated to use the proxy.
81+
ensureOrigin(req)
8382
await ensureAuthenticated(req)
84-
8583
proxy.ws(req, req.ws, req.head, {
8684
ignorePath: true,
8785
target: `http://0.0.0.0:${port}${req.originalUrl}`,

‎src/node/routes/errors.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,5 +63,11 @@ export const errorHandler: express.ErrorRequestHandler = async (err, req, res, n
6363

6464
export const wsErrorHandler: express.ErrorRequestHandler = async (err, req, res, next) => {
6565
logger.error(`${err.message} ${err.stack}`)
66-
;(req as WebsocketRequest).ws.end()
66+
let statusCode = 500
67+
if (errorHasStatusCode(err)) {
68+
statusCode = err.statusCode
69+
} else if (errorHasCode(err) && notFoundCodes.includes(err.code)) {
70+
statusCode = HttpCode.NotFound
71+
}
72+
;(req as WebsocketRequest).ws.end(`HTTP/1.1 ${statusCode} ${err.message}\r\n\r\n`)
6773
}

‎src/node/routes/pathProxy.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import * as path from "path"
33
import * as qs from "qs"
44
import * as pluginapi from "../../../typings/pluginapi"
55
import { HttpCode, HttpError } from "../../common/http"
6-
import { authenticated, ensureAuthenticated, redirect, self } from "../http"
6+
import { authenticated, ensureAuthenticated, ensureOrigin, redirect, self } from "../http"
77
import { proxy as _proxy } from "../proxy"
88

99
const getProxyTarget = (req: Request, passthroughPath?: boolean): string => {
@@ -50,6 +50,7 @@ export async function wsProxy(
5050
passthroughPath?: boolean
5151
},
5252
): Promise<void> {
53+
ensureOrigin(req)
5354
await ensureAuthenticated(req)
5455
_proxy.ws(req, req.ws, req.head, {
5556
ignorePath: true,

‎src/node/routes/vscode.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import { WebsocketRequest } from "../../../typings/pluginapi"
77
import { logError } from "../../common/util"
88
import { CodeArgs, toCodeArgs } from "../cli"
99
import { isDevMode } from "../constants"
10-
import { authenticated, ensureAuthenticated, redirect, replaceTemplates, self } from "../http"
10+
import { authenticated, ensureAuthenticated, ensureOrigin, redirect, replaceTemplates, self } from "../http"
1111
import { SocketProxyProvider } from "../socket"
1212
import { isFile, loadAMDModule } from "../util"
1313
import { Router as WsRouter } from "../wsRouter"
@@ -173,7 +173,7 @@ export class CodeServerRouteWrapper {
173173
this.router.get("/", this.ensureCodeServerLoaded, this.$root)
174174
this.router.get("/manifest.json", this.manifest)
175175
this.router.all("*", ensureAuthenticated, this.ensureCodeServerLoaded, this.$proxyRequest)
176-
this._wsRouterWrapper.ws("*", ensureAuthenticated, this.ensureCodeServerLoaded, this.$proxyWebsocket)
176+
this._wsRouterWrapper.ws("*", ensureOrigin, ensureAuthenticated, this.ensureCodeServerLoaded, this.$proxyWebsocket)
177177
}
178178

179179
dispose() {

‎src/node/util.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -541,3 +541,13 @@ export const loadAMDModule = async <T>(amdPath: string, exportName: string): Pro
541541

542542
return module[exportName] as T
543543
}
544+
545+
/**
546+
* Split a string on the first equals. The result will always be an array with
547+
* two items regardless of how many equals there are. The second item will be
548+
* undefined if empty or missing.
549+
*/
550+
export function splitOnFirstEquals(str: string): [string, string | undefined] {
551+
const split = str.split(/=(.+)?/, 2)
552+
return [split[0], split[1]]
553+
}

‎src/node/wsRouter.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,9 @@ export class WebsocketRouter {
3232
/**
3333
* Handle a websocket at this route. Note that websockets are immediately
3434
* paused when they come in.
35+
*
36+
* If the origin header exists it must match the host or the connection will
37+
* be prevented.
3538
*/
3639
public ws(route: expressCore.PathParams, ...handlers: pluginapi.WebSocketHandler[]): void {
3740
this.router.get(

‎test/unit/node/cli.test.ts‎

Lines changed: 0 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import {
1111
readSocketPath,
1212
setDefaults,
1313
shouldOpenInExistingInstance,
14-
splitOnFirstEquals,
1514
toCodeArgs,
1615
optionDescriptions,
1716
options,
@@ -535,31 +534,6 @@ describe("cli", () => {
535534
})
536535
})
537536

538-
describe("splitOnFirstEquals", () => {
539-
it("should split on the first equals", () => {
540-
const testStr = "enabled-proposed-api=test=value"
541-
const actual = splitOnFirstEquals(testStr)
542-
const expected = ["enabled-proposed-api", "test=value"]
543-
expect(actual).toEqual(expect.arrayContaining(expected))
544-
})
545-
it("should split on first equals regardless of multiple equals signs", () => {
546-
const testStr =
547-
"hashed-password=$argon2i$v=19$m=4096,t=3,p=1$0qR/o+0t00hsbJFQCKSfdQ$oFcM4rL6o+B7oxpuA4qlXubypbBPsf+8L531U7P9HYY"
548-
const actual = splitOnFirstEquals(testStr)
549-
const expected = [
550-
"hashed-password",
551-
"$argon2i$v=19$m=4096,t=3,p=1$0qR/o+0t00hsbJFQCKSfdQ$oFcM4rL6o+B7oxpuA4qlXubypbBPsf+8L531U7P9HYY",
552-
]
553-
expect(actual).toEqual(expect.arrayContaining(expected))
554-
})
555-
it("should always return the first element before an equals", () => {
556-
const testStr = "auth="
557-
const actual = splitOnFirstEquals(testStr)
558-
const expected = ["auth"]
559-
expect(actual).toEqual(expect.arrayContaining(expected))
560-
})
561-
})
562-
563537
describe("shouldSpawnCliProcess", () => {
564538
it("should return false if no 'extension' related args passed in", async () => {
565539
const args = {}

‎test/unit/node/http.test.ts‎

Lines changed: 105 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -1,55 +1,118 @@
11
import { getMockReq } from "@jest-mock/express"
2-
import { constructRedirectPath, relativeRoot } from "../../../src/node/http"
2+
import * as http from "../../../src/node/http"
3+
import { mockLogger } from "../../utils/helpers"
34

45
describe("http", () => {
6+
beforeEach(() => {
7+
mockLogger()
8+
})
9+
10+
afterEach(() => {
11+
jest.clearAllMocks()
12+
})
13+
514
it("should construct a relative path to the root", () => {
6-
expect(relativeRoot("/")).toStrictEqual(".")
7-
expect(relativeRoot("/foo")).toStrictEqual(".")
8-
expect(relativeRoot("/foo/")).toStrictEqual("./..")
9-
expect(relativeRoot("/foo/bar ")).toStrictEqual("./..")
10-
expect(relativeRoot("/foo/bar/")).toStrictEqual("./../..")
15+
expect(http.relativeRoot("/")).toStrictEqual(".")
16+
expect(http.relativeRoot("/foo")).toStrictEqual(".")
17+
expect(http.relativeRoot("/foo/")).toStrictEqual("./..")
18+
expect(http.relativeRoot("/foo/bar ")).toStrictEqual("./..")
19+
expect(http.relativeRoot("/foo/bar/")).toStrictEqual("./../..")
1120
})
12-
})
1321

14-
describe("constructRedirectPath", () => {
15-
it("should preserve slashes in queryString so they are human-readable", () => {
16-
const mockReq = getMockReq({
17-
originalUrl: "localhost:8080",
22+
describe("origin", () => {
23+
;[
24+
{
25+
origin: "",
26+
host: "",
27+
expected: true,
28+
},
29+
{
30+
origin: "http://localhost:8080",
31+
host: "",
32+
expected: false,
33+
},
34+
{
35+
origin: "http://localhost:8080",
36+
host: "localhost:8080",
37+
expected: true,
38+
},
39+
{
40+
origin: "http://localhost:8080",
41+
host: "localhost:8081",
42+
expected: false,
43+
},
44+
{
45+
origin: "localhost:8080",
46+
host: "localhost:8080",
47+
expected: false, // Gets parsed as host: localhost and path: 8080.
48+
},
49+
{
50+
origin: "test.org",
51+
host: "localhost:8080",
52+
expected: false, // Parsing fails completely.
53+
},
54+
].forEach((test) => {
55+
;[
56+
["host", test.host],
57+
["x-forwarded-host", test.host],
58+
["forwarded", `for=127.0.0.1, host=${test.host}, proto=http`],
59+
["forwarded", `for=127.0.0.1;proto=http;host=${test.host}`],
60+
["forwarded", `proto=http;host=${test.host}, for=127.0.0.1`],
61+
].forEach(([key, value]) => {
62+
it(`${test.origin} -> [${key}: ${value}]`, () => {
63+
const req = getMockReq({
64+
originalUrl: "localhost:8080",
65+
headers: {
66+
origin: test.origin,
67+
[key]: value,
68+
},
69+
})
70+
expect(http.authenticateOrigin(req)).toBe(test.expected)
71+
})
72+
})
1873
})
19-
const mockQueryParams = { folder: "/Users/jp/dev/coder" }
20-
const mockTo = ""
21-
const actual = constructRedirectPath(mockReq, mockQueryParams, mockTo)
22-
const expected = "./?folder=/Users/jp/dev/coder"
23-
expect(actual).toBe(expected)
2474
})
25-
it("should use an empty string if no query params", () => {
26-
const mockReq = getMockReq({
27-
originalUrl: "localhost:8080",
75+
76+
describe("constructRedirectPath", () => {
77+
it("should preserve slashes in queryString so they are human-readable", () => {
78+
const mockReq = getMockReq({
79+
originalUrl: "localhost:8080",
80+
})
81+
const mockQueryParams = { folder: "/Users/jp/dev/coder" }
82+
const mockTo = ""
83+
const actual = http.constructRedirectPath(mockReq, mockQueryParams, mockTo)
84+
const expected = "./?folder=/Users/jp/dev/coder"
85+
expect(actual).toBe(expected)
2886
})
29-
const mockQueryParams = {}
30-
const mockTo = ""
31-
const actual = constructRedirectPath(mockReq, mockQueryParams, mockTo)
32-
const expected = "./"
33-
expect(actual).toBe(expected)
34-
})
35-
it("should append the 'to' path relative to the originalUrl", () => {
36-
const mockReq = getMockReq({
37-
originalUrl: "localhost:8080",
87+
it("should use an empty string if no query params", () => {
88+
const mockReq = getMockReq({
89+
originalUrl: "localhost:8080",
90+
})
91+
const mockQueryParams = {}
92+
const mockTo = ""
93+
const actual = http.constructRedirectPath(mockReq, mockQueryParams, mockTo)
94+
const expected = "./"
95+
expect(actual).toBe(expected)
3896
})
39-
const mockQueryParams = {}
40-
const mockTo = "vscode"
41-
const actual = constructRedirectPath(mockReq, mockQueryParams, mockTo)
42-
const expected = "./vscode"
43-
expect(actual).toBe(expected)
44-
})
45-
it("should append append queryParams after 'to' path", () => {
46-
const mockReq = getMockReq({
47-
originalUrl: "localhost:8080",
97+
it("should append the 'to' path relative to the originalUrl", () => {
98+
const mockReq = getMockReq({
99+
originalUrl: "localhost:8080",
100+
})
101+
const mockQueryParams = {}
102+
const mockTo = "vscode"
103+
const actual = http.constructRedirectPath(mockReq, mockQueryParams, mockTo)
104+
const expected = "./vscode"
105+
expect(actual).toBe(expected)
106+
})
107+
it("should append append queryParams after 'to' path", () => {
108+
const mockReq = getMockReq({
109+
originalUrl: "localhost:8080",
110+
})
111+
const mockQueryParams = { folder: "/Users/jp/dev/coder" }
112+
const mockTo = "vscode"
113+
const actual = http.constructRedirectPath(mockReq, mockQueryParams, mockTo)
114+
const expected = "./vscode?folder=/Users/jp/dev/coder"
115+
expect(actual).toBe(expected)
48116
})
49-
const mockQueryParams = { folder: "/Users/jp/dev/coder" }
50-
const mockTo = "vscode"
51-
const actual = constructRedirectPath(mockReq, mockQueryParams, mockTo)
52-
const expected = "./vscode?folder=/Users/jp/dev/coder"
53-
expect(actual).toBe(expected)
54117
})
55118
})

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

Lines changed: 42 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,21 +4,26 @@ import * as http from "http"
44
import nodeFetch from "node-fetch"
55
import { HttpCode } from "../../../src/common/http"
66
import { proxy } from "../../../src/node/proxy"
7-
import { getAvailablePort } from "../../utils/helpers"
7+
import { wss, Router as WsRouter } from "../../../src/node/wsRouter"
8+
import { getAvailablePort, mockLogger } from "../../utils/helpers"
89
import * as httpserver from "../../utils/httpserver"
910
import * as integration from "../../utils/integration"
1011

1112
describe("proxy", () => {
1213
const nhooyrDevServer = new httpserver.HttpServer()
14+
const wsApp = express.default()
15+
const wsRouter = WsRouter()
1316
let codeServer: httpserver.HttpServer | undefined
1417
let proxyPath: string
1518
let absProxyPath: string
1619
let e: express.Express
1720

1821
beforeAll(async () => {
22+
wsApp.use("/", wsRouter.router)
1923
await nhooyrDevServer.listen((req, res) => {
2024
e(req, res)
2125
})
26+
nhooyrDevServer.listenUpgrade(wsApp)
2227
proxyPath = `/proxy/${nhooyrDevServer.port()}/wsup`
2328
absProxyPath = proxyPath.replace("/proxy/", "/absproxy/")
2429
})
@@ -29,13 +34,15 @@ describe("proxy", () => {
2934

3035
beforeEach(() => {
3136
e = express.default()
37+
mockLogger()
3238
})
3339

3440
afterEach(async () => {
3541
if (codeServer) {
3642
await codeServer.dispose()
3743
codeServer = undefined
3844
}
45+
jest.clearAllMocks()
3946
})
4047

4148
it("should rewrite the base path", async () => {
@@ -151,6 +158,35 @@ describe("proxy", () => {
151158
expect(resp.status).toBe(500)
152159
expect(resp.statusText).toMatch("Internal Server Error")
153160
})
161+
162+
it("should pass origin check", async () => {
163+
wsRouter.ws("/wsup", async (req) => {
164+
wss.handleUpgrade(req, req.ws, req.head, (ws) => {
165+
ws.send("hello")
166+
req.ws.resume()
167+
})
168+
})
169+
codeServer = await integration.setup(["--auth=none"], "")
170+
const ws = await codeServer.wsWait(proxyPath, {
171+
headers: {
172+
host: "localhost:8080",
173+
origin: "https://localhost:8080",
174+
},
175+
})
176+
ws.terminate()
177+
})
178+
179+
it("should fail origin check", async () => {
180+
await expect(async () => {
181+
codeServer = await integration.setup(["--auth=none"], "")
182+
await codeServer.wsWait(proxyPath, {
183+
headers: {
184+
host: "localhost:8080",
185+
origin: "https://evil.org",
186+
},
187+
})
188+
}).rejects.toThrow()
189+
})
154190
})
155191

156192
// NOTE@jsjoeio
@@ -190,18 +226,18 @@ describe("proxy (standalone)", () => {
190226
})
191227

192228
// Start both servers
193-
await proxyTarget.listen(PROXY_PORT)
194-
await testServer.listen(PORT)
229+
proxyTarget.listen(PROXY_PORT)
230+
testServer.listen(PORT)
195231
})
196232

197233
afterEach(async () => {
198-
await testServer.close()
199-
await proxyTarget.close()
234+
testServer.close()
235+
proxyTarget.close()
200236
})
201237

202238
it("should return a 500 when proxy target errors ", async () => {
203239
// Close the proxy target so that proxy errors
204-
await proxyTarget.close()
240+
proxyTarget.close()
205241
const errorResp = await nodeFetch(`${URL}/error`)
206242
expect(errorResp.status).toBe(HttpCode.ServerError)
207243
expect(errorResp.statusText).toBe("Internal Server Error")

‎test/unit/node/routes/health.test.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,9 @@ describe("health", () => {
2323
codeServer = await integration.setup(["--auth=none"], "")
2424
const ws = codeServer.ws("/healthz")
2525
const message = await new Promise((resolve, reject) => {
26-
ws.on("error", console.error)
26+
ws.on("error", (err) => {
27+
console.error("[healthz]", err)
28+
})
2729
ws.on("message", (message) => {
2830
try {
2931
const j = JSON.parse(message.toString())

‎test/unit/node/routes/vscode.test.ts‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
import * as httpserver from "../../../utils/httpserver"
2+
import * as integration from "../../../utils/integration"
3+
import { mockLogger } from "../../../utils/helpers"
4+
5+
describe("vscode", () => {
6+
let codeServer: httpserver.HttpServer | undefined
7+
beforeEach(() => {
8+
mockLogger()
9+
})
10+
11+
afterEach(async () => {
12+
if (codeServer) {
13+
await codeServer.dispose()
14+
codeServer = undefined
15+
}
16+
jest.clearAllMocks()
17+
})
18+
19+
it("should fail origin check", async () => {
20+
await expect(async () => {
21+
codeServer = await integration.setup(["--auth=none"], "")
22+
await codeServer.wsWait("/vscode", {
23+
headers: {
24+
host: "localhost:8080",
25+
origin: "https://evil.org",
26+
},
27+
})
28+
}).rejects.toThrow()
29+
})
30+
})

‎test/unit/node/util.test.ts‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -601,3 +601,41 @@ describe("constructOpenOptions", () => {
601601
expect(urlSearch).toBe("?q=^&test")
602602
})
603603
})
604+
605+
describe("splitOnFirstEquals", () => {
606+
const tests = [
607+
{
608+
name: "empty",
609+
key: "",
610+
value: "",
611+
},
612+
{
613+
name: "split on first equals",
614+
key: "foo",
615+
value: "bar",
616+
},
617+
{
618+
name: "split on first equals even with multiple equals",
619+
key: "foo",
620+
value: "bar=baz",
621+
},
622+
{
623+
name: "split with empty value",
624+
key: "foo",
625+
value: "",
626+
},
627+
{
628+
name: "split with no value",
629+
key: "foo",
630+
value: undefined,
631+
},
632+
]
633+
tests.forEach((test) => {
634+
it("should ${test.name}", () => {
635+
const input = test.key && typeof test.value !== "undefined" ? `${test.key}=${test.value}` : test.key
636+
const [key, value] = util.splitOnFirstEquals(input)
637+
expect(key).toStrictEqual(test.key)
638+
expect(value).toStrictEqual(test.value || undefined)
639+
})
640+
})
641+
})

‎test/utils/httpserver.ts‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import { Disposable } from "../../src/common/emitter"
77
import * as util from "../../src/common/util"
88
import { ensureAddress } from "../../src/node/app"
99
import { disposer } from "../../src/node/http"
10-
1110
import { handleUpgrade } from "../../src/node/wsRouter"
1211

1312
// Perhaps an abstraction similar to this should be used in app.ts as well.
@@ -76,14 +75,25 @@ export class HttpServer {
7675
/**
7776
* Open a websocket against the request path.
7877
*/
79-
public ws(requestPath: string): Websocket {
78+
public ws(requestPath: string, options?: Websocket.ClientOptions): Websocket {
8079
const address = ensureAddress(this.hs, "ws")
8180
if (typeof address === "string") {
8281
throw new Error("Cannot open websocket to socket path")
8382
}
8483
address.pathname = requestPath
8584

86-
return new Websocket(address.toString())
85+
return new Websocket(address.toString(), options)
86+
}
87+
88+
/**
89+
* Open a websocket and wait for it to fully open.
90+
*/
91+
public wsWait(requestPath: string, options?: Websocket.ClientOptions): Promise<Websocket> {
92+
const ws = this.ws(requestPath, options)
93+
return new Promise<Websocket>((resolve, reject) => {
94+
ws.on("error", (err) => reject(err))
95+
ws.on("open", () => resolve(ws))
96+
})
8797
}
8898

8999
public port(): number {

0 commit comments

Comments
 (0)
Please sign in to comment.