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

The Policy struct only defined fields it explicitly used, causing any other
fields in policy.json to be silently dropped when AddExtension performed a
read-modify-write cycle. This broke the DefaultGeolocationSetting added in #136.

Changes:
- Add custom UnmarshalJSON/MarshalJSON to preserve unknown JSON fields
- Simplify Policy struct to only include fields we programmatically modify
  (ExtensionInstallForcelist, ExtensionSettings)
- All other Chrome policy settings are now automatically preserved
- Add comprehensive tests for the preservation behavior

This allows policy.json to contain any Chrome policy setting without
requiring Go struct updates, as long as we don't need to modify it in code.

Fixes issue introduced by #136

Author: rgarcia

server/lib/policy/policy.go

@@ -12,30 +12,94 @@ 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
+}
+
+// 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
+	}
+
+	// Use an alias type to avoid infinite recursion when unmarshaling known fields
+	type PolicyAlias Policy
+	var alias PolicyAlias
+	if err := json.Unmarshal(data, &alias); err != nil {
+		return err
+	}
+
+	// Copy the alias back to p (excluding unknownFields which we'll set separately)
+	*p = Policy(alias)
+
+	// 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) {
+	// Use an alias type to avoid infinite recursion
+	type PolicyAlias Policy
+	alias := PolicyAlias(p)
+
+	// Marshal the known fields first
+	knownData, err := json.Marshal(alias)
+	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 +118,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"])
+}