Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -643,6 +643,11 @@ shell arguments. Such changes require a new operation/binding, not an ordinary
respawn under the old request digest. Unbound legacy sessions retain manifest
reload and operation-tag mutation behavior.

Session launch validates the requested working directory, then stores an absolute
cwd in run metadata before starting the child. Relative paths are resolved
against the launching process's current directory without resolving symlinks;
absolute paths retain their spelling. Existing records are not rewritten.

Rust callers use `pty_core::registry::revocation::{root_id, install, read, check,
PublicationBinding, PublicationRevocation, RevocationInstall}`. `root_id()` returns
`io::Result<String>` for prelaunch enrollment. `install(name,
Expand Down
33 changes: 33 additions & 0 deletions crates/pty/src/daemon/env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,25 @@ pub fn invalid_cwd_error(reason: &str, name: &str, command: &str) -> String {
format!("{reason}\nCannot start session \"{name}\" for command \"{command}\".")
}

/// Validate the caller's spelling before fixing the cwd shared by child and
/// metadata. Lexical absolutization preserves symlinks and existing errors.
pub fn prepare_cwd(cfg: &mut DaemonConfig) -> Result<String, String> {
let cwd = cfg.cwd();
if let Some(reason) = describe_invalid_cwd(&cwd) {
return Err(invalid_cwd_error(&reason, &cfg.name, &cfg.command));
}
let cwd = if std::path::Path::new(&cwd).is_absolute() {
cwd
} else {
std::path::absolute(&cwd)
.map_err(|error| error.to_string())?
.to_string_lossy()
.into_owned()
};
cfg.cwd = Some(cwd.clone());
Ok(cwd)
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down Expand Up @@ -407,6 +426,20 @@ mod tests {
);
}

#[test]
fn preparing_cwd_preserves_validation_errors_and_input() {
for cwd in ["", "/no/such/dir", "/bin/sh", "no-such-cwd/child"] {
let mut cfg = cfg();
cfg.cwd = Some(cwd.into());
let reason = describe_invalid_cwd(cwd).unwrap();
assert_eq!(
prepare_cwd(&mut cfg).unwrap_err(),
invalid_cwd_error(&reason, &cfg.name, &cfg.command)
);
assert_eq!(cfg.cwd.as_deref(), Some(cwd));
}
}

#[test]
fn invalid_cwd_texts() {
assert_eq!(
Expand Down
10 changes: 4 additions & 6 deletions crates/pty/src/daemon/lifecycle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ use pty_terminal::{TerminalActor, serialize};
use super::DaemonConfig;
use super::clients::{Client, ClientFacts, Out, REDRAW_SETTLE, Role};
use super::daemon_warn;
use super::env::{build_child_env, describe_invalid_cwd, invalid_cwd_error, permanent_respawn_env};
use super::env::{build_child_env, permanent_respawn_env};
use super::input_budget::{INPUT_BUDGET, InputBudget, InputHold};
use pty_core::process_tree::{
KILL_WAIT, ProcTable, ProcessIdentity, TERM_WAIT, TreeSnapshot, complete_snapshot_from_table,
Expand Down Expand Up @@ -346,7 +346,7 @@ fn install_ordinary_child_test_hook() -> Result<(), String> {

/// Run the daemon for `cfg` to completion; the return value is the process
/// exit status (the child's code after a natural exit, 0 after a kill).
pub(crate) fn run(cfg: DaemonConfig, readiness: super::ReadyNotifier) -> Result<i32, String> {
pub(crate) fn run(mut cfg: DaemonConfig, readiness: super::ReadyNotifier) -> Result<i32, String> {
let mut kill_signals = catch_kill_signals()?;
let name = cfg.name.clone();
let generation = cfg
Expand All @@ -364,12 +364,10 @@ pub(crate) fn run(cfg: DaemonConfig, readiness: super::ReadyNotifier) -> Result<
published_tags.insert(lease.lifecycle_tag.clone(), lease.starting_value.clone());
}
let events = EventWriter::new(&name);
let (rows, cols, cwd) = (cfg.rows(), cfg.cols(), cfg.cwd());
let (rows, cols) = (cfg.rows(), cfg.cols());

let child_env = build_child_env(&cfg, &generation)?;
if let Some(reason) = describe_invalid_cwd(&cwd) {
return Err(invalid_cwd_error(&reason, &name, &cfg.command));
}
let cwd = super::env::prepare_cwd(&mut cfg)?;

// The PTY pair first: opening it fails before anything is published.
let pair = pty_spawn::open(rows, cols)
Expand Down
13 changes: 3 additions & 10 deletions crates/pty/src/daemon/operation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -432,7 +432,7 @@ impl Worker {
}

fn run(
self,
mut self,
commands: Receiver<WorkerCmd>,
rx: Receiver<Msg>,
readiness: super::ReadyNotifier,
Expand Down Expand Up @@ -692,7 +692,7 @@ impl Worker {

/// Everything publication needs that is not itself an effect.
fn prepare(
&self,
&mut self,
generation: &str,
) -> Result<
(
Expand All @@ -702,14 +702,7 @@ impl Worker {
String,
> {
let child_env = super::env::build_child_env(&self.cfg, generation)?;
let cwd = self.cfg.cwd();
if let Some(reason) = super::env::describe_invalid_cwd(&cwd) {
return Err(super::env::invalid_cwd_error(
&reason,
&self.cfg.name,
&self.cfg.command,
));
}
super::env::prepare_cwd(&mut self.cfg)?;
let pair = pty_spawn::open(self.cfg.rows(), self.cfg.cols()).map_err(|e| {
format!(
"Failed to open a PTY for session \"{}\": {e}",
Expand Down
28 changes: 28 additions & 0 deletions crates/pty/tests/daemon_lifecycle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,34 @@ fn publication_order_and_shapes() {
}
}

#[test]
fn launch_stores_an_absolute_cwd_without_resolving_symlinks() {
skip_without_a_real_machine!();
let _s = serial();
let current = std::env::current_dir().unwrap();

for relative in [true, false] {
let root = short_root();
let link = root.join("cwd-link");
std::os::unix::fs::symlink(&current, &link).unwrap();
let cwd = if relative { "." } else { link.to_str().unwrap() };
let name = unique_name("absolute-cwd");
let marker = root.join(format!("{name}.started"));
let script = format!("pwd -P > '{}'; exec sleep 30", marker.display());
let mut cfg = config(&name, "sh", &["-c", &script]);
cfg["cwd"] = json!(cwd);
let d = Daemon::start(&root, cfg);
assert!(wait_until(T, || marker.exists()));
let metadata = d.meta().unwrap();
let stored = std::path::Path::new(metadata["cwd"].as_str().unwrap());
assert!(stored.is_absolute());
let expected = if relative { &current } else { &link };
assert_eq!(stored, expected);
assert!(wait_until(T, || std::fs::read_to_string(&marker)
.is_ok_and(|value| value.trim() == current.to_str().unwrap())));
}
}

/// The owner record is published before the child starts.
///
/// compoundingtech/pty#180: the daemon spawned its child before publishing,
Expand Down
Loading