Skip to content

Commit 1a3cc47

Browse files
jdaltonjdalton
committed
refactor(constants): improve Node.js version detection and flag management
- Add getNodeMinorVersion() and getNodePatchVersion() helper functions - Replace || with ?? for nullish coalescing in version helpers - Refactor getNodeHardenFlags() to properly guard flags by Node version: - --experimental-permission for Node 20-23 - --permission + permission grants for Node 24+ - --force-node-api-uncaught-exceptions-policy only for Node 22+ - Remove --experimental-policy (no policy file provided) - Integrate getNodePermissionFlags() into getNodeHardenFlags() - Update all manual version extraction to use helper functions - Add comprehensive version-specific tests
1 parent 7cb81b4 commit 1a3cc47

File tree

2 files changed

+103
-49
lines changed

2 files changed

+103
-49
lines changed

‎src/constants/node.ts‎

Lines changed: 38 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,15 @@ export function getNodeVersion(): string {
1212
}
1313

1414
export function getNodeMajorVersion(): number {
15-
return Number.parseInt(NODE_VERSION.slice(1).split('.')[0] || '0', 10)
15+
return Number.parseInt(NODE_VERSION.slice(1).split('.')[0] ?? '0', 10)
16+
}
17+
18+
export function getNodeMinorVersion(): number {
19+
return Number.parseInt(NODE_VERSION.split('.')[1] ?? '0', 10)
20+
}
21+
22+
export function getNodePatchVersion(): number {
23+
return Number.parseInt(NODE_VERSION.split('.')[2] ?? '0', 10)
1624
}
1725

