Harden shell execution: fail loudly on missing test directory - #4
Merged
Merged
Conversation
Previously chdir(realpath(...)) would chdir(false) silently if the target dev/tests directory did not exist, potentially running paratest in whatever the previous loop iteration's (or the process's original) working directory was instead of failing loudly. Also document why $commandArguments is intentionally passed through unescaped (mirrors Magento's own dev:tests:run, meant to carry multiple shell tokens; this is a local dev CLI, not fed untrusted remote input).
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.
Summary
realpath()can returnfalseif adev/tests/*directory does not exist; the code then calledchdir(false)without checking, silently running paratest in an unintended working directory instead of failing.--arguments/-cis intentionally passed through unescaped to the shell command (mirrors Magento's nativedev:tests:run; it must carry multiple shell tokens like--filter=Foo bar.php, and this is a local dev CLI invoked by the developer, not fed untrusted remote input) — reviewed as part of an open-source readiness pass, no injection vector found beyond what upstream Magento already accepts.Test plan
testMissingTestDirectoryIsReportedAsFailurecovering the new branch