Support pre-existing LD_PRELOAD/DYLD_INSERT_LIBRARIES#349
Merged
branchseer merged 4 commits intomainfrom Apr 22, 2026
Merged
Conversation
…#340) When the user's shell had already exported LD_PRELOAD (Linux) or DYLD_INSERT_LIBRARIES (macOS), fspy's ensure_env saw the existing value and returned EINVAL, surfacing as "failed to prepare the command for injection: Invalid argument (os error 22)" and aborting the run. Replace the overwrite-only ensure_env call on the preload variables with a new append_path_env that colon-appends fspy's shim. Appending (rather than prepending) keeps the user's preload ahead of fspy in interposition order, so libc calls short-circuited by the user layer remain invisible to fspy — the tracer must record what actually executed, not what was suppressed. The existing ensure_env is kept for PAYLOAD_ENV_NAME, which is single-valued. Adds a Linux-only e2e fixture that demonstrates both properties: a cdylib test preload (crates/preload_test_lib) short-circuits any path containing "preload_test_short_circuit" and forwards everything else via RTLD_NEXT. The fixture runs with LD_PRELOAD pointing at the test preload and verifies that fspy tracks real.txt (cache miss on modification) but never sees the short-circuited file (cache hit despite modification). The path to the test cdylib is discovered at test-runtime from CARGO_CDYLIB_FILE_PRELOAD_TEST_LIB via a <PRELOAD_TEST_LIB_PATH> placeholder in snapshots.toml, so the test binary is cross-compile and transfer-friendly.
On musl, fspy falls back to seccomp-unotify for access tracking and strips LD_PRELOAD from spawned children (spawn/linux/mod.rs). That breaks the fixture's interposer-chain assumption: without preload_test_lib loaded, the "short-circuited" file is actually opened and seccomp records it, so modifying it becomes a cache miss instead of a hit. Add a "linux-gnu" platform filter that matches target_os = linux and target_env != musl, and switch the fixture to use it.
branchseer
force-pushed
the
claude/fix-issue-340-e2e-6QRM5
branch
from
18962a4 to
2351cc4
Compare
Member
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. Bravo. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
There was a problem hiding this comment.
Pull request overview
Enables fspy to coexist with pre-existing LD_PRELOAD (Linux) and DYLD_INSERT_LIBRARIES (macOS) by appending fspy’s shim to the existing preload list instead of rejecting the spawn.
Changes:
- Added
append_path_env()infspy_shared_unixto append to colon-separated preload env vars (idempotently) and updated Linux/macOS spawn handlers to use it. - Introduced a Linux-only test
LD_PRELOADinterposer library (preload_test_lib) plus a new e2e fixture verifying correct preload chaining behavior. - Enhanced the e2e harness with additional platform filters and runtime placeholder substitution for the artifact-built preload library path.
Reviewed changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/fspy_shared_unix/src/exec/mod.rs | Adds append_path_env() + unit tests to append (not overwrite) preload env vars. |
| crates/fspy_shared_unix/src/spawn/linux/mod.rs | Switches Linux injection from ensure_env() to append_path_env() for LD_PRELOAD. |
| crates/fspy_shared_unix/src/spawn/macos.rs | Switches macOS injection to append_path_env() for DYLD_INSERT_LIBRARIES. |
| crates/preload_test_lib/Cargo.toml | New Linux-only cdylib crate metadata/deps for the test preload library. |
| crates/preload_test_lib/src/lib.rs | New test interposer implementing short-circuit + forwarding behavior for open/openat. |
| crates/vite_task_bin/Cargo.toml | Adds Linux-only dev artifact dependency on preload_test_lib for e2e runs. |
| crates/vite_task_bin/tests/e2e_snapshots/main.rs | Adds platform filters + <PRELOAD_TEST_LIB_PATH> env placeholder resolution. |
| crates/vite_task_bin/tests/e2e_snapshots/fixtures/preexisting_ld_preload/** | New fixture + snapshots covering pre-existing LD_PRELOAD behavior. |
| Cargo.lock | Records the new preload_test_lib package and dependency edge. |
| CHANGELOG.md | Documents the fix for pre-existing preload env vars (Issue #340). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Addresses Copilot review feedback: the trailing \n in the write-file step argument made display_command_line() render the ## snapshot header across multiple lines. The file contents change regardless of the trailing newline, so cache invalidation is still exercised.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes issue #340 by allowing fspy to coexist with user-supplied
LD_PRELOAD(Linux) orDYLD_INSERT_LIBRARIES(macOS) environment variables. Previously, fspy would reject spawns when these variables were already set. Now it appends its tracer shim to the end of the preload list, preserving both the user's preload and correct symbol interposition order.Key Changes
New
append_path_env()function infspy_shared_unix/src/exec/mod.rs: Appends a value to colon-separated path environment variables (likeLD_PRELOAD) instead of overwriting them. The function is idempotent and avoids leading colons. Includes comprehensive unit tests covering edge cases.Updated Linux spawn handler (
fspy_shared_unix/src/spawn/linux/mod.rs): Changed fromensure_env()(which would fail if the variable existed) toappend_path_env()to append fspy's shim to any existingLD_PRELOAD.Updated macOS spawn handler (
fspy_shared_unix/src/spawn/macos.rs): Similarly updated to useappend_path_env()forDYLD_INSERT_LIBRARIES.New test library (
crates/preload_test_lib/src/lib.rs): A Linux-onlyLD_PRELOADlibrary that interceptsopen/openatsyscalls. It short-circuits paths containing the markerpreload_test_short_circuit(returningENOENTwithout forwarding), while forwarding all other calls viaRTLD_NEXT. This allows testing that fspy correctly handles user preloads that short-circuit syscalls.New e2e test fixture (
preexisting_ld_preload): Comprehensive end-to-end test that verifies:E2e test harness improvements (
vite_task_bin/tests/e2e_snapshots/main.rs): Added support for Linux platform filter and environment variable placeholder substitution (<PRELOAD_TEST_LIB_PATH>) to inject the test library path at runtime.Implementation Details
The append strategy is critical: by placing fspy's shim last in the preload list, user preloads that short-circuit syscalls (returning without forwarding to libc) remain invisible to fspy—accurately reflecting what the OS actually executed. This preserves cache correctness when user preloads intercept file operations.
https://claude.ai/code/session_018oB9YLpLUKWprpr1RFJq4k