Skip to content

fix(cli): clear LD_PRELOAD after one-shot-token library loads #1232

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Mossaka merged 1 commit into from
Mar 11, 2026
Merged

fix(cli): clear LD_PRELOAD after one-shot-token library loads #1232

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 47 additions & 0 deletions containers/agent/one-shot-token/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,7 @@ fn init_token_list(state: &mut TokenState) {
);
}
state.initialized = true;
clear_ld_preload(state.debug_enabled);
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Calling clear_ld_preload() during init_token_list() runs on the first intercepted getenv() call (even for non-sensitive vars like PATH). In the entrypoint flow, intermediary processes (e.g., capsh/shell wrappers) are likely to call getenv() before exec-ing the real workload; unsetting LD_PRELOAD at that point can prevent the library from being preloaded into the final command at all (since LD_PRELOAD must be present at execve time) and can also remove protection before any sensitive tokens have been cached/unset. Consider delaying the LD_PRELOAD/LD_LIBRARY_PATH cleanup until after the first sensitive token has been accessed and removed from the environment (e.g., gate on a new state flag and trigger after successfully handling a sensitive token), rather than during initialization.

This issue also appears on line 239 of the same file.

Suggested change
clear_ld_preload(state.debug_enabled);

Copilot uses AI. Check for mistakes.
return;
}

Expand Down Expand Up @@ -225,6 +226,26 @@ fn init_token_list(state: &mut TokenState) {
);
}
state.initialized = true;
clear_ld_preload(state.debug_enabled);
}

/// Unset LD_PRELOAD and LD_LIBRARY_PATH from the environment so child processes
/// don't inherit them. The library is already loaded in this process's address space,
/// so getenv interception continues to work. This fixes Deno 2.x's scoped --allow-run
/// permissions which reject spawning subprocesses when LD_PRELOAD is set.
/// See: https://github.com/github/gh-aw-firewall/issues/1001
fn clear_ld_preload(debug_enabled: bool) {
// SAFETY: unsetenv is a standard POSIX function. We pass valid C strings.
unsafe {
let ld_preload = CString::new("LD_PRELOAD").unwrap();
libc::unsetenv(ld_preload.as_ptr());
let ld_library_path = CString::new("LD_LIBRARY_PATH").unwrap();
libc::unsetenv(ld_library_path.as_ptr());
Comment on lines +241 to +243
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

libc::unsetenv returns an int status; currently failures are silently ignored. Since this is a security-related cleanup step, it would be safer to check the return value and (at minimum) emit a debug warning on failure so it's observable when running into unusual libc/env implementations.

Suggested change
libc::unsetenv(ld_preload.as_ptr());
let ld_library_path = CString::new("LD_LIBRARY_PATH").unwrap();
libc::unsetenv(ld_library_path.as_ptr());
let rc_preload = libc::unsetenv(ld_preload.as_ptr());
if debug_enabled && rc_preload != 0 {
eprintln!(
"[one-shot-token] WARNING: Failed to unset LD_PRELOAD (libc::unsetenv returned {})",
rc_preload
);
}
let ld_library_path = CString::new("LD_LIBRARY_PATH").unwrap();
let rc_ld_library_path = libc::unsetenv(ld_library_path.as_ptr());
if debug_enabled && rc_ld_library_path != 0 {
eprintln!(
"[one-shot-token] WARNING: Failed to unset LD_LIBRARY_PATH (libc::unsetenv returned {})",
rc_ld_library_path
);
}

Copilot uses AI. Check for mistakes.
}

if debug_enabled {
eprintln!("[one-shot-token] Cleared LD_PRELOAD and LD_LIBRARY_PATH from environment");
}
}

/// Check if a token name is sensitive
Expand Down Expand Up @@ -433,4 +454,30 @@ mod tests {
assert!(!state.initialized);
}

#[test]
fn test_clear_ld_preload_removes_env_vars() {
// Set LD_PRELOAD and LD_LIBRARY_PATH in the environment
unsafe {
let key = CString::new("LD_PRELOAD").unwrap();
let val = CString::new("/tmp/test.so").unwrap();
libc::setenv(key.as_ptr(), val.as_ptr(), 1);

Comment on lines +460 to +464
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test mutates process-wide environment variables (LD_PRELOAD, LD_LIBRARY_PATH) but doesn't save and restore their previous values. If the test runner (or other tests) relies on either variable, this can cause hard-to-diagnose cross-test interference. Consider capturing the previous values (including the "unset" case) and restoring them in a drop guard / finally-style cleanup at the end of the test.

Copilot uses AI. Check for mistakes.
let key2 = CString::new("LD_LIBRARY_PATH").unwrap();
let val2 = CString::new("/tmp/lib").unwrap();
libc::setenv(key2.as_ptr(), val2.as_ptr(), 1);
}

// Call clear_ld_preload
clear_ld_preload(false);

// Verify they were unset
unsafe {
let key = CString::new("LD_PRELOAD").unwrap();
assert!(call_real_getenv(key.as_ptr()).is_null());

let key2 = CString::new("LD_LIBRARY_PATH").unwrap();
assert!(call_real_getenv(key2.as_ptr()).is_null());
}
}

}
Loading