Skip to content

Bug 2079840 - allow @username in the textarea like github - #2776

Open
dklawren wants to merge 4 commits into
mozilla:masterfrom
dklawren:2079840
Open

dklawren wants to merge 4 commits into
mozilla:masterfrom
dklawren:2079840

Conversation

@dklawren

@dklawren dklawren commented Oct 8, 2026 •

Copy link
Copy Markdown
Collaborator

Bug 2079840 - Support GitHub-style @Nickname mentions to needinfo and CC users

Summary

When a user with editbugs adds a comment to an existing bug that contains @nickname, each mentioned user is CC'd and gets a needinfo request. Several mentions in one comment (@alice, @bob) each get a needinfo. Safeguards limit who can trigger mentions, how many are processed, and make sure a mention never grants access to a restricted bug.

Behavior

  • Matching: a mention matches the profile nickname field of an enabled account. If a nickname matches several accounts, or none, it is skipped silently. It is never guessed.
  • Ignored: email addresses ([email protected]), quoted lines (> ...), inline code (any number of backticks), fenced code blocks, and mentioning yourself.
  • Needinfo vs CC: mentioned users are always CC'd. The needinfo is skipped if the user blocks needinfo or already has a needinfo pending. If a site's needinfo type is not multiplicable, at most one needinfo is requested.
  • Never blocks saving: each mentioned user is processed in an eval under ERROR_MODE_DIE. A failure, such as add_cc's strict_isolation check, skips that user and logs a WARN instead of rolling back the comment.
  • Private comments: a mention that appears only in private comments counts only if that user is an insider.
  • Scope: only comments added to existing bugs. The description of a new bug is not parsed.
  • Mass edit: mentions are processed on every bug in a mass edit.

Safeguards

  • Only editbugs users trigger mentions (product-specific, the same check core uses). For everyone else, @nickname is plain text, so there is no lookup, CC or needinfo.
  • At most 10 mentions per update: only the first 10 distinct nicknames are processed (MAX_MENTIONS), and the rest are ignored. This prevents notification spam from one comment.
  • No access by mention: mentioned users who cannot already see the bug are skipped entirely, with no CC and no needinfo. A typo, or a nickname someone else has claimed (squatting), can never expose a security bug.

Implementation

  • extensions/Needinfo/Extension.pm
    • The needinfo flag type is now created with is_multiplicable => 1, matching production. This only affects new installs.
    • bug_start_of_update now calls the existing form handling (moved unchanged into _process_needinfo_params), then _process_mentions. Mentions run second, so a needinfo just requested through the form is detected and not duplicated.
    • _extract_mentions parses the comment text. It is a pure function covered by unit tests.
    • _users_for_mentions resolves nicknames with one query and drops nicknames shared by more than one account.
    • _process_mentions applies the safeguards. Updates with no mentions return before any database work.
    • _cc_and_needinfo adds the CC and needinfo for one user.
  • extensions/Needinfo/t/mentions.t
    • Parser tests.
    • Behavior tests for CC plus needinfo, skipping yourself, skipping users who can't see the bug, the cap of 10, the editbugs gate, non-multiplicable types, no duplicate needinfo, and a failing CC not throwing.

Known limitations

  • Mentions are parsed with regexes, not the Markdown renderer, so indented code blocks and HTML <code> are not ignored.
  • _users_for_mentions (one SQL query) has no unit test.
  • Unknown or ambiguous nicknames count toward the cap of 10.

Testing

  • test_sanity: extensions/Needinfo/t/mentions.t, t/001compile.t, t/002goodperl.t, t/005whitespace.t, t/critic.t all pass.
  • Not yet covered: webservices or Selenium tests, and a manual check in the dev stack.

This comment was marked as outdated.

…fo and CC users

Each @Nickname in a new comment on an existing bug CCs the matching user
and requests needinfo from them. Mentions in quotes, code and email
addresses are ignored, ambiguous nicknames are skipped, and a mention
never blocks the comment from being saved.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment thread extensions/Needinfo/Extension.pm Outdated
Comment thread extensions/Needinfo/Extension.pm Outdated
Comment thread extensions/Needinfo/Extension.pm Outdated
Comment thread extensions/Needinfo/Extension.pm Outdated
Comment thread extensions/Needinfo/t/mentions.t
Only editbugs users can trigger mentions, at most 10 distinct nicknames
are processed per update, and mentioned users who cannot already see the
bug are skipped so a mention, typo or squatted nickname never grants
access to a restricted bug.
- Create the needinfo flag type as multiplicable, matching production.
- With a non-multiplicable needinfo type, request at most one needinfo
  instead of throwing flag_type_not_multiplicable.
- Process each mentioned user in an eval under ERROR_MODE_DIE so a failure
  (e.g. add_cc's strict_isolation check) skips that user instead of rolling
  back the update.
- Ignore mentions in multi-backtick code spans and allow | in nicknames,
  matching extract_nicks.
@dklawren
dklawren requested review from Xzzz and cgsheeh and a balanced review from Copilot October 8, 2026 21:19

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment thread extensions/Needinfo/Extension.pm Outdated
…prevent premature code block closure'

Co-authored-by: Copilot Autofix powered by AI <[email protected]>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants