[management] Honour If-Match on Agent Network settings writes

With the validator derived and the helper in place, wire both together: reads
and writes hand back an ETag, and PUT and DELETE honour an If-Match against it.
A client that plans against one read and writes the whole object back now gets
refused instead of silently reverting whatever changed in between.

The comparison happens inside the write's own transaction, under the row lock
the fetch already takes, rather than in the handler. Comparing before the
transaction merely narrows the race: two writers read the same row, both find
their precondition satisfied, and both then write. That is the lost update
this is meant to prevent, and the concurrency test added here fails against
that shape while passing against this one.

It also runs ahead of each operation's other guards. A caller stale enough to
be holding an old validator is stale in its identity echo too, so answering
"you are working from an old read" is more useful than "the endpoint is
immutable"; on delete it matters more still, because the provider guard raises
the same 412 and would send a stale client hunting for providers it may not
know exist.

If-Match stays optional. The dashboard is human-driven, where last-write-wins
is acceptable and a surprise 412 is worse than a benign overwrite; Terraform
is machine-driven and can opt in. Making it mandatory would cost a dashboard
change and should use 428 when that day comes.

GET emits a validator for the pre-bootstrap defaults as well: they are a
representation like any other, and validating them means an If-Match taken
before bootstrap cannot match the row that appeared since.

management/internals/modules/agentnetwork/handlers/handlers_test.go