1826
// Maintained Node.js versions.
@@ -43,36 +51,26 @@ export function supportsNodePermissionFlag(): boolean {
4351

4452
export function supportsNodeRequireModule(): boolean {
4553
const major = getNodeMajorVersion()
46-
return (
47-
major >= 23 ||
48-
(major === 22 &&
49-
Number.parseInt(NODE_VERSION.split('.')[1] || '0', 10) >= 12)
50-
)
54+
return major >= 23 || (major === 22 && getNodeMinorVersion() >= 12)
5155
}
5256

5357
export function supportsNodeRun(): boolean {
5458
const major = getNodeMajorVersion()
55-
return (
56-
major >= 23 ||
57-
(major === 22 &&
58-
Number.parseInt(NODE_VERSION.split('.')[1] || '0', 10) >= 11)
59-
)
59+
return major >= 23 || (major === 22 && getNodeMinorVersion() >= 11)
6060
}
6161

6262
export function supportsNodeDisableSigusr1Flag(): boolean {
6363
const major = getNodeMajorVersion()
64+
const minor = getNodeMinorVersion()
6465
// --disable-sigusr1 added in v22.14.0, v23.7.0.
6566
// Stabilized in v22.20.0, v24.8.0.
6667
if (major >= 24) {
67-
const minor = Number.parseInt(NODE_VERSION.split('.')[1] || '0', 10)
6868
return minor >= 8
6969
}
7070
if (major === 23) {
71-
const minor = Number.parseInt(NODE_VERSION.split('.')[1] || '0', 10)
7271
return minor >= 7
7372
}
7473
if (major === 22) {
75-
const minor = Number.parseInt(NODE_VERSION.split('.')[1] || '0', 10)
7674
return minor >= 14
7775
}
7876
return false
@@ -102,27 +100,6 @@ export function supportsProcessSend(): boolean {
102100

103101
// Node.js flags.
104102
let _nodeHardenFlags: string[]
105-
export function getNodeHardenFlags(): string[] {
106-
if (_nodeHardenFlags === undefined) {
107-
const major = getNodeMajorVersion()
108-
const flags = [
109-
'--disable-proto=delete',
110-
// Node.js 24+ uses --permission instead of --experimental-permission.
111-
// The permission model graduated from experimental to production-ready.
112-
major >= 24 ? '--permission' : '--experimental-permission',
113-
// Force uncaught exceptions policy for N-API addons (Node.js 22+).
114-
'--force-node-api-uncaught-exceptions-policy',
115-
]
116-
// Only add policy flag if we're using experimental permission (Node < 24).
117-
// Node 24+ --policy requires a policy file which we don't have.
118-
if (major < 24) {
119-
flags.push('--experimental-policy')
120-
}
121-
_nodeHardenFlags = flags
122-
}
123-
return _nodeHardenFlags
124-
}
125-
126103
let _nodePermissionFlags: string[]
127104
export function getNodePermissionFlags(): string[] {
128105
if (_nodePermissionFlags === undefined) {
@@ -147,6 +124,32 @@ export function getNodePermissionFlags(): string[] {
147124
return _nodePermissionFlags
148125
}
149126

127+
export function getNodeHardenFlags(): string[] {
128+
if (_nodeHardenFlags === undefined) {
129+
const major = getNodeMajorVersion()
130+
const flags: string[] = ['--disable-proto=delete']
131+
132+
// Permission model: Experimental in Node 20-23, stable in Node 24+.
133+
// Node 20-23: --experimental-permission (no explicit grants needed).
134+
// Node 24+: --permission (requires explicit grants via getNodePermissionFlags()).
135+
if (major >= 24) {
136+
flags.push('--permission')
137+
// Add permission-specific grants for Node 24+.
138+
flags.push(...getNodePermissionFlags())
139+
} else if (major >= 20) {
140+
flags.push('--experimental-permission')
141+
}
142+
143+
// Force uncaught exceptions policy for N-API addons (Node.js 22+).
144+
if (major >= 22) {
145+
flags.push('--force-node-api-uncaught-exceptions-policy')
146+
}
147+
148+
_nodeHardenFlags = flags
149+
}
150+
return _nodeHardenFlags
151+
}
152+
150153
let _nodeNoWarningsFlags: string[]
151154
export function getNodeNoWarningsFlags(): string[] {
152155
if (_nodeNoWarningsFlags === undefined) {

‎test/unit/constants/node.test.ts‎

Lines changed: 65 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,9 @@ import {
1717
getNodeDisableSigusr1Flags,
1818
getNodeHardenFlags,
1919
getNodeMajorVersion,
20+
getNodeMinorVersion,
2021
getNodeNoWarningsFlags,
22+
getNodePatchVersion,
2123
getNodePermissionFlags,
2224
getNodeVersion,
2325
supportsNodeCompileCacheApi,
@@ -51,13 +53,39 @@ describe('node constants', () => {
5153

5254
it('should match process.version major', () => {
5355
const expected = Number.parseInt(
54-
process.version.slice(1).split('.')[0] || '0',
56+
process.version.slice(1).split('.')[0] ?? '0',
5557
10,
5658
)
5759
expect(getNodeMajorVersion()).toBe(expected)
5860
})
5961
})
6062

63+
describe('getNodeMinorVersion', () => {
64+
it('should return minor version number', () => {
65+
const minor = getNodeMinorVersion()
66+
expect(typeof minor).toBe('number')
67+
expect(minor).toBeGreaterThanOrEqual(0)
68+
})
69+
70+
it('should match process.version minor', () => {
71+
const expected = Number.parseInt(process.version.split('.')[1] ?? '0', 10)
72+
expect(getNodeMinorVersion()).toBe(expected)
73+
})
74+
})
75+
76+
describe('getNodePatchVersion', () => {
77+
it('should return patch version number', () => {
78+
const patch = getNodePatchVersion()
79+
expect(typeof patch).toBe('number')
80+
expect(patch).toBeGreaterThanOrEqual(0)
81+
})
82+
83+
it('should match process.version patch', () => {
84+
const expected = Number.parseInt(process.version.split('.')[2] ?? '0', 10)
85+
expect(getNodePatchVersion()).toBe(expected)
86+
})
87+
})
88+
6189
describe('getMaintainedNodeVersions', () => {
6290
it('should return maintained versions object', () => {
6391
const versions = getMaintainedNodeVersions()
@@ -165,7 +193,7 @@ describe('node constants', () => {
165193
it('should check minor version for Node.js 22', () => {
166194
const major = getNodeMajorVersion()
167195
if (major === 22) {
168-
const minor = Number.parseInt(process.version.split('.')[1] || '0', 10)
196+
const minor = getNodeMinorVersion()
169197
const result = supportsNodeRequireModule()
170198
if (minor >= 12) {
171199
expect(result).toBe(true)
@@ -193,7 +221,7 @@ describe('node constants', () => {
193221
it('should check minor version for Node.js 22', () => {
194222
const major = getNodeMajorVersion()
195223
if (major === 22) {
196-
const minor = Number.parseInt(process.version.split('.')[1] || '0', 10)
224+
const minor = getNodeMinorVersion()
197225
const result = supportsNodeRun()
198226
if (minor >= 11) {
199227
expect(result).toBe(true)
@@ -212,7 +240,7 @@ describe('node constants', () => {
212240

213241
it('should check version-specific support', () => {
214242
const major = getNodeMajorVersion()
215-
const minor = Number.parseInt(process.version.split('.')[1] || '0', 10)
243+
const minor = getNodeMinorVersion()
216244
const result = supportsNodeDisableSigusr1Flag()
217245

218246
if (major >= 24) {
@@ -263,31 +291,54 @@ describe('node constants', () => {
263291
expect(flags).toContain('--disable-proto=delete')
264292
})
265293

266-
it('should include force-node-api flag', () => {
267-
const flags = getNodeHardenFlags()
268-
expect(flags).toContain('--force-node-api-uncaught-exceptions-policy')
269-
})
270-
271-
it('should use --permission for Node.js 24+', () => {
294+
it('should use --permission for Node.js 24+ with explicit grants', () => {
272295
const major = getNodeMajorVersion()
273296
const flags = getNodeHardenFlags()
274297
if (major >= 24) {
275298
expect(flags).toContain('--permission')
276299
expect(flags).not.toContain('--experimental-permission')
277-
expect(flags).not.toContain('--experimental-policy')
300+
// Should include permission grants from getNodePermissionFlags()
301+
expect(flags).toContain('--allow-fs-read=*')
302+
expect(flags).toContain('--allow-fs-write=*')
303+
expect(flags).toContain('--allow-child-process')
304+
} else {
305+
expect(flags).not.toContain('--permission')
306+
// Permission grants should not be included for Node < 24
307+
expect(flags).not.toContain('--allow-fs-read=*')
308+
expect(flags).not.toContain('--allow-fs-write=*')
309+
expect(flags).not.toContain('--allow-child-process')
278310
}
279311
})
280312

281-
it('should use --experimental-permission for Node.js < 24', () => {
313+
it('should use --experimental-permission for Node.js 20-23', () => {
282314
const major = getNodeMajorVersion()
283315
const flags = getNodeHardenFlags()
284-
if (major < 24) {
316+
if (major >= 20 && major < 24) {
285317
expect(flags).toContain('--experimental-permission')
286-
expect(flags).toContain('--experimental-policy')
318+
expect(flags).not.toContain('--permission')
319+
} else if (major < 20) {
320+
expect(flags).not.toContain('--experimental-permission')
287321
expect(flags).not.toContain('--permission')
288322
}
289323
})
290324

325+
it('should include --force-node-api-uncaught-exceptions-policy for Node.js 22+', () => {
326+
const major = getNodeMajorVersion()
327+
const flags = getNodeHardenFlags()
328+
if (major >= 22) {
329+
expect(flags).toContain('--force-node-api-uncaught-exceptions-policy')
330+
} else {
331+
expect(flags).not.toContain(
332+
'--force-node-api-uncaught-exceptions-policy',
333+
)
334+
}
335+
})
336+
337+
it('should not include --experimental-policy', () => {
338+
const flags = getNodeHardenFlags()
339+
expect(flags).not.toContain('--experimental-policy')
340+
})
341+
291342
it('should return same instance on multiple calls', () => {
292343
const first = getNodeHardenFlags()
293344
const second = getNodeHardenFlags()

0 commit comments

Comments
 (0)