Skip to content

fix(daemon): exec children get a minimal environment; docs say what an agent token can do - #891

Draft
V3RON wants to merge 5 commits into
security/deps-auditfrom
security/exec-env
Draft

V3RON wants to merge 5 commits into
security/deps-auditfrom
security/exec-env

Conversation

@V3RON

@V3RON V3RON commented Oct 22, 2026

Copy link
Copy Markdown
Contributor

Summary

device.exec (HTTP POST /v1/leases/{id}/exec, also through a gateway) lets any agent token run host tools such as xcrun simctl spawn as the daemon's user. That part is by design. Two things were not:

  • The exec child got the daemon's whole environment (execEnv: process.env), so any secret in it (CI tokens, cloud keys, launchd-injected values) was readable by every lease holder.
  • The public docs never said that an agent token equals code execution on the worker.

Now the child starts from a short allowlist of the daemon's variables, plus the driver's own env, minus its unsetEnv (that layering is unchanged). The website says plainly what an agent token can do.

Changes

  • daemon: new src/daemon/exec-env.ts execBaseEnvironment(env) keeps PATH, HOME, TMPDIR, USER, LOGNAME, SHELL, LANG, every LC_*, TERM, DEVELOPER_DIR, ANDROID_HOME, ANDROID_SDK_ROOT, ANDROID_USER_HOME, ANDROID_AVD_HOME, ANDROID_PREFS_ROOT, ANDROID_SDK_HOME, JAVA_HOME, and drops everything else. main.ts passes execBaseEnvironment(process.env) as the dispatcher's execEnv. The daemon's own driver calls are unchanged.
  • comments: the unsetEnv comment in dispatcher.ts and the PassthroughCommand comment in src/core/driver.ts no longer say the base is the daemon's whole environment.
  • website: using-devices/remote-agents.mdx "Running commands with exec" gets a paragraph: commands run on the worker as the daemon's user, an agent token is like shell access, agents that must not see each other's work belong on separate workers, and the list of variables a command gets. gateway/access-control.mdx adds the simctl spawn / shell-access sentence and points to that list.
  • internal docs: src/daemon/AGENTS.md gets the exec-env.ts row and a Security model bullet (a few lines re-wrapped to stay within the 150-line budget). docs/internal/flows/device-exec.md step 5 and its known gap now describe the allowlist.

Tests

  • src/daemon/exec-env.test.ts (new): one table. Kept keys include LC_CTYPE and LC_ALL. Dropped keys include AWS_SECRET_ACCESS_KEY, GITHUB_TOKEN, NPM_TOKEN, SIMLOCK_HOME, ANDROID_SERIAL, SIMCTL_CHILD_*, and look-alikes LCX / XLC_CTYPE. Before the code existed it failed on the toEqual.
  • src/daemon/main.test.ts: a new startDaemon test plants AWS_SECRET_ACCESS_KEY and LC_CTYPE in process.env, leases through a fake driver, and runs device.exec with a scripted runner. The child's env has LC_CTYPE, the daemon's PATH, and the driver's SIMLOCK_SCOPED, and lacks the secret. Before the wiring it failed on not.toHaveProperty("AWS_SECRET_ACCESS_KEY").
  • The existing dispatcher test "spawns without a key of the daemon's environment that the command's unsetEnv names…" still proves the env / unsetEnv layering. The dispatcher did not change there.
  • Run: pnpm test:changed (green on the second run; the first run had one failure in src/cli/index.test.ts "fails at the 20 s deadline … (simlock daemon install and uninstall a LaunchAgent on macOS, and starts go through it #830)", a CLI service-start test with no path to this change, which passed alone twice and in the full second run), pnpm test:e2e e2e/device-exec.test.ts (3 passed; the real child still finds node on PATH), and the website build plus check-openapi (ok).

Assumptions

  • Assumption: the filter is applied at the composition root (main.ts), as the brief says. So the test that proves a planted secret is gone goes through startDaemon in main.test.ts, not through the Dispatcher alone. A test that only uses the dispatcher injects its own execEnv, so it cannot see the filter.
  • Assumption: no keys beyond the brief's list are needed. The drivers' passthrough commands name adb by absolute path and xcrun through PATH, and set ANDROID_ADB_SERVER_PORT themselves. simctl and devicectl need only DEVELOPER_DIR. Keys like ADB_VENDOR_KEYS and ANDROID_EMULATOR_HOME are read by the adb server or the emulator, which the daemon starts with its own environment, so they are left out.

Written by an agent.

@V3RON

V3RON commented Oct 22, 2026

Copy link
Copy Markdown
Contributor Author

Review (maintainer's agent, round 1)

No blocking findings.

  • execBaseEnvironment is a pure allowlist read once at start; the driver's env/unsetEnv layering in the dispatcher is untouched, so the physical-adb ANDROID_SERIAL unset still works. The kept set covers what xcrun, adb (absolute path, HOME for its keys) and devicectl need; nothing in the drivers' passthrough env builders depends on a dropped key.
  • The startDaemon test is the right seam given the filter lives in main.ts: it plants a secret and a locale in the real env, drives device.exec through the dispatcher, and checks the child's env. Env is restored in finally.
  • Docs: the remote-agents paragraph says plainly that an agent token is shell access to the worker and names the kept variables; the access-control caution adds simctl spawn. The flow doc and the daemon manual's security model are updated and the manual stays under its line limit.

Notes, no change needed:

  • The website lists the kept variables literally. Documentation rule 8 allows an end-user page to show a value, so this is fine, but a later change to KEPT_KEYS must update that sentence too.
  • SIMCTL_CHILD_* variables in the daemon's env no longer reach a simulator process through simctl spawn; that is the intended effect, just worth knowing if an operator relied on it.

@V3RON
V3RON changed the base branch from main to security/deps-audit October 23, 2026 06:19
@V3RON
V3RON added this pull request to stack #897 October 23, 2026 06:19

This branch has not been deployed

No deployments
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