Skip to content
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

Do not index Local:: namespace (nor Local) #541

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

thibaultduponchelle
Copy link

The Local:: namespace is advertised as "reserved" and "not conflicting", encouraging people to use it to protect from inadvertently installing public modules. But currently, Local:: namespace can be indexed, like any other namespace. In the scope of CPAN Security, I propose this change to avoid indexing Local::

Please find below the proof of testing of this change:

pause-no-local

pause-no-local-log

pause-no-local-db

Along with this change, we should ideally cleanup the index from any Local:: module (but keep local::lib)

Official documentation

The PAUSE documentation On The Naming of Modules says:

By convention, the top-level Local namespace should never conflict with anything on CPAN. This allows you to be confident that the name you choose under Local isn't going to conflict with anything from the outside world.

The Perl Module Library (perlmodlib) goes further with:

If developing modules for private internal or project specific use, that will never be released to the public, then you should ensure that their names will not clash with any future public module. You can do this either by using the reserved Local::* category or by using a category name that includes an underscore like Foo_Corp::*.

More references

@charsbar
Copy link
Collaborator

Just a quick thought.

  • The current PAUSE treats packages case-insensitively in many cases. Prohibiting Local but allowing local or LOCAL or anything seems inconsistent.
  • Isn't it better not to distribute arbitrary stuff put under the Local:: namespace in the first place, if you consider the security? (In other words, might it be better to reject an upload if its basename looks like m!/local\b!i, though we need to take care of local::;lib and its friends?
  • Only several people seem to have used Local:: namespaces so far. I ran the following script
use Modern::Perl;
use File::Find;

my $ct = 0;
my %found;
find({
    wanted => sub {
        my $file = $File::Find::name;
        if ($file =~ m!/\blocal\b!i) {
            return if $file =~ /local-lib/;
            say "FOUND: $file";
            $found{$file} = 1;
            $ct++;
        }
        say $file;
    },
    no_chdir => 1,
    follow_fast => 1,
}, 'backpan/authors/id');

say " $_" for sort keys %found;
say "total: $ct";

and got

 backpan/authors/id/B/BR/BRIANG/Local-Utils-0.001-TRIAL.tar.gz
 backpan/authors/id/C/CO/CONTRA/Local-Acme-0.01.meta
 backpan/authors/id/C/CO/CONTRA/Local-Acme-0.01.readme
 backpan/authors/id/C/CO/CONTRA/Local-Acme-0.01.tar.gz
 backpan/authors/id/G/GE/GETTY/local-c-0.001.meta
 backpan/authors/id/G/GE/GETTY/local-c-0.001.readme
 backpan/authors/id/G/GE/GETTY/local-c-0.001.tar.gz
 backpan/authors/id/G/GR/GRIF/Local-Babelfish-0.01.readme
 backpan/authors/id/G/GR/GRIF/Local-Babelfish-0.01.tar.gz
 backpan/authors/id/K/KA/KAN/Local-CLI-mcmd-v0.01.meta
 backpan/authors/id/K/KA/KAN/Local-CLI-mcmd-v0.01.readme
 backpan/authors/id/K/KA/KAN/Local-CLI-mcmd-v0.01.tar.gz
 backpan/authors/id/K/KA/KAN/Local-CLI-mcmd-v0.02.meta
 backpan/authors/id/K/KA/KAN/Local-CLI-mcmd-v0.02.readme
 backpan/authors/id/K/KA/KAN/Local-CLI-mcmd-v0.02.tar.gz
 backpan/authors/id/K/KA/KAN/Local-CLI-mtcp-v0.01.meta
 backpan/authors/id/K/KA/KAN/Local-CLI-mtcp-v0.01.readme
 backpan/authors/id/K/KA/KAN/Local-CLI-mtcp-v0.01.tar.gz
 backpan/authors/id/K/KA/KAN/Local-CLI-mtcp-v0.02.meta
 backpan/authors/id/K/KA/KAN/Local-CLI-mtcp-v0.02.readme
 backpan/authors/id/K/KA/KAN/Local-CLI-mtcp-v0.02.tar.gz
 backpan/authors/id/K/KA/KAN/Local-CLI-range-v0.01.meta
 backpan/authors/id/K/KA/KAN/Local-CLI-range-v0.01.readme
 backpan/authors/id/K/KA/KAN/Local-CLI-range-v0.01.tar.gz
 backpan/authors/id/K/KA/KAN/Local-Cluster-0.01.meta
 backpan/authors/id/K/KA/KAN/Local-Cluster-0.01.readme
 backpan/authors/id/K/KA/KAN/Local-Cluster-0.01.tar.gz
 backpan/authors/id/K/KA/KAN/Local-Cluster-0.02.meta
 backpan/authors/id/K/KA/KAN/Local-Cluster-0.02.readme
 backpan/authors/id/K/KA/KAN/Local-Cluster-0.02.tar.gz
 backpan/authors/id/K/KA/KAN/Local-Range-v0.01.meta
 backpan/authors/id/K/KA/KAN/Local-Range-v0.01.readme
 backpan/authors/id/K/KA/KAN/Local-Range-v0.01.tar.gz
 backpan/authors/id/K/KA/KAN/Local-Util-0.01.meta
 backpan/authors/id/K/KA/KAN/Local-Util-0.01.readme
 backpan/authors/id/K/KA/KAN/Local-Util-0.01.tar.gz
 backpan/authors/id/M/MA/MARKOV/home/markov/local
 backpan/authors/id/M/MA/MARKOV/home/markov/local/perl-git
 backpan/authors/id/M/MA/MARKOV/home/markov/local/perl-git/MIME-Types
 backpan/authors/id/M/MA/MARKOV/home/markov/local/perl-git/MIME-Types/public_html
 backpan/authors/id/M/MA/MARKOV/home/markov/local/perl-git/MIME-Types/public_html/mime-types
 backpan/authors/id/M/MA/MARKOV/home/markov/local/perl-git/MIME-Types/public_html/mime-types/source
 backpan/authors/id/M/MA/MARKOV/home/markov/local/perl-git/MIME-Types/public_html/mime-types/source/MIME-Types-219targz
 backpan/authors/id/M/MA/MARKOV/home/markov/local/perl-git/MIME-Types/public_html/mime-types/source/MIME-Types-219targz/MIME-Types-2.19.meta
 backpan/authors/id/M/MA/MARKOV/home/markov/local/perl-git/MIME-Types/public_html/mime-types/source/MIME-Types-219targz/MIME-Types-2.19.readme
 backpan/authors/id/M/MA/MARKOV/home/markov/local/perl-git/MIME-Types/public_html/mime-types/source/MIME-Types-219targz/MIME-Types-2.19.tar.gz
 backpan/authors/id/R/RC/RCAPUTO/home/troc/projects/local
 backpan/authors/id/R/RC/RCAPUTO/home/troc/projects/local/jobqueue
 backpan/authors/id/R/RC/RCAPUTO/home/troc/projects/local/jobqueue/POE-Component-JobQueue-054targz
 backpan/authors/id/R/RC/RCAPUTO/home/troc/projects/local/jobqueue/POE-Component-JobQueue-054targz/POE-Component-JobQueue-0.54.meta
 backpan/authors/id/R/RC/RCAPUTO/home/troc/projects/local/jobqueue/POE-Component-JobQueue-054targz/POE-Component-JobQueue-0.54.readme
 backpan/authors/id/R/RC/RCAPUTO/home/troc/projects/local/jobqueue/POE-Component-JobQueue-054targz/POE-Component-JobQueue-0.54.tar.gz
 backpan/authors/id/R/RC/RCAPUTO/home/troc/projects/local/jobqueue/POE-Component-JobQueue-054targz/POE-Component-JobQueue-0.5401.meta
 backpan/authors/id/R/RC/RCAPUTO/home/troc/projects/local/jobqueue/POE-Component-JobQueue-054targz/POE-Component-JobQueue-0.5401.readme
 backpan/authors/id/R/RC/RCAPUTO/home/troc/projects/local/jobqueue/POE-Component-JobQueue-054targz/POE-Component-JobQueue-0.5401.tar.gz
 backpan/authors/id/R/RS/RST/Local-MifTree-0044
 backpan/authors/id/R/RS/RST/Local-MifTree-0044/IO-Tokenized-Scalar-0.05.meta
 backpan/authors/id/R/RS/RST/Local-MifTree-0044/IO-Tokenized-Scalar-0.05.readme
 backpan/authors/id/R/RS/RST/Local-MifTree-0044/IO-Tokenized-Scalar-0.05.tar.gz
 backpan/authors/id/R/RS/RST/Local-MifTree-0044/Local-MifTree-0.044.meta
 backpan/authors/id/R/RS/RST/Local-MifTree-0044/Local-MifTree-0.044.tar.gz
total: 61

I feel it might be better to fix the docs.

@mohawk2
Copy link
Contributor

mohawk2 commented Nov 24, 2024

I favour this change!

@thibaultduponchelle
Copy link
Author

@charsbar Thank you for the review and quick thoughts. It made me think again about the validity of this code change and test more, but after that I still consider this change as correct 👍🏼 I'm detailing after the "why".

The current PAUSE treats packages case-insensitively in many cases. Prohibiting Local but allowing local or LOCAL or anything seems inconsistent.

