feat: expose library crate for programmatic API access#376
feat: expose library crate for programmatic API access#376geofflittle wants to merge 9 commits intogoogleworkspace:mainfrom
Conversation
🦋 Changeset detectedLatest commit: 88de70a 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 |
googleworkspace-bot
added
area: auth
area: distribution
area: http
area: discovery
area: core
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 successfully refactors the codebase to expose a library crate, which is a great step for enabling programmatic use of gws. The extraction of configuration and sanitization logic into separate modules (config.rs, sanitize.rs) is well-executed and improves the project structure. I appreciate the security enhancements, particularly the GCP location validation to prevent SSRF and the more robust parsing of --api-version. The changes are clean and the addition of integration tests for the new library is a good practice. I have one suggestion regarding the public API of the new library crate to improve its design.
|
/gemini review |
1 similar comment
|
/gemini review |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #376 +/- ##
==========================================
+ Coverage 63.23% 63.38% +0.15%
==========================================
Files 38 40 +2
Lines 15102 15188 +86
==========================================
+ Hits 9549 9627 +78
- Misses 5553 5561 +8 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Please rebase on main. |
geofflittle
force-pushed
the
feat/library-crate
branch
from
ad22647 to
dd49f71
Compare
|
/gemini review |
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.
… 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.
There was a problem hiding this comment.
Code Review
The pull request introduces a new library crate for programmatic API access to the Google Workspace CLI, involving significant refactoring to move shared utilities like config_dir() and Model Armor sanitization logic into dedicated, reusable modules (src/config.rs and src/sanitize.rs). It also enhances security by adding validation for sanitize template parameters to prevent path traversal and SSRF vulnerabilities. Review comments suggest improving the library's public API surface by making several modules pub(crate) instead of pub to limit exposure of CLI-specific or internal details, and refactoring a C-style loop in src/sanitize.rs to a more idiomatic and safer Rust iterator-based approach.
geofflittle
force-pushed
the
feat/library-crate
branch
from
dd49f71 to
45f475f
Compare
|
/gemini review |
- Restrict public API surface: make commands, credential_store, formatter, oauth_config, and token_storage pub(crate) instead of pub - Refactor extract_location in sanitize.rs to use idiomatic iterator pattern instead of C-style indexed loop
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request successfully exposes a library crate for programmatic API access by refactoring shared logic into standalone modules. Key changes include moving configuration and sanitization logic into config.rs and sanitize.rs respectively. The pull request also introduces critical security improvements by validating GCP location parameters to prevent SSRF, and fixes a bug in --api-version flag parsing. The overall structure is sound, and the changes are of high quality. I have no further recommendations for improvement.
There was a problem hiding this comment.
Code Review
This pull request is a significant and well-executed refactoring to expose a library crate, which will greatly improve the reusability of the project's core logic. The code has been thoughtfully decoupled into more logical modules, such as config and sanitize. I particularly appreciate the security enhancements, including the GCP location validation to prevent SSRF and the fix for --api-version flag parsing.
I've found one critical issue regarding module visibility that prevents a key part of the new public API from being used. Please see my comment for details. Once that is addressed, this will be an excellent addition to the codebase.
| pub mod discovery; | ||
| pub mod error; | ||
| pub mod executor; | ||
| pub(crate) mod formatter; |
There was a problem hiding this comment.
The formatter module is declared as pub(crate), which makes its contents, including the OutputFormat enum, inaccessible to external crates. However, the public function executor::execute_method has a parameter of type &crate::formatter::OutputFormat. This makes it impossible for consumers of this library to call execute_method, as they cannot construct a value of the required OutputFormat type. To make the public API usable, the formatter module should be made public.
| pub(crate) mod formatter; | |
| pub mod formatter; |
|
Thanks for the idea, will reconsider with some additional research on the best way to do this. |
3 participants
Summary
lib.rs) sogwscan be used as a Rust dependency for programmatic API accessconfig_dir()and Model Armor sanitization types into standalone modules (config.rs,sanitize.rs) to decouple library from CLIparse_service_and_version()toservices.rsfor dual crate-root access--api-versionflag parsing fix, and config dir env override restorationZero behavior changes to the binary. All existing tests pass.
Supersedes #214 (auto-closed by stale bot). Addresses #211.
Test plan
cargo test— all existing tests passcargo clippy -- -D warningscleantests/lib_integration.rs) verify library surface--api-versionflag parsing🤖 Generated with Claude Code