@@ -92,6 +92,13 @@ func newAgentNetworkHandlerFixture(t *testing.T) *agentNetworkHandlerFixture {
 }
 
 func (f *agentNetworkHandlerFixture) do(t *testing.T, method, path, body string) *httptest.ResponseRecorder {
+	t.Helper()
+	return f.doWithHeaders(t, method, path, body, nil)
+}
+
+// doWithHeaders is do with request headers, for the cases where the header is
+// the thing under test (conditional requests).
+func (f *agentNetworkHandlerFixture) doWithHeaders(t *testing.T, method, path, body string, headers map[string]string) *httptest.ResponseRecorder {
 	t.Helper()
 	var reader io.Reader
 	if body != "" {
@@ -101,6 +108,9 @@ func (f *agentNetworkHandlerFixture) do(t *testing.T, method, path, body string)
 	if body != "" {
 		req.Header.Set("Content-Type", "application/json")
 	}
+	for name, value := range headers {
+		req.Header.Set(name, value)
+	}
 	req = nbcontext.SetUserAuthInRequest(req, auth.UserAuth{
 		UserId:    testUserID,
 		AccountId: testAccountID,

management/internals/modules/agentnetwork/handlers/settings_handler.go

@@ -60,12 +60,20 @@ func (h *handler) createSettings(w http.ResponseWriter, r *http.Request) {
 		util.WriteError(r.Context(), err, w)
 		return
 	}
+	// Emitting the validator here lets a client that just bootstrapped issue a
+	// conditional PUT without an intervening GET.
+	util.SetETag(w, created.ETag())
 	util.WriteJSONObject(r.Context(), w, created.ToAPIResponse())
 }
 
 // updateSettings replaces the mutable settings fields on the account's row.
 // A request carrying a cluster bootstraps the row when the account doesn't
 // have one yet.
+//
+// An If-Match header makes the update conditional: it is honoured against the
+// stored row inside the write's transaction, and a stale validator is refused
+// with 412 rather than overwriting what changed since the client read. Omitting
+// the header keeps the pre-existing last-write-wins behaviour.
 func (h *handler) updateSettings(w http.ResponseWriter, r *http.Request) {
 	userAuth, err := nbcontext.GetUserAuthFromContext(r.Context())
 	if err != nil {
@@ -82,11 +90,12 @@ func (h *handler) updateSettings(w http.ResponseWriter, r *http.Request) {
 	settings := &types.Settings{AccountID: userAuth.AccountId}
 	settings.FromAPIRequest(&req)
 
-	updated, err := h.manager.UpdateSettings(r.Context(), userAuth.UserId, settings)
+	updated, err := h.manager.UpdateSettings(r.Context(), userAuth.UserId, settings, util.IfMatch(r))
 	if err != nil {
 		util.WriteError(r.Context(), err, w)
 		return
 	}
+	util.SetETag(w, updated.ETag())
 	util.WriteJSONObject(r.Context(), w, updated.ToAPIResponse())
 }
 
@@ -94,6 +103,11 @@ func (h *handler) updateSettings(w http.ResponseWriter, r *http.Request) {
 // The manager refuses (412) while providers exist or a proxy is actively
 // serving the endpoint; a later POST bootstraps fresh, allocating a new
 // endpoint.
+//
+// An If-Match header makes the delete conditional, and is worth sending here
+// even more than on update: both existing guards are about state rather than
+// staleness, so nothing else stops a client from deleting a row that was
+// replaced since it read one.
 func (h *handler) deleteSettings(w http.ResponseWriter, r *http.Request) {
 	userAuth, err := nbcontext.GetUserAuthFromContext(r.Context())
 	if err != nil {
@@ -101,7 +115,7 @@ func (h *handler) deleteSettings(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	if err := h.manager.DeleteSettings(r.Context(), userAuth.AccountId, userAuth.UserId); err != nil {
+	if err := h.manager.DeleteSettings(r.Context(), userAuth.AccountId, userAuth.UserId, util.IfMatch(r)); err != nil {
 		util.WriteError(r.Context(), err, w)
 		return
 	}
@@ -123,5 +137,9 @@ func (h *handler) getSettings(w http.ResponseWriter, r *http.Request) {
 		util.WriteError(r.Context(), err, w)
 		return
 	}
+	// The pre-bootstrap defaults are a representation like any other and carry
+	// a validator too, so an If-Match taken before bootstrap cannot silently
+	// match the row that appeared since.
+	util.SetETag(w, settings.ETag())
 	util.WriteJSONObject(r.Context(), w, settings.ToAPIResponse())
 }

management/internals/modules/agentnetwork/handlers/settings_handler_test.go

@@ -393,3 +393,202 @@ func TestSettingsHandler_DeleteReleasesEndpointForFreshBootstrap(t *testing.T) {
 		"the fresh row must carry bootstrap defaults, not the deleted row's toggles")
 	assert.NotNil(t, second.CreatedAt, "the fresh row is persisted and carries timestamps")
 }
+
+// bootstrapForETag bootstraps a settings row and returns the response body
+// alongside the validator the bootstrap emitted, which is what a client would
+// carry into its first conditional write.
+func bootstrapForETag(t *testing.T, f *agentNetworkHandlerFixture) (api.AgentNetworkSettings, string) {
+	t.Helper()
+
+	rec := f.do(t, http.MethodPost, "/agent-network/settings",
+		`{"proxy_address": "eu.proxy.netbird.io", "enable_prompt_collection": true, "redact_pii": true, "access_log_retention_days": 14}`)
+	require.Equal(t, http.StatusOK, rec.Code, "bootstrap POST must succeed: %s", rec.Body.String())
+
+	var settings api.AgentNetworkSettings
+	require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &settings))
+
+	etag := rec.Header().Get("ETag")
+	require.NotEmpty(t, etag, "bootstrap must emit a validator so a client can PUT without an intervening GET")
+	return settings, etag
+}
+
+// putBody renders a complete settings update — every field, with the identity
+// echo the endpoint requires — so the conditional-request tests differ only in
+// their headers.
+func putBody(settings api.AgentNetworkSettings, redactPii bool, retention int) string {
+	return fmt.Sprintf(
+		`{"endpoint": %q, "proxy_address": %q, "enable_log_collection": true, "enable_prompt_collection": true, "redact_pii": %t, "access_log_retention_days": %d}`,
+		settings.Endpoint, settings.ProxyAddress, redactPii, retention)
+}
+
+// TestSettingsHandler_EmitsETag pins that every read and every write hands the
+// client back a validator, quoted as a strong entity-tag. Without one on the
+// write responses a client would have to re-GET after every update to stay
+// able to make the next one conditional.
+func TestSettingsHandler_EmitsETag(t *testing.T) {
+	f := newAgentNetworkHandlerFixture(t)
+
+	// The pre-bootstrap defaults are a representation too, and validate like
+	// one — an If-Match taken here must not match the row that appears later.
+	rec := f.do(t, http.MethodGet, "/agent-network/settings", "")
+	require.Equal(t, http.StatusOK, rec.Code)
+	defaultsETag := rec.Header().Get("ETag")
+	assert.NotEmpty(t, defaultsETag, "the unbootstrapped view must carry a validator")
+
+	settings, bootstrapETag := bootstrapForETag(t, f)
+	assert.Regexp(t, `^"[0-9a-f]+"$`, bootstrapETag, "the validator must be a quoted strong entity-tag")
+	assert.NotEqual(t, defaultsETag, bootstrapETag, "bootstrapping must move the validator")
+
+	rec = f.do(t, http.MethodGet, "/agent-network/settings", "")
+	require.Equal(t, http.StatusOK, rec.Code)
+	assert.Equal(t, bootstrapETag, rec.Header().Get("ETag"),
+		"reading an unchanged row must derive the same validator the bootstrap returned")
+
+	rec = f.do(t, http.MethodPut, "/agent-network/settings", putBody(settings, false, 7))
+	require.Equal(t, http.StatusOK, rec.Code, "update must succeed: %s", rec.Body.String())
+	assert.NotEqual(t, bootstrapETag, rec.Header().Get("ETag"),
+		"an update that changed the representation must return a different validator")
+}
+
+// TestSettingsHandler_PutIfMatch walks the conditional-update contract. The
+// stale case is the one the feature exists for: a client that planned against
+// an earlier read must be refused rather than silently reverting whatever
+// changed in between — RedactPii above all, where a silent revert turns a
+// compliance control off with no error and no drift warning.
+func TestSettingsHandler_PutIfMatch(t *testing.T) {
+	t.Run("matching validator succeeds", func(t *testing.T) {
+		f := newAgentNetworkHandlerFixture(t)
+		settings, etag := bootstrapForETag(t, f)
+
+		rec := f.doWithHeaders(t, http.MethodPut, "/agent-network/settings",
+			putBody(settings, false, 7), map[string]string{"If-Match": etag})
+		require.Equal(t, http.StatusOK, rec.Code,
+			"a matching precondition must be honoured: got %d body=%s", rec.Code, rec.Body.String())
+		assert.NotEqual(t, etag, rec.Header().Get("ETag"),
+			"the response must carry the new validator, not the one that was matched")
+	})
+
+	t.Run("stale validator is refused and changes nothing", func(t *testing.T) {
+		f := newAgentNetworkHandlerFixture(t)
+		settings, stale := bootstrapForETag(t, f)
+
+		// Someone else writes in between — the dashboard operator enabling
+		// something the planning client never saw.
+		rec := f.do(t, http.MethodPut, "/agent-network/settings", putBody(settings, true, 21))
+		require.Equal(t, http.StatusOK, rec.Code, "the intervening update must succeed: %s", rec.Body.String())
+		var intervened api.AgentNetworkSettings
+		require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &intervened))
+
+		rec = f.doWithHeaders(t, http.MethodPut, "/agent-network/settings",
+			putBody(settings, false, 7), map[string]string{"If-Match": stale})
+		require.Equal(t, http.StatusPreconditionFailed, rec.Code,
+			"a stale precondition must be refused: got %d body=%s", rec.Code, rec.Body.String())
+
+		// Asserting the state, not just the status: a partial write would pass
+		// a status-only check.
+		rec = f.do(t, http.MethodGet, "/agent-network/settings", "")
+		require.Equal(t, http.StatusOK, rec.Code)
+		var after api.AgentNetworkSettings
+		require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &after))
+		assert.Equal(t, intervened, after, "the refused update must leave the row byte-identical")
+	})
+
+	t.Run("star matches the existing row", func(t *testing.T) {
+		f := newAgentNetworkHandlerFixture(t)
+		settings, _ := bootstrapForETag(t, f)
+
+		rec := f.doWithHeaders(t, http.MethodPut, "/agent-network/settings",
+			putBody(settings, false, 7), map[string]string{"If-Match": "*"})
+		assert.Equal(t, http.StatusOK, rec.Code,
+			"* must match any current representation: got %d body=%s", rec.Code, rec.Body.String())
+	})
+
+	t.Run("no precondition still succeeds", func(t *testing.T) {
+		f := newAgentNetworkHandlerFixture(t)
+		settings, _ := bootstrapForETag(t, f)
+
+		// The back-compatibility guarantee: clients that predate conditional
+		// requests — the dashboard among them — keep last-write-wins.
+		rec := f.do(t, http.MethodPut, "/agent-network/settings", putBody(settings, false, 7))
+		assert.Equal(t, http.StatusOK, rec.Code,
+			"an unconditional update must keep working: got %d body=%s", rec.Code, rec.Body.String())
+	})
+
+	t.Run("precondition is checked before the immutability echo", func(t *testing.T) {
+		f := newAgentNetworkHandlerFixture(t)
+		settings, stale := bootstrapForETag(t, f)
+
+		rec := f.do(t, http.MethodPut, "/agent-network/settings", putBody(settings, true, 21))
+		require.Equal(t, http.StatusOK, rec.Code, "the intervening update must succeed: %s", rec.Body.String())
+
+		// A client stale enough to hold an old validator may be stale in its
+		// identity echo too. Answering 412 tells it the useful thing — go and
+		// read again — where 422 would send it hunting an immutability bug.
+		body := fmt.Sprintf(
+			`{"endpoint": "other.gateway.example.com", "proxy_address": %q, "enable_log_collection": true, "enable_prompt_collection": true, "redact_pii": false, "access_log_retention_days": 7}`,
+			settings.ProxyAddress)
+		rec = f.doWithHeaders(t, http.MethodPut, "/agent-network/settings", body,
+			map[string]string{"If-Match": stale})
+		assert.Equal(t, http.StatusPreconditionFailed, rec.Code,
+			"staleness must be reported ahead of the identity mismatch: got %d body=%s", rec.Code, rec.Body.String())
+	})
+}
+
+// TestSettingsHandler_DeleteIfMatch covers the conditional delete, which
+// carries more weight than the conditional update: both existing delete guards
+// are about state — no providers, no serving proxy — so nothing else stops a
+// client from deleting a row that was replaced since it read one.
+func TestSettingsHandler_DeleteIfMatch(t *testing.T) {
+	t.Run("stale validator is refused and the row survives", func(t *testing.T) {
+		f := newAgentNetworkHandlerFixture(t)
+		settings, stale := bootstrapForETag(t, f)
+
+		rec := f.do(t, http.MethodPut, "/agent-network/settings", putBody(settings, true, 21))
+		require.Equal(t, http.StatusOK, rec.Code, "the intervening update must succeed: %s", rec.Body.String())
+
+		rec = f.doWithHeaders(t, http.MethodDelete, "/agent-network/settings", "",
+			map[string]string{"If-Match": stale})
+		require.Equal(t, http.StatusPreconditionFailed, rec.Code,
+			"a stale precondition must refuse the delete: got %d body=%s", rec.Code, rec.Body.String())
+
+		rec = f.do(t, http.MethodGet, "/agent-network/settings", "")
+		require.Equal(t, http.StatusOK, rec.Code)
+		var after api.AgentNetworkSettings
+		require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &after))
+		assert.Equal(t, settings.Endpoint, after.Endpoint, "the refused delete must leave the row in place")
+	})
+
+	t.Run("matching validator deletes", func(t *testing.T) {
+		f := newAgentNetworkHandlerFixture(t)
+		_, etag := bootstrapForETag(t, f)
+
+		rec := f.doWithHeaders(t, http.MethodDelete, "/agent-network/settings", "",
+			map[string]string{"If-Match": etag})
+		require.Equal(t, http.StatusOK, rec.Code,
+			"a matching precondition must be honoured: got %d body=%s", rec.Code, rec.Body.String())
+
+		rec = f.do(t, http.MethodGet, "/agent-network/settings", "")
+		require.Equal(t, http.StatusOK, rec.Code)
+		var after api.AgentNetworkSettings
+		require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &after))
+		assert.Empty(t, after.Endpoint, "the row must be gone")
+	})
+
+	t.Run("precondition is checked before the state guards", func(t *testing.T) {
+		f := newAgentNetworkHandlerFixture(t)
+		settings, stale := bootstrapForETag(t, f)
+
+		rec := f.do(t, http.MethodPut, "/agent-network/settings", putBody(settings, true, 21))
+		require.Equal(t, http.StatusOK, rec.Code, "the intervening update must succeed: %s", rec.Body.String())
+		f.seedProvider(t, "prov-precondition")
+
+		// Both refusals are 412, so the status cannot tell them apart — the
+		// message must, or a stale client is sent to delete providers it may
+		// not even know about.
+		rec = f.doWithHeaders(t, http.MethodDelete, "/agent-network/settings", "",
+			map[string]string{"If-Match": stale})
+		require.Equal(t, http.StatusPreconditionFailed, rec.Code, "the delete must be refused: %s", rec.Body.String())
+		assert.Contains(t, rec.Body.String(), "if-match",
+			"staleness must be reported ahead of the provider guard: %s", rec.Body.String())
+	})
+}

