CLI target management, Docker isolation, config hardening, and App permission docs

[bfulton]

Related work on the same runner-and-policy surface, stacked on one branch.

• CLI target management — localmost targets, plus two runtime bugs it exposed
• Docker isolation (stage 1) — localmost stops handing jobs the daemon socket and serves a filtering one, checked against the repository's policy
• Config data-loss hardening — a bad config read can no longer become a permanent overwrite with defaults
• Runtime fixes — per-workflow policy binds to the real workflow name; job notifications name the repository
• GitHub App permission docs — Contents: Read, and a check so the docs stop drifting

Design: docs/superpowers/specs/2026-09-05-docker-isolation-design.md. Plan: docs/superpowers/plans/2026-09-05-docker-isolation-stage1.md. Supersedes docs/roadmap/docker-access.md, which describes the docker: socket level this branch replaces.

---

CLI target management

Registering runners for a repo was UI-only, and hand-editing config.yaml doesn't work — adding a target also has to register runner proxies with GitHub and store their credentials.

localmost targets                        # list (default)
localmost targets add <owner>/<repo>     # register runners for a repo
localmost targets add <owner> --org      # register runners for an org
localmost targets remove <ref> [--yes]
localmost targets enable|disable <ref>

<ref> is owner/repo, a bare owner for org targets, or the 8-char id; every subcommand takes --json. remove unregisters runners from GitHub and isn't locally reversible, so it confirms interactively, takes --yes, and refuses to run unconfirmed outside a terminal.

Supporting changes: the wire protocol moves to src/shared/cli-protocol.ts so the two hand-mirrored copies can't drift; sendCommand gets a per-command timeout (registering four proxies takes ~30s, well past the old hardcoded 5s); and the post-add broker wiring moves into TargetManager so the CLI and the UI share one path.

Two bugs this surfaced, both pre-existing and both affecting the Targets page identically — the CLI just made them reachable without a renderer in the loop:

• Targets added at runtime were lost on quit. targets is a persisted store key; the store loads it at boot and flushes on quit, overwriting config.yaml with its stale snapshot. The UI survived only because the renderer re-lists targets after an add, which happens to re-sync the store.
• Targets added at runtime never got a heartbeat. Heartbeat targets were computed once at auto-start, so LOCALMOST_HEARTBEAT was never written for a later target and workflows gating on it read the machine as offline.

Docker isolation (stage 1)

docker: socket handed the job the operator's real daemon socket, which is a sandbox escape by construction: a container can bind-mount any host path Docker Desktop shares, its egress bypasses the network policy, and credentials worked by letting the job read ~/.docker/config.json.

