fix: harden URL and path construction across helper modules (#102)

* fix: harden URL and path construction across helper modules

Closes #87

- gmail/watch.rs: encode msg_id with encode_path_segment(), use
  .query() for format and history params instead of format!
- modelarmor.rs: validate template with validate_resource_name() in
  handle_sanitize, validate project/location/template_id in
  parse_create_template_args, encode all path segments in
  build_create_template_url
- discovery.rs: validate service/version with validate_api_identifier()
  before use in cache filenames and discovery URLs, encode path segments
- validate.rs: add validate_api_identifier() for safe API name chars
- Add tests for all new validation and encoding paths

* refactor: pass API version as a query parameter for alternative discovery URLs.

.changeset/fix-url-safety-hardening.md

@@ -0,0 +1,5 @@
+---
+fix-url-safety: patch
+---
+
+Harden URL and path construction across helper modules (gmail/watch, modelarmor, discovery)

src/discovery.rs

@@ -188,6 +188,13 @@ pub async fn fetch_discovery_document(
     service: &str,
     version: &str,
 ) -> anyhow::Result<RestDescription> {
+    // Validate service and version to prevent path traversal in cache filenames
+    // and injection in discovery URLs.
+    let service =
+        crate::validate::validate_api_identifier(service).map_err(|e| anyhow::anyhow!("{e}"))?;
+    let version =
+        crate::validate::validate_api_identifier(version).map_err(|e| anyhow::anyhow!("{e}"))?;
+
     let cache_dir = dirs::config_dir()
         .unwrap_or_else(|| std::path::PathBuf::from("."))
         .join("gws")
@@ -209,7 +216,11 @@ pub async fn fetch_discovery_document(
         }
     }
 
-    let url = format!("https://www.googleapis.com/discovery/v1/apis/{service}/{version}/rest");
+    let url = format!(
+        "https://www.googleapis.com/discovery/v1/apis/{}/{}/rest",
+        crate::validate::encode_path_segment(service),
+        crate::validate::encode_path_segment(version),
+    );
 
     let client = crate::client::build_client()?;
     let resp = client.get(&url).send().await?;
@@ -218,8 +229,12 @@ pub async fn fetch_discovery_document(
         resp.text().await?
     } else {
         // Try the $discovery/rest URL pattern used by newer APIs (Forms, Keep, Meet, etc.)
-        let alt_url = format!("https://{service}.googleapis.com/$discovery/rest?version={version}");
-        let alt_resp = client.get(&alt_url).send().await?;
+        let alt_url = format!("https://{service}.googleapis.com/$discovery/rest");
+        let alt_resp = client
+            .get(&alt_url)
+            .query(&[("version", version)])
+            .send()
+            .await?;
         if !alt_resp.status().is_success() {
             anyhow::bail!(
                 "Failed to fetch Discovery Document for {service}/{version}: HTTP {} (tried both standard and $discovery URLs)",

src/helpers/gmail/watch.rs

@@ -385,13 +385,12 @@ async fn fetch_and_output_messages(
     output_dir: Option<&std::path::PathBuf>,
     sanitize_config: &crate::helpers::modelarmor::SanitizeConfig,
 ) -> Result<(), GwsError> {
-    let url = format!(
-        "https://gmail.googleapis.com/gmail/v1/users/me/history?startHistoryId={}&historyTypes=messageAdded",
-        start_history_id
-    );
-
     let resp = client
-        .get(&url)
+        .get("https://gmail.googleapis.com/gmail/v1/users/me/history")
+        .query(&[
+            ("startHistoryId", &start_history_id.to_string()),
+            ("historyTypes", &"messageAdded".to_string()),
+        ])
         .bearer_auth(gmail_token)
         .send()
         .await
@@ -404,10 +403,15 @@ async fn fetch_and_output_messages(
     for msg_id in msg_ids {
         // Fetch full message
         let msg_url = format!(
-            "https://gmail.googleapis.com/gmail/v1/users/me/messages/{}?format={}",
-            msg_id, msg_format
+            "https://gmail.googleapis.com/gmail/v1/users/me/messages/{}",
+            crate::validate::encode_path_segment(&msg_id),
         );
-        let msg_resp = client.get(&msg_url).bearer_auth(gmail_token).send().await;
+        let msg_resp = client
+            .get(&msg_url)
+            .query(&[("format", msg_format)])
+            .bearer_auth(gmail_token)
+            .send()
+            .await;
 
         if let Ok(resp) = msg_resp {
             if let Ok(mut full_msg) = resp.json::<Value>().await {

src/helpers/modelarmor.rs

@@ -315,7 +315,8 @@ async fn handle_sanitize(
     method_name: &str,
     data_field: &str,
 ) -> Result<(), GwsError> {
-    let template = matches.get_one::<String>("template").unwrap();
+    let template_raw = matches.get_one::<String>("template").unwrap();
+    let template = crate::validate::validate_resource_name(template_raw)?;
 
     let location = extract_location(template).ok_or_else(|| {
         GwsError::Validation(
@@ -340,9 +341,12 @@ pub struct CreateTemplateConfig {
 }
 
 fn parse_create_template_args(matches: &ArgMatches) -> Result<CreateTemplateConfig, GwsError> {
-    let project = matches.get_one::<String>("project").unwrap().clone();
-    let location = matches.get_one::<String>("location").unwrap().clone();
-    let template_id = matches.get_one::<String>("template-id").unwrap().clone();
+    let project_raw = matches.get_one::<String>("project").unwrap();
+    let project = crate::validate::validate_resource_name(project_raw)?.to_string();
+    let location_raw = matches.get_one::<String>("location").unwrap();
+    let location = crate::validate::validate_resource_name(location_raw)?.to_string();
+    let template_id_raw = matches.get_one::<String>("template-id").unwrap();
+    let template_id = crate::validate::validate_resource_name(template_id_raw)?.to_string();
 
     let body = if let Some(json_str) = matches.get_one::<String>("json") {
         json_str.clone()
@@ -364,10 +368,12 @@ fn parse_create_template_args(matches: &ArgMatches) -> Result<CreateTemplateConf
 
 pub fn build_create_template_url(config: &CreateTemplateConfig) -> String {
     let base = regional_base_url(&config.location);
-    let parent = format!("projects/{}/locations/{}", config.project, config.location);
+    let project = crate::validate::encode_path_segment(&config.project);
+    let location = crate::validate::encode_path_segment(&config.location);
+    let parent = format!("projects/{project}/locations/{location}");
     format!(
         "{base}/{parent}/templates?templateId={}",
-        config.template_id
+        crate::validate::encode_path_segment(&config.template_id)
     )
 }
 
@@ -665,9 +671,10 @@ mod parsing_tests {
             body: "{}".to_string(),
         };
         let url = build_create_template_url(&config);
+        // encode_path_segment encodes hyphens ('-' → '%2D')
         assert_eq!(
             url,
-            "https://modelarmor.us-central1.rep.googleapis.com/v1/projects/p/locations/us-central1/templates?templateId=t"
+            "https://modelarmor.us-central1.rep.googleapis.com/v1/projects/p/locations/us%2Dcentral1/templates?templateId=t"
         );
     }
 
@@ -740,4 +747,34 @@ mod parsing_tests {
         assert!(subcommands.contains(&"+sanitize-response"));
         assert!(subcommands.contains(&"+create-template"));
     }
+
+    #[test]
+    fn test_build_create_template_url_encodes_segments() {
+        let config = CreateTemplateConfig {
+            project: "my-project".to_string(),
+            location: "us-central1".to_string(),
+            template_id: "my-template".to_string(),
+            body: "{}".to_string(),
+        };
+        let url = build_create_template_url(&config);
+        assert!(url.contains("projects/my%2Dproject"));
+        assert!(url.contains("locations/us%2Dcentral1"));
+        assert!(url.contains("templateId=my%2Dtemplate"));
+    }
+
+    #[test]
+    fn test_parse_create_template_args_rejects_traversal() {
+        let matches = make_matches_create(&[
+            "test",
+            "--project",
+            "../etc",
+            "--location",
+            "us-central1",
+            "--template-id",
+            "t",
+            "--preset",
+            "jailbreak",
+        ]);
+        assert!(parse_create_template_args(&matches).is_err());
+    }
 }

src/validate.rs

@@ -220,6 +220,26 @@ pub fn validate_resource_name(s: &str) -> Result<&str, GwsError> {
     Ok(s)
 }
 
+/// Validate an API identifier (service name, version string) for use in
+/// cache filenames and discovery URLs. Only alphanumeric characters, hyphens,
+/// underscores, and dots are allowed to prevent path traversal and injection.
+pub fn validate_api_identifier(s: &str) -> Result<&str, GwsError> {
+    if s.is_empty() {
+        return Err(GwsError::Validation(
+            "API identifier must not be empty".to_string(),
+        ));
+    }
+    if !s
+        .chars()
+        .all(|c| c.is_ascii_alphanumeric() || c == '-' || c == '_' || c == '.')
+    {
+        return Err(GwsError::Validation(format!(
+            "API identifier contains invalid characters (only alphanumeric, '-', '_', '.' allowed): {s}"
+        )));
+    }
+    Ok(s)
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -503,4 +523,41 @@ mod tests {
         // Just % should be rejected too
         assert!(validate_resource_name("spaces/100%").is_err());
     }
+
+    // --- validate_api_identifier ---
+
+    #[test]
+    fn test_validate_api_identifier_valid() {
+        assert_eq!(validate_api_identifier("drive").unwrap(), "drive");
+        assert_eq!(validate_api_identifier("v3").unwrap(), "v3");
+        assert_eq!(
+            validate_api_identifier("directory_v1").unwrap(),
+            "directory_v1"
+        );
+        assert_eq!(
+            validate_api_identifier("admin.reports_v1").unwrap(),
+            "admin.reports_v1"
+        );
+        assert_eq!(validate_api_identifier("v2beta1").unwrap(), "v2beta1");
+    }
+
+    #[test]
+    fn test_validate_api_identifier_rejects_path_traversal() {
+        assert!(validate_api_identifier("../etc/passwd").is_err());
+        assert!(validate_api_identifier("foo/../bar").is_err());
+    }
+
+    #[test]
+    fn test_validate_api_identifier_rejects_special_chars() {
+        assert!(validate_api_identifier("drive?key=val").is_err());
+        assert!(validate_api_identifier("drive#frag").is_err());
+        assert!(validate_api_identifier("drive%2f..").is_err());
+        assert!(validate_api_identifier("v3 ").is_err());
+        assert!(validate_api_identifier("v3\n").is_err());
+    }
+
+    #[test]
+    fn test_validate_api_identifier_empty() {
+        assert!(validate_api_identifier("").is_err());
+    }
 }