Skip to content

fix: resolve relative file paths in CLI before sending to daemon #667

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

Open
juliobrasa wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: resolve relative file paths in CLI before sending to daemon #667

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
71 changes: 59 additions & 12 deletions cli/src/commands.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,26 @@
use base64::{engine::general_purpose::STANDARD, Engine};
use serde_json::{json, Value};
use std::io::{self, BufRead};
use std::path::Path;

use crate::color;
use crate::flags::Flags;
use crate::validation::{is_valid_session_name, session_name_error};

/// Resolve a path to an absolute path relative to the CLI's current working directory.
/// This is necessary because the daemon runs in a different directory than the CLI,
/// so relative paths would resolve incorrectly on the daemon side.
fn resolve_path(path: &str) -> String {
let p = Path::new(path);
if p.is_absolute() {
path.to_string()
} else {
std::env::current_dir()
.map(|cwd| cwd.join(p).to_string_lossy().to_string())
.unwrap_or_else(|_| path.to_string())
}
}

/// Error type for command parsing with contextual information
#[derive(Debug)]
pub enum ParseError {
Expand Down Expand Up @@ -232,7 +247,8 @@ pub fn parse_command(args: &[String], flags: &Flags) -> Result<Value, ParseError
context: "upload".to_string(),
usage: "upload <selector> <files...>",
})?;
Ok(json!({ "id": id, "action": "upload", "selector": sel, "files": &rest[1..] }))
let files: Vec<String> = rest[1..].iter().map(|f| resolve_path(f)).collect();
Ok(json!({ "id": id, "action": "upload", "selector": sel, "files": files }))
}
"download" => {
let sel = rest.first().ok_or_else(|| ParseError::MissingArguments {
Expand All @@ -243,7 +259,7 @@ pub fn parse_command(args: &[String], flags: &Flags) -> Result<Value, ParseError
context: "download".to_string(),
usage: "download <selector> <path>",
})?;
Ok(json!({ "id": id, "action": "download", "selector": sel, "path": path }))
Ok(json!({ "id": id, "action": "download", "selector": sel, "path": resolve_path(path) }))
}

// === Keyboard ===
Expand Down Expand Up @@ -475,15 +491,15 @@ pub fn parse_command(args: &[String], flags: &Flags) -> Result<Value, ParseError
_ => (None, None),
};
Ok(
json!({ "id": id, "action": "screenshot", "path": path, "selector": selector, "fullPage": flags.full, "annotate": flags.annotate }),
json!({ "id": id, "action": "screenshot", "path": path.map(|p| resolve_path(p)), "selector": selector, "fullPage": flags.full, "annotate": flags.annotate }),
)
}
"pdf" => {
let path = rest.first().ok_or_else(|| ParseError::MissingArguments {
context: "pdf".to_string(),
usage: "pdf <path>",
})?;
Ok(json!({ "id": id, "action": "pdf", "path": path }))
Ok(json!({ "id": id, "action": "pdf", "path": resolve_path(path) }))
}

