From a62bcad27f87bdadd56faf174552e490fe4024b9 Mon Sep 17 00:00:00 2001 From: David Lawrence Date: Thu, 8 Oct 2026 15:13:12 -0400 Subject: [PATCH 1/4] feat: Bug 2079840 - Support GitHub-style @nickname mentions to needinfo 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. --- extensions/Needinfo/Extension.pm | 89 +++++++++++++++++++++++++++++++- extensions/Needinfo/t/mentions.t | 45 ++++++++++++++++ 2 files changed, 132 insertions(+), 2 deletions(-) create mode 100644 extensions/Needinfo/t/mentions.t diff --git a/extensions/Needinfo/Extension.pm b/extensions/Needinfo/Extension.pm index 79331e13eb..a7c6fa10ff 100644 --- a/extensions/Needinfo/Extension.pm +++ b/extensions/Needinfo/Extension.pm @@ -97,10 +97,19 @@ sub bug_end_of_create { $self->bug_start_of_update($args); } -# Clear the needinfo? flag if comment is being given by -# requestee or someone used the override flag. sub bug_start_of_update { my ($self, $args) = @_; + _process_needinfo_params($args); + + # Runs after the explicit needinfo handling so that the duplicate check + # sees any flags that were just requested through the form. + _process_mentions($args->{bug}) if $args->{old_bug}; +} + +# Clear the needinfo? flag if comment is being given by +# requestee or someone used the override flag. +sub _process_needinfo_params { + my ($args) = @_; my $bug = $args->{bug}; my $old_bug = $args->{old_bug}; @@ -248,6 +257,82 @@ sub bug_start_of_update { } } +# Returns the unique nicknames @mentioned in a comment, in order of first +# appearance. Mentions inside quoted lines (> ...) and code are ignored, as +# are email addresses (the @ must not follow a word character). +sub _extract_mentions { + my ($text) = @_; + return () unless defined $text; + + $text =~ s/^[ \t]*(```|~~~).*?(?:^[ \t]*\1[^\n]*$|\z)//msg; + $text =~ s/`[^`\n]*`//g; + $text =~ s/^[ \t]*>.*$//mg; + + my (@nicks, %seen); + while ($text =~ /(?dbh; + my $rows = $dbh->selectall_arrayref( + 'SELECT userid, nickname FROM profiles WHERE is_enabled = 1 AND ' + . $dbh->sql_in('nickname', [map { $dbh->quote($_) } @nicks])); + + my %ids_by_nick; + push @{$ids_by_nick{lc $_->[1]}}, $_->[0] foreach @$rows; + my @ids = map { $_->[0] } grep { @$_ == 1 } values %ids_by_nick; + return @{Bugzilla::User->new_from_list(\@ids)}; +} + +# GitHub-style mentions: each @nickname in a new comment is CC'd and gets a +# needinfo request. Mentions never block the comment from being saved; anyone +# who can't be needinfo'd (blocked, already asked, no permission) is only CC'd. +sub _process_mentions { + my ($bug) = @_; + my $user = Bugzilla->user; + + # Lowercased nick => true if mentioned in at least one public comment. + my %public; + foreach my $comment (@{$bug->{added_comments} || []}) { + $public{lc $_} ||= !$comment->{isprivate} + foreach _extract_mentions($comment->{thetext}); + } + + my @mentioned + = grep { $_->id != $user->id && ($public{lc $_->nick} || $_->is_insider) } + _users_for_mentions(keys %public); + return unless @mentioned; + + my ($type) = grep { $_->name eq 'needinfo' } @{$bug->flag_types}; + my $can_flag + = $type && $bug->check_can_change_field('flagtypes.name', 0, 1)->{allowed}; + + my @new_flags; + foreach my $mentioned (@mentioned) { + $bug->add_cc($mentioned); + + next if !$can_flag || $mentioned->needinfo_blocked; + next if grep { + $_->status eq '?' && ($_->requestee_id // 0) == $mentioned->id + } @{$type->{flags}}; + + push @new_flags, + {type_id => $type->id, status => '?', requestee => $mentioned->login}; + } + + $bug->set_flags([], \@new_flags) if @new_flags; +} + sub _check_requestee { my ($requestee) = @_; my $user diff --git a/extensions/Needinfo/t/mentions.t b/extensions/Needinfo/t/mentions.t new file mode 100644 index 0000000000..bc937e52ef --- /dev/null +++ b/extensions/Needinfo/t/mentions.t @@ -0,0 +1,45 @@ +#!/usr/bin/env perl +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. +# +# This Source Code Form is "Incompatible With Secondary Licenses", as +# defined by the Mozilla Public License, v. 2.0. +use strict; +use warnings; +use lib qw( . lib local/lib/perl5 ); + +BEGIN { + $ENV{LOG4PERL_CONFIG_FILE} = 'log4perl-t.conf'; + $ENV{BUGZILLA_DISABLE_HOSTAGE} = 1; +} + +use Bugzilla::Test::MockDB; +use Bugzilla::Test::MockParams; + +use Test2::V0; + +use Bugzilla; +BEGIN { Bugzilla->extensions } + +my $extract = \&Bugzilla::Extension::Needinfo::_extract_mentions; + +is([$extract->(undef)], [], 'undef text'); +is([$extract->('no mentions here')], [], 'no mentions'); +is([$extract->('@alice can you look?')], ['alice'], 'leading mention'); +is([$extract->('thanks @alice, @bob and @carol.')], + ['alice', 'bob', 'carol'], 'multiple mentions with punctuation'); +is([$extract->('@alice and @Alice again @alice')], ['alice'], + 'duplicates collapsed case-insensitively'); +is([$extract->('(@dk.l_x-y)')], ['dk.l_x-y'], 'nick symbols, parens'); +is([$extract->('mail foo@example.com or a/@b or x.@y')], [], + 'emails and embedded @ ignored'); +is([$extract->("> \@quoted said\n\@alice")], ['alice'], 'quoted lines ignored'); +is([$extract->('run `@decorator` @alice')], ['alice'], 'inline code ignored'); +is( + [$extract->("```perl\n\@inside\n```\n\@after")], + ['after'], 'fenced code block ignored' +); +is([$extract->("~~~\n\@unclosed\n\@more")], [], 'unclosed fence runs to end'); + +done_testing; From d8b9968d72641555bc11b89db7450f47deeabeda Mon Sep 17 00:00:00 2001 From: David Lawrence Date: Thu, 8 Oct 2026 16:28:00 -0400 Subject: [PATCH 2/4] feat: Bug 2079840 - Add misuse safeguards to @nickname mentions 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. --- extensions/Needinfo/Extension.pm | 33 ++++++++++--- extensions/Needinfo/t/mentions.t | 80 ++++++++++++++++++++++++++++++++ 2 files changed, 106 insertions(+), 7 deletions(-) diff --git a/extensions/Needinfo/Extension.pm b/extensions/Needinfo/Extension.pm index a7c6fa10ff..d867a51d67 100644 --- a/extensions/Needinfo/Extension.pm +++ b/extensions/Needinfo/Extension.pm @@ -20,6 +20,8 @@ use Bugzilla::User::Setting; our $VERSION = '0.01'; +use constant MAX_MENTIONS => 10; + BEGIN { *Bugzilla::User::needinfo_blocked = \&_user_needinfo_blocked; } @@ -297,20 +299,37 @@ sub _users_for_mentions { # GitHub-style mentions: each @nickname in a new comment is CC'd and gets a # needinfo request. Mentions never block the comment from being saved; anyone # who can't be needinfo'd (blocked, already asked, no permission) is only CC'd. +# +# Safeguards against misuse: +# - only editbugs users can trigger mentions; for everyone else they are text +# - only the first MAX_MENTIONS distinct nicknames per update are processed +# - users who can't already see the bug are skipped, so a mention (or a typo, +# or a squatted nickname) can never grant access to a restricted bug sub _process_mentions { my ($bug) = @_; - my $user = Bugzilla->user; - # Lowercased nick => true if mentioned in at least one public comment. my %public; + + # The first MAX_MENTIONS distinct lowercased nicks, in order of appearance. + my @nicks; + foreach my $comment (@{$bug->{added_comments} || []}) { - $public{lc $_} ||= !$comment->{isprivate} - foreach _extract_mentions($comment->{thetext}); + foreach my $nick (map {lc} _extract_mentions($comment->{thetext})) { + push @nicks, $nick if !exists $public{$nick} && @nicks < MAX_MENTIONS; + $public{$nick} ||= !$comment->{isprivate}; + } } + return unless @nicks; + + # Checked after parsing so updates without mentions skip the group lookup. + my $user = Bugzilla->user; + return unless $user->in_group('editbugs', $bug->product_id); - my @mentioned - = grep { $_->id != $user->id && ($public{lc $_->nick} || $_->is_insider) } - _users_for_mentions(keys %public); + my @mentioned = grep { + $_->id != $user->id + && ($public{lc $_->nick} || $_->is_insider) + && $_->can_see_bug($bug->id) + } _users_for_mentions(@nicks); return unless @mentioned; my ($type) = grep { $_->name eq 'needinfo' } @{$bug->flag_types}; diff --git a/extensions/Needinfo/t/mentions.t b/extensions/Needinfo/t/mentions.t index bc937e52ef..00716261fa 100644 --- a/extensions/Needinfo/t/mentions.t +++ b/extensions/Needinfo/t/mentions.t @@ -42,4 +42,84 @@ is( ); is([$extract->("~~~\n\@unclosed\n\@more")], [], 'unclosed fence runs to end'); +# _process_mentions safeguards, using minimal stand-ins for users and bugs. +{ + + package FakeUser; + sub new { my ($class, %args) = @_; bless {%args}, $class } + sub id { $_[0]{id} } + sub nick { $_[0]{nick} } + sub login { "$_[0]{nick}\@example.com" } + sub in_group { $_[0]{editbugs} } + sub is_insider { $_[0]{insider} } + sub can_see_bug { $_[0]{sees_bug} // 1 } + sub needinfo_blocked { 0 } + + package FakeBug; + sub new { + my ($class, $text) = @_; + bless { + added_comments => [{thetext => $text}], + cc => [], + needinfo => [], + type => bless({flags => []}, 'FakeType'), + }, $class; + } + sub id {1} + sub product_id {1} + sub flag_types { [$_[0]{type}] } + sub check_can_change_field { {allowed => 1} } + sub add_cc { push @{$_[0]{cc}}, $_[1]->nick } + sub set_flags { push @{$_[0]{needinfo}}, map { $_->{requestee} } @{$_[2]} } + + package FakeType; + sub name {'needinfo'} + sub id {1} +} + +my %users = map { $_->nick => $_ } ( + FakeUser->new(id => 2, nick => 'alice'), + FakeUser->new(id => 3, nick => 'bob'), + FakeUser->new(id => 4, nick => 'hidden', sees_bug => 0), + map { FakeUser->new(id => 10 + $_, nick => "u$_") } 1 .. 12, +); +my $commenter = FakeUser->new(id => 1, nick => 'me', editbugs => 1); +my @looked_up; + +my $mock_bugzilla = mock 'Bugzilla' => (override => [user => sub {$commenter}]); +my $mock_ext = mock 'Bugzilla::Extension::Needinfo' => ( + override => [ + _users_for_mentions => sub { + @looked_up = @_; + return grep {defined} map { $users{$_} } @_; + }, + ], +); + +my $process = \&Bugzilla::Extension::Needinfo::_process_mentions; + +my $bug = FakeBug->new('@alice and @bob please look, cc @me'); +$process->($bug); +is($bug->{cc}, ['alice', 'bob'], 'mentioned users CCd, self skipped'); +is($bug->{needinfo}, ['alice@example.com', 'bob@example.com'], + 'mentioned users needinfod'); + +$bug = FakeBug->new('@hidden @alice'); +$process->($bug); +is($bug->{cc}, ['alice'], 'user who cannot see the bug is not CCd'); +is($bug->{needinfo}, ['alice@example.com'], '... nor needinfod'); + +$bug = FakeBug->new(join ' ', map {"\@u$_"} 1 .. 12); +$process->($bug); +is(\@looked_up, [map {"u$_"} 1 .. 10], 'only the first 10 nicknames are looked up'); +is($bug->{cc}, [map {"u$_"} 1 .. 10], '... and only they are CCd'); + +$commenter->{editbugs} = 0; +@looked_up = (); +$bug = FakeBug->new('@alice'); +$process->($bug); +is(\@looked_up, [], 'no lookup for commenters without editbugs'); +is($bug->{cc}, [], 'no CC for commenters without editbugs'); +is($bug->{needinfo}, [], 'no needinfo for commenters without editbugs'); + done_testing; From 57c588d1cb1633474ef8ba10d89822205c67de84 Mon Sep 17 00:00:00 2001 From: David Lawrence Date: Thu, 8 Oct 2026 17:13:19 -0400 Subject: [PATCH 3/4] fix: Bug 2079840 - Never let @mentions block a comment from saving - 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. --- extensions/Needinfo/Extension.pm | 56 ++++++++++++++++++++---------- extensions/Needinfo/t/mentions.t | 58 ++++++++++++++++++++++++++++---- 2 files changed, 90 insertions(+), 24 deletions(-) diff --git a/extensions/Needinfo/Extension.pm b/extensions/Needinfo/Extension.pm index d867a51d67..36b510c980 100644 --- a/extensions/Needinfo/Extension.pm +++ b/extensions/Needinfo/Extension.pm @@ -12,9 +12,11 @@ use warnings; use base qw(Bugzilla::Extension); +use Bugzilla::Constants; use Bugzilla::Error; use Bugzilla::Flag; use Bugzilla::FlagType; +use Bugzilla::Logging; use Bugzilla::User; use Bugzilla::User::Setting; @@ -62,7 +64,7 @@ sub install_update_db { is_active => 1, is_requestable => 1, is_requesteeble => 1, - is_multiplicable => 0, + is_multiplicable => 1, request_group => '', grant_group => '', inclusions => ['0:0'], @@ -267,12 +269,15 @@ sub _extract_mentions { return () unless defined $text; $text =~ s/^[ \t]*(```|~~~).*?(?:^[ \t]*\1[^\n]*$|\z)//msg; - $text =~ s/`[^`\n]*`//g; + # A code span closes on a backtick run of the same length, within a paragraph. + $text = join "\n\n", + map { s/(?.*$//mg; my (@nicks, %seen); - while ($text =~ /(?name eq 'needinfo' } @{$bug->flag_types}; - my $can_flag - = $type && $bug->check_can_change_field('flagtypes.name', 0, 1)->{allowed}; - - my @new_flags; + undef $type + unless $type && $bug->check_can_change_field('flagtypes.name', 0, 1)->{allowed}; + + # A failure for one user (e.g. add_cc's strict_isolation check) skips that + # user rather than rolling back the whole update. Throw*Error only dies + # (instead of printing an error page and exiting) in ERROR_MODE_DIE, so the + # eval can catch it. + local Bugzilla->request_cache->{error_mode} = ERROR_MODE_DIE; + local $@; foreach my $mentioned (@mentioned) { - $bug->add_cc($mentioned); + eval { _cc_and_needinfo($bug, $mentioned, $type); 1 } + or WARN('Skipped mention of ' . $mentioned->login . " on bug " + . $bug->id . ": $@"); + } +} - next if !$can_flag || $mentioned->needinfo_blocked; - next if grep { - $_->status eq '?' && ($_->requestee_id // 0) == $mentioned->id - } @{$type->{flags}}; +sub _cc_and_needinfo { + my ($bug, $mentioned, $type) = @_; + $bug->add_cc($mentioned); - push @new_flags, - {type_id => $type->id, status => '?', requestee => $mentioned->login}; - } + return if !$type || $mentioned->needinfo_blocked; + + # A non-multiplicable type allows only one needinfo flag per bug. + return if !$type->is_multiplicable && @{$type->{flags}}; + return if grep { + $_->status eq '?' && ($_->requestee_id // 0) == $mentioned->id + } @{$type->{flags}}; - $bug->set_flags([], \@new_flags) if @new_flags; + # One call per user so $type->{flags} reflects each flag as it is added. + $bug->set_flags([], + [{type_id => $type->id, status => '?', requestee => $mentioned->login}]); } sub _check_requestee { diff --git a/extensions/Needinfo/t/mentions.t b/extensions/Needinfo/t/mentions.t index 00716261fa..dabcd7bcad 100644 --- a/extensions/Needinfo/t/mentions.t +++ b/extensions/Needinfo/t/mentions.t @@ -36,6 +36,11 @@ is([$extract->('mail foo@example.com or a/@b or x.@y')], [], 'emails and embedded @ ignored'); is([$extract->("> \@quoted said\n\@alice")], ['alice'], 'quoted lines ignored'); is([$extract->('run `@decorator` @alice')], ['alice'], 'inline code ignored'); +is([$extract->('``@alice`` and `` a`@b ``')], [], 'double backtick code ignored'); +is([$extract->("it's a ` stray\n\n\@alice `x`")], + ['alice'], 'unclosed backtick does not span paragraphs'); +is([$extract->('@foo|bar and |@alice|')], ['foo|bar', 'alice'], + 'pipe allowed in nick, trailing pipe stripped'); is( [$extract->("```perl\n\@inside\n```\n\@after")], ['after'], 'fenced code block ignored' @@ -57,30 +62,49 @@ is([$extract->("~~~\n\@unclosed\n\@more")], [], 'unclosed fence runs to end'); package FakeBug; sub new { - my ($class, $text) = @_; + my ($class, $text, %type) = @_; bless { added_comments => [{thetext => $text}], cc => [], needinfo => [], - type => bless({flags => []}, 'FakeType'), + type => bless({flags => [], multiplicable => 1, %type}, 'FakeType'), }, $class; } sub id {1} sub product_id {1} sub flag_types { [$_[0]{type}] } sub check_can_change_field { {allowed => 1} } - sub add_cc { push @{$_[0]{cc}}, $_[1]->nick } - sub set_flags { push @{$_[0]{needinfo}}, map { $_->{requestee} } @{$_[2]} } + sub add_cc { + my ($self, $user) = @_; + die "add_cc failed\n" if $user->{cc_fails}; + push @{$self->{cc}}, $user->nick; + } + + # Like Bugzilla::Flag->set_flag, new flags are added to the type's list. + sub set_flags { + my ($self, undef, $new_flags) = @_; + foreach my $flag (@$new_flags) { + push @{$self->{needinfo}}, $flag->{requestee}; + push @{$self->{type}{flags}}, + bless({status => '?', requestee_id => 0}, 'FakeFlag'); + } + } package FakeType; - sub name {'needinfo'} - sub id {1} + sub name {'needinfo'} + sub id {1} + sub is_multiplicable { $_[0]{multiplicable} } + + package FakeFlag; + sub status { $_[0]{status} } + sub requestee_id { $_[0]{requestee_id} } } my %users = map { $_->nick => $_ } ( FakeUser->new(id => 2, nick => 'alice'), FakeUser->new(id => 3, nick => 'bob'), FakeUser->new(id => 4, nick => 'hidden', sees_bug => 0), + FakeUser->new(id => 6, nick => 'boom', cc_fails => 1), map { FakeUser->new(id => 10 + $_, nick => "u$_") } 1 .. 12, ); my $commenter = FakeUser->new(id => 1, nick => 'me', editbugs => 1); @@ -114,6 +138,28 @@ $process->($bug); is(\@looked_up, [map {"u$_"} 1 .. 10], 'only the first 10 nicknames are looked up'); is($bug->{cc}, [map {"u$_"} 1 .. 10], '... and only they are CCd'); +$bug = FakeBug->new('@alice @bob', multiplicable => 0); +$process->($bug); +is($bug->{cc}, ['alice', 'bob'], 'non-multiplicable type: everyone CCd'); +is($bug->{needinfo}, ['alice@example.com'], '... but only one needinfo'); + +$bug = FakeBug->new('@alice', multiplicable => 0); +push @{$bug->{type}{flags}}, bless({status => '?', requestee_id => 9}, 'FakeFlag'); +$process->($bug); +is($bug->{cc}, ['alice'], 'non-multiplicable with existing flag: CCd'); +is($bug->{needinfo}, [], '... and no needinfo, instead of an error'); + +$bug = FakeBug->new('@alice'); +push @{$bug->{type}{flags}}, bless({status => '?', requestee_id => 2}, 'FakeFlag'); +$process->($bug); +is($bug->{needinfo}, [], 'no duplicate needinfo for a pending request'); + +$bug = FakeBug->new('@boom @alice'); +ok(lives { $process->($bug) }, + 'a failing CC (e.g. strict_isolation) does not throw'); +is($bug->{cc}, ['alice'], '... that user is skipped'); +is($bug->{needinfo}, ['alice@example.com'], '... others still processed'); + $commenter->{editbugs} = 0; @looked_up = (); $bug = FakeBug->new('@alice'); From fae49e47b987f54b18a534a36776d84cc342815d Mon Sep 17 00:00:00 2001 From: David Lawrence Date: Thu, 8 Oct 2026 17:35:39 -0400 Subject: [PATCH 4/4] Potential fix for pull request finding 'Capture full fence length to prevent premature code block closure' Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- extensions/Needinfo/Extension.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extensions/Needinfo/Extension.pm b/extensions/Needinfo/Extension.pm index 36b510c980..4b6051d390 100644 --- a/extensions/Needinfo/Extension.pm +++ b/extensions/Needinfo/Extension.pm @@ -268,7 +268,7 @@ sub _extract_mentions { my ($text) = @_; return () unless defined $text; - $text =~ s/^[ \t]*(```|~~~).*?(?:^[ \t]*\1[^\n]*$|\z)//msg; + $text =~ s/^[ \t]*(`{3,}|~{3,}).*?(?:^[ \t]*\1[^\n]*$|\z)//msg; # A code span closes on a backtick run of the same length, within a paragraph. $text = join "\n\n", map { s/(?