diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index ddd0f85..6ff4332 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -96,6 +96,26 @@ jobs: go-version-file: go.mod - run: make test + # devicepolicy has Windows-only code (the registry managed-policy probe) and + # Windows-specific behavior (%APPDATA% settings-path resolution) whose tests + # the macOS test job can only cross-compile, not run. This job executes the + # package natively; scoped to it so the rest of the suite (which assumes a + # POSIX host) is untouched. + test-windows-devicepolicy: + name: Test (windows devicepolicy) + runs-on: windows-latest + steps: + - name: Harden the runner (Audit all outbound calls) + uses: step-security/harden-runner@fa2e9d605c4eeb9fcad4c99c224cee0c6c7f3594 # v2.16.0 + with: + egress-policy: audit + + - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1 + - uses: actions/setup-go@40f1582b2485089dde7abd97c1529aa768e1baff # v5.6.0 + with: + go-version-file: go.mod + - run: go test -race -count=1 ./internal/devicepolicy/ + smoke: name: Smoke Tests runs-on: macos-latest diff --git a/cmd/stepsecurity-dev-machine-guard/main.go b/cmd/stepsecurity-dev-machine-guard/main.go index 4782e69..4fff61f 100644 --- a/cmd/stepsecurity-dev-machine-guard/main.go +++ b/cmd/stepsecurity-dev-machine-guard/main.go @@ -19,6 +19,7 @@ import ( "github.com/step-security/dev-machine-guard/internal/config" "github.com/step-security/dev-machine-guard/internal/detector/configaudit" "github.com/step-security/dev-machine-guard/internal/device" + "github.com/step-security/dev-machine-guard/internal/devicepolicy" "github.com/step-security/dev-machine-guard/internal/executor" "github.com/step-security/dev-machine-guard/internal/featuregate" "github.com/step-security/dev-machine-guard/internal/launchd" @@ -250,6 +251,7 @@ func main() { os.Exit(1) } runHookStateReconcile(exec, log) + runIDEExtensionEnforce(exec, log) case "install": _, _ = fmt.Fprintf(os.Stdout, "StepSecurity Dev Machine Guard v%s\n\n", buildinfo.Version) @@ -302,6 +304,7 @@ func main() { log.Warn("could not trigger initial scan (%v) — the scheduled task will fire on its next interval", err) } runHookStateReconcile(exec, log) + runIDEExtensionEnforce(exec, log) return } @@ -337,6 +340,7 @@ func main() { } } runHookStateReconcile(exec, log) + runIDEExtensionEnforce(exec, log) case "uninstall": _, _ = fmt.Fprintf(os.Stdout, "StepSecurity Dev Machine Guard v%s\n\n", buildinfo.Version) @@ -637,3 +641,69 @@ func runHookStateReconcile(exec executor.Executor, log *progress.Logger) { aiagentscli.AppendError("reconcile", "reconcile_failed", err.Error(), "") } } + +// devicePolicyEnforceTimeout caps the entire IDE-extension enforcement step (fetch + +// managed-policy probe + settings.json write/readback + compliance report). +// The two network calls are each bounded by devicepolicy.DefaultHTTPTimeout; the +// rest is local file/registry I/O. +const devicePolicyEnforceTimeout = 30 * time.Second + +// runIDEExtensionEnforce fetches the device's effective IDE-extension policy +// and converges the user-scope VS Code settings.json (extensions.allowed) to +// match, then reports compliance — all on the existing scheduled cycle and the +// existing agent auth channel. Windows, macOS, and Linux are all enforced this +// way; a device whose VS Code is already governed by a real MDM policy +// (registry / policy.json / managed preferences) is detected by the +// reconciler's probe and reported mdm_managed instead. Gated behind +// FeatureDevicePolicy and a silent no-op in community mode (enterprise +// config missing). Failures are logged but never crash main. +func runIDEExtensionEnforce(exec executor.Executor, log *progress.Logger) { + if !featuregate.IsEnabled(featuregate.FeatureDevicePolicy) { + log.Debug("ide-extension enforce: skipped (feature gated)") + return + } + writer, ok := devicepolicy.NewWriter() + if !ok { + log.Debug("ide-extension enforce: skipped (no settings path on this platform)") + return + } + cfg, ok := ingest.Snapshot() + if !ok { + log.Debug("ide-extension enforce: skipped (no enterprise config)") + return + } + fetcher, ok := devicepolicy.NewHTTPFetcher(cfg, nil) + if !ok { + log.Debug("ide-extension enforce: skipped (fetcher init refused config)") + return + } + reporter, ok := devicepolicy.NewHTTPReporter(cfg, nil) + if !ok { + log.Debug("ide-extension enforce: skipped (reporter init refused config)") + return + } + + ctx, cancel := context.WithTimeout(context.Background(), devicePolicyEnforceTimeout) + defer cancel() + + dev := device.Gather(ctx, exec) + if dev.SerialNumber == "" || dev.SerialNumber == "unknown" { + log.Warn("ide-extension enforce: device serial unresolved; skipping") + return + } + + r := &devicepolicy.Reconciler{ + Fetcher: fetcher, + Reporter: reporter, + Writer: writer, + CustomerID: cfg.CustomerID, + DeviceID: dev.SerialNumber, + Platform: dev.Platform, + // Probe defaults to devicepolicy.ProbeManagedPolicy (per-OS) when nil. + Logf: func(format string, args ...any) { log.Debug(format, args...) }, + } + if err := r.Reconcile(ctx); err != nil { + log.Warn("ide-extension enforce: %v", err) + aiagentscli.AppendError("devicepolicy", "enforce_failed", err.Error(), "") + } +} diff --git a/go.mod b/go.mod index 90a2829..ffd4635 100644 --- a/go.mod +++ b/go.mod @@ -8,6 +8,7 @@ require ( github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 github.com/google/uuid v1.6.0 github.com/pelletier/go-toml/v2 v2.3.1 + github.com/tailscale/hujson v0.0.0-20260302212456-ecc657c15afd github.com/tidwall/gjson v1.18.0 github.com/tidwall/pretty v1.2.1 github.com/tidwall/sjson v1.2.5 diff --git a/go.sum b/go.sum index bf6a559..7880f83 100644 --- a/go.sum +++ b/go.sum @@ -1,9 +1,13 @@ +github.com/google/go-cmp v0.5.8 h1:e6P7q2lk1O+qJJb4BtCQXlK8vWEO8V1ZeuEdJNOqZyg= +github.com/google/go-cmp v0.5.8/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 h1:El6M4kTTCOh6aBiKaUGG7oYTSPP8MxqL4YI3kZKwcP4= github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510/go.mod h1:pupxD2MaaD3pAXIBCelhxNneeOaAeabZDe5s4K6zSpQ= github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/pelletier/go-toml/v2 v2.3.1 h1:MYEvvGnQjeNkRF1qUuGolNtNExTDwct51yp7olPtrEc= github.com/pelletier/go-toml/v2 v2.3.1/go.mod h1:2gIqNv+qfxSVS7cM2xJQKtLSTLUE9V8t9Stt+h56mCY= +github.com/tailscale/hujson v0.0.0-20260302212456-ecc657c15afd h1:Rf9uhF1+VJ7ZHqxrG8pJ6YacmHvVCmByDmGbAWCc/gA= +github.com/tailscale/hujson v0.0.0-20260302212456-ecc657c15afd/go.mod h1:EbW0wDK/qEUYI0A5bqq0C2kF8JTQwWONmGDBbzsxxHo= github.com/tidwall/gjson v1.14.2/go.mod h1:/wbyibRr2FHMks5tjHJ5F8dMZh3AcwJEMf5vlfC0lxk= github.com/tidwall/gjson v1.18.0 h1:FIDeeyB800efLX89e5a8Y0BNH+LOngJyGrIWxG2FKQY= github.com/tidwall/gjson v1.18.0/go.mod h1:/wbyibRr2FHMks5tjHJ5F8dMZh3AcwJEMf5vlfC0lxk= diff --git a/internal/aiagents/adapter/adapter.go b/internal/aiagents/adapter/adapter.go index f94fe6e..5a4c320 100644 --- a/internal/aiagents/adapter/adapter.go +++ b/internal/aiagents/adapter/adapter.go @@ -81,7 +81,7 @@ type InstallResult struct { // BackupFiles are pre-existing files Install copied aside before // rewriting, named with the .dmg-.bak suffix from - // internal/aiagents/atomicfile. + // internal/atomicfile. BackupFiles []string // CreatedDirs are parent directories Install mkdir'd. Order is diff --git a/internal/aiagents/adapter/claudecode/settings.go b/internal/aiagents/adapter/claudecode/settings.go index 63cbf64..17b69d2 100644 --- a/internal/aiagents/adapter/claudecode/settings.go +++ b/internal/aiagents/adapter/claudecode/settings.go @@ -14,9 +14,9 @@ import ( "github.com/tidwall/pretty" "github.com/tidwall/sjson" - "github.com/step-security/dev-machine-guard/internal/aiagents/atomicfile" "github.com/step-security/dev-machine-guard/internal/aiagents/configedit" "github.com/step-security/dev-machine-guard/internal/aiagents/event" + "github.com/step-security/dev-machine-guard/internal/atomicfile" ) // settingsDoc holds raw bytes for ~/.claude/settings.json. orig is the diff --git a/internal/aiagents/adapter/codex/settings.go b/internal/aiagents/adapter/codex/settings.go index 909087e..ccaf9a7 100644 --- a/internal/aiagents/adapter/codex/settings.go +++ b/internal/aiagents/adapter/codex/settings.go @@ -15,9 +15,9 @@ import ( "github.com/tidwall/pretty" "github.com/tidwall/sjson" - "github.com/step-security/dev-machine-guard/internal/aiagents/atomicfile" "github.com/step-security/dev-machine-guard/internal/aiagents/configedit" "github.com/step-security/dev-machine-guard/internal/aiagents/event" + "github.com/step-security/dev-machine-guard/internal/atomicfile" ) const ( diff --git a/internal/aiagents/atomicfile/atomicfile.go b/internal/atomicfile/atomicfile.go similarity index 100% rename from internal/aiagents/atomicfile/atomicfile.go rename to internal/atomicfile/atomicfile.go diff --git a/internal/aiagents/atomicfile/atomicfile_test.go b/internal/atomicfile/atomicfile_test.go similarity index 100% rename from internal/aiagents/atomicfile/atomicfile_test.go rename to internal/atomicfile/atomicfile_test.go diff --git a/internal/devicepolicy/api.go b/internal/devicepolicy/api.go new file mode 100644 index 0000000..3791a33 --- /dev/null +++ b/internal/devicepolicy/api.go @@ -0,0 +1,283 @@ +package devicepolicy + +import ( + "bytes" + "context" + "encoding/json" + "errors" + "fmt" + "io" + "net/http" + "net/url" + "strings" + "time" + + "github.com/step-security/dev-machine-guard/internal/aiagents/ingest" + "github.com/step-security/dev-machine-guard/internal/aiagents/redact" + "github.com/step-security/dev-machine-guard/internal/buildinfo" +) + +// DefaultHTTPTimeout caps a single fetch or report round-trip. Enforcement runs +// off the scheduled tick, not a hot path, so a 5s ceiling is comfortable and +// matches the hook fetcher's budget. +const DefaultHTTPTimeout = 5 * time.Second + +// maxBodyBytes bounds a response read. The compiled extensions.allowed payload +// is well under 1 KiB in practice; 256 KiB is generous slack while still +// bounding a pathological backend. +const maxBodyBytes = 256 * 1024 + +// EffectivePolicy is the parsed FR-19 fetch contract. It is the agent-side +// mirror of agent-api's EffectivePolicyResponse. Policy carries the compiled +// VS Code extensions.allowed object as canonical JSON (sorted keys) — the exact +// bytes the backend hashed — so the agent writes it verbatim and never +// re-serializes (re-serialization could reorder keys and break the backend's +// byte-exact applied==desired check). +type EffectivePolicy struct { + Category string + Clear bool + Policy json.RawMessage + Hash string + GeneratedAt string +} + +// policyEnvelope is the wire shape (must match agent-api +// EffectivePolicyResponse). Unknown fields are ignored, so a backend still +// emitting legacy extras (e.g. the removed min_vscode_version) stays +// compatible. +type policyEnvelope struct { + Category string `json:"category"` + Clear bool `json:"clear"` + Policy json.RawMessage `json:"policy,omitempty"` + Hash string `json:"hash,omitempty"` + GeneratedAt string `json:"generated_at"` +} + +// Fetcher returns the effective policy for one device + category. +type Fetcher interface { + Fetch(ctx context.Context, customerID, deviceID, category string) (EffectivePolicy, error) +} + +// HTTPFetcher is the production Fetcher. Safe for concurrent use. +type HTTPFetcher struct { + endpoint string + apiKey string + http *http.Client +} + +// NewHTTPFetcher builds a Fetcher from the same strict enterprise-config gate +// the upload path uses (ingest.Config). ok=false on incomplete config — the +// caller treats that as "skip enforcement", matching the hook reconciler. +func NewHTTPFetcher(cfg ingest.Config, h *http.Client) (*HTTPFetcher, bool) { + endpoint := strings.TrimSpace(cfg.APIEndpoint) + apiKey := strings.TrimSpace(cfg.APIKey) + if endpoint == "" || apiKey == "" { + return nil, false + } + if h == nil { + h = &http.Client{Timeout: DefaultHTTPTimeout} + } + return &HTTPFetcher{ + endpoint: strings.TrimRight(endpoint, "/"), + apiKey: apiKey, + http: h, + }, true +} + +// Fetch issues GET +// /v1/:customer/developer-mdm-agent/devices/:device_id/effective-policy?category=… +// over the existing agent auth channel (Bearer tenant key). It returns a parsed +// EffectivePolicy or an error. Any error is the reconciler's signal to NO-OP +// (never wipe enforcement on a transient failure or malformed payload): +// - transport / non-200 status → error; +// - body that is not a JSON object → error; +// - a non-clear result missing policy or hash → error (a malformed policy +// must not be written, and must not be mistaken for a clear); +// - a non-clear policy that is not itself a JSON object → error (a string or +// array written verbatim could even read back "compliant"). +func (c *HTTPFetcher) Fetch(ctx context.Context, customerID, deviceID, category string) (EffectivePolicy, error) { + if c == nil { + return EffectivePolicy{}, errors.New("devicepolicy: nil fetcher") + } + if strings.TrimSpace(customerID) == "" { + return EffectivePolicy{}, errors.New("devicepolicy: empty customer_id") + } + if strings.TrimSpace(deviceID) == "" { + return EffectivePolicy{}, errors.New("devicepolicy: empty device_id") + } + if strings.TrimSpace(category) == "" { + category = CategoryIDEExtension + } + + endpoint := c.endpoint + + "/v1/" + url.PathEscape(customerID) + + "/developer-mdm-agent/devices/" + url.PathEscape(deviceID) + + "/effective-policy?category=" + url.QueryEscape(category) + + req, err := http.NewRequestWithContext(ctx, http.MethodGet, endpoint, nil) + if err != nil { + return EffectivePolicy{}, fmt.Errorf("devicepolicy: build request: %w", err) + } + req.Header.Set("Authorization", "Bearer "+c.apiKey) + req.Header.Set("Accept", "application/json") + req.Header.Set("User-Agent", "dmg/"+buildinfo.Version) + + resp, err := c.http.Do(req) + if err != nil { + return EffectivePolicy{}, fmt.Errorf("devicepolicy: transport: %s", redact.String(err.Error())) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + snippet, _ := io.ReadAll(io.LimitReader(resp.Body, maxBodyBytes)) + return EffectivePolicy{}, fmt.Errorf("devicepolicy: unexpected status %d: %s", + resp.StatusCode, redact.String(strings.TrimSpace(string(snippet)))) + } + + body, err := io.ReadAll(io.LimitReader(resp.Body, maxBodyBytes)) + if err != nil { + return EffectivePolicy{}, fmt.Errorf("devicepolicy: read body: %w", err) + } + var env policyEnvelope + if err := json.Unmarshal(body, &env); err != nil { + return EffectivePolicy{}, fmt.Errorf("devicepolicy: decode body: %w", err) + } + + ep := EffectivePolicy{ + Category: strings.TrimSpace(env.Category), + Clear: env.Clear, + Policy: env.Policy, + Hash: strings.TrimSpace(env.Hash), + GeneratedAt: env.GeneratedAt, + } + if ep.Category == "" { + ep.Category = category + } + if !ep.Clear { + if len(ep.Policy) == 0 || ep.Hash == "" { + return EffectivePolicy{}, errors.New("devicepolicy: malformed policy: clear=false but policy or hash missing") + } + // The compiled policy is always a JSON object. Shape is checked here so a + // malformed payload no-ops at the reconciler; value-level validation stays + // backend-owned. + if !isJSONObject(ep.Policy) { + return EffectivePolicy{}, errors.New("devicepolicy: malformed policy: policy is not a JSON object") + } + } + return ep, nil +} + +// isJSONObject reports whether raw's first JSON token opens an object. The +// envelope already passed json.Unmarshal, so raw is syntactically valid JSON — +// only the shape needs checking. +func isJSONObject(raw json.RawMessage) bool { + for _, b := range raw { + switch b { + case ' ', '\t', '\r', '\n': + continue + } + return b == '{' + } + return false +} + +// ComplianceReport is the agent's POST body: the verification result it +// computed on-device. It is the agent-side mirror of agent-api's +// complianceReport. AppliedHash is the backend's hash echoed verbatim — never +// recomputed locally — so the backend's byte-exact applied==desired check +// (which gates the `compliant` verdict) can succeed. +type ComplianceReport struct { + Category string `json:"category"` + State string `json:"state"` + AppliedHash string `json:"applied_hash"` + AgentVersion string `json:"agent_version"` + Platform string `json:"platform"` +} + +// Reporter submits a compliance report for one device. +type Reporter interface { + Report(ctx context.Context, customerID, deviceID string, r ComplianceReport) error +} + +// HTTPReporter is the production Reporter. +type HTTPReporter struct { + endpoint string + apiKey string + http *http.Client +} + +// NewHTTPReporter builds a Reporter from the strict enterprise-config gate. +// ok=false on incomplete config. +func NewHTTPReporter(cfg ingest.Config, h *http.Client) (*HTTPReporter, bool) { + endpoint := strings.TrimSpace(cfg.APIEndpoint) + apiKey := strings.TrimSpace(cfg.APIKey) + if endpoint == "" || apiKey == "" { + return nil, false + } + if h == nil { + h = &http.Client{Timeout: DefaultHTTPTimeout} + } + return &HTTPReporter{ + endpoint: strings.TrimRight(endpoint, "/"), + apiKey: apiKey, + http: h, + }, true +} + +// Report issues POST +// /v1/:customer/developer-mdm-agent/devices/:device_id/compliance over the +// existing agent auth channel — a dedicated endpoint, NOT the telemetry +// payload. The backend rejects an unregistered device_id (400) and records the +// per-device state; it computes desired_hash itself and decides compliant vs +// pending. A non-2xx is returned as an error for the caller to log. +func (c *HTTPReporter) Report(ctx context.Context, customerID, deviceID string, r ComplianceReport) error { + if c == nil { + return errors.New("devicepolicy: nil reporter") + } + if strings.TrimSpace(customerID) == "" { + return errors.New("devicepolicy: empty customer_id") + } + if strings.TrimSpace(deviceID) == "" { + return errors.New("devicepolicy: empty device_id") + } + if r.Category == "" { + r.Category = CategoryIDEExtension + } + + body, err := json.Marshal(r) + if err != nil { + return fmt.Errorf("devicepolicy: marshal report: %w", err) + } + + endpoint := c.endpoint + + "/v1/" + url.PathEscape(customerID) + + "/developer-mdm-agent/devices/" + url.PathEscape(deviceID) + + "/compliance" + + req, err := http.NewRequestWithContext(ctx, http.MethodPost, endpoint, bytes.NewReader(body)) + if err != nil { + return fmt.Errorf("devicepolicy: build request: %w", err) + } + req.Header.Set("Authorization", "Bearer "+c.apiKey) + req.Header.Set("Content-Type", "application/json") + req.Header.Set("Accept", "application/json") + req.Header.Set("User-Agent", "dmg/"+buildinfo.Version) + + resp, err := c.http.Do(req) + if err != nil { + return fmt.Errorf("devicepolicy: transport: %s", redact.String(err.Error())) + } + defer resp.Body.Close() + + if resp.StatusCode < 200 || resp.StatusCode >= 300 { + snippet, _ := io.ReadAll(io.LimitReader(resp.Body, maxBodyBytes)) + return fmt.Errorf("devicepolicy: unexpected status %d: %s", + resp.StatusCode, redact.String(strings.TrimSpace(string(snippet)))) + } + _, _ = io.Copy(io.Discard, io.LimitReader(resp.Body, maxBodyBytes)) + return nil +} + +// AgentVersion returns the running agent version reported in compliance +// payloads. Centralized here so the report and any diagnostics agree. +func AgentVersion() string { return buildinfo.Version } diff --git a/internal/devicepolicy/api_test.go b/internal/devicepolicy/api_test.go new file mode 100644 index 0000000..11826f2 --- /dev/null +++ b/internal/devicepolicy/api_test.go @@ -0,0 +1,203 @@ +package devicepolicy + +import ( + "context" + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/step-security/dev-machine-guard/internal/aiagents/ingest" +) + +func newFetchServer(t *testing.T, status int, body string) (*httptest.Server, *HTTPFetcher) { + t.Helper() + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if got := r.Header.Get("Authorization"); got != "Bearer test-key" { + t.Errorf("Authorization = %q, want Bearer test-key", got) + } + if got := r.URL.Query().Get("category"); got != CategoryIDEExtension { + t.Errorf("category = %q, want %q", got, CategoryIDEExtension) + } + if !strings.Contains(r.URL.Path, "/developer-mdm-agent/devices/dev-1/effective-policy") { + t.Errorf("unexpected path: %s", r.URL.Path) + } + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(status) + _, _ = w.Write([]byte(body)) + })) + t.Cleanup(srv.Close) + + f, ok := NewHTTPFetcher(ingest.Config{APIEndpoint: srv.URL, APIKey: "test-key"}, srv.Client()) + if !ok { + t.Fatal("NewHTTPFetcher returned ok=false on valid config") + } + return srv, f +} + +func TestFetchPolicy(t *testing.T) { + // min_vscode_version is no longer part of the contract; it stays in the + // fixture to prove a backend still emitting legacy fields is tolerated. + body := `{"category":"ide_extension","clear":false,` + + `"policy":{"*":false,"ms-python.python":true},` + + `"hash":"sha256:abc","min_vscode_version":"1.96.0","generated_at":"2026-06-08T00:00:00Z"}` + _, f := newFetchServer(t, 200, body) + ep, err := f.Fetch(context.Background(), "cust", "dev-1", CategoryIDEExtension) + if err != nil { + t.Fatalf("Fetch: %v", err) + } + if ep.Clear { + t.Fatal("clear should be false") + } + if ep.Hash != "sha256:abc" { + t.Fatalf("hash = %q", ep.Hash) + } + // Policy must round-trip as the canonical bytes the backend sent. + if got := string(ep.Policy); !strings.Contains(got, `"ms-python.python":true`) { + t.Fatalf("policy = %s", got) + } +} + +func TestFetchClear(t *testing.T) { + _, f := newFetchServer(t, 200, `{"category":"ide_extension","clear":true,"generated_at":"2026-06-08T00:00:00Z"}`) + ep, err := f.Fetch(context.Background(), "cust", "dev-1", CategoryIDEExtension) + if err != nil { + t.Fatalf("Fetch: %v", err) + } + if !ep.Clear { + t.Fatal("clear should be true") + } +} + +func TestFetchMalformedBodyIsError(t *testing.T) { + _, f := newFetchServer(t, 200, `not json`) + if _, err := f.Fetch(context.Background(), "cust", "dev-1", CategoryIDEExtension); err == nil { + t.Fatal("malformed body must be an error (→ reconciler no-op)") + } +} + +func TestFetchNonClearMissingPolicyIsError(t *testing.T) { + // clear=false but no policy/hash → malformed; must not be written or mistaken + // for a clear. + _, f := newFetchServer(t, 200, `{"category":"ide_extension","clear":false,"generated_at":"x"}`) + if _, err := f.Fetch(context.Background(), "cust", "dev-1", CategoryIDEExtension); err == nil { + t.Fatal("non-clear result missing policy/hash must be an error") + } +} + +func TestFetchNonObjectPolicyIsError(t *testing.T) { + // A policy that is not a JSON object must never reach the writer: written + // verbatim it could even read back as "compliant". + for _, body := range []string{ + `{"category":"ide_extension","clear":false,"policy":"bad","hash":"sha256:x","generated_at":"x"}`, + `{"category":"ide_extension","clear":false,"policy":[],"hash":"sha256:x","generated_at":"x"}`, + `{"category":"ide_extension","clear":false,"policy":42,"hash":"sha256:x","generated_at":"x"}`, + `{"category":"ide_extension","clear":false,"policy":null,"hash":"sha256:x","generated_at":"x"}`, + } { + _, f := newFetchServer(t, 200, body) + if _, err := f.Fetch(context.Background(), "cust", "dev-1", CategoryIDEExtension); err == nil { + t.Fatalf("non-object policy must be an error, body: %s", body) + } + } +} + +func TestFetchNon200IsError(t *testing.T) { + _, f := newFetchServer(t, 500, `{"error":"boom"}`) + if _, err := f.Fetch(context.Background(), "cust", "dev-1", CategoryIDEExtension); err == nil { + t.Fatal("5xx should propagate as error") + } +} + +func TestFetchEmptyIDsAreErrors(t *testing.T) { + _, f := newFetchServer(t, 200, `{"clear":true,"generated_at":"x"}`) + if _, err := f.Fetch(context.Background(), "", "dev-1", CategoryIDEExtension); err == nil { + t.Fatal("empty customer should error") + } + if _, err := f.Fetch(context.Background(), "cust", "", CategoryIDEExtension); err == nil { + t.Fatal("empty device should error") + } +} + +func TestNewHTTPFetcherRejectsIncompleteConfig(t *testing.T) { + if _, ok := NewHTTPFetcher(ingest.Config{APIEndpoint: "", APIKey: "k"}, nil); ok { + t.Fatal("missing endpoint should yield ok=false") + } + if _, ok := NewHTTPFetcher(ingest.Config{APIEndpoint: "https://x", APIKey: ""}, nil); ok { + t.Fatal("missing api key should yield ok=false") + } +} + +func TestReportPostsToComplianceEndpoint(t *testing.T) { + var gotPath, gotAuth, gotMethod string + var gotBody ComplianceReport + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotPath = r.URL.Path + gotAuth = r.Header.Get("Authorization") + gotMethod = r.Method + b, _ := io.ReadAll(r.Body) + _ = json.Unmarshal(b, &gotBody) + w.WriteHeader(200) + _, _ = w.Write([]byte(`{"message":"compliance recorded"}`)) + })) + t.Cleanup(srv.Close) + + rep, ok := NewHTTPReporter(ingest.Config{APIEndpoint: srv.URL, APIKey: "test-key"}, srv.Client()) + if !ok { + t.Fatal("NewHTTPReporter ok=false on valid config") + } + err := rep.Report(context.Background(), "cust", "dev-1", ComplianceReport{ + Category: CategoryIDEExtension, State: StateCompliant, AppliedHash: "sha256:abc", + AgentVersion: "1.13.0", Platform: "windows", + }) + if err != nil { + t.Fatalf("Report: %v", err) + } + if gotMethod != http.MethodPost { + t.Fatalf("method = %s, want POST", gotMethod) + } + if !strings.Contains(gotPath, "/developer-mdm-agent/devices/dev-1/compliance") { + t.Fatalf("path = %s", gotPath) + } + if gotAuth != "Bearer test-key" { + t.Fatalf("auth = %q", gotAuth) + } + if gotBody.State != StateCompliant || gotBody.AppliedHash != "sha256:abc" { + t.Fatalf("body = %+v", gotBody) + } + if gotBody.Category != CategoryIDEExtension || gotBody.Platform != "windows" { + t.Fatalf("body = %+v", gotBody) + } +} + +func TestReportNon2xxIsError(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(400) + _, _ = w.Write([]byte(`{"error":"unknown device for this customer"}`)) + })) + t.Cleanup(srv.Close) + rep, _ := NewHTTPReporter(ingest.Config{APIEndpoint: srv.URL, APIKey: "k"}, srv.Client()) + if err := rep.Report(context.Background(), "cust", "dev-1", ComplianceReport{State: StateCompliant}); err == nil { + t.Fatal("400 should propagate as error") + } +} + +func TestReportDefaultsCategory(t *testing.T) { + var gotCategory string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + b, _ := io.ReadAll(r.Body) + var body ComplianceReport + _ = json.Unmarshal(b, &body) + gotCategory = body.Category + w.WriteHeader(200) + })) + t.Cleanup(srv.Close) + rep, _ := NewHTTPReporter(ingest.Config{APIEndpoint: srv.URL, APIKey: "k"}, srv.Client()) + if err := rep.Report(context.Background(), "cust", "dev-1", ComplianceReport{State: StateCompliant}); err != nil { + t.Fatalf("Report: %v", err) + } + if gotCategory != CategoryIDEExtension { + t.Fatalf("category should default to %q, got %q", CategoryIDEExtension, gotCategory) + } +} diff --git a/internal/devicepolicy/cache.go b/internal/devicepolicy/cache.go new file mode 100644 index 0000000..ad3f649 --- /dev/null +++ b/internal/devicepolicy/cache.go @@ -0,0 +1,292 @@ +package devicepolicy + +import ( + "encoding/json" + "os" + "path/filepath" + "sync" + "time" +) + +// CacheFilename is the basename of the enforcement state file. It lives under +// ~/.stepsecurity/ alongside config.json and hooks-state.json, and is distinct +// from the AI-agent hook cache (this is a separate subsystem — no shared state). +const CacheFilename = "device-policy-state.json" + +// CacheSchemaVersion is the on-disk version of the state file. Bump only on a +// breaking shape change. +const CacheSchemaVersion = 1 + +const ( + cacheFileMode os.FileMode = 0o600 + cacheParentDirMode os.FileMode = 0o700 +) + +// AppliedStateFile is the on-disk shape: a schema-versioned wrapper keyed by +// category, so multiple enforcement categories share one file without forcing a +// future migration. Exactly one category (ide_extension) is populated today. +// +// { +// "schema_version": 1, +// "categories": { +// "ide_extension": { "applied_hash": …, "written_value": …, "fetched_at": … } +// } +// } +type AppliedStateFile struct { + SchemaVersion int `json:"schema_version"` + Categories map[string]AppliedCategoryState `json:"categories"` +} + +// AppliedCategoryState records what the agent last wrote to the user-scope VS +// Code settings.json for one category. The category is the map key in +// AppliedStateFile, not a field here. Two fields drive correctness: +// +// - AppliedHash is the backend's content hash, stored VERBATIM (never +// recomputed). Compared against the freshly-fetched hash for idempotency. +// - WrittenValue is the exact compacted extensions.allowed value the agent +// wrote. It drives value-based ownership and drift: on a clear, the agent +// removes the settings key only if the on-disk value still equals +// WrittenValue (a differing value — e.g. the user's own — is left +// untouched); on enforce, an on-disk value differing from WrittenValue is +// drift and is converged back. +// +// An absent category key — or a zero-value entry — means "the agent owns +// nothing on disk" for that category. +type AppliedCategoryState struct { + AppliedHash string `json:"applied_hash"` + WrittenValue string `json:"written_value"` + FetchedAt time.Time `json:"fetched_at"` +} + +// cacheMu serializes the read-modify-write of the shared state file so two +// in-process category writers cannot lose each other's update. It does NOT make +// the file safe across separate agent PROCESSES — that still relies on +// atomic-rename last-writer-wins, and a cross-process lock (flock/LockFileEx) +// would be needed before categories are reconciled concurrently or multiple +// agents run against more than one category. +// +// The lock is NOT reentrant: helpers that already hold it use the unlocked +// readStateFile / persistStateFile, never the public ReadAppliedState / +// WriteAppliedState / ClearAppliedState. +var cacheMu sync.Mutex + +// cachePathOverride lets tests redirect reads/writes to a tempdir. Production +// leaves it empty. Same pattern as state.cachePathOverride. +var cachePathOverride string + +// SetCachePathForTest redirects CachePath() to the given absolute path and +// returns a restore function. Test-only. +func SetCachePathForTest(p string) (restore func()) { + prev := cachePathOverride + cachePathOverride = p + return func() { cachePathOverride = prev } +} + +// CachePath returns the absolute state-file path, honoring the test override. +// Empty string means the home directory could not be resolved. +func CachePath() string { + if cachePathOverride != "" { + return cachePathOverride + } + home, err := os.UserHomeDir() + if err != nil || home == "" { + return "" + } + return filepath.Join(home, ".stepsecurity", CacheFilename) +} + +// readStatus classifies a state file for the read-modify-write callers. +type readStatus int + +const ( + // stateReadable: the file parsed and its schema is this build's or older. + stateReadable readStatus = iota + // stateAbsentOrCorrupt: missing, unreadable, or not a JSON object. Safe to + // recreate from scratch. + stateAbsentOrCorrupt + // stateFuture: a cleanly-parsed file from a NEWER agent (schema_version + // beyond this build). Must NOT be overwritten — its category metadata can't + // be interpreted, and clobbering it would strand a newer agent's ownership. + stateFuture +) + +// peekSchemaVersion extracts schema_version without committing to the full +// shape. ok=false when b is not a JSON object (corrupt); a JSON object with no +// schema_version field yields (0, true). This is what separates a "future" +// file (parseable object, high version → refuse) from a "corrupt" one (not an +// object → recreate). +func peekSchemaVersion(b []byte) (version int, ok bool) { + var probe struct { + SchemaVersion int `json:"schema_version"` + } + if err := json.Unmarshal(b, &probe); err != nil { + return 0, false + } + return probe.SchemaVersion, true +} + +// readStateFile loads and classifies the state file. UNLOCKED: callers that +// also write hold cacheMu and call this (never the public ReadAppliedState), +// because cacheMu is not reentrant. On stateReadable, Categories is non-nil. +func readStateFile() (AppliedStateFile, readStatus) { + path := CachePath() + if path == "" { + return AppliedStateFile{}, stateAbsentOrCorrupt + } + // #nosec G304 -- path is CachePath(): a test override or os.UserHomeDir() + // joined with the package constant CacheFilename. Never external input. + b, err := os.ReadFile(path) + if err != nil { + return AppliedStateFile{}, stateAbsentOrCorrupt + } + ver, ok := peekSchemaVersion(b) + if !ok { + // Not a JSON object — corrupt. Safe to recreate. + return AppliedStateFile{}, stateAbsentOrCorrupt + } + // Refuse a file from a newer agent. A schema beyond what this build knows + // may reuse fields with changed meaning; the reader falls back to "owns + // nothing" and the writer refuses to clobber it. + if ver > CacheSchemaVersion { + return AppliedStateFile{}, stateFuture + } + var f AppliedStateFile + if err := json.Unmarshal(b, &f); err != nil { + return AppliedStateFile{}, stateAbsentOrCorrupt + } + // A 0 version predates the field (or was hand-written); persistStateFile + // always stamps it, so a genuine file from this agent is never 0. A legacy + // single-object file parses here with no "categories" key → empty map → + // "owns nothing" for every category (one harmless re-apply, by design). + if f.SchemaVersion == 0 { + f.SchemaVersion = CacheSchemaVersion + } + if f.Categories == nil { + f.Categories = map[string]AppliedCategoryState{} + } + return f, stateReadable +} + +// ReadAppliedState returns the agent's recorded ownership for one category: +// (state, true) when a record exists, else (zero, false). It never surfaces an +// error — a missing/corrupt file, or one written by a newer agent +// (schema_version beyond this build's CacheSchemaVersion), simply means "no +// recorded ownership". The reconciler treats that as owning nothing: safe, +// because it then refuses to clear a value it has no record of writing and +// re-applies the policy. +func ReadAppliedState(category string) (AppliedCategoryState, bool) { + cacheMu.Lock() + defer cacheMu.Unlock() + + f, status := readStateFile() + if status != stateReadable { + return AppliedCategoryState{}, false + } + s, ok := f.Categories[category] + return s, ok +} + +// WriteAppliedState records ownership for one category, PRESERVING every other +// category already in the file (read-modify-write), then atomically replaces +// the file (temp + sync + rename). It REFUSES to overwrite a file written by a +// newer agent (errFutureSchema) rather than clobber category metadata it cannot +// interpret. A missing or corrupt file is recreated. +func WriteAppliedState(category string, s AppliedCategoryState) error { + cacheMu.Lock() + defer cacheMu.Unlock() + + f, status := readStateFile() + switch status { + case stateFuture: + return errFutureSchema + case stateAbsentOrCorrupt: + f = AppliedStateFile{Categories: map[string]AppliedCategoryState{}} + } + if f.Categories == nil { + f.Categories = map[string]AppliedCategoryState{} + } + f.Categories[category] = s + return persistStateFile(f) +} + +// ClearAppliedState drops one category's ownership record, PRESERVING the rest, +// then atomically rewrites the file. Same future-schema refusal as +// WriteAppliedState. A missing or corrupt file — or an already-absent category +// — is a no-op (nothing recorded to drop). +func ClearAppliedState(category string) error { + cacheMu.Lock() + defer cacheMu.Unlock() + + f, status := readStateFile() + switch status { + case stateFuture: + return errFutureSchema + case stateAbsentOrCorrupt: + return nil + } + if _, ok := f.Categories[category]; !ok { + return nil + } + delete(f.Categories, category) + return persistStateFile(f) +} + +// persistStateFile stamps the current schema version and atomically writes the +// file, creating the parent dir with 0o700 and the file with 0o600. UNLOCKED — +// callers hold cacheMu. +func persistStateFile(f AppliedStateFile) error { + f.SchemaVersion = CacheSchemaVersion + if f.Categories == nil { + f.Categories = map[string]AppliedCategoryState{} + } + path := CachePath() + if path == "" { + return errNoHomeDir + } + data, err := json.MarshalIndent(f, "", " ") + if err != nil { + return err + } + data = append(data, '\n') + + parent := filepath.Dir(path) + if err := os.MkdirAll(parent, cacheParentDirMode); err != nil { + return err + } + + tmp, err := os.CreateTemp(parent, "."+CacheFilename+".tmp-*") + if err != nil { + return err + } + tmpPath := tmp.Name() + defer func() { + if _, statErr := os.Stat(tmpPath); statErr == nil { + _ = os.Remove(tmpPath) + } + }() + + if _, err := tmp.Write(data); err != nil { + _ = tmp.Close() + return err + } + if err := tmp.Sync(); err != nil { + _ = tmp.Close() + return err + } + if err := tmp.Close(); err != nil { + return err + } + if err := os.Chmod(tmpPath, cacheFileMode); err != nil { + return err + } + return os.Rename(tmpPath, path) +} + +type cacheError string + +func (e cacheError) Error() string { return string(e) } + +const ( + errNoHomeDir = cacheError("devicepolicy: cannot resolve home directory") + errFutureSchema = cacheError("devicepolicy: refusing to overwrite a newer-schema state file") +) diff --git a/internal/devicepolicy/cache_test.go b/internal/devicepolicy/cache_test.go new file mode 100644 index 0000000..a99ae4d --- /dev/null +++ b/internal/devicepolicy/cache_test.go @@ -0,0 +1,248 @@ +package devicepolicy + +import ( + "encoding/json" + "errors" + "os" + "path/filepath" + "testing" + "time" +) + +func TestAppliedCategoryRoundTrip(t *testing.T) { + dir := t.TempDir() + restore := SetCachePathForTest(filepath.Join(dir, CacheFilename)) + defer restore() + + want := AppliedCategoryState{ + AppliedHash: "sha256:abc", + WrittenValue: samplePolicy, + FetchedAt: time.Date(2026, 6, 8, 0, 0, 0, 0, time.UTC), + } + if err := WriteAppliedState(CategoryIDEExtension, want); err != nil { + t.Fatalf("WriteAppliedState: %v", err) + } + got, ok := ReadAppliedState(CategoryIDEExtension) + if !ok { + t.Fatal("ReadAppliedState ok=false after write") + } + if got.AppliedHash != want.AppliedHash || got.WrittenValue != want.WrittenValue { + t.Fatalf("got %+v, want %+v", got, want) + } + // On disk it is the schema-versioned wrapper keyed by category. + raw, err := os.ReadFile(CachePath()) + if err != nil { + t.Fatal(err) + } + var f AppliedStateFile + if err := json.Unmarshal(raw, &f); err != nil { + t.Fatalf("on-disk file is not a valid AppliedStateFile: %v", err) + } + if f.SchemaVersion != CacheSchemaVersion { + t.Fatalf("schema_version = %d, want %d", f.SchemaVersion, CacheSchemaVersion) + } + if _, ok := f.Categories[CategoryIDEExtension]; !ok { + t.Fatalf("category %q missing from on-disk wrapper: %+v", CategoryIDEExtension, f) + } +} + +func TestReadAbsentFileOwnsNothing(t *testing.T) { + restore := SetCachePathForTest(filepath.Join(t.TempDir(), "nope.json")) + defer restore() + if _, ok := ReadAppliedState(CategoryIDEExtension); ok { + t.Fatal("absent cache should yield ok=false") + } +} + +func TestReadCorruptFileOwnsNothing(t *testing.T) { + path := filepath.Join(t.TempDir(), CacheFilename) + if err := os.WriteFile(path, []byte("not json"), 0o600); err != nil { + t.Fatal(err) + } + restore := SetCachePathForTest(path) + defer restore() + if _, ok := ReadAppliedState(CategoryIDEExtension); ok { + t.Fatal("corrupt cache should yield ok=false (owns nothing)") + } +} + +func TestReadFutureSchemaOwnsNothing(t *testing.T) { + path := filepath.Join(t.TempDir(), CacheFilename) + // A wrapper written by a newer agent: a schema beyond what this build + // understands. It decodes fine, but its category metadata may mean something + // else, so the reader must refuse it rather than drive ownership/drift off it. + future := `{"schema_version":999,"categories":{"ide_extension":{"applied_hash":"sha256:x","written_value":"{}","fetched_at":"2026-06-08T00:00:00Z"}}}` + if err := os.WriteFile(path, []byte(future), 0o600); err != nil { + t.Fatal(err) + } + restore := SetCachePathForTest(path) + defer restore() + if _, ok := ReadAppliedState(CategoryIDEExtension); ok { + t.Fatal("future schema_version must be unreadable (ok=false) so the agent owns nothing") + } +} + +func TestReadMissingSchemaReadsAsCurrent(t *testing.T) { + path := filepath.Join(t.TempDir(), CacheFilename) + // No schema_version field (legacy or hand-written) but the wrapper shape: + // read it, normalized to the current version — not rejected. + noVer := `{"categories":{"ide_extension":{"applied_hash":"sha256:x","written_value":"{}","fetched_at":"2026-06-08T00:00:00Z"}}}` + if err := os.WriteFile(path, []byte(noVer), 0o600); err != nil { + t.Fatal(err) + } + restore := SetCachePathForTest(path) + defer restore() + got, ok := ReadAppliedState(CategoryIDEExtension) + if !ok { + t.Fatal("missing schema_version should read as current, not be rejected") + } + if got.AppliedHash != "sha256:x" { + t.Fatalf("applied_hash = %q, want sha256:x", got.AppliedHash) + } +} + +func TestReadAbsentCategoryOwnsNothing(t *testing.T) { + restore := SetCachePathForTest(filepath.Join(t.TempDir(), CacheFilename)) + defer restore() + // The file exists and holds one category; a DIFFERENT category owns nothing. + if err := WriteAppliedState("other_category", AppliedCategoryState{WrittenValue: "x"}); err != nil { + t.Fatal(err) + } + if _, ok := ReadAppliedState(CategoryIDEExtension); ok { + t.Fatal("a category with no entry should yield ok=false even when the file exists") + } +} + +func TestWritePreservesOtherCategories(t *testing.T) { + restore := SetCachePathForTest(filepath.Join(t.TempDir(), CacheFilename)) + defer restore() + + other := AppliedCategoryState{AppliedHash: "sha256:OTHER", WrittenValue: "other-value"} + if err := WriteAppliedState("other_category", other); err != nil { + t.Fatal(err) + } + if err := WriteAppliedState(CategoryIDEExtension, AppliedCategoryState{AppliedHash: "sha256:H", WrittenValue: samplePolicy}); err != nil { + t.Fatal(err) + } + // Writing ide_extension must not disturb other_category. + got, ok := ReadAppliedState("other_category") + if !ok || got.AppliedHash != other.AppliedHash || got.WrittenValue != other.WrittenValue { + t.Fatalf("other category not preserved across a sibling write: got %+v ok=%v", got, ok) + } +} + +func TestWriteRefusesFutureSchemaFile(t *testing.T) { + path := filepath.Join(t.TempDir(), CacheFilename) + future := `{"schema_version":999,"categories":{"future_only":{"applied_hash":"sha256:z","written_value":"{}","fetched_at":"2026-06-08T00:00:00Z"}}}` + "\n" + if err := os.WriteFile(path, []byte(future), 0o600); err != nil { + t.Fatal(err) + } + restore := SetCachePathForTest(path) + defer restore() + + err := WriteAppliedState(CategoryIDEExtension, AppliedCategoryState{WrittenValue: samplePolicy}) + if !errors.Is(err, errFutureSchema) { + t.Fatalf("write over a future-schema file must refuse with errFutureSchema, got %v", err) + } + after, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if string(after) != future { + t.Fatalf("future-schema file must be left byte-identical; got %q", string(after)) + } +} + +func TestClearRemovesOnlyTargetCategory(t *testing.T) { + restore := SetCachePathForTest(filepath.Join(t.TempDir(), CacheFilename)) + defer restore() + + if err := WriteAppliedState("keep_me", AppliedCategoryState{WrittenValue: "keep"}); err != nil { + t.Fatal(err) + } + if err := WriteAppliedState(CategoryIDEExtension, AppliedCategoryState{WrittenValue: samplePolicy}); err != nil { + t.Fatal(err) + } + if err := ClearAppliedState(CategoryIDEExtension); err != nil { + t.Fatalf("ClearAppliedState: %v", err) + } + if _, ok := ReadAppliedState(CategoryIDEExtension); ok { + t.Fatal("cleared category should be gone") + } + if got, ok := ReadAppliedState("keep_me"); !ok || got.WrittenValue != "keep" { + t.Fatalf("untouched category must survive a sibling clear: got %+v ok=%v", got, ok) + } +} + +func TestClearReclaimsEmptyCategoryRecord(t *testing.T) { + restore := SetCachePathForTest(filepath.Join(t.TempDir(), CacheFilename)) + defer restore() + + // An empty-ownership entry, as a preflight leaves when its settings write + // then fails: present in the file but with no value/hash. + if err := WriteAppliedState(CategoryIDEExtension, AppliedCategoryState{FetchedAt: time.Unix(0, 0).UTC()}); err != nil { + t.Fatal(err) + } + if err := WriteAppliedState("keep_me", AppliedCategoryState{WrittenValue: "keep"}); err != nil { + t.Fatal(err) + } + // The empty entry is still a present key (ok=true) — the reconciler's + // entry-exists drop is what reclaims it. + if _, ok := ReadAppliedState(CategoryIDEExtension); !ok { + t.Fatal("empty-ownership entry should be a present key") + } + if err := ClearAppliedState(CategoryIDEExtension); err != nil { + t.Fatalf("ClearAppliedState: %v", err) + } + if _, ok := ReadAppliedState(CategoryIDEExtension); ok { + t.Fatal("empty category record should be reclaimed by clear") + } + if got, ok := ReadAppliedState("keep_me"); !ok || got.WrittenValue != "keep" { + t.Fatalf("sibling category must survive: got %+v ok=%v", got, ok) + } +} + +func TestClearRefusesFutureSchemaFile(t *testing.T) { + path := filepath.Join(t.TempDir(), CacheFilename) + future := `{"schema_version":999,"categories":{"future_only":{"applied_hash":"sha256:z"}}}` + "\n" + if err := os.WriteFile(path, []byte(future), 0o600); err != nil { + t.Fatal(err) + } + restore := SetCachePathForTest(path) + defer restore() + + if err := ClearAppliedState(CategoryIDEExtension); !errors.Is(err, errFutureSchema) { + t.Fatalf("clear over a future-schema file must refuse with errFutureSchema, got %v", err) + } + after, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if string(after) != future { + t.Fatalf("future-schema file must be left byte-identical; got %q", string(after)) + } +} + +func TestClearAbsentFileIsNoOp(t *testing.T) { + restore := SetCachePathForTest(filepath.Join(t.TempDir(), CacheFilename)) + defer restore() + if err := ClearAppliedState(CategoryIDEExtension); err != nil { + t.Fatalf("clearing an absent file should be a no-op, got %v", err) + } +} + +func TestLegacySingleObjectReadsAsOwnsNothing(t *testing.T) { + path := filepath.Join(t.TempDir(), CacheFilename) + // The pre-refactor single-object shape (also schema_version 1). It parses as + // a wrapper with no "categories" key → empty map → owns nothing → one + // harmless re-apply. We deliberately do NOT migrate it. + legacy := `{"schema_version":1,"category":"ide_extension","applied_hash":"sha256:x","written_value":"{}","fetched_at":"2026-06-08T00:00:00Z"}` + if err := os.WriteFile(path, []byte(legacy), 0o600); err != nil { + t.Fatal(err) + } + restore := SetCachePathForTest(path) + defer restore() + if _, ok := ReadAppliedState(CategoryIDEExtension); ok { + t.Fatal("legacy single-object file should read as owns-nothing (no migration)") + } +} diff --git a/internal/devicepolicy/doc.go b/internal/devicepolicy/doc.go new file mode 100644 index 0000000..df9c823 --- /dev/null +++ b/internal/devicepolicy/doc.go @@ -0,0 +1,34 @@ +// Package devicepolicy implements the dev-machine-guard agent side of Developer MDM +// on-device policy enforcement (PRD: "Dev Machine Guard Agent: IDE Extension +// Enforcement"). It is a thin agent: each scheduled cycle it fetches the +// backend-compiled policy and converges the `extensions.allowed` key in the +// user-scope VS Code settings.json to it (format-preserving single-key JSONC +// merge), reads it back to verify, and reports a compliance state. VS Code +// itself performs the disabling — the agent never uninstalls extensions, +// never installs anything, and never touches non-VS-Code IDEs. +// +// This subsystem shares NO code or state with the AI-agent hook-policy feature +// in internal/aiagents (PRD N11). The backend computes the compiled +// extensions.allowed object and a content hash; the agent applies it verbatim +// (compacted for canonical comparison) and never re-implements allow/deny +// merging, so on-device and exported enforcement stay at parity. +// +// Scope: Windows, macOS, and Linux, all through the same settings.json writer +// — only the per-OS path differs (%APPDATA%\Code\User, ~/Library/Application +// Support/Code/User, ~/.config/Code/User). Devices whose VS Code is already +// governed by a real MDM/admin policy at the OS policy location (HKLM/HKCU +// registry, /etc/vscode/policy.json, macOS managed preferences) are detected +// by a read-only probe and reported mdm_managed — such a policy outranks user +// settings inside VS Code, so the agent yields rather than writing a value +// that would be ignored. +// +// Seams (highest first), each independently testable: +// - Verify (verify.go): pure {write_ok, readback_match} → state. +// - Writer (settings_writer.go): injected; manages only the +// extensions.allowed key, preserving the rest of the user's settings. +// - Probe (probe.go + per-OS files): read-only managed-policy presence. +// - Fetcher / Reporter (api.go): the two dedicated endpoints on the +// existing developer-mdm-agent auth channel. +// - Reconciler (reconcile.go): orchestrates fetch → probe → idempotency → +// drift → ownership-safe write → verify → report, with malformed-→-no-op. +package devicepolicy diff --git a/internal/devicepolicy/probe.go b/internal/devicepolicy/probe.go new file mode 100644 index 0000000..687065b --- /dev/null +++ b/internal/devicepolicy/probe.go @@ -0,0 +1,58 @@ +package devicepolicy + +import ( + "bytes" + "encoding/json" + "os" +) + +// allowedExtensionsName is VS Code's registered POLICY name for the +// `extensions.allowed` setting — the registry value name on Windows, the JSON +// key in /etc/vscode/policy.json on Linux, and the plist key in macOS managed +// preferences. Policy locations are keyed by policy name; the user settings +// file the agent writes is keyed by the setting id +// (allowedExtensionsSettingKey). The probes below only ever READ policy +// locations: a policy present there outranks user settings inside VS Code, so +// the agent yields (mdm_managed) instead of writing a settings value that +// would be ignored. +const allowedExtensionsName = "AllowedExtensions" + +// ProbeManagedPolicy (probe_.go) reports whether an MDM/admin-managed +// AllowedExtensions policy is present at this OS's policy location, plus a +// human-readable location for logs. Read-only, never elevated, and +// best-effort: an unreadable location reads as "not managed" — enforcement +// must not be blocked by a probe that cannot decide. + +// jsonFileHasKey reports whether the JSON object file at path contains key as +// a top-level member. Falls back to a byte scan for `"key"` when the file is +// not a parseable JSON object (an MDM may have written something lenient) — +// over-detecting yields a safe mdm_managed, under-detecting would fight the +// MDM. A missing or unreadable file is false. +func jsonFileHasKey(path, key string) bool { + // #nosec G304 -- path is a package constant policy location, never + // external input. + b, err := os.ReadFile(path) + if err != nil { + return false + } + m := map[string]json.RawMessage{} + if err := json.Unmarshal(b, &m); err == nil { + _, ok := m[key] + return ok + } + return bytes.Contains(b, []byte(`"`+key+`"`)) +} + +// fileMentionsKey reports whether the file at path contains key's bytes at +// all. Used for formats not worth parsing for a presence check (macOS managed +// preferences are usually binary plists, which store key names as plain ASCII +// runs, so a byte scan detects them in both binary and XML encodings). +func fileMentionsKey(path, key string) bool { + // #nosec G304 -- path is a package constant policy location (or a glob + // expansion under it), never external input. + b, err := os.ReadFile(path) + if err != nil { + return false + } + return bytes.Contains(b, []byte(key)) +} diff --git a/internal/devicepolicy/probe_darwin.go b/internal/devicepolicy/probe_darwin.go new file mode 100644 index 0000000..f49879a --- /dev/null +++ b/internal/devicepolicy/probe_darwin.go @@ -0,0 +1,37 @@ +//go:build darwin + +package devicepolicy + +import "path/filepath" + +// darwinManagedPrefsDir holds MDM-delivered managed preferences. VS Code's +// policy watcher resolves AllowedExtensions through CFPreferences on the +// com.microsoft.VSCode domain, which an MDM materializes as a plist either +// machine-wide (directly in this dir) or per-user (in a subdir). +const ( + darwinManagedPrefsDir = "/Library/Managed Preferences" + darwinVSCodePlistName = "com.microsoft.VSCode.plist" +) + +// ProbeManagedPolicy reports whether an MDM-installed VS Code managed +// preference mentions AllowedExtensions, machine-wide or for any user. The +// plists are typically binary, but plist key names are stored as plain ASCII +// runs in both binary and XML encodings, so a byte scan is a reliable +// presence check without a plist parser dependency. +func ProbeManagedPolicy() (bool, string) { + return probeDarwinManagedPrefs(darwinManagedPrefsDir) +} + +// probeDarwinManagedPrefs is ProbeManagedPolicy parameterized over the +// managed-preferences root so tests can stage a fake tree. +func probeDarwinManagedPrefs(root string) (bool, string) { + candidates := []string{filepath.Join(root, darwinVSCodePlistName)} + perUser, _ := filepath.Glob(filepath.Join(root, "*", darwinVSCodePlistName)) + candidates = append(candidates, perUser...) + for _, p := range candidates { + if fileMentionsKey(p, allowedExtensionsName) { + return true, p + " [" + allowedExtensionsName + "]" + } + } + return false, "" +} diff --git a/internal/devicepolicy/probe_linux.go b/internal/devicepolicy/probe_linux.go new file mode 100644 index 0000000..23be633 --- /dev/null +++ b/internal/devicepolicy/probe_linux.go @@ -0,0 +1,17 @@ +//go:build linux + +package devicepolicy + +// linuxPolicyFilePath is VS Code's managed-policy file on Linux, read by its +// FilePolicyService. World-readable when an MDM/admin creates it, so the probe +// needs no elevation. +const linuxPolicyFilePath = "/etc/vscode/policy.json" + +// ProbeManagedPolicy reports whether an AllowedExtensions policy exists in the +// Linux policy file. +func ProbeManagedPolicy() (bool, string) { + if jsonFileHasKey(linuxPolicyFilePath, allowedExtensionsName) { + return true, linuxPolicyFilePath + " [" + allowedExtensionsName + "]" + } + return false, "" +} diff --git a/internal/devicepolicy/probe_other.go b/internal/devicepolicy/probe_other.go new file mode 100644 index 0000000..dcd8816 --- /dev/null +++ b/internal/devicepolicy/probe_other.go @@ -0,0 +1,9 @@ +//go:build !windows && !linux && !darwin + +package devicepolicy + +// ProbeManagedPolicy: no known VS Code policy location on this OS. The +// platform also has no settings writer (settingsPath returns ok=false), so +// enforcement never runs here — this exists only to keep the package +// compiling on every GOOS. +func ProbeManagedPolicy() (bool, string) { return false, "" } diff --git a/internal/devicepolicy/probe_test.go b/internal/devicepolicy/probe_test.go new file mode 100644 index 0000000..a7a3d5f --- /dev/null +++ b/internal/devicepolicy/probe_test.go @@ -0,0 +1,61 @@ +package devicepolicy + +import ( + "os" + "path/filepath" + "testing" +) + +func TestJSONFileHasKey(t *testing.T) { + dir := t.TempDir() + write := func(name, content string) string { + t.Helper() + p := filepath.Join(dir, name) + if err := os.WriteFile(p, []byte(content), 0o644); err != nil { + t.Fatal(err) + } + return p + } + + cases := []struct { + name string + path string + want bool + }{ + {"key present", write("a.json", `{"AllowedExtensions": "{}", "Other": 1}`), true}, + {"key absent", write("b.json", `{"Other": 1}`), false}, + {"missing file", filepath.Join(dir, "nope.json"), false}, + // Unparseable but mentions the key in quotes: over-detect → safe yield. + {"lenient fallback", write("c.json", `{"AllowedExtensions": trailing-garbage`), true}, + {"unparseable without key", write("d.json", `not json`), false}, + } + for _, tc := range cases { + if got := jsonFileHasKey(tc.path, allowedExtensionsName); got != tc.want { + t.Errorf("%s: jsonFileHasKey = %v, want %v", tc.name, got, tc.want) + } + } +} + +func TestFileMentionsKey(t *testing.T) { + dir := t.TempDir() + // Simulated binary plist: arbitrary bytes with the key name as an ASCII run. + withKey := filepath.Join(dir, "with.plist") + if err := os.WriteFile(withKey, append([]byte{0x62, 0x70, 0x6c, 0x69, 0x73, 0x74, 0x30, 0x30, 0x00}, + []byte("AllowedExtensions\x00more")...), 0o644); err != nil { + t.Fatal(err) + } + withoutKey := filepath.Join(dir, "without.plist") + if err := os.WriteFile(withoutKey, []byte("bplist00\x00SomethingElse"), 0o644); err != nil { + t.Fatal(err) + } + + if !fileMentionsKey(withKey, allowedExtensionsName) { + t.Error("key bytes present: want true") + } + if fileMentionsKey(withoutKey, allowedExtensionsName) { + t.Error("key bytes absent: want false") + } + if fileMentionsKey(filepath.Join(dir, "missing"), allowedExtensionsName) { + t.Error("missing file: want false") + } +} diff --git a/internal/devicepolicy/probe_windows.go b/internal/devicepolicy/probe_windows.go new file mode 100644 index 0000000..51bd860 --- /dev/null +++ b/internal/devicepolicy/probe_windows.go @@ -0,0 +1,69 @@ +//go:build windows + +package devicepolicy + +import ( + "errors" + + "golang.org/x/sys/windows/registry" +) + +// windowsPolicyKeyPath is the VS Code policy key path, relative to a registry +// root. VS Code reads policies from Software\Policies\Microsoft\; +// the stable build's productName is "VSCode". vscode-policy-watcher consults +// HKLM first and falls back to HKCU when HKLM has no policy, so a user-scope +// GPO / user-targeted Intune policy in HKCU governs VS Code too — the probe +// must check both roots. Both are user-readable (only writes are ACL'd), so +// no elevation is needed. +const windowsPolicyKeyPath = `SOFTWARE\Policies\Microsoft\VSCode` + +// registryProbe is one (hive, path) location to check, with a display name +// for log detail. +type registryProbe struct { + root registry.Key + name string + path string +} + +// ProbeManagedPolicy reports whether an AllowedExtensions value of ANY +// registry type exists under the VS Code policy key in HKLM or HKCU. Type +// does not matter: VS Code's policy service claims the setting as soon as the +// value exists, so a wrong-typed value still outranks user settings. +func ProbeManagedPolicy() (bool, string) { + return probeRegistryLocations([]registryProbe{ + {registry.LOCAL_MACHINE, "HKLM", windowsPolicyKeyPath}, + {registry.CURRENT_USER, "HKCU", windowsPolicyKeyPath}, + }) +} + +// probeRegistryLocations checks the locations in order (mirroring the +// watcher's HKLM-then-HKCU precedence) and reports the first hit. +func probeRegistryLocations(locs []registryProbe) (bool, string) { + for _, l := range locs { + if managed, detail := probeRegistry(l); managed { + return true, detail + } + } + return false, "" +} + +// probeRegistry is the single-location presence check, parameterized so tests +// can stage a disposable key under HKCU instead of touching real policy paths. +func probeRegistry(loc registryProbe) (bool, string) { + k, err := registry.OpenKey(loc.root, loc.path, registry.QUERY_VALUE) + if err != nil { + // Key absent (no policy) or unreadable (cannot decide → not managed). + return false, "" + } + defer k.Close() + + // GetValue with a nil buffer asks only for existence/metadata. Any error + // other than ErrNotExist still proves the value exists or the key is + // unreadable; only a clean not-exists reads as unmanaged. + if _, _, err := k.GetValue(allowedExtensionsName, nil); err != nil { + if errors.Is(err, registry.ErrNotExist) { + return false, "" + } + } + return true, loc.name + `\` + loc.path + ` [` + allowedExtensionsName + `]` +} diff --git a/internal/devicepolicy/probe_windows_test.go b/internal/devicepolicy/probe_windows_test.go new file mode 100644 index 0000000..f41c261 --- /dev/null +++ b/internal/devicepolicy/probe_windows_test.go @@ -0,0 +1,103 @@ +//go:build windows + +package devicepolicy + +import ( + "strings" + "testing" + + "golang.org/x/sys/windows/registry" +) + +// testPolicyKeyPath is a disposable key under HKCU (no elevation needed) used +// to exercise the same probe logic ProbeManagedPolicy runs against the real +// HKLM/HKCU policy paths. +const testPolicyKeyPath = `SOFTWARE\StepSecurityTest\DevicePolicyProbe` + +func hkcuProbe(name, path string) registryProbe { + return registryProbe{root: registry.CURRENT_USER, name: name, path: path} +} + +func stageTestPolicyKey(t *testing.T) registry.Key { + t.Helper() + k, _, err := registry.CreateKey(registry.CURRENT_USER, testPolicyKeyPath, + registry.SET_VALUE|registry.QUERY_VALUE) + if err != nil { + t.Fatalf("create test key: %v", err) + } + t.Cleanup(func() { + k.Close() + _ = registry.DeleteKey(registry.CURRENT_USER, testPolicyKeyPath) + _ = registry.DeleteKey(registry.CURRENT_USER, `SOFTWARE\StepSecurityTest`) + }) + return k +} + +func TestProbeRegistry(t *testing.T) { + // Absent key → not managed. + if managed, _ := probeRegistry(hkcuProbe("HKCU", testPolicyKeyPath)); managed { + t.Fatal("absent key: want managed=false") + } + + k := stageTestPolicyKey(t) + + // Key exists but value absent → not managed. + if managed, _ := probeRegistry(hkcuProbe("HKCU", testPolicyKeyPath)); managed { + t.Fatal("key without value: want managed=false") + } + + // String value present → managed. + if err := k.SetStringValue(allowedExtensionsName, `{"a":true}`); err != nil { + t.Fatal(err) + } + managed, detail := probeRegistry(hkcuProbe("HKCU", testPolicyKeyPath)) + if !managed || !strings.HasPrefix(detail, `HKCU\`) { + t.Fatalf("string value present: want managed=true with HKCU detail, got (%v, %q)", managed, detail) + } + + // Wrong-typed value still counts: VS Code claims the setting on existence. + if err := k.DeleteValue(allowedExtensionsName); err != nil { + t.Fatal(err) + } + if err := k.SetDWordValue(allowedExtensionsName, 1); err != nil { + t.Fatal(err) + } + if managed, _ := probeRegistry(hkcuProbe("HKCU", testPolicyKeyPath)); !managed { + t.Fatal("dword value present: want managed=true") + } +} + +func TestProbeRegistryLocationsFallsBackToSecond(t *testing.T) { + // Mirrors vscode-policy-watcher's HKLM→HKCU fallback: the first location + // has no policy key, the second holds the value — the probe must walk past + // the miss and find it (this is the user-scope GPO / user-targeted Intune + // case). Both locations live under HKCU so the test needs no elevation; + // production differs only in the hives probed. + k := stageTestPolicyKey(t) + if err := k.SetStringValue(allowedExtensionsName, `{"a":true}`); err != nil { + t.Fatal(err) + } + + locs := []registryProbe{ + hkcuProbe("FIRST", testPolicyKeyPath+`\Missing`), + hkcuProbe("SECOND", testPolicyKeyPath), + } + managed, detail := probeRegistryLocations(locs) + if !managed || !strings.HasPrefix(detail, `SECOND\`) { + t.Fatalf("want fallback hit (true, SECOND\\...), got (%v, %q)", managed, detail) + } + + // First location wins when both hold the value (precedence order). + locs[0] = hkcuProbe("FIRST", testPolicyKeyPath) + managed, detail = probeRegistryLocations(locs) + if !managed || !strings.HasPrefix(detail, `FIRST\`) { + t.Fatalf("want precedence hit (true, FIRST\\...), got (%v, %q)", managed, detail) + } + + // No location holds a policy → not managed. + if managed, _ := probeRegistryLocations([]registryProbe{ + hkcuProbe("FIRST", testPolicyKeyPath+`\Missing`), + }); managed { + t.Fatal("all locations missing: want managed=false") + } +} diff --git a/internal/devicepolicy/reconcile.go b/internal/devicepolicy/reconcile.go new file mode 100644 index 0000000..5c8c2fc --- /dev/null +++ b/internal/devicepolicy/reconcile.go @@ -0,0 +1,305 @@ +package devicepolicy + +import ( + "context" + "errors" + "fmt" + "time" +) + +// Reconciler converges the user-scope VS Code settings.json to the backend's +// effective policy for one device, once per scheduled cycle. It is OS-agnostic: +// the settings Writer, the managed-policy Probe, the policy Fetcher, and the +// compliance Reporter are all injected, so the whole flow is fake-testable +// with no real I/O. +type Reconciler struct { + Fetcher Fetcher + Reporter Reporter + // Writer is the settings.json writer, or nil when the platform has no + // resolvable settings path. A nil Writer makes Reconcile a no-op. + Writer Writer + + CustomerID string + DeviceID string + Platform string // reported in compliance; e.g. "windows", "linux", "darwin" + Category string // defaults to ide_extension + + // Probe reports whether a real MDM/admin-managed AllowedExtensions policy + // exists at this OS's policy location (registry / policy.json / managed + // preferences). Such a policy outranks user settings inside VS Code, so the + // agent yields (mdm_managed) instead of writing a value VS Code would + // ignore. nil → ProbeManagedPolicy (the per-OS implementation); tests + // inject a stub so results never depend on the host machine. + Probe func() (managed bool, detail string) + + // Now and Logf are optional seams. Now defaults to time.Now().UTC; Logf to a + // no-op. + Now func() time.Time + Logf func(format string, args ...any) + + // writeState and clearState are test seams over the ownership store + // (WriteAppliedState / ClearAppliedState). nil → the real implementation. + writeState func(category string, s AppliedCategoryState) error + clearState func(category string) error +} + +func (r *Reconciler) persistState(cat string, s AppliedCategoryState) error { + if r.writeState != nil { + return r.writeState(cat, s) + } + return WriteAppliedState(cat, s) +} + +func (r *Reconciler) dropState(cat string) error { + if r.clearState != nil { + return r.clearState(cat) + } + return ClearAppliedState(cat) +} + +func (r *Reconciler) now() time.Time { + if r.Now != nil { + return r.Now() + } + return time.Now().UTC() +} + +func (r *Reconciler) logf(format string, args ...any) { + if r.Logf != nil { + r.Logf(format, args...) + } +} + +func (r *Reconciler) category() string { + if r.Category != "" { + return r.Category + } + return CategoryIDEExtension +} + +func (r *Reconciler) probe() (bool, string) { + if r.Probe != nil { + return r.Probe() + } + return ProbeManagedPolicy() +} + +// Reconcile runs one enforcement cycle. It NEVER panics into the caller's hot +// path; failures are returned for logging. The contract: +// +// - fetch error (transport / non-200 / malformed) → NO-OP, error returned. +// Enforcement on disk is never wiped on a transient or malformed response. +// - platform not enforceable (nil Writer) → silent no-op. +// - clear result → remove ONLY the agent-owned settings key; a value the +// agent has no record of writing is left untouched. No compliance report +// (an unassigned device is backend-derived). +// - policy result → probe → ownership/drift-checked write + readback + +// verify + report (handleEnforce). +func (r *Reconciler) Reconcile(ctx context.Context) error { + if r.Fetcher == nil { + return errors.New("devicepolicy: nil fetcher") + } + cat := r.category() + + ep, err := r.Fetcher.Fetch(ctx, r.CustomerID, r.DeviceID, cat) + if err != nil { + // Malformed/transient: do nothing. The on-disk policy (if any) stands. + return fmt.Errorf("devicepolicy: fetch: %w", err) + } + + if r.Writer == nil { + r.logf("devicepolicy: no settings path on this platform; skipping (category=%s)", cat) + return nil + } + + if ep.Clear { + return r.handleClear(cat) + } + return r.handleEnforce(ctx, cat, ep) +} + +// handleClear removes the agent-owned settings key on unassignment. It clears +// the on-disk value ONLY when it still equals what the agent last wrote +// (ownership); a value the agent has no record of writing — the user's own +// extensions.allowed predates enforcement, or the record was lost — is left +// intact. +func (r *Reconciler) handleClear(cat string) error { + prev, hadPrev := ReadAppliedState(cat) + onDisk, present, err := r.Writer.Read() + if err != nil { + return fmt.Errorf("devicepolicy: clear: read %s: %w", r.Writer.Location(), err) + } + + owns := present && prev.WrittenValue != "" && onDisk == prev.WrittenValue + switch { + case owns: + if err := r.Writer.Clear(); err != nil { + return fmt.Errorf("devicepolicy: clear %s: %w", r.Writer.Location(), err) + } + r.logf("devicepolicy: cleared agent-owned policy at %s", r.Writer.Location()) + case present: + // A value the agent did not write — leave it to whoever set it. + r.logf("devicepolicy: clear requested but %s holds a value the agent did not write; leaving it", r.Writer.Location()) + } + + // Drop our ownership record whenever we hold an entry for this category. + // Beyond the obvious case (we owned a value), this also reclaims an empty + // record a preflight may have left after its settings write later failed. + // An absent entry → no-op (idempotent). + if hadPrev { + if err := r.dropState(cat); err != nil { + return fmt.Errorf("devicepolicy: clear: update state: %w", err) + } + } + return nil +} + +// handleEnforce converges settings.json to the compiled policy and reports. +// The ladder, in order: +// +// probe (managed policy exists → mdm_managed, never write) +// → read current value +// → idempotency (hash unchanged ∧ on-disk converged → report, no write) +// → preflight ownership-store writability +// → drift detection (on-disk diverged from the recorded written value) +// → merge-write + readback +// → persist ownership on every successful write (rollback if that fails) +// → Verify → report (drift upgrades a would-be compliant to drift_detected) +func (r *Reconciler) handleEnforce(ctx context.Context, cat string, ep EffectivePolicy) error { + // The compiled policy compacted: the canonical comparison form for + // readback, idempotency, and ownership. (The backend's hash still travels + // verbatim; only the value bytes are normalized for comparison.) + newValue, err := compactJSON(ep.Policy) + if err != nil { + // Defensive: the fetcher already validated object shape, so this is a + // malformed-payload class failure → no-op, never write. + return fmt.Errorf("devicepolicy: enforce: compact policy: %w", err) + } + + // 1. Managed-policy probe. A policy at the OS policy location outranks + // user settings inside VS Code — writing would be ineffective at best and + // fight the MDM at worst. Yield and report. + if managed, detail := r.probe(); managed { + r.logf("devicepolicy: managed policy present at %s → mdm_managed (yielding)", detail) + return r.report(ctx, cat, StateMDMManaged, "") + } + + // 2. Read the current settings value. + prev, hadPrev := ReadAppliedState(cat) + onDisk, present, err := r.Writer.Read() + if err != nil { + // Couldn't read to decide idempotency/drift → verification_failed. + // This includes an unsalvageable settings.json (not valid JSONC), which + // the writer refuses to touch. + _ = r.report(ctx, cat, StateVerificationFailed, "") + return fmt.Errorf("devicepolicy: enforce: read %s: %w", r.Writer.Location(), err) + } + + // 3. Idempotency: the desired policy is already in place and unchanged. + // No write — but still report so the backend sees a fresh evaluation. + if present && onDisk == newValue && prev.AppliedHash == ep.Hash { + r.logf("devicepolicy: policy already applied (hash unchanged) — no write") + return r.report(ctx, cat, StateCompliant, ep.Hash) + } + + // 4. Drift: the agent wrote a value before, and what is on disk now is not + // it (edited or removed — typically the user hand-editing settings.json). + // Enforcement means converging it back; the distinct state lets the + // backend surface that it happened. + drifted := hadPrev && prev.WrittenValue != "" && (!present || onDisk != prev.WrittenValue) + if drifted { + r.logf("devicepolicy: %s diverged from the recorded written value → re-applying (drift)", r.Writer.Location()) + } + + // 5. Preflight: prove the ownership store is writable BEFORE mutating the + // settings file. An enforced value with no ownership record is orphaned — + // a later clear refuses to remove it. Re-persisting the current state is a + // meaning-preserving writability probe. + probe := prev + if !hadPrev { + probe = AppliedCategoryState{FetchedAt: r.now()} + } + if perr := r.persistState(cat, probe); perr != nil { + _ = r.report(ctx, cat, StateWriteFailed, "") + return fmt.Errorf("devicepolicy: enforce: ownership state not writable, refusing to write policy: %w", perr) + } + + // 6. Merge-write + readback. + rb, werr := r.Writer.Write(newValue) + if werr != nil { + _ = r.report(ctx, cat, StateWriteFailed, "") + return fmt.Errorf("devicepolicy: enforce: write %s: %w", r.Writer.Location(), werr) + } + readbackMatch := rb == newValue + + // 7. Ownership is recorded on EVERY successful write — it means "what the + // agent wrote", not "what it verified". On a readback mismatch the write + // may still have landed; without a record the next cycle would classify + // the agent's own value as drift forever. Value-based ownership + // self-corrects: the record only takes effect when the on-disk value + // actually equals it. + if err := r.persistState(cat, AppliedCategoryState{ + AppliedHash: ep.Hash, + WrittenValue: newValue, + FetchedAt: r.now(), + }); err != nil { + // The write happened but ownership couldn't be recorded — undo it so + // no unrecorded value is left behind, and report a failed write. + r.rollbackWrite(onDisk, present) + _ = r.report(ctx, cat, StateWriteFailed, "") + return fmt.Errorf("devicepolicy: enforce: update state: %w", err) + } + r.logf("devicepolicy: wrote policy to %s (readback_match=%v)", r.Writer.Location(), readbackMatch) + + state := Verify(VerifyInput{WriteOK: true, ReadbackMatch: readbackMatch}) + if drifted && state == StateCompliant { + state = StateDriftDetected + } + + // applied_hash is echoed only when we are confident the policy is applied + // (readback-confirmed) — compliant, or drift_detected (drift that was + // successfully re-applied). It is the backend's hash verbatim — never + // recomputed — so the backend's byte-exact applied==desired check gates + // `compliant`. + appliedHash := "" + if state == StateCompliant || state == StateDriftDetected { + appliedHash = ep.Hash + } + return r.report(ctx, cat, state, appliedHash) +} + +// rollbackWrite restores the settings key to its pre-cycle condition after the +// post-write ownership persist failed. WriteAppliedState is atomic +// (temp+rename), so the failed persist left the previous state file intact — +// restoring the previous on-disk value keeps record and disk consistent. +// Best-effort: a rollback failure is logged, and the divergence surfaces as +// drift on the next cycle. +func (r *Reconciler) rollbackWrite(prevOnDisk string, prevPresent bool) { + var err error + if prevPresent { + _, err = r.Writer.Write(prevOnDisk) + } else { + err = r.Writer.Clear() + } + if err != nil { + r.logf("devicepolicy: rollback at %s failed: %v", r.Writer.Location(), err) + } +} + +func (r *Reconciler) report(ctx context.Context, cat, state, appliedHash string) error { + r.logf("devicepolicy: reporting state=%s category=%s", state, cat) + if r.Reporter == nil { + return nil + } + rep := ComplianceReport{ + Category: cat, + State: state, + AppliedHash: appliedHash, + AgentVersion: AgentVersion(), + Platform: r.Platform, + } + if err := r.Reporter.Report(ctx, r.CustomerID, r.DeviceID, rep); err != nil { + return fmt.Errorf("devicepolicy: report %s: %w", state, err) + } + return nil +} diff --git a/internal/devicepolicy/reconcile_test.go b/internal/devicepolicy/reconcile_test.go new file mode 100644 index 0000000..00619be --- /dev/null +++ b/internal/devicepolicy/reconcile_test.go @@ -0,0 +1,589 @@ +package devicepolicy + +import ( + "context" + "encoding/json" + "errors" + "os" + "path/filepath" + "testing" + "time" +) + +// samplePolicy is the compacted compiled policy — the exact value the +// reconciler writes and records (it compacts the fetched payload first). +const samplePolicy = `{"github.copilot":true,"ms-python.python":"1.2.3"}` + +// samplePolicyWire is the same policy as the backend might format it on the +// wire. The reconciler must normalize this to samplePolicy before comparing +// or writing. +const samplePolicyWire = "{\n \"github.copilot\": true,\n \"ms-python.python\": \"1.2.3\"\n}" + +// --- fakes ----------------------------------------------------------------- + +type fakeFetcher struct { + ep EffectivePolicy + err error +} + +func (f *fakeFetcher) Fetch(_ context.Context, _, _, _ string) (EffectivePolicy, error) { + return f.ep, f.err +} + +type fakeReporter struct { + reports []ComplianceReport + err error +} + +func (r *fakeReporter) Report(_ context.Context, _, _ string, rep ComplianceReport) error { + r.reports = append(r.reports, rep) + return r.err +} + +type fakeWriter struct { + value string + present bool + readErr error + writeErr error + readbackOverride string // when set, Write returns this instead of echoing input + writes []string + clears int + reads int +} + +func (w *fakeWriter) Read() (string, bool, error) { + w.reads++ + return w.value, w.present, w.readErr +} + +func (w *fakeWriter) Write(v string) (string, error) { + w.writes = append(w.writes, v) + if w.writeErr != nil { + return "", w.writeErr + } + w.value, w.present = v, true + if w.readbackOverride != "" { + return w.readbackOverride, nil + } + return v, nil +} + +func (w *fakeWriter) Clear() error { + w.clears++ + w.value, w.present = "", false + return nil +} + +func (w *fakeWriter) Location() string { return "fake://settings.json" } + +// --- helpers --------------------------------------------------------------- + +func withTempCache(t *testing.T) { + t.Helper() + restore := SetCachePathForTest(filepath.Join(t.TempDir(), CacheFilename)) + t.Cleanup(restore) +} + +// newRec builds a reconciler over fakes. The managed-policy probe is stubbed +// to "not managed" so results never depend on the host machine; tests for the +// mdm_managed path override r.Probe. +func newRec(t *testing.T, ep EffectivePolicy, fetchErr error, w *fakeWriter) (*Reconciler, *fakeReporter) { + t.Helper() + withTempCache(t) + rep := &fakeReporter{} + r := &Reconciler{ + Fetcher: &fakeFetcher{ep: ep, err: fetchErr}, + Reporter: rep, + Writer: w, + CustomerID: "cust", + DeviceID: "dev-1", + Platform: "linux", + Probe: func() (bool, string) { return false, "" }, + Now: func() time.Time { return time.Date(2026, 6, 11, 0, 0, 0, 0, time.UTC) }, + } + return r, rep +} + +func policyEP(hash string) EffectivePolicy { + return EffectivePolicy{ + Category: CategoryIDEExtension, + Clear: false, + Policy: json.RawMessage(samplePolicyWire), + Hash: hash, + } +} + +func lastReport(t *testing.T, rep *fakeReporter) ComplianceReport { + t.Helper() + if len(rep.reports) != 1 { + t.Fatalf("expected exactly 1 report, got %d: %+v", len(rep.reports), rep.reports) + } + return rep.reports[0] +} + +// --- tests ----------------------------------------------------------------- + +func TestEnforceWritesCompactedPolicyAndReportsCompliant(t *testing.T) { + w := &fakeWriter{} + r, rep := newRec(t, policyEP("sha256:H"), nil, w) + if err := r.Reconcile(context.Background()); err != nil { + t.Fatalf("Reconcile: %v", err) + } + // The wire payload is formatted; the written value must be its compaction. + if len(w.writes) != 1 || w.writes[0] != samplePolicy { + t.Fatalf("expected compacted policy written once, got %v", w.writes) + } + got := lastReport(t, rep) + if got.State != StateCompliant { + t.Fatalf("state = %q, want compliant", got.State) + } + // applied_hash echoed verbatim (never recomputed). + if got.AppliedHash != "sha256:H" { + t.Fatalf("applied_hash = %q, want sha256:H", got.AppliedHash) + } + // Ownership recorded. + st, ok := ReadAppliedState(CategoryIDEExtension) + if !ok || st.WrittenValue != samplePolicy || st.AppliedHash != "sha256:H" { + t.Fatalf("cache = %+v ok=%v", st, ok) + } +} + +func TestEnforceIdempotentSecondRunWritesNothing(t *testing.T) { + withTempCache(t) + // Seed prior ownership + on-disk value matching the desired policy. + if err := WriteAppliedState(CategoryIDEExtension, AppliedCategoryState{AppliedHash: "sha256:H", WrittenValue: samplePolicy}); err != nil { + t.Fatal(err) + } + w := &fakeWriter{value: samplePolicy, present: true} + rep := &fakeReporter{} + r := &Reconciler{ + Fetcher: &fakeFetcher{ep: policyEP("sha256:H")}, Reporter: rep, Writer: w, + CustomerID: "c", DeviceID: "d", Platform: "linux", + Probe: func() (bool, string) { return false, "" }, + } + if err := r.Reconcile(context.Background()); err != nil { + t.Fatalf("Reconcile: %v", err) + } + if len(w.writes) != 0 { + t.Fatalf("idempotent run must not write, got %v", w.writes) + } + got := lastReport(t, rep) + if got.State != StateCompliant || got.AppliedHash != "sha256:H" { + t.Fatalf("report = %+v, want compliant + echoed hash", got) + } +} + +func TestClearRemovesAgentOwnedPolicy(t *testing.T) { + withTempCache(t) + if err := WriteAppliedState(CategoryIDEExtension, AppliedCategoryState{AppliedHash: "sha256:H", WrittenValue: samplePolicy}); err != nil { + t.Fatal(err) + } + w := &fakeWriter{value: samplePolicy, present: true} // on-disk == what we wrote → owned + rep := &fakeReporter{} + r := &Reconciler{ + Fetcher: &fakeFetcher{ep: EffectivePolicy{Category: CategoryIDEExtension, Clear: true}}, + Reporter: rep, Writer: w, CustomerID: "c", DeviceID: "d", Platform: "linux", + Probe: func() (bool, string) { return false, "" }, + Now: func() time.Time { return time.Unix(0, 0).UTC() }, + } + if err := r.Reconcile(context.Background()); err != nil { + t.Fatalf("Reconcile: %v", err) + } + if w.clears != 1 { + t.Fatalf("owned policy should be cleared once, clears=%d", w.clears) + } + if len(rep.reports) != 0 { + t.Fatalf("clear must not report a compliance state, got %+v", rep.reports) + } + if st, _ := ReadAppliedState(CategoryIDEExtension); st.WrittenValue != "" { + t.Fatalf("ownership record should be dropped, got %+v", st) + } +} + +func TestClearLeavesValueAgentDidNotWrite(t *testing.T) { + withTempCache(t) + // We recorded writing "mine", but on disk is "theirs" — the user (or some + // other tool) changed it. Unassignment must not destroy their value. + if err := WriteAppliedState(CategoryIDEExtension, AppliedCategoryState{WrittenValue: "mine"}); err != nil { + t.Fatal(err) + } + w := &fakeWriter{value: "theirs", present: true} + rep := &fakeReporter{} + r := &Reconciler{ + Fetcher: &fakeFetcher{ep: EffectivePolicy{Category: CategoryIDEExtension, Clear: true}}, + Reporter: rep, Writer: w, CustomerID: "c", DeviceID: "d", Platform: "linux", + Probe: func() (bool, string) { return false, "" }, + Now: func() time.Time { return time.Unix(0, 0).UTC() }, + } + if err := r.Reconcile(context.Background()); err != nil { + t.Fatalf("Reconcile: %v", err) + } + if w.clears != 0 { + t.Fatalf("a value the agent did not write must NOT be cleared, clears=%d", w.clears) + } + if len(rep.reports) != 0 { + t.Fatalf("clear path reports nothing, got %+v", rep.reports) + } +} + +func TestEnforceManagedPolicyProbeYieldsMDMManaged(t *testing.T) { + // A real MDM policy at the OS policy location outranks user settings inside + // VS Code: the agent yields without reading or writing settings.json. + w := &fakeWriter{} + r, rep := newRec(t, policyEP("sha256:H"), nil, w) + r.Probe = func() (bool, string) { return true, `HKLM\...\VSCode [AllowedExtensions]` } + if err := r.Reconcile(context.Background()); err != nil { + t.Fatalf("Reconcile: %v", err) + } + if w.reads != 0 || len(w.writes) != 0 || w.clears != 0 { + t.Fatalf("managed probe must short-circuit before any settings I/O: reads=%d writes=%v clears=%d", + w.reads, w.writes, w.clears) + } + got := lastReport(t, rep) + if got.State != StateMDMManaged { + t.Fatalf("state = %q, want mdm_managed", got.State) + } + if got.AppliedHash != "" { + t.Fatalf("applied_hash should be empty when nothing applied, got %q", got.AppliedHash) + } +} + +func TestEnforceOverwritesPreexistingUserValue(t *testing.T) { + // A pre-existing extensions.allowed in the USER's settings (no ownership + // record, no managed policy) is exactly what enforcement is for: the + // compiled policy replaces it. This is the old foreign-value yield + // inverted — settings.json is the enforcement surface now, and the real + // MDM case is handled by the probe. + cases := []struct { + name string + value string + }{ + {"user's own allow-list", `{"user.choice":true}`}, + {"value byte-equal to desired policy", samplePolicy}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + w := &fakeWriter{value: tc.value, present: true} + r, rep := newRec(t, policyEP("sha256:H"), nil, w) + if err := r.Reconcile(context.Background()); err != nil { + t.Fatalf("Reconcile: %v", err) + } + if len(w.writes) != 1 || w.writes[0] != samplePolicy { + t.Fatalf("expected the policy written once, got %v", w.writes) + } + got := lastReport(t, rep) + // No ownership record → this is first enforcement, not drift. + if got.State != StateCompliant || got.AppliedHash != "sha256:H" { + t.Fatalf("report = %+v, want compliant + echoed hash", got) + } + }) + } +} + +func TestEnforceDriftReappliesAndReportsDriftDetected(t *testing.T) { + // The agent wrote the policy before; the user edited or removed it. The + // reconciler converges it back and reports drift_detected (readback + // confirmed → applied_hash still echoed). + cases := []struct { + name string + value string + present bool + }{ + {"key edited by user", `{"user.tampered":true}`, true}, + {"key removed by user", "", false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + withTempCache(t) + if err := WriteAppliedState(CategoryIDEExtension, AppliedCategoryState{AppliedHash: "sha256:H", WrittenValue: samplePolicy}); err != nil { + t.Fatal(err) + } + w := &fakeWriter{value: tc.value, present: tc.present} + rep := &fakeReporter{} + r := &Reconciler{ + Fetcher: &fakeFetcher{ep: policyEP("sha256:H")}, Reporter: rep, Writer: w, + CustomerID: "c", DeviceID: "d", Platform: "linux", + Probe: func() (bool, string) { return false, "" }, + Now: func() time.Time { return time.Unix(0, 0).UTC() }, + } + if err := r.Reconcile(context.Background()); err != nil { + t.Fatalf("Reconcile: %v", err) + } + if len(w.writes) != 1 || w.writes[0] != samplePolicy { + t.Fatalf("drift must re-apply the policy, writes=%v", w.writes) + } + got := lastReport(t, rep) + if got.State != StateDriftDetected { + t.Fatalf("state = %q, want drift_detected", got.State) + } + if got.AppliedHash != "sha256:H" { + t.Fatalf("applied_hash = %q, want echoed hash (re-apply was readback-confirmed)", got.AppliedHash) + } + // Next cycle: converged, hash unchanged → plain compliant again. + if err := r.Reconcile(context.Background()); err != nil { + t.Fatalf("second Reconcile: %v", err) + } + if len(w.writes) != 1 { + t.Fatalf("second cycle must be idempotent, writes=%v", w.writes) + } + if rep.reports[1].State != StateCompliant { + t.Fatalf("second cycle state = %q, want compliant", rep.reports[1].State) + } + }) + } +} + +func TestEnforceWriteFailureReportsWriteFailed(t *testing.T) { + w := &fakeWriter{writeErr: errors.New("permission denied")} + r, rep := newRec(t, policyEP("sha256:H"), nil, w) + err := r.Reconcile(context.Background()) + if err == nil { + t.Fatal("write failure should surface an error") + } + if got := lastReport(t, rep); got.State != StateWriteFailed { + t.Fatalf("state = %q, want write_failed", got.State) + } +} + +func TestEnforceReadbackMismatchReportsPolicyNotApplied(t *testing.T) { + w := &fakeWriter{readbackOverride: `{"*":true}`} // write landed differently than intended + r, rep := newRec(t, policyEP("sha256:H"), nil, w) + if err := r.Reconcile(context.Background()); err != nil { + t.Fatalf("Reconcile: %v", err) + } + got := lastReport(t, rep) + if got.State != StatePolicyNotApplied { + t.Fatalf("state = %q, want policy_not_applied", got.State) + } + if got.AppliedHash != "" { + t.Fatalf("applied_hash must be empty without readback confirmation, got %q", got.AppliedHash) + } + // Ownership IS recorded even on a readback mismatch — it tracks what the + // agent wrote, not what it verified; next-cycle recovery depends on it + // (value-based ownership only takes effect if the value actually landed). + if st, ok := ReadAppliedState(CategoryIDEExtension); !ok || st.WrittenValue != samplePolicy { + t.Fatalf("cache must record the written value even on readback mismatch, got %+v ok=%v", st, ok) + } +} + +func TestEnforceReadbackMismatchRecoversNextCycle(t *testing.T) { + // Cycle 1: the write lands but readback transiently mismatches → + // policy_not_applied. Cycle 2: the on-disk value IS what we wrote; with + // ownership recorded the agent reclaims it and reports compliant — it must + // not classify its own write as drift and churn rewrites forever. + w := &fakeWriter{readbackOverride: `{"*":true}`} + r, rep := newRec(t, policyEP("sha256:H"), nil, w) + if err := r.Reconcile(context.Background()); err != nil { + t.Fatalf("cycle 1: %v", err) + } + if rep.reports[0].State != StatePolicyNotApplied { + t.Fatalf("cycle 1 state = %q, want policy_not_applied", rep.reports[0].State) + } + + w.readbackOverride = "" // transient condition gone; disk holds our value + if err := r.Reconcile(context.Background()); err != nil { + t.Fatalf("cycle 2: %v", err) + } + if len(rep.reports) != 2 || rep.reports[1].State != StateCompliant { + t.Fatalf("cycle 2 reports = %+v, want second report compliant", rep.reports) + } + if len(w.writes) != 1 { + t.Fatalf("cycle 2 must be idempotent (no rewrite), writes=%v", w.writes) + } +} + +func TestEnforceReadErrorReportsVerificationFailed(t *testing.T) { + // Includes the unsalvageable-settings.json case: the writer refuses to + // parse it, the reconciler can't decide idempotency/drift. + w := &fakeWriter{readErr: errors.New("settings.json is not valid JSONC")} + r, rep := newRec(t, policyEP("sha256:H"), nil, w) + err := r.Reconcile(context.Background()) + if err == nil { + t.Fatal("read error should surface") + } + if got := lastReport(t, rep); got.State != StateVerificationFailed { + t.Fatalf("state = %q, want verification_failed", got.State) + } +} + +func TestMalformedFetchIsNoOp(t *testing.T) { + w := &fakeWriter{value: "existing", present: true} + r, rep := newRec(t, EffectivePolicy{}, errors.New("malformed"), w) + probed := false + r.Probe = func() (bool, string) { probed = true; return false, "" } + err := r.Reconcile(context.Background()) + if err == nil { + t.Fatal("fetch error should surface") + } + if len(w.writes) != 0 || w.clears != 0 || w.reads != 0 || probed { + t.Fatalf("malformed fetch must touch nothing: writes=%v clears=%d reads=%d probed=%v", + w.writes, w.clears, w.reads, probed) + } + if len(rep.reports) != 0 { + t.Fatalf("malformed fetch must not report, got %+v", rep.reports) + } +} + +func TestNilWriterPlatformIsNoOp(t *testing.T) { + withTempCache(t) + rep := &fakeReporter{} + r := &Reconciler{ + Fetcher: &fakeFetcher{ep: policyEP("sha256:H")}, + Reporter: rep, Writer: nil, CustomerID: "c", DeviceID: "d", Platform: "freebsd", + Probe: func() (bool, string) { return false, "" }, + } + if err := r.Reconcile(context.Background()); err != nil { + t.Fatalf("nil-writer platform should no-op without error, got %v", err) + } + if len(rep.reports) != 0 { + t.Fatalf("unsupported platform reports nothing, got %+v", rep.reports) + } +} + +func TestEnforceStateUnwritablePreflightWritesNothing(t *testing.T) { + // If the ownership store can't be persisted, the policy must never be + // written: an enforced value with no record would be orphaned (a later + // clear refuses to remove it, and every cycle would misread it as drift + // of unknown origin). + w := &fakeWriter{} + r, rep := newRec(t, policyEP("sha256:H"), nil, w) + r.writeState = func(string, AppliedCategoryState) error { return errors.New("disk full") } + if err := r.Reconcile(context.Background()); err == nil { + t.Fatal("unwritable ownership store should surface an error") + } + if len(w.writes) != 0 { + t.Fatalf("policy must NOT be written when ownership can't be recorded, writes=%v", w.writes) + } + if got := lastReport(t, rep); got.State != StateWriteFailed { + t.Fatalf("state = %q, want write_failed", got.State) + } +} + +func TestEnforceStatePersistFailureRollsBackWrite(t *testing.T) { + // Preflight succeeds but the post-write persist fails: the agent undoes the + // just-written value (no prior value → remove the key) so it never leaves + // an enforced policy it has no ownership record for. + w := &fakeWriter{} + r, rep := newRec(t, policyEP("sha256:H"), nil, w) + calls := 0 + r.writeState = func(string, AppliedCategoryState) error { + calls++ + if calls == 1 { + return nil // preflight probe + } + return errors.New("disk full") + } + if err := r.Reconcile(context.Background()); err == nil { + t.Fatal("persist failure should surface an error") + } + if len(w.writes) != 1 || w.writes[0] != samplePolicy { + t.Fatalf("writes = %v, want exactly one write of the policy", w.writes) + } + if w.clears != 1 || w.present { + t.Fatalf("rolled-back write should remove the key, clears=%d present=%v", w.clears, w.present) + } + if got := lastReport(t, rep); got.State != StateWriteFailed { + t.Fatalf("state = %q, want write_failed", got.State) + } +} + +func TestEnforceStatePersistFailureRestoresPreviousOwnedValue(t *testing.T) { + // Same as above but a previous owned value existed: rollback restores it, + // keeping the (intact, atomic) old state file and the disk consistent. + withTempCache(t) + if err := WriteAppliedState(CategoryIDEExtension, AppliedCategoryState{AppliedHash: "sha256:OLD", WrittenValue: "old-value"}); err != nil { + t.Fatal(err) + } + w := &fakeWriter{value: "old-value", present: true} + rep := &fakeReporter{} + r := &Reconciler{ + Fetcher: &fakeFetcher{ep: policyEP("sha256:NEW")}, Reporter: rep, Writer: w, + CustomerID: "c", DeviceID: "d", Platform: "linux", + Probe: func() (bool, string) { return false, "" }, + Now: func() time.Time { return time.Unix(0, 0).UTC() }, + } + r.writeState = func(_ string, s AppliedCategoryState) error { + if s.WrittenValue == samplePolicy { + return errors.New("disk full") // fail only the post-write persist + } + return nil // preflight probe succeeds + } + if err := r.Reconcile(context.Background()); err == nil { + t.Fatal("persist failure should surface an error") + } + if len(w.writes) != 2 || w.writes[0] != samplePolicy || w.writes[1] != "old-value" { + t.Fatalf("writes = %v, want [new policy, restored old-value]", w.writes) + } + if w.value != "old-value" || !w.present { + t.Fatalf("on-disk should be restored to old-value, got %q present=%v", w.value, w.present) + } + if got := lastReport(t, rep); got.State != StateWriteFailed { + t.Fatalf("state = %q, want write_failed", got.State) + } +} + +func TestEnforcePolicyChangeRewrites(t *testing.T) { + withTempCache(t) + // We own "old-value" and it is still intact on disk; the backend now sends + // a new policy with a new hash. This is a policy CHANGE, not drift — the + // report is plain compliant. + if err := WriteAppliedState(CategoryIDEExtension, AppliedCategoryState{AppliedHash: "sha256:OLD", WrittenValue: "old-value"}); err != nil { + t.Fatal(err) + } + w := &fakeWriter{value: "old-value", present: true} + rep := &fakeReporter{} + r := &Reconciler{ + Fetcher: &fakeFetcher{ep: policyEP("sha256:NEW")}, Reporter: rep, Writer: w, + CustomerID: "c", DeviceID: "d", Platform: "linux", + Probe: func() (bool, string) { return false, "" }, + Now: func() time.Time { return time.Unix(0, 0).UTC() }, + } + if err := r.Reconcile(context.Background()); err != nil { + t.Fatalf("Reconcile: %v", err) + } + if len(w.writes) != 1 || w.writes[0] != samplePolicy { + t.Fatalf("owned policy change should rewrite to new value, writes=%v", w.writes) + } + if got := lastReport(t, rep); got.State != StateCompliant || got.AppliedHash != "sha256:NEW" { + t.Fatalf("report = %+v, want compliant + sha256:NEW", got) + } +} + +func TestEnforceRefusesToClobberFutureSchemaStateFile(t *testing.T) { + // Headline guarantee: an older agent meeting a NEWER agent's state file must + // not overwrite it. The preflight write hits errFutureSchema → the cycle + // reports write_failed, never touches settings.json, and leaves the future + // file byte-identical (its category metadata preserved for the newer agent). + path := filepath.Join(t.TempDir(), CacheFilename) + future := `{"schema_version":999,"categories":{"future_cat":{"applied_hash":"sha256:z","written_value":"{}","fetched_at":"2026-06-08T00:00:00Z"}}}` + "\n" + if err := os.WriteFile(path, []byte(future), 0o600); err != nil { + t.Fatal(err) + } + restore := SetCachePathForTest(path) + defer restore() + + w := &fakeWriter{} + rep := &fakeReporter{} + r := &Reconciler{ + Fetcher: &fakeFetcher{ep: policyEP("sha256:H")}, Reporter: rep, Writer: w, + CustomerID: "c", DeviceID: "d", Platform: "linux", + Probe: func() (bool, string) { return false, "" }, + Now: func() time.Time { return time.Unix(0, 0).UTC() }, + } + if err := r.Reconcile(context.Background()); err == nil { + t.Fatal("refusing to clobber a future-schema state file should surface an error") + } + if len(w.writes) != 0 { + t.Fatalf("settings.json must not be written when the future state file is refused, writes=%v", w.writes) + } + if got := lastReport(t, rep); got.State != StateWriteFailed { + t.Fatalf("state = %q, want write_failed", got.State) + } + after, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if string(after) != future { + t.Fatalf("future-schema state file must be left byte-identical; got %q", string(after)) + } +} diff --git a/internal/devicepolicy/settings_writer.go b/internal/devicepolicy/settings_writer.go new file mode 100644 index 0000000..0a76aa5 --- /dev/null +++ b/internal/devicepolicy/settings_writer.go @@ -0,0 +1,287 @@ +package devicepolicy + +import ( + "bytes" + "encoding/json" + "errors" + "fmt" + "os" + "path/filepath" + "runtime" + + "github.com/tailscale/hujson" + + "github.com/step-security/dev-machine-guard/internal/atomicfile" +) + +// Writer reads, upserts, and removes the `extensions.allowed` key in the +// user-scope VS Code settings.json. It is a thin primitive: it manages ONLY +// that one top-level key — every other key, comment, and formatting detail in +// the file is preserved (single-key JSONC merge), which is what makes editing +// a file the user also owns safe. Ownership and drift decisions (whether the +// agent may overwrite or remove) live in the reconciler, not here, so the +// writer stays pure and fake-testable. +// +// Values are compact JSON object strings — the backend's compiled +// extensions.allowed object, compacted. Read returns the on-disk value +// re-compacted, so equality against a recorded written value is canonical +// regardless of how the file is formatted on disk. +// +// The production implementation is settingsWriter below; the reconciler is +// exercised against fakes. +type Writer interface { + // Read returns the current extensions.allowed value (compacted) and + // whether it is present. (present=false, err=nil) means the file is + // missing or readable-but-without-the-key. An unparseable settings.json + // is an error — the writer refuses to reason about a file it cannot + // understand. + Read() (value string, present bool, err error) + + // Write upserts extensions.allowed to value, then reads it back and + // returns the read-back value. The reconciler compares it to value to + // detect a silent non-apply (policy_not_applied). An error means the + // write itself failed or the file is unsalvageable → write_failed. + Write(value string) (readback string, err error) + + // Clear removes the extensions.allowed key, leaving the rest of the file + // (and the file itself) intact. A missing file or absent key is a no-op. + Clear() error + + // Location is a human-readable description of the target, for logs. + Location() string +} + +// allowedExtensionsSettingKey is the `extensions.allowed` SETTING ID — the key +// VS Code reads from settings.json. This is deliberately NOT the registered +// policy name "AllowedExtensions" (allowedExtensionsName): policy locations +// (registry / policy.json / managed prefs) are keyed by policy name and are +// probed read-only (probe_*.go); the settings file is keyed by setting id and +// is the surface the agent writes. +const allowedExtensionsSettingKey = "extensions.allowed" + +// settingsFileMode is the fallback mode for a settings.json the agent creates. +// An existing file keeps its current mode (atomicfile.PickMode); 0600 for a +// new one is safe because VS Code runs as the same user. +const settingsFileMode os.FileMode = 0o600 + +// settingsWriter implements Writer against the user-scope VS Code +// settings.json (JSONC). One implementation serves every OS — only the path +// differs (settingsPath). +// +// Invariants: +// - Single-key edits: only the top-level "extensions.allowed" member is ever +// added, replaced, or removed. Everything else in the file — other keys, +// comments, trailing commas, whitespace — is preserved byte-for-byte +// (hujson syntax tree + RFC 6902 patch). +// - A file that cannot be parsed as JSONC, or whose root is not an object, +// is NEVER rewritten: every operation fails and the file is untouched +// (surfaces as write_failed; blind overwrite would destroy user settings). +// - Replacement is atomic (temp + fsync + rename in the target dir) and the +// previous file is kept as a capped sibling backup (atomicfile). +// - The file itself is never deleted; Clear removes only the key. +// +// Known limitation: VS Code also accepts the nested spelling +// `"extensions": {"allowed": …}`. The agent reads/writes only the canonical +// flat dotted key, so a pre-existing nested value is invisible to it (VS Code +// resolves the flat key it writes; the nested duplicate is the user's to +// reconcile). +type settingsWriter struct{ path string } + +// newSettingsWriterAt builds a settings writer for an arbitrary path +// (tests use a tempdir; production uses settingsPath()). +func newSettingsWriterAt(path string) *settingsWriter { return &settingsWriter{path: path} } + +// NewWriter returns the user-scope settings.json writer for this OS. ok=false +// when settingsPath cannot resolve the target (unsupported OS, no home dir or +// %APPDATA%) — the reconciler treats that as not agent-enforceable and no-ops. +func NewWriter() (Writer, bool) { + path, ok := settingsPath() + if !ok { + return nil, false + } + return newSettingsWriterAt(path), true +} + +// settingsPath resolves the user-scope VS Code settings.json for this OS, +// matching VS Code's own resolution (the default profile's user settings): +// +// windows %APPDATA%\Code\User\settings.json +// darwin ~/Library/Application Support/Code/User/settings.json +// linux $XDG_CONFIG_HOME/Code/User/settings.json (default ~/.config) +// +// ok=false when the base directory cannot be resolved (no home, no %APPDATA%) +// or the OS is not one of the three supported platforms. +func settingsPath() (string, bool) { + switch runtime.GOOS { + case "windows": + appdata := os.Getenv("APPDATA") + if appdata == "" { + return "", false + } + return filepath.Join(appdata, "Code", "User", "settings.json"), true + case "darwin": + home, err := os.UserHomeDir() + if err != nil || home == "" { + return "", false + } + return filepath.Join(home, "Library", "Application Support", "Code", "User", "settings.json"), true + case "linux": + base := os.Getenv("XDG_CONFIG_HOME") + if base == "" { + home, err := os.UserHomeDir() + if err != nil || home == "" { + return "", false + } + base = filepath.Join(home, ".config") + } + return filepath.Join(base, "Code", "User", "settings.json"), true + default: + return "", false + } +} + +func (w *settingsWriter) Location() string { + return w.path + ` [` + allowedExtensionsSettingKey + `]` +} + +// load reads and parses the settings file. Returns: +// - the syntax tree (an empty object when the file is absent or blank, so +// callers can patch a first key into a fresh file); +// - existed=false when the file is absent; +// - an error when the file exists but is unreadable, is not parseable JSONC, +// or its root is not an object — the never-clobber contract. +func (w *settingsWriter) load() (v hujson.Value, existed bool, err error) { + // #nosec G304 -- w.path is settingsPath() (env/home + fixed segments) or a + // test override, never external input. + b, err := os.ReadFile(w.path) + if errors.Is(err, os.ErrNotExist) { + v, _ := hujson.Parse([]byte("{}")) + return v, false, nil + } + if err != nil { + return hujson.Value{}, false, fmt.Errorf("devicepolicy: read %s: %w", w.path, err) + } + if len(bytes.TrimSpace(b)) == 0 { + // An empty file is how VS Code-adjacent tooling often seeds settings; + // treat it as an empty object rather than a parse error. + v, _ := hujson.Parse([]byte("{}")) + return v, true, nil + } + v, perr := hujson.Parse(b) + if perr != nil { + return hujson.Value{}, true, fmt.Errorf("devicepolicy: %s is not valid JSONC, refusing to touch it: %w", w.path, perr) + } + if _, ok := v.Value.(*hujson.Object); !ok { + return hujson.Value{}, true, fmt.Errorf("devicepolicy: %s root is not a JSON object, refusing to touch it", w.path) + } + return v, true, nil +} + +// extract returns the compacted current value of the extensions.allowed key +// from a parsed tree, or ok=false when the key is absent. Compaction +// normalizes whitespace (and any comments inside the value, which Standardize +// strips) so values compare canonically regardless of on-disk formatting. +func extractAllowedExtensions(v hujson.Value) (string, bool, error) { + std := v.Clone() + std.Standardize() + m := map[string]json.RawMessage{} + if err := json.Unmarshal(std.Pack(), &m); err != nil { + return "", false, fmt.Errorf("devicepolicy: standardize settings: %w", err) + } + raw, ok := m[allowedExtensionsSettingKey] + if !ok { + return "", false, nil + } + s, err := compactJSON(raw) + if err != nil { + return "", false, err + } + return s, true, nil +} + +// compactJSON returns raw with insignificant whitespace removed. Member order +// is preserved, so two compactions are byte-equal iff the underlying JSON has +// identical structure and order — exactly the comparison ownership and +// readback need. +func compactJSON(raw []byte) (string, error) { + var buf bytes.Buffer + if err := json.Compact(&buf, raw); err != nil { + return "", fmt.Errorf("devicepolicy: compact value: %w", err) + } + return buf.String(), nil +} + +func (w *settingsWriter) Read() (string, bool, error) { + v, existed, err := w.load() + if err != nil { + return "", false, err + } + if !existed { + return "", false, nil + } + return extractAllowedExtensions(v) +} + +// Write upserts the extensions.allowed key to value (a compact JSON object +// string — the reconciler passes the backend's compiled policy compacted) and +// returns the value read back from disk. +func (w *settingsWriter) Write(value string) (string, error) { + if !json.Valid([]byte(value)) || !isJSONObject([]byte(value)) { + // The patch document embeds value verbatim; reject anything that is not + // a JSON object before it can corrupt the patch (defense in depth — the + // fetcher already enforces object shape). + return "", fmt.Errorf("devicepolicy: refusing to write non-object policy value to %s", w.path) + } + v, _, err := w.load() + if err != nil { + return "", err + } + // RFC 6902 "add" on an object member is an upsert. The key contains no '/' + // or '~', so it needs no JSON-Pointer escaping; the dot is literal. + patch := `[{"op":"add","path":"/` + allowedExtensionsSettingKey + `","value":` + value + `}]` + if err := v.Patch([]byte(patch)); err != nil { + return "", fmt.Errorf("devicepolicy: patch %s: %w", w.path, err) + } + if err := w.store(v); err != nil { + return "", err + } + rb, _, err := w.Read() + if err != nil { + return "", err + } + return rb, nil +} + +// Clear removes the extensions.allowed key. The file is never deleted (it is +// the user's settings.json); a file or key already absent is a no-op that +// performs no write at all. +func (w *settingsWriter) Clear() error { + v, existed, err := w.load() + if err != nil { + return err + } + if !existed { + return nil + } + if _, present, err := extractAllowedExtensions(v); err != nil { + return err + } else if !present { + return nil + } + patch := `[{"op":"remove","path":"/` + allowedExtensionsSettingKey + `"}]` + if err := v.Patch([]byte(patch)); err != nil { + return fmt.Errorf("devicepolicy: patch %s: %w", w.path, err) + } + return w.store(v) +} + +// store atomically replaces the settings file with the packed tree, preserving +// the existing file mode and keeping a capped sibling backup of the previous +// content (atomicfile: temp in target dir → fsync → rename). +func (w *settingsWriter) store(v hujson.Value) error { + mode := atomicfile.PickMode(w.path, settingsFileMode) + if _, err := atomicfile.WriteAtomic(w.path, v.Pack(), mode); err != nil { + return fmt.Errorf("devicepolicy: write %s: %w", w.path, err) + } + return nil +} diff --git a/internal/devicepolicy/settings_writer_test.go b/internal/devicepolicy/settings_writer_test.go new file mode 100644 index 0000000..2172e92 --- /dev/null +++ b/internal/devicepolicy/settings_writer_test.go @@ -0,0 +1,408 @@ +package devicepolicy + +import ( + "encoding/json" + "os" + "path/filepath" + "runtime" + "strings" + "testing" + + "github.com/step-security/dev-machine-guard/internal/atomicfile" +) + +const samplePolicyObject = `{"github.copilot":true,"ms-python.python":"1.2.3"}` + +// sampleSettings exercises every JSONC feature the writer must preserve: +// line + block comments, trailing commas (object and nested), irregular +// whitespace, and unrelated keys before and after where the policy lands. +const sampleSettings = `// StepSecurity test fixture — user settings +{ + /* appearance */ + "workbench.colorTheme": "Solarized Dark", // user's favorite + "editor.fontSize": 14, + "files.exclude": { + "**/.git": true, + }, + + // telemetry opt-out + "telemetry.telemetryLevel": "off", +} +` + +// preservedFragments are exact byte sequences from sampleSettings that must +// survive any single-key edit untouched. +var preservedFragments = []string{ + "// StepSecurity test fixture — user settings", + "/* appearance */", + `"workbench.colorTheme": "Solarized Dark", // user's favorite`, + `"editor.fontSize": 14,`, + "\"files.exclude\": {\n\t\t\"**/.git\": true,\n\t},", + "// telemetry opt-out", + // No trailing comma asserted: when the policy key is removed from the end + // of the object, hujson also drops the separator comma after this (then + // last) member — separator syntax is part of the touched region. + `"telemetry.telemetryLevel": "off"`, +} + +func newTestSettingsWriter(t *testing.T) (*settingsWriter, string) { + t.Helper() + path := filepath.Join(t.TempDir(), "User", "settings.json") + return newSettingsWriterAt(path), path +} + +func writeSettingsFixture(t *testing.T, path, content string) { + t.Helper() + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte(content), 0o644); err != nil { + t.Fatal(err) + } +} + +func readFileString(t *testing.T, path string) string { + t.Helper() + b, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + return string(b) +} + +func assertFragmentsPreserved(t *testing.T, got string) { + t.Helper() + for _, frag := range preservedFragments { + if !strings.Contains(got, frag) { + t.Errorf("fragment lost after edit:\n%q\n--- file now:\n%s", frag, got) + } + } +} + +func TestSettingsWriteAddsKeyPreservingFile(t *testing.T) { + w, path := newTestSettingsWriter(t) + writeSettingsFixture(t, path, sampleSettings) + + rb, err := w.Write(samplePolicyObject) + if err != nil { + t.Fatalf("Write: %v", err) + } + if rb != samplePolicyObject { + t.Fatalf("readback = %q, want %q", rb, samplePolicyObject) + } + + after := readFileString(t, path) + assertFragmentsPreserved(t, after) + + // The file must remain valid JSONC holding both old and new keys. + got, present, err := w.Read() + if err != nil || !present || got != samplePolicyObject { + t.Fatalf("Read = (%q, %v, %v), want (%q, true, nil)", got, present, err, samplePolicyObject) + } +} + +func TestSettingsWriteReplacesExistingKeyOnly(t *testing.T) { + w, path := newTestSettingsWriter(t) + fixture := strings.Replace(sampleSettings, + "\t// telemetry opt-out", + "\t/* managed below */\n\t\"extensions.allowed\": { \"old.ext\": true /* stale */ },\n\n\t// telemetry opt-out", 1) + writeSettingsFixture(t, path, fixture) + + if _, err := w.Write(samplePolicyObject); err != nil { + t.Fatalf("Write: %v", err) + } + after := readFileString(t, path) + assertFragmentsPreserved(t, after) + if strings.Contains(after, "old.ext") { + t.Fatalf("stale policy value survived the replace:\n%s", after) + } + got, present, err := w.Read() + if err != nil || !present || got != samplePolicyObject { + t.Fatalf("Read = (%q, %v, %v), want (%q, true, nil)", got, present, err, samplePolicyObject) + } +} + +// TestSettingsWriteLeavesRecoverableBackup pins the safety net for editing a +// file the user owns: before overwriting settings.json the writer (through +// atomicfile) drops a sibling `.dmg-.bak` holding the EXACT prior +// bytes, so a botched write is always recoverable. A single write yields +// exactly one backup; retention beyond that (the MaxBackups=3 cap and prune +// ordering) is atomicfile's own concern — and can't be exercised through Write +// here because the stamp has second granularity, so sub-second writes collide +// on one filename. atomicfile_test.go covers the cap with an injectable clock. +func TestSettingsWriteLeavesRecoverableBackup(t *testing.T) { + w, path := newTestSettingsWriter(t) + writeSettingsFixture(t, path, sampleSettings) + + if _, err := w.Write(samplePolicyObject); err != nil { + t.Fatalf("Write: %v", err) + } + + backups, err := filepath.Glob(path + atomicfile.BackupPrefix + "*" + atomicfile.BackupExt) + if err != nil { + t.Fatal(err) + } + if len(backups) != 1 { + t.Fatalf("want exactly 1 backup after one write, got %d: %v", len(backups), backups) + } + // The backup must be the pre-write file verbatim — the point is a usable + // rollback, not merely some file ending in .bak. + if got := readFileString(t, backups[0]); got != sampleSettings { + t.Fatalf("backup is not the original file:\nbackup:\n%s\n--- want:\n%s", got, sampleSettings) + } + // Sanity: the live file took the new key (we backed up the OLD content, and + // the write still landed). + if got, present, err := w.Read(); err != nil || !present || got != samplePolicyObject { + t.Fatalf("live file Read = (%q, %v, %v), want %q", got, present, err, samplePolicyObject) + } +} + +// TestSettingsWriteCreatingFileMakesNoBackup is the boundary of the rule above: +// a first-ever write (no settings.json yet) has nothing to preserve, so it must +// NOT leave a phantom .bak. Locks the behavior so nobody later "fixes" +// TakeBackup to error on a missing source. +func TestSettingsWriteCreatingFileMakesNoBackup(t *testing.T) { + w, path := newTestSettingsWriter(t) + + if _, err := w.Write(samplePolicyObject); err != nil { + t.Fatalf("Write: %v", err) + } + backups, err := filepath.Glob(path + atomicfile.BackupPrefix + "*" + atomicfile.BackupExt) + if err != nil { + t.Fatal(err) + } + if len(backups) != 0 { + t.Fatalf("first-write should make no backup, got %v", backups) + } +} + +func TestSettingsWriteIsByteIdempotent(t *testing.T) { + w, path := newTestSettingsWriter(t) + writeSettingsFixture(t, path, sampleSettings) + + if _, err := w.Write(samplePolicyObject); err != nil { + t.Fatalf("first Write: %v", err) + } + first := readFileString(t, path) + if _, err := w.Write(samplePolicyObject); err != nil { + t.Fatalf("second Write: %v", err) + } + if second := readFileString(t, path); second != first { + t.Fatalf("second identical Write changed the file:\nfirst:\n%s\nsecond:\n%s", first, second) + } +} + +func TestSettingsWriteCreatesMissingFileAndDirs(t *testing.T) { + w, path := newTestSettingsWriter(t) + // No fixture: neither the User dir nor the file exists. + + rb, err := w.Write(samplePolicyObject) + if err != nil { + t.Fatalf("Write: %v", err) + } + if rb != samplePolicyObject { + t.Fatalf("readback = %q, want %q", rb, samplePolicyObject) + } + var m map[string]json.RawMessage + if err := json.Unmarshal([]byte(readFileString(t, path)), &m); err != nil { + t.Fatalf("created file is not plain JSON: %v", err) + } + if _, ok := m[allowedExtensionsSettingKey]; !ok || len(m) != 1 { + t.Fatalf("created file should hold exactly the policy key, got %v", m) + } +} + +func TestSettingsWriteTreatsBlankFileAsEmptyObject(t *testing.T) { + w, path := newTestSettingsWriter(t) + writeSettingsFixture(t, path, "\n \n") + + if _, err := w.Write(samplePolicyObject); err != nil { + t.Fatalf("Write on blank file: %v", err) + } + got, present, err := w.Read() + if err != nil || !present || got != samplePolicyObject { + t.Fatalf("Read = (%q, %v, %v), want (%q, true, nil)", got, present, err, samplePolicyObject) + } +} + +func TestSettingsReadCompactsFormattedValue(t *testing.T) { + w, path := newTestSettingsWriter(t) + writeSettingsFixture(t, path, `{ + "extensions.allowed": { + // allow-list managed elsewhere + "github.copilot": true, + "ms-python.python": "1.2.3", + }, +}`) + got, present, err := w.Read() + if err != nil || !present { + t.Fatalf("Read = (%q, %v, %v), want present", got, present, err) + } + want := `{"github.copilot":true,"ms-python.python":"1.2.3"}` + if got != want { + t.Fatalf("Read = %q, want compacted %q", got, want) + } +} + +func TestSettingsReadAbsent(t *testing.T) { + w, path := newTestSettingsWriter(t) + + // Missing file. + if got, present, err := w.Read(); err != nil || present || got != "" { + t.Fatalf("Read(missing file) = (%q, %v, %v), want absent", got, present, err) + } + // File without the key. + writeSettingsFixture(t, path, sampleSettings) + if got, present, err := w.Read(); err != nil || present || got != "" { + t.Fatalf("Read(no key) = (%q, %v, %v), want absent", got, present, err) + } +} + +func TestSettingsClearRemovesOnlyTheKey(t *testing.T) { + w, path := newTestSettingsWriter(t) + writeSettingsFixture(t, path, sampleSettings) + if _, err := w.Write(samplePolicyObject); err != nil { + t.Fatalf("Write: %v", err) + } + + if err := w.Clear(); err != nil { + t.Fatalf("Clear: %v", err) + } + after := readFileString(t, path) + assertFragmentsPreserved(t, after) + if strings.Contains(after, allowedExtensionsSettingKey) { + t.Fatalf("policy key survived Clear:\n%s", after) + } + if _, present, err := w.Read(); err != nil || present { + t.Fatalf("key still present after Clear (err=%v)", err) + } +} + +func TestSettingsClearAbsentIsNoOp(t *testing.T) { + w, path := newTestSettingsWriter(t) + + // Missing file: Clear must not create it. + if err := w.Clear(); err != nil { + t.Fatalf("Clear(missing file): %v", err) + } + if _, err := os.Stat(path); !os.IsNotExist(err) { + t.Fatalf("Clear created the settings file") + } + + // File without the key: Clear must not rewrite it. + writeSettingsFixture(t, path, sampleSettings) + if err := w.Clear(); err != nil { + t.Fatalf("Clear(no key): %v", err) + } + if got := readFileString(t, path); got != sampleSettings { + t.Fatalf("Clear rewrote a file it had no key in:\n%s", got) + } +} + +func TestSettingsUnsalvageableFileIsNeverTouched(t *testing.T) { + const broken = `{"editor.fontSize": 14, <<