// === Snapshot ===
Expand Down Expand Up @@ -985,7 +1001,7 @@ pub fn parse_command(args: &[String], flags: &Flags) -> Result<Value, ParseError
Some("stop") => {
let mut cmd = json!({ "id": id, "action": "trace_stop" });
if let Some(path) = rest.get(1) {
cmd["path"] = json!(path);
cmd["path"] = json!(resolve_path(path));
}
Ok(cmd)
}
Expand Down Expand Up @@ -1048,7 +1064,7 @@ pub fn parse_command(args: &[String], flags: &Flags) -> Result<Value, ParseError
})?;
// Optional URL parameter
let url = rest.get(2);
let mut cmd = json!({ "id": id, "action": "recording_start", "path": path });
let mut cmd = json!({ "id": id, "action": "recording_start", "path": resolve_path(path) });
if let Some(u) = url {
// Add https:// prefix if needed (preserve special schemes)
let url_str = if u.starts_with("http") || u.contains("://") {
Expand All @@ -1068,7 +1084,7 @@ pub fn parse_command(args: &[String], flags: &Flags) -> Result<Value, ParseError
})?;
// Optional URL parameter
let url = rest.get(2);
let mut cmd = json!({ "id": id, "action": "recording_restart", "path": path });
let mut cmd = json!({ "id": id, "action": "recording_restart", "path": resolve_path(path) });
if let Some(u) = url {
// Add https:// prefix if needed (preserve special schemes)
let url_str = if u.starts_with("http") || u.contains("://") {
Expand Down Expand Up @@ -1115,14 +1131,14 @@ pub fn parse_command(args: &[String], flags: &Flags) -> Result<Value, ParseError
context: "state save".to_string(),
usage: "state save <path>",
})?;
Ok(json!({ "id": id, "action": "state_save", "path": path }))
Ok(json!({ "id": id, "action": "state_save", "path": resolve_path(path) }))
}
Some("load") => {
let path = rest.get(1).ok_or_else(|| ParseError::MissingArguments {
context: "state load".to_string(),
usage: "state load <path>",
})?;
Ok(json!({ "id": id, "action": "state_load", "path": path }))
Ok(json!({ "id": id, "action": "state_load", "path": resolve_path(path) }))
}
Some("list") => Ok(json!({ "id": id, "action": "state_list" })),
Some("clear") => {
Expand Down Expand Up @@ -2588,7 +2604,10 @@ mod tests {
fn test_screenshot_path() {
let cmd = parse_command(&args("screenshot out.png"), &default_flags()).unwrap();
assert_eq!(cmd["action"], "screenshot");
assert_eq!(cmd["path"], "out.png");
// Relative paths are resolved to absolute (relative to CWD)
let path = cmd["path"].as_str().unwrap();
assert!(path.ends_with("/out.png"), "path should end with /out.png, got: {}", path);
assert!(Path::new(path).is_absolute(), "path should be absolute, got: {}", path);
}

#[test]
Expand Down Expand Up @@ -2629,15 +2648,43 @@ mod tests {
let cmd = parse_command(&args("screenshot ./output.png"), &default_flags()).unwrap();
assert_eq!(cmd["action"], "screenshot");
assert_eq!(cmd["selector"], serde_json::Value::Null);
assert_eq!(cmd["path"], "./output.png");
let path = cmd["path"].as_str().unwrap();
assert!(path.ends_with("/output.png"), "path should end with /output.png, got: {}", path);
assert!(Path::new(path).is_absolute(), "path should be absolute, got: {}", path);
}

#[test]
fn test_screenshot_with_selector_and_path() {
let cmd = parse_command(&args("screenshot .btn ./button.png"), &default_flags()).unwrap();
assert_eq!(cmd["action"], "screenshot");
assert_eq!(cmd["selector"], ".btn");
assert_eq!(cmd["path"], "./button.png");
let path = cmd["path"].as_str().unwrap();
assert!(path.ends_with("/button.png"), "path should end with /button.png, got: {}", path);
assert!(Path::new(path).is_absolute(), "path should be absolute, got: {}", path);
}

#[test]
fn test_absolute_path_unchanged() {
let cmd = parse_command(&args("screenshot /tmp/out.png"), &default_flags()).unwrap();
assert_eq!(cmd["path"], "/tmp/out.png");
}

#[test]
fn test_pdf_path_resolved() {
let cmd = parse_command(&args("pdf page.pdf"), &default_flags()).unwrap();
let path = cmd["path"].as_str().unwrap();
assert!(path.ends_with("/page.pdf"), "path should end with /page.pdf, got: {}", path);
assert!(Path::new(path).is_absolute(), "path should be absolute, got: {}", path);
}

#[test]
fn test_upload_paths_resolved() {
let cmd = parse_command(&args("upload #file doc.pdf img.png"), &default_flags()).unwrap();
let files = cmd["files"].as_array().unwrap();
for f in files {
let p = f.as_str().unwrap();
assert!(Path::new(p).is_absolute(), "upload file path should be absolute, got: {}", p);
}
}

// === Snapshot ===
Expand Down