diff --git a/actions/setup/js/add_labels.cjs b/actions/setup/js/add_labels.cjs index 455330b70a..aa3e430cd5 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, blockedPatterns, maxCount); 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/add_labels.test.cjs b/actions/setup/js/add_labels.test.cjs index 10a2251c06..0b7b6a4fa3 100644 --- a/actions/setup/js/add_labels.test.cjs +++ b/actions/setup/js/add_labels.test.cjs @@ -245,6 +245,74 @@ describe("add_labels", () => { expect(result.labelsAdded).toEqual(["bug", "enhancement"]); }); + it("should filter labels based on blocked patterns", async () => { + const handler = await main({ + blocked: ["~*", "\\**"], + max: 10, + }); + + const addLabelsCalls = []; + mockGithub.rest.issues.addLabels = async params => { + addLabelsCalls.push(params); + return {}; + }; + + const result = await handler( + { + item_number: 100, + labels: ["bug", "~triage", "*admin", "enhancement"], + }, + {} + ); + + expect(result.success).toBe(true); + expect(result.labelsAdded).toEqual(["bug", "enhancement"]); + }); + + it("should work with both allowed and blocked patterns", async () => { + const handler = await main({ + allowed: ["bug", "~triage", "enhancement"], + blocked: ["~*"], + max: 10, + }); + + const addLabelsCalls = []; + mockGithub.rest.issues.addLabels = async params => { + addLabelsCalls.push(params); + return {}; + }; + + const result = await handler( + { + item_number: 100, + labels: ["bug", "~triage", "custom", "enhancement"], + }, + {} + ); + + expect(result.success).toBe(true); + expect(result.labelsAdded).toEqual(["bug", "enhancement"]); + }); + + it("should handle all labels being blocked", async () => { + const handler = await main({ + blocked: ["~*"], + max: 10, + }); + + const result = await handler( + { + item_number: 100, + labels: ["~triage", "~workflow"], + }, + {} + ); + + expect(result.success).toBe(true); + expect(result.labelsAdded).toEqual([]); + expect(result.message).toContain("No valid labels found"); + }); + it("should handle empty labels array", async () => { const handler = await main({ max: 10 }); diff --git a/actions/setup/js/glob_pattern_helpers.cjs b/actions/setup/js/glob_pattern_helpers.cjs index 1bdfed3f36..4b4d5b6010 100644 --- a/actions/setup/js/glob_pattern_helpers.cjs +++ b/actions/setup/js/glob_pattern_helpers.cjs @@ -25,6 +25,7 @@ function escapeRegexChars(pattern) { * Supports: * - * matches any characters except / (in path mode) or any characters (in simple mode) * - ** matches any characters including / (only in path mode) + * - \* matches literal asterisk (escaped) * - . is escaped to match literal dots * - \ is escaped properly * @@ -37,11 +38,23 @@ function escapeRegexChars(pattern) { * const regex = globPatternToRegex("metrics/**"); * regex.test("metrics/data.json"); // true * regex.test("metrics/daily/data.json"); // true + * + * @example + * const regex = globPatternToRegex("\\**"); + * regex.test("*admin"); // true + * regex.test("admin"); // false */ function globPatternToRegex(pattern, options) { const { pathMode = true, caseSensitive = true } = options || {}; - let regexPattern = escapeRegexChars(pattern); + // First, handle escaped asterisks before escaping other characters + // This preserves \* as a literal asterisk marker + let regexPattern = pattern + .replace(/\\\*/g, "") // Temporarily mark escaped asterisks + .replace(/\\\\/g, ""); // Temporarily mark escaped backslashes + + // Now escape regex special characters + regexPattern = escapeRegexChars(regexPattern); if (pathMode) { // Path mode: handle ** and * differently @@ -54,6 +67,11 @@ function globPatternToRegex(pattern, options) { regexPattern = regexPattern.replace(/\*/g, ".*"); } + // Restore escaped characters + regexPattern = regexPattern + .replace(//g, "\\*") // Restore escaped asterisks as literal * + .replace(//g, "\\\\"); // Restore escaped backslashes as literal \ + return new RegExp(`^${regexPattern}$`, caseSensitive ? "" : "i"); } diff --git a/actions/setup/js/glob_pattern_helpers.test.cjs b/actions/setup/js/glob_pattern_helpers.test.cjs index b1633108e6..51493c0758 100644 --- a/actions/setup/js/glob_pattern_helpers.test.cjs +++ b/actions/setup/js/glob_pattern_helpers.test.cjs @@ -68,6 +68,32 @@ describe("glob_pattern_helpers.cjs", () => { expect(regex.test("file.min.js")).toBe(true); expect(regex.test("filexminxjs")).toBe(false); }); + + it("should handle escaped asterisks for literal matching", () => { + // Test pattern with escaped asterisk (for label names like "*admin", "~workflow") + const regex = globPatternToRegex("\\**"); + + expect(regex.test("*admin")).toBe(true); + expect(regex.test("*special")).toBe(true); + expect(regex.test("admin")).toBe(false); // Should not match without leading * + expect(regex.test("~admin")).toBe(false); + }); + + it("should handle multiple escaped characters in combination", () => { + // Test pattern combining escaped asterisk with wildcard + const tildePattern = globPatternToRegex("~*"); + const starPattern = globPatternToRegex("\\**"); + + // Tilde pattern: ~ + expect(tildePattern.test("~triage")).toBe(true); + expect(tildePattern.test("~workflow")).toBe(true); + expect(tildePattern.test("triage")).toBe(false); + + // Star pattern: * + expect(starPattern.test("*admin")).toBe(true); + expect(starPattern.test("*special")).toBe(true); + expect(starPattern.test("admin")).toBe(false); + }); }); describe("real-world patterns", () => { diff --git a/actions/setup/js/remove_labels.cjs b/actions/setup/js/remove_labels.cjs index 531f9ec028..61acf28185 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, blockedPatterns, maxCount); 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.test.cjs b/actions/setup/js/remove_labels.test.cjs index 0776578f57..981d7a9547 100644 --- a/actions/setup/js/remove_labels.test.cjs +++ b/actions/setup/js/remove_labels.test.cjs @@ -246,6 +246,83 @@ describe("remove_labels", () => { expect(result.labelsRemoved).toEqual(["bug", "enhancement"]); }); + it("should filter labels based on blocked patterns", async () => { + const handler = await main({ + blocked: ["~*", "\\**"], + max: 10, + }); + + const removeLabelCalls = []; + mockGithub.rest.issues.removeLabel = async params => { + removeLabelCalls.push(params); + return {}; + }; + + const result = await handler( + { + item_number: 100, + labels: ["bug", "~triage", "*admin", "enhancement"], + }, + {} + ); + + expect(result.success).toBe(true); + expect(result.labelsRemoved).toEqual(["bug", "enhancement"]); + // Verify individual blocked labels are logged + expect(mockCore.infos.some(msg => msg.includes('Label "~triage" matched blocked pattern'))).toBe(true); + expect(mockCore.infos.some(msg => msg.includes('Label "*admin" matched blocked pattern'))).toBe(true); + }); + + it("should apply both allowed and blocked filters", async () => { + const handler = await main({ + allowed: ["bug", "~triage", "enhancement"], + blocked: ["~*"], + max: 10, + }); + + const removeLabelCalls = []; + mockGithub.rest.issues.removeLabel = async params => { + removeLabelCalls.push(params); + return {}; + }; + + const result = await handler( + { + item_number: 100, + labels: ["bug", "~triage", "invalid-label", "enhancement"], + }, + {} + ); + + expect(result.success).toBe(true); + // "~triage" is in allowed list but blocked by pattern + expect(result.labelsRemoved).toEqual(["bug", "enhancement"]); + }); + + it("should handle no labels remaining after blocked filtering", async () => { + const handler = await main({ + blocked: ["~*", "\\**"], + max: 10, + }); + + const result = await handler( + { + item_number: 100, + labels: ["~triage", "*admin", "~stale"], + }, + {} + ); + + // Remove labels returns success=true with empty list when all labels are blocked (graceful handling) + expect(result.success).toBe(true); + expect(result.labelsRemoved).toEqual([]); + expect(result.message).toContain("No valid labels"); + // Verify blocked labels are logged individually + expect(mockCore.infos.some(msg => msg.includes('Label "~triage" matched blocked pattern'))).toBe(true); + expect(mockCore.infos.some(msg => msg.includes('Label "*admin" matched blocked pattern'))).toBe(true); + expect(mockCore.infos.some(msg => msg.includes('Label "~stale" matched blocked pattern'))).toBe(true); + }); + it("should handle empty labels array", async () => { const handler = await main({ max: 10 }); diff --git a/actions/setup/js/safe_output_validator.cjs b/actions/setup/js/safe_output_validator.cjs index 3294a80169..8d747807e5 100644 --- a/actions/setup/js/safe_output_validator.cjs +++ b/actions/setup/js/safe_output_validator.cjs @@ -81,12 +81,21 @@ function validateBody(body, fieldName = "body", required = false) { /** * Validate and sanitize an array of labels + * + * Processing pipeline (in order): + * 1. Check for invalid removal attempts (labels starting with '-') + * 2. Filter by allowed list (if configured) + * 3. Sanitize and deduplicate labels + * 4. Filter by blocked patterns (if configured) - TAKES PRECEDENCE over allowed list + * 5. Apply max count limit + * * @param {any} labels - The labels to validate * @param {string[]|undefined} allowedLabels - Optional list of allowed labels + * @param {string[]|undefined} blockedPatterns - Optional list of blocked label patterns (supports glob patterns) * @param {number} maxCount - Maximum number of labels allowed * @returns {{valid: boolean, value?: string[], error?: string}} Validation result */ -function validateLabels(labels, allowedLabels = undefined, maxCount = 3) { +function validateLabels(labels, allowedLabels = undefined, blockedPatterns = undefined, maxCount = 3) { if (!labels || !Array.isArray(labels)) { return { valid: false, error: "labels must be an array" }; } @@ -114,17 +123,45 @@ function validateLabels(labels, allowedLabels = undefined, maxCount = 3) { .map(label => (label.length > 64 ? label.substring(0, 64) : label)) .filter((label, index, arr) => arr.indexOf(label) === index); + // Filter out blocked labels if blocked patterns are provided + let filteredLabels = uniqueLabels; + if (blockedPatterns && blockedPatterns.length > 0) { + const { globPatternToRegex } = require("./glob_pattern_helpers.cjs"); + + // Compile patterns once for performance (outside the filter loop) + /** @type {Array<{pattern: string, regex: RegExp}>} */ + const blockedRegexes = []; + for (const pattern of blockedPatterns) { + try { + // Use simple mode (pathMode: false) for label matching - labels don't contain paths + blockedRegexes.push({ pattern, regex: globPatternToRegex(pattern, { pathMode: false }) }); + } catch (/** @type {any} */ error) { + core.warning(`Invalid blocked pattern "${pattern}": ${error.message}`); + } + } + + filteredLabels = uniqueLabels.filter(label => { + // Check if label matches any blocked pattern + const matchedPattern = blockedRegexes.find(({ regex }) => regex.test(label)); + if (matchedPattern) { + core.info(`Label "${label}" matched blocked pattern "${matchedPattern.pattern}", filtering out`); + return false; + } + return true; + }); + } + // Apply max count limit - if (uniqueLabels.length > maxCount) { - core.info(`Too many labels (${uniqueLabels.length}), limiting to ${maxCount}`); - return { valid: true, value: uniqueLabels.slice(0, maxCount) }; + if (filteredLabels.length > maxCount) { + core.info(`Too many labels (${filteredLabels.length}), limiting to ${maxCount}`); + return { valid: true, value: filteredLabels.slice(0, maxCount) }; } - if (uniqueLabels.length === 0) { + if (filteredLabels.length === 0) { return { valid: false, error: "No valid labels found after sanitization" }; } - return { valid: true, value: uniqueLabels }; + return { valid: true, value: filteredLabels }; } /** diff --git a/actions/setup/js/safe_output_validator.test.cjs b/actions/setup/js/safe_output_validator.test.cjs index 57761abe0a..5085ae216a 100644 --- a/actions/setup/js/safe_output_validator.test.cjs +++ b/actions/setup/js/safe_output_validator.test.cjs @@ -167,7 +167,7 @@ describe("safe_output_validator.cjs", () => { describe("validateLabels", () => { it("should validate and sanitize valid labels", () => { - const result = validator.validateLabels(["bug", " enhancement ", "documentation"], undefined, 10); + const result = validator.validateLabels(["bug", " enhancement ", "documentation"], undefined, undefined, 10); expect(result.valid).toBe(true); expect(result.value).toContain("bug"); @@ -176,14 +176,14 @@ describe("safe_output_validator.cjs", () => { }); it("should reject labels array with removal attempts", () => { - const result = validator.validateLabels(["-bug", "enhancement"], undefined, 10); + const result = validator.validateLabels(["-bug", "enhancement"], undefined, undefined, 10); expect(result.valid).toBe(false); expect(result.error).toContain("Label removal is not permitted"); }); it("should filter labels based on allowed list", () => { - const result = validator.validateLabels(["bug", "custom", "enhancement"], ["bug", "enhancement"], 10); + const result = validator.validateLabels(["bug", "custom", "enhancement"], ["bug", "enhancement"], undefined, 10); expect(result.valid).toBe(true); expect(result.value).toHaveLength(2); @@ -193,7 +193,7 @@ describe("safe_output_validator.cjs", () => { }); it("should limit labels to max count", () => { - const result = validator.validateLabels(["a", "b", "c", "d", "e"], undefined, 3); + const result = validator.validateLabels(["a", "b", "c", "d", "e"], undefined, undefined, 3); expect(result.valid).toBe(true); expect(result.value).toHaveLength(3); @@ -201,7 +201,7 @@ describe("safe_output_validator.cjs", () => { }); it("should deduplicate labels", () => { - const result = validator.validateLabels(["bug", "bug", "enhancement"], undefined, 10); + const result = validator.validateLabels(["bug", "bug", "enhancement"], undefined, undefined, 10); expect(result.valid).toBe(true); expect(result.value).toHaveLength(2); @@ -209,25 +209,80 @@ describe("safe_output_validator.cjs", () => { it("should truncate labels longer than 64 characters", () => { const longLabel = "a".repeat(100); - const result = validator.validateLabels([longLabel], undefined, 10); + const result = validator.validateLabels([longLabel], undefined, undefined, 10); expect(result.valid).toBe(true); expect(result.value[0]).toHaveLength(64); }); it("should reject non-array labels", () => { - const result = validator.validateLabels("bug", undefined, 10); + const result = validator.validateLabels("bug", undefined, undefined, 10); expect(result.valid).toBe(false); expect(result.error).toContain("must be an array"); }); it("should reject when no valid labels remain", () => { - const result = validator.validateLabels([null, undefined, false, 0], undefined, 10); + const result = validator.validateLabels([null, undefined, false, 0], undefined, undefined, 10); expect(result.valid).toBe(false); expect(result.error).toContain("No valid labels found"); }); + + it("should filter labels matching blocked patterns", () => { + const result = validator.validateLabels(["bug", "~triage", "enhancement", "~workflow"], 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("~triage"); + expect(result.value).not.toContain("~workflow"); + }); + + it("should filter labels matching multiple blocked patterns", () => { + const result = validator.validateLabels(["bug", "~triage", "*admin", "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("~triage"); + expect(result.value).not.toContain("*admin"); + }); + + it("should allow labels when blocked patterns is empty", () => { + const result = validator.validateLabels(["bug", "~triage", "enhancement"], undefined, [], 10); + + expect(result.valid).toBe(true); + expect(result.value).toHaveLength(3); + expect(result.value).toContain("~triage"); + }); + + it("should work with both allowed and blocked patterns", () => { + const result = validator.validateLabels(["bug", "~triage", "custom", "enhancement"], ["bug", "~triage", "enhancement"], ["~*"], 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("~triage"); // Filtered by blocked pattern + expect(result.value).not.toContain("custom"); // Filtered by allowed list + }); + + it("should reject when all labels are blocked", () => { + const result = validator.validateLabels(["~triage", "~workflow"], undefined, ["~*"], 10); + + expect(result.valid).toBe(false); + expect(result.error).toContain("No valid labels found"); + }); + + it("should log which pattern matched when blocking labels", () => { + validator.validateLabels(["bug", "~triage"], undefined, ["~*"], 10); + + // Verify the pattern is mentioned in the log + expect(mockCore.info).toHaveBeenCalledWith(expect.stringContaining('matched blocked pattern "~*"')); + }); }); describe("validateMaxCount", () => { diff --git a/docs/src/content/docs/reference/safe-outputs-specification.md b/docs/src/content/docs/reference/safe-outputs-specification.md index ce9559e6e0..c36f0da37c 100644 --- a/docs/src/content/docs/reference/safe-outputs-specification.md +++ b/docs/src/content/docs/reference/safe-outputs-specification.md @@ -7,7 +7,7 @@ sidebar: # Safe Outputs MCP Gateway Specification -**Version**: 1.13.0 +**Version**: 1.14.0 **Status**: Working Draft **Publication Date**: 2026-02-18 **Editor**: GitHub Agentic Workflows Team @@ -1412,6 +1412,21 @@ create-issue: allowed-labels: [bug, feature] # Agent label restrictions ``` +**Label Management Extensions**: +```yaml +add-labels: + allowed: [bug, enhancement, documentation] # Label allowlist + blocked: ["~*", "\\**"] # Block patterns: ~* and *prefix + max: 5 # Maximum labels per run + target-repo: owner/repo # Cross-repository target + allowed-repos: [owner/repo1] # Cross-repo allowlist + +remove-labels: + allowed: [stale, wontfix] # Removable label allowlist + blocked: ["~*"] # Block workflow trigger labels + max: 3 # Maximum removals per run +``` + **Comment Extensions**: ```yaml add-comment: @@ -2385,6 +2400,72 @@ This section provides complete definitions for all remaining safe output types. **Cross-Repository Support**: Yes **Mandatory**: No +**MCP Tool Schema**: + +```json +{ + "name": "add_labels", + "description": "Add labels to an issue or pull request.", + "inputSchema": { + "type": "object", + "required": ["labels"], + "properties": { + "labels": { + "type": "array", + "items": {"type": "string"}, + "description": "Array of label names to add" + }, + "item_number": { + "type": "number", + "description": "Issue/PR number (auto-resolved from context if omitted)" + } + }, + "additionalProperties": false + } +} +``` + +**Operational Semantics**: + +1. **Label Filtering Pipeline**: Labels undergo multi-stage filtering: + - **Stage 1 - Allowlist**: If `allowed` configured, filter to allowed labels only + - **Stage 2 - Blocklist**: If `blocked` configured, remove labels matching any blocked pattern + - **Stage 3 - Sanitization**: Trim, deduplicate, truncate to 64 characters + - **Stage 4 - Max Limit**: Enforce maximum label count +2. **Pattern Matching**: The `blocked` field supports glob-style patterns: + - Wildcards: `~*` matches labels starting with `~` + - Escaped asterisks: `\\**` matches labels starting with literal `*` + - Multiple patterns: All patterns are evaluated; matching ANY pattern blocks the label +3. **Context Resolution**: When `item_number` omitted, resolves from workflow trigger context +4. **Label Creation**: Labels are created if they don't exist in the repository +5. **Idempotency**: Adding already-present labels is a no-op (no error) + +**Configuration Parameters**: +- `max`: Operation limit (default: 3) +- `allowed`: Array of allowed label names (optional allowlist) +- `blocked`: Array of blocked label patterns supporting glob syntax (optional denylist) +- `target`: Target entity (`"triggering"`, `"*"`, or explicit number) +- `target-repo`: Cross-repository target in `owner/repo` format +- `allowed-repos`: Cross-repository allowlist +- `staged`: Staged mode override + +**Security Requirements**: +- **SR-AL1**: Implementations MUST apply allowlist filtering before blocklist filtering +- **SR-AL2**: Implementations MUST reject labels matching ANY blocked pattern +- **SR-AL3**: Blocked pattern matching MUST be case-sensitive +- **SR-AL4**: Pattern syntax errors MUST cause compilation failure, not runtime errors +- **SR-AL5**: When both `allowed` and `blocked` are configured, a label MUST pass both filters +- **SR-AL6**: Filtering MUST log blocked labels at info level for audit trails + +**Rationale for Blocked Patterns**: + +In large repositories with hundreds of labels, maintaining exhaustive allowlists is impractical. However, certain label classes pose security risks: +- Labels prefixed with `~` may trigger workflow cascades (e.g., `~stale` activates triage workflows) +- Labels prefixed with `*` may have administrative significance +- Without infrastructure-level enforcement, agents processing untrusted input (e.g., public issues) can bypass prompt-level restrictions through injection attacks + +The `blocked` field provides defense-in-depth: it's the "you literally can't" enforcement layer, complementing the "please don't" prompt guidance. + **Required Permissions**: *GitHub Actions Token*: @@ -2399,7 +2480,31 @@ This section provides complete definitions for all remaining safe output types. **Notes**: - Requires both `issues: write` and `pull-requests: write` to support labeling both entity types -- Labels must exist in repository; non-existent labels generate warnings +- Labels are created if they don't already exist in the repository; blocked patterns still apply to newly created labels + +**Example Configuration**: + +```yaml +# Block workflow trigger labels and admin labels +safe-outputs: + add-labels: + blocked: ["~*", "\\**"] # Deny ~prefix and *prefix + max: 5 + +# Combine allowlist and blocklist +safe-outputs: + add-labels: + allowed: ["bug", "enhancement", "~triage", "documentation"] + blocked: ["~*"] # Blocks ~triage despite being in allowed list + max: 5 +``` + +**Conformance Tests**: + +- **T-AL-001**: With `blocked: ["~*"]`, agent attempts `["bug", "~triage"]` → Only `["bug"]` applied +- **T-AL-002**: With `allowed: ["bug", "~test"]` and `blocked: ["~*"]`, agent attempts `["~test"]` → No labels applied +- **T-AL-003**: With `blocked: ["\\**"]`, agent attempts `["*admin", "urgent"]` → Only `["urgent"]` applied +- **T-AL-004**: Invalid pattern syntax causes compilation error, not runtime failure --- @@ -2411,6 +2516,53 @@ This section provides complete definitions for all remaining safe output types. **Cross-Repository Support**: Yes **Mandatory**: No +**MCP Tool Schema**: + +```json +{ + "name": "remove_labels", + "description": "Remove labels from an issue or pull request.", + "inputSchema": { + "type": "object", + "required": ["labels"], + "properties": { + "labels": { + "type": "array", + "items": {"type": "string"}, + "description": "Array of label names to remove" + }, + "item_number": { + "type": "number", + "description": "Issue/PR number (auto-resolved from context if omitted)" + } + }, + "additionalProperties": false + } +} +``` + +**Operational Semantics**: + +1. **Label Filtering Pipeline**: Same as `add_labels` (allowlist → blocklist → sanitization → max limit) +2. **Pattern Matching**: Identical glob-style pattern support as `add_labels` +3. **Context Resolution**: When `item_number` omitted, resolves from workflow trigger context +4. **Missing Labels**: Attempting to remove non-present labels is silently ignored (no error) +5. **Partial Failure**: If some labels are blocked or missing, remaining valid labels are still removed + +**Configuration Parameters**: +- `max`: Operation limit (default: 3) +- `allowed`: Array of allowed label names (optional allowlist for removable labels) +- `blocked`: Array of blocked label patterns supporting glob syntax (prevents removal of critical labels) +- `target`: Target entity (`"triggering"`, `"*"`, or explicit number) +- `target-repo`: Cross-repository target in `owner/repo` format +- `allowed-repos`: Cross-repository allowlist +- `staged`: Staged mode override + +**Security Requirements**: +- Same security requirements as `add_labels` (SR-AL1 through SR-AL6) +- **SR-RL1**: Missing labels MUST NOT cause operation failure +- **SR-RL2**: Partial success MUST be logged clearly (which labels removed, which blocked/missing) + **Required Permissions**: *GitHub Actions Token*: @@ -2426,6 +2578,29 @@ This section provides complete definitions for all remaining safe output types. **Notes**: - Same permissions as `add_labels` - Missing labels are silently ignored (no error) +- Blocked patterns prevent removal of protected labels (e.g., blocking `~*` prevents agents from removing workflow trigger labels) + +**Example Configuration**: + +```yaml +# Prevent removal of workflow trigger labels +safe-outputs: + remove-labels: + blocked: ["~*"] # Protect workflow trigger labels + max: 3 + +# Allow removal of specific stale labels only +safe-outputs: + remove-labels: + allowed: ["stale", "needs-info", "waiting"] + max: 5 +``` + +**Conformance Tests**: + +- **T-RL-001**: With `blocked: ["~*"]`, agent attempts to remove `["~triage", "wontfix"]` → Only `wontfix` removed +- **T-RL-002**: Removing non-existent label succeeds without error +- **T-RL-003**: Blocked pattern prevents removal even if label exists on issue --- @@ -3704,6 +3879,16 @@ safe-outputs: ## Appendix F: Document History +**Version 1.14.0** (2026-02-18): +- **Added**: `blocked` configuration parameter for `add_labels` and `remove_labels` types enabling glob-style pattern matching to deny specific label patterns +- **Added**: Glob pattern matching support with wildcards (`~*` for prefix matching) and escaped literal asterisks (`\\**` for literal `*` prefix) +- **Enhanced**: Label filtering pipeline documentation specifying allowlist → blocklist → sanitization → max count enforcement order +- **Enhanced**: Complete operational semantics, MCP tool schemas, and security requirements for `add_labels` and `remove_labels` types +- **Security**: Infrastructure-level pattern enforcement prevents prompt injection attacks from bypassing label restrictions in workflows processing untrusted input +- **Added**: Security requirements SR-AL1 through SR-AL6 and SR-RL1 through SR-RL2 for label management operations +- **Added**: Conformance tests T-AL-001 through T-AL-004 and T-RL-001 through T-RL-003 for blocked pattern validation +- **Rationale**: Documented motivation for blocked patterns in large repositories (600+ labels) where exhaustive allowlists are impractical + **Version 1.13.0** (2026-02-18): - **Added**: Optional `discussions` field for `add-comment` and `hide-comment` safe output types to control `discussions:write` permission - **Enhanced**: Permission documentation for `add-comment` and `hide-comment` to explain conditional `discussions:write` inclusion diff --git a/pkg/cli/workflows/test-blocked-label-patterns.md b/pkg/cli/workflows/test-blocked-label-patterns.md new file mode 100644 index 0000000000..9d9c47aeba --- /dev/null +++ b/pkg/cli/workflows/test-blocked-label-patterns.md @@ -0,0 +1,50 @@ +--- +on: + workflow_dispatch: +engine: copilot +safe-outputs: + add-labels: + blocked: ["~*", "\\**"] # Block labels starting with ~ or * + allowed: ["bug", "enhancement", "documentation", "~triage", "*admin"] # Allowed list (but blocked patterns take precedence) + max: 5 +--- + +# Test Blocked Label Patterns + +This is a test workflow to verify that the `blocked` pattern matching works for the `add-labels` safe output. + +## Configuration + +The workflow demonstrates **defense-in-depth** security: +- **Blocked patterns**: `["~*", "\\**"]` - Denies any labels starting with `~` or `*` +- **Allowed list**: Permits specific labels including `~triage` and `*admin` +- **Max count**: Limits to 5 labels per operation + +## Pattern Precedence + +**Blocked patterns are applied AFTER allowed list filtering**, meaning: +1. Labels are first filtered by the allowed list (if configured) +2. Then blocked patterns remove matching labels +3. Finally, the max count limit is enforced + +This means even if `~triage` is in the allowed list, it will be blocked by the `~*` pattern. + +## Test Case + +Please add the following labels to issue #1: +- "bug" ✓ (should succeed - in allowed list, not blocked) +- "enhancement" ✓ (should succeed - in allowed list, not blocked) +- "~triage" ✗ (should be blocked by `~*` pattern despite being in allowed list) +- "*admin" ✗ (should be blocked by `\\**` pattern despite being in allowed list) +- "documentation" ✓ (should succeed - in allowed list, not blocked) +- "custom-label" ✗ (should be filtered by allowed list before reaching blocked patterns) + +**Expected result**: Only "bug", "enhancement", and "documentation" should be added to the issue. + +## Security Rationale + +This configuration prevents agentic workflows from: +- Applying workflow trigger labels (`~*`) that could cause cascading automation +- Setting administrative labels (`*admin`, `*urgent`) reserved for maintainers + +The blocked patterns provide infrastructure-level enforcement that cannot be bypassed through prompt injection attacks. diff --git a/pkg/parser/schemas/main_workflow_schema.json b/pkg/parser/schemas/main_workflow_schema.json index 755a9fba3b..b4093b977e 100644 --- a/pkg/parser/schemas/main_workflow_schema.json +++ b/pkg/parser/schemas/main_workflow_schema.json @@ -5223,6 +5223,15 @@ "minItems": 1, "maxItems": 50 }, + "blocked": { + "type": "array", + "description": "Optional list of blocked label patterns. Supports glob-style patterns (e.g., '~*' blocks labels starting with '~', '\\**' blocks labels starting with '*'). Labels matching any blocked pattern will be filtered out.", + "items": { + "type": "string" + }, + "minItems": 1, + "maxItems": 50 + }, "max": { "type": "integer", "description": "Optional maximum number of labels to add (default: 3)", @@ -5272,6 +5281,15 @@ "minItems": 1, "maxItems": 50 }, + "blocked": { + "type": "array", + "description": "Optional list of blocked label patterns. Supports glob-style patterns (e.g., '~*' blocks labels starting with '~', '\\**' blocks labels starting with '*'). Labels matching any blocked pattern will be filtered out.", + "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..3445667ec0 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-style patterns (e.g., "~*" blocks labels starting with "~"). } // 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/compiler_safe_outputs_config.go b/pkg/workflow/compiler_safe_outputs_config.go index bcf3bbed2b..7bf0d6f008 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..214a5950d1 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-style patterns (e.g., "~*" blocks labels starting with "~"). } // 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..2348f53725 100644 --- a/pkg/workflow/safe_output_builder.go +++ b/pkg/workflow/safe_output_builder.go @@ -164,6 +164,13 @@ func BuildAllowedListEnvVar(envVarName string, allowed []string) []string { return []string{fmt.Sprintf(" %s: %q\n", envVarName, allowedStr)} } +// BuildBlockedListEnvVar creates environment variable lines for blocked patterns. +// Always outputs the env var, even when empty (empty string means "block none"). +func BuildBlockedListEnvVar(envVarName string, blocked []string) []string { + blockedStr := strings.Join(blocked, ",") + return []string{fmt.Sprintf(" %s: %q\n", envVarName, blockedStr)} +} + // ====================================== // Close Job Config Helpers // ====================================== @@ -218,6 +225,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-style patterns) } // ParseListJobConfig parses common list job fields from a config map. @@ -247,6 +255,21 @@ func ParseListJobConfig(configMap map[string]any, allowedKey string) (ListJobCon } } + // Parse blocked list (always uses "blocked" key) + 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 +282,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, BuildBlockedListEnvVar(prefix+"_BLOCKED", config.Blocked)...) + // Add max count envVars = append(envVars, BuildMaxCountEnvVar(prefix+"_MAX_COUNT", maxCount)...) diff --git a/pkg/workflow/safe_outputs_config_generation.go b/pkg/workflow/safe_outputs_config_generation.go index 6f369a54c8..9660bfe43d 100644 --- a/pkg/workflow/safe_outputs_config_generation.go +++ b/pkg/workflow/safe_outputs_config_generation.go @@ -184,6 +184,9 @@ func generateSafeOutputsConfig(data *WorkflowData) string { if len(data.SafeOutputs.AddLabels.Allowed) > 0 { additionalFields["allowed"] = data.SafeOutputs.AddLabels.Allowed } + if len(data.SafeOutputs.AddLabels.Blocked) > 0 { + additionalFields["blocked"] = data.SafeOutputs.AddLabels.Blocked + } safeOutputsConfig["add_labels"] = generateTargetConfigWithRepos( data.SafeOutputs.AddLabels.SafeOutputTargetConfig, data.SafeOutputs.AddLabels.Max, @@ -192,10 +195,11 @@ func generateSafeOutputsConfig(data *WorkflowData) string { ) } if data.SafeOutputs.RemoveLabels != nil { - safeOutputsConfig["remove_labels"] = generateMaxWithAllowedConfig( + safeOutputsConfig["remove_labels"] = generateMaxWithAllowedAndBlockedConfig( data.SafeOutputs.RemoveLabels.Max, 3, // default max data.SafeOutputs.RemoveLabels.Allowed, + data.SafeOutputs.RemoveLabels.Blocked, ) } if data.SafeOutputs.AddReviewer != nil {