Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/fix-api-version-syntax.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@googleworkspace/cli": patch
---

Fix `<api>:<version>` syntax so unlisted Discovery APIs can be called directly
106 changes: 103 additions & 3 deletions crates/google-workspace-cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -325,10 +325,12 @@ pub fn parse_service_and_version(
let mut service_arg = first_arg;
let mut version_override: Option<String> = None;

// Check for --api-version flag anywhere in args
// Check for --api-version flag anywhere in args (space-separated or = form)
for i in 0..args.len() {
if args[i] == "--api-version" && i + 1 < args.len() {
version_override = Some(args[i + 1].clone());
} else if let Some(ver) = args[i].strip_prefix("--api-version=") {
version_override = Some(ver.to_string());
}
}

Expand All @@ -340,8 +342,36 @@ pub fn parse_service_and_version(
}
}

let (api_name, default_version) = services::resolve_service(service_arg)?;
let version = version_override.unwrap_or(default_version);
let is_valid_api_token = |s: &str| {
!s.is_empty()
&& s.chars()
.all(|c| c.is_ascii_alphanumeric() || c == '_' || c == '-' || c == '.')
};
Comment on lines +345 to +349
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-high high

The is_valid_api_token helper allows tokens consisting entirely of dots (such as . or ..) because . is in the allowed character set. This can lead to path traversal vulnerabilities or unexpected behavior if a malicious service name or version override like .. is supplied.

To prevent this, explicitly reject tokens that start with a dot or contain consecutive dots (..).

    let is_valid_api_token = |s: &str| {
        !s.is_empty()
            && !s.starts_with('.')
            && !s.contains("..")
            && s.chars()
                .all(|c| c.is_ascii_alphanumeric() || c == '_' || c == '-' || c == '.')
    };


let (api_name, version) = match services::resolve_service(service_arg) {
Comment thread
nuthalapativarun marked this conversation as resolved.
Ok((name, default_ver)) => {
let ver = version_override.unwrap_or(default_ver);
if !is_valid_api_token(&ver) {
return Err(GwsError::Validation(
"Invalid version override: only alphanumeric characters, underscores, hyphens, and dots are allowed.".to_string()
));
}
(name, ver)
}
Err(e) => {
if let Some(ver) = version_override {
if is_valid_api_token(service_arg) && is_valid_api_token(&ver) {
(service_arg.to_string(), ver)
} else {
return Err(GwsError::Validation(
"Invalid service name or version: only alphanumeric characters, underscores, hyphens, and dots are allowed.".to_string()
));
}
} else {
return Err(e);
}
}
};
Comment thread
nuthalapativarun marked this conversation as resolved.
Comment thread
nuthalapativarun marked this conversation as resolved.
Ok((api_name, version))
}

Expand Down Expand Up @@ -735,4 +765,74 @@ mod tests {
let scopes: Vec<String> = vec![];
assert_eq!(select_scope(&scopes), None);
}

#[test]
fn test_parse_service_and_version_unlisted_with_colon() {
// admob is not in the known services list, but admob:v1 should work
let args = vec![
"gws".to_string(),
"admob:v1".to_string(),
"accounts".to_string(),
"list".to_string(),
];
let result = parse_service_and_version(&args, "admob:v1");
assert_eq!(result.unwrap(), ("admob".to_string(), "v1".to_string()));
}

#[test]
fn test_parse_service_and_version_unlisted_with_equals_flag() {
// admob is not in the known services list; --api-version=v1 (equals form) should work
let args = vec![
"gws".to_string(),
"admob".to_string(),
"--api-version=v1".to_string(),
"accounts".to_string(),
"list".to_string(),
];
let result = parse_service_and_version(&args, "admob");
assert_eq!(result.unwrap(), ("admob".to_string(), "v1".to_string()));
}

#[test]
fn test_parse_service_and_version_rejects_path_traversal_in_service() {
let args = vec![
"gws".to_string(),
"../evil:v1".to_string(),
];
let result = parse_service_and_version(&args, "../evil:v1");
assert!(result.is_err(), "path traversal in service name must be rejected");
}

#[test]
fn test_parse_service_and_version_rejects_path_traversal_in_version() {
let args = vec![
"gws".to_string(),
"admob".to_string(),
"--api-version=../evil".to_string(),
];
let result = parse_service_and_version(&args, "admob");
assert!(result.is_err(), "path traversal in version must be rejected");
}

#[test]
fn test_parse_service_and_version_rejects_special_chars_in_service() {
let args = vec!["gws".to_string(), "svc?foo:v1".to_string()];
let result = parse_service_and_version(&args, "svc?foo:v1");
assert!(result.is_err(), "special chars in service name must be rejected");
}

#[test]
fn test_parse_service_and_version_rejects_malicious_version_override_for_listed_service() {
// drive IS a known listed service; a malicious version override must still be rejected
let args = vec![
"gws".to_string(),
"drive".to_string(),
"--api-version=../evil".to_string(),
];
let result = parse_service_and_version(&args, "drive");
assert!(
result.is_err(),
"path traversal in version_override must be rejected even for listed services"
);
}
}
Loading