Skip to content

fix: [sc-106111] Sweep downloaded installer binaries so temp storage stops accumulating dead agents - #102

Merged
mlataza merged 2 commits into
mainfrom
feature/sc-106111/sweep-downloaded-installer-binaries-so-temp
Aug 13, 2026
Merged

mlataza merged 2 commits into
mainfrom
feature/sc-106111/sweep-downloaded-installer-binaries-so-temp

Conversation

@mlataza

@mlataza mlataza commented Aug 13, 2026

Copy link
Copy Markdown
Collaborator

Problem

Every successful auto-update leaves its downloaded installer binary in the system temp directory forever. Download creates the file with os.CreateTemp("", "installer-*.bin") and its deferred cleanup removes it only on the failure path — success = true deliberately skips the removal so the installer can be executed. Nothing removes it afterwards: Update spawns it detached and returns, and no sweep covered installer-*.bin. This is the same accumulating-temp-file leak SweepStaleScripts fixed for exec-*.ps1 in sc-103967; the installer files were not covered.

Each leaked file is a full agent binary, so a long-lived endpoint quietly accumulates hundreds of megabytes. On the space-constrained systems where that matters most — thin VDI images, small VM system disks, appliances — a full temp volume is not just an agent problem: it breaks Windows Installer, application logging, and anything else needing scratch space, and the agent's own next update then fails because os.CreateTemp cannot allocate. Secondary concern: these are executable files sitting 0755 in a world-readable directory indefinitely.

The download path cannot clean up after itself, because the process that would delete the file is the one the installer replaces. So the fix is a startup sweep, plus a move that makes the sweep safe.

Changes

Downloads land in a directory the agent owns (c305c83). Installers now go to <data directory>/updatesC:\ProgramData\RewstRemoteAgent\<orgId>\updates, /etc/rewst_remote_agent/<orgId>/updates, /Library/Application Support/rewst_remote_agent/<orgId>/updates — created 0700, instead of the shared system temp directory. This buys three things: the sweep only ever runs against a directory this agent created, a full agent binary is no longer left executable and world-readable, and endpoints that mount /tmp noexec (a common hardening baseline) can execute the installer at all. Uninstall already RemoveAlls the data directory, so nothing is left behind by the move and no uninstall change is needed. The ticket asked for this to be considered explicitly; the shared temp directory was not retained.

SweepStaleInstallers at service startup (c305c83). Runs once per process, after the service reports itself running (same placement and best-effort contract as the script sweep), removing installer binaries older than 24 hours. Because a successful update restarts the agent, each start reclaims the previous update's installer while leaving the current one — far younger than the threshold — alone: steady state is a single file rather than one per update. The legacy shared temp directory is swept as well, so an upgraded endpoint reclaims what it has accumulated since it was installed rather than only stopping the growth from here on.

The matcher is the conservative one SweepStaleScripts established, which is what makes it safe to point at a directory shared with the rest of the system: regular files only (never symlinks or device nodes), the exact installer-<digits>.bin shape os.CreateTemp produces, and nothing under the age threshold. The os.CreateTemp pattern is a shared constant used by both the download and the sweep, so the two cannot drift. Every failure — unreadable directory, unremovable file (a running Windows installer holds its own image open) — is logged and skipped; the sweep never returns an error. A non-zero number of removals logs at Info with the count and directory; individual removals log at Debug.

Integration coverage (f097a1d). A scenario on all three platforms plants an aged installer, a fresh one, and a similarly named file the agent never creates in the org's updates directory, restarts, and asserts only the agent's own stale download is reclaimed. New installer-sweep-fixture action seeds/asserts elevated, since the updates directory belongs to the service account.

Verification

Probed live against the shipped binary, not a test re-run. Built cmd/agent_smith, planted a 12 MiB fixture in the real shared temp directory (one aged agent-created installer, one fresh, one aged non-agent file), and ran the binary in service mode:

[DEBUG] Removed stale installer file: file=/var/folders/.../T/installer-900000001.bin size=4194304 modified="2026-08-11 07:43:40 -0700"
[INFO]  Swept stale installer files: dir=/var/folders/.../T/ removed=1 max_age=24h0m0s

4 MiB reclaimed; the fresh installer and the non-agent file both survived; startup was neither delayed nor failed; the nonexistent updates directory logged nothing at all (a missing directory is the fresh-install case, not an error).

Second live probe, same binary rebuilt with -X ...updateIntervalStr=3s and auto-updates on, confirming the download target moved (run unprivileged, so the mkdir under the root-owned data directory is expected to fail — the point is which path it names):

[INFO]  Latest release: tag_name=v1.5.1
[ERROR] Update failed, starting retry backoff: error="failed to create updates directory
        /Library/Application Support/rewst_remote_agent/<org>/updates: ... permission denied"

Incidental corroboration of the leak: the developer machine this was built on had six orphaned installer-*.bin files in its temp directory from prior runs of the pre-fix test suite, mode 0755, never cleaned by anything.

Falsification of the new assertion. Mutated service.go in a disposable detached worktree to drop the legacy-temp sweep call; TestExecute_SweepsStaleInstallerFilesOnStartup went red on both the surviving-file assertion and the missing log line. Restored, green. The author worktree was never mutated.

Unit tests cover every case the ticket asked for: a stale installer is removed; a fresh one is not; files matching the prefix but not the full pattern are left alone (non-numeric middle, empty middle, wrong suffix, wrong prefix, a matching directory); a symlink is not followed (asserted with a threshold below the link's own age, plus a control file proving the sweep really was active — otherwise the assertion would pass on the age check and prove nothing); a missing directory is a silent no-op; a ReadDir failure is logged not fatal; a non-positive maxAge falls back to the default; and an unremovable file is logged and skipped without failing the sweep (Unix, via a read-only parent, skipped as root). Plus round-trip tests that a real os.CreateTemp name matches the sweep's matcher, that a real download lands in the swept directory under a swept name, and that the updates directory is a subdirectory of the data directory and is not the shared temp directory.

go test ./..., go vet ./..., golangci-lint run ./... all clean; GOOS=windows and GOOS=linux cross-builds clean.

Not probed: the integration workflow scenario itself. It needs GitHub-hosted runners plus the org's IT_* vars and secrets, so it cannot be exercised locally — it wants a CI run on this PR before merge. It is an assertion scenario, not an enforcement gate: it can produce a false green (if the fixture or restart step silently no-ops) but cannot let a violation through elsewhere. The behaviour it asserts is the same behaviour the live probe above demonstrated in the real binary, one directory over.

Probe Command

go build -o /tmp/agent_smith ./cmd/agent_smith
T="$(dirname "$(mktemp -u)")"
for n in installer-900000001.bin installer-900000002.bin vendor-installer.bin; do
  dd if=/dev/zero of="$T/$n" bs=1m count=4 2>/dev/null
done
touch -t "$(date -v-48H +%Y%m%d%H%M)" "$T/installer-900000001.bin" "$T/vendor-installer.bin"
cat > /tmp/probe-config.json <<'JSON'
{"device_id":"probe-device","shared_access_key":"dGVzdC1zaGFyZWQta2V5LXRoYXQtaXMtbG9uZy1lbm91Z2gtZm9yLWJhc2U2NC1kZWNvZGluZw==","azure_iot_hub_host":"invalid.probe.local","rewst_org_id":"probe-org","rewst_engine_host":"invalid.probe.local","logging_level":"debug","disable_auto_updates":true,"disable_agent_postback":true}
JSON
/tmp/agent_smith --config-file /tmp/probe-config.json --log-file /tmp/probe.log --org-id probe-org &
sleep 6; kill %1
grep -i installer /tmp/probe.log; ls -l "$T"/installer-*.bin "$T"/vendor-installer.bin

Expect: installer-900000001.bin gone, the other two present, one Swept stale installer files ... removed=1 line.

Notes

Failing to create the updates directory now fails the update, where previously the download would have gone to temp. This adds no new failure mode on a read-only data directory: the update flow already rewrites config.json and the agent already writes its log there, so such an endpoint could not update before this change either.

…lling

Every successful auto-update leaked its installer. Download creates the file
with os.CreateTemp("", "installer-*.bin") and its deferred cleanup removes it
only on the failure path -- success = true deliberately skips the removal so the
installer can be executed -- and nothing removes it afterwards: Update spawns it
detached and returns, and no sweep covered installer-*.bin. This is the same
accumulating-temp-file leak SweepStaleScripts fixed for exec-*.ps1 in sc-103967;
the installer files were not covered.

Each leaked file is a full agent binary, so a long-lived endpoint quietly
accumulates hundreds of megabytes. On the space-constrained systems where that
matters most -- thin VDI images, small VM system disks, appliances -- a full
temp volume is not just an agent problem: it breaks Windows Installer,
application logging, and anything else needing scratch space, and the agent's
own next update then fails because os.CreateTemp cannot allocate.

The download path cannot clean up after itself, because the process that would
delete the file is the one the installer replaces. So the fix is a startup
sweep, plus a move that makes the sweep safe:

Downloads now land in <data directory>/updates, created 0700, instead of the
shared system temp directory. The sweep only ever runs against a directory this
agent created, a full agent binary is no longer left executable and
world-readable, and endpoints that mount /tmp noexec can execute the installer
at all. Uninstall already removes the data directory wholesale, so nothing is
left behind by the move.

SweepStaleInstallers runs once per process at service startup, after the service
reports itself running, and removes installer binaries older than 24 hours. A
successful update restarts the agent, so each start reclaims the previous
update's installer while leaving the current one -- far younger than the
threshold -- alone; steady state is a single file rather than one per update.
The legacy shared temp directory is swept too, so an upgraded endpoint reclaims
what it has accumulated since it was installed rather than only stopping the
growth from here on.

The matcher is the conservative one SweepStaleScripts established: regular files
only (never symlinks or device nodes), the exact installer-<digits>.bin shape
os.CreateTemp produces, and nothing under the age threshold -- which is what
makes it safe to point at a directory shared with the rest of the system. The
os.CreateTemp pattern is a shared constant used by both the download and the
sweep, so the two cannot drift. Every failure is logged and skipped; the sweep
never returns an error, because housekeeping must not block the agent from
starting.
Exercise the startup sweep end to end on all three platforms: plant an aged
installer binary (plus a fresh one and a similarly named file the agent never
creates) in the org's updates directory, restart, and assert only the agent's
own stale download is reclaimed and the removal is logged.

The fixture is planted rather than produced by a real update cycle: the sweep is
what this ticket changed, and where the download lands is covered by unit tests
that assert both halves share one pattern constant. Ageing is the only simulated
part, so the production 24h threshold stays in play and no test-only override is
needed in the agent. The files are megabytes rather than bytes so the reclaimed
space shows in the directory listing the fixture prints.

Only the org's own updates directory is asserted here. The sweep also covers the
legacy shared temp directory older agents downloaded into, but that is the
service account's temp directory -- notably root's private TMPDIR on macOS, not
the runner's -- so a fixture planted from the runner would land somewhere the
service never reads. That half is covered by
TestExecute_SweepsStaleInstallerFilesOnStartup, which runs Execute in-process
and can see its own temp directory.

New action, installer-sweep-fixture: seed/assert the file fixture elevated,
since the updates directory lives under the installation's data directory and
belongs to the service account.
@mlataza
mlataza merged commit 8428a0f into main Aug 13, 2026
25 checks passed
@mlataza
mlataza deleted the feature/sc-106111/sweep-downloaded-installer-binaries-so-temp branch August 13, 2026 15:14
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