Skip to content

WIP Allow running a tainted runner #264

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .github/workflows/linux.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ jobs:
fail-fast: false
matrix:
perl-version:
- '5.36'
- '5.34'
- '5.32'
- '5.30'
- '5.28'
- '5.26'
Expand Down
4 changes: 2 additions & 2 deletions lib/App/Yath.pm
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use Test2::Harness::Util::HashBase qw{
use Time::HiRes qw/time/;

use App::Yath::Util qw/find_pfile/;
use Test2::Harness::Util qw/find_libraries clean_path/;
use Test2::Harness::Util qw/find_libraries clean_path mod2file/;
use App::Yath::Options();
use Scalar::Util qw/blessed/;

Expand Down Expand Up @@ -286,7 +286,7 @@ sub load_command {
my ($cmd_name, %params) = @_;

my $cmd_class = "App::Yath::Command::$cmd_name";
my $cmd_file = "App/Yath/Command/$cmd_name.pm";
my $cmd_file = mod2file($cmd_class);

return $cmd_class if eval { require $cmd_file; 1 };
my $error = $@ || 'unknown error';
Expand Down
14 changes: 10 additions & 4 deletions lib/App/Yath/Command/test.pm
Original file line number Diff line number Diff line change
Expand Up @@ -874,20 +874,26 @@ sub start_runner {

my $settings = $self->settings;
my $dir = $settings->workspace->workdir;
my @cmd = $^X;
my %env;

my @prof;
if ($settings->runner->nytprof) {
push @prof => '-d:NYTProf';
push @cmd => '-d:NYTProf';
$env{NYTPROF} = 'start=no:addpid=1';
}

if ($settings->runner->taint) {
push @cmd => '-T';
}

my $ipc = $self->ipc;
my $proc = $ipc->spawn(
stderr => File::Spec->catfile($dir, 'error.log'),
stdout => File::Spec->catfile($dir, 'output.log'),
env_vars => { @prof ? (NYTPROF => 'start=no:addpid=1') : () },
env_vars => \%env,
no_set_pgrp => 1,
command => [
$^X, @prof, $self->spawn_args($settings), $settings->harness->script,
@cmd, $self->spawn_args($settings), $settings->harness->script,
(map { "-D$_" } @{$settings->harness->dev_libs}),
'--no-scan-plugins', # Do not preload any plugin modules
runner => $dir,
Expand Down
6 changes: 6 additions & 0 deletions lib/App/Yath/Options/Runner.pm
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,12 @@ option_group {prefix => 'runner', category => "Runner Options"} => sub {
default => sub { gen_uuid() },
description => 'Runner ID (usually a generated uuid)',
);

option taint => (
type => 'b',
default => 0,
description => '(Default: off) Enable taint mode for the runner. This is very useful when combined with preload and tests that contain a taint flag in their shebang.',
);
};

sub jobs_post_process {
Expand Down
4 changes: 2 additions & 2 deletions lib/Test2/Formatter/Stream.pm
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use List::Util qw/first/;

use Test2::Harness::Util::UUID qw/gen_uuid/;
use Test2::Harness::Util::JSON qw/JSON JSON_IS_XS/;
use Test2::Harness::Util qw/hub_truth apply_encoding/;
use Test2::Harness::Util qw/hub_truth apply_encoding untaint/;

use Test2::Util qw/get_tid ipc_separator/;

Expand Down Expand Up @@ -89,7 +89,7 @@ sub fh {
$pid = $self->{+_PID} = $$;
$tid = $self->{+_TID} = get_tid();

my $file = File::Spec->catfile($dir, join(ipc_separator() => 'events', $pid, $tid) . ".jsonl");
my $file = untaint(File::Spec->catfile($dir, join(ipc_separator() => 'events', $pid, $tid) . ".jsonl"));

my @now = ($<, $>, $(, $));
local ($<, $>, $(, $)) = @{$self->{+UGIDS}} if $self->{+UGIDS} && first { $self->{+UGIDS}->[$_] ne $now[$_] } 0 .. $#now;
Expand Down
4 changes: 2 additions & 2 deletions lib/Test2/Harness/Runner.pm
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use Long::Jump qw/setjump longjump/;
use Time::HiRes qw/sleep time/;
use Scope::Guard;

use Test2::Harness::Util qw/clean_path file2mod mod2file open_file parse_exit write_file_atomic process_includes chmod_tmp write_file/;
use Test2::Harness::Util qw/clean_path file2mod mod2file open_file parse_exit write_file_atomic process_includes chmod_tmp write_file untaint/;
use Test2::Harness::Util::Queue();
use Test2::Harness::Util::JSON(qw/encode_json/);

Expand Down Expand Up @@ -97,7 +97,7 @@ sub init {
$self->{+SIGNAL} = $sig;
};

my $tmp_dir = File::Spec->catdir($self->{+DIR}, 'tmp');
my $tmp_dir = untaint(File::Spec->catdir($self->{+DIR}, 'tmp'));
unless (-d $tmp_dir) {
mkdir($tmp_dir) or die "Could not create temp dir: $!";
chmod_tmp($tmp_dir);
Expand Down
21 changes: 17 additions & 4 deletions lib/Test2/Harness/Runner/Job.pm
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use Time::HiRes qw/time/;
use File::Spec();
use File::Temp();

use Test2::Harness::Util qw/fqmod clean_path write_file_atomic write_file mod2file open_file parse_exit process_includes chmod_tmp/;
use Test2::Harness::Util qw/fqmod clean_path write_file_atomic write_file mod2file open_file parse_exit process_includes chmod_tmp untaint/;
use Test2::Harness::IPC;

use parent 'Test2::Harness::IPC::Process';
Expand Down Expand Up @@ -354,7 +354,7 @@ sub job_dir {
my $self = shift;
return $self->{+JOB_DIR} if $self->{+JOB_DIR};

my $job_dir = File::Spec->catdir($self->run_dir, $self->{+TASK}->{job_id} . '+' . $self->is_try);
my $job_dir = untaint(File::Spec->catdir($self->run_dir, $self->{+TASK}->{job_id} . '+' . $self->is_try));
mkdir($job_dir) or die "$$ $0 Could not create job directory '$job_dir': $!";
chmod_tmp($job_dir);
$self->{+JOB_DIR} = $job_dir;
Expand Down Expand Up @@ -415,8 +415,21 @@ sub use_fork {
return $self->{+USE_FORK} = 0 if defined($task->{use_fork}) && !$task->{use_fork};
return $self->{+USE_FORK} = 0 if defined($task->{use_preload}) && !$task->{use_preload};

# -w switch is ok, otherwise it is a no-go
return $self->{+USE_FORK} = 0 if grep { !m/\s*-w\s*/ } $self->switches;
# Ugh I hate this logic!!!

# This approach won't scale if we allow even more swiches.
my @allowed_switches = '-w';

# Allow taint and taint + warnings if we're a tainted runner.
push @allowed_switches => qw/-T -wT -Tw/ if ${^TAINT};

my $allowed_switches = join '|', map { quotemeta } @allowed_switches;
my $allowed_switches_re = qr/\s*(?:$allowed_switches)\s*/;

return $self->{+USE_FORK} = 0 if grep { $_ !~ $allowed_switches_re } $self->switches;

# We're running under the taint but the test hasn't requested taint.
return $self->{+USE_FORK} = 0 if ${^TAINT} && !grep { /\s*-w?Tw?\s*/ } $self->switches;

my $runner = $self->{+RUNNER};
return $self->{+USE_FORK} = 0 unless $runner->use_fork;
Expand Down
13 changes: 9 additions & 4 deletions lib/Test2/Harness/Util.pm
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ our @EXPORT_OK = qw{

looks_like_uuid
is_same_file

untaint
};

sub is_same_file {
Expand Down Expand Up @@ -115,7 +117,7 @@ sub process_includes {

confess "Invalid parameters: " . join(', ' => sort keys %params) if keys %params;

return @list;
return map { untaint($_) } @list;
}

sub apply_encoding {
Expand Down Expand Up @@ -212,7 +214,7 @@ sub open_file {
}
}

open(my $fh, $mode, $file) or confess "Could not open file '$file' ($mode): $!";
open(my $fh, $mode, untaint($file)) or confess "Could not open file '$file' ($mode): $!";
return $fh;
}

Expand All @@ -232,6 +234,7 @@ sub close_file {
sub write_file_atomic {
my ($file, @content) = @_;

$file = untaint($file);
my $pend = "$file.pend";

my ($ok, $err) = try_sig_mask {
Expand All @@ -253,7 +256,7 @@ sub lock_file {
$fh = $file;
}
else {
open($fh, $mode // '>>', $file) or die "Could not open file '$file': $!";
open($fh, $mode // '>>', untaint($file)) or die "Could not open file '$file': $!";
}

for (1 .. 21) {
Expand Down Expand Up @@ -293,7 +296,7 @@ sub mod2file {
my $file = $mod;
$file =~ s{::}{/}g;
$file .= ".pm";
return $file;
return untaint($file);
}

sub file2mod {
Expand Down Expand Up @@ -372,6 +375,8 @@ sub find_libraries {
return \%out;
}

*untaint = ${^TAINT} ? sub { $_[0] =~ /(.*)/; $1 } : sub { $_[0] };

1;

__END__
Expand Down
4 changes: 3 additions & 1 deletion lib/Test2/Harness/Util/IPC.pm
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ our $VERSION = '1.000152';
use Cwd qw/getcwd/;
use Config qw/%Config/;
use Test2::Util qw/CAN_REALLY_FORK/;
use Test2::Harness::Util qw/untaint/;

use Importer Importer => 'import';

Expand Down Expand Up @@ -80,6 +81,7 @@ sub _run_cmd_fork {
$_->() for @{$params{run_in_child} // []};
}
%ENV = (%ENV, %{$params{env}}) if $params{env};
$_ = untaint($_) for values %ENV;
setpgrp(0, 0) if USE_P_GROUPS && !$params{no_set_pgrp};

$cmd = [$cmd->()] if ref($cmd) eq 'CODE';
Expand Down Expand Up @@ -108,7 +110,7 @@ sub _run_cmd_fork {
swap_io(\*STDIN, $stdin, $die) if $stdin;
open(STDIN, "<", "/dev/null") if !$stdin;

@$cmd = map { ref($_) eq 'CODE' ? $_->() : $_ } @$cmd;
@$cmd = map { untaint($_) } map { ref($_) eq 'CODE' ? $_->() : $_ } @$cmd;

exec(@$cmd) or $die->("Failed to exec!");
}
Expand Down
3 changes: 3 additions & 0 deletions t/integration/reload_syntax_error.t
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ use Test2::Util qw/CAN_REALLY_FORK/;
skip_all "Cannot fork, skipping preload test"
if $ENV{T2_NO_FORK} || !CAN_REALLY_FORK;

# yath-runner /__w/Test2-Harness/Test2-Harness/t/integration/reload_syntax_error.t did not respond to SIGTERM, sending SIGKILL to 1019...
skip_all "Currently borked on CI";

my $tx = __FILE__ . 'x';

my $tmpdir = tempdir(CLEANUP => 1);
Expand Down
6 changes: 6 additions & 0 deletions t/unit/Test2/Harness/TestFile.t
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ my $tmp = gen_temp(
notime => "#HARNESS-NO-TIMEOUT\n",
warn => "#!/usr/bin/perl -w\n",
taint => "#!/usr/bin/env perl -t -w\n",
bundle => "#!perl -Tw\n",
foo => "#HARNESS-CATEGORY-FOO\n#HARNESS-STAGE-FoO",
meta => "#HARNESS-META-mykey-myval\n# HARNESS-META-otherkey-otherval\n# HARNESS-META mykey my-val2\n# HARNESS-META slack #my-val # comment after harness statement\n",

Expand Down Expand Up @@ -96,6 +97,11 @@ subtest package => sub {
is($one->queue_item(42)->{use_preload}, 0, "No preload");
};

subtest bundle => sub {
my $bundle = $CLASS->new(file => File::Spec->catfile($tmp, 'bundle'));
is($bundle->switches, ['-Tw'], "Bundled switches");
};

subtest taint => sub {
my $taint = $CLASS->new(file => File::Spec->catfile($tmp, 'taint'), queue_args => [via => ['xxx']]);

Expand Down