feat(devicepolicy): enforce VS Code private-marketplace URL + add MDM verify-only channel#178
Merged
ashishkurmi merged 5 commits intoJul 24, 2026
Conversation
Fold an optional extensions.gallery.serviceUrl into the existing ide_extension#vscode reconciler, written into the user-scope settings.json beside extensions.allowed via one atomic multi-key write. The agent owns both keys: set is authoritative, removal is ownership-gated (never deletes a user-configured value), and drift, convergence, selective clear, and post-write rollback all cover both. - api: lift gallery_service_url from run-config into EffectivePolicy - settings_writer: managedSettingsWriter (ReadManaged / ApplyManaged / RestoreManaged) over a set of keyed ops; single-key Write/Read/Clear kept - reconcile: ownership-gated set/remove/preserve for the gallery key; convergence over both keys before the idempotency short-circuit; drift over owned keys; atomic multi-key rollback; per-key ownership-safe clear - cache: additive WrittenSettings map (omitempty; an allowlist-only record is unchanged on disk) - probe: yield mdm_managed when an MDM owns AllowedExtensions OR ExtensionGalleryServiceUrl (Windows / macOS / Linux) Additive and regression-guarded: an allowlist-only policy writes exactly as before (byte-identical settings.json and ownership record).
…tings Move the VS Code allowlist's ownership record from the scalar WrittenValue into the WrittenSettings map alongside the gallery URL, so every managed key is tracked uniformly by setting id. WrittenValue is now used only by the single-key (npm) writer; adds omitempty so managed records no longer carry a dead written_value field. No behavior change: ownership is read back from the same values the agent writes.
… map) The run-config policy payload is now a settings map (setting id → compiled value) instead of the bare extensions.allowed object plus a sibling gallery_service_url field; the hash covers the whole map. enforce and clear are fully driven by the settings map and recorded ownership — no per-key special-casing — so a new managed setting rides through with no code change. Fetch now requires extensions.allowed to be present and a JSON object. Deleted managedKeys/managedGalleryValue; added compactSettings/sortedUnion/sortedKeys. Breaking run-config wire change (pre-GA): requires agent-api to emit the settings-map shape.
…xtension policy
Enforcement selects a channel per cycle: "dmg" (or "") is the write-and-verify
path; "mdm" is verify-only — the agent probes the OS-managed VS Code policy and
reports what it observed, and never writes, patches, or clears settings.json
(an external MDM owns the policy).
- ProbeManagedContent (per-OS) reads AllowedExtensions and
ExtensionGalleryServiceUrl into the observed bag keyed by VS Code setting id:
- windows: registry, gating on the types VS Code honors (REG_SZ / REG_MULTI_SZ;
REG_EXPAND_SZ and others are ignored, matching vscode-policy-watcher).
- darwin: managed-preferences plist, machine-wide overlaid by the console
user's per-user plist (user resolved via $HOME to match the settings writer).
- linux: /etc/vscode/policy.json; malformed/null values are verification_failed,
never a silent absence.
- Compliance reports carry observed + evaluated_enforcement (the normalized,
canonical channel) so the backend can diff like-for-like.
- Channel routing is case/space-insensitive; unknown channels fail safe to the
DMG write path.
- The presence probe (DMG-mode yield) also gates on REG_SZ/REG_MULTI_SZ, so a
wrong-typed policy value that VS Code ignores no longer suppresses enforcement.
raysubham
changed the title
There was a problem hiding this comment.
Pull request overview
Extends the internal/devicepolicy VS Code IDE extension reconciler to manage the full VS Code enterprise policy surface (extension allowlist + private marketplace URL) and introduces a second enforcement channel: DMG (write-and-verify) vs MDM (verify-only) with per-OS policy probing.
Changes:
- Generalizes VS Code policy enforcement from a single managed key to a multi-key settings map (atomic merge/patch/write + per-key ownership).
- Adds verify-only
"mdm"enforcement routing, including per-OS managed-policy content probes and reporting of observed values. - Updates run-config parsing and compliance reporting wire shapes to carry the settings map + observed evidence + evaluated enforcement.
Reviewed changes
Copilot reviewed 19 out of 20 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/devicepolicy/settings_writer.go | Adds multi-key managed settings API (read/apply/restore) and gallery URL setting key support. |
| internal/devicepolicy/settings_writer_test.go | Adds regression tests for multi-key writer behavior (bytes preserved, presence semantics, restore, invalid inputs). |
| internal/devicepolicy/reconcile.go | Adds enforcement routing (DMG vs MDM), multi-key enforcement/clear logic, and report stamping. |
| internal/devicepolicy/reconcile_test.go | Expands reconcile tests for multi-key ownership/drift/rollback and MDM verify-only reporting behavior. |
| internal/devicepolicy/probe.go | Adds gallery policy name, shared policy-name detection list, and shared observed-bag building/parsing helpers. |
| internal/devicepolicy/probe_windows.go | Updates Windows managed-policy presence probe (type-gated) and adds registry content probing for MDM verify-only. |
| internal/devicepolicy/probe_windows_test.go | Adds Windows tests for type gating, per-key hive fallback, and verify-only content probe behavior. |
| internal/devicepolicy/probe_test.go | Adds cross-platform tests for “either policy name” detection and buildObserved translation/error handling. |
| internal/devicepolicy/probe_other.go | Adds no-op stub for ProbeManagedContent on unsupported OSes. |
| internal/devicepolicy/probe_linux.go | Extends Linux presence probe to either key and adds policy.json content parsing for verify-only. |
| internal/devicepolicy/probe_linux_test.go | Adds Linux-only tests for presence detection and content parsing/error semantics. |
| internal/devicepolicy/probe_darwin.go | Extends macOS presence probe to either key and adds managed-preferences plist parsing for verify-only. |
| internal/devicepolicy/probe_darwin_test.go | Adds macOS-only tests for presence detection and plist content probing/precedence/user resolution. |
| internal/devicepolicy/cache.go | Extends cached applied state to include per-key ownership (written_settings) alongside legacy written_value. |
| internal/devicepolicy/cache_test.go | Adds tests for written_settings round-trip and omission behavior. |
| internal/devicepolicy/api.go | Changes effective policy policy to a settings map, adds enforcement, and adds observed/enforcement fields to compliance report. |
| internal/devicepolicy/api_test.go | Updates fetch/report tests for new settings-map shape, allowlist requirements, gallery URL lift, and enforcement field handling. |
| go.mod | Adds howett.net/plist dependency for macOS plist parsing in the MDM probe. |
| go.sum | Updates checksums for the new plist dependency and transitive entries. |
| cmd/stepsecurity-dev-machine-guard/main.go | Ensures verify-only enforcement can still run/report even when no settings path exists (nil writer). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+395
to
+418
| switch { | ||
| case op.Set: | ||
| // The patch document embeds the value verbatim; reject anything that | ||
| // is not valid JSON before it can corrupt the patch (defense in depth | ||
| // — the caller passes already-compacted, validated values). Object | ||
| // shape is NOT required: a managed value may be a JSON string (the | ||
| // gallery URL) as well as an object (the allowlist). | ||
| if !json.Valid(op.Value) { | ||
| return nil, fmt.Errorf("devicepolicy: refusing to write invalid JSON value for %q to %s", op.Key, w.path) | ||
| } | ||
| // op.Key is a dotted setting id with no '/' or '~', so it needs no | ||
| // JSON-Pointer escaping; the dot is literal. | ||
| patchOps = append(patchOps, `{"op":"add","path":"/`+op.Key+`","value":`+string(op.Value)+`}`) | ||
| case op.Remove: | ||
| // RFC 6902 "remove" errors on an absent member; pre-check presence so | ||
| // a Remove of a key that is not there is simply skipped. | ||
| _, present, perr := extractKey(v, op.Key) | ||
| if perr != nil { | ||
| return nil, perr | ||
| } | ||
| if present { | ||
| patchOps = append(patchOps, `{"op":"remove","path":"/`+op.Key+`"}`) | ||
| } | ||
| } |
Comment on lines
+154
to
+167
| // Normalize once for routing AND reporting: the backend gates on an exact | ||
| // "mdm"/"dmg", so EvaluatedEnforcement must carry the canonical value, not | ||
| // whatever casing/spacing arrived. | ||
| r.enforcement = strings.ToLower(strings.TrimSpace(ep.Enforcement)) | ||
|
|
||
| // MDM is verify-only and owns nothing on disk, so it routes before the | ||
| // Writer/clear checks below. | ||
| switch r.enforcement { | ||
| case enforcementMDM: | ||
| return r.verifyMDM(ctx, cat, tgt, ep) | ||
| case enforcementDMG, "": | ||
| default: | ||
| r.logf("devicepolicy: unknown enforcement %q; running DMG path", ep.Enforcement) | ||
| } |
Comment on lines
+667
to
+678
| func ownedKeys(prev AppliedTargetState, hadPrev bool) map[string]string { | ||
| owned := map[string]string{} | ||
| if !hadPrev { | ||
| return owned | ||
| } | ||
| for k, v := range prev.WrittenSettings { | ||
| if v != "" { | ||
| owned[k] = v | ||
| } | ||
| } | ||
| return owned | ||
| } |
…nical enforcement channel Address PR review comments: - ApplyManaged built the RFC-6902 patch path by raw string concat with the backend-supplied key. RFC 6901-escape the pointer token (~->~0, /->~1) and JSON-encode the path (jsonPointerPath) so an unusual key cannot forge a nested pointer path or corrupt the patch document. The value is still spliced verbatim (never re-encoded, which would reorder members or HTML-escape it). - EvaluatedEnforcement reported the raw request, so an unknown channel ran the DMG path yet reported the raw string. Resolve and stamp the canonical channel the cycle actually ran -- always "dmg" or "mdm" (empty/unknown -> "dmg") -- so the backend's exact-match gate sees the channel that executed.
ashishkurmi
approved these changes
Jul 24, 2026
raysubham
added a commit
to raysubham/dev-machine-guard
that referenced
this pull request
Jul 25, 2026
Brings in the merged IDE-extension policy work (step-security#178) that the npm lane's reconciler, probe, and api layers build on. reconcile.go conflicted because both lanes changed the enforce ladder; resolved by keeping both paths on the shared seams - marker-based clearing for the ~/.npmrc block writer, the multi-key managed path for VS Code settings.
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.
Summary
Extends the
ide_extension#vscodereconciler to cover the full VS Code enterprise-policy surface — the extension allowlist (extensions.allowed) and the private-marketplace URL (extensions.gallery.serviceUrl) — delivered through two enforcement channels:enforcement: "dmg"or empty). The agent writes both keys into user-scopesettings.jsonin one atomic merge, tracks per-key ownership, and reports compliance.enforcement: "mdm"), the agent never writes; it probes the OS-managed policy, reports what it observed, and lets the backend decide drift.Additive and regression-guarded: an allowlist-only DMG policy compiles, writes, and records ownership exactly as before (byte-identical
settings.jsonand cache record).Part 1 — Private-marketplace URL enforcement (DMG)
Folds
extensions.gallery.serviceUrlinto the reconciler beside the allowlist, and generalizes the model from a single key to a settings map (setting id → compiled value; the hash covers the whole map):extensions.allowedis required and must be a JSON object. (Breaking run-config wire change, pre-GA: requires agent-api to emit the settings-map shape.)managedSettingsWriterseam —ReadManaged/ApplyManaged/RestoreManagedover a set of keyed ops (one atomic load→patch→store). Single-keyWrite/Read/Clearretained for the npm writer.WrittenSettings(map,omitempty→ an allowlist-only record is unchanged on disk);WrittenValuenow used only by the single-key npm writer.Semantics: URL present → set authoritatively (overwrite whatever is there); URL absent → remove only the value the agent wrote (a user-configured URL is preserved);
clear:true→ remove each owned key independently; VS Code adopts a changed gallery at next restart while the agent convergessettings.jsonimmediately (file state is the contract).Part 2 — MDM verify-only channel
enforcementselects the channel per cycle."mdm"is verify-only — the agent probes the OS-managed VS Code policy and reports what it observed, and never writes, patches, or clearssettings.json(an external MDM owns it). It routes before the writer/clear gates, so it runs even on a platform with no settings path.ProbeManagedContent, per-OS) readsAllowedExtensionsandExtensionGalleryServiceUrlinto the observed bag keyed by VS Code setting id:REG_SZ/REG_MULTI_SZonly. AREG_EXPAND_SZ/DWORDvalue is dropped byvscode-policy-watcher(confirmed against its source:StringPolicysupportedTypes = {REG_SZ, REG_MULTI_SZ}, wrong type →nullopt), so it reads as unmanaged in DMG mode (agent enforces via write) andverification_failedin MDM mode. The presence probe gates on the same set, so a wrong-typed value no longer suppresses enforcement.howett.net/plist), machine-wide overlaid by the console user's per-user plist. The user is resolved via$HOMEto match the settings writer (a root LaunchDaemon bakes$HOME→console user but leaves$USER=root)./etc/vscode/policy.json; a malformed ornullvalue isverification_failed, never a silent absence.observed+evaluated_enforcement(the normalized, canonical channel) so the backend can diff like-for-like. Channel routing is case/space-insensitive; an unknown channel fails safe to the DMG write path.howett.net/plist v1.0.1— pure-Go plist parsing for the darwin content probe (CGO stays off).Testing
go test ./internal/devicepolicy/ -race— DMG writer goldens + reconcile transitions (unchanged), plus the MDM path: verify-only states (clear/absent no-op, probe error →verification_failed, not-present →policy_not_applied, present →mdm_managed+ observed), verify runs with a nil writer, case-insensitive routing with canonical echo, and per-OS content probes (darwin native; linux + windows fixtures, incl. WindowsREG_EXPAND_SZ/DWORD rejection and unsupported-HKLM→valid-HKCU fallthrough).gofmt -lclean ·go vet(darwin/linux/windows) ·golangci-lint0 issues (×3 GOOS) · cross-compileCGO_ENABLED=0linux/amd64 + darwin/arm64 + windows/amd64 (andinternal/devicepolicyfreebsd) ·go mod tidyno-diff. Windows-tagged probe tests execute in CI (Test (windows devicepolicy)).No version bump or CHANGELOG entry (handled at release cut).