Add blocked deny list for assign-to-user and unassign-from-user safe outputs#16628
Add blocked deny list for assign-to-user and unassign-from-user safe outputs#16628
blocked deny list for assign-to-user and unassign-from-user safe outputs#16628Conversation
…outputs Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
…assign-from-user Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
blocked deny list for assign-to-user outputblocked deny list for assign-to-user and unassign-from-user safe outputs
|
|
||
| // Simple glob pattern matching | ||
| // Convert glob pattern to regex: "*[bot]" -> "^.*\[bot\]$" | ||
| const regexPattern = pattern |
There was a problem hiding this comment.
Refactored in commit 7102cba. The regex pattern escaping now uses the generic simpleGlobToRegex() function from glob_pattern_helpers.cjs, which properly escapes all special regex characters except *.
| .replace(/[.+?^${}()|[\]\\]/g, "\\$&") // Escape special regex chars except * | ||
| .replace(/\*/g, ".*"); // Convert * to .* | ||
|
|
||
| const regex = new RegExp(`^${regexPattern}$`, "i"); // Case-insensitive |
There was a problem hiding this comment.
Refactored in commit 7102cba. The regex construction is now delegated to matchesSimpleGlob() in glob_pattern_helpers.cjs, which handles case-insensitive matching by default.
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
| return patterns.some(pattern => pattern.test(filePath)); | ||
| } | ||
|
|
||
| /** |
There was a problem hiding this comment.
Deduplicated in commit 0626e1a. The simpleGlobToRegex() function now reuses globPatternToRegex() with pathMode: false option, sharing the same escaping logic through a new escapeRegexChars() helper.
…e globPatternToRegex Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR adds a blocked deny list feature to assign-to-user and unassign-from-user safe outputs to provide defense-in-depth against prompt injection attacks. The feature allows workflow authors to block specific users or patterns (e.g., copilot, *[bot]) from being assigned to issues, preventing attackers from escalating prompt injection attacks into triggering automated agent sessions.
Changes:
- Added
blockedfield toAssignToUserConfigandUnassignFromUserConfigstructs with pattern matching support - Implemented generic glob pattern matching helpers with deduplication of regex escaping logic
- Updated safe output processing pipeline to filter blocked items after allowed filtering
- Added comprehensive test coverage for pattern matching and the assign-to-user handler
- Updated JSON schema and documentation with clear examples and security context
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
pkg/workflow/assign_to_user.go |
Added Blocked field to config struct with YAML tag and documentation |
pkg/workflow/unassign_from_user.go |
Added Blocked field to config struct with YAML tag and documentation |
pkg/workflow/safe_outputs_config_generation_helpers.go |
Created generateMaxWithAllowedAndBlockedConfig helper function |
pkg/workflow/safe_outputs_config_generation.go |
Updated config generation to use new helper with blocked parameter |
pkg/parser/schemas/main_workflow_schema.json |
Added blocked field schema with clear descriptions for both handlers |
docs/src/content/docs/reference/safe-outputs.md |
Updated documentation with examples and security explanation |
actions/setup/js/glob_pattern_helpers.cjs |
Refactored with escapeRegexChars() helper and enhanced globPatternToRegex() with options |
actions/setup/js/safe_output_processor.cjs |
Added filterByBlocked() function and integrated into processItems pipeline |
actions/setup/js/safe_output_helpers.cjs |
Added matchesBlockedPattern() and isUsernameBlocked() helper functions |
actions/setup/js/assign_to_user.cjs |
Updated to extract and use blocked configuration |
actions/setup/js/unassign_from_user.cjs |
Updated to extract and use blocked configuration |
actions/setup/js/glob_pattern_helpers.test.cjs |
Added comprehensive tests for simple glob matching |
actions/setup/js/safe_output_helpers.test.cjs |
Added tests for blocked pattern matching functions |
actions/setup/js/assign_to_user.test.cjs |
Added comprehensive tests for blocked pattern filtering |
actions/setup/js/push_repo_memory.cjs |
Updated to use arrow function syntax for pattern mapping |
actions/setup/js/push_repo_memory.test.cjs |
Updated test to use arrow function syntax |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| async function main(config = {}) { | ||
| // Extract configuration | ||
| const allowedAssignees = config.allowed || []; | ||
| const blockedAssignees = config.blocked || []; |
There was a problem hiding this comment.
The unassign_from_user.test.cjs file is missing test coverage for the new blocked patterns feature. While assign_to_user.test.cjs has comprehensive tests for blocked patterns (lines 487-587), the unassign_from_user handler should have similar test coverage to ensure the blocked filtering works correctly for unassignments as well.
Consider adding tests for:
- Filtering out blocked users by exact match
- Filtering out blocked users by pattern (e.g.,
*[bot]) - Combining allowed and blocked filters
- Handling empty results when all users are blocked
| * @param {string[]|undefined} blockedPatterns - Blocked patterns list (undefined means no blocking) | ||
| * @returns {string[]} Filtered items with blocked items removed | ||
| */ | ||
| function filterByBlocked(items, blockedPatterns) { |
There was a problem hiding this comment.
The safe_output_processor.test.cjs file is missing test coverage for the new filterByBlocked function. The function is exported from the module and used in the processItems pipeline, but there are no direct tests to verify its behavior.
Consider adding tests for:
- Filtering items with exact pattern matches
- Filtering items with glob patterns
- Handling empty or undefined blocked patterns
- Integration with the processItems pipeline
|
🧪 Smoke Project is now testing project operations... |
|
📰 BREAKING: Smoke Copilot is now investigating this pull request. Sources say the story is developing... |
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
|
🧪 Smoke Temporary ID is now testing temporary ID functionality... |
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 |
Agent Container Tool Check
Result: 12/12 tools available ✅ Overall Status: PASS
|
|
✅ Smoke Temporary ID completed successfully. Temporary ID validation passed. |
|
✅ Smoke Project completed successfully. All project operations validated. |
|
Smoke test results
|
|
Smoke Test: Copilot - 22157553390
Failure: Serena MCP —
|
There was a problem hiding this comment.
This PR adds blocked assignee support to assign_to_user.cjs. The implementation is clean and the test coverage is solid. Two minor suggestions added as inline comments. Overall looks good!
📰 BREAKING: Report filed by Smoke Copilot for issue #16628
| async function main(config = {}) { | ||
| // Extract configuration | ||
| const allowedAssignees = config.allowed || []; | ||
| const blockedAssignees = config.blocked || []; |
There was a problem hiding this comment.
Nice addition of blockedAssignees support. Consider adding a comment here explaining that blocked takes precedence over allowed to clarify the filtering priority for future readers.
| describe("blocked patterns", () => { | ||
| it("should filter out blocked users by exact match", async () => { | ||
| const { main } = require("./assign_to_user.cjs"); | ||
| const handler = await main({ |
There was a problem hiding this comment.
Good test coverage for blocked patterns. It would also be worth adding a test case that verifies an empty blocked array (the default) doesn't filter any valid assignees.
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
Smoke Test Results — Run §22157554498Core Tests #1-10: ✅✅✅✅✅✅✅✅✅✅ All pass Overall: PARTIAL (all non-skipped tests passed)
|
There was a problem hiding this comment.
💥 Automated smoke test review - all systems nominal!
💥 [THE END] — Illustrated by Smoke Claude for issue #16628
Implementation Complete:
blockeddeny list forassign-to-usersafe outputBlockedfield toAssignToUserConfigstruct inpkg/workflow/assign_to_user.goBlockedfield toUnassignFromUserConfigstruct inpkg/workflow/unassign_from_user.gogenerateMaxWithAllowedAndBlockedConfighelper inpkg/workflow/safe_outputs_config_generation_helpers.gopkg/workflow/safe_outputs_config_generation.goactions/setup/js/glob_pattern_helpers.cjsprocessItemsfunction inactions/setup/js/safe_output_processor.cjsto filter blocked itemsactions/setup/js/assign_to_user.cjsto use blocked configurationactions/setup/js/unassign_from_user.cjsto use blocked configurationactions/setup/js/glob_pattern_helpers.test.cjsactions/setup/js/assign_to_user.test.cjsdocs/src/content/docs/reference/safe-outputs.mdpkg/parser/schemas/main_workflow_schema.jsonmake fmtandmake lintLatest Changes
Deduplicated glob pattern matching implementation to eliminate code duplication:
Refactored
glob_pattern_helpers.cjs:escapeRegexChars()internal helper for consistent special character escapingglobPatternToRegex()with optionaloptionsparameter:pathMode(default: true): Controls how*is interpretedcaseSensitive(default: true): Controls case sensitivitysimpleGlobToRegex()to callglobPatternToRegex()withpathMode: falseUpdated callers:
parseGlobPatterns()- Uses arrow function to pass pattern correctlypush_repo_memory.cjs- Updated map call to use arrow functionBenefits:
Original prompt
blockeddeny list forassign-to-usersafe output #16626✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.
✨ PR Review Safe Output Test - Run 22157554498
Changeset