diff --git a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md index 7e9bc83107..bcae488f77 100644 --- a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md +++ b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md @@ -15,6 +15,19 @@ ## Purpose +### Shared origin and webhook edit boundaries + +The referenced `internal/api/` settings boundary treats explicit empty +`allowedOrigins` as a durable clear and omitted values as unchanged. Effective +GET values and deployment-owned overrides remain consistent; failed/rejected +saves cannot change live trust. This does not change agent identity, enrolment, +credential scope, execution permission, installer transport or TLS policy. +Notification Create/Update/Test consistently reject unresolved saved masks and +conflicting HTTP header identities before mutation or sending; literal edits, +masked responses and persist-before-publish remain intact. Source verification +uses `TestAllowedOrigins*` and `TestWebhookPlaceholderBoundaryAgreement` plus +the existing settings/notification authority and ordinary-delivery controls. + ### Cluster installation keyboard continuity `ClusterAgentDeployDialog` keeps one mounted dismissal button while loading, diff --git a/docs/release-control/v6/internal/subsystems/api-contracts.md b/docs/release-control/v6/internal/subsystems/api-contracts.md index 02781fb449..ae1493c62a 100644 --- a/docs/release-control/v6/internal/subsystems/api-contracts.md +++ b/docs/release-control/v6/internal/subsystems/api-contracts.md @@ -20,6 +20,39 @@ ## Purpose +### Origin-trust reversal and consistent webhook placeholders + +`POST /api/system/settings/update` treats the presence of a string +`allowedOrigins` as a patch: an explicit empty string removes saved and runtime +CORS trust, while omission changes neither. Null is not an empty-string clear. +Persist first; a failed save changes no runtime policy or settings cache. +`GET /api/system/settings` reports the effective value. Deployment-owned origins +remain authoritative: changing them returns `409 env_locked` before any save; +the form's unchanged locked value does not replace the underlying saved value +or prevent other settings edits. Authentication, settings scopes, CSRF, TLS, +comma-separated exact-origin matching and embedding policy stay independent. +`TestAllowedOriginsPatchBoundaries`, `TestAllowedOriginsSavedEffectiveLifecycle`, +`TestAllowedOriginsSaveFailurePreservesPolicy` and +`TestAllowedOriginsGetUsesDeploymentPolicy` bind these boundaries to the real +handlers, middleware, persisted settings, settings reload and production config +loader. This is source acceptance, not an installed deployment observation. + +Webhook Create, Update and saved-form Test use one masked-value resolver. +`***REDACTED***` means keep the corresponding saved value, not omit the field: +missing, masked or conflicting saved identities return `400` before saving, +live publication or a Test send. HTTP header names share case-insensitive +identity, including aliases alongside an exact match; custom-field keys remain +case-sensitive. Conflicting submitted header aliases are also rejected. +A masked signing secret needs a non-empty, non-placeholder saved secret. +Explicit additions/changes, omitted-map replacement/removals and blank-header +normalization retain the existing semantics; list/create/update responses stay +masked. Unsaved forms must supply literal values rather than saved masks. +`TestWebhookPlaceholderBoundaryAgreement` and +`TestWebhookSavedFormTestMatchesEncryptedEdit` exercise every entry point and a +real Test request plus encrypted edit/readback. Existing ordinary-delivery +integration retains grouped/ungrouped firing, resolution and restart controls; +these do not establish native Telegram acceptance or alter old queued jobs. + ### Existing disk History keeps native thermal fallback reachable The authenticated disk History endpoint supplements a missing or shallow diff --git a/docs/release-control/v6/internal/subsystems/notifications.md b/docs/release-control/v6/internal/subsystems/notifications.md index c0b64a6bca..81b2176a01 100644 --- a/docs/release-control/v6/internal/subsystems/notifications.md +++ b/docs/release-control/v6/internal/subsystems/notifications.md @@ -15,6 +15,28 @@ ## Purpose +### Saved webhook identity is consistent for editing and Test + +Create, Update and Test share one resolver for masked header/custom-field and +signing-secret values. Reject missing or ambiguous saved placeholders before +persisting, changing live destinations or sending a test request. HTTP header +identity is case-insensitive across the complete saved map, even where an exact +spelling is present; conflicting submitted aliases cannot depend on map order. +Custom fields retain exact, case-sensitive identity. A saved signing secret +must be non-empty and not itself a placeholder to satisfy a signing mask. + +The replacement map still honours explicit add/change/delete and omitted keys; +cleared headers are normalized away so the sender's JSON default survives. +List/create/update responses remain masked. Create and unsaved Test reject masks +instead of silently dropping credentials; a valid literal create still returns +masked values. Saved-form Test uses the same resolved edit as Update but does +not change live configuration or encrypted persistence. Persist-before-publish, +SSRF/TLS policy, routing/scopes, notification pause honesty and already-admitted +queue snapshots remain unchanged. Source controls +`TestWebhookPlaceholderBoundaryAgreement` and +`TestWebhookSavedFormTestMatchesEncryptedEdit` complement the existing real +ordinary-delivery/restart/grouping controls; none proves native #2540 relief. + ### Delivery health reconciliation follows the complete send transition The queue's automatic send worker records the retry/dead-letter outcome and diff --git a/docs/release-control/v6/internal/subsystems/security-privacy.md b/docs/release-control/v6/internal/subsystems/security-privacy.md index d87a1a3121..28c5431305 100644 --- a/docs/release-control/v6/internal/subsystems/security-privacy.md +++ b/docs/release-control/v6/internal/subsystems/security-privacy.md @@ -18,6 +18,26 @@ ## Purpose +### Removing browser-origin trust is a durable explicit patch + +The system settings handler accepts an explicit empty string for +`allowedOrigins` and applies it only after the durable save succeeds. Omitted +origins do not reapply a different saved value to the runtime. Null and +non-string values are invalid requests. GET presents the effective runtime +policy, not a saved value concealed by a deployment override. + +Either existing override marker (`ALLOWED_ORIGINS` or `allowedOrigins`) keeps +the deployment policy authoritative. A requested different value is rejected +with `409 env_locked` before any part of the patch is persisted. The unchanged +value sent by a disabled form field leaves underlying disk policy untouched +and permits other settings to be saved. Clearing origins grants no CORS headers; +production config reload does not revive removed trust. No change to the +existing development localhost fallback or deployment environment precedence +is made. CORS never replaces authentication, `settings:write`, CSRF, TLS or +iframe policy. The `TestAllowedOrigins*` controls in +`internal/api/system_settings_telemetry_test.go` cover real middleware responses, +exact-list matching, protected writes, rejected/failed saves and reload. + Assistant retained-history wiring uses the store belonging to its current monitor and read-only metric queries. Canonical identity resolution precedes store lookup, and ambiguous names fail with a request for canonical identity. diff --git a/docs/release-control/v6/internal/subsystems/storage-recovery.md b/docs/release-control/v6/internal/subsystems/storage-recovery.md index 254bd34b54..f14af8d245 100644 --- a/docs/release-control/v6/internal/subsystems/storage-recovery.md +++ b/docs/release-control/v6/internal/subsystems/storage-recovery.md @@ -21,6 +21,22 @@ ## Purpose +### Persisted trust edits and restart consistency + +The shared `internal/api/` settings path durably saves an explicit empty +`allowedOrigins` before clearing live CORS policy. An omitted field does not +replace a different effective value; a failed or deployment-locked patch does +not partially change disk/runtime/cache state. Production config reload retains +the saved clear while existing deployment overrides stay authoritative. +Webhook edits resolve every saved placeholder before persistence or live +publication, and saved-form Test uses the same resolution without changing +configuration. Unknown/ambiguous placeholders fail rather than silently lose a +credential. `TestAllowedOriginsSavedEffectiveLifecycle`, +`TestAllowedOriginsSaveFailurePreservesPolicy` and +`TestWebhookSavedFormTestMatchesEncryptedEdit` cover these shared boundaries. +No database/driver/schema, backup recovery, retention or old queue snapshot +policy changes are part of this repair. + ### Guest-memory read state stays independent of backup disk state The existing guest drawer retains Core's selected memory provenance through diff --git a/internal/api/alerting/notifications.go b/internal/api/alerting/notifications.go index ccf64de6dc..2c30e52232 100644 --- a/internal/api/alerting/notifications.go +++ b/internal/api/alerting/notifications.go @@ -422,41 +422,26 @@ func maskedWebhookResponse(webhook notifications.WebhookConfig) map[string]inter return whMap } -// restoreMaskedWebhookSecrets replaces each masked value in an incoming -// webhook with the saved value of the same header, custom field or signing -// secret. Restoring per key keeps any value the user typed in the same edit; -// a masked key with no saved value is dropped, so the mask is never stored or -// sent as a real value. -func restoreMaskedWebhookSecrets(incoming, saved notifications.WebhookConfig) notifications.WebhookConfig { - incoming.Headers = restoreMaskedWebhookValues(incoming.Headers, saved.Headers) - incoming.CustomFields = restoreMaskedWebhookValues(incoming.CustomFields, saved.CustomFields) +// restoreMaskedWebhookSecrets is the shared Create/Update/Test policy. A mask +// means keep this saved value, not discard the field or store the placeholder. +// Reject missing/ambiguous saved identities before persistence or any test send. +func restoreMaskedWebhookSecrets(incoming, saved notifications.WebhookConfig) (notifications.WebhookConfig, error) { + var err error + incoming.Headers, err = restoreStrictMaskedWebhookValues(incoming.Headers, saved.Headers, true) + if err != nil { + return notifications.WebhookConfig{}, err + } + incoming.CustomFields, err = restoreStrictMaskedWebhookValues(incoming.CustomFields, saved.CustomFields, false) + if err != nil { + return notifications.WebhookConfig{}, err + } if incoming.SigningSecret == maskedWebhookSecret { + if saved.SigningSecret == "" || saved.SigningSecret == maskedWebhookSecret { + return notifications.WebhookConfig{}, fmt.Errorf("no saved value for masked signing secret") + } incoming.SigningSecret = saved.SigningSecret } - return incoming -} - -func restoreMaskedWebhookValues(incoming, saved map[string]string) map[string]string { - if len(incoming) == 0 { - return incoming - } - restored := make(map[string]string, len(incoming)) - for key, value := range incoming { - if value != maskedWebhookSecret { - restored[key] = value - continue - } - if savedValue, ok := saved[key]; ok { - restored[key] = savedValue - } - } - return restored -} - -// stripMaskedWebhookSecrets drops masked values from a webhook that has no -// saved counterpart, such as a create or an unsaved form test. -func stripMaskedWebhookSecrets(webhook notifications.WebhookConfig) notifications.WebhookConfig { - return restoreMaskedWebhookSecrets(webhook, notifications.WebhookConfig{}) + return notifications.NormalizeWebhookConfig(incoming), nil } // CreateWebhook creates a new webhook @@ -479,7 +464,11 @@ func (h *NotificationHandlers) CreateWebhook(w http.ResponseWriter, r *http.Requ http.Error(w, err.Error(), http.StatusBadRequest) return } - webhook = notifications.NormalizeWebhookConfig(stripMaskedWebhookSecrets(webhook)) + webhook, err = restoreMaskedWebhookSecrets(webhook, notifications.WebhookConfig{}) + if err != nil { + http.Error(w, err.Error(), http.StatusBadRequest) + return + } monitor := h.getMonitor(r.Context()) manager := monitor.GetNotificationManager() @@ -518,25 +507,42 @@ func restoreStrictMaskedWebhookValues(incoming, existing map[string]string, head if incoming == nil { return nil, nil } - resolved := make(map[string]string, len(incoming)) - for key, value := range incoming { - if value == "***REDACTED***" { - stored, found := existing[key] - if !found && headers { - for oldKey, oldValue := range existing { - if strings.EqualFold(oldKey, key) { - if found && stored != oldValue { - return nil, fmt.Errorf("ambiguous saved value for masked header %q", key) - } - stored, found = oldValue, true - } - } + // Build the full HTTP identity index, even if an exact spelling exists: + // a legacy case-varied alias must not override or conceal a conflicting value. + savedHeaders := make(map[string]string) + ambiguousHeaders := make(map[string]bool) + if headers { + for key, value := range existing { + identity := strings.ToLower(key) + if previous, found := savedHeaders[identity]; found && previous != value { + ambiguousHeaders[identity] = true } - if !found || stored == "***REDACTED***" { + savedHeaders[identity] = value + } + } + resolved := make(map[string]string, len(incoming)) + incomingHeaders := make(map[string]string) + for key, value := range incoming { + identity := strings.ToLower(key) + if value == maskedWebhookSecret { + stored, found := existing[key] + if headers { + if ambiguousHeaders[identity] { + return nil, fmt.Errorf("ambiguous saved value for masked header %q", key) + } + stored, found = savedHeaders[identity] + } + if !found || stored == maskedWebhookSecret { return nil, fmt.Errorf("no saved value for masked field %q", key) } value = stored } + if headers { + if previous, found := incomingHeaders[identity]; found && previous != value { + return nil, fmt.Errorf("ambiguous submitted values for header %q", key) + } + incomingHeaders[identity] = value + } resolved[key] = value } return resolved, nil @@ -578,7 +584,6 @@ func (h *NotificationHandlers) UpdateWebhook(w http.ResponseWriter, r *http.Requ MinimumSeverity *string `json:"minimumSeverity"` } _ = json.Unmarshal(bodyBytes, &routingPresence) - webhook = notifications.NormalizeWebhookConfig(webhook) monitor := h.getMonitor(r.Context()) manager := monitor.GetNotificationManager() @@ -598,20 +603,11 @@ func (h *NotificationHandlers) UpdateWebhook(w http.ResponseWriter, r *http.Requ if routingPresence.MinimumSeverity == nil { webhook.MinimumSeverity = existing.MinimumSeverity } - webhook.Headers, err = restoreStrictMaskedWebhookValues(webhook.Headers, existing.Headers, true) + webhook, err = restoreMaskedWebhookSecrets(webhook, existing) if err != nil { http.Error(w, err.Error(), http.StatusBadRequest) return } - webhook.CustomFields, err = restoreStrictMaskedWebhookValues(webhook.CustomFields, existing.CustomFields, false) - if err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) - return - } - // Preserve the signing secret if the incoming value is redacted - if webhook.SigningSecret == "***REDACTED***" { - webhook.SigningSecret = existing.SigningSecret - } break } } @@ -619,7 +615,6 @@ func (h *NotificationHandlers) UpdateWebhook(w http.ResponseWriter, r *http.Requ http.Error(w, fmt.Sprintf("webhook not found: %s", webhookID), http.StatusNotFound) return } - webhook = notifications.NormalizeWebhookConfig(webhook) // Validate webhook URL if err := manager.ValidateWebhookURL(webhook.URL); err != nil { @@ -962,7 +957,11 @@ func (h *NotificationHandlers) TestWebhook(w http.ResponseWriter, r *http.Reques } } } - basicWebhook = notifications.NormalizeWebhookConfig(restoreMaskedWebhookSecrets(basicWebhook, saved)) + basicWebhook, err = restoreMaskedWebhookSecrets(basicWebhook, saved) + if err != nil { + http.Error(w, err.Error(), http.StatusBadRequest) + return + } // Try to extract service from body if present var serviceCheck struct { diff --git a/internal/api/alerting/notifications_test.go b/internal/api/alerting/notifications_test.go index d16f2204ed..fd2c682938 100644 --- a/internal/api/alerting/notifications_test.go +++ b/internal/api/alerting/notifications_test.go @@ -1603,6 +1603,8 @@ func TestWebhookMaskedEditPersistsAndDelivers(t *testing.T) { if edit.Headers["Authorization"] != "***REDACTED***" || edit.CustomFields["token"] != "***REDACTED***" { t.Fatal("list disclosed credentials") } + edit.Headers["authorization"] = edit.Headers["Authorization"] + delete(edit.Headers, "Authorization") edit.Headers["Content-Type"], edit.Headers["X-Route"] = "application/json", "new" delete(edit.Headers, "X-Remove") edit.CustomFields["channel"] = "new" @@ -1624,7 +1626,7 @@ func TestWebhookMaskedEditPersistsAndDelivers(t *testing.T) { monitor.manager = m saved := m.GetWebhooks()[0] if saved.Service != service || saved.Template != hook.Template || saved.URL != hook.URL || - saved.Headers["Authorization"] != "synthetic-auth" || saved.Headers["Content-Type"] != "application/json" || + saved.Headers["authorization"] != "synthetic-auth" || saved.Headers["Content-Type"] != "application/json" || saved.Headers["X-Route"] != "new" || len(saved.Headers) != 3 || saved.CustomFields["token"] != "synthetic-token" || saved.CustomFields["channel"] != "new" || len(saved.CustomFields) != 2 { t.Fatal("masked edit did not survive encrypted reload with explicit edits/removals intact") @@ -1819,7 +1821,7 @@ func TestWebhookMaskedValuesResolvePerKey(t *testing.T) { assert.Equal(t, "", published.SigningSecret, "an omitted signing secret is removed, as before") }) - t.Run("create drops masked values and responds masked", func(t *testing.T) { + t.Run("create keeps literal values and responds masked", func(t *testing.T) { h, manager, persistence := newHandlers() var added notifications.WebhookConfig manager.On("ValidateWebhookURL", "https://hooks.example.com/new").Return(nil).Once() @@ -1831,7 +1833,7 @@ func TestWebhookMaskedValuesResolvePerKey(t *testing.T) { body, _ := json.Marshal(map[string]interface{}{ "name": "New", "url": "https://hooks.example.com/new", "enabled": true, - "headers": map[string]string{"Authorization": "***REDACTED***", "X-Api-Key": "typed-key"}, + "headers": map[string]string{"X-Api-Key": "typed-key"}, "signingSecret": "typed-signing-secret", }) rec := httptest.NewRecorder() @@ -1872,25 +1874,187 @@ func TestWebhookMaskedValuesResolvePerKey(t *testing.T) { assert.Equal(t, "saved-signing-secret", tested.SigningSecret) }) - t.Run("form test of an unsaved webhook never sends the mask", func(t *testing.T) { + t.Run("form test of an unsaved webhook rejects the mask before sending", func(t *testing.T) { h, manager, _ := newHandlers() - var tested notifications.EnhancedWebhookConfig - manager.On("TestEnhancedWebhook", mock.Anything).Run(func(args mock.Arguments) { - tested = args.Get(0).(notifications.EnhancedWebhookConfig) - }).Return(200, "OK", nil).Once() - manager.On("IsEnabled").Return(true).Once() - body, _ := json.Marshal(map[string]interface{}{ "url": "https://hooks.example.com/new", - "headers": map[string]string{"Authorization": "***REDACTED***", "X-Team": "platform"}, + "headers": map[string]string{"Authorization": maskedWebhookSecret, "X-Team": "platform"}, }) rec := httptest.NewRecorder() h.TestWebhook(rec, httptest.NewRequest(http.MethodPost, "/api/notifications/webhooks/test", bytes.NewReader(body))) - - assert.Equal(t, http.StatusOK, rec.Code) - _, hasAuthorization := tested.Headers["Authorization"] - assert.False(t, hasAuthorization, "a mask with no saved value must not be sent") - assert.Equal(t, "platform", tested.Headers["X-Team"]) + assert.Equal(t, http.StatusBadRequest, rec.Code) + manager.AssertNotCalled(t, "TestEnhancedWebhook", mock.Anything) manager.AssertNotCalled(t, "GetWebhooks") }) } + +// A masked form must fail before persistence or a Test send if its saved +// identity is missing or ambiguous. Test and Update must resolve the same map. +func TestWebhookPlaceholderBoundaryAgreement(t *testing.T) { + for _, route := range []string{"update", "test", "create"} { + for _, tc := range []struct { + name string + savedHeaders, incomingHeaders, savedFields, incomingFields map[string]string + savedSigning, incomingSigning string + valid bool + }{ + {"case varied saved header", map[string]string{"Authorization": "synthetic-auth"}, map[string]string{"authorization": maskedWebhookSecret}, nil, nil, "", "", true}, + {"unknown header", nil, map[string]string{"X-Missing": maskedWebhookSecret}, nil, nil, "", "", false}, + {"stored header is a mask", map[string]string{"Authorization": maskedWebhookSecret}, map[string]string{"Authorization": maskedWebhookSecret}, nil, nil, "", "", false}, + {"exact match also has conflicting alias", map[string]string{"Authorization": "synthetic-one", "authorization": "synthetic-two"}, map[string]string{"Authorization": maskedWebhookSecret}, nil, nil, "", "", false}, + {"incoming aliases conflict", nil, map[string]string{"X-Route": "one", "x-route": "two"}, nil, nil, "", "", false}, + {"custom fields are case sensitive", nil, nil, map[string]string{"Token": "synthetic-token"}, map[string]string{"token": maskedWebhookSecret}, "", "", false}, + {"missing signing secret", nil, nil, nil, nil, "", maskedWebhookSecret, false}, + {"saved signing secret is a mask", nil, nil, nil, nil, maskedWebhookSecret, maskedWebhookSecret, false}, + } { + if route == "create" && tc.valid { + continue + } + t.Run(route+"/"+tc.name, func(t *testing.T) { + manager := new(MockNotificationManager) + persistence := new(MockNotificationConfigPersistence) + monitor := new(MockNotificationMonitor) + monitor.On("GetNotificationManager").Return(manager) + monitor.On("GetConfigPersistence").Return(persistence) + saved := notifications.WebhookConfig{ID: "saved", URL: "https://hooks.example.test/alerts", Headers: tc.savedHeaders, CustomFields: tc.savedFields, SigningSecret: tc.savedSigning} + manager.On("GetWebhooks").Return([]notifications.WebhookConfig{saved}).Maybe() + manager.On("ValidateWebhookURL", mock.Anything).Return(nil).Maybe() + manager.On("IsEnabled").Return(true).Maybe() + manager.On("UpdateWebhook", mock.Anything, mock.Anything).Return(nil).Maybe() + manager.On("AddWebhook", mock.Anything).Return().Maybe() + persistence.On("SaveWebhooks", mock.Anything).Return(nil).Maybe() + var tested notifications.EnhancedWebhookConfig + manager.On("TestEnhancedWebhook", mock.Anything).Run(func(a mock.Arguments) { tested = a.Get(0).(notifications.EnhancedWebhookConfig) }).Return(200, "OK", nil).Maybe() + incoming := notifications.WebhookConfig{ID: "saved", URL: saved.URL, Headers: tc.incomingHeaders, CustomFields: tc.incomingFields, SigningSecret: tc.incomingSigning} + body, err := json.Marshal(incoming) + if err != nil { + t.Fatal(err) + } + rec := httptest.NewRecorder() + h := NewNotificationHandlers(nil, monitor) + switch route { + case "create": + h.CreateWebhook(rec, httptest.NewRequest(http.MethodPost, "/api/notifications/webhooks", bytes.NewReader(body))) + case "update": + h.UpdateWebhook(rec, httptest.NewRequest(http.MethodPut, "/api/notifications/webhooks/saved", bytes.NewReader(body))) + case "test": + h.TestWebhook(rec, httptest.NewRequest(http.MethodPost, "/api/notifications/webhooks/test", bytes.NewReader(body))) + } + if tc.valid { + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want 200", rec.Code) + } + if route == "test" && tested.Headers["authorization"] != "synthetic-auth" { + t.Fatal("Test silently dropped the case-varied saved credential") + } + } else { + if rec.Code != http.StatusBadRequest { + t.Errorf("status = %d, want 400", rec.Code) + } + manager.AssertNotCalled(t, "TestEnhancedWebhook", mock.Anything) + manager.AssertNotCalled(t, "UpdateWebhook", mock.Anything, mock.Anything) + manager.AssertNotCalled(t, "AddWebhook", mock.Anything) + persistence.AssertNotCalled(t, "SaveWebhooks", mock.Anything) + } + for _, secret := range []string{"synthetic-auth", "synthetic-one", "synthetic-two", "synthetic-token"} { + if strings.Contains(rec.Body.String(), secret) { + t.Error("response disclosed a synthetic saved value") + } + } + }) + } + } +} + +func TestWebhookSavedFormTestMatchesEncryptedEdit(t *testing.T) { + type observation struct { + header http.Header + payload string + } + var mu sync.Mutex + var observed []observation + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var payload json.RawMessage + if err := json.NewDecoder(r.Body).Decode(&payload); err != nil { + w.WriteHeader(400) + return + } + mu.Lock() + observed = append(observed, observation{r.Header.Clone(), string(payload)}) + mu.Unlock() + fmt.Fprint(w, "OK") + })) + defer server.Close() + dir := t.TempDir() + persistence := config.NewConfigPersistence(dir) + saved := notifications.WebhookConfig{ID: "saved-form", Name: "Ops", URL: server.URL, Enabled: true, Service: "generic", Headers: map[string]string{"Authorization": "synthetic-auth", "Content-Type": "text/plain", "X-Delete": "remove"}, CustomFields: map[string]string{"token": "synthetic-token", "channel": "old", "delete": "remove"}, SigningSecret: "synthetic-signing"} + if err := persistence.SaveWebhooks([]notifications.WebhookConfig{saved}); err != nil { + t.Fatal(err) + } + manager := notifications.NewNotificationManagerWithDeferredQueue("", dir) + defer manager.Stop() + if err := manager.UpdateAllowedPrivateCIDRs("127.0.0.1/32,::1/128"); err != nil { + t.Fatal(err) + } + manager.AddWebhook(saved) + liveBefore := manager.GetWebhooks()[0] + h := NewNotificationHandlers(nil, &webhookEditMonitor{manager, persistence}) + listed := httptest.NewRecorder() + h.GetWebhooks(listed, httptest.NewRequest(http.MethodGet, "/api/notifications/webhooks", nil)) + var editable []notifications.WebhookConfig + if err := json.Unmarshal(listed.Body.Bytes(), &editable); err != nil || len(editable) != 1 { + t.Fatal("masked saved form unavailable") + } + form := editable[0] + form.Headers["authorization"] = form.Headers["Authorization"] + delete(form.Headers, "Authorization") + form.Headers["Content-Type"] = "" + delete(form.Headers, "X-Delete") + form.Headers["X-New"] = "new" + form.CustomFields["channel"] = "new" + delete(form.CustomFields, "delete") + body, err := json.Marshal(form) + if err != nil { + t.Fatal(err) + } + before, err := os.ReadFile(filepath.Join(dir, "webhooks.enc")) + if err != nil { + t.Fatal(err) + } + tested := httptest.NewRecorder() + h.TestWebhook(tested, httptest.NewRequest(http.MethodPost, "/api/notifications/webhooks/test", bytes.NewReader(body))) + if tested.Code != http.StatusOK { + t.Fatalf("saved form test: %d", tested.Code) + } + after, err := os.ReadFile(filepath.Join(dir, "webhooks.enc")) + if err != nil || !bytes.Equal(before, after) || !reflect.DeepEqual(manager.GetWebhooks()[0], liveBefore) { + t.Fatal("form Test mutated durable or live configuration") + } + mu.Lock() + got := append([]observation(nil), observed...) + mu.Unlock() + if len(got) != 1 || got[0].header.Get("Authorization") != "synthetic-auth" || got[0].header.Get("Content-Type") != "application/json" || got[0].header.Get("X-New") != "new" || got[0].header.Get("X-Delete") != "" || strings.Contains(got[0].payload, maskedWebhookSecret) { + t.Fatal("real Test did not honour saved identities, explicit edits/removals and JSON default") + } + updated := httptest.NewRecorder() + h.UpdateWebhook(updated, httptest.NewRequest(http.MethodPut, "/api/notifications/webhooks/saved-form", bytes.NewReader(body))) + if updated.Code != http.StatusOK { + t.Fatalf("saved form update: %d", updated.Code) + } + loaded, err := persistence.LoadWebhooks() + if err != nil || len(loaded) != 1 { + t.Fatal("encrypted edit unavailable after reload") + } + if loaded[0].Headers["authorization"] != "synthetic-auth" || loaded[0].Headers["X-New"] != "new" || len(loaded[0].Headers) != 2 || loaded[0].CustomFields["token"] != "synthetic-token" || loaded[0].CustomFields["channel"] != "new" || len(loaded[0].CustomFields) != 2 || loaded[0].SigningSecret != "synthetic-signing" { + t.Fatal("Test and persisted edit resolved different identities") + } + for _, response := range []string{listed.Body.String(), updated.Body.String(), tested.Body.String()} { + for _, secret := range []string{"synthetic-auth", "synthetic-token", "synthetic-signing"} { + if strings.Contains(response, secret) { + t.Fatal("configuration response disclosed a synthetic saved secret") + } + } + } + // The existing ordinary-delivery integration separately covers restart, + // grouped/un-grouped firing and resolution using the saved destination. +} diff --git a/internal/api/system_settings.go b/internal/api/system_settings.go index 77b991047e..c5951c71c1 100644 --- a/internal/api/system_settings.go +++ b/internal/api/system_settings.go @@ -380,6 +380,12 @@ func applyDiscoveryConfigOverrides(current config.DiscoveryConfig, cfgMap map[st // validateSystemSettings validates settings before applying them func validateSystemSettings(_ *config.SystemSettings, rawRequest map[string]interface{}) error { + if value, present := rawRequest["allowedOrigins"]; present { + if _, ok := value.(string); !ok { + return fmt.Errorf("allowedOrigins must be a string (use an empty string for same-origin only)") + } + } + if val, ok := rawRequest["reportBranding"]; ok { settings, ok := val.(map[string]interface{}) if !ok { @@ -713,6 +719,8 @@ func (h *SystemSettingsHandler) HandleGetSystemSettings(w http.ResponseWriter, r Msg("Loaded system settings for API response") settings.UpdateChannel = config.EffectiveUpdateChannel(settings.UpdateChannel, h.config.UpdateChannel) + // Expose the effective policy, including deployment overrides and clears. + settings.AllowedOrigins = h.config.AllowedOrigins // Always expose effective backup polling configuration settings.PVEPollingInterval = int(h.config.PVEPollingInterval.Seconds()) settings.BackupPollingInterval = int(h.config.BackupPollingInterval.Seconds()) @@ -866,6 +874,16 @@ func (h *SystemSettingsHandler) HandleUpdateSystemSettings(w http.ResponseWriter return } + // A disabled deployment-owned field is still included by the settings form. + // Accept an unchanged value without copying the override into system.json; + // reject attempts to change it, before persisting any part of the patch. + _, allowedOriginsUpdated := rawRequest["allowedOrigins"] + allowedOriginsLocked := h.config.EnvOverrides["ALLOWED_ORIGINS"] || h.config.EnvOverrides["allowedOrigins"] + if allowedOriginsUpdated && allowedOriginsLocked && updates.AllowedOrigins != h.config.AllowedOrigins { + writeErrorResponse(w, http.StatusConflict, "env_locked", "allowedOrigins is locked by the ALLOWED_ORIGINS environment variable", nil) + return + } + // Start with existing settings settings := *existingSettings discoveryConfigUpdated := false @@ -886,7 +904,7 @@ func (h *SystemSettingsHandler) HandleUpdateSystemSettings(w http.ResponseWriter if _, ok := rawRequest["backupPollingInterval"]; ok { settings.BackupPollingInterval = updates.BackupPollingInterval } - if updates.AllowedOrigins != "" { + if allowedOriginsUpdated && !allowedOriginsLocked { settings.AllowedOrigins = updates.AllowedOrigins } if _, ok := rawRequest["connectionTimeout"]; ok { @@ -1026,7 +1044,7 @@ func (h *SystemSettingsHandler) HandleUpdateSystemSettings(w http.ResponseWriter } h.config.PVEPollingInterval = newInterval } - if settings.AllowedOrigins != "" { + if allowedOriginsUpdated && !allowedOriginsLocked { h.config.AllowedOrigins = settings.AllowedOrigins } if settings.ConnectionTimeout > 0 { diff --git a/internal/api/system_settings_telemetry_test.go b/internal/api/system_settings_telemetry_test.go index b0fa5d75cc..bc955e59c7 100644 --- a/internal/api/system_settings_telemetry_test.go +++ b/internal/api/system_settings_telemetry_test.go @@ -748,3 +748,224 @@ func TestIssue1638SettingsSaveResetsSSHFailureBackoff(t *testing.T) { t.Fatalf("settings save called ResetSSHFailureBackoff %d times, want 1", monitor.resetSSHFailureBackoffCalls) } } + +// Clearing browser trust must be an explicit patch, not a falsy-value no-op. +func TestAllowedOriginsPatchBoundaries(t *testing.T) { + for _, tc := range []struct { + name, saved, effective, body, wantSaved, wantEffective string + override string + wantStatus int + }{ + {"clear wildcard", "*", "*", `{"allowedOrigins":""}`, "", "", "", http.StatusOK}, + {"clear exact origins", "https://old.example", "https://old.example", `{"allowedOrigins":""}`, "", "", "", http.StatusOK}, + {"omission keeps effective policy", "https://saved.example", "https://effective.example", `{"theme":"dark"}`, "https://saved.example", "https://effective.example", "", http.StatusOK}, + {"null is not a clear", "*", "*", `{"allowedOrigins":null}`, "*", "*", "", http.StatusBadRequest}, + {"deployment lock uppercase", "https://saved.example", "https://deploy.example", `{"allowedOrigins":""}`, "https://saved.example", "https://deploy.example", "ALLOWED_ORIGINS", http.StatusConflict}, + {"deployment lock field name", "https://saved.example", "https://deploy.example", `{"allowedOrigins":"*"}`, "https://saved.example", "https://deploy.example", "allowedOrigins", http.StatusConflict}, + {"unchanged locked form saves other fields", "https://saved.example", "https://deploy.example", `{"allowedOrigins":"https://deploy.example","theme":"dark"}`, "https://saved.example", "https://deploy.example", "allowedOrigins", http.StatusOK}, + } { + t.Run(tc.name, func(t *testing.T) { + dir := t.TempDir() + cfg := &config.Config{DataPath: dir, ConfigPath: dir, AllowedOrigins: tc.effective, EnvOverrides: map[string]bool{}} + if tc.override != "" { + cfg.EnvOverrides[tc.override] = true + } + h, persistence, token := setupTelemetryTest(t, cfg) + initial := config.DefaultSystemSettings() + initial.AllowedOrigins = tc.saved + if err := persistence.SaveSystemSettings(*initial); err != nil { + t.Fatal(err) + } + reloads := 0 + h.reloadSystemSettingsFunc = func() { reloads++ } + req := httptest.NewRequest(http.MethodPost, "/api/system/settings/update", strings.NewReader(tc.body)) + req.Header.Set("X-API-Token", token) + rec := httptest.NewRecorder() + h.HandleUpdateSystemSettings(rec, req) + if rec.Code != tc.wantStatus { + t.Fatalf("status = %d, want %d", rec.Code, tc.wantStatus) + } + saved, err := persistence.LoadSystemSettings() + if err != nil || saved == nil { + t.Fatalf("read saved policy: %v", err) + } + if saved.AllowedOrigins != tc.wantSaved || cfg.AllowedOrigins != tc.wantEffective { + t.Fatal("saved/effective policy did not honour field presence and deployment precedence") + } + if tc.wantStatus != http.StatusOK && reloads != 0 { + t.Fatal("rejected patch reloaded runtime state") + } + if tc.wantStatus == http.StatusOK && reloads != 1 { + t.Fatal("successful patch did not reload settings") + } + if tc.name == "unchanged locked form saves other fields" && saved.Theme != "dark" { + t.Fatal("locked value prevented an unrelated settings save") + } + }) + } +} + +// Exercise the production middleware and handlers, disk, settings-cache reload +// and the production config loader; no simulated setter stands in for policy. +func TestAllowedOriginsSavedEffectiveLifecycle(t *testing.T) { + for _, original := range []string{"*", "https://old.example"} { + t.Run(original, func(t *testing.T) { + dir := t.TempDir() + t.Setenv("PULSE_DATA_DIR", dir) + t.Setenv("ALLOWED_ORIGINS", "") + t.Setenv("PULSE_DEV", "false") + t.Setenv("NODE_ENV", "production") + InitPersistentAuthStores(dir) + t.Cleanup(resetSessionStoreForTests) + t.Cleanup(resetCSRFStoreForTests) + cfg := &config.Config{DataPath: dir, ConfigPath: dir, AllowedOrigins: original, PublicURL: "https://pulse.example", AuthUser: "admin", AuthPass: "synthetic-unused-hash", EnvOverrides: map[string]bool{}, TLSCertFile: "synthetic-cert", TLSKeyFile: "synthetic-key"} + h, persistence, token := setupTelemetryTest(t, cfg) + initial := config.DefaultSystemSettings() + initial.AllowedOrigins = original + initial.AllowedEmbedOrigins = "https://frame.example" + if err := persistence.SaveSystemSettings(*initial); err != nil { + t.Fatal(err) + } + router := &Router{config: cfg, persistence: persistence, mux: http.NewServeMux()} + h.reloadSystemSettingsFunc = router.reloadSystemSettings + router.mux.HandleFunc("/api/system/settings", RequireAdmin(cfg, RequireScope(config.ScopeSettingsRead, h.HandleGetSystemSettings))) + router.mux.HandleFunc("/api/system/settings/update", RequireAdmin(cfg, RequireScope(config.ScopeSettingsWrite, h.HandleUpdateSystemSettings))) + router.reloadSystemSettings() + request := func(method, path, origin, body, credential string) *httptest.ResponseRecorder { + req := httptest.NewRequest(method, path, strings.NewReader(body)) + if origin != "" { + req.Header.Set("Origin", origin) + } + if credential != "" { + req.Header.Set("X-API-Token", credential) + } + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + return rec + } + assertOrigin := func(origin, want string) { + t.Helper() + rec := request(http.MethodGet, "/api/system/settings", origin, "", token) + if rec.Code != http.StatusOK || rec.Header().Get("Access-Control-Allow-Origin") != want { + t.Fatal("effective CORS response does not match saved policy") + } + if rec.Header().Get("Access-Control-Allow-Credentials") != "" && want == "" { + t.Fatal("removed trust still enables credentialed CORS") + } + if want != "" && want != "*" && (rec.Header().Get("Access-Control-Allow-Credentials") != "true" || !strings.Contains(rec.Header().Get("Vary"), "Origin")) { + t.Fatal("exact-origin credential/Vary contract changed") + } + if want == "*" && rec.Header().Get("Access-Control-Allow-Credentials") != "" { + t.Fatal("wildcard gained credentialed browser access") + } + } + origin := "https://old.example" + assertOrigin(origin, original) + rec := request(http.MethodPost, "/api/system/settings/update", "", `{"allowedOrigins":""}`, token) + if rec.Code != http.StatusOK { + t.Fatalf("clear: status %d", rec.Code) + } + for _, candidate := range []string{origin, "https://untrusted.example", ""} { + assertOrigin(candidate, "") + } + preflight := request(http.MethodOptions, "/api/system/settings", origin, "", "") + if preflight.Code != http.StatusOK || preflight.Header().Get("Access-Control-Allow-Origin") != "" { + t.Fatal("cleared policy still grants preflight access") + } + get := request(http.MethodGet, "/api/system/settings", "", "", token) + var visible SystemSettingsResponse + if err := json.Unmarshal(get.Body.Bytes(), &visible); err != nil || visible.AllowedOrigins != "" { + t.Fatal("GET does not expose the effective clear") + } + saved, err := persistence.LoadSystemSettings() + if err != nil || saved == nil || saved.AllowedOrigins != "" || saved.AllowedEmbedOrigins != initial.AllowedEmbedOrigins || saved.AllowEmbedding { + t.Fatal("clear lost saved policy or widened embedding") + } + if cfg.TLSCertFile != "synthetic-cert" || cfg.TLSKeyFile != "synthetic-key" { + t.Fatal("CORS edit altered TLS configuration") + } + restarted, err := config.LoadWithoutLoggingInit() + if err != nil || restarted.AllowedOrigins != "" { + t.Fatal("production config reload revived removed trust") + } + if rec := request(http.MethodGet, "/api/system/settings", origin, "", ""); rec.Code != http.StatusUnauthorized { + t.Fatal("same-origin policy bypassed authentication") + } + // CORS grants never replace the session mutation's CSRF requirement. + session := generateSessionToken() + GetSessionStore().CreateSession(session, time.Hour, "cors-fixture", "192.0.2.1", "admin") + t.Cleanup(func() { GetSessionStore().DeleteSession(session) }) + req := httptest.NewRequest(http.MethodPost, "/api/system/settings/update", strings.NewReader(`{"allowedOrigins":"*"}`)) + req.Header.Set("User-Agent", "cors-fixture") + req.AddCookie(&http.Cookie{Name: sessionCookieName(false), Value: session}) + csrf := httptest.NewRecorder() + router.ServeHTTP(csrf, req) + if csrf.Code != http.StatusForbidden || cfg.AllowedOrigins != "" { + t.Fatal("CORS clear weakened CSRF") + } + const readToken = "cors-read-synthetic.12345678" + cfg.APITokens = append(cfg.APITokens, config.APITokenRecord{ID: "read-only", Hash: internalauth.HashAPIToken(readToken), Scopes: []string{config.ScopeSettingsRead}}) + if rec := request(http.MethodPost, "/api/system/settings/update", "", `{"allowedOrigins":"*"}`, readToken); rec.Code != http.StatusForbidden || cfg.AllowedOrigins != "" { + t.Fatal("settings:read token widened trust") + } + // An explicit new exact list works; prefixes and paths do not match. + rec = request(http.MethodPost, "/api/system/settings/update", "", `{"allowedOrigins":"https://trusted.example, https://other.example"}`, token) + if rec.Code != http.StatusOK { + t.Fatalf("set exact list: %d", rec.Code) + } + assertOrigin("https://trusted.example", "https://trusted.example") + assertOrigin("https://other.example", "https://other.example") + assertOrigin("https://trusted.example.evil", "") + assertOrigin("https://trusted.example/", "") + assertOrigin("", "") + }) + } +} + +func TestAllowedOriginsSaveFailurePreservesPolicy(t *testing.T) { + dir := t.TempDir() + cfg := &config.Config{DataPath: dir, ConfigPath: dir, AllowedOrigins: "https://trusted.example"} + h, persistence, token := setupTelemetryTest(t, cfg) + initial := config.DefaultSystemSettings() + initial.AllowedOrigins = cfg.AllowedOrigins + if err := persistence.SaveSystemSettings(*initial); err != nil { + t.Fatal(err) + } + before, err := os.ReadFile(filepath.Join(dir, "system.json")) + if err != nil { + t.Fatal(err) + } + persistence.SetFileSystem(failingWriteFileSystem{}) + reloads := 0 + h.reloadSystemSettingsFunc = func() { reloads++ } + req := httptest.NewRequest(http.MethodPost, "/api/system/settings/update", strings.NewReader(`{"allowedOrigins":""}`)) + req.Header.Set("X-API-Token", token) + rec := httptest.NewRecorder() + h.HandleUpdateSystemSettings(rec, req) + after, err := os.ReadFile(filepath.Join(dir, "system.json")) + if err != nil { + t.Fatal(err) + } + if rec.Code != http.StatusInternalServerError || cfg.AllowedOrigins != initial.AllowedOrigins || reloads != 0 || !bytes.Equal(before, after) { + t.Fatal("failed save changed durable/effective browser trust") + } +} + +func TestAllowedOriginsGetUsesDeploymentPolicy(t *testing.T) { + dir := t.TempDir() + cfg := &config.Config{DataPath: dir, ConfigPath: dir, AllowedOrigins: "https://deploy.example", EnvOverrides: map[string]bool{"allowedOrigins": true}} + h, persistence, token := setupTelemetryTest(t, cfg) + initial := config.DefaultSystemSettings() + initial.AllowedOrigins = "https://saved.example" + if err := persistence.SaveSystemSettings(*initial); err != nil { + t.Fatal(err) + } + req := httptest.NewRequest(http.MethodGet, "/api/system/settings", nil) + req.Header.Set("X-API-Token", token) + rec := httptest.NewRecorder() + h.HandleGetSystemSettings(rec, req) + var response SystemSettingsResponse + if err := json.Unmarshal(rec.Body.Bytes(), &response); err != nil || rec.Code != http.StatusOK || response.AllowedOrigins != cfg.AllowedOrigins || !response.EnvOverrides["allowedOrigins"] { + t.Fatal("GET misrepresented deployment-owned browser trust") + } +}