Summary
install.command has three separable problems. The middle one is a security control that silently does not apply.
1. --ignore-scripts is bypassed, so allowInstallScripts: false is not binding
The default install path (components/Application.ts:424) and the devEngines.packageManager path (:377) both add --ignore-scripts unless install.allowInstallScripts is true. The custom-command path (:309-337) runs the user's string verbatim and never adds it.
So when install.command is set, package install scripts run regardless of allowInstallScripts: false. A component config (or API request) that explicitly disables install scripts does not get what it asked for. This is the part I'd treat as a defect independent of everything else here.
Same divergence exists in the deprecated install_node_modules operation (utility/npmUtilities.ts:45), whose args are ['install', '--force', '--omit=dev', '--json'] — no --ignore-scripts either.
2. Naive argument splitting
components/Application.ts:310:
const [command, ...args] = application.install.command.split(' ');
Splitting on a single space breaks any quoted argument or path containing spaces. Straightforward bug.
3. Policy inconsistency — asking for a ruling, not necessarily a change
install_command is an operations API parameter on both deploy_component and add_component (components/operations.js), executed through a shell, with no allowlist.
Meanwhile the component-runtime process globals are allowlisted against applications_allowedSpawnCommands (security/jsLoader.ts:987) — and unevenly, since fork is registered with alwaysAllow:
exec: createSpawn(child_process.exec),
execFile: createSpawn(child_process.execFile),
fork: createSpawn(child_process.fork, true), // this is launching node, so deemed safe
spawn: createSpawn(child_process.spawn),
So within one process there are three different policies for starting a subprocess: allowlisted (spawn/exec/execFile), unconditionally allowed (fork), and unbounded shell via config or API (install.command). That may all be intentional under a threat model where application developers are administrators — but right now it reads as four independent historical decisions rather than one policy. Worth an explicit decision recorded somewhere, even if the answer is "this is fine."
Constraint on any fix
shell: true in nonInteractiveSpawn (:719) is load-bearing. applications_packageManagerPrefix works by string-prepending the prefix to the command (prefix + ' ' + 'npm') and relying on the shell to split it; jsLoader.ts:987 uses the matching command.split(' ')[0] convention. Removing the shell breaks the prefix feature.
That also means adding an argv-array form alongside the string creates two paths with different semantics — an argv form would not go through a shell, so the prefix would silently not apply to it. If argv is added, it should either apply the prefix as argv[0] explicitly or be validated as mutually exclusive with the prefix. Shipping both undecided is worse than either alone.
Migration
install.command is an operations API parameter, so removing it is a breaking API change and presumably off the table for 5.x. The realistic path is to keep it, fix (1) and (2), and give the lifecycle enough first-class knobs that it stops being the default answer for routine adjustments.
🤖 Filed by Claude on behalf of @heskew
Summary
install.commandhas three separable problems. The middle one is a security control that silently does not apply.1.
--ignore-scriptsis bypassed, soallowInstallScripts: falseis not bindingThe default install path (
components/Application.ts:424) and thedevEngines.packageManagerpath (:377) both add--ignore-scriptsunlessinstall.allowInstallScriptsis true. The custom-command path (:309-337) runs the user's string verbatim and never adds it.So when
install.commandis set, package install scripts run regardless ofallowInstallScripts: false. A component config (or API request) that explicitly disables install scripts does not get what it asked for. This is the part I'd treat as a defect independent of everything else here.Same divergence exists in the deprecated
install_node_modulesoperation (utility/npmUtilities.ts:45), whose args are['install', '--force', '--omit=dev', '--json']— no--ignore-scriptseither.2. Naive argument splitting
components/Application.ts:310:Splitting on a single space breaks any quoted argument or path containing spaces. Straightforward bug.
3. Policy inconsistency — asking for a ruling, not necessarily a change
install_commandis an operations API parameter on bothdeploy_componentandadd_component(components/operations.js), executed through a shell, with no allowlist.Meanwhile the component-runtime process globals are allowlisted against
applications_allowedSpawnCommands(security/jsLoader.ts:987) — and unevenly, sinceforkis registered withalwaysAllow:So within one process there are three different policies for starting a subprocess: allowlisted (
spawn/exec/execFile), unconditionally allowed (fork), and unbounded shell via config or API (install.command). That may all be intentional under a threat model where application developers are administrators — but right now it reads as four independent historical decisions rather than one policy. Worth an explicit decision recorded somewhere, even if the answer is "this is fine."Constraint on any fix
shell: trueinnonInteractiveSpawn(:719) is load-bearing.applications_packageManagerPrefixworks by string-prepending the prefix to the command (prefix + ' ' + 'npm') and relying on the shell to split it;jsLoader.ts:987uses the matchingcommand.split(' ')[0]convention. Removing the shell breaks the prefix feature.That also means adding an argv-array form alongside the string creates two paths with different semantics — an argv form would not go through a shell, so the prefix would silently not apply to it. If argv is added, it should either apply the prefix as
argv[0]explicitly or be validated as mutually exclusive with the prefix. Shipping both undecided is worse than either alone.Migration
install.commandis an operations API parameter, so removing it is a breaking API change and presumably off the table for 5.x. The realistic path is to keep it, fix (1) and (2), and give the lifecycle enough first-class knobs that it stops being the default answer for routine adjustments.🤖 Filed by Claude on behalf of @heskew