mirror of
https://github.com/n8n-io/n8n.git
synced 2026-10-17 05:29:44 +02:00
ci: Report single-instance dependency findings without failing advisory runs (#36437)
This commit is contained in:
Bernhard Wittmann
GitHub
parent
a8754b4040
commit
0dffaa9723
20
.github/workflows/test-single-instance-npm.yml
vendored
20
.github/workflows/test-single-instance-npm.yml
vendored
|
|
@ -34,13 +34,19 @@ on:
|
|||
jobs:
|
||||
verify:
|
||||
runs-on: ubuntu-latest
|
||||
continue-on-error: ${{ !inputs.blocking }}
|
||||
timeout-minutes: ${{ inputs.timeout-minutes }}
|
||||
permissions:
|
||||
contents: read
|
||||
env:
|
||||
# Advisory runs downgrade findings to `::warning::` annotations and exit 0, so the known
|
||||
# third-party splits don't leave a permanently red job that everyone learns to ignore.
|
||||
# This deliberately replaces `continue-on-error`, which also swallowed the failures we do
|
||||
# want — a usage error, a rejected input, or a crash that verified nothing.
|
||||
REPORT_ONLY: ${{ !inputs.blocking && '--report-only' || '' }}
|
||||
steps:
|
||||
# Every rejected combination below otherwise degrades into a run that verifies nothing and
|
||||
# still exits 0 — silently, since this job is advisory by default.
|
||||
# still exits 0. These fail the job even in advisory mode — `--report-only` downgrades
|
||||
# findings, not misconfiguration.
|
||||
- name: Validate inputs
|
||||
env:
|
||||
SCOPE: ${{ inputs.scope }}
|
||||
|
|
@ -88,8 +94,14 @@ jobs:
|
|||
if: inputs.scope == 'changed'
|
||||
env:
|
||||
BASE_REF: ${{ inputs.base-ref }}
|
||||
run: pnpm --dir packages/testing/code-health exec tsx src/cli.ts verify-npm-install --changed="$BASE_REF"
|
||||
run: |
|
||||
# REPORT_ONLY is a single flag or empty. Quoting it would pass an empty argument, which
|
||||
# the CLI reads as an (unknown) package name and rejects.
|
||||
# shellcheck disable=SC2086
|
||||
pnpm --dir packages/testing/code-health exec tsx src/cli.ts verify-npm-install --changed="$BASE_REF" $REPORT_ONLY
|
||||
|
||||
- name: Verify npm-install closure (all packages)
|
||||
if: inputs.scope == 'all'
|
||||
run: pnpm --dir packages/testing/code-health exec tsx src/cli.ts verify-npm-install --all
|
||||
run: |
|
||||
# shellcheck disable=SC2086 # see the scoped step above
|
||||
pnpm --dir packages/testing/code-health exec tsx src/cli.ts verify-npm-install --all $REPORT_ONLY
|
||||
|
|
|
|||
|
|
@ -1,6 +1,11 @@
|
|||
import { describe, expect, it } from 'vitest';
|
||||
|
||||
import { closureOf, filesTriggerFullRun, matchChangedFiles } from './verify-npm-install.js';
|
||||
import {
|
||||
closureOf,
|
||||
filesTriggerFullRun,
|
||||
matchChangedFiles,
|
||||
resolveTargets,
|
||||
} from './verify-npm-install.js';
|
||||
|
||||
/** Build a WorkspacePkg fixture from `[depName, section]` pairs (only names + sections matter). */
|
||||
function ws(name: string, deps: Array<[string, string]>) {
|
||||
|
|
@ -94,3 +99,25 @@ describe('filesTriggerFullRun', () => {
|
|||
expect(filesTriggerFullRun(['packages/core/src/index.ts'])).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('resolveTargets', () => {
|
||||
it('does not mistake --report-only for a package name', () => {
|
||||
expect(resolveTargets(['a', '--report-only'], byName, '/x')).toEqual(['a']);
|
||||
});
|
||||
|
||||
// Flag-only args must reach the usage error rather than silently verifying nothing.
|
||||
it('resolves no targets when only flags are passed', () => {
|
||||
expect(resolveTargets(['--report-only'], byName, '/x')).toEqual([]);
|
||||
});
|
||||
|
||||
it('still expands --all alongside the flag', () => {
|
||||
expect(resolveTargets(['--all', '--report-only'], byName, '/x')?.sort()).toEqual([
|
||||
'a',
|
||||
'b',
|
||||
'c',
|
||||
'd',
|
||||
'dev-only',
|
||||
'solo',
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -2,6 +2,7 @@ import { execFileSync } from 'node:child_process';
|
|||
import { mkdirSync, mkdtempSync, readdirSync, rmSync, writeFileSync } from 'node:fs';
|
||||
import { tmpdir } from 'node:os';
|
||||
import { dirname, join } from 'node:path';
|
||||
import { setTimeout } from 'node:timers/promises';
|
||||
|
||||
import { analyze, collectCopies } from './collect-copies.js';
|
||||
import type { PackageJsonInfo } from '../utils/package-json-scanner.js';
|
||||
|
|
@ -23,6 +24,35 @@ export function filesTriggerFullRun(files: string[]): boolean {
|
|||
return files.some((f) => ROOT_TRIGGERS.some((t) => f === t || f.startsWith(t)));
|
||||
}
|
||||
|
||||
/**
|
||||
* Surface a finding in the Actions UI. Report-only runs exit 0, and nobody opens the log of a
|
||||
* green check — without an annotation the finding is indistinguishable from silence.
|
||||
*/
|
||||
function annotate(message: string): void {
|
||||
if (process.env.GITHUB_ACTIONS) console.log(`::warning::${message}`);
|
||||
}
|
||||
|
||||
/**
|
||||
* The scratch install is the one network-bound step, and a registry blip must not read as a
|
||||
* dependency finding. Retries only the install; a persistent failure still throws.
|
||||
*/
|
||||
async function installWithRetry(scratch: string, attempts = 3): Promise<void> {
|
||||
for (let attempt = 1; ; attempt++) {
|
||||
try {
|
||||
execFileSync(
|
||||
'npm',
|
||||
['install', '--no-audit', '--no-fund', '--ignore-scripts', '--no-package-lock'],
|
||||
{ cwd: scratch, stdio: ['ignore', 'inherit', 'inherit'] },
|
||||
);
|
||||
return;
|
||||
} catch (error) {
|
||||
if (attempt >= attempts) throw error;
|
||||
console.error(`npm install failed (attempt ${attempt}/${attempts}); retrying...`);
|
||||
await setTimeout(attempt * 5_000);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
interface WorkspacePkg {
|
||||
dir: string;
|
||||
relDir: string;
|
||||
|
|
@ -114,7 +144,8 @@ export function closureOf(targets: string[], byName: Map<string, WorkspacePkg>):
|
|||
return [...seen];
|
||||
}
|
||||
|
||||
function resolveTargets(
|
||||
/** Targets to verify: `null` means "nothing to do", `[]` means the args were unusable. */
|
||||
export function resolveTargets(
|
||||
args: string[],
|
||||
byName: Map<string, WorkspacePkg>,
|
||||
rootDir: string,
|
||||
|
|
@ -141,11 +172,14 @@ function resolveTargets(
|
|||
* `workspace:` like publishing does), installs into a scratch dir with npm, then verifies.
|
||||
*/
|
||||
export async function runVerifyNpmInstall(args: string[], rootDir: string): Promise<number> {
|
||||
const reportOnly = args.includes('--report-only');
|
||||
const byName = await loadWorkspace(rootDir);
|
||||
const targets = resolveTargets(args, byName, rootDir);
|
||||
if (targets === null) return 0;
|
||||
if (targets.length === 0) {
|
||||
console.error('Usage: verify-npm-install (--all | --changed=<ref> | <pkgName>...)');
|
||||
console.error(
|
||||
'Usage: verify-npm-install (--all | --changed=<ref> | <pkgName>...) [--report-only]',
|
||||
);
|
||||
return 2;
|
||||
}
|
||||
const unknown = targets.filter((t) => !byName.has(t));
|
||||
|
|
@ -163,64 +197,70 @@ export async function runVerifyNpmInstall(args: string[], rootDir: string): Prom
|
|||
mkdirSync(tarballs, { recursive: true });
|
||||
mkdirSync(scratch, { recursive: true });
|
||||
|
||||
console.log(`Packing ${toPack.length} workspace package(s) (targets: ${targets.length})...`);
|
||||
const tarballByName: Record<string, string> = {};
|
||||
for (const name of toPack) {
|
||||
const entry = byName.get(name);
|
||||
if (!entry) continue;
|
||||
const before = new Set(readdirSync(tarballs));
|
||||
execFileSync('pnpm', ['pack', '--pack-destination', tarballs], {
|
||||
cwd: entry.dir,
|
||||
stdio: ['ignore', 'ignore', 'inherit'],
|
||||
});
|
||||
const produced = readdirSync(tarballs).find((f) => !before.has(f) && f.endsWith('.tgz'));
|
||||
if (!produced) throw new Error(`pnpm pack produced no tarball for ${name}`);
|
||||
tarballByName[name] = join(tarballs, produced);
|
||||
}
|
||||
// An enforcing-mode finding is the only reason to keep the tree. A report-only run exits 0 so
|
||||
// nothing downstream reads it, and a throw (pack failure, registry outage) leaves nothing worth
|
||||
// inspecting — either way, keeping it just leaks a full node_modules into tmpdir per run.
|
||||
let keepScratch = false;
|
||||
try {
|
||||
console.log(`Packing ${toPack.length} workspace package(s) (targets: ${targets.length})...`);
|
||||
const tarballByName: Record<string, string> = {};
|
||||
for (const name of toPack) {
|
||||
const entry = byName.get(name);
|
||||
if (!entry) continue;
|
||||
const before = new Set(readdirSync(tarballs));
|
||||
execFileSync('pnpm', ['pack', '--pack-destination', tarballs], {
|
||||
cwd: entry.dir,
|
||||
stdio: ['ignore', 'ignore', 'inherit'],
|
||||
});
|
||||
const produced = readdirSync(tarballs).find((f) => !before.has(f) && f.endsWith('.tgz'));
|
||||
if (!produced) throw new Error(`pnpm pack produced no tarball for ${name}`);
|
||||
tarballByName[name] = join(tarballs, produced);
|
||||
}
|
||||
|
||||
// Scratch project: install targets as file: deps, force ALL packed workspace deps to their
|
||||
// local tarballs. Third-party deps resolve from the real npm registry.
|
||||
const fileDep = (n: string): [string, string] => [n, `file:${tarballByName[n]}`];
|
||||
const overrides = Object.fromEntries(toPack.map(fileDep));
|
||||
const deps = Object.fromEntries(targets.map(fileDep));
|
||||
writeFileSync(
|
||||
join(scratch, 'package.json'),
|
||||
JSON.stringify(
|
||||
{ name: 'single-instance-scratch', private: true, dependencies: deps, overrides },
|
||||
null,
|
||||
2,
|
||||
),
|
||||
);
|
||||
|
||||
console.log('Installing into scratch dir with npm...');
|
||||
execFileSync(
|
||||
'npm',
|
||||
['install', '--no-audit', '--no-fund', '--ignore-scripts', '--no-package-lock'],
|
||||
{
|
||||
cwd: scratch,
|
||||
stdio: ['ignore', 'inherit', 'inherit'],
|
||||
},
|
||||
);
|
||||
|
||||
const { duplicates, failures } = analyze(collectCopies(scratch));
|
||||
|
||||
console.log(`\nnpm-install closure — scratch: ${scratch}\n`);
|
||||
for (const d of duplicates) {
|
||||
const tag = d.isCurated ? (d.allowed ? 'ALLOWED DUP' : 'FAIL') : 'dup (report)';
|
||||
console.log(
|
||||
` ${d.name}: ${tag} — ${d.copies.length} copies (${d.copies.map((c) => `v${c.version}`).join(', ')})`,
|
||||
// Scratch project: install targets as file: deps, force ALL packed workspace deps to their
|
||||
// local tarballs. Third-party deps resolve from the real npm registry.
|
||||
const fileDep = (n: string): [string, string] => [n, `file:${tarballByName[n]}`];
|
||||
const overrides = Object.fromEntries(toPack.map(fileDep));
|
||||
const deps = Object.fromEntries(targets.map(fileDep));
|
||||
writeFileSync(
|
||||
join(scratch, 'package.json'),
|
||||
JSON.stringify(
|
||||
{ name: 'single-instance-scratch', private: true, dependencies: deps, overrides },
|
||||
null,
|
||||
2,
|
||||
),
|
||||
);
|
||||
}
|
||||
|
||||
if (failures.length === 0) rmSync(work, { recursive: true, force: true });
|
||||
else console.log(`\nScratch install kept for inspection: ${scratch}`);
|
||||
console.log('Installing into scratch dir with npm...');
|
||||
await installWithRetry(scratch);
|
||||
|
||||
if (failures.length > 0) {
|
||||
console.error(
|
||||
`\nFAIL: ${failures.length} curated library duplicate(s) in the npm-install graph.`,
|
||||
);
|
||||
return 1;
|
||||
const { duplicates, failures } = analyze(collectCopies(scratch));
|
||||
|
||||
console.log(`\nnpm-install closure — scratch: ${scratch}\n`);
|
||||
const curatedTag = reportOnly ? 'CURATED DUP (report)' : 'FAIL';
|
||||
for (const d of duplicates) {
|
||||
const tag = d.isCurated ? (d.allowed ? 'ALLOWED DUP' : curatedTag) : 'dup (report)';
|
||||
console.log(
|
||||
` ${d.name}: ${tag} — ${d.copies.length} copies (${d.copies.map((c) => `v${c.version}`).join(', ')})`,
|
||||
);
|
||||
}
|
||||
|
||||
if (failures.length > 0) {
|
||||
keepScratch = !reportOnly;
|
||||
for (const f of failures) {
|
||||
annotate(
|
||||
`${f.name}: ${f.copies.length} copies in the npm-install graph (${f.copies.map((c) => `v${c.version}`).join(', ')})`,
|
||||
);
|
||||
}
|
||||
console.error(
|
||||
`\n${reportOnly ? 'REPORT' : 'FAIL'}: ${failures.length} curated library duplicate(s) in the npm-install graph.`,
|
||||
);
|
||||
return reportOnly ? 0 : 1;
|
||||
}
|
||||
console.log('\nOK: no curated duplicates in the npm-install graph.');
|
||||
return 0;
|
||||
} finally {
|
||||
if (keepScratch) console.log(`\nScratch install kept for inspection: ${scratch}`);
|
||||
else rmSync(work, { recursive: true, force: true });
|
||||
}
|
||||
console.log('\nOK: no curated duplicates in the npm-install graph.');
|
||||
return 0;
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user