diff --git a/.changeset/current-clippy-baseline.md b/.changeset/current-clippy-baseline.md new file mode 100644 index 000000000..a14e86242 --- /dev/null +++ b/.changeset/current-clippy-baseline.md @@ -0,0 +1,5 @@ +--- +"@googleworkspace/cli": patch +--- + +Keep Apps Script file selection compatible with the current Clippy checks. diff --git a/.changeset/scoped-file-roots.md b/.changeset/scoped-file-roots.md new file mode 100644 index 000000000..6e17edcc3 --- /dev/null +++ b/.changeset/scoped-file-roots.md @@ -0,0 +1,13 @@ +--- +"@googleworkspace/cli": minor +--- + +Allow operators to set `GOOGLE_WORKSPACE_CLI_FILE_ROOT` to an existing directory +for `--output` and `--upload` paths while keeping CWD confinement by default. +Relative CLI paths remain CWD-relative. Reject invalid roots, parent traversal +with an explicit root, control characters, and symlink escapes, including +dangling symlinks. Directory flags retain their existing boundaries. + +Reject canonical file paths that cannot be represented as UTF-8 at the CLI +string boundary, so explicit output/upload paths cannot silently become omitted +arguments. diff --git a/AGENTS.md b/AGENTS.md index 722112264..196fc4fda 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -112,6 +112,7 @@ When adding new helpers or CLI flags that accept file paths, **always validate** | -------------------------------------- | ---------------------------------------- | -------------------------------------------------------------------- | | File path for writing (`--output-dir`) | `validate::validate_safe_output_dir()` | Absolute paths, `../` traversal, symlinks outside CWD, control chars | | File path for reading (`--dir`) | `validate::validate_safe_dir_path()` | Absolute paths, `../` traversal, symlinks outside CWD, control chars | +| File path (`--output`, `--upload`) | `validate::validate_safe_file_path()` | Paths outside CWD or the trusted `GOOGLE_WORKSPACE_CLI_FILE_ROOT`, control chars, symlink escapes; `..` components with an explicit root | | Enum/allowlist values (`--msg-format`) | clap `value_parser` (see `gmail/mod.rs`) | Any value not in the allowlist | ```rust @@ -208,6 +209,7 @@ See [`src/helpers/README.md`](crates/google-workspace-cli/src/helpers/README.md) | Variable | Description | |---|---| | `GOOGLE_WORKSPACE_CLI_CONFIG_DIR` | Override the config directory (default: `~/.config/gws`) | +| `GOOGLE_WORKSPACE_CLI_FILE_ROOT` | Trusted boundary for `--output` / `--upload` files (default: CWD). Must be an existing directory; canonicalized. Relative CLI paths remain CWD-relative. Does not expand directory validators. | ### OAuth Client diff --git a/README.md b/README.md index 04c532d0a..e286c75d8 100644 --- a/README.md +++ b/README.md @@ -266,6 +266,39 @@ Installing this extension gives your Gemini CLI agent direct access to all `gws` gws drive files create --json '{"name": "report.pdf"}' --upload ./report.pdf ``` +### Output and upload file roots + +`--output` and `--upload` accept paths within the current working directory +(CWD) by default, including absolute paths that resolve inside CWD. To allow +files elsewhere, set a trusted operator environment variable to an existing +directory: + +```bash +mkdir -p /tmp/gws-files +export GOOGLE_WORKSPACE_CLI_FILE_ROOT=/tmp/gws-files +gws drive files get --params '{"fileId":"FILE_ID","alt":"media"}' \ + --output /tmp/gws-files/report.pdf +gws drive files create --json '{"name":"report.pdf"}' \ + --upload /tmp/gws-files/report.pdf +``` + +The root replaces the allowed file boundary; relative CLI paths still resolve +from CWD. For example, `--output report.pdf` is rejected if CWD is outside the +configured root. The root is canonicalized and must exist as a directory; an +empty or invalid value fails validation. Relative root settings resolve from +CWD too. With an explicit root, CLI paths containing `..` components are +rejected. Control characters and symlinks escaping the boundary are rejected; +symlinks resolving inside it are allowed, but dangling symlinks are rejected. +These CLI file flags require a UTF-8 canonical path. If a symlink resolves to a +path with unsupported encoding, the command returns a validation error rather +than dropping the upload or selecting the default output file. + +This setting affects only these file flags, not `--dir` or `--output-dir`. +It does not create parent directories or change the default download filename +when `--output` is omitted. Validation cannot prevent another local process +from replacing a path component between validation and I/O; choose a root +whose directories you control. Unset the variable to restore the CWD boundary. + ### Pagination | Flag | Description | Default | @@ -382,6 +415,7 @@ All variables are optional. See [`.env.example`](.env.example) for a copy-paste | `GOOGLE_WORKSPACE_CLI_CLIENT_ID` | OAuth client ID (alternative to `client_secret.json`) | | `GOOGLE_WORKSPACE_CLI_CLIENT_SECRET` | OAuth client secret (paired with `CLIENT_ID`) | | `GOOGLE_WORKSPACE_CLI_CONFIG_DIR` | Override config directory (default: `~/.config/gws`) | +| `GOOGLE_WORKSPACE_CLI_FILE_ROOT` | Existing directory allowed for `--output` / `--upload` paths (default: CWD); relative CLI paths remain CWD-relative | | `GOOGLE_WORKSPACE_CLI_SANITIZE_TEMPLATE` | Default Model Armor template | | `GOOGLE_WORKSPACE_CLI_SANITIZE_MODE` | `warn` (default) or `block` | | `GOOGLE_WORKSPACE_CLI_LOG` | Log level for stderr (e.g., `gws=debug`). Off by default. | diff --git a/crates/google-workspace-cli/src/helpers/gmail/mod.rs b/crates/google-workspace-cli/src/helpers/gmail/mod.rs index caeb8b6b0..27de3eb9c 100644 --- a/crates/google-workspace-cli/src/helpers/gmail/mod.rs +++ b/crates/google-workspace-cli/src/helpers/gmail/mod.rs @@ -3008,6 +3008,28 @@ mod tests { // --- Attachment tests --- + // Attachment parsing calls the public file validator. Default-policy tests + // must ignore and restore an inherited operator root, including on panic. + // All users of this guard are serialized with the other environment tests. + struct DefaultFileRoot(Option); + + impl DefaultFileRoot { + fn unset() -> Self { + let saved = Self(std::env::var_os("GOOGLE_WORKSPACE_CLI_FILE_ROOT")); + std::env::remove_var("GOOGLE_WORKSPACE_CLI_FILE_ROOT"); + saved + } + } + + impl Drop for DefaultFileRoot { + fn drop(&mut self) { + match &self.0 { + Some(root) => std::env::set_var("GOOGLE_WORKSPACE_CLI_FILE_ROOT", root), + None => std::env::remove_var("GOOGLE_WORKSPACE_CLI_FILE_ROOT"), + } + } + } + fn make_attach_matches(args: &[&str]) -> ArgMatches { let cmd = Command::new("test").arg( Arg::new("attach") @@ -3095,14 +3117,18 @@ mod tests { } #[test] + #[serial_test::serial] fn test_parse_attachments_rejects_control_chars() { + let _root = DefaultFileRoot::unset(); let matches = make_attach_matches(&["test", "-a", "file\0name.pdf"]); let err = parse_attachments(&matches).unwrap_err(); assert!(err.to_string().contains("control characters")); } #[test] + #[serial_test::serial] fn test_parse_attachments_rejects_directory() { + let _root = DefaultFileRoot::unset(); // Use a relative directory that exists in CWD let matches = make_attach_matches(&["test", "-a", "src"]); let err = parse_attachments(&matches).unwrap_err(); @@ -3110,14 +3136,18 @@ mod tests { } #[test] + #[serial_test::serial] fn test_parse_attachments_empty_returns_empty_vec() { + let _root = DefaultFileRoot::unset(); let matches = make_attach_matches(&["test"]); let attachments = parse_attachments(&matches).unwrap(); assert!(attachments.is_empty()); } #[test] + #[serial_test::serial] fn test_parse_attachments_reads_real_file() { + let _root = DefaultFileRoot::unset(); use std::io::Write; let cwd = std::env::current_dir().unwrap().canonicalize().unwrap(); let dir = tempfile::tempdir_in(&cwd).unwrap(); @@ -3137,7 +3167,9 @@ mod tests { } #[test] + #[serial_test::serial] fn test_parse_attachments_nonexistent_file() { + let _root = DefaultFileRoot::unset(); let matches = make_attach_matches(&["test", "-a", "nonexistent_file.pdf"]); let err = parse_attachments(&matches).unwrap_err(); assert!( @@ -3148,7 +3180,9 @@ mod tests { } #[test] + #[serial_test::serial] fn test_parse_attachments_unknown_extension_falls_back_to_octet_stream() { + let _root = DefaultFileRoot::unset(); use std::io::Write; let cwd = std::env::current_dir().unwrap().canonicalize().unwrap(); let dir = tempfile::tempdir_in(&cwd).unwrap(); @@ -3165,7 +3199,9 @@ mod tests { } #[test] + #[serial_test::serial] fn test_parse_attachments_size_limit_accumulates() { + let _root = DefaultFileRoot::unset(); let cwd = std::env::current_dir().unwrap().canonicalize().unwrap(); let dir = tempfile::tempdir_in(&cwd).unwrap(); @@ -3193,7 +3229,9 @@ mod tests { } #[test] + #[serial_test::serial] fn test_parse_attachments_rejects_empty_file() { + let _root = DefaultFileRoot::unset(); let cwd = std::env::current_dir().unwrap().canonicalize().unwrap(); let dir = tempfile::tempdir_in(&cwd).unwrap(); let file_path = dir.path().join("empty.txt"); diff --git a/crates/google-workspace-cli/src/helpers/script.rs b/crates/google-workspace-cli/src/helpers/script.rs index 11bcdebec..4b31db62d 100644 --- a/crates/google-workspace-cli/src/helpers/script.rs +++ b/crates/google-workspace-cli/src/helpers/script.rs @@ -169,13 +169,7 @@ fn process_file(path: &Path) -> Result, GwsError> { filename.trim_end_matches(".js").trim_end_matches(".gs"), ), "html" => ("HTML", filename.trim_end_matches(".html")), - "json" => { - if filename == "appsscript.json" { - ("JSON", "appsscript") - } else { - return Ok(None); - } - } + "json" if filename == "appsscript.json" => ("JSON", "appsscript"), _ => return Ok(None), }; diff --git a/crates/google-workspace-cli/src/main.rs b/crates/google-workspace-cli/src/main.rs index 41dcc1e1f..40052cfae 100644 --- a/crates/google-workspace-cli/src/main.rs +++ b/crates/google-workspace-cli/src/main.rs @@ -225,7 +225,7 @@ async fn run() -> Result<(), GwsError> { // Validate file paths against traversal before any I/O. // Use the returned canonical paths so the validated path is the one - // actually used for I/O (closes TOCTOU gap). + // actually used for I/O. Local path-replacement races still apply. let upload_path_buf = if let Some(p) = upload_path { Some(crate::validate::validate_safe_file_path(p, "--upload")?) } else { @@ -236,8 +236,8 @@ async fn run() -> Result<(), GwsError> { } else { None }; - let upload_path = upload_path_buf.as_deref().and_then(|p| p.to_str()); - let output_path = output_path_buf.as_deref().and_then(|p| p.to_str()); + let upload_path = optional_file_path_as_str(upload_path_buf.as_deref(), "--upload")?; + let output_path = optional_file_path_as_str(output_path_buf.as_deref(), "--output")?; let upload = { let upload_content_type = matched_args @@ -299,6 +299,22 @@ async fn run() -> Result<(), GwsError> { .map(|_| ()) } +// The executor takes strings. An explicit path must never become an omitted +// argument just because canonicalization found a non-UTF-8 component. +fn optional_file_path_as_str<'a>( + path: Option<&'a std::path::Path>, + flag_name: &str, +) -> Result, GwsError> { + path.map(|path| { + path.to_str().ok_or_else(|| { + GwsError::Validation(format!( + "{flag_name} resolves to a path that is not valid UTF-8; choose a path whose canonical components are valid UTF-8" + )) + }) + }) + .transpose() +} + /// Select the best scope from a method's scope list. /// /// Discovery Documents list method scopes as alternatives — any single scope @@ -525,6 +541,57 @@ fn is_version_flag(arg: &str) -> bool { mod tests { use super::*; + #[test] + fn file_root_path_encoding_preserves_present_and_absent_paths() { + for flag in ["--output", "--upload"] { + assert_eq!(optional_file_path_as_str(None, flag).unwrap(), None); + assert_eq!( + optional_file_path_as_str(Some(std::path::Path::new("résumé.pdf")), flag).unwrap(), + Some("résumé.pdf") + ); + } + } + + #[cfg(any(unix, windows))] + fn non_utf8_canonical_path() -> std::path::PathBuf { + // Construct an OS path in memory: no filesystem support is required. + #[cfg(unix)] + { + use std::os::unix::ffi::OsStringExt; + std::ffi::OsString::from_vec(b"/files/bytes-\xff/report.pdf".to_vec()).into() + } + #[cfg(windows)] + { + use std::os::windows::ffi::OsStringExt; + let mut units: Vec = r"C:\files\bytes-".encode_utf16().collect(); + units.push(0xD800); // unpaired surrogate + units.extend(r"\report.pdf".encode_utf16()); + std::ffi::OsString::from_wide(&units).into() + } + } + + #[cfg(any(unix, windows))] + #[test] + fn file_root_path_encoding_rejects_explicit_output_instead_of_fallback() { + let path = non_utf8_canonical_path(); + let error = optional_file_path_as_str(Some(&path), "--output").unwrap_err(); + assert!(matches!(error, GwsError::Validation(_))); + let message = error.to_string(); + assert!(message.contains("--output"), "{message}"); + assert!(message.contains("UTF-8"), "{message}"); + } + + #[cfg(any(unix, windows))] + #[test] + fn file_root_path_encoding_rejects_explicit_upload_instead_of_omitting_it() { + let path = non_utf8_canonical_path(); + let error = optional_file_path_as_str(Some(&path), "--upload").unwrap_err(); + assert!(matches!(error, GwsError::Validation(_))); + let message = error.to_string(); + assert!(message.contains("--upload"), "{message}"); + assert!(message.contains("UTF-8"), "{message}"); + } + #[test] fn test_parse_pagination_config_defaults() { let matches = clap::Command::new("test") diff --git a/crates/google-workspace-cli/tests/file_roots.rs b/crates/google-workspace-cli/tests/file_roots.rs new file mode 100644 index 000000000..9dc516821 --- /dev/null +++ b/crates/google-workspace-cli/tests/file_roots.rs @@ -0,0 +1,274 @@ +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Exercise the real CLI with synthetic cached Discovery and child-only env. + +use std::fs; +use std::io::{Read, Write}; +use std::net::TcpListener; +use std::path::{Path, PathBuf}; +use std::process::{Command, Output}; +use std::time::{Duration, Instant}; + +use serde_json::{json, Value}; +use tempfile::{tempdir, TempDir}; + +struct Fixture { + _temp: TempDir, + cwd: PathBuf, + root: PathBuf, + config: PathBuf, +} + +impl Fixture { + fn new(root_url: &str) -> Self { + let temp = tempdir().unwrap(); + let base = temp.path().canonicalize().unwrap(); + let cwd = base.join("working"); + let root = base.join("files"); + let config = base.join("config"); + fs::create_dir(&cwd).unwrap(); + fs::create_dir(&root).unwrap(); + fs::create_dir_all(config.join("cache")).unwrap(); + // Stop dotenv from searching parent directories for real configuration. + fs::write(cwd.join(".env"), "").unwrap(); + fs::write(config.join("cache/drive_v3.json"), json!({ + "name": "drive", "version": "v3", "rootUrl": root_url, + "resources": {"files": {"methods": { + "get": {"httpMethod": "GET", "path": "files/synthetic"}, + "create": {"httpMethod": "POST", "path": "files", "supportsMediaUpload": true, + "mediaUpload": {"protocols": {"simple": {"path": "/upload/files", "multipart": true}}}} + }}} + }).to_string()).unwrap(); + Self { + _temp: temp, + cwd, + root, + config, + } + } + + fn command(&self, configured: bool) -> Command { + let mut command = Command::new(env!("CARGO_BIN_EXE_gws")); + command + .env_clear() + .current_dir(&self.cwd) + .env("HOME", &self.cwd) + .env("USERPROFILE", &self.cwd) + .env("GOOGLE_WORKSPACE_CLI_CONFIG_DIR", &self.config) + .env("GOOGLE_WORKSPACE_CLI_TOKEN", "synthetic-test-token") + .env("GOOGLE_WORKSPACE_CLI_KEYRING_BACKEND", "file") + .env("NO_COLOR", "1") + // A cache regression must fail locally, never fetch real Discovery. + .env("HTTPS_PROXY", "http://127.0.0.1:1") + .env("HTTP_PROXY", "http://127.0.0.1:1") + .env("NO_PROXY", "127.0.0.1,localhost"); + if configured { + command.env("GOOGLE_WORKSPACE_CLI_FILE_ROOT", &self.root); + } + command + } + + fn dry_run(&self, flag: &str, path: &Path, configured: bool) -> Output { + let method = if flag == "--upload" { "create" } else { "get" }; + self.command(configured) + .args(["drive", "files", method, "--dry-run", flag]) + .arg(path) + .output() + .unwrap() + } +} + +fn successful_json(output: &Output) -> Value { + assert!( + output.status.success(), + "status: {:?}\nstdout: {}\nstderr: {}", + output.status.code(), + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + serde_json::from_slice(&output.stdout).unwrap() +} + +#[test] +fn file_root_cli_dry_run_accepts_external_output_and_upload() { + let fixture = Fixture::new("http://127.0.0.1:1/"); + fs::write(fixture.root.join("upload.txt"), "synthetic upload").unwrap(); + for (flag, name) in [("--output", "new.bin"), ("--upload", "upload.txt")] { + let output = fixture.dry_run(flag, &fixture.root.join(name), true); + let result = successful_json(&output); + assert_eq!(result["dry_run"], true); + assert_eq!(result["is_multipart_upload"], flag == "--upload"); + } + assert!(!fixture.root.join("new.bin").exists()); +} + +#[test] +fn file_root_cli_default_rejects_external_paths_but_keeps_local_output() { + let fixture = Fixture::new("http://127.0.0.1:1/"); + let output = fixture.dry_run("--output", &fixture.root.join("new.bin"), false); + assert_eq!(output.status.code(), Some(3)); + let error: Value = serde_json::from_slice(&output.stdout).unwrap(); + assert!(error["error"]["message"] + .as_str() + .unwrap() + .contains("current directory")); + assert_eq!( + successful_json(&fixture.dry_run("--output", Path::new("new.bin"), false))["dry_run"], + true + ); +} + +#[test] +fn file_root_cli_rejects_cwd_relative_path_outside_configured_root() { + let fixture = Fixture::new("http://127.0.0.1:1/"); + let output = fixture.dry_run("--output", Path::new("new.bin"), true); + assert_eq!( + output.status.code(), + Some(3), + "{}", + String::from_utf8_lossy(&output.stdout) + ); + let error: Value = serde_json::from_slice(&output.stdout).unwrap(); + assert!(error["error"]["message"] + .as_str() + .unwrap() + .contains("GOOGLE_WORKSPACE_CLI_FILE_ROOT")); +} + +#[test] +fn file_root_cli_download_propagates_canonical_output() { + let listener = TcpListener::bind("127.0.0.1:0").unwrap(); + listener.set_nonblocking(true).unwrap(); + let fixture = Fixture::new(&format!("http://{}/", listener.local_addr().unwrap())); + let output_path = fixture.root.join("./output.bin"); + let mut child = fixture + .command(true) + .args(["drive", "files", "get", "--output"]) + .arg(&output_path) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .spawn() + .unwrap(); + let deadline = Instant::now() + Duration::from_secs(15); + loop { + match listener.accept() { + Ok((mut stream, _)) => { + stream + .set_read_timeout(Some(Duration::from_secs(5))) + .unwrap(); + let mut request = Vec::new(); + let mut buffer = [0; 1024]; + while !request.windows(4).any(|part| part == b"\r\n\r\n") { + let read = stream.read(&mut buffer).unwrap(); + assert!(read > 0, "request ended before headers"); + request.extend_from_slice(&buffer[..read]); + } + assert!(request.starts_with(b"GET /files/synthetic HTTP/1.1\r\n")); + stream.write_all(b"HTTP/1.1 200 OK\r\nContent-Type: application/octet-stream\r\nContent-Length: 9\r\nConnection: close\r\n\r\nsynthetic").unwrap(); + break; + } + Err(err) if err.kind() == std::io::ErrorKind::WouldBlock => { + if child.try_wait().unwrap().is_some() { + break; + } + if Instant::now() >= deadline { + child.kill().unwrap(); + let output = child.wait_with_output().unwrap(); + panic!( + "CLI did not contact local fixture: {}", + String::from_utf8_lossy(&output.stderr) + ); + } + std::thread::sleep(Duration::from_millis(10)); + } + Err(err) => panic!("local fixture failed: {err}"), + } + } + let result = successful_json(&child.wait_with_output().unwrap()); + assert_eq!( + result["saved_file"], + fixture.root.join("output.bin").to_str().unwrap() + ); + assert_eq!(result["bytes"], 9); + assert_eq!( + fs::read(fixture.root.join("output.bin")).unwrap(), + b"synthetic" + ); + assert!(!fixture.cwd.join("output.bin").exists()); +} + +// macOS filesystems commonly reject invalid UTF-8 directory names. The CLI +// conversion itself is covered without filesystem access by main.rs unit tests; +// these Linux regressions exercise the complete canonical symlink handoff. +#[cfg(target_os = "linux")] +fn non_utf8_alias(fixture: &Fixture, filename: &str) -> PathBuf { + use std::os::unix::{ffi::OsStringExt, fs::symlink}; + let target = fixture + .root + .join(std::ffi::OsString::from_vec(b"bytes-\xff".to_vec())); + fs::create_dir(&target).unwrap(); + fs::write(target.join("upload.txt"), b"synthetic upload").unwrap(); + let alias = fixture.root.join("alias"); + symlink(&target, &alias).unwrap(); + alias.join(filename) +} + +#[cfg(target_os = "linux")] +#[test] +fn file_root_cli_rejects_non_utf8_output_without_fallback_write() { + let fixture = Fixture::new("http://127.0.0.1:1/"); + let output_path = non_utf8_alias(&fixture, "new.bin"); + let fallback = fixture.cwd.join("download.bin"); + fs::write(&fallback, b"keep existing download").unwrap(); + // A non-dry run must fail at validation, before HTTP or the default output + // can be selected. No live service or credentials are involved. + let output = fixture + .command(true) + .args(["drive", "files", "get", "--output"]) + .arg(&output_path) + .output() + .unwrap(); + assert_eq!(fs::read(&fallback).unwrap(), b"keep existing download"); + assert!(!output_path.exists()); + assert_eq!( + output.status.code(), + Some(3), + "{}", + String::from_utf8_lossy(&output.stdout) + ); + let error: Value = serde_json::from_slice(&output.stdout).unwrap(); + let message = error["error"]["message"].as_str().unwrap(); + assert!(message.contains("--output"), "{message}"); + assert!(message.contains("UTF-8"), "{message}"); +} + +#[cfg(target_os = "linux")] +#[test] +fn file_root_cli_rejects_non_utf8_upload_instead_of_omitting_it() { + let fixture = Fixture::new("http://127.0.0.1:1/"); + let upload_path = non_utf8_alias(&fixture, "upload.txt"); + let output = fixture.dry_run("--upload", &upload_path, true); + assert_eq!( + output.status.code(), + Some(3), + "{}", + String::from_utf8_lossy(&output.stdout) + ); + let error: Value = serde_json::from_slice(&output.stdout).unwrap(); + let message = error["error"]["message"].as_str().unwrap(); + assert!(message.contains("--upload"), "{message}"); + assert!(message.contains("UTF-8"), "{message}"); + assert_eq!(fs::read(&upload_path).unwrap(), b"synthetic upload"); +} diff --git a/crates/google-workspace/src/validate.rs b/crates/google-workspace/src/validate.rs index 32ef200f9..a951cae56 100644 --- a/crates/google-workspace/src/validate.rs +++ b/crates/google-workspace/src/validate.rs @@ -161,11 +161,13 @@ pub fn validate_safe_dir_path(dir: &str) -> Result { /// Validates that a file path (e.g. `--upload` or `--output`) is safe. /// -/// Rejects paths that escape above CWD via `..` traversal, contain -/// control characters, or follow symlinks to locations outside CWD. -/// Absolute paths are allowed (reading an existing file from a known -/// location is legitimate) but the resolved target must still live -/// under CWD. +/// By default, the resolved target must live under CWD. The trusted operator +/// environment variable `GOOGLE_WORKSPACE_CLI_FILE_ROOT` can select a different +/// boundary: an existing directory, canonicalized before validation. With an +/// explicit root, CLI paths must not contain `..` components. Relative CLI paths +/// always resolve from CWD, not from the configured root. Absolute paths within +/// the boundary are allowed. Control characters and symlink escapes are rejected. +/// Directory validators do not use this setting. /// /// # TOCTOU caveat /// @@ -175,48 +177,127 @@ pub fn validate_safe_dir_path(dir: &str) -> Result { /// TOCTOU would require `openat(O_NOFOLLOW)` on each path component, /// which is tracked as a follow-up for Unix platforms. pub fn validate_safe_file_path(path_str: &str, flag_name: &str) -> Result { - reject_dangerous_chars(path_str, flag_name)?; - - let path = Path::new(path_str); let cwd = std::env::current_dir() .map_err(|e| GwsError::Validation(format!("Failed to determine current directory: {e}")))?; + let file_root = std::env::var_os("GOOGLE_WORKSPACE_CLI_FILE_ROOT"); + validate_file_path_with_root( + path_str, + flag_name, + &cwd, + file_root.as_deref().map(Path::new), + ) +} - let resolved = if path.is_absolute() { - path.to_path_buf() - } else { - cwd.join(path) - }; +/// Explicit policy keeps filesystem validation independent of process-global env. +fn validate_file_path_with_root( + path_str: &str, + flag_name: &str, + cwd: &Path, + file_root: Option<&Path>, +) -> Result { + reject_dangerous_chars(path_str, flag_name)?; - // For existing files, canonicalize to resolve symlinks. - // For non-existing files, get the prefix canonicalized then normalize - // the remaining components to resolve any `..` or `.` segments. - let canonical = if resolved.exists() { - resolved.canonicalize().map_err(|e| { - GwsError::Validation(format!("Failed to resolve {flag_name} '{}': {e}", path_str)) + let canonical_root = if let Some(root) = file_root { + if root.as_os_str().is_empty() { + return Err(GwsError::Validation( + "GOOGLE_WORKSPACE_CLI_FILE_ROOT must name an existing directory; got an empty value" + .to_string(), + )); + } + // Environment is trusted: relative roots (including `..`) are valid. + let canonical = cwd.join(root).canonicalize().map_err(|e| { + GwsError::Validation(format!( + "GOOGLE_WORKSPACE_CLI_FILE_ROOT {root:?} must name an existing directory: {e}" + )) + })?; + if !canonical.is_dir() { + return Err(GwsError::Validation(format!( + "GOOGLE_WORKSPACE_CLI_FILE_ROOT {root:?} must name an existing directory" + ))); + } + canonical + } else { + cwd.canonicalize().map_err(|e| { + GwsError::Validation(format!("Failed to canonicalize current directory: {e}")) })? + }; + let boundary = if file_root.is_some() { + format!("GOOGLE_WORKSPACE_CLI_FILE_ROOT directory {canonical_root:?}") } else { - let raw = normalize_non_existing(&resolved)?; - // normalize_non_existing does NOT resolve `..` in the non-existent - // suffix. We must resolve them here to prevent bypass via paths like - // `non_existent/../../etc/passwd`. - normalize_dotdot(&raw) + format!("current directory {canonical_root:?}") }; - let canonical_cwd = cwd.canonicalize().map_err(|e| { - GwsError::Validation(format!("Failed to canonicalize current directory: {e}")) + let path = Path::new(path_str); + if file_root.is_some() + && path + .components() + .any(|component| component == std::path::Component::ParentDir) + { + return Err(GwsError::Validation(format!( + "{flag_name} must not contain parent traversal ('..') components within the {boundary}; use a path without '..'" + ))); + } + + // Path::join preserves absolute arguments; relative arguments stay CWD-relative. + let resolved = cwd.join(path); + let canonical = canonicalize_file_path(&resolved).map_err(|e| { + GwsError::Validation(format!( + "Failed to resolve {flag_name} {path_str:?} within the {boundary}: {e}" + )) })?; + // Preserve default handling of paths that normalize safely within CWD. + let canonical = normalize_dotdot(&canonical); - if !canonical.starts_with(&canonical_cwd) { + if !canonical.starts_with(&canonical_root) { return Err(GwsError::Validation(format!( - "{flag_name} '{}' resolves to '{}' which is outside the current directory", - path_str, - canonical.display() + "{flag_name} {path_str:?} resolves to {canonical:?} which is outside the {boundary}; set GOOGLE_WORKSPACE_CLI_FILE_ROOT to an existing directory containing the intended file" ))); } Ok(canonical) } +/// Canonicalize the existing file or nearest existing parent, then append the +/// missing suffix. Unlike `exists()`, symlink_metadata does not mistake dangling +/// symlinks for missing files. Keep this stricter resolver local to file flags. +fn canonicalize_file_path(path: &Path) -> std::io::Result { + let mut current = path; + let mut remaining = Vec::new(); + loop { + match std::fs::symlink_metadata(current) { + Ok(_) => { + let mut canonical = current.canonicalize()?; + if !remaining.is_empty() && !canonical.is_dir() { + return Err(std::io::Error::new( + std::io::ErrorKind::InvalidInput, + "existing file parent must be a directory", + )); + } + for component in remaining.into_iter().rev() { + canonical.push(component); + } + return Ok(canonical); + } + Err(error) if error.kind() == std::io::ErrorKind::NotFound => { + let name = current.file_name().ok_or_else(|| { + std::io::Error::new( + std::io::ErrorKind::InvalidInput, + "cannot resolve an existing directory prefix", + ) + })?; + remaining.push(name); + current = current.parent().ok_or_else(|| { + std::io::Error::new( + std::io::ErrorKind::InvalidInput, + "cannot resolve a file parent", + ) + })?; + } + Err(error) => return Err(error), + } + } +} + /// Resolve `.` and `..` components in a path without touching the filesystem. fn normalize_dotdot(path: &Path) -> PathBuf { let mut out = PathBuf::new(); @@ -763,11 +844,9 @@ mod tests { let canonical_dir = dir.path().canonicalize().unwrap(); fs::write(canonical_dir.join("test.txt"), "data").unwrap(); - let saved_cwd = std::env::current_dir().unwrap(); - std::env::set_current_dir(&canonical_dir).unwrap(); + let _environment = FilePathEnvironment::set(&canonical_dir, None); let result = validate_safe_file_path("test.txt", "--upload"); - std::env::set_current_dir(&saved_cwd).unwrap(); assert!(result.is_ok(), "expected Ok, got: {result:?}"); } @@ -778,11 +857,9 @@ mod tests { let dir = tempdir().unwrap(); let canonical_dir = dir.path().canonicalize().unwrap(); - let saved_cwd = std::env::current_dir().unwrap(); - std::env::set_current_dir(&canonical_dir).unwrap(); + let _environment = FilePathEnvironment::set(&canonical_dir, None); let result = validate_safe_file_path("../../etc/passwd", "--upload"); - std::env::set_current_dir(&saved_cwd).unwrap(); assert!(result.is_err(), "path traversal should be rejected"); assert!( @@ -792,7 +869,10 @@ mod tests { } #[test] + #[serial] fn test_file_path_rejects_control_chars() { + let dir = tempdir().unwrap(); + let _environment = FilePathEnvironment::set(dir.path(), None); let result = validate_safe_file_path("file\x00.txt", "--output"); assert!(result.is_err(), "null bytes should be rejected"); } @@ -808,11 +888,9 @@ mod tests { let link_path = canonical_dir.join("escape"); std::os::unix::fs::symlink("/tmp", &link_path).unwrap(); - let saved_cwd = std::env::current_dir().unwrap(); - std::env::set_current_dir(&canonical_dir).unwrap(); + let _environment = FilePathEnvironment::set(&canonical_dir, None); let result = validate_safe_file_path("escape/secret.txt", "--output"); - std::env::set_current_dir(&saved_cwd).unwrap(); assert!(result.is_err(), "symlink escape should be rejected"); } @@ -824,15 +902,337 @@ mod tests { let dir = tempdir().unwrap(); let canonical_dir = dir.path().canonicalize().unwrap(); - let saved_cwd = std::env::current_dir().unwrap(); - std::env::set_current_dir(&canonical_dir).unwrap(); + let _environment = FilePathEnvironment::set(&canonical_dir, None); let result = validate_safe_file_path("doesnt_exist/../../etc/passwd", "--output"); - std::env::set_current_dir(&saved_cwd).unwrap(); assert!( result.is_err(), "traversal via non-existent prefix should be rejected" ); } + + // Default public-validator and directory-scope tests isolate global state; + // scoped file-policy tests pass CWD and the trusted root explicitly. + struct FilePathEnvironment { + cwd: PathBuf, + root: Option, + } + + impl FilePathEnvironment { + fn set(cwd: &Path, root: Option<&Path>) -> Self { + let saved = Self { + cwd: std::env::current_dir().unwrap(), + root: std::env::var_os("GOOGLE_WORKSPACE_CLI_FILE_ROOT"), + }; + std::env::set_current_dir(cwd).unwrap(); + match root { + Some(root) => std::env::set_var("GOOGLE_WORKSPACE_CLI_FILE_ROOT", root), + None => std::env::remove_var("GOOGLE_WORKSPACE_CLI_FILE_ROOT"), + } + saved + } + } + + impl Drop for FilePathEnvironment { + fn drop(&mut self) { + std::env::set_current_dir(&self.cwd).unwrap(); + match &self.root { + Some(root) => std::env::set_var("GOOGLE_WORKSPACE_CLI_FILE_ROOT", root), + None => std::env::remove_var("GOOGLE_WORKSPACE_CLI_FILE_ROOT"), + } + } + } + + fn file_path_under_root( + path: &Path, + flag: &str, + cwd: &Path, + root: Option<&Path>, + ) -> Result { + validate_file_path_with_root(path.to_str().unwrap(), flag, cwd, root) + } + + #[test] + fn file_root_default_preserves_cwd_boundary_and_resolution() { + let dir = tempdir().unwrap(); + let cwd = dir.path().canonicalize().unwrap(); + fs::create_dir(cwd.join("nested")).unwrap(); + fs::write(cwd.join("upload.txt"), "synthetic upload").unwrap(); + for path in [cwd.join("upload.txt"), PathBuf::from("upload.txt")] { + assert_eq!( + file_path_under_root(&path, "--upload", &cwd, None).unwrap(), + cwd.join("upload.txt") + ); + } + assert_eq!( + file_path_under_root(Path::new("nested/../new.txt"), "--output", &cwd, None).unwrap(), + cwd.join("new.txt") + ); + for path in [ + cwd.parent().unwrap().join("outside.txt"), + PathBuf::from("../outside.txt"), + ] { + let err = file_path_under_root(&path, "--output", &cwd, None) + .unwrap_err() + .to_string(); + assert!(err.contains("outside the current directory"), "{err}"); + assert!(err.contains("GOOGLE_WORKSPACE_CLI_FILE_ROOT"), "{err}"); + } + assert!(file_path_under_root( + Path::new("missing/../../outside.txt"), + "--output", + &cwd, + None + ) + .is_err()); + } + + #[test] + fn file_root_accepts_absolute_output_and_existing_upload() { + let cwd = tempdir().unwrap(); + let root = tempdir().unwrap(); + let canonical_root = root.path().canonicalize().unwrap(); + fs::write(root.path().join("upload.txt"), "synthetic upload").unwrap(); + for (name, flag) in [("new.txt", "--output"), ("upload.txt", "--upload")] { + assert_eq!( + file_path_under_root(&root.path().join(name), flag, cwd.path(), Some(root.path())) + .unwrap(), + canonical_root.join(name) + ); + } + assert!(!root.path().join("new.txt").exists()); + } + + #[test] + fn file_root_rejects_sibling_even_with_shared_name_prefix() { + let dir = tempdir().unwrap(); + // A literal backslash on Unix, a separator on Windows; both must be + // compared using the escaped canonical representation in diagnostics. + let parent = dir.path().join(r"back\slash"); + fs::create_dir_all(&parent).unwrap(); + let root = parent.join("allowed"); + let sibling = parent.join("allowed-sibling"); + fs::create_dir(&root).unwrap(); + fs::create_dir(&sibling).unwrap(); + let err = file_path_under_root( + &sibling.join("new.txt"), + "--output", + dir.path(), + Some(&root), + ) + .unwrap_err() + .to_string(); + assert!(err.contains("outside"), "{err}"); + assert!(err.contains("GOOGLE_WORKSPACE_CLI_FILE_ROOT"), "{err}"); + assert!( + err.contains(&format!("{:?}", root.canonicalize().unwrap())), + "{err}" + ); + } + + #[test] + fn file_root_rejects_parent_components_even_inside_boundary() { + let root = tempdir().unwrap(); + fs::create_dir(root.path().join("nested")).unwrap(); + for path in ["nested/../new.txt", "missing/../new.txt", "../outside.txt"] { + assert!( + file_path_under_root(Path::new(path), "--output", root.path(), Some(root.path())) + .is_err(), + "accepted {path}" + ); + } + } + + #[test] + fn file_root_rejects_control_and_dangerous_unicode_arguments() { + let root = tempdir().unwrap(); + for path in [ + "bad\0.txt", + "bad\n.txt", + "bad\u{202e}.txt", + "bad\u{200b}.txt", + ] { + assert!( + file_path_under_root(Path::new(path), "--output", root.path(), Some(root.path())) + .is_err(), + "accepted {path:?}" + ); + } + } + + #[test] + fn file_root_rejects_invalid_roots_without_falling_back_to_cwd() { + let cwd = tempdir().unwrap(); + let file = cwd.path().join("file.txt"); + fs::write(&file, "synthetic file").unwrap(); + for root in [PathBuf::new(), cwd.path().join("missing"), file] { + let err = + file_path_under_root(Path::new("new.txt"), "--output", cwd.path(), Some(&root)) + .unwrap_err() + .to_string(); + assert!(err.contains("GOOGLE_WORKSPACE_CLI_FILE_ROOT"), "{err}"); + assert!(err.contains("directory"), "{err}"); + } + } + + #[test] + fn file_root_keeps_relative_arguments_cwd_relative() { + let root = tempdir().unwrap(); + let cwd = root.path().join("working"); + fs::create_dir(&cwd).unwrap(); + assert_eq!( + file_path_under_root(Path::new("new.txt"), "--output", &cwd, Some(root.path())) + .unwrap(), + cwd.canonicalize().unwrap().join("new.txt") + ); + let unrelated = tempdir().unwrap(); + assert!(file_path_under_root( + Path::new("new.txt"), + "--output", + unrelated.path(), + Some(root.path()) + ) + .is_err()); + } + + #[test] + fn file_root_canonicalizes_trusted_relative_root_with_parent_components() { + let root = tempdir().unwrap(); + let cwd = root.path().join("working"); + fs::create_dir(&cwd).unwrap(); + assert_eq!( + file_path_under_root( + Path::new("new.txt"), + "--output", + &cwd, + Some(Path::new("..")) + ) + .unwrap(), + cwd.canonicalize().unwrap().join("new.txt") + ); + } + + #[test] + fn file_root_validates_nested_new_file_parents_without_creating_them() { + let cwd = tempdir().unwrap(); + let root = tempdir().unwrap(); + let output = root.path().join("new/nested/output.bin"); + assert_eq!( + file_path_under_root(&output, "--output", cwd.path(), Some(root.path())).unwrap(), + root.path() + .canonicalize() + .unwrap() + .join("new/nested/output.bin") + ); + assert!(!root.path().join("new").exists()); + let file = root.path().join("file.txt"); + fs::write(&file, "synthetic file").unwrap(); + assert!(file_path_under_root( + &file.join("output.bin"), + "--output", + cwd.path(), + Some(root.path()) + ) + .is_err()); + } + + #[test] + #[serial] + fn file_root_does_not_expand_directory_validators() { + let cwd = tempdir().unwrap(); + let root = tempdir().unwrap(); + let _environment = FilePathEnvironment::set(cwd.path(), Some(root.path())); + assert!(validate_safe_output_dir(root.path().to_str().unwrap()).is_err()); + assert!(validate_safe_dir_path(root.path().to_str().unwrap()).is_err()); + assert_eq!( + validate_safe_output_dir("new").unwrap(), + cwd.path().canonicalize().unwrap().join("new") + ); + assert!(validate_safe_dir_path(".").is_ok()); + } + + #[test] + #[serial] + fn file_root_environment_is_restored_on_unwind() { + let cwd = std::env::current_dir().unwrap(); + let root = std::env::var_os("GOOGLE_WORKSPACE_CLI_FILE_ROOT"); + let dir = tempdir().unwrap(); + let result = std::panic::catch_unwind(|| { + let _environment = FilePathEnvironment::set(dir.path(), Some(dir.path())); + panic!("exercise restoration"); + }); + assert!(result.is_err()); + assert_eq!(std::env::current_dir().unwrap(), cwd); + assert_eq!(std::env::var_os("GOOGLE_WORKSPACE_CLI_FILE_ROOT"), root); + } + + #[cfg(unix)] + #[test] + fn file_root_resolves_inside_symlinks_and_rejects_escapes() { + use std::os::unix::fs::symlink; + let root = tempdir().unwrap(); + let outside = tempdir().unwrap(); + fs::create_dir(root.path().join("inside")).unwrap(); + fs::write(root.path().join("inside/upload.txt"), "inside").unwrap(); + fs::write(outside.path().join("upload.txt"), "outside").unwrap(); + symlink(root.path().join("inside"), root.path().join("safe")).unwrap(); + symlink(outside.path(), root.path().join("escape")).unwrap(); + for (suffix, flag) in [("upload.txt", "--upload"), ("new/output.bin", "--output")] { + assert_eq!( + file_path_under_root( + &root.path().join("safe").join(suffix), + flag, + root.path(), + Some(root.path()) + ) + .unwrap(), + root.path() + .canonicalize() + .unwrap() + .join("inside") + .join(suffix) + ); + assert!(file_path_under_root( + &root.path().join("escape").join(suffix), + flag, + root.path(), + Some(root.path()) + ) + .is_err()); + } + // A trusted root may itself be a symlink to an existing directory. + assert_eq!( + file_path_under_root( + &root.path().join("safe/upload.txt"), + "--upload", + outside.path(), + Some(&root.path().join("safe")) + ) + .unwrap(), + root.path() + .canonicalize() + .unwrap() + .join("inside/upload.txt") + ); + } + + #[cfg(unix)] + #[test] + fn file_root_rejects_dangling_symlinks_and_loops() { + use std::os::unix::fs::symlink; + let root = tempdir().unwrap(); + let outside = tempdir().unwrap(); + symlink(outside.path().join("new.txt"), root.path().join("dangling")).unwrap(); + symlink("loop", root.path().join("loop")).unwrap(); + for root_policy in [None, Some(root.path())] { + for name in ["dangling", "dangling/new.txt", "loop", "loop/new.txt"] { + assert!( + file_path_under_root(Path::new(name), "--output", root.path(), root_policy) + .is_err(), + "accepted {name}" + ); + } + } + } }