Skip to content

Commit 3d1580d

Browse files
committed
fix(optimize): preserve inherited NODE_OPTIONS in agent installer
runAgentInstall set the child's NODE_OPTIONS to only our own Node flags (harden / no-warnings / disable-sigusr1). A child process' NODE_OPTIONS REPLACES the parent's rather than extending it, so any NODE_OPTIONS the user configured globally was silently dropped when `socket optimize` ran the package-manager install. Merge the inherited process.env.NODE_OPTIONS ahead of our added flags via a new pure `mergeNodeOptions` helper in util/process/cmd.mts, unit-tested in isolation. The value is left unquoted (it is assigned to an env var, not passed through a shell). Same class of NODE_OPTIONS-clobber bug as the v1.x shadow-npm fix for #1160/#1036, but a different code path: this is the `socket optimize` agent installer on main, not the `socket npm` shadow wrapper. On main `socket npm` hands off to Socket Firewall and no longer builds Node flags itself, so this is the remaining place on main that needed it.
1 parent 4fd2488 commit 3d1580d

4 files changed

Lines changed: 101 additions & 2 deletions

File tree

packages/cli/src/commands/optimize/agent-installer.mts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import { WIN32 } from '@socketsecurity/lib-stable/constants/platform'
2222
import { getOwn } from '@socketsecurity/lib-stable/objects/inspect'
2323
import { spawn } from '@socketsecurity/lib-stable/process/spawn/child'
2424

25-
import { cmdFlagsToString } from '../../util/process/cmd.mts'
25+
import { mergeNodeOptions } from '../../util/process/cmd.mts'
2626

2727
import type { EnvDetails } from '../../util/ecosystem/environment.mjs'
2828
import type { SpinnerInstance } from '@socketsecurity/lib-stable/spinner/types'
@@ -91,7 +91,9 @@ export function runAgentInstall(
9191
...process.env,
9292
// Set CI mode for pnpm to ensure consistent behavior.
9393
...(isPnpm ? { CI: '1' } : {}),
94-
NODE_OPTIONS: cmdFlagsToString([
94+
// Merge our flags into any inherited NODE_OPTIONS instead of replacing
95+
// it, so a user's globally-configured NODE_OPTIONS is preserved.
96+
NODE_OPTIONS: mergeNodeOptions(process.env['NODE_OPTIONS'], [
9597
...(skipNodeHardenFlags ? [] : getNodeHardenFlags()),
9698
...getNodeNoWarningsFlags(),
9799
...getNodeDisableSigusr1Flags(),

packages/cli/src/util/process/cmd.mts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,3 +146,25 @@ export function filterFlags(
146146
export function isHelpFlag(cmdArg: string): boolean {
147147
return helpFlags.has(cmdArg)
148148
}
149+
150+
/**
151+
* Merge Node flags into a NODE_OPTIONS value without clobbering an inherited
152+
* one.
153+
*
154+
* A child process' NODE_OPTIONS env var REPLACES (does not extend) the
155+
* parent's, so setting it to only our own flags silently drops any NODE_OPTIONS
156+
* the user configured globally. This joins, in order, the caller's existing
157+
* NODE_OPTIONS ahead of the flags we add so both are honoured.
158+
*
159+
* The value is intentionally not quoted: it is assigned directly to an env var
160+
* (not passed through a shell), and consumers that re-tokenize NODE_OPTIONS on
161+
* whitespace (e.g. Next.js) mishandle embedded quotes.
162+
*/
163+
export function mergeNodeOptions(
164+
envNodeOptions: string | undefined,
165+
addedFlags: string[] | readonly string[],
166+
): string {
167+
return [envNodeOptions, cmdFlagsToString(addedFlags)]
168+
.filter(Boolean)
169+
.join(' ')
170+
}

packages/cli/test/unit/commands/optimize/agent-installer.test.mts

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,9 @@ vi.mock(import('../../../../src/util/process/cmd.mts'), () => ({
4343
.map(([k, v]) => `--${k}=${String(v)}`)
4444
.join(' '),
4545
),
46+
mergeNodeOptions: vi.fn((envNodeOptions, addedFlags) =>
47+
[envNodeOptions, ...(addedFlags || [])].filter(Boolean).join(' '),
48+
),
4649
}))
4750

4851
vi.mock(
@@ -109,6 +112,37 @@ describe('agent installer utilities', () => {
109112
)
110113
})
111114

115+
it('preserves an inherited NODE_OPTIONS instead of clobbering it', async () => {
116+
const { spawn } = vi.mocked(
117+
await import('@socketsecurity/lib-stable/process/spawn/child'),
118+
)
119+
spawn.mockReturnValue(Promise.resolve({ status: 0 }) as unknown)
120+
121+
const pkgEnvDetails = {
122+
agent: 'npm',
123+
agentExecPath: '/usr/bin/npm',
124+
pkgPath: '/test/project',
125+
agentVersion: { major: 10, minor: 0, patch: 0 },
126+
} as unknown
127+
128+
const originalNodeOptions = process.env['NODE_OPTIONS']
129+
process.env['NODE_OPTIONS'] = '--max-old-space-size=4096'
130+
try {
131+
await runAgentInstall(pkgEnvDetails)
132+
} finally {
133+
if (originalNodeOptions === undefined) {
134+
delete process.env['NODE_OPTIONS']
135+
} else {
136+
process.env['NODE_OPTIONS'] = originalNodeOptions
137+
}
138+
}
139+
140+
const spawnEnv = (
141+
spawn.mock.calls[0]![2] as { env: Record<string, string> }
142+
).env
143+
expect(spawnEnv['NODE_OPTIONS']).toContain('--max-old-space-size=4096')
144+
})
145+
112146
it('uses spawn for pnpm agent', async () => {
113147
const { spawn } = vi.mocked(
114148
await import('@socketsecurity/lib-stable/process/spawn/child'),

packages/cli/test/unit/util/process/cmd.test.mts

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import {
3131
cmdPrefixMessage,
3232
filterFlags,
3333
isHelpFlag,
34+
mergeNodeOptions,
3435
} from '../../../../src/util/process/cmd.mts'
3536

3637
describe('cmd utilities', () => {
@@ -99,6 +100,46 @@ describe('cmd utilities', () => {
99100
})
100101
})
101102

103+
describe('mergeNodeOptions', () => {
104+
const addedFlags = [
105+
'--disable-warning=ExperimentalWarning',
106+
'--no-warnings',
107+
]
108+
109+
it('prepends the inherited NODE_OPTIONS ahead of added flags', () => {
110+
const result = mergeNodeOptions('--max-old-space-size=4096', addedFlags)
111+
expect(result).toBe(
112+
'--max-old-space-size=4096 --disable-warning=ExperimentalWarning --no-warnings',
113+
)
114+
})
115+
116+
it('returns only the added flags when NODE_OPTIONS is undefined', () => {
117+
const result = mergeNodeOptions(undefined, addedFlags)
118+
expect(result).toBe('--disable-warning=ExperimentalWarning --no-warnings')
119+
})
120+
121+
it('drops an empty NODE_OPTIONS without adding a stray separator', () => {
122+
const result = mergeNodeOptions('', addedFlags)
123+
expect(result).toBe('--disable-warning=ExperimentalWarning --no-warnings')
124+
})
125+
126+
it('preserves the inherited NODE_OPTIONS when there are no added flags', () => {
127+
expect(mergeNodeOptions('--enable-source-maps', [])).toBe(
128+
'--enable-source-maps',
129+
)
130+
})
131+
132+
it('does not wrap or quote the merged value', () => {
133+
const result = mergeNodeOptions('--enable-source-maps', addedFlags)
134+
expect(result).not.toContain("'")
135+
expect(result).not.toContain('"')
136+
})
137+
138+
it('returns an empty string when nothing is provided', () => {
139+
expect(mergeNodeOptions(undefined, [])).toBe('')
140+
})
141+
})
142+
102143
describe('isHelpFlag', () => {
103144
it('identifies --help flag', () => {
104145
expect(isHelpFlag('--help')).toBe(true)

0 commit comments

Comments
 (0)