From 0f80c68c5859d0660b2b565cc3745f128dd945f7 Mon Sep 17 00:00:00 2001 From: Arthur Cinader <700572+acinader@users.noreply.github.com> Date: Mon, 24 Aug 2026 15:02:34 -0700 Subject: [PATCH] dev:web: add browser tests for the web UI These Playwright tests drive a browser to cover the parts of the web UI implemented client side in javascript: add form, autocomplete, date picker, keyboard shortcuts, sidebar state, and hash highlighting. This suite is a complement to the existing yesod-test suite. In addition to testing web functionality, the security suite also confirms journal data renders as text and not markup. The suite is available for on demand use and is not part of stack build or stack test. CSRF tokens and restricting file access to the journal's own files don't require a browser, so this PR adds those tests to Hledger/Web/Test.hs, which already runs in CI. Each negative test there has a positive control posting the same request with a valid token. The suite runs with pnpm (>= 10) to reduce supply-chain exposure: pnpm does not run dependency install scripts by default, pnpm-workspace.yaml adds a 24h release-age gate, and pnpm-lock.yaml is committed with integrity hashes. The one dependency is @playwright/test. Lives in hledger-web/test/browser ("browser tests" rather than "e2e"), runnable as `just browsertest`. The README notes that running these means installing javascript tooling from the npm registry, which is more exposure than hledger's other test suites carry. AI usage: Claude Opus 4.8, ~90k output tokens --- Justfile | 6 + hledger-web/Hledger/Web/Test.hs | 143 +++++++++++ hledger-web/test/browser/.gitignore | 3 + hledger-web/test/browser/README.md | 58 +++++ hledger-web/test/browser/fixture.journal | 34 +++ hledger-web/test/browser/global-setup.js | 54 ++++ hledger-web/test/browser/global-teardown.js | 15 ++ hledger-web/test/browser/package.json | 13 + hledger-web/test/browser/playwright.config.js | 17 ++ hledger-web/test/browser/pnpm-lock.yaml | 52 ++++ hledger-web/test/browser/pnpm-workspace.yaml | 2 + hledger-web/test/browser/security.spec.js | 83 +++++++ hledger-web/test/browser/webui.spec.js | 233 ++++++++++++++++++ 13 files changed, 713 insertions(+) create mode 100644 hledger-web/test/browser/.gitignore create mode 100644 hledger-web/test/browser/README.md create mode 100644 hledger-web/test/browser/fixture.journal create mode 100644 hledger-web/test/browser/global-setup.js create mode 100644 hledger-web/test/browser/global-teardown.js create mode 100644 hledger-web/test/browser/package.json create mode 100644 hledger-web/test/browser/playwright.config.js create mode 100644 hledger-web/test/browser/pnpm-lock.yaml create mode 100644 hledger-web/test/browser/pnpm-workspace.yaml create mode 100644 hledger-web/test/browser/security.spec.js create mode 100644 hledger-web/test/browser/webui.spec.js diff --git a/Justfile b/Justfile index 63a5ea89f25..03f1432588d 100644 --- a/Justfile +++ b/Justfile @@ -478,6 +478,12 @@ SHELLTEST := STACK + ' exec -- shelltest --execdir --threads=40' # --test so that subsequent `stack test` won't recompile everything # --no-run-tests to avoid running the slow doctest suite every time +# run hledger-web's browser tests against the current build, with any playwright OPTS (needs setup, see hledger-web/test/browser/README.md) +@browsertest *PWOPTS: + {{ STACK }} build hledger-web + cd hledger-web/test/browser && \ + HLEDGER_WEB="{{ STACK }} exec -- hledger-web" pnpm test {{ PWOPTS }} + # too fragile: # echo # just perftest {{ STOPTS }} diff --git a/hledger-web/Hledger/Web/Test.hs b/hledger-web/Hledger/Web/Test.hs index 2048264679e..9fa7e89a0aa 100644 --- a/hledger-web/Hledger/Web/Test.hs +++ b/hledger-web/Hledger/Web/Test.hs @@ -44,6 +44,11 @@ module Hledger.Web.Test ( import Data.String (fromString) import Data.Function ((&)) import Data.Text qualified as T +import Data.Text.IO qualified as TIO +import Data.Text.Lazy qualified as TL +import Data.Text.Lazy.Encoding qualified as TLE +import System.Directory (getTemporaryDirectory) +import System.FilePath (()) import Test.Hspec (hspec) import Yesod.Default.Config import Yesod.Test @@ -85,6 +90,31 @@ runTests testsdesc rawopts j tests = do app <- makeAppWith j yconf wopts hspec $ yesodSpec app $ ydescribe testsdesc tests -- https://hackage.haskell.org/package/yesod-test/docs/Yesod-Test.html +-- | Assert that a journal file on disk does not contain the given text, +-- ie that a request which should have been refused did not write to it. +journalFileLacks :: FilePath -> T.Text -> YesodExample App () +journalFileLacks f t = do + txt <- liftIO $ TIO.readFile f + assertEq (f ++ " should not contain " ++ T.unpack t) (T.isInfixOf t txt) False + +-- | The name of the edit form's textarea in the current page. The edit form +-- does not name that field, so yesod generates one (eg "f1"); find it rather +-- than hardcode it, or a post can silently do nothing. +editFieldName :: YesodExample App T.Text +editFieldName = do + els <- htmlQuery "textarea" + case els of + [] -> error' "no textarea in the edit form" + (e:_) -> do + let needle = "name=\"" + html = TL.toStrict (TLE.decodeUtf8 e) + (_, fromneedle) = T.breakOn needle html + afterneedle = T.drop (T.length needle) fromneedle + fieldname = T.takeWhile (/= '"') afterneedle + if T.null fromneedle + then error' "the edit form's textarea has no name" + else return fieldname + -- | Run hledger-web's built-in tests using the hspec test runner. hledgerWebTest :: IO () hledgerWebTest = do @@ -168,3 +198,116 @@ hledgerWebTest = do -- bodyContains "href=\"https://base" -- bodyContains "src=\"https://files" + -- Tests for the write side: yesod's CSRF protection, and the restriction of + -- file access to the journal's own files. These use a journal in a temp file, + -- so that if one of these protections ever fails, the test writes there + -- rather than to the journal the developer happens to have configured. + tmpdir <- getTemporaryDirectory + let + jfile = tmpdir "hledger-web-test.journal" + jtext = T.pack $ unlines + ["2025-01-01 gift" + ," assets:bank:checking 10" + ," income:gifts" + ] + -- A path is only editable if it is one of the journal's own files, so + -- these must all be refused however they are spelled. + otherfiles = + ["/etc/passwd" + ,"../../../../etc/passwd" + ,"....//....//etc/passwd" + ,jfile ++ "/../../etc/passwd" + ] + TIO.writeFile jfile jtext + let wiopts = rawOptsToInputOpts d usecolor $ mkRawOpts [("file", jfile)] + wpj <- readJournal'' jtext + wj <- fmap (either error' id) . runExceptT $ journalFinalise wiopts jfile jtext wpj + runTests "hledger-web write requests" [("file", jfile), ("allow", "edit")] wj $ do + + yit "puts a CSRF token in the add form" $ do + get JournalR + statusIs 200 + bodyContains "name=\"_token\"" + + -- These three post the same valid, balanced transaction, and differ only + -- in the CSRF token, so that the two failures can only be about the token. + -- The form is wrapped in identifyForm, so _formid must be sent too, or the + -- post is ignored as FormMissing and these would pass either way. + let postTransaction desc = do + setMethod "POST" + setUrl AddR + addPostParam "_formid" "identify-add" + addPostParam "date" "2025-02-02" + addPostParam "description" desc + addPostParam "account" "assets:bank:checking" + addPostParam "amount" "1" + addPostParam "account" "income:gifts" + addPostParam "amount" "" + + yit "does not add a transaction when the CSRF token is missing" $ do + request $ postTransaction "CsrfNoToken" + bodyNotContains "Transaction added" + journalFileLacks jfile "CsrfNoToken" + + yit "does not add a transaction when the CSRF token is wrong" $ do + request $ do + postTransaction "CsrfBadToken" + addPostParam "_token" "not-the-token" + bodyNotContains "Transaction added" + journalFileLacks jfile "CsrfBadToken" + + -- The control for the two tests above: the same request, with a real + -- token, is accepted. Without this they could pass for the wrong reason. + yit "adds a transaction when the CSRF token is present" $ do + get JournalR + statusIs 200 + request $ do + postTransaction "CsrfGoodToken" + addToken -- from the page just fetched + statusIs 303 -- a successful add redirects to the journal + _ <- followRedirect + bodyContains "Transaction added" + txt <- liftIO $ TIO.readFile jfile + assertEq "journal should contain the added transaction" + (T.isInfixOf "CsrfGoodToken" txt) True + + -- Likewise for the edit form: the same save, with and without the token. + let editJournal fld desc = do + setMethod "POST" + setUrl (EditR jfile) + addPostParam "_formid" "identify-edit" + addPostParam fld $ + "2025-03-03 " <> desc <> "\n assets:bank:checking 1\n income:gifts\n" + + yit "does not save the journal when the CSRF token is missing" $ do + get (EditR jfile) + statusIs 200 + fld <- editFieldName + request $ editJournal fld "CsrfEdit" + bodyNotContains "Saved journal" + journalFileLacks jfile "CsrfEdit" + + yit "saves the journal when the CSRF token is present" $ do + get (EditR jfile) + statusIs 200 + fld <- editFieldName + request $ do + editJournal fld "CsrfEditOk" + addToken -- from the page just fetched + txt <- liftIO $ TIO.readFile jfile + assertEq "journal should contain the saved text" + (T.isInfixOf "CsrfEditOk" txt) True + + yit "serves its own journal file for editing" $ do + get (EditR jfile) + statusIs 200 + + forM_ otherfiles $ \otherfile -> do + yit ("refuses to edit " ++ otherfile) $ do + get (EditR otherfile) + statusIs 404 + yit ("refuses to download " ++ otherfile) $ do + get (DownloadR otherfile) + statusIs 404 + + diff --git a/hledger-web/test/browser/.gitignore b/hledger-web/test/browser/.gitignore new file mode 100644 index 00000000000..dbd64df8309 --- /dev/null +++ b/hledger-web/test/browser/.gitignore @@ -0,0 +1,3 @@ +node_modules/ +test-results/ +playwright-report/ diff --git a/hledger-web/test/browser/README.md b/hledger-web/test/browser/README.md new file mode 100644 index 00000000000..874630fb8d0 --- /dev/null +++ b/hledger-web/test/browser/README.md @@ -0,0 +1,58 @@ +# hledger-web browser tests + +End-to-end tests for the hledger-web UI, using [Playwright](https://playwright.dev). +Unlike the yesod-test suite (`Hledger/Web/Test.hs`), these run a real browser, so +they cover the parts of hledger-web that only exist once javascript runs: the add +form, autocomplete, the date picker, keyboard shortcuts, sidebar state, and hash +highlighting. + +- `webui.spec.js` — the UI's current behavior, so that changes to it are deliberate. +- `security.spec.js` — journal data is rendered as text and not markup, including the + data handed to the autocomplete's javascript. `fixture.journal` deliberately + contains html/javascript payloads for this. + +Nothing here is part of `stack build` or `stack test`; the suite is opt-in and needs +node only to run it. + +Each run starts its own hledger-web on port 5099 (override with `HLEDGER_WEB_PORT`) +against a scratch copy of `fixture.journal`, and stops it afterwards. The tests add +and edit transactions, so they need `--allow=edit`, which the setup passes. + +## Setup (once) + +Note: running these tests means installing and running javascript tooling from +the npm registry, and a browser it downloads. That is more exposure than the +rest of hledger's test suites carry, and the npm ecosystem has a history of +compromised packages. The settings below reduce the risk but do not remove it; +if that is not a trade you want to make on a machine you care about, run these +tests in a container or a throwaway environment, or not at all. The Haskell +suites (`stack test`) need none of this. + +Uses [pnpm](https://pnpm.io) (>= 10). Config is in `pnpm-workspace.yaml`: +- `onlyBuiltDependencies: []` (no dependency build scripts run) and +- `minimumReleaseAge: 1440` (nothing published in the last 24h). +- `pnpm-lock.yaml` is committed. The one dependency is `@playwright/test`. + +```sh +corepack enable # use the pnpm version pinned in package.json +cd hledger-web/test/browser +pnpm install --frozen-lockfile # install exactly what pnpm-lock.yaml pins +pnpm exec playwright install chromium # download the test browser +``` + +## Run + + # using a hledger-web binary on $PATH: + pnpm test + + # or say how to run hledger-web (eg to test your working copy): + HLEDGER_WEB="stack exec -- hledger-web" pnpm test + + # just the security tests: + pnpm test security + + # watch the browser while it runs: + pnpm test:headed + + # run one test: + pnpm exec playwright test -g "adds a transaction" diff --git a/hledger-web/test/browser/fixture.journal b/hledger-web/test/browser/fixture.journal new file mode 100644 index 00000000000..5465b5f0def --- /dev/null +++ b/hledger-web/test/browser/fixture.journal @@ -0,0 +1,34 @@ +; Test journal for the e2e suite. Tests add to and edit a scratch copy of +; this file, so it is safe to modify. +; +; The last transaction deliberately carries html/javascript payloads in a +; description, an account name and a comment. security.spec.js asserts that +; every view renders them as text. Please keep them here. + +2025-01-01 opening balances + assets:bank:checking 1000.00 + equity:opening + +2025-01-05 Grocer Green | weekly shop + expenses:food:groceries 52.10 + assets:bank:checking + +2025-01-10 Metro Transit + expenses:transport:transit 21.50 + assets:bank:checking + +2025-02-01 * Employer Inc | payroll + assets:bank:checking 3200.00 + income:salary + +2025-02-03 Grocer Green | weekly shop + expenses:food:groceries 48.75 + assets:bank:checking + +2025-02-14 Cafe Luna + expenses:food:dining 18.00 + assets:bank:checking + +2025-03-01 Payee | desc ; comment + expenses:food:dining:xss 1.00 + assets:bank:checking diff --git a/hledger-web/test/browser/global-setup.js b/hledger-web/test/browser/global-setup.js new file mode 100644 index 00000000000..1f1451484ae --- /dev/null +++ b/hledger-web/test/browser/global-setup.js @@ -0,0 +1,54 @@ +// Start hledger-web for the browser test run, on a scratch copy of fixture.journal +// (tests add and edit transactions, so the journal must be disposable). +// +// The binary is located by, in order: +// 1. $HLEDGER_WEB (a command, may contain spaces, e.g. "stack exec -- hledger-web") +// 2. `stack exec -- hledger-web` if a stack project is detected two dirs up +// 3. plain `hledger-web` from $PATH +const { spawn } = require('child_process'); +const fs = require('fs'); +const os = require('os'); +const path = require('path'); +const http = require('http'); + +const PORT = process.env.HLEDGER_WEB_PORT || '5099'; +const URL = process.env.HLEDGER_WEB_URL || `http://127.0.0.1:${PORT}`; + +function serverCommand() { + if (process.env.HLEDGER_WEB) return process.env.HLEDGER_WEB.split(/\s+/); + const repoRoot = path.resolve(__dirname, '..', '..', '..'); + if (fs.existsSync(path.join(repoRoot, 'stack.yaml'))) + return ['stack', 'exec', '--', 'hledger-web']; + return ['hledger-web']; +} + +function waitForServer(url, timeoutMs) { + const deadline = Date.now() + timeoutMs; + return new Promise((resolve, reject) => { + (function poll() { + http.get(url + '/journal', res => { + res.resume(); + res.statusCode < 500 ? resolve() : retry(); + }).on('error', retry); + function retry() { + if (Date.now() > deadline) return reject(new Error(`hledger-web did not start at ${url}`)); + setTimeout(poll, 300); + } + })(); + }); +} + +module.exports = async () => { + const journal = path.join(os.tmpdir(), `hledger-web-browser-${process.pid}.journal`); + fs.copyFileSync(path.join(__dirname, 'fixture.journal'), journal); + process.env.BROWSER_JOURNAL = journal; + + const [cmd, ...args] = serverCommand(); + const child = spawn(cmd, [...args, + '-f', journal, '--serve', '--host', '127.0.0.1', '--port', PORT, '--allow=edit', + ], { stdio: 'ignore', detached: true }); + child.unref(); + fs.writeFileSync(path.join(os.tmpdir(), 'hledger-web-browser.pid'), String(child.pid)); + + await waitForServer(URL, 60000); +}; diff --git a/hledger-web/test/browser/global-teardown.js b/hledger-web/test/browser/global-teardown.js new file mode 100644 index 00000000000..2a7d03ea212 --- /dev/null +++ b/hledger-web/test/browser/global-teardown.js @@ -0,0 +1,15 @@ +const fs = require('fs'); +const os = require('os'); +const path = require('path'); + +module.exports = async () => { + const pidFile = path.join(os.tmpdir(), 'hledger-web-browser.pid'); + try { + const pid = parseInt(fs.readFileSync(pidFile, 'utf8'), 10); + if (pid) process.kill(pid); + fs.unlinkSync(pidFile); + } catch (e) { /* already gone */ } + try { + if (process.env.BROWSER_JOURNAL) fs.unlinkSync(process.env.BROWSER_JOURNAL); + } catch (e) { /* already gone */ } +}; diff --git a/hledger-web/test/browser/package.json b/hledger-web/test/browser/package.json new file mode 100644 index 00000000000..02163113df3 --- /dev/null +++ b/hledger-web/test/browser/package.json @@ -0,0 +1,13 @@ +{ + "name": "hledger-web-browser", + "private": true, + "description": "Browser end-to-end tests for hledger-web (Playwright). See README.md.", + "scripts": { + "test": "playwright test", + "test:headed": "playwright test --headed" + }, + "devDependencies": { + "@playwright/test": "^1.49.0" + }, + "packageManager": "pnpm@10.33.0" +} diff --git a/hledger-web/test/browser/playwright.config.js b/hledger-web/test/browser/playwright.config.js new file mode 100644 index 00000000000..ec8689b388b --- /dev/null +++ b/hledger-web/test/browser/playwright.config.js @@ -0,0 +1,17 @@ +// Playwright config for hledger-web browser tests. +// The web server is started per-run by global-setup.js (see there for +// how the hledger-web binary is located and the test journal prepared). +const { defineConfig } = require('@playwright/test'); + +module.exports = defineConfig({ + testDir: '.', + globalSetup: './global-setup.js', + globalTeardown: './global-teardown.js', + timeout: 30000, + // hledger-web mutates one shared journal file; keep tests serial. + workers: 1, + use: { + baseURL: process.env.HLEDGER_WEB_URL || 'http://127.0.0.1:5099', + }, + reporter: [['list']], +}); diff --git a/hledger-web/test/browser/pnpm-lock.yaml b/hledger-web/test/browser/pnpm-lock.yaml new file mode 100644 index 00000000000..0d9c688d9be --- /dev/null +++ b/hledger-web/test/browser/pnpm-lock.yaml @@ -0,0 +1,52 @@ +lockfileVersion: '9.0' + +settings: + autoInstallPeers: true + excludeLinksFromLockfile: false + +importers: + + .: + devDependencies: + '@playwright/test': + specifier: ^1.49.0 + version: 1.62.1 + +packages: + + '@playwright/test@1.62.1': + resolution: {integrity: sha512-DTcUc8qii+cpHvtOwggMtBRMjKZHXYWdw8syRYu2vtzuq4Wxphqq4NfCs5Zt44L6mA8rfDfj+PHnxFc/FeK6mQ==} + engines: {node: '>=20'} + hasBin: true + + fsevents@2.3.2: + resolution: {integrity: sha512-xiqMQR4xAeHTuB9uWm+fFRcIOgKBMiOBP+eXiyT7jsgVCq1bkVygt00oASowB7EdtpOHaaPgKt812P9ab+DDKA==} + engines: {node: ^8.16.0 || ^10.6.0 || >=11.0.0} + os: [darwin] + + playwright-core@1.62.1: + resolution: {integrity: sha512-wPYSwEBJY9GHraISXqyqtx0na0LpO3XEX7jNDhntbex7tzUS7kLnZsOlFruFJB4Hi/rhDMjXGqHewDZ68nYZVw==} + engines: {node: '>=20'} + hasBin: true + + playwright@1.62.1: + resolution: {integrity: sha512-0M+L3LAD8/nm554LOla9Ayx0j0tmFZ0FBcoQ7F1VuVHpM/XpiC8RcDzBQB8W5+hA8L22THxELzeF+2WcUzvcLg==} + engines: {node: '>=20'} + hasBin: true + +snapshots: + + '@playwright/test@1.62.1': + dependencies: + playwright: 1.62.1 + + fsevents@2.3.2: + optional: true + + playwright-core@1.62.1: {} + + playwright@1.62.1: + dependencies: + playwright-core: 1.62.1 + optionalDependencies: + fsevents: 2.3.2 diff --git a/hledger-web/test/browser/pnpm-workspace.yaml b/hledger-web/test/browser/pnpm-workspace.yaml new file mode 100644 index 00000000000..7959e356a5d --- /dev/null +++ b/hledger-web/test/browser/pnpm-workspace.yaml @@ -0,0 +1,2 @@ +minimumReleaseAge: 1440 +onlyBuiltDependencies: [] diff --git a/hledger-web/test/browser/security.spec.js b/hledger-web/test/browser/security.spec.js new file mode 100644 index 00000000000..3d0d573fc11 --- /dev/null +++ b/hledger-web/test/browser/security.spec.js @@ -0,0 +1,83 @@ +// Security regression tests for the hledger-web UI. +// +// hledger-web is meant to be run locally, but journal data is not always +// trusted (imported CSV, shared files), and the browser is a hostile place to +// interpolate text. Yesod escapes template output by default; these tests +// exist so that a future refactor cannot quietly opt out of it. +// +// Run with: npx playwright test security (see README.md) +const { test, expect } = require('@playwright/test'); + +// A payload that executes if it is ever inserted as markup rather than text. +// Tests assert window.__xss stays undefined and the text is shown literally. +const PAYLOAD = ''; + +let pageErrors; +test.beforeEach(({ page }) => { + pageErrors = []; + page.on('pageerror', err => pageErrors.push(String(err))); +}); + +async function xssFired(page) { + return page.evaluate(() => window.__xss !== undefined); +} + +test.describe('journal data is rendered as text, not markup', () => { + + // fixture.journal carries the payload in a description, an account name and + // a comment, so every view that renders them is covered. + test('the journal view escapes a payload in a description', async ({ page }) => { + await page.goto('/journal'); + expect(await xssFired(page)).toBe(false); + await expect(page.locator('#main-content')).toContainText(PAYLOAD); + expect(await page.locator('#main-content img[src="x"]').count()).toBe(0); + }); + + test('the register view escapes a payload in a description', async ({ page }) => { + await page.goto('/register?q=inacct:expenses:food:dining'); + expect(await xssFired(page)).toBe(false); + expect(await page.locator('#main-content img[src="x"]').count()).toBe(0); + }); + + test('the sidebar escapes a payload in an account name', async ({ page }) => { + await page.goto('/journal'); + await expect(page.locator('#sidebar-menu')).toContainText('xss