Stage and retire backup certificate pins#419
Conversation
|
Warning Review limit reached
Next review available in: 50 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
WalkthroughAdds certificate pin staging and per-pin retirement APIs, wires them through the settings UI, validates SPKI hashes, and adds retirement/staging dialogs with supporting tests and strings. ChangesCertificate pin lifecycle
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant CertificatePinsCard
participant SettingsTab
participant KeepMobileApp
participant KeepMobile
User->>CertificatePinsCard: Confirm staging or retirement
CertificatePinsCard->>SettingsTab: Invoke pin callback
SettingsTab->>KeepMobileApp: Stage or remove certificate pin
KeepMobileApp->>KeepMobile: Call compatibility API
KeepMobile-->>KeepMobileApp: Return operation result
KeepMobileApp-->>SettingsTab: Return status
SettingsTab-->>CertificatePinsCard: Refresh displayed pins
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/src/main/kotlin/io/privkey/keep/SecurityCards.kt (1)
225-228: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueCompile the regular expression once.
isValidSpkiHashis called on every keystroke because it evaluatescanStageduring the text field's recomposition. Compiling a newRegexobject each time is inefficient. Consider extracting theRegexto a top-level constant.♻️ Proposed refactor
+private val SPKI_HASH_REGEX = Regex("[0-9a-fA-F]{64}") + /** A valid SPKI pin is a SHA-256 digest: exactly 64 hex characters. */ internal fun isValidSpkiHash(hash: String): Boolean = - hash.matches(Regex("[0-9a-fA-F]{64}")) + hash.matches(SPKI_HASH_REGEX)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/kotlin/io/privkey/keep/SecurityCards.kt` around lines 225 - 228, Extract the SPKI hash pattern used by isValidSpkiHash into a top-level compiled Regex constant, then have isValidSpkiHash reuse that constant for validation on each call.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@app/src/main/kotlin/io/privkey/keep/SecurityCards.kt`:
- Around line 225-228: Extract the SPKI hash pattern used by isValidSpkiHash
into a top-level compiled Regex constant, then have isValidSpkiHash reuse that
constant for validation on each call.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3e1d2ad3-c3f4-4db9-a473-d3e4044d0360
📒 Files selected for processing (7)
app/src/main/kotlin/io/privkey/keep/CertificatePinning.ktapp/src/main/kotlin/io/privkey/keep/KeepMobileApp.ktapp/src/main/kotlin/io/privkey/keep/MainActivity.ktapp/src/main/kotlin/io/privkey/keep/SecurityCards.ktapp/src/main/res/values/strings_settings.xmlapp/src/test/kotlin/io/privkey/keep/CertificatePinsCardTest.ktkeep.version
1 participant
Makes RFC 7469 backup certificate pins usable from the app. keep-frost-net already supports multiple SPKI pins per host so a relay certificate rotation verifies against either the current or the next key during the overlap, but on mobile that capability was dormant: the automatic flow only pins an unpinned host, and the UI could only clear a host's pins wholesale.
Changes
keep.versionto pick up the keep-mobilestage_certificate_pin/remove_certificate_pinFFI (backend merged in keep-mobile: expose backup cert-pin staging and per-pin removal keep#807).stageCertificatePinCompat/removeCertificatePinCompat) alongside the existinggetCertificatePinsCompat, so the app compiles regardless of binding freshness.Testing
CertificatePinsCardTest).compileDebugKotlin+testDebugUnitTestgreen against freshly generated bindings.Summary by CodeRabbit
New Features
Bug Fixes
Tests