Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 41 additions & 1 deletion src/shadow/npm-base.mts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,42 @@ export type ShadowBinResult = {
spawnPromise: SpawnResult<string, SpawnExtra | undefined>
}

// Length of the '--node-options=' prefix, used to slice the value out of an
// npm '--node-options=<value>' argument.
const NODE_OPTIONS_FLAG_PREFIX_LENGTH = '--node-options='.length

/**
* Build the '--node-options=<value>' argument passed to npm so its lifecycle
* scripts run with our Node permission flags.
*
* npm assigns this value to NODE_OPTIONS for the scripts it runs, REPLACING any
* inherited NODE_OPTIONS. To avoid clobbering the user's configuration we merge,
* in order, the caller's existing process.env.NODE_OPTIONS and any
* '--node-options=<value>' npm argument ahead of our permission flags (issue
* #1036).
*
* The value is intentionally NOT wrapped in quotes. shadowNpmBase spawns npm
* without a shell, so any quotes would be passed literally to npm and become
* part of the NODE_OPTIONS value. Consumers that re-tokenize NODE_OPTIONS on
* whitespace (e.g. Next.js) then strip the leading quote off '--permission',
* dropping it while keeping the '--allow-*' flags, which crashes Node >= 24
* with ERR_MISSING_OPTION (issue #1160).
*/
export function buildNpmNodeOptionsArg(
envNodeOptions: string | undefined,
nodeOptionsArg: string | undefined,
permArgs: string[] | readonly string[],
): string {
const value = [
envNodeOptions,
nodeOptionsArg ? nodeOptionsArg.slice(NODE_OPTIONS_FLAG_PREFIX_LENGTH) : '',
cmdFlagsToString(permArgs),
]
.filter(Boolean)
.join(' ')
return `--node-options=${value}`
}

export default async function shadowNpmBase(
binName: typeof NPM | typeof NPX,
args: string[] | readonly string[] = process.argv.slice(2),
Expand Down Expand Up @@ -113,7 +149,11 @@ export default async function shadowNpmBase(
...noAuditArgs,
...(useNodeOptions
? [
`--node-options='${nodeOptionsArg ? nodeOptionsArg.slice(15) : ''}${cmdFlagsToString(permArgs)}'`,
buildNpmNodeOptionsArg(
process.env['NODE_OPTIONS'],
nodeOptionsArg,
permArgs,
),
]
: []),
'--no-fund',
Expand Down
88 changes: 88 additions & 0 deletions src/shadow/npm-base.test.mts
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
import { describe, expect, it } from 'vitest'

import { buildNpmNodeOptionsArg } from './npm-base.mts'

// The permission flags shadowNpmBase injects on Node versions that support
// --permission. Mirrors the array built in npm-base.mts.
const permArgs = [
'--permission',
'--allow-child-process',
'--allow-fs-read=*',
'--allow-fs-write=/project/*',
]

describe('buildNpmNodeOptionsArg', () => {
it('does not wrap the value in quotes (issue #1160)', () => {
// Literal quotes survive spawn() (no shell) and get re-tokenized by
// consumers like Next.js, which strips the leading quote off --permission
// and crashes Node >= 24 with ERR_MISSING_OPTION.
const result = buildNpmNodeOptionsArg(undefined, undefined, permArgs)

expect(result).not.toContain("'")
expect(result).not.toContain('"')
expect(result).toBe(
'--node-options=--permission --allow-child-process --allow-fs-read=* --allow-fs-write=/project/*',
)
// The NODE_OPTIONS value must keep --permission as a clean, standalone
// token so a whitespace re-tokenizer (Next.js) doesn't orphan the
// --allow-* flags.
const value = result.slice('--node-options='.length)
expect(value.split(' ')).toContain('--permission')
})

it('merges an inherited NODE_OPTIONS env value ahead of perm flags (issue #1036)', () => {
const result = buildNpmNodeOptionsArg(
'--max-old-space-size=4096',
undefined,
permArgs,
)

expect(result).toBe(
'--node-options=--max-old-space-size=4096 --permission --allow-child-process --allow-fs-read=* --allow-fs-write=/project/*',
)
})

it('preserves the inherited NODE_OPTIONS even when there are no perm flags', () => {
const result = buildNpmNodeOptionsArg('--enable-source-maps', undefined, [])

expect(result).toBe('--node-options=--enable-source-maps')
})

it('strips the --node-options= prefix from an npm --node-options arg', () => {
const result = buildNpmNodeOptionsArg(
undefined,
'--node-options=--trace-warnings',
permArgs,
)

expect(result).toBe(
'--node-options=--trace-warnings --permission --allow-child-process --allow-fs-read=* --allow-fs-write=/project/*',
)
})

it('merges env NODE_OPTIONS, the npm --node-options arg, and perm flags in order', () => {
const result = buildNpmNodeOptionsArg(
'--enable-source-maps',
'--node-options=--trace-warnings',
permArgs,
)

expect(result).toBe(
'--node-options=--enable-source-maps --trace-warnings --permission --allow-child-process --allow-fs-read=* --allow-fs-write=/project/*',
)
})

it('emits an empty value when nothing is provided', () => {
const result = buildNpmNodeOptionsArg(undefined, undefined, [])

expect(result).toBe('--node-options=')
})

it('drops an empty-string env NODE_OPTIONS instead of adding a stray space', () => {
const result = buildNpmNodeOptionsArg('', undefined, permArgs)

expect(result).toBe(
'--node-options=--permission --allow-child-process --allow-fs-read=* --allow-fs-write=/project/*',
)
})
})
Loading