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

CHANGELOG.md

@@ -20,6 +20,30 @@ Code v99.99.999
 
 -->
 
+## Unreleased
+
+Code v1.75.1
+
+### Security
+
+Add an origin check to web sockets to prevent a cross-site hijacking attack that
+affects those who use older or niche browsers that do not support SameSite
+cookies and those who access code-server under a shared domain with other users
+on separate sub-domains. The check requires the host header to be set so if you
+use a reverse proxy ensure it forwards that information.
+
+## [4.10.0](https://github.com/coder/code-server/releases/tag/v4.10.0) - 2023-02-15
+
+Code v1.75.1
+
+### Changed
+
+- Updated to Code 1.75.1
+
+### Removed
+
+- Removed `--link` (was deprecated over thirteen months ago in 4.0.1).
+
 ## [4.9.1](https://github.com/coder/code-server/releases/tag/v4.9.1) - 2022-12-15
 
 Code v1.73.1

ci/build/build-code-server.sh

@@ -14,22 +14,6 @@ main() {
     sed -i.bak "1s;^;#!/usr/bin/env node\n;" out/node/entry.js && rm out/node/entry.js.bak
     chmod +x out/node/entry.js
   fi
-
-  # for arch; we do not use OS from lib.sh and get our own.
-  # lib.sh normalizes macos to darwin - but cloud-agent's binaries do not
-  source ./ci/lib.sh
-  OS="$(uname | tr '[:upper:]' '[:lower:]')"
-
-  mkdir -p ./lib
-
-  if ! [ -f ./lib/coder-cloud-agent ]; then
-    echo "Downloading the cloud agent..."
-
-    set +e
-    curl -fsSL "https://github.com/coder/cloud-agent/releases/latest/download/cloud-agent-$OS-$ARCH" -o ./lib/coder-cloud-agent
-    chmod +x ./lib/coder-cloud-agent
-    set -e
-  fi
 }
 
 main "$@"

ci/build/build-release.sh

@@ -63,8 +63,6 @@ EOF
 
   if [ "$KEEP_MODULES" = 1 ]; then
     rsync node_modules/ "$RELEASE_PATH/node_modules"
-    mkdir -p "$RELEASE_PATH/lib"
-    rsync ./lib/coder-cloud-agent "$RELEASE_PATH/lib"
   fi
 }
 

ci/build/npm-postinstall.sh

@@ -1,18 +1,8 @@
 #!/usr/bin/env sh
 set -eu
 
-# Copied from ../lib.sh.
+# Copied from ../lib.sh except we do not rename Darwin and we do not need to
-arch() {
+# detect Alpine.
-  cpu="$(uname -m)"
-  case "$cpu" in
-    aarch64) cpu=arm64 ;;
-    x86_64) cpu=amd64 ;;
-  esac
-  echo "$cpu"
-}
-
-# Copied from ../lib.sh except we do not rename Darwin since the cloud agent
-# uses "darwin" in the release names and we do not need to detect Alpine.
 os() {
   osname=$(uname | tr '[:upper:]' '[:lower:]')
   case $osname in
@@ -61,7 +51,6 @@ symlink_bin_script() {
   cd "$oldpwd"
 }
 
-ARCH="${NPM_CONFIG_ARCH:-$(arch)}"
 OS="$(os)"
 
 # This is due to an upstream issue with RHEL7/CentOS 7 comptability with node-argon2
@@ -102,14 +91,6 @@ main() {
     ;;
   esac
 
-  mkdir -p ./lib
-
-  if curl -fsSL "https://github.com/coder/cloud-agent/releases/latest/download/cloud-agent-$OS-$ARCH" -o ./lib/coder-cloud-agent; then
-    chmod +x ./lib/coder-cloud-agent
-  else
-    echo "Failed to download cloud agent; --link will not work"
-  fi
-
   if ! vscode_install; then
     echo "You may not have the required dependencies to build the native modules."
     echo "Please see https://github.com/coder/code-server/blob/main/docs/npm.md"

ci/dev/watch.ts

@@ -31,11 +31,12 @@ class Watcher {
     // Pass CLI args, save for `node` and the initial script name.
     const args = process.argv.slice(2)
     this.webServer = spawn("node", [path.join(this.rootPath, "out/node/entry.js"), ...args])
+    onLine(this.webServer, (line) => console.log("[code-server]", line))
     const { pid } = this.webServer
 
-    this.webServer.on("exit", () => console.log("[Code Server]", `Web process ${pid} exited`))
+    this.webServer.on("exit", () => console.log("[code-server]", `Web process ${pid} exited`))
 
-    console.log("\n[Code Server]", `Spawned web server process ${pid}`)
+    console.log("\n[code-server]", `Spawned web server process ${pid}`)
   }
 
   //#endregion
@@ -82,10 +83,10 @@ class Watcher {
   private parseVSCodeLine: OnLineCallback = (strippedLine, originalLine) => {
     if (!strippedLine.length) return
 
-    console.log("[VS Code]", originalLine)
+    console.log("[Code OSS]", originalLine)
 
     if (strippedLine.includes("Finished compilation with")) {
-      console.log("[VS Code] ✨ Finished compiling! ✨", "(Refresh your web browser ♻️)")
+      console.log("[Code OSS] ✨ Finished compiling! ✨", "(Refresh your web browser ♻️)")
       this.reloadWebServer()
     }
   }
@@ -93,10 +94,10 @@ class Watcher {
   private parseCodeServerLine: OnLineCallback = (strippedLine, originalLine) => {
     if (!strippedLine.length) return
 
-    console.log("[Compiler][Code Server]", originalLine)
+    console.log("[Compiler][code-server]", originalLine)
 
     if (strippedLine.includes("Watching for file changes")) {
-      console.log("[Compiler][Code Server]", "Finished compiling!", "(Refresh your web browser ♻️)")
+      console.log("[Compiler][code-server]", "Finished compiling!", "(Refresh your web browser ♻️)")
       this.reloadWebServer()
     }
   }

ci/helm-chart/Chart.yaml

@@ -15,9 +15,9 @@ type: application
 # This is the chart version. This version number should be incremented each time you make changes
 # to the chart and its templates, including the app version.
 # Versions are expected to follow Semantic Versioning (https://semver.org/)
-version: 3.4.1
+version: 3.6.0
 
 # This is the version number of the application being deployed. This version number should be
 # incremented each time you make changes to the application. Versions are not expected to
 # follow Semantic Versioning. They should reflect the version the application is using.
-appVersion: 4.9.1
+appVersion: 4.10.0

ci/helm-chart/templates/deployment.yaml

@@ -102,6 +102,7 @@ spec:
           {{- range .Values.extraSecretMounts }}
           - name: {{ .name }}
             mountPath: {{ .mountPath }}
+            subPath: {{ .subPath | default "" }}
             readOnly: {{ .readOnly }}
           {{- end }}
           {{- range .Values.extraVolumeMounts }}
@@ -114,6 +115,11 @@ spec:
             - name: http
               containerPort: 8080
               protocol: TCP
+          {{- range .Values.extraPorts }}
+            - name: {{ .name }}
+              containerPort: {{ .port }}
+              protocol: {{ .protocol }}
+          {{- end }}
           livenessProbe:
             httpGet:
               path: /

ci/helm-chart/templates/service.yaml

@@ -14,6 +14,12 @@ spec:
       targetPort: http
       protocol: TCP
       name: http
+  {{- range .Values.extraPorts }}
+    - port: {{ .port }}
+      targetPort: {{ .port }}
+      protocol: {{ .protocol }}
+      name: {{ .name }}
+  {{- end }}
   selector:
     app.kubernetes.io/name: {{ include "code-server.name" . }}
     app.kubernetes.io/instance: {{ .Release.Name }}

ci/helm-chart/values.yaml

@@ -179,6 +179,7 @@ extraInitContainers: |
 extraSecretMounts: []
   # - name: secret-files
   #   mountPath: /etc/secrets
+  #   subPath: private.key # (optional)
   #   secretName: code-server-secret-files
   #   readOnly: true
 
@@ -196,3 +197,8 @@ extraConfigmapMounts: []
   #   subPath: certificates.crt # (optional)
   #   configMap: certs-configmap
   #   readOnly: true
+
+extraPorts: []
+  # - name: minecraft
+  #   port: 25565
+  #   protocol: tcp

docs/FAQ.md

@@ -390,19 +390,13 @@ Theia doesn't allow you to reuse your existing VS Code config.
 ## What's the difference between code-server and OpenVSCode-Server?
 
 code-server and OpenVSCode-Server both allow you to access VS Code via a
-browser. The two projects also use their own [forks of VS Code](https://github.com/coder/vscode) to
+browser. OpenVSCode-Server is a direct fork of VS Code with changes comitted
-leverage modern VS Code APIs and stay up to date with the upsteam version.
+directly while code-server pulls VS Code in via a submodule and makes changes
+via patch files.
 
-However, OpenVSCode-Server is scoped at only making VS Code available in the web browser.
+However, OpenVSCode-Server is scoped at only making VS Code available as-is in
-code-server includes some other features:
+the web browser. code-server contains additional changes to make the self-hosted
-
+experience better (see the next section for details).
-- password auth
-- proxy web ports
-- certificate support
-- plugin API
-- settings sync (coming soon)
-
-For more details, see [this discussion post](https://github.com/coder/code-server/discussions/4267#discussioncomment-1411583).
 
 ## What's the difference between code-server and GitHub Codespaces?
 
@@ -410,8 +404,24 @@ Both code-server and GitHub Codespaces allow you to access VS Code via a
 browser. GitHub Codespaces, however, is a closed-source, paid service offered by
 GitHub and Microsoft.
 
-On the other hand, code-server is self-hosted, free, open-source, and
+On the other hand, code-server is self-hosted, free, open-source, and can be run
-can be run on any machine with few limitations.
+on any machine with few limitations.
+
+Specific changes include:
+
+- Password authentication
+- The ability to host at sub-paths
+- Self-contained web views that do not call out to Microsoft's servers
+- The ability to use your own marketplace and collect your own telemetry
+- Built-in proxy for accessing ports on the remote machine integrated into
+  VS Code's ports panel
+- Wrapper process that spawns VS Code on-demand and has a separate CLI
+- Notification when updates are available
+- [Some other things](https://github.com/coder/code-server/tree/main/patches)
+
+Some of these changes appear very unlikely to ever be adopted by Microsoft.
+Some may make their way upstream, further closing the gap, but at the moment it
+looks like there will always be some subtle differences.
 
 ## Does code-server have any security login validation?
 

docs/MAINTAINING.md

@@ -142,24 +142,28 @@ changelog](https://github.com/emacs-mirror/emacs/blob/master/etc/NEWS).
 
 ### Publishing a release
 
-1. Go to GitHub Actions > Draft release > Run workflow off commit you want to
+1. Go to GitHub Actions > Draft release > Run workflow on the commit you want to
-   release. CI will automatically upload the artifacts to the release. Make sure CI
+   release. Make sure CI has finished the build workflow on that commit or this
-   has finished on that commit.
+   will fail.
-1. CI will automatically grab the
+2. CI will automatically grab the build artifact on that commit, inject the
-   artifacts, publish the NPM package from `npm-package`, and publish the Docker
+   version into the `package.json`, put together platform-specific packages, and
-   Hub image from `release-images`.
+   upload those packages to a draft release.
-1. Publish release.
+3. Summarize the major changes in the `CHANGELOG.md`.
-1. After, create a new branch called `release/v0.0.0` (replace 0s with actual version aka v4.5.0)
+4. Copy the relevant changelog section to the release then publish it.
-1. Summarize the major changes in the `CHANGELOG.md`
+5. CI will automatically publish the NPM package, Docker image, and update
-1. Bump chart version in `Chart.yaml`.
+   Homebrew using the published release assets.
+6. Bump the chart version in `Chart.yaml` and merge in the changelog updates.
 
 #### Release Candidates
 
-We prefer to do release candidates so the community can test things before a full-blown release. To do this follow the same steps as above but:
+We prefer to do release candidates so the community can test things before a
+full-blown release. To do this follow the same steps as above but:
 
-1. Only bump version in `package.json`
+1. Add a `-rc.<number>` suffix to the version.
-1. use `0.0.0-rc.0`
+2. When you publish the release select "pre-release". CI will not automatically
-1. When you publish the release, select "pre-release"
+   publish pre-releases.
+3. Do not update the chart version or merge in the changelog until the final
+   release.
 
 #### AUR
 

docs/install.md

@@ -297,9 +297,9 @@ You can install code-server using the [Helm package manager](https://coder.com/d
 
 ## Windows
 
-We currently [do not publish Windows releases](https://github.com/coder/code-server/issues/1397). We recommend installing code-server onto Windows with [`npm`](#npm).
+We currently [do not publish Windows
-
+releases](https://github.com/coder/code-server/issues/1397). We recommend
-> Note: You will also need to [build coder/cloud-agent manually](https://github.com/coder/cloud-agent/issues/17) if you would like to use `code-server --link` on Windows.
+installing code-server onto Windows with [`npm`](#npm).
 
 ## Raspberry Pi
 

docs/link.md

@@ -1,11 +0,0 @@
-# code-server --link
-
-> Note: This feature is no longer recommended due to instability. Stay tuned for a revised version.
-
-Run code-server with the flag `--link` and you'll get TLS, authentication, and a dedicated URL
-for accessing your IDE out of the box.
-
-```console
-$ code-server --link
-Proxying code-server, you can access your IDE at https://example.coder.co
-```

docs/manifest.json

@@ -42,11 +42,6 @@
           "description": "How to run code-server in Coder",
           "path": "./coder.md"
         },
-        {
-          "title": "--link",
-          "description": "How to run code-server --link",
-          "path": "./link.md"
-        },
         {
           "title": "iPad",
           "description": "How to access your code-server installation using an iPad.",

install.sh

@@ -46,7 +46,7 @@ Usage:
       Sets the prefix used by standalone release archives. Defaults to ~/.local
       The release is unarchived into ~/.local/lib/code-server-X.X.X
       and the binary symlinked into ~/.local/bin/code-server
-      To install system wide pass ---prefix=/usr/local
+      To install system wide pass --prefix=/usr/local
 
   --rsh <bin>
       Specifies the remote shell for remote installation. Defaults to ssh.

patches/webview.diff

@@ -101,7 +101,7 @@ Index: code-server/lib/vscode/src/vs/workbench/contrib/webview/browser/pre/index
 ===================================================================
 --- code-server.orig/lib/vscode/src/vs/workbench/contrib/webview/browser/pre/index-no-csp.html
 +++ code-server/lib/vscode/src/vs/workbench/contrib/webview/browser/pre/index-no-csp.html
-@@ -322,6 +322,12 @@
+@@ -324,6 +324,12 @@
  
  				const hostname = location.hostname;
  

src/common/http.ts

@@ -4,6 +4,7 @@ export enum HttpCode {
   NotFound = 404,
   BadRequest = 400,
   Unauthorized = 401,
+  Forbidden = 403,
   LargePayload = 413,
   ServerError = 500,
 }

src/node/cli.ts

@@ -3,7 +3,15 @@ import { promises as fs } from "fs"
 import { load } from "js-yaml"
 import * as os from "os"
 import * as path from "path"
-import { canConnect, generateCertificate, generatePassword, humanPath, paths, isNodeJSErrnoException } from "./util"
+import {
+  canConnect,
+  generateCertificate,
+  generatePassword,
+  humanPath,
+  paths,
+  isNodeJSErrnoException,
+  splitOnFirstEquals,
+} from "./util"
 
 const DEFAULT_SOCKET_PATH = path.join(os.tmpdir(), "vscode-ipc")
 
@@ -84,7 +92,6 @@ export interface UserProvidedArgs extends UserProvidedCodeArgs {
   "reuse-window"?: boolean
   "new-window"?: boolean
   "ignore-last-opened"?: boolean
-  link?: OptionalString
   verbose?: boolean
   "app-name"?: string
   "welcome-text"?: string
@@ -262,15 +269,6 @@ export const options: Options<Required<UserProvidedArgs>> = {
     short: "w",
     description: "Text to show on login page",
   },
-  link: {
-    type: OptionalString,
-    description: `
-      Securely bind code-server via our cloud service with the passed name. You'll get a URL like
-      https://hostname-username.coder.co at which you can easily access your code-server instance.
-      Authorization is done via GitHub.
-    `,
-    deprecated: true,
-  },
 }
 
 export const optionDescriptions = (opts: Partial<Options<Required<UserProvidedArgs>>> = options): string[] => {
@@ -302,19 +300,6 @@ export const optionDescriptions = (opts: Partial<Options<Required<UserProvidedAr
   })
 }
 
-export function splitOnFirstEquals(str: string): string[] {
-  // we use regex instead of "=" to ensure we split at the first
-  // "=" and return the following substring with it
-  // important for the hashed-password which looks like this
-  // $argon2i$v=19$m=4096,t=3,p=1$0qR/o+0t00hsbJFQCKSfdQ$oFcM4rL6o+B7oxpuA4qlXubypbBPsf+8L531U7P9HYY
-  // 2 means return two items
-  // Source: https://stackoverflow.com/a/4607799/3015595
-  // We use the ? to say the the substr after the = is optional
-  const split = str.split(/=(.+)?/, 2)
-
-  return split
-}
-
 /**
  * Parse arguments into UserProvidedArgs.  This should not go beyond checking
  * that arguments are valid types and have values when required.
@@ -547,17 +532,6 @@ export async function setDefaults(cliArgs: UserProvidedArgs, configArgs?: Config
   args.host = addr.host
   args.port = addr.port
 
-  // If we're being exposed to the cloud, we listen on a random address and
-  // disable auth.
-  if (args.link) {
-    args.host = "localhost"
-    args.port = 0
-    args.socket = undefined
-    args["socket-mode"] = undefined
-    args.cert = undefined
-    args.auth = AuthType.None
-  }
-
   if (args.cert && !args.cert.value) {
     const { cert, certKey } = await generateCertificate(args["cert-host"] || "localhost")
     args.cert = {

src/node/coder_cloud.ts

@@ -1,43 +0,0 @@
-import { logger } from "@coder/logger"
-import { spawn } from "child_process"
-import path from "path"
-import split2 from "split2"
-
-// https://github.com/coder/coder-cloud
-const coderCloudAgent = path.resolve(__dirname, "../../lib/coder-cloud-agent")
-
-function runAgent(...args: string[]): Promise<void> {
-  logger.debug(`running agent with ${args}`)
-
-  const agent = spawn(coderCloudAgent, args, {
-    stdio: ["inherit", "inherit", "pipe"],
-  })
-
-  agent.stderr.pipe(split2()).on("data", (line) => {
-    line = line.replace(/^[0-9-]+ [0-9:]+ [^ ]+\t/, "")
-    logger.info(line)
-  })
-
-  return new Promise((res, rej) => {
-    agent.on("error", rej)
-
-    agent.on("close", (code) => {
-      if (code !== 0) {
-        rej({
-          message: `--link agent exited with ${code}`,
-        })
-        return
-      }
-      res()
-    })
-  })
-}
-
-export function coderCloudBind(address: URL | string, serverName = ""): Promise<void> {
-  if (typeof address === "string") {
-    throw new Error("Cannot link socket paths")
-  }
-
-  // Address needs to be in hostname:port format without the protocol.
-  return runAgent("bind", `--code-server-addr=${address.host}`, serverName)
-}

src/node/http.ts

@@ -12,7 +12,15 @@ import { version as codeServerVersion } from "./constants"
 import { Heart } from "./heart"
 import { CoderSettings, SettingsProvider } from "./settings"
 import { UpdateProvider } from "./update"
-import { getPasswordMethod, IsCookieValidArgs, isCookieValid, sanitizeString, escapeHtml, escapeJSON } from "./util"
+import {
+  getPasswordMethod,
+  IsCookieValidArgs,
+  isCookieValid,
+  sanitizeString,
+  escapeHtml,
+  escapeJSON,
+  splitOnFirstEquals,
+} from "./util"
 
 /**
  * Base options included on every page.
@@ -308,3 +316,68 @@ export const getCookieOptions = (req: express.Request): express.CookieOptions =>
 export const self = (req: express.Request): string => {
   return normalize(`${req.baseUrl}${req.originalUrl.endsWith("/") ? "/" : ""}`, true)
 }
+
+function getFirstHeader(req: http.IncomingMessage, headerName: string): string | undefined {
+  const val = req.headers[headerName]
+  return Array.isArray(val) ? val[0] : val
+}
+
+/**
+ * Throw an error if origin checks fail. Call `next` if provided.
+ */
+export function ensureOrigin(req: express.Request, _?: express.Response, next?: express.NextFunction): void {
+  if (!authenticateOrigin(req)) {
+    throw new HttpError("Forbidden", HttpCode.Forbidden)
+  }
+  if (next) {
+    next()
+  }
+}
+
+/**
+ * Authenticate the request origin against the host.
+ */
+export function authenticateOrigin(req: express.Request): boolean {
+  // A missing origin probably means the source is non-browser.  Not sure we
+  // have a use case for this but let it through.
+  const originRaw = getFirstHeader(req, "origin")
+  if (!originRaw) {
+    return true
+  }
+
+  let origin: string
+  try {
+    origin = new URL(originRaw).host.trim().toLowerCase()
+  } catch (error) {
+    return false // Malformed URL.
+  }
+
+  // Honor Forwarded if present.
+  const forwardedRaw = getFirstHeader(req, "forwarded")
+  if (forwardedRaw) {
+    const parts = forwardedRaw.split(/[;,]/)
+    for (let i = 0; i < parts.length; ++i) {
+      const [key, value] = splitOnFirstEquals(parts[i])
+      if (key.trim().toLowerCase() === "host" && value) {
+        return origin === value.trim().toLowerCase()
+      }
+    }
+  }
+
+  // Honor X-Forwarded-Host if present.
+  const xHost = getFirstHeader(req, "x-forwarded-host")
+  if (xHost) {
+    return origin === xHost.trim().toLowerCase()
+  }
+
+  // A missing host likely means the reverse proxy has not been configured to
+  // forward the host which means we cannot perform the check.  Emit a warning
+  // so an admin can fix the issue.
+  const host = getFirstHeader(req, "host")
+  if (!host) {
+    logger.warn(`no host headers found; blocking request to ${req.originalUrl}`)
+    return false
+  }
+
+  return origin === host.trim().toLowerCase()
+}

src/node/main.ts

@@ -6,7 +6,6 @@ import { Disposable } from "../common/emitter"
 import { plural } from "../common/util"
 import { createApp, ensureAddress } from "./app"
 import { AuthType, DefaultedArgs, Feature, SpawnCodeCli, toCodeArgs, UserProvidedArgs } from "./cli"
-import { coderCloudBind } from "./coder_cloud"
 import { commit, version } from "./constants"
 import { register } from "./routes"
 import { humanPath, isDirectory, loadAMDModule, open } from "./util"
@@ -127,11 +126,7 @@ export const runCodeServer = async (
   const disposeRoutes = await register(app, args)
 
   logger.info(`Using config file ${humanPath(os.homedir(), args.config)}`)
-  logger.info(
+  logger.info(`${protocol.toUpperCase()} server listening on ${serverAddress.toString()}`)
-    `${protocol.toUpperCase()} server listening on ${serverAddress.toString()} ${
-      args.link ? "(randomized by --link)" : ""
-    }`,
-  )
 
   if (args.auth === AuthType.Password) {
     logger.info("  - Authentication is enabled")
@@ -143,13 +138,13 @@ export const runCodeServer = async (
       logger.info(`    - Using password from ${humanPath(os.homedir(), args.config)}`)
     }
   } else {
-    logger.info(`  - Authentication is disabled ${args.link ? "(disabled by --link)" : ""}`)
+    logger.info("  - Authentication is disabled")
   }
 
   if (args.cert) {
     logger.info(`  - Using certificate for HTTPS: ${humanPath(os.homedir(), args.cert.value)}`)
   } else {
-    logger.info(`  - Not serving HTTPS ${args.link ? "(disabled by --link)" : ""}`)
+    logger.info("  - Not serving HTTPS")
   }
 
   if (args["proxy-domain"].length > 0) {
@@ -157,11 +152,6 @@ export const runCodeServer = async (
     args["proxy-domain"].forEach((domain) => logger.info(`    - *.${domain}`))
   }
 
-  if (args.link) {
-    await coderCloudBind(serverAddress, args.link.value)
-    logger.info("  - Connected to cloud agent")
-  }
-
   if (args.enable && args.enable.length > 0) {
     logger.info("Enabling the following experimental features:")
     args.enable.forEach((feature) => {

src/node/routes/domainProxy.ts

@@ -1,6 +1,6 @@
 import { Request, Router } from "express"
 import { HttpCode, HttpError } from "../../common/http"
-import { authenticated, ensureAuthenticated, redirect, self } from "../http"
+import { authenticated, ensureAuthenticated, ensureOrigin, redirect, self } from "../http"
 import { proxy } from "../proxy"
 import { Router as WsRouter } from "../wsRouter"
 
@@ -78,10 +78,8 @@ wsRouter.ws("*", async (req, _, next) => {
   if (!port) {
     return next()
   }
-
+  ensureOrigin(req)
-  // Must be authenticated to use the proxy.
   await ensureAuthenticated(req)
-
   proxy.ws(req, req.ws, req.head, {
     ignorePath: true,
     target: `http://0.0.0.0:${port}${req.originalUrl}`,

src/node/routes/errors.ts

@@ -63,5 +63,11 @@ export const errorHandler: express.ErrorRequestHandler = async (err, req, res, n
 
 export const wsErrorHandler: express.ErrorRequestHandler = async (err, req, res, next) => {
   logger.error(`${err.message} ${err.stack}`)
-  ;(req as WebsocketRequest).ws.end()
+  let statusCode = 500
+  if (errorHasStatusCode(err)) {
+    statusCode = err.statusCode
+  } else if (errorHasCode(err) && notFoundCodes.includes(err.code)) {
+    statusCode = HttpCode.NotFound
+  }
+  ;(req as WebsocketRequest).ws.end(`HTTP/1.1 ${statusCode} ${err.message}\r\n\r\n`)
 }

src/node/routes/index.ts

@@ -33,7 +33,15 @@ import { CodeServerRouteWrapper } from "./vscode"
 export const register = async (app: App, args: DefaultedArgs): Promise<Disposable["dispose"]> => {
   const heart = new Heart(path.join(paths.data, "heartbeat"), async () => {
     return new Promise((resolve, reject) => {
+      // getConnections appears to not call the callback when there are no more
+      // connections.  Feels like it must be a bug?  For now add a timer to make
+      // sure we eventually resolve.
+      const timer = setTimeout(() => {
+        logger.debug("Node failed to respond with connections; assuming zero")
+        resolve(false)
+      }, 5000)
       app.server.getConnections((error, count) => {
+        clearTimeout(timer)
         if (error) {
           return reject(error)
         }

src/node/routes/pathProxy.ts

@@ -3,7 +3,7 @@ import * as path from "path"
 import * as qs from "qs"
 import * as pluginapi from "../../../typings/pluginapi"
 import { HttpCode, HttpError } from "../../common/http"
-import { authenticated, ensureAuthenticated, redirect, self } from "../http"
+import { authenticated, ensureAuthenticated, ensureOrigin, redirect, self } from "../http"
 import { proxy as _proxy } from "../proxy"
 
 const getProxyTarget = (req: Request, passthroughPath?: boolean): string => {
@@ -50,6 +50,7 @@ export async function wsProxy(
     passthroughPath?: boolean
   },
 ): Promise<void> {
+  ensureOrigin(req)
   await ensureAuthenticated(req)
   _proxy.ws(req, req.ws, req.head, {
     ignorePath: true,

src/node/routes/vscode.ts

@@ -7,7 +7,7 @@ import { WebsocketRequest } from "../../../typings/pluginapi"
 import { logError } from "../../common/util"
 import { CodeArgs, toCodeArgs } from "../cli"
 import { isDevMode } from "../constants"
-import { authenticated, ensureAuthenticated, redirect, replaceTemplates, self } from "../http"
+import { authenticated, ensureAuthenticated, ensureOrigin, redirect, replaceTemplates, self } from "../http"
 import { SocketProxyProvider } from "../socket"
 import { isFile, loadAMDModule } from "../util"
 import { Router as WsRouter } from "../wsRouter"
@@ -173,7 +173,7 @@ export class CodeServerRouteWrapper {
     this.router.get("/", this.ensureCodeServerLoaded, this.$root)
     this.router.get("/manifest.json", this.manifest)
     this.router.all("*", ensureAuthenticated, this.ensureCodeServerLoaded, this.$proxyRequest)
-    this._wsRouterWrapper.ws("*", ensureAuthenticated, this.ensureCodeServerLoaded, this.$proxyWebsocket)
+    this._wsRouterWrapper.ws("*", ensureOrigin, ensureAuthenticated, this.ensureCodeServerLoaded, this.$proxyWebsocket)
   }
 
   dispose() {

src/node/util.ts

@@ -541,3 +541,13 @@ export const loadAMDModule = async <T>(amdPath: string, exportName: string): Pro
 
   return module[exportName] as T
 }
+
+/**
+ * Split a string on the first equals.  The result will always be an array with
+ * two items regardless of how many equals there are.  The second item will be
+ * undefined if empty or missing.
+ */
+export function splitOnFirstEquals(str: string): [string, string | undefined] {
+  const split = str.split(/=(.+)?/, 2)
+  return [split[0], split[1]]
+}

src/node/wsRouter.ts

@@ -32,6 +32,9 @@ export class WebsocketRouter {
   /**
    * Handle a websocket at this route. Note that websockets are immediately
    * paused when they come in.
+   *
+   * If the origin header exists it must match the host or the connection will
+   * be prevented.
    */
   public ws(route: expressCore.PathParams, ...handlers: pluginapi.WebSocketHandler[]): void {
     this.router.get(

test/unit/node/cli.test.ts

@@ -11,7 +11,6 @@ import {
   readSocketPath,
   setDefaults,
   shouldOpenInExistingInstance,
-  splitOnFirstEquals,
   toCodeArgs,
   optionDescriptions,
   options,
@@ -297,26 +296,6 @@ describe("parser", () => {
     })
   })
 
-  it("should override with --link", async () => {
-    const args = parse(
-      "--cert test --cert-key test --socket test --socket-mode 777 --host 0.0.0.0 --port 8888 --link test".split(" "),
-    )
-    const defaultArgs = await setDefaults(args)
-    expect(defaultArgs).toEqual({
-      ...defaults,
-      auth: "none",
-      host: "localhost",
-      link: {
-        value: "test",
-      },
-      port: 0,
-      cert: undefined,
-      "cert-key": path.resolve("test"),
-      socket: undefined,
-      "socket-mode": undefined,
-    })
-  })
-
   it("should use env var password", async () => {
     process.env.PASSWORD = "test"
     const args = parse([])
@@ -555,31 +534,6 @@ describe("cli", () => {
   })
 })
 
-describe("splitOnFirstEquals", () => {
-  it("should split on the first equals", () => {
-    const testStr = "enabled-proposed-api=test=value"
-    const actual = splitOnFirstEquals(testStr)
-    const expected = ["enabled-proposed-api", "test=value"]
-    expect(actual).toEqual(expect.arrayContaining(expected))
-  })
-  it("should split on first equals regardless of multiple equals signs", () => {
-    const testStr =
-      "hashed-password=$argon2i$v=19$m=4096,t=3,p=1$0qR/o+0t00hsbJFQCKSfdQ$oFcM4rL6o+B7oxpuA4qlXubypbBPsf+8L531U7P9HYY"
-    const actual = splitOnFirstEquals(testStr)
-    const expected = [
-      "hashed-password",
-      "$argon2i$v=19$m=4096,t=3,p=1$0qR/o+0t00hsbJFQCKSfdQ$oFcM4rL6o+B7oxpuA4qlXubypbBPsf+8L531U7P9HYY",
-    ]
-    expect(actual).toEqual(expect.arrayContaining(expected))
-  })
-  it("should always return the first element before an equals", () => {
-    const testStr = "auth="
-    const actual = splitOnFirstEquals(testStr)
-    const expected = ["auth"]
-    expect(actual).toEqual(expect.arrayContaining(expected))
-  })
-})
-
 describe("shouldSpawnCliProcess", () => {
   it("should return false if no 'extension' related args passed in", async () => {
     const args = {}
@@ -881,21 +835,13 @@ describe("optionDescriptions", () => {
 
   it("should show if an option is deprecated", () => {
     const opts: Partial<Options<Required<UserProvidedArgs>>> = {
-      link: {
+      cert: {
         type: OptionalString,
-        description: `
+        description: "foo",
-          Securely bind code-server via our cloud service with the passed name. You'll get a URL like
-          https://hostname-username.coder.co at which you can easily access your code-server instance.
-          Authorization is done via GitHub.
-        `,
         deprecated: true,
       },
     }
-    expect(optionDescriptions(opts)).toStrictEqual([
+    expect(optionDescriptions(opts)).toStrictEqual(["  --cert (deprecated) foo"])
-      `  --link (deprecated) Securely bind code-server via our cloud service with the passed name. You'll get a URL like
-          https://hostname-username.coder.co at which you can easily access your code-server instance.
-          Authorization is done via GitHub.`,
-    ])
   })
 
   it("should show newlines in description", () => {

test/unit/node/http.test.ts

@@ -1,55 +1,118 @@
 import { getMockReq } from "@jest-mock/express"
-import { constructRedirectPath, relativeRoot } from "../../../src/node/http"
+import * as http from "../../../src/node/http"
+import { mockLogger } from "../../utils/helpers"
 
 describe("http", () => {
-  it("should construct a relative path to the root", () => {
+  beforeEach(() => {
-    expect(relativeRoot("/")).toStrictEqual(".")
+    mockLogger()
-    expect(relativeRoot("/foo")).toStrictEqual(".")
-    expect(relativeRoot("/foo/")).toStrictEqual("./..")
-    expect(relativeRoot("/foo/bar ")).toStrictEqual("./..")
-    expect(relativeRoot("/foo/bar/")).toStrictEqual("./../..")
   })
-})
 
-describe("constructRedirectPath", () => {
+  afterEach(() => {
-  it("should preserve slashes in queryString so they are human-readable", () => {
+    jest.clearAllMocks()
-    const mockReq = getMockReq({
-      originalUrl: "localhost:8080",
-    })
-    const mockQueryParams = { folder: "/Users/jp/dev/coder" }
-    const mockTo = ""
-    const actual = constructRedirectPath(mockReq, mockQueryParams, mockTo)
-    const expected = "./?folder=/Users/jp/dev/coder"
-    expect(actual).toBe(expected)
   })
-  it("should use an empty string if no query params", () => {
+
-    const mockReq = getMockReq({
+  it("should construct a relative path to the root", () => {
-      originalUrl: "localhost:8080",
+    expect(http.relativeRoot("/")).toStrictEqual(".")
-    })
+    expect(http.relativeRoot("/foo")).toStrictEqual(".")
-    const mockQueryParams = {}
+    expect(http.relativeRoot("/foo/")).toStrictEqual("./..")
-    const mockTo = ""
+    expect(http.relativeRoot("/foo/bar ")).toStrictEqual("./..")
-    const actual = constructRedirectPath(mockReq, mockQueryParams, mockTo)
+    expect(http.relativeRoot("/foo/bar/")).toStrictEqual("./../..")
-    const expected = "./"
-    expect(actual).toBe(expected)
   })
-  it("should append the 'to' path relative to the originalUrl", () => {
+
-    const mockReq = getMockReq({
+  describe("origin", () => {
-      originalUrl: "localhost:8080",
+    ;[
+      {
+        origin: "",
+        host: "",
+        expected: true,
+      },
+      {
+        origin: "http://localhost:8080",
+        host: "",
+        expected: false,
+      },
+      {
+        origin: "http://localhost:8080",
+        host: "localhost:8080",
+        expected: true,
+      },
+      {
+        origin: "http://localhost:8080",
+        host: "localhost:8081",
+        expected: false,
+      },
+      {
+        origin: "localhost:8080",
+        host: "localhost:8080",
+        expected: false, // Gets parsed as host: localhost and path: 8080.
+      },
+      {
+        origin: "test.org",
+        host: "localhost:8080",
+        expected: false, // Parsing fails completely.
+      },
+    ].forEach((test) => {
+      ;[
+        ["host", test.host],
+        ["x-forwarded-host", test.host],
+        ["forwarded", `for=127.0.0.1, host=${test.host}, proto=http`],
+        ["forwarded", `for=127.0.0.1;proto=http;host=${test.host}`],
+        ["forwarded", `proto=http;host=${test.host}, for=127.0.0.1`],
+      ].forEach(([key, value]) => {
+        it(`${test.origin} -> [${key}: ${value}]`, () => {
+          const req = getMockReq({
+            originalUrl: "localhost:8080",
+            headers: {
+              origin: test.origin,
+              [key]: value,
+            },
+          })
+          expect(http.authenticateOrigin(req)).toBe(test.expected)
+        })
+      })
     })
-    const mockQueryParams = {}
-    const mockTo = "vscode"
-    const actual = constructRedirectPath(mockReq, mockQueryParams, mockTo)
-    const expected = "./vscode"
-    expect(actual).toBe(expected)
   })
-  it("should append append queryParams after 'to' path", () => {
+
-    const mockReq = getMockReq({
+  describe("constructRedirectPath", () => {
-      originalUrl: "localhost:8080",
+    it("should preserve slashes in queryString so they are human-readable", () => {
+      const mockReq = getMockReq({
+        originalUrl: "localhost:8080",
+      })
+      const mockQueryParams = { folder: "/Users/jp/dev/coder" }
+      const mockTo = ""
+      const actual = http.constructRedirectPath(mockReq, mockQueryParams, mockTo)
+      const expected = "./?folder=/Users/jp/dev/coder"
+      expect(actual).toBe(expected)
+    })
+    it("should use an empty string if no query params", () => {
+      const mockReq = getMockReq({
+        originalUrl: "localhost:8080",
+      })
+      const mockQueryParams = {}
+      const mockTo = ""
+      const actual = http.constructRedirectPath(mockReq, mockQueryParams, mockTo)
+      const expected = "./"
+      expect(actual).toBe(expected)
+    })
+    it("should append the 'to' path relative to the originalUrl", () => {
+      const mockReq = getMockReq({
+        originalUrl: "localhost:8080",
+      })
+      const mockQueryParams = {}
+      const mockTo = "vscode"
+      const actual = http.constructRedirectPath(mockReq, mockQueryParams, mockTo)
+      const expected = "./vscode"
+      expect(actual).toBe(expected)
+    })
+    it("should append append queryParams after 'to' path", () => {
+      const mockReq = getMockReq({
+        originalUrl: "localhost:8080",
+      })
+      const mockQueryParams = { folder: "/Users/jp/dev/coder" }
+      const mockTo = "vscode"
+      const actual = http.constructRedirectPath(mockReq, mockQueryParams, mockTo)
+      const expected = "./vscode?folder=/Users/jp/dev/coder"
+      expect(actual).toBe(expected)
     })
-    const mockQueryParams = { folder: "/Users/jp/dev/coder" }
-    const mockTo = "vscode"
-    const actual = constructRedirectPath(mockReq, mockQueryParams, mockTo)
-    const expected = "./vscode?folder=/Users/jp/dev/coder"
-    expect(actual).toBe(expected)
   })
 })

test/unit/node/proxy.test.ts

@@ -4,21 +4,26 @@ import * as http from "http"
 import nodeFetch from "node-fetch"
 import { HttpCode } from "../../../src/common/http"
 import { proxy } from "../../../src/node/proxy"
-import { getAvailablePort } from "../../utils/helpers"
+import { wss, Router as WsRouter } from "../../../src/node/wsRouter"
+import { getAvailablePort, mockLogger } from "../../utils/helpers"
 import * as httpserver from "../../utils/httpserver"
 import * as integration from "../../utils/integration"
 
 describe("proxy", () => {
   const nhooyrDevServer = new httpserver.HttpServer()
+  const wsApp = express.default()
+  const wsRouter = WsRouter()
   let codeServer: httpserver.HttpServer | undefined
   let proxyPath: string
   let absProxyPath: string
   let e: express.Express
 
   beforeAll(async () => {
+    wsApp.use("/", wsRouter.router)
     await nhooyrDevServer.listen((req, res) => {
       e(req, res)
     })
+    nhooyrDevServer.listenUpgrade(wsApp)
     proxyPath = `/proxy/${nhooyrDevServer.port()}/wsup`
     absProxyPath = proxyPath.replace("/proxy/", "/absproxy/")
   })
@@ -29,6 +34,7 @@ describe("proxy", () => {
 
   beforeEach(() => {
     e = express.default()
+    mockLogger()
   })
 
   afterEach(async () => {
@@ -36,6 +42,7 @@ describe("proxy", () => {
       await codeServer.dispose()
       codeServer = undefined
     }
+    jest.clearAllMocks()
   })
 
   it("should rewrite the base path", async () => {
@@ -151,6 +158,35 @@ describe("proxy", () => {
     expect(resp.status).toBe(500)
     expect(resp.statusText).toMatch("Internal Server Error")
   })
+
+  it("should pass origin check", async () => {
+    wsRouter.ws("/wsup", async (req) => {
+      wss.handleUpgrade(req, req.ws, req.head, (ws) => {
+        ws.send("hello")
+        req.ws.resume()
+      })
+    })
+    codeServer = await integration.setup(["--auth=none"], "")
+    const ws = await codeServer.wsWait(proxyPath, {
+      headers: {
+        host: "localhost:8080",
+        origin: "https://localhost:8080",
+      },
+    })
+    ws.terminate()
+  })
+
+  it("should fail origin check", async () => {
+    await expect(async () => {
+      codeServer = await integration.setup(["--auth=none"], "")
+      await codeServer.wsWait(proxyPath, {
+        headers: {
+          host: "localhost:8080",
+          origin: "https://evil.org",
+        },
+      })
+    }).rejects.toThrow()
+  })
 })
 
 // NOTE@jsjoeio
@@ -190,18 +226,18 @@ describe("proxy (standalone)", () => {
     })
 
     // Start both servers
-    await proxyTarget.listen(PROXY_PORT)
+    proxyTarget.listen(PROXY_PORT)
-    await testServer.listen(PORT)
+    testServer.listen(PORT)
   })
 
   afterEach(async () => {
-    await testServer.close()
+    testServer.close()
-    await proxyTarget.close()
+    proxyTarget.close()
   })
 
   it("should return a 500 when proxy target errors ", async () => {
     // Close the proxy target so that proxy errors
-    await proxyTarget.close()
+    proxyTarget.close()
     const errorResp = await nodeFetch(`${URL}/error`)
     expect(errorResp.status).toBe(HttpCode.ServerError)
     expect(errorResp.statusText).toBe("Internal Server Error")

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

@@ -23,7 +23,9 @@ describe("health", () => {
     codeServer = await integration.setup(["--auth=none"], "")
     const ws = codeServer.ws("/healthz")
     const message = await new Promise((resolve, reject) => {
-      ws.on("error", console.error)
+      ws.on("error", (err) => {
+        console.error("[healthz]", err)
+      })
       ws.on("message", (message) => {
         try {
           const j = JSON.parse(message.toString())

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

@@ -0,0 +1,30 @@
+import * as httpserver from "../../../utils/httpserver"
+import * as integration from "../../../utils/integration"
+import { mockLogger } from "../../../utils/helpers"
+
+describe("vscode", () => {
+  let codeServer: httpserver.HttpServer | undefined
+  beforeEach(() => {
+    mockLogger()
+  })
+
+  afterEach(async () => {
+    if (codeServer) {
+      await codeServer.dispose()
+      codeServer = undefined
+    }
+    jest.clearAllMocks()
+  })
+
+  it("should fail origin check", async () => {
+    await expect(async () => {
+      codeServer = await integration.setup(["--auth=none"], "")
+      await codeServer.wsWait("/vscode", {
+        headers: {
+          host: "localhost:8080",
+          origin: "https://evil.org",
+        },
+      })
+    }).rejects.toThrow()
+  })
+})

test/unit/node/util.test.ts

@@ -601,3 +601,41 @@ describe("constructOpenOptions", () => {
     expect(urlSearch).toBe("?q=^&test")
   })
 })
+
+describe("splitOnFirstEquals", () => {
+  const tests = [
+    {
+      name: "empty",
+      key: "",
+      value: "",
+    },
+    {
+      name: "split on first equals",
+      key: "foo",
+      value: "bar",
+    },
+    {
+      name: "split on first equals even with multiple equals",
+      key: "foo",
+      value: "bar=baz",
+    },
+    {
+      name: "split with empty value",
+      key: "foo",
+      value: "",
+    },
+    {
+      name: "split with no value",
+      key: "foo",
+      value: undefined,
+    },
+  ]
+  tests.forEach((test) => {
+    it("should ${test.name}", () => {
+      const input = test.key && typeof test.value !== "undefined" ? `${test.key}=${test.value}` : test.key
+      const [key, value] = util.splitOnFirstEquals(input)
+      expect(key).toStrictEqual(test.key)
+      expect(value).toStrictEqual(test.value || undefined)
+    })
+  })
+})

test/utils/httpserver.ts

@@ -7,7 +7,6 @@ import { Disposable } from "../../src/common/emitter"
 import * as util from "../../src/common/util"
 import { ensureAddress } from "../../src/node/app"
 import { disposer } from "../../src/node/http"
-
 import { handleUpgrade } from "../../src/node/wsRouter"
 
 // Perhaps an abstraction similar to this should be used in app.ts as well.
@@ -76,14 +75,25 @@ export class HttpServer {
   /**
    * Open a websocket against the request path.
    */
-  public ws(requestPath: string): Websocket {
+  public ws(requestPath: string, options?: Websocket.ClientOptions): Websocket {
     const address = ensureAddress(this.hs, "ws")
     if (typeof address === "string") {
       throw new Error("Cannot open websocket to socket path")
     }
     address.pathname = requestPath
 
-    return new Websocket(address.toString())
+    return new Websocket(address.toString(), options)
+  }
+
+  /**
+   * Open a websocket and wait for it to fully open.
+   */
+  public wsWait(requestPath: string, options?: Websocket.ClientOptions): Promise<Websocket> {
+    const ws = this.ws(requestPath, options)
+    return new Promise<Websocket>((resolve, reject) => {
+      ws.on("error", (err) => reject(err))
+      ws.on("open", () => resolve(ws))
+    })
   }
 
   public port(): number {