feat: expose library crate for programmatic API access#214
feat: expose library crate for programmatic API access#214geofflittle wants to merge 1 commit intogoogleworkspace:mainfrom
Conversation
🦋 Changeset detectedLatest commit: e90ae20 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 |
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.
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.
| pub fn build_sanitize_request_data( | ||
| template: &str, | ||
| text: &str, | ||
| method: &str, | ||
| ) -> Result<(String, String), GwsError> { | ||
| let location = extract_location(template).ok_or_else(|| { | ||
| GwsError::Validation( | ||
| "Cannot extract location from --sanitize template. Expected format: projects/PROJECT/locations/LOCATION/templates/TEMPLATE".to_string(), | ||
| ) | ||
| })?; |
There was a problem hiding this comment.
The template parameter, which can be user-controlled, is used to construct a request URL without validation. The location extracted from it is used to form a hostname, and the template string itself is part of the URL path. This could lead to a Server-Side Request Forgery (SSRF) vulnerability if a malicious value is provided. I recommend validating both the template and the extracted location using the existing validation utilities to prevent this.
| pub fn build_sanitize_request_data( | |
| template: &str, | |
| text: &str, | |
| method: &str, | |
| ) -> Result<(String, String), GwsError> { | |
| let location = extract_location(template).ok_or_else(|| { | |
| GwsError::Validation( | |
| "Cannot extract location from --sanitize template. Expected format: projects/PROJECT/locations/LOCATION/templates/TEMPLATE".to_string(), | |
| ) | |
| })?; | |
| pub fn build_sanitize_request_data( | |
| template: &str, | |
| text: &str, | |
| method: &str, | |
| ) -> Result<(String, String), GwsError> { | |
| let template = crate::validate::validate_resource_name(template)?; | |
| let location = extract_location(template).ok_or_else(|| { | |
| GwsError::Validation( | |
| "Cannot extract location from --sanitize template. Expected format: projects/PROJECT/locations/LOCATION/templates/TEMPLATE".to_string(), | |
| ) | |
| })?; | |
| let location = crate::validate::validate_api_identifier(location)?; |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #214 +/- ##
==========================================
+ Coverage 57.52% 57.58% +0.05%
==========================================
Files 38 40 +2
Lines 14188 14209 +21
==========================================
+ Hits 8162 8182 +20
- Misses 6026 6027 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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).