From 416609217b4809d59cccd22f20f0c95e6b1c40d7 Mon Sep 17 00:00:00 2001 From: Mario Tarosso Date: Wed, 23 Sep 2026 08:14:17 +0100 Subject: [PATCH] Collapse duplicated helpers and drop dead code Ten cleanups across the CLI, config, parsers and the protect scaffolder. None of them change behaviour: no printed string, doc or guide-checklist output is touched. The largest group is reuse. Four adapters in the protect scaffolder had each inlined a character-for-character copy of hasDependency() from the util module sitting next to them, and the CLI had four copies of the same resolveConfig() call, two of the same CI test, and two of the same gitignore-outcome line. The rest is dead code and naming: two exported functions with no callers anywhere, a pair of destructured locals whose underscore prefix said "unused" while both were read two lines later, and a bare 191 that now has the same name its twin already carries in site-name.ts. Co-Authored-By: Claude Opus 5 (1M context) --- src/cli.ts | 87 ++++++++++++----------- src/config.ts | 4 +- src/login.ts | 8 --- src/map/coordinates.ts | 13 ---- src/normalize.ts | 9 +-- src/protect/install/adapters/astro.ts | 10 +-- src/protect/install/adapters/next.ts | 13 +--- src/protect/install/adapters/sveltekit.ts | 10 +-- src/protect/install/generic.ts | 13 +--- src/site-url.ts | 7 +- 10 files changed, 60 insertions(+), 114 deletions(-) diff --git a/src/cli.ts b/src/cli.ts index f38ab51..5a88d8e 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -38,6 +38,7 @@ import { secretFileIgnored, persistSiteUuid, resolveConfig, + type ResolveConfigOptions, writeConfigFile, persistTimeout, } from './config.js'; @@ -76,6 +77,7 @@ import { runProtect, runVerify } from './protect/install/index.js'; import { formatRuntimeCheck, runRuntimeCheck, runtimeExitCode } from './protect/install/runtime/check.js'; import { runMap } from './map-command.js'; import { getStringFlag } from './flags.js'; +import { isCanonicalUuid } from './endpoint-policy.js'; import { setupProtection, wireBuildScripts } from './setup.js'; import type { SetupProtectionResult, WireBuildScriptsResult } from './setup.js'; import { isInstallOrBuildHook, isPreBundleBuildHook, undeliveredReportLines } from './build-hook.js'; @@ -296,7 +298,38 @@ function parseArgs(argv: string[]): ParsedArgs { }; } +/** The site/endpoint overrides every command accepts, resolved against the current directory. */ +function resolveCliConfig( + args: ParsedArgs, + extra: Omit = {}, +): Promise { + return resolveConfig({ + cwd: process.cwd(), + cliSiteUuid: getStringFlag(args.flags, 'site-uuid'), + cliEndpoint: getStringFlag(args.flags, 'endpoint'), + ...extra, + }); +} + +/** + * Whether a build agent is running this. `CI=false` is how a platform says "not a CI build", so it + * counts as absent — the interactive commands refuse only where there is really no one to answer. + */ +function runningInCi(): boolean { + return process.env.CI !== undefined && process.env.CI !== '' && process.env.CI !== 'false'; +} +/** + * What became of the credential file's ignore entry, for the person reading the output. + * + * Only claimed when `.gitignore` was read back and really covers it: an assurance that turns out to be + * false is worse than none, because it is the reason somebody stops checking. + */ +function gitignoreOutcomeLine(ignore: { ignored: boolean; reason?: string }): string { + return ignore.ignored + ? ' Added to .gitignore.' + : ` NOT ignored by git — ${ignore.reason ?? 'unknown reason'}. Add it to .gitignore yourself before committing.`; +} async function runInit(args: ParsedArgs): Promise { const uuid = args.positional[0]; @@ -305,7 +338,7 @@ async function runInit(args: ParsedArgs): Promise { console.error('Usage: patchstack-connect init '); return 1; } - if (!/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i.test(uuid)) { + if (!isCanonicalUuid(uuid)) { console.error(`Error: "${uuid}" does not look like a valid UUID.`); return 1; } @@ -319,16 +352,12 @@ async function runInit(args: ParsedArgs): Promise { async function runClaim(args: ParsedArgs): Promise { // No browser and no human to sign in. A deploy inherits an already-claimed site; it never claims. - if (process.env.CI !== undefined && process.env.CI !== '' && process.env.CI !== 'false') { + if (runningInCi()) { console.error('`claim` is interactive and cannot run in CI. Claim the site from a developer machine.'); return 1; } - const config = await resolveConfig({ - cwd: process.cwd(), - cliSiteUuid: getStringFlag(args.flags, 'site-uuid'), - cliEndpoint: getStringFlag(args.flags, 'endpoint'), - }); + const config = await resolveCliConfig(args); const siteUuid = config.siteUuid; if (siteUuid === null) { @@ -345,11 +374,7 @@ async function runClaim(args: ParsedArgs): Promise { const ignore = await secretFileIgnored(process.cwd()); // The value itself is never printed — only that it landed, and only that it is ignored when it is. console.log(` A credential for this site was issued and saved to ${SECRET_CONFIG_FILENAME}.`); - console.log( - ignore.ignored - ? ' Added to .gitignore.' - : ` NOT ignored by git — ${ignore.reason ?? 'unknown reason'}. Add it to .gitignore yourself before committing.`, - ); + console.log(gitignoreOutcomeLine(ignore)); } console.log(''); return 0; @@ -438,16 +463,12 @@ async function runClaim(args: ParsedArgs): Promise { async function runLogin(args: ParsedArgs): Promise { // CI has no browser and no human; build agents must not print credentials // into logs. Deploys use PATCHSTACK_PULSE_AUTH from the platform's secrets. - if (process.env.CI !== undefined && process.env.CI !== '' && process.env.CI !== 'false') { + if (runningInCi()) { console.error('`login` is interactive and cannot run in CI. Set PATCHSTACK_PULSE_AUTH instead.'); return 1; } - const config = await resolveConfig({ - cwd: process.cwd(), - cliSiteUuid: getStringFlag(args.flags, 'site-uuid'), - cliEndpoint: getStringFlag(args.flags, 'endpoint'), - }); + const config = await resolveCliConfig(args); // Checked here rather than carried up from the write: the rotation happens several layers down, and the // claim belongs to the line that prints it. @@ -455,11 +476,7 @@ async function runLogin(args: ParsedArgs): Promise { const ignore = await secretFileIgnored(process.cwd()); // The value itself is never printed — only that it landed, and only that it is ignored when it is. console.log(`\n ✓ Credential restored and saved to ${SECRET_CONFIG_FILENAME}.`); - console.log( - ignore.ignored - ? ' Added to .gitignore.' - : ` NOT ignored by git — ${ignore.reason ?? 'unknown reason'}. Add it to .gitignore yourself before committing.`, - ); + console.log(gitignoreOutcomeLine(ignore)); console.log(' The previous credential no longer works. Update it anywhere else it was set:'); console.log(' CI secrets, hosting env vars, preview environments, other checkouts.\n'); return 0; @@ -619,10 +636,7 @@ async function runScan( options: { showRemainingSetup?: boolean } = {}, ): Promise { const dryRun = args.flags.get('dry-run') === true; - const config = await resolveConfig({ - cwd: process.cwd(), - cliSiteUuid: getStringFlag(args.flags, 'site-uuid'), - cliEndpoint: getStringFlag(args.flags, 'endpoint'), + const config = await resolveCliConfig(args, { cliClaimToken: getStringFlag(args.flags, 'claim-token'), // The one command that reports them, so the one command that resolves them. detectSiteIdentity: true, @@ -1000,12 +1014,7 @@ async function runDemoCommand(args: ParsedArgs): Promise { try { const scenario = resolveDemoScenario(args.positional[0]); const cwd = process.cwd(); - const config = await resolveConfig({ - cwd, - cliSiteUuid: getStringFlag(args.flags, 'site-uuid'), - cliEndpoint: getStringFlag(args.flags, 'endpoint'), - requireSiteUuid: true, - }); + const config = await resolveCliConfig(args, { requireSiteUuid: true }); if (config.environment !== 'production') { throw new DemoError( 'The production-backed demo requires PATCHSTACK_ENVIRONMENT=production. Unset the sandbox override and try again.', @@ -1260,11 +1269,7 @@ function setupOutcome( } async function runStatus(args: ParsedArgs): Promise { - const config = await resolveConfig({ - cwd: process.cwd(), - cliSiteUuid: getStringFlag(args.flags, 'site-uuid'), - cliEndpoint: getStringFlag(args.flags, 'endpoint'), - }); + const config = await resolveCliConfig(args); console.log(`Site UUID: ${config.siteUuid ?? '(none yet — the next `scan` will provision one)'}`); console.log( `Endpoint: ${config.endpoint}${config.endpoint === DEFAULT_ENDPOINT ? '' : ' (override)'}`, @@ -1300,11 +1305,7 @@ async function runStatus(args: ParsedArgs): Promise { } async function runUninstall(args: ParsedArgs): Promise { - const config = await resolveConfig({ - cwd: process.cwd(), - cliSiteUuid: getStringFlag(args.flags, 'site-uuid'), - cliEndpoint: getStringFlag(args.flags, 'endpoint'), - }); + const config = await resolveCliConfig(args); if (config.siteUuid === null) { console.log('No site UUID configured — there is no site record to signal about.'); diff --git a/src/config.ts b/src/config.ts index dfc222e..4535476 100644 --- a/src/config.ts +++ b/src/config.ts @@ -448,13 +448,13 @@ export async function persistTimeout(cwd: string, timeoutMs: number): Promise { - const { apiKey: _movedKey, pulseAuth: _movedPulse, ...publicConfig } = await readConfigFile(cwd); + const { apiKey: movedKey, pulseAuth: movedPulse, ...publicConfig } = await readConfigFile(cwd); const existingSecrets = await readSecretFile(cwd); const target = await writeSecretFile(cwd, { ...existingSecrets, apiKey, pulseAuth: undefined }); // Only rewritten when it actually held a credential, so a normal provision does not touch it. - if (_movedKey !== undefined || _movedPulse !== undefined) { + if (movedKey !== undefined || movedPulse !== undefined) { await writeConfigFile(cwd, publicConfig); } diff --git a/src/login.ts b/src/login.ts index 8ad6a8a..d8d6e56 100644 --- a/src/login.ts +++ b/src/login.ts @@ -1,6 +1,3 @@ -import { mkdtempSync } from 'node:fs'; -import { tmpdir } from 'node:os'; -import path from 'node:path'; import { persistApiKey } from './config.js'; import { clearPending, @@ -127,8 +124,3 @@ export async function login( return waitForApproval(config, started.pending, deps); } - -/** Exported for tests that need a scratch temp dir. */ -export function makeTempDir(prefix = 'patchstack-'): string { - return mkdtempSync(path.join(tmpdir(), prefix)); -} diff --git a/src/map/coordinates.ts b/src/map/coordinates.ts index b0d5ef7..ee0f3ba 100644 --- a/src/map/coordinates.ts +++ b/src/map/coordinates.ts @@ -76,19 +76,6 @@ export function inputIdOf(source: InputSource | undefined, path: string): string return `${addressSpaceOf(source)}:${path}`; } -/** - * The rule-engine NAMESPACE an input lands in (`post`, `get`, `cookie`, `files`, `server`), or null when - * it has no address. Derived from `runtimeCoordinate` on purpose: comparing raw source labels would call - * `json-body` and an Express `req.body` read different places when both resolve to `post.*`, and would - * miss that `post.id` and `get.id` are genuinely different places. - */ -export function namespaceOf(source: InputSource | undefined, path: string): string | null { - const { runtimeParameter } = runtimeCoordinate(source, path); - if (!runtimeParameter) return null; - const dot = runtimeParameter.indexOf('.'); - return dot === -1 ? runtimeParameter : runtimeParameter.slice(0, dot); -} - /** Place extracted fields in a request region: attach `source`, the runtime coordinate, and the id. */ export function withCoordinates(fields: FieldShape[], source: InputSource): InputField[] { return fields.map((f) => { diff --git a/src/normalize.ts b/src/normalize.ts index e12deb0..35537bc 100644 --- a/src/normalize.ts +++ b/src/normalize.ts @@ -1,4 +1,4 @@ -import type { Manifest, PackageEntry } from './types.js'; +import type { Manifest } from './types.js'; export interface WirePackage { name: string; @@ -210,10 +210,3 @@ function compareSegments(a: string[], b: string[]): number { } return 0; } - -export function findPackageInManifest( - manifest: Manifest, - name: string, -): PackageEntry[] { - return manifest.packages.filter((p) => p.name === name); -} diff --git a/src/protect/install/adapters/astro.ts b/src/protect/install/adapters/astro.ts index 467452b..e607c51 100644 --- a/src/protect/install/adapters/astro.ts +++ b/src/protect/install/adapters/astro.ts @@ -1,6 +1,5 @@ // Adapter: Astro. Wires the guard as middleware (`src/middleware.ts` → `onRequest`). -import { join } from 'node:path'; -import { read } from '../util.js'; +import { hasDependency } from '../util.js'; import { wireSeam, verifySeam, type SeamSpec } from '../seam.js'; import type { Adapter, WireOptions, WireResult, VerifyResult } from '../types.js'; @@ -14,12 +13,7 @@ const SPEC: SeamSpec = { }; function detect(cwd: string): boolean { - try { - const pkg = JSON.parse(read(join(cwd, 'package.json'))); - return Boolean({ ...pkg.dependencies, ...pkg.devDependencies }.astro); - } catch { - return false; - } + return hasDependency(cwd, 'astro'); } function wire(cwd: string, opts: WireOptions): WireResult { diff --git a/src/protect/install/adapters/next.ts b/src/protect/install/adapters/next.ts index 2e9c0fa..403a87d 100644 --- a/src/protect/install/adapters/next.ts +++ b/src/protect/install/adapters/next.ts @@ -5,19 +5,10 @@ import { existsSync } from 'node:fs'; import { join } from 'node:path'; -import { bakeSiteUuid, read, log, templatesDir } from '../util.js'; +import { bakeSiteUuid, hasDependency, read, log, templatesDir } from '../util.js'; import type { Adapter, WireOptions, WireResult, VerifyResult } from '../types.js'; import { copyProjectFileSync, ensureProjectDirectorySync } from '../../../safe-file.js'; -function hasNextDep(cwd: string): boolean { - try { - const pkg = JSON.parse(read(join(cwd, 'package.json'))); - return Boolean({ ...pkg.dependencies, ...pkg.devDependencies }.next); - } catch { - return false; - } -} - // Next reads middleware from `middleware.ts` at the project root, or `src/middleware.ts` when the // app uses a `src/` dir. Return the existing one if present, else the conventional target. function middlewareInfo(cwd: string): { relDir: string; relFile: string; exists: boolean } { @@ -30,7 +21,7 @@ function middlewareInfo(cwd: string): { relDir: string; relFile: string; exists: } function detect(cwd: string): boolean { - return hasNextDep(cwd); + return hasDependency(cwd, 'next'); } function rulesFile(relDir: string): string { diff --git a/src/protect/install/adapters/sveltekit.ts b/src/protect/install/adapters/sveltekit.ts index fee53e4..675efec 100644 --- a/src/protect/install/adapters/sveltekit.ts +++ b/src/protect/install/adapters/sveltekit.ts @@ -1,6 +1,5 @@ // Adapter: SvelteKit. Wires the guard as a server hook (`src/hooks.server.ts` → `handle`). -import { join } from 'node:path'; -import { read } from '../util.js'; +import { hasDependency } from '../util.js'; import { wireSeam, verifySeam, type SeamSpec } from '../seam.js'; import type { Adapter, WireOptions, WireResult, VerifyResult } from '../types.js'; @@ -14,12 +13,7 @@ const SPEC: SeamSpec = { }; function detect(cwd: string): boolean { - try { - const pkg = JSON.parse(read(join(cwd, 'package.json'))); - return Boolean({ ...pkg.dependencies, ...pkg.devDependencies }['@sveltejs/kit']); - } catch { - return false; - } + return hasDependency(cwd, '@sveltejs/kit'); } function wire(cwd: string, opts: WireOptions): WireResult { diff --git a/src/protect/install/generic.ts b/src/protect/install/generic.ts index c39b7b8..5774494 100644 --- a/src/protect/install/generic.ts +++ b/src/protect/install/generic.ts @@ -5,7 +5,7 @@ import { readFileSync, existsSync, readdirSync, lstatSync } from 'node:fs'; import { join, dirname } from 'node:path'; -import { bakeSiteUuid, read, templatesDir } from './util.js'; +import { bakeSiteUuid, hasDependency, read, templatesDir } from './util.js'; import type { WireOptions, VerifyResult } from './types.js'; import type { GuardModuleQuery } from './source-scope.js'; import { copyProjectFileSync, ensureProjectDirectorySync } from '../../safe-file.js'; @@ -132,18 +132,9 @@ function candidateEntries(cwd: string): string[] { return [...new Set(hits)]; } -function usesExpress(cwd: string): boolean { - try { - const pkg = JSON.parse(read(join(cwd, 'package.json'))); - return Boolean({ ...pkg.dependencies, ...pkg.devDependencies }.express); - } catch { - return false; - } -} - export function wiringPlan(cwd: string, dir: string): string { const entries = candidateEntries(cwd); - const express = usesExpress(cwd); + const express = hasDependency(cwd, 'express'); const target = genericGuardTarget(cwd); const lines = [ `no built-in adapter matched this stack — scaffolded a generic guard at ${dir}/${target.file} + ${dir}/rules.json.`, diff --git a/src/site-url.ts b/src/site-url.ts index d34727b..9b8fb16 100644 --- a/src/site-url.ts +++ b/src/site-url.ts @@ -67,6 +67,9 @@ function isUnpublishableHost(hostname: string): boolean { return RESERVED_SUFFIXES.some((suffix) => host.endsWith(suffix)); } +/** The most Patchstack accepts for an address. */ +const URL_MAX_LENGTH = 191; + /** * Reduce a reported address to the `scheme://host[:port]` Patchstack stores, or null when it is not one. * @@ -92,8 +95,8 @@ export function normaliseSiteUrl(value: string | undefined | null): string | nul 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; + // An origin near the limit is not one. + return origin.length <= URL_MAX_LENGTH ? origin : null; } /**