From de87cfa37d1332b172f06baa5c89764c24a356cd Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sat, 3 Oct 2026 12:35:39 +0000 Subject: [PATCH 1/3] fix(drivers): bind rotated OAuth secrets to a credential owner Rename and name reuse looked up driver_secret:, so a rotated refresh token could be dropped or attached to another account. Mint credential_owner on first save, key KV rows by that id, and migrate continuing name-keyed leftovers atomically. Forecast learning ignores the new field so an upgrade does not reset PV/load models. Refs #1109 Signed-off-by: Cursor Agent Co-authored-by: Fredrik Ahlgren --- .changeset/oauth-credential-owner.md | 5 + go/cmd/ftw/driver_registry.go | 13 +- go/cmd/ftw/driver_registry_test.go | 127 ++++++++++- go/cmd/ftw/forecast_site.go | 6 + go/cmd/ftw/main.go | 4 + go/internal/api/api_myuplink_oauth_test.go | 24 ++- go/internal/config/config.go | 17 +- go/internal/config/credentials.go | 154 ++++++++++++++ go/internal/config/credentials_test.go | 232 +++++++++++++++++++++ go/internal/config/storage.go | 31 +-- go/internal/config/storage_test.go | 10 +- go/internal/drivers/registry.go | 12 +- go/internal/state/config_prefix_test.go | 26 +++ go/internal/state/configuration.go | 26 +++ 14 files changed, 646 insertions(+), 41 deletions(-) create mode 100644 .changeset/oauth-credential-owner.md create mode 100644 go/internal/config/credentials.go create mode 100644 go/internal/config/credentials_test.go create mode 100644 go/internal/state/config_prefix_test.go diff --git a/.changeset/oauth-credential-owner.md b/.changeset/oauth-credential-owner.md new file mode 100644 index 000000000..05cde1c04 --- /dev/null +++ b/.changeset/oauth-credential-owner.md @@ -0,0 +1,5 @@ +--- +"ftw": patch +--- + +Keep rotated OAuth tokens with a stable driver owner so a rename or reused name cannot attach the wrong account. diff --git a/go/cmd/ftw/driver_registry.go b/go/cmd/ftw/driver_registry.go index 148c603a9..4a8df4fb3 100644 --- a/go/cmd/ftw/driver_registry.go +++ b/go/cmd/ftw/driver_registry.go @@ -1,6 +1,7 @@ package main import ( + "github.com/srcfl/ftw/go/internal/config" "github.com/srcfl/ftw/go/internal/drivers" "github.com/srcfl/ftw/go/internal/state" "github.com/srcfl/ftw/go/internal/telemetry" @@ -11,14 +12,14 @@ func newDriverRegistry(tel *telemetry.Store, st *state.Store) *drivers.Registry // Install both callbacks before Add can initialize or poll any driver. // Rotations keep their own KV rows so they do not apply the whole config // or restart the driver that just refreshed its credential. - driverSecretKey := func(driverName, key string) string { - return "driver_secret:" + driverName + ":" + key + driverSecretKey := func(owner, key string) string { + return config.DriverSecretStateKey(owner, key) } - reg.SecretPersister = func(driverName, key, value string) error { - return st.SaveConfig(driverSecretKey(driverName, key), value) + reg.SecretPersister = func(owner, key, value string) error { + return st.SaveConfig(driverSecretKey(owner, key), value) } - reg.SecretOverride = func(driverName, key string) (string, bool) { - return st.LoadConfig(driverSecretKey(driverName, key)) + reg.SecretOverride = func(owner, key string) (string, bool) { + return st.LoadConfig(driverSecretKey(owner, key)) } return reg } diff --git a/go/cmd/ftw/driver_registry_test.go b/go/cmd/ftw/driver_registry_test.go index 9f876b8c9..477c863be 100644 --- a/go/cmd/ftw/driver_registry_test.go +++ b/go/cmd/ftw/driver_registry_test.go @@ -96,9 +96,9 @@ function driver_default_mode() end if got := metric(tel, "persist_ok"); got != 1 { t.Errorf("secret persistence during %s failed", scenario.phase) } - if got, ok := st.LoadConfig("driver_secret:oauth-test:refresh_token"); !ok || got != "synthetic-B" { - t.Errorf("rotated token B was not stored") - } + if got, ok := st.LoadConfig(config.DriverSecretStateKey(cfg.SecretOwner(), "refresh_token")); !ok || got != "synthetic-B" { + t.Errorf("rotated token B was not stored") + } }() _, tel, stop = start() defer stop() @@ -111,3 +111,124 @@ function driver_default_mode() end }) } } + +func oauthProbeSource(want string) string { + return fmt.Sprintf(` +function driver_init(config) + host.emit_metric("started_with_target", config.refresh_token == %q and 1 or 0) + host.emit_metric("started_with_B", config.refresh_token == "synthetic-B" and 1 or 0) + host.set_poll_interval(10) +end +function driver_poll() return 60000 end +function driver_command() end +function driver_default_mode() end +`, want) +} + +func waitDriverMetric(t *testing.T, tel *telemetry.Store, name, key string) float64 { + t.Helper() + deadline := time.Now().Add(2 * time.Second) + for time.Now().Before(deadline) { + if value, _, ok := tel.LatestMetric(name, key); ok { + return value + } + time.Sleep(time.Millisecond) + } + t.Fatalf("driver %s did not emit %s", name, key) + return 0 +} + +func TestDriverRegistryRotatedSecretFollowsOwnerNotName(t *testing.T) { + dir := t.TempDir() + lua := filepath.Join(dir, "oauth.lua") + if err := os.WriteFile(lua, []byte(oauthProbeSource("synthetic-B")), 0600); err != nil { + t.Fatal(err) + } + path, database := filepath.Join(dir, "config.yaml"), filepath.Join(dir, "state.db") + st, err := state.Open(database) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = st.Close() }) + if err := st.SaveConfig(config.DriverSecretStateKey("old-name", "refresh_token"), "synthetic-B"); err != nil { + t.Fatal(err) + } + + cfg, err := config.Parse([]byte(` +site: + name: Test +fuse: + max_amps: 16 +drivers: + - name: ferroamp + lua: drivers/ferroamp.lua + is_site_meter: true + capabilities: + mqtt: + host: 192.168.1.153 +api: + port: 8080 +`), dir) + if err != nil { + t.Fatal(err) + } + cfg.ConfigDatabase = database + oauth := config.Driver{ + Name: "old-name", Lua: lua, Capabilities: config.Capabilities{Standalone: true}, + Config: map[string]any{"refresh_token": "synthetic-A"}, + } + cfg.Drivers = append(cfg.Drivers, oauth) + if err := config.SaveStored(st, path, cfg); err != nil { + t.Fatal(err) + } + owner := cfg.Drivers[len(cfg.Drivers)-1].CredentialOwner + if owner == "" || owner == "old-name" { + t.Fatalf("credential_owner = %q", owner) + } + + start := func(d config.Driver) (*telemetry.Store, func()) { + t.Helper() + tel := telemetry.NewStore() + reg := newDriverRegistry(tel, st) + if err := reg.Add(context.Background(), d); err != nil { + reg.ShutdownAll() + t.Fatal(err) + } + return tel, func() { reg.ShutdownAll() } + } + + renamed := cfg.Drivers[len(cfg.Drivers)-1] + renamed.Name = "renamed" + tel, stop := start(renamed) + if got := waitDriverMetric(t, tel, "renamed", "started_with_target"); got != 1 { + t.Error("rename discarded rotated token B") + } + stop() + + if err := os.WriteFile(lua, []byte(oauthProbeSource("synthetic-C")), 0600); err != nil { + t.Fatal(err) + } + replaced := config.Driver{ + Name: "old-name", Lua: lua, Capabilities: config.Capabilities{Standalone: true}, + Config: map[string]any{"refresh_token": "synthetic-C"}, + } + replacedCfg := *cfg + replacedCfg.Drivers = append([]config.Driver(nil), cfg.Drivers[:len(cfg.Drivers)-1]...) + replacedCfg.Drivers = append(replacedCfg.Drivers, replaced) + replacedCfg.Revision = cfg.Revision + if err := config.SaveStored(st, path, &replacedCfg); err != nil { + t.Fatal(err) + } + got := replacedCfg.Drivers[len(replacedCfg.Drivers)-1] + if got.CredentialOwner == owner { + t.Fatal("reused name kept the previous credential_owner") + } + tel, stop = start(got) + defer stop() + if got := waitDriverMetric(t, tel, "old-name", "started_with_B"); got != 0 { + t.Error("reused name received leftover rotated token B") + } + if got := waitDriverMetric(t, tel, "old-name", "started_with_target"); got != 1 { + t.Error("reused name did not start with the new account token C") + } +} diff --git a/go/cmd/ftw/forecast_site.go b/go/cmd/ftw/forecast_site.go index 47aab852e..3237bd394 100644 --- a/go/cmd/ftw/forecast_site.go +++ b/go/cmd/ftw/forecast_site.go @@ -293,6 +293,12 @@ func forecastScriptDigests(inputs []config.Driver) map[string]string { // Keep this encoding compatible with beta.3 so an upgrade can prove that only // the hash policy changed. Never infer compatibility from a driver name. func forecastRevisionOf(v forecastSite, weather *config.Weather, hashed []config.Driver, scripts map[string]string) (string, error) { + hashed = append([]config.Driver(nil), hashed...) + for i := range hashed { + // credential_owner is bookkeeping for OAuth rotation. It must not + // reset learned PV/load models when Core mints or migrates it. + hashed[i].CredentialOwner = "" + } data, err := json.Marshal(struct { Meter, Timezone string Options telemetry.ForecastOptions diff --git a/go/cmd/ftw/main.go b/go/cmd/ftw/main.go index d440e7f6b..4ae5b19fb 100644 --- a/go/cmd/ftw/main.go +++ b/go/cmd/ftw/main.go @@ -521,6 +521,10 @@ func main() { } else if len(removed) > 0 { slog.Info("deleted the stored settings of removed features", "removed", strings.Join(removed, "; ")) } + if err := config.BindCredentialOwners(st, *configPath, cfg); err != nil { + slog.Error("bind driver credential owners", "err", err) + os.Exit(1) + } if cfg.State != nil && cfg.State.ColdRetentionDays != 0 { slog.Warn("state.cold_retention_days is retired; fixed EMS history retention applies", "previous_days", cfg.State.ColdRetentionDays) diff --git a/go/internal/api/api_myuplink_oauth_test.go b/go/internal/api/api_myuplink_oauth_test.go index df133e361..3f252a872 100644 --- a/go/internal/api/api_myuplink_oauth_test.go +++ b/go/internal/api/api_myuplink_oauth_test.go @@ -56,6 +56,22 @@ func buildMyUplinkOAuthServer(t *testing.T) (*Server, *config.Config, *state.Sto return srv, cfg, st } +func myUplinkRefreshSecret(t *testing.T, cfg *config.Config, st *state.Store) string { + t.Helper() + for _, d := range cfg.Drivers { + if d.Name != "myuplink" { + continue + } + v, ok := st.LoadConfig(config.DriverSecretStateKey(d.SecretOwner(), "refresh_token")) + if !ok { + t.Fatalf("missing refresh secret for owner %q", d.SecretOwner()) + } + return v + } + t.Fatal("myuplink driver missing") + return "" +} + func TestNewPKCEPair(t *testing.T) { v, c, err := newPKCEPair() if err != nil { @@ -255,8 +271,8 @@ func TestMyUplinkOAuthCallbackExchangesAndPersists(t *testing.T) { t.Errorf("config refresh_token = %v, want RT-from-consent", got) } // ...and in the unwatched KV so SecretOverride supersedes any stale value. - if v, ok := st.LoadConfig("driver_secret:myuplink:refresh_token"); !ok || v != "RT-from-consent" { - t.Errorf("KV refresh_token = %q (ok=%v), want RT-from-consent", v, ok) + if v := myUplinkRefreshSecret(t, cfg, st); v != "RT-from-consent" { + t.Errorf("KV refresh_token = %q, want RT-from-consent", v) } } @@ -304,8 +320,8 @@ func TestMyUplinkOAuthManualExchange(t *testing.T) { if got := cfg.Drivers[0].Config["refresh_token"]; got != "RT-manual" { t.Errorf("config refresh_token = %v, want RT-manual", got) } - if v, ok := st.LoadConfig("driver_secret:myuplink:refresh_token"); !ok || v != "RT-manual" { - t.Errorf("KV refresh_token = %q (ok=%v), want RT-manual", v, ok) + if v := myUplinkRefreshSecret(t, cfg, st); v != "RT-manual" { + t.Errorf("KV refresh_token = %q, want RT-manual", v) } } diff --git a/go/internal/config/config.go b/go/internal/config/config.go index 774ecf9d9..026e130e5 100644 --- a/go/internal/config/config.go +++ b/go/internal/config/config.go @@ -903,7 +903,12 @@ func (f Fuse) EffectiveSafetyMarginA() float64 { // Driver is one driver entry. Each driver is a Lua script loaded by // the driver host at startup (or on hot-reload via the file watcher). type Driver struct { - Name string `yaml:"name" json:"name"` + Name string `yaml:"name" json:"name"` + // CredentialOwner is a stable id for rotated OAuth secrets. It is not a + // household setting: Core mints it on first save and the UI posts it + // back unchanged. Secrets are stored as driver_secret:: so + // a rename keeps the rotation and a reused name cannot inherit it. + CredentialOwner string `yaml:"credential_owner,omitempty" json:"credential_owner,omitempty"` Lua string `yaml:"lua,omitempty" json:"lua,omitempty"` // path to .lua file IsSiteMeter bool `yaml:"is_site_meter,omitempty" json:"is_site_meter,omitempty"` BatteryCapacityWh float64 `yaml:"battery_capacity_wh,omitempty" json:"battery_capacity_wh,omitempty"` @@ -1939,6 +1944,7 @@ func (c *Config) Validate() error { // exists. siteMeters := 0 names := make(map[string]bool, len(c.Drivers)) + owners := make(map[string]string, len(c.Drivers)) for _, d := range c.Drivers { if d.Name == "" { return errors.New("driver: name is required") @@ -1947,6 +1953,15 @@ func (c *Config) Validate() error { return fmt.Errorf("driver %q: duplicate name", d.Name) } names[d.Name] = true + if owner := strings.TrimSpace(d.CredentialOwner); owner != "" { + if strings.Contains(owner, ":") { + return fmt.Errorf("driver %q: credential_owner must not contain ':'", d.Name) + } + if other, ok := owners[owner]; ok { + return fmt.Errorf("drivers %q and %q share credential_owner", other, d.Name) + } + owners[owner] = d.Name + } if d.IsSiteMeter { siteMeters++ diff --git a/go/internal/config/credentials.go b/go/internal/config/credentials.go new file mode 100644 index 000000000..efe534c65 --- /dev/null +++ b/go/internal/config/credentials.go @@ -0,0 +1,154 @@ +package config + +import ( + "fmt" + "strings" + + "github.com/google/uuid" + "github.com/srcfl/ftw/go/internal/state" +) + +const driverSecretPrefix = "driver_secret:" + +// DriverSecretStateKey is the unwatched KV row for a rotated driver secret. +func DriverSecretStateKey(owner, key string) string { + return driverSecretPrefix + strings.TrimSpace(owner) + ":" + strings.TrimSpace(key) +} + +// SecretOwner is the durable credential id for this driver. An assigned +// credential_owner wins; the display name is only the pre-migration fallback. +func (d Driver) SecretOwner() string { + if id := strings.TrimSpace(d.CredentialOwner); id != "" { + return id + } + return strings.TrimSpace(d.Name) +} + +func assignCredentialOwners(cfg *Config) error { + if cfg == nil { + return nil + } + seen := make(map[string]string, len(cfg.Drivers)) + for i := range cfg.Drivers { + d := &cfg.Drivers[i] + if strings.TrimSpace(d.CredentialOwner) == "" { + d.CredentialOwner = uuid.NewString() + } + owner := strings.TrimSpace(d.CredentialOwner) + if owner == "" || strings.Contains(owner, ":") { + return fmt.Errorf("driver %q: invalid credential_owner", d.Name) + } + if other, ok := seen[owner]; ok { + return fmt.Errorf("drivers %q and %q share credential_owner %s", other, d.Name, owner) + } + seen[owner] = d.Name + } + return nil +} + +func previousDriverForSecrets(previous *Config, d Driver) *Driver { + if previous == nil { + return nil + } + owner := strings.TrimSpace(d.CredentialOwner) + if owner != "" { + for i := range previous.Drivers { + if strings.TrimSpace(previous.Drivers[i].CredentialOwner) == owner { + return &previous.Drivers[i] + } + } + } + for i := range previous.Drivers { + if previous.Drivers[i].Name == d.Name { + return &previous.Drivers[i] + } + } + return nil +} + +func continuesNamedDriver(previous *Driver, d Driver) bool { + if previous == nil || previous.Name != d.Name { + return false + } + prevOwner := strings.TrimSpace(previous.CredentialOwner) + return prevOwner == "" || prevOwner == strings.TrimSpace(d.CredentialOwner) +} + +func collectDriverSecretCredentials(cfg *Config, previous *Config, stored map[string]string) (map[string]string, error) { + credentials := map[string]string{} + if cfg == nil { + return credentials, nil + } + for _, d := range cfg.Drivers { + prev := previousDriverForSecrets(previous, d) + // First import has no previous document; still take leftover + // name-keyed rows for this display name. A later save only + // migrates when this entry continues that same named driver. + if previous == nil || continuesNamedDriver(prev, d) { + prefix := driverSecretPrefix + d.Name + ":" + for key, value := range stored { + secretKey, ok := strings.CutPrefix(key, prefix) + if !ok || secretKey == "" { + continue + } + owned := DriverSecretStateKey(d.SecretOwner(), secretKey) + if existing, ok := stored[owned]; ok && existing != value { + return nil, fmt.Errorf("driver %q: ambiguous %s secret ownership", d.Name, secretKey) + } + if owned != key { + credentials[owned] = value + } + } + } + token, ok := d.Config["refresh_token"].(string) + if !ok { + continue + } + oldToken := "" + if prev != nil { + oldToken, _ = prev.Config["refresh_token"].(string) + } + if previous != nil && token != oldToken { + credentials[DriverSecretStateKey(d.SecretOwner(), "refresh_token")] = token + } + } + return credentials, nil +} + +func driversNeedCredentialOwnerBind(cfg *Config, stored map[string]string) bool { + if cfg == nil { + return false + } + for _, d := range cfg.Drivers { + if strings.TrimSpace(d.CredentialOwner) == "" { + return true + } + prefix := driverSecretPrefix + d.Name + ":" + ownerPrefix := driverSecretPrefix + d.SecretOwner() + ":" + if ownerPrefix == prefix { + continue + } + for key := range stored { + if strings.HasPrefix(key, prefix) { + return true + } + } + } + return false +} + +// BindCredentialOwners assigns durable owners and migrates name-keyed secret +// rows into the settings document in one SaveStored transaction. +func BindCredentialOwners(st *state.Store, path string, cfg *Config) error { + if st == nil || cfg == nil { + return nil + } + stored, err := st.LoadConfigByPrefix(driverSecretPrefix) + if err != nil { + return err + } + if !driversNeedCredentialOwnerBind(cfg, stored) { + return nil + } + return SaveStored(st, path, cfg) +} diff --git a/go/internal/config/credentials_test.go b/go/internal/config/credentials_test.go new file mode 100644 index 000000000..4134b0300 --- /dev/null +++ b/go/internal/config/credentials_test.go @@ -0,0 +1,232 @@ +package config + +import ( + "os" + "path/filepath" + "testing" + + "github.com/srcfl/ftw/go/internal/state" +) + +func oauthSettings(t *testing.T, dir, database string, d Driver) *Config { + t.Helper() + cfg, err := Parse([]byte(minimalYAML), dir) + if err != nil { + t.Fatal(err) + } + cfg.Drivers = append(cfg.Drivers, d) + cfg.ConfigDatabase = database + return cfg +} + +func testOAuthDriver(name, owner, token, lua string) Driver { + return Driver{ + Name: name, + CredentialOwner: owner, + Lua: lua, + Capabilities: Capabilities{Standalone: true}, + Config: map[string]any{"refresh_token": token}, + } +} + +func oauthDriver(t *testing.T, cfg *Config, name string) *Driver { + t.Helper() + for i := range cfg.Drivers { + if cfg.Drivers[i].Name == name { + return &cfg.Drivers[i] + } + } + t.Fatalf("driver %q missing", name) + return nil +} + +func TestSaveStoredMigratesNameKeyedSecretAndSurvivesRename(t *testing.T) { + dir := t.TempDir() + path, database := filepath.Join(dir, "config.yaml"), filepath.Join(dir, "state.db") + lua := filepath.Join(dir, "oauth.lua") + if err := os.WriteFile(lua, []byte("function driver_init() end"), 0600); err != nil { + t.Fatal(err) + } + st, err := state.Open(database) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = st.Close() }) + if err := st.SaveConfig(DriverSecretStateKey("old-name", "refresh_token"), "rotated-B"); err != nil { + t.Fatal(err) + } + + cfg := oauthSettings(t, dir, database, testOAuthDriver("old-name", "", "config-A", lua)) + if err := SaveStored(st, path, cfg); err != nil { + t.Fatal(err) + } + owner := oauthDriver(t, cfg, "old-name").CredentialOwner + if owner == "" || owner == "old-name" { + t.Fatalf("credential_owner = %q, want a minted id", owner) + } + if got, ok := st.LoadConfig(DriverSecretStateKey(owner, "refresh_token")); !ok || got != "rotated-B" { + t.Fatalf("migrated secret = %q ok=%v, want rotated-B", got, ok) + } + + oauthDriver(t, cfg, "old-name").Name = "renamed" + if err := SaveStored(st, path, cfg); err != nil { + t.Fatal(err) + } + if oauthDriver(t, cfg, "renamed").CredentialOwner != owner { + t.Fatalf("rename changed credential_owner to %q", oauthDriver(t, cfg, "renamed").CredentialOwner) + } + if got, ok := st.LoadConfig(DriverSecretStateKey(owner, "refresh_token")); !ok || got != "rotated-B" { + t.Fatalf("renamed secret = %q ok=%v, want rotated-B", got, ok) + } +} + +func TestSaveStoredDoesNotGiveReusedNameTheOldSecret(t *testing.T) { + dir := t.TempDir() + path, database := filepath.Join(dir, "config.yaml"), filepath.Join(dir, "state.db") + lua := filepath.Join(dir, "oauth.lua") + if err := os.WriteFile(lua, []byte("function driver_init() end"), 0600); err != nil { + t.Fatal(err) + } + st, err := state.Open(database) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = st.Close() }) + if err := st.SaveConfig(DriverSecretStateKey("old-name", "refresh_token"), "rotated-B"); err != nil { + t.Fatal(err) + } + + first := oauthSettings(t, dir, database, testOAuthDriver("old-name", "owner-old", "config-A", lua)) + if err := SaveStored(st, path, first); err != nil { + t.Fatal(err) + } + + replaced := oauthSettings(t, dir, database, testOAuthDriver("old-name", "", "config-C", lua)) + replaced.Revision = first.Revision + if err := SaveStored(st, path, replaced); err != nil { + t.Fatal(err) + } + newOwner := oauthDriver(t, replaced, "old-name").CredentialOwner + if newOwner == "" || newOwner == "owner-old" { + t.Fatalf("reused name kept old owner %q", newOwner) + } + if got, ok := st.LoadConfig(DriverSecretStateKey(newOwner, "refresh_token")); !ok || got != "config-C" { + t.Fatalf("reused-name secret = %q ok=%v, want config-C", got, ok) + } + if got, ok := st.LoadConfig(DriverSecretStateKey("owner-old", "refresh_token")); ok && got == "config-C" { + t.Fatalf("reused name overwrote the previous owner's secret: %q", got) + } +} + +func TestSaveStoredWritesExplicitReauthorizationUnderOwner(t *testing.T) { + dir := t.TempDir() + path, database := filepath.Join(dir, "config.yaml"), filepath.Join(dir, "state.db") + lua := filepath.Join(dir, "oauth.lua") + if err := os.WriteFile(lua, []byte("function driver_init() end"), 0600); err != nil { + t.Fatal(err) + } + st, err := state.Open(database) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = st.Close() }) + + cfg := oauthSettings(t, dir, database, testOAuthDriver("myuplink", "owner-1", "token-A", lua)) + if err := SaveStored(st, path, cfg); err != nil { + t.Fatal(err) + } + oauthDriver(t, cfg, "myuplink").Config["refresh_token"] = "token-D" + if err := SaveStored(st, path, cfg); err != nil { + t.Fatal(err) + } + if got, ok := st.LoadConfig(DriverSecretStateKey("owner-1", "refresh_token")); !ok || got != "token-D" { + t.Fatalf("reauth secret = %q ok=%v, want token-D", got, ok) + } +} + +func TestSaveStoredRejectsAmbiguousSecretOwnership(t *testing.T) { + dir := t.TempDir() + path, database := filepath.Join(dir, "config.yaml"), filepath.Join(dir, "state.db") + lua := filepath.Join(dir, "oauth.lua") + if err := os.WriteFile(lua, []byte("function driver_init() end"), 0600); err != nil { + t.Fatal(err) + } + st, err := state.Open(database) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = st.Close() }) + if err := st.SaveConfig(DriverSecretStateKey("myuplink", "refresh_token"), "name-keyed"); err != nil { + t.Fatal(err) + } + if err := st.SaveConfig(DriverSecretStateKey("owner-1", "refresh_token"), "owner-keyed"); err != nil { + t.Fatal(err) + } + + cfg := oauthSettings(t, dir, database, testOAuthDriver("myuplink", "owner-1", "config-A", lua)) + if err := SaveStored(st, path, cfg); err == nil { + t.Fatal("ambiguous name-keyed and owner-keyed secrets were accepted") + } +} + +func TestBindCredentialOwnersMigratesExistingNameKeyedSecret(t *testing.T) { + dir := t.TempDir() + path, database := filepath.Join(dir, "config.yaml"), filepath.Join(dir, "state.db") + lua := filepath.Join(dir, "oauth.lua") + if err := os.WriteFile(lua, []byte("function driver_init() end"), 0600); err != nil { + t.Fatal(err) + } + st, err := state.Open(database) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = st.Close() }) + + cfg := oauthSettings(t, dir, database, testOAuthDriver("old-name", "", "config-A", lua)) + if err := SaveStored(st, path, cfg); err != nil { + t.Fatal(err) + } + if err := st.SaveConfig(DriverSecretStateKey("old-name", "refresh_token"), "rotated-B"); err != nil { + t.Fatal(err) + } + if err := BindCredentialOwners(st, path, cfg); err != nil { + t.Fatal(err) + } + owner := oauthDriver(t, cfg, "old-name").CredentialOwner + if owner == "" || owner == "old-name" { + t.Fatalf("bind left credential_owner = %q", owner) + } + if got, ok := st.LoadConfig(DriverSecretStateKey(owner, "refresh_token")); !ok || got != "rotated-B" { + t.Fatalf("bound secret = %q ok=%v, want rotated-B", got, ok) + } + if err := BindCredentialOwners(st, path, cfg); err != nil { + t.Fatal(err) + } + if oauthDriver(t, cfg, "old-name").CredentialOwner != owner { + t.Fatalf("second bind reminted owner %q", oauthDriver(t, cfg, "old-name").CredentialOwner) + } +} + +func TestValidateRejectsInvalidCredentialOwner(t *testing.T) { + dir := t.TempDir() + lua := filepath.Join(dir, "oauth.lua") + if err := os.WriteFile(lua, []byte("function driver_init() end"), 0600); err != nil { + t.Fatal(err) + } + cfg, err := Parse([]byte(minimalYAML), dir) + if err != nil { + t.Fatal(err) + } + bad := testOAuthDriver("a", "owner:bad", "t", lua) + bad.IsSiteMeter = true + cfg.Drivers = []Driver{bad} + if err := cfg.Validate(); err == nil { + t.Fatal("credential_owner containing ':' was accepted") + } + first, second := testOAuthDriver("a", "same", "t", lua), testOAuthDriver("b", "same", "t", lua) + first.IsSiteMeter = true + cfg.Drivers = []Driver{first, second} + if err := cfg.Validate(); err == nil { + t.Fatal("duplicate credential_owner was accepted") + } +} diff --git a/go/internal/config/storage.go b/go/internal/config/storage.go index 75bd44b7d..8b40a8da1 100644 --- a/go/internal/config/storage.go +++ b/go/internal/config/storage.go @@ -276,6 +276,9 @@ func saveStored(st *state.Store, path string, cfg *Config, sourceHash string) er return errors.New("move the state database offline; its path cannot change in Settings") } } + if err := assignCredentialOwners(cfg); err != nil { + return err + } if err := cfg.Validate(); err != nil { return err } @@ -290,28 +293,16 @@ func saveStored(st *state.Store, path string, cfg *Config, sourceHash string) er if cfg.EVCharger != nil { password = cfg.EVCharger.Password } - credentials := map[string]string{ - "ev_charger_password": password, - "lan_auth_password": cfg.LANPasswordHash, + storedSecrets, err := st.LoadConfigByPrefix(driverSecretPrefix) + if err != nil { + return err } - for _, d := range cfg.Drivers { - token, ok := d.Config["refresh_token"].(string) - if !ok { - continue - } - oldToken := "" - if previous != nil { - for _, old := range previous.Drivers { - if old.Name == d.Name { - oldToken, _ = old.Config["refresh_token"].(string) - break - } - } - } - if previous != nil && token != oldToken { - credentials["driver_secret:"+d.Name+":refresh_token"] = token - } + credentials, err := collectDriverSecretCredentials(cfg, previous, storedSecrets) + if err != nil { + return err } + credentials["ev_charger_password"] = password + credentials["lan_auth_password"] = cfg.LANPasswordHash revision, err := st.SaveConfiguration(raw, cfg.Revision, credentials) if err != nil { return err diff --git a/go/internal/config/storage_test.go b/go/internal/config/storage_test.go index 5eb483d7b..5339d591e 100644 --- a/go/internal/config/storage_test.go +++ b/go/internal/config/storage_test.go @@ -135,7 +135,15 @@ ev_charger: if err != nil { t.Fatal(err) } - after, _ := json.Marshal(reloaded) + stripped := *reloaded + stripped.Drivers = append([]Driver(nil), reloaded.Drivers...) + for i := range stripped.Drivers { + if stripped.Drivers[i].CredentialOwner == "" { + t.Fatalf("import left driver %q without credential_owner", stripped.Drivers[i].Name) + } + stripped.Drivers[i].CredentialOwner = "" + } + after, _ := json.Marshal(&stripped) if string(before) != string(after) { t.Fatalf("config meaning changed across SQLite import\nbefore=%s\nafter=%s", before, after) } diff --git a/go/internal/drivers/registry.go b/go/internal/drivers/registry.go index ca1f5a9e7..99118a8d0 100644 --- a/go/internal/drivers/registry.go +++ b/go/internal/drivers/registry.go @@ -93,15 +93,15 @@ type Registry struct { // Optional — when nil, devices fall back to endpoint-hash IDs. ARPLookup func(host string) (mac string, ok bool) // SecretPersister, when set, durably stores a driver secret (keyed by - // driver name + key) in the unwatched state KV. Wired by main.go. + // credential owner + key) in the unwatched state KV. Wired by main.go. // Optional — when nil, host.persist_secret returns an error and the // driver degrades (an OAuth driver re-uses its last in-memory token). - SecretPersister func(driverName, key, value string) error + SecretPersister func(owner, key, value string) error // SecretOverride, when set, returns a durably-persisted secret for a // driver (the counterpart to SecretPersister). Applied over the // config.yaml value at driver_init so a rotated token survives a // restart. Returns ("", false) when no override exists. - SecretOverride func(driverName, key string) (string, bool) + SecretOverride func(owner, key string) (string, bool) // RuntimePolicyResolver returns the verified signed policy of a managed // read-only artifact. Nil means bundled, local and control-capable // signed drivers, which run without one. @@ -540,12 +540,12 @@ func (r *Registry) add(ctx context.Context, cfg config.Driver, startupDefault bo // Wire secret write-back (rotated OAuth tokens). The host must install // SecretPersister and SecretOverride before Add: init may persist a // secret, and the poll loop starts before Add returns. - driverName := cfg.Name + secretOwner := cfg.SecretOwner() env.PersistSecret = func(key, value string) error { if r.SecretPersister == nil { return fmt.Errorf("persist_secret: not supported on this host") } - return r.SecretPersister(driverName, key, value) + return r.SecretPersister(secretOwner, key, value) } if mq := cfg.EffectiveMQTT(); mq != nil && r.MQTTFactory != nil { dialCfg := *mq @@ -668,7 +668,7 @@ func (r *Registry) add(ctx context.Context, cfg config.Driver, startupDefault bo if r.SecretOverride != nil && len(cfg.Config) > 0 { merged := make(map[string]any, len(cfg.Config)) for k, v := range cfg.Config { - if ov, ok := r.SecretOverride(cfg.Name, k); ok { + if ov, ok := r.SecretOverride(secretOwner, k); ok { merged[k] = ov } else { merged[k] = v diff --git a/go/internal/state/config_prefix_test.go b/go/internal/state/config_prefix_test.go new file mode 100644 index 000000000..f38fa1ccd --- /dev/null +++ b/go/internal/state/config_prefix_test.go @@ -0,0 +1,26 @@ +package state + +import "testing" + +func TestLoadConfigByPrefix(t *testing.T) { + s := freshStore(t) + if err := s.SaveConfig("driver_secret:old-name:refresh_token", "B"); err != nil { + t.Fatal(err) + } + if err := s.SaveConfig("driver_secret:old-name:access_token", "A"); err != nil { + t.Fatal(err) + } + if err := s.SaveConfig("other", "x"); err != nil { + t.Fatal(err) + } + got, err := s.LoadConfigByPrefix("driver_secret:old-name:") + if err != nil { + t.Fatal(err) + } + if len(got) != 2 || got["driver_secret:old-name:refresh_token"] != "B" || got["driver_secret:old-name:access_token"] != "A" { + t.Fatalf("prefix rows = %#v", got) + } + if _, ok := got["other"]; ok { + t.Fatal("unrelated key included") + } +} diff --git a/go/internal/state/configuration.go b/go/internal/state/configuration.go index 2e1b68889..57559aceb 100644 --- a/go/internal/state/configuration.go +++ b/go/internal/state/configuration.go @@ -75,6 +75,32 @@ func (s *Store) Configuration() (Configuration, bool, error) { return c, true, err } +func escapeConfigPrefix(prefix string) string { + replacer := strings.NewReplacer(`\`, `\\`, `%`, `\%`, `_`, `\_`) + return replacer.Replace(prefix) +} + +// LoadConfigByPrefix returns unwatched config rows whose keys start with prefix. +func (s *Store) LoadConfigByPrefix(prefix string) (map[string]string, error) { + if s == nil || s.db == nil || prefix == "" { + return map[string]string{}, nil + } + rows, err := s.db.Query(`SELECT key, value FROM config WHERE key LIKE ? ESCAPE '\'`, escapeConfigPrefix(prefix)+"%") + if err != nil { + return nil, err + } + defer rows.Close() + out := map[string]string{} + for rows.Next() { + var key, value string + if err := rows.Scan(&key, &value); err != nil { + return nil, err + } + out[key] = value + } + return out, rows.Err() +} + // ConfigValue distinguishes a missing legacy key from a failed read. func (s *Store) ConfigValue(key string) (string, bool, error) { var raw string From 54b3bc7674eea4fa75ad5241e2ec81d18a224b5c Mon Sep 17 00:00:00 2001 From: Fredrik Ahlgren Date: Tue, 6 Oct 2026 09:42:16 +0200 Subject: [PATCH 2/3] fix(config): preserve OAuth rotations through restart and rollback Signed-off-by: Fredrik Ahlgren --- go/cmd/ftw/driver_registry.go | 2 +- go/cmd/ftw/driver_registry_test.go | 6 +- go/internal/config/credentials.go | 58 +++++++++---- go/internal/config/credentials_test.go | 106 +++++++++++++++++++++++- go/internal/config/storage.go | 4 +- go/internal/state/configuration.go | 5 +- go/internal/state/driver_secrets.go | 109 +++++++++++++++++++++++++ 7 files changed, 267 insertions(+), 23 deletions(-) create mode 100644 go/internal/state/driver_secrets.go diff --git a/go/cmd/ftw/driver_registry.go b/go/cmd/ftw/driver_registry.go index 4a8df4fb3..f96194d5e 100644 --- a/go/cmd/ftw/driver_registry.go +++ b/go/cmd/ftw/driver_registry.go @@ -16,7 +16,7 @@ func newDriverRegistry(tel *telemetry.Store, st *state.Store) *drivers.Registry return config.DriverSecretStateKey(owner, key) } reg.SecretPersister = func(owner, key, value string) error { - return st.SaveConfig(driverSecretKey(owner, key), value) + return st.SaveDriverSecret(owner, key, value) } reg.SecretOverride = func(owner, key string) (string, bool) { return st.LoadConfig(driverSecretKey(owner, key)) diff --git a/go/cmd/ftw/driver_registry_test.go b/go/cmd/ftw/driver_registry_test.go index 477c863be..08dda6988 100644 --- a/go/cmd/ftw/driver_registry_test.go +++ b/go/cmd/ftw/driver_registry_test.go @@ -96,9 +96,9 @@ function driver_default_mode() end if got := metric(tel, "persist_ok"); got != 1 { t.Errorf("secret persistence during %s failed", scenario.phase) } - if got, ok := st.LoadConfig(config.DriverSecretStateKey(cfg.SecretOwner(), "refresh_token")); !ok || got != "synthetic-B" { - t.Errorf("rotated token B was not stored") - } + if got, ok := st.LoadConfig(config.DriverSecretStateKey(cfg.SecretOwner(), "refresh_token")); !ok || got != "synthetic-B" { + t.Errorf("rotated token B was not stored") + } }() _, tel, stop = start() defer stop() diff --git a/go/internal/config/credentials.go b/go/internal/config/credentials.go index efe534c65..945d47856 100644 --- a/go/internal/config/credentials.go +++ b/go/internal/config/credentials.go @@ -12,7 +12,7 @@ const driverSecretPrefix = "driver_secret:" // DriverSecretStateKey is the unwatched KV row for a rotated driver secret. func DriverSecretStateKey(owner, key string) string { - return driverSecretPrefix + strings.TrimSpace(owner) + ":" + strings.TrimSpace(key) + return state.DriverSecretKey(strings.TrimSpace(owner), strings.TrimSpace(key)) } // SecretOwner is the durable credential id for this driver. An assigned @@ -24,13 +24,22 @@ func (d Driver) SecretOwner() string { return strings.TrimSpace(d.Name) } -func assignCredentialOwners(cfg *Config) error { +func assignCredentialOwners(cfg, previous *Config) error { if cfg == nil { return nil } seen := make(map[string]string, len(cfg.Drivers)) for i := range cfg.Drivers { d := &cfg.Drivers[i] + if old := previousDriverForSecrets(previous, *d); old != nil && old.CredentialOwner != "" && old.SecretOwner() == d.SecretOwner() { + oldToken, _ := old.Config["refresh_token"].(string) + newToken, _ := d.Config["refresh_token"].(string) + if newToken != oldToken { + // Consent starts a new token family. A late callback from the + // old driver must not overwrite the replacement account. + d.CredentialOwner = "" + } + } if strings.TrimSpace(d.CredentialOwner) == "" { d.CredentialOwner = uuid.NewString() } @@ -41,6 +50,7 @@ func assignCredentialOwners(cfg *Config) error { if other, ok := seen[owner]; ok { return fmt.Errorf("drivers %q and %q share credential_owner %s", other, d.Name, owner) } + d.CredentialOwner = owner seen[owner] = d.Name } return nil @@ -81,6 +91,12 @@ func collectDriverSecretCredentials(cfg *Config, previous *Config, stored map[st } for _, d := range cfg.Drivers { prev := previousDriverForSecrets(previous, d) + token, hasToken := d.Config["refresh_token"].(string) + oldToken := "" + if prev != nil { + oldToken, _ = prev.Config["refresh_token"].(string) + } + newToken := hasToken && previous != nil && (token != oldToken || prev == nil || prev.SecretOwner() != d.SecretOwner()) // First import has no previous document; still take leftover // name-keyed rows for this display name. A later save only // migrates when this entry continues that same named driver. @@ -92,7 +108,27 @@ func collectDriverSecretCredentials(cfg *Config, previous *Config, stored map[st continue } owned := DriverSecretStateKey(d.SecretOwner(), secretKey) - if existing, ok := stored[owned]; ok && existing != value { + if existing, ok := stored[owned]; ok { + if existing == value { + continue + } + if secretKey == "refresh_token" && newToken { + continue + } + lastMirror, mirrored := stored[state.DriverSecretLegacyHashKey(d.SecretOwner(), secretKey)] + if mirrored && lastMirror == state.DriverSecretValueHash(existing) { + // Older Core rotated the name-keyed copy after rollback. + credentials[owned] = value + continue + } + if mirrored && lastMirror != state.DriverSecretValueHash(value) { + return nil, fmt.Errorf("driver %q: ambiguous %s secret ownership", d.Name, secretKey) + } + if prev != nil && strings.TrimSpace(prev.CredentialOwner) == d.SecretOwner() { + // The owner is already bound. An old leftover must not + // overwrite a subsequent owner-keyed rotation. + continue + } return nil, fmt.Errorf("driver %q: ambiguous %s secret ownership", d.Name, secretKey) } if owned != key { @@ -100,15 +136,7 @@ func collectDriverSecretCredentials(cfg *Config, previous *Config, stored map[st } } } - token, ok := d.Config["refresh_token"].(string) - if !ok { - continue - } - oldToken := "" - if prev != nil { - oldToken, _ = prev.Config["refresh_token"].(string) - } - if previous != nil && token != oldToken { + if newToken { credentials[DriverSecretStateKey(d.SecretOwner(), "refresh_token")] = token } } @@ -128,8 +156,8 @@ func driversNeedCredentialOwnerBind(cfg *Config, stored map[string]string) bool if ownerPrefix == prefix { continue } - for key := range stored { - if strings.HasPrefix(key, prefix) { + for key, value := range stored { + if suffix, ok := strings.CutPrefix(key, prefix); ok && stored[ownerPrefix+suffix] != value { return true } } @@ -143,7 +171,7 @@ func BindCredentialOwners(st *state.Store, path string, cfg *Config) error { if st == nil || cfg == nil { return nil } - stored, err := st.LoadConfigByPrefix(driverSecretPrefix) + stored, err := st.LoadConfigByPrefix("driver_secret") if err != nil { return err } diff --git a/go/internal/config/credentials_test.go b/go/internal/config/credentials_test.go index 4134b0300..c7f999ca7 100644 --- a/go/internal/config/credentials_test.go +++ b/go/internal/config/credentials_test.go @@ -139,9 +139,113 @@ func TestSaveStoredWritesExplicitReauthorizationUnderOwner(t *testing.T) { if err := SaveStored(st, path, cfg); err != nil { t.Fatal(err) } - if got, ok := st.LoadConfig(DriverSecretStateKey("owner-1", "refresh_token")); !ok || got != "token-D" { + owner := oauthDriver(t, cfg, "myuplink").SecretOwner() + if owner == "owner-1" { + t.Fatal("reauthorization retained the old token family's owner") + } + if got, ok := st.LoadConfig(DriverSecretStateKey(owner, "refresh_token")); !ok || got != "token-D" { t.Fatalf("reauth secret = %q ok=%v, want token-D", got, ok) } + if err := st.SaveDriverSecret("owner-1", "refresh_token", "late-old-token"); err != nil { + t.Fatal(err) + } + if got, _ := st.LoadConfig(DriverSecretStateKey("myuplink", "refresh_token")); got != "token-D" { + t.Fatalf("old token family overwrote reauthorization: %q", got) + } +} + +func TestCredentialOwnerRotationSurvivesSettingsSaveAndRestart(t *testing.T) { + dir := t.TempDir() + path, database := filepath.Join(dir, "config.yaml"), filepath.Join(dir, "state.db") + lua := filepath.Join(dir, "oauth.lua") + if err := os.WriteFile(lua, []byte("function driver_init() end"), 0600); err != nil { + t.Fatal(err) + } + st, err := state.Open(database) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = st.Close() }) + if err := st.SaveConfig(DriverSecretStateKey("myuplink", "refresh_token"), "rotated-B"); err != nil { + t.Fatal(err) + } + cfg := oauthSettings(t, dir, database, testOAuthDriver("myuplink", "", "config-A", lua)) + if err := SaveStored(st, path, cfg); err != nil { + t.Fatal(err) + } + owner := oauthDriver(t, cfg, "myuplink").SecretOwner() + if err := st.SaveConfig(DriverSecretStateKey(owner, "refresh_token"), "rotated-C"); err != nil { + t.Fatal(err) + } + if err := SaveStored(st, path, cfg); err != nil { + t.Fatalf("settings save after rotation: %v", err) + } + if err := BindCredentialOwners(st, path, cfg); err != nil { + t.Fatalf("restart after rotation: %v", err) + } + if got, _ := st.LoadConfig(DriverSecretStateKey(owner, "refresh_token")); got != "rotated-C" { + t.Fatalf("rotation changed to %q", got) + } +} + +func TestCredentialOwnerSurvivesLegacyCoreRotationAndRename(t *testing.T) { + dir := t.TempDir() + path, database := filepath.Join(dir, "config.yaml"), filepath.Join(dir, "state.db") + lua := filepath.Join(dir, "oauth.lua") + if err := os.WriteFile(lua, []byte("function driver_init() end"), 0600); err != nil { + t.Fatal(err) + } + st, err := state.Open(database) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = st.Close() }) + if err := st.SaveConfig(DriverSecretStateKey("myuplink", "refresh_token"), "rotated-B"); err != nil { + t.Fatal(err) + } + cfg := oauthSettings(t, dir, database, testOAuthDriver("myuplink", "", "config-A", lua)) + if err := SaveStored(st, path, cfg); err != nil { + t.Fatal(err) + } + owner := oauthDriver(t, cfg, "myuplink").SecretOwner() + if err := st.SaveDriverSecret(owner, "refresh_token", "rotated-C"); err != nil { + t.Fatal(err) + } + if got, _ := st.LoadConfig(DriverSecretStateKey("myuplink", "refresh_token")); got != "rotated-C" { + t.Fatalf("older Core would reload %q after rollback", got) + } + // Older Core only knows the display-name key and can rotate it again. + if err := st.SaveConfig(DriverSecretStateKey("myuplink", "refresh_token"), "rotated-D"); err != nil { + t.Fatal(err) + } + if err := BindCredentialOwners(st, path, cfg); err != nil { + t.Fatalf("return from older Core: %v", err) + } + if got, _ := st.LoadConfig(DriverSecretStateKey(owner, "refresh_token")); got != "rotated-D" { + t.Fatalf("return from older Core lost rotation: %q", got) + } + oauthDriver(t, cfg, "myuplink").Name = "renamed" + if err := SaveStored(st, path, cfg); err != nil { + t.Fatal(err) + } + if got, _ := st.LoadConfig(DriverSecretStateKey("renamed", "refresh_token")); got != "rotated-D" { + t.Fatalf("renamed legacy lookup got %q", got) + } + // Reuse the old name for another account. A late callback from the old + // owner must update the renamed alias, never the new account's alias. + cfg.Drivers = append(cfg.Drivers, testOAuthDriver("myuplink", "", "new-account", lua)) + if err := SaveStored(st, path, cfg); err != nil { + t.Fatal(err) + } + if err := st.SaveDriverSecret(owner, "refresh_token", "rotated-E"); err != nil { + t.Fatal(err) + } + if got, _ := st.LoadConfig(DriverSecretStateKey("myuplink", "refresh_token")); got != "new-account" { + t.Fatalf("reused name received the other account's token: %q", got) + } + if got, _ := st.LoadConfig(DriverSecretStateKey("renamed", "refresh_token")); got != "rotated-E" { + t.Fatalf("renamed driver lost its new rotation: %q", got) + } } func TestSaveStoredRejectsAmbiguousSecretOwnership(t *testing.T) { diff --git a/go/internal/config/storage.go b/go/internal/config/storage.go index 8b40a8da1..9351242bd 100644 --- a/go/internal/config/storage.go +++ b/go/internal/config/storage.go @@ -276,7 +276,7 @@ func saveStored(st *state.Store, path string, cfg *Config, sourceHash string) er return errors.New("move the state database offline; its path cannot change in Settings") } } - if err := assignCredentialOwners(cfg); err != nil { + if err := assignCredentialOwners(cfg, previous); err != nil { return err } if err := cfg.Validate(); err != nil { @@ -293,7 +293,7 @@ func saveStored(st *state.Store, path string, cfg *Config, sourceHash string) er if cfg.EVCharger != nil { password = cfg.EVCharger.Password } - storedSecrets, err := st.LoadConfigByPrefix(driverSecretPrefix) + storedSecrets, err := st.LoadConfigByPrefix("driver_secret") if err != nil { return err } diff --git a/go/internal/state/configuration.go b/go/internal/state/configuration.go index 57559aceb..050d35827 100644 --- a/go/internal/state/configuration.go +++ b/go/internal/state/configuration.go @@ -216,7 +216,10 @@ func (s *Store) SaveConfiguration(document []byte, expected int64, credentials m values[k] = v } values[configurationKey] = string(encoded) - return saveConfigValues(tx, values) + if err := saveConfigValues(tx, values); err != nil { + return err + } + return mirrorDriverSecrets(tx, document) }) if err != nil { return 0, err diff --git a/go/internal/state/driver_secrets.go b/go/internal/state/driver_secrets.go new file mode 100644 index 000000000..0235784b3 --- /dev/null +++ b/go/internal/state/driver_secrets.go @@ -0,0 +1,109 @@ +package state + +import ( + "crypto/sha256" + "database/sql" + "encoding/hex" + "encoding/json" + "errors" +) + +func DriverSecretKey(owner, key string) string { + return "driver_secret:" + owner + ":" + key +} + +func DriverSecretLegacyHashKey(owner, key string) string { + return "driver_secret_legacy_hash:" + owner + ":" + key +} + +func DriverSecretValueHash(value string) string { + sum := sha256.Sum256([]byte(value)) + return hex.EncodeToString(sum[:]) +} + +func driverSecretAliases(document []byte) (map[string]string, error) { + var saved struct { + Config struct { + Drivers []struct { + Name string `json:"name"` + Owner string `json:"credential_owner"` + } `json:"drivers"` + } `json:"config"` + } + if err := json.Unmarshal(document, &saved); err != nil { + return nil, err + } + aliases := map[string]string{} + for _, d := range saved.Config.Drivers { + if d.Owner != "" && d.Owner != d.Name { + aliases[d.Owner] = d.Name + } + } + return aliases, nil +} + +// SaveDriverSecret keeps the current name-keyed copy usable by an older Core +// after rollback. Read the binding and write both copies in one transaction so +// a concurrent rename cannot give a rotation to a different driver's name. +func (s *Store) SaveDriverSecret(owner, key, value string) error { + return s.durableConfigWrite(func(tx *sql.Tx) error { + values := map[string]string{DriverSecretKey(owner, key): value} + var raw string + err := tx.QueryRow(`SELECT value FROM config WHERE key = ?`, configurationKey).Scan(&raw) + if err != nil && !errors.Is(err, sql.ErrNoRows) { + return err + } + if err == nil { + c, err := decodeConfiguration(raw) + if err != nil { + return err + } + aliases, err := driverSecretAliases(c.Document) + if err != nil { + return err + } + if name, ok := aliases[owner]; ok { + values[DriverSecretKey(name, key)] = value + values[DriverSecretLegacyHashKey(owner, key)] = DriverSecretValueHash(value) + } + } + return saveConfigValues(tx, values) + }) +} + +// Settings saves mirror the latest rotation inside their own transaction, +// after applying explicit credentials. A token rotated during the settings +// read cannot be replaced by an older snapshot. +func mirrorDriverSecrets(tx *sql.Tx, document []byte) error { + aliases, err := driverSecretAliases(document) + if err != nil { + return err + } + for owner, name := range aliases { + prefix := DriverSecretKey(owner, "") + rows, err := tx.Query(`SELECT key, value FROM config WHERE key LIKE ? ESCAPE '\'`, escapeConfigPrefix(prefix)+"%") + if err != nil { + return err + } + values := map[string]string{} + for rows.Next() { + var key, value string + if err := rows.Scan(&key, &value); err != nil { + rows.Close() + return err + } + secretKey := key[len(prefix):] + values[DriverSecretKey(name, secretKey)] = value + values[DriverSecretLegacyHashKey(owner, secretKey)] = DriverSecretValueHash(value) + } + err = rows.Err() + rows.Close() + if err != nil { + return err + } + if err := saveConfigValues(tx, values); err != nil { + return err + } + } + return nil +} From 26dd14fe2a451f1c181227dd50f58fbab92cefbb Mon Sep 17 00:00:00 2001 From: Fredrik Ahlgren Date: Tue, 6 Oct 2026 09:50:43 +0200 Subject: [PATCH 3/3] fix(config): retain the live token during first owner migration Signed-off-by: Fredrik Ahlgren --- go/internal/config/credentials.go | 2 +- go/internal/config/credentials_test.go | 34 ++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/go/internal/config/credentials.go b/go/internal/config/credentials.go index 945d47856..bf70a7db4 100644 --- a/go/internal/config/credentials.go +++ b/go/internal/config/credentials.go @@ -96,7 +96,7 @@ func collectDriverSecretCredentials(cfg *Config, previous *Config, stored map[st if prev != nil { oldToken, _ = prev.Config["refresh_token"].(string) } - newToken := hasToken && previous != nil && (token != oldToken || prev == nil || prev.SecretOwner() != d.SecretOwner()) + newToken := hasToken && previous != nil && (token != oldToken || prev == nil || (prev.CredentialOwner != "" && prev.SecretOwner() != d.SecretOwner())) // First import has no previous document; still take leftover // name-keyed rows for this display name. A later save only // migrates when this entry continues that same named driver. diff --git a/go/internal/config/credentials_test.go b/go/internal/config/credentials_test.go index c7f999ca7..a8a4d4782 100644 --- a/go/internal/config/credentials_test.go +++ b/go/internal/config/credentials_test.go @@ -1,6 +1,7 @@ package config import ( + "encoding/json" "os" "path/filepath" "testing" @@ -311,6 +312,39 @@ func TestBindCredentialOwnersMigratesExistingNameKeyedSecret(t *testing.T) { } } +func TestBindCredentialOwnersPreservesRotationFromPreviousCore(t *testing.T) { + dir := t.TempDir() + path, database := filepath.Join(dir, "config.yaml"), filepath.Join(dir, "state.db") + lua := filepath.Join(dir, "oauth.lua") + if err := os.WriteFile(lua, []byte("function driver_init() end"), 0600); err != nil { + t.Fatal(err) + } + st, err := state.Open(database) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = st.Close() }) + cfg := oauthSettings(t, dir, database, testOAuthDriver("myuplink", "", "original-A", lua)) + raw, err := json.Marshal(storedSettings{Config: cfg}) + if err != nil { + t.Fatal(err) + } + cfg.Revision, err = st.SaveConfiguration(raw, 0, nil) + if err != nil { + t.Fatal(err) + } + if err := st.SaveConfig(DriverSecretStateKey("myuplink", "refresh_token"), "current-B"); err != nil { + t.Fatal(err) + } + if err := BindCredentialOwners(st, path, cfg); err != nil { + t.Fatal(err) + } + owner := oauthDriver(t, cfg, "myuplink").SecretOwner() + if got, _ := st.LoadConfig(DriverSecretStateKey(owner, "refresh_token")); got != "current-B" { + t.Fatalf("upgrade replaced the current rotation with %q", got) + } +} + func TestValidateRejectsInvalidCredentialOwner(t *testing.T) { dir := t.TempDir() lua := filepath.Join(dir, "oauth.lua")