localmost now serves each worker its own unix socket at the root of the worker's ephemeral sandbox directory and points DOCKER_HOST at it. A filtering proxy behind that socket parses every Docker Engine API request, checks it against the repository's approved policy, and forwards only what passes to a backend daemon. Because localmost owns the socket: host reach is decided per request rather than granted wholesale, policy can vary per workflow, credentials are attached by the proxy and never enter the sandbox, and the daemon behind the socket is a swappable DockerBackend (stage 2's managed VM is seamed, not built).

shared:
  docker:
    pull:
      registries: [docker.io]
    run:
      images: ["alpine:3"]
      mounts:
        - path: ./
          mode: ro
      network: bridge

Actions are CLI-shaped (pull, run, build) with the conditions the proxy checks: registries, images, mounts (resolved through symlinks and required to stay inside the job workspace, so ../ and absolute host paths fail structurally), network, context. Anything not listed is denied. There is no key that spells PidMode, Devices, SecurityOpt, NetworkMode: host, or a daemon-socket mount — a capability that cannot be named cannot be requested. privileged is in the grammar but rejected until a managed-VM backend exists. /_ping, /version, /info and reads about the job's own containers are an always-on baseline. The filter fails closed on unknown endpoints, unknown API versions and unparseable bodies, and every denial logs the exact policy line that would permit it, which localmost test --updaterc writes back.

Breaking policy change: docker: socket | contexts | credentials is now a validation error naming the actions that replace it. This repo's own .localmostrc is migrated. The sandbox profile no longer punches a hole for the daemon socket; ~/.docker stays denied in full.

Verified at three levels: the SECURITY.md escapes are executable tests against the evaluator (host bind mount, daemon-socket mount, --privileged/--pid=host/--network=host/--device, ../ and symlink traversal, undeclared image/registry/mount); the proxy is tested over a real unix socket; and an end-to-end test drives the real docker CLI and a real daemon through the filter. That e2e runs on every CI leg — serving the socket itself where the native daemon is reachable, and driving the socket the runner serves inside a localmost job — and fails, never skips, when Docker is absent. docker-localmost likewise now runs on every push, queueing for a runner rather than skipping.

Config data-loss hardening

A stale localmost.app.bak launched against the shared ~/.localmost and left config.yaml empty; the next real launch loaded the empty file and cleanly re-saved it, erasing every target and setting. The root cause was that the persistence layer treated "read nothing" as success and then overwrote disk with factory defaults. Three defenses, so a transient bad read or an older bundle can no longer destroy a good config:

• persist: if the config file exists but parses to nothing, or was written by a newer build, refuse to persist rather than stamping defaults over it.
• config: saveConfig is atomic (temp file + rename), so a reader or second instance never sees a half-written file.
• both: every save stamps a configVersion; neither writer will overwrite a file carrying a higher one, so a stale or older app bundle cannot downgrade it.

Runtime fixes

• Per-workflow policy bound to the wrong name. workflows.<name> keys match the workflow filename, but the runtime looked them up by the job name scraped from runner stdout, so per-workflow sections fired only by coincidence. The broker already receives github.workflow; it is now threaded through to policy binding. This was a live defect in per-workflow network policy and a blocker for workflows.<name>.docker.
• Job notifications said "on unknown". The job-started handler looked up the job's target context by the runner's full name, which is never a storage key; it now uses the numeric instance id where spawnWorkerForJob stores it.

GitHub App permission docs

The App now requests Contents: Read, needed to fetch .localmostrc from private repos — without it the API answers Resource not accessible by integration and the job is refused rather than run under a weaker sandbox.

The permission set was written out in three places nothing kept in sync: the App description text, the README table, and the SECURITY.md list. They're now checked against one declared list in src/shared/github-app-permissions.ts, in both directions. That check found existing drift on its first run — neither README nor SECURITY.md documented Variables: Read & Write, at either scope, though the App requests it and the heartbeat depends on it.

---

Copilot's review on the CLI portion is addressed and its threads resolved.

🤖 Generated with Claude Code

https://claude.ai/code/session_01XVXbqWE6zBi1HV7b4uobKh

[Copilot]

🟡 Changes recommended

A few correctness/robustness issues in the new CLI target command paths (notably input validation over the CLI socket and heartbeat clearing semantics) should be addressed before merging.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds first-class target management to the localmost CLI, enabling users to list/add/remove/enable/disable repo/org targets via the running app’s CLI socket, while consolidating target attach/detach + heartbeat behavior so CLI and UI share the same runtime codepaths.

Changes:

• Introduces a shared CLI wire protocol (CliRequest/CliResponse) and extends it with target-management commands and arguments.
• Adds localmost targets subcommands (with --json support) and implements corresponding main-process handlers in CliServer.
• Refactors target add/remove to attach/detach broker proxy + update heartbeat immediately, and fixes runtime-added targets persistence/heartbeat behavior.

File summaries

File	Description
src/shared/cli-protocol.ts	New shared CLI socket protocol definitions (commands, args, responses, target summaries).
src/main/target-manager.ts	Adds ref resolution + shared add/attach and remove/detach paths; syncs target writes back into persisted store.
src/main/target-manager.test.ts	Expands tests for store sync, ref resolution, attach/detach, and heartbeat hooks.
src/main/ipc-handlers/targets.ts	Switches IPC add/remove to the new TargetManager shared attach/detach methods.
src/main/index.ts	Uses toHeartbeatTarget helper when seeding heartbeat targets at startup.
src/main/heartbeat-manager.ts	Adds dynamic add/remove target support, immediate heartbeat writes, and fixes shutdown timeout timer leak.
src/main/heartbeat-manager.test.ts	New tests for dynamic target add/remove behavior and heartbeat clearing.
src/main/cli-server.ts	Migrates request/response types to shared protocol and adds targets-list/add/remove/update commands.
src/main/cli-server.test.ts	Adds CLI server tests for target commands and summaries (including runner counts).
src/cli/targets.ts	New CLI implementation for localmost targets UX, formatting, confirmation flow, and JSON output.
src/cli/targets.test.ts	New unit tests covering parsing, formatting, ref resolution, prompts, and JSON behavior.
src/cli/index.ts	Adds targets command routing; moves to request-shaped socket messages and per-command timeouts.
README.md	Documents localmost targets usage and behavioral notes.
CHANGELOG.md	Adds release notes entry for CLI target management.

Review details

• Files reviewed: 14/14 changed files
• Comments generated: 6
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🔵 Needs a closer look

Heartbeat add/remove introduces new awaited GitHub variable writes without timeouts, which can block target add/remove indefinitely on stalled network calls.

Review details

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

src/main/heartbeat-manager.ts:103

• addTarget awaits a GitHub variable write with no timeout. Since the underlying GitHub API client uses fetch without a request timeout, a hung network call can block target adds (and therefore localmost targets add) indefinitely until the CLI socket timeout fires, leaving the user with a perceived failure even if the target was added.

This issue also appears on line 122 of the same file.

src/main/heartbeat-manager.ts:122

• removeTarget awaits clearHeartbeat without any timeout. Because clearHeartbeat ultimately calls the GitHub API via fetch (no request timeout), a stalled request can block target removal and runner unregistration indefinitely. This contradicts the docstring intent that clearing must not block target removal.

    await this.clearHeartbeat(target);

• Files reviewed: 14/14 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

The Docker request classification/serialization currently creates an unintended security surface (container enumeration without grants and YAML injection risk via unquoted scalars) that should be addressed before merging.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 59/59 changed files
• Comments generated: 2
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

The persistence guard in loadPersistedConfig() can still treat non-object YAML parses as “valid” and risk incorrect hydration/persistence behavior unless it explicitly rejects scalar/array parse results.

Review details

Suppressed comments (2)

Previously missed (2) — in code that hasn't changed since the last review.

src/main/heartbeat-manager.ts:32

• sameTarget compares owner/repo/org case-sensitively, but GitHub repo/org identity is case-insensitive and this PR explicitly mentions older configs may already contain case-variants. With case-sensitive comparison, addTarget/removeTarget can leave duplicate entries in this.targets and cause redundant heartbeat writes/clears. Normalize case when comparing targets.
  src/main/store/middleware/persist.ts:99
• yaml.load() can return a scalar or array for a corrupted config.yaml (e.g. [] or "oops"). In that case diskConfig is truthy and Object.keys(diskConfig) is non-empty, so the "parsed to nothing" guard won’t trip and the middleware may hydrate nonsense and later persist over the on-disk config instead of treating it as unreadable. Treat any non-plain-object parse result as a failed load and set saveBlocked.

• Files reviewed: 59/59 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

It makes broad, security-sensitive changes (sandboxing, Docker request filtering, config persistence) and still has correctness issues to address before approval.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

src/shared/localmostrc.ts:392

• This leftover "Merge sockets policies" JSDoc no longer has a corresponding function (the sockets policy was removed). Leaving it in place is misleading and reads like a partially reverted change.

• Files reviewed: 59/59 changed files
• Comments generated: 1
• Review effort level: Lite