Skip to content

Fix regex logic for SecretsUsedInArgOrEnv to match comment and test #6089

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
+40 −1
Open

Fix regex logic for SecretsUsedInArgOrEnv to match comment and test #6089

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dcda7e3
Fix regex logic for SecretsUsedInArgOrEnv to match comment and test
Jul 16, 2025
ae3cbeb
Fix linting error on new test cases for getSecretsRegex
Jul 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion frontend/dockerfile/dockerfile2llb/convert.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2576,7 +2576,7 @@ func getSecretsRegex() (*regexp.Regexp, *regexp.Regexp) {
"secret",
"token",
}
pattern := `(?i)(?:_|^)(?:` + strings.Join(secretTokens, "|") + `)(?:_|$)`
pattern := `(?i)^(?:(` + strings.Join(secretTokens, "|") + `)(?:_.*)?|.*_(` + strings.Join(secretTokens, "|") + `))$`
secretsRegexp = regexp.MustCompile(pattern)

allowTokens := []string{
Expand Down
39 changes: 39 additions & 0 deletions frontend/dockerfile/dockerfile2llb/convert_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,3 +230,42 @@ RUN echo bar
assert.Equal(t, []digest.Digest{"sha256:2e112031b4b923a873c8b3d685d48037e4d5ccd967b658743d93a6e56c3064b9"}, baseImg.RootFS.DiffIDs)
assert.Equal(t, "2024-01-17 21:49:12 +0000 UTC", baseImg.Created.String())
}

func TestGetSecretsRegex(t *testing.T) {
t.Parallel()
deny, allow := getSecretsRegex()

testCases := []struct {
name string
key string
isSecret bool
}{
// Positive matches
Copy link
Member

@tonistiigi tonistiigi Jul 16, 2025

Choose a reason for hiding this comment

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

None of these tests seem to actually check for the change of behavior (handling secrets in the middle of string).

Copy link
Author

@markercm markercm Jul 17, 2025

Choose a reason for hiding this comment

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

{name: "contains key but not fitst of last or full", key: "new_key_file_path", isSecret: false},

This matches on the old behavior even though key is neither the first nor last nor full match but doesn't with the new logic.

{name: "exact api_key", key: "api_key", isSecret: true},
{name: "uppercase token", key: "GITHUB_TOKEN", isSecret: true},
{name: "contains password", key: "DATABASE_PASSWORD", isSecret: true},
{name: "contains secret", key: "secret_MESSAGE", isSecret: true},
{name: "exact auth", key: "AUTH", isSecret: true},
{name: "contains credential", key: "USER_CREDENTIAL", isSecret: true},
{name: "contains passwd", key: "DB_PASSWD", isSecret: true},
{name: "contains pword", key: "MY_PWORD", isSecret: true},

// Negative matches (allowed keywords)
{name: "public key", key: "public_key", isSecret: false},
{name: "ssh public key", key: "SSH_PUBLIC_KEY", isSecret: false},

// Negative matches (should not match)
{name: "normal variable", key: "myvar", isSecret: false},
{name: "contains key but not fitst of last or full", key: "new_key_file_path", isSecret: false},
{name: "contains auth but not as whole word", key: "authority", isSecret: false},
{name: "not a secret", key: "some_variable", isSecret: false},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
matched := deny.MatchString(tc.key) && !allow.MatchString(tc.key)
assert.Equal(t, tc.isSecret, matched)
})
}
}