diff --git a/lib/dev/deps/gem_skill_linker.rb b/lib/dev/deps/gem_skill_linker.rb index 366466a..2df5767 100644 --- a/lib/dev/deps/gem_skill_linker.rb +++ b/lib/dev/deps/gem_skill_linker.rb @@ -23,27 +23,58 @@ class GemSkillLinker SKILLS_SUBDIR = "skills" AGENT_SKILLS_SUBDIRS = [".agents", "skills"].freeze + # Env overrides a harness (e.g. a sandboxed agent session) may have + # exported into dev's own environment, redirecting bundler to an + # ephemeral gem cache. The `bundle list` child gets them explicitly + # unset so paths resolve from the project's canonical bundler config — + # dev never runs under bundler itself, so these unsets are its + # equivalent of Bundler.original_env (dev#89). + HARNESS_ENV_SCRUB = [ + "BUNDLE_PATH", + "BUNDLE_APP_CONFIG", + "BUNDLE_BIN", + "GEM_HOME", + "GEM_PATH", + "RUBYOPT", + "RUBYLIB", + ].to_h { |name| [name, nil] }.freeze + # @param project_root [Pathname, String] repo root (Gemfile + link target) # @param skills_dir [Pathname, String, nil] override for tests; defaults # to /.agents/skills - def initialize(project_root:, skills_dir: nil) + # @param tmpdir [Pathname, String] ephemeral temp root that links must + # never target; defaults to Dir.tmpdir (override for tests, whose + # fixture gem trees themselves live under the real temp dir) + def initialize(project_root:, skills_dir: nil, tmpdir: Dir.tmpdir) @project_root = Pathname(project_root) @skills_dir = Pathname(skills_dir || @project_root.join(*AGENT_SKILLS_SUBDIRS)) @skill_installer = SkillInstaller.new(skills_dir: @skills_dir) + @tmpdir_roots = tmpdir_roots(Pathname(tmpdir)) end # Scan the locked gem set for shipped skills and refresh the project's # links: install one per skill found, prune gem links whose gem left the - # lock. Never raises — skill links are hygiene riding a dependency - # install, and hygiene must not block correctness (failures are reported - # on stderr). + # lock. A skill that resolves under the temp dir is never linked (warned + # and skipped — a persistent link to purgeable state silently dangles + # later), but its gem still counts as present for pruning, so an + # ephemeral resolution cannot delete a durable link minted earlier. + # Never raises — skill links are hygiene riding a dependency install, + # and hygiene must not block correctness (failures are reported on + # stderr). # # @return [void] def link_all return unless gemfile_path.exist? expected = expected_links - expected.each { |name, skill_dir| @skill_installer.install(name, skill_dir) } + expected.each do |name, skill_dir| + if ephemeral?(skill_dir) + $stderr.puts "dev: warning: not linking #{name} — #{skill_dir} is under the temp dir " \ + "and would dangle once it is purged." + else + @skill_installer.install(name, skill_dir) + end + end prune_stale_links(expected.keys) rescue StandardError => e $stderr.puts "dev: warning: could not refresh gem skill links (#{e.message})." @@ -84,12 +115,15 @@ def gem_roots # Runs under the project's shadowenv for the same reason as # BundlerIntegration: the dev process's own PATH is the invoking - # service's, which on headless boxes carries the wrong Ruby. + # service's, which on headless boxes carries the wrong Ruby. Harness + # bundler/gem overrides are scrubbed from the child env (see + # HARNESS_ENV_SCRUB) so a sandboxed session cannot redirect the + # resolution into its ephemeral cache. # # @return [Array] install paths of every gem in the bundle def bundled_gem_paths out, err, status = Open3.capture3( - { "BUNDLE_GEMFILE" => gemfile_path.to_s }, + HARNESS_ENV_SCRUB.merge("BUNDLE_GEMFILE" => gemfile_path.to_s), "shadowenv", "exec", "--", "bundle", "list", "--paths", chdir: @project_root.to_s, ) @@ -128,6 +162,30 @@ def locked_gem_names names.uniq end + # The temp root in both its raw and fully-resolved forms — on macOS + # Dir.tmpdir is under /var/... while bundler reports the + # /private/var/... realpath, so containment must check both spellings. + # + # @param tmpdir [Pathname] + # @return [Array] + def tmpdir_roots(tmpdir) + expanded = tmpdir.expand_path + roots = [expanded] + roots << expanded.realpath if expanded.exist? + roots.uniq + end + + # Whether a resolved skill directory lives under the temp dir — the + # signature of a harness resolving the bundle into its own purgeable + # cache, whatever produced it. + # + # @param path [Pathname] + # @return [Boolean] + def ephemeral?(path) + resolved = path.exist? ? path.realpath : path.expand_path + @tmpdir_roots.any? { |root| resolved.to_s.start_with?("#{root}#{File::SEPARATOR}") } + end + # Remove gem links that no current gem accounts for (the gem left the # lock; its tree may still exist on disk, so broken-link pruning alone # would miss it). Only `gem-`-prefixed symlinks are candidates — diff --git a/test/dev/deps/gem_skill_linker_test.rb b/test/dev/deps/gem_skill_linker_test.rb index 87d5ad7..f5c9188 100644 --- a/test/dev/deps/gem_skill_linker_test.rb +++ b/test/dev/deps/gem_skill_linker_test.rb @@ -50,14 +50,23 @@ def build_gem(gems_root, dir_name, skills: []) # `bundle list` must run under the project's shadowenv — same reasoning as # BundlerIntegration: the dev process's PATH is the invoking service's, - # which on headless boxes carries the wrong Ruby. + # which on headless boxes carries the wrong Ruby — with harness bundler + # overrides scrubbed from the child env. def stub_bundle_list(project, paths) + env = Dev::Deps::GemSkillLinker::HARNESS_ENV_SCRUB.merge("BUNDLE_GEMFILE" => (project / "Gemfile").to_s) Open3.stubs(:capture3) - .with({ "BUNDLE_GEMFILE" => (project / "Gemfile").to_s }, - "shadowenv", "exec", "--", "bundle", "list", "--paths", chdir: project.to_s) + .with(env, "shadowenv", "exec", "--", "bundle", "list", "--paths", chdir: project.to_s) .returns([paths.map { |p| "#{p}\n" }.join, "", stub(success?: true)]) end + # Linker under test. The real Dir.tmpdir contains these tests' own fixture + # trees, so every linker gets a tmpdir override pointing inside the fixture + # dir — gems built by build_gem under `gems/` then read as durable, and a + # test opts into ephemerality by building under `/tmp`. + def build_linker(project, dir) + Dev::Deps::GemSkillLinker.new(project_root: project, tmpdir: Pathname(dir) / "tmp") + end + test "links a locked gem's shipped skills as gem---" do Given "a locked gem whose tree ships a skill" dir = Dir.mktmpdir("dev-gem-skill-test-") @@ -65,7 +74,7 @@ def stub_bundle_list(project, paths) rspock = build_gem(gems, "rspock-1.2.0", skills: ["rspock"]) minitest = build_gem(gems, "minitest-5.25.0") stub_bundle_list(project, [rspock, minitest]) - linker = Dev::Deps::GemSkillLinker.new(project_root: project) + linker = build_linker(project, dir) When "linking" linker.link_all @@ -85,7 +94,7 @@ def stub_bundle_list(project, paths) project, gems = build_project(dir) reporters = build_gem(gems, "minitest-reporters-1.7.1", skills: ["reporting"]) stub_bundle_list(project, [reporters]) - linker = Dev::Deps::GemSkillLinker.new(project_root: project) + linker = build_linker(project, dir) When "linking" linker.link_all @@ -104,7 +113,7 @@ def stub_bundle_list(project, paths) project, gems = build_project(dir) stray = build_gem(gems, "stray-9.9.9", skills: ["stray"]) stub_bundle_list(project, [stray]) - linker = Dev::Deps::GemSkillLinker.new(project_root: project) + linker = build_linker(project, dir) When "linking" linker.link_all @@ -128,7 +137,7 @@ def stub_bundle_list(project, paths) File.symlink(departed / "skills" / "departed", skills_dir / "gem-departed--departed") File.symlink(gems, skills_dir / "my-own-link") (skills_dir / "notes.md").write("mine\n") - linker = Dev::Deps::GemSkillLinker.new(project_root: project) + linker = build_linker(project, dir) When "linking" linker.link_all @@ -149,7 +158,7 @@ def stub_bundle_list(project, paths) project = Pathname(dir) / "repo" FileUtils.mkdir_p(project) Open3.expects(:capture3).never - linker = Dev::Deps::GemSkillLinker.new(project_root: project) + linker = build_linker(project, dir) When "linking" linker.link_all @@ -169,7 +178,7 @@ def stub_bundle_list(project, paths) skills_dir = project / ".agents" / "skills" FileUtils.mkdir_p(skills_dir) FileUtils.chmod(0o000, skills_dir) - linker = Dev::Deps::GemSkillLinker.new(project_root: project) + linker = build_linker(project, dir) old_stderr = $stderr $stderr = StringIO.new @@ -185,12 +194,116 @@ def stub_bundle_list(project, paths) FileUtils.rm_rf(dir) end + # Pins the exact scrub set rather than referencing HARNESS_ENV_SCRUB: a + # sandboxed session (Cursor sandbox cache, dev#89) leaks these overrides + # into dev's env, and dropping any of them from the scrub would silently + # re-open the leak. + test "bundle list runs with harness bundler and gem overrides explicitly unset" do + Given "a project" + dir = Dir.mktmpdir("dev-gem-skill-test-") + project, = build_project(dir) + linker = build_linker(project, dir) + + When "linking" + linker.link_all + + Then "every harness override is nil'd in the child env" + 1 * Open3.capture3( + { + "BUNDLE_PATH" => nil, + "BUNDLE_APP_CONFIG" => nil, + "BUNDLE_BIN" => nil, + "GEM_HOME" => nil, + "GEM_PATH" => nil, + "RUBYOPT" => nil, + "RUBYLIB" => nil, + "BUNDLE_GEMFILE" => (project / "Gemfile").to_s, + }, + "shadowenv", "exec", "--", "bundle", "list", "--paths", chdir: project.to_s + ) >> ["", "", stub(success?: true)] + + Cleanup + FileUtils.rm_rf(dir) + end + + test "refuses to link a skill resolved under the temp dir and warns" do + Given "a locked gem whose tree resolves into the ephemeral temp dir" + dir = Dir.mktmpdir("dev-gem-skill-test-") + project, = build_project(dir) + ephemeral = build_gem(Pathname(dir) / "tmp" / "gems", "rspock-1.2.0", skills: ["rspock"]) + stub_bundle_list(project, [ephemeral]) + linker = build_linker(project, dir) + old_stderr = $stderr + $stderr = StringIO.new + + When "linking" + linker.link_all + + Then "no link is created and the skip is warned" + !File.exist?(project / ".agents" / "skills" / "gem-rspock--rspock") + $stderr.string.include?("not linking gem-rspock--rspock") + + Cleanup + $stderr = old_stderr + FileUtils.rm_rf(dir) + end + + test "an ephemeral resolution does not prune the durable link it shadows" do + Given "a durable link for a gem that now resolves into the temp dir" + dir = Dir.mktmpdir("dev-gem-skill-test-") + project, gems = build_project(dir) + durable = build_gem(gems, "rspock-1.2.0", skills: ["rspock"]) + skills_dir = project / ".agents" / "skills" + FileUtils.mkdir_p(skills_dir) + File.symlink(durable / "skills" / "rspock", skills_dir / "gem-rspock--rspock") + ephemeral = build_gem(Pathname(dir) / "tmp" / "gems", "rspock-1.2.0", skills: ["rspock"]) + stub_bundle_list(project, [ephemeral]) + linker = build_linker(project, dir) + old_stderr = $stderr + $stderr = StringIO.new + + When "linking" + linker.link_all + + Then "the durable link survives, still pointing at its durable target" + File.symlink?(skills_dir / "gem-rspock--rspock") + File.readlink(skills_dir / "gem-rspock--rspock") == (durable / "skills" / "rspock").to_s + + Cleanup + $stderr = old_stderr + FileUtils.rm_rf(dir) + end + + test "temp dir containment sees through symlinked temp roots" do + Given "a tmpdir override that is a symlink to the dir the gem resolves under" + dir = Dir.mktmpdir("dev-gem-skill-test-") + project, = build_project(dir) + real_tmp = Pathname(dir) / "tmp" + ephemeral = build_gem(real_tmp / "gems", "rspock-1.2.0", skills: ["rspock"]) + tmp_alias = Pathname(dir) / "tmp-alias" + File.symlink(real_tmp, tmp_alias) + stub_bundle_list(project, [ephemeral]) + linker = Dev::Deps::GemSkillLinker.new(project_root: project, tmpdir: tmp_alias) + old_stderr = $stderr + $stderr = StringIO.new + + When "linking" + linker.link_all + + Then "the path is recognized as ephemeral and never links" + !File.exist?(project / ".agents" / "skills" / "gem-rspock--rspock") + + Cleanup + $stderr = old_stderr + FileUtils.rm_rf(dir) + end + test "a failing bundle list warns instead of failing the install" do Given "bundler erroring out" dir = Dir.mktmpdir("dev-gem-skill-test-") project, = build_project(dir) Open3.stubs(:capture3).returns(["", "bundler exploded", stub(success?: false)]) - linker = Dev::Deps::GemSkillLinker.new(project_root: project) + linker = build_linker(project, dir) old_stderr = $stderr $stderr = StringIO.new