Yes, internally, but then when modules reach the index, it is with case. Later when installing, modules are always referred with case (e.g. cpanm dbi or cpanm Dbi won't find nor install DBI)
Also, if you look around in PAUSE code, other skips are using same logic and matching with case (e.g. main or DB...)

Isn't it better not to distribute arbitrary stuff put under the Local:: namespace in the first place, if you consider the security? (In other words, might it be better to reject an upload if its basename looks like m!/local\b!i, though we need to take care of local::;lib and its friends?

I got your point and I somewhat agree with you. Though it's going a bit further than what I'm thinking. Current proposal of removing from index will already well protect at install time (CPAN client).

Only several people seem to have used Local:: namespaces so far. I ran the following script

Yes, and we think that we could clean these few modules (from index) 😃

@karenetheridge
Copy link
Contributor

I am in favour of this change.

$; ack -i '^local::' ~/.cpanm/02packages.details.txt 
Local::Abstract                   undef  A/AH/AHERNIT/Class-Injection-1.11.tar.gz
Local::Acme                        0.01  C/CO/CONTRA/Local-Acme-0.01.tar.gz
Local::After                      undef  O/OA/OALDERS/App-perlimports-0.000001.tar.gz
Local::CustomImport               undef  O/OA/OALDERS/App-perlimports-0.000001.tar.gz
Local::DBD::ADO::DSN              undef  S/SG/SGOELDNER/DBD-ADO-2.99.tar.gz
Local::DBRecord                   0.111  R/RE/REID/Net-IP-Identifier-0.111.tar.gz
Local::DiskWatcher                 0.01  D/DM/DMALONE/App-Diskd/App-Diskd-0.01.tar.gz
Local::Info                        0.01  D/DM/DMALONE/App-Diskd/App-Diskd-0.01.tar.gz
local::lib                     2.000029  H/HA/HAARG/local-lib-2.000029.tar.gz
Local::MooseTypeLibrary           undef  O/OA/OALDERS/App-perlimports-0.000001.tar.gz
Local::MountWatcher                0.01  D/DM/DMALONE/App-Diskd/App-Diskd-0.01.tar.gz
Local::MrGamoo::MySelf            undef  S/SO/SOLVE/AC-MrGamoo-1.tar.gz
Local::MrMagoo::FileList          undef  S/SO/SOLVE/AC-MrGamoo-1.tar.gz
Local::MrMagoo::ReadInput         undef  S/SO/SOLVE/AC-MrGamoo-1.tar.gz
Local::MulticastServer             0.01  D/DM/DMALONE/App-Diskd/App-Diskd-0.01.tar.gz
Local::MyOwnMoo                   undef  O/OA/OALDERS/App-perlimports-0.000001.tar.gz
Local::MyOwnMoose                 undef  O/OA/OALDERS/App-perlimports-0.000001.tar.gz
Local::NoUnused                   undef  O/OA/OALDERS/App-perlvars-0.000003.tar.gz
Local::Null::Logger               1.281  B/BD/BDFOY/MyCPAN-App-DPAN-1.281.tar.gz
Local::Over                       undef  A/AH/AHERNIT/Class-Injection-1.11.tar.gz
Local::Payload                    0.111  R/RE/REID/Net-IP-Identifier-0.111.tar.gz
Local::Plugin1                    undef  A/AH/AHERNIT/Class-Injection-1.11.tar.gz
Local::Plugin2                    undef  A/AH/AHERNIT/Class-Injection-1.11.tar.gz
Local::RequireExporter            undef  O/OA/OALDERS/App-perlimports-0.000001.tar.gz
Local::Seamstress::Base           undef  T/TB/TBONE/HTML-Seamstress-6.112830.tar.gz
Local::STDOUT                     undef  O/OA/OALDERS/App-perlimports-0.000001.tar.gz
Local::SymbolInExport             undef  O/OA/OALDERS/App-perlimports-0.000001.tar.gz
Local::Target                     undef  A/AH/AHERNIT/Class-Injection-1.11.tar.gz
Local::UnixSocketClient            0.01  D/DM/DMALONE/App-Diskd/App-Diskd-0.01.tar.gz
Local::UnixSocketServer            0.01  D/DM/DMALONE/App-Diskd/App-Diskd-0.01.tar.gz
Local::UnixSocketServer::Session   0.01  D/DM/DMALONE/App-Diskd/App-Diskd-0.01.tar.gz
Local::Unused                     undef  O/OA/OALDERS/App-perlvars-0.000003.tar.gz
Local::UsesMoo                    undef  O/OA/OALDERS/App-perlimports-0.000001.tar.gz
Local::UsesMoose                  undef  O/OA/OALDERS/App-perlimports-0.000001.tar.gz
Local::UsesMyOwnMoo               undef  O/OA/OALDERS/App-perlimports-0.000001.tar.gz
Local::UsesMyOwnMoose             undef  O/OA/OALDERS/App-perlimports-0.000001.tar.gz
Local::UsesTypesStandard          undef  O/OA/OALDERS/App-perlimports-0.000001.tar.gz
Local::UsesUsesImportInto         undef  O/OA/OALDERS/App-perlimports-0.000001.tar.gz
Local::ViaExporter                undef  O/OA/OALDERS/App-perlimports-0.000001.tar.gz
Local::ViaSubExporter             undef  O/OA/OALDERS/App-perlimports-0.000001.tar.gz
Local::Yenta::MySelf              undef  S/SO/SOLVE/AC-Yenta-1.tar.gz

This doesn't look bad at all. Aside from Local::Acme, all of these are "cuckoo" modules hiding in another distribution and all look unintentionally indexed.

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.

4 participants