Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lint Rule for catching common secret related env/arg keys #5105

Merged
merged 7 commits into from
Jul 3, 2024

Conversation

daghack
Copy link
Collaborator

@daghack daghack commented Jul 1, 2024

Adds a SecretsUsedInArgOrEnv lint rule check that checks for common secret-related prefixes or affixes in ENV and ARG keys.

Example Dockerfile

FROM alpine
ENV passwd=banana secret=passwd

Produces the following lint warnings.

WARNING: SecretsUsedInArgOrEnv
Secrets should not be used in the ARG or ENV commands (key named "passwd")
Dockerfile:2
--------------------
   1 |     FROM alpine
   2 | >>> ENV passwd=banana secret=passwd
   3 |
--------------------

WARNING: SecretsUsedInArgOrEnv
Secrets should not be used in the ARG or ENV commands (key named "secret")
Dockerfile:2
--------------------
   1 |     FROM alpine
   2 | >>> ENV passwd=banana secret=passwd
   3 |
-------------------

@daghack daghack marked this pull request as draft July 1, 2024 01:09
@daghack daghack self-assigned this Jul 1, 2024
@daghack daghack force-pushed the secrets-used-in-args-rule branch from bdc83e4 to 81c05a9 Compare July 1, 2024 01:19
@thompson-shaun thompson-shaun added this to the v0.15.0 milestone Jul 1, 2024
@daghack daghack force-pushed the secrets-used-in-args-rule branch from 81c05a9 to 53d8e75 Compare July 2, 2024 16:27
…ommon secret names

Signed-off-by: Talon Bowler <talon.bowler@docker.com>
@daghack daghack force-pushed the secrets-used-in-args-rule branch from 53d8e75 to 6e04857 Compare July 2, 2024 16:45
Signed-off-by: Talon Bowler <talon.bowler@docker.com>
@thompson-shaun
Copy link
Collaborator

/cc @dvdksn for some docs support 😄

@daghack daghack marked this pull request as ready for review July 2, 2024 18:14
@daghack daghack requested a review from tonistiigi July 2, 2024 18:14
@daghack daghack force-pushed the secrets-used-in-args-rule branch 4 times, most recently from 4ff538e to 0e524f7 Compare July 2, 2024 19:10
frontend/dockerfile/dockerfile2llb/convert.go Outdated Show resolved Hide resolved
passing secrets in as build arguments, via the `ARG` command, will similarly
expose the secret. This rule reports violations where `ENV` and `ARG` key names
appear to be secret-related.

Copy link
Member

Choose a reason for hiding this comment

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

This should explain that build secrets or SSH forwarding should be used instead to configure build with sensitive credentials in a secure way. Example can show build secret in action. We can link to https://docs.docker.com/build/building/secrets/ for extra reading.

cc @dvdksn

Signed-off-by: Talon Bowler <talon.bowler@docker.com>
@daghack daghack force-pushed the secrets-used-in-args-rule branch from 0e524f7 to 14e2cab Compare July 3, 2024 00:37
frontend/dockerfile/linter/ruleset.go Outdated Show resolved Hide resolved
frontend/dockerfile/linter/ruleset.go Outdated Show resolved Hide resolved
daghack and others added 3 commits July 3, 2024 09:18
Co-authored-by: David Karlsson <35727626+dvdksn@users.noreply.github.com>
Signed-off-by: Talon James Bowler <nolat301@gmail.com>
Co-authored-by: David Karlsson <35727626+dvdksn@users.noreply.github.com>
Signed-off-by: Talon James Bowler <nolat301@gmail.com>
Signed-off-by: Talon Bowler <talon.bowler@docker.com>
…v rule description

Signed-off-by: Talon Bowler <talon.bowler@docker.com>
@tonistiigi tonistiigi merged commit 71275f9 into moby:master Jul 3, 2024
75 checks passed
@daghack daghack deleted the secrets-used-in-args-rule branch July 3, 2024 18:36
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.

4 participants