diff --git a/DESIGN.md b/DESIGN.md index c889c2e236..cd21cb0f0f 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -296,6 +296,20 @@ A package-manager timeout must not release this lock while npm descendants are s Boot's `harper-application-lock.json` records an application configuration only after preparation fulfills. Recording at queue time would make a failed install look complete and suppress its retry on the next boot. +Custom `install_command` spawns receive `npm_config_ignore_scripts=true` unless +`install_allow_scripts` (or `install.allowInstallScripts` in root config) is true. The child-only +environment setting reaches npm nested in shell commands without changing the command's arguments; +other package managers must honor npm's configuration namespace for it to apply. An explicit opt-in +does not clear an inherited host restriction. Omitted policy emits a warning naming the opt-ins, +because existing custom commands can rely on lifecycle scripts, including `npm run` pre/post hooks. +This is best-effort enforcement for arbitrary commands: an explicit override in the command or a +package manager that ignores npm configuration can bypass it. + +The deprecated `install_node_modules` operation defaults to scripts enabled for compatibility. +`installModules()` consumes Joi's converted `allowInstallScripts` value (also accepted as +`install_allow_scripts`), so a string `'false'` suppresses lifecycle scripts. Both spellings together +are rejected. Only camelCase `dryRun` controls dry-run invocation. + ## Peer-side deploy_component payload read: retryable blob stalls and `Readable.from()` cancellation `readPayloadBlobWithRetry` (`components/deploymentRecorder.ts`) wraps the peer's read of a replicated `hdb_deployment` row's `payload_blob` so a transient 503 `BlobReadError` (`BLOB_UNAVAILABLE_STATUS`, `resources/blob.ts`) — content bytes not arriving within `blobReadTimeout`, e.g. a parked blob send on the origin — retries instead of failing the whole deploy. Two non-obvious constraints shaped the design: diff --git a/components/Application.ts b/components/Application.ts index a6f269a0a2..1285daa35a 100644 --- a/components/Application.ts +++ b/components/Application.ts @@ -888,8 +888,15 @@ export async function installApplication(application: Application) { // If node_modules doesn't exist, we need to install dependencies } + const allowInstallScripts = !!application.install?.allowInstallScripts; + // If custom install command is specified, run it if (application.install?.command) { + if (application.install.allowInstallScripts === undefined) { + application.logger.warn( + `Application ${application.name} uses install_command without install_allow_scripts; package lifecycle scripts are disabled by default for npm and tools that honor npm_config_ignore_scripts, including npm run pre/post hooks. Set install_allow_scripts (or install.allowInstallScripts in root config) to true to opt in` + ); + } const [command, ...args] = application.install.command.split(' '); const customOnLine = application.onInstallLine ? (stream: 'stdout' | 'stderr', line: string) => application.onInstallLine!(command, stream, line) @@ -901,7 +908,9 @@ export async function installApplication(application: Application) { application.dirPath, application.install?.timeout, customOnLine, - application.npmUserconfigPath + application.npmUserconfigPath, + undefined, + !allowInstallScripts ); // if it succeeds, return if (code === 0) { @@ -1695,7 +1704,8 @@ export async function nonInteractiveSpawn( timeoutMs: number = DEFAULT_COMMAND_TIMEOUT_MS, onLine?: (stream: 'stdout' | 'stderr', line: string) => void, npmUserconfigPath?: string, - gitCredentialEnv?: Record + gitCredentialEnv?: Record, + ignoreNpmScripts = false ): Promise<{ stdout: string; stderr: string; code: number }> { const gitSSH = await materializeGitSSH(); try { @@ -1708,7 +1718,8 @@ export async function nonInteractiveSpawn( onLine, npmUserconfigPath, gitSSH?.command, - gitCredentialEnv + gitCredentialEnv, + ignoreNpmScripts ); } finally { await gitSSH?.cleanup(); @@ -1724,7 +1735,8 @@ function spawnWithEnv( onLine: ((stream: 'stdout' | 'stderr', line: string) => void) | undefined, npmUserconfigPath: string | undefined, gitSSHCommand: string | undefined, - gitCredentialEnv: Record | undefined + gitCredentialEnv: Record | undefined, + ignoreNpmScripts: boolean ): Promise<{ stdout: string; stderr: string; code: number }> { return new Promise((resolve, reject) => { logger @@ -1760,6 +1772,12 @@ function spawnWithEnv( } env.npm_config_userconfig = npmUserconfigPath; } + if (ignoreNpmScripts) { + for (const key of Object.keys(env)) { + if (key.toLowerCase() === 'npm_config_ignore_scripts') delete env[key]; + } + env.npm_config_ignore_scripts = 'true'; + } if (process.platform === 'win32' && command === 'npm') { command = 'npm.cmd'; diff --git a/unitTests/components/applicationInstall.test.js b/unitTests/components/applicationInstall.test.js new file mode 100644 index 0000000000..802e63d707 --- /dev/null +++ b/unitTests/components/applicationInstall.test.js @@ -0,0 +1,169 @@ +'use strict'; + +const assert = require('node:assert'); +const { access, mkdir, mkdtemp, readFile, rm, writeFile } = require('node:fs/promises'); +const { tmpdir } = require('node:os'); +const { join } = require('node:path'); + +const testUtils = require('../testUtils.js'); +testUtils.preTestPrep(); + +const { Application, installApplication } = require('#src/components/Application'); + +async function createApplication(root, name, packageJSON, install) { + const directory = join(root, name); + await mkdir(directory, { recursive: true }); + await writeFile(join(directory, 'package.json'), JSON.stringify({ name, version: '1.0.0', ...packageJSON })); + const application = new Application({ name, install }); + application.dirPath = directory; + return application; +} + +async function createLifecycleDependency(root, name, markerPath) { + const dependencyName = `${name}-dependency`; + const directory = join(root, dependencyName); + await mkdir(directory); + await writeFile( + join(directory, 'install.cjs'), + `require('node:fs').writeFileSync(${JSON.stringify(markerPath)}, 'ran');\n` + ); + await writeFile( + join(directory, 'package.json'), + JSON.stringify({ name: dependencyName, version: '1.0.0', scripts: { install: 'node install.cjs' } }) + ); + return { dependencyName, directory }; +} + +async function configureInstallCapture(application, root, name) { + const capturePath = join(root, `${name}-args.json`); + const captureScript = join(root, `${name}-capture.cjs`); + await writeFile( + captureScript, + `require('node:fs').writeFileSync(${JSON.stringify(capturePath)}, JSON.stringify(process.argv.slice(2)));\n` + ); + application.packageManagerPrefix = `"${process.execPath}" "${captureScript}"`; + return capturePath; +} + +describe('automatic application installation', () => { + beforeEach(async function () { + this.root = await mkdtemp(join(tmpdir(), 'application-install-')); + }); + + afterEach(async function () { + await rm(this.root, { recursive: true, force: true }); + }); + + it('suppresses lifecycle scripts for the default and declared npm paths', async function () { + const defaultApplication = await createApplication(this.root, 'default-npm', { + dependencies: { runtime: '1.0.0' }, + }); + const defaultCapture = await configureInstallCapture(defaultApplication, this.root, 'default-npm'); + await installApplication(defaultApplication); + assert.deepEqual(JSON.parse(await readFile(defaultCapture, 'utf8')), [ + 'npm', + 'install', + '--force', + '--ignore-scripts', + ]); + + const declaredApplication = await createApplication(this.root, 'declared-npm', { + dependencies: { runtime: '1.0.0' }, + devEngines: { packageManager: { name: 'npm' } }, + }); + const declaredCapture = await configureInstallCapture(declaredApplication, this.root, 'declared-npm'); + await installApplication(declaredApplication); + assert.deepEqual(JSON.parse(await readFile(declaredCapture, 'utf8')), ['npm', 'install', '--ignore-scripts']); + }); + + it('suppresses lifecycle scripts for a declared non-npm package manager', async function () { + const application = await createApplication(this.root, 'declared-pnpm', { + devEngines: { packageManager: { name: 'pnpm' } }, + }); + const capturePath = await configureInstallCapture(application, this.root, 'declared-pnpm'); + + await installApplication(application); + + assert.deepEqual(JSON.parse(await readFile(capturePath, 'utf8')), ['pnpm', 'install', '--ignore-scripts']); + }); + + it('omits script suppression when the default npm path opts in', async function () { + const application = await createApplication(this.root, 'allowed-lifecycle', {}, { allowInstallScripts: true }); + const capturePath = await configureInstallCapture(application, this.root, 'allowed-lifecycle'); + + await installApplication(application); + + assert.deepEqual(JSON.parse(await readFile(capturePath, 'utf8')), ['npm', 'install', '--force']); + assert.equal(application.installationIsOpaque, true); + }); + + it('preserves the custom install command arguments', async function () { + const application = await createApplication( + this.root, + 'custom-command', + {}, + { command: 'node custom-install.cjs' } + ); + const automaticCapture = await configureInstallCapture(application, this.root, 'custom-command'); + const customMarker = join(application.dirPath, 'custom-command-ran'); + await writeFile( + join(application.dirPath, 'custom-install.cjs'), + `require('node:fs').writeFileSync(${JSON.stringify(customMarker)}, JSON.stringify(process.argv.slice(2)));\n` + ); + + await installApplication(application); + + assert.deepEqual(JSON.parse(await readFile(customMarker, 'utf8')), []); + await assert.rejects(access(automaticCapture), (error) => error.code === 'ENOENT'); + assert.equal(application.installationIsOpaque, true); + }); + + it('applies the lifecycle-script policy to custom install commands', async function () { + this.timeout(60_000); + const inheritedPolicies = Object.entries(process.env).filter( + ([key]) => key.toLowerCase() === 'npm_config_ignore_scripts' + ); + for (const [key] of inheritedPolicies) delete process.env[key]; + process.env.npm_config_ignore_scripts = 'false'; + try { + for (const allowInstallScripts of [undefined, false, true]) { + const name = + allowInstallScripts === undefined + ? 'custom-scripts-default' + : allowInstallScripts + ? 'custom-scripts-allowed' + : 'custom-scripts-blocked'; + const markerPath = join(this.root, `${name}-marker`); + const dependency = await createLifecycleDependency(this.root, name, markerPath); + const install = { command: 'npm install --no-audit --no-fund && node -e "process.exit(0)"' }; + if (allowInstallScripts !== undefined) install.allowInstallScripts = allowInstallScripts; + const application = await createApplication( + this.root, + name, + { dependencies: { [dependency.dependencyName]: `file:${dependency.directory}` } }, + install + ); + const warnings = []; + application.logger.warn = (message) => warnings.push(message); + + await installApplication(application); + + await access(join(application.dirPath, 'node_modules', dependency.dependencyName, 'package.json')); + assert.equal( + await access(markerPath).then( + () => true, + () => false + ), + allowInstallScripts === true + ); + assert.equal(warnings.length, allowInstallScripts === undefined ? 1 : 0); + if (warnings.length) assert.match(warnings[0], /install\.allowInstallScripts/); + } + } finally { + for (const key of Object.keys(process.env)) { + if (key.toLowerCase() === 'npm_config_ignore_scripts') delete process.env[key]; + } + Object.assign(process.env, Object.fromEntries(inheritedPolicies)); + } + }); +}); diff --git a/unitTests/utility/npmUtilities.test.js b/unitTests/utility/npmUtilities.test.js new file mode 100644 index 0000000000..f69ebdaf14 --- /dev/null +++ b/unitTests/utility/npmUtilities.test.js @@ -0,0 +1,160 @@ +'use strict'; + +const assert = require('node:assert'); +const fs = require('node:fs'); +const os = require('node:os'); +const path = require('node:path'); + +const testUtils = require('../testUtils.js'); +testUtils.preTestPrep(); + +const env = require('#src/utility/environment/environmentManager'); +const { CONFIG_PARAMS } = require('#src/utility/hdbTerms'); +const { installModules } = require('#src/utility/npmUtilities'); + +describe('install_node_modules', function () { + this.timeout(60_000); // .mocharc.json sets `timeout: 0`, and these cases spawn real npm + + let componentsRoot; + let lifecycleMarker; + let inheritedAuditPolicy; + let inheritedComponentsRoot; + + before(() => { + inheritedAuditPolicy = process.env.npm_config_audit; + process.env.npm_config_audit = 'false'; + componentsRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'harper-install-modules-')); + lifecycleMarker = path.join(componentsRoot, 'lifecycle-marker'); + inheritedComponentsRoot = env.get(CONFIG_PARAMS.COMPONENTSROOT); + env.setProperty(CONFIG_PARAMS.COMPONENTSROOT, componentsRoot); + + fs.mkdirSync(path.join(componentsRoot, 'dependency')); + fs.writeFileSync( + path.join(componentsRoot, 'dependency', 'install.cjs'), + `require('node:fs').writeFileSync(${JSON.stringify(lifecycleMarker)}, 'ran');\n` + ); + fs.writeFileSync( + path.join(componentsRoot, 'dependency', 'package.json'), + JSON.stringify({ + name: 'local-dependency', + version: '1.0.0', + scripts: { install: 'node install.cjs' }, + }) + ); + fs.mkdirSync(path.join(componentsRoot, 'application')); + fs.writeFileSync( + path.join(componentsRoot, 'application', 'package.json'), + JSON.stringify({ + name: 'application', + version: '1.0.0', + dependencies: { 'local-dependency': 'file:../dependency' }, + }) + ); + }); + + after(() => { + if (inheritedAuditPolicy === undefined) delete process.env.npm_config_audit; + else process.env.npm_config_audit = inheritedAuditPolicy; + env.setProperty(CONFIG_PARAMS.COMPONENTSROOT, inheritedComponentsRoot); + fs.rmSync(componentsRoot, { recursive: true, force: true }); + }); + + afterEach(() => { + fs.rmSync(path.join(componentsRoot, 'application', 'node_modules'), { recursive: true, force: true }); + fs.rmSync(lifecycleMarker, { force: true }); + }); + + // npm still reports the dependency it would add under --dry-run, so asserting on that output + // distinguishes a real dry run from npm never running at all + function assertDryRun(response) { + assert.match(JSON.stringify(response.application.npm_output), /add local-dependency/); + assert.equal(installedDependencyExists(), false); + } + + function assertInstalled(response) { + assert.equal(response.application.npm_output.added, 1, JSON.stringify(response.application)); + assert.equal(installedDependencyExists(), true); + } + + function installedDependencyExists() { + return fs.existsSync(path.join(componentsRoot, 'application', 'node_modules', 'local-dependency')); + } + + async function withNpmLifecycleScriptsEnabled(callback) { + const inheritedPolicies = Object.entries(process.env).filter( + ([key]) => key.toLowerCase() === 'npm_config_ignore_scripts' + ); + for (const [key] of inheritedPolicies) delete process.env[key]; + process.env.npm_config_ignore_scripts = 'false'; + try { + return await callback(); + } finally { + for (const key of Object.keys(process.env)) { + if (key.toLowerCase() === 'npm_config_ignore_scripts') delete process.env[key]; + } + Object.assign(process.env, Object.fromEntries(inheritedPolicies)); + } + } + + it('honors the legacy camelCase dryRun field', async () => { + const response = await installModules({ projects: ['application'], dryRun: true }); + + assertDryRun(response); + }); + + it('allows lifecycle scripts when the policy is omitted', async () => { + const response = await withNpmLifecycleScriptsEnabled(() => installModules({ projects: ['application'] })); + + assertInstalled(response); + assert.equal(fs.existsSync(lifecycleMarker), true); + }); + + it('applies the lifecycle-script policy while preserving the allowed positive control', async () => { + await withNpmLifecycleScriptsEnabled(async () => { + const blockedResponse = await installModules({ projects: ['application'], allowInstallScripts: false }); + + assertInstalled(blockedResponse); + assert.equal(fs.existsSync(lifecycleMarker), false); + fs.rmSync(path.join(componentsRoot, 'application', 'node_modules'), { recursive: true, force: true }); + + const allowedResponse = await installModules({ projects: ['application'], allowInstallScripts: true }); + + assertInstalled(allowedResponse); + assert.equal(fs.existsSync(lifecycleMarker), true); + }); + }); + + it('honors install_allow_scripts and rejects both policy spellings together', async () => { + const response = await withNpmLifecycleScriptsEnabled(() => + installModules({ projects: ['application'], install_allow_scripts: false }) + ); + + assertInstalled(response); + assert.equal(fs.existsSync(lifecycleMarker), false); + await assert.rejects( + installModules({ projects: ['application'], install_allow_scripts: false, allowInstallScripts: true }), + { statusCode: 400, message: /install_allow_scripts/ } + ); + }); + + it('converts a string false policy while accepting operation metadata', async () => { + const response = await withNpmLifecycleScriptsEnabled(() => + installModules({ operation: 'install_node_modules', projects: ['application'], install_allow_scripts: 'false' }) + ); + + assertInstalled(response); + assert.equal(fs.existsSync(lifecycleMarker), false); + }); + + it('rejects an invalid lifecycle-script policy before installation', async () => { + await assert.rejects(installModules({ projects: ['application'], install_allow_scripts: 'invalid' }), { + statusCode: 400, + message: /allowInstallScripts/, + }); + assert.equal(installedDependencyExists(), false); + }); + + it('rejects a request without projects', async () => { + await assert.rejects(installModules({ dry_run: true }), { statusCode: 400, message: /'projects'/ }); + }); +}); diff --git a/utility/npmUtilities.ts b/utility/npmUtilities.ts index 7e29427aaa..93ff39f5b4 100644 --- a/utility/npmUtilities.ts +++ b/utility/npmUtilities.ts @@ -7,7 +7,6 @@ import { handleHDBError, hdbErrors } from './errors/hdbError.ts'; const { HTTP_STATUS_CODES } = hdbErrors; -import * as validator from '../validation/validationWrapper.ts'; import harperLogger from './logging/harper_logger.ts'; import { CONFIG_PARAMS } from './hdbTerms.ts'; @@ -26,18 +25,19 @@ export async function installModules(req: any) { 'install_node_modules is deprecated. Dependencies are automatically installed on' + ' deploy, and install_node_modules can lead to inconsistent behavior'; harperLogger.warn(deprecationWarning, req.projects); - const validation = modulesValidator(req); + const { error: validation, value: validatedRequest } = modulesValidator(req); if (validation) { throw handleHDBError(validation, validation.message, HTTP_STATUS_CODES.BAD_REQUEST); } - let { projects, dryRun } = req; + const { projects, dryRun, allowInstallScripts } = validatedRequest; const componentsRootDirPath = getConfigPath(CONFIG_PARAMS.COMPONENTSROOT); const responseObject: any = {}; const args = ['install', '--force', '--omit=dev', '--json']; + if (!allowInstallScripts) args.push('--ignore-scripts'); if (dryRun) args.push('--dry-run'); for (const project of projects) { @@ -99,7 +99,8 @@ function modulesValidator(req: any) { const funcSchema = Joi.object({ projects: Joi.array().min(1).items(Joi.string()).required(), dry_run: Joi.boolean().default(false), - }); + allowInstallScripts: Joi.boolean().default(true), + }).rename('install_allow_scripts', 'allowInstallScripts', { ignoreUndefined: true }); - return validator.validateBySchema(req, funcSchema); + return funcSchema.validate(req, { allowUnknown: true, abortEarly: false, errors: { wrap: { label: "'" } } }); }