Skip to content

Commit c04d039

Browse files
committed
fix: [sc-109762] Isolate the Linux auto-update helper from its own systemd cgroup
The --update helper inherited the running service's cgroup, so calling systemctl stop on its own unit let KillMode=control-group kill the helper along with the service before it could restart it, leaving the endpoint offline with no auto-recovery. The helper now runs in its own transient systemd scope (systemd-run --scope) so it survives the stop. Adds integration-test coverage that asserts the helper actually logs a restart, the service is active, and it resubscribes after a Linux auto-update, closing a gap where the existing check could pass even when the helper never restarted anything.
1 parent de69b98 commit c04d039

8 files changed

Lines changed: 268 additions & 18 deletions

File tree

.github/workflows/integration-test.yml

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -290,6 +290,25 @@ jobs:
290290
binary: ${{ matrix.it_binary }}
291291
args: --update --org-id ${{ vars.IT_ORG_ID }} --github-token ${{ secrets.GITHUB_TOKEN }}
292292

293+
# Captured before the real auto-update fires, so the assertions below
294+
# measure what that update did rather than the restart the IT binary
295+
# install above already caused.
296+
- name: Capture pre-auto-update baseline counts
297+
id: autoupdate_baseline
298+
shell: pwsh
299+
env:
300+
LOG_FILE: ${{ matrix.log_file }}
301+
run: |
302+
$content = Get-Content $env:LOG_FILE -Raw -ErrorAction SilentlyContinue
303+
function Get-PatternCount($text, $needle) {
304+
if ($text) { ([regex]::Matches($text, [regex]::Escape($needle))).Count } else { 0 }
305+
}
306+
$started = Get-PatternCount $content "Service started"
307+
$subscribed = Get-PatternCount $content "Subscribed to messages"
308+
"started=$started" >> $env:GITHUB_OUTPUT
309+
"subscribed=$subscribed" >> $env:GITHUB_OUTPUT
310+
Write-Output "Baseline counts -> started=$started subscribed=$subscribed"
311+
293312
- name: Wait for auto updater to trigger and update
294313
uses: ./.github/actions/wait-for-log-line
295314
with:
@@ -301,6 +320,82 @@ jobs:
301320
with:
302321
log_file: ${{ matrix.log_file }}
303322

323+
# ---- Linux systemd cgroup-kill regression (sc-109762) ----
324+
# verify-auto-update above only checks the log for "Updating agent" and
325+
# that the last "Agent Smith started" line isn't the IT version - but
326+
# that line is logged by the --update helper on its own startup, before
327+
# it ever stops the old service, so it already names the new version
328+
# even when the helper is killed moments later and never restarts
329+
# anything. On Linux, the helper used to inherit the running service's
330+
# systemd cgroup, so calling `systemctl stop` on its own unit triggered
331+
# KillMode=control-group and killed the helper along with the service it
332+
# asked to stop - the endpoint was left offline with the old binary
333+
# still in place and no automatic recovery. The real proof the update
334+
# finished is the helper's own "Service started" line, logged only after
335+
# it replaced the binary and called svc.Start() - the exact point the
336+
# cgroup kill used to cut it off. Counts are compared as deltas because
337+
# "Service started" also appears once from installing the IT binary
338+
# above, and "Subscribed to messages" already has lines from earlier
339+
# scenarios.
340+
- name: Assert the update helper survived to restart the service
341+
shell: pwsh
342+
env:
343+
LOG_FILE: ${{ matrix.log_file }}
344+
BASELINE: ${{ steps.autoupdate_baseline.outputs.started }}
345+
run: |
346+
$baseline = [int]$env:BASELINE
347+
for ($i = 1; $i -le 30; $i++) {
348+
$content = Get-Content $env:LOG_FILE -Raw -ErrorAction SilentlyContinue
349+
$count = if ($content) { ([regex]::Matches($content, [regex]::Escape("Service started"))).Count } else { 0 }
350+
if ($count -gt $baseline) {
351+
Write-Output "Update helper restarted the service (count $count > baseline $baseline) after ~$($i * 2)s"
352+
exit 0
353+
}
354+
Start-Sleep -Seconds 2
355+
}
356+
Write-Output "---- last 60 lines of the agent log ----"
357+
Get-Content $env:LOG_FILE -Tail 60 -ErrorAction SilentlyContinue
358+
Write-Error "Update helper never logged 'Service started' after the auto-update; it was likely killed mid-update (count stayed at $baseline)"
359+
exit 1
360+
361+
- name: Check service is active after auto-update
362+
uses: ./.github/actions/check-service
363+
with:
364+
service: ${{ matrix.service }}
365+
366+
- name: Wait for fresh subscription after auto-update
367+
shell: pwsh
368+
env:
369+
LOG_FILE: ${{ matrix.log_file }}
370+
BASELINE: ${{ steps.autoupdate_baseline.outputs.subscribed }}
371+
run: |
372+
$baseline = [int]$env:BASELINE
373+
for ($i = 1; $i -le 30; $i++) {
374+
$content = Get-Content $env:LOG_FILE -Raw -ErrorAction SilentlyContinue
375+
$count = if ($content) { ([regex]::Matches($content, [regex]::Escape("Subscribed to messages"))).Count } else { 0 }
376+
if ($count -gt $baseline) {
377+
Write-Output "Agent resubscribed after auto-update (count $count > baseline $baseline) after ~$($i * 2)s"
378+
exit 0
379+
}
380+
Start-Sleep -Seconds 2
381+
}
382+
Write-Error "Agent never resubscribed after auto-update (subscribed count stayed at $baseline)"
383+
exit 1
384+
385+
- name: Send command after auto-update
386+
uses: ./.github/actions/send-command
387+
with:
388+
trigger_url: ${{ vars.IT_SEND_COMMAND_TRIGGER_URL }}
389+
device_id: ${{ steps.config.outputs.device_id }}
390+
commands: ${{ matrix.success_commands }}
391+
392+
- name: Check logs for command completion after auto-update
393+
uses: ./.github/actions/assert-log-contains
394+
with:
395+
log_file: ${{ matrix.log_file }}
396+
patterns: ${{ matrix.success_patterns }}
397+
wait_seconds: "15"
398+
304399
- name: Update logging level to debug
305400
uses: ./.github/actions/run-agent
306401
with:

CLAUDE.md

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,15 @@ Required tools:
6161
leave an endpoint offline. The agent executable and config file are written to a
6262
temp file and atomically renamed into place, so a failed write leaves the
6363
previous file byte-identical. See the README's "Waiting for the Old Agent
64-
Process to Exit" section.
64+
Process to Exit" section. On Linux, the `--update` helper that performs this
65+
flow is launched in its own transient systemd scope
66+
(`cmd/agent_smith/run_command_linux.go`, `systemd-run --scope --collect`)
67+
rather than as a plain child of the running service, so it is never a member
68+
of that service's cgroup — otherwise `systemctl stop`'s default
69+
`KillMode=control-group` kills the helper along with the service it just
70+
stopped, before it can restart it, leaving the endpoint offline with no
71+
automatic recovery. See the README's "Surviving Its Own systemd Stop
72+
(Linux)" section.
6573

6674
- **internal/agent/**: Device configuration, installation paths, and OS-specific host information. Auto-update installers are downloaded into `<data directory>/updates` (a `0700` directory the agent owns) rather than the shared system temp directory, and `SweepStaleInstallers` reclaims installer binaries older than 24 hours at service startup — from that directory and from the legacy temp location — so the binaries a detached installer necessarily leaves behind stop accumulating one per update. See the README's "Reclaiming Downloaded Installer Binaries" section. The auto-update retry schedule is capped (1 hour, or a quarter of the check interval when shorter) and jittered (±25%) via `utils.JitteredBackoff`, the same helper the postback retry schedule uses, so the doubling cannot overflow into a negative sleep that busy-spins and a fleet-wide release-endpoint outage cannot produce a synchronized retry storm. See the README's "Capped and Jittered Auto-Update Retries" section.
6775
- **internal/interpreter/**: Command execution engine supporting both PowerShell and Bash interpreters

README.md

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -536,6 +536,34 @@ pattern the postback spool uses. An interrupted or failed write therefore leaves
536536
the previous file byte-identical rather than truncated: the endpoint keeps running
537537
the old agent instead of a binary that cannot start.
538538

539+
### Surviving Its Own systemd Stop (Linux)
540+
541+
The `--update` helper that an auto-update spawns has to stop the running
542+
service before it can replace the binary and start it again. On Linux that
543+
service runs as a systemd unit, and systemd's default `KillMode=control-group`
544+
tears down every process in the unit's cgroup — not just its main process —
545+
when the unit is stopped. The helper is launched as a child of the running
546+
service, so without intervention it inherits that cgroup: the moment it calls
547+
`systemctl stop` on its own unit, systemd kills the helper along with the
548+
service it just asked to stop, mid-update. The service is left stopped, the
549+
binary and config were never touched, and — because the kill is a signal, not
550+
a normal return — the helper's own deferred recovery never runs either.
551+
`Restart=always` does not help: the unit was stopped by an explicit
552+
`systemctl stop`, which systemd treats as a clean, intentional exit, not the
553+
unexpected one `Restart=` reacts to.
554+
555+
The helper now runs inside its own transient systemd **scope**
556+
(`systemd-run --scope --collect`) rather than as a plain child process, so it
557+
is never a member of the unit's cgroup in the first place. Stopping the unit
558+
it was launched from tears down only that unit's cgroup; the helper's scope is
559+
untouched, so it survives to replace the binary, update the config, and start
560+
the service again — the same flow already used on Windows and macOS. macOS
561+
needed no equivalent change: launchd tears down a stopped job by BSD process
562+
group (`killpg`), and the `Setsid` the helper already sets moves it into a new
563+
process group, which is enough to escape that teardown. Linux's cgroup-based
564+
`KillMode` is inherited across `fork()` and untouched by `setsid()`, so the
565+
same call that protects the helper on macOS does not protect it on Linux.
566+
539567
### Capped and Jittered Auto-Update Retries
540568

541569
When an update check or download fails, the agent retries on an exponential
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
//go:build darwin
2+
3+
package main
4+
5+
import (
6+
"os"
7+
"os/exec"
8+
"syscall"
9+
)
10+
11+
// detachedCommand launches the auto-update helper detached from the running
12+
// service process. Setsid alone is sufficient on macOS: launchd tears down a
13+
// stopped job by BSD process group (killpg), and setsid() moves the helper
14+
// into a brand new session/process group, so it is already outside the group
15+
// launchd kills. Linux's KillMode=control-group instead kills by cgroup
16+
// membership, which setsid() does not change, so Linux needs a different
17+
// mechanism — see run_command_linux.go.
18+
func detachedCommand(path string, args []string, stdout, stderr *os.File) *exec.Cmd {
19+
cmd := exec.Command(path, args...)
20+
cmd.Stdout = stdout
21+
cmd.Stderr = stderr
22+
cmd.SysProcAttr = &syscall.SysProcAttr{Setsid: true}
23+
return cmd
24+
}
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
//go:build darwin
2+
3+
package main
4+
5+
import (
6+
"os"
7+
"testing"
8+
)
9+
10+
func TestDetachedCommandRunsDirectlyOnDarwin(t *testing.T) {
11+
path := "/opt/agent/rewst_agent_config.mac-os.bin"
12+
args := []string{"--update", "--org-id", "abc"}
13+
cmd := detachedCommand(path, args, os.Stdout, os.Stderr)
14+
15+
if cmd.Path != path {
16+
t.Fatalf("Path = %q, want %q", cmd.Path, path)
17+
}
18+
wantArgs := append([]string{path}, args...)
19+
if len(cmd.Args) != len(wantArgs) {
20+
t.Fatalf("Args = %v, want %v", cmd.Args, wantArgs)
21+
}
22+
for i, want := range wantArgs {
23+
if cmd.Args[i] != want {
24+
t.Fatalf("Args[%d] = %q, want %q", i, cmd.Args[i], want)
25+
}
26+
}
27+
28+
if cmd.SysProcAttr == nil {
29+
t.Fatalf("SysProcAttr = nil, want non-nil")
30+
}
31+
if !cmd.SysProcAttr.Setsid {
32+
t.Fatalf("Setsid = false, want true")
33+
}
34+
}
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
//go:build linux
2+
3+
package main
4+
5+
import (
6+
"os"
7+
"os/exec"
8+
"syscall"
9+
)
10+
11+
// detachedCommand launches the auto-update helper (path, args) inside its own
12+
// transient systemd scope instead of running it as a direct child of the
13+
// service process. The helper's job is to call `systemctl stop` on the very
14+
// unit it was launched from; a plain child inherits that unit's cgroup, and
15+
// systemd's default KillMode=control-group kills every process in the
16+
// cgroup — including the helper itself — the moment the stop is issued, before
17+
// it can replace the binary or start the service again. `systemd-run --scope`
18+
// creates a new transient scope (and cgroup) for the helper up front, so it is
19+
// never a member of the unit's cgroup and survives stopping it. `--collect`
20+
// releases the transient scope unit's bookkeeping once the helper exits so
21+
// scopes don't accumulate one per update.
22+
func detachedCommand(path string, args []string, stdout, stderr *os.File) *exec.Cmd {
23+
scopeArgs := append([]string{"--scope", "--collect", "--quiet", "--", path}, args...)
24+
cmd := exec.Command("systemd-run", scopeArgs...)
25+
cmd.Stdout = stdout
26+
cmd.Stderr = stderr
27+
cmd.SysProcAttr = &syscall.SysProcAttr{Setsid: true}
28+
return cmd
29+
}
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
//go:build linux
2+
3+
package main
4+
5+
import (
6+
"os"
7+
"strings"
8+
"testing"
9+
)
10+
11+
func TestDetachedCommandWrapsInSystemdScope(t *testing.T) {
12+
cmd := detachedCommand("/opt/agent/rewst_agent_config.linux.bin", []string{"--update", "--org-id", "abc"}, os.Stdout, os.Stderr)
13+
14+
wantPath := "systemd-run"
15+
if got := cmd.Path; got != wantPath && !strings.HasSuffix(got, "/"+wantPath) {
16+
t.Fatalf("Path = %q, want %q (or a resolved path ending in it)", got, wantPath)
17+
}
18+
19+
wantArgs := []string{
20+
"systemd-run",
21+
"--scope",
22+
"--collect",
23+
"--quiet",
24+
"--",
25+
"/opt/agent/rewst_agent_config.linux.bin",
26+
"--update",
27+
"--org-id",
28+
"abc",
29+
}
30+
if len(cmd.Args) != len(wantArgs) {
31+
t.Fatalf("Args = %v, want %v", cmd.Args, wantArgs)
32+
}
33+
for i, want := range wantArgs {
34+
if cmd.Args[i] != want {
35+
t.Fatalf("Args[%d] = %q, want %q (full: %v)", i, cmd.Args[i], want, cmd.Args)
36+
}
37+
}
38+
39+
if cmd.SysProcAttr == nil {
40+
t.Fatalf("SysProcAttr = nil, want non-nil")
41+
}
42+
if !cmd.SysProcAttr.Setsid {
43+
t.Fatalf("Setsid = false, want true")
44+
}
45+
}
46+
47+
func endsWith(s, suffix string) bool {
48+
return len(s) >= len(suffix) && s[len(s)-len(suffix):] == suffix
49+
}

cmd/agent_smith/run_command_unix.go

Lines changed: 0 additions & 17 deletions
This file was deleted.

0 commit comments

Comments
 (0)