Skip to content

fix: audit batch 1 — critical Android updater + security hardening + bug fixes#695

Merged
appergb merged 15 commits into
betafrom
fix/audit-batch1-security-bugs
Jun 17, 2026
Merged

fix: audit batch 1 — critical Android updater + security hardening + bug fixes#695
appergb merged 15 commits into
betafrom
fix/audit-batch1-security-bugs

Conversation

@appergb

@appergb appergb commented Jun 17, 2026

Copy link
Copy Markdown
Collaborator

User description

Summary

Fixes for batch 1 of the multi-agent audit of 1-app: 1 critical + 9 high + 4 medium/low-severity findings (security hardening, correctness bugs, one critical Android-updater bug). All behavior-preserving except the intentional security/correctness changes. Excludes the large structural refactors (coordinator/types/LocalAsr), tracked separately.

Critical

  • Android auto-update was fully broken: android/updater.rs read Build.SUPPORTED_ABIS as a method instead of a static field → device_arch() always errored. Fixed via get_static_field.

Security hardening

  • coding_agent detect/run-test: validate exe against an allowlist + restrict to the main window (settings caller).
  • less_computer_approve: scope to the Less Computer window (the approval surface); block the other windows.
  • Android update download: validate the URL against trusted prefixes before fetching.
  • Remote server: global PIN failure rate-limit; rejection-sampling for the PIN (remove modulo bias).
  • Prefer the direct GitHub update endpoint over the third-party mirror.
  • Sanitize the frontmost-app/window title before embedding it in the LLM system prompt.

Bug fixes

  • PreferencesStore::set: hold the lock across disk write + in-memory update (no more divergence).
  • QaShortcutPressed: use block_on (not spawn) to match other hotkey events and avoid a visibility race.
  • run_voice_agent_transcript: clear focus_target on completion (matches the normal dictation path).

Build hygiene

  • Update the stale windows-startup-lifecycle contract script and wire it into package.json.

Android (CI-verified-only)

  • REPLACE_OVERLAY/REFRESH_LAYOUT use startService (not startForegroundService, blocked on Android 12+).
  • 200 MB size guard on the APK download buffer.

Verification (macOS host)

  • cargo test --lib: 446 passed, 0 failed; cargo check clean; npx tsc --noEmit clean; npm run build clean; all 7 contract scripts pass.
  • Android- and Windows-cfg changes can't compile on macOS — relying on the Android cargo-check CI job and the Windows checks.

Notes for review

  • less_computer_approve is now callable only from the Less Computer window (was: any of 5 windows).
  • coding_agent exe allowlist covers macOS/Linux paths + bare claude; a Windows custom install path would need a %APPDATA%\npm prefix (bare claude still works).
  • The Android updater fix compiles-verifies via CI but should be confirmed on a real device.

🤖 Generated with Claude Code


PR Type

Bug fix, Enhancement


Description

  • Critical Android auto-update broken due to JNI call error

  • Security hardening: exe allowlist, window restrictions, URL validation, PIN rate-limit, prompt sanitization

  • Bug fixes: persistence lock race, hotkey spawn race, focus_target leak, Android startService

  • Build hygiene: updated contract script and wired into package.json, preferred GitHub endpoint


Diagram Walkthrough

flowchart LR
    A["Audit Batch 1 PR"] --> B["Critical Android Updater Bug Fix"]
    A --> C["Security Hardening (9 high/med findings)"]
    A --> D["Bug Fixes (4 medium/low)"]
    A --> E["Build Hygiene (2 items)"]
Loading

File Walkthrough

