fix: preserve unknown fields in policy.json during read-modify-write cycles

server/lib/policy/policy.go

@@ -12,30 +12,103 @@ import (
 )
 
 const PolicyPath = "/etc/chromium/policies/managed/policy.json"
-const DefaultSearchProviderName = "DuckDuckGo"
-const DefaultSearchProviderSearchURL = "https://duckduckgo.com/?q={searchTerms}"
-const DefaultSearchProviderSuggestURL = "https://duckduckgo.com/ac/?q={searchTerms}"
-const NewTabPageLocation = "https://start.duckduckgo.com/"
 
 // Chrome extension IDs are 32 lowercase a-p characters
 var extensionIDRegex = regexp.MustCompile(`^[a-p]{32}$`)
 var extensionPathRegex = regexp.MustCompile(`/extensions/[^/]+/`)
 
-// Policy represents the Chrome enterprise policy structure
+// Policy represents the Chrome enterprise policy structure.
+// Only fields that are programmatically modified are defined here.
+// All other fields (like DefaultGeolocationSetting, PasswordManagerEnabled, etc.)
+// are preserved through the unknownFields mechanism during read-modify-write cycles.
 type Policy struct {
 	mu sync.Mutex
 
-	PasswordManagerEnabled          bool                        `json:"PasswordManagerEnabled"`
-	AutofillCreditCardEnabled       bool                        `json:"AutofillCreditCardEnabled"`
-	TranslateEnabled                bool                        `json:"TranslateEnabled"`
-	DefaultNotificationsSetting     int                         `json:"DefaultNotificationsSetting"`
-	DefaultSearchProviderEnabled    bool                        `json:"DefaultSearchProviderEnabled"`
-	DefaultSearchProviderName       string                      `json:"DefaultSearchProviderName"`
-	DefaultSearchProviderSearchURL  string                      `json:"DefaultSearchProviderSearchURL"`
-	DefaultSearchProviderSuggestURL string                      `json:"DefaultSearchProviderSuggestURL"`
-	NewTabPageLocation              string                      `json:"NewTabPageLocation"`
-	ExtensionInstallForcelist       []string                    `json:"ExtensionInstallForcelist,omitempty"`
-	ExtensionSettings               map[string]ExtensionSetting `json:"ExtensionSettings"`
+	// ExtensionInstallForcelist is modified when adding force-installed extensions
+	ExtensionInstallForcelist []string `json:"ExtensionInstallForcelist,omitempty"`
+	// ExtensionSettings is modified when adding/configuring extensions
+	ExtensionSettings map[string]ExtensionSetting `json:"ExtensionSettings"`
+
+	// unknownFields preserves all JSON fields not explicitly defined in this struct.
+	// This allows policy.json to contain any Chrome policy settings without
+	// requiring updates to this Go struct.
+	unknownFields map[string]json.RawMessage
+}
+
+// policyJSON is used for JSON marshaling/unmarshaling without the mutex.
+// This avoids go vet warnings about copying mutex values.
+type policyJSON struct {
+	ExtensionInstallForcelist []string                    `json:"ExtensionInstallForcelist,omitempty"`
+	ExtensionSettings         map[string]ExtensionSetting `json:"ExtensionSettings"`
+}
+
+// knownPolicyFields lists all JSON field names that have corresponding struct fields.
+// All other fields are automatically preserved in unknownFields.
+var knownPolicyFields = map[string]bool{
+	"ExtensionInstallForcelist": true,
+	"ExtensionSettings":         true,
+}
+
+// UnmarshalJSON implements custom JSON unmarshaling that preserves unknown fields.
+func (p *Policy) UnmarshalJSON(data []byte) error {
+	// First, unmarshal into a map to capture all fields
+	var allFields map[string]json.RawMessage
+	if err := json.Unmarshal(data, &allFields); err != nil {
+		return err
+	}
+
+	// Unmarshal known fields into the helper struct (no mutex)
+	var pj policyJSON
+	if err := json.Unmarshal(data, &pj); err != nil {
+		return err
+	}
+
+	// Copy the known fields to p
+	p.ExtensionInstallForcelist = pj.ExtensionInstallForcelist
+	p.ExtensionSettings = pj.ExtensionSettings
+
+	// Extract unknown fields
+	p.unknownFields = make(map[string]json.RawMessage)
+	for key, value := range allFields {
+		if !knownPolicyFields[key] {
+			p.unknownFields[key] = value
+		}
+	}
+
+	return nil
+}
+
+// MarshalJSON implements custom JSON marshaling that includes unknown fields.
+func (p *Policy) MarshalJSON() ([]byte, error) {
+	// Create helper struct with known fields (no mutex)
+	pj := policyJSON{
+		ExtensionInstallForcelist: p.ExtensionInstallForcelist,
+		ExtensionSettings:         p.ExtensionSettings,
+	}
+
+	// Marshal the known fields first
+	knownData, err := json.Marshal(pj)
+	if err != nil {
+		return nil, err
+	}
+
+	// If no unknown fields, return as-is
+	if len(p.unknownFields) == 0 {
+		return knownData, nil
+	}
+
+	// Unmarshal known fields into a map so we can add unknown fields
+	var result map[string]json.RawMessage
+	if err := json.Unmarshal(knownData, &result); err != nil {
+		return nil, err
+	}
+
+	// Add unknown fields
+	for key, value := range p.unknownFields {
+		result[key] = value
+	}
+
+	return json.Marshal(result)
 }
 
 // ExtensionSetting represents settings for a specific extension
@@ -54,19 +127,11 @@ func (p *Policy) readPolicyUnlocked() (*Policy, error) {
 	data, err := os.ReadFile(PolicyPath)
 	if err != nil {
 		if os.IsNotExist(err) {
-			// Return default policy if file doesn't exist
+			// Return minimal policy if file doesn't exist.
+			// In practice, policy.json ships with the container image.
 			return &Policy{
-				PasswordManagerEnabled:          false,
-				AutofillCreditCardEnabled:       false,
-				TranslateEnabled:                false,
-				DefaultNotificationsSetting:     2,
-				DefaultSearchProviderEnabled:    true,
-				DefaultSearchProviderName:       DefaultSearchProviderName,
-				DefaultSearchProviderSearchURL:  DefaultSearchProviderSearchURL,
-				DefaultSearchProviderSuggestURL: DefaultSearchProviderSuggestURL,
-				NewTabPageLocation:              NewTabPageLocation,
-				ExtensionInstallForcelist:       []string{},
-				ExtensionSettings:               make(map[string]ExtensionSetting),
+				ExtensionInstallForcelist: []string{},
+				ExtensionSettings:         make(map[string]ExtensionSetting),
 			}, nil
 		}
 		return nil, fmt.Errorf("failed to read policy file: %w", err)

server/lib/policy/policy_test.go

@@ -0,0 +1,170 @@
+package policy
+
+import (
+	"encoding/json"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestPolicy_PreservesUnknownFields(t *testing.T) {
+	// Simulate the policy.json with many fields not in the struct
+	// Only ExtensionInstallForcelist and ExtensionSettings are in the struct
+	input := `{
+		"PasswordManagerEnabled": false,
+		"AutofillCreditCardEnabled": false,
+		"TranslateEnabled": false,
+		"DefaultNotificationsSetting": 2,
+		"DefaultGeolocationSetting": 2,
+		"DefaultSearchProviderEnabled": true,
+		"DefaultSearchProviderName": "DuckDuckGo",
+		"DefaultSearchProviderSearchURL": "https://duckduckgo.com/?q={searchTerms}",
+		"DefaultSearchProviderSuggestURL": "https://duckduckgo.com/ac/?q={searchTerms}",
+		"NewTabPageLocation": "https://start.duckduckgo.com/",
+		"SomeOtherUnknownField": {"nested": "value"},
+		"ExtensionSettings": {
+			"*": {
+				"allowed_types": ["extension"],
+				"install_sources": ["*"]
+			}
+		}
+	}`
+
+	var policy Policy
+	err := json.Unmarshal([]byte(input), &policy)
+	require.NoError(t, err)
+
+	// Verify known struct fields are parsed correctly
+	assert.NotNil(t, policy.ExtensionSettings)
+	assert.Contains(t, policy.ExtensionSettings, "*")
+
+	// Verify all non-struct fields are captured as unknown
+	// All fields except ExtensionSettings should be in unknownFields
+	expectedUnknownFields := []string{
+		"PasswordManagerEnabled",
+		"AutofillCreditCardEnabled",
+		"TranslateEnabled",
+		"DefaultNotificationsSetting",
+		"DefaultGeolocationSetting",
+		"DefaultSearchProviderEnabled",
+		"DefaultSearchProviderName",
+		"DefaultSearchProviderSearchURL",
+		"DefaultSearchProviderSuggestURL",
+		"NewTabPageLocation",
+		"SomeOtherUnknownField",
+	}
+	for _, field := range expectedUnknownFields {
+		assert.Contains(t, policy.unknownFields, field, "field %s should be preserved", field)
+	}
+
+	// Now marshal it back
+	output, err := json.Marshal(&policy)
+	require.NoError(t, err)
+
+	// Unmarshal into a map to verify all fields are present
+	var result map[string]interface{}
+	err = json.Unmarshal(output, &result)
+	require.NoError(t, err)
+
+	// Verify all fields are preserved
+	assert.Equal(t, float64(2), result["DefaultGeolocationSetting"])
+	assert.Equal(t, float64(2), result["DefaultNotificationsSetting"])
+	assert.Equal(t, false, result["PasswordManagerEnabled"])
+	assert.Equal(t, "DuckDuckGo", result["DefaultSearchProviderName"])
+	assert.NotNil(t, result["SomeOtherUnknownField"])
+}
+
+func TestPolicy_ModifyAndPreserveUnknownFields(t *testing.T) {
+	// This test simulates the AddExtension read-modify-write cycle
+	input := `{
+		"PasswordManagerEnabled": false,
+		"DefaultGeolocationSetting": 2,
+		"DefaultNotificationsSetting": 2,
+		"ExtensionSettings": {}
+	}`
+
+	var policy Policy
+	err := json.Unmarshal([]byte(input), &policy)
+	require.NoError(t, err)
+
+	// Add an extension setting (this is what AddExtension does)
+	policy.ExtensionSettings["test-extension"] = ExtensionSetting{
+		UpdateUrl: "http://127.0.0.1:10001/extensions/test/update.xml",
+	}
+
+	// Marshal it back
+	output, err := json.Marshal(&policy)
+	require.NoError(t, err)
+
+	// Verify the unknown fields are still there
+	var result map[string]interface{}
+	err = json.Unmarshal(output, &result)
+	require.NoError(t, err)
+
+	assert.Equal(t, float64(2), result["DefaultGeolocationSetting"], "DefaultGeolocationSetting should be preserved")
+	assert.Equal(t, float64(2), result["DefaultNotificationsSetting"], "DefaultNotificationsSetting should be preserved")
+	assert.Equal(t, false, result["PasswordManagerEnabled"], "PasswordManagerEnabled should be preserved")
+
+	// Verify extension was added
+	extSettings, ok := result["ExtensionSettings"].(map[string]interface{})
+	require.True(t, ok)
+	assert.Contains(t, extSettings, "test-extension")
+}
+
+func TestPolicy_ExtensionInstallForcelist(t *testing.T) {
+	// Test that ExtensionInstallForcelist works correctly
+	input := `{
+		"DefaultGeolocationSetting": 2,
+		"ExtensionInstallForcelist": ["existing-ext;http://example.com/update.xml"],
+		"ExtensionSettings": {}
+	}`
+
+	var policy Policy
+	err := json.Unmarshal([]byte(input), &policy)
+	require.NoError(t, err)
+
+	// Verify forcelist is parsed
+	assert.Len(t, policy.ExtensionInstallForcelist, 1)
+	assert.Equal(t, "existing-ext;http://example.com/update.xml", policy.ExtensionInstallForcelist[0])
+
+	// Add to forcelist
+	policy.ExtensionInstallForcelist = append(policy.ExtensionInstallForcelist, "new-ext;http://example.com/new.xml")
+
+	// Marshal back
+	output, err := json.Marshal(&policy)
+	require.NoError(t, err)
+
+	var result map[string]interface{}
+	err = json.Unmarshal(output, &result)
+	require.NoError(t, err)
+
+	// Both unknown field and forcelist should be present
+	assert.Equal(t, float64(2), result["DefaultGeolocationSetting"])
+	forcelist, ok := result["ExtensionInstallForcelist"].([]interface{})
+	require.True(t, ok)
+	assert.Len(t, forcelist, 2)
+}
+
+func TestPolicy_EmptyPolicy(t *testing.T) {
+	// Test with minimal input
+	input := `{}`
+
+	var policy Policy
+	err := json.Unmarshal([]byte(input), &policy)
+	require.NoError(t, err)
+
+	assert.Empty(t, policy.unknownFields)
+	assert.Nil(t, policy.ExtensionSettings)
+	assert.Nil(t, policy.ExtensionInstallForcelist)
+
+	output, err := json.Marshal(&policy)
+	require.NoError(t, err)
+
+	var result map[string]interface{}
+	err = json.Unmarshal(output, &result)
+	require.NoError(t, err)
+
+	// Should have empty ExtensionSettings as null/missing
+	assert.Nil(t, result["ExtensionSettings"])
+}