From 9f72ba738e9622ddaae48456b45fec1d791a3839 Mon Sep 17 00:00:00 2001 From: Danilo Date: Sat, 5 Sep 2026 08:42:26 +0200 Subject: [PATCH 1/2] Report the app's address and name alongside the manifest MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A site provisioned from a scan has no address and no name of its own. The address is not decoration: it is what Patchstack fetches to check the published page still carries what was scanned, and it is what the dashboard shows. Until something reports one, the site holds a placeholder. The project already states both, on disk, before the app has a visitor. So `scan` now sends them with the manifest. `url`: `url` in `.patchstackrc.json` or `PATCHSTACK_SITE_URL` wins, since a person naming their own site outranks any inference. Otherwise the single variable a host publishes to name its own PRODUCTION address — Vercel, Netlify, Render, Railway. Preview and branch deployments are excluded on purpose: the address is adopted once and then belongs to the site, so a per-deployment URL would freeze it at whatever preview arrived first. Cloudflare Pages publishes no way to tell a production deployment from a branch one, so those projects set `url` explicitly rather than have it guessed at. `name`: an explicit `name` in `.patchstackrc.json` or `PATCHSTACK_SITE_NAME` wins; otherwise the static `` of `index.html`; otherwise the `package.json` name made readable. Starter-template values count as no name, because an owner can fix a wrong name but a placeholder hides that there is anything to fix. Both are omitted rather than guessed when nothing qualifies, and Patchstack applies either only to a site that still lacks it. What the address is allowed to be The reported address is fetched later, so a host that resolves inside a network is worse than a wrong answer. The rule is therefore the shape of a public site's address rather than a list of private ranges to exclude — such a list has to be kept current and is in any case only the ranges somebody thought of. Refused: an IP literal in either family (`URL` canonicalises the alternative spellings first, so this covers all of them), a single-label host, and the reserved suffixes. A `url` that someone configured and that fails those checks raises CONFIG_INVALID; falling through to an inferred address would quietly report a different site than the one they wrote down. Resolved only where it is reported `resolveConfig` is shared by every command, so resolution is opt-in (`detectSiteIdentity`) and only the manifest push asks for it. `status`, `guide`, `login`, `uninstall`, `mark-build` and the map upload do not read the host's URL variables and do not open `index.html` or `package.json`. `siteUrl` and `siteName` are optional on the exported `Config`: it is public, consumers build one, and the push already omits a field it was not given. `tests/public-config-type.test.ts` compiles a pre-existing consumer to keep that true. The body is built once, by `buildManifestBody`, and used for both the POST and the `--dry-run` preview. A preview that omits fields is a preview of a different request, and `--dry-run` is how someone decides whether to send it. Docs `README.md` and `AGENT-INSTALL.md` said `scan` sends no environment variable value and reads no project file. They now name the four host variables read for the address, say the name comes from the project's own `index.html` and `package.json`, state which commands read them (only `scan`, and the two that run it), and describe what an address is refused for. `AGENT-INSTALL.md` is a gated artifact: `node field-test/run.mjs --persona hostile --rounds 3` should run against a published build before release. Not run here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --- .gitignore | 1 + AGENT-INSTALL.md | 11 +- README.md | 12 +- src/cli.ts | 15 ++- src/client.ts | 25 ++++- src/config.ts | 85 ++++++++++++++ src/index.ts | 4 +- src/site-name.ts | 184 +++++++++++++++++++++++++++++++ src/site-url.ts | 153 +++++++++++++++++++++++++ src/types.ts | 16 +++ tests/bin-invocation.test.ts | 46 ++++++++ tests/client.test.ts | 140 +++++++++++++++++++++++ tests/config.test.ts | 114 +++++++++++++++++++ tests/public-config-type.test.ts | 74 +++++++++++++ tests/site-name.test.ts | 181 ++++++++++++++++++++++++++++++ tests/site-url.test.ts | 165 +++++++++++++++++++++++++++ 16 files changed, 1217 insertions(+), 9 deletions(-) create mode 100644 src/site-name.ts create mode 100644 src/site-url.ts create mode 100644 tests/public-config-type.test.ts create mode 100644 tests/site-name.test.ts create mode 100644 tests/site-url.test.ts diff --git a/.gitignore b/.gitignore index 28a9de11..e3744b62 100644 --- a/.gitignore +++ b/.gitignore @@ -25,3 +25,4 @@ test-build/.work/ # A lockfile committed from that directory would put the vulnerable package into the repository's # dependency graph, where its advisories cannot be told apart from advisories about the shipped package. examples/protect/package-lock.json +.public-types-check/ diff --git a/AGENT-INSTALL.md b/AGENT-INSTALL.md index 163df54d..02a70b5e 100644 --- a/AGENT-INSTALL.md +++ b/AGENT-INSTALL.md @@ -8,8 +8,8 @@ Every command at a glance — what it does, whether it reads your source, what i | Command | What it does | Reads your source? | Writes to your project | Sends over the network | |---|---|---|---|---| -| `scan` | Provision (or reuse) the site and POST the dependency list for vulnerability matching. Also runs automatically via `setup` and the install/build hooks. | No — lockfile only; `node_modules/` is enumerated when no lockfile can be read (e.g. `bun.lockb`) or when the lockfiles present disagree | `.patchstackrc.json` (public: site UUID + settings); `.patchstackrc.local.json` (the API key, created owner-only) and a `.gitignore` entry for it — the CLI says so if it could not add one; the widget `<script>` tag in the root HTML shell — only after a successful post; the production marker in a code root shell — before the post, since it needs no site UUID | Package names + versions | -| `setup` | One bounded command: `scan` → manage the widget → install + verify `protect` → wire the install/build scans. Never runs the project build. | No | Config, widget tag, production marker, guard files, `package.json` scripts | Package names + versions (via `scan`) | +| `scan` | Provision (or reuse) the site and POST the dependency list for vulnerability matching. Also runs automatically via `setup` and the install/build hooks. | No — lockfile only; `node_modules/` is enumerated when no lockfile can be read (e.g. `bun.lockb`) or when the lockfiles present disagree. It also reads the `<title>` of the root `index.html` and the `name` in `package.json`, to report what the site is called | `.patchstackrc.json` (public: site UUID + settings); `.patchstackrc.local.json` (the API key, created owner-only) and a `.gitignore` entry for it — the CLI says so if it could not add one; the widget `<script>` tag in the root HTML shell — only after a successful post; the production marker in a code root shell — before the post, since it needs no site UUID | Package names + versions; this site's public address and name, where the project or build environment states them | +| `setup` | One bounded command: `scan` → manage the widget → install + verify `protect` → wire the install/build scans. Never runs the project build. | No | Config, widget tag, production marker, guard files, `package.json` scripts | Package names + versions and the site's public address and name (via `scan`) | | `map` | Local, read-only attack-surface analysis (entry points → inputs → sinks → evidence-backed flows). Never run by another command. | **Yes** — via the app's own TypeScript | Nothing (only the file named by `--out`) | Nothing — **unless `--upload`**: structure only (routes, parameter names, the package behind each sink, file:line). Never source code or env values | | `protect` | Install the always-on runtime guard; auto-wire known stacks, or scaffold a generic guard + print a wiring plan. `--check` verifies the guard is wired (exit 1 if not); `--demo` seeds a broad sample rule set. Runs automatically **only** via `setup` — never by `scan`, `guide`, `status`, or `mark-build`. | No — writes guard files, does not analyze your code | Guard/framework files (e.g. `middleware.ts`, `src/patchstack/`) | Nothing | | `demo node-serialize` | Production-backed walkthrough: confirm the vulnerable package is present, scan, wait for live rule `18843`, install + verify the guard, print test requests. Does not install the package or start/restart the app. | No | Same files as `scan` + `protect` | `scan` payload; polls the public Pulse rules endpoint (never the printed test requests) | @@ -21,15 +21,18 @@ Every command at a glance — what it does, whether it reads your source, what i | `login` | Recover a lost credential for an existing site: print an owner-approval link and poll (10 min). Approving **rotates** the credential. Not usable in CI. | No | New credential into `.patchstackrc.local.json` on approval | Device-code request + approval poll | | `uninstall` | Signal Patchstack that the package is being removed: an unclaimed record is deleted, a claimed one is flagged. Does **not** touch local files. | No | Nothing local | Removal signal | -Only `map` reads your source, and only `map --upload` sends anything derived from it. `scan` transmits nothing but package names + versions — never source code, env var values, file paths, or git history. `scan --install-paths` additionally sends where each package sits in the dependency tree; it is off unless you pass it. +Only `map` reads your source, and only `map --upload` sends anything derived from it. `scan` additionally reads two declarations the project makes about itself — the `<title>` in the root `index.html` and the `name` in `package.json` — to report what the site is called; no other command reads either. `scan` transmits package names + versions, plus the site's own public address and name where the project states them — never source code, file paths, git history, or any environment variable value other than the published URL of this site. `scan --install-paths` additionally sends where each package sits in the dependency tree; it is off unless you pass it. ## Package and command behavior - Package: [`@patchstack/connect`](https://www.npmjs.com/package/@patchstack/connect), MIT-licensed, source at https://github.com/patchstack/connect. `npm view @patchstack/connect` shows the live registry metadata. -- **What is sent to Patchstack is the dependency list only** — read from the lockfile (`package-lock.json`, `pnpm-lock.yaml`, `yarn.lock`) or, on bun projects (`bun.lock`/`bun.lockb`), by enumerating the installed packages under `node_modules/` — package names + versions, for vulnerability matching. No source code, no env var values, no file paths, no git history is ever transmitted. +- **What is sent to Patchstack is the dependency list, plus this site's public address and name** — the dependencies are read from the lockfile (`package-lock.json`, `pnpm-lock.yaml`, `yarn.lock`) or, on bun projects (`bun.lock`/`bun.lockb`), by enumerating the installed packages under `node_modules/` — package names + versions, for vulnerability matching. No source code, no file paths, no git history is ever transmitted. - **`scan --install-paths` is the one exception, and it is opt-in.** It adds where each package sits in the dependency tree — repo-relative paths made of `node_modules` segments, plus a workspace directory name when a workspace pins its own copy. They are read from the lockfile's own keys or from the `node_modules` walk, **never from your source tree**: no path to a file you wrote is sent by either form of `scan`. - Why it exists: the same package is routinely installed twice at different versions, and without the locations an advisory affecting only one of them cannot be matched to the copy your code actually loads. Node resolves an import by walking up from the importing file, so the location is what distinguishes "you are running the vulnerable copy" from "the vulnerable copy is installed but nothing reaches it". Absent them, every installed version has to be treated as if the app used it — warnings about code you never call, and protection rules pinned to routes that run the safe copy. - Why it is off by default: it widens what leaves the machine, so it is your explicit choice and not a consequence of upgrading the package. (`mark-build` additionally stamps built HTML with a coarse stack descriptor that may include hosting-related env variable *names* — e.g. `VERCEL`, `CF_PAGES` — never their values.) +- **Only `scan` looks for the address and the name.** They are resolved in the one code path that reports them, so `guide`, `status`, `login`, `uninstall`, `mark-build`, `init`, `protect`, `demo-guide` and `map` neither read the host's URL variables nor open `index.html` or `package.json` for this. `setup` and `demo` do, because both run `scan`. +- **The address is the one your visitors use, and, apart from the tool's own `PATCHSTACK_*` settings, it is the only env var value read.** A site provisioned by a scan from a developer machine has no address, so the dashboard shows a placeholder and Patchstack cannot check that the published page still carries what was scanned. `scan` therefore sends `url` when — and only when — it can know it: `url` in `.patchstackrc.json` or `PATCHSTACK_SITE_URL` if you set one, otherwise the single variable a host publishes to name its own **production** URL (`VERCEL_PROJECT_PRODUCTION_URL` on a Vercel production deployment, Netlify's `URL` in the production context, `RENDER_EXTERNAL_URL`, `RAILWAY_PUBLIC_DOMAIN` in a production environment). Preview and branch deployments are excluded, as are hosts that publish no production signal. An address that is not how the public reaches a website is dropped: any IP address (in either family, however it is written), any single-label host such as `localhost` or `production`, and the reserved suffixes (`.local`, `.internal`, `.test`, `.invalid`, `.home.arpa`, …). A `url` you set explicitly that fails those checks is refused with an error rather than replaced by a guess. When nothing qualifies, `url` is omitted from the payload rather than guessed. Patchstack only ever applies it to a site that still has no address; it never re-points a site whose address is already real. +- **The name is read from your project, never from the host environment.** `name` in `.patchstackrc.json` (or `PATCHSTACK_SITE_NAME`) if you set one; otherwise the `<title>` of the project's root `index.html` (`public/index.html` if there is no root one), read from the file as text — a title your app sets from script is not seen; otherwise the `name` in `package.json`, unless it is a template placeholder such as `vite_react_shadcn_ts`. It is omitted when nothing qualifies, and it only ever fills in a site that has no name yet — a name set in the dashboard is never replaced. - **One command reads source files:** `map` (see below) parses your server source to report your app's attack surface. It runs only when you invoke it and prints to stdout. It transmits nothing unless you explicitly pass `--upload`, which sends that description of your app's structure to your own site's Patchstack endpoint — never source code, and never without that flag. No other command reads source (`protect` writes guard files but does not analyze your code). - **`scan` makes up to two source edits, both in the project's root shell:** the disclosure widget's `<script>` tag, and the production marker. Neither runs on `--dry-run`, both are idempotent, both leave a pre-existing manual install untouched, and `"widget": false` in `.patchstackrc.json` disables both. - The **widget tag** goes in the root HTML shell — the first of `index.html`, `public/index.html`, or `src/app.html` that exists — and only after a successful post, because it carries the site UUID. diff --git a/README.md b/README.md index acd060d2..ee44b68d 100644 --- a/README.md +++ b/README.md @@ -226,11 +226,19 @@ Lower-level pieces are also exported: `scanLockfile`, `buildWirePayload`, `postM { "name": "axios", "version": "1.6.0" }, { "name": "lodash", "version": "4.17.15" }, { "name": "lodash", "version": "4.17.21" } - ] + ], + "url": "https://your-app.example.com", + "name": "ToDo Application" } ``` -That's the entire payload. No source code, no environment variable values, no file paths — just the package names and versions from your lockfile. +That's the entire payload: the package names and versions from your lockfile, plus what your project says about the site itself — its public address and its name. No source code, no file paths, no secrets. + +The address is included so the site in your dashboard shows where it lives instead of a placeholder, and so Patchstack can check the published page still carries what was scanned. It is taken from `url` in `.patchstackrc.json` (or `PATCHSTACK_SITE_URL`) if you set one; otherwise from the one variable a host publishes to name its own production URL — `VERCEL_PROJECT_PRODUCTION_URL` on a Vercel production deployment, Netlify's `URL` in the production context, `RENDER_EXTERNAL_URL`, `RAILWAY_PUBLIC_DOMAIN` in a production environment. No other environment variable's value is read, and `url` is left out entirely when none of those says anything — a build on your own machine sends no address. An address that is not how the public reaches a website is refused: any IP address, any single-label host such as `localhost`, and the reserved suffixes (`.local`, `.internal`, `.test`, `.invalid`). If you set `url` yourself and it fails those checks, `scan` stops and says so rather than sending a different address. + +The name is what the dashboard calls the site. It is taken from `name` in `.patchstackrc.json` (or `PATCHSTACK_SITE_NAME`) if you set one; otherwise from the `<title>` of the project's root `index.html`; otherwise from the `name` in `package.json`, unless that is a template placeholder such as `vite_react_shadcn_ts`. It is left out when nothing qualifies. Those two files are read only by `scan` — the commands that never post a manifest do not open them. + +You can see exactly what would be sent, without sending it, by running `npx @patchstack/connect scan --dry-run`: the preview it prints is the request body itself. Patchstack only applies either field to a site that does not have one yet: it never re-points a site whose address is real, and never replaces a name set in the dashboard. ### `scan --install-paths` (opt-in) diff --git a/src/cli.ts b/src/cli.ts index ae84b6dd..defdcfb9 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -6,6 +6,7 @@ import { buildWirePayload } from './normalize.js'; import { computeManifestChecksum } from './checksum.js'; import { postInputMap, + buildManifestBody, DEFAULT_ENDPOINT, buildClaimUrl, fetchSiteStatus, @@ -366,6 +367,8 @@ async function runScan( cwd: process.cwd(), cliSiteUuid: getStringFlag(args.flags, 'site-uuid'), cliEndpoint: getStringFlag(args.flags, 'endpoint'), + // The one command that reports them, so the one command that resolves them. + detectSiteIdentity: true, }); const manifest = await scanLockfile(process.cwd()); for (const warning of manifest.warnings ?? []) { @@ -404,6 +407,16 @@ async function runScan( ); } + // Ahead of the --dry-run return, so a preview says what a real run would report. This is the part of + // the payload someone might disagree with, and it is easier to disagree with here than in the dashboard. + const body = buildManifestBody(config, payload); + if (typeof body.url === 'string') { + console.log(`Reporting this app's address as ${body.url}.`); + } + if (typeof body.name === 'string') { + console.log(`Reporting this app's name as "${body.name}".`); + } + if (dryRun) { console.log(''); if (config.siteUuid === null) { @@ -412,7 +425,7 @@ async function runScan( console.log(`--dry-run: not posting to Patchstack (site UUID ${config.siteUuid}).`); } console.log('Payload preview:'); - const preview = JSON.stringify(payload, null, 2).split('\n'); + const preview = JSON.stringify(body, null, 2).split('\n'); console.log(preview.slice(0, Math.min(preview.length, 30)).join('\n')); if (preview.length > 30) { console.log(` ... (${preview.length - 30} more lines)`); diff --git a/src/client.ts b/src/client.ts index 7f70ad67..09134baa 100644 --- a/src/client.ts +++ b/src/client.ts @@ -266,6 +266,29 @@ export async function fetchSiteStatus(config: Config): Promise<SiteStatus> { } } +/** + * The whole body of a manifest push. + * + * Built here rather than inline at the `fetch` so that `--dry-run` prints the object that would be sent + * instead of a subset of it: a preview that omits fields is a preview of a different request. + * + * `url` and `name` ride along on every push, not only the one that provisions the site. A site created + * by a scan from a developer machine carries a placeholder address until some later push reports a real + * one, and that push is a re-scan. Patchstack applies either field only to a site that still lacks it, + * so a repeated push does not re-point an address or replace a name. Both are omitted when they are not + * strings, which is what a caller that built its own `Config` without them sends. + */ +export function buildManifestBody(config: Config, payload: WirePayload): Record<string, unknown> { + return { + ...payload, + environment: config.environment, + ...(typeof config.siteUrl === 'string' && config.siteUrl !== '' ? { url: config.siteUrl } : {}), + ...(typeof config.siteName === 'string' && config.siteName !== '' + ? { name: config.siteName } + : {}), + }; +} + export async function postManifest( config: Config, payload: WirePayload, @@ -284,7 +307,7 @@ export async function postManifest( Accept: 'application/json', 'User-Agent': '@patchstack/connect', }, - body: JSON.stringify({ ...payload, environment: config.environment }), + body: JSON.stringify(buildManifestBody(config, payload)), signal: AbortSignal.timeout(timeoutMs), }); } catch (cause) { diff --git a/src/config.ts b/src/config.ts index 7d9d01b9..a0802ef6 100644 --- a/src/config.ts +++ b/src/config.ts @@ -2,6 +2,8 @@ import { readFile, writeFile, chmod } from 'node:fs/promises'; import path from 'node:path'; import { PatchstackError, type Config, type Environment } from './types.js'; import { DEFAULT_ENDPOINT, DEFAULT_TIMEOUT_MS } from './client.js'; +import { detectSiteUrl, normaliseSiteUrl } from './site-url.js'; +import { detectSiteName, normaliseSiteName } from './site-name.js'; const CONFIG_FILENAME = '.patchstackrc.json'; @@ -35,6 +37,17 @@ interface ConfigFile { timeoutMs?: number; environment?: string; widget?: boolean; + /** + * Where this app is published. Committed with the UUID rather than kept secret — it is the address + * the site already serves to everyone. Set it for platforms whose build environment does not say + * which deployment is the production one. + */ + url?: string; + /** + * What this app is called in the dashboard. Set it when the project's own files do not say — a + * `package.json` still named after its template, an HTML shell whose title is filled in by script. + */ + name?: string; } export interface ResolveConfigOptions { @@ -48,6 +61,69 @@ export interface ResolveConfigOptions { * server response. */ requireSiteUuid?: boolean; + /** + * Resolve the site's public address and name into `siteUrl` and `siteName`. + * + * Off by default, and left off by every command that does not push a manifest. Resolving them reads + * the project's `index.html` and `package.json` and the host's production-URL variables, and the + * manifest push is the only thing that reports the result — so `status`, `guide`, `login`, + * `uninstall`, `mark-build` and the map upload have no reason to look, and do not. Both fields come + * back `null` when it is off. + */ + detectSiteIdentity?: boolean; +} + +interface SiteIdentity { + url: string | null; + name: string | null; +} + +/** + * The address and name to report: what the configuration states, or what the project and its build + * environment say when it states nothing. + * + * A configured value that cannot be used is an error rather than a reason to look elsewhere. The setting + * exists so that the person naming their own site outranks any inference, and falling through to an + * inferred address would report a different site than the one they wrote — quietly, and permanently, + * since the address is adopted once and then belongs to the site. + */ +async function resolveSiteIdentity( + cwd: string, + fromEnv: ConfigFile, + fromFile: ConfigFile, +): Promise<SiteIdentity> { + const configuredUrl = stated(fromEnv.url) ?? stated(fromFile.url); + const configuredName = stated(fromEnv.name) ?? stated(fromFile.name); + + let url: string | null; + if (configuredUrl === null) { + url = detectSiteUrl(process.env)?.url ?? null; + } else { + url = normaliseSiteUrl(configuredUrl); + if (url === null) { + throw new PatchstackError( + `Site URL "${configuredUrl}" is not an address the public can reach this site at. ` + + 'Set "url" in .patchstackrc.json (or PATCHSTACK_SITE_URL) to the http(s) address visitors ' + + 'use — a hostname, not an IP address — or remove it to let the build environment answer.', + 'CONFIG_INVALID', + ); + } + } + + // `normaliseSiteName` only rejects a blank string, which `stated` has already ruled out, so a + // configured name always wins outright and there is no invalid case to refuse here. + const name = + configuredName === null + ? ((await detectSiteName(cwd))?.name ?? null) + : normaliseSiteName(configuredName); + + return { url, name }; +} + +/** A setting someone actually wrote. Blank counts as unset, which is how a CI variable is cleared. */ +function stated(value: string | undefined): string | null { + const trimmed = (value ?? '').trim(); + return trimmed === '' ? null : trimmed; } export async function resolveConfig(options: ResolveConfigOptions): Promise<Config> { @@ -100,10 +176,17 @@ export async function resolveConfig(options: ResolveConfigOptions): Promise<Conf // without re-provisioning: today both hold the same credential. const pulseAuthRaw = fromEnv.pulseAuth ?? fromSecretFile.pulseAuth ?? fromFile.pulseAuth ?? apiKeyRaw; + const identity = + options.detectSiteIdentity === true + ? await resolveSiteIdentity(options.cwd, fromEnv, fromFile) + : { url: null, name: null }; + return { siteUuid: siteUuid === null || siteUuid.length === 0 ? null : siteUuid, apiKey: apiKeyRaw === null || apiKeyRaw.length === 0 ? null : apiKeyRaw, pulseAuth: pulseAuthRaw === null || pulseAuthRaw.length === 0 ? null : pulseAuthRaw, + siteUrl: identity.url, + siteName: identity.name, endpoint, timeoutMs, environment, @@ -373,6 +456,8 @@ function readEnv(): ConfigFile { const environmentRaw = process.env.PATCHSTACK_ENVIRONMENT; return { siteUuid: process.env.PATCHSTACK_SITE_UUID ?? undefined, + url: process.env.PATCHSTACK_SITE_URL ?? undefined, + name: process.env.PATCHSTACK_SITE_NAME ?? undefined, apiKey: process.env.PATCHSTACK_API_KEY ?? undefined, pulseAuth: process.env.PATCHSTACK_PULSE_AUTH ?? undefined, endpoint: process.env.PATCHSTACK_ENDPOINT ?? undefined, diff --git a/src/index.ts b/src/index.ts index fe0a2aa8..97ebf2be 100644 --- a/src/index.ts +++ b/src/index.ts @@ -56,7 +56,9 @@ export async function scanAndReport( options: ScanAndReportOptions = {}, ): Promise<ScanAndReportResult> { const cwd = options.cwd ?? process.cwd(); - const config = options.config ?? (await resolveConfig({ cwd })); + // Reports the manifest, so it resolves what the manifest reports. A caller passing its own `config` + // decides for itself: `siteUrl`/`siteName` are optional, and omitting them omits them from the push. + const config = options.config ?? (await resolveConfig({ cwd, detectSiteIdentity: true })); const manifest = await scanLockfile(cwd); // Opt-in, matching the CLI: a library caller does not get a widened payload by upgrading either. const { payload, stats } = buildWirePayload(manifest, { installPaths: options.installPaths === true }); diff --git a/src/site-name.ts b/src/site-name.ts new file mode 100644 index 00000000..cd1df631 --- /dev/null +++ b/src/site-name.ts @@ -0,0 +1,184 @@ +/** + * What the app this manifest describes is called. + * + * The project states this itself, on disk, before the app has a single visitor: in its HTML shell's + * `<title>` and in its package manifest's `name`. `scan` reports it under the site's credential, and + * Patchstack applies it only to a site that has no name yet; an owner can rename a site at any time. + * + * The values starter templates ship with are treated as "no name" rather than reported: an owner can + * fix a wrong name, but a placeholder name hides that there is anything to fix. + * + * Like `site-url.ts`, this reads a value and sends it. It reads exactly two files, both of which the + * project already serves or publishes: the root `index.html` (or `public/index.html`) and `package.json`. + * No environment variable is read here; `PATCHSTACK_SITE_NAME` is handled with the rest of the config. + */ + +import { readFile } from 'node:fs/promises'; +import path from 'node:path'; + +/** The most Patchstack accepts for a name. */ +const NAME_MAX_LENGTH = 191; + +/** + * `package.json` names that arrive with a starter template. A project still carrying one has not been + * named; reporting it would label a real app "vite_react_shadcn_ts" in someone's dashboard. + */ +const TEMPLATE_PACKAGE_NAMES: ReadonlySet<string> = new Set([ + // Lovable + 'vite_react_shadcn_ts', + // Bolt + 'vite-react-typescript-starter', + // Scaffolders and the names people accept from them + 'vite-project', + 'my-app', + 'my-project', + 'my-v0-project', + 'app', + 'frontend', + 'web', + 'client', + 'project', + 'template', + 'example', + 'react-app', + 'next-app', + 'nextjs', + 'starter', + 'test', +]); + +/** `<title>` values that arrive with a starter template, compared case-insensitively. */ +const TEMPLATE_TITLES: ReadonlySet<string> = new Set([ + 'vite + react', + 'vite + react + ts', + 'vite + vue', + 'vite + vue + ts', + 'vite app', + 'react app', + 'create next app', + 'document', + 'untitled', + 'index', + 'home', + 'app', +]); + +const NAMED_ENTITIES: Record<string, string> = { + amp: '&', + lt: '<', + gt: '>', + quot: '"', + apos: "'", + nbsp: ' ', +}; + +/** Whitespace-collapsed, trimmed, capped to what the server stores; null when nothing is left. */ +export function normaliseSiteName(value: string | undefined | null): string | null { + const collapsed = (value ?? '').replace(/\s+/g, ' ').trim(); + if (collapsed === '') return null; + + return collapsed.slice(0, NAME_MAX_LENGTH); +} + +/** + * Whether `code` is a Unicode scalar value, the only thing `String.fromCodePoint` accepts. + * + * Everything else in the numeric range an HTML author can write throws: past `0x10FFFF` there is no + * character, and a lone surrogate is half of one. `<title>` is arbitrary text from a file this package + * did not write, so a number it cannot represent has to be left as the text it was. + */ +function isScalarValue(code: number): boolean { + return ( + Number.isInteger(code) && code >= 0 && code <= 0x10ffff && !(code >= 0xd800 && code <= 0xdfff) + ); +} + +function decodeEntities(text: string): string { + return text.replace(/&(#x[0-9a-f]+|#\d+|[a-z]+);/gi, (whole, body: string) => { + if (body.startsWith('#x') || body.startsWith('#X')) { + const code = Number.parseInt(body.slice(2), 16); + return isScalarValue(code) ? String.fromCodePoint(code) : whole; + } + if (body.startsWith('#')) { + const code = Number.parseInt(body.slice(1), 10); + return isScalarValue(code) ? String.fromCodePoint(code) : whole; + } + return NAMED_ENTITIES[body.toLowerCase()] ?? whole; + }); +} + +/** + * The static `<title>` of an HTML shell, or null when there is none worth reporting. Read from the file, + * so a title an app only sets from script is not seen here — that is the case the `name` config exists for. + */ +export function nameFromHtmlTitle(html: string): string | null { + const match = /<title\b[^>]*>([\s\S]*?)<\/title>/i.exec(html); + if (match === null) return null; + + const name = normaliseSiteName(decodeEntities(match[1] ?? '')); + if (name === null || TEMPLATE_TITLES.has(name.toLowerCase())) return null; + + return name; +} + +/** + * A `package.json` name made readable: the scope dropped, separators turned into spaces, each word + * capitalised. `@acme/my-todo-app` reports as "My Todo App". Null for a template placeholder or anything + * that is not a usable string. + */ +export function nameFromPackageName(raw: unknown): string | null { + if (typeof raw !== 'string') return null; + + const unscoped = raw.trim().replace(/^@[^/]+\//, ''); + if (unscoped === '' || TEMPLATE_PACKAGE_NAMES.has(unscoped.toLowerCase())) return null; + + const words = unscoped + .split(/[-_.\s]+/) + .filter((word) => word !== '') + .map((word) => (word === word.toUpperCase() ? word : word.charAt(0).toUpperCase() + word.slice(1))); + + return normaliseSiteName(words.join(' ')); +} + +export interface DetectedSiteName { + name: string; + /** Which file it came from, so the CLI can print it. */ + source: 'index.html' | 'package.json'; +} + +async function readIfPresent(file: string): Promise<string | null> { + try { + return await readFile(file, 'utf8'); + } catch { + return null; + } +} + +/** + * The name the project states for itself, or null when it states none. The HTML shell is preferred: it is + * written for people, where a package name is written for a registry. + */ +export async function detectSiteName(cwd: string): Promise<DetectedSiteName | null> { + for (const shell of ['index.html', path.join('public', 'index.html')]) { + const html = await readIfPresent(path.join(cwd, shell)); + if (html === null) continue; + + const name = nameFromHtmlTitle(html); + if (name !== null) return { name, source: 'index.html' }; + } + + const manifest = await readIfPresent(path.join(cwd, 'package.json')); + if (manifest !== null) { + try { + const parsed: unknown = JSON.parse(manifest); + const name = nameFromPackageName( + typeof parsed === 'object' && parsed !== null ? (parsed as { name?: unknown }).name : undefined, + ); + if (name !== null) return { name, source: 'package.json' }; + } catch { + /* an unreadable package.json is the scan's problem to report, not the name's */ + } + } + + return null; +} diff --git a/src/site-url.ts b/src/site-url.ts new file mode 100644 index 00000000..d34727bd --- /dev/null +++ b/src/site-url.ts @@ -0,0 +1,153 @@ +/** + * Where the app this manifest describes is published. + * + * Patchstack provisions a site from the first manifest, and a manifest posted from a laptop carries no + * address — so those sites are created with a synthetic `*.placeholder.invalid` host. That address is not + * cosmetic: it is what Patchstack fetches to check the live build still carries what was scanned, and what + * the dashboard shows. Something has to replace it, and the manifest push is the only thing that reports + * from inside the deployment while holding the site's own credential. + * + * Reporting a wrong address is worse than reporting none, because the address is adopted once and then + * belongs to the site. So a URL is only derived from a build environment that says, in its own variables, + * that this build is the production one — never from a per-deployment preview URL, and never from a host + * that is not how the public reaches a website. A platform that publishes no such signal is left alone: + * set `url` in `.patchstackrc.json` (or `PATCHSTACK_SITE_URL`) and that wins over everything here. + * + * Unlike the hosting fingerprint in `stack.ts`, which reports variable NAMES only, this reads values. It + * reads exactly the ones below, each of which is a public address the deployed site serves to every + * visitor. + */ + +/** + * Suffixes reserved for names that are deliberately not reachable from the public internet: the + * special-use domains (RFC 2606, RFC 6761), mDNS, the reverse-lookup zones, and the private zones cloud + * networks hand out. `.invalid` also covers the `*.placeholder.invalid` value Patchstack itself stores + * for "no address known" — reporting that back as an address would be circular. + */ +const RESERVED_SUFFIXES: readonly string[] = [ + '.local', + '.localhost', + '.localdomain', + '.internal', + '.intranet', + '.invalid', + '.test', + '.example', + '.home.arpa', + '.in-addr.arpa', + '.ip6.arpa', +]; + +/** + * Hosts that cannot be a published site, whatever a build environment claims. + * + * The reported address is not only stored: it is what Patchstack fetches to check the published page. A + * host that resolves inside a network is therefore worse than a wrong answer — it aims a later fetch at + * something that was never the site. So the shape of a public website's address is what is accepted here, + * rather than a list of the private ranges to exclude: such a list has to be kept current, and is in any + * case only the ranges somebody thought of. + */ +function isUnpublishableHost(hostname: string): boolean { + // `URL` keeps a trailing root dot, and `localhost.` resolves exactly as `localhost` does. + const host = hostname.toLowerCase().replace(/\.+$/, ''); + if (host === '') return true; + + // No IP literal, in either family. A hosting platform names its production site with a hostname, so a + // literal arriving here is a build machine or an address on a network rather than a site — and deciding + // which would mean carrying every reserved range of both families. `URL` has already reduced the + // alternative spellings to the canonical one (`0x7f.1` and `2130706433` both arrive as `127.0.0.1`, + // `[::ffff:127.0.0.1]` as `[::ffff:7f00:1]`), so these two tests match every way of writing them. + if (host.startsWith('[')) return true; + if (/^\d{1,3}(\.\d{1,3}){3}$/.test(host)) return true; + + // A single-label host is a name on someone's own network — `production`, `web`, a build container's + // hostname. A site the public can reach sits under a registered domain. + if (!host.includes('.')) return true; + + return RESERVED_SUFFIXES.some((suffix) => host.endsWith(suffix)); +} + +/** + * Reduce a reported address to the `scheme://host[:port]` Patchstack stores, or null when it is not one. + * + * Several platforms report a bare hostname, so a missing scheme is filled in rather than rejected. A path + * is dropped: the site's address is its origin, and a build variable that happens to include one is still + * naming the same site. + */ +export function normaliseSiteUrl(value: string | undefined | null): string | null { + const trimmed = (value ?? '').trim(); + if (trimmed === '') return null; + + const withScheme = /^[a-z][a-z0-9+.-]*:\/\//i.test(trimmed) ? trimmed : `https://${trimmed}`; + + let parsed: URL; + try { + parsed = new URL(withScheme); + } catch { + return null; + } + + if (parsed.protocol !== 'http:' && parsed.protocol !== 'https:') return null; + if (parsed.hostname === '' || isUnpublishableHost(parsed.hostname)) return null; + + const origin = `${parsed.protocol}//${parsed.host}`; + + // 191 characters is the most Patchstack accepts for an address; an origin near that length is not one. + return origin.length <= 191 ? origin : null; +} + +/** + * The platforms that publish a production address AND a way to know the current build is the production + * one. Both halves are required: a deploy URL without that signal names a preview, and a preview URL + * adopted as the site's address sends every later check to the wrong place. + */ +const PRODUCTION_URL_SOURCES: ReadonlyArray<{ + platform: string; + read: (env: NodeJS.ProcessEnv) => string | undefined; +}> = [ + { + // VERCEL_URL is per-deployment and changes every build, so it is deliberately not read here. + platform: 'vercel', + read: (env) => (env.VERCEL_ENV === 'production' ? env.VERCEL_PROJECT_PRODUCTION_URL : undefined), + }, + { + // `URL` is generic enough to belong to anything, so it counts only alongside Netlify's own markers. + platform: 'netlify', + read: (env) => (env.NETLIFY === 'true' && env.CONTEXT === 'production' ? env.URL : undefined), + }, + { + platform: 'render', + read: (env) => + env.RENDER === 'true' && env.IS_PULL_REQUEST !== 'true' ? env.RENDER_EXTERNAL_URL : undefined, + }, + { + platform: 'railway', + read: (env) => + env.RAILWAY_ENVIRONMENT_NAME === 'production' ? env.RAILWAY_PUBLIC_DOMAIN : undefined, + }, +]; + +export interface DetectedSiteUrl { + /** The `scheme://host[:port]` to report. */ + url: string; + /** Which build environment it came from, so the CLI can print it. */ + platform: string; +} + +/** + * The production address this build environment reports, or null when it reports none. + * + * Cloudflare Pages is a deliberate omission: `CF_PAGES_URL` is the deployment's own URL and the build + * environment gives no way to tell a production deployment from a branch one, so there is nothing here + * that could be adopted safely. Those projects set `url` explicitly. + */ +export function detectSiteUrl(env: NodeJS.ProcessEnv = process.env): DetectedSiteUrl | null { + for (const source of PRODUCTION_URL_SOURCES) { + const url = normaliseSiteUrl(source.read(env)); + if (url !== null) { + return { url, platform: source.platform }; + } + } + + return null; +} diff --git a/src/types.ts b/src/types.ts index 21d6ca3f..4ac5979f 100644 --- a/src/types.ts +++ b/src/types.ts @@ -45,6 +45,22 @@ export interface Config { * Falls back to `apiKey` when unset. Prefer `PATCHSTACK_PULSE_AUTH`. */ pulseAuth: string | null; + /** + * Where this app is published, reported alongside the manifest so a site provisioned without an + * address can learn one. `null` when nothing reliable is known — a laptop build, or a platform that + * publishes no production URL. + * + * Optional, and absent unless the config was resolved with `detectSiteIdentity`. Only the manifest + * push reads it, and it is omitted from that push when it is not a string — so a caller holding a + * `Config` it built itself keeps working without knowing this field exists. + */ + siteUrl?: string | null; + /** + * What this app is called, reported alongside the manifest so a site provisioned nameless can learn a + * name. `null` when the project states none — a name Patchstack would only be guessing at is not + * reported. Optional on the same terms as `siteUrl`. + */ + siteName?: string | null; endpoint: string; timeoutMs: number; /** Environment to report the manifest under. Defaults to 'production'. */ diff --git a/tests/bin-invocation.test.ts b/tests/bin-invocation.test.ts index 28abbcc7..7118c2a7 100644 --- a/tests/bin-invocation.test.ts +++ b/tests/bin-invocation.test.ts @@ -82,6 +82,52 @@ describe.skipIf(!built)('the packaged bin, invoked as npm invokes it', () => { expect(pkg['patchstack-connect']).toBe('./dist/cli.js'); }); + /** + * `--dry-run` is how someone finds out what a scan would send before sending it, so a field the preview + * omits is a field nobody gets to object to. The request body is built once and used for both, and this + * drives the real bin to prove the preview is that body — including the address and name, which are the + * two fields a reader is most likely to want to check. + */ + it('previews every field a real post would send', () => { + const project = mkdtempSync(path.join(tmpdir(), 'ps-bin-dry-')); + try { + writeFileSync( + path.join(project, 'package.json'), + JSON.stringify({ name: 'example-app', version: '1.0.0' }), + ); + copyFileSync( + path.join(root, 'tests', 'fixtures', 'package-lock-v3.json'), + path.join(project, 'package-lock.json'), + ); + writeFileSync(path.join(project, 'index.html'), '<title>Recipe Box'); + writeFileSync( + path.join(project, '.patchstackrc.json'), + JSON.stringify({ + siteUuid: '11111111-1111-4111-8111-111111111111', + url: 'https://recipes.example.com', + }), + ); + + const stdout = execFileSync('node', [bin, 'scan', '--dry-run'], { + cwd: project, + env: { PATH: process.env.PATH, HOME: process.env.HOME }, + encoding: 'utf8', + }); + + // Said in prose before the preview, so it is noticed here rather than in the dashboard. + expect(stdout).toContain(`Reporting this app's address as https://recipes.example.com.`); + expect(stdout).toContain(`Reporting this app's name as "Recipe Box".`); + + const preview = stdout.slice(stdout.indexOf('Payload preview:')); + expect(preview).toContain('"url": "https://recipes.example.com"'); + expect(preview).toContain('"name": "Recipe Box"'); + expect(preview).toContain('"environment": "production"'); + expect(preview).toContain('"packages"'); + } finally { + rmSync(project, { recursive: true, force: true }); + } + }); + /** * A report the server refuses must not fail the build `scan` is hooked into, and must fail a direct run. * diff --git a/tests/client.test.ts b/tests/client.test.ts index 0457b950..4b306c5a 100644 --- a/tests/client.test.ts +++ b/tests/client.test.ts @@ -8,6 +8,7 @@ import { fetchSiteStatus, postManifest, postPackageRemoved, + buildManifestBody, } from '../src/client.js'; import { PatchstackError } from '../src/types.js'; @@ -308,6 +309,53 @@ describe('postManifest', () => { expect(body.ecosystem).toBe('npm'); }); + it('sends where the app is published, so a placeholder site can learn its address', async () => { + const fetchMock = vi.fn().mockResolvedValue( + new Response(JSON.stringify({ stored: true }), { status: 200 }), + ); + vi.stubGlobal('fetch', fetchMock); + + await postManifest( + { + siteUuid: 'uuid', + siteUrl: 'https://shop.example.com', + endpoint: 'https://example.com', + timeoutMs: 30_000, + widget: true, + environment: 'production', + }, + { ecosystem: 'npm', packages: [{ name: 'lodash', version: '4.17.21' }] }, + ); + + const [, init] = fetchMock.mock.calls[0] as [string, RequestInit]; + const body = JSON.parse(init.body as string) as { url?: string }; + expect(body.url).toBe('https://shop.example.com'); + }); + + it('omits the url entirely when the build knows of none', async () => { + // Not sent as null or an empty string: the server treats an absent url as "nothing to say about + // this site's address", and a present-but-empty one as a value to consider. + const fetchMock = vi.fn().mockResolvedValue( + new Response(JSON.stringify({ stored: true }), { status: 200 }), + ); + vi.stubGlobal('fetch', fetchMock); + + await postManifest( + { + siteUuid: 'uuid', + siteUrl: null, + endpoint: 'https://example.com', + timeoutMs: 30_000, + widget: true, + environment: 'production', + }, + { ecosystem: 'npm', packages: [{ name: 'lodash', version: '4.17.21' }] }, + ); + + const [, init] = fetchMock.mock.calls[0] as [string, RequestInit]; + expect(JSON.parse(init.body as string)).not.toHaveProperty('url'); + }); + it('throws SITE_NOT_FOUND on 404', async () => { vi.stubGlobal( 'fetch', @@ -401,3 +449,95 @@ describe('postManifest', () => { ).rejects.toMatchObject({ code: 'NETWORK_TIMEOUT' }); }); }); + +describe('postManifest site name', () => { + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it('sends the site name when known, and omits the field when not', async () => { + // A fresh Response per call: a body can only be read once. + const fetchMock = vi.fn().mockImplementation( + async () => new Response(JSON.stringify({ stored: true }), { status: 200 }), + ); + vi.stubGlobal('fetch', fetchMock); + + const base = { + siteUuid: 'uuid', + apiKey: null, + pulseAuth: null, + siteUrl: null, + endpoint: 'https://example.com', + timeoutMs: 30_000, + widget: true, + environment: 'production' as const, + }; + const payload = { ecosystem: 'npm' as const, packages: [{ name: 'lodash', version: '4.17.21' }] }; + + await postManifest({ ...base, siteName: 'ToDo Application' }, payload); + expect(JSON.parse(fetchMock.mock.calls[0][1].body as string).name).toBe('ToDo Application'); + + await postManifest({ ...base, siteName: null }, payload); + expect(JSON.parse(fetchMock.mock.calls[1][1].body as string)).not.toHaveProperty('name'); + }); + + it('omits both fields for a Config built without them', async () => { + // `Config` is exported, so a consumer can build one. Theirs predates these fields, and a push that + // says nothing about the address is exactly right for a caller that knows nothing about it. + const fetchMock = vi.fn().mockImplementation( + async () => new Response(JSON.stringify({ stored: true }), { status: 200 }), + ); + vi.stubGlobal('fetch', fetchMock); + + await postManifest( + { + siteUuid: 'uuid', + apiKey: null, + pulseAuth: null, + endpoint: 'https://example.com', + timeoutMs: 30_000, + widget: true, + environment: 'production', + }, + { ecosystem: 'npm', packages: [{ name: 'lodash', version: '4.17.21' }] }, + ); + + const body = JSON.parse(fetchMock.mock.calls[0][1].body as string); + expect(body).not.toHaveProperty('url'); + expect(body).not.toHaveProperty('name'); + expect(body.environment).toBe('production'); + }); +}); + +describe('buildManifestBody', () => { + const base = { + siteUuid: 'uuid', + apiKey: null, + pulseAuth: null, + endpoint: 'https://example.com', + timeoutMs: 30_000, + widget: true, + environment: 'production' as const, + }; + const payload = { ecosystem: 'npm' as const, packages: [{ name: 'lodash', version: '4.17.21' }] }; + + it('is the whole body, so --dry-run and the post cannot describe different requests', () => { + expect( + buildManifestBody({ ...base, siteUrl: 'https://shop.example.com', siteName: 'Shop' }, payload), + ).toEqual({ + ecosystem: 'npm', + packages: [{ name: 'lodash', version: '4.17.21' }], + environment: 'production', + url: 'https://shop.example.com', + name: 'Shop', + }); + }); + + it('leaves out what is not known rather than sending an empty value', () => { + expect(buildManifestBody({ ...base, siteUrl: null, siteName: '' }, payload)).toEqual({ + ecosystem: 'npm', + packages: [{ name: 'lodash', version: '4.17.21' }], + environment: 'production', + }); + }); +}); diff --git a/tests/config.test.ts b/tests/config.test.ts index 1e794a31..b1439f9a 100644 --- a/tests/config.test.ts +++ b/tests/config.test.ts @@ -19,6 +19,23 @@ describe('resolveConfig', () => { delete process.env.PATCHSTACK_ENDPOINT; delete process.env.PATCHSTACK_TIMEOUT_MS; delete process.env.PATCHSTACK_ENVIRONMENT; + delete process.env.PATCHSTACK_SITE_URL; + // The build-environment variables the site URL is inferred from, so a developer's own shell (or a + // CI runner that happens to be one of these platforms) cannot decide what these tests see. + for (const key of [ + 'VERCEL_ENV', + 'VERCEL_PROJECT_PRODUCTION_URL', + 'NETLIFY', + 'CONTEXT', + 'URL', + 'RENDER', + 'RENDER_EXTERNAL_URL', + 'IS_PULL_REQUEST', + 'RAILWAY_ENVIRONMENT_NAME', + 'RAILWAY_PUBLIC_DOMAIN', + ]) { + delete process.env[key]; + } }); afterEach(async () => { @@ -176,3 +193,100 @@ describe('credential resolution', () => { expect((await resolveConfig({ cwd })).pulseAuth).toBe('env-2'); }); }); + +describe('resolveConfig: where the app is published', () => { + let cwd: string; + const originalEnv = { ...process.env }; + + beforeEach(async () => { + cwd = await mkdtemp(path.join(tmpdir(), 'patchstack-connect-url-')); + delete process.env.PATCHSTACK_SITE_URL; + delete process.env.VERCEL_ENV; + delete process.env.VERCEL_PROJECT_PRODUCTION_URL; + }); + + afterEach(async () => { + process.env = { ...originalEnv }; + await rm(cwd, { recursive: true, force: true }); + }); + + it('reports nothing from a laptop build', async () => { + await writeConfigFile(cwd, { siteUuid: VALID_UUID }); + const config = await resolveConfig({ cwd, detectSiteIdentity: true }); + expect(config.siteUrl).toBeNull(); + }); + + it('reads an explicit url from the committed config', async () => { + await writeConfigFile(cwd, { siteUuid: VALID_UUID, url: 'https://shop.example.com/home' }); + const config = await resolveConfig({ cwd, detectSiteIdentity: true }); + expect(config.siteUrl).toBe('https://shop.example.com'); + }); + + it('infers the production url from the build environment', async () => { + await writeConfigFile(cwd, { siteUuid: VALID_UUID }); + process.env.VERCEL_ENV = 'production'; + process.env.VERCEL_PROJECT_PRODUCTION_URL = 'shop.example.com'; + + const config = await resolveConfig({ cwd, detectSiteIdentity: true }); + expect(config.siteUrl).toBe('https://shop.example.com'); + }); + + it('prefers what a person configured over what the build environment reports', async () => { + await writeConfigFile(cwd, { siteUuid: VALID_UUID, url: 'https://www.example.com' }); + process.env.VERCEL_ENV = 'production'; + process.env.VERCEL_PROJECT_PRODUCTION_URL = 'shop-abc.vercel.app'; + + const config = await resolveConfig({ cwd, detectSiteIdentity: true }); + expect(config.siteUrl).toBe('https://www.example.com'); + }); + + it('lets the environment variable override the committed config', async () => { + await writeConfigFile(cwd, { siteUuid: VALID_UUID, url: 'https://old.example.com' }); + process.env.PATCHSTACK_SITE_URL = 'https://new.example.com'; + + const config = await resolveConfig({ cwd, detectSiteIdentity: true }); + expect(config.siteUrl).toBe('https://new.example.com'); + }); + + it('refuses a configured url that cannot be a published address, rather than inferring one', async () => { + // The configured value wins even when it is wrong. Silently reporting the inferred address instead + // would tell Patchstack about a different site than the one the owner wrote down, and the address is + // adopted once and then belongs to the site. + await writeConfigFile(cwd, { siteUuid: VALID_UUID, url: 'http://localhost:3000' }); + process.env.VERCEL_ENV = 'production'; + process.env.VERCEL_PROJECT_PRODUCTION_URL = 'shop.example.com'; + + await expect(resolveConfig({ cwd, detectSiteIdentity: true })).rejects.toMatchObject({ + code: 'CONFIG_INVALID', + }); + }); + + it('treats a blank PATCHSTACK_SITE_URL as unset, which is how CI clears one', async () => { + await writeConfigFile(cwd, { siteUuid: VALID_UUID }); + process.env.PATCHSTACK_SITE_URL = ' '; + process.env.VERCEL_ENV = 'production'; + process.env.VERCEL_PROJECT_PRODUCTION_URL = 'shop.example.com'; + + const config = await resolveConfig({ cwd, detectSiteIdentity: true }); + expect(config.siteUrl).toBe('https://shop.example.com'); + }); + + it('resolves nothing, and reads no project file, unless the caller asks for it', async () => { + // Only the manifest push reports these, so only the manifest push resolves them. Every other command + // shares resolveConfig, and none of them has a reason to open index.html or read a host's URL. + await writeFile(path.join(cwd, 'index.html'), 'Recipe Box', 'utf8'); + await writeConfigFile(cwd, { siteUuid: VALID_UUID, url: 'https://shop.example.com' }); + process.env.VERCEL_ENV = 'production'; + process.env.VERCEL_PROJECT_PRODUCTION_URL = 'shop.example.com'; + + const config = await resolveConfig({ cwd }); + expect(config.siteUrl).toBeNull(); + expect(config.siteName).toBeNull(); + }); + + it('does not refuse a bad configured url for a command that never reports one', async () => { + await writeConfigFile(cwd, { siteUuid: VALID_UUID, url: 'http://localhost:3000' }); + + await expect(resolveConfig({ cwd })).resolves.toMatchObject({ siteUrl: null }); + }); +}); diff --git a/tests/public-config-type.test.ts b/tests/public-config-type.test.ts new file mode 100644 index 00000000..3a1d7520 --- /dev/null +++ b/tests/public-config-type.test.ts @@ -0,0 +1,74 @@ +import { afterAll, describe, expect, it } from 'vitest'; +import { execFileSync } from 'node:child_process'; +import { mkdirSync, rmSync, writeFileSync } from 'node:fs'; +import path from 'node:path'; + +/** + * `Config` is exported from the package entry point, so a consumer can build one and hand it to + * `scanAndReport` or `postManifest`. Adding a REQUIRED field to it stops that consumer compiling on an + * upgrade, even though nothing about their code became wrong — the push omits a field it was not given. + * + * So this compiles a consumer written against the shape as it shipped, the way that consumer's own `tsc` + * would. It is the check that a new field on `Config` was added as optional. + */ +const SCRATCH = path.join(process.cwd(), '.public-types-check'); + +function compiles(source: string): { ok: boolean; output: string } { + rmSync(SCRATCH, { recursive: true, force: true }); + mkdirSync(SCRATCH, { recursive: true }); + writeFileSync(path.join(SCRATCH, 'consumer.ts'), source, 'utf8'); + + try { + execFileSync( + path.join(process.cwd(), 'node_modules/.bin/tsc'), + ['--noEmit', '--strict', '--target', 'es2022', '--module', 'esnext', '--moduleResolution', 'bundler', 'consumer.ts'], + { cwd: SCRATCH, encoding: 'utf8', stdio: 'pipe' }, + ); + return { ok: true, output: '' }; + } catch (err) { + return { ok: false, output: String((err as { stdout?: string }).stdout ?? err) }; + } +} + +describe('the exported Config stays buildable by an existing consumer', () => { + afterAll(() => rmSync(SCRATCH, { recursive: true, force: true })); + + it('compiles a Config literal written before siteUrl and siteName existed', () => { + const result = compiles(` + import type { Config } from '../src/types.js'; + + export const config: Config = { + siteUuid: '550e8400-e29b-41d4-a716-446655440000', + apiKey: 'k', + pulseAuth: 'k', + endpoint: 'https://api.patchstack.com/monitor/pulse/manifest', + timeoutMs: 30_000, + environment: 'production', + widget: true, + }; + `); + + expect(result.output).toBe(''); + expect(result.ok).toBe(true); + // Spawning a real `tsc` is slow, and slower again alongside the rest of the suite. + }, 60_000); + + it('still rejects a Config that is missing a field the package has always required', () => { + // Guards the guard: a check that accepts anything would pass the test above for the wrong reason. + const result = compiles(` + import type { Config } from '../src/types.js'; + + export const config: Config = { + siteUuid: null, + apiKey: null, + pulseAuth: null, + endpoint: 'https://api.patchstack.com/monitor/pulse/manifest', + timeoutMs: 30_000, + environment: 'production', + }; + `); + + expect(result.ok).toBe(false); + expect(result.output).toContain('widget'); + }, 60_000); +}); diff --git a/tests/site-name.test.ts b/tests/site-name.test.ts new file mode 100644 index 00000000..bc72e724 --- /dev/null +++ b/tests/site-name.test.ts @@ -0,0 +1,181 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { mkdtemp, rm, writeFile, mkdir } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import path from 'node:path'; +import { + detectSiteName, + nameFromHtmlTitle, + nameFromPackageName, + normaliseSiteName, +} from '../src/site-name.js'; +import { resolveConfig, writeConfigFile } from '../src/config.js'; + +const VALID_UUID = '11111111-2222-4333-8444-555555555555'; + +describe('normaliseSiteName', () => { + it('collapses whitespace and trims', () => { + expect(normaliseSiteName(' ToDo \n Application ')).toBe('ToDo Application'); + }); + + it('is null for nothing', () => { + expect(normaliseSiteName('')).toBeNull(); + expect(normaliseSiteName(' ')).toBeNull(); + expect(normaliseSiteName(undefined)).toBeNull(); + }); + + it('caps at what the server stores', () => { + expect(normaliseSiteName('x'.repeat(300))).toHaveLength(191); + }); +}); + +describe('nameFromHtmlTitle', () => { + it('reads the static title and decodes entities', () => { + const html = 'Arch & Studio – Design'; + expect(nameFromHtmlTitle(html)).toBe('Arch & Studio – Design'); + }); + + it('ignores attributes on the tag and whitespace inside it', () => { + expect(nameFromHtmlTitle('\n My App\n')).toBe('My App'); + }); + + it('is null when the shell has no title, or an empty one', () => { + expect(nameFromHtmlTitle('')).toBeNull(); + expect(nameFromHtmlTitle('')).toBeNull(); + expect(nameFromHtmlTitle(' ')).toBeNull(); + }); + + it('treats a template title as no title', () => { + expect(nameFromHtmlTitle('Vite + React + TS')).toBeNull(); + expect(nameFromHtmlTitle('Create Next App')).toBeNull(); + expect(nameFromHtmlTitle('DOCUMENT')).toBeNull(); + }); +}); + +describe('nameFromHtmlTitle: numeric entities it cannot represent', () => { + it('leaves a code point outside Unicode as the text it was, instead of throwing', () => { + // `` is arbitrary text out of a file this package did not write, and `String.fromCodePoint` + // throws on anything that is not a scalar value. resolveConfig is shared, so a throw here would take + // down whichever command happened to resolve the config. + for (const entity of ['�', '�', '�', '�', '�']) { + expect(nameFromHtmlTitle(`<title>Shop ${entity}`)).toBe(`Shop ${entity}`); + } + }); + + it('still decodes the ones that are characters', () => { + expect(nameFromHtmlTitle('Café 😀 & Bar')).toBe('Café 😀 & Bar'); + }); +}); + +describe('nameFromPackageName', () => { + it('humanises a package name', () => { + expect(nameFromPackageName('my-todo-app')).toBe('My Todo App'); + expect(nameFromPackageName('recipe_box.v2')).toBe('Recipe Box V2'); + }); + + it('drops the scope and keeps acronyms', () => { + expect(nameFromPackageName('@acme/crm-dashboard')).toBe('Crm Dashboard'); + expect(nameFromPackageName('API-gateway')).toBe('API Gateway'); + }); + + it('treats a template name as no name', () => { + expect(nameFromPackageName('vite_react_shadcn_ts')).toBeNull(); + expect(nameFromPackageName('vite-react-typescript-starter')).toBeNull(); + expect(nameFromPackageName('my-app')).toBeNull(); + expect(nameFromPackageName('@scope/my-app')).toBeNull(); + }); + + it('is null for anything that is not a usable string', () => { + expect(nameFromPackageName(undefined)).toBeNull(); + expect(nameFromPackageName(42)).toBeNull(); + expect(nameFromPackageName('')).toBeNull(); + expect(nameFromPackageName('@scope/')).toBeNull(); + }); +}); + +describe('detectSiteName', () => { + let cwd: string; + + beforeEach(async () => { + cwd = await mkdtemp(path.join(tmpdir(), 'patchstack-connect-name-')); + }); + + afterEach(async () => { + await rm(cwd, { recursive: true, force: true }); + }); + + it('prefers the HTML shell over the package manifest', async () => { + await writeFile(path.join(cwd, 'index.html'), 'ToDo Application'); + await writeFile(path.join(cwd, 'package.json'), JSON.stringify({ name: 'todo-app' })); + + expect(await detectSiteName(cwd)).toEqual({ name: 'ToDo Application', source: 'index.html' }); + }); + + it('reads public/index.html when the root has no shell', async () => { + await mkdir(path.join(cwd, 'public')); + await writeFile(path.join(cwd, 'public', 'index.html'), 'Recipe Box'); + + expect(await detectSiteName(cwd)).toEqual({ name: 'Recipe Box', source: 'index.html' }); + }); + + it('falls back to the package name when the shell title is a template default', async () => { + await writeFile(path.join(cwd, 'index.html'), 'Vite + React + TS'); + await writeFile(path.join(cwd, 'package.json'), JSON.stringify({ name: 'recipe-box' })); + + expect(await detectSiteName(cwd)).toEqual({ name: 'Recipe Box', source: 'package.json' }); + }); + + it('reports nothing for an unnamed template', async () => { + await writeFile(path.join(cwd, 'index.html'), ''); + await writeFile(path.join(cwd, 'package.json'), JSON.stringify({ name: 'vite_react_shadcn_ts' })); + + expect(await detectSiteName(cwd)).toBeNull(); + }); + + it('reports nothing for an empty directory or a broken package.json', async () => { + expect(await detectSiteName(cwd)).toBeNull(); + + await writeFile(path.join(cwd, 'package.json'), '{not json'); + expect(await detectSiteName(cwd)).toBeNull(); + }); +}); + +describe('resolveConfig site name', () => { + const originalEnv = { ...process.env }; + let cwd: string; + + beforeEach(async () => { + cwd = await mkdtemp(path.join(tmpdir(), 'patchstack-connect-name-cfg-')); + delete process.env.PATCHSTACK_SITE_NAME; + }); + + afterEach(async () => { + process.env = { ...originalEnv }; + await rm(cwd, { recursive: true, force: true }); + }); + + it('is null when the project states no name', async () => { + await writeConfigFile(cwd, { siteUuid: VALID_UUID }); + expect((await resolveConfig({ cwd, detectSiteIdentity: true })).siteName).toBeNull(); + }); + + it('reads an explicit name from the committed config over the project files', async () => { + await writeConfigFile(cwd, { siteUuid: VALID_UUID, name: ' Owner Chosen ' }); + await writeFile(path.join(cwd, 'index.html'), 'Shell Title'); + + expect((await resolveConfig({ cwd, detectSiteIdentity: true })).siteName).toBe('Owner Chosen'); + }); + + it('lets the environment variable override the committed config', async () => { + await writeConfigFile(cwd, { siteUuid: VALID_UUID, name: 'From File' }); + process.env.PATCHSTACK_SITE_NAME = 'From Env'; + + expect((await resolveConfig({ cwd, detectSiteIdentity: true })).siteName).toBe('From Env'); + }); + + it('detects the name from the project files when nothing is configured', async () => { + await writeConfigFile(cwd, { siteUuid: VALID_UUID }); + await writeFile(path.join(cwd, 'package.json'), JSON.stringify({ name: 'recipe-box' })); + + expect((await resolveConfig({ cwd, detectSiteIdentity: true })).siteName).toBe('Recipe Box'); + }); +}); diff --git a/tests/site-url.test.ts b/tests/site-url.test.ts new file mode 100644 index 00000000..f654fe3b --- /dev/null +++ b/tests/site-url.test.ts @@ -0,0 +1,165 @@ +import { describe, expect, it } from 'vitest'; +import { detectSiteUrl, normaliseSiteUrl } from '../src/site-url.js'; + +describe('normaliseSiteUrl', () => { + it('keeps a plain origin as it is', () => { + expect(normaliseSiteUrl('https://app.example.com')).toBe('https://app.example.com'); + }); + + it('fills in a scheme for the platforms that report a bare hostname', () => { + expect(normaliseSiteUrl('app.example.com')).toBe('https://app.example.com'); + }); + + it('reduces a reported page to the site origin', () => { + expect(normaliseSiteUrl('https://app.example.com/dashboard?ref=build#top')).toBe( + 'https://app.example.com', + ); + }); + + it('keeps a non-default port, which is part of the address', () => { + expect(normaliseSiteUrl('https://app.example.com:8443/')).toBe('https://app.example.com:8443'); + }); + + it('refuses hosts that cannot be a published site', () => { + // The address is what Patchstack fetches to check the live build. A build machine's own hostname + // would send every later check to a host that is not the site. + for (const host of [ + 'http://localhost:3000', + 'http://127.0.0.1:5173', + 'https://my-mac.local', + 'http://10.0.0.4', + 'http://192.168.1.20:3000', + 'http://172.20.0.5', + 'http://0.0.0.0:8080', + ]) { + expect(normaliseSiteUrl(host)).toBeNull(); + } + }); + + it('refuses every address that names a network rather than a published site', () => { + // The reported address is fetched by Patchstack later, so a host that resolves inside a network is + // not merely wrong. `URL` canonicalises the alternative spellings before this sees them, so matching + // the canonical form matches `0x7f.1`, `2130706433` and `[::ffff:127.0.0.1]` too. + for (const host of [ + 'http://169.254.169.254/latest/meta-data/', // cloud instance metadata + 'http://100.64.0.1', // carrier-grade NAT + 'http://198.18.0.1', // benchmarking range + 'http://[::]', + 'http://[fe80::1]', + 'http://[fc00::1]', + 'http://[::ffff:127.0.0.1]', + 'http://0x7f.1', + 'http://2130706433', + 'https://203.0.113.10', // a public literal is still a literal + ]) { + expect(normaliseSiteUrl(host)).toBeNull(); + } + }); + + it('refuses a host that is only a name on somebody’s own network', () => { + for (const host of [ + 'https://production', + 'https://localhost.', + 'https://LOCALHOST', + 'https://build-01', + 'https://api.internal', + 'https://box.home.arpa', + 'https://shop.test', + 'https://shop.example', + ]) { + expect(normaliseSiteUrl(host)).toBeNull(); + } + }); + + it('still accepts an ordinary published address', () => { + for (const host of [ + 'https://shop.example.com', + 'https://my-app-abc123.vercel.app', + 'https://internal-tools.acme.io', + 'https://example.com.au', + ]) { + expect(normaliseSiteUrl(host)).toBe(host); + } + }); + + it('refuses the placeholder host Patchstack uses for "no address known"', () => { + expect(normaliseSiteUrl('https://pulse-abc123.placeholder.invalid')).toBeNull(); + }); + + it('refuses anything that is not http(s), empty, or unparseable', () => { + expect(normaliseSiteUrl('ftp://files.example.com')).toBeNull(); + expect(normaliseSiteUrl(' ')).toBeNull(); + expect(normaliseSiteUrl(undefined)).toBeNull(); + expect(normaliseSiteUrl(null)).toBeNull(); + expect(normaliseSiteUrl('https://')).toBeNull(); + }); + + it('refuses an origin longer than Patchstack accepts', () => { + expect(normaliseSiteUrl(`https://${'a'.repeat(200)}.example.com`)).toBeNull(); + }); +}); + +describe('detectSiteUrl', () => { + it('reads Vercel’s production domain on a production deployment', () => { + expect( + detectSiteUrl({ + VERCEL: '1', + VERCEL_ENV: 'production', + VERCEL_PROJECT_PRODUCTION_URL: 'shop.example.com', + VERCEL_URL: 'shop-git-abc123-team.vercel.app', + }), + ).toEqual({ url: 'https://shop.example.com', platform: 'vercel' }); + }); + + it('reports nothing for a Vercel preview build', () => { + // The address is adopted once and then belongs to the site, so a per-deployment preview URL must + // never become it. + expect( + detectSiteUrl({ + VERCEL: '1', + VERCEL_ENV: 'preview', + VERCEL_PROJECT_PRODUCTION_URL: 'shop.example.com', + VERCEL_URL: 'shop-git-abc123-team.vercel.app', + }), + ).toBeNull(); + }); + + it('reads Netlify’s site URL only in the production context', () => { + const env = { NETLIFY: 'true', URL: 'https://site.example.com' }; + expect(detectSiteUrl({ ...env, CONTEXT: 'production' })).toEqual({ + url: 'https://site.example.com', + platform: 'netlify', + }); + expect(detectSiteUrl({ ...env, CONTEXT: 'deploy-preview' })).toBeNull(); + }); + + it('ignores a bare URL variable with no Netlify marker beside it', () => { + // `URL` is generic enough to belong to any tool in the build. + expect(detectSiteUrl({ URL: 'https://something-else.example.com' })).toBeNull(); + }); + + it('reads Render’s external URL, except on a pull-request preview', () => { + const env = { RENDER: 'true', RENDER_EXTERNAL_URL: 'https://api.example.com' }; + expect(detectSiteUrl(env)).toEqual({ url: 'https://api.example.com', platform: 'render' }); + expect(detectSiteUrl({ ...env, IS_PULL_REQUEST: 'true' })).toBeNull(); + }); + + it('reads Railway’s public domain in the production environment only', () => { + const env = { RAILWAY_PUBLIC_DOMAIN: 'app.up.railway.app' }; + expect(detectSiteUrl({ ...env, RAILWAY_ENVIRONMENT_NAME: 'production' })).toEqual({ + url: 'https://app.up.railway.app', + platform: 'railway', + }); + expect(detectSiteUrl({ ...env, RAILWAY_ENVIRONMENT_NAME: 'staging' })).toBeNull(); + }); + + it('reports nothing for a laptop build', () => { + expect(detectSiteUrl({})).toBeNull(); + }); + + it('reports nothing on Cloudflare Pages, which cannot say which deployment is production', () => { + expect( + detectSiteUrl({ CF_PAGES: '1', CF_PAGES_URL: 'https://abc123.project.pages.dev' }), + ).toBeNull(); + }); +}); From 0a98ada032c03e48fe11880237ab0ad179f6273e Mon Sep 17 00:00:00 2001 From: Danilo Date: Sun, 6 Sep 2026 09:37:55 +0200 Subject: [PATCH 2/2] Read a title's characters rather than computing them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `` decoding handled numeric entities as well as named ones, which meant passing arbitrary numbers out of a file this package did not write to `String.fromCodePoint`. That throws for anything which is not a Unicode scalar value, and it threw out of `resolveConfig` — so a single malformed entity took down whichever command happened to be resolving the config, not just `scan`. The guard for it was a scalar-range check. This removes the reason to have one instead: numeric entities are no longer decoded, and are reported as the text they were. Any editor writing a UTF-8 file puts those characters in literally, so the case this handled is the legacy one, and a prettier dashboard name — a name the owner can edit, and which is only ever applied to a site that has none — does not justify arithmetic on untrusted input. `&` and its neighbours stay: `&` is the one character HTML forces an author to escape, so it is the entity a title actually carries, and a fixed dictionary lookup cannot fail. The crash cases keep their test. It passes trivially now, which is the point: decoding numeric entities again would need the scalar check back, and this is what says so. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --- src/site-name.ts | 35 +++++++++++------------------------ tests/site-name.test.ts | 23 ++++++++++++----------- 2 files changed, 23 insertions(+), 35 deletions(-) diff --git a/src/site-name.ts b/src/site-name.ts index cd1df631..4c58ddc2 100644 --- a/src/site-name.ts +++ b/src/site-name.ts @@ -63,6 +63,16 @@ const TEMPLATE_TITLES: ReadonlySet<string> = new Set([ 'app', ]); +/** + * The named entities an HTML title realistically carries. `&` is the one that has to be escaped in HTML, + * so `&` is what actually turns up; the rest cost nothing to decode alongside it. + * + * Numeric entities (`é`, `😀`) are deliberately NOT decoded, and are left as the text they + * were. Any editor writing a UTF-8 file puts those characters in literally, so decoding them bought + * close to nothing — and it meant turning arbitrary numbers out of a file this package did not write + * into code points, which throws for anything that is not a Unicode scalar value. A prettier dashboard + * name does not justify a crash in the settings every command resolves. + */ const NAMED_ENTITIES: Record<string, string> = { amp: '&', lt: '<', @@ -80,31 +90,8 @@ export function normaliseSiteName(value: string | undefined | null): string | nu return collapsed.slice(0, NAME_MAX_LENGTH); } -/** - * Whether `code` is a Unicode scalar value, the only thing `String.fromCodePoint` accepts. - * - * Everything else in the numeric range an HTML author can write throws: past `0x10FFFF` there is no - * character, and a lone surrogate is half of one. `<title>` is arbitrary text from a file this package - * did not write, so a number it cannot represent has to be left as the text it was. - */ -function isScalarValue(code: number): boolean { - return ( - Number.isInteger(code) && code >= 0 && code <= 0x10ffff && !(code >= 0xd800 && code <= 0xdfff) - ); -} - function decodeEntities(text: string): string { - return text.replace(/&(#x[0-9a-f]+|#\d+|[a-z]+);/gi, (whole, body: string) => { - if (body.startsWith('#x') || body.startsWith('#X')) { - const code = Number.parseInt(body.slice(2), 16); - return isScalarValue(code) ? String.fromCodePoint(code) : whole; - } - if (body.startsWith('#')) { - const code = Number.parseInt(body.slice(1), 10); - return isScalarValue(code) ? String.fromCodePoint(code) : whole; - } - return NAMED_ENTITIES[body.toLowerCase()] ?? whole; - }); + return text.replace(/&([a-z]+);/gi, (whole, name: string) => NAMED_ENTITIES[name.toLowerCase()] ?? whole); } /** diff --git a/tests/site-name.test.ts b/tests/site-name.test.ts index bc72e724..c43d4edc 100644 --- a/tests/site-name.test.ts +++ b/tests/site-name.test.ts @@ -29,9 +29,9 @@ describe('normaliseSiteName', () => { }); describe('nameFromHtmlTitle', () => { - it('reads the static title and decodes entities', () => { - const html = '<!doctype html><html><head><title>Arch & Studio – Design'; - expect(nameFromHtmlTitle(html)).toBe('Arch & Studio – Design'); + it('reads the static title and decodes the entity HTML forces an author to write', () => { + const html = 'Arch & Studio'; + expect(nameFromHtmlTitle(html)).toBe('Arch & Studio'); }); it('ignores attributes on the tag and whitespace inside it', () => { @@ -51,18 +51,19 @@ describe('nameFromHtmlTitle', () => { }); }); -describe('nameFromHtmlTitle: numeric entities it cannot represent', () => { - it('leaves a code point outside Unicode as the text it was, instead of throwing', () => { - // `` is arbitrary text out of a file this package did not write, and `String.fromCodePoint` - // throws on anything that is not a scalar value. resolveConfig is shared, so a throw here would take - // down whichever command happened to resolve the config. - for (const entity of ['�', '�', '�', '�', '�']) { +describe('nameFromHtmlTitle: numeric entities', () => { + it('leaves them as the text they were, rather than turning numbers into code points', () => { + // Decoding these meant handing arbitrary numbers out of a file this package did not write to + // `String.fromCodePoint`, which throws for anything that is not a Unicode scalar value — and it + // threw out of `resolveConfig`, which every command calls. `�` is that crash; the rest + // are the neighbouring cases. Kept as a regression guard: decoding these again needs a scalar check. + for (const entity of ['�', '�', '�', '�', '�', 'é']) { expect(nameFromHtmlTitle(`<title>Shop ${entity}`)).toBe(`Shop ${entity}`); } }); - it('still decodes the ones that are characters', () => { - expect(nameFromHtmlTitle('Café 😀 & Bar')).toBe('Café 😀 & Bar'); + it('reads a title whose characters are written literally, which is what a UTF-8 file does', () => { + expect(nameFromHtmlTitle('Café 😀 & Bar')).toBe('Café 😀 & Bar'); }); });