Skip to content

Commit

Permalink
Add check rule that looks at keynames in arg and env and checks for c…
Browse files Browse the repository at this point in the history
…ommon secret names

Signed-off-by: Talon Bowler <[email protected]>
  • Loading branch information
daghack committed Jul 1, 2024
1 parent 2ec1338 commit 81c05a9
Show file tree
Hide file tree
Showing 3 changed files with 110 additions and 0 deletions.
29 changes: 29 additions & 0 deletions frontend/dockerfile/dockerfile2llb/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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) {
Expand Down
74 changes: 74 additions & 0 deletions frontend/dockerfile/dockerfile_lint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,82 @@ var lintTests = integration.TestFuncs(
testBaseImagePlatformMismatch,
testAllTargetUnmarshal,
testRedundantTargetPlatform,
testSecretsUsedInArgOrEnv,
)

func testSecretsUsedInArgOrEnv(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: "SecretsUsedInArgOrEnv",
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: "SecretsUsedInArgOrEnv",
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: "SecretsUsedInArgOrEnv",
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: "SecretsUsedInArgOrEnv",
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: "SecretsUsedInArgOrEnv",
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: "SecretsUsedInArgOrEnv",
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: "SecretsUsedInArgOrEnv",
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: "SecretsUsedInArgOrEnv",
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
Expand Down
7 changes: 7 additions & 0 deletions frontend/dockerfile/linter/ruleset.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
},
}
)

0 comments on commit 81c05a9

Please sign in to comment.