-
Notifications
You must be signed in to change notification settings - Fork 55
Add CLI flags for explicit OCI authentication credentials #578
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: copilot/add-oci-authentication-support
Are you sure you want to change the base?
Add CLI flags for explicit OCI authentication credentials #578
Conversation
- Add --registry-user, --registry-password, and --registry-password-stdin flags to component load command - Add OciCredentials struct for passing credentials through the stack - Add get_registry_auth_with_credentials function with priority: explicit > Docker config > Anonymous - Update LifecycleManager to accept optional credentials - Update loader to thread credentials through to OCI auth - Enable clap env feature for environment variable support Co-authored-by: Mossaka <5447827+Mossaka@users.noreply.github.com>
- Test explicit credentials take precedence over Docker config - Test fallback to Docker config when no explicit credentials provided - Test explicit credentials work without Docker config - Test anonymous auth when no credentials available - All 9 tests pass successfully Co-authored-by: Mossaka <5447827+Mossaka@users.noreply.github.com>
- Test validation rejects incomplete credentials (username only or password only) - Test help text documents all new flags and environment variables - Both integration tests pass successfully Co-authored-by: Mossaka <5447827+Mossaka@users.noreply.github.com>
Co-authored-by: Mossaka <5447827+Mossaka@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for explicit OCI registry authentication via CLI flags and environment variables, implementing the credential resolution precedence: explicit credentials > Docker config > anonymous. This complements the existing Docker config authentication from Subtask 1.
Key Changes:
- Added CLI flags
--registry-user,--registry-password, and--registry-password-stdinwith corresponding environment variables - Implemented credential pairing validation and stdin password reading
- Threaded credentials through the stack from CLI → LifecycleManager → loader → OCI client
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| Cargo.toml | Added "env" feature to clap for environment variable support |
| src/commands.rs | Added three new CLI arguments for OCI authentication |
| src/main.rs | Implemented credential validation, stdin reading, and credential object creation |
| crates/wassette/src/lib.rs | Added load_component_with_credentials and credential threading methods |
| crates/wassette/src/loader.rs | Extended Loadable trait with credential-aware methods |
| crates/wassette/src/oci_auth.rs | Added OciCredentials struct and get_registry_auth_with_credentials function |
| crates/mcp-server/src/components.rs | Added handle_load_component_cli_with_credentials function |
| tests/cli_integration_test.rs | Added integration tests for credential validation and help text |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const FILE_EXTENSION: &'static str; | ||
| const RESOURCE_TYPE: &'static str; | ||
|
|
||
| async fn from_local_file(path: &Path) -> Result<DownloadedResource>; |
Copilot
AI
Nov 17, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The #[allow(dead_code)] attribute suggests this method is now unused. Since from_oci_reference_with_progress_and_credentials is the new implementation and this method is kept as a wrapper, consider whether this attribute is needed. If the method is part of the public Loadable trait API and should remain available for backward compatibility, it's fine to keep it. Otherwise, if it's truly unused, the attribute is appropriate but consider documenting why it's retained.
| async fn from_local_file(path: &Path) -> Result<DownloadedResource>; | |
| async fn from_local_file(path: &Path) -> Result<DownloadedResource>; | |
| /// Retained for backward compatibility with existing consumers of the Loadable trait. | |
| /// The preferred method is `from_oci_reference_with_progress_and_credentials`. | |
| /// The `#[allow(dead_code)]` attribute suppresses warnings since this method may not be used internally. |
| #[allow(dead_code)] | ||
| async fn resolve_component_resource(&self, uri: &str) -> Result<(String, DownloadedResource)> { | ||
| self.resolve_component_resource_with_credentials(uri, None) | ||
| .await | ||
| } | ||
|
|
Copilot
AI
Nov 17, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar to the loader.rs case, this private method now has #[allow(dead_code)] because it's been replaced by resolve_component_resource_with_credentials. Since this appears to be a private method (not pub), consider whether it should be removed entirely rather than kept with the dead_code attribute. If it's intentionally kept for potential future use or backward compatibility, add a comment explaining why.
| #[allow(dead_code)] | |
| async fn resolve_component_resource(&self, uri: &str) -> Result<(String, DownloadedResource)> { | |
| self.resolve_component_resource_with_credentials(uri, None) | |
| .await | |
| } |
| Ok(()) | ||
| } | ||
|
|
||
| #[test(tokio::test)] |
Copilot
AI
Nov 17, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test attribute #[test(tokio::test)] is non-standard. The correct tokio test macro is #[tokio::test]. While this appears to be using the test_log crate's wrapper (as seen in the imports), verify this is the intended test attribute format for the project. The pattern #[test(tokio::test)] is consistent with other tests in the file, but ensure it's compatible with the project's test infrastructure.
| Ok(()) | ||
| } | ||
|
|
||
| #[test(tokio::test)] |
Copilot
AI
Nov 17, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as the previous comment - the test attribute #[test(tokio::test)] should be verified as the correct format. The standard tokio test attribute is #[tokio::test]. This appears to be using the test_log crate's wrapper pattern consistently throughout the file, so if this is the project's standard, it's acceptable, but worth confirming.
Adds support for explicit OCI registry authentication via CLI flags and environment variables, complementing the existing Docker config authentication from Subtask 1.
Changes
CLI Interface
--registry-user/OCI_REGISTRY_USERfor username--registry-password/OCI_REGISTRY_PASSWORDfor password--registry-password-stdinfor secure password input without shell historyAuthentication Priority
Implemented credential resolution with explicit precedence:
~/.docker/config.json)Implementation
OciCredentialsstruct for passing credentials through the stackget_registry_auth_with_credentials()inoci_authmodulehandle_load_component_cli_with_credentials()for credential injectionUsage
Testing
Part of #559 (Subtask 2/4)
Original prompt
This section details on the original issue you should resolve
<issue_title>[subtask] [Subtask 2/4] Add CLI flags for explicit OCI authentication credentials</issue_title>
<issue_description>Parent Issue: #559
Objective
Add CLI flags to the
component loadcommand to support explicit authentication credentials for OCI registries.Context
Building on Subtask 1 which implemented Docker config authentication, this subtask adds explicit CLI options for users to provide credentials directly via command-line flags or environment variables. This is useful in CI/CD environments or when Docker config is not available.
Implementation Details
Files to Modify
src/commands.rs(line 141-149):ComponentCommands::Load:src/cli_handlers.rs:--registry-password-stdinby reading from stdincrates/mcp-server/src/components.rs(line 44-86):handle_load_component()to accept optional credentialslifecycle_manager.load_component()crates/wassette/src/lib.rs(LifecycleManager):load_component()signature to accept optionalOciCredentialsstructcrates/wassette/src/oci_auth.rs(from Subtask 1):fn get_registry_auth_with_credentials(reference: &Reference, user: Option(String), password: Option(String)) -> Result(RegistryAuth)Key Implementation Notes
OCI_REGISTRY_USER,OCI_REGISTRY_PASSWORD) for CI/CD friendliness--registry-password-stdinfor better security (avoids password in shell history)--insecure-skip-tls-verifyflag for development (optional)Acceptance Criteria
--registry-userand--registry-passwordflags work for explicit auth--registry-password-stdinreads password securely from stdinOCI_REGISTRY_USERandOCI_REGISTRY_PASSWORDwork--helpoutputTesting Strategy
Unit tests:
Integration tests:
Manual testing:
Dependencies
Depends on Subtask 1 being completed (Docker config support and
oci_auth.rsmodule).Example Usage
Related to #559
Comments on the Issue (you are @copilot in this section)
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.