fix: harden URL and path construction across helper modules by NeekChaw · Pull Request #133 · googleworkspace/cli

NeekChaw · 2026-03-05T07:15:16Z

Closes #87

Summary

Audit of helper modules found several places where user-provided values are interpolated into URLs or file paths via format! without going through encode_path_segment() or validate_resource_name().

Changes

`src/discovery.rs`

Add validate_discovery_id() — only allows [a-zA-Z0-9._-] for service names/versions. Prevents path traversal in the local cache file path and injection in Discovery Document endpoint URLs.
Validate service and version at entry to fetch_discovery_document().
Move version query parameter in the alt-URL code path to reqwest .query() instead of string interpolation.

`src/helpers/modelarmor.rs`

Call validate_resource_name() on --template in handle_sanitize() and build_sanitize_request_data() before building URLs.
Validate --project, --location, --template-id in parse_create_template_args().
Use encode_path_segment() for templateId in build_create_template_url().

`src/helpers/gmail/watch.rs`

Extract message-URL building to build_message_url() using encode_path_segment() on the message ID.
Switch msg_format to reqwest .query() builder instead of string interpolation.

Testing

Added 15 new unit tests (happy-path + error-path) covering each fix:

6 tests for validate_discovery_id in discovery.rs
5 tests for resource name/template validation in modelarmor.rs
4 tests for build_message_url in watch.rs

changeset-bot · 2026-03-05T07:15:20Z

🦋 Changeset detected

Latest commit: 4ecdc98

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@googleworkspace/cli	Patch

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

gemini-code-assist · 2026-03-05T07:15:20Z

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

google-cla · 2026-03-05T07:15:21Z

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

NeekChaw · 2026-03-05T07:22:25Z

@googlebot I signed it!

jpoehnelt · 2026-03-06T00:37:03Z

please fix the conflicts

jpoehnelt · 2026-03-06T00:37:19Z

/gemini review

gemini-code-assist

Code Review

This pull request does a great job of hardening URL and path construction across several modules by introducing validation and proper encoding for user-provided inputs. The changes significantly improve the security posture of the application against path traversal and injection attacks. I've found one area in the new validation logic in discovery.rs where the check can be made more robust to cover additional edge cases for invalid identifiers. My specific feedback is in the comment below.

gemini-code-assist · 2026-03-06T00:38:45Z

src/discovery.rs

+    // Reject dot-only segments that could act as a traversal (e.g. "..")
+    if s.split('.').any(|seg| seg.is_empty()) && s.starts_with('.') {
+        anyhow::bail!("{label} '{s}' must not start with '.'");
+    }


The current validation for dot-separated identifiers is not comprehensive enough. It correctly rejects identifiers starting with . (like .foo, ., and ..), but it fails to reject other invalid sequences such as foo..bar or foo.. When interpolated into a hostname like {service}.googleapis.com, these would result in invalid domain names (foo..bar.googleapis.com or foo..googleapis.com).

A more robust approach is to reject any empty segment that results from splitting the string by ..

// Reject any empty dot-separated segments, which are invalid in hostnames // and can be used for path traversal (e.g. '..', '.foo', 'foo.'). if s.split('.').any(str::is_empty) { anyhow::bail!( "{label} '{s}' contains invalid dot-sequences (e.g. '..', leading/trailing '.', or empty segments)" ); }

googleworkspace-bot · 2026-03-06T01:04:31Z

/gemini review

gemini-code-assist

Code Review

This pull request introduces important security hardening by validating user-provided values used in URL and file path construction, which helps prevent path traversal and injection attacks. The changes are well-implemented across the discovery, gmail, and modelarmor modules. I've identified a couple of instances of redundant validation in src/helpers/modelarmor.rs that should be removed to improve code clarity and maintainability.

gemini-code-assist · 2026-03-06T01:06:19Z

src/helpers/modelarmor.rs

+    // Validate template resource name before using it in URLs (defense-in-depth;
+    // prevents injection even when the CLI is invoked by an untrusted agent).
+    crate::validate::validate_resource_name(template)?;


This validation is redundant. The template variable is assigned the result of crate::validate::validate_resource_name(template_raw)? on line 319, so it has already been validated. This second validation call is unnecessary and can be removed.

gemini-code-assist · 2026-03-06T01:06:19Z

src/helpers/modelarmor.rs

