securitypolicy tool: don't add the pause container to open-door policies - #2933
Merged
Maksim An (anmaxvl) merged 1 commit intoSep 15, 2026
Merged
Maksim An (anmaxvl) merged 1 commit into
Maksim An (anmaxvl) merged 1 commit into
Conversation
The non-fragment linux path unconditionally appended helpers.DefaultContainerConfigs() to config.Containers before calling MarshalPolicy. MarshalPolicy rejects allow_all combined with a non-empty container list, so `securitypolicytool -c <cfg> -t rego` with `allow_all = true` always failed with "Invalid policy for open-door enforcer" -- but only after pulling k8s.gcr.io/pause:3.1 and computing its dm-verity root hash, so the failure was both guaranteed and slow. The equivalent windows path does not append the default containers and works correctly today. Move the decision into linuxPolicyContainerConfigs and skip the append when allow_all is set, so an open-door policy enumerates no containers. It now generates in about a second with no registry access. Behaviour for every other config is unchanged, and the fragment path is untouched: allow_all does not apply to fragments, since MarshalFragment takes no allowAll argument. Extracting the helper makes the rule testable; main_test.go locks it in, and also asserts the underlying constraint that an open-door policy must not enumerate containers. Verified that the new test fails against the previous behaviour. Also document open-door generation in the tool README, including that such a policy is stamped with the generating hcsshim's api_version and lists exactly that version's enforcement points, so it must not be checked in as a Base64 literal and reused across versions. Signed-off-by: Veeraiah Chowdary Nuvvula <6164424+veerun14@users.noreply.github.com>
Maksim An (anmaxvl)
approved these changes
Sep 15, 2026
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
2 participants
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.
Problem
internal/tools/securitypolicyunconditionally appendssecuritypolicy.DefaultContainerConfigs()(the pause container) to the container list on the Linux path, even when the input TOML setsallow_all = true.securitypolicy.MarshalPolicyrejects that combination: an open-door policy must have no containers. Sosecuritypolicy -c open-door.toml -t regoalways fails — and it fails after doing the work of resolving and pulling the pause image, so the user waits on a registry round-trip for a guaranteed error.Fix
Skip the default container configs when
allow_allis set. The pause container is only meaningful for a policy that actually enumerates containers; an open-door policy enumerates none by definition.-t fragmentpath is deliberately left alone.MarshalFragmenttakes noallowAllargument, so a fragment can never be open-door and the default configs remain correct there.Testing
The package had no tests. Added
main_test.gocovering the three cases:allow_all = true-> no containers appendedallow_all = false-> pause container appendedallow_all = false-> user containers preserved, pause appendedTo make the rule testable without invoking the whole command, the container-list construction is extracted into
linuxPolicyContainerConfigs. Behaviour is otherwise unchanged.I negative-tested the new test by restoring the original bug: it fails as expected.
End to end,
securitypolicy -c <open-door>.toml -t regonow succeeds and emits a policy atapi_version 0.12.0with 26 rules and no pause container.