diff --git a/.github/workflows/publish.crates.rust.sdk.yml b/.github/workflows/publish.crates.rust.sdk.yml index 1c8673117..5ae56364f 100644 --- a/.github/workflows/publish.crates.rust.sdk.yml +++ b/.github/workflows/publish.crates.rust.sdk.yml @@ -4,8 +4,38 @@ on: workflow_dispatch: jobs: + check-version: + name: Check version not already published + permissions: + contents: read + runs-on: ubuntu-latest + timeout-minutes: 5 + + defaults: + run: + working-directory: ./sdk/rust + + steps: + - name: Get the source code + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 + with: + persist-credentials: false + + - name: Verify version does not exist on crates.io + run: | + VERSION=$(grep -Po '^version\s*=\s*"\K[^"]*' Cargo.toml) + echo "Checking crates.io for keeper-secrets-manager-core v${VERSION}..." + STATUS=$(curl -s -o /dev/null -w "%{http_code}" \ + "https://crates.io/api/v1/crates/keeper_secrets_manager_core/${VERSION}") + if [ "$STATUS" = "200" ]; then + echo "❌ Version ${VERSION} already exists on crates.io — bump the version before publishing" + exit 1 + fi + echo "✅ Version ${VERSION} is new — proceeding with publish" + test-rust-sdk: name: Test Rust SDK (${{ matrix.rust }}) + needs: check-version permissions: contents: read runs-on: ubuntu-latest @@ -22,10 +52,12 @@ jobs: steps: - name: Get the source code - uses: actions/checkout@v3 + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 + with: + persist-credentials: false - name: Setup Rust toolchain - uses: actions-rust-lang/setup-rust-toolchain@v1 + uses: actions-rust-lang/setup-rust-toolchain@150fca883cd4034361b621bd4e6a9d34e5143606 # v1 with: toolchain: ${{ matrix.rust }} components: rustfmt, clippy @@ -53,8 +85,13 @@ jobs: cargo test --all-features --test integration_tests cargo test --all-features --test notation_tests cargo test --all-features --test payload_test + cargo test --all-features --test proxy_test cargo test --all-features --test totp_test cargo test --all-features --test update_secret_tests + cargo test --all-features --test caching_transmission_key_tests + cargo test --all-features --test download_file_by_title_tests + cargo test --all-features --test duplicate_uid_notation_test + cargo test --all-features --test empty_config_test - name: Run caching tests (serial execution required) run: cargo test --all-features --test caching_tests -- --test-threads=1 @@ -76,7 +113,9 @@ jobs: steps: - name: Get the source code - uses: actions/checkout@v3 + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 + with: + persist-credentials: false - name: Detect Rust SDK version id: detect-version @@ -92,7 +131,7 @@ jobs: echo "version=${VERSION}" >> "$GITHUB_OUTPUT" - name: Setup Rust toolchain - uses: actions-rust-lang/setup-rust-toolchain@v1 + uses: actions-rust-lang/setup-rust-toolchain@150fca883cd4034361b621bd4e6a9d34e5143606 # v1 with: toolchain: stable @@ -113,7 +152,7 @@ jobs: jq '.metadata.component.name' rust-sdk-sbom.json - name: Publish SBOM to Manifest Cyber - uses: manifest-cyber/manifest-github-action@main + uses: manifest-cyber/manifest-github-action@9aa4e84c80e6d232c3f49506a17f0ff1151e6896 # main with: apiKey: ${{ secrets.MANIFEST_TOKEN }} bomFilePath: ./sdk/rust/rust-sdk-sbom.json @@ -122,7 +161,7 @@ jobs: asset-labels: application,sbom-generated,rust,cargo,secrets-manager - name: Archive SBOM - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4 with: name: sbom-rust-sdk-${{ steps.detect-version.outputs.version }} path: ./sdk/rust/rust-sdk-sbom.json @@ -145,15 +184,17 @@ jobs: steps: - name: Get the source code - uses: actions/checkout@v3 + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 + with: + persist-credentials: false - name: Setup Rust toolchain - uses: actions-rust-lang/setup-rust-toolchain@v1 + uses: actions-rust-lang/setup-rust-toolchain@150fca883cd4034361b621bd4e6a9d34e5143606 # v1 with: toolchain: stable - name: Authenticate with crates.io via OIDC - uses: rust-lang/crates-io-auth-action@v1 + uses: rust-lang/crates-io-auth-action@bbd81622f20ce9e2dd9622e3218b975523e45bbe # v1 id: auth - name: Dry-run publish (validates package and authentication) @@ -180,15 +221,17 @@ jobs: steps: - name: Get the source code - uses: actions/checkout@v3 + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 + with: + persist-credentials: false - name: Setup Rust toolchain - uses: actions-rust-lang/setup-rust-toolchain@v1 + uses: actions-rust-lang/setup-rust-toolchain@150fca883cd4034361b621bd4e6a9d34e5143606 # v1 with: toolchain: stable - name: Authenticate with crates.io via OIDC - uses: rust-lang/crates-io-auth-action@v1 + uses: rust-lang/crates-io-auth-action@bbd81622f20ce9e2dd9622e3218b975523e45bbe # v1 id: auth - name: Publish to crates.io diff --git a/.github/workflows/test.rust.yml b/.github/workflows/test.rust.yml index 323103ea0..2e263b955 100644 --- a/.github/workflows/test.rust.yml +++ b/.github/workflows/test.rust.yml @@ -32,10 +32,12 @@ jobs: steps: - name: Get the source code - uses: actions/checkout@v3 + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 + with: + persist-credentials: false - name: Setup Rust toolchain - uses: actions-rust-lang/setup-rust-toolchain@v1 + uses: actions-rust-lang/setup-rust-toolchain@150fca883cd4034361b621bd4e6a9d34e5143606 # v1 with: toolchain: ${{ matrix.rust }} components: rustfmt, clippy @@ -67,6 +69,10 @@ jobs: cargo test --all-features --test proxy_test cargo test --all-features --test totp_test cargo test --all-features --test update_secret_tests + cargo test --all-features --test caching_transmission_key_tests + cargo test --all-features --test download_file_by_title_tests + cargo test --all-features --test duplicate_uid_notation_test + cargo test --all-features --test empty_config_test - name: Run caching tests (serial execution required) run: cargo test --all-features --test caching_tests -- --test-threads=1 diff --git a/sdk/rust/Cargo.lock b/sdk/rust/Cargo.lock index a05cc1c89..7c031d062 100644 --- a/sdk/rust/Cargo.lock +++ b/sdk/rust/Cargo.lock @@ -1025,7 +1025,7 @@ dependencies = [ [[package]] name = "keeper-secrets-manager-core" -version = "17.1.0" +version = "17.2.0" dependencies = [ "aes", "aes-gcm", diff --git a/sdk/rust/Cargo.toml b/sdk/rust/Cargo.toml index 4af2d138e..53910ced9 100644 --- a/sdk/rust/Cargo.toml +++ b/sdk/rust/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "keeper-secrets-manager-core" -version = "17.1.0" +version = "17.2.0" authors = ["Keeper Security "] edition = "2021" rust-version = "1.87" diff --git a/sdk/rust/src/core/core.rs b/sdk/rust/src/core/core.rs index 955a980bd..2eacfe5e5 100644 --- a/sdk/rust/src/core/core.rs +++ b/sdk/rust/src/core/core.rs @@ -195,12 +195,16 @@ pub struct SecretsManager { pub cache: KSMCache, pub proxy_url: Option, custom_post_function: Option, + /// Pre-built HTTP client shared across all operations (API calls + file downloads). + /// Built once during init to avoid constructing reqwest::blocking::Client inside + /// tokio::spawn_blocking, which fails due to nested runtime conflicts. + /// See: https://github.com/seanmonstar/reqwest/issues/1017 + http_client: Option, } impl Clone for SecretsManager { fn clone(&self) -> Self { SecretsManager { - // Clone each field of the struct token: self.token.clone(), hostname: self.hostname.clone(), verify_ssl_certs: self.verify_ssl_certs, @@ -209,6 +213,7 @@ impl Clone for SecretsManager { cache: self.cache.clone(), proxy_url: self.proxy_url.clone(), custom_post_function: self.custom_post_function, + http_client: self.http_client.clone(), } } } @@ -251,10 +256,11 @@ impl SecretsManager { hostname: String::new(), verify_ssl_certs: false, config: KvStoreType::None, - log_level: Level::Info, // Default to Info if not provided - cache: KSMCache::None, // Default is no cache + log_level: Level::Info, + cache: KSMCache::None, proxy_url: client_options.proxy_url.clone(), custom_post_function: client_options.custom_post_function, + http_client: None, // built after SSL/proxy config is resolved }; let mut config = client_options.config; @@ -389,10 +395,24 @@ impl SecretsManager { } secrets_manager.config = config.clone(); - match secrets_manager._init() { - Ok(secrets_manager) => Ok(secrets_manager), - Err(e) => Err(e), + let mut sm = secrets_manager._init()?; + + // Build a shared HTTP client once, outside any async runtime. + // Reused for API calls and file downloads. Avoids constructing + // reqwest::blocking::Client inside tokio::spawn_blocking which fails + // due to nested runtime conflicts (reqwest#1017). + let mut client_builder = + reqwest::blocking::Client::builder().danger_accept_invalid_certs(sm.verify_ssl_certs); + if let Some(proxy_url) = &sm.proxy_url { + if let Ok(proxy) = SecretsManager::build_proxy(proxy_url) { + client_builder = client_builder.proxy(proxy); + } } + sm.http_client = Some(client_builder.build().map_err(|e| { + KSMRError::SecretManagerCreationError(format!("Failed to build HTTP client: {}", e)) + })?); + + Ok(sm) } fn _init(&mut self) -> Result { @@ -1194,10 +1214,12 @@ impl SecretsManager { let record_result = Record::new_from_json(record_hashmap_parsed, &_secret_key, None); if let Ok(mut unwrapped_record) = record_result { - if let Some(proxy) = &self.proxy_url { - for file in &mut unwrapped_record.files { + for file in &mut unwrapped_record.files { + if let Some(proxy) = &self.proxy_url { file.proxy_url = Some(proxy.clone()); } + file.skip_ssl_verify = self.verify_ssl_certs; + file.http_client = self.http_client.clone(); } records_count += 1; records.push(unwrapped_record); @@ -1219,11 +1241,13 @@ impl SecretsManager { if let Some(unwrapped_folder) = folder_result { shared_folders_count += 1; let mut folder_records = unwrapped_folder.records()?; - if let Some(proxy) = &self.proxy_url { - for record in &mut folder_records { - for file in &mut record.files { + for record in &mut folder_records { + for file in &mut record.files { + if let Some(proxy) = &self.proxy_url { file.proxy_url = Some(proxy.clone()); } + file.skip_ssl_verify = self.verify_ssl_certs; + file.http_client = self.http_client.clone(); } } records_count += folder_records.len(); @@ -1291,7 +1315,7 @@ impl SecretsManager { Ok(secrets_manager_response) } - fn fetch_and_decrypt_folders(mut self) -> Result, KSMRError> { + fn fetch_and_decrypt_folders(&mut self) -> Result, KSMRError> { let payload = self .clone() .prepare_get_payload(self.config.clone(), None)?; @@ -1462,7 +1486,7 @@ impl SecretsManager { /// /// * `HTTPError` - If the API request fails /// * `CryptoError` - If folder decryption fails - pub fn get_folders(self) -> Result, KSMRError> { + pub fn get_folders(&mut self) -> Result, KSMRError> { let folders = self.fetch_and_decrypt_folders()?; Ok(folders) } diff --git a/sdk/rust/src/dto/dtos.rs b/sdk/rust/src/dto/dtos.rs index 6fd378949..85e5e3bea 100644 --- a/sdk/rust/src/dto/dtos.rs +++ b/sdk/rust/src/dto/dtos.rs @@ -1050,6 +1050,13 @@ pub struct KeeperFile { pub url: Option, // Download URL (v16.7.0+) pub thumbnail_url: Option, // Thumbnail URL (v16.7.0+) pub proxy_url: Option, // Proxy URL for HTTP requests + pub(crate) skip_ssl_verify: bool, // Skip SSL cert verification (for corporate proxies like Zscaler) + /// Pre-built HTTP client for file downloads. Avoids constructing a new + /// reqwest::blocking::Client inside tokio::spawn_blocking, which fails + /// because reqwest's blocking module creates an internal tokio runtime. + /// See: https://github.com/seanmonstar/reqwest/issues/1017 + #[serde(skip)] + pub(crate) http_client: Option, f: HashMap, record_key_bytes: Vec, @@ -1071,11 +1078,32 @@ impl KeeperFile { url: self.url.clone(), thumbnail_url: self.thumbnail_url.clone(), proxy_url: self.proxy_url.clone(), + skip_ssl_verify: self.skip_ssl_verify, + http_client: self.http_client.clone(), f: self.f.clone(), record_key_bytes: self.record_key_bytes.clone(), } } + /// Returns the pre-built HTTP client if available, or builds a fallback one. + /// The pre-built client is propagated from SecretsManager to avoid constructing + /// reqwest::blocking::Client inside tokio::spawn_blocking (reqwest#1017). + fn resolve_http_client(&self) -> Result { + if let Some(client) = &self.http_client { + return Ok(client.clone()); + } + let mut client_builder = reqwest::blocking::Client::builder() + .danger_accept_invalid_certs(self.skip_ssl_verify); + if let Some(ref proxy_url) = self.proxy_url { + if let Ok(proxy) = reqwest::Proxy::all(proxy_url) { + client_builder = client_builder.proxy(proxy); + } + } + client_builder + .build() + .map_err(|e| KSMRError::FileError(format!("Failed to build HTTP client: {}", e))) + } + /// Decrypts the file key using the record key bytes. pub fn decrypt_file_key(&self) -> Result, KSMRError> { // Retrieve the Base64-encoded file key from metadata @@ -1151,16 +1179,7 @@ impl KeeperFile { .get_url() .map_err(|_| KSMRError::FileError("File URL is invalid".to_string()))?; - // Fetch the file data from the URL - let mut client_builder = reqwest::blocking::Client::builder(); - if let Some(ref proxy_url) = self.proxy_url { - if let Ok(proxy) = reqwest::Proxy::all(proxy_url) { - client_builder = client_builder.proxy(proxy); - } - } - let http_client = client_builder - .build() - .map_err(|e| KSMRError::FileError(format!("Failed to build HTTP client: {}", e)))?; + let http_client = self.resolve_http_client()?; let mut response = http_client .get(&file_url) .send() @@ -1236,16 +1255,7 @@ impl KeeperFile { // Decrypt the file key let file_key = self.decrypt_file_key()?; - // Fetch the thumbnail data from the URL - let mut client_builder = reqwest::blocking::Client::builder(); - if let Some(ref proxy_url) = self.proxy_url { - if let Ok(proxy) = reqwest::Proxy::all(proxy_url) { - client_builder = client_builder.proxy(proxy); - } - } - let http_client = client_builder - .build() - .map_err(|e| KSMRError::FileError(format!("Failed to build HTTP client: {}", e)))?; + let http_client = self.resolve_http_client()?; let mut response = http_client .get(&thumbnail_url) .send() @@ -1300,6 +1310,8 @@ impl KeeperFile { url, thumbnail_url, proxy_url: None, + skip_ssl_verify: false, + http_client: None, f: file_dict.clone(), record_key_bytes, }; @@ -1846,6 +1858,8 @@ mod tests { url: None, thumbnail_url: None, proxy_url, + skip_ssl_verify: false, + http_client: None, f: HashMap::new(), record_key_bytes: vec![], } diff --git a/sdk/rust/tests/empty_config_test.rs b/sdk/rust/tests/empty_config_test.rs index fc968fb0e..7a4d5813f 100644 --- a/sdk/rust/tests/empty_config_test.rs +++ b/sdk/rust/tests/empty_config_test.rs @@ -35,6 +35,7 @@ mod empty_config_tests { Level::Error, None, None, + None, KSMCache::None, ); @@ -70,6 +71,7 @@ mod empty_config_tests { Level::Error, None, None, + None, KSMCache::None, ); @@ -98,6 +100,7 @@ mod empty_config_tests { Level::Error, None, None, + None, KSMCache::None, ); diff --git a/sdk/rust/tests/feature_validation_tests.rs b/sdk/rust/tests/feature_validation_tests.rs index 87ddfa18e..fbdf9c8be 100644 --- a/sdk/rust/tests/feature_validation_tests.rs +++ b/sdk/rust/tests/feature_validation_tests.rs @@ -725,4 +725,41 @@ mod feature_validation_tests { // This test passing means all new features are properly exported and compile assert!(true); } + + /// Regression test for KSM-812: get_folders() must take &mut self, not self. + /// + /// Before the fix, get_folders() consumed the SecretsManager, so any subsequent + /// call on the same instance would fail to compile. This test verifies the + /// instance is still usable after calling get_folders(). + #[test] + fn test_get_folders_does_not_consume_secrets_manager() { + fn mock_empty_folders( + _url: String, + transmission_key: TransmissionKey, + _encrypted_payload: EncryptedPayload, + ) -> Result { + let response = json!({"folders": [], "records": [], "expiresOn": 0, "warnings": []}); + let response_bytes = response.to_string().into_bytes(); + let encrypted = + CryptoUtils::encrypt_aes_gcm(&response_bytes, &transmission_key.key, None)?; + Ok(KsmHttpResponse { + status_code: 200, + data: encrypted, + http_response: None, + }) + } + + let storage = create_test_storage().expect("Failed to create storage"); + let mut client_options = ClientOptions::new_client_options(storage); + client_options.set_custom_post_function(mock_empty_folders); + let mut sm = SecretsManager::new(client_options).expect("Failed to create SecretsManager"); + + // First call + let first = sm.get_folders(); + // Second call on the same instance — would not compile if get_folders() took self + let second = sm.get_folders(); + + assert!(first.is_ok(), "First get_folders() call failed: {:?}", first); + assert!(second.is_ok(), "Second get_folders() call failed: {:?}", second); + } }