fix(security-guidance): use a cross-platform Python launcher - #47502
fix(security-guidance): use a cross-platform Python launcher#47502Rohan5commit wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes the security-guidance plugin’s Python hook invocation so it works on Windows (where python3 may not exist) by introducing a small Node-based launcher that tries multiple Python entrypoints and preserves hook I/O behavior.
Changes:
- Add a Node launcher (
run_python_hook.js) that attempts platform-appropriate Python commands and falls back when commands are missing. - Update
hooks.jsonto invoke the Node launcher instead of callingpython3directly. - Add Node tests covering launcher selection and ENOENT fallback behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| plugins/security-guidance/hooks/run_python_hook.js | New Node launcher that selects/falls back between Python launchers and proxies stdin/stdout/stderr. |
| plugins/security-guidance/hooks/run_python_hook.test.js | Tests for launcher ordering and fallback behavior when the first launcher is missing. |
| plugins/security-guidance/hooks/hooks.json | Switch hook command to use the Node launcher with the existing Python hook script. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| process.stderr.write(result.stderr); | ||
| } | ||
|
|
||
| return result.status ?? (result.error ? 1 : 0); |
There was a problem hiding this comment.
spawnSync returns status: null when the child is terminated by a signal (with signal set). The current return expression treats that case as success (0) when result.error is falsy, which can silently mask real failures. Consider returning a non-zero exit code when status is null due to result.signal (or generally defaulting null status to 1 unless the error is ENOENT and you intend to fall through).
| return result.status ?? (result.error ? 1 : 0); | |
| return result.status ?? 1; |
Summary
Testing
Closes #46449