Skip to content

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

Open
fettpl wants to merge 2 commits into
sybil-solutions:devfrom
fettpl:fix/228-prove-orphan-ownership
Open

[Security] Scope orphan cleanup to Local Studio-owned processes#370
fettpl wants to merge 2 commits into
sybil-solutions:devfrom
fettpl:fix/228-prove-orphan-ownership

Conversation

@fettpl

@fettpl fettpl commented Aug 6, 2026

Copy link
Copy Markdown

Summary

Make compute cleanup fail closed: Local Studio now signals a native process group or removes a Docker container only after proving that the live target is the exact launch generation recorded by this controller.

This replaces shape-based cleanup authority with durable identity and leaves ambiguous or stale state untouched.

Closes #228

Maintainer alignment

  • Built as one controller-only commit directly on current dev (a8d6e6ab5d2e7676a11e3f297883c81464892042).
  • Extends the current compute launcher and lifecycle rather than carrying the historical main-targeted implementation.
  • Persists native process-group identity across controller restart.
  • Removes unproved pre-launch Docker deletion.
  • Adds zero-signal negative coverage for malformed records, identity mismatch, PID/PGID reuse, foreign group members, and unproved containers.

Solution design

Durable ownership records

  • Decode persisted ownership with Effect Schema and reject invalid records without replacing or deleting them.
  • Store records atomically below an owner-only directory with owner-only file permissions.
  • Retain records whenever cleanup cannot prove that the owned target is gone.

Native launch proof

  • On Linux, bind the record to PID, process group, session, stable start identity, and a random launch nonce.
  • Require every live process-group member to carry that nonce, then revalidate identity immediately before TERM or KILL.
  • On other platforms, authorize cleanup only through the exact in-memory ChildProcess; restart recovery remains fail closed when durable OS identity is unavailable.

Docker launch proof

  • Bind cleanup to the canonical Docker executable identity, daemon identity, full container ID, generated name, and launch labels.
  • Prove the newly started container before accepting the launch.
  • Re-prove identity after stop and before removal; never remove a pre-existing or unproved container.

Lifecycle behavior

  • Keep ownership while a target is still alive or ownership is uncertain.
  • Drop ownership only after the launcher proves that the recorded target no longer exists.

Acceptance criteria

  • Command text, port, PID, or container name alone cannot authorize cleanup.
  • Linux TERM/KILL requires exact persisted and live process identity.
  • Malformed state, PID/PGID reuse, missing nonce, foreign group members, and restart ambiguity produce zero signals.
  • Docker stop/remove requires canonical client, daemon, full container, and label identity.
  • Unproved pre-launch Docker removal is eliminated.
  • Ownership persistence is schema validated, atomic, and owner only.
  • Incomplete cleanup retains state for later reconciliation.

Validation

  • npm run check — passed.
  • npm run test:integration — passed.
  • Focused ownership/lifecycle tests — 22 passed, 81 assertions, 0 failures.
  • Comparable controller coverage versus dev: 90 to 97 tests, 40.36% to 42.03% functions (+1.67 points), and 51.05% to 52.17% lines (+1.12 points).
  • git diff --check — passed.
  • Normal pre-push hook — passed, including frontend static, cleanup, dependency, duplication, and standalone-runtime gates; no hook was bypassed.
  • Independent exact-head P0/P1 review of 63b47503a74d13cd3ac0b653044a0572f9c71ef9 — no blockers found.

Risk and rollout

Fail-closed behavior can leave a genuinely owned orphan running when the host cannot supply enough identity. That residue is intentional and safer than signaling an unrelated workload; the retained record can be reconciled later when ownership becomes provable.

No frontend or packaged desktop behavior changes.

@fettpl
fettpl requested a review from 0xSero as a code owner August 6, 2026 22:22
@fettpl

fettpl commented Aug 6, 2026

Copy link
Copy Markdown
Author

@0xSero, maintainer update: #370 is the narrow current-dev replacement requested in the historical PR closure: one controller-only commit with durable record-bound native and Docker ownership proof, no unproved pre-launch removal, and zero-signal negative coverage. The full local gates, normal pre-push hook, and independent P0/P1 review passed. Comparable controller coverage rises by 1.67 function points and 1.12 line points with seven additional tests and no retained-requirement gap. Please review when convenient.

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