Skip to content

feat: let agents drive hosted macOS apps through agent-device macos-app leases - #2516

Merged
janicduplessis merged 5 commits into
mainfrom
feat/2477-macos-app-lease
Oct 5, 2026
Merged

janicduplessis merged 5 commits into
mainfrom
feat/2477-macos-app-lease

Conversation

@janicduplessis

@janicduplessis janicduplessis commented Oct 5, 2026 •

Copy link
Copy Markdown
Collaborator

No description provided.

@janicduplessis janicduplessis left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fresh review of the diff against origin/main, the issue, the tests and guidance, and the agent-device branch (macos-app-lease.ts, host-lease-http.ts, daemon-proxy.ts, http-server.ts). The wire assumptions match: /admin/leases/<id> PUT and DELETE with the daemon token from daemon.json, the PUT body fields, /health upstream.leaseBackends, the policy keys (leases.require, commands.allow), AGENT_DEVICE_DAEMON_POLICY, AGENT_DEVICE_MACOS_APP_BACKEND, the meta field names and the remote-config keys. CI is green. Findings below; I read the code and did not run the agent-device daemon.

1. Blocking: the pinned RPC still passes host paths and runtime hints to the daemon (packages/server/src/agent-device-driver.ts:520-541)

pinLease overwrites the owner fields in meta but spreads the rest of the client's params and meta through unchanged. It is a list of fields to override, not a list of fields to allow. agent-device's macos-app admission (ad/src/daemon/macos-app-lease.ts:81-150) checks only the command name, surface, platform, screenshotFullscreen, and the open target and flags.launchUrl. It does not check any host-filesystem or URL field. Reading the daemon code, a client holding a grant can send:

  • screenshot with positionals[0] or flags.out set to an absolute path. readScreenshotRequest (ad/src/daemon/screenshot-runtime.ts:313-329) expands it with meta.cwd and writes the capture there. That is a write to any file the hosting user can write, including via a screenshot step inside batch.
  • diff screenshot with --baseline and --out paths (diff is on the allow list).
  • open <bundleId> with runtime.launchUrl (the RPC runtime param, not flags.launchUrl). assertOpensLeasedApp checks only fields.launchUrl, and session-open-execution.ts:240 passes runtimeHints.launchUrl on to a follow-up URL open.
  • flags.launchConsole, meta.cwd, meta.developerDir, meta.lockPolicy, meta.lockPlatform, meta.installSource, meta.uploadedArtifactId, meta.clientArtifactPaths, meta.retainMaterializedPaths.

The issue says no unscoped access is handed out and the lease confines screenshots to the window. A client can still reach the host filesystem and open URLs on the host, so this contradicts the PR's stated guarantee. The pins every command... test only covers the owner fields, so nothing would catch it.

Suggested fix, in two places:

  • Stim: build meta from an allowlist (requestId, debug, includeCost, responseLevel, sessionExplicit) plus the owner fields, drop params.runtime, and for the command RPC reject path-bearing arguments (screenshot or diff positionals and flags.out/baseline/launchConsole). Either recurse into flags.batchSteps or remove batch from the policy allow list until agent-device covers it.
  • agent-device (the actual enforcement point): have assertMacOsAppLeaseAdmitsRequest refuse output and baseline paths and req.runtime.launchUrl. Worth raising on callstack/agent-device#3229.
  • Add a test with each of these fields in a client request.

2. Non-blocking: issue() puts the new lease before revoking the old one (agent-device-driver.ts:416-417)

putHostLease refuses a second lease for the same tenant, run and device key (DEVICE_IN_USE, ad/src/daemon/lease-registry.ts:101-120). HostedAgentHost.appRunning drops an existing entry before calling issue(), so the normal path is fine. A direct second issue() for the same session fails on the PUT. If the later revoke throws (daemon down), issue() rejects with the new lease already allocated and no renew timer, so it lingers up to 10 minutes. Revoke first, or make issue() idempotent per session.

3. Non-blocking: lease renewal failure is only logged (agent-device-driver.ts:418-424)

