Skip to content

Conversation

@JRaspass
Copy link
Contributor

STILL WIP. DO NOT MERGE

This allows tainted tests to benefit from the preload feature, fixes #262.

@exodist
Copy link
Member

exodist commented Feb 25, 2023

I am not bothered by the untaint, this proof of concept looks good. It will need a t/integration test that verifies it, as well as the cleanup and polish you already mentioned needing.

The one thing I would change about untaint() is that I would make it so that at load time it checks if taint mode is enabled, if it is then define sub untaint as you have, but if taint mode is not enabled just return the argument and skip the regex, as a minor performance enhancement when taint is not used.

The test is essential becaus eI will almost certainly break this feature without a test to save me from that. The test should skip on platforms/perls where taint mode is not available.

@JRaspass JRaspass force-pushed the master branch 4 times, most recently from 139efce to c19d90f Compare February 28, 2023 21:42
@JRaspass JRaspass force-pushed the master branch 2 times, most recently from 2b43beb to bd884cd Compare March 20, 2023 18:09
Adds a `--taint` flag to run the runner under taint mode. Only tests
with a taint argument in their shebang will be subject to preloading.
@JRaspass
Copy link
Contributor Author

JRaspass commented Jun 3, 2025

@exodist So I've tried writing an integration test for this but I'm kinda stuck. I'm basing it on t/integration/preload.t as I feel that's the closest to what I'm trying to test (basically that files are still preloaded for tests ran under taint mode but tainting the runner).

yath(
    command => 'test',
    args    => [$dir, '--ext=tx', '--taint', '-A', '-PTestTaintPreload'],
    exit    => 0,

But I quickly ran into problems, it appears the entire yath script is running under taint mode and not just the runner, I tried adding various "fixes" like this but kept running into insecure dependencies under -T so I'm not really sure how best to proceed.

                # Avoid loading File::Glob in this process...
                ($path) = $path =~ /(.*)/;
                ($val)  = $val  =~ /(.*)/;
                ($^X)   = $^X   =~ /(.*)/;
                warn "$path$val";
                warn "$^X";
                delete local $ENV{PATH};
                my $out = `$^X -e 'print join "\\n" => glob("${path}${val}")'`;

Does App::Yath::Tester do something different to just calling yath test --taint ... on the command line? What we really want is the yath process to be untainted, the runner to be tainted, and all forked children to be tainted.

I'm also wondering if we need a --taint-warnings flag or something to match perl -t, it still feels kinda hacky to have to pass the right flag to the runner to match the bulk of your tests.

@exodist
Copy link
Member

exodist commented Jun 3, 2025

I have to consider this for a while. I am not sure of the best way to handle this..

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.

Preload is useless with tainting

2 participants