Bugzilla – Attachment 192088 Details for
Bug 41651
Allow plugins to specify additional dependencies
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 41651: (QA follow-up) Fix issues in plugin file copy
fc433cb.patch (text/plain), 4.21 KB, created by
Martin Renvoize (ashimema)
on 2026-01-27 08:06:39 UTC
(
hide
)
Description:
Bug 41651: (QA follow-up) Fix issues in plugin file copy
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2026-01-27 08:06:39 UTC
Size:
4.21 KB
patch
obsolete
>From fc433cbaf5023627d239ed158fc27ef31e7a4356 Mon Sep 17 00:00:00 2001 >From: Martin Renvoize <martin.renvoize@openfifth.co.uk> >Date: Tue, 27 Jan 2026 07:50:34 +0000 >Subject: [PATCH] Bug 41651: (QA follow-up) Fix issues in plugin file copy > >We're focusing on reliability and security here, the improvements >include: > >- Escape regex metacharacters in path using \Q...\E to prevent > regex injection issues >- Check copy() return value and collect errors, die if any > copies fail to prevent silent partial installations >- Make basename filter more specific to only skip cpanfile and > META.json/META.yml, not files containing "META" anywhere >--- > Koha/Plugins.pm | 48 ++++++++++++++++++++++++++++-------------------- > 1 file changed, 28 insertions(+), 20 deletions(-) > >diff --git a/Koha/Plugins.pm b/Koha/Plugins.pm >index f3a9e849f2b..4e137c4e66d 100644 >--- a/Koha/Plugins.pm >+++ b/Koha/Plugins.pm >@@ -23,11 +23,11 @@ use Array::Utils qw( array_minus ); > use Class::Inspector; > use CPAN::Meta; > use CPAN::Meta::Check qw( verify_dependencies ); >-use File::Basename qw( fileparse ); >-use File::Copy qw( copy ); >-use File::Find qw( find ); >-use File::Path qw( make_path ); >-use List::MoreUtils qw( any none ); >+use File::Basename qw( fileparse ); >+use File::Copy qw( copy ); >+use File::Find qw( find ); >+use File::Path qw( make_path ); >+use List::MoreUtils qw( any none ); > use Module::CPANfile; > use Module::Load::Conditional qw( can_load ); > use Module::Load; >@@ -242,8 +242,8 @@ sub GetPlugins { > { > enable_plugins => $self->{'enable_plugins'} > >- # loads even if plugins are disabled >- # FIXME: is this for testing without bothering to mock config? >+ # loads even if plugins are disabled >+ # FIXME: is this for testing without bothering to mock config? > } > ); > } catch { >@@ -311,24 +311,32 @@ sub InstallPlugins { > my $verbose = $params->{verbose} // $self->_verbose; > > if ( $params->{from} ) { >+ > # We have "staged" plugin(s) passed in; check the dependencies, > # copy to the regular location if we're happy >- $self->_check_dependencies($params->{from}); >+ $self->_check_dependencies( $params->{from} ); > > my $plugins_dir = C4::Context->config("pluginsdir"); > $plugins_dir = ref($plugins_dir) eq 'ARRAY' ? $plugins_dir->[0] : $plugins_dir; > >- find({ >- wanted => sub { >- return unless -f $_; >- my ($basename) = fileparse($_); >- return if $basename =~ /^cpanfile|META/; >- my $destdir = $File::Find::dir =~ s/^$params->{from}//r; >- $destdir = "$plugins_dir/$destdir"; >- make_path($destdir); >- copy($File::Find::name, $destdir); >+ my @copy_errors; >+ find( >+ { >+ wanted => sub { >+ return unless -f $_; >+ my ($basename) = fileparse($_); >+ return if $basename =~ /^(cpanfile|META\.(json|yml))$/; >+ my $destdir = $File::Find::dir =~ s/^\Q$params->{from}\E//r; >+ $destdir = "$plugins_dir/$destdir"; >+ make_path($destdir); >+ unless ( copy( $File::Find::name, $destdir ) ) { >+ push @copy_errors, "Failed to copy $File::Find::name to $destdir: $!"; >+ } >+ }, > }, >- }, $params->{from}); >+ $params->{from} >+ ); >+ die join( "\n", @copy_errors ) . "\n" if @copy_errors; > } > > my @plugin_classes = $self->plugins(); >@@ -493,7 +501,7 @@ sub get_valuebuilders_installed { > } > > sub _check_dependencies { >- my ($self, $path) = @_; >+ my ( $self, $path ) = @_; > > my $prereqs; > >@@ -512,7 +520,7 @@ sub _check_dependencies { > return unless $prereqs; > > # Good for both file variants: per the docs, "$meta should be a CPAN::Meta::Prereqs or CPAN::Meta object" >- die "$_\n" for verify_dependencies($prereqs, 'runtime', 'requires'); >+ die "$_\n" for verify_dependencies( $prereqs, 'runtime', 'requires' ); > } > > 1; >-- >2.52.0 >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 41651
:
191567
|
191651
|
192087
| 192088 |
192089
|
192090