If a renewal PUT fails (daemon restarting, or the Mac slept past the 10-minute window so the lease expired), the grant stays live in HostedAgentHost while every client call fails with LEASE_NOT_FOUND until the next 2-minute tick, and forever if PUT keeps failing. The PUT on an expired id recreates it, so it self-heals, but a persistent failure should revoke the grant and report none with a notice. A test with FAKE_ADMIN=refuse after a successful issue would pin the behavior.

4. Non-blocking, unverified: daemon sessions across an app relaunch

The tenant is stim.<session>, which is stable for the Stim session. When the hosted app restarts (new pid, new lease id, same tenant), a daemon session opened under the old lease may still exist. DELETE /admin/leases/<id> only calls registry.releaseLease (host-lease-http.ts:76-84) and does not visibly close sessions bound to that lease. assertRequestSessionLeaseMatches could then refuse commands from the new lease against that old session. The PR's real-tool run covers open to close on one app, not stop and relaunch. Worth one manual check on the two-Mac run; if it sticks, derive the tenant from the lease id or app attempt.

5. Non-blocking: doctor finding text is now wrong (packages/stim-cli/src/device-host/agent-driver.ts:12-15)

It still says "agent-device starts only once it can lease a single macOS app" and the fix reads "once agent-device can lease one macOS app". The setting guide and website were updated to feature detection, but this finding was not.

6. Non-blocking: guidance does not tell an agent how to use the route (packages/stim-cli/src/guide/macos.ts:185-205, website/docs/macos.md:190-196)

  • The agent-device rows say they "work once the host's agent field names agent-device". With a macos-app lease the agent must first run agent-device open <host.bundleId> --remote-config <path>, and only the allow-listed commands work (open close snapshot diff wait find get is click fill press type focus scroll screenshot batch). Installs, other apps, --surface desktop and full-screen capture are refused. None of that is stated.
  • The client's own agent-device must also know leaseBackend: macos-app. An older one rejects the remote config on the enum, which gives an opaque error. This is not documented or checked.
  • STIM_AGENT_DEVICE_BIN is documented as "in stim-server's environment" without saying how to set it for the LaunchAgent (stim-server service install --env). The PR body does name that, so put it in the README or guide.
  • STIM_AGENT_DEVICE_BIN is a new machine-level override that is not in settings-registry.ts. CLAUDE.md says every setting and its environment override is defined there. Either register it or note why it is an env var only.

7. Test gaps for real failure modes

  • No test for path, runtime or meta fields in a client request (finding 1).
  • No test for a body over MAX_RPC_BYTES (expect 400) or a client that aborts mid-body.
  • No test that a failed renewal or a failed DELETE on revoke leaves a consistent state (findings 2 and 3).
  • The restart path (daemon exits, HostedAgentHost.lost, re-issue, old renew timers cleared) is covered only against a fake driver in agent-driver.test.ts. AgentDeviceDriver.stop() clearing leases and the timers on that path has no direct test.

Minor, no action needed: readBody keeps reading an oversized body to the end instead of destroying the request, and has no read timeout.

What I checked and found sound

  • Route allow list: /rpc POST, /health GET and /artifacts/... GET only; the .. and encoded-path cases are refused.
  • The client cannot name another tenant, lease or session through meta. The daemon honors meta.sessionIsolation and tenantId without an auth hook, and the pin overrides both. flags.tenant loses to meta.tenantId. params.session is prefixed with the tenant.
  • The client credential and headers are never forwarded.
  • Fail-closed behavior: an older agent-device rejects the unknown leases policy key, and a missing macos-app in /health tears the daemon and claim down.
  • parseHostedAgentGrant and the protocol schema agree, including the pid-pinned deviceKey pattern.

@janicduplessis

Copy link
Copy Markdown
Collaborator Author

