Skip to content

Commit 6e04857

Browse files
committed
Add check rule that looks at keynames in arg and env and checks for common secret names
Signed-off-by: Talon Bowler <[email protected]>
1 parent 2ec1338 commit 6e04857

File tree

3 files changed

+113
-3
lines changed

3 files changed

+113
-3
lines changed

frontend/dockerfile/dockerfile2llb/convert.go

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1122,6 +1122,7 @@ func dispatchEnv(d *dispatchState, c *instructions.EnvCommand, lint *linter.Lint
11221122
msg := linter.RuleLegacyKeyValueFormat.Format(c.Name())
11231123
lint.Run(&linter.RuleLegacyKeyValueFormat, c.Location(), msg)
11241124
}
1125+
validateNoSecretKey(e.Key, c.Location(), lint)
11251126
commitMessage.WriteString(" " + e.String())
11261127
d.state = d.state.AddEnv(e.Key, e.Value)
11271128
d.image.Config.Env = addEnv(d.image.Config.Env, e.Key, e.Value)
@@ -1700,6 +1701,7 @@ func dispatchShell(d *dispatchState, c *instructions.ShellCommand) error {
17001701
func dispatchArg(d *dispatchState, c *instructions.ArgCommand, opt *dispatchOpt) error {
17011702
commitStrs := make([]string, 0, len(c.Args))
17021703
for _, arg := range c.Args {
1704+
validateNoSecretKey(arg.Key, c.Location(), opt.lint)
17031705
_, hasValue := opt.buildArgValues[arg.Key]
17041706
hasDefault := arg.Value != nil
17051707

@@ -2344,6 +2346,33 @@ func validateBaseImagePlatform(name string, expected, actual ocispecs.Platform,
23442346
}
23452347
}
23462348

2349+
func validateNoSecretKey(key string, location []parser.Range, lint *linter.Linter) {
2350+
// Check for either full value or first/last word.
2351+
// Examples: api_key, DATABASE_PASSWORD, GITHUB_TOKEN, secret_MESSAGE, AUTH
2352+
// Case insensitive.
2353+
secretTokens := []string{
2354+
"apikey",
2355+
"auth",
2356+
"credential",
2357+
"credentials",
2358+
"key",
2359+
"password",
2360+
"pword",
2361+
"passwd",
2362+
"secret",
2363+
"token",
2364+
}
2365+
2366+
keyWords := strings.Split(strings.ToLower(key), "_")
2367+
for _, token := range secretTokens {
2368+
if token == keyWords[0] || token == keyWords[len(keyWords)-1] {
2369+
msg := linter.RuleSecretsUsedInArgOrEnv.Format(key)
2370+
lint.Run(&linter.RuleSecretsUsedInArgOrEnv, location, msg)
2371+
return
2372+
}
2373+
}
2374+
}
2375+
23472376
type emptyEnvs struct{}
23482377

23492378
func (emptyEnvs) Get(string) (string, bool) {

frontend/dockerfile/dockerfile_lint_test.go

Lines changed: 77 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,82 @@ var lintTests = integration.TestFuncs(
4141
testBaseImagePlatformMismatch,
4242
testAllTargetUnmarshal,
4343
testRedundantTargetPlatform,
44+
testSecretsUsedInArgOrEnv,
4445
)
4546

47+
func testSecretsUsedInArgOrEnv(t *testing.T, sb integration.Sandbox) {
48+
dockerfile := []byte(`
49+
FROM scratch
50+
ARG SECRET_PASSPHRASE
51+
ENV SUPER_Secret=foo
52+
ENV password=bar secret=baz
53+
ARG super_duper_secret_token=foo auth=bar
54+
ENV apikey=bar sunflower=foo
55+
ENV git_key=
56+
`)
57+
checkLinterWarnings(t, sb, &lintTestParams{
58+
Dockerfile: dockerfile,
59+
Warnings: []expectedLintWarning{
60+
{
61+
RuleName: "SecretsUsedInArgOrEnv",
62+
Description: "Potentially sensitive data should not be used in the ARG or ENV commands",
63+
Detail: `Secrets should not be used in the ARG or ENV commands (key named "SECRET_PASSPHRASE")`,
64+
Level: 1,
65+
Line: 3,
66+
},
67+
{
68+
RuleName: "SecretsUsedInArgOrEnv",
69+
Description: "Potentially sensitive data should not be used in the ARG or ENV commands",
70+
Detail: `Secrets should not be used in the ARG or ENV commands (key named "SUPER_Secret")`,
71+
Level: 1,
72+
Line: 4,
73+
},
74+
{
75+
RuleName: "SecretsUsedInArgOrEnv",
76+
Description: "Potentially sensitive data should not be used in the ARG or ENV commands",
77+
Detail: `Secrets should not be used in the ARG or ENV commands (key named "password")`,
78+
Level: 1,
79+
Line: 5,
80+
},
81+
{
82+
RuleName: "SecretsUsedInArgOrEnv",
83+
Description: "Potentially sensitive data should not be used in the ARG or ENV commands",
84+
Detail: `Secrets should not be used in the ARG or ENV commands (key named "secret")`,
85+
Level: 1,
86+
Line: 5,
87+
},
88+
{
89+
RuleName: "SecretsUsedInArgOrEnv",
90+
Description: "Potentially sensitive data should not be used in the ARG or ENV commands",
91+
Detail: `Secrets should not be used in the ARG or ENV commands (key named "super_duper_secret_token")`,
92+
Level: 1,
93+
Line: 6,
94+
},
95+
{
96+
RuleName: "SecretsUsedInArgOrEnv",
97+
Description: "Potentially sensitive data should not be used in the ARG or ENV commands",
98+
Detail: `Secrets should not be used in the ARG or ENV commands (key named "auth")`,
99+
Level: 1,
100+
Line: 6,
101+
},
102+
{
103+
RuleName: "SecretsUsedInArgOrEnv",
104+
Description: "Potentially sensitive data should not be used in the ARG or ENV commands",
105+
Detail: `Secrets should not be used in the ARG or ENV commands (key named "apikey")`,
106+
Level: 1,
107+
Line: 7,
108+
},
109+
{
110+
RuleName: "SecretsUsedInArgOrEnv",
111+
Description: "Potentially sensitive data should not be used in the ARG or ENV commands",
112+
Detail: `Secrets should not be used in the ARG or ENV commands (key named "git_key")`,
113+
Level: 1,
114+
Line: 8,
115+
},
116+
},
117+
})
118+
}
119+
46120
func testAllTargetUnmarshal(t *testing.T, sb integration.Sandbox) {
47121
dockerfile := []byte(`
48122
FROM scratch AS first
@@ -858,7 +932,7 @@ HEALTHCHECK CMD ["/myotherapp"]
858932
func testLegacyKeyValueFormat(t *testing.T, sb integration.Sandbox) {
859933
dockerfile := []byte(`
860934
FROM scratch
861-
ENV key value
935+
ENV testkey value
862936
LABEL key value
863937
`)
864938
checkLinterWarnings(t, sb, &lintTestParams{
@@ -885,7 +959,7 @@ LABEL key value
885959

886960
dockerfile = []byte(`
887961
FROM scratch
888-
ENV key=value
962+
ENV testkey=value
889963
LABEL key=value
890964
`)
891965
checkLinterWarnings(t, sb, &lintTestParams{Dockerfile: dockerfile})
@@ -896,7 +970,7 @@ LABEL key=value
896970
FROM scratch AS a
897971
898972
FROM a AS b
899-
ENV key value
973+
ENV testkey value
900974
LABEL key value
901975
902976
FROM a AS c

frontend/dockerfile/linter/ruleset.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,4 +132,11 @@ var (
132132
return fmt.Sprintf("Setting platform to predefined %s in FROM is redundant as this is the default behavior", platformVar)
133133
},
134134
}
135+
RuleSecretsUsedInArgOrEnv = LinterRule[func(string) string]{
136+
Name: "SecretsUsedInArgOrEnv",
137+
Description: "Potentially sensitive data should not be used in the ARG or ENV commands",
138+
Format: func(secretKey string) string {
139+
return fmt.Sprintf("Secrets should not be used in the ARG or ENV commands (key named %q)", secretKey)
140+
},
141+
}
135142
)

0 commit comments

Comments
 (0)