Skip to content

Commit e4dc69c

Browse files
committed
Ensure RT-Attach headers are validated when they are configured
Previously, we did not verify whether these attachment ids exist or if the current user has permission to view them. Although RT::Action::SendEmail performs similar checks, it is still beneficial to validate them beforehand.
1 parent 17c79b6 commit e4dc69c

2 files changed

Lines changed: 25 additions & 6 deletions

File tree

lib/RT/Ticket.pm

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1718,9 +1718,22 @@ sub _RecordNote {
17181718
$args{'AttachExisting'} = [$args{'AttachExisting'}]
17191719
if not ref $args{'AttachExisting'} eq 'ARRAY';
17201720

1721-
for my $attach (@{$args{'AttachExisting'}}) {
1722-
next if $attach =~ /\D/;
1723-
$args{'MIMEObj'}->head->add( 'RT-Attach' => $attach );
1721+
for my $attach_id (@{$args{'AttachExisting'}}) {
1722+
next if !$attach_id || $attach_id =~ /\D/;
1723+
my $attach = RT::Attachment->new( $self->CurrentUser );
1724+
$attach->Load($attach_id);
1725+
if ( $attach->Id ) {
1726+
if ( $attach->CurrentUserCanSee ) {
1727+
$args{'MIMEObj'}->head->add( 'RT-Attach' => $attach_id );
1728+
}
1729+
else {
1730+
RT->Logger->warning(
1731+
'User ' . $self->CurrentUser->Name . " does not have permission to view attachment #$attach_id" );
1732+
}
1733+
}
1734+
else {
1735+
RT->Logger->warning("Couldn't load attachment #$attach_id");
1736+
}
17241737
}
17251738
}
17261739

t/web/attach-from-txn.t

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
use strict;
22
use warnings;
33

4-
use RT::Test tests => 70;
4+
use RT::Test tests => undef;
5+
use Test::Warn;
56

67
my $LogoName = 'image.png';
78
my $ImageName = 'owls.jpg';
@@ -172,13 +173,18 @@ my $ticket = RT::Ticket->new($peter);
172173
$ticket->Load(1);
173174
ok $ticket->Id, "loaded ticket";
174175

175-
my ($ok, $msg, $txn) = $ticket->Correspond( AttachExisting => $LogoId, Content => 'Hi' );
176+
my ( $ok, $msg );
177+
warnings_like {
178+
( $ok, $msg ) = $ticket->Correspond( AttachExisting => $LogoId, Content => 'Hi' );
179+
} [qr/User peter does not have permission to view attachment #$LogoId/], 'Invalid attachment';
176180
ok $ok, $msg;
177181

178182
# check mail that went out doesn't contain the logo
179183
@mails = RT::Test->fetch_caught_mails;
180184
is scalar @mails, 1, "got one outgoing email";
181185
$mail = shift @mails;
182-
like $mail, qr/RT-Attach: $LogoId/, "found header we expected";
186+
unlike $mail, qr/RT-Attach: $LogoId/, "lacks header we expected";
183187
unlike $mail, qr/RT-Attachment: \d+\/\d+\/$LogoId/, "lacks RT-Attachment header";
184188
unlike $mail, qr/filename=.?\Q$LogoName\E.?/, "lacks filename";
189+
190+
done_testing;

0 commit comments

Comments
 (0)