management/internals/modules/agentnetwork/manager.go

@@ -22,6 +22,7 @@ import (
 	"github.com/netbirdio/netbird/management/server/permissions/modules"
 	"github.com/netbirdio/netbird/management/server/permissions/operations"
 	"github.com/netbirdio/netbird/management/server/store"
+	httputil "github.com/netbirdio/netbird/shared/management/http/util"
 	"github.com/netbirdio/netbird/shared/management/status"
 )
 
@@ -71,8 +72,8 @@ type Manager interface {
 
 	GetSettings(ctx context.Context, accountID, userID string) (*types.Settings, error)
 	CreateSettings(ctx context.Context, userID string, settings *types.Settings, proxyAddress, endpoint string) (*types.Settings, error)
-	UpdateSettings(ctx context.Context, userID string, settings *types.Settings) (*types.Settings, error)
-	DeleteSettings(ctx context.Context, accountID, userID string) error
+	UpdateSettings(ctx context.Context, userID string, settings *types.Settings, precondition *httputil.Precondition) (*types.Settings, error)
+	DeleteSettings(ctx context.Context, accountID, userID string, precondition *httputil.Precondition) error
 
 	ListConsumption(ctx context.Context, accountID, userID string) ([]*types.Consumption, error)
 	ListAccessLogs(ctx context.Context, accountID, userID string, filter types.AgentNetworkAccessLogFilter) ([]*types.AgentNetworkAccessLog, int64, error)
@@ -554,7 +555,11 @@ func (m *managerImpl) DeleteBudgetRule(ctx context.Context, accountID, userID, r
 // Because the collection toggles change the synthesised service config
 // (prompt-capture gating, access-log emission), a reconcile is triggered so
 // the proxy and peer network maps converge on the new state.
-func (m *managerImpl) UpdateSettings(ctx context.Context, userID string, settings *types.Settings) (*types.Settings, error) {
+//
+// precondition carries the caller's If-Match, and is nil for an unconditional
+// update — last write wins, which is what the dashboard wants and what every
+// client that predates conditional requests gets.
+func (m *managerImpl) UpdateSettings(ctx context.Context, userID string, settings *types.Settings, precondition *httputil.Precondition) (*types.Settings, error) {
 	if err := m.requirePermission(ctx, settings.AccountID, userID, modules.AgentNetworkSettings, operations.Update); err != nil {
 		return nil, err
 	}
@@ -573,6 +578,20 @@ func (m *managerImpl) UpdateSettings(ctx context.Context, userID string, setting
 			return fmt.Errorf("get agent network settings: %w", err)
 		}
 
+		// Evaluated here, under the row lock and inside the write's own
+		// transaction, rather than in the handler: comparing before the
+		// transaction only narrows the race, since two requests can both pass
+		// the check before either writes. Locking the row first makes it a
+		// genuine compare-and-set.
+		//
+		// It comes before the identity comparison because a client holding a
+		// stale validator is stale in its identity echo too, and "you are
+		// working from an old read" is the more accurate answer than "the
+		// endpoint is immutable".
+		if !precondition.Matches(existing.ETag()) {
+			return status.Errorf(status.PreconditionFailed, "if-match precondition failed: the settings have changed since they were read; GET them again and retry")
+		}
+
 		// The identity echo is compared leniently (trimmed, case-insensitive):
 		// the stored values are normalized lowercase, and a client replaying a
 		// GET response must never be rejected over casing it didn't choose.
@@ -635,7 +654,12 @@ func hostnamesEquivalent(supplied, stored string) bool {
 // is not reserved. That full-reset semantic is what gives clients that model
 // immutability as replace-on-change (e.g. Terraform's RequiresReplace) a real
 // path: tear down providers, delete, re-create.
-func (m *managerImpl) DeleteSettings(ctx context.Context, accountID, userID string) error {
+//
+// precondition carries the caller's If-Match, and is nil for an unconditional
+// delete. It matters more here than on update: the two guards above are about
+// state rather than staleness, so without it nothing stops a client from
+// deleting a row that was replaced since it last read one.
+func (m *managerImpl) DeleteSettings(ctx context.Context, accountID, userID string, precondition *httputil.Precondition) error {
 	if err := m.requirePermission(ctx, accountID, userID, modules.AgentNetworkSettings, operations.Delete); err != nil {
 		return err
 	}
@@ -651,6 +675,13 @@ func (m *managerImpl) DeleteSettings(ctx context.Context, accountID, userID stri
 			return fmt.Errorf("get agent network settings: %w", err)
 		}
 
+		// Under the row lock, for the same reason as in UpdateSettings, and
+		// before the state guards: a caller working from an old read should
+		// learn that first, not be told about providers it may not know exist.
+		if !precondition.Matches(existing.ETag()) {
+			return status.Errorf(status.PreconditionFailed, "if-match precondition failed: the settings have changed since they were read; GET them again and retry")
+		}
+
 		providers, err := tx.GetAccountAgentNetworkProviders(ctx, store.LockingStrengthNone, accountID)
 		if err != nil {
 			return fmt.Errorf("get agent network providers: %w", err)
@@ -1100,11 +1131,13 @@ func (*mockManager) CreateSettings(_ context.Context, _ string, s *types.Setting
 	return s, nil
 }
 
-func (*mockManager) UpdateSettings(_ context.Context, _ string, s *types.Settings) (*types.Settings, error) {
+func (*mockManager) UpdateSettings(_ context.Context, _ string, s *types.Settings, _ *httputil.Precondition) (*types.Settings, error) {
 	return s, nil
 }
 
-func (*mockManager) DeleteSettings(_ context.Context, _, _ string) error { return nil }
+func (*mockManager) DeleteSettings(_ context.Context, _, _ string, _ *httputil.Precondition) error {
+	return nil
+}
 
 func (*mockManager) ListConsumption(_ context.Context, _, _ string) ([]*types.Consumption, error) {
 	return nil, nil

management/internals/modules/agentnetwork/settings_etag_test.go

@@ -0,0 +1,191 @@
+package agentnetwork
+
+import (
+	"context"
+	"errors"
+	"net/http"
+	"net/http/httptest"
+	"strconv"
+	"sync"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/netbirdio/netbird/management/internals/modules/agentnetwork/types"
+	"github.com/netbirdio/netbird/management/server/permissions/modules"
+	"github.com/netbirdio/netbird/management/server/permissions/operations"
+	"github.com/netbirdio/netbird/management/server/store"
+	httputil "github.com/netbirdio/netbird/shared/management/http/util"
+	"github.com/netbirdio/netbird/shared/management/status"
+)
+
+// ifMatch builds the precondition a client sending this validator would
+// produce, by going through the same header parse the handler uses rather than
+// reaching past it.
+func ifMatch(t *testing.T, etag string) *httputil.Precondition {
+	t.Helper()
+
+	r := httptest.NewRequest(http.MethodPut, "/", nil)
+	r.Header.Set("If-Match", strconv.Quote(etag))
+	return httputil.IfMatch(r)
+}
+
+// updateFor renders a complete update for the given row, echoing the identity
+// fields the endpoint requires and setting retention to tell writers apart.
+func updateFor(settings *types.Settings, retention int) *types.Settings {
+	return &types.Settings{
+		AccountID:              settings.AccountID,
+		Domain:                 settings.Domain,
+		ProxyAddress:           settings.ProxyAddress,
+		EnableLogCollection:    true,
+		EnablePromptCollection: true,
+		RedactPii:              true,
+		AccessLogRetentionDays: retention,
+	}
+}
+
+// TestUpdateSettingsPreconditionSerializesConcurrentWriters is the test the
+// design rests on. Two writers start from the same validator and race; exactly
+// one may win.
+//
+// An implementation that compares the validator before opening the write
+// transaction passes every sequential test in this suite and fails here: both
+// writers read the same row, both find their precondition satisfied, and both
+// then write — which is the lost update the feature exists to prevent, merely
+// narrowed to a smaller window. Holding the row under LockingStrengthUpdate
+// and comparing inside the write's own transaction is what makes it a genuine
+// compare-and-set.
+//
+// The test store is sqlite, which serializes writers of its own accord, so
+// what this pins directly is the outcome — exactly one success — rather than
+// the mechanism. It still has teeth against the check-before-transaction
+// shape, whose two reads interleave freely before either write. Running it
+// against postgres (NB_STORE_ENGINE_POSTGRES_DSN) exercises real concurrent
+// transactions.
+func TestUpdateSettingsPreconditionSerializesConcurrentWriters(t *testing.T) {
+	ctx := context.Background()
+	f := newBootstrapFixture(t)
+
+	const accountID, userID = "account1", "user1"
+	f.expectPermission(accountID, userID, modules.AgentNetworkSettings, operations.Create, true)
+	f.expectPermission(accountID, userID, modules.AgentNetworkSettings, operations.Update, true)
+	f.expectPermission(accountID, userID, modules.AgentNetworkSettings, operations.Update, true)
+
+	created, err := f.createSettings(ctx, accountID, userID, "cluster1.example.com", "")
+	require.NoError(t, err, "bootstrap must succeed")
+
+	// Both writers plan against this one read, as a client that read, computed
+	// a diff and is about to write the whole object back would.
+	shared := created.ETag()
+
+	var (
+		wg     sync.WaitGroup
+		start  = make(chan struct{})
+		errs   = make([]error, 2)
+		wrote  = []int{7, 21}
+		winner = make([]int, 2)
+	)
+	for i := range 2 {
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+			<-start
+			updated, err := f.manager.UpdateSettings(ctx, userID, updateFor(created, wrote[i]), ifMatch(t, shared))
+			errs[i] = err
+			if err == nil {
+				winner[i] = updated.AccessLogRetentionDays
+			}
+		}()
+	}
+	close(start)
+	wg.Wait()
+
+	succeeded := 0
+	for i, err := range errs {
+		if err == nil {
+			succeeded++
+			continue
+		}
+		assert.Truef(t, isPreconditionFailed(err),
+			"the losing writer must be refused for staleness, got: %v (writer %d)", err, i)
+	}
+	require.Equal(t, 1, succeeded, "exactly one writer may win: %v", errs)
+
+	// The row must carry the winner's value and nothing blended.
+	stored, err := f.store.GetAgentNetworkSettings(ctx, store.LockingStrengthNone, accountID)
+	require.NoError(t, err, "the row must survive the race")
+	assert.Contains(t, winner, stored.AccessLogRetentionDays,
+		"the stored row must be exactly what the winning writer sent")
+	assert.NotEqual(t, shared, stored.ETag(), "the surviving row must derive a new validator")
+}
+
+// TestUpdateSettingsUnconditionalIgnoresStaleness pins the back-compatibility
+// half: without a precondition the manager keeps last-write-wins, which is
+// what the dashboard relies on and what every client written before this
+// existed does.
+func TestUpdateSettingsUnconditionalIgnoresStaleness(t *testing.T) {
+	ctx := context.Background()
+	f := newBootstrapFixture(t)
+
+	const accountID, userID = "account1", "user1"
+	f.expectPermission(accountID, userID, modules.AgentNetworkSettings, operations.Create, true)
+	f.expectPermission(accountID, userID, modules.AgentNetworkSettings, operations.Update, true)
+	f.expectPermission(accountID, userID, modules.AgentNetworkSettings, operations.Update, true)
+
+	created, err := f.createSettings(ctx, accountID, userID, "cluster1.example.com", "")
+	require.NoError(t, err, "bootstrap must succeed")
+
+	_, err = f.manager.UpdateSettings(ctx, userID, updateFor(created, 21), nil)
+	require.NoError(t, err, "the first unconditional update must succeed")
+
+	// The second writer is working from a read that is now stale, and with no
+	// precondition it overwrites regardless.
+	updated, err := f.manager.UpdateSettings(ctx, userID, updateFor(created, 7), nil)
+	require.NoError(t, err, "an unconditional update must not be refused for staleness")
+	assert.Equal(t, 7, updated.AccessLogRetentionDays, "last write wins without a precondition")
+}
+
+// TestDeleteSettingsPreconditionRefusesStale pins the conditional delete at
+// the manager level: a stale validator refuses, and the row is still there
+// afterwards. Deletion is the destructive operation and its two other guards
+// are about state rather than staleness, so this is the only thing standing
+// between a client working from an old read and a released endpoint.
+func TestDeleteSettingsPreconditionRefusesStale(t *testing.T) {
+	ctx := context.Background()
+	f := newBootstrapFixture(t)
+
+	const accountID, userID = "account1", "user1"
+	f.expectPermission(accountID, userID, modules.AgentNetworkSettings, operations.Create, true)
+	f.expectPermission(accountID, userID, modules.AgentNetworkSettings, operations.Update, true)
+	f.expectPermission(accountID, userID, modules.AgentNetworkSettings, operations.Delete, true)
+	f.expectPermission(accountID, userID, modules.AgentNetworkSettings, operations.Delete, true)
+
+	created, err := f.createSettings(ctx, accountID, userID, "cluster1.example.com", "")
+	require.NoError(t, err, "bootstrap must succeed")
+	stale := created.ETag()
+
+	updated, err := f.manager.UpdateSettings(ctx, userID, updateFor(created, 21), nil)
+	require.NoError(t, err, "the intervening update must succeed")
+
+	err = f.manager.DeleteSettings(ctx, accountID, userID, ifMatch(t, stale))
+	require.Error(t, err, "a stale precondition must refuse the delete")
+	assert.True(t, isPreconditionFailed(err), "the refusal must be a precondition failure, got: %v", err)
+
+	stored, err := f.store.GetAgentNetworkSettings(ctx, store.LockingStrengthNone, accountID)
+	require.NoError(t, err, "the refused delete must leave the row in place")
+	assert.Equal(t, created.Domain, stored.Domain, "the endpoint must not have been released")
+
+	// The validator the intervening update returned is the current one, and
+	// deleting with it goes through.
+	require.NoError(t, f.manager.DeleteSettings(ctx, accountID, userID, ifMatch(t, updated.ETag())),
+		"a matching precondition must be honoured")
+	_, err = f.store.GetAgentNetworkSettings(ctx, store.LockingStrengthNone, accountID)
+	assert.Error(t, err, "the row must be gone")
+}
+
+// isPreconditionFailed reports whether err is the 412-mapped status error.
+func isPreconditionFailed(err error) bool {
+	var sErr *status.Error
+	return errors.As(err, &sErr) && sErr.Type() == status.PreconditionFailed
+}

management/server/agentnetwork_budgetrule_realstack_test.go

@@ -117,7 +117,7 @@ func TestAgentNetwork_UpdateSettings_PreservesImmutableAndTogglesCollection(t *t
 		EnablePromptCollection: true,
 		RedactPii:              true,
 		AccessLogRetentionDays: before.AccessLogRetentionDays,
-	})
+	}, nil)
 	require.NoError(t, err, "UpdateSettings must succeed")
 	assert.Equal(t, before.Domain, updated.Domain, "domain is immutable and must be preserved")
 	assert.Equal(t, before.ProxyAddress, updated.ProxyAddress, "proxy address is immutable and must be preserved")
@@ -147,7 +147,7 @@ func TestAgentNetwork_UpdateSettings_PreservesImmutableAndTogglesCollection(t *t
 				EnablePromptCollection: false,
 				RedactPii:              false,
 				AccessLogRetentionDays: before.AccessLogRetentionDays,
-			})
+			}, nil)
 			assert.Error(t, err, "a mismatched identity echo must be rejected")
 			assert.ErrorContains(t, err, "immutable", "the rejection must name the immutability rule")
 		})