Merge pull request #5105 from daghack/secrets-used-in-args-rule

Lint Rule for catching common secret related env/arg keys

frontend/dockerfile/dockerfile2llb/convert.go

@@ -11,6 +11,7 @@ import (
 	"os"
 	"path"
 	"path/filepath"
+	"regexp"
 	"runtime"
 	"sort"
 	"strconv"
@@ -54,6 +55,11 @@ const (
 	sbomScanStage   = "BUILDKIT_SBOM_SCAN_STAGE"
 )
 
+var (
+	secretsRegexpOnce sync.Once
+	secretsRegexp     *regexp.Regexp
+)
+
 var nonEnvArgs = map[string]struct{}{
 	sbomScanContext: {},
 	sbomScanStage:   {},
@@ -1122,6 +1128,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("ENV", 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 +1707,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", arg.Key, c.Location(), opt.lint)
 		_, hasValue := opt.buildArgValues[arg.Key]
 		hasDefault := arg.Value != nil
 
@@ -2344,6 +2352,37 @@ func validateBaseImagePlatform(name string, expected, actual ocispecs.Platform,
 	}
 }
 
+func getSecretsRegex() *regexp.Regexp {
+	// Check for either full value or first/last word.
+	// Examples: api_key, DATABASE_PASSWORD, GITHUB_TOKEN, secret_MESSAGE, AUTH
+	// Case insensitive.
+	secretsRegexpOnce.Do(func() {
+		secretTokens := []string{
+			"apikey",
+			"auth",
+			"credential",
+			"credentials",
+			"key",
+			"password",
+			"pword",
+			"passwd",
+			"secret",
+			"token",
+		}
+		pattern := `(?i)(?:_|^)(?:` + strings.Join(secretTokens, "|") + `)(?:_|$)`
+		secretsRegexp = regexp.MustCompile(pattern)
+	})
+	return secretsRegexp
+}
+
+func validateNoSecretKey(instruction, key string, location []parser.Range, lint *linter.Linter) {
+	pattern := getSecretsRegex()
+	if pattern.MatchString(key) {
+		msg := linter.RuleSecretsUsedInArgOrEnv.Format(instruction, key)
+		lint.Run(&linter.RuleSecretsUsedInArgOrEnv, location, msg)
+	}
+}
+
 type emptyEnvs struct{}
 
 func (emptyEnvs) Get(string) (string, bool) {

frontend/dockerfile/dockerfile_lint_test.go

@@ -41,8 +41,90 @@ 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: "Sensitive data should not be used in the ARG or ENV commands",
+				Detail:      `Do not use ARG or ENV instructions for sensitive data (ARG "SECRET_PASSPHRASE")`,
+				URL:         "https://docs.docker.com/go/dockerfile/rule/secrets-used-in-arg-or-env/",
+				Level:       1,
+				Line:        3,
+			},
+			{
+				RuleName:    "SecretsUsedInArgOrEnv",
+				Description: "Sensitive data should not be used in the ARG or ENV commands",
+				Detail:      `Do not use ARG or ENV instructions for sensitive data (ENV "SUPER_Secret")`,
+				URL:         "https://docs.docker.com/go/dockerfile/rule/secrets-used-in-arg-or-env/",
+				Level:       1,
+				Line:        4,
+			},
+			{
+				RuleName:    "SecretsUsedInArgOrEnv",
+				Description: "Sensitive data should not be used in the ARG or ENV commands",
+				Detail:      `Do not use ARG or ENV instructions for sensitive data (ENV "password")`,
+				URL:         "https://docs.docker.com/go/dockerfile/rule/secrets-used-in-arg-or-env/",
+				Level:       1,
+				Line:        5,
+			},
+			{
+				RuleName:    "SecretsUsedInArgOrEnv",
+				Description: "Sensitive data should not be used in the ARG or ENV commands",
+				Detail:      `Do not use ARG or ENV instructions for sensitive data (ENV "secret")`,
+				URL:         "https://docs.docker.com/go/dockerfile/rule/secrets-used-in-arg-or-env/",
+				Level:       1,
+				Line:        5,
+			},
+			{
+				RuleName:    "SecretsUsedInArgOrEnv",
+				Description: "Sensitive data should not be used in the ARG or ENV commands",
+				Detail:      `Do not use ARG or ENV instructions for sensitive data (ARG "super_duper_secret_token")`,
+				URL:         "https://docs.docker.com/go/dockerfile/rule/secrets-used-in-arg-or-env/",
+				Level:       1,
+				Line:        6,
+			},
+			{
+				RuleName:    "SecretsUsedInArgOrEnv",
+				Description: "Sensitive data should not be used in the ARG or ENV commands",
+				Detail:      `Do not use ARG or ENV instructions for sensitive data (ARG "auth")`,
+				URL:         "https://docs.docker.com/go/dockerfile/rule/secrets-used-in-arg-or-env/",
+				Level:       1,
+				Line:        6,
+			},
+			{
+				RuleName:    "SecretsUsedInArgOrEnv",
+				Description: "Sensitive data should not be used in the ARG or ENV commands",
+				Detail:      `Do not use ARG or ENV instructions for sensitive data (ENV "apikey")`,
+				URL:         "https://docs.docker.com/go/dockerfile/rule/secrets-used-in-arg-or-env/",
+				Level:       1,
+				Line:        7,
+			},
+			{
+				RuleName:    "SecretsUsedInArgOrEnv",
+				Description: "Sensitive data should not be used in the ARG or ENV commands",
+				Detail:      `Do not use ARG or ENV instructions for sensitive data (ENV "git_key")`,
+				URL:         "https://docs.docker.com/go/dockerfile/rule/secrets-used-in-arg-or-env/",
+				Level:       1,
+				Line:        8,
+			},
+		},
+	})
+}
+
 func testAllTargetUnmarshal(t *testing.T, sb integration.Sandbox) {
 	dockerfile := []byte(`
 FROM scratch AS first
@@ -858,7 +940,7 @@ HEALTHCHECK CMD ["/myotherapp"]
 func testLegacyKeyValueFormat(t *testing.T, sb integration.Sandbox) {
 	dockerfile := []byte(`
 FROM scratch
-ENV key value
+ENV testkey value
 LABEL key value
 `)
 	checkLinterWarnings(t, sb, &lintTestParams{
@@ -885,7 +967,7 @@ LABEL key value
 
 	dockerfile = []byte(`
 FROM scratch
-ENV key=value
+ENV testkey=value
 LABEL key=value
 `)
 	checkLinterWarnings(t, sb, &lintTestParams{Dockerfile: dockerfile})
@@ -896,7 +978,7 @@ LABEL key=value
 FROM scratch AS a
 
 FROM a AS b
-ENV key value
+ENV testkey value
 LABEL key value
 
 FROM a AS c

frontend/dockerfile/docs/rules/_index.md

@@ -84,5 +84,9 @@ $ docker build --check .
       <td><a href="./redundant-target-platform/">RedundantTargetPlatform</a></td>
       <td>Setting platform to predefined $TARGETPLATFORM in FROM is redundant as this is the default behavior</td>
     </tr>
+    <tr>
+      <td><a href="./secrets-used-in-arg-or-env/">SecretsUsedInArgOrEnv</a></td>
+      <td>Sensitive data should not be used in the ARG or ENV commands</td>
+    </tr>
   </tbody>
 </table>

frontend/dockerfile/docs/rules/secrets-used-in-arg-or-env.md

@@ -0,0 +1,36 @@
+---
+title: SecretsUsedInArgOrEnv
+description: Sensitive data should not be used in the ARG or ENV commands
+aliases:
+  - /go/dockerfile/rule/secrets-used-in-arg-or-env/
+---
+
+## Output
+
+```text
+Potentially sensitive data should not be used in the ARG or ENV commands
+```
+
+## Description
+
+While it is common to pass secrets to running processes
+through environment variables during local development,
+setting secrets in a Dockerfile using `ENV` or `ARG`
+is insecure because they persist in the final image.
+This rule reports violations where `ENV` and `ARG` keys
+indicate that they contain sensitive data.
+
+Instead of `ARG` or `ENV`, you should use secret mounts,
+which expose secrets to your builds in a secure manner,
+and do not persist in the final image or its metadata.
+See [Build secrets](https://docs.docker.com/build/building/secrets/).
+
+## Examples
+
+❌ Bad: `AWS_SECRET_ACCESS_KEY` is a secret value.
+
+```dockerfile
+FROM scratch
+ARG AWS_SECRET_ACCESS_KEY
+```
+

frontend/dockerfile/linter/docs/SecretsUsedInArgOrEnv.md

@@ -0,0 +1,28 @@
+## Output
+
+```text
+Potentially sensitive data should not be used in the ARG or ENV commands
+```
+
+## Description
+
+While it is common to pass secrets to running processes
+through environment variables during local development,
+setting secrets in a Dockerfile using `ENV` or `ARG`
+is insecure because they persist in the final image.
+This rule reports violations where `ENV` and `ARG` keys
+indicate that they contain sensitive data.
+
+Instead of `ARG` or `ENV`, you should use secret mounts,
+which expose secrets to your builds in a secure manner,
+and do not persist in the final image or its metadata.
+See [Build secrets](https://docs.docker.com/build/building/secrets/).
+
+## Examples
+
+❌ Bad: `AWS_SECRET_ACCESS_KEY` is a secret value.
+
+```dockerfile
+FROM scratch
+ARG AWS_SECRET_ACCESS_KEY
+```

frontend/dockerfile/linter/ruleset.go

@@ -132,4 +132,12 @@ 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) string]{
+		Name:        "SecretsUsedInArgOrEnv",
+		Description: "Sensitive data should not be used in the ARG or ENV commands",
+		URL:         "https://docs.docker.com/go/dockerfile/rule/secrets-used-in-arg-or-env/",
+		Format: func(instruction, secretKey string) string {
+			return fmt.Sprintf("Do not use ARG or ENV instructions for sensitive data (%s %q)", instruction, secretKey)
+		},
+	}
 )