feat: guard ADB SMS deletion with fingerprints - #7
Conversation
🤖 CodeAnt AI — Review Status
|
Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
📝 WalkthroughWalkthroughThe ADB provider now supports guarded deletion of one system SMS. It validates a SHA-256 fingerprint, requires QUIK to be the default SMS app, compares the live system record, deletes matching system and local records, and verifies both are absent. ChangesGuarded SMS deletion
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant ADB client
participant AdbMessagesProvider
participant SmsDeleteGuard
participant Android SMS provider
participant Local SMS repositories
ADB client->>AdbMessagesProvider: Request /sms/{contentId} with fingerprint
AdbMessagesProvider->>SmsDeleteGuard: Parse fingerprint
SmsDeleteGuard-->>AdbMessagesProvider: Validated fingerprint
AdbMessagesProvider->>Android SMS provider: Read current SMS record
Android SMS provider-->>AdbMessagesProvider: SMS fields
AdbMessagesProvider->>SmsDeleteGuard: Generate record fingerprint
SmsDeleteGuard-->>AdbMessagesProvider: SHA-256 fingerprint
AdbMessagesProvider->>Android SMS provider: Delete system SMS
AdbMessagesProvider->>Local SMS repositories: Delete local SMS
AdbMessagesProvider->>Android SMS provider: Verify system record absence
AdbMessagesProvider->>Local SMS repositories: Verify local record absence
AdbMessagesProvider-->>ADB client: Return verified deletion result
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
codeant-ai
Bot
added
the
size:L
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
| val live = readSystemSms(contentId) ?: return 0 | ||
| if (!MessageDigest.isEqual( | ||
| SmsDeleteGuard.fingerprint(live).toByteArray(Charsets.US_ASCII), | ||
| expectedFingerprint.toByteArray(Charsets.US_ASCII) | ||
| )) { | ||
| throw SecurityException("SMS changed since it was read") | ||
| } |
There was a problem hiding this comment.
Suggestion: The fingerprint is checked against a snapshot obtained by readSystemSms, but the subsequent unconditional delete only targets the numeric content ID. If the row is modified or replaced between these operations, the supplied fingerprint can validate one row while this delete removes a different row. Make the deletion conditional on the same fingerprint fields, or otherwise perform validation and deletion atomically. [race condition]
Severity Level: Critical 🚨
- ❌ A concurrent SMS update can bypass the deletion guard.
- ⚠️ A stale fingerprint may delete modified message content.
- ⚠️ Default SMS provider operations create the race window.
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** presentation/src/main/java/com/moez/QKSMS/adb/AdbMessagesProvider.kt
**Line:** 234:240
**Comment:**
*Race Condition: The fingerprint is checked against a snapshot obtained by `readSystemSms`, but the subsequent unconditional delete only targets the numeric content ID. If the row is modified or replaced between these operations, the supplied fingerprint can validate one row while this delete removes a different row. Make the deletion conditional on the same fingerprint fields, or otherwise perform validation and deletion atomically.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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.
Inline comments:
In `@presentation/src/main/java/com/moez/QKSMS/adb/AdbMessagesProvider.kt`:
- Around line 247-254: Update the deletion flow around the systemUri delete,
messageRepo.deleteMessages, and subsequent verification to durably reconcile
failures between the system provider and local repository. Ensure a process
death or repository exception after the system delete cannot leave the local row
permanently present, using the existing persistence/recovery mechanisms where
available; retain verification as a final consistency check rather than the sole
recovery mechanism.
- Around line 247-248: Update the deletion flow in AdbMessagesProvider around
the systemUri delete and messageRepo.deleteMessages: delete through
Sms.CONTENT_URI using a parameterized predicate covering every raw value in
SMS_FINGERPRINT_COLUMNS, including _id, with IS NULL for null fingerprints, and
treat any count other than 1 as a mismatch. Add recovery or reconciliation
handling so a committed system deletion followed by a crash or local-delete
failure cannot leave the local row undeleted.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 93d94c58-a4d6-4142-a827-6790ba7318e6
📒 Files selected for processing (3)
presentation/src/main/java/com/moez/QKSMS/adb/AdbMessagesProvider.ktpresentation/src/main/java/com/moez/QKSMS/adb/SmsDeleteGuard.ktpresentation/src/test/java/com/moez/QKSMS/adb/SmsDeleteGuardTest.kt
| val systemUri = ContentUris.withAppendedId(Sms.CONTENT_URI, contentId) | ||
| if (appContext.contentResolver.delete(systemUri, null, null) != 1) return 0 |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🌐 Web query:
For Android API 34, does the Telephony SMS ContentProvider honor ContentResolver.delete selection and selectionArgs when the URI is content://sms/{id}? What is the supported way to atomically delete an SMS only when multiple current column values match?
💡 Result:
For Android API 34, the Telephony SMS ContentProvider does not effectively honor the selection and selectionArgs parameters when the URI is specifically formatted as content://sms/{id} [1][2]. When a URI ending in an ID (e.g., content://sms/5) is provided to the delete() method, the SmsProvider matches it against the SMS_ALL_ID case [2]. In this implementation, the provider parses the ID directly from the URI path and calls an internal method (e.g., deleteOneSms) that targets that specific row by its primary key, effectively ignoring any provided selection or selectionArgs [2]. To atomically delete an SMS only when multiple column values match, you should not use the single-ID URI. Instead, use the general Telephony.Sms.CONTENT_URI (which matches the SMS_ALL case in the provider) along with a combined selection string [2]. The supported way to achieve this is: 1. Construct a selection string that combines your desired column criteria using standard SQL logic (e.g., "address=? AND date=? AND body=?") [3]. 2. Provide the corresponding values in the selectionArgs array [4]. 3. Execute the delete operation using the base URI: contentResolver.delete(Telephony.Sms.CONTENT_URI, selection, selectionArgs) [2][3]. Because the SmsProvider uses db.delete(TABLE_SMS, where, whereArgs) for SMS_ALL queries, the database engine will process the selection criteria atomically within the transaction [1][2]. Note that your application must be the default SMS handler on the device to have write access to the Telephony provider [5].
Citations:
- 1: https://android.googlesource.com/platform/packages/providers/TelephonyProvider/+/master/src/com/android/providers/telephony/SmsProvider.java
- 2: https://android.googlesource.com/platform/packages/providers/TelephonyProvider/+/7e7c274/src/com/android/providers/telephony/SmsProvider.java
- 3: https://stackoverflow.com/questions/17351143/how-to-delete-last-received-sms-from-inbox-in-android
- 4: https://stackoverflow.com/questions/30745844/how-to-pass-a-selectionargs-when-i-use-the-keyword-in-in-android
- 5: https://stackoverflow.com/questions/8614211/deleting-android-sms-programmatically
🏁 Script executed:
#!/bin/bash
set -eu
file="presentation/src/main/java/com/moez/QKSMS/adb/AdbMessagesProvider.kt"
printf '%s\n' "== target section =="
sed -n '210,265p' "$file"
printf '%s\n' "== fingerprint and delete call sites =="
rg -n -C 4 'fingerprint|contentId|ContentUris\.withAppendedId|contentResolver\.delete|Sms\.CONTENT_URI' "$file"
printf '%s\n' "== related helpers and tests =="
rg -n -C 3 'delete.*sms|sms.*delete|fingerprint|/sms/' presentation/src test .github 2>/dev/null || trueRepository: circlesac/quik
Length of output: 16422
🏁 Script executed:
#!/bin/bash
set -eu
file="presentation/src/main/java/com/moez/QKSMS/adb/AdbMessagesProvider.kt"
sed -n '210,265p' "$file"
rg -n -C 4 'fingerprint|contentId|ContentUris\.withAppendedId|contentResolver\.delete|Sms\.CONTENT_URI' "$file"
rg -n -C 3 'delete.*sms|sms.*delete|fingerprint|/sms/' presentation/src test .github 2>/dev/null || trueRepository: circlesac/quik
Length of output: 16329
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '== fingerprint implementation =='
cat -n presentation/src/main/java/com/moez/QKSMS/adb/SmsDeleteGuard.kt
printf '%s\n' '== SMS record and query projection =='
rg -n -C 6 'data class SmsFingerprintRecord|SMS_FINGERPRINT_COLUMNS|SmsFingerprintRecord\(' presentation/src/main/java/com/moez/QKSMS/adb
printf '%s\n' '== provider source references and repository metadata =='
rg -n 'TelephonyProvider|SmsProvider|compileSdk|minSdk|targetSdk' build.gradle* settings.gradle* gradle.properties presentation 2>/dev/null | head -80Repository: circlesac/quik
Length of output: 9270
🏁 Script executed:
#!/bin/bash
set -eu
cat -n presentation/src/main/java/com/moez/QKSMS/adb/SmsDeleteGuard.kt
rg -n -C 6 'data class SmsFingerprintRecord|SMS_FINGERPRINT_COLUMNS|SmsFingerprintRecord\(' presentation/src/main/java/com/moez/QKSMS/adb
rg -n 'TelephonyProvider|SmsProvider|compileSdk|minSdk|targetSdk' build.gradle* settings.gradle* gradle.properties presentation 2>/dev/null | head -80Repository: circlesac/quik
Length of output: 9142
Make the system delete conditional and handle partial commits.
SmsProviderignoresselectionandselectionArgsfor an ID URI. Delete throughSms.CONTENT_URIwith a parameterized selection for every raw value inSMS_FINGERPRINT_COLUMNS, including_id, and useIS NULLfor null values. Treat a delete count other than1as a mismatch.- The system delete commits before
messageRepo.deleteMessages. A crash or local-delete failure can leave the system SMS deleted while the local row remains. Add recovery or reconciliation handling.
🤖 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 `@presentation/src/main/java/com/moez/QKSMS/adb/AdbMessagesProvider.kt` around
lines 247 - 248, Update the deletion flow in AdbMessagesProvider around the
systemUri delete and messageRepo.deleteMessages: delete through Sms.CONTENT_URI
using a parameterized predicate covering every raw value in
SMS_FINGERPRINT_COLUMNS, including _id, with IS NULL for null fingerprints, and
treat any count other than 1 as a mismatch. Add recovery or reconciliation
handling so a committed system deletion followed by a crash or local-delete
failure cannot leave the local row undeleted.
| val systemUri = ContentUris.withAppendedId(Sms.CONTENT_URI, contentId) | ||
| if (appContext.contentResolver.delete(systemUri, null, null) != 1) return 0 | ||
|
|
||
| messageRepo.deleteMessages(listOf(local.id)) | ||
| conversationRepo.updateConversations(listOf(threadId)) | ||
|
|
||
| check(readSystemSms(contentId) == null && messageRepo.getMessage(local.id) == null) { | ||
| "SMS deletion could not be verified" |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Recover from a partial system and local delete.
The system provider delete completes before messageRepo.deleteMessages. A process death, database failure, or repository exception between these calls leaves the local SMS row present after the system SMS row is deleted.
Add durable recovery or reconciliation for this two-store operation. The verification check only detects the partial state after mutation.
🤖 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 `@presentation/src/main/java/com/moez/QKSMS/adb/AdbMessagesProvider.kt` around
lines 247 - 254, Update the deletion flow around the systemUri delete,
messageRepo.deleteMessages, and subsequent verification to durably reconcile
failures between the system provider and local repository. Ensure a process
death or repository exception after the system delete cannot leave the local row
permanently present, using the existing persistence/recovery mechanisms where
available; retain verification as a final consistency check rather than the sole
recovery mechanism.
1 participant
User description
Closes #6
Changes
Testing
./gradlew testDebugUnitTest— 4 tests, 0 failures./gradlew :presentation:assembleRelease/messages/{id}delete URI is rejectedNotes
CodeAnt-AI Description
Guard ADB SMS deletion against accidental or unauthorized removal
What Changed
/sms/{id}path.Impact
✅ Fewer accidental SMS deletions✅ Safer ADB message automation✅ Clear rejection of stale or unauthorized delete requests💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.
Summary by CodeRabbit
New Features
Bug Fixes
Tests