fix: harden URL encoding and input validation for AI/LLM callers

.changeset/encode-path-params.md

@@ -0,0 +1,5 @@
+---
+"@googleworkspace/cli": patch
+---
+
+fix: percent-encode path parameters to prevent path traversal

.changeset/harden-input-validation.md

@@ -0,0 +1,12 @@
+---
+"@googleworkspace/cli": patch
+---
+
+fix: harden input validation for AI/LLM callers
+
+- Add `src/validate.rs` with `validate_safe_output_dir`, `validate_msg_format`, and `validate_safe_dir_path` helpers
+- Validate `--output-dir` against path traversal in `gmail +watch` and `events +subscribe`
+- Validate `--msg-format` against allowlist (full, metadata, minimal, raw) in `gmail +watch`
+- Validate `--dir` against path traversal in `script +push`
+- Add clap `value_parser` constraint for `--msg-format`
+- Document input validation patterns in `AGENTS.md`

.changeset/harden-security-validation.md

@@ -0,0 +1,5 @@
+---
+"@googleworkspace/cli": patch
+---
+
+Security: Harden validate_resource_name and fix Gmail watch path traversal

.changeset/remove-manual-urlencoded.md

@@ -0,0 +1,5 @@
+---
+"@googleworkspace/cli": patch
+---
+
+Replace manual `urlencoded()` with reqwest `.query()` builder for safer URL encoding

.changeset/url-safety-helpers.md

@@ -0,0 +1,5 @@
+---
+"@googleworkspace/cli": patch
+---
+
+fix: add shared URL safety helpers for path params (`encode_path_segment`, `validate_resource_name`)

.changeset/warn-api-failures.md

@@ -0,0 +1,5 @@
+---
+"@googleworkspace/cli": patch
+---
+
+fix: warn on stderr when API calls fail silently

AGENTS.md

@@ -72,6 +72,77 @@ vhs docs/demo.tape
 
 ASCII art title cards live in `art/`. The `scripts/show-art.sh` helper clears the screen and cats the file. Portrait scenes use `scene*.txt`; landscape chapters use `long-*.txt`.
 
