Skip to content

Commit 6a310ea

Browse files
hsbtclaude
andcommitted
Ignore plugin index entries that point outside the plugin root
A path stored relative to the plugin root only means anything inside it, so an entry that climbs out of it did not come from a plugin Bundler installed there. Paths recorded as absolute are left alone, which is how `bundle plugin install --path` and older index files record them. An event left with no plugins once its own are gone goes unmerged rather than empty, so it cannot overwrite one the other index file registered. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 4846a48 commit 6a310ea

2 files changed

Lines changed: 147 additions & 5 deletions

File tree

lib/bundler/plugin/index.rb

Lines changed: 44 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -176,11 +176,15 @@ def load_index(index_file, global = false)
176176
# older Bundler versions, which dumped empty hashes as a bare key.
177177
index = Gem::YAMLSerializer.load(data) || {}
178178

179-
@commands.merge!(index["commands"] || {})
180-
@hooks.merge!(index["hooks"] || {})
181-
@load_paths.merge!(transform_index_paths(index["load_paths"]) {|p| absolutize_path(p, base) })
182-
@plugin_paths.merge!(transform_index_paths(index["plugin_paths"]) {|p| absolutize_path(p, base) })
183-
@sources.merge!(index["sources"] || {}) unless global
179+
escaping = escaping_plugins(index, base)
180+
hooks = (index["hooks"] || {}).transform_values {|names| Array(names) - escaping }
181+
182+
@commands.merge!(owned_by(index["commands"] || {}, escaping))
183+
# An event whose plugins all escaped is left out rather than merged in empty.
184+
@hooks.merge!(hooks.reject {|_, names| names.empty? })
185+
@load_paths.merge!(named(transform_index_paths(index["load_paths"]) {|p| absolutize_path(p, base) }, escaping))
186+
@plugin_paths.merge!(named(transform_index_paths(index["plugin_paths"]) {|p| absolutize_path(p, base) }, escaping))
187+
@sources.merge!(owned_by(index["sources"] || {}, escaping)) unless global
184188
end
185189
end
186190

@@ -209,6 +213,41 @@ def base_for_index(global)
209213
global ? Plugin.global_root : Plugin.root
210214
end
211215

216+
# A relative path only means anything inside the root, so an entry that escapes is not one Bundler installed.
217+
def escaping_plugins(index, base)
218+
names = []
219+
220+
%w[load_paths plugin_paths].each do |key|
221+
(index[key] || {}).each do |name, value|
222+
escapes = Array(value).any? do |path|
223+
!Pathname.new(path).absolute? && !contained_in?(absolutize_path(path, base), base)
224+
end
225+
226+
names << name if escapes
227+
end
228+
end
229+
230+
names.uniq
231+
end
232+
233+
# Expanded here, not by the caller: what gets stored stays joined, because
234+
# the rest of the class matches it against Plugin.root as written.
235+
def contained_in?(path, base)
236+
path = File.expand_path(path)
237+
base = File.expand_path(base)
238+
239+
path == base || path.start_with?("#{base}#{File::SEPARATOR}")
240+
end
241+
242+
# commands and sources are keyed by what they provide, load_paths and plugin_paths by the plugin.
243+
def owned_by(mapping, names)
244+
mapping.reject {|_, plugin| names.include?(plugin) }
245+
end
246+
247+
def named(mapping, names)
248+
mapping.reject {|plugin, _| names.include?(plugin) }
249+
end
250+
212251
def transform_index_paths(paths)
213252
return {} unless paths
214253

spec/bundler/plugin/index_spec.rb

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -238,6 +238,45 @@
238238
expect(new_index.load_paths(plugin_name)).to eq([plugin_root.join(plugin_name, "lib").to_s])
239239
end
240240

241+
it "ignores entries that climb out only after an interior parent reference" do
242+
require "rubygems/yaml_serializer"
243+
244+
escaping_index = {
245+
"commands" => {},
246+
"hooks" => {},
247+
"load_paths" => { "escaping-plugin" => [File.join("escaping-plugin", "..", "..", "elsewhere", "lib")] },
248+
"plugin_paths" => { "escaping-plugin" => File.join("escaping-plugin", "..", "..", "elsewhere") },
249+
"sources" => {},
250+
}
251+
252+
File.open(index.index_file, "w") {|f| f.puts Gem::YAMLSerializer.dump(escaping_index) }
253+
254+
new_index = Index.new
255+
256+
expect(new_index.installed?("escaping-plugin")).to be_nil
257+
expect(new_index.load_paths("escaping-plugin")).to be_nil
258+
end
259+
260+
it "reads a leading tilde in a relative path literally" do
261+
require "rubygems/yaml_serializer"
262+
263+
plugin_root = Bundler::Plugin.root
264+
265+
tilde_index = {
266+
"commands" => {},
267+
"hooks" => {},
268+
"load_paths" => { plugin_name => [File.join("~nosuchuser", "lib")] },
269+
"plugin_paths" => { plugin_name => "~nosuchuser" },
270+
"sources" => {},
271+
}
272+
273+
File.open(index.index_file, "w") {|f| f.puts Gem::YAMLSerializer.dump(tilde_index) }
274+
275+
new_index = Index.new
276+
expect(new_index.plugin_path(plugin_name)).to eq(plugin_root.join("~nosuchuser"))
277+
expect(new_index.load_paths(plugin_name)).to eq([plugin_root.join("~nosuchuser", "lib").to_s])
278+
end
279+
241280
it "keeps paths outside the plugin root as absolute" do
242281
outside_path = tmp.join("outside", "external-plugin")
243282
FileUtils.mkdir_p(outside_path.join("lib"))
@@ -251,6 +290,70 @@
251290
expect(data["load_paths"]["external-plugin"]).to eq([outside_path.join("lib").to_s])
252291
end
253292

293+
it "ignores entries whose relative paths climb out of the plugin root" do
294+
require "rubygems/yaml_serializer"
295+
296+
escaping_index = {
297+
"commands" => { "escape" => "escaping-plugin" },
298+
"hooks" => { "before-eval" => ["escaping-plugin", plugin_name] },
299+
"load_paths" => {
300+
"escaping-plugin" => ["../../elsewhere/lib"],
301+
plugin_name => [File.join(plugin_name, "lib")],
302+
},
303+
"plugin_paths" => { "escaping-plugin" => "../../elsewhere", plugin_name => plugin_name },
304+
"sources" => { "escape" => "escaping-plugin" },
305+
}
306+
307+
File.open(index.index_file, "w") {|f| f.puts Gem::YAMLSerializer.dump(escaping_index) }
308+
309+
new_index = Index.new
310+
311+
expect(new_index.installed?("escaping-plugin")).to be_nil
312+
expect(new_index.load_paths("escaping-plugin")).to be_nil
313+
expect(new_index.command_plugin("escape")).to be_nil
314+
expect(new_index.source_plugin("escape")).to be_nil
315+
expect(new_index.hook_plugins("before-eval")).to eq([plugin_name])
316+
expect(new_index.installed?(plugin_name)).to eq(Bundler::Plugin.root.join(plugin_name).to_s)
317+
end
318+
319+
it "ignores entries whose load paths alone climb out of the plugin root" do
320+
require "rubygems/yaml_serializer"
321+
322+
escaping_index = {
323+
"commands" => {},
324+
"hooks" => {},
325+
"load_paths" => { "escaping-plugin" => ["../../elsewhere/lib"] },
326+
"plugin_paths" => { "escaping-plugin" => "escaping-plugin" },
327+
"sources" => {},
328+
}
329+
330+
File.open(index.index_file, "w") {|f| f.puts Gem::YAMLSerializer.dump(escaping_index) }
331+
332+
new_index = Index.new
333+
334+
expect(new_index.installed?("escaping-plugin")).to be_nil
335+
expect(new_index.load_paths("escaping-plugin")).to be_nil
336+
end
337+
338+
it "drops hook events whose plugins all climb out of the plugin root" do
339+
require "rubygems/yaml_serializer"
340+
341+
escaping_index = {
342+
"commands" => {},
343+
"hooks" => { "before-eval" => ["escaping-plugin"] },
344+
"load_paths" => { "escaping-plugin" => ["../../elsewhere/lib"] },
345+
"plugin_paths" => { "escaping-plugin" => "../../elsewhere" },
346+
"sources" => {},
347+
}
348+
349+
File.open(index.index_file, "w") {|f| f.puts Gem::YAMLSerializer.dump(escaping_index) }
350+
351+
new_index = Index.new
352+
new_index.register_plugin("aplugin", lib_path("aplugin").to_s, [lib_path("aplugin").join("lib").to_s], [], [], [])
353+
354+
expect(new_index.index_file.read).to_not include("before-eval")
355+
end
356+
254357
it "reads legacy index files with absolute paths" do
255358
require "rubygems/yaml_serializer"
256359

0 commit comments

Comments
 (0)