Incomplete Error Handling in Issue Label Changes

[pedrogaudencio]

Problem

Location: modules/actions/workflows.go (lines 377-398)

actions := []string{}
switch issuePayload.Action {
case api.HookIssueLabelUpdated:
    if len(issuePayload.Changes.AddedLabels) > 0 {
        actions = append(actions, "labeled")
    }
    if len(issuePayload.Changes.RemovedLabels) > 0 {
        actions = append(actions, "unlabeled")
    }

Issue: If both AddedLabels and RemovedLabels are empty (which could happen with a malformed webhook payload), actions will be empty and the subsequent loop will not match anything, potentially causing unexpected behavior.

Solution

Fix: Add validation:

actions := []string{}
switch issuePayload.Action {
case api.HookIssueLabelUpdated:
    if len(issuePayload.Changes.AddedLabels) > 0 {
        actions = append(actions, "labeled")
    }
    if len(issuePayload.Changes.RemovedLabels) > 0 {
        actions = append(actions, "unlabeled")
    }
    // If no labels were added or removed, this is invalid
    if len(actions) == 0 {
        log.Warn("HookIssueLabelUpdated action without added or removed labels")
        return false
    }

[pedrogaudencio]

go-gitea/pull/24048 introduced the issue of HookIssueLabelUpdated always mapping to "labeled", even when labels were removed; go-gitea/pull/34316 fixed it by correctly distinguishing between label additions and removals (added RemovedLabels field to IssuePayload struct).

Can HookIssueLabelUpdated legitimately occur with empty AddedLabels and RemovedLabels? Based on code analysis, no:
From services/actions/notifier.go:

// lines 194-205
func (n *actionsNotifier) IssueChangeLabels(ctx context.Context, doer *user_model.User, issue *issues_model.Issue,
    addedLabels, removedLabels []*issues_model.Label,
) {
    // ...
    notifyIssueChange(ctx, doer, issue, hookEvent, api.HookIssueLabelUpdated, addedLabels, removedLabels)
}

Key observations:

• IssueChangeLabels is called with explicit addedLabels and removedLabels parameters
• The function is only called when labels are actually being changed
• The caller (issue_service.AddLabel, issue_service.RemoveLabel, issue_service.ReplaceLabels) always provides at least one non-empty slice
• If both were empty, there would be no label change to notify about

What happens when actions is empty?

// lines 400-405:
for _, val := range vals {
    if slices.ContainsFunc(actions, glob.MustCompile(val, '/').Match) {
        matchTimes++
        break
    }
}

Behavior when actions is empty:

• The loop iterates over vals (the workflow trigger types)
• slices.ContainsFunc(actions, ...) will return false for every val
• matchTimes will not be incremented

Result: The function returns false (line 410), meaning the workflow will not be triggered. This is actually the correct behaviour - if there are no actions, the workflow shouldn't trigger.

Note that there is no test for the edge case where both AddedLabels and RemovedLabels are empty. It's missing because this scenario cannot legitimately occur in normal operation.

Why the proposed fix is unnecessary

/cc @taoeffect

1. Cannot occur in practice: the IssueChangeLabels function is only called when labels are actually being changed. If both addedLabels and removedLabels are empty, the function wouldn't be called in the first place.
2. Correct behaviour if it did occur: If somehow both slices were empty:

• actions would be empty
• The workflow matching would fail (return false)
• No workflow would be triggered
• This is the correct behaviour - no label change means no workflow trigger

3. Adding a warning would be noise: since this cannot legitimately occur, adding a warning would only create log spam if there's a bug elsewhere. The proper place to validate this is at the caller level, not here.
4. Defensive programming vs. trust: the code trusts its callers to provide valid data. Adding validation here would be defensive programming against a scenario that the architecture prevents.

Evidence:

• Merged into upstream Gitea on [Fix] Trigger 'unlabeled' event when label is Deleted from PR go-gitea/gitea#34316, merged September 24, 2025
• Approved by maintainers (@lunny)
• Comprehensive test coverage for all realistic scenarios
• Fixes a real bug Incorrect labeled event fired when label is Deleted from PR go-gitea/gitea#34317 where label deletions weren't triggering workflows ("unlabeled" events)
• The empty actions case is handled correctly (workflow doesn't trigger)
• The scenario cannot occur in normal operation (prevented by architecture)