Skip to content

Commit bf94b97

Browse files
mariojgtclaude
andauthored
Keep a refused report from failing the build it is hooked into (#204)
`setup` wires `scan` into postinstall, prebuild, or the Bun build chain. In that position, a manifest Patchstack refused or could not receive ended the build, and the usual reason is one the build cannot fix: the credential file is git-ignored, so a clean checkout never has it. When the package manager names the running script as an install or build lifecycle event, a report that cannot be delivered is now printed in full on stderr and `scan` exits 0. A direct `scan` still exits 1 for the same failure. The no-credential line names both config files and the directory it checked, and points at PATCHSTACK_API_KEY rather than at `login`, which cannot run in a build. Bun sets npm_lifecycle_event for `bun run` but not for install-time scripts, so a postinstall scan under `bun install` still fails closed; the code comment and README say so. Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
1 parent b8f043e commit bf94b97

5 files changed

Lines changed: 283 additions & 5 deletions

File tree

‎README.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,12 @@ PATCHSTACK_ENVIRONMENT=sandbox npx @patchstack/connect setup
160160

161161
The generated `prebuild` scan deliberately carries no hard-coded environment. A production builder with no override reports `production`; a preview/sandbox builder must receive `PATCHSTACK_ENVIRONMENT=sandbox` from its host. Runtime protection itself is not environment-specific: `PATCHSTACK_ENVIRONMENT` labels manifests only. Use `PATCHSTACK_MODE=dry-run` when protection should observe rather than block.
162162

163+
### `scan` as a build hook
164+
165+
`setup` wires `scan` into `postinstall`, `prebuild`, or the Bun `build` chain. Run from one of those, a report Patchstack cannot accept — no credential in the build environment, a rejected credential, a site that no longer exists, an outage — is printed on stderr and `scan` exits 0, so the install or build it is attached to carries on. Patchstack keeps the last manifest it accepted for the site until a scan that can report. Run directly (`npx @patchstack/connect scan`), the same failure exits 1.
166+
167+
A deploy never has `.patchstackrc.local.json`, so the usual cause is a missing `PATCHSTACK_API_KEY` in the platform's environment (see *Configuration*). The hook is recognised through `npm_lifecycle_event`, which npm, pnpm, Yarn and `bun run` set to the running script's name. `bun install` does not set it, so a `postinstall` scan under Bun still fails the install when it cannot report.
168+
163169
## Production virtual-patch demo
164170

165171
The `node-serialize` scenario demonstrates dependency detection and a live, version-scoped virtual patch against a throwaway Express application. Connect/provision the project first, deliberately add the known-vulnerable package, then run:

‎src/build-hook.ts‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
import { SECRET_CONFIG_FILENAME } from './config.js';
2+
import type { Config, PatchstackError } from './types.js';
3+
4+
/**
5+
* Lifecycle names under which `scan` is a hook on somebody else's install or build: the `postinstall`,
6+
* `prebuild` and Bun `build` chain that `setup` wires, and the neighbours a project wires by hand.
7+
*/
8+
const INSTALL_AND_BUILD_EVENTS: ReadonlySet<string> = new Set([
9+
'preinstall',
10+
'install',
11+
'postinstall',
12+
'prepare',
13+
'prebuild',
14+
'build',
15+
'postbuild',
16+
]);
17+
18+
/**
19+
* Whether this process is a lifecycle hook on an install or build.
20+
*
21+
* In that position the report is the connector's concern and the build is not: a manifest Patchstack
22+
* cannot accept is said in full, and the build goes on. Run directly, the same failure exits non-zero.
23+
*
24+
* npm, pnpm, Yarn and `bun run` name the running script in `npm_lifecycle_event`; a direct invocation
25+
* carries a different name (`npx`, or the bin's own name under Bun) or none at all. Bun does not set it
26+
* for install-time scripts, so a `postinstall` hook under `bun install` is not recognised and fails
27+
* closed. The variable is inherited by child processes, so a direct run from inside a build's process
28+
* tree counts as a hook too — the error is still printed, so what that costs is an exit code, not silence.
29+
*/
30+
export function isInstallOrBuildHook(env: NodeJS.ProcessEnv = process.env): boolean {
31+
const event = env.npm_lifecycle_event;
32+
33+
return typeof event === 'string' && INSTALL_AND_BUILD_EVENTS.has(event);
34+
}
35+
36+
/**
37+
* What a hooked `scan` prints when it could not deliver its report.
38+
*
39+
* The error's own message names the remedy for a developer machine. A build environment needs a different
40+
* one: it never has the credential file, `login` refuses to run there, and the only fix is the platform's
41+
* environment. Said for the credential cases only; a network or server failure has nothing to set.
42+
*
43+
* When no credential was found, the line names every source that was checked and where. Somebody who can
44+
* see the file on their own machine reads "none is configured" as "the file was not read", and the log is
45+
* the only place that can settle it.
46+
*/
47+
export function undeliveredReportLines(err: PatchstackError, config: Config, cwd: string): string[] {
48+
const lines = [`patchstack: manifest not reported — ${err.message}`];
49+
50+
if (err.code === 'UNAUTHORIZED') {
51+
const hasCredential = typeof config.pulseAuth === 'string' && config.pulseAuth.length > 0;
52+
if (hasCredential) {
53+
lines.push(
54+
'patchstack: the credential this environment holds was rejected. If `login` rotated it, set the new value as PATCHSTACK_API_KEY here.',
55+
);
56+
} else {
57+
lines.push(
58+
`patchstack: PATCHSTACK_API_KEY is not set, and neither ${SECRET_CONFIG_FILENAME} nor .patchstackrc.json in ${cwd} holds a credential.`,
59+
`patchstack: ${SECRET_CONFIG_FILENAME} is git-ignored, so a clean checkout never has it. Set PATCHSTACK_API_KEY in the platform's environment so builds can report.`,
60+
);
61+
}
62+
}
63+
64+
lines.push(
65+
'patchstack: continuing the build. Patchstack keeps the last manifest it accepted for this site until a scan that can report.',
66+
);
67+
68+
return lines;
69+
}

‎src/cli.ts‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,9 @@ import { runProtect, runVerify } from './protect/install/index.js';
5555
import { runMap } from './map-command.js';
5656
import { getStringFlag } from './flags.js';
5757
import { setupProtection, wireBuildScripts } from './setup.js';
58+
import { isInstallOrBuildHook, undeliveredReportLines } from './build-hook.js';
5859
import { detectStack, type StackDescriptor } from './stack.js';
59-
import { PatchstackError } from './types.js';
60+
import { PatchstackError, type StoreManifestResponse } from './types.js';
6061
import { buildWidgetTag, ensureSourceWidget, ensureWidgetInHtml } from './widget.js';
6162

6263
const HELP = `@patchstack/connect — scan your lockfile and report packages to Patchstack.
@@ -69,7 +70,11 @@ Usage:
6970
disclosure-widget <script> tag in the root
7071
HTML shell (index.html, public/index.html,
7172
or src/app.html) — opt out with
72-
"widget": false in .patchstackrc.json
73+
"widget": false in .patchstackrc.json.
74+
Run as a postinstall/prebuild/build hook, a
75+
report Patchstack cannot accept is printed
76+
and exits 0 so the build goes on; run
77+
directly, the same failure exits 1
7378
patchstack-connect setup [options] Finish the bounded project setup: run scan,
7479
manage the widget, install + verify runtime
7580
protection, and wire dependency/build scans.
@@ -428,7 +433,16 @@ async function runScan(
428433
console.log('No site UUID configured — provisioning a new Patchstack site from this manifest…');
429434
}
430435

431-
const response = await postManifest(config, payload);
436+
// Hooked into an install or build, the report is this command's concern and the build is not: the
437+
// failure is said in full on stderr and the build goes on. A direct `scan` still exits non-zero for it.
438+
let response: StoreManifestResponse;
439+
try {
440+
response = await postManifest(config, payload);
441+
} catch (err) {
442+
if (!(err instanceof PatchstackError) || !isInstallOrBuildHook()) throw err;
443+
for (const line of undeliveredReportLines(err, config, process.cwd())) console.error(line);
444+
return 0;
445+
}
432446

433447
// The server always returns the UUID. If we didn't have one, persist it so
434448
// every subsequent scan targets the same site.

‎tests/bin-invocation.test.ts‎

Lines changed: 101 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
import { describe, expect, it } from 'vitest';
2-
import { execFileSync } from 'node:child_process';
3-
import { existsSync, mkdtempSync, rmSync, symlinkSync } from 'node:fs';
2+
import { execFile, execFileSync } from 'node:child_process';
3+
import { promisify } from 'node:util';
4+
import { copyFileSync, existsSync, mkdtempSync, rmSync, symlinkSync, writeFileSync } from 'node:fs';
5+
import { createServer } from 'node:http';
6+
import type { AddressInfo } from 'node:net';
47
import { fileURLToPath } from 'node:url';
58
import { tmpdir } from 'node:os';
69
import path from 'node:path';
@@ -78,4 +81,100 @@ describe.skipIf(!built)('the packaged bin, invoked as npm invokes it', () => {
7881
}));
7982
expect(pkg['patchstack-connect']).toBe('./dist/cli.js');
8083
});
84+
85+
/**
86+
* A report the server refuses must not fail the build `scan` is hooked into, and must fail a direct run.
87+
*
88+
* Driven through the real bin against a local server that refuses everything, because the decision sits
89+
* between the network failure and the process exit code, and only the process can show both.
90+
*/
91+
describe('a report the server refuses', () => {
92+
async function refusingServer(): Promise<{ endpoint: string; close: () => Promise<void> }> {
93+
const server = createServer((_req, res) => {
94+
res.writeHead(401, { 'Content-Type': 'application/json' });
95+
res.end('{"error":"unauthorized"}');
96+
});
97+
await new Promise<void>((resolve) => server.listen(0, '127.0.0.1', resolve));
98+
const { port } = server.address() as AddressInfo;
99+
100+
return {
101+
endpoint: `http://127.0.0.1:${port}/monitor/pulse/manifest`,
102+
close: () => new Promise<void>((resolve) => server.close(() => resolve())),
103+
};
104+
}
105+
106+
// An existing site with no credential anywhere: the state a deploy is in when the credential file
107+
// stayed behind on the developer's machine.
108+
function projectWithSite(): string {
109+
const dir = mkdtempSync(path.join(tmpdir(), 'ps-bin-hook-'));
110+
writeFileSync(
111+
path.join(dir, 'package.json'),
112+
JSON.stringify({ name: 'example-app', version: '1.0.0', dependencies: { axios: '^1.6.0', lodash: '^4.17.21' } }),
113+
);
114+
copyFileSync(path.join(root, 'tests', 'fixtures', 'package-lock-v3.json'), path.join(dir, 'package-lock.json'));
115+
writeFileSync(path.join(dir, '.patchstackrc.json'), JSON.stringify({ siteUuid: '11111111-1111-4111-8111-111111111111' }));
116+
117+
return dir;
118+
}
119+
120+
// The environment is built from scratch rather than inherited: the parent may itself be running under
121+
// a package manager, and the lifecycle name it exported is the very thing under test. Asynchronous
122+
// because the refusing server lives on this thread's event loop, and a blocking spawn would starve it.
123+
async function runScan(
124+
cwd: string,
125+
endpoint: string,
126+
lifecycleEvent?: string,
127+
): Promise<{ status: number; stderr: string }> {
128+
const env: NodeJS.ProcessEnv = {
129+
PATH: process.env.PATH,
130+
HOME: process.env.HOME,
131+
PATCHSTACK_ENDPOINT: endpoint,
132+
PATCHSTACK_TIMEOUT_MS: '5000',
133+
};
134+
if (lifecycleEvent !== undefined) env.npm_lifecycle_event = lifecycleEvent;
135+
136+
try {
137+
const { stderr } = await promisify(execFile)('node', [bin, 'scan'], { cwd, env, encoding: 'utf8' });
138+
139+
return { status: 0, stderr };
140+
} catch (err) {
141+
const failed = err as { code?: unknown; stderr?: unknown };
142+
143+
return {
144+
status: typeof failed.code === 'number' ? failed.code : -1,
145+
stderr: typeof failed.stderr === 'string' ? failed.stderr : '',
146+
};
147+
}
148+
}
149+
150+
it('exits 0 from a build hook and says what was not reported', async () => {
151+
const server = await refusingServer();
152+
const dir = projectWithSite();
153+
try {
154+
const result = await runScan(dir, server.endpoint, 'build');
155+
156+
expect(result.status).toBe(0);
157+
expect(result.stderr).toContain('manifest not reported');
158+
expect(result.stderr).toContain('PATCHSTACK_API_KEY');
159+
} finally {
160+
await server.close();
161+
rmSync(dir, { recursive: true, force: true });
162+
}
163+
});
164+
165+
it('exits 1 for the same refusal when run directly', async () => {
166+
const server = await refusingServer();
167+
const dir = projectWithSite();
168+
try {
169+
const result = await runScan(dir, server.endpoint);
170+
171+
expect(result.status).toBe(1);
172+
expect(result.stderr).toContain('Error (UNAUTHORIZED)');
173+
expect(result.stderr).not.toContain('continuing the build');
174+
} finally {
175+
await server.close();
176+
rmSync(dir, { recursive: true, force: true });
177+
}
178+
});
179+
});
81180
});

