From bdc83e481837d7f1e4be618c3a33d28e86183a98 Mon Sep 17 00:00:00 2001 From: Talon Bowler Date: Sun, 30 Jun 2024 17:59:13 -0700 Subject: [PATCH] Add check rule that looks at keynames in arg and env and checks for common secret names Signed-off-by: Talon Bowler --- frontend/dockerfile/dockerfile2llb/convert.go | 29 ++++++++ frontend/dockerfile/dockerfile_lint_test.go | 74 +++++++++++++++++++ frontend/dockerfile/linter/ruleset.go | 7 ++ 3 files changed, 110 insertions(+) diff --git a/frontend/dockerfile/dockerfile2llb/convert.go b/frontend/dockerfile/dockerfile2llb/convert.go index 4b56dc7cbc5b8..3b512f693962e 100644 --- a/frontend/dockerfile/dockerfile2llb/convert.go +++ b/frontend/dockerfile/dockerfile2llb/convert.go @@ -1122,6 +1122,7 @@ func dispatchEnv(d *dispatchState, c *instructions.EnvCommand, lint *linter.Lint msg := linter.RuleLegacyKeyValueFormat.Format(c.Name()) lint.Run(&linter.RuleLegacyKeyValueFormat, c.Location(), msg) } + validateNoSecretKey(e.Key, c.Location(), lint) commitMessage.WriteString(" " + e.String()) d.state = d.state.AddEnv(e.Key, e.Value) 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 { func dispatchArg(d *dispatchState, c *instructions.ArgCommand, opt *dispatchOpt) error { commitStrs := make([]string, 0, len(c.Args)) for _, arg := range c.Args { + validateNoSecretKey(arg.Key, c.Location(), opt.lint) _, hasValue := opt.buildArgValues[arg.Key] hasDefault := arg.Value != nil @@ -2344,6 +2346,33 @@ func validateBaseImagePlatform(name string, expected, actual ocispecs.Platform, } } +func validateNoSecretKey(key string, location []parser.Range, lint *linter.Linter) { + // Check for either full value or first/last word. + // Examples: api_key, DATABASE_PASSWORD, GITHUB_TOKEN, secret_MESSAGE, AUTH + // Case insensitive. + secretTokens := []string{ + "apikey", + "auth", + "credential", + "credentials", + "key", + "password", + "pword", + "passwd", + "secret", + "token", + } + + keyWords := strings.Split(strings.ToLower(key), "_") + for _, token := range secretTokens { + if token == keyWords[0] || token == keyWords[len(keyWords)-1] { + msg := linter.RuleSecretsUsedInArgOrEnv.Format(key) + lint.Run(&linter.RuleSecretsUsedInArgOrEnv, location, msg) + return + } + } +} + type emptyEnvs struct{} func (emptyEnvs) Get(string) (string, bool) { diff --git a/frontend/dockerfile/dockerfile_lint_test.go b/frontend/dockerfile/dockerfile_lint_test.go index 736a468b03bd1..1e9eb64b37815 100644 --- a/frontend/dockerfile/dockerfile_lint_test.go +++ b/frontend/dockerfile/dockerfile_lint_test.go @@ -41,8 +41,82 @@ var lintTests = integration.TestFuncs( testBaseImagePlatformMismatch, testAllTargetUnmarshal, testRedundantTargetPlatform, + testNoSecretArgEnv, ) +func testNoSecretArgEnv(t *testing.T, sb integration.Sandbox) { + dockerfile := []byte(` +FROM scratch +ARG SECRET_PASSPHRASE +ENV SUPER_Secret=foo +ENV password=bar secret=baz +ARG super_duper_secret_token=foo auth=bar +ENV apikey=bar sunflower=foo +ENV git_key= +`) + checkLinterWarnings(t, sb, &lintTestParams{ + Dockerfile: dockerfile, + Warnings: []expectedLintWarning{ + { + RuleName: "NoSecretArgEnv", + Description: "Secrets should not be set as ARG or ENV", + Detail: "Secrets should not be used in the ARG or ENV commands (key named 'SECRET_PASSPHRASE')", + Level: 1, + Line: 3, + }, + { + RuleName: "NoSecretArgEnv", + Description: "Secrets should not be set as ARG or ENV", + Detail: "Secrets should not be used in the ARG or ENV commands (key named 'SUPER_Secret')", + Level: 1, + Line: 4, + }, + { + RuleName: "NoSecretArgEnv", + Description: "Secrets should not be set as ARG or ENV", + Detail: "Secrets should not be used in the ARG or ENV commands (key named 'password')", + Level: 1, + Line: 5, + }, + { + RuleName: "NoSecretArgEnv", + Description: "Secrets should not be set as ARG or ENV", + Detail: "Secrets should not be used in the ARG or ENV commands (key named 'secret')", + Level: 1, + Line: 5, + }, + { + RuleName: "NoSecretArgEnv", + Description: "Secrets should not be set as ARG or ENV", + Detail: "Secrets should not be used in the ARG or ENV commands (key named 'super_duper_secret_token')", + Level: 1, + Line: 6, + }, + { + RuleName: "NoSecretArgEnv", + Description: "Secrets should not be set as ARG or ENV", + Detail: "Secrets should not be used in the ARG or ENV commands (key named 'auth')", + Level: 1, + Line: 6, + }, + { + RuleName: "NoSecretArgEnv", + Description: "Secrets should not be set as ARG or ENV", + Detail: "Secrets should not be used in the ARG or ENV commands (key named 'apikey')", + Level: 1, + Line: 7, + }, + { + RuleName: "NoSecretArgEnv", + Description: "Secrets should not be set as ARG or ENV", + Detail: "Secrets should not be used in the ARG or ENV commands (key named 'git_key')", + Level: 1, + Line: 8, + }, + }, + }) +} + func testAllTargetUnmarshal(t *testing.T, sb integration.Sandbox) { dockerfile := []byte(` FROM scratch AS first diff --git a/frontend/dockerfile/linter/ruleset.go b/frontend/dockerfile/linter/ruleset.go index 173dc434141a1..b2365e280a0f5 100644 --- a/frontend/dockerfile/linter/ruleset.go +++ b/frontend/dockerfile/linter/ruleset.go @@ -132,4 +132,11 @@ var ( return fmt.Sprintf("Setting platform to predefined %s in FROM is redundant as this is the default behavior", platformVar) }, } + RuleSecretsUsedInArgOrEnv = LinterRule[func(string) string]{ + Name: "SecretsUsedInArgOrEnv", + Description: "Secrets should not be used in ARG or ENV", + Format: func(secretKey string) string { + return fmt.Sprintf("Secrets should not be used in the ARG or ENV commands (key named %q)", secretKey) + }, + } )