From f96abc6c128e22d66be04a49b87a60fcd73bab63 Mon Sep 17 00:00:00 2001 From: Damon Blais Date: Wed, 11 Nov 2026 12:43:15 -0700 Subject: [PATCH 1/2] fix(walk): write a screenshot only under a validated name in the shots folder --- src/walk.test.ts | 16 +++++++++++++++- src/walk.ts | 30 ++++++++++++++++++++++++------ 2 files changed, 39 insertions(+), 7 deletions(-) diff --git a/src/walk.test.ts b/src/walk.test.ts index e4363fb..8adc965 100644 --- a/src/walk.test.ts +++ b/src/walk.test.ts @@ -1,5 +1,6 @@ import { describe, expect, test } from 'bun:test' -import { shotBytes } from './walk.ts' +import { resolve } from 'node:path' +import { shotBytes, shotFile } from './walk.ts' const png = Buffer.concat([ Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]), @@ -21,3 +22,16 @@ describe('shotBytes', () => { expect(() => shotBytes(huge)).toThrow('over') }) }) + +describe('shotFile', () => { + test('puts the PNG under the shots directory', () => { + expect(shotFile('/tmp/shots', 'home')).toBe(resolve('/tmp/shots', 'home.png')) + }) + + test('refuses a name that would leave the shots directory', () => { + expect(() => shotFile('/tmp/shots', '../outside')).toThrow('file name') + expect(() => shotFile('/tmp/shots', 'a/b')).toThrow('file name') + expect(() => shotFile('/tmp/shots', '..')).toThrow('file name') + expect(() => shotFile('/tmp/shots', '')).toThrow('file name') + }) +}) diff --git a/src/walk.ts b/src/walk.ts index 72562a6..8f8feea 100644 --- a/src/walk.ts +++ b/src/walk.ts @@ -2,7 +2,7 @@ import { spawn } from 'node:child_process' import { existsSync } from 'node:fs' import { mkdir, writeFile } from 'node:fs/promises' import { createServer } from 'node:net' -import { join } from 'node:path' +import { basename, isAbsolute, join, relative, resolve } from 'node:path' import process from 'node:process' import { setTimeout as sleep } from 'node:timers/promises' import { collapsed, covered, type LayoutBreak, overflow } from './page/layout.ts' @@ -137,8 +137,26 @@ export function shotBytes(answer: unknown): Buffer { return bytes } -async function shoot({ base, id }: Session, file: string) { - await writeFile(file, shotBytes(await call(base, 'GET', `/session/${id}/screenshot`))) +/** A PNG path under dir named for that screen: a slash or `..` would otherwise let the driver's bytes land outside + * the shots folder. */ +export function shotFile(dir: string, name: string): string { + if (name === '' || name === '.' || name === '..' || name !== basename(name)) { + throw new Error('screenshot name is not a file name') + } + const root = resolve(dir) + const dest = resolve(root, `${name}.png`) + const rel = relative(root, dest) + if (rel.startsWith('..') || isAbsolute(rel)) { + throw new Error('screenshot path leaves the shots directory') + } + return dest +} + +async function shoot({ base, id }: Session, dir: string, name: string) { + await writeFile( + shotFile(dir, name), + shotBytes(await call(base, 'GET', `/session/${id}/screenshot`)), + ) } // Reaches one screen from the one before, reloading only when a step finds nothing, and returns its layout breaks. The @@ -149,7 +167,7 @@ async function visit(session: Session, screen: Screen, w: Walk): Promise Date: Wed, 11 Nov 2026 12:43:32 -0700 Subject: [PATCH 2/2] fix(walk): alias path.resolve so it does not shadow Promise resolve --- src/walk.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/walk.ts b/src/walk.ts index 8f8feea..fc71fbf 100644 --- a/src/walk.ts +++ b/src/walk.ts @@ -2,7 +2,7 @@ import { spawn } from 'node:child_process' import { existsSync } from 'node:fs' import { mkdir, writeFile } from 'node:fs/promises' import { createServer } from 'node:net' -import { basename, isAbsolute, join, relative, resolve } from 'node:path' +import { basename, isAbsolute, join, relative, resolve as resolvePath } from 'node:path' import process from 'node:process' import { setTimeout as sleep } from 'node:timers/promises' import { collapsed, covered, type LayoutBreak, overflow } from './page/layout.ts' @@ -143,8 +143,8 @@ export function shotFile(dir: string, name: string): string { if (name === '' || name === '.' || name === '..' || name !== basename(name)) { throw new Error('screenshot name is not a file name') } - const root = resolve(dir) - const dest = resolve(root, `${name}.png`) + const root = resolvePath(dir) + const dest = resolvePath(root, `${name}.png`) const rel = relative(root, dest) if (rel.startsWith('..') || isAbsolute(rel)) { throw new Error('screenshot path leaves the shots directory')