feat(slack): add threaded messaging for PR notifications

[kdenhartog]

• Messages for the same PR are grouped into threads
• Subsequent runs reply to existing thread instead of new messages
• Completion message with ✅ reaction when needs-security-review label removed
• Add securityReviewCompleted detection function
• Update README with threading documentation

Test trigger words:

• "authentication" "password"

[github-actions]

[puLL-Merge] - brave/security-action@901

Description

This PR adds Slack threading support and security review completion notifications to the security-action GitHub composite action. The main changes include:

1. Slack Threading: Messages for the same PR are now grouped into threads, reducing channel noise. The first notification for a PR creates a new thread, and subsequent notifications reply to that thread.

2. Completion Notifications: When the needs-security-review label is removed by an assignee, a completion message is posted to the thread with a ✅ checkmark reaction on the parent message.

3. Code Formatting: The codebase has been reformatted from single quotes to double quotes and semicolons have been added (likely via Prettier).

Possible Issues

• Thread lookup performance: The findExistingThreadForPR function searches the last 100 messages from the past 30 days. For very active channels, this may miss older threads or incur latency.
• Metadata dependency: Thread discovery relies on Slack message metadata (security_action_thread event type). If Slack changes metadata handling or if messages are deleted, thread continuity may break.
• Missing unlabeled event in default workflow: The README documents adding unlabeled to the event types, but existing users won't receive completion notifications until they update their workflows.

Security Hotspots

1. Slack token handling (lines 418-435, sendSlackMessage.js): The Slack token is passed through the action. While core.setSecret() is used for user mappings, ensure the token itself is never logged, especially in debug mode.

2. JSON parsing of user map (lines 349-353): JSON.parse(options.gh_to_slack_user_map) could throw on malformed input, but this is caught. However, the catch block only logs a message and continues with an empty map, which may mask configuration errors.

Privacy Hotspots

1. User mapping exposure in debug logs: The debugLog('Slack assignees:', slackAssignees) call (line 364) may expose Slack user IDs or handles in logs. While core.setSecret() is called, debug mode might still reveal this information in action logs.

2. Actor information in completion messages: The completion message includes ${context.actor} which exposes the GitHub username publicly in Slack. This is likely intentional but worth noting for privacy-conscious environments.

Changes

Changes

README.md

• Added documentation for Slack threading and completion notifications
• Added example workflow configuration showing the unlabeled event type
• Minor formatting improvements (line wrapping, escaping asterisks)

actions/main/action.cjs

• Added prIdentifier construction for thread tracking
• Added isCompletion parameter support for completion messages
• Imported and integrated securityReviewCompleted step
• Added completion notification logic when security review label is removed by assignee
• Passed prIdentifier to all sendSlackMessage calls
• Code reformatted (quotes, semicolons, spacing)

src/sendSlackMessage.js

• Added findExistingThreadForPR() function to search for existing threads by PR identifier
• Added addCompletionReaction() function to add ✅ reaction to parent messages
• Added prIdentifier and isCompletion parameters to main function
• Implemented threading logic: creates new thread if none exists, replies to existing thread otherwise
• Added metadata with security_action_thread event type and PR identifier for thread discovery
• Non-threaded mode preserves existing debounce behavior
• Code reformatted (quotes, semicolons, spacing)

src/sendSlackMessage.test.js (new file)

• Unit tests for findExistingThreadForPR function
• Unit tests for addCompletionReaction function
• Tests cover thread discovery, PR identifier matching, and error handling

src/steps/securityReviewCompleted.js (new file)

• New step to detect when needs-security-review label is removed
• Checks that the event is unlabeled, the label is correct, and the actor is an assignee
• Case-insensitive matching for assignee usernames

src/steps/securityReviewCompleted.test.js (new file)

• Comprehensive unit tests for the securityReviewCompleted step
• Tests various event types, actions, labels, and actor scenarios

sequenceDiagram
    participant GH as GitHub Action
    participant Action as action.cjs
    participant Slack as sendSlackMessage
    participant API as Slack API

    GH->>Action: pull_request event (opened/sync/unlabeled)
    Action->>Action: Check if reviewdog enabled
    Action->>Action: Run security checks
    
    alt New findings or hotwords
        Action->>Slack: sendSlackMessage(prIdentifier)
        Slack->>API: conversations.history (find existing thread)
        alt Thread exists
            Slack->>API: postMessage(thread_ts)
        else No thread
            Slack->>API: postMessage(metadata with pr_identifier)
        end
    end

    alt Label unlabeled event
        Action->>Action: securityReviewCompleted()
        alt Label removed by assignee
            Action->>Slack: sendSlackMessage(isCompletion=true)
            Slack->>API: postMessage to thread
            Slack->>API: reactions.add(white_check_mark)
        end
    end

Loading

[github-actions]

The security team is monitoring all repositories for certain keywords. This PR includes the word(s) "password, authentication" and so security team members have been added as reviewers to take a look.

No need to request a full security review at this stage, the security team will take a look shortly and either clear the label or request more information/changes.

Notifications have already been sent, but if this is blocking your merge feel free to reach out directly to the security team on Slack so that we can expedite this check.