Add input parameter validation for --source and --minionversion on Windows and Linux#64
Merged
Conversation
Windows: Validate the Source and MinionVersion parameters early in the script (after the test-mode guard, before Main) so malformed or potentially malicious values are rejected with exit code 126 before any network or filesystem activity occurs. - Test-SourceParameter: rejects whitespace, control characters, and shell-injection characters; validates URL structure via System.Uri for HTTP/HTTPS/FTP; enforces UNC and local path format - Test-MinionVersionParameter: validates against the known Salt CalVer format (latest, YYYY, YYYY.N, YYYY.N.N, YYYY.NrcN) - _parse_config: skips any key or value containing a control character to prevent YAML injection via ConfigOptions - Functional tests cover valid and invalid cases for both validators and the control-character config guard; integration tests verify exit code 126 end-to-end via subprocess invocation
Adds _validate_source_param and _validate_minion_version_param helper functions to linux/svtminion.sh, mirroring the Windows validation added in 681125a. Both functions exit 126 via _error_log on invalid input, consistent with all other error handling in the script. Adds 5 integration test cases to tests/linux/test-linux.sh covering URL spaces, unknown schemes, injection characters, and malformed version strings.
Changing SOURCE_PARAMS and MINION_VERSION_PARAMS from "$*" to "$1" in the argument parser ensures the validators receive only the user-supplied value, not the full remaining argument list. This allows _validate_source_param to correctly detect spaces in HTTP/S/FTP URLs rather than silently truncating to the first word. Removes the cut workaround from both validator functions since the value is now unambiguous.
Pre-3005 Linux Salt packages used a -N build suffix in their version strings. The CalVer regex only allowed rc suffixes, causing _validate_minion_version_param to reject 3004.2-1 and 3003.3-1 with exit 126 and breaking existing CI tests that rely on those versions defaulting to latest. Adds (-[0-9]+) as an alternative to (rc[0-9]+) in the Linux validator only — Windows packages were not released with this suffix format.
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
2 participants
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.
Neither svtminion.ps1 nor svtminion.sh validated the --source or --minionversion parameters before use. Both values reached
wget/curl/path operations unvalidated, and on Windows they reached Invoke-WebRequest and file system calls. This PR adds
explicit validation to both scripts, with consistent exit code 126 on failure — matching the existing Administrator privilege
check behavior.
Windows (windows/svtminion.ps1)
• Adds Test-SourceParameter: rejects empty values, control characters, shell injection characters ( ; & | < > ), and values that don't match a recognised scheme (HTTP/S, FTP, UNC, or local drive path). Uses [System.Uri]::TryCreate` for URL structural validation.
• Adds Test-MinionVersionParameter: validates against the Salt CalVer pattern (latest, 3006, 3006.2, 3008.0rc1, etc.).
• Adds control-character filtering to _parse_config to guard against YAML injection via key=value config options.
• Both validators are called after script initialisation but before Main, so they run in all production paths and are skipped during test imports.
• Parameter comments for -Source and -MinionVersion updated to document the exit 126 behaviour.
Linux (linux/svtminion.sh)
• Adds _validate_source_param: same checks as Windows adapted to bash — control characters via [[:cntrl:]], injection characters via grep -qE, and scheme-specific structural checks (HTTP/S/FTP, absolute path, file://).
• Adds _validate_minion_version_param: same CalVer regex as Windows expressed as a bash ERE.
• Both functions call _error_log on failure, which exits 126 consistently with all other error handling in the script.
• Both validators are called in the MAIN section immediately before _source_fn and _set_install_minion_version_fn.
• --help output updated to note that invalid values exit with code 126.
Tests
• tests/windows/functional/test_input_validation.ps1 — 32 table-driven unit test cases covering valid, invalid, and edge-case inputs for both validators.
• tests/windows/functional/test_parse_config.ps1 — extended with a test for control-character rejection in _parse_config.
• tests/windows/integration/test_input_validation.ps1 — 5 integration tests invoking svtminion.ps1 as a subprocess and asserting exit code 126 for invalid inputs.
• tests/linux/test-linux.sh — 5 integration test cases added (URL with space, unknown scheme, semicolon injection, version with space, version with semicolon), each asserting exit code 126.
Test plan
• Run powershell.exe -file tests\windows\runtests.ps1 -path tests\windows\functional\test_input_validation.ps1 — all cases
pass
• Run powershell.exe -file tests\windows\runtests.ps1 -path tests\windows\functional\test_parse_config.ps1 — all cases pass
• Run powershell.exe -file tests\windows\runtests.ps1 -path tests\windows\integration\test_input_validation.ps1 — all 5
integration cases exit 126
• Run Linux CI pipeline — new test-linux.sh validation cases pass before any install tests