diff --git a/.changeset/validate-resource-ids-in-helpers.md b/.changeset/validate-resource-ids-in-helpers.md new file mode 100644 index 00000000..72dabaee --- /dev/null +++ b/.changeset/validate-resource-ids-in-helpers.md @@ -0,0 +1,11 @@ +--- +"@googleworkspace/cli": patch +--- + +Validate resource IDs in docs, sheets, calendar, and drive helpers + +`document_id` (docs `+write`), `spreadsheet_id` (sheets `+append` and `+read`), +`calendar_id` (calendar `+insert`), and `parent_id` (drive `+upload`) are now +validated with `validate_resource_name()` before use. This rejects path traversal +segments (`../`), control characters, and URL-special characters (`?`, `#`, `%`) +that could be injected by adversarial AI-agent inputs. diff --git a/crates/google-workspace-cli/src/helpers/calendar.rs b/crates/google-workspace-cli/src/helpers/calendar.rs index cf28b249..2df89bb1 100644 --- a/crates/google-workspace-cli/src/helpers/calendar.rs +++ b/crates/google-workspace-cli/src/helpers/calendar.rs @@ -423,7 +423,8 @@ fn build_insert_request( matches: &ArgMatches, doc: &crate::discovery::RestDescription, ) -> Result<(String, String, Vec), GwsError> { - let calendar_id = matches.get_one::("calendar").unwrap(); + let calendar_id = + crate::validate::validate_resource_name(matches.get_one::("calendar").unwrap())?; let summary = matches.get_one::("summary").unwrap(); let start = matches.get_one::("start").unwrap(); let end = matches.get_one::("end").unwrap(); @@ -576,6 +577,26 @@ mod tests { assert_eq!(scopes[0], "https://scope"); } + #[test] + fn test_build_insert_request_rejects_traversal_calendar_id() { + let doc = make_mock_doc(); + let matches = make_matches_insert(&[ + "test", + "--calendar", + "../../.ssh/id_rsa", + "--summary", + "X", + "--start", + "2024-01-01T10:00:00Z", + "--end", + "2024-01-01T11:00:00Z", + ]); + assert!( + build_insert_request(&matches, &doc).is_err(), + "path traversal in --calendar must be rejected" + ); + } + #[test] fn test_build_insert_request_with_meet() { let doc = make_mock_doc(); diff --git a/crates/google-workspace-cli/src/helpers/docs.rs b/crates/google-workspace-cli/src/helpers/docs.rs index d3ef7fa2..0cd654d0 100644 --- a/crates/google-workspace-cli/src/helpers/docs.rs +++ b/crates/google-workspace-cli/src/helpers/docs.rs @@ -120,7 +120,8 @@ fn build_write_request( matches: &ArgMatches, doc: &crate::discovery::RestDescription, ) -> Result<(String, String, Vec), GwsError> { - let document_id = matches.get_one::("document").unwrap(); + let document_id = + crate::validate::validate_resource_name(matches.get_one::("document").unwrap())?; let text = matches.get_one::("text").unwrap(); let documents_res = doc @@ -203,4 +204,20 @@ mod tests { assert!(body.contains("endOfSegmentLocation")); assert_eq!(scopes[0], "https://scope"); } + + #[test] + fn test_build_write_request_rejects_traversal_document_id() { + let doc = make_mock_doc(); + let matches = make_matches_write(&["test", "--document", "../../.ssh/id_rsa", "--text", "x"]); + let result = build_write_request(&matches, &doc); + assert!(result.is_err(), "path traversal in --document must be rejected"); + } + + #[test] + fn test_build_write_request_rejects_query_injection_document_id() { + let doc = make_mock_doc(); + let matches = make_matches_write(&["test", "--document", "abc?evil=1", "--text", "x"]); + let result = build_write_request(&matches, &doc); + assert!(result.is_err(), "'?' in --document must be rejected"); + } } diff --git a/crates/google-workspace-cli/src/helpers/drive.rs b/crates/google-workspace-cli/src/helpers/drive.rs index 68662ec6..e35d6874 100644 --- a/crates/google-workspace-cli/src/helpers/drive.rs +++ b/crates/google-workspace-cli/src/helpers/drive.rs @@ -91,7 +91,7 @@ TIPS: })?; // Build metadata - let metadata = build_metadata(&filename, parent_id.map(|s| s.as_str())); + let metadata = build_metadata(&filename, parent_id.map(|s| s.as_str()))?; let body_str = metadata.to_string(); @@ -142,16 +142,17 @@ fn determine_filename(file_path: &str, name_arg: Option<&str>) -> Result) -> Value { +fn build_metadata(filename: &str, parent_id: Option<&str>) -> Result { let mut metadata = json!({ "name": filename }); if let Some(parent) = parent_id { + let parent = crate::validate::validate_resource_name(parent)?; metadata["parents"] = json!([parent]); } - metadata + Ok(metadata) } #[cfg(test)] @@ -182,15 +183,31 @@ mod tests { #[test] fn test_build_metadata_no_parent() { - let meta = build_metadata("file.txt", None); + let meta = build_metadata("file.txt", None).unwrap(); assert_eq!(meta["name"], "file.txt"); assert!(meta.get("parents").is_none()); } #[test] fn test_build_metadata_with_parent() { - let meta = build_metadata("file.txt", Some("folder123")); + let meta = build_metadata("file.txt", Some("folder123")).unwrap(); assert_eq!(meta["name"], "file.txt"); assert_eq!(meta["parents"][0], "folder123"); } + + #[test] + fn test_build_metadata_rejects_traversal_parent_id() { + assert!( + build_metadata("file.txt", Some("../../.ssh/id_rsa")).is_err(), + "path traversal in --parent must be rejected" + ); + } + + #[test] + fn test_build_metadata_rejects_query_injection_parent_id() { + assert!( + build_metadata("file.txt", Some("folder?evil=1")).is_err(), + "'?' in --parent must be rejected" + ); + } } diff --git a/crates/google-workspace-cli/src/helpers/sheets.rs b/crates/google-workspace-cli/src/helpers/sheets.rs index 4357edec..6893a66c 100644 --- a/crates/google-workspace-cli/src/helpers/sheets.rs +++ b/crates/google-workspace-cli/src/helpers/sheets.rs @@ -209,6 +209,8 @@ fn build_append_request( config: &AppendConfig, doc: &crate::discovery::RestDescription, ) -> Result<(String, String, Vec), GwsError> { + let spreadsheet_id = crate::validate::validate_resource_name(&config.spreadsheet_id)?; + let spreadsheets_res = doc .resources .get("spreadsheets") @@ -221,7 +223,7 @@ fn build_append_request( })?; let params = json!({ - "spreadsheetId": config.spreadsheet_id, + "spreadsheetId": spreadsheet_id, "range": config.range, "valueInputOption": "USER_ENTERED" }); @@ -240,6 +242,8 @@ fn build_read_request( config: &ReadConfig, doc: &crate::discovery::RestDescription, ) -> Result<(String, Vec), GwsError> { + let spreadsheet_id = crate::validate::validate_resource_name(&config.spreadsheet_id)?; + // ... resource lookup omitted for brevity ... let spreadsheets_res = doc .resources @@ -253,7 +257,7 @@ fn build_read_request( })?; let params = json!({ - "spreadsheetId": config.spreadsheet_id, + "spreadsheetId": spreadsheet_id, "range": config.range }); @@ -309,6 +313,7 @@ pub fn parse_append_args(matches: &ArgMatches) -> AppendConfig { } } + /// Configuration for reading values from a spreadsheet. pub struct ReadConfig { pub spreadsheet_id: String, @@ -522,4 +527,31 @@ mod tests { assert!(subcommands.contains(&"+append")); assert!(subcommands.contains(&"+read")); } + + #[test] + fn test_build_append_request_rejects_traversal() { + let doc = make_mock_doc(); + let config = AppendConfig { + spreadsheet_id: "../../.ssh/id_rsa".to_string(), + range: "A1".to_string(), + values: vec![vec!["x".to_string()]], + }; + assert!( + build_append_request(&config, &doc).is_err(), + "path traversal in spreadsheet ID must be rejected" + ); + } + + #[test] + fn test_build_read_request_rejects_query_injection() { + let doc = make_mock_doc(); + let config = ReadConfig { + spreadsheet_id: "abc?evil=1".to_string(), + range: "A1:B2".to_string(), + }; + assert!( + build_read_request(&config, &doc).is_err(), + "'?' in spreadsheet ID must be rejected" + ); + } }