Conversation
Docker/podman login built its command as a bash -c string with the password spliced in directly. A password containing a quote, a dollar sign, or backticks could break the login or get executed by the shell. It also meant login only worked on hosts with bash installed. Login now runs the container runtime binary directly with exec.Command and writes the password to its stdin pipe for --password-stdin, so the shell never sees it. Same change in airflow/docker_image.go (Pull and pushWithBash) and airflow/docker_registry.go (dockerLogin). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…eal runtime The bash-less login fallback added for the password-stdin fix runs through cmdExecWithStdin, which this subtest never stubbed, so CI executed a real docker login against a dummy registry and failed on connection refused. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P6DEdUpFBFaAiPKEu81Wti
Coverage Report for CI Build 29870733103Coverage increased (+0.01%) to 43.894%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
Every existing test stubs cmdExecWithStdin, so the helper body itself never ran under coverage and coveralls flagged a drop. Add a direct test mirroring TestExecCmd: stdin reaches the child's stdout, a missing binary errors with "failed to find", and a nonzero exit errors with "failed to execute cmd". Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P6DEdUpFBFaAiPKEu81Wti
Contributor
|
This PR is stale because it has been open for 60 days with no activity. |
This branch has not been deployed
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.
Description
Three registry-login sites built a
bash -cstring of the formecho "<password>" | docker login ... --password-stdin, splicing the credential into shell text. A password containing a quote,$, or backticks either broke the login or got interpreted by the shell, and login required bash on the host for no reason.What changed: a new
cmdExecWithStdinhelper (matching the existingcmdExec's style) runs the runtime binary directly with the password written to the child's stdin pipe — no shell, no quoting rules, no bash dependency. All three sites converted; the rest of the tree was checked for the same pattern and none remain.Breaking changes
None for legitimate use — same login outcome, same error format. Bash is no longer required for login. One edge-case wording change: a missing runtime binary now errors as "failed to find the command" rather than a bash "not found" error.
Testing
Tests cover passwords containing
$(...), backticks, and quotes reaching the login command unchanged via stdin, plus mock updates across the touched files.go test ./...,go vet ./airflow/..., andmake lintpass.🤖 Generated with Claude Code
https://claude.ai/code/session_01P6DEdUpFBFaAiPKEu81Wti