fix: Switch to rustls-tls, add Android emulator support, expose set_tokens#68
Conversation
📝 WalkthroughWalkthroughUpdated the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
Deploying opensecret-sdk with
|
| Latest commit: |
783e611
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://2988fa4c.opensecret-sdk.pages.dev |
| Branch Preview URL: | https://fix-rustls-android-session-r.opensecret-sdk.pages.dev |
41ccca9 to
0b984c6
Compare
0b984c6 to
0b129f3
Compare
- Switch reqwest to rustls-tls for Android cross-compilation compatibility - Add 10.0.2.2 to mock attestation whitelist (Android emulator host alias) - Expose set_tokens() on OpenSecretClient for session restore Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
0b129f3 to
783e611
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
rust/src/client.rs (1)
31-34:⚠️ Potential issue | 🟠 MajorUse exact host matching before enabling mock attestation.
Line 31-Line 34 and Line 47-Line 50 still use substring checks; this can accidentally enable mock mode for non-local hosts. Parse URL and match
host_str()exactly.🔧 Suggested fix
impl OpenSecretClient { + fn should_use_mock_attestation(base_url: &str) -> bool { + if let Ok(url) = reqwest::Url::parse(base_url) { + matches!( + url.host_str(), + Some("localhost" | "127.0.0.1" | "0.0.0.0" | "10.0.2.2") + ) + } else { + false + } + } + pub fn new(base_url: impl Into<String>) -> Result<Self> { let base_url = base_url.into(); - let use_mock = base_url.contains("localhost") - || base_url.contains("127.0.0.1") - || base_url.contains("0.0.0.0") - || base_url.contains("10.0.2.2"); + let use_mock = Self::should_use_mock_attestation(&base_url); @@ pub fn new_with_api_key(base_url: impl Into<String>, api_key: String) -> Result<Self> { let base_url = base_url.into(); - let use_mock = base_url.contains("localhost") - || base_url.contains("127.0.0.1") - || base_url.contains("0.0.0.0") - || base_url.contains("10.0.2.2"); + let use_mock = Self::should_use_mock_attestation(&base_url);Also applies to: 47-50
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust/src/client.rs` around lines 31 - 34, The substring checks enabling mock attestation are too broad; update the logic that sets use_mock (and the similar block at the other occurrence) to parse base_url as a URL (e.g., using Url::parse) and call host_str() to compare the host exactly against "localhost", "127.0.0.1", "0.0.0.0", and "10.0.2.2"; if URL parsing fails, fall back to conservative behavior (do not enable mock) or handle the error explicitly. Locate the code that sets use_mock and the duplicate block later and replace the contains(...) checks with exact host_str() equality checks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@rust/src/client.rs`:
- Around line 31-34: The substring checks enabling mock attestation are too
broad; update the logic that sets use_mock (and the similar block at the other
occurrence) to parse base_url as a URL (e.g., using Url::parse) and call
host_str() to compare the host exactly against "localhost", "127.0.0.1",
"0.0.0.0", and "10.0.2.2"; if URL parsing fails, fall back to conservative
behavior (do not enable mock) or handle the error explicitly. Locate the code
that sets use_mock and the duplicate block later and replace the contains(...)
checks with exact host_str() equality checks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 88fb52b0-ef52-49e1-ad81-b3709ffc4952
📒 Files selected for processing (2)
rust/Cargo.tomlrust/src/client.rs
Changes
rustls-tls: Dropsnative-tls/openssl-sysdependency, fixing Android cross-compilation (no moreopenssl-syslinking errors withcargo-ndk). Works on all platforms (macOS, iOS, Linux, Windows, Android).10.0.2.2to mock attestation whitelist: Android emulator uses this IP to reach the host machine's localhost.set_tokens()onOpenSecretClient: Enables native platforms to restore persisted session tokens on cold launch without going through the full login flow.Summary by CodeRabbit
New Features
Improvements