Repository navigation
Inconsistent validation of custom signals #44749
Description
Version
18.9.0
Platform
Darwin
Subsystem
child_process
os
What steps will reproduce the bug?
In the current implementation, spawn and related functions throw synchronous errors for invalid signals, e.g.:
import { spawn } from 'node:child_process';
// TypeError [ERR_UNKNOWN_SIGNAL]
spawn(process.execPath, ['-v'], { killSignal: 12345 });Anyway, it's easy to define custom signals that pass the validation:
import { spawn } from 'node:child_process';
import { constants } from 'node:os';
constants.signals.FOOBAR = 12345;
// no error
spawn(process.execPath, ['-v'], { killSignal: 12345 });The same applies when signals are identified by name, i.e:
import { spawn } from 'node:child_process';
// TypeError [ERR_UNKNOWN_SIGNAL]
spawn(process.execPath, ['-v'], { killSignal: 'FOOBAR' });and
import { spawn } from 'node:child_process';
import { constants } from 'node:os';
constants.signals.FOOBAR = 12345;
// no error
spawn(process.execPath, ['-v'], { killSignal: 'FOOBAR' });but if a custom signal is added after the internal mapping of numeric codes to names signals has been generated, validation fails for numeric codes (but not for signal names):
import { spawn } from 'node:child_process';
import { constants } from 'node:os';
spawn(process.execPath, ['-v'], { killSignal: 1 });
constants.signals.FOOBAR = 12345;
spawn(process.execPath, ['-v'], { killSignal: 'FOOBAR' }); // no error
spawn(process.execPath, ['-v'], { killSignal: 12345 }); // TypeError [ERR_UNKNOWN_SIGNAL]This strikes to me as issue of improper memoization:
Lines 272 to 283 in 9ae2af4
where signalsToNamesMapping never changes once created.
On the other hand, I'm not convinced that the current behavior - being able to add, remove, and change signals at will - is intended. If it is, then the validation logic should be fixed. If it is not, then constants.signals should be better frozen to prevent changes.
How often does it reproduce? Is there a required condition?
All the time.
What is the expected behavior?
Either custom numeric signals should always be accepted as valid, or they should always be rejected. If they should be rejected, then custom string signals should be rejected, too.
What do you see instead?
Inconsistent behavior, depending on (possibly unrelated) previous operations.
Additional information
No response
Activity
kvakil commentedon Oct 1, 2022
Do you have a usecase for "custom signals"? My gut says that it makes more sense to freeze all of the
constantsobjects to prevent changing it. We already prevent changing/deleting the properties ofconstantsobjects, but presumably it's an oversight that the object itself is not frozen. It sounds like that would match your "expected behavior"?fasttime commentedon Oct 1, 2022
@kvakil I'm also under the impression that allowing changes to the the predefined signals was never intended. This has been possible ever since
os.constants.signalswas made public in Node 6.3, so I imagine that someone could be relying on the current behavior, but I don't know any examples. Freezingsignalswould be probably a breaking change, but also the safest solution.developeruzairsaleem commentedon Sep 26, 2025
Hi I would love to work on this issue.
BharathPESU commentedon Nov 4, 2025
Hi — I’d like to take this one.
Summary: spawn/exec validation for killSignal is inconsistent when os.constants.signals is mutated at runtime. If a numeric signal is added via constants.signals.FOO = before internal memoization runs, numeric validation accepts it; if added after memoization, numeric validation rejects it while name-based validation accepts it.
Root cause (brief): getSignalsToNamesMapping() memoizes a numeric→name mapping at first call and never refreshes it, so subsequent mutations of constants.signals are not reflected.
Proposed plan: either (A) remove the memoization and compute the mapping on each validation call (keeps dynamic behavior but consistent), or (B) freeze constants.signals at start-up so it cannot be mutated (prevent dynamic changes). I’ll prepare a PR with both alternatives (defaulting to A) and tests.
Could you assign this to me? I’ll open the PR with a proposed patch + unit tests and mark it for review.
os: freeze signals constant
os: freeze signals constant
os: freeze signals constant
os: freeze signals constant
os: freeze signals constant
os: freeze signals constant
os: freeze signals constant
os: freeze signals constant
os: freeze signals constant
os: freeze signals constant