fix: return escaped HTML from login failure

Before, we were sending raw HTML from the Express server to the client.

This opens an opportunity for cross-site scripting. By escaping first, we remove
that opportunity.

Author: jsjoeio

src/node/routes/login.ts

@@ -4,7 +4,7 @@ import { RateLimiter as Limiter } from "limiter"
 import * as path from "path"
 import { rootPath } from "../constants"
 import { authenticated, getCookieDomain, redirect, replaceTemplates } from "../http"
-import { getPasswordMethod, handlePasswordValidation, humanPath, sanitizeString } from "../util"
+import { getPasswordMethod, handlePasswordValidation, humanPath, sanitizeString, escapeHtml } from "../util"
 
 export enum Cookie {
   Key = "key",
@@ -37,9 +37,6 @@ const getRoot = async (req: Request, error?: Error): Promise<string> => {
     passwordMsg = "Password was set from $HASHED_PASSWORD."
   }
 
-  if (error) {
-    console.log("there is an error", error.message)
-  }
   return replaceTemplates(
     req,
     content
@@ -115,6 +112,8 @@ router.post("/", async (req, res) => {
 
     throw new Error("Incorrect password")
   } catch (error) {
-    res.send(await getRoot(req, error))
+    const html = await getRoot(req, error)
+    const escapedHtml = escapeHtml(html)
+    res.send(escapedHtml)
   }
 })

src/node/util.ts

@@ -515,7 +515,7 @@ export const isFile = async (path: string): Promise<boolean> => {
  *
  * Source: https://stackoverflow.com/a/6234804/3015595
  **/
-export function escapeHTML(unsafe: string): string {
+export function escapeHtml(unsafe: string): string {
   return unsafe
     .replace(/&/g, "&amp;")
     .replace(/</g, "&lt;")

test/unit/node/util.test.ts

@@ -435,9 +435,9 @@ describe("onLine", () => {
   })
 })
 
-describe("escapeHTML", () => {
+describe("escapeHtml", () => {
   it("should escape HTML", () => {
-    expect(util.escapeHTML(`<div class="error">"Hello & world"</div>`)).toBe(
+    expect(util.escapeHtml(`<div class="error">"Hello & world"</div>`)).toBe(
       "&lt;div class=&quot;error&quot;&gt;&quot;Hello &amp; world&quot;&lt;/div&gt;",
     )
   })

test/unit/routes/login.test.ts

@@ -37,7 +37,7 @@ describe("login", () => {
       expect(limiter.removeToken()).toBe(false)
     })
   })
-  describe.only("/", () => {
+  describe("/login", () => {
     let _codeServer: httpserver.HttpServer | undefined
     function codeServer(): httpserver.HttpServer {
       if (!_codeServer) {
@@ -60,20 +60,18 @@ describe("login", () => {
       process.env.PASSWORD = previousEnvPassword
     })
 
-    // TODO@jsjoeio fix this name of test
-    it("should return an error", async () => {
-      const resp = await codeServer().fetch("/", { method: "POST" })
-      // TODO@jsjoeio um not sure why we are getting a 404
-      expect(resp.status).toBe(404)
+    it("should return escaped HTML with 'Missing password' message", async () => {
+      const resp = await codeServer().fetch("/login", { method: "POST" })
 
-      try {
-        const content = JSON.parse(await resp.text())
+      expect(resp.status).toBe(200)
 
-        expect(content.error).toMatch("ENOENT")
-      } catch (error) {
-        console.log("heree")
-        console.error(error)
-      }
+      const htmlContent = await resp.text()
+
+      expect(htmlContent).not.toContain(">")
+      expect(htmlContent).not.toContain("<")
+      expect(htmlContent).not.toContain('"')
+      expect(htmlContent).not.toContain("'")
+      expect(htmlContent).toContain("&lt;div class=&quot;error&quot;&gt;Missing password&lt;/div&gt;")
     })
   })
 })