diff --git a/0001-AMQP-allow-passing-headers-to-publish_amqp.patch b/0001-AMQP-allow-passing-headers-to-publish_amqp.patch new file mode 100644 index 0000000..f5d4ad8 --- /dev/null +++ b/0001-AMQP-allow-passing-headers-to-publish_amqp.patch @@ -0,0 +1,134 @@ +From ef621c739aa892985937b7a2260111a102d98847 Mon Sep 17 00:00:00 2001 +From: Adam Williamson +Date: Wed, 24 Jul 2019 11:40:10 -0700 +Subject: [PATCH] AMQP: allow passing headers to publish_amqp + +Fedora's new AMQP-based messaging system requires messages to +have certain headers: + +https://fedora-messaging.readthedocs.io/en/stable/wire-format.html + +I'm planning to write a plugin for emitting Fedora-compliant +AMQP messages as a subclass of the existing AMQP plugin, but in +order to be able to use `publish_amqp` in that plugin, I need to +be able to pass it headers like this. Otherwise I'd have to +reimplement it. + +This implementation aims to ensure it works just as before if +you *don't* want to pass it headers, and without needing to +duplicate the `publish_p` calls for "headers" and "no headers" +cases - that's the reason for making `$headers` a reference to +an empty hash if it's not defined, and then checking that it's +a hashref: to be sure about what we're asking `publish_p` to do. +Passing it a ref to an empty hash should effectively be a no-op +so far as headers are concerned, and not interfere with passing +`routing_key` as a param. + +Signed-off-by: Adam Williamson +--- + lib/OpenQA/WebAPI/Plugin/AMQP.pm | 6 ++-- + t/23-amqp.t | 59 ++++++++++++++++++++++++++++++++ + 2 files changed, 63 insertions(+), 2 deletions(-) + +diff --git a/lib/OpenQA/WebAPI/Plugin/AMQP.pm b/lib/OpenQA/WebAPI/Plugin/AMQP.pm +index e46d7915..7553bc3c 100644 +--- a/lib/OpenQA/WebAPI/Plugin/AMQP.pm ++++ b/lib/OpenQA/WebAPI/Plugin/AMQP.pm +@@ -71,7 +71,9 @@ sub log_event { + } + + sub publish_amqp { +- my ($self, $topic, $event_data) = @_; ++ my ($self, $topic, $event_data, $headers) = @_; ++ $headers //= {}; ++ die "publish_amqp headers must be a hashref!" unless (ref($headers) eq 'HASH'); + + log_debug("Sending AMQP event: $topic"); + my $publisher = Mojo::RabbitMQ::Client::Publisher->new( +@@ -79,7 +81,7 @@ sub publish_amqp { + + # A hard reference to the publisher object needs to be kept until the event + # has been published asynchronously, or it gets destroyed too early +- $publisher->publish_p($event_data, routing_key => $topic)->then( ++ $publisher->publish_p($event_data, $headers, routing_key => $topic)->then( + sub { + log_debug "$topic published"; + } +diff --git a/t/23-amqp.t b/t/23-amqp.t +index b6d6f68d..4b7d82f3 100644 +--- a/t/23-amqp.t ++++ b/t/23-amqp.t +@@ -29,6 +29,7 @@ use lib "$FindBin::Bin/lib"; + use OpenQA::Client; + use OpenQA::Jobs::Constants; + use OpenQA::Test::Database; ++use Test::Exception; + use Test::MockModule; + use Test::More; + use Test::Mojo; +@@ -189,4 +190,62 @@ subtest 'create parent group comment' => sub { + is($json->{parent_group_id}, 2000, 'parent group id'); + }; + ++# Now let's unmock publish_amqp so we can test it... ++$plugin_mock->unmock('publish_amqp'); ++%published = (); ++# ...but we'll mock the thing it calls. ++my $publisher_mock = Test::MockModule->new('Mojo::RabbitMQ::Client::Publisher'); ++$publisher_mock->mock( ++ publish_p => sub { ++ # copied from upstream git master as of 2019-07-24 ++ my $self = shift; ++ my $body = shift; ++ my $headers = {}; ++ my %args = (); ++ ++ if (ref($_[0]) eq 'HASH') { ++ $headers = shift; ++ } ++ if (@_) { ++ %args = (@_); ++ } ++ # end copying ++ $published{body} = $body; ++ $published{headers} = $headers; ++ $published{args} = \%args; ++ # we need to return a Promise or stuff breaks ++ my $client_promise = Mojo::Promise->new(); ++ return $client_promise; ++ }); ++ ++# we need an instance of the plugin now. I can't find a documented ++# way to access the one that's already loaded... ++my $amqp = OpenQA::WebAPI::Plugin::AMQP->new; ++$amqp->register($app); ++ ++subtest 'amqp_publish call without headers' => sub { ++ $amqp->publish_amqp('some.topic', 'some message'); ++ is($published{body}, 'some message', "message body correctly passed"); ++ is_deeply($published{headers}, {}, "headers is empty hashref"); ++ is_deeply($published{args}->{routing_key}, 'some.topic', "topic appears as routing key"); ++}; ++ ++subtest 'amqp_publish call with headers' => sub { ++ %published = (); ++ $amqp->publish_amqp('some.topic', 'some message', {'someheader' => 'something'}); ++ is($published{body}, 'some message', "message body correctly passed"); ++ is_deeply($published{headers}, {'someheader' => 'something'}, "headers is expected hashref"); ++ is_deeply($published{args}->{routing_key}, 'some.topic', "topic appears as routing key"); ++}; ++ ++subtest 'amqp_publish call with incorrect headers' => sub { ++ throws_ok( ++ sub { ++ $amqp->publish_amqp('some.topic', 'some message', 'some headers'); ++ }, ++ qr/publish_amqp headers must be a hashref!/, ++ 'dies on bad headers' ++ ); ++}; ++ + done_testing(); +-- +2.22.0 + diff --git a/0001-Improve-rendering-summary-in-test-result-overview.patch b/0001-Improve-rendering-summary-in-test-result-overview.patch new file mode 100644 index 0000000..078e68d --- /dev/null +++ b/0001-Improve-rendering-summary-in-test-result-overview.patch @@ -0,0 +1,190 @@ +From c172e8883d8f32fced5e02f9b6faaacc913df27b Mon Sep 17 00:00:00 2001 +From: Marius Kittler +Date: Mon, 22 Jul 2019 13:35:41 +0200 +Subject: [PATCH] Improve rendering summary in test result overview + +--- + lib/OpenQA/WebAPI/Controller/Test.pm | 73 ++++++++++++++++++---------- + t/10-tests_overview.t | 19 ++++++++ + t/ui/10-tests_overview.t | 2 +- + templates/test/overview.html.ep | 4 +- + 4 files changed, 69 insertions(+), 29 deletions(-) + +diff --git a/lib/OpenQA/WebAPI/Controller/Test.pm b/lib/OpenQA/WebAPI/Controller/Test.pm +index 33cb5a4b..9de2ec9b 100644 +--- a/lib/OpenQA/WebAPI/Controller/Test.pm ++++ b/lib/OpenQA/WebAPI/Controller/Test.pm +@@ -1,4 +1,4 @@ +-# Copyright (C) 2015-2016 SUSE LLC ++# Copyright (C) 2015-2019 SUSE LLC + # + # This program is free software; you can redistribute it and/or modify + # it under the terms of the GNU General Public License as published by +@@ -22,9 +22,10 @@ use OpenQA::Jobs::Constants; + use OpenQA::Schema::Result::Jobs; + use OpenQA::Schema::Result::JobDependencies; + use OpenQA::WebAPI::Controller::Developer; ++use OpenQA::Utils qw(determine_web_ui_web_socket_url get_ws_status_only_url); ++use Mojo::ByteStream; + use File::Basename; + use POSIX 'strftime'; +-use OpenQA::Utils qw(determine_web_ui_web_socket_url get_ws_status_only_url); + + sub referer_check { + my ($self) = @_; +@@ -569,6 +570,31 @@ sub prepare_job_results { + return (\%archs, \%results, $aggregated); + } + ++# appends the specified $distri and $version to $array_to_add_parts_to as string or if $raw as Mojo::ByteStream ++sub _add_distri_and_version_to_summary { ++ my ($array_to_add_parts_to, $distri, $version, $raw) = @_; ++ ++ for my $part ($distri, $version) { ++ # handle case when multiple distri/version parameters have been specified ++ $part = $part->{-in} if (ref $part eq 'HASH'); ++ next unless $part; ++ ++ # separate distri and version with a whitespace ++ push(@$array_to_add_parts_to, ' ') if (@$array_to_add_parts_to); ++ ++ if (ref $part eq 'ARRAY') { ++ # separate multiple distris/versions using a slash ++ if (@$part) { ++ push(@$array_to_add_parts_to, map { ($raw ? Mojo::ByteStream->new($_) : $_, '/') } @$part); ++ pop(@$array_to_add_parts_to); ++ } ++ } ++ elsif (ref $part ne 'HASH') { ++ push(@$array_to_add_parts_to, $raw ? Mojo::ByteStream->new($part) : $part); ++ } ++ } ++} ++ + # A generic query page showing test results in a configurable matrix + sub overview { + my ($self) = @_; +@@ -585,43 +611,38 @@ sub overview { + ($stash{archs}, $stash{results}, $stash{aggregated}) = $self->prepare_job_results(\@latest_jobs); + + # determine distri/version from job results if not explicitely specified via search args +- my @distris = keys %{$stash{results}}; ++ my @distris = keys %{$stash{results}}; ++ my $formatted_distri; ++ my $formatted_version; + my $only_distri = scalar @distris == 1; + if (!defined $stash{distri} && $only_distri) { +- my $distri = $stash{distri} = $distris[0]; ++ $formatted_distri = $distris[0]; + if (!defined $stash{version}) { +- my @versions = keys %{$stash{results}->{$distri}}; +- $stash{version} = $versions[0] if (scalar @versions == 1); ++ my @versions = keys %{$stash{results}->{$formatted_distri}}; ++ $formatted_version = $versions[0] if (scalar @versions == 1); + } + } + +- # determine summary name for "Overall Summary of ..." +- my $summary_name; ++ # compose summary for "Overall Summary of ..." ++ my @summary_parts; + if (@$groups) { +- $summary_name = join(', ', +- map { $self->link_to($_->name => $self->url_for('group_overview', groupid => $_->id)) } @$groups); ++ # use groups if present ++ push(@summary_parts, ++ map { ($self->link_to($_->name => $self->url_for('group_overview', groupid => $_->id)), ', ') } @$groups); ++ pop(@summary_parts); + } + else { +- my @variables = ($stash{distri}, $stash{version}); +- my @formatted_parts; +- for my $part (@variables) { +- $part = $part->{-in} if (ref $part eq 'HASH'); +- next unless $part; +- +- if (ref $part eq 'ARRAY') { +- push(@formatted_parts, join('/', @$part)); +- } +- elsif (ref $part ne 'HASH') { +- push(@formatted_parts, $part); +- } +- } +- $summary_name = join(' ', @formatted_parts) if (@formatted_parts); ++ # add pre-formatted distri and version as Mojo::ByteStream ++ _add_distri_and_version_to_summary(\@summary_parts, $formatted_distri, $formatted_version, 1); ++ ++ # add distri and version from query parameters as regular strings ++ _add_distri_and_version_to_summary(\@summary_parts, $stash{distri}, $stash{version}, 0); + } + + $self->stash( + %stash, +- summary_name => $summary_name, +- only_distri => $only_distri, ++ summary_parts => \@summary_parts, ++ only_distri => $only_distri, + ); + $self->respond_to( + json => {json => \%stash}, +diff --git a/t/10-tests_overview.t b/t/10-tests_overview.t +index f6d9d294..e8f60a7c 100644 +--- a/t/10-tests_overview.t ++++ b/t/10-tests_overview.t +@@ -65,6 +65,25 @@ like(get_summary, qr/Overall Summary of opensuse 13\.1 build 0091/i, 'specifying + $form = {distri => 'opensuse', version => '13.1', build => '0091', groupid => 1001}; + $t->get_ok('/tests/overview' => form => $form)->status_is(200); + like(get_summary, qr/Overall Summary of opensuse build 0091/i, 'specifying groupid parameter'); ++subtest 'escaping works' => sub { ++ $form = { ++ distri => '', ++ version => ['', ''], ++ build => '' ++ }; ++ $t->get_ok('/tests/overview' => form => $form)->status_is(200); ++ my $body = $t->tx->res->body; ++ unlike($body, qr//, 'no unescaped image tag for distri'); ++ unlike($body, qr/.*/, 'no unescaped image tags for version'); ++ unlike($body, qr//, 'no unescaped image tag for build'); ++ like($body, qr/<img src="distri">/, 'image tag for distri escaped'); ++ like( ++ $body, ++ qr/<img src="version1">.*<img src="version2">/, ++ 'image tags for version escaped' ++ ); ++ like($body, qr/<img src="build">/, 'image tag for build escaped'); ++}; + + # + # Overview of build 0048 +diff --git a/t/ui/10-tests_overview.t b/t/ui/10-tests_overview.t +index 1e83f93d..ea78e440 100644 +--- a/t/ui/10-tests_overview.t ++++ b/t/ui/10-tests_overview.t +@@ -208,7 +208,7 @@ subtest 'filtering by distri' => sub { + $driver->get('/tests/overview?distri=foo&distri=opensuse&distri=bar&version=13.1&build=0091'); + check_build_0091_defaults; + is( +- OpenQA::Test::Case::trim_whitespace($driver->find_element('#summary .card-header b')->get_text()), ++ OpenQA::Test::Case::trim_whitespace($driver->find_element('#summary .card-header strong')->get_text()), + 'foo/opensuse/bar 13.1', + 'filter also visible in summary' + ); +diff --git a/templates/test/overview.html.ep b/templates/test/overview.html.ep +index f618337e..2d5cb72f 100644 +--- a/templates/test/overview.html.ep ++++ b/templates/test/overview.html.ep +@@ -11,8 +11,8 @@ +
+
+ Overall Summary of +- % if ($summary_name) { +- <%= b $summary_name %> ++ % if (@$summary_parts) { ++ <% for my $part (@$summary_parts) { %><%= $part %><% } %> + % } + % else { + multiple distri/version +-- +2.22.0 + diff --git a/openqa.spec b/openqa.spec index 4cba9dc..354d95f 100644 --- a/openqa.spec +++ b/openqa.spec @@ -46,7 +46,7 @@ Name: openqa Version: %{github_version} -Release: 17%{?github_date:.%{github_date}git%{shortcommit}}%{?dist} +Release: 18%{?github_date:.%{github_date}git%{shortcommit}}%{?dist} Summary: OS-level automated testing framework License: GPLv2+ Url: http://os-autoinst.github.io/openQA/ @@ -68,6 +68,12 @@ Patch0: 2192.patch # https://github.com/os-autoinst/openQA/pull/2205 # fixes another test failure Patch1: 0001-t-api-03-auth.t-don-t-actually-wipe-asset-files-from.patch +# https://github.com/os-autoinst/openQA/pull/2217 +# Allow sending headers when publishing AMQP +Patch2: 0001-AMQP-allow-passing-headers-to-publish_amqp.patch +# https://github.com/os-autoinst/openQA/pull/2213 +# very unimportant definitely not security fixes +Patch3: 0001-Improve-rendering-summary-in-test-result-overview.patch BuildRequires: %{t_requires} BuildRequires: perl-generators @@ -528,6 +534,10 @@ fi %{_datadir}/openqa/lib/OpenQA/WebAPI/Plugin/FedoraUpdateRestart.pm %changelog +* Thu Jul 25 2019 Adam Williamson - 4.6-18.20190716git5bfa647 +- Backport PR #2213 (fixes vulnerability to maliciously-formed API requests) +- Backport PR #2217 (allow passing headers to publish_amqp) + * Thu Jul 25 2019 Fedora Release Engineering - 4.6-17.20190716git5bfa647 - Rebuilt for https://fedoraproject.org/wiki/Fedora_31_Mass_Rebuild