Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions core/config/runtime_settings_persist.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"errors"
"os"
"path/filepath"
"reflect"
)

// runtimeSettingsFile is the on-disk filename inside DynamicConfigsDir.
Expand Down Expand Up @@ -33,6 +34,35 @@ func (o *ApplicationConfig) ReadPersistedSettings() (RuntimeSettings, error) {
return settings, nil
}

// MergeNonNil overlays every set (non-nil) field of overlay onto the
// receiver, leaving the receiver's value untouched wherever overlay left a
// field unset. Every RuntimeSettings field is a pointer precisely so "set"
// can be told apart from "absent" (see the type doc), which makes this a
// faithful partial update: a caller that submits only the field it owns
// changes exactly that field and never clobbers unrelated settings.
//
// This is the read-modify-write contract the persistence helpers exist for.
// UpdateSettingsEndpoint reads the on-disk settings, merges the request body
// on top, and writes the result — so a focused admin page that POSTs only its
// own field (the Middleware page sends only mitm_listen; the detector table
// only pii_default_detectors) no longer nulls every other setting.
//
// Reflection keeps the merge total over the struct: a field added to
// RuntimeSettings later is merged automatically, so the persistence path can
// never silently drop a new setting the way a hand-maintained field list
// would. Non-pointer fields (none today) are skipped — they cannot express
// "absent", so the receiver wins.
func (s *RuntimeSettings) MergeNonNil(overlay RuntimeSettings) {
dst := reflect.ValueOf(s).Elem()
src := reflect.ValueOf(overlay)
for i := 0; i < src.NumField(); i++ {
f := src.Field(i)
if f.Kind() == reflect.Pointer && !f.IsNil() {
dst.Field(i).Set(f)
}
}
}

// WritePersistedSettings serialises the given RuntimeSettings to
// runtime_settings.json with restricted permissions (it may carry API
// keys and P2P tokens).
Expand Down
42 changes: 42 additions & 0 deletions core/config/runtime_settings_persist_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
)

func strPtr(s string) *string { return &s }
func boolPtr(b bool) *bool { return &b }

var _ = Describe("RuntimeSettings persistence helpers", func() {
var (
Expand Down Expand Up @@ -51,6 +52,47 @@ var _ = Describe("RuntimeSettings persistence helpers", func() {
})
})

// MergeNonNil is the partial-update primitive UpdateSettingsEndpoint
// relies on: a focused admin page POSTs only the field it owns, and the
// handler reads the on-disk settings and overlays the request on top.
// Without it, the body would be written verbatim and every field the
// caller omitted would be nulled (the reported regression: changing
// mitm_listen wiped the galleries, api keys, watchdog config, etc.).
Describe("MergeNonNil partial update", func() {
It("overlays set fields and preserves unset ones", func() {
base := config.RuntimeSettings{
MITMListen: strPtr(":9000"),
Galleries: &[]config.Gallery{{Name: "g1", URL: "http://example/g1"}},
WatchdogIdleEnabled: boolPtr(true),
ApiKeys: &[]string{"persisted-key"},
PIIDefaultDetectors: &[]string{"det-a"},
}

// Simulate the Middleware proxy tab: only mitm_listen is sent.
overlay := config.RuntimeSettings{MITMListen: strPtr(":8443")}
base.MergeNonNil(overlay)

Expect(base.MITMListen).ToNot(BeNil())
Expect(*base.MITMListen).To(Equal(":8443"), "set field should be overlaid")
// Everything the overlay left unset must survive untouched.
Expect(base.Galleries).ToNot(BeNil(), "galleries were clobbered")
Expect(*base.Galleries).To(HaveLen(1))
Expect(base.WatchdogIdleEnabled).ToNot(BeNil())
Expect(*base.WatchdogIdleEnabled).To(BeTrue())
Expect(base.ApiKeys).ToNot(BeNil(), "api_keys were clobbered")
Expect(*base.ApiKeys).To(Equal([]string{"persisted-key"}))
Expect(base.PIIDefaultDetectors).ToNot(BeNil(), "pii_default_detectors were clobbered")
Expect(*base.PIIDefaultDetectors).To(Equal([]string{"det-a"}))
})

It("lets an explicit empty slice clear a field", func() {
base := config.RuntimeSettings{PIIDefaultDetectors: &[]string{"det-a"}}
base.MergeNonNil(config.RuntimeSettings{PIIDefaultDetectors: &[]string{}})
Expect(base.PIIDefaultDetectors).ToNot(BeNil())
Expect(*base.PIIDefaultDetectors).To(BeEmpty(), "an explicit empty slice should clear, not preserve")
})
})

// MITM round trip pins the contract that loadRuntimeSettingsFromFile
// MITM listener address must survive a write/read round trip so the
// next process restart can bring the listener back up. (Intercept
Expand Down
43 changes: 26 additions & 17 deletions core/http/endpoints/localai/settings.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@ import (
"encoding/json"
"io"
"net/http"
"os"
"path/filepath"
"time"

"github.com/labstack/echo/v4"
Expand Down Expand Up @@ -110,6 +108,18 @@ func UpdateSettingsEndpoint(app *application.Application) echo.HandlerFunc {
})
}

// Read whatever is already persisted: it is both the source of truth
// for branding asset filenames (below) and the base we merge this
// request onto before writing. A read failure must not let a Save
// silently discard the existing settings — surface it instead.
persisted, err := appConfig.ReadPersistedSettings()
if err != nil {
return c.JSON(http.StatusInternalServerError, schema.SettingsResponse{
Success: false,
Error: "Failed to read existing settings: " + err.Error(),
})
}

