feat: expose library crate for programmatic API access#214
feat: expose library crate for programmatic API access#214geofflittle wants to merge 6 commits intogoogleworkspace:mainfrom
Conversation
🦋 Changeset detectedLatest commit: 0a078bd 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 |
|
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. |
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 significantly enhances the Highlights
Changelog
Activity
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 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 counter productive. 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. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request effectively refactors the codebase to expose a library crate, which is a great step towards making gws more reusable. The extraction of configuration and sanitization logic into their own modules is clean and well-executed. I have one suggestion to make the new library API for SanitizeMode more idiomatic.
|
I have signed the CLA — https://cla.developers.google.com/ shows it under g.little712@gmail.com / geofflittle. |
geofflittle
force-pushed
the
feat/library-crate
branch
from
12b596f to
bca21b0
Compare
|
Removed the |
geofflittle
force-pushed
the
feat/library-crate
branch
from
bca21b0 to
e90ae20
Compare
googleworkspace-bot
added
area: distribution
area: core
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request effectively refactors the codebase to expose a library crate, enabling programmatic use of gws. The separation of concerns is well-executed by extracting shared logic into new modules like config.rs and sanitize.rs. The new library structure in lib.rs and the corresponding Cargo.toml changes are correct. I've identified one critical security vulnerability in the new sanitize.rs module related to input validation, which I've detailed in a specific comment.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #214 +/- ##
==========================================
+ Coverage 59.09% 59.32% +0.23%
==========================================
Files 36 38 +2
Lines 12953 13039 +86
==========================================
+ Hits 7654 7736 +82
- Misses 5299 5303 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
geofflittle
force-pushed
the
feat/library-crate
branch
from
e90ae20 to
3bd106c
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request is a well-executed refactoring that successfully exposes a library crate for programmatic API access. The modularization of config and sanitize logic is clean, and the introduction of lib.rs provides a clear API surface. I have one high-severity comment regarding argument parsing that should be addressed to ensure consistent behavior.
geofflittle
force-pushed
the
feat/library-crate
branch
from
3bd106c to
b330947
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request successfully refactors the codebase to expose a library crate, which is a great step for enabling programmatic use of gws. The changes are well-structured, moving shared logic into new config and sanitize modules and creating a lib.rs entry point.
I have one critical piece of feedback regarding a behavioral change. The logic for overriding the configuration directory via the GOOGLE_WORKSPACE_CLI_CONFIG_DIR environment variable has been unintentionally restricted to test builds only. This is a breaking change for users of the binary, as this is a documented feature. My review comment includes a suggestion to fix this.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request is a well-executed refactoring that successfully exposes the gws tool as a library crate. The code is cleanly split into a new lib.rs target, with shared logic like configuration and sanitization extracted into their own modules. The addition of integration tests for the library surface is a great touch. However, I've identified a critical issue where the GOOGLE_WORKSPACE_CLI_CONFIG_DIR environment variable override has been unintentionally restricted to test builds only. This is a breaking change for users who rely on this variable and contradicts the PR's claim of zero behavior changes.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request is a well-executed refactoring to expose a library crate, which is a great architectural improvement. The changes to split out configuration and sanitization logic into their own modules are clean. The included security fix to validate sanitize template parameters against path traversal and SSRF is also a critical addition. I've found one high-severity issue related to argument parsing that could lead to confusing behavior, and I've provided a suggestion to make it more robust. Overall, this is a solid contribution.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request is a well-executed refactoring that successfully exposes a library crate for programmatic API access. The changes are logically structured, with shared functionality like configuration management and sanitization logic cleanly extracted into their own modules. The introduction of validation for sanitize template parameters is a crucial security improvement that hardens the API against path traversal and SSRF vulnerabilities. The addition of integration tests for the new library crate is also a valuable contribution that enhances the project's robustness. Overall, the code quality is high, and the changes align well with the stated goals of the pull request.
There was a problem hiding this comment.
Code Review
This pull request is a well-executed refactoring that successfully exposes a library crate for programmatic API access. The changes are logically structured, with shared functionality like configuration management and sanitization logic cleanly extracted into their own modules. The introduction of validation for sanitize template parameters is a crucial security improvement that hardens the API against path traversal and SSRF vulnerabilities. The addition of integration tests for the new library crate is also a valuable contribution that enhances the project's robustness. Overall, the code quality is high, and the changes align well with the stated goals of the pull request.
Extract `config_dir()` into `src/config.rs` and Model Armor sanitization types into `src/sanitize.rs` so they can be shared between the binary and library targets without pulling in CLI-only code. Add `src/lib.rs` with public module re-exports and `tests/lib_integration.rs` with offline tests. Also moves `parse_service_and_version()` from `main.rs` to `services.rs` so it is accessible from both the lib and bin crate roots. Zero behavior changes to the binary.
Add validate_resource_name() and validate_api_identifier() checks in build_sanitize_request_data() to prevent path traversal, query injection, and percent-encoded bypasses in the template parameter.
…ntax - Remove #[cfg(test)] from GOOGLE_WORKSPACE_CLI_CONFIG_DIR check in config_dir() so the override works in release builds. - Handle --api-version=v3 syntax in parse_service_and_version(), not just --api-version v3.
geofflittle
force-pushed
the
feat/library-crate
branch
from
b849ab0 to
03f112c
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request is a well-executed refactoring to expose a library crate for programmatic API access. The code is cleanly split into a library and binary, with thoughtful module organization and visibility. The changes are logical and effectively decouple the core logic from the CLI. I appreciate the simultaneous inclusion of a security enhancement to validate sanitize template parameters against path traversal and SSRF, which strengthens the tool. The addition of integration tests for the new library surface is also a great touch. Overall, this is a high-quality contribution that improves the project's structure and security.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request is a significant and well-executed refactoring to expose a library crate for programmatic access. The code has been cleanly split into library and binary targets, with shared logic extracted into new modules like config.rs and sanitize.rs. The changes are logical and follow the description.
However, I've identified a critical security vulnerability in the new sanitize.rs module. The validation for the location parameter, which is used to construct a hostname for an API call, is insufficient and could lead to a Server-Side Request Forgery (SSRF) attack. My review includes a specific comment with a suggested fix for this issue.
… template Replace validate_api_identifier (which allows dots) with a stricter validate_gcp_location that only permits lowercase alphanumeric and hyphens. This prevents an attacker from injecting a dotted domain (e.g. evil.com) into the Model Armor regional hostname.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request successfully refactors the codebase to expose a library crate, enabling programmatic use of gws. The changes are well-structured, moving shared logic into new modules like config.rs and sanitize.rs and setting up a dual-target compilation with lib.rs and main.rs. A notable improvement is the addition of input validation in the sanitization logic, which effectively mitigates potential SSRF and path traversal vulnerabilities. The refactoring of functions into the services module also enhances code organization. The new integration tests for the library crate are a welcome addition. Overall, this is a high-quality contribution that improves the project's structure and security.
3 participants
Description
Adds a library crate (
lib.rs) so thatgwscan be used as a Rust dependency for programmatic access to Google Workspace APIs — without shelling out to the binary.Addresses #211.
Two coupling points previously prevented a clean lib/bin split:
config_dir()lived in the CLI-onlyauth_commandsmodule but was called by 6 library modules (accounts,credential_store,oauth_config,auth,discovery)SanitizeMode,SanitizeConfig,SanitizationResult,sanitize_text()) lived inhelpers/modelarmorbut were used byexecutorThis PR extracts both into standalone modules (
config.rs,sanitize.rs), createslib.rsre-exporting library-worthy modules, and adds a[lib]section toCargo.toml. The original call sites delegate to the new modules, so all existing code paths are unchanged.Also moves
parse_service_and_version()frommain.rstoservices.rsso it is accessible from both crate roots.Zero behavior changes to the binary. All existing tests pass.
Dry Run Output:
Checklist:
AGENTS.mdguidelines (no generatedgoogle-*crates).cargo fmt --allto format the code perfectly.cargo clippy -- -D warningsand resolved all warnings.pnpx changeset) to document my changes.Notes
cargo clippy -- -D warningsis clean for all new/modified code. There is 1 pre-existingshould_implement_traitwarning informatter.rs:55(unrelated to this PR).tests/lib_integration.rs) verify the library surface:RestDescriptiondeserialization,resolve_service(),GwsError,validate_api_identifier().#![allow(dead_code)]onlib.rssuppresses warnings forpub(crate)modules that are only used by the binary target (dual compilation — same.rsfiles compiled for both targets).