[rjbs]

Rather than falling back to evaluating VERSION assignments inside a forked Safe compartment, just don't. Instead, refuse to index, giving the authors instructions on how to assign a version that can be statically indexed.

[rspier]

Looks good.

Is there user documentation that needs to be updated somewhere? (Possibly in a follow-up PR.)

[rspier]

Nice work on this — replacing the Safe compartment and process forks with static extraction is a great cleanup and security improvement.

A few review notes:

1. Boolean check on version object rejects version 0

In lib/PAUSE/pmfile.pm (_static_version_from_line):

    my ($method) = $match{call} =~ /(\w+)\z/;
    my $object = eval {
        $method eq 'qv' ? version::qv($value) : version->$method($value);
    };

    return { refused => $line } unless $object;
    return { version => $object->numify };

Because version.pm objects overload boolean/numeric context, a version object representing 0 (e.g., version->declare("0"), version->new(0), or qv("0.0.0")) evaluates to false in boolean context ($object ? 1 : 0 is false).

With unless $object;, any version constructor evaluating to 0 is treated as a refusal. Changing this to check definedness resolves it:

    return { refused => $line } unless defined $object;

(Suggest also adding a test case like version_is(q{our $VERSION = version->new(0);}, '0.000'); in t/static-version.t.)

2. Immediate refusal on lines containing inline $VERSION =

In static_version_from_file:

    next unless /([\$*])(([\w\:\']*)\bVERSION)\b.*(?<![!><=])\=(?![=>])/;

    return _static_version_from_line($_);

If a line contains $VERSION = inside an inline comment or string literal (e.g. $flag = 1; # note: $VERSION = 1.0 below), it matches this check, fails _static_version_from_line, and immediately returns { refused => $line } without continuing to check subsequent lines for the real $VERSION assignment. Stripping trailing comments (s/\s*#.*//) prior to this check or continuing rather than immediately returning on refusal would avoid false rejections.

3. Readability: Chained s///gr

In lib/PAUSE/Indexer/Errors.pm:

    my $line = $arg->{line} =~ s/[^\x20-\x7e]/?/gr =~ s/\A\s+|\s+\z//gr;

Chaining =~ s///gr across two expressions is valid in modern Perl, but can be easy to misread. Splitting or parenthesizing makes the intent clearer:

    my $line = ($arg->{line} =~ s/[^\x20-\x7e]/?/gr) =~ s/\A\s+|\s+\z//gr;

[rjbs]

@rspier I addressed #1 and #2. #3, I reject.

[Grinnz]

I think this is a good idea overall. I would just make two suggestions for community outreach (I know, well volunteered):

1. It would be nice to analyze the currently indexed releases and find which exactly will now fail to index that previously did thanks to this feature.

2. The PAUSE operating model (https://github.com/andk/pause/blob/master/doc/operating-model.md) should be updated to reference the specific accepted formats under "Factors considered in the indexing phase".