diff --git a/actions/setup/js/add_labels.cjs b/actions/setup/js/add_labels.cjs index 455330b70a..a28b41c57c 100644 --- a/actions/setup/js/add_labels.cjs +++ b/actions/setup/js/add_labels.cjs @@ -28,6 +28,7 @@ const MAX_LABELS = 10; async function main(config = {}) { // Extract configuration const allowedLabels = config.allowed || []; + const blockedPatterns = config.blocked || []; const maxCount = config.max || 10; const { defaultTargetRepo, allowedRepos } = resolveTargetRepoConfig(config); @@ -38,6 +39,9 @@ async function main(config = {}) { if (allowedLabels.length > 0) { core.info(`Allowed labels: ${allowedLabels.join(", ")}`); } + if (blockedPatterns.length > 0) { + core.info(`Blocked patterns: ${blockedPatterns.join(", ")}`); + } core.info(`Default target repo: ${defaultTargetRepo}`); if (allowedRepos.size > 0) { core.info(`Allowed repos: ${[...allowedRepos].join(", ")}`); @@ -105,7 +109,7 @@ async function main(config = {}) { } // Use validation helper to sanitize and validate labels - const labelsResult = validateLabels(requestedLabels, allowedLabels, maxCount); + const labelsResult = validateLabels(requestedLabels, allowedLabels, maxCount, blockedPatterns); if (!labelsResult.valid) { // If no valid labels, log info and return gracefully if (labelsResult.error?.includes("No valid labels")) { diff --git a/actions/setup/js/remove_labels.cjs b/actions/setup/js/remove_labels.cjs index 531f9ec028..26515a5630 100644 --- a/actions/setup/js/remove_labels.cjs +++ b/actions/setup/js/remove_labels.cjs @@ -20,6 +20,7 @@ const { resolveTargetRepoConfig, resolveAndValidateRepo } = require("./repo_help async function main(config = {}) { // Extract configuration const allowedLabels = config.allowed || []; + const blockedPatterns = config.blocked || []; const maxCount = config.max || 10; const { defaultTargetRepo, allowedRepos } = resolveTargetRepoConfig(config); @@ -30,6 +31,9 @@ async function main(config = {}) { if (allowedLabels.length > 0) { core.info(`Allowed labels to remove: ${allowedLabels.join(", ")}`); } + if (blockedPatterns.length > 0) { + core.info(`Blocked patterns: ${blockedPatterns.join(", ")}`); + } core.info(`Default target repo: ${defaultTargetRepo}`); if (allowedRepos.size > 0) { core.info(`Allowed repos: ${Array.from(allowedRepos).join(", ")}`); @@ -100,7 +104,7 @@ async function main(config = {}) { } // Use validation helper to sanitize and validate labels - const labelsResult = validateLabels(requestedLabels, allowedLabels, maxCount); + const labelsResult = validateLabels(requestedLabels, allowedLabels, maxCount, blockedPatterns); if (!labelsResult.valid) { // If no valid labels, log info and return gracefully if (labelsResult.error?.includes("No valid labels")) { diff --git a/actions/setup/js/safe_output_validator.cjs b/actions/setup/js/safe_output_validator.cjs index 3294a80169..650d23f36f 100644 --- a/actions/setup/js/safe_output_validator.cjs +++ b/actions/setup/js/safe_output_validator.cjs @@ -84,9 +84,10 @@ function validateBody(body, fieldName = "body", required = false) { * @param {any} labels - The labels to validate * @param {string[]|undefined} allowedLabels - Optional list of allowed labels * @param {number} maxCount - Maximum number of labels allowed + * @param {string[]|undefined} blockedPatterns - Optional list of blocked label patterns (supports glob patterns like "~*", "*[bot]") * @returns {{valid: boolean, value?: string[], error?: string}} Validation result */ -function validateLabels(labels, allowedLabels = undefined, maxCount = 3) { +function validateLabels(labels, allowedLabels = undefined, maxCount = 3, blockedPatterns = undefined) { if (!labels || !Array.isArray(labels)) { return { valid: false, error: "labels must be an array" }; } @@ -98,10 +99,27 @@ function validateLabels(labels, allowedLabels = undefined, maxCount = 3) { } } - // Filter labels based on allowed list if provided + // Filter out blocked labels first (security boundary) let validLabels = labels; + if (blockedPatterns && blockedPatterns.length > 0) { + const { matchesSimpleGlob } = require("./glob_pattern_helpers.cjs"); + const blockedLabels = []; + validLabels = labels.filter(label => { + const labelStr = String(label).trim(); + const isBlocked = blockedPatterns.some(pattern => matchesSimpleGlob(labelStr, pattern)); + if (isBlocked) { + blockedLabels.push(labelStr); + } + return !isBlocked; + }); + if (blockedLabels.length > 0) { + core.info(`Filtered out ${blockedLabels.length} blocked labels: ${blockedLabels.join(", ")}`); + } + } + + // Filter labels based on allowed list if provided if (allowedLabels && allowedLabels.length > 0) { - validLabels = labels.filter(label => allowedLabels.includes(label)); + validLabels = validLabels.filter(label => allowedLabels.includes(label)); } // Sanitize and deduplicate labels diff --git a/actions/setup/js/safe_output_validator.test.cjs b/actions/setup/js/safe_output_validator.test.cjs index 57761abe0a..013637dbfe 100644 --- a/actions/setup/js/safe_output_validator.test.cjs +++ b/actions/setup/js/safe_output_validator.test.cjs @@ -228,6 +228,85 @@ describe("safe_output_validator.cjs", () => { expect(result.valid).toBe(false); expect(result.error).toContain("No valid labels found"); }); + + it("should filter out labels matching blocked patterns", () => { + const result = validator.validateLabels(["bug", "~stale", "~archived", "enhancement"], undefined, 10, ["~*"]); + + expect(result.valid).toBe(true); + expect(result.value).toHaveLength(2); + expect(result.value).toContain("bug"); + expect(result.value).toContain("enhancement"); + expect(result.value).not.toContain("~stale"); + expect(result.value).not.toContain("~archived"); + expect(mockCore.info).toHaveBeenCalledWith(expect.stringContaining("Filtered out 2 blocked labels")); + }); + + it("should apply blocked filter before allowed filter", () => { + const result = validator.validateLabels( + ["bug", "~stale", "enhancement", "custom"], + ["bug", "~stale", "enhancement"], // ~stale is in allowed list + 10, + ["~*"] // but blocked by pattern + ); + + expect(result.valid).toBe(true); + expect(result.value).toHaveLength(2); + expect(result.value).toContain("bug"); + expect(result.value).toContain("enhancement"); + expect(result.value).not.toContain("~stale"); // blocked despite being in allowed list + expect(result.value).not.toContain("custom"); + }); + + it("should handle exact match blocking", () => { + const result = validator.validateLabels( + ["bug", "stale", "enhancement"], + undefined, + 10, + ["stale"] // exact match, no pattern + ); + + expect(result.valid).toBe(true); + expect(result.value).toHaveLength(2); + expect(result.value).toContain("bug"); + expect(result.value).toContain("enhancement"); + expect(result.value).not.toContain("stale"); + }); + + it("should handle empty blocked patterns", () => { + const result = validator.validateLabels(["bug", "~stale"], undefined, 10, []); + + expect(result.valid).toBe(true); + expect(result.value).toHaveLength(2); + expect(result.value).toContain("bug"); + expect(result.value).toContain("~stale"); + }); + + it("should handle undefined blocked patterns", () => { + const result = validator.validateLabels(["bug", "~stale"], undefined, 10, undefined); + + expect(result.valid).toBe(true); + expect(result.value).toHaveLength(2); + expect(result.value).toContain("bug"); + expect(result.value).toContain("~stale"); + }); + + it("should reject when all labels are blocked", () => { + const result = validator.validateLabels(["~stale", "~archived"], undefined, 10, ["~*"]); + + expect(result.valid).toBe(false); + expect(result.error).toContain("No valid labels found"); + }); + + it("should handle bot pattern blocking", () => { + const result = validator.validateLabels(["bug", "dependabot[bot]", "github-actions[bot]", "enhancement"], undefined, 10, ["*[bot]"]); + + expect(result.valid).toBe(true); + expect(result.value).toHaveLength(2); + expect(result.value).toContain("bug"); + expect(result.value).toContain("enhancement"); + expect(result.value).not.toContain("dependabot[bot]"); + expect(result.value).not.toContain("github-actions[bot]"); + }); }); describe("validateMaxCount", () => { diff --git a/pkg/parser/schemas/main_workflow_schema.json b/pkg/parser/schemas/main_workflow_schema.json index 3f50865abb..0bd5b23a0b 100644 --- a/pkg/parser/schemas/main_workflow_schema.json +++ b/pkg/parser/schemas/main_workflow_schema.json @@ -5232,6 +5232,15 @@ "minItems": 1, "maxItems": 50 }, + "blocked": { + "type": "array", + "description": "Optional list of blocked label patterns (supports glob patterns like '~*', '*[bot]'). Labels matching these patterns will be rejected. Applied before allowed list filtering for security.", + "items": { + "type": "string" + }, + "minItems": 1, + "maxItems": 50 + }, "max": { "type": "integer", "description": "Optional maximum number of labels to add (default: 3)", @@ -5281,6 +5290,15 @@ "minItems": 1, "maxItems": 50 }, + "blocked": { + "type": "array", + "description": "Optional list of blocked label patterns (supports glob patterns like '~*', '*[bot]'). Labels matching these patterns will be rejected. Applied before allowed list filtering for security.", + "items": { + "type": "string" + }, + "minItems": 1, + "maxItems": 50 + }, "max": { "type": "integer", "description": "Optional maximum number of labels to remove (default: 3)", diff --git a/pkg/workflow/add_labels.go b/pkg/workflow/add_labels.go index a601dc08d8..825f018893 100644 --- a/pkg/workflow/add_labels.go +++ b/pkg/workflow/add_labels.go @@ -13,6 +13,7 @@ type AddLabelsConfig struct { BaseSafeOutputConfig `yaml:",inline"` SafeOutputTargetConfig `yaml:",inline"` Allowed []string `yaml:"allowed,omitempty"` // Optional list of allowed labels. Labels will be created if they don't already exist in the repository. If omitted, any labels are allowed (including creating new ones). + Blocked []string `yaml:"blocked,omitempty"` // Optional list of blocked label patterns (supports glob patterns like "~*", "*[bot]"). Labels matching these patterns will be rejected. } // parseAddLabelsConfig handles add-labels configuration @@ -33,7 +34,7 @@ func (c *Compiler) parseAddLabelsConfig(outputMap map[string]any) *AddLabelsConf return &AddLabelsConfig{} } - addLabelsLog.Printf("Parsed configuration: allowed_count=%d, target=%s", len(config.Allowed), config.Target) + addLabelsLog.Printf("Parsed configuration: allowed_count=%d, blocked_count=%d, target=%s", len(config.Allowed), len(config.Blocked), config.Target) return &config } @@ -52,6 +53,7 @@ func (c *Compiler) buildAddLabelsJob(data *WorkflowData, mainJobName string) (*J listJobConfig := ListJobConfig{ SafeOutputTargetConfig: cfg.SafeOutputTargetConfig, Allowed: cfg.Allowed, + Blocked: cfg.Blocked, } // Use shared builder for list-based safe-output jobs diff --git a/pkg/workflow/compile_outputs_label_test.go b/pkg/workflow/compile_outputs_label_test.go index 6a6494dd2b..7864a65292 100644 --- a/pkg/workflow/compile_outputs_label_test.go +++ b/pkg/workflow/compile_outputs_label_test.go @@ -751,3 +751,164 @@ This workflow tests that missing allowed field is now optional. t.Fatal("Expected lock file to be created") } } + +func TestOutputLabelBlockedPatternsConfig(t *testing.T) { + // Create temporary directory for test files + tmpDir := testutil.TempDir(t, "output-label-blocked-test") + + // Test case with blocked patterns configuration + testContent := `--- +on: + issues: + types: [opened] +permissions: + contents: read + issues: write + pull-requests: write +engine: claude +features: + dangerous-permissions-write: true +strict: false +safe-outputs: + add-labels: + blocked: ["~*", "*\\**"] + allowed: [triage, bug, enhancement, needs-review] +--- + +# Test Blocked Label Patterns + +This workflow tests blocked label pattern filtering. +` + + testFile := filepath.Join(tmpDir, "test-blocked-labels.md") + if err := os.WriteFile(testFile, []byte(testContent), 0644); err != nil { + t.Fatal(err) + } + + compiler := NewCompiler() + + // Parse the workflow data + workflowData, err := compiler.ParseWorkflowFile(testFile) + if err != nil { + t.Fatalf("Unexpected error parsing workflow with blocked labels config: %v", err) + } + + // Verify output configuration is parsed correctly + if workflowData.SafeOutputs == nil { + t.Fatal("Expected output configuration to be parsed") + } + + if workflowData.SafeOutputs.AddLabels == nil { + t.Fatal("Expected labels configuration to be parsed") + } + + // Verify blocked patterns + expectedBlocked := []string{"~*", "*\\**"} + if len(workflowData.SafeOutputs.AddLabels.Blocked) != len(expectedBlocked) { + t.Errorf("Expected %d blocked patterns, got %d", len(expectedBlocked), len(workflowData.SafeOutputs.AddLabels.Blocked)) + } + + for i, expectedPattern := range expectedBlocked { + if i >= len(workflowData.SafeOutputs.AddLabels.Blocked) || workflowData.SafeOutputs.AddLabels.Blocked[i] != expectedPattern { + t.Errorf("Expected blocked[%d] to be '%s', got '%s'", i, expectedPattern, workflowData.SafeOutputs.AddLabels.Blocked[i]) + } + } + + // Verify allowed labels + expectedLabels := []string{"triage", "bug", "enhancement", "needs-review"} + if len(workflowData.SafeOutputs.AddLabels.Allowed) != len(expectedLabels) { + t.Errorf("Expected %d allowed labels, got %d", len(expectedLabels), len(workflowData.SafeOutputs.AddLabels.Allowed)) + } + + // Compile to verify env vars are generated + if err := compiler.CompileWorkflow(testFile); err != nil { + t.Fatalf("Failed to compile workflow: %v", err) + } + + // Read the generated lock file + lockFile := stringutil.MarkdownToLockFile(testFile) + lockBytes, err := os.ReadFile(lockFile) + if err != nil { + t.Fatal(err) + } + lockContent := string(lockBytes) + + // Verify blocked patterns are in handler config + if !strings.Contains(lockContent, "GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG") { + t.Error("Expected handler config to be passed as environment variable") + } + if !strings.Contains(lockContent, `\"blocked\"`) { + t.Error("Expected blocked field in handler config") + } +} + +func TestOutputLabelRemoveBlockedPatternsConfig(t *testing.T) { + // Create temporary directory for test files + tmpDir := testutil.TempDir(t, "remove-label-blocked-test") + + // Test case with blocked patterns for remove-labels + testContent := `--- +on: + issues: + types: [opened] +permissions: + contents: read + issues: write + pull-requests: write +engine: claude +features: + dangerous-permissions-write: true +strict: false +safe-outputs: + remove-labels: + blocked: ["~*", "*\\**"] + max: 3 +--- + +# Test Remove Labels with Blocked Patterns + +This workflow tests blocked patterns for remove-labels. +` + + testFile := filepath.Join(tmpDir, "test-remove-blocked.md") + if err := os.WriteFile(testFile, []byte(testContent), 0644); err != nil { + t.Fatal(err) + } + + compiler := NewCompiler() + + // Parse the workflow data + workflowData, err := compiler.ParseWorkflowFile(testFile) + if err != nil { + t.Fatalf("Unexpected error parsing workflow: %v", err) + } + + // Verify output configuration + if workflowData.SafeOutputs == nil || workflowData.SafeOutputs.RemoveLabels == nil { + t.Fatal("Expected remove-labels configuration to be parsed") + } + + // Verify blocked patterns + expectedBlocked := []string{"~*", "*\\**"} + if len(workflowData.SafeOutputs.RemoveLabels.Blocked) != len(expectedBlocked) { + t.Errorf("Expected %d blocked patterns, got %d", len(expectedBlocked), len(workflowData.SafeOutputs.RemoveLabels.Blocked)) + } + + // Compile to verify env vars + if err := compiler.CompileWorkflow(testFile); err != nil { + t.Fatalf("Failed to compile workflow: %v", err) + } + + // Read lock file + lockFile := stringutil.MarkdownToLockFile(testFile) + lockBytes, err := os.ReadFile(lockFile) + if err != nil { + t.Fatal(err) + } + lockContent := string(lockBytes) + + // Verify blocked patterns in handler config + if !strings.Contains(lockContent, `\"blocked\"`) { + t.Error("Expected blocked field in remove-labels handler config") + } +} diff --git a/pkg/workflow/compiler_safe_outputs_config.go b/pkg/workflow/compiler_safe_outputs_config.go index cc9d1a4434..fab5b9f632 100644 --- a/pkg/workflow/compiler_safe_outputs_config.go +++ b/pkg/workflow/compiler_safe_outputs_config.go @@ -212,6 +212,7 @@ var handlerRegistry = map[string]handlerBuilder{ config := newHandlerConfigBuilder(). AddIfPositive("max", c.Max). AddStringSlice("allowed", c.Allowed). + AddStringSlice("blocked", c.Blocked). AddIfNotEmpty("target", c.Target). AddIfNotEmpty("target-repo", c.TargetRepoSlug). AddStringSlice("allowed_repos", c.AllowedRepos). @@ -233,6 +234,7 @@ var handlerRegistry = map[string]handlerBuilder{ return newHandlerConfigBuilder(). AddIfPositive("max", c.Max). AddStringSlice("allowed", c.Allowed). + AddStringSlice("blocked", c.Blocked). AddIfNotEmpty("target", c.Target). AddIfNotEmpty("target-repo", c.TargetRepoSlug). AddStringSlice("allowed_repos", c.AllowedRepos). diff --git a/pkg/workflow/remove_labels.go b/pkg/workflow/remove_labels.go index 03611cda57..c4bbdb42f5 100644 --- a/pkg/workflow/remove_labels.go +++ b/pkg/workflow/remove_labels.go @@ -11,6 +11,7 @@ type RemoveLabelsConfig struct { BaseSafeOutputConfig `yaml:",inline"` SafeOutputTargetConfig `yaml:",inline"` Allowed []string `yaml:"allowed,omitempty"` // Optional list of allowed labels to remove. If omitted, any labels can be removed. + Blocked []string `yaml:"blocked,omitempty"` // Optional list of blocked label patterns (supports glob patterns like "~*", "*[bot]"). Labels matching these patterns will be rejected. } // parseRemoveLabelsConfig handles remove-labels configuration @@ -31,7 +32,7 @@ func (c *Compiler) parseRemoveLabelsConfig(outputMap map[string]any) *RemoveLabe return &RemoveLabelsConfig{} } - removeLabelsLog.Printf("Parsed configuration: allowed_count=%d, target=%s", len(config.Allowed), config.Target) + removeLabelsLog.Printf("Parsed configuration: allowed_count=%d, blocked_count=%d, target=%s", len(config.Allowed), len(config.Blocked), config.Target) return &config } diff --git a/pkg/workflow/safe_output_builder.go b/pkg/workflow/safe_output_builder.go index ad65e67c52..e8c78485b2 100644 --- a/pkg/workflow/safe_output_builder.go +++ b/pkg/workflow/safe_output_builder.go @@ -218,6 +218,7 @@ func BuildCloseJobEnvVars(prefix string, config CloseJobConfig) []string { type ListJobConfig struct { SafeOutputTargetConfig `yaml:",inline"` Allowed []string `yaml:"allowed,omitempty"` // Optional list of allowed values + Blocked []string `yaml:"blocked,omitempty"` // Optional list of blocked patterns (supports glob patterns) } // ParseListJobConfig parses common list job fields from a config map. @@ -247,6 +248,21 @@ func ParseListJobConfig(configMap map[string]any, allowedKey string) (ListJobCon } } + // Parse blocked list + if blocked, exists := configMap["blocked"]; exists { + // Handle single string format + if blockedStr, ok := blocked.(string); ok { + config.Blocked = []string{blockedStr} + } else if blockedArray, ok := blocked.([]any); ok { + // Handle array format + for _, item := range blockedArray { + if itemStr, ok := item.(string); ok { + config.Blocked = append(config.Blocked, itemStr) + } + } + } + } + return config, false } @@ -259,6 +275,9 @@ func BuildListJobEnvVars(prefix string, config ListJobConfig, maxCount int) []st // Add allowed list envVars = append(envVars, BuildAllowedListEnvVar(prefix+"_ALLOWED", config.Allowed)...) + // Add blocked list + envVars = append(envVars, BuildAllowedListEnvVar(prefix+"_BLOCKED", config.Blocked)...) + // Add max count envVars = append(envVars, BuildMaxCountEnvVar(prefix+"_MAX_COUNT", maxCount)...)