#32 Upgrade hardening: temp download sweep, extension cleanup on rollback, snapshot restore guard Pull request opened by javuto Upgrade hardening: temp download sweep, extension cleanup on rollback, snapshot restore guard Follow-up to #31. Small hardening items found in the final review of the osquery upgrade change. No new routes or wire fields, and no change to the upgrade flow. Problem • Leaked downloads: a package download is written to a temp file in the system temp directory and ignores cancellation. If osctrld is killed during a download, tens of MB are left behind and nothing cleans them up. • Extension files left after a rollback: the upgrade engine detects files a package adds to the extensions directory (

config_changed

) and rolls back, but the rollback did not remove those files. • Snapshot restore trusted its own file: the autoload path is validated with

checkAutoloadPath

when the snapshot is built, but a restore from a persisted

snapshot.json

trusted the recorded path. • Confirmation loop boundary: after a 429 once the confirmation window had run out, the wait became zero and the loop could send a few extra polls before returning. •

install

ignored certificate checks: the

install

command downloaded from the official package server with

appConfig.Insecure

when

--insecure

was set. What changes Stale download sweep • Download temp files are now named

osctrld-pkg-*

.

sweepStaleDownloads(maxAge)

(default one hour, a var for tests) removes files in the temp directory that match that prefix. A file is removed only if all of these hold: • it is a regular file (checked with

Lstat

); • it is owned by the current user; • it is at least

maxAge

old. • It never follows symlinks or recurses into directories, removes through the existing safe helper, and leaves the upgrade stash directory alone. • It runs once when the service starts and before each upgrade's downloads. • It does not run on Windows, where file ownership cannot be checked. Rollback removes files a package added to the extensions directory • After the previous package is installed and the snapshot is restored, the rollback removes regular files directly in the extensions directory that are not in the snapshot's listing and whose change time is not before the upgrade's start minus one minute. Symlinks, subdirectories and listed files are never touched. • It uses the inode change time, not the modification time, because dpkg, rpm and the macOS installer set mtimes from the package payload (before the upgrade started). The helper is

fileChangeTime

, with Linux and macOS implementations and a fall back to mtime elsewhere. It sits behind a seam for tests. • Removal errors are logged and never abort the rollback. Snapshot restore re-checks the autoload path • Snapshot entries now record which one is the autoload file (

autoload

, omitted when false so old files load). Restore re-runs

checkAutoloadPath

for it and, if the check fails, logs a warning that names the path and skips only that entry. • For snapshots written before the field existed, every entry that is not the flagfile, secret or certificate path is checked as the autoload entry. That is stricter than matching the exact autoload path, so a tampered entry cannot be restored unchecked. Smaller fixes •

confirmNewVersion

returns as soon as the window has run out, and returns the context error instead of nil so a cancellation at that moment still resumes the upgrade rather than rolling back. •

ensureOsquery

(the

install

command) verifies the certificate when it downloads from the default official URL, even with

--insecure

.

--insecure

still applies when

--osquery-package

points to a mirror, and osctrl calls are unchanged. • The README documents the sweep, the rollback cleanup and the

install

certificate rule. Security and operational impact • The two new places where root removes files are narrow: • Sweep: one directory, files named

osctrld-pkg-*

, owned by the current user, regular files only. • Rollback cleanup: one directory, regular files not in the pre-upgrade listing and newer than the upgrade. • Both go through the existing

Lstat

-then-unlink helper. With a sticky temp directory, a planted file is owned by someone else and fails the owner check. • The snapshot restore can no longer be steered to write an arbitrary path through a modified

snapshot.json

. It still requires the root-only upgrade directory. • Package downloads from the official server always verify its certificate, in both

install

and upgrades. Known limits (deferred) • The rollback removal window widens if an upgrade stays pending for days: an operator-added file dropped into the extensions directory in that time can be removed when it finally resumes. Fresh files are only removed if they are not in the listing. • On BSDs the change-time helper falls back to mtime, so the old weaker rule applies there. Windows upgrades are off. • A packaged file that overwrites a name already in the listing is not reverted by the rollback cleanup (it is still detected as

config_changed

). • The Linux change-time path was compiled, vetted and linted for linux amd64, arm64 and 386, but its tests have not been run on a Linux host. Validation • `golangci-lint run ./...`: 0 issues • `go test ./... -count=1`: passes (also with

-race

on the upgrade, package and install tests) •

go vet ./...

for linux,

GOOS=windows

and `GOOS=darwin`;

gofmt -l cmd

clean • New tests, each seen failing first: • Sweep: the new temp name; stale files removed and fresh ones kept; other names, symlinks, a matching directory and the stash directory left alone; it runs before an upgrade's downloads and at service start. • Rollback cleanup: a file the package added (with a 24-hour-old mtime but a fresh change time) is removed; listed files, a symlink with its target, a subdirectory, and a file whose change time is old (faked through the seam) are kept; a removal failure is only logged and the rollback completes;

fileChangeTime

reports a fresh change time after the mtime is set back 24 hours. • Confirmation loop: a deterministic poll count when 429s arrive after the window has run out. • Snapshot restore: the autoload entry is recorded; a tampered entry (new and legacy format) is skipped, with its content, mode and owner untouched and a warning naming the path; a valid entry and an old snapshot without the field still restore. • `install`: against a self-signed server, the default URL is refused with no package request, and an override URL still works with

--insecure

. jmpsec/osctrld