From 976eb43d05a24581f234f46efa83d0929c7b6e06 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Mon, 22 Dec 2025 12:16:05 +0000 Subject: [PATCH] Security fix: Remove secret key names from error messages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Removed key parameter from validateSecretsExpression error message - Updated all tests to verify key names are NOT in error messages - Prevents exposure of security infrastructure details via JSON logging Fixes CodeQL Alert #71 (go/clear-text-logging) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 --- pkg/workflow/secrets_validation.go | 6 +++- pkg/workflow/secrets_validation_test.go | 42 ++++++++++++++----------- 2 files changed, 29 insertions(+), 19 deletions(-) diff --git a/pkg/workflow/secrets_validation.go b/pkg/workflow/secrets_validation.go index 28ebdc4116..d098510040 100644 --- a/pkg/workflow/secrets_validation.go +++ b/pkg/workflow/secrets_validation.go @@ -19,7 +19,11 @@ var secretsExpressionPattern = regexp.MustCompile(`^\$\{\{\s*secrets\.[A-Za-z_][ func validateSecretsExpression(key, value string) error { if !secretsExpressionPattern.MatchString(value) { secretsValidationLog.Printf("Invalid secret expression detected") - return fmt.Errorf("jobs.secrets.%s must be a GitHub Actions expression with secrets reference (e.g., '${{ secrets.MY_SECRET }}' or '${{ secrets.SECRET1 || secrets.SECRET2 }}')", key) + // Note: We intentionally do NOT include the key name in the error message to avoid + // logging sensitive information (secret key names) that could expose details about + // the organization's security infrastructure. The key name is available in the + // calling context for debugging purposes if needed. + return fmt.Errorf("invalid secrets expression: must be a GitHub Actions expression with secrets reference (e.g., '${{ secrets.MY_SECRET }}' or '${{ secrets.SECRET1 || secrets.SECRET2 }}')") } secretsValidationLog.Printf("Valid secret expression validated") return nil diff --git a/pkg/workflow/secrets_validation_test.go b/pkg/workflow/secrets_validation_test.go index b273fa6198..589a25515f 100644 --- a/pkg/workflow/secrets_validation_test.go +++ b/pkg/workflow/secrets_validation_test.go @@ -64,7 +64,7 @@ func TestSecretsExpressionPattern(t *testing.T) { } // TestValidateSecretsExpressionErrorMessages tests that error messages are descriptive -// but do NOT include sensitive values to prevent clear-text logging +// but do NOT include sensitive values OR KEY NAMES to prevent clear-text logging func TestValidateSecretsExpressionErrorMessages(t *testing.T) { tests := []struct { name string @@ -74,24 +74,25 @@ func TestValidateSecretsExpressionErrorMessages(t *testing.T) { notExpectedInErrs []string }{ { - name: "plaintext does NOT show value in error", + name: "plaintext does NOT show value OR key name in error", key: "token", value: "plaintext", - expectedInErrs: []string{"jobs.secrets.token"}, - notExpectedInErrs: []string{"plaintext"}, + expectedInErrs: []string{"invalid secrets expression", "must be a GitHub Actions expression"}, + notExpectedInErrs: []string{"plaintext", "token"}, }, { - name: "env context does NOT show value in error", + name: "env context does NOT show value OR key name in error", key: "api_key", value: "${{ env.TOKEN }}", - expectedInErrs: []string{"jobs.secrets.api_key"}, - notExpectedInErrs: []string{"${{ env.TOKEN }}"}, + expectedInErrs: []string{"invalid secrets expression"}, + notExpectedInErrs: []string{"${{ env.TOKEN }}", "api_key"}, }, { - name: "key name in error", - key: "database_password", - value: "hardcoded", - expectedInErrs: []string{"jobs.secrets.database_password"}, + name: "key name NOT in error (security fix)", + key: "database_password", + value: "hardcoded", + expectedInErrs: []string{"invalid secrets expression"}, + notExpectedInErrs: []string{"database_password"}, }, { name: "example format in error", @@ -106,11 +107,11 @@ func TestValidateSecretsExpressionErrorMessages(t *testing.T) { expectedInErrs: []string{"${{ secrets.SECRET1 || secrets.SECRET2 }}"}, }, { - name: "mixed context error does NOT show value", - key: "token", + name: "mixed context error does NOT show value OR key name", + key: "deploy_token", value: "${{ secrets.TOKEN || env.FALLBACK }}", - expectedInErrs: []string{"jobs.secrets.token"}, - notExpectedInErrs: []string{"${{ secrets.TOKEN || env.FALLBACK }}"}, + expectedInErrs: []string{"invalid secrets expression"}, + notExpectedInErrs: []string{"${{ secrets.TOKEN || env.FALLBACK }}", "deploy_token"}, }, } @@ -164,9 +165,14 @@ func TestValidateSecretsExpressionWithDifferentKeys(t *testing.T) { if err == nil { t.Errorf("Expected error for invalid value with key %q, got nil", key) } - // Error message should include the key name (if not empty) - if key != "" && !strings.Contains(err.Error(), "jobs.secrets."+key) { - t.Errorf("Expected error to contain key name %q, got: %s", key, err.Error()) + // Security fix: Error message should NOT include the key name to prevent + // logging sensitive information about the organization's security infrastructure + if key != "" && strings.Contains(err.Error(), key) { + t.Errorf("Error should NOT contain sensitive key name %q, but got: %s", key, err.Error()) + } + // Error should still be descriptive + if !strings.Contains(err.Error(), "invalid secrets expression") { + t.Errorf("Error should contain descriptive message, got: %s", err.Error()) } }) }