‎tests/build-hook.test.ts‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
import { describe, expect, it } from 'vitest';
2+
import { isInstallOrBuildHook, undeliveredReportLines } from '../src/build-hook.js';
3+
import { PatchstackError, type Config } from '../src/types.js';
4+
5+
/**
6+
* A hooked `scan` must not fail the build it is attached to, and must say why the report did not land.
7+
*
8+
* The hook is recognised by the lifecycle name the package manager exports, so these pin which names
9+
* count: the ones `setup` wires and their neighbours, and none of the names a person or an assistant
10+
* produces by running the command directly. The messages are pinned because the error's own remedy —
11+
* run `login` — is the wrong one in a build environment, and that text is what the build log shows.
12+
*/
13+
describe('isInstallOrBuildHook', () => {
14+
it('recognises the install and build lifecycle names', () => {
15+
for (const event of ['preinstall', 'install', 'postinstall', 'prepare', 'prebuild', 'build', 'postbuild']) {
16+
expect(isInstallOrBuildHook({ npm_lifecycle_event: event })).toBe(true);
17+
}
18+
});
19+
20+
it('does not recognise a direct invocation', () => {
21+
// `npx` names its event `npx`, `bun <bin>` names it after the bin, a project script after itself, and a
22+
// bare `node` sets nothing.
23+
for (const event of ['npx', 'patchstack-connect', 'scan', 'dev', 'start', 'test', '']) {
24+
expect(isInstallOrBuildHook({ npm_lifecycle_event: event })).toBe(false);
25+
}
26+
expect(isInstallOrBuildHook({})).toBe(false);
27+
});
28+
});
29+
30+
describe('undeliveredReportLines', () => {
31+
const config = (over: Partial<Config> = {}): Config => ({
32+
siteUuid: '11111111-1111-4111-8111-111111111111',
33+
apiKey: null,
34+
pulseAuth: null,
35+
endpoint: 'https://api.test/monitor/pulse/manifest',
36+
timeoutMs: 5_000,
37+
environment: 'production',
38+
widget: true,
39+
...over,
40+
});
41+
42+
it('points a build environment with no credential at the env var, not at login', () => {
43+
const said = undeliveredReportLines(new PatchstackError('none configured', 'UNAUTHORIZED'), config(), '/srv/app').join('\n');
44+
45+
expect(said).toContain('none configured');
46+
expect(said).toContain('PATCHSTACK_API_KEY');
47+
expect(said).toContain('continuing the build');
48+
});
49+
50+
it('names every source it checked, and where, when no credential was found', () => {
51+
// The question a build log has to settle is whether the file was read at all. Naming both files and
52+
// the directory is what turns "none is configured" into an answer.
53+
const said = undeliveredReportLines(new PatchstackError('none configured', 'UNAUTHORIZED'), config(), '/srv/app').join('\n');
54+
55+
expect(said).toContain('.patchstackrc.local.json');
56+
expect(said).toContain('.patchstackrc.json');
57+
expect(said).toContain('/srv/app');
58+
expect(said).toContain('git-ignored');
59+
});
60+
61+
it('says a held credential was rejected rather than missing', () => {
62+
const held = config({ apiKey: 'a-secret', pulseAuth: 'a-secret' });
63+
const said = undeliveredReportLines(new PatchstackError('rejected', 'UNAUTHORIZED'), held, '/srv/app').join('\n');
64+
65+
expect(said).toContain('rejected');
66+
expect(said).toContain('PATCHSTACK_API_KEY');
67+
expect(said).not.toContain('never has');
68+
expect(said).not.toContain('/srv/app');
69+
});
70+
71+
it('adds no credential advice to a failure that is not about credentials', () => {
72+
// The control: a network failure with credential advice attached would send someone to set a variable
73+
// that changes nothing.
74+
for (const code of ['NETWORK_ERROR', 'NETWORK_TIMEOUT', 'SERVER_ERROR', 'SITE_NOT_FOUND', 'VALIDATION_ERROR'] as const) {
75+
const lines = undeliveredReportLines(new PatchstackError(`failed: ${code}`, code), config(), '/srv/app');
76+
77+
expect(lines).toHaveLength(2);
78+
expect(lines[0]).toContain(`failed: ${code}`);
79+
expect(lines.join('\n')).not.toContain('PATCHSTACK_API_KEY');
80+
expect(lines[1]).toContain('continuing the build');
81+
}
82+
});
83+
84+
it('never prints the credential itself', () => {
85+
const held = config({ apiKey: 'the-secret-value-0001', pulseAuth: 'the-secret-value-0001' });
86+
const said = undeliveredReportLines(new PatchstackError('rejected', 'UNAUTHORIZED'), held, '/srv/app').join('\n');
87+
88+
expect(said).not.toContain('the-secret-value-0001');
89+
});
90+
});

0 commit comments

Comments
 (0)