fix(supervisor): stop apps under the shipped unit; never hang on a failed kill by linyiru · Pull Request #20 · solcreek/creekd

linyiru · 2026-09-30T22:25:48Z

Problem

Since v0.1.3 every app runs as its own UID. Under init/creekd.service creekd is the non-root creekd user holding CAP_NET_BIND_SERVICE CAP_SETUID CAP_SETGID — but not CAP_KILL, which the kernel requires to signal a process of another UID. So:

DELETE, restart and blue-green deploy send SIGTERM/SIGKILL that fail with EPERM, logged at debug as "process likely gone";
the old process keeps running;
escalateAndWait then blocks on <-done forever, and so does the admin request. Observed: a DELETE hung for 23 minutes; SIGQUIT showed the goroutine in escalateAndWait → stopApp → StopWithTimeout → adminapi.StopApp.

$ setpriv --reuid=creekd … --ambient-caps=+net_bind_service,+setuid,+setgid kill -0 <app pid>
kill: (4553): Operation not permitted
$ … +kill …
(ok)

Reach today: the shipped unit cannot spawn apps as-is (#18), so this bites the #18 workaround (clearing the cgroup variables). Root mode is unaffected.

Change

Unit: CAP_KILL added to CapabilityBoundingSet and AmbientCapabilities; hardening-check expects it. Apps never inherit it (setpriv drops inheritable + ambient).
Startup: creekd logs an error when per-app UIDs are on but it cannot signal other UIDs (Supervisor.CanSignalApps). Per-app UIDs stay on — leaking secrets is worse than a failing stop, and the error says what to grant.
Stop never hangs: a SIGKILL that cannot be delivered fails Stop after KillWaitTimeout (default 5s) with the PID; the app is logged as "no longer supervised".
Visibility: signal failures other than os.ErrProcessDone log as warnings.

Granting CAP_KILL lets a compromised creekd signal any process on the host. The capability-free alternative is for creekd's spawn helper to stay in the app as its init and relay signals (with #17); that is a larger change being prototyped for #18.

Verification (2026-09-30, Ubuntu 26.04, shipped unit, cgroup vars cleared)

	with `CAP_KILL` (`CapEff 0x4e0`)	without (`0x4c0`)
startup	—	error logged
restart a handler-less app	200, new PID	—
blue-green deploy (SIGTERM-handling June app)	200 in 215 ms, v1 gone	—
`DELETE`	204 in 8–9 ms, nothing left running	returns after 35 s (reply lost to #17's write timeout), log: `cannot stop app … pid 3282 … operation not permitted` — no longer hangs

Unit tests: TestStopReturnsWhenSignalsCannotBeDelivered (EPERM injected → Stop errors in ~300 ms), TestSignalFailed, TestParseCapBitsKill, hardening baseline. go test ./... passes.

…iled kill Since 0.1.3 each app runs as its own UID, and under init/creekd.service creekd is the non-root creekd user without CAP_KILL: SIGTERM and SIGKILL to an app failed with EPERM (logged at debug as 'process likely gone'). Stop, restart and deploy left the old process running, and escalateAndWait then blocked on <-done forever, hanging the admin request (seen: 23 minutes, goroutine dump in escalateAndWait). - init/creekd.service grants CAP_KILL (bounding + ambient); hardening-check expects it. - creekd logs an error at startup when per-app UIDs are on but it cannot signal other UIDs (CanSignalApps). - A SIGKILL that cannot be delivered fails Stop after KillWaitTimeout (5s) with the PID, and the app is reported as no longer supervised. - Signal failures other than 'process already finished' log as warnings. Verified 2026-09-30 on Ubuntu 26.04 under the shipped unit (cgroup vars cleared, see #18): with CAP_KILL, restart 200, blue-green deploy 200 with v1 gone, DELETE 204 in 8-9 ms for a SIGTERM-handling and a handler-less app; without CAP_KILL, the startup error is logged and DELETE returns after 35 s with 'cannot stop app ... operation not permitted' in the log instead of hanging.

Copilot

Copilot review overview

🟡 Changes recommended

A failed stop leaves inconsistent supervisor, routing, and persistent state, while deploy diagnostics also misstate actual behavior.

Review effort: Balanced
Findings: 1 · 1 · 2

Open (4)

Failed stop leaves orphaned app state and routing · New
Deploy reports success when stopping the old process fails · New
Deploy hides old-process shutdown failures · New
Successful deploy can leave both app versions running · New

What changed in this PR

Adds CAP_KILL support so creekd can manage per-UID apps and bounds failed stop attempts.

Changes:

Grants and validates CAP_KILL.
Adds startup diagnostics and bounded SIGKILL failure handling.
Adds signal/capability tests and documentation.

File	Description
`internal/supervisor/supervisor.go`	Bounds failed kills and improves signal logging.
`internal/supervisor/signal_test.go`	Tests failed signals and capability parsing.
`internal/supervisor/runas.go`	Detects effective `CAP_KILL`.
`internal/hardening/validator.go`	Requires `CAP_KILL`.
`internal/hardening/validator_test.go`	Updates capability baselines.
`init/creekd.service`	Grants `CAP_KILL`.
`docs/DESIGN.md`	Documents the capability model.
`docs/CONFIG.md`	Documents per-app UID requirements.
`cmd/creekd/main.go`	Reports missing `CAP_KILL` at startup.
`CHANGELOG.md`	Records the stop fix.

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

linyiru · 2026-10-01T01:04:31Z

+				s.logger.Error("cannot stop app: it keeps running and is no longer supervised",
+					"id", id, "pid", cmd.Process.Pid, "err", err,
+				)
+				return fmt.Errorf("supervisor: stop %q: SIGKILL pid %d: %w", id, cmd.Process.Pid, err)


Fixed in 554f218. stopApp now skips its teardown when the process did not stop, and StopWithTimeout puts the app back in the registry: it stays supervised, routed and persisted, consistent across GET/DELETE, and a retry works once signals do. The test asserts it is still registered after the failed stop and that a retry succeeds. On a VM without CAP_KILL: after the failed DELETE, GET 200, still served, still in state.json.

linyiru · 2026-10-01T01:04:32Z

+		logger.Error("creekd cannot signal apps that run as their own UID (no CAP_KILL): " +
+			"stop, restart and deploy will fail and leave the old process running. " +
+			"Run creekd as root or grant it CAP_KILL (see init/creekd.service)")


Right — fixed in 554f218. The startup error now says stop and restart fail while a deploy starts the new version and leaves the old one running; the v1 stop failure in Deploy is now an error log with v1_pid.

linyiru · 2026-10-01T01:04:34Z

 - **Fails closed**: an app with a recorded UID does not start when creekd cannot switch UIDs (no root, no `CAP_SETUID`/`CAP_SETGID`); it is never silently run as creekd's own user.
 - **User namespaces**: an app spawned with `sandbox.user_namespace` gets no allocated UID — its identity is its UID mapping. A `run_as` for such an app must be a UID mapped inside its namespace.
- **Requires**: creekd running as root, or holding `CAP_SETUID` + `CAP_SETGID` (the shipped `init/creekd.service` grants both). Without them creekd warns at startup and every app shares its UID.
+- **Requires**: creekd running as root, or holding `CAP_SETUID` + `CAP_SETGID` to switch UIDs and `CAP_KILL` to signal the apps afterwards (the shipped `init/creekd.service` grants all three). Without `CAP_SETUID`/`CAP_SETGID` creekd warns at startup and every app shares its UID; without `CAP_KILL` it logs an error at startup, and stop, restart and deploy fail with `EPERM` while the old process keeps running.


Fixed in 554f218: CONFIG.md now says a deploy promotes the new version while the old one keeps running unsupervised, logged as an error with its PID.

linyiru · 2026-10-01T01:04:35Z

+// CanSignalApps reports whether creekd can signal an app that runs as
+// another UID: root, or CAP_KILL. Without it, a per-app UID app cannot
+// be stopped, restarted or replaced by a deploy; creekd reports this
+// at startup.


Fixed in 554f218: the CanSignalApps comment now describes the split deployment (v2 promoted, v1 left running unsupervised) instead of a failing deploy.

The child advanced its counter file before printing run-N, and the test stops the app as soon as the counter reads 2: a run stopped between the two steps had counted without printing, and the log lacked run-2. It failed once on CI for #20 (race detector on); locally 1 in 300 runs on this branch and 0 in 300 on main, a rate too low to tell apart. With the line printed first, reading 2 implies run-2 was written: 0 in 300.

Copilot

Copilot review overview

🔵 Needs a closer look

Failed stops currently leave supervisor, router, and persisted state inconsistent, and deploy behavior is documented inaccurately.

Review effort: Balanced
Findings: 1 · 1 · 2

Open (4)

Failed stop leaves orphaned app state and routing
Deploy reports success when stopping the old process fails
Successful deploy can leave both app versions running
Deploy hides old-process shutdown failures

Previously missed (1)

In code that hasn't changed since last review

Test stop timeout delay and PID in error message

internal/supervisor/signal_test.go:41

The test comment promises both the KillWaitTimeout delay and an error naming the PID, but this branch only checks for any non-nil error within five seconds. It would still pass if Stop returned immediately without using the new timeout or omitted the PID. Assert a lower bound near the configured 300 ms and that err.Error() contains pid.

…loy as it is Review on #20: - StopWithTimeout removed the app from the registry before stopping it; when the stop failed, the handler returned early and the process kept running, still routed and in state.json, behind 404s on GET and DELETE. stopApp now skips its teardown (log rotator, cgroup, network) when the process did not stop, and StopWithTimeout puts the app back: supervised, visible, and stoppable once the cause is fixed. - Deploy promotes v2 even when v1 cannot be stopped, so the hazard is two versions running, not a failing deploy: the v1 failure now logs at error with v1_pid, and the startup message, CanSignalApps, CONFIG.md and the changelog say so. - TestStopReturnsWhenSignalsCannotBeDelivered asserts the wait (graceful window + KillWaitTimeout), the PID in the error, that the app is still registered afterwards, and that a retry succeeds once signals work. Verified 2026-09-30 on Ubuntu 26.04 without CAP_KILL: after the failed DELETE, GET 200, still served, still in state.json; a deploy leaves v1 (pid logged) beside v2 with the error logged.

Copilot AI balanced review requested due to automatic review settings September 30, 2026 22:25

Copilot started reviewing on behalf of linyiru September 30, 2026 22:26

View session

Copilot AI reviewed Sep 30, 2026

View reviewed changes

linyiru requested a balanced review from Copilot October 1, 2026 00:45

Copilot started reviewing on behalf of linyiru October 1, 2026 00:46

View session

Copilot AI reviewed Oct 1, 2026

View reviewed changes

linyiru merged commit c2520d6 into main Oct 1, 2026
10 checks passed

linyiru deleted the fix/stop-per-app-uid branch October 1, 2026 01:22

linyiru mentioned this pull request Oct 1, 2026

Spawn gate as the app's init: forward signals, stop without CAP_KILL #25

Open

Navigation Menu

Repository navigation

fix(supervisor): stop apps under the shipped unit; never hang on a failed kill - #20

fix(supervisor): stop apps under the shipped unit; never hang on a failed kill#20
linyiru merged 3 commits into
mainfrom
fix/stop-per-app-uid

linyiru commented Sep 30, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

linyiru Oct 1, 2026

Uh oh!

linyiru Oct 1, 2026

Uh oh!

linyiru Oct 1, 2026

Uh oh!

linyiru Oct 1, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

linyiru commented Sep 30, 2026

Problem

Change

Verification (2026-09-30, Ubuntu 26.04, shipped unit, cgroup vars cleared)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

linyiru Oct 1, 2026

Choose a reason for hiding this comment

Uh oh!

linyiru Oct 1, 2026

Choose a reason for hiding this comment

Uh oh!

linyiru Oct 1, 2026

Choose a reason for hiding this comment

Uh oh!

linyiru Oct 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🔵 Needs a closer look

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants