refactor: parse options with multiple = in cli

There was a case with the hashed-password which had multiple equal signs in the value and it wasn't being parsed correctly. This uses a new function and adds a few tests.
coder · Jun 4, 2021 · cf2a570 · cf2a570
1 parent e3171dd
commit cf2a570
Show file tree

Hide file tree

Showing 8 changed files with 75 additions and 65 deletions.
diff --git a/docs/FAQ.md b/docs/FAQ.md
@@ -2,39 +2,38 @@
 <!-- DON'T EDIT THIS SECTION, INSTEAD RE-RUN doctoc TO UPDATE -->
 # FAQ
 
-- [FAQ](#faq)
-  - [Questions?](#questions)
-  - [iPad Status?](#ipad-status)
-  - [Community Projects (awesome-code-server)](#community-projects-awesome-code-server)
-  - [How can I reuse my VS Code configuration?](#how-can-i-reuse-my-vs-code-configuration)
-  - [Differences compared to VS Code?](#differences-compared-to-vs-code)
-    - [Installing an extension](#installing-an-extension)
-  - [How can I request a missing extension?](#how-can-i-request-a-missing-extension)
-  - [Installing an extension manually](#installing-an-extension-manually)
-  - [How do I configure the marketplace URL?](#how-do-i-configure-the-marketplace-url)
-  - [Where are extensions stored?](#where-are-extensions-stored)
-  - [How is this different from VS Code Codespaces?](#how-is-this-different-from-vs-code-codespaces)
-  - [How should I expose code-server to the internet?](#how-should-i-expose-code-server-to-the-internet)
-  - [Can I store my password hashed?](#can-i-store-my-password-hashed)
-  - [How do I securely access web services?](#how-do-i-securely-access-web-services)
-    - [Sub-paths](#sub-paths)
-    - [Sub-domains](#sub-domains)
-  - [Why does the code-server proxy strip `/proxy/<port>` from the request path?](#why-does-the-code-server-proxy-strip-proxyport-from-the-request-path)
-    - [Proxying to Create React App](#proxying-to-create-react-app)
-  - [Multi-tenancy](#multi-tenancy)
-  - [Docker in code-server container?](#docker-in-code-server-container)
-  - [How can I disable telemetry?](#how-can-i-disable-telemetry)
-  - [How does code-server decide what workspace or folder to open?](#how-does-code-server-decide-what-workspace-or-folder-to-open)
-  - [How do I debug issues with code-server?](#how-do-i-debug-issues-with-code-server)
-  - [Heartbeat File](#heartbeat-file)
-  - [Healthz endpoint](#healthz-endpoint)
-  - [How does the config file work?](#how-does-the-config-file-work)
-  - [Isn't an install script piped into sh insecure?](#isnt-an-install-script-piped-into-sh-insecure)
-  - [How do I make my keyboard shortcuts work?](#how-do-i-make-my-keyboard-shortcuts-work)
-  - [How do I access my Documents/Downloads/Desktop folders in code-server on OSX?](#how-do-i-access-my-documentsdownloadsdesktop-folders-in-code-server-on-osx)
-  - [Differences compared to Theia?](#differences-compared-to-theia)
-  - [`$HTTP_PROXY`, `$HTTPS_PROXY`, `$NO_PROXY`](#http_proxy-https_proxy-no_proxy)
-  - [Enterprise](#enterprise)
+- [Questions?](#questions)
+- [iPad Status?](#ipad-status)
+- [Community Projects (awesome-code-server)](#community-projects-awesome-code-server)
+- [How can I reuse my VS Code configuration?](#how-can-i-reuse-my-vs-code-configuration)
+- [Differences compared to VS Code?](#differences-compared-to-vs-code)
+  - [Installing an extension](#installing-an-extension)
+- [How can I request a missing extension?](#how-can-i-request-a-missing-extension)
+- [Installing an extension manually](#installing-an-extension-manually)
+- [How do I configure the marketplace URL?](#how-do-i-configure-the-marketplace-url)
+- [Where are extensions stored?](#where-are-extensions-stored)
+- [How is this different from VS Code Codespaces?](#how-is-this-different-from-vs-code-codespaces)
+- [How should I expose code-server to the internet?](#how-should-i-expose-code-server-to-the-internet)
+- [Can I store my password hashed?](#can-i-store-my-password-hashed)
+- [How do I securely access web services?](#how-do-i-securely-access-web-services)
+  - [Sub-paths](#sub-paths)
+  - [Sub-domains](#sub-domains)
+- [Why does the code-server proxy strip `/proxy/<port>` from the request path?](#why-does-the-code-server-proxy-strip-proxyport-from-the-request-path)
+  - [Proxying to Create React App](#proxying-to-create-react-app)
+- [Multi-tenancy](#multi-tenancy)
+- [Docker in code-server container?](#docker-in-code-server-container)
+- [How can I disable telemetry?](#how-can-i-disable-telemetry)
+- [How does code-server decide what workspace or folder to open?](#how-does-code-server-decide-what-workspace-or-folder-to-open)
+- [How do I debug issues with code-server?](#how-do-i-debug-issues-with-code-server)
+- [Heartbeat File](#heartbeat-file)
+- [Healthz endpoint](#healthz-endpoint)
+- [How does the config file work?](#how-does-the-config-file-work)
+- [Isn't an install script piped into sh insecure?](#isnt-an-install-script-piped-into-sh-insecure)
+- [How do I make my keyboard shortcuts work?](#how-do-i-make-my-keyboard-shortcuts-work)
+- [How do I access my Documents/Downloads/Desktop folders in code-server on OSX?](#how-do-i-access-my-documentsdownloadsdesktop-folders-in-code-server-on-osx)
+- [Differences compared to Theia?](#differences-compared-to-theia)
+- [`$HTTP_PROXY`, `$HTTPS_PROXY`, `$NO_PROXY`](#http_proxy-https_proxy-no_proxy)
+- [Enterprise](#enterprise)
 
 <!-- END doctoc generated TOC please keep comment here to allow auto update -->
 
@@ -209,7 +208,6 @@ Yes you can! Set the value of `hashed-password` instead of `password`. Generate
 ```shell
 echo -n "password" | npx argon2-cli -e
 $argon2i$v=19$m=4096,t=3,p=1$wst5qhbgk2lu1ih4dmuxvg$ls1alrvdiwtvzhwnzcm1dugg+5dto3dt1d5v9xtlws4
-
 ```
 
 Of course replace `thisismypassword` with your actual password and **remember to put it inside quotes**!

diff --git a/package.json b/package.json
@@ -36,7 +36,6 @@
   "main": "out/node/entry.js",
   "devDependencies": {
     "@schemastore/package": "^0.0.6",
-    "@types/bcrypt": "^5.0.0",
     "@types/body-parser": "^1.19.0",
     "@types/compression": "^1.7.0",
     "@types/cookie-parser": "^1.4.2",
@@ -90,7 +89,6 @@
   "dependencies": {
     "@coder/logger": "1.1.16",
     "argon2": "^0.28.0",
-    "bcrypt": "^5.0.1",
     "body-parser": "^1.19.0",
     "compression": "^1.7.4",
     "cookie-parser": "^1.4.5",

diff --git a/src/node/cli.ts b/src/node/cli.ts
@@ -247,14 +247,8 @@ export function splitOnFirstEquals(str: string): string[] {
   // $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
-  const split = str.split(/=(.+)/, 2)
-
-  // It should always return two elements
-  // because it's used in a place where
-  // it expected two elements
-  if (split.length === 1) {
-    split.push("")
-  }
+  // We use the ? to say the the substr after the = is optional
+  const split = str.split(/=(.+)?/, 2)
 
   return split
 }
@@ -289,10 +283,11 @@ export const parse = (
       let key: keyof Args | undefined
       let value: string | undefined
       if (arg.startsWith("--")) {
-        // TODO fix this
-        const split = arg.replace(/^--/, "").split("=", 2)
+        const split = splitOnFirstEquals(arg.replace(/^--/, ""))
         key = split[0] as keyof Args
         value = split[1]
+
+        console.log(`Hello key: ${key}, and value: ${value}`)
       } else {
         const short = arg.replace(/^-/, "")
         const pair = Object.entries(options).find(([, v]) => v.short === short)
@@ -301,6 +296,7 @@ export const parse = (
         }
       }
 
+      console.log(`What is key: ${key} and options: ${key ? options[key] : ""}`)
       if (!key || !options[key]) {
         throw error(`Unknown option ${arg}`)
       }
@@ -563,7 +559,6 @@ export function parseConfigFile(configFile: string, configPath: string): ConfigA
   const config = yaml.load(configFile, {
     filename: configPath,
   })
-  console.log("what is this config", config)
   if (!config || typeof config === "string") {
     throw new Error(`invalid config: ${config}`)
   }
@@ -576,11 +571,9 @@ export function parseConfigFile(configFile: string, configPath: string): ConfigA
     }
     return `--${optName}=${opt}`
   })
-  console.log("what are the configFileArgv", configFileArgv)
   const args = parse(configFileArgv, {
     configFile: configPath,
   })
-  console.log(args, "args")
   return {
     ...args,
     config: configPath,

diff --git a/src/node/http.ts b/src/node/http.ts
@@ -63,9 +63,10 @@ export const ensureAuthenticated = async (
  */
 export const authenticated = async (req: express.Request): Promise<boolean> => {
   switch (req.args.auth) {
-    case AuthType.None:
+    case AuthType.None: {
       return true
-    case AuthType.Password:
+    }
+    case AuthType.Password: {
       // The password is stored in the cookie after being hashed.
       const hashedPasswordFromArgs = req.args["hashed-password"]
       const passwordMethod = getPasswordMethod(hashedPasswordFromArgs)
@@ -77,8 +78,10 @@ export const authenticated = async (req: express.Request): Promise<boolean> => {
       }
 
       return await isCookieValid(isCookieValidArgs)
-    default:
+    }
+    default: {
       throw new Error(`Unsupported auth type ${req.args.auth}`)
+    }
   }
 }
 

diff --git a/src/node/util.ts b/src/node/util.ts
@@ -1,15 +1,15 @@
+import { logger } from "@coder/logger"
+import * as argon2 from "argon2"
 import * as cp from "child_process"
 import * as crypto from "crypto"
-import * as argon2 from "argon2"
 import envPaths from "env-paths"
 import { promises as fs } from "fs"
 import * as net from "net"
 import * as os from "os"
 import * as path from "path"
+import safeCompare from "safe-compare"
 import * as util from "util"
 import xdgBasedir from "xdg-basedir"
-import safeCompare from "safe-compare"
-import { logger } from "@coder/logger"
 
 export interface Paths {
   data: string

diff --git a/test/package.json b/test/package.json
@@ -18,6 +18,5 @@
   "resolutions": {
     "@playwright/test/playwright": "^1.11.0-next-alpha-apr-13-2021"
   },
-  "dependencies": {
-  }
+  "dependencies": {}
 }
diff --git a/test/unit/cli.test.ts b/test/unit/cli.test.ts
@@ -349,6 +349,21 @@ describe("parser", () => {
       ],
     })
   })
+  it("should parse options with double-dash and multiple equal signs ", async () => {
+    const args = parse(
+      [
+        "--hashed-password=$argon2i$v=19$m=4096,t=3,p=1$0qr/o+0t00hsbjfqcksfdq$ofcm4rl6o+b7oxpua4qlxubypbbpsf+8l531u7p9hyy",
+      ],
+      {
+        configFile: "/pathtoconfig",
+      },
+    )
+    expect(args).toEqual({
+      _: [],
+      "hashed-password":
+        "$argon2i$v=19$m=4096,t=3,p=1$0qr/o+0t00hsbjfqcksfdq$ofcm4rl6o+b7oxpua4qlxubypbbpsf+8l531u7p9hyy",
+    })
+  })
 })
 
 describe("cli", () => {
@@ -426,25 +441,25 @@ describe("cli", () => {
 
 describe("splitOnFirstEquals", () => {
   it("should split on the first equals", () => {
-    const testStr = "--enabled-proposed-api=test=value"
+    const testStr = "enabled-proposed-api=test=value"
     const actual = splitOnFirstEquals(testStr)
-    const expected = ["--enabled-proposed-api", "test=value"]
+    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"
+      "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",
+      "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 two elements", () => {
-    const testStr = ""
+  it("should always return the first element before an equals", () => {
+    const testStr = "auth="
     const actual = splitOnFirstEquals(testStr)
-    const expected = ["", ""]
+    const expected = ["auth"]
     expect(actual).toEqual(expect.arrayContaining(expected))
   })
 })
diff --git a/typings/pluginapi.d.ts b/typings/pluginapi.d.ts
@@ -145,7 +145,11 @@ export const proxy: ProxyServer
 /**
  * Middleware to ensure the user is authenticated. Throws if they are not.
  */
-export function ensureAuthenticated(req: express.Request, res?: express.Response, next?: express.NextFunction): Promise<void>
+export function ensureAuthenticated(
+  req: express.Request,
+  res?: express.Response,
+  next?: express.NextFunction,
+): Promise<void>
 
 /**
  * Returns true if the user is authenticated.