src: add --disable-sigusr1 to prevent signal i/o thread

[RafaelGSS]

This commit adds a new flag --disable-sigusr1 to prevent the SignalIOThread to be up listening the SIGUSR1 events and then starting the debugging session.

---

This flag is handy when dealing with multiple apps in the same environment. I wouldn't call it a full security guarantee since one would require access to the environment in order to expose the debugger, and with that assumption, one can do anything one wants. However, I do believe this would serve as a seatbelt in some scenarios.

Please note users already do it by require('inspector').close(), but a flag seems more suitable.

[juanarbol]

Idk if this is the wait to do it. There's also SIGUSR2 among other signals. Wouldn't that be better to have a "blacklist" approach instead? So the logic scales to any signal.

[RafaelGSS]

Idk if this is the wait to do it. There's also SIGUSR2 among other signals. Wouldn't that be better to have a "blacklist" approach instead? So the logic scales to any signal.

Initially, I thought about the --disable-signal=SIGUSR1 but then I realized that there's no "damage" in receiving a SIGUSR2. AFAIK only SIGUSR1 would initiate the debugging session.

I could also rename the flag to --disable-sigusr1-debug to make it more explicit but, I don't think there's a need for that.

Also, having a --disable-signal and not implementing other signals might sound confusing to users.

[juanarbol]

I don't disagree w/ this patch. But if this is intended to disable/flip a default behaviour, it may need some update in the documentation side as well.

Useful when activating the inspector by sending the SIGUSR1 signal.

Refs: https://nodejs.org/docs/latest/api/cli.html#--inspect-porthostport

[RafaelGSS]

Ok. I can update the --inspect documentation to explicitly mention when --disable-sigusr1 is passed, the debugging session initialized with SIGUSR1 won't work.

[codecov]

Codecov Report

Attention: Patch coverage is 75.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 88.54%. Comparing base (98d4ebc) to head (384dc99).
Report is 6 commits behind head on main.

Files with missing lines	Patch %	Lines
src/env-inl.h	50.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #56441   +/-   ##
=======================================
  Coverage   88.53%   88.54%           
=======================================
  Files         657      657           
  Lines      190741   190768   +27     
  Branches    36607    36609    +2     
=======================================
+ Hits       168881   168918   +37     
- Misses      15036    15040    +4     
+ Partials     6824     6810   -14     

Files with missing lines	Coverage Δ
src/node_options.cc	87.78% <100.00%> (-0.15%)	⬇️
src/node_options.h	98.31% <100.00%> (+<0.01%)	⬆️
src/env-inl.h	96.99% <50.00%> (-0.21%)	⬇️

... and 36 files with indirect coverage changes

[RafaelGSS]

PTAL @juanarbol

I have also changed the flag to be in stability 1.1 so we can re-evaluate if we need to rename or make it more generic in the future.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/64314/