Skip to content

Replace docker/login-action with inline docker login#8694

Open
inahga wants to merge 5 commits intomainfrom
replace-docker-login-action
Open

Replace docker/login-action with inline docker login#8694
inahga wants to merge 5 commits intomainfrom
replace-docker-login-action

Conversation

@inahga
Copy link
Copy Markdown
Contributor

@inahga inahga commented Mar 30, 2026

Drop dependency on docker/login-action by replacing it with a simple docker login command. Just reduces our supply chain exposure a bit.

This is done in a way consistent with GitHub's suggestions https://docs.github.com/en/actions/how-tos/write-workflows/choose-what-workflows-do/use-secrets#using-secrets-in-a-workflow, in particular: the secret is passed to the inner step as an environment variable; printenv prevents the secret from being visible in the process list, and is piped over stdin to docker login.

Drop dependency on docker/login-action by replacing it with a simple
docker login command. Just reduces our supply chain exposure a bit.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@inahga inahga requested a review from a team as a code owner March 30, 2026 18:04
@inahga inahga requested a review from aarongable March 30, 2026 18:04
@inahga inahga marked this pull request as draft March 30, 2026 18:05
@inahga inahga marked this pull request as ready for review March 30, 2026 18:09
# Password or personal access token used to log against the Docker registry
password: ${{ secrets.DOCKER_PASSWORD}}
# Log out from the Docker registry at the end of a job
logout: true
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We lose logout at the end of the job, but I think this is moot. The runner is discarded after each matrix run.

@aarongable aarongable requested a review from jsha April 2, 2026 21:13
@inahga inahga requested a review from aarongable April 6, 2026 18:36
jsha
jsha previously approved these changes Apr 6, 2026
Copy link
Copy Markdown
Contributor

@aarongable aarongable left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, with the caveat that I'd like to see some sort of indication (e.g. in the PR description) that this is an accepted best-practice way to do this. GitHub Action supply chain dependency attacks are very real. They also protect us from very real accidental errors, like getting the difference between "$DOCKER_PASSWORD" and ${DOCKER_PASSWORD} wrong.

@aarongable aarongable requested a review from jsha April 14, 2026 20:23
@inahga
Copy link
Copy Markdown
Contributor Author

inahga commented Apr 14, 2026

Note that zizmor flags both the main branch and this PR for:

warning[secrets-outside-env]: secrets referenced without a dedicated environment
  --> .github/workflows/boulder-ci.yml:84:25
   |
28 |   b:
   |   - this job
...
84 |           username: ${{ secrets.DOCKER_USERNAME}}
   |                         ^^^^^^^^^^^^^^^^^^^^^^^ secret is accessed outside of a dedicated environment
   |
   = note: audit confidence → High

My interpretation of this and https://docs.zizmor.sh/audits/#secrets-outside-env, is that it is currently benign. The belt-and-suspenders fix would be to scope the token to an environment and configure environment protection rules. But since the token is still not available to forks, I think we're fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants