fix: harden URL encoding and input validation for AI/LLM callers (#21)
* refactor: replace manual urlencoded() with reqwest .query() builder Remove duplicate hand-rolled urlencoded() functions from workflows.rs and calendar.rs. All query parameters are now passed via reqwest's .query() API, which handles percent-encoding correctly and completely. * fix: percent-encode path parameters to prevent path traversal Use percent_encoding::utf8_percent_encode for calendar_id, cal.id, message_id, and file_id before interpolating into URL path segments. Addresses code review feedback on security regression. * fix: add shared URL safety helpers for path params Add encode_path_segment() for single-segment IDs and validate_resource_name() for multi-segment resource names. encode_path_segment: percent-encodes all non-alphanumeric chars, used for calendar IDs, file IDs, and message IDs. validate_resource_name: rejects path traversal (..) and control chars while preserving intentional / structure, used for Chat space names, task list IDs, and subscription names. Returns clear error messages for LLM callers. * test: add AI edge case tests for URL safety helpers Cover query/fragment injection, double-encoding, unicode, spaces, path traversal via encoding, control chars (CR/tab), and clear error message assertions for LLM callers. * fix: warn on stderr when API calls fail silently - Daily briefing calendar events fetch - Daily briefing tasks fetch - Daily summary calendar events fetch - Daily summary unread email count fetch Addresses PR review feedback about confusing silent failures, especially for LLM callers that cannot see visual cues. * 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 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 Closes #23 * chore: add changesets for PR #21 commits * test: add comprehensive test coverage for input validation handlers * docs: document input validation and URL safety patterns in AGENTS.md and CONTRIBUTING.md * fix: address PR review comments — reject ?/# in resource names, validate subscription arg, remove redundant validate_msg_format * fix: store validated PathBuf, remove dead code, delete duplicate SubscribeConfig Addresses review comments: - Store validated PathBuf from validate_safe_output_dir instead of discarding it (output_dir is now Option<PathBuf>) - Remove duplicate SubscribeConfig from events/mod.rs - Delete unused validate_msg_format (clap value_parser handles this) - Remove all #[allow(dead_code)] annotations * fix: per-segment traversal check in validate_resource_name, fix docs * fix: harden security validation and deduplicate logic --------- Co-authored-by: jpoehnelt-bot <jpoehnelt-bot@users.noreply.github.com>
This commit is contained in:
Justin Poehnelt
@@ -0,0 +1,5 @@
|
||||
---
|
||||
"@googleworkspace/cli": patch
|
||||
---
|
||||
|
||||
fix: percent-encode path parameters to prevent path traversal
|
||||
@@ -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`
|
||||
@@ -0,0 +1,5 @@
|
||||
---
|
||||
"@googleworkspace/cli": patch
|
||||
---
|
||||
|
||||
Security: Harden validate_resource_name and fix Gmail watch path traversal
|
||||
@@ -0,0 +1,5 @@
|
||||
---
|
||||
"@googleworkspace/cli": patch
|
||||
---
|
||||
|
||||
Replace manual `urlencoded()` with reqwest `.query()` builder for safer URL encoding
|
||||
@@ -0,0 +1,5 @@
|
||||
---
|
||||
"@googleworkspace/cli": patch
|
||||
---
|
||||
|
||||
fix: add shared URL safety helpers for path params (`encode_path_segment`, `validate_resource_name`)
|
||||
@@ -0,0 +1,5 @@
|
||||
---
|
||||
"@googleworkspace/cli": patch
|
||||
---
|
||||
|
||||
fix: warn on stderr when API calls fail silently
|
||||
@@ -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)
|
||||
|
||||
Generated
+1
@@ -860,6 +860,7 @@ dependencies = [
|
||||
"futures-util",
|
||||
"hostname",
|
||||
"keyring",
|
||||
"percent-encoding",
|
||||
"rand 0.8.5",
|
||||
"ratatui",
|
||||
"reqwest",
|
||||
|
||||
@@ -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
|
||||
|
||||
+36
-1
@@ -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`
|
||||
@@ -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();
|
||||
|
||||
|
||||
+12
-13
@@ -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,
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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!(
|
||||
|
||||
@@ -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(&[
|
||||
|
||||
@@ -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(
|
||||
|
||||
+66
-26
@@ -5,7 +5,7 @@ pub(super) async fn handle_watch(
|
||||
matches: &ArgMatches,
|
||||
sanitize_config: &crate::helpers::modelarmor::SanitizeConfig,
|
||||
) -> Result<(), GwsError> {
|
||||
let config = parse_watch_args(matches);
|
||||
let config = parse_watch_args(matches)?;
|
||||
|
||||
if let Some(ref dir) = config.output_dir {
|
||||
std::fs::create_dir_all(dir).context("Failed to create output dir")?;
|
||||
@@ -37,8 +37,9 @@ pub(super) async fn handle_watch(
|
||||
|
||||
let suffix = format!("{:08x}", rand::random::<u32>());
|
||||
let topic = if let Some(ref t) = config.topic {
|
||||
t.clone()
|
||||
crate::validate::validate_resource_name(t)?.to_string()
|
||||
} else {
|
||||
let project = crate::validate::validate_resource_name(&project)?;
|
||||
let t = format!("projects/{project}/topics/gws-gmail-watch-{suffix}");
|
||||
// Create Pub/Sub topic
|
||||
eprintln!("Creating Pub/Sub topic: {t}");
|
||||
@@ -97,6 +98,7 @@ pub(super) async fn handle_watch(
|
||||
t
|
||||
};
|
||||
|
||||
let project = crate::validate::validate_resource_name(&project)?;
|
||||
let sub = format!("projects/{project}/subscriptions/gws-gmail-watch-{suffix}");
|
||||
|
||||
// 3. Create Pub/Sub subscription
|
||||
@@ -207,14 +209,15 @@ pub(super) async fn handle_watch(
|
||||
eprintln!("\nCleaning up Pub/Sub resources...");
|
||||
let _ = client
|
||||
.delete(format!(
|
||||
"https://pubsub.googleapis.com/v1/{pubsub_subscription}"
|
||||
"https://pubsub.googleapis.com/v1/{}",
|
||||
pubsub_subscription
|
||||
))
|
||||
.bearer_auth(&pubsub_token)
|
||||
.send()
|
||||
.await;
|
||||
if let Some(ref topic) = topic_name {
|
||||
let _ = client
|
||||
.delete(format!("https://pubsub.googleapis.com/v1/{topic}"))
|
||||
.delete(format!("https://pubsub.googleapis.com/v1/{}", topic))
|
||||
.bearer_auth(&pubsub_token)
|
||||
.send()
|
||||
.await;
|
||||
@@ -227,9 +230,9 @@ pub(super) async fn handle_watch(
|
||||
pubsub_subscription
|
||||
);
|
||||
if let Some(ref topic) = topic_name {
|
||||
eprintln!("Pub/Sub topic: {topic}");
|
||||
eprintln!("Pub/Sub topic: {}", topic);
|
||||
}
|
||||
eprintln!("Pub/Sub subscription: {pubsub_subscription}");
|
||||
eprintln!("Pub/Sub subscription: {}", pubsub_subscription);
|
||||
eprintln!("Note: Gmail watch expires after 7 days. Re-run +watch to renew.");
|
||||
}
|
||||
}
|
||||
@@ -293,7 +296,7 @@ async fn watch_pull_loop(
|
||||
gmail_token,
|
||||
*last_history_id,
|
||||
&config.format,
|
||||
config.output_dir.as_deref(),
|
||||
config.output_dir.as_ref(),
|
||||
sanitize_config,
|
||||
)
|
||||
.await?;
|
||||
@@ -375,7 +378,7 @@ async fn fetch_and_output_messages(
|
||||
gmail_token: &str,
|
||||
start_history_id: u64,
|
||||
msg_format: &str,
|
||||
output_dir: Option<&str>,
|
||||
output_dir: Option<&std::path::PathBuf>,
|
||||
sanitize_config: &crate::helpers::modelarmor::SanitizeConfig,
|
||||
) -> Result<(), GwsError> {
|
||||
let url = format!(
|
||||
@@ -430,11 +433,14 @@ async fn fetch_and_output_messages(
|
||||
let json_str =
|
||||
serde_json::to_string_pretty(&full_msg).unwrap_or_else(|_| "{}".to_string());
|
||||
if let Some(dir) = output_dir {
|
||||
let path = format!("{dir}/{msg_id}.json");
|
||||
let path = dir.join(format!(
|
||||
"{}.json",
|
||||
crate::validate::encode_path_segment(&msg_id)
|
||||
));
|
||||
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!(
|
||||
@@ -499,7 +505,7 @@ fn extract_message_ids_from_history(history_body: &Value) -> Vec<String> {
|
||||
result
|
||||
}
|
||||
|
||||
#[derive(Clone)]
|
||||
#[derive(Debug, Clone)]
|
||||
struct WatchConfig {
|
||||
project: Option<String>,
|
||||
subscription: Option<String>,
|
||||
@@ -510,11 +516,22 @@ struct WatchConfig {
|
||||
format: String,
|
||||
once: bool,
|
||||
cleanup: bool,
|
||||
output_dir: Option<String>,
|
||||
output_dir: Option<std::path::PathBuf>,
|
||||
}
|
||||
|
||||
fn parse_watch_args(matches: &ArgMatches) -> WatchConfig {
|
||||
WatchConfig {
|
||||
fn parse_watch_args(matches: &ArgMatches) -> Result<WatchConfig, GwsError> {
|
||||
let format_str = matches
|
||||
.get_one::<String>("msg-format")
|
||||
.map(|s| s.as_str())
|
||||
.unwrap_or("full");
|
||||
// Note: msg-format is already constrained by clap's value_parser
|
||||
|
||||
let output_dir = matches
|
||||
.get_one::<String>("output-dir")
|
||||
.map(|dir| crate::validate::validate_safe_output_dir(dir))
|
||||
.transpose()?;
|
||||
|
||||
Ok(WatchConfig {
|
||||
project: matches.get_one::<String>("project").cloned(),
|
||||
subscription: matches.get_one::<String>("subscription").cloned(),
|
||||
topic: matches.get_one::<String>("topic").cloned(),
|
||||
@@ -527,15 +544,11 @@ fn parse_watch_args(matches: &ArgMatches) -> WatchConfig {
|
||||
.get_one::<String>("poll-interval")
|
||||
.and_then(|s| s.parse().ok())
|
||||
.unwrap_or(5),
|
||||
format: matches
|
||||
.get_one::<String>("msg-format")
|
||||
.map(|s| s.as_str())
|
||||
.unwrap_or("full")
|
||||
.to_string(),
|
||||
format: format_str.to_string(),
|
||||
once: matches.get_flag("once"),
|
||||
cleanup: matches.get_flag("cleanup"),
|
||||
output_dir: matches.get_one::<String>("output-dir").cloned(),
|
||||
}
|
||||
output_dir,
|
||||
})
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
@@ -623,7 +636,34 @@ mod tests {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_parse_watch_args() {
|
||||
fn test_parse_watch_args_invalid_format_rejected_by_clap() {
|
||||
// msg-format is constrained by clap's value_parser, so invalid values
|
||||
// are rejected at the clap level before parse_watch_args is called.
|
||||
// Verify the real command definition rejects bad formats:
|
||||
let helper = super::super::GmailHelper;
|
||||
let doc = crate::discovery::RestDescription::default();
|
||||
let cmd = helper.inject_commands(Command::new("test"), &doc);
|
||||
let watch_cmd = cmd
|
||||
.get_subcommands()
|
||||
.find(|c| c.get_name() == "+watch")
|
||||
.unwrap()
|
||||
.clone();
|
||||
let result =
|
||||
watch_cmd.try_get_matches_from(vec!["+watch", "--msg-format", "invalid-format"]);
|
||||
assert!(result.is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_parse_watch_args_invalid_output_dir() {
|
||||
let matches = make_matches_watch(&["test", "--output-dir", "../../etc"]);
|
||||
let result = parse_watch_args(&matches);
|
||||
assert!(result.is_err());
|
||||
let msg = result.unwrap_err().to_string();
|
||||
assert!(msg.contains("outside the current directory"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_parse_watch_args_full() {
|
||||
let matches = make_matches_watch(&[
|
||||
"test",
|
||||
"--project",
|
||||
@@ -634,7 +674,7 @@ mod tests {
|
||||
"20",
|
||||
"--once",
|
||||
]);
|
||||
let config = parse_watch_args(&matches);
|
||||
let config = parse_watch_args(&matches).unwrap();
|
||||
assert_eq!(config.project.unwrap(), "p1");
|
||||
assert_eq!(config.subscription.unwrap(), "s1");
|
||||
assert_eq!(config.max_messages, 20);
|
||||
@@ -651,7 +691,7 @@ mod tests {
|
||||
#[test]
|
||||
fn test_parse_watch_args_defaults() {
|
||||
let matches = make_matches_watch(&["test"]);
|
||||
let config = parse_watch_args(&matches);
|
||||
let config = parse_watch_args(&matches).unwrap();
|
||||
assert_eq!(config.project, None);
|
||||
assert_eq!(config.subscription, None);
|
||||
assert_eq!(config.max_messages, 10);
|
||||
@@ -670,7 +710,7 @@ mod tests {
|
||||
"--poll-interval",
|
||||
"invalid",
|
||||
]);
|
||||
let config = parse_watch_args(&matches);
|
||||
let config = parse_watch_args(&matches).unwrap();
|
||||
// Should fallback to defaults
|
||||
assert_eq!(config.max_messages, 10);
|
||||
assert_eq!(config.poll_interval, 5);
|
||||
|
||||
@@ -76,9 +76,10 @@ TIPS:
|
||||
.get_one::<String>("dir")
|
||||
.map(|s| s.as_str())
|
||||
.unwrap_or(".");
|
||||
let safe_dir = crate::validate::validate_safe_dir_path(dir_path)?;
|
||||
|
||||
let mut files = Vec::new();
|
||||
visit_dirs(Path::new(dir_path), &mut files)?;
|
||||
visit_dirs(&safe_dir, &mut files)?;
|
||||
|
||||
if files.is_empty() {
|
||||
return Err(GwsError::Validation(format!(
|
||||
|
||||
+158
-44
@@ -228,9 +228,15 @@ TIPS:
|
||||
// Handlers
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
async fn get_json(client: &reqwest::Client, url: &str, token: &str) -> Result<Value, GwsError> {
|
||||
async fn get_json(
|
||||
client: &reqwest::Client,
|
||||
url: &str,
|
||||
token: &str,
|
||||
query: &[(&str, &str)],
|
||||
) -> Result<Value, GwsError> {
|
||||
let resp = client
|
||||
.get(url)
|
||||
.query(query)
|
||||
.bearer_auth(token)
|
||||
.send()
|
||||
.await
|
||||
@@ -279,14 +285,24 @@ async fn handle_standup_report(matches: &ArgMatches) -> Result<(), GwsError> {
|
||||
let time_max = epoch_to_rfc3339(day_end);
|
||||
|
||||
// Fetch today's events
|
||||
let events_url = format!(
|
||||
"https://www.googleapis.com/calendar/v3/calendars/primary/events?timeMin={}&timeMax={}&singleEvents=true&orderBy=startTime&maxResults=25",
|
||||
urlencoded(&time_min),
|
||||
urlencoded(&time_max),
|
||||
);
|
||||
let events_json = get_json(&client, &events_url, &token)
|
||||
.await
|
||||
.unwrap_or(json!({}));
|
||||
let events_json = get_json(
|
||||
&client,
|
||||
"https://www.googleapis.com/calendar/v3/calendars/primary/events",
|
||||
&token,
|
||||
&[
|
||||
("timeMin", time_min.as_str()),
|
||||
("timeMax", time_max.as_str()),
|
||||
("singleEvents", "true"),
|
||||
("orderBy", "startTime"),
|
||||
("maxResults", "25"),
|
||||
],
|
||||
)
|
||||
.await
|
||||
.map_err(|e| {
|
||||
eprintln!("Warning: Failed to fetch calendar events: {e}");
|
||||
e
|
||||
})
|
||||
.unwrap_or(json!({}));
|
||||
let events = events_json
|
||||
.get("items")
|
||||
.and_then(|i| i.as_array())
|
||||
@@ -305,10 +321,18 @@ async fn handle_standup_report(matches: &ArgMatches) -> Result<(), GwsError> {
|
||||
.collect();
|
||||
|
||||
// Fetch open tasks
|
||||
let tasks_url = "https://tasks.googleapis.com/tasks/v1/lists/@default/tasks?showCompleted=false&maxResults=20";
|
||||
let tasks_json = get_json(&client, tasks_url, &token)
|
||||
.await
|
||||
.unwrap_or(json!({}));
|
||||
let tasks_json = get_json(
|
||||
&client,
|
||||
"https://tasks.googleapis.com/tasks/v1/lists/@default/tasks",
|
||||
&token,
|
||||
&[("showCompleted", "false"), ("maxResults", "20")],
|
||||
)
|
||||
.await
|
||||
.map_err(|e| {
|
||||
eprintln!("Warning: Failed to fetch tasks: {e}");
|
||||
e
|
||||
})
|
||||
.unwrap_or(json!({}));
|
||||
let tasks = tasks_json
|
||||
.get("items")
|
||||
.and_then(|i| i.as_array())
|
||||
@@ -358,11 +382,21 @@ async fn handle_meeting_prep(matches: &ArgMatches) -> Result<(), GwsError> {
|
||||
);
|
||||
|
||||
let events_url = format!(
|
||||
"https://www.googleapis.com/calendar/v3/calendars/{}/events?timeMin={}&singleEvents=true&orderBy=startTime&maxResults=1",
|
||||
urlencoded(calendar_id),
|
||||
urlencoded(&now_rfc),
|
||||
"https://www.googleapis.com/calendar/v3/calendars/{}/events",
|
||||
crate::validate::encode_path_segment(calendar_id),
|
||||
);
|
||||
let events_json = get_json(&client, &events_url, &token).await?;
|
||||
let events_json = get_json(
|
||||
&client,
|
||||
&events_url,
|
||||
&token,
|
||||
&[
|
||||
("timeMin", now_rfc.as_str()),
|
||||
("singleEvents", "true"),
|
||||
("orderBy", "startTime"),
|
||||
("maxResults", "1"),
|
||||
],
|
||||
)
|
||||
.await?;
|
||||
let items = events_json
|
||||
.get("items")
|
||||
.and_then(|i| i.as_array())
|
||||
@@ -424,10 +458,16 @@ async fn handle_email_to_task(matches: &ArgMatches) -> Result<(), GwsError> {
|
||||
|
||||
// 1. Fetch the email
|
||||
let msg_url = format!(
|
||||
"https://gmail.googleapis.com/gmail/v1/users/me/messages/{}?format=metadata&metadataHeaders=Subject",
|
||||
message_id,
|
||||
"https://gmail.googleapis.com/gmail/v1/users/me/messages/{}",
|
||||
crate::validate::encode_path_segment(message_id),
|
||||
);
|
||||
let msg_json = get_json(&client, &msg_url, &token).await?;
|
||||
let msg_json = get_json(
|
||||
&client,
|
||||
&msg_url,
|
||||
&token,
|
||||
&[("format", "metadata"), ("metadataHeaders", "Subject")],
|
||||
)
|
||||
.await?;
|
||||
|
||||
let subject = msg_json
|
||||
.get("payload")
|
||||
@@ -455,6 +495,7 @@ async fn handle_email_to_task(matches: &ArgMatches) -> Result<(), GwsError> {
|
||||
"notes": format!("From email: {}\n\n{}", message_id, snippet),
|
||||
});
|
||||
|
||||
let tasklist = crate::validate::validate_resource_name(tasklist)?;
|
||||
let task_url = format!(
|
||||
"https://tasks.googleapis.com/tasks/v1/lists/{}/tasks",
|
||||
tasklist,
|
||||
@@ -508,14 +549,24 @@ async fn handle_weekly_digest(matches: &ArgMatches) -> Result<(), GwsError> {
|
||||
let time_max = epoch_to_rfc3339(week_end);
|
||||
|
||||
// Fetch this week's events
|
||||
let events_url = format!(
|
||||
"https://www.googleapis.com/calendar/v3/calendars/primary/events?timeMin={}&timeMax={}&singleEvents=true&orderBy=startTime&maxResults=50",
|
||||
urlencoded(&time_min),
|
||||
urlencoded(&time_max),
|
||||
);
|
||||
let events_json = get_json(&client, &events_url, &token)
|
||||
.await
|
||||
.unwrap_or(json!({}));
|
||||
let events_json = get_json(
|
||||
&client,
|
||||
"https://www.googleapis.com/calendar/v3/calendars/primary/events",
|
||||
&token,
|
||||
&[
|
||||
("timeMin", time_min.as_str()),
|
||||
("timeMax", time_max.as_str()),
|
||||
("singleEvents", "true"),
|
||||
("orderBy", "startTime"),
|
||||
("maxResults", "50"),
|
||||
],
|
||||
)
|
||||
.await
|
||||
.map_err(|e| {
|
||||
eprintln!("Warning: Failed to fetch calendar events: {e}");
|
||||
e
|
||||
})
|
||||
.unwrap_or(json!({}));
|
||||
let events = events_json
|
||||
.get("items")
|
||||
.and_then(|i| i.as_array())
|
||||
@@ -533,11 +584,18 @@ async fn handle_weekly_digest(matches: &ArgMatches) -> Result<(), GwsError> {
|
||||
.collect();
|
||||
|
||||
// Fetch unread email count
|
||||
let gmail_url =
|
||||
"https://gmail.googleapis.com/gmail/v1/users/me/messages?q=is%3Aunread&maxResults=1";
|
||||
let gmail_json = get_json(&client, gmail_url, &token)
|
||||
.await
|
||||
.unwrap_or(json!({}));
|
||||
let gmail_json = get_json(
|
||||
&client,
|
||||
"https://gmail.googleapis.com/gmail/v1/users/me/messages",
|
||||
&token,
|
||||
&[("q", "is:unread"), ("maxResults", "1")],
|
||||
)
|
||||
.await
|
||||
.map_err(|e| {
|
||||
eprintln!("Warning: Failed to fetch unread email count: {e}");
|
||||
e
|
||||
})
|
||||
.unwrap_or(json!({}));
|
||||
let unread_estimate = gmail_json
|
||||
.get("resultSizeEstimate")
|
||||
.and_then(|v| v.as_u64())
|
||||
@@ -569,10 +627,16 @@ async fn handle_file_announce(matches: &ArgMatches) -> Result<(), GwsError> {
|
||||
|
||||
// 1. Fetch file metadata from Drive
|
||||
let file_url = format!(
|
||||
"https://www.googleapis.com/drive/v3/files/{}?fields=id,name,webViewLink",
|
||||
file_id,
|
||||
"https://www.googleapis.com/drive/v3/files/{}",
|
||||
crate::validate::encode_path_segment(file_id),
|
||||
);
|
||||
let file_json = get_json(&client, &file_url, &token).await?;
|
||||
let file_json = get_json(
|
||||
&client,
|
||||
&file_url,
|
||||
&token,
|
||||
&[("fields", "id,name,webViewLink")],
|
||||
)
|
||||
.await?;
|
||||
let file_name = file_json
|
||||
.get("name")
|
||||
.and_then(|v| v.as_str())
|
||||
@@ -589,6 +653,7 @@ async fn handle_file_announce(matches: &ArgMatches) -> Result<(), GwsError> {
|
||||
.unwrap_or_else(|| format!("📎 {file_name}\n{file_link}"));
|
||||
|
||||
let chat_body = json!({ "text": msg_text });
|
||||
let space = crate::validate::validate_resource_name(space)?;
|
||||
let chat_url = format!("https://chat.googleapis.com/v1/{}/messages", space);
|
||||
|
||||
let chat_resp = client
|
||||
@@ -629,14 +694,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")
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
@@ -662,4 +719,61 @@ mod tests {
|
||||
fn test_helper_only() {
|
||||
assert!(WorkflowHelper.helper_only());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_epoch_to_rfc3339() {
|
||||
assert_eq!(epoch_to_rfc3339(0), "1970-01-01T00:00:00+00:00");
|
||||
assert_eq!(epoch_to_rfc3339(1710000000), "2024-03-09T16:00:00+00:00");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_build_standup_report_cmd() {
|
||||
let cmd = build_standup_report_cmd();
|
||||
assert_eq!(cmd.get_name(), "+standup-report");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_build_meeting_prep_cmd() {
|
||||
let cmd = build_meeting_prep_cmd();
|
||||
assert_eq!(cmd.get_name(), "+meeting-prep");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_build_email_to_task_cmd() {
|
||||
let cmd = build_email_to_task_cmd();
|
||||
assert_eq!(cmd.get_name(), "+email-to-task");
|
||||
|
||||
// message-id is required
|
||||
let args = cmd
|
||||
.clone()
|
||||
.try_get_matches_from(vec!["+email-to-task", "--message-id", "123"]);
|
||||
assert!(args.is_ok());
|
||||
|
||||
let args_err = cmd.try_get_matches_from(vec!["+email-to-task"]);
|
||||
assert!(args_err.is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_build_weekly_digest_cmd() {
|
||||
let cmd = build_weekly_digest_cmd();
|
||||
assert_eq!(cmd.get_name(), "+weekly-digest");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_build_file_announce_cmd() {
|
||||
let cmd = build_file_announce_cmd();
|
||||
assert_eq!(cmd.get_name(), "+file-announce");
|
||||
|
||||
let args = cmd.clone().try_get_matches_from(vec![
|
||||
"+file-announce",
|
||||
"--file-id",
|
||||
"123",
|
||||
"--space",
|
||||
"spaces/test",
|
||||
]);
|
||||
assert!(args.is_ok());
|
||||
|
||||
let args_err = cmd.try_get_matches_from(vec!["+file-announce"]);
|
||||
assert!(args_err.is_err());
|
||||
}
|
||||
}
|
||||
|
||||
@@ -36,6 +36,7 @@ mod services;
|
||||
mod setup;
|
||||
mod setup_tui;
|
||||
mod token_storage;
|
||||
pub(crate) mod validate;
|
||||
|
||||
use error::{print_error_json, GwsError};
|
||||
|
||||
|
||||
+474
@@ -0,0 +1,474 @@
|
||||
// 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.
|
||||
|
||||
//! Shared input validation helpers.
|
||||
//!
|
||||
//! These functions harden CLI inputs against adversarial or accidentally
|
||||
//! malformed values — especially important when the CLI is invoked by an
|
||||
//! LLM agent rather than a human operator.
|
||||
|
||||
use crate::error::GwsError;
|
||||
use std::path::{Path, PathBuf};
|
||||
|
||||
/// Validates that `dir` is a safe output directory.
|
||||
///
|
||||
/// The path is resolved relative to CWD. The function rejects paths that
|
||||
/// would escape above CWD (e.g. `../../.ssh`) or contain null bytes /
|
||||
/// control characters.
|
||||
///
|
||||
/// Returns the canonicalized path on success.
|
||||
pub fn validate_safe_output_dir(dir: &str) -> Result<PathBuf, GwsError> {
|
||||
reject_control_chars(dir, "--output-dir")?;
|
||||
|
||||
let path = Path::new(dir);
|
||||
|
||||
// Reject absolute paths — force everything relative to CWD
|
||||
if path.is_absolute() {
|
||||
return Err(GwsError::Validation(format!(
|
||||
"--output-dir must be a relative path, got absolute path '{}'",
|
||||
dir
|
||||
)));
|
||||
}
|
||||
|
||||
// Canonicalize CWD and resolve the target under it
|
||||
let cwd = std::env::current_dir()
|
||||
.map_err(|e| GwsError::Validation(format!("Failed to determine current directory: {e}")))?;
|
||||
let resolved = cwd.join(path);
|
||||
|
||||
// If the directory already exists, canonicalize. Otherwise, canonicalize
|
||||
// the longest existing prefix and append the remaining segments.
|
||||
let canonical = if resolved.exists() {
|
||||
resolved.canonicalize().map_err(|e| {
|
||||
GwsError::Validation(format!("Failed to resolve --output-dir '{}': {e}", dir))
|
||||
})?
|
||||
} else {
|
||||
normalize_non_existing(&resolved)?
|
||||
};
|
||||
|
||||
let canonical_cwd = cwd.canonicalize().map_err(|e| {
|
||||
GwsError::Validation(format!("Failed to canonicalize current directory: {e}"))
|
||||
})?;
|
||||
|
||||
if !canonical.starts_with(&canonical_cwd) {
|
||||
return Err(GwsError::Validation(format!(
|
||||
"--output-dir '{}' resolves to '{}' which is outside the current directory",
|
||||
dir,
|
||||
canonical.display()
|
||||
)));
|
||||
}
|
||||
|
||||
Ok(canonical)
|
||||
}
|
||||
|
||||
/// Validates that `dir` is a safe directory for reading files (e.g. `--dir`
|
||||
/// in `script +push`).
|
||||
///
|
||||
/// Similar to [`validate_safe_output_dir`] but also follows symlinks
|
||||
/// safely and ensures the resolved path stays under CWD.
|
||||
pub fn validate_safe_dir_path(dir: &str) -> Result<PathBuf, GwsError> {
|
||||
reject_control_chars(dir, "--dir")?;
|
||||
|
||||
let path = Path::new(dir);
|
||||
|
||||
// "." is always safe (CWD itself)
|
||||
if dir == "." {
|
||||
return std::env::current_dir().map_err(|e| {
|
||||
GwsError::Validation(format!("Failed to determine current directory: {e}"))
|
||||
});
|
||||
}
|
||||
|
||||
if path.is_absolute() {
|
||||
return Err(GwsError::Validation(format!(
|
||||
"--dir must be a relative path, got absolute path '{}'",
|
||||
dir
|
||||
)));
|
||||
}
|
||||
|
||||
let cwd = std::env::current_dir()
|
||||
.map_err(|e| GwsError::Validation(format!("Failed to determine current directory: {e}")))?;
|
||||
let resolved = cwd.join(path);
|
||||
|
||||
let canonical = resolved
|
||||
.canonicalize()
|
||||
.map_err(|e| GwsError::Validation(format!("Failed to resolve --dir '{}': {e}", dir)))?;
|
||||
|
||||
let canonical_cwd = cwd.canonicalize().map_err(|e| {
|
||||
GwsError::Validation(format!("Failed to canonicalize current directory: {e}"))
|
||||
})?;
|
||||
|
||||
if !canonical.starts_with(&canonical_cwd) {
|
||||
return Err(GwsError::Validation(format!(
|
||||
"--dir '{}' resolves to '{}' which is outside the current directory",
|
||||
dir,
|
||||
canonical.display()
|
||||
)));
|
||||
}
|
||||
|
||||
Ok(canonical)
|
||||
}
|
||||
|
||||
/// Rejects strings containing null bytes or ASCII control characters.
|
||||
fn reject_control_chars(value: &str, flag_name: &str) -> Result<(), GwsError> {
|
||||
if value.bytes().any(|b| b < 0x20) {
|
||||
return Err(GwsError::Validation(format!(
|
||||
"{flag_name} contains invalid control characters"
|
||||
)));
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Resolves a path that may not exist yet by canonicalizing the existing
|
||||
/// prefix and appending remaining components.
|
||||
fn normalize_non_existing(path: &Path) -> Result<PathBuf, GwsError> {
|
||||
let mut resolved = PathBuf::new();
|
||||
let mut remaining = Vec::new();
|
||||
|
||||
// Walk backwards until we find a component that exists
|
||||
let mut current = path.to_path_buf();
|
||||
loop {
|
||||
if current.exists() {
|
||||
resolved = current
|
||||
.canonicalize()
|
||||
.map_err(|e| GwsError::Validation(format!("Failed to canonicalize path: {e}")))?;
|
||||
break;
|
||||
}
|
||||
if let Some(name) = current.file_name() {
|
||||
remaining.push(name.to_os_string());
|
||||
} else {
|
||||
// We've exhausted the path without finding an existing prefix
|
||||
return Err(GwsError::Validation(format!(
|
||||
"Cannot resolve path '{}'",
|
||||
path.display()
|
||||
)));
|
||||
}
|
||||
current = match current.parent() {
|
||||
Some(p) => p.to_path_buf(),
|
||||
None => break,
|
||||
};
|
||||
}
|
||||
|
||||
// Append remaining segments (in reverse since we collected them backwards)
|
||||
for seg in remaining.into_iter().rev() {
|
||||
resolved.push(seg);
|
||||
}
|
||||
|
||||
Ok(resolved)
|
||||
}
|
||||
|
||||
/// Percent-encode a value for use as a single URL path segment (e.g., file ID,
|
||||
/// calendar ID, message ID). All non-alphanumeric characters are encoded.
|
||||
pub fn encode_path_segment(s: &str) -> String {
|
||||
use percent_encoding::{utf8_percent_encode, NON_ALPHANUMERIC};
|
||||
utf8_percent_encode(s, NON_ALPHANUMERIC).to_string()
|
||||
}
|
||||
|
||||
/// Validate a multi-segment resource name (e.g., `spaces/ABC`, `subscriptions/123`).
|
||||
/// Rejects path traversal, control characters, and URL-special characters including `%`
|
||||
/// to prevent URL-encoded bypasses. Returns the validated name or an error.
|
||||
pub fn validate_resource_name(s: &str) -> Result<&str, GwsError> {
|
||||
if s.is_empty() {
|
||||
return Err(GwsError::Validation(
|
||||
"Resource name must not be empty".to_string(),
|
||||
));
|
||||
}
|
||||
if s.split('/').any(|seg| seg == "..") {
|
||||
return Err(GwsError::Validation(format!(
|
||||
"Resource name must not contain path traversal ('..') segments: {s}"
|
||||
)));
|
||||
}
|
||||
if s.contains('\0') || s.chars().any(|c| c.is_control()) {
|
||||
return Err(GwsError::Validation(format!(
|
||||
"Resource name contains invalid characters: {s}"
|
||||
)));
|
||||
}
|
||||
// Reject URL-special characters that could inject query params or fragments
|
||||
if s.contains('?') || s.contains('#') {
|
||||
return Err(GwsError::Validation(format!(
|
||||
"Resource name must not contain '?' or '#': {s}"
|
||||
)));
|
||||
}
|
||||
// Reject '%' to prevent URL-encoded bypasses (e.g. %2e%2e for ..)
|
||||
if s.contains('%') {
|
||||
return Err(GwsError::Validation(format!(
|
||||
"Resource name must not contain '%' (URL encoding bypass attempt): {s}"
|
||||
)));
|
||||
}
|
||||
Ok(s)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use serial_test::serial;
|
||||
use std::fs;
|
||||
use tempfile::tempdir;
|
||||
|
||||
// --- validate_safe_output_dir ---
|
||||
|
||||
#[test]
|
||||
#[serial]
|
||||
fn test_output_dir_relative_subdir() {
|
||||
// Create a real temp dir and change into it for the test
|
||||
let dir = tempdir().unwrap();
|
||||
// Canonicalize to handle macOS /var -> /private/var symlink
|
||||
let canonical_dir = dir.path().canonicalize().unwrap();
|
||||
let sub = canonical_dir.join("output");
|
||||
fs::create_dir_all(&sub).unwrap();
|
||||
|
||||
let saved_cwd = std::env::current_dir().unwrap();
|
||||
std::env::set_current_dir(&canonical_dir).unwrap();
|
||||
|
||||
let result = validate_safe_output_dir("output");
|
||||
std::env::set_current_dir(&saved_cwd).unwrap();
|
||||
|
||||
assert!(result.is_ok(), "expected Ok, got: {result:?}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
#[serial]
|
||||
fn test_output_dir_rejects_symlink_traversal() {
|
||||
let dir = tempdir().unwrap();
|
||||
let canonical_dir = dir.path().canonicalize().unwrap();
|
||||
|
||||
// Create a directory inside the tempdir
|
||||
let allowed_dir = canonical_dir.join("allowed");
|
||||
fs::create_dir(&allowed_dir).unwrap();
|
||||
|
||||
// Create a symlink pointing OUTSIDE the tempdir (e.g. to /tmp)
|
||||
let symlink_path = canonical_dir.join("sneaky_link");
|
||||
#[cfg(unix)]
|
||||
std::os::unix::fs::symlink("/tmp", &symlink_path).unwrap();
|
||||
#[cfg(windows)]
|
||||
return; // Skip on Windows due to privilege requirements for symlinks
|
||||
|
||||
let saved_cwd = std::env::current_dir().unwrap();
|
||||
std::env::set_current_dir(&canonical_dir).unwrap();
|
||||
|
||||
// Try to validate the symlink resolving outside CWD
|
||||
let result = validate_safe_output_dir("sneaky_link");
|
||||
std::env::set_current_dir(&saved_cwd).unwrap();
|
||||
|
||||
assert!(result.is_err());
|
||||
let msg = result.unwrap_err().to_string();
|
||||
assert!(msg.contains("outside the current directory"), "got: {msg}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
#[serial]
|
||||
fn test_output_dir_rejects_traversal() {
|
||||
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 result = validate_safe_output_dir("../../.ssh");
|
||||
std::env::set_current_dir(&saved_cwd).unwrap();
|
||||
|
||||
assert!(result.is_err());
|
||||
let msg = result.unwrap_err().to_string();
|
||||
assert!(msg.contains("outside the current directory"), "got: {msg}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_output_dir_rejects_absolute() {
|
||||
assert!(validate_safe_output_dir("/tmp/evil").is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_output_dir_rejects_null_bytes() {
|
||||
assert!(validate_safe_output_dir("foo\0bar").is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_output_dir_rejects_control_chars() {
|
||||
assert!(validate_safe_output_dir("foo\x01bar").is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
#[serial]
|
||||
fn test_output_dir_non_existing_subdir() {
|
||||
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 result = validate_safe_output_dir("new/nested/dir");
|
||||
std::env::set_current_dir(&saved_cwd).unwrap();
|
||||
|
||||
assert!(
|
||||
result.is_ok(),
|
||||
"expected Ok for non-existing subdir, got: {result:?}"
|
||||
);
|
||||
}
|
||||
|
||||
// --- validate_safe_dir_path ---
|
||||
|
||||
#[test]
|
||||
fn test_dir_path_cwd() {
|
||||
assert!(validate_safe_dir_path(".").is_ok());
|
||||
}
|
||||
|
||||
#[test]
|
||||
#[serial]
|
||||
fn test_dir_path_rejects_traversal() {
|
||||
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 result = validate_safe_dir_path("../../etc");
|
||||
std::env::set_current_dir(&saved_cwd).unwrap();
|
||||
|
||||
assert!(result.is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_dir_path_rejects_absolute() {
|
||||
assert!(validate_safe_dir_path("/usr/local").is_err());
|
||||
}
|
||||
|
||||
// --- reject_control_chars ---
|
||||
|
||||
#[test]
|
||||
fn test_reject_control_chars_clean() {
|
||||
assert!(reject_control_chars("hello/world", "test").is_ok());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_reject_control_chars_tab() {
|
||||
assert!(reject_control_chars("hello\tworld", "test").is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_reject_control_chars_newline() {
|
||||
assert!(reject_control_chars("hello\nworld", "test").is_err());
|
||||
}
|
||||
|
||||
// -- encode_path_segment --------------------------------------------------
|
||||
|
||||
#[test]
|
||||
fn test_encode_path_segment_plain_id() {
|
||||
assert_eq!(encode_path_segment("abc123"), "abc123");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_encode_path_segment_email() {
|
||||
// Calendar IDs are often email addresses
|
||||
let encoded = encode_path_segment("user@gmail.com");
|
||||
assert!(!encoded.contains('@'));
|
||||
assert!(!encoded.contains('.'));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_encode_path_segment_query_injection() {
|
||||
// LLM might include query params in an ID by mistake
|
||||
let encoded = encode_path_segment("fileid?fields=name");
|
||||
assert!(!encoded.contains('?'));
|
||||
assert!(!encoded.contains('='));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_encode_path_segment_fragment_injection() {
|
||||
let encoded = encode_path_segment("fileid#section");
|
||||
assert!(!encoded.contains('#'));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_encode_path_segment_path_traversal() {
|
||||
// Encoding makes traversal segments harmless
|
||||
let encoded = encode_path_segment("../../etc/passwd");
|
||||
assert!(!encoded.contains('/'));
|
||||
assert!(!encoded.contains(".."));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_encode_path_segment_unicode() {
|
||||
// LLM might pass unicode characters
|
||||
let encoded = encode_path_segment("日本語ID");
|
||||
assert!(!encoded.contains('日'));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_encode_path_segment_spaces() {
|
||||
let encoded = encode_path_segment("my file id");
|
||||
assert!(!encoded.contains(' '));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_encode_path_segment_already_encoded() {
|
||||
// LLM might double-encode by passing pre-encoded values
|
||||
let encoded = encode_path_segment("user%40gmail.com");
|
||||
// The % itself gets encoded to %25, so %40 becomes %2540
|
||||
// This prevents double-encoding issues at the HTTP layer
|
||||
assert!(encoded.contains("%2540"));
|
||||
}
|
||||
|
||||
// -- validate_resource_name -----------------------------------------------
|
||||
|
||||
#[test]
|
||||
fn test_validate_resource_name_valid() {
|
||||
assert!(validate_resource_name("spaces/ABC123").is_ok());
|
||||
assert!(validate_resource_name("subscriptions/my-sub").is_ok());
|
||||
assert!(validate_resource_name("@default").is_ok());
|
||||
assert!(validate_resource_name("projects/p1/topics/t1").is_ok());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_validate_resource_name_traversal() {
|
||||
assert!(validate_resource_name("../../etc/passwd").is_err());
|
||||
assert!(validate_resource_name("spaces/../other").is_err());
|
||||
assert!(validate_resource_name("..").is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_validate_resource_name_control_chars() {
|
||||
assert!(validate_resource_name("spaces/\0bad").is_err());
|
||||
assert!(validate_resource_name("spaces/\nbad").is_err());
|
||||
assert!(validate_resource_name("spaces/\rbad").is_err());
|
||||
assert!(validate_resource_name("spaces/\tbad").is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_validate_resource_name_empty() {
|
||||
assert!(validate_resource_name("").is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_validate_resource_name_query_injection() {
|
||||
// LLMs might append query strings or fragments to resource names
|
||||
assert!(validate_resource_name("spaces/ABC?key=val").is_err());
|
||||
assert!(validate_resource_name("spaces/ABC#fragment").is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_validate_resource_name_error_messages_are_clear() {
|
||||
let err = validate_resource_name("").unwrap_err();
|
||||
assert!(err.to_string().contains("must not be empty"));
|
||||
|
||||
let err = validate_resource_name("../bad").unwrap_err();
|
||||
assert!(err.to_string().contains("path traversal"));
|
||||
|
||||
let err = validate_resource_name("bad\0id").unwrap_err();
|
||||
assert!(err.to_string().contains("invalid characters"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_validate_resource_name_percent_bypass() {
|
||||
// %2e%2e is ..
|
||||
assert!(validate_resource_name("%2e%2e").is_err());
|
||||
assert!(validate_resource_name("spaces/%2e%2e/etc").is_err());
|
||||
// Just % should be rejected too
|
||||
assert!(validate_resource_name("spaces/100%").is_err());
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user