Relevant files
Bug fix
5 files
jni.rs
Use startService for overlay/layout intents                           
+15/-7   
updater.rs
Fix device_arch JNI call and add size guard                           
+17/-4   
dictation.rs
Clear focus_target on voice agent completion                         
+5/-1     
hotkey_loops.rs
Use block_on for QaShortcutPressed                                             
+3/-1     
persistence.rs
Hold lock across disk write and memory update                       
+1/-1     
Enhancement
5 files
commands.rs
Add exe validation allowlist and window scope                       
+62/-9   
qa.rs
Restrict approve to less-computer window                                 
+13/-1   
settings.rs
Validate update URL against trusted prefixes                         
+7/-0     
prompt_compose.rs
Sanitize frontmost app title before prompt                             
+15/-1   
mod.rs
Add global PIN rate-limit and rejection sampling                 
+40/-4   
Build configuration
1 files
package.json
Wire windows-startup-lifecycle script into npm                     
+2/-1     
Tests
1 files
windows-startup-lifecycle-contract.test.mjs
Update stale patterns for startup test                                     
+6/-8     
Configuration changes
1 files
tauri.conf.json
Prefer direct GitHub update endpoint                                         
+2/-2     

sim and others added 15 commits June 17, 2026 10:46
…tatic_method

android.os.Build.SUPPORTED_ABIS is a static final String[] field (API 21),
not a static method. call_static_method threw NoSuchMethodError at runtime,
causing device_arch() to always fail and the entire Android auto-update path
to be silently broken on every device.

CI-verified-only (Android cfg; macOS build still passes).
…::set

Two concurrent callers could interleave: A writes prefs_A to disk, B writes
prefs_B to disk, then they acquire the lock in reverse order. Result: disk
and in-memory state diverge, silently rolling back settings after restart.

Hold the lock across both atomic_write and the in-memory update so the two
operations form a single critical section.
All other hotkey events (Pressed, Released, Cancelled) use block_on so the
bridge thread serialises handlers. QaShortcutPressed used spawn, so the
bridge looped immediately and could start dictation concurrently while
handle_qa_hotkey_pressed was still running. Consistent block_on eliminates
the panel_visible race against the next Pressed event.
The normal dictation path clears focus_target when returning to Idle, but
run_voice_agent_transcript only set phase=Idle and left focus_target set.
A stale focus_target from a completed voice agent session could cause the
next dictation session to restore focus to a no-longer-valid window.
Three assertMatch patterns referenced the old 'checking' gate state and
pollHotkeyStatus function that were removed during App.tsx refactor. The
script exited with code 1 silently since it was not wired to any CI step.

Updated patterns to match the current inline while-loop implementation:
- checks for os === 'win' branch
- checks for status.state !== 'starting' -> setGate('ready') sequence
Also registered the script in package.json as check:windows-startup-lifecycle.
… only

coding_agent_detect and coding_agent_run_test previously accepted any non-
empty string as exe and were callable from any Tauri window. This allowed
any compromised webview to spawn arbitrary local binaries.

- validate_exe: allows bare 'claude' or absolute paths under known install
  dirs (~/.local/bin, /usr/local/bin, /opt/homebrew/bin, /usr/bin)
- ensure_main_window: both commands now require label == 'main'
- Window param added to both command signatures (Tauri injects it, no
  frontend change needed)
Any window with IPC access could call less_computer_approve with a token
received from the less-computer event stream and force approval of a
high-risk agent command. Guard restricts calls to the main window.

The less-computer window should relay approve/deny via a Tauri event to
the main window rather than calling this IPC directly.
…efore download

app_download_and_install_android_update accepted any URL from the frontend
and passed it directly to the HTTP client, enabling SSRF (e.g., requests to
169.254.169.254, localhost, or internal services). The signature check runs
only after the full download, so the malicious request still completed.

Validate url starts with the known GitHub direct or fastgit.cc mirror prefix
before making any network request.

CI-verified-only for the Android cfg path; the guard runs on all mobile
builds and the macOS build stays green.
Per-IP rate limit (5 fails/60s) can be bypassed by rotating source IPs on
a LAN (/16 gives ~327k attempts/minute across 65535 IPs).

Added a global (cross-IP) failure counter: 20 failures within 60s triggers
a Locked response for all callers until the window resets. A successful
authentication resets the global counter so legitimate users are not
permanently penalised after a brute-force attempt.
fastgit.cc is a third-party proxy not controlled by the project. With it as
the primary endpoint a compromised mirror could serve a tampered manifest
(e.g. pointing to an older version for a downgrade attack). Move the direct
GitHub URL to first position; fastgit.cc remains as a fallback for users
in regions with poor GitHub connectivity.
…rompt