// Branding asset filenames are owned exclusively by
// /api/branding/asset/{kind} (upload/delete). The Settings page also
// round-trips them via GET /api/settings, but its local state is stale
Expand All @@ -118,11 +128,9 @@ func UpdateSettingsEndpoint(app *application.Application) echo.HandlerFunc {
// at page open. Replace whatever the body sent for these three fields
// with the values currently on disk so /api/settings can never
// regress them.
if existing, err := appConfig.ReadPersistedSettings(); err == nil {
settings.LogoFile = existing.LogoFile
settings.LogoHorizontalFile = existing.LogoHorizontalFile
settings.FaviconFile = existing.FaviconFile
}
settings.LogoFile = persisted.LogoFile
settings.LogoHorizontalFile = persisted.LogoHorizontalFile
settings.FaviconFile = persisted.FaviconFile

// The UI reads ApiKeys from GET /api/settings, which already returns the
// merged env+runtime list. When the user clicks Save, the same merged
Expand All @@ -145,16 +153,17 @@ func UpdateSettingsEndpoint(app *application.Application) echo.HandlerFunc {
settings.ApiKeys = &runtimeOnly
}

settingsFile := filepath.Join(appConfig.DynamicConfigsDir, "runtime_settings.json")
settingsJSON, err := json.MarshalIndent(settings, "", " ")
if err != nil {
return c.JSON(http.StatusInternalServerError, schema.SettingsResponse{
Success: false,
Error: "Failed to marshal settings: " + err.Error(),
})
}

if err := os.WriteFile(settingsFile, settingsJSON, 0600); err != nil {
// Persist as a partial update: overlay only the fields this request set
// onto the settings already on disk. Focused admin pages POST just the
// keys they own (the Middleware proxy tab sends only mitm_listen; the
// detector table only pii_default_detectors), so writing the request
// body verbatim would null every unrelated setting (the no-omitempty
// api_keys / pii_default_detectors fields even round-trip as JSON
// null). The full Settings page still round-trips every field, so its
// Save is unchanged.
toPersist := persisted
toPersist.MergeNonNil(settings)
if err := appConfig.WritePersistedSettings(toPersist); err != nil {
return c.JSON(http.StatusInternalServerError, schema.SettingsResponse{
Success: false,
Error: "Failed to write settings file: " + err.Error(),
Expand Down
37 changes: 37 additions & 0 deletions core/http/endpoints/localai/settings_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,10 @@ var _ = Describe("Settings endpoints", func() {
// Settings are persisted here; set after construction since there's no
// dedicated AppOption for it.
app.ApplicationConfig().DynamicConfigsDir = tmp
// Contain the MITM CA inside tmp too. The partial-save spec flips
// mitm_listen, which starts the listener and writes a CA; without this
// it defaults to ./mitm-ca and litters the package source tree.
app.ApplicationConfig().MITMCADir = filepath.Join(tmp, "mitm-ca")

e = echo.New()
e.GET("/api/settings", GetSettingsEndpoint(app))
Expand Down Expand Up @@ -109,6 +113,39 @@ var _ = Describe("Settings endpoints", func() {
Expect(err).ToNot(HaveOccurred())
})

// Regression: a focused admin page (the Middleware proxy tab) POSTs only
// the one field it owns — mitm_listen. The old handler wrote the request
// body verbatim, so every other persisted setting was dropped (and
// api_keys / pii_default_detectors, which lack omitempty, were written as
// null). A partial POST must now merge onto what is already on disk.
It("preserves unrelated persisted settings when a partial POST sets only mitm_listen", func() {
// First save establishes a fuller settings file (as the full Settings
// page would): galleries, an API key, and the MITM listener. The
// listener restart binds a real socket, so use 127.0.0.1:0 for an
// ephemeral free port rather than a fixed one that may be in use.
rec := post(`{"mitm_listen":"127.0.0.1:0","galleries":[{"name":"g1","url":"http://example/g1"}],"api_keys":["k1"],"pii_default_detectors":["det-a"]}`)
Expect(rec.Code).To(Equal(http.StatusOK), rec.Body.String())

// The Middleware proxy tab then changes only the listen address — the
// exact partial body that nulled everything else before the fix.
rec = post(`{"mitm_listen":"127.0.0.1:0"}`)
Expect(rec.Code).To(Equal(http.StatusOK), rec.Body.String())

raw, err := os.ReadFile(filepath.Join(tmp, "runtime_settings.json"))
Expect(err).ToNot(HaveOccurred())
var ondisk config.RuntimeSettings
Expect(json.Unmarshal(raw, &ondisk)).To(Succeed())

Expect(ondisk.MITMListen).ToNot(BeNil())
Expect(*ondisk.MITMListen).To(Equal("127.0.0.1:0"), "the changed field should be saved")
Expect(ondisk.Galleries).ToNot(BeNil(), "galleries were clobbered by the partial save")
Expect(*ondisk.Galleries).To(HaveLen(1))
Expect(ondisk.ApiKeys).ToNot(BeNil(), "api_keys were nulled by the partial save")
Expect(*ondisk.ApiKeys).To(Equal([]string{"k1"}))
Expect(ondisk.PIIDefaultDetectors).ToNot(BeNil(), "pii_default_detectors were nulled by the partial save")
Expect(*ondisk.PIIDefaultDetectors).To(Equal([]string{"det-a"}))
})

// Residual #9125: enabling the watchdog from a cold (off) state via the
// React master toggle must start the live watchdog immediately, without a
// restart. The toggle posts watchdog_idle_enabled/busy_enabled=true while
Expand Down
Loading