+## Input Validation & URL Safety
+
+> [!IMPORTANT]
+> This CLI is frequently invoked by AI/LLM agents. Always assume inputs can be adversarial — validate paths against traversal (`../../.ssh`), restrict format strings to allowlists, reject control characters, and encode user values before embedding them in URLs.
+
+### Path Safety (`src/validate.rs`)
+
+When adding new helpers or CLI flags that accept file paths, **always validate** using the shared helpers:
+
+| Scenario | Validator | Rejects |
+|---|---|---|
+| 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 |
+| Enum/allowlist values (`--msg-format`) | clap `value_parser` (see `gmail/mod.rs`) | Any value not in the allowlist |
+
+```rust
+// In your argument parser:
+if let Some(output_dir) = matches.get_one::<String>("output-dir") {
+    crate::validate::validate_safe_output_dir(output_dir)?;
+    builder.output_dir(Some(output_dir.clone()));
+}
+```
+
+### URL Encoding (`src/helpers/mod.rs`)
+
+User-supplied values embedded in URL **path segments** must be percent-encoded. Use the shared helper:
+
+```rust
+// CORRECT — encodes slashes, spaces, and special characters
+let url = format!(
+    "https://www.googleapis.com/drive/v3/files/{}",
+    crate::helpers::encode_path_segment(file_id),
+);
+
+// WRONG — raw user input in URL path
+let url = format!("https://www.googleapis.com/drive/v3/files/{}", file_id);
+```
+
+For **query parameters**, use reqwest's `.query()` builder which handles encoding automatically:
+
+```rust
+// CORRECT — reqwest encodes query values
+client.get(url).query(&[("q", user_query)]).send().await?;
+
+// WRONG — manual string interpolation in query strings
+let url = format!("{}?q={}", base_url, user_query);
+```
+
+### Resource Name Validation (`src/helpers/mod.rs`)
+
+When a user-supplied string is used as a GCP resource identifier (project ID, topic name, space name, etc.) that gets embedded in a URL path, validate it first:
+
+```rust
+// Validates the string does not contain path traversal segments (`..`), control characters, or URL-breaking characters like `?` and `#`.
+let project = crate::helpers::validate_resource_name(&project_id)?;
+let url = format!("https://pubsub.googleapis.com/v1/projects/{}/topics/my-topic", project);
+```
+
+This prevents injection of query parameters, path traversal, or other malicious payloads through resource name arguments like `--project` or `--space`.
+
+### Checklist for New Features
+
+When adding a new helper or CLI command:
+
+1. **File paths** → Use `validate_safe_output_dir` / `validate_safe_dir_path`
+2. **Enum flags** → Constrain via clap `value_parser` or `validate_msg_format`
+3. **URL path segments** → Use `encode_path_segment()`
+4. **Query parameters** → Use reqwest `.query()` builder
+5. **Resource names** (project IDs, space names, topic names) → Use `validate_resource_name()`
+6. **Write tests** for both the happy path AND the rejection path (e.g., pass `../../.ssh` and assert `Err`)
+
 ## Environment Variables
 
 - `GOOGLE_WORKSPACE_CLI_TOKEN` — Pre-obtained OAuth2 access token (highest priority; bypasses all credential file loading)

Cargo.toml

@@ -55,6 +55,7 @@ chrono = "0.4.44"
 keyring = "3.6.3"
 async-trait = "0.1.89"
 serde_yaml = "0.9.34"
+percent-encoding = "2.3.2"
 
 
 # The profile that 'cargo dist' will build with

docs/CONTRIBUTING.md

@@ -55,4 +55,39 @@ If the OAuth refresh token used in the GitHub Actions smoketest expires or needs
    ```bash
    rm smoketest-creds.json
    unset GOOGLE_WORKSPACE_CLI_CREDENTIALS_FILE
-   ```
+   ```
+
+## Development Patterns
+
+### Changesets
+
+Every PR must include a changeset file at `.changeset/<descriptive-name>.md`:
+
+```markdown
+---
+"@googleworkspace/cli": patch
+---
+
+Brief description of the change
+```
+
+Use `patch` for fixes/chores, `minor` for new features, `major` for breaking changes.
+
+### Input Validation & URL Safety
+
+This CLI is designed to be invoked by AI/LLM agents, so all user-supplied inputs must be treated as potentially adversarial. See [AGENTS.md](../AGENTS.md#input-validation--url-safety) for the full reference. The key rules are:
+
+| What you're doing | What to use |
+|---|---|
+| Accepting a file path (`--output-dir`, `--dir`) | `validate::validate_safe_output_dir()` or `validate_safe_dir_path()` |
+| Embedding a value in a URL path segment | `helpers::encode_path_segment()` |
+| Passing query parameters | reqwest `.query()` builder (never string interpolation) |
+| Using a resource name in a URL (`--project`, `--space`) | `helpers::validate_resource_name()` |
+| Accepting an enum flag (`--msg-format`) | clap `value_parser` (see `gmail/mod.rs`) |
+
+### Testing Expectations
+
+- All new validation logic must include **both happy-path and error-path tests**
+- Tests that modify the process CWD must use `#[serial]` from `serial_test`
+- Tempdir paths should be canonicalized before use to handle macOS `/var` → `/private/var` symlinks
+- Run the full suite before submitting: `cargo test && cargo clippy -- -D warnings`

src/generate_skills.rs

@@ -68,7 +68,9 @@ struct SkillIndexEntry {
 /// Entry point for `gws generate-skills`.
 pub async fn handle_generate_skills(args: &[String]) -> Result<(), GwsError> {
     let output_dir = parse_output_dir(args);
-    let output_path = Path::new(&output_dir);
+    // Validate output_dir to prevent path traversal
+    let output_path_buf = crate::validate::validate_safe_output_dir(&output_dir)?;
+    let output_path = output_path_buf.as_path();
     let filter = parse_filter(args);
     let mut index: Vec<SkillIndexEntry> = Vec::new();
 

src/helpers/calendar.rs

@@ -301,14 +301,21 @@ async fn handle_agenda(matches: &ArgMatches) -> Result<(), GwsError> {
             let time_max = &time_max;
             async move {
                 let events_url = format!(
-                    "https://www.googleapis.com/calendar/v3/calendars/{}/events?timeMin={}&timeMax={}&singleEvents=true&orderBy=startTime&maxResults=50",
-                    urlencoded(&cal.id),
-                    urlencoded(time_min),
-                    urlencoded(time_max),
+                    "https://www.googleapis.com/calendar/v3/calendars/{}/events",
+                    crate::validate::encode_path_segment(&cal.id),
                 );
 
                 let resp = crate::client::send_with_retry(|| {
-                    client.get(&events_url).bearer_auth(token)
+                    client
+                        .get(&events_url)
+                        .query(&[
+                            ("timeMin", time_min.as_str()),
+                            ("timeMax", time_max.as_str()),
+                            ("singleEvents", "true"),
+                            ("orderBy", "startTime"),
+                            ("maxResults", "50"),
+                        ])
+                        .bearer_auth(token)
                 })
                 .await;
 
@@ -391,14 +398,6 @@ fn epoch_to_rfc3339(epoch: u64) -> String {
     Utc.timestamp_opt(epoch as i64, 0).unwrap().to_rfc3339()
 }
 
-fn urlencoded(s: &str) -> String {
-    s.replace('%', "%25")
-        .replace(' ', "%20")
-        .replace('@', "%40")
-        .replace('+', "%2B")
-        .replace(':', "%3A")
-}
-
 fn build_insert_request(
     matches: &ArgMatches,
     doc: &crate::discovery::RestDescription,

src/helpers/events/mod.rs

@@ -49,31 +49,6 @@ impl std::fmt::Display for SubscriptionName {
     }
 }
 
-#[derive(Debug, Clone, Builder)]
-#[builder(setter(into))]
-pub struct SubscribeConfig {
-    #[builder(default)]
-    pub target: Option<String>,
-    #[builder(default)]
-    pub event_types: Vec<String>,
-    #[builder(default)]
-    pub project: Option<ProjectId>,
-    #[builder(default)]
-    pub subscription: Option<SubscriptionName>,
-    #[builder(default = "10")]
-    pub max_messages: u32,
-    #[builder(default = "5")]
-    pub poll_interval: u64,
-    #[builder(default = "false")]
-    pub once: bool,
-    #[builder(default = "false")]
-    pub cleanup: bool,
-    #[builder(default = "false")]
-    pub no_ack: bool,
-    #[builder(default)]
-    pub output_dir: Option<String>,
-}
-
 impl Helper for EventsHelper {
     fn inject_commands(
         &self,

src/helpers/events/renew.rs

@@ -37,6 +37,7 @@ pub(super) async fn handle_renew(
 
     if let Some(name) = config.name {
         // Reactivate a specific subscription
+        let name = crate::validate::validate_resource_name(&name)?;
         eprintln!("Reactivating subscription: {name}");
         let resp = client
             .post(format!(
@@ -78,6 +79,7 @@ pub(super) async fn handle_renew(
             let to_renew = filter_subscriptions_to_renew(subs, now, within_secs);
 
             for name in to_renew {
+                let name = crate::validate::validate_resource_name(&name)?;
                 eprintln!("Renewing {name}...");
                 let _ = client
                     .post(format!(

src/helpers/events/subscribe.rs

@@ -1,4 +1,30 @@
 use super::*;
+use std::path::PathBuf;
+
+#[derive(Debug, Clone, Default, Builder)]
+#[builder(setter(into))]
+pub struct SubscribeConfig {
+    #[builder(default)]
+    target: Option<String>,
+    #[builder(default)]
+    event_types: Vec<String>,
+    #[builder(default)]
+    project: Option<ProjectId>,
+    #[builder(default)]
+    subscription: Option<SubscriptionName>,
+    #[builder(default = "10")]
+    max_messages: u32,
+    #[builder(default = "2")]
+    poll_interval: u64,
+    #[builder(default)]
+    once: bool,
+    #[builder(default)]
+    cleanup: bool,
+    #[builder(default)]
+    no_ack: bool,
+    #[builder(default)]
+    output_dir: Option<PathBuf>,
+}
 
 fn parse_subscribe_args(matches: &ArgMatches) -> Result<SubscribeConfig, GwsError> {
     let mut builder = SubscribeConfigBuilder::default();
@@ -22,6 +48,7 @@ fn parse_subscribe_args(matches: &ArgMatches) -> Result<SubscribeConfig, GwsErro
         builder.project(Some(ProjectId(project)));
     }
     if let Some(subscription) = matches.get_one::<String>("subscription") {
+        crate::validate::validate_resource_name(subscription)?;
         builder.subscription(Some(SubscriptionName(subscription.clone())));
     }
     if let Some(max_messages) = matches
@@ -40,7 +67,7 @@ fn parse_subscribe_args(matches: &ArgMatches) -> Result<SubscribeConfig, GwsErro
     builder.cleanup(matches.get_flag("cleanup"));
     builder.no_ack(matches.get_flag("no-ack"));
     if let Some(output_dir) = matches.get_one::<String>("output-dir") {
-        builder.output_dir(Some(output_dir.clone()));
+        builder.output_dir(Some(crate::validate::validate_safe_output_dir(output_dir)?));
     }
 
     let config = builder
@@ -96,7 +123,9 @@ pub(super) async fn handle_subscribe(
         } else {
             // Full setup: create Pub/Sub topic + subscription + Workspace Events subscription
             let target = config.target.clone().unwrap();
-            let project = config.project.clone().unwrap().0;
+            let project =
+                crate::validate::validate_resource_name(&config.project.clone().unwrap().0)?
+                    .to_string();
             let event_types_str: Vec<&str> =
                 config.event_types.iter().map(|s| s.as_str()).collect();
 
@@ -326,11 +355,11 @@ async fn pull_loop(
                     .duration_since(std::time::UNIX_EPOCH)
                     .map(|d| d.as_millis())
                     .unwrap_or(0);
-                let path = format!("{dir}/{ts}_{file_counter}.json");
+                let path = dir.join(format!("{ts}_{file_counter}.json"));
                 if let Err(e) = std::fs::write(&path, &json_str) {
-                    eprintln!("Warning: failed to write {path}: {e}");
+                    eprintln!("Warning: failed to write {}: {e}", path.display());
                 } else {
-                    eprintln!("Wrote {path}");
+                    eprintln!("Wrote {}", path.display());
                 }
             } else {
                 println!(
@@ -514,6 +543,15 @@ mod tests {
         cmd.try_get_matches_from(args).unwrap()
     }
 
+    #[test]
+    fn test_parse_subscribe_args_invalid_output_dir() {
+        let matches = make_matches_subscribe(&["test", "--output-dir", "../../etc"]);
+        let result = parse_subscribe_args(&matches);
+        assert!(result.is_err());
+        let msg = result.unwrap_err().to_string();
+        assert!(msg.contains("outside the current directory"));
+    }
+
     #[test]
     fn test_parse_subscribe_args() {
         let matches = make_matches_subscribe(&[

src/helpers/gmail/mod.rs

@@ -161,6 +161,7 @@ TIPS:
                         .long("msg-format")
                         .help("Gmail message format: full, metadata, minimal, raw")
                         .value_name("FORMAT")
+                        .value_parser(["full", "metadata", "minimal", "raw"])
                         .default_value("full"),
                 )
                 .arg(