GetWindowTextW (Windows) returns the raw window title string, which is fully
attacker-controlled. Embedding it unsanitized in the system prompt allows any
foreground application to inject LLM instructions (prompt injection).

Sanitize the front_app string before use in context_premise:
- strip newlines and CR (prevent multi-line instruction injection)
- strip Markdown/XML delimiters # < > (prevent structural injection)
- truncate to 100 chars (no legitimate app name is longer)
…dulo bias

u32::MAX (4294967295) is not a multiple of 1,000,000, so a simple modulo
gives ~295 low-valued PINs a ~0.03% higher probability. For a security PIN
the correct approach is to discard values in the biased tail region and
resample. The rejection probability is ~0.007% so the loop terminates
in one iteration with overwhelming probability.
… actions

On Android 12+ (API 31+), startForegroundService called from a backgrounded
app throws ForegroundServiceStartNotAllowedException. REPLACE_OVERLAY and
REFRESH_LAYOUT are sent to an already-running overlay service (same as
SHOW/HIDE), so they must use startService, not startForegroundService.

Extend the startService guard to include both action suffixes.

CI-verified-only (Android cfg; macOS build stays green).
The entire APK was accumulated in memory with no size cap. A compromised
manifest could point to an unlimited stream, exhausting the device heap.

Check content-length upfront and reject if > 200 MB; also enforce the cap
during streaming so chunked responses without Content-Length are also guarded.
200 MB is well above any real APK size (~50 MB current).

CI-verified-only (Android cfg; macOS build stays green).
…w, not main

The approval UI (LessComputerPanel) renders in the 'less-computer' window and
calls less_computer_approve directly, so a main-only guard would reject every
approval and break the feature. Allow the 'less-computer' window (the single
legitimate approval surface) and block main/capsule/qa/glow.
@github-actions

Copy link
Copy Markdown
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 No relevant tests
🔒 Security concerns

Path traversal in validate_exe:
The allowlist for claude executable can be bypassed by including .. in the absolute path (e.g. /usr/local/bin/../../tmp/evil). An attacker who can invoke coding_agent_detect or coding_agent_run_test from the main window could execute arbitrary executables. This is a security vulnerability. Fix by canonicalizing the path before the prefix check or by rejecting paths containing ...

⚡ Recommended focus areas for review

Security Bypass

validate_exe checks allowed paths via starts_with without normalizing the path first. An attacker can supply a path like /usr/local/bin/../../tmp/evil which passes the prefix check but resolves to an arbitrary executable outside the allowed directories. This undermines the allowlist intended to restrict claude to known installation paths. The function should canonicalize the path (e.g. via std::fs::canonicalize) before checking the prefix.

fn validate_exe(exe: &str) -> Result<(), String> {
    // 纯可执行文件名,不含任何路径分隔符 — 交给 PATH 解析即可
    if !exe.contains('/') && !exe.contains('\\') {
        if exe == "claude" {
            return Ok(());
        }
        return Err(format!("不允许的可执行文件名: {exe}(只接受 'claude' 或已知安装目录下的绝对路径)"));
    }
    // 绝对路径:必须规范化到已知 claude 安装目录之一
    let path = std::path::Path::new(exe);
    if !path.is_absolute() {
        return Err(format!("不允许的相对路径: {exe}"));
    }
    // 已知 claude 安装目录前缀
    let known_prefixes: &[&str] = &[
        "/usr/local/bin/",
        "/usr/bin/",
        "/opt/homebrew/bin/",
    ];
    // 也允许 ~/.local/bin/claude(用户目录绝对路径,动态计算)
    let home_prefix = std::env::var("HOME")
        .ok()
        .map(|h| format!("{h}/.local/bin/"));

    let exe_norm = exe.replace('\\', "/");
    let allowed = known_prefixes.iter().any(|p| exe_norm.starts_with(p))
        || home_prefix.as_deref().map_or(false, |p| exe_norm.starts_with(p));
    if allowed {
        Ok(())
    } else {
        Err(format!("不允许的 claude 路径: {exe}(必须位于已知安装目录)"))

@appergb appergb merged commit 121bb11 into beta Jun 17, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant