Skip to content

Conversation

@mamazzol
Copy link
Contributor

If a non directory file is in the plugins folder during installation, there will be a non descriptive error during findBundle, as it assumes everything is a directory. This new preemptive check adds a descriptive error message to avoid this scenario.

Also, for consistency and to not use a deprecated URL constructor, in the download method we now use URI.

Closes #27401

If a non directory file is in the plugins folder during installation,
there will be a non descriptive error during findBundle, as it assumes
everything is a directory. This new preemptive check adds a descripive
error message to avoid this scenario.

Also, for consistency and to not use a deprecated URL constructor, in
the download method we now use URI.

Closes elastic#27401
@mamazzol mamazzol requested a review from a team as a code owner October 30, 2025 17:55
@mamazzol mamazzol added >enhancement :Core/Infra/Plugins Plugin API and infrastructure labels Oct 30, 2025
@elasticsearchmachine elasticsearchmachine added v9.3.0 Team:Core/Infra Meta label for core/infra team labels Oct 30, 2025
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (Team:Core/Infra)

@elasticsearchmachine
Copy link
Collaborator

Hi @mamazzol, I've created a changelog YAML for you.

@mark-vieira
Copy link
Contributor

FWIW, is there a reason this isn't allowed? Did we consider simply removing this limitation?

@rjernst
Copy link
Member

rjernst commented Oct 30, 2025

is there a reason this isn't allowed?

We want to maintain control of what is in the plugins directory. Otherwise we would have to have leniency that could hide an error (eg broken plugin installation, looks like the plugin is there but it's not complete, etc)

@mamazzol
Copy link
Contributor Author

FWIW, is there a reason this isn't allowed? Did we consider simply removing this limitation?

At the moment it's stopped by the "bug/feature" present here.

findPluginDirs will interpret everything in the plugins directory as another directory, so when the readPluginBundle tries to read properties file from the zip, it fails. This acts as a block for unrecognised things in the plugins directory and the user will get a message like "random-uuid has no properties file".

Perhaps findPluginDirs could validate the retrieved folders in some way and throw otherwise? Or are we happy with the existing behaviour?

@mark-vieira
Copy link
Contributor

Perhaps findPluginDirs could validate the retrieved folders in some way and throw otherwise? Or are we happy with the existing behaviour?

Sounds like we want to keep the existing behavior of preventing folks from installing plugins located inside the plugins dir.

@mamazzol
Copy link
Contributor Author

mamazzol commented Nov 3, 2025

💚 All backports created successfully

Status Branch Result
9.1
9.2
8.19

Questions ?

Please refer to the Backport tool documentation

mamazzol added a commit to mamazzol/elasticsearch that referenced this pull request Nov 3, 2025
* Error if installed plugin is inside plugins folder

If a non directory file is in the plugins folder during installation,
there will be a non descriptive error during findBundle, as it assumes
everything is a directory. This new preemptive check adds a descripive
error message to avoid this scenario.

Also, for consistency and to not use a deprecated URL constructor, in
the download method we now use URI.

Closes elastic#27401

(cherry picked from commit 941f302)
mamazzol added a commit to mamazzol/elasticsearch that referenced this pull request Nov 3, 2025
* Error if installed plugin is inside plugins folder

If a non directory file is in the plugins folder during installation,
there will be a non descriptive error during findBundle, as it assumes
everything is a directory. This new preemptive check adds a descripive
error message to avoid this scenario.

Also, for consistency and to not use a deprecated URL constructor, in
the download method we now use URI.

Closes elastic#27401

(cherry picked from commit 941f302)
@joegallo joegallo added the v9.2.1 label Nov 3, 2025
mamazzol added a commit that referenced this pull request Nov 4, 2025
* Error if installed plugin is inside plugins folder

If a non directory file is in the plugins folder during installation,
there will be a non descriptive error during findBundle, as it assumes
everything is a directory. This new preemptive check adds a descripive
error message to avoid this scenario.

Also, for consistency and to not use a deprecated URL constructor, in
the download method we now use URI.

Closes #27401
mamazzol added a commit that referenced this pull request Nov 4, 2025
* Error if installed plugin is inside plugins folder

If a non directory file is in the plugins folder during installation,
there will be a non descriptive error during findBundle, as it assumes
everything is a directory. This new preemptive check adds a descripive
error message to avoid this scenario.

Also, for consistency and to not use a deprecated URL constructor, in
the download method we now use URI.

Closes #27401
mamazzol added a commit that referenced this pull request Nov 4, 2025
* Error if installed plugin is inside plugins folder

If a non directory file is in the plugins folder during installation,
there will be a non descriptive error during findBundle, as it assumes
everything is a directory. This new preemptive check adds a descripive
error message to avoid this scenario.

Also, for consistency and to not use a deprecated URL constructor, in
the download method we now use URI.

Closes #27401
@mamazzol mamazzol deleted the plugin-install-directory branch November 4, 2025 09:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Provide more actionable error message when installing an offline plugin in the plugins directory

5 participants