From b50262974c4b1ec61668d9f42b7a2efea70ae7f9 Mon Sep 17 00:00:00 2001 From: sunrisepeak Date: Thu, 6 Aug 2026 07:42:41 +0800 Subject: [PATCH 1/3] fix(elfpatch): the tool that stamps our artifacts was chosen by the view MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `_find_tool` ordered its candidates subos/bin -> home/bin -> /usr/bin -> PATH, and the payload was not a candidate at all. So patchelf -- the tool that writes INTERP and RPATH onto every payload we ship -- had its identity decided by a mutable, user-selected view, with a silent fallback to whatever `/usr/bin/patchelf` the machine happened to have. That is not a detail. patchelf versions differ in how they grow the dynamic segment and in `--force-rpath` semantics, so which one runs decides what our artifacts look like. It survived because in the default configuration all four candidates resolve to the same file: the answers agree by coincidence until a second home, a second version, or a host install exists. Payload first now, via a new pkginfo.tool_payload_dir. The view and the host stay as fallbacks -- a store that predates this has to keep working -- but using one is reported rather than preferred. tool_payload_dir rather than dep_install_dir because a missing resolver record means different things for the two. For a *dependency of the package being installed* it means the client predates resolved_deps, which is worth warning about. patchelf is a dependency of libxpkg's elfpatch module, not of the package being installed; nothing should have recorded it, so the same warning would fire on every install and name a condition the user cannot act on. R6, .agents/docs/2026-08-06-subos-architecture-proposal.md §1.5. Two tests, and the first was falsified before being kept: breaking the payload lookup makes it fail while the fallback test still passes. --- src/lua-stdlib/xim/libxpkg/elfpatch.lua | 111 +++++++++++++---- src/lua-stdlib/xim/libxpkg/pkginfo.lua | 22 ++++ tests/test_executor.cpp | 155 ++++++++++++++++++++++++ 3 files changed, 262 insertions(+), 26 deletions(-) diff --git a/src/lua-stdlib/xim/libxpkg/elfpatch.lua b/src/lua-stdlib/xim/libxpkg/elfpatch.lua index aa5070d..07c6e0c 100644 --- a/src/lua-stdlib/xim/libxpkg/elfpatch.lua +++ b/src/lua-stdlib/xim/libxpkg/elfpatch.lua @@ -54,8 +54,45 @@ local function _iorun(cmd) return output end --- Find a tool by searching fixed paths then system PATH. --- Search order: subos/bin → _RUNTIME.bin_dir → system PATH (/usr/bin etc.) +-- Which package's payload provides each tool. +-- +-- A tool listed here has a payload answer, and that answer wins. A tool NOT +-- listed here has no payload by design -- `otool` and `install_name_tool` are +-- Xcode's, there is no xpkg that could provide them -- so for those the host +-- is the correct source rather than a fallback, and using it is not reported. +local _tool_provider = { + patchelf = "patchelf", + readelf = "binutils", +} + +-- Find a tool. +-- +-- Resolution order, and why it is this order: +-- +-- 1. the PAYLOAD data/xpkgs/-x-//bin/ +-- 2. the VIEW subos//bin, /bin (reported) +-- 3. the HOST /usr/bin, /usr/local/bin, PATH (reported) +-- +-- The payload comes first because of R6 (xlings/.agents/docs/ +-- 2026-08-06-subos-architecture-proposal.md §1.5): when xlings itself needs a +-- tool it must resolve the payload, never the view. The view -- shims under +-- `subos//bin` -- is a *selection made by the user*: it is mutable, it +-- follows `xlings use`, and a shim reached through PATH re-enters xlings and +-- anchors to whichever home owns the shim. +-- +-- This is not a stylistic preference. `patchelf` is the tool that stamps +-- INTERP and RPATH onto every payload we ship, and patchelf versions differ in +-- how they grow the dynamic segment and in `--force-rpath` semantics. Letting +-- a mutable view -- with a silent fallback to whatever `/usr/bin/patchelf` the +-- build machine happens to have -- decide which one runs means the shape of +-- our artifacts is decided by the environment rather than by us. +-- +-- The old order was 1) subos bin 2) home bin 3) /usr/bin 4) PATH, with the +-- payload not a candidate at all. In the default configuration every one of +-- those resolves to the same file, which is why it survived: the answers agree +-- by coincidence until a second home, a second version, or a host install of +-- the tool exists. +-- -- Returns { program = "/abs/path/to/tool" } or nil. local function _find_tool(toolname) if _tool_cache[toolname] ~= nil then @@ -63,46 +100,68 @@ local function _find_tool(toolname) return _tool_cache[toolname] end - local candidates = {} + local function _accept(p, how) + local tool = { program = p } + if how then + -- Not a debug line. Landing here means the artifact about to be + -- produced was stamped by a tool we did not choose, and the only + -- moment that is observable is now. + _warn(string.format( + "%s resolved to %s (%s), not to a payload. The package that " + .. "provides it (%s) is not in this home's store; declare it " + .. "as a build dep to make this deterministic.", + toolname, p, how, tostring(_tool_provider[toolname]))) + else + _info("using " .. toolname .. ": " .. p .. " (payload)") + end + _tool_cache[toolname] = tool + return tool + end + + -- 1. The payload. One answer, immutable, not reachable through any view. + -- + -- type(), not truthiness: on a client whose libxpkg predates + -- tool_payload_dir the field is nil here, but the same probe written as + -- `if pkginfo.tool_payload_dir then` on a module proxy is true everywhere. + local provider = _tool_provider[toolname] + if provider then + local pkginfo = _LIBXPKG_MODULES and _LIBXPKG_MODULES["pkginfo"] + if pkginfo and type(pkginfo.tool_payload_dir) == "function" then + local ok, dir = pcall(pkginfo.tool_payload_dir, provider) + if ok and dir and dir ~= "" then + local exe = path.join(dir, "bin", toolname) + if is_host("windows") then exe = exe .. ".exe" end + if os.isfile(exe) then return _accept(exe, nil) end + end + end + end - -- 1. subos bin (patchelf, readelf live here) + -- 2. The view. Kept because a home whose store predates this change still + -- has to work, and because a user may deliberately have put a tool + -- there -- but it is now reported rather than preferred. local sysroot = _RUNTIME and _RUNTIME.subos_sysrootdir if sysroot and sysroot ~= "" then - candidates[#candidates + 1] = path.join(sysroot, "bin", toolname) + local p = path.join(sysroot, "bin", toolname) + if os.isfile(p) then return _accept(p, "subos view") end end - -- 2. _RUNTIME.bin_dir (~/.xlings/bin) local bin_dir = _RUNTIME and _RUNTIME.bin_dir if bin_dir then - candidates[#candidates + 1] = path.join(bin_dir, toolname) - end - - -- 3. macOS system tools - if is_host("macosx") then - candidates[#candidates + 1] = "/usr/bin/" .. toolname + local p = path.join(bin_dir, toolname) + if os.isfile(p) then return _accept(p, "home bin") end end - -- 4. common system paths - candidates[#candidates + 1] = "/usr/bin/" .. toolname - candidates[#candidates + 1] = "/usr/local/bin/" .. toolname - - for _, p in ipairs(candidates) do + -- 3. The host. + for _, p in ipairs({ "/usr/bin/" .. toolname, "/usr/local/bin/" .. toolname }) do if os.isfile(p) then - local tool = { program = p } - _info("using " .. toolname .. ": " .. p) - _tool_cache[toolname] = tool - return tool + return _accept(p, provider and "host" or nil) end end - -- 5. Last resort: search system PATH via shell local which_cmd = is_host("windows") and "where" or "which" local resolved = _trim(_iorun(which_cmd .. " " .. _shell_quote(toolname))) if resolved and resolved ~= "" and os.isfile(resolved) then - local tool = { program = resolved } - _info("using " .. toolname .. ": " .. resolved .. " (PATH)") - _tool_cache[toolname] = tool - return tool + return _accept(resolved, provider and "host PATH" or nil) end _warn(toolname .. " not found") diff --git a/src/lua-stdlib/xim/libxpkg/pkginfo.lua b/src/lua-stdlib/xim/libxpkg/pkginfo.lua index 56b4d3c..1c4f5c7 100644 --- a/src/lua-stdlib/xim/libxpkg/pkginfo.lua +++ b/src/lua-stdlib/xim/libxpkg/pkginfo.lua @@ -261,6 +261,28 @@ function M.install_dir(pkgname, pkgversion) return nil end +-- The payload of a tool libxpkg ITSELF needs, e.g. patchelf for elfpatch. +-- +-- Same resolution as dep_install_dir, one difference in diagnostics: a missing +-- resolver record is not reported. That warning exists because a *dependency* +-- of the package being installed should have been resolved by the resolver, so +-- its absence means the client predates resolved_deps. A tool of libxpkg is not +-- a dependency of the package being installed -- nothing should have recorded +-- it -- so the same warning here would be a false report on every install, and +-- one the user cannot act on. +-- +-- Returns the install_dir, or nil. Callers must handle nil: on a home where the +-- tool's package is not installed there is no payload answer, and inventing one +-- is exactly the failure this function exists to prevent. +function M.tool_payload_dir(tool_pkg, tool_version) + if not tool_pkg or tool_pkg == "" then return nil end + local rec = M.resolved_dep(tool_pkg, tool_version) + if rec and rec.install_dir and rec.install_dir ~= "" then + return rec.install_dir + end + return _resolve_dep_via_scan(tool_pkg, tool_version) +end + -- ───────────────────────────────────────────────────────────────────── -- build_dep API -- ───────────────────────────────────────────────────────────────────── diff --git a/tests/test_executor.cpp b/tests/test_executor.cpp index 8692926..7590ddd 100644 --- a/tests/test_executor.cpp +++ b/tests/test_executor.cpp @@ -435,6 +435,161 @@ TEST(ExecutorTest, ApplyElfpatchAuto_LinuxUsesPatchelfForElf) { fs::remove_all(temp_dir); } +// patchelf is what stamps INTERP and RPATH onto every payload we ship, so +// "which patchelf" decides what our artifacts look like -- versions differ in +// how they grow the dynamic segment and in --force-rpath semantics. +// +// It used to be decided by a mutable view: subos//bin first, then the +// home's bin, then /usr/bin, then PATH, with the payload not a candidate at +// all. Every one of those resolves to the same file in the default +// configuration, which is why it survived -- the answers agree by coincidence +// until a second home, a second version, or a host install exists. +// +// Two patchelf binaries here, distinguishable only by what they log. The +// payload one is not on PATH and not in bin_dir; the view one is both. If the +// payload does not win, the assertion below fails on the marker. +// See the architecture proposal's R6 (internal consumers bind the payload). +TEST(ExecutorTest, FindTool_PrefersPayloadOverViewAndHost) { +#ifdef _WIN32 + GTEST_SKIP() << "Tool emulation test is POSIX-specific"; +#endif + + const fs::path temp_dir = make_temp_dir("libxpkg-findtool-payload-"); + const fs::path store = temp_dir / "xpkgs"; + const fs::path payload_bin = store / "xim-x-patchelf" / "0.18.0" / "bin"; + const fs::path view_dir = temp_dir / "tools"; + const fs::path install_dir = store / "xim-x-findtool" / "1.0.0"; + const fs::path lib_dir = install_dir / "lib"; + const fs::path log_path = temp_dir / "tool.log"; + const fs::path pkg_path = temp_dir / "findtool.lua"; + const fs::path binary_path = install_dir / "demo-bin"; + + fs::create_directories(payload_bin); + fs::create_directories(view_dir); + fs::create_directories(lib_dir); + + write_executable_script(payload_bin / "patchelf", + "#!/bin/sh\n" + "printf 'PAYLOAD %s\\n' \"$*\" >> \"$ELFPATCH_LOG\"\n"); + write_executable_script(view_dir / "patchelf", + "#!/bin/sh\n" + "printf 'VIEW %s\\n' \"$*\" >> \"$ELFPATCH_LOG\"\n"); + + { + std::ofstream binary(binary_path, std::ios::binary); + ASSERT_TRUE(binary.good()); + const unsigned char magic[] = {0x7f, 'E', 'L', 'F', 0, 0, 0, 0}; + binary.write(reinterpret_cast(magic), sizeof(magic)); + } + fs::permissions(binary_path, + fs::perms::owner_read | fs::perms::owner_write | fs::perms::owner_exec, + fs::perm_options::replace); + + write_text(pkg_path, + "package = { spec = \"1\", name = \"findtool\", xpm = { linux = { [\"latest\"] = { ref = \"1.0.0\" }, [\"1.0.0\"] = { url = \"https://example.com/demo.tar.gz\", sha256 = \"0\" } } } }\n" + "local elfpatch = import(\"xim.libxpkg.elfpatch\")\n" + "function install()\n" + " elfpatch.auto({ enable = true })\n" + " return true\n" + "end\n"); + + const std::string original_path = std::getenv("PATH") ? std::getenv("PATH") : ""; + ScopedEnvVar path_env("PATH", view_dir.string() + ":" + original_path); + ScopedEnvVar log_env("ELFPATCH_LOG", log_path.string()); + + auto exec = create_executor(pkg_path); + ASSERT_TRUE(exec.has_value()) << (exec ? "" : exec.error()); + + auto ctx = make_context(install_dir, "linux", view_dir); + ctx.xpkg_dir = store; + auto hook_result = exec->run_hook(HookType::Install, ctx); + ASSERT_TRUE(hook_result.success) << hook_result.error; + + auto patch_result = exec->apply_elfpatch_auto(); + EXPECT_TRUE(patch_result.success) << patch_result.error; + + std::ifstream log_file(log_path); + std::ostringstream log_buffer; + log_buffer << log_file.rdbuf(); + const std::string log = log_buffer.str(); + + EXPECT_NE(log.find("PAYLOAD"), std::string::npos) + << "the payload patchelf never ran; log was:\n" << log; + EXPECT_EQ(log.find("VIEW"), std::string::npos) + << "the view's patchelf ran even though a payload exists. The tool that " + "stamps INTERP and RPATH must not be selected by a mutable view.\n" + "log was:\n" << log; + + fs::remove_all(temp_dir); +} + +// The other half of the contract: with no payload in the store, the view is +// still usable. A home whose store predates this change has to keep working -- +// the change is which answer WINS, not the removal of the others. +TEST(ExecutorTest, FindTool_FallsBackToViewWhenNoPayloadExists) { +#ifdef _WIN32 + GTEST_SKIP() << "Tool emulation test is POSIX-specific"; +#endif + + const fs::path temp_dir = make_temp_dir("libxpkg-findtool-fallback-"); + const fs::path store = temp_dir / "xpkgs"; + const fs::path view_dir = temp_dir / "tools"; + const fs::path install_dir = store / "xim-x-findtool" / "1.0.0"; + const fs::path lib_dir = install_dir / "lib"; + const fs::path log_path = temp_dir / "tool.log"; + const fs::path pkg_path = temp_dir / "findtool.lua"; + const fs::path binary_path = install_dir / "demo-bin"; + + fs::create_directories(view_dir); + fs::create_directories(lib_dir); + + write_executable_script(view_dir / "patchelf", + "#!/bin/sh\n" + "printf 'VIEW %s\\n' \"$*\" >> \"$ELFPATCH_LOG\"\n"); + + { + std::ofstream binary(binary_path, std::ios::binary); + ASSERT_TRUE(binary.good()); + const unsigned char magic[] = {0x7f, 'E', 'L', 'F', 0, 0, 0, 0}; + binary.write(reinterpret_cast(magic), sizeof(magic)); + } + fs::permissions(binary_path, + fs::perms::owner_read | fs::perms::owner_write | fs::perms::owner_exec, + fs::perm_options::replace); + + write_text(pkg_path, + "package = { spec = \"1\", name = \"findtool\", xpm = { linux = { [\"latest\"] = { ref = \"1.0.0\" }, [\"1.0.0\"] = { url = \"https://example.com/demo.tar.gz\", sha256 = \"0\" } } } }\n" + "local elfpatch = import(\"xim.libxpkg.elfpatch\")\n" + "function install()\n" + " elfpatch.auto({ enable = true })\n" + " return true\n" + "end\n"); + + const std::string original_path = std::getenv("PATH") ? std::getenv("PATH") : ""; + ScopedEnvVar path_env("PATH", view_dir.string() + ":" + original_path); + ScopedEnvVar log_env("ELFPATCH_LOG", log_path.string()); + + auto exec = create_executor(pkg_path); + ASSERT_TRUE(exec.has_value()) << (exec ? "" : exec.error()); + + auto ctx = make_context(install_dir, "linux", view_dir); + ctx.xpkg_dir = store; + auto hook_result = exec->run_hook(HookType::Install, ctx); + ASSERT_TRUE(hook_result.success) << hook_result.error; + + auto patch_result = exec->apply_elfpatch_auto(); + EXPECT_TRUE(patch_result.success) << patch_result.error; + + std::ifstream log_file(log_path); + std::ostringstream log_buffer; + log_buffer << log_file.rdbuf(); + const std::string log = log_buffer.str(); + EXPECT_NE(log.find("VIEW"), std::string::npos) + << "no payload and no view means no patching at all; log was:\n" << log; + + fs::remove_all(temp_dir); +} + // Regression: patchelf 0.18.0 corrupts compact ELFs (e.g. ninja 1.12.1 // at 273 KB) when --set-interpreter runs before --set-rpath. The interp // op extends PT_LOAD and shifts the dynamic section; the subsequent From 5d80a039eabd0c86f5fe915ed94cb737bb74d8b2 Mon Sep 17 00:00:00 2001 From: sunrisepeak Date: Thu, 6 Aug 2026 07:48:09 +0800 Subject: [PATCH 2/3] =?UTF-8?q?feat(elfpatch):=20relocate=5Fbuild=5Fpaths?= =?UTF-8?q?=20=E2=80=94=20enumerate,=20anchor,=20and=20assert?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A downloaded prebuilt carries the build machine's absolute paths in its text files: linker scripts, .pc files, shell wrappers. Recipes have been rewriting them by hand, and glibc's version got all three parts wrong at once. This is the shared capability that replaces it, not a fourth copy. 1. ENUMERATE, do not list. glibc named six files. Measured on the payload on disk: five files hold build paths, four of them ARE on that list and were processed wrongly, and the fifth was never looked at. A list of what someone thought of is not a measurement (R7). 2. ANCHOR the path token. glibc matched `([^%s)]+)//lib`. `[^%s)]+` runs leftward through anything that is not whitespace or `)` -- including variable names and quotes. On the shipped `bin/ldd` it swallowed `RTLDLIST="` along with the path: RTLDLIST="/p/lib/ld-linux.so.2 /p/lib64/... /p/libx32/..." ./ld-linux.so.2 .64/ld-linux-x86-64.so.2 .x32/ld-linux-x32.so.2" The ldd in the 2.39 and 2.44 payloads does not survive `bash -n`. Here the match walks back from the marker to a character that cannot occur in a path and requires what is left to be absolute -- deliberately a walk, not a pattern, because a leftward pattern is greedy by construction. 3. ANCHOR AT THE MARKER, not at `/lib`. glibc's tail anchor meant the same file's `share/locale` line was never touched, so the build path stayed in the artifact -- the one thing the code existed to remove. And `/libx32` matched `/lib` plus a leftover `x32`, which is where `.x32/` came from. Then the part that made all of the above invisible: it wrote if it had substituted anything and called that success. "Still has build paths" and "we corrupted the file" produced the same output as a clean run. So the result is now ASSERTED (R4): no absolute build path may remain anywhere in the payload, and every rewritten shell script must parse -- checked with the script's OWN shebang interpreter, because a fixed `sh -n` gives a verdict that depends on what the host's /bin/sh is. Binaries are skipped (a text substitution changes their length), symlinks are never written through, and an occurrence that is already relative is left relative rather than made absolute. Two tests. The fixture reproduces the shipped damage byte for byte: running glibc's current pattern over it produces exactly the line that is in ~/.xlings/data/xpkgs/xim-x-glibc/2.39/bin/ldd today. Version 0.0.51. --- mcpp.toml | 2 +- src/lua-stdlib/xim/libxpkg/elfpatch.lua | 257 ++++++++++++++++++++++++ tests/test_executor.cpp | 161 +++++++++++++++ 3 files changed, 419 insertions(+), 1 deletion(-) diff --git a/mcpp.toml b/mcpp.toml index 0bfd66c..a5584b5 100644 --- a/mcpp.toml +++ b/mcpp.toml @@ -1,7 +1,7 @@ [package] namespace = "mcpplibs" name = "xpkg" -version = "0.0.50" +version = "0.0.51" description = "C++23 reference implementation of the xpkg V2 spec (multi-arch)" license = "Apache-2.0" repo = "https://github.com/openxlings/libxpkg" diff --git a/src/lua-stdlib/xim/libxpkg/elfpatch.lua b/src/lua-stdlib/xim/libxpkg/elfpatch.lua index 07c6e0c..3325f75 100644 --- a/src/lua-stdlib/xim/libxpkg/elfpatch.lua +++ b/src/lua-stdlib/xim/libxpkg/elfpatch.lua @@ -1135,4 +1135,261 @@ function M.is_shrink() return false end +-- ───────────────────────────────────────────────────────────────────── +-- Build-path relocation +-- ───────────────────────────────────────────────────────────────────── +-- +-- A downloaded prebuilt carries the absolute paths of the machine that built +-- it, baked into text files: linker scripts, .pc files, shell wrappers. +-- Those paths do not exist here, and they leak the build machine's layout +-- into every artifact we ship. +-- +-- Recipes have been doing this by hand, and glibc's hand-rolled version got +-- all three parts wrong at once -- which is why this is a shared capability +-- rather than a fourth copy: +-- +-- 1. It named six files. Enumerate the payload instead. (R7: a list of +-- what someone thought of is not a measurement. glibc's list had four +-- of the five affected files on it and still missed one, and processed +-- the four wrongly.) +-- +-- 2. Its pattern was `([^%s)]+)//lib`. `[^%s)]+` runs LEFTWARD +-- through anything that is not whitespace or `)` -- including variable +-- names and quotes. On the real payload it ate `RTLDLIST="` along with +-- the path, and the `ldd` we ship does not survive `bash -n`. Match an +-- anchored path TOKEN instead: walk back from the marker to a character +-- that cannot occur in a path, and require what is left to be absolute. +-- +-- 3. It anchored the tail at `/lib`, so the same file's +-- `.../share/locale` was left untouched -- the build path stayed in the +-- artifact, which was the one thing the code existed to prevent. Anchor +-- at the marker and keep whatever follows. +-- +-- And it reported success on writing anything at all. Here the rewrite is +-- ASSERTED, not hoped for (R4): afterwards no marker may remain anywhere in +-- the payload, and every rewritten shell script must parse. Either failure +-- raises. +-- +-- elfpatch.relocate_build_paths{ +-- marker = "fromsource-x-glibc/" .. pkginfo.version(), +-- dir = pkginfo.install_dir(), -- default: install_dir +-- to = pkginfo.install_dir(), -- default: dir +-- } +-- +-- `marker` is the part of the build path that identifies this payload -- it +-- is what makes an absolute path OURS rather than a legitimate reference to +-- /usr or /etc, which must not be touched. + +-- Characters that cannot appear inside a path token in the files we rewrite. +-- `:` is included because these strings appear in PATH-like lists; `=` and +-- the quotes because of shell assignments, which is where the old pattern +-- did its damage. +local _PATH_DELIMS = { + [" "]="", ["\t"]="", ["\n"]="", ["\r"]="", ["\0"]="", + ["\""]="", ["'"]="", ["`"]="", + ["("]="", [")"]="", ["{"]="", ["}"]="", ["["]="", ["]"]="", + ["="]="", [","]="", [";"]="", [":"]="", + ["<"]="", [">"]="", ["|"]="", ["&"]="", ["*"]="", +} + +-- Where the absolute path containing [s,e] starts, or nil if the token that +-- contains the marker is not an absolute path. +-- +-- Deliberately NOT a Lua pattern. A pattern that scans leftward is greedy by +-- construction and there is no way to say "stop at the start of the token" +-- without enumerating the stop set anyway -- so enumerate it, and walk. +local function _abs_token_start(content, s) + local i = s - 1 + while i >= 1 do + local c = content:sub(i, i) + if _PATH_DELIMS[c] then break end + i = i - 1 + end + local start = i + 1 + if content:sub(start, start) ~= "/" then return nil end + return start +end + +local function _is_binary(content) + -- A NUL in the first 8 KiB. Text files we rewrite (scripts, .pc, linker + -- scripts) have none; ELF has one in byte 5. Cheaper and more portable + -- than magic-number tables, and wrong only in the safe direction: a + -- misjudged binary is skipped, not corrupted. + return content:sub(1, 8192):find("\0", 1, true) ~= nil +end + +-- The interpreter to syntax-check a rewritten script with, or nil. +-- +-- The script's OWN shebang, not a fixed `sh -n`. glibc's `ldd` is +-- `#! /bin/bash` and uses bash's `$"..."`; checking it with dash would either +-- reject valid input or accept broken input depending on the host's /bin/sh, +-- and a check whose verdict depends on the machine is not a check. +local function _script_checker(filepath, content) + local shebang = content:sub(1, 256):match("^#!([^\n]*)") + if shebang then + local interp = shebang:match("^%s*(%S+)") + -- `#!/usr/bin/env bash` names the shell in the argument. + if interp and interp:match("env$") then + interp = shebang:match("^%s*%S+%s+(%S+)") + end + if interp then + local base = interp:match("([^/]+)$") or interp + if base == "sh" or base == "bash" or base == "dash" + or base == "ksh" or base == "zsh" or base == "ash" then + return base + end + return nil -- python, perl, ... not ours to check + end + end + if filepath:sub(-3) == ".sh" then return "sh" end + return nil +end + +function M.relocate_build_paths(opt) + opt = opt or {} + local marker = opt.marker + if not marker or marker == "" then + error("elfpatch.relocate_build_paths: `marker` is required -- it is " + .. "what distinguishes a build path of ours from a legitimate " + .. "reference to /usr or /etc") + end + + local pkginfo = _LIBXPKG_MODULES and _LIBXPKG_MODULES["pkginfo"] + local dir = opt.dir + if (not dir or dir == "") and pkginfo then dir = pkginfo.install_dir() end + if not dir or dir == "" or not os.isdir(dir) then + error("elfpatch.relocate_build_paths: no payload directory to scan (" + .. tostring(dir) .. ")") + end + local to = opt.to + if not to or to == "" then to = dir end + to = to:gsub("/+$", "") + + local fs = _LIBXPKG_MODULES and _LIBXPKG_MODULES["fs"] + if not fs or type(fs.files) ~= "function" then + error("elfpatch.relocate_build_paths: this client's libxpkg has no " + .. "recursive file walk; cannot enumerate the payload") + end + + local files = fs.files(dir, true) or {} + local scanned, rewritten, occurrences = 0, 0, 0 + local touched_scripts = {} + + local is_symlink = type(fs.is_symlink) == "function" + and fs.is_symlink or function() return false end + + for _, filepath in ipairs(files) do + -- Never through a symlink. fs.files reports a symlink to a regular + -- file as a regular file, and rewriting through one would write + -- outside the payload -- possibly onto a file another package owns. + local f = (not is_symlink(filepath)) and io.open(filepath, "rb") or nil + if f then + local content = f:read("*a") or "" + f:close() + scanned = scanned + 1 + if not _is_binary(content) and content:find(marker, 1, true) then + local out, pos, hits = {}, 1, 0 + while true do + local s, e = content:find(marker, pos, true) + if not s then break end + local tok = _abs_token_start(content, s) + if tok then + out[#out + 1] = content:sub(pos, tok - 1) + out[#out + 1] = to + hits = hits + 1 + pos = e + 1 + else + -- A relative or already-rewritten occurrence. Copied + -- through untouched: rewriting it would invent an + -- absolute path where the file deliberately has none. + out[#out + 1] = content:sub(pos, e) + pos = e + 1 + end + end + out[#out + 1] = content:sub(pos) + local new_content = table.concat(out) + if hits > 0 and new_content ~= content then + local w = io.open(filepath, "wb") + if not w then + error("elfpatch.relocate_build_paths: cannot write " + .. filepath) + end + w:write(new_content) + w:close() + rewritten = rewritten + 1 + occurrences = occurrences + hits + local checker = _script_checker(filepath, new_content) + if checker then + touched_scripts[#touched_scripts + 1] = + { path = filepath, interp = checker } + end + end + end + end + end + + -- ── assert, do not hope (R4) ────────────────────────────────────── + -- + -- Both checks run over the result, not over the intent. The version this + -- replaces treated "we wrote something" as success, so "there is still a + -- build path in the payload" and "we corrupted the file" produced exactly + -- the same output as a clean run: nothing. + + local leftovers = {} + for _, filepath in ipairs(files) do + local f = (not is_symlink(filepath)) and io.open(filepath, "rb") or nil + if f then + local content = f:read("*a") or "" + f:close() + if not _is_binary(content) then + -- Every occurrence, not just the first: a file may hold a + -- deliberately relative reference and an absolute leftover, + -- and checking only the first would pass on the relative one. + local pos = 1 + while true do + local s, e = content:find(marker, pos, true) + if not s then break end + if _abs_token_start(content, s) then + leftovers[#leftovers + 1] = filepath + break + end + pos = e + 1 + end + end + end + end + if #leftovers > 0 then + error(string.format( + "elfpatch.relocate_build_paths: %d file(s) still contain an " + .. "absolute build path matching '%s' after relocation: %s", + #leftovers, marker, table.concat(leftovers, ", "))) + end + + -- Only where a shell exists to ask. On Windows there is none, and a + -- payload of shell scripts is not a Windows payload anyway. + if not is_host("windows") then + local broken = {} + for _, s in ipairs(touched_scripts) do + if not _exec_ok(s.interp .. " -n " .. _shell_quote(s.path)) then + broken[#broken + 1] = s.path + end + end + if #broken > 0 then + error(string.format( + "elfpatch.relocate_build_paths: rewriting broke %d shell " + .. "script(s) -- they no longer parse: %s", + #broken, table.concat(broken, ", "))) + end + end + + _info(string.format( + "relocated %d occurrence(s) of '%s' in %d of %d file(s) -> %s" + .. " (%d script(s) re-parsed)", + occurrences, marker, rewritten, scanned, to, #touched_scripts)) + + return { scanned = scanned, rewritten = rewritten, + occurrences = occurrences, scripts = #touched_scripts } +end + + return M diff --git a/tests/test_executor.cpp b/tests/test_executor.cpp index 7590ddd..8d91fde 100644 --- a/tests/test_executor.cpp +++ b/tests/test_executor.cpp @@ -435,6 +435,167 @@ TEST(ExecutorTest, ApplyElfpatchAuto_LinuxUsesPatchelfForElf) { fs::remove_all(temp_dir); } +// A downloaded prebuilt carries the build machine's absolute paths in its text +// files. glibc's recipe knew this and rewrote them, and got all three parts of +// the job wrong -- so this is the shared capability that replaces it. +// +// The fixture reproduces the real damage byte for byte. glibc's pattern was +// `([^%s)]+)//lib`, whose `[^%s)]+` runs leftward through anything that +// is not whitespace or `)`. On the shipped `bin/ldd` it swallowed `RTLDLIST="` +// along with the path, and the ldd in the 2.39 and 2.44 payloads on disk today +// does not survive `bash -n`. It also anchored the tail at `/lib`, so the same +// file's `share/locale` line was left alone -- a build path still in the +// artifact, which was the only thing the code existed to remove. +TEST(ExecutorTest, RelocateBuildPaths_AnchorsTheTokenAndAssertsTheResult) { +#ifdef _WIN32 + GTEST_SKIP() << "Shell syntax check is POSIX-specific"; +#endif + + const fs::path temp_dir = make_temp_dir("libxpkg-relocate-"); + const fs::path install_dir = temp_dir / "install"; + const fs::path pkg_path = temp_dir / "relocate.lua"; + const std::string marker = "fromsource-x-glibc/2.39"; + const std::string built = "/home/xlings/.xlings_data/xim/xpkgs/" + marker; + + fs::create_directories(install_dir / "bin"); + fs::create_directories(install_dir / "lib"); + fs::create_directories(install_dir / "lib/pkgconfig"); + + // The exact two lines that broke, in the order they appear in ldd. + write_executable_script(install_dir / "bin/ldd", + "#! /bin/bash\n" + "TEXTDOMAIN=libc\n" + "TEXTDOMAINDIR=" + built + "/share/locale\n" + "RTLDLIST=\"" + built + "/lib/ld-linux.so.2 " + + built + "/lib64/ld-linux-x86-64.so.2 " + + built + "/libx32/ld-linux-x32.so.2\"\n" + "case \"$1\" in\n" + " --version) printf $\"Copyright (C) %s\\n\" \"2024\" ;;\n" + "esac\n"); + + // A linker script: the path sits inside parentheses, which the old + // pattern's stop set treated specially and this one does not need to. + write_text(install_dir / "lib/libc.so", + "/* GNU ld script */\n" + "GROUP ( " + built + "/lib/libc.so.6 " + built + "/lib/libc_nonshared.a" + " AS_NEEDED ( " + built + "/lib/ld-linux-x86-64.so.2 ) )\n"); + + // NOT on glibc's six-file list. Enumeration is the point: a list of what + // someone thought of is not a measurement of what is there. + write_text(install_dir / "lib/pkgconfig/libc.pc", + "prefix=" + built + "\n" + "libdir=${prefix}/lib\n" + "Name: libc\n"); + + // A binary holding the same bytes. Rewriting it would change its length + // and corrupt it; that is patchelf's job, not a text substitution's. + const std::string binary_body = std::string("\x7f", 1) + "ELF" + + std::string("\0\0\0\0", 4) + built + "/lib\n"; + { + std::ofstream b(install_dir / "lib/probe.so", std::ios::binary); + b.write(binary_body.data(), + static_cast(binary_body.size())); + } + + // A relative reference, already correct. Rewriting it would invent an + // absolute path where the file deliberately has none. + write_text(install_dir / "lib/relative.txt", "./" + marker + "/lib\n"); + + write_text(pkg_path, + "package = { spec = \"1\", name = \"relocate\", xpm = { linux = { [\"latest\"] = { ref = \"1.0.0\" }, [\"1.0.0\"] = { url = \"https://example.com/demo.tar.gz\", sha256 = \"0\" } } } }\n" + "local elfpatch = import(\"xim.libxpkg.elfpatch\")\n" + "local pkginfo = import(\"xim.libxpkg.pkginfo\")\n" + "function install()\n" + " elfpatch.relocate_build_paths{ marker = \"" + marker + "\" }\n" + " return true\n" + "end\n"); + + auto exec = create_executor(pkg_path); + ASSERT_TRUE(exec.has_value()) << (exec ? "" : exec.error()); + + auto hook_result = exec->run_hook(HookType::Install, + make_context(install_dir, "linux")); + ASSERT_TRUE(hook_result.success) << hook_result.error; + + const auto read = [](const fs::path& p) { + std::ifstream in(p, std::ios::binary); + std::ostringstream ss; ss << in.rdbuf(); return ss.str(); + }; + + const auto ldd = read(install_dir / "bin/ldd"); + // The assignment survived. This is the whole regression. + EXPECT_NE(ldd.find("RTLDLIST=\""), std::string::npos) + << "the shell assignment was swallowed with the path:\n" << ldd; + EXPECT_EQ(ldd.find(built), std::string::npos) + << "a build path is still in the artifact:\n" << ldd; + // All three loader dirs, including the two the /lib anchor mangled. + EXPECT_NE(ldd.find(install_dir.string() + "/lib/ld-linux.so.2"), + std::string::npos) << ldd; + EXPECT_NE(ldd.find(install_dir.string() + "/lib64/ld-linux-x86-64.so.2"), + std::string::npos) << ldd; + EXPECT_NE(ldd.find(install_dir.string() + "/libx32/ld-linux-x32.so.2"), + std::string::npos) << ldd; + // The line the /lib anchor never reached. + EXPECT_NE(ldd.find(install_dir.string() + "/share/locale"), + std::string::npos) << ldd; + + // And it still parses. The payload on disk today does not. + EXPECT_EQ(std::system(("bash -n " + (install_dir / "bin/ldd").string() + + " 2>/dev/null").c_str()), 0) + << "the rewritten script no longer parses:\n" << ldd; + + const auto libc_so = read(install_dir / "lib/libc.so"); + EXPECT_EQ(libc_so.find(built), std::string::npos) << libc_so; + EXPECT_NE(libc_so.find(install_dir.string() + "/lib/libc.so.6"), + std::string::npos) << libc_so; + // The closing parens of the linker script survived. + EXPECT_NE(libc_so.find(") )"), std::string::npos) << libc_so; + + const auto pc = read(install_dir / "lib/pkgconfig/libc.pc"); + EXPECT_EQ(pc.find(built), std::string::npos) + << "a file that was not on the old hand-written list kept its build " + "path:\n" << pc; + + EXPECT_EQ(read(install_dir / "lib/probe.so"), binary_body) + << "a binary was rewritten as text"; + EXPECT_EQ(read(install_dir / "lib/relative.txt"), "./" + marker + "/lib\n") + << "a deliberately relative reference was made absolute"; + + fs::remove_all(temp_dir); +} + +// The assertion half. A rewrite that corrupts a script must fail the install, +// not report success -- glibc's version treated "we wrote something" as +// success, so "still has build paths" and "we broke the file" both produced +// exactly the output of a clean run: nothing. +TEST(ExecutorTest, RelocateBuildPaths_FailsWhenAMarkerIsMissing) { +#ifdef _WIN32 + GTEST_SKIP() << "Shell syntax check is POSIX-specific"; +#endif + const fs::path temp_dir = make_temp_dir("libxpkg-relocate-nomarker-"); + const fs::path install_dir = temp_dir / "install"; + const fs::path pkg_path = temp_dir / "relocate.lua"; + fs::create_directories(install_dir); + + write_text(pkg_path, + "package = { spec = \"1\", name = \"relocate\", xpm = { linux = { [\"latest\"] = { ref = \"1.0.0\" }, [\"1.0.0\"] = { url = \"https://example.com/demo.tar.gz\", sha256 = \"0\" } } } }\n" + "local elfpatch = import(\"xim.libxpkg.elfpatch\")\n" + "function install()\n" + " elfpatch.relocate_build_paths{}\n" + " return true\n" + "end\n"); + + auto exec = create_executor(pkg_path); + ASSERT_TRUE(exec.has_value()) << (exec ? "" : exec.error()); + auto hook_result = exec->run_hook(HookType::Install, + make_context(install_dir, "linux")); + EXPECT_FALSE(hook_result.success) + << "relocation without a marker would rewrite any absolute path in " + "the payload, including legitimate references to /usr"; + + fs::remove_all(temp_dir); +} + // patchelf is what stamps INTERP and RPATH onto every payload we ship, so // "which patchelf" decides what our artifacts look like -- versions differ in // how they grow the dynamic segment and in --force-rpath semantics. From 8a3a666936dd410a7d0e8a919f37889f8759adcd Mon Sep 17 00:00:00 2001 From: sunrisepeak Date: Thu, 6 Aug 2026 08:22:17 +0800 Subject: [PATCH 3/3] fix(elfpatch): two defects the self-review found in relocate_build_paths The backward walk had no lower bound. A marker occurring TWICE inside one path token would walk back past the previous match, and the slice that copies the text between them -- `content:sub(pos, tok - 1)` -- would come out empty, silently deleting it. Not reachable through any path in the glibc payload, because `:` and whitespace bound the tokens there, but "not reachable today" is how the pattern this replaces was justified too. Bounded at the first byte not yet emitted. And a missing interpreter read as a broken script. `_exec_ok` cannot tell "syntax error" from "command not found", so a payload holding a zsh script on a machine without zsh would have FAILED THE INSTALL -- a check inventing the defect it exists to find. The interpreter is now probed once per name, and a script that cannot be checked is reported as unchecked rather than counted as broken. --- src/lua-stdlib/xim/libxpkg/elfpatch.lua | 31 +++++++++++++++++++++---- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/src/lua-stdlib/xim/libxpkg/elfpatch.lua b/src/lua-stdlib/xim/libxpkg/elfpatch.lua index 3325f75..4f4814f 100644 --- a/src/lua-stdlib/xim/libxpkg/elfpatch.lua +++ b/src/lua-stdlib/xim/libxpkg/elfpatch.lua @@ -1198,14 +1198,20 @@ local _PATH_DELIMS = { -- Deliberately NOT a Lua pattern. A pattern that scans leftward is greedy by -- construction and there is no way to say "stop at the start of the token" -- without enumerating the stop set anyway -- so enumerate it, and walk. -local function _abs_token_start(content, s) +-- `floor` bounds the walk at the first byte not yet emitted. Without it a +-- marker occurring TWICE inside one path token would walk back past the +-- previous match, and `content:sub(pos, tok - 1)` would then be empty -- +-- silently deleting everything between the two occurrences. +local function _abs_token_start(content, s, floor) + floor = floor or 1 local i = s - 1 - while i >= 1 do + while i >= floor do local c = content:sub(i, i) if _PATH_DELIMS[c] then break end i = i - 1 end local start = i + 1 + if start < floor then return nil end if content:sub(start, start) ~= "/" then return nil end return start end @@ -1292,7 +1298,7 @@ function M.relocate_build_paths(opt) while true do local s, e = content:find(marker, pos, true) if not s then break end - local tok = _abs_token_start(content, s) + local tok = _abs_token_start(content, s, pos) if tok then out[#out + 1] = content:sub(pos, tok - 1) out[#out + 1] = to @@ -1368,12 +1374,27 @@ function M.relocate_build_paths(opt) -- Only where a shell exists to ask. On Windows there is none, and a -- payload of shell scripts is not a Windows payload anyway. if not is_host("windows") then - local broken = {} + local broken, unchecked = {}, {} + local have = {} for _, s in ipairs(touched_scripts) do - if not _exec_ok(s.interp .. " -n " .. _shell_quote(s.path)) then + if have[s.interp] == nil then + have[s.interp] = _exec_ok("command -v " .. _shell_quote(s.interp)) + end + if not have[s.interp] then + -- Not a failure. `_exec_ok` cannot tell "syntax error" from + -- "command not found", and failing an install because the + -- machine has no zsh would be a check inventing a defect. + unchecked[#unchecked + 1] = s.path .. " (no " .. s.interp .. ")" + elseif not _exec_ok(s.interp .. " -n " .. _shell_quote(s.path)) then broken[#broken + 1] = s.path end end + if #unchecked > 0 then + _warn(string.format( + "rewrote %d script(s) whose interpreter is not on this machine, " + .. "so they were not re-parsed: %s", + #unchecked, table.concat(unchecked, ", "))) + end if #broken > 0 then error(string.format( "elfpatch.relocate_build_paths: rewriting broke %d shell "