fix(services): support api:version syntax for unlisted Discovery APIs#826
fix(services): support api:version syntax for unlisted Discovery APIs#826nuthalapativarun wants to merge 4 commits into
Conversation
…APIs When resolve_service fails but a version override is present (via colon syntax or --api-version flag), fall through to use the raw service arg as the API name with the provided version instead of returning an error. Fixes googleworkspace#670
🦋 Changeset detectedLatest commit: 5aebd68 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request improves the flexibility of the CLI by enabling support for unlisted Discovery APIs. By modifying the service resolution logic, the tool now correctly handles service arguments that include a version override, even if the service itself is not present in the internal known services list. This ensures that users can interact with a broader range of Google APIs without requiring explicit registration in the CLI. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request fixes the <api>:<version> syntax handling to allow unlisted Discovery APIs to be called directly. It updates parse_service_and_version to fall back to the service argument and version override if the service resolution fails, and adds a corresponding unit test. The reviewer identified an issue where the --api-version=VER syntax (using an equals sign) is ignored in parse_service_and_version, causing unlisted APIs using this syntax to fail, and provided a code suggestion to resolve this inconsistency.
…ed APIs The version-override loop only matched the space-separated form (--api-version VER). Add a strip_prefix branch to also capture the equals-sign form (--api-version=VER), which filter_args_for_subcommand already stripped but parse_service_and_version silently ignored. Add a test to cover the equals-sign path with an unlisted service.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces support for calling unlisted Discovery APIs directly by updating the argument parsing logic to handle the <api>:<version> syntax and the --api-version= flag format. When a service is not found in the pre-defined list, the parser now falls back to using the raw service argument and the provided version override. The review feedback identifies a potential path traversal and terminal escape sequence injection vulnerability resulting from allowing arbitrary unlisted service names and versions, and suggests implementing strict character validation to mitigate these security risks.
…path traversal Add alphanumeric + _-. allowlist check when falling back to a raw service name and version for unlisted Discovery APIs. Rejects inputs containing ../ traversal sequences or special URL characters before they can reach the Discovery URL or filesystem cache. Resolves Gemini r2 comment on PR googleworkspace#826.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces support for the --api-version=<version> flag syntax and enables calling unlisted Discovery APIs directly by validating the service name and version when they are not in the known services list. The review feedback identifies a critical security vulnerability where the version_override is not validated when a listed service is successfully resolved, potentially allowing path traversal attacks. A code suggestion is provided to ensure the version override is validated in all code paths.
…nt path traversal Apply the same alphanumeric+dot/dash validation to version_override in the listed-service (Ok) branch that was already applied in the unlisted-service fallback. A malicious caller could previously pass --api-version=../evil to a known service like drive and bypass the check. Also extract is_valid_api_token as a shared closure to remove duplication.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request fixes the <api>:<version> syntax to allow unlisted Discovery APIs to be called directly, supports the --api-version= flag format, and adds input validation to prevent path traversal. A security review identified a vulnerability in the is_valid_api_token helper, which currently permits tokens consisting entirely of dots (such as . or ..), and recommended a code improvement to explicitly reject them.
| let is_valid_api_token = |s: &str| { | ||
| !s.is_empty() | ||
| && s.chars() | ||
| .all(|c| c.is_ascii_alphanumeric() || c == '_' || c == '-' || c == '.') | ||
| }; |
There was a problem hiding this comment.
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 == '.')
};
Summary
When a user passes
gws admob:v1 ...,parse_service_and_versionnow falls through to use the raw API name and version rather than returning an error for unknown service aliases. The original error is still returned when no version override is present.Fixes #670
Changes
crates/google-workspace-cli/src/main.rs: whenresolve_servicefails but a version override is present, use the raw service arg as the API name#[cfg(test)]block covering theadmob:v1unlisted API caseChecklist
cargo fmt --allpassed (CI)cargo clippy -- -D warningspassed (CI)cargo testpassed (CI)