Open
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthrough
Changes
Sequence Diagram(s)sequenceDiagram
participant Test as Test Code
participant Enter as enterText()
participant Shell as Shell path
participant Paste as pasteText()
participant Main as Main Thread
participant Clipboard as Android Clipboard
participant Device as UiDevice
Test->>Enter: enterText(text)
Enter->>Enter: validate chars (ASCII & no metachars)
alt ASCII-safe
Enter->>Shell: escape spaces -> "input text ..."<br/>executeShellCommand(...)
Shell->>Device: input text
else non-ASCII / unsafe
Enter->>Paste: pasteText(text)
Paste->>Main: runOnUiThread { setPrimaryClip(text) }
Main->>Clipboard: set clip content (rgba(0,0,0,0.5))
Paste->>Paste: wait up to 2s for clip update
Paste->>Device: executeShellCommand("input keyevent 279")
Paste->>Main: runOnUiThread { restore previous clip or clear }
Main->>Clipboard: restore clip (rgba(0,0,0,0.5))
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@drivers/android/app/src/androidTest/java/app/finalrun/android/action/DeviceActions.kt`:
- Around line 244-257: The pasteText function writes user text to the global
clipboard and never restores/clears it, leaking sensitive non-ASCII input;
modify pasteText to capture the current clipboard state (e.g., the existing
android.content.ClipData or null) before calling cm.setPrimaryClip, then after
triggering the paste (uiDevice.executeShellCommand("input keyevent 279"))
restore the original clipboard on the main thread (or clear it if it was null)
using Handler(Looper.getMainLooper()).post, and ensure any exceptions still
countDown the CountDownLatch so the restore always runs; reference pasteText, cm
(ClipboardManager), setPrimaryClip, and uiDevice.executeShellCommand when making
the changes.
- Around line 255-256: The code calls latch.await(2,
java.util.concurrent.TimeUnit.SECONDS) and ignores its boolean result, then
unconditionally executes uiDevice.executeShellCommand("input keyevent 279");
update the logic around latch.await in the method that performs the paste action
(reference latch.await and uiDevice.executeShellCommand) to capture the
await(boolean) return value and only call uiDevice.executeShellCommand("input
keyevent 279") when await returned true; otherwise handle the timeout path
(e.g., throw an informative exception or log an error and skip the paste) so
stale clipboard data is not used.
- Around line 236-238: The current fast-path uses
uiDevice.executeShellCommand("input text $escaped") with client-provided text
(flowing from DriverServiceImpl.enterText -> request.value) but only checks
ASCII range; update DeviceActions.kt so the executeShellCommand path is only
used when the string contains no shell metacharacters (e.g., & | ; < > $ ` \ " '
) — otherwise call pasteText(text) (or the existing safe pasteText helper) to
avoid shell injection. Concretely, modify the condition around
uiDevice.executeShellCommand to verify text.none { it in
setOf('&','|',';','<','>','$','`','\\','"','\'') } (or simply always call
pasteText for untrusted input) so executeShellCommand is only invoked for fully
safe payloads; reference uiDevice.executeShellCommand, pasteText, and
DriverServiceImpl.enterText to locate the code.
🪄 Autofix (Beta)
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0bb4c9d2-572d-4dbf-b8ff-7c02d7b4e2c0
📒 Files selected for processing (1)
drivers/android/app/src/androidTest/java/app/finalrun/android/action/DeviceActions.kt
drivers/android/app/src/androidTest/java/app/finalrun/android/action/DeviceActions.kt
Outdated
Show resolved
Hide resolved
drivers/android/app/src/androidTest/java/app/finalrun/android/action/DeviceActions.kt
Show resolved
Hide resolved
drivers/android/app/src/androidTest/java/app/finalrun/android/action/DeviceActions.kt
Outdated
Show resolved
Hide resolved
… leaks - Reject shell metacharacters (& | ; < > $ ` \ " ' etc.) from the executeShellCommand fast-path in enterText, falling back to the safe clipboard-based pasteText for any untrusted input - Save and restore the previous clipboard content around pasteText so sensitive text (e.g. passwords) is not left on the global clipboard - Check the CountDownLatch await return value before issuing the paste keyevent to avoid pasting stale data on timeout Made-with: Cursor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Unicode text input support for Android driver
Problem
enterTextpassed all text directly toadb shell input text, which only supports ASCII characters. Any non-ASCII character (e.g.ñ,é, CJK characters, emoji) caused aNullPointerExceptionin Android'sInputShellCommand.sendTextbecauseKeyCharacterMap.getEvents()returnsnullfor characters outside the ASCII key map.Example:
enterText("Español", ...)would crash with:java.lang.NullPointerException: Attempt to get length of null array at com.android.server.input.InputShellCommand.sendText(InputShellCommand.java:319)
Solution
enterTextnow branches based on character content:input textshell command.pasteTexthelper that:ClipboardManageron the main thread.KEYCODE_PASTE(keyevent 279) to insert the text.The clipboard API has no character restrictions, so this works for any Unicode string.
Changed files
drivers/android/app/src/androidTest/java/app/finalrun/android/action/DeviceActions.ktSummary by CodeRabbit