Drop the custom-binary character allowlist - #1187
Open
derekm wants to merge 2 commits into
Open
Conversation
🦋 Changeset detectedLatest commit: 4fa5b24 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Removes the regex validation in customBinaryPlugin that restricted the git binary/prefix to /^([a-z]:)?([a-z0-9/._~-]+)$/i. Why it is safe to remove: - The binary is always spawned directly by the OS with shell: false (the spawn.options plugin only forwards uid/gid, so shell cannot be enabled via user options). Shell metacharacters (; | & $ ( )) are therefore never interpreted by a shell - they are inert, and cannot be used for command injection. The allowlist provided no protection that shell: false did not already provide. - The allowlist was actively harmful: its character set was too narrow and rejected legitimate paths such as C:\Program Files\Git\bin\git.exe (the space in the path is outside the allowed charset). What changes: - toBinaryConfig now only validates shape (1-2 non-empty string elements); the per-character regex check is gone. Previously-rejected values pass through to spawn as-is. - unsafe.allowUnsafeCustomBinary previously bypassed the allowlist; with the allowlist removed it is retained for backwards compatibility but is now a no-op that emits a deprecation warning when supplied (validation always throws on an empty element regardless of the flag). The type is marked @deprecated. - Docs updated: the Caveats/Security section no longer claims a character allowlist, and documents the option as deprecated/no-op. Tests updated: syntax the old allowlist rejected (spaces, quotes, shell metacharacters, Windows drive-colon paths) is now asserted to pass through to spawn. Full unit suite: 634 passed. Build green.
derekm
force-pushed
the
custom-binary-drop-regex
branch
from
August 2, 2026 03:36
ceb3f73 to
32f8141
Compare
24 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Removes the regex validation in customBinaryPlugin that restricted the git binary/prefix to
/^([a-z]:)?([a-z0-9/._~-]+)$/i.Why it is safe to remove:
The binary is always spawned directly by the OS with
shell: false(thespawn.optionsplugin only forwardsuid/gid, so shell cannot be enabled via user options). Shell metacharacters (; | & $ ( )) are therefore never interpreted by a shell - they are inert, and cannot be used for command injection. The allowlist provided no protection thatshell: falsedid not already provide.The allowlist was actively harmful: its character set was too narrow and rejected legitimate paths such as
C:\Program Files\Git\bin\git.exe(the space in the path is outside the allowed charset).What changes:
toBinaryConfignow only validates shape (1-2 non-empty string elements); the per-character regex check is gone. Previously-rejected values pass through to spawn as-is.unsafe.allowUnsafeCustomBinarypreviously bypassed the allowlist; with the allowlist removed it is retained for backwards compatibility but is now a no-op that emits a deprecation warning when supplied (validation always throws on an empty element regardless of the flag). The type is marked@deprecated.Tests updated: the values the old allowlist rejected ($, !, spaces, quotes, Windows drive-colon paths) are now asserted to pass through to spawn.
Full unit suite: 634 passed. Build green.
Fixes: #1186