Skip to content

[Security] Scope orphan cleanup to Local Studio-owned processes #228

Description

@fettpl

Audit metadata

  • Priority: P1
  • Estimated effort: L
  • Implementation risk: HIGH
  • Category: security / bug
  • Evidence baseline: 07be5be7ce69bea0c3118744ab90d148b010fce0 (origin/main on 2026-07-17)

Dependencies

None.

Description and impact

Before launch and during stop confirmation, Local Studio kills every non-zombie PPID-1 process whose command contains VLLM::Worker, including workers from unrelated vLLM deployments. The cleanup can escalate to sudo -n kill. Replace global signature killing with fail-closed ownership records and process-group cleanup so only processes created by this controller are signaled.

Existing issue overlap: open issue #210 covers Windows process discovery and child-tree cleanup. this issue is the distinct Unix/global ownership problem; coordinate interfaces but do not absorb Windows work.

Current state and reproduction

controller/src/modules/engines/process/process-manager.ts:195-245 defines:

const isOrphanedInferenceWorker = (entry: ProcessInventoryEntry): boolean => {
  if (entry.ppid !== 1 || entry.stat.includes("Z")) return false;
  return entry.command.includes("VLLM::Worker");
};

Every match receives SIGTERM and possibly SIGKILL; sendSignal falls back to sudo at lines 195-202. Cleanup runs from confirmInferenceStopped and launchModel at lines 250-253 and 282. process-inventory.ts:29-40 collects PID, PPID, state, and argv only; no process group or start identity exists. The baseline test at process-manager-spawn.test.ts:238-252 positively expects an arbitrary signature match to be killed.

Reproduce safely with FakeProcessRunner: return an unrelated PPID-1 VLLM::Worker; confirmInferenceStopped records a sudo kill invocation for its PID.

Solution design

Use the detached launch's process group as the ownership boundary. Persist an owner-only launch record beneath config.data_dir containing a random launch ID, root PID/process-group ID, process start identity, recipe ID, backend, port, and creation time. Extend inventory to read process group and a stable start marker on supported Unix hosts. Signal a process group only when the live inventory can prove it corresponds to the persisted record; if identity is missing, stale, reused, or ambiguous, log and skip. Remove command-substring discovery as an authority to signal.

Write the record atomically with mode 0600 after spawn identity is confirmed, clear it only after the owned group is gone, and reconcile it on controller restart. Docker cleanup remains container-name/process ownership aware. Do not signal an individual VLLM::Worker merely because its argv matches. Keep asynchronous waits in Effect. Add no source comments.

Verification commands

Purpose Command Expected on success
Process seam tests cd controller && bun test tests/integration/process-manager-spawn.test.ts exit 0
Controller unit tests cd controller && bun test src exit 0
Controller gate npm run check:controller exit 0
Full integration npm run test:integration exit 0

Scope

In scope

  • controller/src/core/command.ts
  • controller/src/modules/engines/process/process-inventory.ts
  • controller/src/modules/engines/process/process-manager.ts
  • controller/tests/support/fake-process-runner.ts
  • controller/tests/integration/process-manager-spawn.test.ts
  • One new ownership-record module/test beneath controller/src/modules/engines/process/ if separation is needed.

Out of scope

Implementation plan

Step 1: Reverse the unsafe characterization test

Add fake inventory cases for an unrelated matching worker, an owned orphan group, PID/group reuse, a zombie, an unreadable/stale record, and a restart with a valid record. Assert unrelated/ambiguous PIDs never reach process.kill, kill, or sudo kill.

Verify: cd controller && bun test tests/integration/process-manager-spawn.test.ts -> ownership cases fail on baseline.

Step 2: Add stable process identity and durable ownership

Extend the process seam and inventory parser with PGID and start identity. Implement atomic read/write/remove of one active launch record with 0700 parent and 0600 file permissions. Validate decoded data with Effect Schema.

Verify: targeted ownership module/parser tests pass and malformed records fail closed.

Step 3: Record launches and clean only owned groups

After spawnDetached, confirm PID/PGID/start identity, persist ownership, and use that record for stop/restart reconciliation. Replace isOrphanedInferenceWorker cleanup and the confirmInferenceStopped global check. Group signalling must validate identity immediately before each signal and must not sudo an unproven target.

Verify: process seam suite passes with zero signal calls for unrelated and reused identities.

Step 4: Verify lifecycle and failure cleanup

Exercise normal stop, fast exit, controller-restart reconciliation, and an owned orphan that needs TERM then KILL. Ensure records are retained when cleanup is incomplete and removed only after no owned group member remains.

Verify: npm run check:controller and npm run test:integration -> exit 0.

Test plan

Use FakeProcessRunner and process-manager-spawn.test.ts; never signal real host processes. Include native and Docker launches, PID reuse, PGID reuse, malformed/stale ownership data, failed persistence, TERM success, KILL escalation, and controller restart. The principal negative assertion is no signal invocation for every unowned or ambiguous candidate.

Acceptance criteria

  • No argv substring is sufficient authority to signal a process.
  • Only a validated Local Studio ownership record can authorize TERM/KILL.
  • PID/PGID reuse and malformed/stale records fail closed.
  • Owned worker groups are cleaned after parent/controller failure.
  • Ownership data is atomic and owner-only.
  • No unrelated test PID reaches direct or sudo kill.
  • npm run check and npm run test:integration exit 0.

Risks and stop conditions

  • The target platform cannot provide a stable process-group/start identity; remove global cleanup and report the limitation instead of weakening proof.
  • Correctness requires cgroups, systemd scopes, or Windows job objects; propose a separate platform design.
  • A valid ownership record cannot be written atomically/privately before cleanup could be needed.
  • Any proposed fallback still kills by command signature, port alone, or PPID alone.
  • Source drift or two verification failures occur.

Maintenance considerations

Process ownership is a safety boundary. Review every new runtime/container launch path for ownership registration, and treat inability to prove ownership as a reason not to signal.


Generated from Local Studio 2.0 main-branch audit proposal 009.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions