Skip to content

Typed device-shell execution boundary (ShellSafe) — mechanism + complete migration, one PR #2026

Description

@thymikee

Status (2026-09-14): rewritten. The original text pointed at src/platforms/* and src/utils/exec.ts, which no longer exist, and at a WIP branch (claude/shell-safe-boundary-wip) that is 340 commits behind and touches deleted paths. It also planned an AST backstop gate, which conflicts with the "fix-shaped, not detector-shaped" direction of #2531. Neither is a starting point. The design below replaces both; the WIP branch can be deleted.

Threat

adb shell, adb exec-out <cmd>, and hdc shell join their argv after the subcommand into one string that the device's sh evaluates (adb deliberately does not escape: "just like ssh(1)"). The host never runs a shell (runCmd spawns with shell: false), so the surface is purely device-side: any unquoted dynamic element in a ['shell', …] argv is a command injection.

State on main at ad9b906140:

  • 165 raw ['shell'|'exec-out', …] argv builds across 57 non-test files, with no funnel and no guard.
  • Android quoting is ad hoc: shellQuoteIfNeeded is hand-applied in five files. The audited sites (input text, cmd clipboard set text, am start targets and deep links, launch args) are quoted.
  • HarmonyOS has 35 hdc shell sites and zero quoting. uitest uiInput text <text> passes user text unquoted, and bundle ids, pids, and paths are inlined everywhere. This is the one live gap.

Design: make the raw argv unrepresentable

One kernel module, @agent-device/kernel/device-shell (kernel because platform-android, platform-harmonyos, and provider-limrun all depend on it and nothing else in common):

  • deviceShellArgv(subcommand, words, prefix?) is the only producer of a device-shell argv. It passes every string word through shellQuoteIfNeeded (identity on [A-Za-z0-9_@%+=:,./-]+, so a migrated site is byte-identical to today unless the word was an injection vector), stringifies numbers, and passes a shellFragment(script) verbatim. prefix carries transport options (['-s', serial], ['-t', target]).
  • shellFragment(script) is the one escape hatch for genuine shell syntax (pipes, & background, redirects). Every interpolated value inside a fragment is shellQuote-d at the call site. It is greppable; that is its whole enforcement.
  • assertDeviceShellArgv(args, boundary) refuses an argv containing shell/exec-out that deviceShellArgv did not build. It checks the value, so a variable-built or indirect argv fails the same as a literal one. No static gate, no allowlist.
  • shellQuote/shellQuoteIfNeeded move here from host-kit/command (one implementation, as refactor(shell-quote): one implementation, reached through the runner host port #2595 established; host-side hint text keeps importing them from the new path).

Guarded boundaries: runAndroidAdb; every executor handed out by resolveAndroidAdbExecutor / resolveAndroidAdbProvider / createDeviceAdbExecutor / createLocalAndroidAdbProvider; runAndroidHostAdb; runHarmonyHdc; and the generic host command port (host.commands.run) when the executable is adb or hdc. Derived arrays inside the provider scope (serial stripping, prefixing) are built after the check and are not re-checked, so the limrun executor needs no guard of its own.

Funnels: runAndroidShell / runAndroidExecOut (device-scoped, next to runAndroidAdb), runAdbShell / runAdbExecOut (for the injected-executor …WithAdb helpers, no DeviceInfo needed), runHarmonyShell. Host-port callers mint the argv with deviceShellArgv directly.

Call-site rule: drop the 'shell' element, drop any hand-applied shellQuoteIfNeeded, split whole-command-line strings into words, and wrap only real shell syntax in shellFragment.

Behavior changes to expect

  • A word containing a space or metacharacter that was previously passed unquoted now arrives single-quoted. On Android this only affects values that were injection vectors; on HarmonyOS it fixes uiInput text.
  • An empty word now renders as '' (an explicit empty argument) instead of vanishing from the joined command line.
  • sh -c '<script>' name path sites now hand sh the script as one argument, which is what -c requires.

Delivery

One PR: mechanism, guards, all call sites, and the tests that assert argv shape (most stay identical because safe words render identically). No compatibility route is kept; a raw ['shell', …] reaching any boundary throws INVALID_ARGS with reason unguarded-device-shell-argv.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions