Generalize the boot security upgrade into profile::boot_security_upgrade - #294
Merged
Conversation
The symptom is fleet-wide, not runner-specific: a fresh instance comes up
unpatched, unattended-upgrades is on a timer, and AWS Inspector scans the gap
and opens a finding. The finding closes on the next unattended-upgrades run,
but it has already REOPENED its vulnerability group by then -- and a group old
enough to be reopened that way breaks the remediation SLA.
Lift the runner-specific script and exec into an opt-in profile any role can
include, and have profile::github_runner consume it.
Beyond patching early, the script now drops the InspectorEc2Exclusion tag when
it is done, so Inspector's first scan sees an already-patched host and the
window never exists. Removal is best effort and never fails the run: it needs
ec2:DeleteTags on the instance profile and the launch-time tag from Terraform,
and neither is in place fleet-wide yet. A host with neither is no worse off
than before. The tag is dropped whether or not the upgrade succeeded, since
leaving it on a host that failed to patch would hide a genuinely vulnerable
instance from Inspector.
fail_on_error defaults to false (log and continue) because an unpatched-but-
running host beats an ABANDONed one on anything stateful. github_runner passes
true to keep today's semantics, which are right for a disposable runner.
logoutput => true makes the "log" half of that real -- the default on_failure
prints nothing for a status we just declared a success.
Also fixed while generalizing:
- the retry loop hot-spun through the last 15s of budget instead of sleeping,
hammering the dpkg lock; it now sleeps min(remaining, interval)
- cloud-guest-utils and awscli are declared and required rather than probed
for at runtime, so the exec cannot evaluate before them on a first boot and
silently no-op the tag drop
Verified on a live sandbox instance: upgrade succeeded on attempt 1, the tag
call returned clean (so ec2:DeleteTags is already granted there), and the
second catalog apply skipped the exec on the creates marker.
Development only; sandbox and modules/ follow once this settles.
infrahouse8
approved these changes
Aug 21, 2026
akuzminsky
added a commit
that referenced
this pull request
Aug 21, 2026
Copies #294 verbatim from environments/development into environments/sandbox and the global modules/ tree, so all three environments carry an identical profile module again. No behavior change from what ran in development: the same opt-in profile, the same best-effort InspectorEc2Exclusion drop, and profile::github_runner consuming it with budget 480 / fail_on_error true. The runner-specific gha-boot-security-upgrade.sh is removed from both trees. Validated in development before promoting: a full instance refresh completed at 100% (i-0028068b3d44d1b4b launched, i-0d969d2f86041003a terminated), with the upgrade succeeding on attempt 1 and the tag call returning clean. modules/ has no profile::apt_lock_timeout yet, so it was worth confirming the global tree satisfies this profile on its own: catalog compiles against modulepath "modules" alone, and against both environment paths, produce an identical exec -- same command, same creates marker, and all four require edges resolving including Package[awscli] and Package[cloud-guest-utils].
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
The symptom is fleet-wide, not runner-specific. A fresh instance comes up unpatched,
unattended-upgradesis on a timer, and AWS Inspector scans the gap and opens a finding. The finding closes on the nextunattended-upgradesrun — but it has already reopened its vulnerability group by then, and a group old enough to be reopened that way breaks the remediation SLA.So the fix isn't "patch more often on a timer", it's "don't present an unpatched host to Inspector at all".
What
Lifts the runner-specific script and exec out of
profile::github_runnerintoprofile::boot_security_upgrade, an opt-in profile any role can include. Not wired intoprofile::base— adding it to a role stays a deliberate decision about that role's provisioning time.Beyond patching early, the script now drops the
InspectorEc2Exclusiontag when it's done, so Inspector's first scan sees an already-patched host.budget480fail_on_errorfalseexclusion_tagInspectorEc2ExclusionTag removal is best effort
It needs
ec2:DeleteTagson the instance profile and the launch-time tag from Terraform, and neither is in place fleet-wide yet. Every failure path — no permission, noec2metadata, unreadable instance id — logs a distinct line and returns 0. A host with neither is no worse off than before this PR; it just keeps being scanned on Inspector's own schedule.The tag is dropped whether or not the upgrade succeeded. Leaving it on a host that failed to patch would hide a genuinely vulnerable instance from Inspector, which inverts the point.
Failure semantics
fail_on_errordefaults tofalsebecause an unpatched-but-running host beats an ABANDONed one on anything stateful.profile::github_runnerpassestrueto keep today's behavior, which is right for a disposable runner the ASG will just replace.logoutput => trueis what makes the "log" half real — the defaulton_failureprints nothing for a status we just declared a success.Fixed while generalizing
min(remaining, interval).cloud-guest-utilsandawscliare declared and required rather than probed for withcommand -v. Without the ordering edge a first-boot exec can evaluate before the packages and silently no-op the tag drop — on exactly the boot it matters most.createsreplacesunless => "test -f ..."— no shell fork per run, no dependency ontestbeing onpath, and absolute-path validation at compile time.Verification
Live sandbox instance (
i-00e8ef380bdd1b0dc, env development):The second catalog apply in that same run has no
Exec[boot-security-upgrade]line at all — thecreatesmarker suppressed it. The tag call returning clean also meansec2:DeleteTagsis already granted in that account.Also: puppet-lint clean on all four targets;
puppet parser validateplus catalog compiles across four parameter variants checking the renderedcommand/returns/timeout/createsand all fourrequireedges; script exercised over ~15 cases in a noble container (success, give-up, mid-budget recovery, permission denied, non-EC2, missing binaries, arg-parsing errors); shellcheck clean.Scope
Development only.
environments/sandboxandmodules/still carry the old runner-specific script — they follow once this settles.Not covered here
The Terraform side still has to tag instances with
InspectorEc2Exclusionat launch.delete-tagsignores a key that isn't there, so the sandbox run above proves the permission exists but not that the tag was ever set. Until that lands, this profile patches early — useful on its own — but doesn't yet close the Inspector window.🤖 Generated with Claude Code
https://claude.ai/code/session_01KLGAXtoyoakLXUjaJSL4M3