+    // Validate inputs before embedding them in URLs.
+    crate::validate::validate_resource_name(&project)?;
+    crate::validate::validate_resource_name(&location)?;
+    crate::validate::validate_resource_name(&template_id)?;


This block of validations is redundant. The project, location, and template_id variables are created from the results of validate_resource_name on lines 349, 351, and 353 respectively, so they have already been validated. This block can be removed.

…oogleworkspace#87) Audit of helper modules found several places where user-provided values are interpolated into URLs or file paths via format! without going through encode_path_segment() or validate_resource_name(). Changes: src/discovery.rs - Add validate_discovery_id() that allows only [a-zA-Z0-9._-] for service names and version strings, preventing path traversal in the cache directory and injection in Discovery Document endpoint URLs. - Validate service and version at the entry point of fetch_discovery_document. - Move the version query parameter in the alt-URL code path to reqwest .query(). src/helpers/modelarmor.rs - Call validate_resource_name() on --template before embedding it in URLs in handle_sanitize() and build_sanitize_request_data(). - Validate --project, --location, --template-id in parse_create_template_args(). - Use encode_path_segment() for templateId in build_create_template_url(). src/helpers/gmail/watch.rs - Extract message-URL building to build_message_url() using encode_path_segment on the message ID (prevents path/query injection from API-supplied IDs). - Switch msg_format to reqwest .query() builder instead of string interpolation. Adds 15 new unit tests (happy-path + error-path) for each fix.

googleworkspace-bot · 2026-03-06T02:07:22Z

/gemini review

gemini-code-assist

Code Review

This pull request does a great job of hardening URL and path construction across several modules, which is a significant security improvement. The introduction of validation for user-provided values and the use of safer URL-building APIs effectively reduce the risk of injection and path traversal vulnerabilities. However, I've identified a critical issue in src/helpers/modelarmor.rs where project and location IDs are being over-encoded, which will likely cause API calls to fail. Please see my detailed comment.

gemini-code-assist · 2026-03-06T02:11:28Z

src/helpers/modelarmor.rs

    format!(
-        "{base}/{parent}/templates?templateId={}",
-        crate::validate::encode_path_segment(&config.template_id)
+        "{base}/{parent}/templates?templateId={template_id_encoded}"
    )


The parent variable used in this format! macro is constructed on line 373 using project and location values that have been overly-encoded on lines 371-372. Using encode_path_segment on them incorrectly escapes valid characters like hyphens (e.g., us-central1 becomes us%2Dcentral1), which will likely cause the API call to fail. Since these values are validated in parse_create_template_args, they should be safe to be used directly in the URL path. Please remove the encode_path_segment calls for project and location.

NeekChaw requested a review from jpoehnelt as a code owner March 5, 2026 07:15

jpoehnelt added area: http cla: no This human has *not* signed the Contributor License Agreement. complexity: medium Moderate change, some review needed labels Mar 5, 2026

gemini-code-assist bot reviewed Mar 6, 2026

View reviewed changes

NeekChaw force-pushed the fix/harden-url-path-construction branch from ee3609f to 8bf9b14 Compare March 6, 2026 01:03

googleworkspace-bot added area: discovery area: core Core CLI parsing, commands, error handling, utilities and removed area: http labels Mar 6, 2026

gemini-code-assist bot reviewed Mar 6, 2026

View reviewed changes

NeekChaw force-pushed the fix/harden-url-path-construction branch from 8bf9b14 to 4ecdc98 Compare March 6, 2026 02:06

gemini-code-assist bot reviewed Mar 6, 2026

View reviewed changes

Conversation

NeekChaw commented Mar 5, 2026

Summary

Changes

src/discovery.rs

src/helpers/modelarmor.rs

src/helpers/gmail/watch.rs

Testing

Uh oh!

changeset-bot bot commented Mar 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

gemini-code-assist bot commented Mar 5, 2026

Uh oh!

google-cla bot commented Mar 5, 2026

Uh oh!

NeekChaw commented Mar 5, 2026

Uh oh!

jpoehnelt commented Mar 6, 2026

Uh oh!

jpoehnelt commented Mar 6, 2026

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

googleworkspace-bot commented Mar 6, 2026

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

googleworkspace-bot commented Mar 6, 2026

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

`src/discovery.rs`

`src/helpers/modelarmor.rs`

`src/helpers/gmail/watch.rs`

changeset-bot bot commented Mar 5, 2026 •

edited

Loading