Cwcow policy changes - #2842
Open
Takuro Sato (takuro-sato) wants to merge 2 commits into
Open
Conversation
Takuro Sato (takuro-sato)
marked this pull request as ready for review
August 6, 2026 16:27
Takuro Sato (takuro-sato)
force-pushed
the
cwcow-policy-changes
branch
from
August 18, 2026 14:45
cd6a0ec to
6f9c1ac
Compare
Contributor
|
Takuro Sato (@takuro-sato) are all the commits necessary? can you clean them up (e.g. rebase drop/squash/edit messages)? if we squash merge, the message is going to be way too long. |
Takuro Sato (takuro-sato)
force-pushed
the
cwcow-policy-changes
branch
from
September 3, 2026 15:56
6f9c1ac to
02a8eee
Compare
Contributor
Author
|
Maksim An (@anmaxvl) I squashed the branch into a single commit with a much shorter message. I considered splitting it into logical commits, but the changes are too tightly coupled to keep each commit buildable. |
Apply policy decisions to container creation, exec, environment, stdio, storage, mounts, registry changes, and CIM lifecycle operations. Validate forwarded requests and unsupported fields, maintain policy state consistently, and fail closed when host operations fail. Co-authored-by: Mahati Chamarthy <mahati.chamarthy@gmail.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Takuro Sato <takurosato@microsoft.com>
Takuro Sato (takuro-sato)
force-pushed
the
cwcow-policy-changes
branch
from
September 4, 2026 10:05
02a8eee to
f265533
Compare
Maksim An (anmaxvl)
requested review from
KenGordon and
Tingmao Wang (micromaomao)
September 4, 2026 17:29
Contributor
|
Tingmao Wang (@micromaomao) , KenGordon review policy changes, I'll take a look at sidecar code. |
Tingmao Wang (micromaomao)
approved these changes
Sep 7, 2026
Contributor
Author
|
Maksim An (@anmaxvl) Ken asked Tingmao to review this on his behalf (also he knows overrall direction of the changes and I think he is happy with it). Can we merge it before he is back once you are happy with the changes? (cc KenGordon ) |
There was a problem hiding this comment.
🟡 Changes recommended
escapeRegoString currently does not correctly escape control characters, which can produce invalid Rego for certain registry key/value inputs and should be fixed before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR expands Windows (CWCOW) security policy enforcement across the pkg/securitypolicy Rego framework and the internal/gcs-sidecar runtime enforcement layer, adding new enforcement points (mapped directories, CIM unmount), strengthening host/guest cross-checks, and wiring policy “dropping” semantics (env + registry changes) through to what the sidecar actually forwards/applies.
Changes:
- Add/extend securitypolicy enforcer APIs and Rego rules for mapped-directory mount/unmount, CIM unmount, Windows mount matching, and registry-changes narrowing/dropping.
- Update policy marshalling/tooling to carry new config flags and Windows-specific allow-lists (mapped directories, registry changes, mounts).
- Harden the gcs-sidecar to (a) fail closed on forwarded mount/unmount failures, (b) reconcile host-forwarded create settings against enforced OCI spec, and (c) apply policy outputs (kept env/stdio/registry changes) to forwarded requests.
File summaries
| File | Description |
|---|---|
| test/pkg/securitypolicy/policy.go | Enables AllowRegistryChangesDropping in test policy helpers. |
| pkg/securitypolicy/windows_tooling_test.go | Updates Marshal*Policy callsites for new parameter. |
| pkg/securitypolicy/securitypolicyenforcer.go | Extends enforcer interface (mapped dirs, CIM unmount, registry changes result). |
| pkg/securitypolicy/securitypolicyenforcer_rego.go | Implements new enforcement points and registry-changes “kept” result handling. |
| pkg/securitypolicy/securitypolicy.go | Adds config flag + Windows mapped-directory/registry-change container constraints. |
| pkg/securitypolicy/securitypolicy_marshal.go | Threads new flag, emits new policy objects (mapped dirs, registry changes), adds string escaping helper. |
| pkg/securitypolicy/securitypolicy_internal.go | Adds internal representations for Windows mounts + registry changes. |
| pkg/securitypolicy/regopolicy_windows_test.go | Adds coverage for Windows mount policy, CIM unmount, mapped dirs, registry narrowing/dropping behavior. |
| pkg/securitypolicy/regopolicy_linux_test.go | Threads new marshal parameter through Linux tests. |
| pkg/securitypolicy/rego_utils_test.go | Extends generators/helpers (mapped dirs, CIM volume GUID, new flag). |
| pkg/securitypolicy/policy.rego | Wires new framework enforcement point names. |
| pkg/securitypolicy/opts.go | Adds WithAllowRegistryChangesDropping. |
| pkg/securitypolicy/open_door.rego | Allows new enforcement points in open-door policy. |
| pkg/securitypolicy/framework.rego | Implements CIM volume tracking + unmount rule, Windows mount enforcement, mapped dirs, registry narrowing/dropping, plus improved errors. |
| pkg/securitypolicy/api.rego | Registers new enforcement points and introduced versions. |
| internal/tools/securitypolicy/main.go | Threads new config flag into tooling marshal calls. |
| internal/hcs/schema2/virtual_machine.go | Formatting/alignment change only. |
| internal/guest/runtime/hcsv2/uvm.go | Ensures security-context dir is written whenever a security policy is present. |
| internal/gcs-sidecar/host.go | Adds UVM fail-closed state, container root tracking, and termination tracking. |
| internal/gcs-sidecar/handlers.go | Enforces new rules, reconciles host-forwarded config vs enforced spec, applies env/stdio filtering, adds storage/mount cross-checks. |
| internal/gcs-sidecar/handlers_test.go | Adds tests for fail-closed gating, response monitoring, env/stdio rewriting, root-in-use/mounted protections, etc. |
| internal/gcs-sidecar/bridge.go | Adds monitoring of forwarded mount/unmount responses and fail-closed behavior on inbox failures. |
Review details
Suppressed comments (1)
pkg/securitypolicy/regopolicy_windows_test.go:2017
- This TODO is non-actionable ("maybe delete") and will likely linger. Either remove it or replace it with a stable rationale for why the test belongs here.
// TODO: maybe delete it if it's too much.
- Files reviewed: 21/22 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+541
to
+545
| func escapeRegoString(s string) string { | ||
| s = strings.ReplaceAll(s, `\`, `\\`) | ||
| s = strings.ReplaceAll(s, `"`, `\"`) | ||
| return s | ||
| } |
| // value is dropped when create(cmd) narrows to A first). With dropping off, a | ||
| // request is only allowed if a matched container authorizes every requested | ||
| // value, so registry(dangerous) against A is denied outright. | ||
| // TODO: maybe delete it if it's too much. |
| if len(nonDefaultValues) > 0 { | ||
| log.G(ctx).Tracef("Validating %d registry values against policy", len(nonDefaultValues)) | ||
| // Reject HostedSystem Container fields we don't yet support. | ||
| if err := denyUnsupportedContainerFields(container); err != nil { |
Contributor
There was a problem hiding this comment.
this is a recurring complain from me that more and more policy enforcement logic is "leaking" into sidecar
| } | ||
| // The security-context dir must always be written; it must not be gated | ||
| // by a host-controlled annotation. | ||
| securityContextDir, err := b.hostState.securityOptions.WriteSecurityContextDir(&spec) |
Contributor
There was a problem hiding this comment.
LCOW side calls to "HasSecurityPolicy", we don't have similar on wcow side?
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Takuro Sato (takuro-sato)
force-pushed
the
cwcow-policy-changes
branch
from
September 9, 2026 15:41
ca469e5 to
9ff8f46
Compare
Move Windows policy adaptation into the securitypolicy package and gate supplemental confidential-container setup on an encoded workload policy while preserving closed-door enforcement. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Takuro Sato <takurosato@microsoft.com>
Takuro Sato (takuro-sato)
force-pushed
the
cwcow-policy-changes
branch
from
September 9, 2026 18:07
9ff8f46 to
d3ca085
Compare
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.
Changes
Misc changes