Addressed in 2e5bc9f (Stim) and in the agent-device branch (callstack/agent-device#3236, 4e13a8d2a):

  1. Host paths and runtime hints, fixed on both sides.
    • Stim: pinLease now keeps only requestId, debug, includeCost, responseLevel, sessionExplicit and clientArtifactPaths from the client's meta, and it drops params.runtime. The test sends cwd, developerDir, installSource, lockPolicy and runtime.launchUrl and checks that none of them is forwarded.
    • agent-device: macos-app admission now refuses inputs that name a host path or a launch. It reads them from flags, input, runtime.launchUrl/bundleUrl and batch steps, and covers out, saveScript, baseline, launchConsole, launchUrl, artifactsDir and others. A screenshot path must be the client's own /tmp/agent-device-screenshot-<ts>-<id>.png temp file. diff is off the allow list in both places. Each rule is mutation-checked.
    • Re-verified on this Mac: open, snapshot, click, fill, screenshot and close still work through the forward, and close --save-script is refused.
  2. issue() now revokes the session's old lease before allocating the new one.
  3. Not changed: renewal is a PUT, and a PUT re-creates a missing lease, so a single failed renewal recovers on the next tick. An unreachable daemon is handled by the exit watcher and restart.
  4. Not changed here. Each lease gets a new tenant only per Stim session, and DELETE releases the lease only. This goes on the list for the two-Mac run, which is still blocked.
  5. Fixed the doctor note text.
  6. The guide and the website now say to start with agent-device open <host bundleId> --remote-config <path>, list what the lease allows, note that the client's agent-device needs the macos-app backend, and show stim-server service install --env STIM_AGENT_DEVICE_BIN=<path>. STIM_AGENT_DEVICE_BIN is a stim-server process environment variable, not a project or machine setting, the same as STIM_BIN, so it is not in the settings registry.
  7. Added a test for meta and runtime pinning. I did not add tests for an oversized body or a failed DELETE.

@janicduplessis janicduplessis left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review clear. Re-checked 2e5bc9f and the agent-device branch at 4e13a8d2a by reading the code; I did not run the daemon. Two non-blocking items below, one of them worth closing on the agent-device side before callstack/agent-device#3236 merges.

Earlier findings

  • 1 (host paths and runtime hints): resolved on both sides.
    • Stim: pinLease (packages/server/src/agent-device-driver.ts:526-558) now builds meta from requestId, debug, includeCost, responseLevel, sessionExplicit and clientArtifactPaths plus the owner fields, and drops params.runtime. The daemon's commandRpcParamsSchema accepts only token/session/command/positionals/input/flags/runtime/meta from the wire, so no internal or other field gets through.
    • agent-device: assertInvocation (ad/src/daemon/macos-app-lease.ts:149-163) refuses HOST_INPUT_KEYS on flags and input for every command, runtime.launchUrl and runtime.bundleUrl on the request, and any screenshot target that is not the client's /tmp/agent-device-screenshot-<ts>-<id>.png temp name. Each batch step re-enters admission with its own runtime, flags and input (session-batch.ts calls invoke), so a step carrying a host path or runtime.launchUrl is refused when it runs. diff is off the allow list in both places and the two lists match.
    • clientArtifactPaths is the one client-supplied path left in meta. The daemon only echoes it back as the artifact's localPath and the client downloads to it on its own machine (request-finalization.ts:122, daemon-artifacts.ts:404). The daemon never writes there, so it does not reach the host.
  • 2 (revoke after put): resolved. issue() now revokes first (agent-device-driver.ts:414). If the DELETE throws, issue() rejects with nothing allocated and the old lease expires on its TTL.
  • 3 (renewal failure only logged): the reply is reasonable. A failed PUT re-creates the lease on the next tick, and a dead daemon goes through the exit watcher.
  • 4 (sessions across a relaunch): deferred to the two-Mac run. Fine, but keep it on the list.
  • 5 (doctor text) and 6 (guide, website, STIM_AGENT_DEVICE_BIN): resolved. The STIM_AGENT_DEVICE_BIN answer holds; it is a stim-server process variable like STIM_BIN, not a project or machine setting.
  • 7 (tests): the meta and runtime pinning test covers cwd, developerDir, installSource, lockPolicy and runtime.launchUrl. The oversized-body, failed-DELETE and stop()-after-restart cases are still untested; not blocking.

Does dropping meta break a normal client flow? No. In the daemon, meta.cwd is used to resolve relative host paths, as the session scope root for non-tenant sessions (tenant isolation is forced here), and as the workspace of a device claim, which a macos-app open skips (session-open-execution.ts). The client sends cwd, lockPolicy and lockPlatform on every request; none matters for open close snapshot wait find get is click fill press type focus scroll screenshot batch. installSource and uploadedArtifactId only apply to install, which is refused. developerDir is never sent to a remote daemon. A remote screenshot already rewrites its target to the /tmp/agent-device-screenshot-<ts>-<id>.png form (daemon-artifacts.ts:249-262), so it matches the new admission regex, and clientArtifactPaths is kept so the artifact download lands at the client's chosen path. The platform: 'macos' in the CLI's remote config still passes assertAppSurface.

Non-blocking findings

  1. A client can omit platform and select a device. The macos-app admission checks flags.platform only when it is present (ad/src/daemon/macos-app-lease.ts:206-217), and assertSessionIsLeasedApp runs only when a session already exists. A first open <bundleId> carrying flags.udid, serial or device with no platform therefore passes admission and goes to resolveTargetDevice (session-device-resolution.ts:28-37), which can resolve an iOS simulator or Android emulator on the hosting Mac. After that the session is refused by the device check, and open of the macOS app's bundle id will normally fail on a device where it is not installed, so I rate the impact low. It could still boot a host simulator, and I did not verify that end to end. Fix in agent-device: refuse udid, serial, device, iosSimulatorDeviceSet, androidDeviceAllowlist and target under a macos-app lease, or require platform to be macos rather than allowing it to be absent, and re-check the resolved device on a new open. A cheap Stim-side mitigation is for pinLease to set flags.platform = 'macos' and delete those selectors from the top-level flags, but a batch step's own flags override the parent's, so the agent-device check is the one that covers it. Add a case to macos-app-lease.test.ts.
  2. The whole-batch check does not read step.runtime (ad/src/daemon/macos-app-lease.ts:129-136). A step with runtime.launchUrl is still refused when it runs, but the earlier steps have already executed, so the "no step runs before a later one is refused" comment is not fully true. Check step.runtime?.launchUrl and bundleUrl in the pre-pass.

@janicduplessis janicduplessis left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fresh review of 2e5bc9f (Stim) against the agent-device branch at 4e13a8d2a (callstack/agent-device#3236). I read both sides and ran the agent-device checkout (proxy with the exact policy, native backend, host-allocated leases through /admin/leases, then RPCs shaped like pinLease output) to check the items below. I did not re-report the already-fixed host path and runtime hint finding.

Confirmed issues

1. Blocking: a client can select host devices; pinLease does not pin platform (packages/server/src/agent-device-driver.ts:529-540)

The command branch pins meta but passes flags, input and positionals through unchanged. agent-device's macos-app admission only checks flags.platform when it is present (macos-app-lease.ts:196), and assertSessionIsLeasedApp runs only when a session already exists. A client that sends a device selector and no platform escapes the lease. Against the live daemon, with a valid lease and a pinned meta:

  • open <leased bundle id> with flags: { udid: <booted simulator UDID> } reached xcrun simctl launch on that real simulator ("Simulator device failed to launch ..."). The launch failed only because the bundle id is not installed there.
  • snapshot with no session and flags: { serial: 'emulator-9999' } returned "No Android, HarmonyOS device, or Vega VVD with serial emulator-9999", and with udid returned "No Apple device with UDID ...". So the lookup runs against the host's device inventory before anything lease-related refuses it. With a real serial, the allowed sessionless commands (snapshot, click, type, press, screenshot) would run against that device. emulator-5554 is a predictable serial, and adb on a hosting Mac can also see a plugged-in phone.
  • Same lease, flags: { platform: 'macos', serial|udid: ... } is refused ("--serial selects Android ... but this request selected --platform macos", "No Apple device with UDID"). A batch whose steps carry serial is refused the same way, because steps inherit the parent's platform.

Exploit: an agent holding a grant for one hosted app drives other simulators, emulators or an attached phone on the hosting Mac. That contradicts the "never unscoped access" guarantee in the issue and the README.

Fix (Stim): in the COMMAND_METHODS branch, set flags: { ...flags, platform: 'macos' } after the spread and drop udid, serial, device, target, iosSimulatorDeviceSet and androidDeviceAllowlist from flags and input. Forcing platform alone closed every case I tried, including the batch one. Also fix it in agent-device admission (refuse selectors under a macos-app lease, require platform === 'macos' instead of allowing it to be absent, and re-check the resolved device on a new open), since the existing allow-list stays a blacklist on the Stim side. Add a test with udid/serial and no platform in a client request. This is the same gap an earlier review listed as low impact and unverified; it is reachable.

2. Non-blocking, fix in this PR if cheap: params.command is not checked, so commands.allow does not bound what runs (agent-device-driver.ts:53-79, 529-540)

agent-device's policy treats protocol commands (lease_*, session_list, session_save_script, release_materialized_paths, human_control) as outside commands.allow (daemon-policy-file.ts PROTOCOL_COMMANDS), and lease_*, session_list and release_materialized_paths skip lease admission. Stim lets any params.command through the agent_device.command method. Verified on the live daemon: command: 'session_list' returned 200 and command: 'lease_release' released the lease ({"released":true}) with all pinning in place; install, devices and artifacts were correctly denied. Impact: the client can end its own lease (every call then fails with LEASE_NOT_FOUND until the next 2 minute renewal PUT re-creates it), and session_list returns sessionStateDir and runnerLogPath, which are host paths. The comment on POLICY ("only these commands run at all") is therefore not true for what Stim exposes.

Fix: in pinLease, refuse a command RPC whose params.command is not in POLICY.commands.allow, and do the same for each flags.batchSteps[].command. Then the README line "admits only the commands that drive one app" is true.

3. Non-blocking: revoke can lose a race with an in-flight renewal (agent-device-driver.ts:417-434)

revoke() clears the interval and sends DELETE, but a renewal PUT already in flight can reach the daemon after the DELETE. putHostLease creates a missing lease, so the revoked lease reappears for up to 10 minutes with nobody renewing it. A request that was authorized before the revoke and is still reading its body (relayPinned captured lease) can also complete against it. The window is narrow and the pid-pinned lease dies with the app, so I rate it low. Fix: track the in-flight renewal promise on the Lease and await it before sending DELETE, or send DELETE twice.

Minor notes

  • Every daemon error response carries logPath and diagnosticsRecord with absolute host paths (seen on every 401/500 above). The forward could strip data.logPath from JSON-RPC errors, or leave it and say so in the README.
  • POLICY has no capabilities.deny: ['device-shutdown']. With platform pinned this is probably moot for a macOS target, but the key is already supported by the same agent-device version and costs one line. I did not verify what close --shutdown does on a macOS target.
  • readDaemon parses daemon.json, which holds the daemon token. If the file is truncated, Node 22's SyntaxError message includes a snippet of the source, and HostedAgentHost.reason writes error.message to stderr. Unlikely and short, but it is the one path I found where token bytes could reach a log.
  • Test gaps for real failure modes: nothing sends udid/serial without platform (item 1) or a non-allowed params.command (item 2); no test of a body over MAX_RPC_BYTES returning 400; no test that a failed DELETE on revoke leaves a consistent state.

What I checked and found clean

  • Route confinement. DAEMON_PATHS is anchored and its segment class excludes %, \, ;, ?; .. is refused; query strings are forwarded but cannot change the route (the proxy routes on URL.pathname). /rpc/. and /health/. normalise to routes the proxy 404s; /artifacts/. normalises to the tenant inventory. POST is required for /rpc and GET for the other two, so HEAD and method override headers do nothing (override headers are not forwarded). server.ts only dispatches URLs matching ^/device-host/agent/<36 hex> with [/?] or end after it, so absolute-form and // URLs never reach forward, and a session id that is a prefix of another cannot match (the remainder must start with /).
  • Methods. Only agent_device.command and lease heartbeat/release (both spellings) pass; lease.allocate, install_from_source, release_materialized_paths and array bodies get 400. meta is an allow-list; the daemon's commandRpcParamsSchema limits top-level params, and batch steps accept only command, positionals, input, flags, runtime, so a step cannot carry its own meta or session. leaseScopeFromRequest prefers meta over flags for tenant, run, lease, provider, device key and client, and all are pinned. scopeRequestSession prefixes the session with the pinned tenant, so naming stim.other:default becomes stim.mine:stim.other:default. lease_allocate for macos-app is refused by the daemon even if named through the command method.
  • Tenant and artifacts. The forward sets x-agent-device-tenant from the lease and never forwards a client one; the proxy forwards that header and canReadArtifact filters by it, for both /artifacts/ inventory and /artifacts/<id>. Every Stim request carries a tenant, so the "no tenant means readable" branch is not reachable from a grant.
  • Token handling. The shared proxy token and the daemon admin token are different. Only the proxy token is used on forwarded requests, set from the host side; the client's Authorization, x-agent-device-token, cookies and params.token do not reach the daemon as authority (the proxy rewrites params.token to its own). The daemon token is read from daemon.json (mode 0600 in a 0700 dir), is used only on the loopback /admin/leases call, and appears in no argv, state file, response or the messages the client sees (non-AgentDriverUnavailable errors become a generic string). The proxy token is in the child's environment only.
  • Inertness and fail-closed. hosting.agentDriver none constructs no driver; with agent-device, a daemon without macos-app in /health upstream.leaseBackends is torn down with its claim removed, and grants are { driver: 'none' } with a notice. A missing or expired lease gives 503 in forward (no fall-through to an unscoped call); an expired lease at the daemon gives LEASE_NOT_FOUND.
  • Lifecycle. HostedAgentHost serialises appRunning, appStopped and lost, so concurrent issue/revoke for one session cannot interleave. Failed allocation leaves no lease or timer and stops the daemon if nothing else is live. Stop, app removal and device-host revocation all go through appStopped or drop; a crashed daemon clears the lease map in stop() and re-issues new grants with new tokens, and the old grant gets 503 meanwhile. STIM_AGENT_DEVICE_BIN pins the binary and fails with a clear message when unreadable.
  • Docs. The settings guide, website and README match the implementation for feature detection, STIM_AGENT_DEVICE_BIN, stim-server service install --env and the command list, apart from the "only the commands that drive one app" claim, which needs item 2.

Not tested: a real run through a physical host, and relaunch of the same app under the same tenant (deferred to the two-Mac run). The one real-device interaction I made was the failed open against a booted simulator in item 1; I ran nothing against the attached Android phone.

…pp leases

The agent-device adapter starts only when the daemon advertises the macos-app lease backend, allocates a host lease per hosted app process over agent-device's loopback admin route, renews and releases it, and pins every forwarded command to that lease. The client writes the lease into its remote config. STIM_AGENT_DEVICE_BIN names the agent-device binary. Refs #2477.
The forward keeps only reporting metadata from the client, drops runtime hints, revokes a session's old lease before allocating a new one, and drops diff from the allowed commands. The doctor note, guide and website say how an agent starts and what the lease allows.
@janicduplessis
janicduplessis force-pushed the feat/2477-macos-app-lease branch from 2e5bc9f to c700ce2 Compare October 5, 2026 17:56

@janicduplessis janicduplessis left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: clear on correctness and security. No blocking findings. Three non-blocking findings, two of them test gaps I confirmed by mutation.

Reviewed da8b2b7e8..c700ce291 in packages/server/src/agent-device-driver.ts against agent-device 5c958e109 (src/daemon/macos-app-lease.ts, packages/command-registry/src/batch.ts, src/daemon/server/http-server.ts). pnpm test packages/server/__tests__/agent-device-driver.test.ts passes (18/18) and agent-device's macos-app-lease.test.ts passes (18/18). I ran the mutations on a scratch copy; the worktree is untouched.

The three earlier findings

  1. Selectors and platform. Closed. pinLease strips device, udid, serial, target, iosSimulatorDeviceSet and androidDeviceAllowlist from top-level flags, top-level input, each step's flags and each step's input. It forces platform: 'macos' on top-level and step flags. That list is the full set the daemon uses to pick a device (src/daemon/session-selector.ts reads exactly those six, plus platform). The daemon builds its request from params.flags, so input.platform selects nothing; the lease check merges {...input, ...flags} and flags wins. flags, input and steps that are not objects are handled: a non-object flags or input becomes {} or is dropped, and a non-object step returns 400.
  2. Command allow-list. Closed. isAllowedCommand is an exact, case-sensitive match against POLICY.commands.allow, which is also the list given to the daemon, so the two cannot drift.
    • Snapshot and snapshot are refused. agent-device's normalizeBatchCommandName is only trim().toLowerCase(), so there are no aliases. Exact matching is stricter than the daemon, never looser.
    • A step command of ['snapshot'] or undefined is refused, and so is a missing top-level command.
    • Nested batch passes Stim's list, and the daemon refuses it (assertBatchRuntimeCommandAllowed).
    • lease_heartbeat and lease_release use their own RPC methods (LEASE_METHODS) and never go through command, so the allow-list does not touch them. The client builds them that way in buildHttpRpcPayload. install_from_source and release_materialized_paths stay refused.
  3. Revoke racing a renewal. Closed. lease.renewing holds the latest renewal promise. The promise ends in .catch, so awaiting it never rejects and cannot leave an unhandled rejection. It also never waits on revoke, so it cannot deadlock. adminRequest has a 10s socket timeout, so the await is bounded. The renew interval is 120s, so two renewals do not overlap and tracking only the latest is enough. this.running is read after the await, which is correct if the daemon stops meanwhile. stop() clears the intervals and the lease map without awaiting. A renewal still in flight then fails against the stopping daemon and is caught and logged. That path is acceptable.

I also checked {"__proto__": {...}} keys. JSON.parse gives an own data property, and the spread, Object.fromEntries and rest destructuring all keep it as an own property. It does not reach any prototype and flags.udid stays undefined. I found no Object.assign or target[k] = v copy of request flags in agent-device (not an exhaustive audit). Not exploitable as far as I can tell. See finding 3.

Findings (all non-blocking)

  1. The renewal-await has no test that would fail without it. agent-device-driver.test.ts:181-215 (the revoke test with leaseRenewMs: 50) passes 3/3 with await lease.renewing; removed (agent-device-driver.ts:439). It only fails if the interval happens to fire within a few milliseconds of revoke. Failing input: make the fake admin PUT slow (say 100ms), call revoke while the PUT is in flight, and assert that the last admin call is the DELETE. Without the await the PUT lands after it.

  2. The input and step-flag selector stripping is not asserted. agent-device-driver.test.ts:404-423 uses toMatchObject, which accepts extra keys. Three mutants all pass 3/3:

    • top-level input not stripped (input: { udid: 'SIM-3', text: 'hi' } still matches input: { text: 'hi' });
    • step input not stripped (the expectation input: {} matches any object);
    • step flags not stripped (flags: { udid: 'SIM-2', platform: 'android' } still matches { platform: 'macos' }).

    The top-level flags check does fail on mutation, because of the Object.keys(...).toEqual(['batchSteps','platform','surface']) assertion. Add the same exact-equality check for input, step flags and step input (toEqual, not toMatchObject). The other mutants fail the test as intended: top-level command check removed, step check removed, platform removed, pinStep skipped, top-level flags not stripped, step platform removed.

  3. Hardening, optional. batchSteps that is present but not an array (for example a string) goes through unvalidated at agent-device-driver.ts:539-544. The daemon rejects it (validateAndNormalizeBatchSteps requires an array), so it is not a hole today. The selector strip is a denylist, so any new daemon selector key would pass until it is added. Refusing a non-array batchSteps and dropping __proto__ keys would make pinLease independent of the daemon. The refusal is also a bare 400 text/plain ("Unsupported request."), not a JSON-RPC error, so the agent-device client will show an opaque transport error. A JSON-RPC error naming the refused command would be easier to debug.

I did not run any daemon or touch a simulator or device.

@janicduplessis
janicduplessis marked this pull request as ready for review October 5, 2026 18:10
@janicduplessis
janicduplessis merged commit 38da60b into main Oct 5, 2026
13 checks passed
@janicduplessis
janicduplessis deleted the feat/2477-macos-app-lease branch October 5, 2026 18:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant