From 8e107cb40268b966c6a796fde830feea3a3ae084 Mon Sep 17 00:00:00 2001 From: Fredrik Ahlgren Date: Sun, 18 Oct 2026 16:55:34 +0200 Subject: [PATCH 1/6] Define agent-led setup and durable typed configuration drafts --- docs/adr/0011-agent-led-setup.md | 168 +++++++++++++++ go/internal/onboarding/draft.go | 305 +++++++++++++++++++++++++++ go/internal/onboarding/draft_test.go | 223 ++++++++++++++++++++ go/internal/onboarding/schema.go | 57 +++++ 4 files changed, 753 insertions(+) create mode 100644 docs/adr/0011-agent-led-setup.md create mode 100644 go/internal/onboarding/draft.go create mode 100644 go/internal/onboarding/draft_test.go create mode 100644 go/internal/onboarding/schema.go diff --git a/docs/adr/0011-agent-led-setup.md b/docs/adr/0011-agent-led-setup.md new file mode 100644 index 000000000..c1970145a --- /dev/null +++ b/docs/adr/0011-agent-led-setup.md @@ -0,0 +1,168 @@ +# ADR 0011: Set up a home through an authorized agent + +- Status: proposed; implementation and field tests remain work to do. +- Date: 2026-10-18 +- Owner: Fredrik. Requested complete setup through MCP on 18 October 2026. + +## Decision + +FTW is an energy management primitive for people, agents and other clients. +An owner should be able to connect a new box to an assistant and complete its +setup in that conversation. The box owns configuration, drafts, validation, +measurement and control. The assistant asks questions and proposes changes. +The same setup service must serve MCP and the box page. + +Normal status, history and control keep their direct tools. Setup and deep +troubleshooting are distinct workflows; neither should add a long interview +to a request to read today's energy use. + +## From a new box to a running home + +1. **Connect and claim.** The user powers the box and connects its network. + The box page proves local access and offers either the current wizard or + setup with an assistant. Assistant setup starts an empty, passive system + without device commands. The user claims the box and approves the agent + through the existing owner pairing path. Cloud remains opt-in. A public + directory entry may start this flow, but cannot claim a box for its user. +2. **Read what is known.** The agent reads setup state, supported configuration + fields, the driver catalogue and current measurements. It offers discovery + through an explicit discovery action; a read must not scan the network. + It separates a device it found from a device the owner chose to add. +3. **Ask the next useful question.** Ask for site name, country, price area, + currency, location and time zone; then main fuse, phase count and voltage. + Ask what the home has: site meter, solar, battery, cars, chargers, heating + and other equipment. Read capacities and limits from equipment where the + driver supports them. Ask only for missing facts and household choices. +4. **Build a durable draft.** Store answers and proposed config on the box. + Return its revision, outstanding questions and validation results after + each edit. A new conversation can resume it. Apply patches to the current + typed config, preserving fields and saved secrets that the caller did not + change. A stale revision must conflict rather than overwrite another edit. +5. **Check and save.** Validate with Core's existing config checks. Show the + proposed changes, assumptions, effects and required restarts. The owner + confirms the proposed configuration in the conversation. Commit the exact + checked revision through the same durable save and reload path as Settings. +6. **Verify the installation.** Start with measurement. Require a fresh site + meter, confirmed fuse and phase mapping before active dispatch. A driver + that reports data has not thereby proved control. Offer bounded control + tests only with the user's agreement; report command, reply and measured + effect separately. A failed test leaves that device out of active control. +7. **Choose everyday goals.** Explain useful choices for this equipment, then + save battery policy, reserves, export limits, charge goals and schedules. + Core keeps every existing safety check. Finish with what works, what is + still estimated and what needs attention. Losing the assistant never + removes saved goals or stops local operation. + +The workflow covers the full supported config, including driver connection +fields, price and grid tariffs, weather, battery windows, vehicles and +loadpoints, OCPP, notifications, Home Assistant and other integrations. A +generated schema and live driver metadata describe what this build accepts; +the Cloud service must not keep a second list of config fields. Credentials +that need a vendor sign-in use that vendor's sign-in flow. Do not ask users to +paste tokens when a secure sign-in flow exists. + +## Contract and authority + +Provide setup tools for status/schema, draft edits, validation and commit. +Read tools explain missing setup without changing anything. Mutation needs +an owner-approved setup grant, distinct from diagnostic read access. Keep the +agent's role and identity; never turn an agent into an owner internally. +Check expiry and revocation on every operation, including just before commit. +An agent cannot grant itself access or accept Cloud terms for its owner. + +The box binds a checked draft to its base config revision and content digest. +Commit must name both. It cannot substitute a different patch after the user +has reviewed the result. Bound messages and drafts, reject unknown fields, +preserve exact numbers, and audit the caller and changed field paths without +logging credentials. A reply must distinguish saved, applied, restart needed +and verified on hardware. + +Current first-run bootstrap serves only the web wizard. It must gain a +passive owner-claim path before claiming that a fresh installation can use +Cloud MCP. Existing agents can read and propose goals; that is not authority +to replace configuration. Both gaps need code and tests. + +## Useful forecasts before learning + +Ask optional questions that can improve a forecast: known annual household +electricity use, whether that figure includes EV charging or electric heat, +heating type, usual occupancy, and known solar nameplate capacity. A user who +does not know these values must still be able to finish setup. Roof geometry +is optional expert input, never a prerequisite. + +Use existing weather, solar scale and heating inputs where they fit. A load +prior derived from annual energy must exclude separately forecast EV use and +must not count heating twice. Store the answers, unit, source, time and +assumptions beside the resulting prior. Return uncertainty and label it as an +estimate. Do not add synthetic samples to measured history or increase a +model's sample count. Measured days progressively replace the prior through +the model's existing learning weights. Changing a prior must preserve learned +history and must not change an equipment safety limit. + +Missing solar capacity stays unknown until the owner supplies an estimate or +measurement supports one. An observed production peak is not a verified +inverter limit. More detailed solar priors can follow measured comparison; +they do not justify sending new hardware commands. + +## Share access with a code + +An owner can create a single-use code for another person, choose a lifetime +and choose full technical read access or additional setup/control rights. +Technical read access covers the same complete diagnostic API as the owner's +MCP grant, including energy logs, source and bug evidence. Sharing does not +copy an owner's token, private key or role. + +The recipient connects their own MCP client and redeems the code. The box +binds the resulting grant to that client's own key. An interrupted or repeated +claim must not create a second grant. The owner sees who claimed the code, +what rights they received, when access ends and how to revoke it. Revoking a +grant ends its open sessions; cancelling an unclaimed code prevents later use. +The code itself has a short claim window, distinct from the granted access's +lifetime. Store only a verifier, limit failed attempts, and never log the code. + +Cloud may carry a claim but cannot widen the owner's selected rights. A code +cannot name another home or mint owner access. Setup cannot issue a sharing +code merely because an assistant has permission to edit equipment settings. +Test expiry, cancellation, simultaneous claims, retries, guessed codes, +revocation, narrower grants and a client that loses its original connection. + +## Entry through ChatGPT and other clients + +The remote MCP endpoint remains client-neutral. Its tool descriptions tell +clients when to start setup, resume a draft, use normal tools or enter deep +troubleshooting. Return source links and running versions in troubleshooting. +No model account or OpenAI API key belongs on the box for this workflow. + +A public ChatGPT plugin is a distribution step after the workflow works. +OpenAI's current submission docs require a verified publisher, working OAuth, +product/support/privacy/terms URLs, review cases, a demo and reviewer access +for MCP review. Prepare those against a test home with simulated equipment. +Directory review and approval remain external; a working MCP server does not +mean FTW is listed. Other clients can use the same MCP without that listing. + +Source: [OpenAI plugin submission](https://developers.openai.com/plugins/deploy/submission). + +## Delivery and acceptance + +Ship in separate beta increments so the current full diagnostic read release +does not wait for a new setup architecture: + +- A shared setup model, complete schema, durable drafts, conflict checks and + validation; test a blank config, an existing home, partial answers, unknown + fields, secret preservation, failed saves and restart/resume. +- The paired Core/Cloud setup contract and explicit setup grant; test real + OAuth and Noise sessions, revocation, narrowed grants and older boxes. +- Passive first-run claim and the full conversation through saved config; + test a truly empty data directory, interrupted pairing, no Cloud consent, + a lost client and restart. Keep the existing wizard usable. +- Owner-issued sharing codes and redemption from another MCP client; test + the recipient's independent key and complete revocation through a real + OAuth/Noise session before describing sharing as shipped. +- Optional forecast priors; test energy accounting, no invented samples, + learned-data preservation and measured replacement of the prior. +- Commissioning and directory packaging; verify actual supported hardware + before claiming physical setup or control works. Run a complete novice + trial from a new box through ChatGPT and through another MCP client. + +No beta or directory description may claim the later steps work on the +strength of an earlier increment's tests. diff --git a/go/internal/onboarding/draft.go b/go/internal/onboarding/draft.go new file mode 100644 index 000000000..00c22cf07 --- /dev/null +++ b/go/internal/onboarding/draft.go @@ -0,0 +1,305 @@ +// Package onboarding keeps an unfinished setup on the box. It does not +// change live configuration or issue device commands. +package onboarding + +import ( + "bytes" + "crypto/sha256" + "encoding/hex" + "encoding/json" + "errors" + "fmt" + "io" + "sync" + + "github.com/srcfl/ftw/go/internal/config" +) + +const ( + draftKey = "onboarding/draft_v1" + MaxDraftBytes = 1 << 20 +) + +var ErrConflict = errors.New("setup changed; read the current draft before editing") + +// Storage is the box's durable settings store. A failed write must leave +// the previous value intact. +type Storage interface { + LoadConfig(string) (string, bool) + SaveConfig(string, string) error +} + +// Draft is an uncommitted configuration. Config can contain credentials; +// a transport must mask them before returning it to a client. +type Draft struct { + Version int `json:"version"` + Revision int64 `json:"revision"` + BaseRevision int64 `json:"base_revision"` + Config json.RawMessage `json:"config"` + SHA256 string `json:"sha256"` +} + +// Service serializes draft changes. Live config saves remain the caller's +// responsibility, including a final compare-and-save of BaseRevision. +type Service struct { + mu sync.Mutex + store Storage +} + +func New(store Storage) *Service { return &Service{store: store} } + +func (s *Service) current() (Draft, bool, error) { + if s.store == nil { + return Draft{}, false, errors.New("setup storage is unavailable") + } + raw, ok := s.store.LoadConfig(draftKey) + if !ok || raw == "" { + return Draft{}, false, nil + } + var d Draft + if len(raw) > MaxDraftBytes || json.Unmarshal([]byte(raw), &d) != nil || d.Version != 1 || d.Revision < 1 || d.BaseRevision < 0 { + return Draft{}, false, errors.New("stored setup draft is invalid; it has not been replaced") + } + if _, err := decodeConfig(d.Config); err != nil || d.SHA256 != digest(d.Config, d.BaseRevision) { + return Draft{}, false, errors.New("stored setup draft failed its integrity check; it has not been replaced") + } + return d, true, nil +} + +// Read returns a saved draft, or a draft based on this exact live config. +// Reading never creates a stored draft or changes settings. +func (s *Service) Read(live *config.Config) (Draft, error) { + s.mu.Lock() + defer s.mu.Unlock() + if d, found, err := s.current(); found || err != nil { + return d, err + } + return initial(live) +} + +func initial(live *config.Config) (Draft, error) { + if live == nil { + return Draft{}, errors.New("live configuration is unavailable") + } + raw, err := json.Marshal(live) + if err != nil { + return Draft{}, err + } + return Draft{Version: 1, BaseRevision: live.Revision, Config: raw, SHA256: digest(raw, live.Revision)}, nil +} + +// Edit applies a JSON merge patch to a typed config. Fields omitted by the +// client keep their current value, including credentials. Arrays replace +// the whole array; the client must read it before editing an entry. +// Partial drafts may fail Config.Validate while still having valid types. +func (s *Service) Edit(live *config.Config, expected int64, patch json.RawMessage) (Draft, error) { + s.mu.Lock() + defer s.mu.Unlock() + d, found, err := s.current() + if err != nil { + return Draft{}, err + } + if !found { + d, err = initial(live) + if err != nil { + return Draft{}, err + } + } + if d.Revision != expected || live == nil || d.BaseRevision != live.Revision { + return Draft{}, ErrConflict + } + if len(patch) == 0 || len(patch) > MaxDraftBytes { + return Draft{}, errors.New("setup patch is empty or too large") + } + var before, changes map[string]json.RawMessage + if err := strictObject(patch, &changes); err != nil { + return Draft{}, err + } + if err := json.Unmarshal(d.Config, &before); err != nil { + return Draft{}, err + } + merged, err := merge(before, changes) + if err != nil { + return Draft{}, err + } + cfg, err := decodeConfig(merged) + if err != nil { + return Draft{}, err + } + // Canonicalize through the current type. Do not default or validate yet: + // both would hide missing answers or reject an unfinished installation. + d.Config, err = json.Marshal(cfg) + if err != nil { + return Draft{}, err + } + d.Revision++ + d.SHA256 = digest(d.Config, d.BaseRevision) + raw, err := json.Marshal(d) + if err != nil || len(raw) > MaxDraftBytes { + return Draft{}, errors.New("setup draft is too large") + } + if err := s.store.SaveConfig(draftKey, string(raw)); err != nil { + return Draft{}, fmt.Errorf("save setup draft: %w", err) + } + return d, nil +} + +// Checked returns a normalized, valid candidate only when the caller names +// the exact current draft and live base. It neither persists nor applies it. +func (s *Service) Checked(liveRevision, revision int64, sha string) (*config.Config, error) { + s.mu.Lock() + defer s.mu.Unlock() + d, found, err := s.current() + if err != nil { + return nil, err + } + if !found || d.BaseRevision != liveRevision || d.Revision != revision || d.SHA256 != sha { + return nil, ErrConflict + } + cfg, err := decodeConfig(d.Config) + if err != nil { + return nil, err + } + if err := cfg.Validate(); err != nil { + return nil, fmt.Errorf("configuration is incomplete or invalid: %w", err) + } + return cfg, nil +} + +// Restart abandons a stale draft only when the caller names its current +// revision. It stores a new draft from live settings, never an empty config. +func (s *Service) Restart(live *config.Config, expected int64) (Draft, error) { + s.mu.Lock() + defer s.mu.Unlock() + old, found, err := s.current() + if err != nil { + return Draft{}, err + } + if (!found && expected != 0) || (found && old.Revision != expected) { + return Draft{}, ErrConflict + } + d, err := initial(live) + if err != nil { + return Draft{}, err + } + d.Revision = old.Revision + 1 + raw, err := json.Marshal(d) + if err != nil || len(raw) > MaxDraftBytes { + return Draft{}, errors.New("setup draft is too large") + } + if err := s.store.SaveConfig(draftKey, string(raw)); err != nil { + return Draft{}, err + } + return d, nil +} + +func digest(raw []byte, revision int64) string { + h := sha256.New() + fmt.Fprintf(h, "%d\n", revision) + h.Write(raw) + return hex.EncodeToString(h.Sum(nil)) +} + +func decodeConfig(raw []byte) (*config.Config, error) { + if len(raw) > MaxDraftBytes { + return nil, errors.New("setup config is too large") + } + var cfg config.Config + dec := json.NewDecoder(bytes.NewReader(raw)) + dec.DisallowUnknownFields() + if err := dec.Decode(&cfg); err != nil { + return nil, fmt.Errorf("setup config: %w", err) + } + if err := dec.Decode(new(any)); err != io.EOF { + return nil, errors.New("setup config must contain one JSON object") + } + return &cfg, nil +} + +func strictObject(raw []byte, out *map[string]json.RawMessage) error { + if err := rejectDuplicates(raw); err != nil { + return err + } + if err := json.Unmarshal(raw, out); err != nil || *out == nil { + return errors.New("setup patch must be a JSON object") + } + return nil +} + +func merge(before, patch map[string]json.RawMessage) ([]byte, error) { + for key, value := range patch { + if bytes.Equal(bytes.TrimSpace(value), []byte("null")) { + delete(before, key) + continue + } + var object map[string]json.RawMessage + if json.Unmarshal(value, &object) == nil && object != nil { + var previous map[string]json.RawMessage + _ = json.Unmarshal(before[key], &previous) + if previous == nil { + previous = map[string]json.RawMessage{} + } + merged, err := merge(previous, object) + if err != nil { + return nil, err + } + before[key] = merged + } else { + before[key] = value + } + } + return json.Marshal(before) +} + +// JSON decoders normally accept repeated keys with the last one winning. +// A checked configuration must have one meaning at every boundary. +func rejectDuplicates(raw []byte) error { + dec := json.NewDecoder(bytes.NewReader(raw)) + dec.UseNumber() + var value func() error + value = func() error { + token, err := dec.Token() + if err != nil { + return err + } + delim, container := token.(json.Delim) + if !container { + return nil + } + switch delim { + case '{': + seen := map[string]bool{} + for dec.More() { + key, err := dec.Token() + if err != nil { + return err + } + name, ok := key.(string) + if !ok || seen[name] { + return errors.New("setup patch contains a repeated key") + } + seen[name] = true + if err := value(); err != nil { + return err + } + } + case '[': + for dec.More() { + if err := value(); err != nil { + return err + } + } + default: + return errors.New("invalid setup JSON") + } + _, err = dec.Token() + return err + } + if err := value(); err != nil { + return fmt.Errorf("setup patch: %w", err) + } + if _, err := dec.Token(); err != io.EOF { + return errors.New("setup patch must contain one JSON object") + } + return nil +} diff --git a/go/internal/onboarding/draft_test.go b/go/internal/onboarding/draft_test.go new file mode 100644 index 000000000..2d07c984f --- /dev/null +++ b/go/internal/onboarding/draft_test.go @@ -0,0 +1,223 @@ +package onboarding + +import ( + "encoding/json" + "errors" + "strings" + "sync" + "testing" + + "github.com/srcfl/ftw/go/internal/config" + "github.com/srcfl/ftw/go/internal/state" +) + +func testLive(t *testing.T) *config.Config { + t.Helper() + c, err := config.Parse([]byte("site:\n name: Original\nfuse:\n max_amps: 16\n phases: 3\n voltage: 230\nprice:\n provider: entsoe\n api_key: saved-secret\n"), t.TempDir()) + if err != nil { + t.Fatal(err) + } + c.Revision = 7 + return c +} + +func draftStore(t *testing.T) *state.Store { + t.Helper() + s, err := state.Open(t.TempDir() + "/state.db") + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { s.Close() }) + return s +} + +func TestDraftResumesFromDurableStorageWithoutChangingLiveConfig(t *testing.T) { + live := testLive(t) + store := draftStore(t) + s := New(store) + first, err := s.Read(live) + if err != nil || first.Revision != 0 { + t.Fatalf("initial: %+v %v", first, err) + } + if _, exists := store.LoadConfig(draftKey); exists { + t.Fatal("reading wrote a draft") + } + draft, err := s.Edit(live, 0, json.RawMessage(`{"site":{"name":"My home"},"price":{"zone":"SE3"}}`)) + if err != nil { + t.Fatal(err) + } + // A new service has no in-memory draft to restore from. + resumed, err := New(store).Read(live) + if err != nil || resumed.SHA256 != draft.SHA256 || resumed.Revision != 1 { + t.Fatalf("resume: %+v %v", resumed, err) + } + checked, err := New(store).Checked(live.Revision, resumed.Revision, resumed.SHA256) + if err != nil { + t.Fatal(err) + } + if checked.Site.Name != "My home" || checked.Price.APIKey != "saved-secret" || checked.Price.Zone != "SE3" { + t.Fatal("patch lost an unedited value or credential") + } + if live.Site.Name != "Original" || live.Price.Zone != "" { + t.Fatal("draft changed live config") + } +} + +func TestDraftConflictsProtectBothDraftAndLiveSettings(t *testing.T) { + live := testLive(t) + s := New(draftStore(t)) + draft, err := s.Edit(live, 0, json.RawMessage(`{"site":{"name":"First"}}`)) + if err != nil { + t.Fatal(err) + } + if _, err = s.Edit(live, 0, json.RawMessage(`{"site":{"name":"Lost update"}}`)); !errors.Is(err, ErrConflict) { + t.Fatal(err) + } + if _, err = s.Checked(live.Revision, draft.Revision, "another digest"); !errors.Is(err, ErrConflict) { + t.Fatal(err) + } + live.Revision++ + if _, err = s.Edit(live, draft.Revision, json.RawMessage(`{"site":{"name":"Old base"}}`)); !errors.Is(err, ErrConflict) { + t.Fatal(err) + } + if _, err = s.Checked(live.Revision, draft.Revision, draft.SHA256); !errors.Is(err, ErrConflict) { + t.Fatal(err) + } + if _, err = s.Restart(live, 0); !errors.Is(err, ErrConflict) { + t.Fatal(err) + } + restarted, err := s.Restart(live, draft.Revision) + if err != nil || restarted.BaseRevision != live.Revision || restarted.Revision != 2 { + t.Fatalf("restart: %+v %v", restarted, err) + } + if strings.Contains(string(restarted.Config), "First") { + t.Fatal("restart kept the stale draft") + } +} + +func TestIncompleteDraftCanResumeButCannotPassValidation(t *testing.T) { + live := testLive(t) + s := New(draftStore(t)) + draft, err := s.Edit(live, 0, json.RawMessage(`{"fuse":{"max_amps":0},"drivers":[{"name":"new meter"}]}`)) + if err != nil { + t.Fatal(err) + } + if _, err = s.Checked(live.Revision, draft.Revision, draft.SHA256); err == nil { + t.Fatal("unfinished driver passed validation") + } + resumed, err := s.Read(live) + if err != nil || resumed.SHA256 != draft.SHA256 { + t.Fatal("validation changed the saved draft", err) + } +} + +func TestMalformedOrUnknownFieldsNeverReplaceTheDraft(t *testing.T) { + live := testLive(t) + s := New(draftStore(t)) + for _, raw := range []string{ + `null`, `[]`, `{} {}`, `{"unknown":1}`, `{"site":{"not_a_field":1}}`, + `{"fuse":{"max_amps":"sixteen"}}`, `{"site":{"name":"A","name":"B"}}`, + `{"drivers":[{"name":"A","name":"B"}]}`, `{"site":{`, + } { + t.Run(raw, func(t *testing.T) { + if _, err := s.Edit(live, 0, json.RawMessage(raw)); err == nil { + t.Fatal("accepted invalid patch") + } + if got, err := s.Read(live); err != nil || got.Revision != 0 { + t.Fatal("invalid patch changed the draft", err) + } + }) + } +} + +type failedStore struct{ Storage } + +func (s failedStore) SaveConfig(string, string) error { return errors.New("disk full") } + +func TestFailedDraftSaveLeavesPreviousRevisionIntact(t *testing.T) { + live := testLive(t) + store := draftStore(t) + draft, err := New(store).Edit(live, 0, json.RawMessage(`{"site":{"name":"Saved"}}`)) + if err != nil { + t.Fatal(err) + } + if _, err := New(failedStore{store}).Edit(live, draft.Revision, json.RawMessage(`{"site":{"name":"Unsaved"}}`)); err == nil { + t.Fatal("reported a failed save as success") + } + got, err := New(store).Read(live) + if err != nil || got.SHA256 != draft.SHA256 { + t.Fatal("failed save replaced saved state", err) + } +} + +func TestConcurrentEditsHaveOneWinner(t *testing.T) { + live := testLive(t) + s := New(draftStore(t)) + var wg sync.WaitGroup + results := make(chan error, 2) + for _, name := range []string{"First", "Second"} { + wg.Go(func() { + _, err := s.Edit(live, 0, json.RawMessage(`{"site":{"name":"`+name+`"}}`)) + results <- err + }) + } + wg.Wait() + close(results) + winners, conflicts := 0, 0 + for err := range results { + if err == nil { + winners++ + } else if errors.Is(err, ErrConflict) { + conflicts++ + } else { + t.Fatal(err) + } + } + if winners != 1 || conflicts != 1 { + t.Fatalf("%d winners, %d conflicts", winners, conflicts) + } +} + +func TestDamagedDraftIsNotSilentlyReplaced(t *testing.T) { + live := testLive(t) + store := draftStore(t) + if err := store.SaveConfig(draftKey, `{"version":99}`); err != nil { + t.Fatal(err) + } + s := New(store) + if _, err := s.Read(live); err == nil { + t.Fatal("read hid a damaged draft") + } + if _, err := s.Edit(live, 0, json.RawMessage(`{}`)); err == nil { + t.Fatal("edit replaced a damaged draft") + } +} + +func TestSchemaCoversAllPublicConfigFieldsWithoutRuntimeAuthority(t *testing.T) { + live := testLive(t) + raw, err := json.Marshal(live) + if err != nil { + t.Fatal(err) + } + var fields map[string]any + if err := json.Unmarshal(raw, &fields); err != nil { + t.Fatal(err) + } + schema := Schema() + props := schema["properties"].(map[string]any) + for field := range fields { + if _, ok := props[field]; !ok { + t.Fatalf("schema omitted public field %q", field) + } + } + for _, field := range []string{"Revision", "revision", "LANPasswordHash", "lan_password_hash", "config_database"} { + if _, ok := props[field]; ok { + t.Fatalf("schema exposed runtime field %q", field) + } + } + for _, field := range []string{"site", "fuse", "drivers", "price", "weather", "batteries", "loadpoints", "vehicles", "ocpp", "homeassistant", "notifications"} { + if _, ok := props[field]; !ok { + t.Fatalf("schema omitted setup section %q", field) + } + } +} diff --git a/go/internal/onboarding/schema.go b/go/internal/onboarding/schema.go new file mode 100644 index 000000000..aa4374292 --- /dev/null +++ b/go/internal/onboarding/schema.go @@ -0,0 +1,57 @@ +package onboarding + +import ( + "reflect" + "strings" + + "github.com/srcfl/ftw/go/internal/config" +) + +// Schema describes this build's complete public configuration type. Driver +// Config maps use the live driver catalogue for their vendor-specific fields. +// Runtime-only fields and stored password hashes never enter this schema. +func Schema() map[string]any { + schema := schemaFor(reflect.TypeFor[config.Config]()) + schema["$schema"] = "https://json-schema.org/draft/2020-12/schema" + schema["description"] = "FTW configuration. Patch a draft, then use Core validation. Units are W, Wh, A, V, seconds and SoC fractions 0..1. Arrays replace the complete array; retain entries you did not change. Driver config fields come from /api/drivers/catalog." + return schema +} + +func schemaFor(t reflect.Type) map[string]any { + if t.Kind() == reflect.Pointer { + return map[string]any{"anyOf": []any{schemaFor(t.Elem()), map[string]any{"type": "null"}}} + } + switch t.Kind() { + case reflect.Struct: + properties := map[string]any{} + for i := 0; i < t.NumField(); i++ { + f := t.Field(i) + if !f.IsExported() { + continue + } + name := strings.Split(f.Tag.Get("json"), ",")[0] + if name == "-" { + continue + } + if name == "" { + name = f.Name + } + properties[name] = schemaFor(f.Type) + } + return map[string]any{"type": "object", "properties": properties, "additionalProperties": false} + case reflect.Map: + return map[string]any{"type": "object", "additionalProperties": schemaFor(t.Elem())} + case reflect.Slice, reflect.Array: + return map[string]any{"type": "array", "items": schemaFor(t.Elem())} + case reflect.String: + return map[string]any{"type": "string"} + case reflect.Bool: + return map[string]any{"type": "boolean"} + case reflect.Float32, reflect.Float64: + return map[string]any{"type": "number"} + case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64, reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64: + return map[string]any{"type": "integer"} + default: + return map[string]any{} + } +} From 8e604db807665672aac9eb716d0e6c981d442d49 Mon Sep 17 00:00:00 2001 From: Fredrik Ahlgren Date: Sun, 18 Oct 2026 17:12:11 +0200 Subject: [PATCH 2/6] Add setup drafts, secret preservation and atomic commit receipts --- .changeset/agent-setup-drafts.md | 5 + docs/adr/0011-agent-led-setup.md | 19 ++ go/internal/api/api.go | 40 ++-- go/internal/api/api_agent_catalog.go | 1 + go/internal/api/api_setup.go | 261 +++++++++++++++++++++++++++ go/internal/api/api_setup_test.go | 179 ++++++++++++++++++ go/internal/api/security.go | 1 + go/internal/config/storage.go | 16 ++ go/internal/onboarding/draft.go | 44 ++++- go/internal/onboarding/draft_test.go | 15 ++ go/internal/onboarding/receipt.go | 37 ++++ 11 files changed, 601 insertions(+), 17 deletions(-) create mode 100644 .changeset/agent-setup-drafts.md create mode 100644 go/internal/api/api_setup.go create mode 100644 go/internal/api/api_setup_test.go create mode 100644 go/internal/onboarding/receipt.go diff --git a/.changeset/agent-setup-drafts.md b/.changeset/agent-setup-drafts.md new file mode 100644 index 000000000..c3549f8b0 --- /dev/null +++ b/.changeset/agent-setup-drafts.md @@ -0,0 +1,5 @@ +--- +"ftw": minor +--- + +Add a complete configuration schema and durable setup drafts. Draft edits preserve unedited settings and credentials; validation and a confirmed commit use the current Settings revision and save path. This prepares setup through MCP; first-run pairing, MCP writes and code sharing remain separate work. diff --git a/docs/adr/0011-agent-led-setup.md b/docs/adr/0011-agent-led-setup.md index c1970145a..47e404bf9 100644 --- a/docs/adr/0011-agent-led-setup.md +++ b/docs/adr/0011-agent-led-setup.md @@ -38,10 +38,15 @@ to a request to read today's energy use. each edit. A new conversation can resume it. Apply patches to the current typed config, preserving fields and saved secrets that the caller did not change. A stale revision must conflict rather than overwrite another edit. + Return the next question, missing facts, field sources and units as + structured data. A schema of field types alone is not a question plan. + Keep new credentials in the draft when a client edits a masked list again. 5. **Check and save.** Validate with Core's existing config checks. Show the proposed changes, assumptions, effects and required restarts. The owner confirms the proposed configuration in the conversation. Commit the exact checked revision through the same durable save and reload path as Settings. + Commit a receipt in the same transaction. If the reply is lost, a retry + must report the saved revision rather than ask the user to save it again. 6. **Verify the installation.** Start with measurement. Require a fresh site meter, confirmed fuse and phase mapping before active dispatch. A driver that reports data has not thereby proved control. Offer bounded control @@ -53,6 +58,12 @@ to a request to read today's energy use. still estimated and what needs attention. Losing the assistant never removes saved goals or stops local operation. +Use small groups of questions, allow "I don't know", and explain how to find +the fuse rating and phase count. Distinguish saved, connected, measuring, +tested and active control. A device failure must not force the user to +discard working parts of the installation. A conflict should show which +fields changed and allow answers to be carried forward. + The workflow covers the full supported config, including driver connection fields, price and grid tariffs, weather, battery windows, vehicles and loadpoints, OCPP, notifications, Home Assistant and other integrations. A @@ -89,6 +100,8 @@ electricity use, whether that figure includes EV charging or electric heat, heating type, usual occupancy, and known solar nameplate capacity. A user who does not know these values must still be able to finish setup. Roof geometry is optional expert input, never a prerequisite. +Ask whether an annual figure means purchased electricity or total use: +solar self-consumption makes those different quantities. Use existing weather, solar scale and heating inputs where they fit. A load prior derived from annual energy must exclude separately forecast EV use and @@ -119,6 +132,12 @@ what rights they received, when access ends and how to revoke it. Revoking a grant ends its open sessions; cancelling an unclaimed code prevents later use. The code itself has a short claim window, distinct from the granted access's lifetime. Store only a verifier, limit failed attempts, and never log the code. +The owner creates an invitation; the recipient selects "I have an invitation" +when connecting their own client. This is distinct from today's flow where +the owner types a code that their assistant generated. Show the owner's label, +the client and claim time. Possession of a code does not verify a person's +real identity. Treat expired, cancelled, already used and network failure as +different states with a clear next step. Cloud may carry a claim but cannot widen the owner's selected rights. A code cannot name another home or mint owner access. Setup cannot issue a sharing diff --git a/go/internal/api/api.go b/go/internal/api/api.go index 555377741..3413f353b 100644 --- a/go/internal/api/api.go +++ b/go/internal/api/api.go @@ -52,6 +52,7 @@ import ( "github.com/srcfl/ftw/go/internal/mpc" "github.com/srcfl/ftw/go/internal/notifications" "github.com/srcfl/ftw/go/internal/ocpp" + "github.com/srcfl/ftw/go/internal/onboarding" "github.com/srcfl/ftw/go/internal/prices" "github.com/srcfl/ftw/go/internal/pvmodel" "github.com/srcfl/ftw/go/internal/scanner" @@ -118,17 +119,18 @@ type Deps struct { // minimal embeddings; the /api/cloud routes then say it is unavailable. Cloud CloudGrant - CfgMu *sync.RWMutex - Cfg *config.Config - ConfigPath string - DriverDir string // where to scan for Lua drivers (default: /drivers) - UserDriverDir string // persistent user-drivers overlay; searched before DriverDir - Models map[string]*battery.Model - ModelsMu *sync.Mutex - ModelWrites *state.ControlWrites - SelfTune *selftune.Coordinator - DtS float64 // control interval seconds (for model τ / age displays) - SaveConfig func(path string, c *config.Config) error // injection for testability + CfgMu *sync.RWMutex + Cfg *config.Config + ConfigPath string + DriverDir string // where to scan for Lua drivers (default: /drivers) + UserDriverDir string // persistent user-drivers overlay; searched before DriverDir + Models map[string]*battery.Model + ModelsMu *sync.Mutex + ModelWrites *state.ControlWrites + SelfTune *selftune.Coordinator + DtS float64 // control interval seconds (for model τ / age displays) + SaveConfig func(path string, c *config.Config) error // injection for testability + SaveConfigWithValues func(path string, c *config.Config, values map[string]string) error // ConfigApplier is main.go's config-applied callback — the same // closure the configreload watcher runs (registry reload with SoC // bounds, capacities, inverter groups, fuse and mpc/loadmodel @@ -261,6 +263,7 @@ type Server struct { configWriteMu sync.Mutex // Covers persistence and apply for every config writer. deps *Deps mux *http.ServeMux + setup *onboarding.Service // dailyCache memoizes per-local-day energy totals keyed by "YYYY-MM-DD". // Past days are immutable once the day ends, so we only ever recompute @@ -327,6 +330,14 @@ func New(deps *Deps) *Server { drafts: newDriverDrafts(), marks: make(map[string]routeMark), } + s.setup = onboarding.NewWithSecrets(deps.State, func(incoming, previous *config.Config) { + restoreDriverConfigSecrets(incoming, previous, s.driverSecretKeys()) + }) + if deps.SaveConfigWithValues == nil && deps.State != nil { + deps.SaveConfigWithValues = func(path string, cfg *config.Config, values map[string]string) error { + return config.SaveStoredWithValues(deps.State, path, cfg, values) + } + } if deps.Registry != nil { // Registry removal is the lifecycle boundary for a driver generation. // Clear the API hold before a replacement instance can be added. @@ -441,6 +452,7 @@ func (s *Server) routes() { s.handle("GET /api/fleet-ping", Read, s.handleFleetPing) s.cloudRoutes() s.agentRoutes() + s.setupRoutes() s.handle("POST /api/mode", Actuate, s.handleSetMode, Via(appproto.OpSetMode)) s.handle("GET /api/planner/prefs", Read, s.handleGetPlannerPrefs) s.handle("POST /api/planner/prefs", Actuate, s.handleSetPlannerPrefs, Via(appproto.OpPlannerPrefsSet)) @@ -1615,6 +1627,10 @@ func restoreDriverConfigSecrets(incoming, existing *config.Config, secretsByLua } func (s *Server) handlePostConfig(w http.ResponseWriter, r *http.Request) { + s.handlePostConfigWith(w, r, s.deps.SaveConfig) +} + +func (s *Server) handlePostConfigWith(w http.ResponseWriter, r *http.Request, save func(string, *config.Config) error) { s.configWriteMu.Lock() defer s.configWriteMu.Unlock() var posted struct { @@ -1672,7 +1688,7 @@ func (s *Server) handlePostConfig(w http.ResponseWriter, r *http.Request) { restartReasons := config.RestartRequiredFor(&oldCfg, &newCfg) // Persist atomically (Password has yaml:"-" so it won't appear in YAML) - if err := s.deps.SaveConfig(s.deps.ConfigPath, &newCfg); err != nil { + if err := save(s.deps.ConfigPath, &newCfg); err != nil { writeJSON(w, 500, map[string]string{"error": "save failed: " + err.Error()}) return } diff --git a/go/internal/api/api_agent_catalog.go b/go/internal/api/api_agent_catalog.go index c5d0e6e9c..f654143ac 100644 --- a/go/internal/api/api_agent_catalog.go +++ b/go/internal/api/api_agent_catalog.go @@ -47,6 +47,7 @@ var agentAPIHints = []protocol.AgentAPIRoute{ // are not reads. Configure and Actuate handlers keep their command path. var agentLocalReads = map[string]bool{ "/api/config": true, + "/api/setup/draft": true, "/api/drivers/{id}/source": true, "/api/drivers/{name}/logs": true, "/api/logs": true, diff --git a/go/internal/api/api_setup.go b/go/internal/api/api_setup.go new file mode 100644 index 000000000..861d33b69 --- /dev/null +++ b/go/internal/api/api_setup.go @@ -0,0 +1,261 @@ +package api + +import ( + "bytes" + "encoding/json" + "errors" + "io" + "net/http" + "path/filepath" + + "github.com/srcfl/ftw/go/internal/config" + "github.com/srcfl/ftw/go/internal/onboarding" +) + +func (s *Server) setupRoutes() { + s.handle("GET /api/setup/schema", Read, s.handleSetupSchema) + s.handle("GET /api/setup/draft", Local, s.handleSetupDraft) + s.handle("POST /api/setup/draft", Configure, s.handleSetupEdit) + s.handle("POST /api/setup/restart", Configure, s.handleSetupRestart) + s.handle("POST /api/setup/validate", Configure, s.handleSetupValidate) + s.handle("POST /api/setup/commit", Configure, s.handleSetupCommit) +} + +func (s *Server) handleSetupSchema(w http.ResponseWriter, _ *http.Request) { + writeJSON(w, http.StatusOK, map[string]any{ + "schema": onboarding.Schema(), "drivers": "/api/drivers/catalog", "price_areas": "/api/prices/zones", + "workflow": []string{ + "Read the current draft. Ask only for missing facts and choices.", + "Discover equipment only when the user asks; reading a draft does not scan.", + "Patch the current draft revision. Omitted fields and saved credentials remain on the box.", + "Arrays replace the whole array. Read and retain all existing entries before editing one.", + "Validate the exact draft, show its changes and assumptions, then ask the user to confirm before commit.", + "Saving configuration does not prove hardware operation. Check fresh measurements and bounded control tests separately.", + }, + "normal_use": "Use status, history and control tools for everyday questions. Setup is a separate workflow.", + }) +} + +func (s *Server) setupLive() (*config.Config, error) { + if s.deps.Cfg == nil || s.deps.CfgMu == nil || s.deps.State == nil { + return nil, errors.New("setup is not available on this box") + } + s.deps.CfgMu.RLock() + raw, err := json.Marshal(s.deps.Cfg) + revision := s.deps.Cfg.Revision + s.deps.CfgMu.RUnlock() + if err != nil { + return nil, err + } + var cfg config.Config + if err := json.Unmarshal(raw, &cfg); err != nil { + return nil, err + } + cfg.Revision = revision + cfg.UnresolveDriverPaths(filepath.Dir(s.deps.ConfigPath)) + return &cfg, nil +} + +func (s *Server) setupError(w http.ResponseWriter, err error) { + status := http.StatusUnprocessableEntity + if errors.Is(err, onboarding.ErrConflict) { + status = http.StatusConflict + } + writeJSON(w, status, map[string]string{"error": err.Error()}) +} + +func (s *Server) handleSetupDraft(w http.ResponseWriter, _ *http.Request) { + live, err := s.setupLive() + if err != nil { + s.setupError(w, err) + return + } + d, err := s.setup.Read(live) + if err != nil { + s.setupError(w, err) + return + } + s.writeSetupDraft(w, live, d) +} + +func (s *Server) writeSetupDraft(w http.ResponseWriter, live *config.Config, d onboarding.Draft) { + var candidate config.Config + if err := json.Unmarshal(d.Config, &candidate); err != nil { + s.setupError(w, err) + return + } + masked := candidate.MaskSecrets() + maskDriverConfigSecrets(&masked, s.driverSecretKeys()) + validation := "" + if err := candidate.Validate(); err != nil { + validation = err.Error() + } + receipt, err := s.setup.Receipt() + if err != nil { + s.setupError(w, err) + return + } + saved := receipt != nil && receipt.DraftRevision == d.Revision && receipt.SHA256 == d.SHA256 + applied := saved && receipt.SavedRevision == live.Revision + w.Header().Set("Cache-Control", "no-store") + writeJSON(w, http.StatusOK, map[string]any{ + "revision": d.Revision, "base_revision": d.BaseRevision, "live_revision": live.Revision, + "sha256": d.SHA256, "config": masked, "stale": !saved && d.BaseRevision != live.Revision, + "valid": validation == "", "validation_error": validation, + "saved": saved, "applied": applied, "hardware_verified": false, "last_commit": receipt, + "restart_reasons": config.RestartRequiredFor(live, &candidate), + }) +} + +type setupEdit struct { + Revision int64 `json:"revision"` + Patch json.RawMessage `json:"patch"` +} + +func setupJSON(w http.ResponseWriter, r *http.Request, v any) bool { + r.Body = http.MaxBytesReader(w, r.Body, onboarding.MaxDraftBytes) + dec := json.NewDecoder(r.Body) + dec.DisallowUnknownFields() + if err := dec.Decode(v); err != nil { + writeJSON(w, http.StatusBadRequest, map[string]string{"error": "invalid setup request"}) + return false + } + if err := dec.Decode(new(any)); err != io.EOF { + writeJSON(w, http.StatusBadRequest, map[string]string{"error": "send one setup request"}) + return false + } + return true +} + +func (s *Server) handleSetupEdit(w http.ResponseWriter, r *http.Request) { + var q setupEdit + if !setupJSON(w, r, &q) { + return + } + live, err := s.setupLive() + if err != nil { + s.setupError(w, err) + return + } + d, err := s.setup.Edit(live, q.Revision, q.Patch) + if err != nil { + s.setupError(w, err) + return + } + s.writeSetupDraft(w, live, d) +} + +func (s *Server) handleSetupRestart(w http.ResponseWriter, r *http.Request) { + var q struct { + Revision int64 `json:"revision"` + } + if !setupJSON(w, r, &q) { + return + } + live, err := s.setupLive() + if err != nil { + s.setupError(w, err) + return + } + d, err := s.setup.Restart(live, q.Revision) + if err != nil { + s.setupError(w, err) + return + } + s.writeSetupDraft(w, live, d) +} + +type setupCheck struct { + Revision int64 `json:"revision"` + BaseRevision int64 `json:"base_revision"` + SHA256 string `json:"sha256"` + Confirmed bool `json:"confirmed"` +} + +func (s *Server) handleSetupValidate(w http.ResponseWriter, r *http.Request) { + var q setupCheck + if !setupJSON(w, r, &q) { + return + } + live, err := s.setupLive() + if err != nil { + s.setupError(w, err) + return + } + if q.BaseRevision != live.Revision { + s.setupError(w, onboarding.ErrConflict) + return + } + if _, err := s.setup.Checked(live.Revision, q.Revision, q.SHA256); err != nil { + s.setupError(w, err) + return + } + d, err := s.setup.Read(live) + if err != nil { + s.setupError(w, err) + return + } + s.writeSetupDraft(w, live, d) +} + +func (s *Server) handleSetupCommit(w http.ResponseWriter, r *http.Request) { + var q setupCheck + if !setupJSON(w, r, &q) { + return + } + live, err := s.setupLive() + if err != nil { + s.setupError(w, err) + return + } + previous, err := s.setup.Receipt() + if err != nil { + s.setupError(w, err) + return + } + if previous != nil && previous.DraftRevision == q.Revision && previous.SHA256 == q.SHA256 && previous.BaseRevision == q.BaseRevision { + writeJSON(w, http.StatusOK, map[string]any{"status": "ok", "saved": true, + "applied": live.Revision == previous.SavedRevision, "live_revision": live.Revision, + "later_changes": live.Revision > previous.SavedRevision, "commit": previous, "hardware_verified": false}) + return + } + if !q.Confirmed { + writeJSON(w, http.StatusBadRequest, map[string]string{"error": "review the checked draft with the user before confirming it"}) + return + } + if s.deps.SaveConfigWithValues == nil { + writeJSON(w, http.StatusServiceUnavailable, map[string]string{"error": "atomic setup save is not available"}) + return + } + if q.BaseRevision != live.Revision { + s.setupError(w, onboarding.ErrConflict) + return + } + err = s.setup.ApplyChecked(live.Revision, q.Revision, q.SHA256, func(candidate *config.Config) error { + raw, err := json.Marshal(candidate) + if err != nil { + return err + } + // Reuse Settings' final revision check, durable save and shared applier. + // This does not dispatch commands, change mode or claim commissioning. + request := r.Clone(r.Context()) + request.Body = io.NopCloser(bytes.NewReader(raw)) + request.ContentLength = int64(len(raw)) + request.Header = r.Header.Clone() + request.Header.Set("If-Match", configETag(q.BaseRevision)) + receipt := onboarding.Receipt{DraftRevision: q.Revision, SHA256: q.SHA256, + BaseRevision: q.BaseRevision, SavedRevision: q.BaseRevision + 1, + RestartReasons: config.RestartRequiredFor(live, candidate)} + record, err := json.Marshal(receipt) + if err != nil { + return err + } + s.handlePostConfigWith(w, request, func(path string, cfg *config.Config) error { + return s.deps.SaveConfigWithValues(path, cfg, map[string]string{onboarding.CommitKey: string(record)}) + }) + return nil + }) + if err != nil { + s.setupError(w, err) + } +} diff --git a/go/internal/api/api_setup_test.go b/go/internal/api/api_setup_test.go new file mode 100644 index 000000000..22c6add00 --- /dev/null +++ b/go/internal/api/api_setup_test.go @@ -0,0 +1,179 @@ +package api + +import ( + "bytes" + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/srcfl/ftw/go/internal/config" +) + +func setupTestServer(t *testing.T) (*Server, *config.Config) { + t.Helper() + s, cfg, _ := storedConfigServer(t) + return New(s.deps), cfg +} + +func setupCall(t *testing.T, s *Server, method, path string, body any) (*httptest.ResponseRecorder, map[string]any) { + t.Helper() + raw, err := json.Marshal(body) + if err != nil { + t.Fatal(err) + } + r := httptest.NewRequest(method, path, bytes.NewReader(raw)) + r.Header.Set("Content-Type", "application/json") + w := httptest.NewRecorder() + s.Handler().ServeHTTP(w, r) + var result map[string]any + if err := json.Unmarshal(w.Body.Bytes(), &result); err != nil { + t.Fatalf("%s %s: %d %s", method, path, w.Code, w.Body.String()) + } + return w, result +} + +func TestSetupDraftCommitUsesTheDurableSettingsPath(t *testing.T) { + s, cfg := setupTestServer(t) + beforeRevision := cfg.Revision + w, draft := setupCall(t, s, http.MethodPost, "/api/setup/draft", map[string]any{"revision": 0, "patch": map[string]any{"site": map[string]any{"name": "New home"}}}) + if w.Code != http.StatusOK || draft["revision"] != float64(1) || draft["applied"] != false { + t.Fatalf("draft: %d %s", w.Code, w.Body.String()) + } + if cfg.Site.Name != "Before" || cfg.Revision != beforeRevision { + t.Fatal("editing applied settings") + } + if strings.Contains(w.Body.String(), "before-secret") { + t.Fatal("draft returned a saved credential") + } + checked := map[string]any{"revision": draft["revision"], "base_revision": draft["base_revision"], "sha256": draft["sha256"]} + w, _ = setupCall(t, s, http.MethodPost, "/api/setup/commit", checked) + if w.Code != http.StatusBadRequest || cfg.Site.Name != "Before" { + t.Fatal("unconfirmed draft changed settings") + } + w, _ = setupCall(t, s, http.MethodPost, "/api/setup/validate", checked) + if w.Code != http.StatusOK { + t.Fatalf("validate: %d %s", w.Code, w.Body.String()) + } + checked["confirmed"] = true + w, _ = setupCall(t, s, http.MethodPost, "/api/setup/commit", checked) + if w.Code != http.StatusOK || cfg.Site.Name != "New home" || cfg.Revision != beforeRevision+1 { + t.Fatalf("commit: %d %s", w.Code, w.Body.String()) + } + restarted, err := config.Load(s.deps.ConfigPath) + if err != nil { + t.Fatal(err) + } + if restarted.Site.Name != "New home" || restarted.EVCharger.Password != "before-secret" { + t.Fatal("commit did not survive reload with its saved credential") + } + w, retried := setupCall(t, New(s.deps), http.MethodPost, "/api/setup/commit", checked) + if w.Code != http.StatusOK || retried["saved"] != true || retried["applied"] != true || cfg.Revision != beforeRevision+1 { + t.Fatal("retry after a lost reply did not find the saved receipt", w.Body.String()) + } +} + +func TestSetupDraftResumesAcrossAPIServerRestartAndRejectsOtherSettingsEdits(t *testing.T) { + s, cfg := setupTestServer(t) + w, draft := setupCall(t, s, http.MethodPost, "/api/setup/draft", map[string]any{"revision": 0, "patch": map[string]any{"site": map[string]any{"name": "Draft"}}}) + if w.Code != http.StatusOK { + t.Fatal(w.Body.String()) + } + s = New(s.deps) + w, resumed := setupCall(t, s, http.MethodGet, "/api/setup/draft", nil) + if w.Code != http.StatusOK || resumed["sha256"] != draft["sha256"] { + t.Fatal("server restart lost draft") + } + // A concurrent Settings writer commits a different name first. + next := *cfg + next.Site.Name = "Another writer" + raw, err := json.Marshal(next) + if err != nil { + t.Fatal(err) + } + if code := postConfig(t, s, string(raw)); code != http.StatusOK { + t.Fatal(code) + } + w, _ = setupCall(t, s, http.MethodPost, "/api/setup/commit", map[string]any{"revision": draft["revision"], "base_revision": draft["base_revision"], "sha256": draft["sha256"], "confirmed": true}) + if w.Code != http.StatusConflict || cfg.Site.Name != "Another writer" { + t.Fatal("stale draft overwrote another writer") + } +} + +func TestSetupDraftDoesNotClaimCommissioningFromAValidConfig(t *testing.T) { + s, _ := setupTestServer(t) + w, result := setupCall(t, s, http.MethodGet, "/api/setup/draft", nil) + if w.Code != http.StatusOK || result["hardware_verified"] != false || result["applied"] != false { + t.Fatal(w.Body.String()) + } + w, result = setupCall(t, s, http.MethodGet, "/api/setup/schema", nil) + if w.Code != http.StatusOK || result["schema"] == nil || result["drivers"] != "/api/drivers/catalog" { + t.Fatal(w.Body.String()) + } +} + +func TestSetupFailedSaveLeavesLiveAndStoredConfigUntouched(t *testing.T) { + s, cfg := setupTestServer(t) + w, draft := setupCall(t, s, http.MethodPost, "/api/setup/draft", map[string]any{"revision": 0, "patch": map[string]any{"site": map[string]any{"name": "Unsaved"}}}) + if w.Code != http.StatusOK { + t.Fatal(w.Body.String()) + } + s.deps.SaveConfigWithValues = func(string, *config.Config, map[string]string) error { return http.ErrHandlerTimeout } + w, _ = setupCall(t, s, http.MethodPost, "/api/setup/commit", map[string]any{"revision": draft["revision"], "base_revision": draft["base_revision"], "sha256": draft["sha256"], "confirmed": true}) + if w.Code != http.StatusInternalServerError || cfg.Site.Name != "Before" { + t.Fatal("failed commit changed live settings") + } + restarted, err := config.Load(s.deps.ConfigPath) + if err != nil || restarted.Site.Name != "Before" { + t.Fatal("failed commit changed durable settings", err) + } + if receipt, err := s.setup.Receipt(); err != nil || receipt != nil { + t.Fatal("failed save left a success receipt", err) + } +} + +func TestSetupMaskedArrayRoundTripKeepsNewDraftCredentials(t *testing.T) { + s, _ := setupTestServer(t) + first := map[string]any{"name": "first meter", "lua": "drivers/test.lua", "is_site_meter": true, + "capabilities": map[string]any{"standalone": true}, "config": map[string]any{"api_token": "first-draft-secret"}} + w, draft := setupCall(t, s, http.MethodPost, "/api/setup/draft", map[string]any{"revision": 0, "patch": map[string]any{"drivers": []any{first}}}) + if w.Code != http.StatusOK || strings.Contains(w.Body.String(), "first-draft-secret") { + t.Fatal(w.Body.String()) + } + maskedDrivers := draft["config"].(map[string]any)["drivers"].([]any) + maskedDrivers = append(maskedDrivers, map[string]any{"name": "second device", "lua": "drivers/second.lua", + "capabilities": map[string]any{"standalone": true}, "config": map[string]any{"api_token": "second-draft-secret"}}) + w, draft = setupCall(t, s, http.MethodPost, "/api/setup/draft", map[string]any{"revision": draft["revision"], "patch": map[string]any{"drivers": maskedDrivers}}) + if w.Code != http.StatusOK || strings.Contains(w.Body.String(), "first-draft-secret") || strings.Contains(w.Body.String(), "second-draft-secret") { + t.Fatal(w.Body.String()) + } + candidate, err := s.setup.Checked(int64(draft["base_revision"].(float64)), int64(draft["revision"].(float64)), draft["sha256"].(string)) + if err != nil { + t.Fatal(err) + } + if candidate.Drivers[0].Config["api_token"] != "first-draft-secret" || candidate.Drivers[1].Config["api_token"] != "second-draft-secret" { + t.Fatal("masked list round trip lost a new credential") + } +} + +func TestSetupReceiptAndSettingsCommitTogether(t *testing.T) { + s, cfg := setupTestServer(t) + w, draft := setupCall(t, s, http.MethodPost, "/api/setup/draft", map[string]any{"revision": 0, "patch": map[string]any{"site": map[string]any{"name": "Committed"}}}) + if w.Code != http.StatusOK { + t.Fatal(w.Body.String()) + } + check := map[string]any{"revision": draft["revision"], "base_revision": draft["base_revision"], "sha256": draft["sha256"], "confirmed": true} + w, _ = setupCall(t, s, http.MethodPost, "/api/setup/commit", check) + if w.Code != http.StatusOK { + t.Fatal(w.Body.String()) + } + s = New(s.deps) + w, resumed := setupCall(t, s, http.MethodGet, "/api/setup/draft", nil) + if w.Code != http.StatusOK || resumed["saved"] != true || resumed["applied"] != true || resumed["stale"] != false || resumed["hardware_verified"] != false { + t.Fatal(w.Body.String()) + } + if got := resumed["last_commit"].(map[string]any)["saved_revision"]; got != float64(cfg.Revision) { + t.Fatal("receipt names a different settings revision") + } +} diff --git a/go/internal/api/security.go b/go/internal/api/security.go index 20848e9cb..842fd4a8e 100644 --- a/go/internal/api/security.go +++ b/go/internal/api/security.go @@ -261,6 +261,7 @@ func requiresMutationProtection(r *http.Request) bool { func protectedReadPath(path string) bool { switch path { case "/api/config", + "/api/setup/draft", "/api/support/dump", "/api/support/report", "/api/logs", diff --git a/go/internal/config/storage.go b/go/internal/config/storage.go index 9351242bd..735660cce 100644 --- a/go/internal/config/storage.go +++ b/go/internal/config/storage.go @@ -215,6 +215,12 @@ func SaveStored(st *state.Store, path string, cfg *Config) error { return saveStored(st, path, cfg, "") } +// SaveStoredWithValues commits a settings change and its companion records +// in the same SQLite transaction, for example a setup commit receipt. +func SaveStoredWithValues(st *state.Store, path string, cfg *Config, values map[string]string) error { + return saveStoredValues(st, path, cfg, "", values) +} + // DropRetiredSettings rewrites stored settings that still carry settings of a // removed feature. Ask why kept an OpenRouter key there; nothing reads it, so // it must not stay in state.db and every backup. Device Support repositories, @@ -241,6 +247,10 @@ func DropRetiredSettings(st *state.Store, path string, cfg *Config) ([]string, e } func saveStored(st *state.Store, path string, cfg *Config, sourceHash string) error { + return saveStoredValues(st, path, cfg, sourceHash, nil) +} + +func saveStoredValues(st *state.Store, path string, cfg *Config, sourceHash string, values map[string]string) error { if cfg.ConfigDatabase == "" { return errors.New("settings database is not initialized") } @@ -303,6 +313,12 @@ func saveStored(st *state.Store, path string, cfg *Config, sourceHash string) er } credentials["ev_charger_password"] = password credentials["lan_auth_password"] = cfg.LANPasswordHash + for key, value := range values { + if _, credential := credentials[key]; credential || key == "settings/config_v1" { + return errors.New("companion settings record conflicts with configuration or credentials") + } + credentials[key] = value + } revision, err := st.SaveConfiguration(raw, cfg.Revision, credentials) if err != nil { return err diff --git a/go/internal/onboarding/draft.go b/go/internal/onboarding/draft.go index 00c22cf07..5e627ef14 100644 --- a/go/internal/onboarding/draft.go +++ b/go/internal/onboarding/draft.go @@ -25,7 +25,7 @@ var ErrConflict = errors.New("setup changed; read the current draft before editi // Storage is the box's durable settings store. A failed write must leave // the previous value intact. type Storage interface { - LoadConfig(string) (string, bool) + ConfigValue(string) (string, bool, error) SaveConfig(string, string) error } @@ -42,17 +42,27 @@ type Draft struct { // Service serializes draft changes. Live config saves remain the caller's // responsibility, including a final compare-and-save of BaseRevision. type Service struct { - mu sync.Mutex - store Storage + mu sync.Mutex + store Storage + preserveSecrets func(incoming, previous *config.Config) } -func New(store Storage) *Service { return &Service{store: store} } +func New(store Storage) *Service { return NewWithSecrets(store, nil) } + +// NewWithSecrets adds the running driver's secret metadata to the common +// config secret handling. The callback must be safe to call during an edit. +func NewWithSecrets(store Storage, preserve func(incoming, previous *config.Config)) *Service { + return &Service{store: store, preserveSecrets: preserve} +} func (s *Service) current() (Draft, bool, error) { if s.store == nil { return Draft{}, false, errors.New("setup storage is unavailable") } - raw, ok := s.store.LoadConfig(draftKey) + raw, ok, err := s.store.ConfigValue(draftKey) + if err != nil { + return Draft{}, false, fmt.Errorf("read setup draft: %w", err) + } if !ok || raw == "" { return Draft{}, false, nil } @@ -126,6 +136,14 @@ func (s *Service) Edit(live *config.Config, expected int64, patch json.RawMessag if err != nil { return Draft{}, err } + previous, err := decodeConfig(d.Config) + if err != nil { + return Draft{}, err + } + cfg.PreserveMaskedSecrets(previous) + if s.preserveSecrets != nil { + s.preserveSecrets(cfg, previous) + } // Canonicalize through the current type. Do not default or validate yet: // both would hide missing answers or reject an unfinished installation. d.Config, err = json.Marshal(cfg) @@ -149,6 +167,22 @@ func (s *Service) Edit(live *config.Config, expected int64, patch json.RawMessag func (s *Service) Checked(liveRevision, revision int64, sha string) (*config.Config, error) { s.mu.Lock() defer s.mu.Unlock() + return s.checkedLocked(liveRevision, revision, sha) +} + +// ApplyChecked keeps the draft fixed while the caller compares and saves +// its live base. The callback must enforce the live configuration revision. +func (s *Service) ApplyChecked(liveRevision, revision int64, sha string, apply func(*config.Config) error) error { + s.mu.Lock() + defer s.mu.Unlock() + cfg, err := s.checkedLocked(liveRevision, revision, sha) + if err != nil { + return err + } + return apply(cfg) +} + +func (s *Service) checkedLocked(liveRevision, revision int64, sha string) (*config.Config, error) { d, found, err := s.current() if err != nil { return nil, err diff --git a/go/internal/onboarding/draft_test.go b/go/internal/onboarding/draft_test.go index 2d07c984f..c7127b0af 100644 --- a/go/internal/onboarding/draft_test.go +++ b/go/internal/onboarding/draft_test.go @@ -193,6 +193,21 @@ func TestDamagedDraftIsNotSilentlyReplaced(t *testing.T) { } } +func TestUnavailableStorageDoesNotLookLikeANewInstallation(t *testing.T) { + live := testLive(t) + store := draftStore(t) + s := New(store) + if err := store.Close(); err != nil { + t.Fatal(err) + } + if _, err := s.Read(live); err == nil { + t.Fatal("closed storage looked like an empty draft") + } + if _, err := s.Receipt(); err == nil { + t.Fatal("closed storage looked like an uncommitted draft") + } +} + func TestSchemaCoversAllPublicConfigFieldsWithoutRuntimeAuthority(t *testing.T) { live := testLive(t) raw, err := json.Marshal(live) diff --git a/go/internal/onboarding/receipt.go b/go/internal/onboarding/receipt.go new file mode 100644 index 000000000..8689e2a9d --- /dev/null +++ b/go/internal/onboarding/receipt.go @@ -0,0 +1,37 @@ +package onboarding + +import ( + "encoding/json" + "errors" +) + +// CommitKey names the record committed atomically with live Settings. +const CommitKey = "onboarding/commit_v1" + +// Receipt proves that the named draft reached durable Settings. Whether +// the running process applied it and hardware works are separate facts. +type Receipt struct { + DraftRevision int64 `json:"draft_revision"` + SHA256 string `json:"sha256"` + BaseRevision int64 `json:"base_revision"` + SavedRevision int64 `json:"saved_revision"` + RestartReasons []string `json:"restart_reasons"` +} + +func (s *Service) Receipt() (*Receipt, error) { + if s.store == nil { + return nil, errors.New("setup storage is unavailable") + } + raw, found, err := s.store.ConfigValue(CommitKey) + if err != nil { + return nil, err + } + if !found || raw == "" { + return nil, nil + } + var receipt Receipt + if len(raw) > MaxDraftBytes || json.Unmarshal([]byte(raw), &receipt) != nil || receipt.DraftRevision < 1 || receipt.SavedRevision != receipt.BaseRevision+1 || len(receipt.SHA256) != 64 { + return nil, errors.New("stored setup receipt is invalid") + } + return &receipt, nil +} From 6c40df76076ddad773f24cd7699c352be95e04d3 Mon Sep 17 00:00:00 2001 From: Fredrik Ahlgren Date: Sun, 18 Oct 2026 17:19:57 +0200 Subject: [PATCH 3/6] Keep draft credentials bound and pending restarts visible --- go/internal/api/api.go | 12 +-- go/internal/api/api_setup.go | 74 ++++++++++++++-- go/internal/api/api_setup_test.go | 110 ++++++++++++++++++++++++ go/internal/config/draft_credentials.go | 7 ++ go/internal/onboarding/draft.go | 6 +- go/internal/onboarding/receipt.go | 1 + 6 files changed, 196 insertions(+), 14 deletions(-) create mode 100644 go/internal/config/draft_credentials.go diff --git a/go/internal/api/api.go b/go/internal/api/api.go index 3413f353b..00ef191ee 100644 --- a/go/internal/api/api.go +++ b/go/internal/api/api.go @@ -330,9 +330,7 @@ func New(deps *Deps) *Server { drafts: newDriverDrafts(), marks: make(map[string]routeMark), } - s.setup = onboarding.NewWithSecrets(deps.State, func(incoming, previous *config.Config) { - restoreDriverConfigSecrets(incoming, previous, s.driverSecretKeys()) - }) + s.setup = onboarding.NewWithSecrets(deps.State, s.preserveSetupSecrets) if deps.SaveConfigWithValues == nil && deps.State != nil { deps.SaveConfigWithValues = func(path string, cfg *config.Config, values map[string]string) error { return config.SaveStoredWithValues(deps.State, path, cfg, values) @@ -1627,10 +1625,10 @@ func restoreDriverConfigSecrets(incoming, existing *config.Config, secretsByLua } func (s *Server) handlePostConfig(w http.ResponseWriter, r *http.Request) { - s.handlePostConfigWith(w, r, s.deps.SaveConfig) + s.handlePostConfigWith(w, r, s.deps.SaveConfig, nil) } -func (s *Server) handlePostConfigWith(w http.ResponseWriter, r *http.Request, save func(string, *config.Config) error) { +func (s *Server) handlePostConfigWith(w http.ResponseWriter, r *http.Request, save func(string, *config.Config) error, saved func(*config.Config, []string)) { s.configWriteMu.Lock() defer s.configWriteMu.Unlock() var posted struct { @@ -1705,6 +1703,10 @@ func (s *Server) handlePostConfigWith(w http.ResponseWriter, r *http.Request, sa if restartReasons == nil { restartReasons = []string{} } + if saved != nil { + saved(&newCfg, restartReasons) + return + } writeJSON(w, 200, map[string]any{ "status": "ok", "restart_required": len(restartReasons) > 0, diff --git a/go/internal/api/api_setup.go b/go/internal/api/api_setup.go index 861d33b69..02b428121 100644 --- a/go/internal/api/api_setup.go +++ b/go/internal/api/api_setup.go @@ -7,16 +7,49 @@ import ( "io" "net/http" "path/filepath" + "slices" + "github.com/google/uuid" "github.com/srcfl/ftw/go/internal/config" "github.com/srcfl/ftw/go/internal/onboarding" ) +// One identity per process, rather than per API server, so rebuilding the +// HTTP handler cannot claim that restart-only settings became active. +var setupProcessID = uuid.NewString() + +func setupRestartPending(receipt *onboarding.Receipt) bool { + return receipt != nil && len(receipt.RestartReasons) > 0 && (receipt.ProcessID == "" || receipt.ProcessID == setupProcessID) +} + +func (s *Server) preserveSetupSecrets(incoming, previous *config.Config) { + // Bind each driver's secrets before applying any display-name fallback. + // Separate pairs also handle two drivers swapping their names in a draft. + withoutDrivers := *previous + withoutDrivers.Drivers = nil + incoming.PreserveMaskedSecrets(&withoutDrivers) + metadata := s.driverSecretKeys() + for i, driver := range incoming.Drivers { + for _, old := range previous.Drivers { + if driver.Lua != old.Lua || (driver.CredentialOwner != "" && driver.CredentialOwner != old.CredentialOwner) || + (driver.CredentialOwner == "" && driver.Name != old.Name) { + continue + } + old.Name = driver.Name + next, before := config.Config{Drivers: []config.Driver{driver}}, config.Config{Drivers: []config.Driver{old}} + next.PreserveMaskedSecrets(&before) + restoreDriverConfigSecrets(&next, &before, metadata) + incoming.Drivers[i] = next.Drivers[0] + break + } + } +} + func (s *Server) setupRoutes() { s.handle("GET /api/setup/schema", Read, s.handleSetupSchema) s.handle("GET /api/setup/draft", Local, s.handleSetupDraft) s.handle("POST /api/setup/draft", Configure, s.handleSetupEdit) - s.handle("POST /api/setup/restart", Configure, s.handleSetupRestart) + s.handle("POST /api/setup/reset", Configure, s.handleSetupReset) s.handle("POST /api/setup/validate", Configure, s.handleSetupValidate) s.handle("POST /api/setup/commit", Configure, s.handleSetupCommit) } @@ -96,14 +129,20 @@ func (s *Server) writeSetupDraft(w http.ResponseWriter, live *config.Config, d o return } saved := receipt != nil && receipt.DraftRevision == d.Revision && receipt.SHA256 == d.SHA256 - applied := saved && receipt.SavedRevision == live.Revision + restartPending := saved && setupRestartPending(receipt) + applied := saved && receipt.SavedRevision == live.Revision && !restartPending + restartReasons := config.RestartRequiredFor(live, &candidate) + if saved { + restartReasons = receipt.RestartReasons + } w.Header().Set("Cache-Control", "no-store") writeJSON(w, http.StatusOK, map[string]any{ "revision": d.Revision, "base_revision": d.BaseRevision, "live_revision": live.Revision, "sha256": d.SHA256, "config": masked, "stale": !saved && d.BaseRevision != live.Revision, "valid": validation == "", "validation_error": validation, "saved": saved, "applied": applied, "hardware_verified": false, "last_commit": receipt, - "restart_reasons": config.RestartRequiredFor(live, &candidate), + "restart_pending": restartPending, "restart_reasons": restartReasons, + "next_step": "Continue this draft, or create a new draft from current settings after a completed save.", }) } @@ -145,7 +184,7 @@ func (s *Server) handleSetupEdit(w http.ResponseWriter, r *http.Request) { s.writeSetupDraft(w, live, d) } -func (s *Server) handleSetupRestart(w http.ResponseWriter, r *http.Request) { +func (s *Server) handleSetupReset(w http.ResponseWriter, r *http.Request) { var q struct { Revision int64 `json:"revision"` } @@ -214,9 +253,7 @@ func (s *Server) handleSetupCommit(w http.ResponseWriter, r *http.Request) { return } if previous != nil && previous.DraftRevision == q.Revision && previous.SHA256 == q.SHA256 && previous.BaseRevision == q.BaseRevision { - writeJSON(w, http.StatusOK, map[string]any{"status": "ok", "saved": true, - "applied": live.Revision == previous.SavedRevision, "live_revision": live.Revision, - "later_changes": live.Revision > previous.SavedRevision, "commit": previous, "hardware_verified": false}) + s.writeSetupCommit(w, live.Revision, previous) return } if !q.Confirmed { @@ -243,15 +280,26 @@ func (s *Server) handleSetupCommit(w http.ResponseWriter, r *http.Request) { request.ContentLength = int64(len(raw)) request.Header = r.Header.Clone() request.Header.Set("If-Match", configETag(q.BaseRevision)) - receipt := onboarding.Receipt{DraftRevision: q.Revision, SHA256: q.SHA256, + receipt := onboarding.Receipt{ProcessID: setupProcessID, DraftRevision: q.Revision, SHA256: q.SHA256, BaseRevision: q.BaseRevision, SavedRevision: q.BaseRevision + 1, RestartReasons: config.RestartRequiredFor(live, candidate)} + // A later live edit does not rebind listeners or rebuild clients left + // pending by an earlier setup commit in this process. + if setupRestartPending(previous) { + for _, reason := range previous.RestartReasons { + if !slices.Contains(receipt.RestartReasons, reason) { + receipt.RestartReasons = append(receipt.RestartReasons, reason) + } + } + } record, err := json.Marshal(receipt) if err != nil { return err } s.handlePostConfigWith(w, request, func(path string, cfg *config.Config) error { return s.deps.SaveConfigWithValues(path, cfg, map[string]string{onboarding.CommitKey: string(record)}) + }, func(cfg *config.Config, _ []string) { + s.writeSetupCommit(w, cfg.Revision, &receipt) }) return nil }) @@ -259,3 +307,13 @@ func (s *Server) handleSetupCommit(w http.ResponseWriter, r *http.Request) { s.setupError(w, err) } } + +func (s *Server) writeSetupCommit(w http.ResponseWriter, liveRevision int64, receipt *onboarding.Receipt) { + pending := setupRestartPending(receipt) + w.Header().Set("Cache-Control", "no-store") + writeJSON(w, http.StatusOK, map[string]any{ + "status": "ok", "saved": true, "applied": liveRevision == receipt.SavedRevision && !pending, + "live_revision": liveRevision, "restart_required": pending, "restart_reasons": receipt.RestartReasons, + "later_changes": liveRevision > receipt.SavedRevision, "commit": receipt, "hardware_verified": false, + }) +} diff --git a/go/internal/api/api_setup_test.go b/go/internal/api/api_setup_test.go index 22c6add00..0316ba92e 100644 --- a/go/internal/api/api_setup_test.go +++ b/go/internal/api/api_setup_test.go @@ -177,3 +177,113 @@ func TestSetupReceiptAndSettingsCommitTogether(t *testing.T) { t.Fatal("receipt names a different settings revision") } } + +func TestSetupNewDraftDriverRenamePreservesCredential(t *testing.T) { + s, _ := setupTestServer(t) + first := map[string]any{"name": "first meter", "lua": "drivers/test.lua", "is_site_meter": true, + "capabilities": map[string]any{"standalone": true}, "config": map[string]any{"api_token": "draft-only-token"}} + w, draft := setupCall(t, s, http.MethodPost, "/api/setup/draft", map[string]any{"revision": 0, "patch": map[string]any{"drivers": []any{first}}}) + if w.Code != http.StatusOK { + t.Fatal(w.Body.String()) + } + drivers := draft["config"].(map[string]any)["drivers"].([]any) + drivers[0].(map[string]any)["name"] = "Garage meter" + w, draft = setupCall(t, s, http.MethodPost, "/api/setup/draft", map[string]any{"revision": draft["revision"], "patch": map[string]any{"drivers": drivers}}) + if w.Code != http.StatusOK { + t.Fatal(w.Body.String()) + } + candidate, err := s.setup.Checked(int64(draft["base_revision"].(float64)), int64(draft["revision"].(float64)), draft["sha256"].(string)) + if err != nil { + t.Fatal(err) + } + if candidate.Drivers[0].Config["api_token"] != "draft-only-token" { + t.Fatal("renaming the new driver lost its draft-only credential") + } +} + +func TestSetupResumeKeepsPendingRestartVisible(t *testing.T) { + s, _ := setupTestServer(t) + w, draft := setupCall(t, s, http.MethodPost, "/api/setup/draft", map[string]any{"revision": 0, "patch": map[string]any{"api": map[string]any{"port": 8899}}}) + if w.Code != http.StatusOK { + t.Fatal(w.Body.String()) + } + check := map[string]any{"revision": draft["revision"], "base_revision": draft["base_revision"], "sha256": draft["sha256"], "confirmed": true} + w, commit := setupCall(t, s, http.MethodPost, "/api/setup/commit", check) + if w.Code != http.StatusOK || commit["restart_required"] != true { + t.Fatal("initial response did not identify pending restart") + } + w, resumed := setupCall(t, s, http.MethodGet, "/api/setup/draft", nil) + if w.Code != http.StatusOK { + t.Fatal(w.Body.String()) + } + reasons, ok := resumed["restart_reasons"].([]any) + if !ok || len(reasons) == 0 { + t.Fatal("resume hid the pending listener restart while returning applied=true") + } + if resumed["restart_pending"] != true || resumed["applied"] != false { + t.Fatal("resume claimed restart-only settings were active", w.Body.String()) + } + w, retried := setupCall(t, New(s.deps), http.MethodPost, "/api/setup/commit", check) + if w.Code != http.StatusOK || retried["restart_required"] != true || retried["applied"] != false { + t.Fatal("retry hid the pending restart", w.Body.String()) + } + if commit["saved"] != true || commit["commit"] == nil || retried["commit"] == nil { + t.Fatal("initial reply and retry must both return a durable receipt") + } +} + +func TestSetupSwappedDriverNamesKeepOwnPasswords(t *testing.T) { + s, _ := setupTestServer(t) + a := map[string]any{"name": "meter A", "lua": "drivers/test.lua", "is_site_meter": true, + "capabilities": map[string]any{"standalone": true}, "config": map[string]any{"password": "password-A"}} + b := map[string]any{"name": "device B", "lua": "drivers/test.lua", + "capabilities": map[string]any{"standalone": true}, "config": map[string]any{"password": "password-B"}} + w, draft := setupCall(t, s, http.MethodPost, "/api/setup/draft", map[string]any{"revision": 0, "patch": map[string]any{"drivers": []any{a, b}}}) + if w.Code != http.StatusOK { + t.Fatal(w.Body.String()) + } + drivers := draft["config"].(map[string]any)["drivers"].([]any) + drivers[0].(map[string]any)["name"] = "device B" + drivers[1].(map[string]any)["name"] = "meter A" + w, draft = setupCall(t, s, http.MethodPost, "/api/setup/draft", map[string]any{"revision": draft["revision"], "patch": map[string]any{"drivers": drivers}}) + if w.Code != http.StatusOK { + t.Fatal(w.Body.String()) + } + candidate, err := s.setup.Checked(int64(draft["base_revision"].(float64)), int64(draft["revision"].(float64)), draft["sha256"].(string)) + if err != nil { + t.Fatal(err) + } + if candidate.Drivers[0].Config["password"] != "password-A" || candidate.Drivers[1].Config["password"] != "password-B" { + t.Fatal("swapping display names crossed the drivers' credentials") + } +} + +func TestSetupLaterSetupSaveKeepsEarlierPendingRestart(t *testing.T) { + s, _ := setupTestServer(t) + w, draft := setupCall(t, s, http.MethodPost, "/api/setup/draft", map[string]any{"revision": 0, "patch": map[string]any{"api": map[string]any{"port": 8899}}}) + if w.Code != http.StatusOK { + t.Fatal(w.Body.String()) + } + check := map[string]any{"revision": draft["revision"], "base_revision": draft["base_revision"], "sha256": draft["sha256"], "confirmed": true} + w, result := setupCall(t, s, http.MethodPost, "/api/setup/commit", check) + if w.Code != http.StatusOK || result["restart_required"] != true { + t.Fatal("first save had no pending restart") + } + w, draft = setupCall(t, s, http.MethodPost, "/api/setup/reset", map[string]any{"revision": draft["revision"]}) + if w.Code != http.StatusOK { + t.Fatal(w.Body.String()) + } + w, draft = setupCall(t, s, http.MethodPost, "/api/setup/draft", map[string]any{"revision": draft["revision"], "patch": map[string]any{"site": map[string]any{"name": "New display name"}}}) + if w.Code != http.StatusOK { + t.Fatal(w.Body.String()) + } + check = map[string]any{"revision": draft["revision"], "base_revision": draft["base_revision"], "sha256": draft["sha256"], "confirmed": true} + w, result = setupCall(t, s, http.MethodPost, "/api/setup/commit", check) + if w.Code != http.StatusOK || result["restart_required"] != true || result["applied"] != false { + t.Fatal("a later unrelated save forgot the pending runtime change") + } + w, result = setupCall(t, s, http.MethodGet, "/api/setup/draft", nil) + if w.Code != http.StatusOK || result["restart_pending"] != true || result["applied"] != false { + t.Fatal("resume forgot the pending restart after the second save") + } +} diff --git a/go/internal/config/draft_credentials.go b/go/internal/config/draft_credentials.go new file mode 100644 index 000000000..148e2655c --- /dev/null +++ b/go/internal/config/draft_credentials.go @@ -0,0 +1,7 @@ +package config + +// PrepareDraftCredentials gives new drivers the same stable credential +// identity used by Settings, before masked drafts can be renamed or reordered. +func PrepareDraftCredentials(cfg, previous *Config) error { + return assignCredentialOwners(cfg, previous) +} diff --git a/go/internal/onboarding/draft.go b/go/internal/onboarding/draft.go index 5e627ef14..6ab69e26f 100644 --- a/go/internal/onboarding/draft.go +++ b/go/internal/onboarding/draft.go @@ -140,9 +140,13 @@ func (s *Service) Edit(live *config.Config, expected int64, patch json.RawMessag if err != nil { return Draft{}, err } - cfg.PreserveMaskedSecrets(previous) if s.preserveSecrets != nil { s.preserveSecrets(cfg, previous) + } else { + cfg.PreserveMaskedSecrets(previous) + } + if err := config.PrepareDraftCredentials(cfg, previous); err != nil { + return Draft{}, err } // Canonicalize through the current type. Do not default or validate yet: // both would hide missing answers or reject an unfinished installation. diff --git a/go/internal/onboarding/receipt.go b/go/internal/onboarding/receipt.go index 8689e2a9d..fd8663385 100644 --- a/go/internal/onboarding/receipt.go +++ b/go/internal/onboarding/receipt.go @@ -11,6 +11,7 @@ const CommitKey = "onboarding/commit_v1" // Receipt proves that the named draft reached durable Settings. Whether // the running process applied it and hardware works are separate facts. type Receipt struct { + ProcessID string `json:"process_id"` DraftRevision int64 `json:"draft_revision"` SHA256 string `json:"sha256"` BaseRevision int64 `json:"base_revision"` From 38a2d86d08385043a1cf501e443c7d3b8bdbe2b0 Mon Sep 17 00:00:00 2001 From: Fredrik Ahlgren Date: Sun, 18 Oct 2026 17:21:46 +0200 Subject: [PATCH 4/6] Keep house-password changes on their dedicated path --- go/internal/api/api_setup_test.go | 10 ++++++++++ go/internal/onboarding/draft.go | 3 +++ go/internal/onboarding/schema.go | 7 ++++++- 3 files changed, 19 insertions(+), 1 deletion(-) diff --git a/go/internal/api/api_setup_test.go b/go/internal/api/api_setup_test.go index 0316ba92e..77f29ec2d 100644 --- a/go/internal/api/api_setup_test.go +++ b/go/internal/api/api_setup_test.go @@ -113,6 +113,16 @@ func TestSetupDraftDoesNotClaimCommissioningFromAValidConfig(t *testing.T) { } } +func TestSetupDoesNotPretendToChangeHousePasswordProtection(t *testing.T) { + s, cfg := setupTestServer(t) + w, _ := setupCall(t, s, http.MethodPost, "/api/setup/draft", map[string]any{ + "revision": 0, "patch": map[string]any{"api": map[string]any{"lan_auth": !cfg.API.LANAuth}}, + }) + if w.Code != http.StatusUnprocessableEntity || !strings.Contains(w.Body.String(), "house-password") { + t.Fatal("draft accepted a password flag Settings would ignore", w.Body.String()) + } +} + func TestSetupFailedSaveLeavesLiveAndStoredConfigUntouched(t *testing.T) { s, cfg := setupTestServer(t) w, draft := setupCall(t, s, http.MethodPost, "/api/setup/draft", map[string]any{"revision": 0, "patch": map[string]any{"site": map[string]any{"name": "Unsaved"}}}) diff --git a/go/internal/onboarding/draft.go b/go/internal/onboarding/draft.go index 6ab69e26f..24bb9342a 100644 --- a/go/internal/onboarding/draft.go +++ b/go/internal/onboarding/draft.go @@ -140,6 +140,9 @@ func (s *Service) Edit(live *config.Config, expected int64, patch json.RawMessag if err != nil { return Draft{}, err } + if cfg.API.LANAuth != previous.API.LANAuth { + return Draft{}, errors.New("api.lan_auth changes through the house-password action on the box, not a setup draft") + } if s.preserveSecrets != nil { s.preserveSecrets(cfg, previous) } else { diff --git a/go/internal/onboarding/schema.go b/go/internal/onboarding/schema.go index aa4374292..bfb96aa59 100644 --- a/go/internal/onboarding/schema.go +++ b/go/internal/onboarding/schema.go @@ -36,7 +36,12 @@ func schemaFor(t reflect.Type) map[string]any { if name == "" { name = f.Name } - properties[name] = schemaFor(f.Type) + field := schemaFor(f.Type) + if t == reflect.TypeFor[config.API]() && name == "lan_auth" { + field["readOnly"] = true + field["description"] = "Read current house-password protection. Change it through /api/auth/password on the box, not a setup draft." + } + properties[name] = field } return map[string]any{"type": "object", "properties": properties, "additionalProperties": false} case reflect.Map: From ab0f0ee5f7dda94aec5af3e557a45c2fec9132e8 Mon Sep 17 00:00:00 2001 From: Fredrik Ahlgren Date: Sun, 18 Oct 2026 17:49:30 +0200 Subject: [PATCH 5/6] Keep a pending process restart visible across new setup drafts --- go/internal/api/api_setup.go | 8 +++++++- go/internal/api/api_setup_test.go | 24 ++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/go/internal/api/api_setup.go b/go/internal/api/api_setup.go index 02b428121..d64939438 100644 --- a/go/internal/api/api_setup.go +++ b/go/internal/api/api_setup.go @@ -129,11 +129,17 @@ func (s *Server) writeSetupDraft(w http.ResponseWriter, live *config.Config, d o return } saved := receipt != nil && receipt.DraftRevision == d.Revision && receipt.SHA256 == d.SHA256 - restartPending := saved && setupRestartPending(receipt) + restartPending := setupRestartPending(receipt) applied := saved && receipt.SavedRevision == live.Revision && !restartPending restartReasons := config.RestartRequiredFor(live, &candidate) if saved { restartReasons = receipt.RestartReasons + } else if restartPending { + for _, reason := range receipt.RestartReasons { + if !slices.Contains(restartReasons, reason) { + restartReasons = append(restartReasons, reason) + } + } } w.Header().Set("Cache-Control", "no-store") writeJSON(w, http.StatusOK, map[string]any{ diff --git a/go/internal/api/api_setup_test.go b/go/internal/api/api_setup_test.go index 77f29ec2d..c1ec9b679 100644 --- a/go/internal/api/api_setup_test.go +++ b/go/internal/api/api_setup_test.go @@ -242,6 +242,30 @@ func TestSetupResumeKeepsPendingRestartVisible(t *testing.T) { } } +func TestSetupNewDraftKeepsTheProcessRestartPending(t *testing.T) { + s, _ := setupTestServer(t) + w, draft := setupCall(t, s, http.MethodPost, "/api/setup/draft", map[string]any{"revision": 0, "patch": map[string]any{"api": map[string]any{"port": 8899}}}) + if w.Code != http.StatusOK { + t.Fatal(w.Body.String()) + } + w, _ = setupCall(t, s, http.MethodPost, "/api/setup/commit", map[string]any{"revision": draft["revision"], "base_revision": draft["base_revision"], "sha256": draft["sha256"], "confirmed": true}) + if w.Code != http.StatusOK { + t.Fatal(w.Body.String()) + } + w, draft = setupCall(t, s, http.MethodPost, "/api/setup/reset", map[string]any{"revision": draft["revision"]}) + if w.Code != http.StatusOK || draft["saved"] != false || draft["restart_pending"] != true || len(draft["restart_reasons"].([]any)) == 0 { + t.Fatal("new draft hid the pending restart", w.Body.String()) + } + w, draft = setupCall(t, s, http.MethodPost, "/api/setup/draft", map[string]any{"revision": draft["revision"], "patch": map[string]any{"site": map[string]any{"name": "Next change"}}}) + if w.Code != http.StatusOK || draft["restart_pending"] != true { + t.Fatal(w.Body.String()) + } + w, resumed := setupCall(t, New(s.deps), http.MethodGet, "/api/setup/draft", nil) + if w.Code != http.StatusOK || resumed["restart_pending"] != true || len(resumed["restart_reasons"].([]any)) == 0 { + t.Fatal("resume hid process state", w.Body.String()) + } +} + func TestSetupSwappedDriverNamesKeepOwnPasswords(t *testing.T) { s, _ := setupTestServer(t) a := map[string]any{"name": "meter A", "lua": "drivers/test.lua", "is_site_meter": true, From 00fba8896f76c8dd311ca8da4f4fcfde3462ebd5 Mon Sep 17 00:00:00 2001 From: Fredrik Ahlgren Date: Sun, 18 Oct 2026 19:55:47 +0200 Subject: [PATCH 6/6] Bind setup confirmation to the exact saved configuration --- .changeset/agent-setup-drafts.md | 2 + go/cmd/ftw/solar_feed_send.go | 2 + go/internal/api/api.go | 81 ++++-- go/internal/api/api_pv_manual.go | 2 + go/internal/api/api_setup.go | 57 ++-- go/internal/api/api_setup_integrity_test.go | 269 +++++++++++++++++++ go/internal/api/api_setup_test.go | 2 +- go/internal/config/json_numbers.go | 106 ++++++++ go/internal/config/json_numbers_test.go | 41 +++ go/internal/config/storage.go | 47 +++- go/internal/drivers/registry.go | 2 + go/internal/onboarding/draft.go | 152 +++++++++-- go/internal/onboarding/normalization_test.go | 144 ++++++++++ 13 files changed, 843 insertions(+), 64 deletions(-) create mode 100644 go/internal/api/api_setup_integrity_test.go create mode 100644 go/internal/config/json_numbers.go create mode 100644 go/internal/config/json_numbers_test.go create mode 100644 go/internal/onboarding/normalization_test.go diff --git a/.changeset/agent-setup-drafts.md b/.changeset/agent-setup-drafts.md index c3549f8b0..3772cd0f3 100644 --- a/.changeset/agent-setup-drafts.md +++ b/.changeset/agent-setup-drafts.md @@ -3,3 +3,5 @@ --- Add a complete configuration schema and durable setup drafts. Draft edits preserve unedited settings and credentials; validation and a confirmed commit use the current Settings revision and save path. This prepares setup through MCP; first-run pairing, MCP writes and code sharing remain separate work. + +Validation returns normalized settings as a new draft revision for review. The final save checks those exact portable settings before writing to SQLite, including credential bindings. Drafts, Settings saves and reloads preserve exact integer driver identifiers; numbers that would change are rejected with a field path. diff --git a/go/cmd/ftw/solar_feed_send.go b/go/cmd/ftw/solar_feed_send.go index 2da54d6f3..9949b1d68 100644 --- a/go/cmd/ftw/solar_feed_send.go +++ b/go/cmd/ftw/solar_feed_send.go @@ -57,6 +57,8 @@ func configNumber(v any) float64 { return float64(n) case int64: return float64(n) + case uint64: + return float64(n) case float64: return n } diff --git a/go/internal/api/api.go b/go/internal/api/api.go index 00ef191ee..ed2c327f5 100644 --- a/go/internal/api/api.go +++ b/go/internal/api/api.go @@ -130,7 +130,7 @@ type Deps struct { SelfTune *selftune.Coordinator DtS float64 // control interval seconds (for model τ / age displays) SaveConfig func(path string, c *config.Config) error // injection for testability - SaveConfigWithValues func(path string, c *config.Config, values map[string]string) error + SaveConfigWithValues func(path string, c *config.Config, values map[string]string, reviewedJSON []byte) error // ConfigApplier is main.go's config-applied callback — the same // closure the configreload watcher runs (registry reload with SoC // bounds, capacities, inverter groups, fuse and mpc/loadmodel @@ -330,10 +330,10 @@ func New(deps *Deps) *Server { drafts: newDriverDrafts(), marks: make(map[string]routeMark), } - s.setup = onboarding.NewWithSecrets(deps.State, s.preserveSetupSecrets) + s.setup = onboarding.NewWithSecrets(deps.State, s.preserveSetupSecrets, filepath.Dir(deps.ConfigPath)) if deps.SaveConfigWithValues == nil && deps.State != nil { - deps.SaveConfigWithValues = func(path string, cfg *config.Config, values map[string]string) error { - return config.SaveStoredWithValues(deps.State, path, cfg, values) + deps.SaveConfigWithValues = func(path string, cfg *config.Config, values map[string]string, reviewedJSON []byte) error { + return config.SaveStoredReviewedWithValues(deps.State, path, cfg, values, reviewedJSON) } } if deps.Registry != nil { @@ -703,15 +703,41 @@ func writeJSON(w http.ResponseWriter, status int, v any) { const maxJSONBody = 1 << 20 func readJSON(r *http.Request, v any) error { + body, err := readJSONBody(r) + if err != nil { + return err + } + return json.Unmarshal(body, v) +} + +// Settings contain opaque vendor identifiers. Other API payloads keep their +// existing decoder and numeric types. +func readSettingsJSON(r *http.Request, v any) error { + body, err := readJSONBody(r) + if err != nil { + return err + } + dec := json.NewDecoder(bytes.NewReader(body)) + dec.UseNumber() + if err := dec.Decode(v); err != nil { + return err + } + if err := dec.Decode(new(any)); err != io.EOF { + return errors.New("send one settings object") + } + return nil +} + +func readJSONBody(r *http.Request) ([]byte, error) { defer r.Body.Close() body, err := io.ReadAll(http.MaxBytesReader(nil, r.Body, maxJSONBody)) if err != nil { - return err + return nil, err } if len(body) == 0 { - return errors.New("empty body") + return nil, errors.New("empty body") } - return json.Unmarshal(body, v) + return body, nil } // ---- /api/health ---- @@ -1625,21 +1651,25 @@ func restoreDriverConfigSecrets(incoming, existing *config.Config, secretsByLua } func (s *Server) handlePostConfig(w http.ResponseWriter, r *http.Request) { - s.handlePostConfigWith(w, r, s.deps.SaveConfig, nil) + s.handlePostConfigWith(w, r, false, s.deps.SaveConfig, nil) } -func (s *Server) handlePostConfigWith(w http.ResponseWriter, r *http.Request, save func(string, *config.Config) error, saved func(*config.Config, []string)) { +func (s *Server) handlePostConfigWith(w http.ResponseWriter, r *http.Request, reviewed bool, save func(string, *config.Config) error, saved func(*config.Config, []string)) { s.configWriteMu.Lock() defer s.configWriteMu.Unlock() var posted struct { config.Config AppLink json.RawMessage `json:"app_link"` } - if err := readJSON(r, &posted); err != nil { + if err := readSettingsJSON(r, &posted); err != nil { writeJSON(w, 400, map[string]string{"error": "invalid config: " + err.Error()}) return } newCfg := posted.Config + if err := newCfg.DecodeDriverNumbers(); err != nil { + writeJSON(w, 400, map[string]string{"error": "invalid config: " + err.Error()}) + return + } if posted.AppLink != nil { if bytes.Equal(bytes.TrimSpace(posted.AppLink), []byte("null")) { newCfg.AppLink = &config.AppLink{Enabled: false} @@ -1648,7 +1678,7 @@ func (s *Server) handlePostConfigWith(w http.ResponseWriter, r *http.Request, sa return } } - if newCfg.EVCharger != nil && newCfg.EVCharger.Password == maskedPlaceholder { + if !reviewed && newCfg.EVCharger != nil && newCfg.EVCharger.Password == maskedPlaceholder { newCfg.EVCharger.Password = "" } // Preserve secrets the UI sent back as empty (masked) values. @@ -1658,13 +1688,22 @@ func (s *Server) handlePostConfigWith(w http.ResponseWriter, r *http.Request, sa writeJSON(w, http.StatusConflict, map[string]string{"error": "Settings changed. Close and reopen Settings before saving."}) return } - newCfg.PreserveMaskedSecrets(s.deps.Cfg) - // Restore catalog-declared driver secrets (api_token etc.) the UI - // returned as maskedPlaceholder or empty. Same semantics as - // PreserveMaskedSecrets but scoped to keys the driver itself - // declared via DRIVER.config_secrets — keeps config-package - // catalog-agnostic. - restoreDriverConfigSecrets(&newCfg, s.deps.Cfg, s.driverSecretKeys()) + if reviewed { + // Setup already resolved masks against the draft's stable owners. + // Only private runtime state comes from the live configuration here. + newCfg.ConfigDatabase = s.deps.Cfg.ConfigDatabase + newCfg.Revision = s.deps.Cfg.Revision + newCfg.LANPasswordHash = s.deps.Cfg.LANPasswordHash + newCfg.ModbusProxyWriteOwners = s.deps.Cfg.ModbusProxyWriteOwners + } else { + newCfg.PreserveMaskedSecrets(s.deps.Cfg) + // Restore catalog-declared driver secrets (api_token etc.) the UI + // returned as maskedPlaceholder or empty. Same semantics as + // PreserveMaskedSecrets but scoped to keys the driver itself + // declared via DRIVER.config_secrets — keeps config-package + // catalog-agnostic. + restoreDriverConfigSecrets(&newCfg, s.deps.Cfg, s.driverSecretKeys()) + } s.deps.CfgMu.RUnlock() if err := newCfg.Validate(); err != nil { @@ -1687,7 +1726,11 @@ func (s *Server) handlePostConfigWith(w http.ResponseWriter, r *http.Request, sa // Persist atomically (Password has yaml:"-" so it won't appear in YAML) if err := save(s.deps.ConfigPath, &newCfg); err != nil { - writeJSON(w, 500, map[string]string{"error": "save failed: " + err.Error()}) + status := http.StatusInternalServerError + if errors.Is(err, config.ErrReviewedConfigChanged) { + status = http.StatusConflict + } + writeJSON(w, status, map[string]string{"error": "save failed: " + err.Error()}) return } // Apply the committed config through the same path as all Settings writers. diff --git a/go/internal/api/api_pv_manual.go b/go/internal/api/api_pv_manual.go index dedef7ba0..aac27d1f3 100644 --- a/go/internal/api/api_pv_manual.go +++ b/go/internal/api/api_pv_manual.go @@ -236,6 +236,8 @@ func readNominalW(d config.Driver) float64 { return float64(n) case int64: return float64(n) + case uint64: + return float64(n) } return 0 } diff --git a/go/internal/api/api_setup.go b/go/internal/api/api_setup.go index d64939438..5d5a49251 100644 --- a/go/internal/api/api_setup.go +++ b/go/internal/api/api_setup.go @@ -4,6 +4,7 @@ import ( "bytes" "encoding/json" "errors" + "fmt" "io" "net/http" "path/filepath" @@ -22,7 +23,7 @@ func setupRestartPending(receipt *onboarding.Receipt) bool { return receipt != nil && len(receipt.RestartReasons) > 0 && (receipt.ProcessID == "" || receipt.ProcessID == setupProcessID) } -func (s *Server) preserveSetupSecrets(incoming, previous *config.Config) { +func (s *Server) preserveSetupSecrets(incoming, previous *config.Config) error { // Bind each driver's secrets before applying any display-name fallback. // Separate pairs also handle two drivers swapping their names in a draft. withoutDrivers := *previous @@ -43,6 +44,14 @@ func (s *Server) preserveSetupSecrets(incoming, previous *config.Config) { break } } + for _, driver := range incoming.Drivers { + for key, value := range driver.Config { + if text, ok := value.(string); ok && text == maskedPlaceholder { + return fmt.Errorf("driver %q: enter or clear %s after changing the driver; its masked value belongs to the earlier configuration", driver.Name, key) + } + } + } + return nil } func (s *Server) setupRoutes() { @@ -63,6 +72,7 @@ func (s *Server) handleSetupSchema(w http.ResponseWriter, _ *http.Request) { "Patch the current draft revision. Omitted fields and saved credentials remain on the box.", "Arrays replace the whole array. Read and retain all existing entries before editing one.", "Validate the exact draft, show its changes and assumptions, then ask the user to confirm before commit.", + "Validation can normalize fields into a new draft revision and checksum. Review that returned configuration and its warnings; an earlier confirmation cannot save the changed draft.", "Saving configuration does not prove hardware operation. Check fresh measurements and bounded control tests separately.", }, "normal_use": "Use status, history and control tools for everyday questions. Setup is a separate workflow.", @@ -81,7 +91,7 @@ func (s *Server) setupLive() (*config.Config, error) { return nil, err } var cfg config.Config - if err := json.Unmarshal(raw, &cfg); err != nil { + if err := config.DecodeJSON(raw, &cfg); err != nil { return nil, err } cfg.Revision = revision @@ -113,16 +123,30 @@ func (s *Server) handleSetupDraft(w http.ResponseWriter, _ *http.Request) { func (s *Server) writeSetupDraft(w http.ResponseWriter, live *config.Config, d onboarding.Draft) { var candidate config.Config - if err := json.Unmarshal(d.Config, &candidate); err != nil { + if err := config.DecodeJSON(d.Config, &candidate); err != nil { s.setupError(w, err) return } - masked := candidate.MaskSecrets() - maskDriverConfigSecrets(&masked, s.driverSecretKeys()) + var validating config.Config + if err := config.DecodeJSON(d.Config, &validating); err != nil { + s.setupError(w, err) + return + } + validating.ResolveDriverPaths(filepath.Dir(s.deps.ConfigPath)) + validating.UnresolveDriverPaths(filepath.Dir(s.deps.ConfigPath)) validation := "" - if err := candidate.Validate(); err != nil { + if err := config.PrepareDraftCredentials(&validating, live); err != nil { + validation = err.Error() + } else if err := validating.Validate(); err != nil { validation = err.Error() } + masked := candidate.MaskSecrets() + maskDriverConfigSecrets(&masked, s.driverSecretKeys()) + normalizedJSON, err := json.Marshal(&validating) + if err != nil { + s.setupError(w, err) + return + } receipt, err := s.setup.Receipt() if err != nil { s.setupError(w, err) @@ -131,7 +155,7 @@ func (s *Server) writeSetupDraft(w http.ResponseWriter, live *config.Config, d o saved := receipt != nil && receipt.DraftRevision == d.Revision && receipt.SHA256 == d.SHA256 restartPending := setupRestartPending(receipt) applied := saved && receipt.SavedRevision == live.Revision && !restartPending - restartReasons := config.RestartRequiredFor(live, &candidate) + restartReasons := config.RestartRequiredFor(live, &validating) if saved { restartReasons = receipt.RestartReasons } else if restartPending { @@ -144,11 +168,14 @@ func (s *Server) writeSetupDraft(w http.ResponseWriter, live *config.Config, d o w.Header().Set("Cache-Control", "no-store") writeJSON(w, http.StatusOK, map[string]any{ "revision": d.Revision, "base_revision": d.BaseRevision, "live_revision": live.Revision, - "sha256": d.SHA256, "config": masked, "stale": !saved && d.BaseRevision != live.Revision, + "normalized_from_revision": d.NormalizedFromRevision, "validation_warnings": d.ValidationWarnings, + "normalized": d.NormalizedFromRevision > 0, + "sha256": d.SHA256, "config": masked, "stale": !saved && d.BaseRevision != live.Revision, "valid": validation == "", "validation_error": validation, - "saved": saved, "applied": applied, "hardware_verified": false, "last_commit": receipt, + "normalization_required": validation == "" && !bytes.Equal(normalizedJSON, d.Config), + "saved": saved, "applied": applied, "hardware_verified": false, "last_commit": receipt, "restart_pending": restartPending, "restart_reasons": restartReasons, - "next_step": "Continue this draft, or create a new draft from current settings after a completed save.", + "next_step": "Review the returned draft after validation, including a new revision, checksum or normalization warnings. Save only that exact reviewed revision. After a completed save, a new draft starts from current settings.", }) } @@ -231,11 +258,7 @@ func (s *Server) handleSetupValidate(w http.ResponseWriter, r *http.Request) { s.setupError(w, onboarding.ErrConflict) return } - if _, err := s.setup.Checked(live.Revision, q.Revision, q.SHA256); err != nil { - s.setupError(w, err) - return - } - d, err := s.setup.Read(live) + d, err := s.setup.Validate(live, q.Revision, q.SHA256) if err != nil { s.setupError(w, err) return @@ -302,8 +325,8 @@ func (s *Server) handleSetupCommit(w http.ResponseWriter, r *http.Request) { if err != nil { return err } - s.handlePostConfigWith(w, request, func(path string, cfg *config.Config) error { - return s.deps.SaveConfigWithValues(path, cfg, map[string]string{onboarding.CommitKey: string(record)}) + s.handlePostConfigWith(w, request, true, func(path string, cfg *config.Config) error { + return s.deps.SaveConfigWithValues(path, cfg, map[string]string{onboarding.CommitKey: string(record)}, raw) }, func(cfg *config.Config, _ []string) { s.writeSetupCommit(w, cfg.Revision, &receipt) }) diff --git a/go/internal/api/api_setup_integrity_test.go b/go/internal/api/api_setup_integrity_test.go new file mode 100644 index 000000000..bb3048924 --- /dev/null +++ b/go/internal/api/api_setup_integrity_test.go @@ -0,0 +1,269 @@ +package api + +import ( + "encoding/json" + "fmt" + "net/http" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/srcfl/ftw/go/internal/config" +) + +func setupIntegrityCheck(d map[string]any) map[string]any { + return map[string]any{"revision": d["revision"], "base_revision": d["base_revision"], "sha256": d["sha256"], "confirmed": true} +} + +func TestSetupValidationReturnsTheNormalizedDraftBeforeConfirmation(t *testing.T) { + s, cfg := setupTestServer(t) + w, draft := setupCall(t, s, http.MethodPost, "/api/setup/draft", map[string]any{"revision": 0, + "patch": json.RawMessage(`{"planner":{"engine":"python","soc_min":0,"soc_min_pct":15}}`)}) + if w.Code != http.StatusOK { + t.Fatal(w.Body.String()) + } + old := setupIntegrityCheck(draft) + w, read := setupCall(t, s, http.MethodGet, "/api/setup/draft", nil) + if w.Code != http.StatusOK || read["sha256"] != draft["sha256"] || !strings.Contains(w.Body.String(), `"soc_min_pct":15`) { + t.Fatal("GET rewrote the draft or showed normalized data under an old hash", w.Body.String()) + } + w, _ = setupCall(t, s, http.MethodPost, "/api/setup/commit", old) + if w.Code != http.StatusConflict { + t.Fatal("unreviewed normalization saved", w.Code, w.Body.String()) + } + w, normalized := setupCall(t, s, http.MethodPost, "/api/setup/validate", old) + if w.Code != http.StatusOK || normalized["normalized"] != true || normalized["sha256"] == draft["sha256"] || + normalized["normalized_from_revision"] != draft["revision"] || !strings.Contains(w.Body.String(), `"soc_min":0.15`) { + t.Fatal("normalization did not return a new reviewed candidate", w.Code, w.Body.String()) + } + w, _ = setupCall(t, s, http.MethodPost, "/api/setup/commit", old) + if w.Code != http.StatusConflict { + t.Fatal("old confirmation still valid", w.Code) + } + w, saved := setupCall(t, s, http.MethodPost, "/api/setup/commit", setupIntegrityCheck(normalized)) + if w.Code != http.StatusOK || cfg.Planner.SoCMin != 0.15 || cfg.Planner.SoCMinPct != 0 || cfg.Planner.Engine != config.PlannerEngineEnergyplan { + t.Fatal("saved settings differ from the checked draft", w.Code, w.Body.String()) + } + if saved["commit"].(map[string]any)["sha256"] != normalized["sha256"] { + t.Fatal("receipt bound the wrong configuration") + } + loaded, err := config.Load(s.deps.ConfigPath) + if err != nil || loaded.Planner.SoCMin != cfg.Planner.SoCMin || loaded.Planner.Engine != cfg.Planner.Engine { + t.Fatal("normalized save did not survive reload", err) + } +} + +func TestSetupUnrelatedEditsKeepExactVendorNumbersThroughSaveAndReload(t *testing.T) { + s, cfg := setupTestServer(t) + w, draft := setupCall(t, s, http.MethodPost, "/api/setup/draft", map[string]any{"revision": 0, + "patch": json.RawMessage(`{"drivers":[{"name":"meter","lua":"meter.lua","is_site_meter":true,"capabilities":{"standalone":true},"config":{"vendor_id":9007199254740993,"unsigned_id":18446744073709551615,"nested":[{"unit_id":1,"ratio":0.1}],"string_id":"9007199254740993"}}]}`)}) + if w.Code != http.StatusOK { + t.Fatal(w.Body.String()) + } + w, draft = setupCall(t, s, http.MethodPost, "/api/setup/draft", map[string]any{"revision": draft["revision"], + "patch": map[string]any{"site": map[string]any{"name": "Unrelated change"}}}) + if w.Code != http.StatusOK || !strings.Contains(w.Body.String(), `"vendor_id":9007199254740993`) || + !strings.Contains(w.Body.String(), `"unsigned_id":18446744073709551615`) { + t.Fatal("draft/status rounded an untouched identifier", w.Body.String()) + } + w, checked := setupCall(t, s, http.MethodPost, "/api/setup/validate", setupIntegrityCheck(draft)) + if w.Code != http.StatusOK { + t.Fatal(w.Body.String()) + } + w, _ = setupCall(t, s, http.MethodPost, "/api/setup/commit", setupIntegrityCheck(checked)) + if w.Code != http.StatusOK { + t.Fatal(w.Body.String()) + } + loaded, err := config.Load(s.deps.ConfigPath) + if err != nil { + t.Fatal(err) + } + for _, snapshot := range []*config.Config{cfg, loaded} { + c := snapshot.Drivers[0].Config + if fmt.Sprint(c["vendor_id"]) != "9007199254740993" || fmt.Sprint(c["unsigned_id"]) != "18446744073709551615" || c["string_id"] != "9007199254740993" { + t.Fatal("Settings save or reload changed an identifier", c) + } + } + // The recovery export keeps integer scalars, rather than quoting a new + // special numeric type or rounding through a JSON map. + doc, found, err := s.deps.State.Configuration() + if err != nil || !found { + t.Fatal("missing stored configuration", err) + } + path := filepath.Join(t.TempDir(), "recovery.yaml") + if err := config.ExportStored(path, doc, loaded.ConfigDatabase); err != nil { + t.Fatal(err) + } + raw, err := os.ReadFile(path) + if err != nil || !strings.Contains(string(raw), "vendor_id: 9007199254740993") || !strings.Contains(string(raw), "unsigned_id: 18446744073709551615") { + t.Fatal("recovery export changed numeric identifiers", string(raw), err) + } +} + +func setupIntegrityLiveDriver(t *testing.T, s *Server, cfg *config.Config) { + t.Helper() + cfg.Drivers = []config.Driver{{Name: "meter", Lua: filepath.Join(filepath.Dir(s.deps.ConfigPath), "drivers", "old.lua"), + IsSiteMeter: true, Capabilities: config.Capabilities{Standalone: true}, + Config: map[string]any{"password": "old-password", "refresh_token": "old-token"}}} + if err := config.SaveStored(s.deps.State, s.deps.ConfigPath, cfg); err != nil { + t.Fatal(err) + } +} + +func TestSetupChangingDriverImplementationDoesNotRestoreOldSecretsAtSave(t *testing.T) { + s, cfg := setupTestServer(t) + setupIntegrityLiveDriver(t, s, cfg) + w, draft := setupCall(t, s, http.MethodGet, "/api/setup/draft", nil) + if w.Code != http.StatusOK { + t.Fatal(w.Body.String()) + } + drivers := draft["config"].(map[string]any)["drivers"].([]any) + driver := drivers[0].(map[string]any) + driver["lua"] = "drivers/new.lua" + driver["config"] = map[string]any{"password": "", "refresh_token": ""} + w, draft = setupCall(t, s, http.MethodPost, "/api/setup/draft", map[string]any{"revision": draft["revision"], "patch": map[string]any{"drivers": drivers}}) + if w.Code != http.StatusOK { + t.Fatal(w.Body.String()) + } + w, draft = setupCall(t, s, http.MethodPost, "/api/setup/validate", setupIntegrityCheck(draft)) + if w.Code != http.StatusOK { + t.Fatal(w.Body.String()) + } + w, _ = setupCall(t, s, http.MethodPost, "/api/setup/commit", setupIntegrityCheck(draft)) + if w.Code != http.StatusOK { + t.Fatal(w.Body.String()) + } + loaded, err := config.Load(s.deps.ConfigPath) + if err != nil { + t.Fatal(err) + } + for _, c := range []*config.Config{cfg, loaded} { + if c.Drivers[0].Config["password"] != "" || c.Drivers[0].Config["refresh_token"] != "" { + t.Fatal("old driver secrets crossed into the reviewed replacement") + } + } +} + +func TestSetupCannotRebindAReplacementTokenToTheLiveOwnerAfterReview(t *testing.T) { + s, cfg := setupTestServer(t) + setupIntegrityLiveDriver(t, s, cfg) + liveOwner := cfg.Drivers[0].CredentialOwner + w, draft := setupCall(t, s, http.MethodGet, "/api/setup/draft", nil) + if w.Code != http.StatusOK { + t.Fatal(w.Body.String()) + } + drivers := draft["config"].(map[string]any)["drivers"].([]any) + driver := drivers[0].(map[string]any) + driver["config"].(map[string]any)["refresh_token"] = "new-token" + w, draft = setupCall(t, s, http.MethodPost, "/api/setup/draft", map[string]any{"revision": 0, "patch": map[string]any{"drivers": drivers}}) + if w.Code != http.StatusOK { + t.Fatal(w.Body.String()) + } + bound := draft + drivers = draft["config"].(map[string]any)["drivers"].([]any) + owner := drivers[0].(map[string]any)["credential_owner"] + drivers[0].(map[string]any)["credential_owner"] = liveOwner + w, _ = setupCall(t, s, http.MethodPost, "/api/setup/draft", map[string]any{"revision": draft["revision"], "patch": map[string]any{"drivers": drivers}}) + if w.Code != http.StatusUnprocessableEntity { + t.Fatal("caller could replace a bound credential owner", w.Code, w.Body.String()) + } + w, draft = setupCall(t, s, http.MethodGet, "/api/setup/draft", nil) + if w.Code != http.StatusOK || draft["sha256"] != bound["sha256"] { + t.Fatal("refused owner edit changed draft", w.Body.String()) + } + // Omitting the technical owner field keeps the current named identity, + // including a new draft-only token and the other masked credentials. + drivers = draft["config"].(map[string]any)["drivers"].([]any) + delete(drivers[0].(map[string]any), "credential_owner") + w, draft = setupCall(t, s, http.MethodPost, "/api/setup/draft", map[string]any{"revision": draft["revision"], "patch": map[string]any{"drivers": drivers}}) + if w.Code != http.StatusOK { + t.Fatal(w.Body.String()) + } + w, draft = setupCall(t, s, http.MethodPost, "/api/setup/validate", setupIntegrityCheck(draft)) + if w.Code != http.StatusOK { + t.Fatal(w.Body.String()) + } + w, _ = setupCall(t, s, http.MethodPost, "/api/setup/commit", setupIntegrityCheck(draft)) + if w.Code != http.StatusOK || cfg.Drivers[0].CredentialOwner != owner || cfg.Drivers[0].Config["refresh_token"] != "new-token" || cfg.Drivers[0].Config["password"] != "old-password" { + t.Fatal("save changed the reviewed owner", w.Code, w.Body.String()) + } + loaded, err := config.Load(s.deps.ConfigPath) + if err != nil || loaded.Drivers[0].CredentialOwner != owner || loaded.Drivers[0].Config["refresh_token"] != "new-token" || loaded.Drivers[0].Config["password"] != "old-password" { + t.Fatal("credential binding or secrets changed after reload", err) + } +} + +func TestSetupFinalSaveRefusesLateMutationBeforeAnySQLWrite(t *testing.T) { + s, cfg := setupTestServer(t) + beforeRevision := cfg.Revision + w, draft := setupCall(t, s, http.MethodPost, "/api/setup/draft", map[string]any{"revision": 0, "patch": map[string]any{"site": map[string]any{"name": "Reviewed"}}}) + if w.Code != http.StatusOK { + t.Fatal(w.Body.String()) + } + s.deps.SaveConfigWithValues = func(path string, candidate *config.Config, values map[string]string, reviewedJSON []byte) error { + candidate.Site.Name = "Not reviewed" + return config.SaveStoredReviewedWithValues(s.deps.State, path, candidate, values, reviewedJSON) + } + w, _ = setupCall(t, s, http.MethodPost, "/api/setup/commit", setupIntegrityCheck(draft)) + if w.Code != http.StatusConflict || cfg.Revision != beforeRevision || cfg.Site.Name != "Before" { + t.Fatal("late mutation reached live settings", w.Code, w.Body.String()) + } + loaded, err := config.Load(s.deps.ConfigPath) + if err != nil || loaded.Revision != beforeRevision || loaded.Site.Name != "Before" { + t.Fatal("late mutation reached SQL", err) + } + if receipt, err := s.setup.Receipt(); err != nil || receipt != nil { + t.Fatal("refused save left a success receipt", err) + } +} + +func TestSetupEquivalentDriverPathsKeepSecretsAndCanBeCommitted(t *testing.T) { + for _, spelling := range []string{"./drivers/old.lua", "absolute"} { + t.Run(spelling, func(t *testing.T) { + s, cfg := setupTestServer(t) + setupIntegrityLiveDriver(t, s, cfg) + w, draft := setupCall(t, s, http.MethodGet, "/api/setup/draft", nil) + if w.Code != http.StatusOK { + t.Fatal(w.Body.String()) + } + drivers := draft["config"].(map[string]any)["drivers"].([]any) + path := spelling + if spelling == "absolute" { + path = cfg.Drivers[0].Lua + } + drivers[0].(map[string]any)["lua"] = path + w, draft = setupCall(t, s, http.MethodPost, "/api/setup/draft", map[string]any{"revision": 0, "patch": map[string]any{"drivers": drivers}}) + if w.Code != http.StatusOK { + t.Fatal(w.Body.String()) + } + w, draft = setupCall(t, s, http.MethodPost, "/api/setup/validate", setupIntegrityCheck(draft)) + if w.Code != http.StatusOK || draft["normalization_required"] != false { + t.Fatal("path normalization cannot complete", w.Body.String()) + } + w, _ = setupCall(t, s, http.MethodPost, "/api/setup/commit", setupIntegrityCheck(draft)) + if w.Code != http.StatusOK || cfg.Drivers[0].Config["password"] != "old-password" || cfg.Drivers[0].Config["refresh_token"] != "old-token" { + t.Fatal("equivalent path lost a credential or could not save", w.Code, w.Body.String()) + } + }) + } +} + +func TestSetupChangingDriverRefusesAnUnresolvedMaskedToken(t *testing.T) { + s, cfg := setupTestServer(t) + setupIntegrityLiveDriver(t, s, cfg) + w, before := setupCall(t, s, http.MethodGet, "/api/setup/draft", nil) + if w.Code != http.StatusOK { + t.Fatal(w.Body.String()) + } + drivers := before["config"].(map[string]any)["drivers"].([]any) + drivers[0].(map[string]any)["lua"] = "drivers/replacement.lua" + w, _ = setupCall(t, s, http.MethodPost, "/api/setup/draft", map[string]any{"revision": 0, "patch": map[string]any{"drivers": drivers}}) + if w.Code != http.StatusUnprocessableEntity { + t.Fatal("a different driver accepted the earlier driver's masked token", w.Code, w.Body.String()) + } + w, after := setupCall(t, s, http.MethodGet, "/api/setup/draft", nil) + if w.Code != http.StatusOK || after["sha256"] != before["sha256"] || after["revision"] != before["revision"] { + t.Fatal("refused secret edit replaced the draft", w.Body.String()) + } +} diff --git a/go/internal/api/api_setup_test.go b/go/internal/api/api_setup_test.go index c1ec9b679..8e72502a9 100644 --- a/go/internal/api/api_setup_test.go +++ b/go/internal/api/api_setup_test.go @@ -129,7 +129,7 @@ func TestSetupFailedSaveLeavesLiveAndStoredConfigUntouched(t *testing.T) { if w.Code != http.StatusOK { t.Fatal(w.Body.String()) } - s.deps.SaveConfigWithValues = func(string, *config.Config, map[string]string) error { return http.ErrHandlerTimeout } + s.deps.SaveConfigWithValues = func(string, *config.Config, map[string]string, []byte) error { return http.ErrHandlerTimeout } w, _ = setupCall(t, s, http.MethodPost, "/api/setup/commit", map[string]any{"revision": draft["revision"], "base_revision": draft["base_revision"], "sha256": draft["sha256"], "confirmed": true}) if w.Code != http.StatusInternalServerError || cfg.Site.Name != "Before" { t.Fatal("failed commit changed live settings") diff --git a/go/internal/config/json_numbers.go b/go/internal/config/json_numbers.go new file mode 100644 index 000000000..7d912cf80 --- /dev/null +++ b/go/internal/config/json_numbers.go @@ -0,0 +1,106 @@ +package config + +import ( + "bytes" + "encoding/json" + "errors" + "fmt" + "io" + "math/big" + "strconv" + "strings" +) + +func DecodeJSON(raw []byte, cfg *Config) error { return decodeJSON(raw, cfg, false) } + +func DecodeStrictJSON(raw []byte, cfg *Config) error { return decodeJSON(raw, cfg, true) } + +func decodeJSON(raw []byte, cfg *Config, strict bool) error { + if raw = bytes.TrimSpace(raw); len(raw) == 0 || raw[0] != '{' { + return errors.New("settings must be a JSON object") + } + dec := json.NewDecoder(bytes.NewReader(raw)) + dec.UseNumber() + if strict { + dec.DisallowUnknownFields() + } + if err := dec.Decode(cfg); err != nil { + return err + } + if err := dec.Decode(new(any)); err != io.EOF { + return errors.New("settings must contain one JSON object") + } + return cfg.DecodeDriverNumbers() +} + +// DecodeDriverNumbers is for a configuration decoded with UseNumber. Keep +// the numeric types existing control/driver code already accepts, without +// rounding an identifier or another untouched opaque value. +func (c *Config) DecodeDriverNumbers() error { + for i := range c.Drivers { + for key, value := range c.Drivers[i].Config { + next, err := driverJSONNumber(value, fmt.Sprintf("/drivers/%d/config/%s", i, jsonPathKey(key))) + if err != nil { + return err + } + c.Drivers[i].Config[key] = next + } + } + return nil +} + +func jsonPathKey(key string) string { + return strings.ReplaceAll(strings.ReplaceAll(key, "~", "~0"), "/", "~1") +} + +func driverJSONNumber(value any, path string) (any, error) { + switch v := value.(type) { + case json.Number: + if n, err := v.Int64(); err == nil { + return n, nil + } + if n, err := strconv.ParseUint(string(v), 10, 64); err == nil { + return n, nil + } + n, err := v.Float64() + if err != nil { + return nil, fmt.Errorf("%s: numeric value is outside the supported range", path) + } + if n == 0 { + // Do not build a large decimal denominator for an underflowed + // exponent. A literal zero needs no exponent comparison. + mantissa := strings.FieldsFunc(string(v), func(r rune) bool { return r == 'e' || r == 'E' })[0] + if strings.Trim(mantissa, "-+.0") == "" { + return n, nil + } + return nil, fmt.Errorf("%s: this number would lose precision", path) + } + encoded, err := json.Marshal(n) + if err != nil { + return nil, err + } + before, ok := new(big.Rat).SetString(string(v)) + after, encodedOK := new(big.Rat).SetString(string(encoded)) + if ok && encodedOK && before.Cmp(after) == 0 { + return n, nil + } + return nil, fmt.Errorf("%s: this number would lose precision; use a string if it is an identifier", path) + case map[string]any: + for key, item := range v { + next, err := driverJSONNumber(item, path+"/"+jsonPathKey(key)) + if err != nil { + return nil, err + } + v[key] = next + } + case []any: + for i, item := range v { + next, err := driverJSONNumber(item, fmt.Sprintf("%s/%d", path, i)) + if err != nil { + return nil, err + } + v[i] = next + } + } + return value, nil +} diff --git a/go/internal/config/json_numbers_test.go b/go/internal/config/json_numbers_test.go new file mode 100644 index 000000000..c0d866118 --- /dev/null +++ b/go/internal/config/json_numbers_test.go @@ -0,0 +1,41 @@ +package config + +import ( + "encoding/json" + "strings" + "testing" +) + +func TestDriverJSONKeepsIntegerIdentifiersAndOrdinaryNumbers(t *testing.T) { + raw := []byte(`{"drivers":[{"name":"meter","config":{"vendor_id":9007199254740993,"unsigned_id":18446744073709551615,"nested":[{"port":502,"ratio":0.1,"scale":1e3}],"string_id":"9007199254740993"}}]}`) + var cfg Config + if err := DecodeStrictJSON(raw, &cfg); err != nil { + t.Fatal(err) + } + c := cfg.Drivers[0].Config + if c["vendor_id"] != int64(9007199254740993) || c["unsigned_id"] != uint64(18446744073709551615) { + t.Fatal(c) + } + row := c["nested"].([]any)[0].(map[string]any) + if row["port"] != int64(502) || row["ratio"] != float64(0.1) || row["scale"] != float64(1000) { + t.Fatal(row) + } + encoded, err := json.Marshal(cfg) + if err != nil || !strings.Contains(string(encoded), `"vendor_id":9007199254740993`) || !strings.Contains(string(encoded), `"unsigned_id":18446744073709551615`) { + t.Fatal(string(encoded), err) + } +} + +func TestDriverJSONRefusesSilentNumericLossWithItsFieldPath(t *testing.T) { + for _, number := range []string{"0.123456789012345678901", "18446744073709551617", "1e-999999999", "1e999999999"} { + var cfg Config + err := DecodeStrictJSON([]byte(`{"drivers":[{"config":{"nested":[{"vendor/id":`+number+`}]}}]}`), &cfg) + if err == nil || !strings.Contains(err.Error(), "/drivers/0/config/nested/0/vendor~1id") { + t.Fatal("silent numeric loss or missing path", number, err) + } + } + var cfg Config + if err := DecodeStrictJSON([]byte(`{"drivers":[{"config":{"zero":0e-999999999}}]}`), &cfg); err != nil { + t.Fatal("literal zero rejected", err) + } +} diff --git a/go/internal/config/storage.go b/go/internal/config/storage.go index 735660cce..61a34fa6f 100644 --- a/go/internal/config/storage.go +++ b/go/internal/config/storage.go @@ -22,6 +22,26 @@ type storedSettings struct { YAMLSourceHash string `json:"yaml_source_hash,omitempty"` } +func (s *storedSettings) UnmarshalJSON(raw []byte) error { + var doc struct { + Config json.RawMessage `json:"config"` + LANPasswordHash string `json:"lan_password_hash"` + YAMLSourceHash string `json:"yaml_source_hash"` + } + if err := json.Unmarshal(raw, &doc); err != nil { + return err + } + if len(doc.Config) == 0 || bytes.Equal(bytes.TrimSpace(doc.Config), []byte("null")) { + return errors.New("stored settings have no config") + } + cfg := new(Config) + if err := DecodeJSON(doc.Config, cfg); err != nil { + return err + } + *s = storedSettings{Config: cfg, LANPasswordHash: doc.LANPasswordHash, YAMLSourceHash: doc.YAMLSourceHash} + return nil +} + func decodeStored(c state.Configuration, database, baseDir string) (*Config, error) { var doc storedSettings if err := json.Unmarshal(c.Document, &doc); err != nil { @@ -218,7 +238,7 @@ func SaveStored(st *state.Store, path string, cfg *Config) error { // SaveStoredWithValues commits a settings change and its companion records // in the same SQLite transaction, for example a setup commit receipt. func SaveStoredWithValues(st *state.Store, path string, cfg *Config, values map[string]string) error { - return saveStoredValues(st, path, cfg, "", values) + return saveStoredValues(st, path, cfg, "", values, nil) } // DropRetiredSettings rewrites stored settings that still carry settings of a @@ -247,10 +267,22 @@ func DropRetiredSettings(st *state.Store, path string, cfg *Config) ([]string, e } func saveStored(st *state.Store, path string, cfg *Config, sourceHash string) error { - return saveStoredValues(st, path, cfg, sourceHash, nil) + return saveStoredValues(st, path, cfg, sourceHash, nil, nil) } -func saveStoredValues(st *state.Store, path string, cfg *Config, sourceHash string, values map[string]string) error { +// ErrReviewedConfigChanged means a save would differ from the reviewed document. +var ErrReviewedConfigChanged = errors.New("configuration changed after review; validate and review the returned draft again") + +// SaveStoredReviewedWithValues checks the portable document after all save-time +// normalization, before committing either settings or companion records. +func SaveStoredReviewedWithValues(st *state.Store, path string, cfg *Config, values map[string]string, reviewedJSON []byte) error { + if len(reviewedJSON) == 0 { + return errors.New("reviewed configuration is required") + } + return saveStoredValues(st, path, cfg, "", values, reviewedJSON) +} + +func saveStoredValues(st *state.Store, path string, cfg *Config, sourceHash string, values map[string]string, reviewedJSON []byte) error { if cfg.ConfigDatabase == "" { return errors.New("settings database is not initialized") } @@ -295,6 +327,15 @@ func saveStoredValues(st *state.Store, path string, cfg *Config, sourceHash stri portable := *cfg portable.Drivers = append([]Driver(nil), cfg.Drivers...) portable.UnresolveDriverPaths(filepath.Dir(path)) + if reviewedJSON != nil { + actual, err := json.Marshal(&portable) + if err != nil { + return err + } + if !bytes.Equal(actual, reviewedJSON) { + return ErrReviewedConfigChanged + } + } raw, err := json.Marshal(storedSettings{Config: &portable, LANPasswordHash: cfg.LANPasswordHash, YAMLSourceHash: sourceHash}) if err != nil { return err diff --git a/go/internal/drivers/registry.go b/go/internal/drivers/registry.go index 4afafc619..a265ef7dc 100644 --- a/go/internal/drivers/registry.go +++ b/go/internal/drivers/registry.go @@ -1532,6 +1532,8 @@ func tcpAllowedHostsFor(cfg config.Driver) []string { port = p case int64: port = int(p) + case uint64: + port = int(p) case float64: port = int(p) } diff --git a/go/internal/onboarding/draft.go b/go/internal/onboarding/draft.go index 24bb9342a..bcae89dbb 100644 --- a/go/internal/onboarding/draft.go +++ b/go/internal/onboarding/draft.go @@ -10,6 +10,7 @@ import ( "errors" "fmt" "io" + "strings" "sync" "github.com/srcfl/ftw/go/internal/config" @@ -32,11 +33,13 @@ type Storage interface { // Draft is an uncommitted configuration. Config can contain credentials; // a transport must mask them before returning it to a client. type Draft struct { - Version int `json:"version"` - Revision int64 `json:"revision"` - BaseRevision int64 `json:"base_revision"` - Config json.RawMessage `json:"config"` - SHA256 string `json:"sha256"` + Version int `json:"version"` + Revision int64 `json:"revision"` + BaseRevision int64 `json:"base_revision"` + Config json.RawMessage `json:"config"` + SHA256 string `json:"sha256"` + NormalizedFromRevision int64 `json:"normalized_from_revision,omitempty"` + ValidationWarnings []string `json:"validation_warnings,omitempty"` } // Service serializes draft changes. Live config saves remain the caller's @@ -44,15 +47,23 @@ type Draft struct { type Service struct { mu sync.Mutex store Storage - preserveSecrets func(incoming, previous *config.Config) + preserveSecrets func(incoming, previous *config.Config) error + baseDir string } -func New(store Storage) *Service { return NewWithSecrets(store, nil) } +func New(store Storage) *Service { return NewWithSecrets(store, nil, "") } // NewWithSecrets adds the running driver's secret metadata to the common // config secret handling. The callback must be safe to call during an edit. -func NewWithSecrets(store Storage, preserve func(incoming, previous *config.Config)) *Service { - return &Service{store: store, preserveSecrets: preserve} +func NewWithSecrets(store Storage, preserve func(incoming, previous *config.Config) error, baseDir string) *Service { + return &Service{store: store, preserveSecrets: preserve, baseDir: baseDir} +} + +func (s *Service) portablePaths(cfg *config.Config) { + if s.baseDir != "" { + cfg.ResolveDriverPaths(s.baseDir) + cfg.UnresolveDriverPaths(s.baseDir) + } } func (s *Service) current() (Draft, bool, error) { @@ -140,23 +151,35 @@ func (s *Service) Edit(live *config.Config, expected int64, patch json.RawMessag if err != nil { return Draft{}, err } + s.portablePaths(cfg) + s.portablePaths(previous) if cfg.API.LANAuth != previous.API.LANAuth { return Draft{}, errors.New("api.lan_auth changes through the house-password action on the box, not a setup draft") } + if err := keepDraftCredentialOwners(cfg, previous); err != nil { + return Draft{}, err + } if s.preserveSecrets != nil { - s.preserveSecrets(cfg, previous) + if err := s.preserveSecrets(cfg, previous); err != nil { + return Draft{}, err + } } else { cfg.PreserveMaskedSecrets(previous) } if err := config.PrepareDraftCredentials(cfg, previous); err != nil { return Draft{}, err } + if err := config.PrepareDraftCredentials(cfg, live); err != nil { + return Draft{}, err + } // Canonicalize through the current type. Do not default or validate yet: // both would hide missing answers or reject an unfinished installation. d.Config, err = json.Marshal(cfg) if err != nil { return Draft{}, err } + d.NormalizedFromRevision = 0 + d.ValidationWarnings = nil d.Revision++ d.SHA256 = digest(d.Config, d.BaseRevision) raw, err := json.Marshal(d) @@ -169,8 +192,78 @@ func (s *Service) Edit(live *config.Config, expected int64, patch json.RawMessag return d, nil } -// Checked returns a normalized, valid candidate only when the caller names -// the exact current draft and live base. It neither persists nor applies it. +// Credential owners come from the box. A masked list may retain or omit +// one, but it cannot replace a bound identity with a caller-chosen value. +func keepDraftCredentialOwners(cfg, previous *config.Config) error { + owners := make(map[string]bool) + for _, d := range previous.Drivers { + if d.CredentialOwner != "" { + owners[d.CredentialOwner] = true + } + } + for i := range cfg.Drivers { + d := &cfg.Drivers[i] + d.CredentialOwner = strings.TrimSpace(d.CredentialOwner) + if d.CredentialOwner != "" { + if !owners[d.CredentialOwner] { + return fmt.Errorf("driver %q: credential_owner is read-only; retain the current identity or omit it for a new driver", d.Name) + } + continue + } + for _, old := range previous.Drivers { + if d.Name == old.Name && d.Lua == old.Lua { + d.CredentialOwner = old.CredentialOwner + break + } + } + } + return nil +} + +// Validate saves any normalization as a new draft for the user to review. +// An invalid partial draft stays unchanged; reading never calls this method. +func (s *Service) Validate(live *config.Config, revision int64, sha string) (Draft, error) { + s.mu.Lock() + defer s.mu.Unlock() + if live == nil { + return Draft{}, errors.New("live configuration is unavailable") + } + d, cfg, err := s.candidateLocked(live.Revision, revision, sha) + if err != nil { + return Draft{}, err + } + if err := config.PrepareDraftCredentials(cfg, live); err != nil { + return Draft{}, err + } + s.portablePaths(cfg) + if err := cfg.Validate(); err != nil { + return Draft{}, fmt.Errorf("configuration is incomplete or invalid: %w", err) + } + normalized, err := json.Marshal(cfg) + if err != nil { + return Draft{}, err + } + if bytes.Equal(normalized, d.Config) { + return d, nil + } + d.Config = normalized + d.NormalizedFromRevision = d.Revision + d.ValidationWarnings = append([]string(nil), cfg.LoadpointWarnings...) + d.Revision++ + d.SHA256 = digest(d.Config, d.BaseRevision) + raw, err := json.Marshal(d) + if err != nil || len(raw) > MaxDraftBytes { + return Draft{}, errors.New("setup draft is too large") + } + if err := s.store.SaveConfig(draftKey, string(raw)); err != nil { + return Draft{}, fmt.Errorf("save checked setup draft: %w", err) + } + return d, nil +} + +// Checked returns the exact valid configuration the user reviewed. A caller +// must first validate and review a draft that still needs normalization. +// It neither persists nor applies anything. func (s *Service) Checked(liveRevision, revision int64, sha string) (*config.Config, error) { s.mu.Lock() defer s.mu.Unlock() @@ -190,23 +283,39 @@ func (s *Service) ApplyChecked(liveRevision, revision int64, sha string, apply f } func (s *Service) checkedLocked(liveRevision, revision int64, sha string) (*config.Config, error) { - d, found, err := s.current() + d, cfg, err := s.candidateLocked(liveRevision, revision, sha) if err != nil { return nil, err } - if !found || d.BaseRevision != liveRevision || d.Revision != revision || d.SHA256 != sha { - return nil, ErrConflict + s.portablePaths(cfg) + if err := cfg.Validate(); err != nil { + return nil, fmt.Errorf("configuration is incomplete or invalid: %w", err) } - cfg, err := decodeConfig(d.Config) + normalized, err := json.Marshal(cfg) if err != nil { return nil, err } - if err := cfg.Validate(); err != nil { - return nil, fmt.Errorf("configuration is incomplete or invalid: %w", err) + if !bytes.Equal(normalized, d.Config) { + return nil, fmt.Errorf("%w: run setup_validate and review its returned draft before saving", ErrConflict) } return cfg, nil } +func (s *Service) candidateLocked(liveRevision, revision int64, sha string) (Draft, *config.Config, error) { + d, found, err := s.current() + if err != nil { + return Draft{}, nil, err + } + if !found || d.BaseRevision != liveRevision || d.Revision != revision || d.SHA256 != sha { + return Draft{}, nil, ErrConflict + } + cfg, err := decodeConfig(d.Config) + if err != nil { + return Draft{}, nil, err + } + return d, cfg, nil +} + // Restart abandons a stale draft only when the caller names its current // revision. It stores a new draft from live settings, never an empty config. func (s *Service) Restart(live *config.Config, expected int64) (Draft, error) { @@ -246,14 +355,9 @@ func decodeConfig(raw []byte) (*config.Config, error) { return nil, errors.New("setup config is too large") } var cfg config.Config - dec := json.NewDecoder(bytes.NewReader(raw)) - dec.DisallowUnknownFields() - if err := dec.Decode(&cfg); err != nil { + if err := config.DecodeStrictJSON(raw, &cfg); err != nil { return nil, fmt.Errorf("setup config: %w", err) } - if err := dec.Decode(new(any)); err != io.EOF { - return nil, errors.New("setup config must contain one JSON object") - } return &cfg, nil } diff --git a/go/internal/onboarding/normalization_test.go b/go/internal/onboarding/normalization_test.go new file mode 100644 index 000000000..cd83bed7d --- /dev/null +++ b/go/internal/onboarding/normalization_test.go @@ -0,0 +1,144 @@ +package onboarding + +import ( + "bytes" + "encoding/json" + "errors" + "testing" + + "github.com/srcfl/ftw/go/internal/config" +) + +func TestValidationBindsNormalizedConfigurationToANewReviewedDraft(t *testing.T) { + for _, tc := range []struct { + name, patch, expected string + warning bool + }{ + {"legacy units", `{"planner":{"soc_min":0,"soc_min_pct":15,"engine":"python"}}`, `"soc_min":0.15`, false}, + {"legacy email", `{"ev_charger":{"provider":"easee","email":"test@example.com"}}`, `"username":"test@example.com"`, false}, + {"charger steps", `{"loadpoints":[{"id":"Easee","driver_name":"easee_cloud","min_charge_w":1400,"max_charge_w":7000,"allowed_steps_w":[0,1400,11000]}]}`, `"allowed_steps_w":[0,1400,7000]`, true}, + } { + t.Run(tc.name, func(t *testing.T) { + live := testLive(t) + store := draftStore(t) + s := New(store) + original, err := s.Edit(live, 0, json.RawMessage(tc.patch)) + if err != nil { + t.Fatal(err) + } + if _, err := s.Checked(live.Revision, original.Revision, original.SHA256); !errors.Is(err, ErrConflict) { + t.Fatal("commit accepted unreviewed normalization", err) + } + read, err := s.Read(live) + if err != nil || read.SHA256 != original.SHA256 || !bytes.Equal(read.Config, original.Config) { + t.Fatal("read or refused commit changed draft", err) + } + checked, err := s.Validate(live, original.Revision, original.SHA256) + if err != nil { + t.Fatal(err) + } + if checked.Revision != original.Revision+1 || checked.SHA256 == original.SHA256 || + checked.NormalizedFromRevision != original.Revision || !bytes.Contains(checked.Config, []byte(tc.expected)) { + t.Fatalf("normalization did not produce a reviewable draft: %+v", checked) + } + if tc.warning && len(checked.ValidationWarnings) == 0 { + t.Fatal("normalization warning was lost") + } + // A lost validation reply resumes the same normalized bytes and warning. + resumed, err := New(store).Read(live) + if err != nil || resumed.SHA256 != checked.SHA256 || (tc.warning && len(resumed.ValidationWarnings) == 0) { + t.Fatal("normalized receipt did not resume", err) + } + if _, err := s.Checked(live.Revision, original.Revision, original.SHA256); !errors.Is(err, ErrConflict) { + t.Fatal("old confirmation survived normalization", err) + } + candidate, err := s.Checked(live.Revision, checked.Revision, checked.SHA256) + if err != nil { + t.Fatal(err) + } + raw, err := json.Marshal(candidate) + if err != nil || !bytes.Equal(raw, checked.Config) { + t.Fatal("candidate differs from reviewed bytes", err) + } + again, err := s.Validate(live, checked.Revision, checked.SHA256) + if err != nil || again.Revision != checked.Revision || again.SHA256 != checked.SHA256 { + t.Fatal("validating twice changed the receipt", err) + } + }) + } +} + +func TestFailedOrInvalidNormalizationKeepsTheOldDraft(t *testing.T) { + live := testLive(t) + store := draftStore(t) + s := New(store) + d, err := s.Edit(live, 0, json.RawMessage(`{"planner":{"soc_min_pct":15},"drivers":[{"name":"unfinished"}]}`)) + if err != nil { + t.Fatal(err) + } + if _, err := s.Validate(live, d.Revision, d.SHA256); err == nil { + t.Fatal("incomplete setup passed") + } + after, err := s.Read(live) + if err != nil || after.SHA256 != d.SHA256 || !bytes.Equal(after.Config, d.Config) { + t.Fatal("invalid validation wrote half-normalized data", err) + } + // A persistence failure also leaves the original revision retryable. + d, err = s.Edit(live, d.Revision, json.RawMessage(`{"drivers":[]}`)) + if err != nil { + t.Fatal(err) + } + broken := New(failedStore{store}) + if _, err := broken.Validate(live, d.Revision, d.SHA256); err == nil { + t.Fatal("normalization save failure hidden") + } + after, err = s.Read(live) + if err != nil || after.SHA256 != d.SHA256 || !bytes.Equal(after.Config, d.Config) { + t.Fatal("failed save changed draft", err) + } +} + +func TestValidationRepairsAnOlderDraftCredentialBindingBeforeReview(t *testing.T) { + live := testLive(t) + live.Drivers = []config.Driver{{Name: "meter", Lua: "meter.lua", IsSiteMeter: true, CredentialOwner: "live-owner", + Capabilities: config.Capabilities{Standalone: true}, Config: map[string]any{"refresh_token": "old"}}} + store := draftStore(t) + s := New(store) + d, err := s.Edit(live, 0, json.RawMessage(`{"site":{"name":"Changed"}}`)) + if err != nil { + t.Fatal(err) + } + // Simulate a draft from an earlier version, with a new token still bound + // to the live token family. Validation must change the reviewed version. + var cfg config.Config + if err := config.DecodeJSON(d.Config, &cfg); err != nil { + t.Fatal(err) + } + cfg.Drivers[0].Config["refresh_token"] = "new" + d.Config, err = json.Marshal(&cfg) + if err != nil { + t.Fatal(err) + } + d.SHA256 = digest(d.Config, d.BaseRevision) + raw, _ := json.Marshal(d) + if err := store.SaveConfig(draftKey, string(raw)); err != nil { + t.Fatal(err) + } + next, err := s.Validate(live, d.Revision, d.SHA256) + if err != nil { + t.Fatal(err) + } + if next.SHA256 == d.SHA256 || next.Revision != d.Revision+1 { + t.Fatal("late credential binding kept the old confirmation") + } + if err := config.DecodeJSON(next.Config, &cfg); err != nil { + t.Fatal(err) + } + if cfg.Drivers[0].CredentialOwner == "live-owner" || cfg.Drivers[0].CredentialOwner == "" { + t.Fatal("replacement token family was not bound") + } + again, err := s.Validate(live, next.Revision, next.SHA256) + if err != nil || again.SHA256 != next.SHA256 { + t.Fatal("credential binding changed on retry", err) + } +}