fix(security): prevent shell injection from Jenkins job parameters#147
Open
lfrancke wants to merge 1 commit into
Open
fix(security): prevent shell injection from Jenkins job parameters#147lfrancke wants to merge 1 commit into
lfrancke wants to merge 1 commit into
Conversation
The operator test runner spliced free-text Jenkins parameters straight into shell command strings executed via os.system() / run_command() (which runs `/bin/bash -c`). A user able to trigger a test job could get arbitrary code execution in a container holding REPLICATED_API_TOKEN, IONOS credentials and the cluster kubeconfig. Quote every externally-controlled value with shlex.quote(): - operator-test-runner.py: TEST_SCRIPT_PARAMS (both the run-tests and auto-retry branches), OUTPUT_FILE_USER (chown), GIT_BRANCH + repo (git clone), OPERATOR/OPERATOR_VERSION (SDP install). - cluster_logging.py: the OpenSearch endpoint/user/password interpolated into `kubectl create secret` (single quotes there did not survive a value containing a quote), plus the cluster id. Verified: a param like `foo; touch X` executes under the old code and is passed as a literal argument under the new code.
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.
Security fix (blocker)
The operator test runner spliced free-text Jenkins job parameters directly into shell command strings run via
os.system()/run_command()(/bin/bash -c). Anyone able to trigger an*-it-customtest job could achieve arbitrary code execution inside a container holdingREPLICATED_API_TOKEN, IONOS credentials, and the cluster kubeconfig.Injection points closed (all
shlex.quote()d)apps/src/operator-test-runner.py:TEST_SCRIPT_PARAMS— both therun-testsbranch and the auto-retry branch (whitespace-split tokens were rejoined verbatim, so;/|/backticks survived).OUTPUT_FILE_USER→chown.GIT_BRANCH+ repo name →git clone.OPERATOR/OPERATOR_VERSION→ SDP install command.apps/src/modules/cluster_logging.py:endpoint/user/passwordinterpolated intokubectl create secret— the hand-written single quotes broke on any value containing a quote. Also the cluster id.Verification
ruff check apps/clean; both files parse.foo; touch Xexecutes under the old code and is passed as a literal argument under the new code.Note
Values are still passed via a shell (the runner relies on
tee, redirection andecho $?), so quoting is the fix; a follow-up could move secrets off the command line entirely (--from-env-file=) to also avoid process-list exposure.Part of the review follow-ups; independent of the other PRs.