Improve stamping of fields in the config

client/internal/profilemanager/migration.go

@@ -204,7 +204,8 @@ func (s *ServiceManager) stampActiveUserDir(profiles []Profile, active *ActivePr
 			continue
 		}
 		if err := stampPrincipal(p.Path, principal); err != nil {
-			return fmt.Errorf("stamp %s: %w", p.ID, err)
+			log.Warnf("leaving %s unowned, its owner could not be recorded: %v", p.Path, err)
+			continue
 		}
 		log.Infof("recorded %s as the owner of %s, the directory it sits in is that account's", principal, p.Path)
 	}

client/internal/profilemanager/migration_test.go

@@ -266,3 +266,26 @@ func TestMigrate_LeavesAProfileThatAlreadyNamesAnOwnerAlone(t *testing.T) {
 			"only the profiles with no owner of their own are attributed")
 	})
 }
+
+func TestMigrate_SkipsAProfileItCannotStamp(t *testing.T) {
+	username, principal := currentUserPrincipal(t)
+
+	withLegacyLayout(t, func(sm *ServiceManager, configDir string) {
+		dir := sanitizeProfileName(username)
+		good := writeLegacyProfile(t, configDir, dir, "work", nil)
+		broken := filepath.Join(configDir, dir, "broken.json")
+		require.NoError(t, os.WriteFile(broken, []byte("null"), 0600))
+		require.NoError(t, sm.SetActiveProfileState(&ActiveProfileState{ID: "work", Username: username}))
+
+		require.NoError(t, sm.MigrateLegacyProfiles())
+
+		assert.Equal(t, []string{principal}, readOwners(t, good),
+			"one profile that cannot be stamped does not hold up the rest")
+		assert.DirExists(t, filepath.Join(configDir, DefaultProfilePathDir),
+			"and the marker still lands, since the claim path retries the one left behind")
+
+		data, err := os.ReadFile(broken)
+		require.NoError(t, err)
+		assert.Equal(t, "null", string(data), "the profile it could not stamp is untouched")
+	})
+}

client/internal/profilemanager/service.go

@@ -63,7 +63,12 @@ type ownerMeta struct {
 	Owners []string
 }
 
-// ownersFieldName is the key on disk.
+// Config JSON keys on disk.
+const (
+	ownersFieldName = "Owners"
+	nameFieldName   = "Name"
+)
+
 func (e *ErrAmbiguousHandle) Error() string {
 	switch e.Kind {
 	case AmbiguityKindIDPrefix:
@@ -900,37 +905,45 @@ func StampOwner(path string, owner ipcauth.Identity) error {
 // resolves an account name rather than a caller, and a name the kernel never
 // vouched for must not become an Identity on the way.
 func stampPrincipal(path, principal string) error {
-	return updateProfileConfig(path, func(cfg *Config) {
-		cfg.Owners = []string{principal}
-	})
+	return setProfileField(path, ownersFieldName, []string{principal})
 }
 
 // writeProfileName sets a profile's display name. Renaming does it on request,
 // migration does it to move a name out of a filename that is about to change.
 func writeProfileName(path, name string) error {
-	return updateProfileConfig(path, func(cfg *Config) {
-		cfg.Name = name
-	})
+	return setProfileField(path, nameFieldName, name)
 }
 
-// updateProfileConfig reads a profile, applies mutate and writes it back.
-//
-// The whole config makes the round trip, which is what every writer here does,
-// so a field this version does not model is dropped. That only happens after a
-// downgrade, and a downgrade already drops the owners it cannot read.
-func updateProfileConfig(path string, mutate func(*Config)) error {
+// setProfileField replaces one top-level key of a profile's JSON and leaves the
+// rest of the document as it found it.
+func setProfileField(path, field string, value any) error {
 	data, err := os.ReadFile(path)
 	if err != nil {
 		return err
 	}
 
-	var cfg Config
-	if err := json.Unmarshal(data, &cfg); err != nil {
+	doc := map[string]json.RawMessage{}
+	if err := json.Unmarshal(data, &doc); err != nil {
 		return err
 	}
-	mutate(&cfg)
+	if doc == nil {
+		return fmt.Errorf("profile %s holds no object to set %s on", path, field)
+	}
 
-	if err := util.WriteJsonWithRestrictedPermission(context.Background(), path, cfg); err != nil {
+	raw, err := json.Marshal(value)
+	if err != nil {
+		return fmt.Errorf("encode %s of %s: %w", field, path, err)
+	}
+
+	// Decoding matches keys case-insensitively
+	for k := range doc {
+		if k != field && strings.EqualFold(k, field) {
+			delete(doc, k)
+		}
+	}
+	doc[field] = raw
+
+	if err := util.WriteJsonWithRestrictedPermission(context.Background(), path, doc); err != nil {
 		return fmt.Errorf("write profile %s: %w", path, err)
 	}
 	return nil

client/internal/profilemanager/service_test.go

@@ -659,3 +659,104 @@ func TestActiveProfilePath_RefusesToGuessBetweenNamesakes(t *testing.T) {
 			"the recorded directory is what tells the namesakes apart")
 	})
 }
+
+func TestListProfiles_ClaimKeepsFieldsThisVersionDoesNotModel(t *testing.T) {
+	withLegacyLayout(t, func(sm *ServiceManager, configDir string) {
+		// What a client newer than this one leaves behind: a key Config has no
+		// field for, next to one it does.
+		newer := map[string]any{"Enabled": true, "Hosts": []any{"a", "b"}}
+		path := writeLegacyProfile(t, configDir, "alice", "work", map[string]any{
+			"MTU":            1280,
+			"SomethingNewer": newer,
+		})
+		stubLegacyDir(t, "alice")
+
+		alice := ipcauth.KnownForTest(ipcauth.Identity{UID: 4242})
+		_, err := sm.ListProfiles(alice)
+		require.NoError(t, err)
+		assert.Equal(t, []string{"uid:4242"}, readOwners(t, path), "the claim still lands")
+
+		data, err := os.ReadFile(path)
+		require.NoError(t, err)
+		var doc map[string]any
+		require.NoError(t, json.Unmarshal(data, &doc))
+
+		assert.Equal(t, newer, doc["SomethingNewer"],
+			"a listing must not drop the settings of a client that models more than this one")
+		assert.Equal(t, float64(1280), doc["MTU"], "and leaves the ones it does model alone")
+		assert.NotContains(t, doc, "PrivateKey",
+			"nor write out the rest of Config just because it has fields for it")
+	})
+}
+
+func TestSetProfileField_ReplacesAKeySpelledInAnotherCase(t *testing.T) {
+	withLegacyLayout(t, func(sm *ServiceManager, configDir string) {
+		path := writeLegacyProfile(t, configDir, "alice", "work", map[string]any{
+			"owners": []any{"uid:1"},
+		})
+
+		require.NoError(t, stampPrincipal(path, "uid:4242"))
+
+		data, err := os.ReadFile(path)
+		require.NoError(t, err)
+		var doc map[string]any
+		require.NoError(t, json.Unmarshal(data, &doc))
+
+		assert.NotContains(t, doc, "owners",
+			"two spellings of one field would leave the reader to pick")
+		assert.Equal(t, []any{"uid:4242"}, doc["Owners"])
+		assert.Equal(t, []string{"uid:4242"}, readOwners(t, path))
+	})
+}
+
+func TestSetProfileField_RefusesADocumentThatIsNotAnObject(t *testing.T) {
+	withLegacyLayout(t, func(sm *ServiceManager, configDir string) {
+		path := filepath.Join(configDir, "alice", "work.json")
+		require.NoError(t, os.MkdirAll(filepath.Dir(path), 0700))
+		require.NoError(t, os.WriteFile(path, []byte("null"), 0600))
+
+		require.Error(t, stampPrincipal(path, "uid:4242"),
+			"a profile that is not an object is not one an owner can be set on")
+
+		data, err := os.ReadFile(path)
+		require.NoError(t, err)
+		assert.Equal(t, "null", string(data), "and it is left as it was found")
+	})
+}
+
+func TestSetProfileField_KeepsKeysItWasNotAskedToWrite(t *testing.T) {
+	withLegacyLayout(t, func(sm *ServiceManager, configDir string) {
+		// Keys Config has no field for, in every shape a newer client could
+		// leave one behind.
+		unknown := map[string]any{
+			"AString":  "keep me",
+			"ANumber":  float64(7),
+			"ABool":    true,
+			"ANull":    nil,
+			"AList":    []any{"a", float64(2), false},
+			"AnObject": map[string]any{"Nested": map[string]any{"Deep": []any{float64(1)}}},
+		}
+		fields := map[string]any{"MTU": 1280}
+		for k, v := range unknown {
+			fields[k] = v
+		}
+		path := writeLegacyProfile(t, configDir, "alice", "work", fields)
+
+		require.NoError(t, setProfileField(path, ownersFieldName, []string{"uid:4242"}))
+		require.NoError(t, setProfileField(path, nameFieldName, "Work"))
+
+		data, err := os.ReadFile(path)
+		require.NoError(t, err)
+		var doc map[string]any
+		require.NoError(t, json.Unmarshal(data, &doc))
+
+		for k, want := range unknown {
+			assert.Equal(t, want, doc[k],
+				"%s is not a key this version models, so it is not this version's to drop", k)
+		}
+		assert.Equal(t, float64(1280), doc["MTU"], "a key it does model is left where it was too")
+		assert.Equal(t, []any{"uid:4242"}, doc["Owners"], "and the fields it was asked for are written")
+		assert.Equal(t, "Work", doc["Name"])
+		assert.Len(t, doc, len(unknown)+3, "with nothing else added")
+	})
+}