fix: resolve all P1/P2 security and crash issues (#6–#33)#84
Draft
Copilot wants to merge 2 commits into
Draft
Conversation
- #33/#19: Shell-quote all args passed to Android adb shell (device.rs) - #32: Cap HTTP scrape body at 10 MB to prevent OOM (system.rs) - #31: Replace get_unchecked() with safe CFArray.get() (ax.rs) - #30: Return error when HOME is unset instead of resolving ~/x as /x - #29: Use unwrap_or_else(|e| e.into_inner()) for all Mutex locks - #28: Escape \n and \r in adb input text to prevent injection - #27: Validate press_key against KEYCODE_* allowlist - #26: Validate launch_app package_name against [a-zA-Z0-9._] regex - #25: Validate tabId with /^[\w-]+$/ before using in file paths - #24: Restrict coworkerFetchUrl local file reads to home directory - #23: Configure marked renderer to strip raw HTML (XSS prevention) - #18: Replace unbounded Maps with BoundedMap(100) in gateway adapter - #17: Guard proc.stdout null assertion with explicit null check - #16: Fix NaN score when denom is 0.0 or non-finite in NCC - #15: Map unknown message roles to 'user' for Anthropic API - #14: Cap setTimeout delay at 2^31-1 ms to prevent overflow wraparound - #13: Use tasklist on Windows instead of Unix-only ps flags - #12: Canonicalize search root path to prevent path traversal in find - #11: Check cp exit status and return error on failure - #10: Exit on thread spawn failure instead of silently ignoring - #9: Use -KILL/-TERM flag for pkill (not -s which is session filter) - #8: Use flock(LOCK_EX) to eliminate TOCTOU race in singleton.rs - #7: Generate random bearer token; enforce Authorization on HTTP MCP - #6: Guard search_region against u32 underflow before subtraction Agent-Logs-Url: https://github.com/unbug/tday/sessions/8970ef9a-ecea-4326-845b-afe8d08d83ed Co-authored-by: unbug <799578+unbug@users.noreply.github.com>
Agent-Logs-Url: https://github.com/unbug/tday/sessions/8970ef9a-ecea-4326-845b-afe8d08d83ed Co-authored-by: unbug <799578+unbug@users.noreply.github.com>
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
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.
Summary
This PR fixes all 25 P1/P2 issues found in the issue tracker, covering shell injection, path traversal, XSS, authentication, memory safety, crash/panic scenarios, and logic bugs.
Changes
🔴 P1 Security
android/device.rshandlers/system.rscontent-lengthearlyplatform/macos/ax.rsget_unchecked()on CFArray with safe.get()to prevent UB on inconsistent AX API lengthsandroid/input.rs\nand\rin adbinput text— they were treated as command terminators by the device shellandroid/input.rspress_keyargument againstKEYCODE_[A-Z0-9_]+allowlistandroid/navigation.rslaunch_apppackage name against[a-zA-Z0-9._]to prevent shell injectionmain/index.tstabIdwith/^[\w-]+$/before using it in file paths; validation moved to top of handlermain/index.tscoworkerFetchUrllocal file reads to within the user's home directorySettings/shared.tsxmarkedrenderer to strip raw HTML blocks, preventing XSS viadangerouslySetInnerHTMLmain.rs+nativecore-service.ts+computer-use.tsAuthorization: Bearer <token>on every HTTP MCP request; thread token through all MCP config injection paths🔴 P1 Crash / Safety
handlers/system.rsHOMEis unset instead of silently resolving~/xas/xhover_tracker.rs,screen_recorder.rs.unwrap()onMutex::lock()with.unwrap_or_else(|e| e.into_inner())to avoid cascading panics on mutex poisonparent_watch.rs🔴 P1 Correctness
handlers/system.rs-KILL/-TERMflag forpkill;-sis a session-ID filter, not a signal selectorsingleton.rsflock(LOCK_EX)around the read-kill-write critical section to eliminate the TOCTOU race🟡 P2
gateway/adapter.tsMapwithBoundedMap(100)— evicts least-recently-inserted entries above limitnativecore-service.tsproc.stdout!non-null assertion with explicit null guard and early rejectionfind_image.rs0.0when denominator is zero or non-finite in NCC, rather than propagating NaNgateway/bridge/input.ts'assistant'to'user'to prevent Anthropic API 400 errorscron.tssetTimeoutdelay to2^31 - 1ms (≈24.8 days); reschedule without firing when cappedhandlers/system.rstasklist /FO CSV /NHon Windows instead of Unix-onlyps -axohandlers/system.rsstd::fs::canonicalizeto preventfind ../../etctraversalhandlers/system.rscpexit status and return an error when it failsfind_image.rssearch_regionboundaries againstu32underflow before performingss_w - r.xTest Results
test_execute_command_osascript) is pre-existing (macOS-only test, Linux CI environment lacksosascript)injectOpencodeMcpUrl) is pre-existing (XDG_CONFIG_HOMEset in CI overrides the test'shomeparameter)