From ff8dc81e96effbd309e7dc358bf53ca9bcdf5645 Mon Sep 17 00:00:00 2001 From: dprevost-perso Date: Sun, 20 Sep 2026 22:31:21 -0400 Subject: [PATCH 1/7] Review 1 --- .husky/pre-commit | 1 + .husky/pre-push | 1 + README.md | 12 ++-- .../node-safaridriver/.github/FUNDING.yml | 3 - .../node-safaridriver/.github/dependabot.yml | 34 ---------- .../.github/workflows/audit.yml | 35 ---------- .../.github/workflows/expense.yml | 65 ------------------- 7 files changed, 6 insertions(+), 145 deletions(-) create mode 100755 .husky/pre-commit create mode 100755 .husky/pre-push delete mode 100644 packages/node-safaridriver/.github/FUNDING.yml delete mode 100644 packages/node-safaridriver/.github/dependabot.yml delete mode 100644 packages/node-safaridriver/.github/workflows/audit.yml delete mode 100644 packages/node-safaridriver/.github/workflows/expense.yml diff --git a/.husky/pre-commit b/.husky/pre-commit new file mode 100755 index 0000000..40fb2cd --- /dev/null +++ b/.husky/pre-commit @@ -0,0 +1 @@ +pnpm run test:lint diff --git a/.husky/pre-push b/.husky/pre-push new file mode 100755 index 0000000..98475b5 --- /dev/null +++ b/.husky/pre-push @@ -0,0 +1 @@ +pnpm test diff --git a/README.md b/README.md index 110feec..f80523d 100644 --- a/README.md +++ b/README.md @@ -106,7 +106,7 @@ const browser = await remote({ } }); -await browser.url('[https://webdriver.io](https://webdriver.io)'); +await browser.url('https://webdriver.io'); console.log(await browser.getTitle()); // 4. Terminate process when finished @@ -121,7 +121,7 @@ import safaridriver from 'safaridriver'; import { remote } from 'webdriverio'; // 1. Start Safaridriver server -await safaridriver.start({ port: 4444 }); +safaridriver.start({ port: 4444 }); // 2. Connect WebdriverIO session const browser = await remote({ @@ -130,7 +130,7 @@ const browser = await remote({ } }); -await browser.url('[https://webdriver.io](https://webdriver.io)'); +await browser.url('https://webdriver.io'); console.log(await browser.getTitle()); // 3. Stop Safaridriver process @@ -172,8 +172,4 @@ Passed into `safaridriver.start(options)`: ## License -[MIT](https://www.google.com/search?q=LICENSE&utm_source=gemini) - -``` - -``` +[MIT](LICENSE) diff --git a/packages/node-safaridriver/.github/FUNDING.yml b/packages/node-safaridriver/.github/FUNDING.yml deleted file mode 100644 index 5ec869a..0000000 --- a/packages/node-safaridriver/.github/FUNDING.yml +++ /dev/null @@ -1,3 +0,0 @@ -tidelift: "npm/safaridriver" -open_collective: webdriverio -github: [christian-bromann,webdriverio] diff --git a/packages/node-safaridriver/.github/dependabot.yml b/packages/node-safaridriver/.github/dependabot.yml deleted file mode 100644 index 9a8fcd2..0000000 --- a/packages/node-safaridriver/.github/dependabot.yml +++ /dev/null @@ -1,34 +0,0 @@ -version: 2 -updates: -- package-ecosystem: npm - directory: "/" - schedule: - interval: weekly - open-pull-requests-limit: 10 - - groups: - patch-deps-updates-main: - update-types: - - "patch" - minor-deps-updates-main: - update-types: - - "minor" - major-deps-updates-main: - update-types: - - "major" -- package-ecosystem: github-actions - directory: "/" - schedule: - interval: weekly - time: "11:00" - open-pull-requests-limit: 10 - groups: - patch-deps-updates: - update-types: - - "patch" - minor-deps-updates: - update-types: - - "minor" - major-deps-updates: - update-types: - - "major" diff --git a/packages/node-safaridriver/.github/workflows/audit.yml b/packages/node-safaridriver/.github/workflows/audit.yml deleted file mode 100644 index 22ebbc0..0000000 --- a/packages/node-safaridriver/.github/workflows/audit.yml +++ /dev/null @@ -1,35 +0,0 @@ -name: Audit -on: - push: - branches: - - main - paths-ignore: - - '**.md' - pull_request: - paths-ignore: - - '**.md' - -jobs: - - build: - name: Audit - - runs-on: ubuntu-latest - - steps: - - name: Checkout - uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6 - - - name: Set Node.js 20.x - uses: actions/setup-node@395ad3262231945c25e8478fd5baf05154b1d79f # v6 - with: - node-version: 20.x - - - name: npm install - run: npm install - - - name: Build - run: npm run build - - - name: Audit - run: npm audit --audit-level=moderate --omit=dev diff --git a/packages/node-safaridriver/.github/workflows/expense.yml b/packages/node-safaridriver/.github/workflows/expense.yml deleted file mode 100644 index dae02b4..0000000 --- a/packages/node-safaridriver/.github/workflows/expense.yml +++ /dev/null @@ -1,65 +0,0 @@ -name: Expense Contribution - -on: - workflow_dispatch: - inputs: - prNumber: - description: "Number of the PR (without #)" - required: true - amount: - description: "The expense amount you like to grant for the contribution in $" - required: true - type: choice - options: - - "5" - - "10" - - "15" - - "25" - - "35" - - "50" - - "100" - - "150" - - "200" - - "250" - - "300" - - "350" - - "400" - - "450" - - "500" - - "550" - - "600" - - "650" - - "700" - - "750" - - "800" - - "850" - - "900" - - "950" - - "1000" - -jobs: - authorize: - runs-on: ubuntu-latest - steps: - - uses: octokit/request-action@dad4362715b7fb2ddedf9772c8670824af564f0d # v2.4.0 - with: - route: GET /orgs/:organisation/teams/:team/memberships/${{ github.actor }} - team: technical-steering-committee - organisation: webdriverio - env: - GITHUB_TOKEN: ${{ secrets.WDIO_BOT_GITHUB_TOKEN }} - expense: - permissions: - contents: write - id-token: write - needs: [authorize] - runs-on: ubuntu-latest - steps: - - name: Run Expense Flow - uses: webdriverio/expense-action@fbcf7b225dfc474104d0830fe50a9b356bcec810 # v1 - with: - prNumber: ${{ github.event.inputs.prNumber }} - amount: ${{ github.event.inputs.amount }} - env: - RESEND_API_KEY: ${{ secrets.RESEND_API_KEY }} - GH_TOKEN: ${{ secrets.WDIO_BOT_GITHUB_TOKEN }} From b9121eb1a9f3680751decf6687bf80575eeb590c Mon Sep 17 00:00:00 2001 From: dprevost-perso Date: Sun, 20 Sep 2026 22:40:04 -0400 Subject: [PATCH 2/7] Review 2 --- README.md | 22 +++++++++++++++++++--- packages/node-edgedriver/README.md | 6 +++--- packages/node-edgedriver/src/types.ts | 2 +- packages/node-geckodriver/README.md | 6 +++--- packages/node-geckodriver/src/types.ts | 4 ++-- packages/node-safaridriver/README.md | 8 ++++---- 6 files changed, 32 insertions(+), 16 deletions(-) diff --git a/README.md b/README.md index f80523d..701971c 100644 --- a/README.md +++ b/README.md @@ -142,7 +142,7 @@ safaridriver.stop(); ## Configuration & Options -### Options for `geckodriver` & `edgedriver` +### Options for `geckodriver` Passed into the `start(options)` method: @@ -150,12 +150,28 @@ Passed into the `start(options)` method: | --- | --- | --- | --- | | `port` | `number` | — | Port to listen on. | | `host` | `string` | `0.0.0.0` | Host IP address to bind server. | -| `customGeckoDriverPath` / `customEdgeDriverPath` | `string` | `process.env.GECKODRIVER_PATH` | Path to custom/cached driver binary. | -| `cacheDir` | `string` | `os.tmpdir()` | Root directory for caching downloaded binaries. | +| `customGeckoDriverPath` | `string` | `process.env.GECKODRIVER_PATH` | Path to custom/cached driver binary. | +| `cacheDir` | `string` | `process.env.GECKODRIVER_CACHE_DIR \|\| os.tmpdir()` | Root directory for caching downloaded binaries. | | `spawnOpts` | `object` | `undefined` | Spawn options passed directly to Node.js `child_process.spawn`. | | `allowHosts` | `string[]` | `[]` | List of explicit host names allowed to connect. | | `allowOrigins` | `string[]` | `[]` | List of allowed request origins (`scheme://host:port`). | +See the [geckodriver README](packages/node-geckodriver/README.md) for the full list of options. + +### Options for `edgedriver` + +Passed into the `start(options)` method: + +| Option | Type | Default | Description | +| --- | --- | --- | --- | +| `port` | `number` | — | Port to listen on. | +| `customEdgeDriverPath` | `string` | `process.env.EDGEDRIVER_PATH` | Path to custom/cached driver binary. | +| `cacheDir` | `string` | `process.env.EDGEDRIVER_CACHE_DIR \|\| os.tmpdir()` | Root directory for caching downloaded binaries. | +| `allowedIps` | `string[]` | `['']` | List of remote IP addresses allowed to connect. | +| `allowedOrigins` | `string[]` | `['*']` | List of allowed request origins. Using `*` to allow any origin is dangerous! | + +See the [edgedriver README](packages/node-edgedriver/README.md) for the full list of options. + ### Options for `safaridriver` Passed into `safaridriver.start(options)`: diff --git a/packages/node-edgedriver/README.md b/packages/node-edgedriver/README.md index 112cae9..ca33df3 100644 --- a/packages/node-edgedriver/README.md +++ b/packages/node-edgedriver/README.md @@ -1,7 +1,7 @@ EdgeDriver [![CI](https://github.com/webdriverio-community/node-edgedriver/actions/workflows/ci.yml/badge.svg)](https://github.com/webdriverio-community/node-edgedriver/actions/workflows/ci.yml) [![Audit](https://github.com/webdriverio-community/node-edgedriver/actions/workflows/audit.yml/badge.svg)](https://github.com/webdriverio-community/node-edgedriver/actions/workflows/audit.yml) ========== -An NPM wrapper for Microsofts' [EdgeDriver](https://developer.microsoft.com/en-us/microsoft-edge/tools/webdriver/). It manages to download various (or the latest) Edgedriver versions and provides a programmatic interface to start and stop it within Node.js. __Note:__ this is a wrapper module. If you discover any bugs with EdgeDriver, please report them in the [official repository](https://github.com/MicrosoftEdge/EdgeWebDriver). +An NPM wrapper for Microsoft's [EdgeDriver](https://developer.microsoft.com/en-us/microsoft-edge/tools/webdriver/). It manages to download various (or the latest) Edgedriver versions and provides a programmatic interface to start and stop it within Node.js. __Note:__ this is a wrapper module. If you discover any bugs with EdgeDriver, please report them in the [official repository](https://github.com/MicrosoftEdge/EdgeWebDriver). # Installing @@ -23,7 +23,7 @@ By default, this package downloads Edgedriver when used for the first time throu EDGEDRIVER_AUTO_INSTALL=1 npm i ``` -To get a list of available CLI options run `npx edgedriver --help`. By default this package tries to find the Mircosoft Edge version installed on a given system. If you prefer to have it install a custom EdgeDriver version you can define the environment variable `EDGEDRIVER_VERSION` when running in CLI, e.g.: +To get a list of available CLI options run `npx edgedriver --help`. By default this package tries to find the Microsoft Edge version installed on a given system. If you prefer to have it install a custom EdgeDriver version you can define the environment variable `EDGEDRIVER_VERSION` when running in CLI, e.g.: ```sh $ npm i edgedriver @@ -124,7 +124,7 @@ The `start` method offers the following options to be passed on to the actual Ed ### edgeDriverVersion -The version of EdgeDriver to start. See [Egdedriver directory list](https://msedgewebdriverstorage.z22.web.core.windows.net/) for all available versions, platforms and architecture. +The version of EdgeDriver to start. See [Edgedriver directory list](https://msedgewebdriverstorage.z22.web.core.windows.net/) for all available versions, platforms and architecture. Type: `number`
Default: `latest` diff --git a/packages/node-edgedriver/src/types.ts b/packages/node-edgedriver/src/types.ts index e924678..dfeca66 100644 --- a/packages/node-edgedriver/src/types.ts +++ b/packages/node-edgedriver/src/types.ts @@ -68,7 +68,7 @@ export interface EdgedriverParameters { allowedOrigins?: string[] /** * The path to the root of the cache directory. - * @default process.env.GECKODRIVER_CACHE_DIR || os.tmpdir() + * @default process.env.EDGEDRIVER_CACHE_DIR || os.tmpdir() */ cacheDir?: string } diff --git a/packages/node-geckodriver/README.md b/packages/node-geckodriver/README.md index f1cc841..2f1ea96 100644 --- a/packages/node-geckodriver/README.md +++ b/packages/node-geckodriver/README.md @@ -173,15 +173,15 @@ Set Gecko log level [possible values: `fatal`, `error`, `warn`, `info`, `config` Type: `string` -### `logNoTruncated` -Write server log to file instead of stderr, increases log level to `INFO`. +### `logNoTruncate` +Log verbosely and don't truncate long strings so that the log can be replayed. Type: `boolean` ### `marionetteHost` Host to use to connect to Gecko. -Type: `boolean`
+Type: `string`
Default: `127.0.0.1` ### `marionettePort` diff --git a/packages/node-geckodriver/src/types.ts b/packages/node-geckodriver/src/types.ts index a1076b4..d3d4d8a 100644 --- a/packages/node-geckodriver/src/types.ts +++ b/packages/node-geckodriver/src/types.ts @@ -29,7 +29,7 @@ export interface GeckodriverParameters { connectExisting?: boolean /** * Host IP to use for WebDriver server - * @default 127.0.0.1 + * @default 0.0.0.0 */ host?: string /** @@ -42,7 +42,7 @@ export interface GeckodriverParameters { */ log?: LogLevel /** - * write server log to file instead of stderr, increases log level to INFO + * log verbosely and don't truncate long strings so that the log can be replayed */ logNoTruncate?: boolean /** diff --git a/packages/node-safaridriver/README.md b/packages/node-safaridriver/README.md index 013c3c5..a68672b 100644 --- a/packages/node-safaridriver/README.md +++ b/packages/node-safaridriver/README.md @@ -1,6 +1,6 @@ # Safaridriver for Node.js [![CI](https://github.com/webdriverio-community/node-safaridriver/actions/workflows/ci.yml/badge.svg)](https://github.com/webdriverio-community/node-safaridriver/actions/workflows/ci.yml) [![Audit](https://github.com/webdriverio-community/node-safaridriver/actions/workflows/audit.yml/badge.svg)](https://github.com/webdriverio-community/node-safaridriver/actions/workflows/audit.yml) -> A Node.js untility to manage Safaridriver sessions. +> A Node.js utility to manage Safaridriver sessions. The Safaridriver utility is used to launch an HTTP server that implements the [WebDriver](https://w3c.github.io/webdriver/) REST API. When launched, Safaridriver allows for automated testing of web content using the version of Safari that is installed with macOS. @@ -19,7 +19,7 @@ To start a Safaridriver server, import the package and run: ```js import safaridriver from 'safaridriver' -await safaridriver.start() +safaridriver.start() // run some automation... @@ -45,7 +45,7 @@ __Default:__ `/usr/bin/safaridriver` ### `useTechnologyPreview` -If set to `true`, it will start the Safaridriver for Safari Technology Preview. Make sure you have it installed by installing Safari Technology Preview on Apple's [offical website](https://developer.apple.com/safari/technology-preview/). +If set to `true`, it will start the Safaridriver for Safari Technology Preview. Make sure you have it installed by installing Safari Technology Preview on Apple's [official website](https://developer.apple.com/safari/technology-preview/). __Type:__ `boolean`
__Default:__ `false` @@ -68,4 +68,4 @@ __Default:__ `false` ---- -For more information on WebdriverIO see the [homepage](http://webdriver.io). +For more information on WebdriverIO see the [homepage](https://webdriver.io). From f9bd78a32825a3372ac2ab72428f04dcc0713b4f Mon Sep 17 00:00:00 2001 From: dprevost-perso Date: Sun, 20 Sep 2026 22:45:11 -0400 Subject: [PATCH 3/7] Review 3 --- package.json | 2 +- packages/node-edgedriver/package.json | 4 ++-- packages/node-safaridriver/package.json | 1 - 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/package.json b/package.json index edefdba..11687da 100644 --- a/package.json +++ b/package.json @@ -17,9 +17,9 @@ "checks:all": "pnpm run test:lint && pnpm run test:unit && pnpm run test:e2e" }, "devDependencies": { + "@stylistic/eslint-plugin": "^5.10.0", "@types/node": "^25.0.3", "@types/which": "^3.0.4", - "@stylistic/eslint-plugin": "^5.10.0", "@vitest/coverage-v8": "^4.1.11", "husky": "^9.1.7", "npm-run-all2": "^9.0.3", diff --git a/packages/node-edgedriver/package.json b/packages/node-edgedriver/package.json index 389f93f..df47674 100644 --- a/packages/node-edgedriver/package.json +++ b/packages/node-edgedriver/package.json @@ -1,7 +1,7 @@ { "name": "edgedriver", "version": "6.3.1", - "description": "Microsofts' EdgeDriver for Node.js", + "description": "Microsoft's EdgeDriver for Node.js", "homepage": "https://webdriver.io", "license": "MIT", "author": { @@ -40,7 +40,7 @@ "scripts": { "build": "tsc -b . && node ../../scripts/copy-cjs-pkg.mjs", "clean": "rimraf ./dist", - "postinstall": "test -f ./dist/install.js && node ./dist/install.js || echo \"Skipping install, project not build!\"", + "postinstall": "test -f ./dist/install.js && node ./dist/install.js || echo \"Skipping install, project not built!\"", "test": "run-s test:*", "test:lint": "oxlint .", "test:unit": "vitest --run", diff --git a/packages/node-safaridriver/package.json b/packages/node-safaridriver/package.json index 1ef2bc2..8c054f9 100644 --- a/packages/node-safaridriver/package.json +++ b/packages/node-safaridriver/package.json @@ -39,7 +39,6 @@ "typeScriptVersion": "3.8.3", "scripts": { "build": "tsc -b . && node ../../scripts/copy-cjs-pkg.mjs", - "ci": "run-s lint build test", "clean": "rimraf ./dist", "test": "run-s test:*", "test:lint": "oxlint .", From facbeed939e1916542470364804952270555fffb Mon Sep 17 00:00:00 2001 From: dprevost-perso Date: Sun, 20 Sep 2026 22:55:53 -0400 Subject: [PATCH 4/7] fix: harden edgedriver install.ts against Windows and Zip Slip bugs - EDGEDRIVER_AUTO_INSTALL never fired on Windows: the postinstall entrypoint check hardcoded a forward slash (`/dist/install.js`), which never matches a backslash-separated argv[1]. Extracted into isAutoInstallEntrypoint(), using path.normalize/path.sep so it works cross-platform, matching geckodriver's existing equivalent. - downloadZip() had no protection against Zip Slip: a malicious zip entry (e.g. "../../evil.exe") could write outside the cache directory. Added the same path-containment guard geckodriver already has. Adds unit tests for both, following geckodriver's existing test patterns (mocked @zip.js/zip.js, fs/promises, hasAccess). Co-Authored-By: Claude Sonnet 5 --- packages/node-edgedriver/src/install.ts | 23 +++++- packages/node-edgedriver/tests/unit.test.ts | 85 ++++++++++++++++++++- 2 files changed, 104 insertions(+), 4 deletions(-) diff --git a/packages/node-edgedriver/src/install.ts b/packages/node-edgedriver/src/install.ts index 25f4c28..097c779 100644 --- a/packages/node-edgedriver/src/install.ts +++ b/packages/node-edgedriver/src/install.ts @@ -205,11 +205,20 @@ export async function fetchVersion (edgeVersion: string) { async function downloadZip(res: Awaited>, cacheDir: string) { const zipBlob = await res.blob() const zip = new ZipReader(new BlobReader(zipBlob)) + const resolvedCacheDir = path.resolve(cacheDir) for (const entry of await zip.getEntries()) { const unzippedFilePath = path.join(cacheDir, entry.filename) if (entry.directory) { continue } + /** + * guard against Zip Slip: a malicious archive could contain entries + * with `../` or absolute paths that escape the cache directory + */ + const resolvedPath = path.resolve(unzippedFilePath) + if (resolvedPath !== resolvedCacheDir && !resolvedPath.startsWith(resolvedCacheDir + path.sep)) { + throw new Error(`Zip entry "${entry.filename}" resolves outside the cache directory`) + } const fileEntry = entry as FileEntry if (!await hasAccess(path.dirname(unzippedFilePath))) { await fsp.mkdir(path.dirname(unzippedFilePath), { recursive: true }) @@ -227,10 +236,22 @@ function sanitizeVersion (version: string) { return version.replace(/\0/g, '').slice(2).trim() } +/** + * True when this module was invoked directly as the package's postinstall + * entrypoint (`dist/install.js`), as opposed to being imported as a dependency. + */ +export function isAutoInstallEntrypoint (argv1: string | undefined) { + if (!argv1) { + return false + } + const installJsPath = path.join('dist', 'install.js') + return path.normalize(argv1).endsWith(path.sep + installJsPath) +} + /** * download on install */ -if (process.argv[1] && process.argv[1].endsWith('/dist/install.js') && Boolean(process.env.EDGEDRIVER_AUTO_INSTALL)) { +if (isAutoInstallEntrypoint(process.argv[1]) && Boolean(process.env.EDGEDRIVER_AUTO_INSTALL)) { await download().then( () => log.info('Success!'), (err) => log.error(`Failed to install Edgedriver: ${err.stack}`) diff --git a/packages/node-edgedriver/tests/unit.test.ts b/packages/node-edgedriver/tests/unit.test.ts index bda0f60..6f999fa 100644 --- a/packages/node-edgedriver/tests/unit.test.ts +++ b/packages/node-edgedriver/tests/unit.test.ts @@ -1,18 +1,49 @@ import os from 'node:os' -import { vi, test, expect, describe } from 'vitest' +import path from 'node:path' +import fsp from 'node:fs/promises' +import { vi, test, expect, describe, beforeEach } from 'vitest' import * as pkgExports from '../src/index.js' -import { fetchVersion } from '../src/install.js' +import { fetchVersion, download, isAutoInstallEntrypoint } from '../src/install.js' import { getNameByArchitecture, parseParams, extractBasicAuthFromUrl } from '../src/utils.js' import { EDGE_PRODUCTS_API } from '../src/constants.js' +// All vi.mock calls must be at module scope so Vitest hoists them before any +// imports — mocks inside test() bodies are not hoisted. vi.mock('node:os', () => ({ default: { arch: vi.fn(), - platform: vi.fn() + platform: vi.fn(), + tmpdir: vi.fn(() => '/tmp') } })) +vi.mock('node:fs/promises', () => ({ + default: { + access: vi.fn(), + mkdir: vi.fn().mockResolvedValue(undefined), + chmod: vi.fn().mockResolvedValue(undefined), + }, + writeFile: vi.fn().mockResolvedValue(undefined), +})) + +vi.mock('../src/utils.js', async (original) => { + const actual: any = await original() + return { + ...actual, + hasAccess: vi.fn(), + } +}) + +const zipState = vi.hoisted(() => ({ entries: [] as any[] })) +vi.mock('@zip.js/zip.js', () => ({ + BlobReader: class { }, + BlobWriter: class { }, + ZipReader: class { + getEntries() { return Promise.resolve(zipState.entries) } + }, +})) + // Mock the global fetch function const mockFetch = vi.fn() vi.stubGlobal('fetch', mockFetch) @@ -183,3 +214,51 @@ test('extractBasicAuthFromUrl with invalid URL returns original', () => { expect(result.authHeader).toBeUndefined() }) +describe('isAutoInstallEntrypoint', () => { + // uses the native `path` module (matching the implementation), so this + // only proves correctness for the host OS running the test — real + // cross-platform coverage (including Windows) comes from CI running this + // same file on the macOS/Ubuntu/Windows matrix, not from simulating + // another platform's paths in-process. + test('matches when argv[1] points at dist/install.js', () => { + const installJsPath = path.join('dist', 'install.js') + expect(isAutoInstallEntrypoint(path.join('/home/user/node_modules/edgedriver', installJsPath))).toBe(true) + }) + + test('does not match when the package is only imported as a dependency', () => { + expect(isAutoInstallEntrypoint(path.join('/home/user/my-project/index.js'))).toBe(false) + }) + + test('does not match when argv[1] is undefined', () => { + expect(isAutoInstallEntrypoint(undefined)).toBe(false) + }) +}) + +describe('download', () => { + const CACHE_DIR = path.resolve(os.tmpdir(), 'test-cache') + let hasAccess: ReturnType + + beforeEach(async () => { + const utils = await import('../src/utils.js') + hasAccess = vi.mocked(utils.hasAccess) + hasAccess.mockResolvedValue(false) // cache miss + vi.mocked(fsp.mkdir).mockClear() + vi.mocked(fsp.chmod).mockClear() + mockFetch.mockReset() + }) + + test('rejects zip entries that escape the cache directory (Zip Slip)', async () => { + mockFetch.mockResolvedValue({ + ok: true, + status: 200, + body: {}, + blob: vi.fn().mockResolvedValue(new Blob([])), + }) + zipState.entries = [ + { filename: '../../evil.exe', directory: false, getData: vi.fn() }, + ] + + await expect(download('123.456.789.0', CACHE_DIR)).rejects.toThrow('resolves outside the cache directory') + }) +}) + From 7087afa68459da6ac795d6c21c9166ddf659ea87 Mon Sep 17 00:00:00 2001 From: dprevost-perso Date: Sun, 20 Sep 2026 22:59:18 -0400 Subject: [PATCH 5/7] test: split edgedriver's unit.test.ts by source module MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit unit.test.ts mixed tests for three unrelated modules (install.ts, utils.ts, index.ts) in one file, so the heavy install.ts mocking (fs/promises, @zip.js/zip.js, fetch) was inherited by the pure utils.ts tests for no reason. Split into install.test.ts, utils.test.ts, and index.test.ts, each carrying only the mocks it needs — matching geckodriver's existing convention of separate test files per concern. Also grouped extractBasicAuthFromUrl's 4 cases under a describe block, consistent with how every other multi-case function in this file is already organized. Co-Authored-By: Claude Sonnet 5 --- .../{unit.test.ts.snap => utils.test.ts.snap} | 0 packages/node-edgedriver/tests/index.test.ts | 9 +++ .../tests/{unit.test.ts => install.test.ts} | 59 ----------------- packages/node-edgedriver/tests/utils.test.ts | 63 +++++++++++++++++++ 4 files changed, 72 insertions(+), 59 deletions(-) rename packages/node-edgedriver/tests/__snapshots__/{unit.test.ts.snap => utils.test.ts.snap} (100%) create mode 100644 packages/node-edgedriver/tests/index.test.ts rename packages/node-edgedriver/tests/{unit.test.ts => install.test.ts} (73%) create mode 100644 packages/node-edgedriver/tests/utils.test.ts diff --git a/packages/node-edgedriver/tests/__snapshots__/unit.test.ts.snap b/packages/node-edgedriver/tests/__snapshots__/utils.test.ts.snap similarity index 100% rename from packages/node-edgedriver/tests/__snapshots__/unit.test.ts.snap rename to packages/node-edgedriver/tests/__snapshots__/utils.test.ts.snap diff --git a/packages/node-edgedriver/tests/index.test.ts b/packages/node-edgedriver/tests/index.test.ts new file mode 100644 index 0000000..a2ff226 --- /dev/null +++ b/packages/node-edgedriver/tests/index.test.ts @@ -0,0 +1,9 @@ +import { test, expect } from 'vitest' + +import * as pkgExports from '../src/index.js' + +test('exports', () => { + expect(typeof pkgExports.download).toBe('function') + expect(typeof pkgExports.findEdgePath).toBe('function') + expect(typeof pkgExports.start).toBe('function') +}) diff --git a/packages/node-edgedriver/tests/unit.test.ts b/packages/node-edgedriver/tests/install.test.ts similarity index 73% rename from packages/node-edgedriver/tests/unit.test.ts rename to packages/node-edgedriver/tests/install.test.ts index 6f999fa..309bcdd 100644 --- a/packages/node-edgedriver/tests/unit.test.ts +++ b/packages/node-edgedriver/tests/install.test.ts @@ -3,9 +3,7 @@ import path from 'node:path' import fsp from 'node:fs/promises' import { vi, test, expect, describe, beforeEach } from 'vitest' -import * as pkgExports from '../src/index.js' import { fetchVersion, download, isAutoInstallEntrypoint } from '../src/install.js' -import { getNameByArchitecture, parseParams, extractBasicAuthFromUrl } from '../src/utils.js' import { EDGE_PRODUCTS_API } from '../src/constants.js' // All vi.mock calls must be at module scope so Vitest hoists them before any @@ -158,62 +156,6 @@ describe('fetchVersion', () => { }) }) -test('getNameByArchitecture', () => { - vi.mocked(os.arch).mockReturnValue('arm') - vi.mocked(os.platform).mockReturnValue('linux') - expect(getNameByArchitecture()).toBe('edgedriver_linux32') - vi.mocked(os.arch).mockReturnValue('arm64') - vi.mocked(os.platform).mockReturnValue('linux') - expect(getNameByArchitecture()).toBe('edgedriver_linux64') - vi.mocked(os.arch).mockReturnValue('arm') - vi.mocked(os.platform).mockReturnValue('win32') - expect(getNameByArchitecture()).toBe('edgedriver_win32') - vi.mocked(os.arch).mockReturnValue('arm64') - vi.mocked(os.platform).mockReturnValue('win32') - expect(getNameByArchitecture()).toBe('edgedriver_win64') - vi.mocked(os.arch).mockReturnValue('x64') - vi.mocked(os.platform).mockReturnValue('darwin') - expect(getNameByArchitecture()).toBe('edgedriver_mac64') - vi.mocked(os.arch).mockReturnValue('arm64') - vi.mocked(os.platform).mockReturnValue('darwin') - expect(getNameByArchitecture()).toBe('edgedriver_mac64_m1') -}) - -test('parseParams', () => { - expect(parseParams({ baseUrl: 'foobar', silent: true, verbose: false, allowedIps: ['123', '321'] })) - .toMatchSnapshot() -}) - -test('exports', () => { - expect(typeof pkgExports.download).toBe('function') - expect(typeof pkgExports.findEdgePath).toBe('function') - expect(typeof pkgExports.start).toBe('function') -}) - -test('extractBasicAuthFromUrl with credentials', () => { - const result = extractBasicAuthFromUrl('https://myuser:mypassword@cdn.example.com/path/file.zip') - expect(result.url).toBe('https://cdn.example.com/path/file.zip') - expect(result.authHeader).toBe('Basic ' + Buffer.from('myuser:mypassword').toString('base64')) -}) - -test('extractBasicAuthFromUrl without credentials', () => { - const result = extractBasicAuthFromUrl('https://cdn.example.com/path/file.zip') - expect(result.url).toBe('https://cdn.example.com/path/file.zip') - expect(result.authHeader).toBeUndefined() -}) - -test('extractBasicAuthFromUrl with only username', () => { - const result = extractBasicAuthFromUrl('https://myuser@cdn.example.com/path/file.zip') - expect(result.url).toBe('https://cdn.example.com/path/file.zip') - expect(result.authHeader).toBe('Basic ' + Buffer.from('myuser:').toString('base64')) -}) - -test('extractBasicAuthFromUrl with invalid URL returns original', () => { - const result = extractBasicAuthFromUrl('not-a-valid-url') - expect(result.url).toBe('not-a-valid-url') - expect(result.authHeader).toBeUndefined() -}) - describe('isAutoInstallEntrypoint', () => { // uses the native `path` module (matching the implementation), so this // only proves correctness for the host OS running the test — real @@ -261,4 +203,3 @@ describe('download', () => { await expect(download('123.456.789.0', CACHE_DIR)).rejects.toThrow('resolves outside the cache directory') }) }) - diff --git a/packages/node-edgedriver/tests/utils.test.ts b/packages/node-edgedriver/tests/utils.test.ts new file mode 100644 index 0000000..8b1dab8 --- /dev/null +++ b/packages/node-edgedriver/tests/utils.test.ts @@ -0,0 +1,63 @@ +import os from 'node:os' +import { vi, test, expect, describe } from 'vitest' + +import { getNameByArchitecture, parseParams, extractBasicAuthFromUrl } from '../src/utils.js' + +vi.mock('node:os', () => ({ + default: { + arch: vi.fn(), + platform: vi.fn() + } +})) + +test('getNameByArchitecture', () => { + vi.mocked(os.arch).mockReturnValue('arm') + vi.mocked(os.platform).mockReturnValue('linux') + expect(getNameByArchitecture()).toBe('edgedriver_linux32') + vi.mocked(os.arch).mockReturnValue('arm64') + vi.mocked(os.platform).mockReturnValue('linux') + expect(getNameByArchitecture()).toBe('edgedriver_linux64') + vi.mocked(os.arch).mockReturnValue('arm') + vi.mocked(os.platform).mockReturnValue('win32') + expect(getNameByArchitecture()).toBe('edgedriver_win32') + vi.mocked(os.arch).mockReturnValue('arm64') + vi.mocked(os.platform).mockReturnValue('win32') + expect(getNameByArchitecture()).toBe('edgedriver_win64') + vi.mocked(os.arch).mockReturnValue('x64') + vi.mocked(os.platform).mockReturnValue('darwin') + expect(getNameByArchitecture()).toBe('edgedriver_mac64') + vi.mocked(os.arch).mockReturnValue('arm64') + vi.mocked(os.platform).mockReturnValue('darwin') + expect(getNameByArchitecture()).toBe('edgedriver_mac64_m1') +}) + +test('parseParams', () => { + expect(parseParams({ baseUrl: 'foobar', silent: true, verbose: false, allowedIps: ['123', '321'] })) + .toMatchSnapshot() +}) + +describe('extractBasicAuthFromUrl', () => { + test('with credentials', () => { + const result = extractBasicAuthFromUrl('https://myuser:mypassword@cdn.example.com/path/file.zip') + expect(result.url).toBe('https://cdn.example.com/path/file.zip') + expect(result.authHeader).toBe('Basic ' + Buffer.from('myuser:mypassword').toString('base64')) + }) + + test('without credentials', () => { + const result = extractBasicAuthFromUrl('https://cdn.example.com/path/file.zip') + expect(result.url).toBe('https://cdn.example.com/path/file.zip') + expect(result.authHeader).toBeUndefined() + }) + + test('with only username', () => { + const result = extractBasicAuthFromUrl('https://myuser@cdn.example.com/path/file.zip') + expect(result.url).toBe('https://cdn.example.com/path/file.zip') + expect(result.authHeader).toBe('Basic ' + Buffer.from('myuser:').toString('base64')) + }) + + test('with invalid URL returns original', () => { + const result = extractBasicAuthFromUrl('not-a-valid-url') + expect(result.url).toBe('not-a-valid-url') + expect(result.authHeader).toBeUndefined() + }) +}) From 1e5a690fe8fb905e8a14842d807cfd0a31fd85c1 Mon Sep 17 00:00:00 2001 From: dprevost-perso Date: Sun, 20 Sep 2026 23:05:42 -0400 Subject: [PATCH 6/7] fix: stop safaridriver's kill() mock leaking state across tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit execFile's mock used .mockReturnValue({ kill: vi.fn() }), so every call returned the exact same object — the kill mock's call count kept accumulating across the whole file instead of resetting per test. 'can stop server' only passed because it happened to run last, asserting the accumulated count of 5 rather than the 1 call it actually intended to check; running it in isolation failed outright. Switched to .mockImplementation() so each execFile() call gets its own kill mock, and fixed the assertion to the correct toBeCalledTimes(1). Also fixed an unrelated 'intance' -> 'instance' typo in the same test. Co-Authored-By: Claude Sonnet 5 --- packages/node-safaridriver/tests/mod.test.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/node-safaridriver/tests/mod.test.ts b/packages/node-safaridriver/tests/mod.test.ts index f79eefa..1780ef3 100644 --- a/packages/node-safaridriver/tests/mod.test.ts +++ b/packages/node-safaridriver/tests/mod.test.ts @@ -5,7 +5,9 @@ import safaridriver from '../src/index.js' vi.mock('node:child_process', () => ({ default: { - execFile: vi.fn().mockReturnValue({ kill: vi.fn() }) + // fresh mock per call, so each test's process handle has its own + // independent kill() call count rather than sharing one across the file + execFile: vi.fn().mockImplementation(() => ({ kill: vi.fn() })) } })) @@ -65,7 +67,7 @@ test('can start with options', () => { }) test('can stop server', () => { - const intance = safaridriver.start() + const instance = safaridriver.start() safaridriver.stop() - expect(intance.kill).toBeCalledTimes(5) + expect(instance.kill).toBeCalledTimes(1) }) From 85d7fda1d016b768681d69766b6b261cb21a48b9 Mon Sep 17 00:00:00 2001 From: dprevost-perso Date: Sun, 20 Sep 2026 23:06:14 -0400 Subject: [PATCH 7/7] fix: correct .npmignore packaging rules across all 3 driver packages - removed the stale `vitest.config.ts` entry: that file was deleted from the repo at the very start of this project's cleanup (the monorepo uses vitest.config.mjs instead, already covered by the existing /*.mjs pattern), so this line had no effect - removed geckodriver's CHANGELOG.md exclusion, matching the npm ecosystem's default of shipping it (edgedriver/safaridriver don't exclude anything comparable either) - added *.tsbuildinfo: TypeScript's local incremental build cache was actually leaking into the published tarball (confirmed via `pnpm pack --dry-run`) since none of the existing patterns matched a non-dotfile like tsconfig.tsbuildinfo Co-Authored-By: Claude Sonnet 5 --- packages/node-edgedriver/.npmignore | 2 +- packages/node-geckodriver/.npmignore | 3 +-- packages/node-safaridriver/.npmignore | 1 + 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/node-edgedriver/.npmignore b/packages/node-edgedriver/.npmignore index 02c133b..c1b2ee7 100644 --- a/packages/node-edgedriver/.npmignore +++ b/packages/node-edgedriver/.npmignore @@ -2,7 +2,7 @@ src tests coverage *.tgz +*.tsbuildinfo /.* /*.mjs /tsconfig.json -vitest.config.ts diff --git a/packages/node-geckodriver/.npmignore b/packages/node-geckodriver/.npmignore index b8be233..c1b2ee7 100644 --- a/packages/node-geckodriver/.npmignore +++ b/packages/node-geckodriver/.npmignore @@ -2,8 +2,7 @@ src tests coverage *.tgz +*.tsbuildinfo /.* /*.mjs /tsconfig.json -vitest.config.ts -CHANGELOG.md diff --git a/packages/node-safaridriver/.npmignore b/packages/node-safaridriver/.npmignore index e52ae25..c1b2ee7 100644 --- a/packages/node-safaridriver/.npmignore +++ b/packages/node-safaridriver/.npmignore @@ -2,6 +2,7 @@ src tests coverage *.tgz +*.tsbuildinfo /.* /*.mjs /tsconfig.json