Compare commits

...

8 Commits

Author SHA1 Message Date
Brad Ison
6c9267c7d3 [management] State the sqlite precision caveat without the review history
A comment explaining why the truncation guard sits on the type recorded how
the gap came to be noticed rather than the constraint itself, which AGENTS.md
rules out. The technical point is the one worth keeping: sqlite preserves
nanoseconds, so a sqlite-only suite cannot observe the truncation.

The neighbouring back-compatibility comment gets the same treatment — "clients
written before this existed" leans on the reader knowing what "this" was, and
naming conditional requests outright says it plainly.
2026-08-11 12:33:23 +02:00
Brad Ison
29eff3b207 [management] Derive the settings ETag at a precision the store preserves
The validator a bootstrap returns was permanently unusable on PostgreSQL. POST
derives it from the in-memory row, whose CreatedAt carries nanoseconds, while
every later comparison derives it from a row read back out of the store — and
PostgreSQL truncates timestamps to microseconds, MySQL DATETIME to whole
seconds without an fsp. The two never agreed again, so the documented
"conditional PUT without an intervening GET" answered 412 forever.

Hashing CreatedAt at whole-second precision fixes it at the root: seconds is
the floor every supported engine round-trips, so the validator no longer
depends on which store is behind it. The cost is that a delete and
re-bootstrap inside the same second, onto the same endpoint with the same
toggles, derives the same validator — which needs a self-addressed endpoint
reclaimed within one second, since a labeled bootstrap draws a fresh label.

The whole suite ran green against this bug, because every test uses the sqlite
test store and sqlite preserves nanoseconds. The regression guard is therefore
on the type rather than through a store: it asserts the validator is unchanged
by truncation at each engine's precision, so it holds without running the
suite against all three.

Also from review: the concurrency test recorded the winning writer's value in a
slice left zero for the loser, and zero is a retention the API documents as
"keep indefinitely" — so the assertion could be satisfied without matching the
writer that won. It now uses a sentinel and asserts equality. The stale-delete
e2e assertion accepted any error, which a server error or a state-guard refusal
would have satisfied; it now requires a precondition failure. And the 412
message both conditional writes return is a single constant, since on delete
the message is the only thing separating staleness from the state guards.
2026-08-11 12:24:53 +02:00
Brad Ison
35137326f7 [management] Expose the ETag response header to browser clients
ETag is not a CORS-safelisted response header, so JavaScript cannot read it
unless the server names it in Access-Control-Expose-Headers. Without that the
API hands a browser client a validator it has no way to see, and conditional
requests are available to the CLI, the REST client and Terraform but silently
not to the dashboard — the one client where a lost update is a person's work
disappearing rather than a plan reverting.

The library offers no way to extend cors.AllowAll(), so the policy is spelled
out verbatim with ExposedHeaders added. That is a wider blast radius than the
one field deserves, which is why the tests assert the rest of the policy too:
any origin, no credentials, the full method set. If-Match needs no grant of
its own, since AllowedHeaders is already "*" — the preflight test guards
against a later narrowing that would leave writes readable but not
conditional.

Collapsing this back to cors.AllowAll() is the obvious tidy-up and would
silently undo it, which is what the tests are there to catch.
2026-08-11 11:51:00 +02:00
Brad Ison
5085a2f96b [management] Surface the settings ETag through the REST client
The Terraform provider consumes this client, and it is the client that has the
read-modify-write problem conditional requests solve, so the server-side work
is inert until the validator reaches it.

The new methods are additive and the existing four delegate to them unchanged.
A breaking signature change would buy a tidier surface at the cost of every
current caller, with no deprecation window, and the plain methods remain the
right default for callers that do not care.

CreateSettingsWithETag exists because the bootstrap emits a validator too, and
discarding it would force a client to read again before its first conditional
write — the round trip the header is there to avoid.

IsPreconditionFailed joins IsNotFound because a refused precondition is
otherwise an opaque APIError. Telling "someone else changed this, read again
and retry" apart from a genuine failure is the decision a conditional client
has to make, and it should not have to compare status codes by hand.

Validators arrive unquoted and go back out quoted, so no caller handles the
wire syntax. The e2e coverage drives all of it through the typed client
against a real server, playing out the case that motivates the feature: a
client plans an update, an operator turns PII redaction on in between, and the
client's write is refused instead of silently turning it back off.
2026-08-11 11:44:15 +02:00
Brad Ison
30c2010c09 [management] Document the Agent Network settings ETag in the API spec
Declares what the handlers already do: the settings reads and writes return an
ETag, and PUT and DELETE honour an If-Match against it. The 412 that PUT can
now return is documented alongside, and DELETE's existing 412 grows a note
that a precondition failure shares the status with its two state guards and is
told apart by the message rather than the code.

Response headers generate nothing under a models-only codegen config, so those
entries are for human and third-party consumers. The If-Match parameters do
generate — hence the two Params structs in types.gen.go, which nothing calls
yet but which the REST client can take up.
2026-08-11 11:36:18 +02:00
Brad Ison
9b06290240 [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.
2026-08-11 11:28:17 +02:00
Brad Ison
829156f53d [management] Add ETag and If-Match helpers for HTTP handlers
The read-modify-write pattern conditional requests guard is not specific to
one resource — every Terraform-managed resource has the same shape, so the
second consumer of this is a matter of when, not if. Deciding the entity-tag
quoting, the "*" form, the list form and the weak-validator rule once here
means an adopting resource inherits all of them instead of re-deriving each.
Only the derivation stays on the type, since only the type knows which of its
fields the representation covers.

IfMatch returns a nil-safe precondition rather than a slice of tags. If-Match
is defined in terms of the strong comparison function, so a weak validator can
never satisfy it — and a header carrying nothing but weak validators then
parses to an empty list, which the obvious call-site check reads as "no
precondition given" and lets the write through unguarded. That is precisely
the failure this is meant to prevent, and it is reachable by a proxy that
weakens an ETag in transit. An absent header is unconditional; a header that
was sent but carries nothing usable refuses.

Weak validators are dropped rather than unwrapped, for the same reason:
unwrapping one into a strong tag would quietly grant a match that the client's
own header said it could not have.

No handler uses this yet.
2026-08-11 11:16:27 +02:00
Brad Ison
4f6caa1110 [management] Derive an ETag for Agent Network settings
A client that reads the Agent Network settings, computes a change and writes
the whole object back will silently revert anything that changed in between —
Terraform's read-modify-write is exactly this shape. The consequence worth
guarding is RedactPii: an operator enabling PII redaction in the dashboard can
have it turned back off by an apply that was planned before their change, with
no error and no drift warning on that run.

Conditional requests need a validator. Derive one instead of storing it: a
timestamp cannot separate two writes within a clock tick, and a revision column
costs a migration plus the bump-discipline that goes with it. A hash of the
representation needs neither and is correct by construction.

The hash covers an explicit field tuple rather than the marshalled API types,
whose field ordering is not a contract. AccountID stays out — it identifies the
resource, not the representation — and so does UpdatedAt, so that a write
changing nothing observable cannot invalidate a precondition someone else is
holding. The identity fields and CreatedAt are in: a validator over only the
mutable toggles would survive a delete and fresh bootstrap onto the same toggle
values, and an If-Match held across that gap would then authorize a write
against what is really a different resource.

This only derives the validator. Nothing emits or honours it yet.
2026-08-11 11:12:06 +02:00
19 changed files with 1518 additions and 27 deletions

View File

@@ -11,6 +11,7 @@ import (
"github.com/stretchr/testify/require"
"github.com/netbirdio/netbird/e2e/harness"
"github.com/netbirdio/netbird/shared/management/client/rest"
"github.com/netbirdio/netbird/shared/management/http/api"
)
@@ -177,3 +178,89 @@ func TestSettingsBootstrapSelfAddressed(t *testing.T) {
require.NoError(t, err, "bootstrap after delete must succeed")
assert.Equal(t, "gw2.e2e.netbird.selfhosted", recreated.Endpoint, "the fresh bootstrap claims the new hostname")
}
// TestSettingsConditionalWrites covers the lost-update guard end to end, over
// the same REST client the Terraform provider uses: read the settings, take
// the entity-tag, and have a write refused when the row moved underneath it.
//
// The scenario is the one that motivates the feature. A client reads the
// settings and computes an update. An operator turns PII redaction on in the
// dashboard in the meantime. Without a precondition the client's write puts
// redaction straight back off — no error, no drift warning, a
// compliance-relevant control silently disabled. With one, the write is
// refused and the client can read again.
func TestSettingsConditionalWrites(t *testing.T) {
ctx := context.Background()
fresh, err := harnessStartFresh(ctx, t)
require.NoError(t, err, "start dedicated combined server")
const cluster = "eu.e2e.netbird.selfhosted"
bootstrapped, err := fresh.CreateSettings(ctx, api.AgentNetworkSettingsCreateRequest{
ProxyAddress: ptr(cluster),
})
require.NoError(t, err, "bootstrap must succeed")
// What the client plans against.
planned, etag, err := fresh.GetSettingsWithETag(ctx)
require.NoError(t, err, "read must succeed")
require.NotEmpty(t, etag, "the read must carry a validator")
assert.Equal(t, bootstrapped.Endpoint, planned.Endpoint)
_, again, err := fresh.GetSettingsWithETag(ctx)
require.NoError(t, err, "second read must succeed")
assert.Equal(t, etag, again, "an unchanged row must read as the same validator")
update := func(redactPii bool, retention int) api.AgentNetworkSettingsRequest {
return api.AgentNetworkSettingsRequest{
Endpoint: planned.Endpoint,
ProxyAddress: planned.ProxyAddress,
EnableLogCollection: true,
EnablePromptCollection: true,
RedactPii: redactPii,
AccessLogRetentionDays: retention,
}
}
// The operator's change, which the planning client never saw.
_, err = fresh.UpdateSettings(ctx, update(true, 21))
require.NoError(t, err, "the intervening update must succeed")
// The client's write, planned against the earlier read, would have turned
// redaction back off. It is refused instead.
_, _, err = fresh.UpdateSettingsIfMatch(ctx, update(false, 7), etag)
require.Error(t, err, "a stale precondition must be refused")
require.True(t, rest.IsPreconditionFailed(err),
"the refusal must be a precondition failure, got: %v", err)
intact, current, err := fresh.GetSettingsWithETag(ctx)
require.NoError(t, err, "read after the refusal must succeed")
assert.True(t, intact.RedactPii, "the refused write must not have turned redaction off")
require.NotNil(t, intact.AccessLogRetentionDays)
assert.Equal(t, 21, *intact.AccessLogRetentionDays, "the refused write must not have changed retention")
assert.NotEqual(t, etag, current, "the validator must have moved with the intervening update")
// Retrying against the current validator goes through, and hands back the
// validator for the write after it.
updated, next, err := fresh.UpdateSettingsIfMatch(ctx, update(true, 7), current)
require.NoError(t, err, "a matching precondition must be honoured")
require.NotNil(t, updated.AccessLogRetentionDays)
assert.Equal(t, 7, *updated.AccessLogRetentionDays, "the conditional write must apply")
assert.NotEmpty(t, next, "the write must return a validator")
assert.NotEqual(t, current, next, "the write must move the validator")
// The delete is conditional too, and refusing a stale one leaves the
// endpoint claimed.
err = fresh.DeleteSettingsIfMatch(ctx, etag)
require.Error(t, err, "a stale precondition must refuse the delete")
require.True(t, rest.IsPreconditionFailed(err),
"the delete must be refused for staleness rather than for a state guard or a server error, got: %v", err)
stillThere, err := fresh.GetSettings(ctx)
require.NoError(t, err, "read after the refused delete must succeed")
assert.Equal(t, planned.Endpoint, stillThere.Endpoint, "the refused delete must leave the endpoint claimed")
require.NoError(t, fresh.DeleteSettingsIfMatch(ctx, next), "a matching precondition must be honoured")
gone, err := fresh.GetSettings(ctx)
require.NoError(t, err, "read after the delete must succeed")
assert.Empty(t, gone.Endpoint, "the row must be gone")
}

View File

@@ -153,6 +153,37 @@ func (c *Combined) DeleteSettings(ctx context.Context) error {
return anDelete(ctx, c, "/api/agent-network/settings")
}
// The conditional-request wrappers go through the typed REST client rather
// than anRequest, so the e2e run exercises the client's own header handling —
// the quoting on the way out and the unquoting on the way back — against a
// real server, which is the path the Terraform provider takes.
// GetSettingsWithETag reads the settings along with the entity-tag that makes
// a following write conditional.
func (c *Combined) GetSettingsWithETag(ctx context.Context) (api.AgentNetworkSettings, string, error) {
settings, etag, err := c.api.AgentNetwork.GetSettingsWithETag(ctx)
if err != nil {
return api.AgentNetworkSettings{}, "", err
}
return *settings, etag, nil
}
// UpdateSettingsIfMatch applies the update only if etag is still current,
// returning the entity-tag of the row it wrote.
func (c *Combined) UpdateSettingsIfMatch(ctx context.Context, req api.AgentNetworkSettingsRequest, etag string) (api.AgentNetworkSettings, string, error) {
settings, newETag, err := c.api.AgentNetwork.UpdateSettingsIfMatch(ctx, req, etag)
if err != nil {
return api.AgentNetworkSettings{}, "", err
}
return *settings, newETag, nil
}
// DeleteSettingsIfMatch deletes the settings row only if etag is still
// current.
func (c *Combined) DeleteSettingsIfMatch(ctx context.Context, etag string) error {
return c.api.AgentNetwork.DeleteSettingsIfMatch(ctx, etag)
}
// ListConsumption returns the account's consumption rows (possibly empty).
func (c *Combined) ListConsumption(ctx context.Context) ([]api.AgentNetworkConsumption, error) {
return anRequest[[]api.AgentNetworkConsumption](ctx, c, http.MethodGet, "/api/agent-network/consumption", nil)

View File

@@ -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,

View File

@@ -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())
}

View File

@@ -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())
})
}

View File

@@ -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)
@@ -544,6 +545,13 @@ func (m *managerImpl) DeleteBudgetRule(ctx context.Context, accountID, userID, r
return nil
}
// stalePreconditionMsg is the refusal both conditional settings writes return.
// Shared so the two cannot drift: DeleteSettings answers 412 for its state
// guards as well, so the message is the only thing telling a client that it is
// working from an old read rather than tripping over providers or a serving
// proxy.
const stalePreconditionMsg = "if-match precondition failed: the settings have changed since they were read; GET them again and retry"
// UpdateSettings replaces the mutable account-level settings — the collection
// toggles and retention — on the account's row. The identity fields (Domain,
// ProxyAddress) are assigned at bootstrap (CreateSettings) and immutable: the
@@ -554,7 +562,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 +585,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, "%s", stalePreconditionMsg)
}
// 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 +661,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 +682,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, "%s", stalePreconditionMsg)
}
providers, err := tx.GetAccountAgentNetworkProviders(ctx, store.LockingStrengthNone, accountID)
if err != nil {
return fmt.Errorf("get agent network providers: %w", err)
@@ -1100,11 +1138,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

View File

@@ -0,0 +1,199 @@
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()
// noWrite is a retention value neither writer sends and the API would
// never store, so an assertion that lands on it is a test bug rather than
// a silently satisfied comparison. Zero would not do: the API documents 0
// as "keep indefinitely", so it is a value the row could legitimately hold.
const noWrite = -1
var (
wg sync.WaitGroup
start = make(chan struct{})
errs = make([]error, 2)
wrote = []int{7, 21}
returned = []int{noWrite, noWrite}
)
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 {
returned[i] = updated.AccessLogRetentionDays
}
}()
}
close(start)
wg.Wait()
succeeded, winner := 0, noWrite
for i, err := range errs {
if err == nil {
succeeded++
winner = wrote[i]
assert.Equal(t, wrote[i], returned[i], "the winner's response must carry what it sent")
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.Equal(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 any client that predates conditional
// requests 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
}

View File

@@ -1,6 +1,8 @@
package types
import (
"crypto/sha256"
"encoding/hex"
"fmt"
"strings"
"time"
@@ -67,6 +69,64 @@ func DefaultSettings(accountID string) *Settings {
}
}
// etagLength is how much of the hash the validator carries. 16 hex characters
// — 64 bits — is far more than enough to make an accidental collision between
// two representations of one account's settings unreachable, and keeps the
// header short enough to read in a log line.
const etagLength = 16
// ETag returns a strong validator over the settings representation, for
// conditional requests (RFC 9110 If-Match). The value is unquoted; applying
// the quoting is the transport layer's job.
//
// The hash covers an explicit field tuple rather than the marshalled API
// representation: field ordering in the generated API types is not a contract,
// so hashing serialized output would make the validator churn with codegen.
// Two exclusions are deliberate:
//
// - AccountID identifies the resource — it is the URL, not the
// representation. Including it would make the validator differ between
// accounts whose settings are genuinely identical, which no client can
// observe and no precondition needs.
// - UpdatedAt is excluded so that equal representations always yield equal
// validators. A write that changes nothing must not invalidate a
// precondition another client is holding.
//
// Everything else is in, including the identity fields and CreatedAt. A
// validator that covered only the mutable toggles would survive a delete
// followed by a fresh bootstrap onto the same toggle values, and an If-Match
// held across that gap would then authorize a write against what is really a
// different resource. CreatedAt is what distinguishes the re-bootstrapped row.
//
// CreatedAt is hashed at whole-second precision because the validator has to
// agree across a store round-trip. A freshly bootstrapped row derives its
// validator in memory, from a time.Time carrying nanoseconds, while every
// later comparison derives it from a row read back out of the store — and the
// engines truncate: PostgreSQL to microseconds, MySQL DATETIME to whole
// seconds without an fsp. At nanosecond precision the two never agree again,
// so the validator a bootstrap hands out is permanently unusable. Seconds is
// the floor every supported engine preserves. The cost is that a delete and
// re-bootstrap within the same second, onto the same endpoint and the same
// toggles, derives the same validator; a labeled bootstrap draws a fresh
// random label, so that needs a self-addressed endpoint reclaimed inside one
// second.
//
// Adding a field to Settings means deciding whether it belongs here; the
// field-count guard in the tests is what forces that decision.
func (s *Settings) ETag() string {
h := sha256.New()
fmt.Fprintf(h, "%s\x00%s\x00%t\x00%t\x00%t\x00%d\x00%d",
s.Domain,
s.ProxyAddress,
s.EnableLogCollection,
s.EnablePromptCollection,
s.RedactPii,
s.AccessLogRetentionDays,
s.CreatedAt.Unix(),
)
return hex.EncodeToString(h.Sum(nil))[:etagLength]
}
// Endpoint returns the bare hostname agents reach this account at — the
// Domain column. Empty until the row is bootstrapped.
func (s *Settings) Endpoint() string { return s.Domain }

View File

@@ -0,0 +1,184 @@
package types
import (
"reflect"
"testing"
"time"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
// etagSettings is a fully populated settings row — every hashed field set to a
// distinctive value — so a mutation test can flip exactly one thing at a time.
// The timestamp carries sub-second precision on purpose: a whole-second value
// would make the precision test below pass without proving anything.
func etagSettings() *Settings {
created := time.Date(2026, 8, 11, 9, 30, 0, 123456789, time.UTC)
return &Settings{
AccountID: "acc-1",
Domain: "cool-otter.eu.proxy.netbird.io",
ProxyAddress: "eu.proxy.netbird.io",
EnableLogCollection: true,
EnablePromptCollection: true,
RedactPii: true,
AccessLogRetentionDays: 30,
CreatedAt: created,
UpdatedAt: created,
}
}
// TestSettings_ETagShape pins the wire shape of the validator: a bare
// lowercase hex string of the documented length, with no quoting — quoting is
// the transport layer's job, and a validator that arrived pre-quoted would be
// double-quoted on the way out.
func TestSettings_ETagShape(t *testing.T) {
etag := etagSettings().ETag()
assert.Len(t, etag, etagLength, "the validator must be exactly etagLength characters")
assert.NotContains(t, etag, `"`, "the derived validator must not carry its own quoting")
for _, r := range etag {
require.Truef(t, (r >= '0' && r <= '9') || (r >= 'a' && r <= 'f'),
"the validator must be lowercase hex, got %q in %q", r, etag)
}
}
// TestSettings_ETagIsStable covers the guarantee every conditional request
// rests on: an unchanged row derives the same validator every time, including
// across a fresh struct built from the same values. A validator that varied
// per derivation would fail every If-Match and make the feature unusable.
func TestSettings_ETagIsStable(t *testing.T) {
s := etagSettings()
first := s.ETag()
assert.Equal(t, first, s.ETag(), "repeated derivation from one value must agree")
assert.Equal(t, first, etagSettings().ETag(), "an equal row must derive an equal validator")
}
// TestSettings_ETagSensitivity is the other half of the contract: every field
// the validator covers must actually move it. The cases are also what makes
// the field-count guard meaningful — a new field that belongs in the tuple but
// is missing from it has no case here, and the guard is what catches that.
//
// The mutations are checked to be pairwise distinct, not merely different from
// the baseline: that is what catches an ambiguous concatenation, where moving
// a character across a field boundary would hash identically without the
// delimiter.
func TestSettings_ETagSensitivity(t *testing.T) {
cases := []struct {
name string
mutate func(*Settings)
}{
{"domain", func(s *Settings) { s.Domain = "brave-otter.eu.proxy.netbird.io" }},
{"proxy address", func(s *Settings) { s.ProxyAddress = "us.proxy.netbird.io" }},
{"log collection", func(s *Settings) { s.EnableLogCollection = false }},
{"prompt collection", func(s *Settings) { s.EnablePromptCollection = false }},
{"redact pii", func(s *Settings) { s.RedactPii = false }},
{"retention", func(s *Settings) { s.AccessLogRetentionDays = 14 }},
{"created at", func(s *Settings) { s.CreatedAt = s.CreatedAt.Add(time.Second) }},
// Moving characters across the Domain/ProxyAddress boundary leaves
// the two fields' concatenation byte-identical, so this case passes
// only because the tuple is delimited.
{"identity boundary shifted", func(s *Settings) {
joined := s.Domain + s.ProxyAddress
split := len(s.Domain) - 3
s.Domain, s.ProxyAddress = joined[:split], joined[split:]
}},
}
baseline := etagSettings().ETag()
seen := map[string]string{"baseline": baseline}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
s := etagSettings()
tc.mutate(s)
etag := s.ETag()
assert.NotEqual(t, baseline, etag, "changing %s must change the validator", tc.name)
if other, clash := seen[etag]; clash {
t.Fatalf("changing %s derives the same validator as %s (%s) — the field tuple is ambiguous", tc.name, other, etag)
}
seen[etag] = tc.name
})
}
}
// TestSettings_ETagExclusions pins the two deliberate omissions. AccountID is
// the resource's identity rather than its representation. UpdatedAt is left
// out so that a write which changes nothing observable does not invalidate a
// precondition another client is holding — equal representations must always
// derive equal validators.
func TestSettings_ETagExclusions(t *testing.T) {
baseline := etagSettings().ETag()
other := etagSettings()
other.AccountID = "acc-2"
assert.Equal(t, baseline, other.ETag(), "the account id must not reach the validator")
touched := etagSettings()
touched.UpdatedAt = touched.UpdatedAt.Add(time.Hour)
assert.Equal(t, baseline, touched.ETag(), "a write that changed nothing must not move the validator")
}
// TestSettings_ETagSurvivesTimestampTruncation pins the store round-trip the
// validator has to survive. A freshly bootstrapped row derives its validator
// in memory, from a time.Time carrying nanoseconds; every later comparison
// derives it from a row read back out of the store, and the engines truncate
// on the way through — PostgreSQL to microseconds, MySQL DATETIME to whole
// seconds without an fsp. If the hash is sensitive below its coarsest engine's
// precision, the validator a bootstrap hands out never matches again and the
// documented "conditional PUT without an intervening GET" is a permanent 412.
//
// Asserted on the type rather than through a store, so it holds without running
// the suite against every engine. The sqlite test store preserves nanoseconds,
// so a sqlite-only suite cannot observe the truncation at all.
func TestSettings_ETagSurvivesTimestampTruncation(t *testing.T) {
inMemory := etagSettings()
require.NotZero(t, inMemory.CreatedAt.Nanosecond(), "the fixture must carry sub-second precision to prove anything")
for name, truncation := range map[string]time.Duration{
"postgres (microseconds)": time.Microsecond,
"mysql (milliseconds)": time.Millisecond,
"mysql datetime (seconds)": time.Second,
} {
t.Run(name, func(t *testing.T) {
roundTripped := etagSettings()
roundTripped.CreatedAt = roundTripped.CreatedAt.Truncate(truncation)
assert.Equal(t, inMemory.ETag(), roundTripped.ETag(),
"a validator derived before the write must still match one derived after reading the row back")
})
}
}
// TestSettings_ETagOfDefaults covers the pre-bootstrap view, which GET serves
// as a real representation and therefore validates like one. It must derive
// without panicking on the zero CreatedAt, and it must not collide with a
// bootstrapped row — otherwise an If-Match taken before bootstrap would
// authorize a write against the row that appeared since.
func TestSettings_ETagOfDefaults(t *testing.T) {
defaults := DefaultSettings("acc-1").ETag()
assert.Len(t, defaults, etagLength, "the default view must derive a well-formed validator")
assert.NotEqual(t, etagSettings().ETag(), defaults,
"the unbootstrapped view must not validate as a bootstrapped row")
}
// etagFieldCount is the number of fields Settings carries. ETag hashes an
// explicit tuple rather than the struct, so a field added here is silently
// outside the validator until someone decides otherwise — the worst kind of
// gap, because the mechanism looks present and works for every other field.
//
// If this constant needs updating, that is the decision point: either add the
// new field to ETag and give it a case in TestSettings_ETagSensitivity, or
// record here why it stays out.
const etagFieldCount = 9
// TestSettings_ETagFieldCountGuard fails when a field is added to or removed
// from Settings, forcing the question of whether it belongs in the validator.
func TestSettings_ETagFieldCountGuard(t *testing.T) {
assert.Equal(t, etagFieldCount, reflect.TypeFor[Settings]().NumField(),
"Settings gained or lost a field: decide whether it belongs in ETag(), then update etagFieldCount")
}

View File

@@ -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")
})

View File

@@ -98,7 +98,7 @@ func NewAPIHandler(ctx context.Context, router *mux.Router, accountManager accou
isValidChildAccount,
)
corsMiddleware := cors.AllowAll()
corsMiddleware := newCORSMiddleware()
metricsMiddleware := appMetrics.HTTPMiddleware()
@@ -145,3 +145,32 @@ func NewAPIHandler(ctx context.Context, router *mux.Router, accountManager accou
return router, nil
}
// newCORSMiddleware builds the API's CORS policy: cors.AllowAll() plus ETag in
// ExposedHeaders.
//
// The addition is what makes conditional requests usable from a browser. A
// response header that is not CORS-safelisted is invisible to JavaScript
// unless it is named in Access-Control-Expose-Headers, and ETag is not on that
// list — so without this the server can hand a browser client a validator it
// has no way to read, leaving conditional requests to non-browser clients
// only. If-Match needs nothing further, since AllowedHeaders is already "*".
//
// Everything else mirrors cors.AllowAll() exactly. It is spelled out rather
// than called because the library offers no way to extend it.
func newCORSMiddleware() *cors.Cors {
return cors.New(cors.Options{
AllowedOrigins: []string{"*"},
AllowedMethods: []string{
http.MethodHead,
http.MethodGet,
http.MethodPost,
http.MethodPut,
http.MethodPatch,
http.MethodDelete,
},
AllowedHeaders: []string{"*"},
ExposedHeaders: []string{"ETag"},
AllowCredentials: false,
})
}

View File

@@ -0,0 +1,88 @@
package http
import (
"net/http"
"net/http/httptest"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
// TestCORSExposesETag pins the reason this policy is spelled out instead of
// being cors.AllowAll(). ETag is not a CORS-safelisted response header, so
// without it named in Access-Control-Expose-Headers a browser client is handed
// a validator it cannot read — conditional requests would work for the CLI,
// the REST client and Terraform, and silently not for the dashboard.
//
// Collapsing this back to cors.AllowAll() is exactly the simplification that
// would reintroduce that, which is what this test is here to catch.
func TestCORSExposesETag(t *testing.T) {
handler := newCORSMiddleware().Handler(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
w.Header().Set("ETag", `"9f86d081884c7d65"`)
w.WriteHeader(http.StatusOK)
}))
req := httptest.NewRequest(http.MethodGet, "/api/agent-network/settings", nil)
req.Header.Set("Origin", "https://app.netbird.io")
rec := httptest.NewRecorder()
handler.ServeHTTP(rec, req)
require.Equal(t, http.StatusOK, rec.Code)
// Compared canonicalized: the library normalizes the name it echoes, so
// this reads "Etag" rather than "ETag". Browsers match the exposed-header
// list case-insensitively, so the spelling does not matter — but asserting
// it byte-exactly would fail for a reason that has nothing to do with the
// behaviour being pinned.
assert.Equal(t, http.CanonicalHeaderKey("ETag"),
http.CanonicalHeaderKey(rec.Header().Get("Access-Control-Expose-Headers")),
"browser clients must be allowed to read the validator they are sent")
}
// TestCORSAllowsIfMatchPreflight covers the request half. It needs nothing
// beyond the wildcard AllowedHeaders that was already there, so this is a
// regression guard rather than a new grant: narrowing AllowedHeaders to a list
// later must not drop If-Match and leave writes readable but not conditional.
func TestCORSAllowsIfMatchPreflight(t *testing.T) {
handler := newCORSMiddleware().Handler(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusOK)
}))
req := httptest.NewRequest(http.MethodOptions, "/api/agent-network/settings", nil)
req.Header.Set("Origin", "https://app.netbird.io")
req.Header.Set("Access-Control-Request-Method", http.MethodPut)
req.Header.Set("Access-Control-Request-Headers", "If-Match")
rec := httptest.NewRecorder()
handler.ServeHTTP(rec, req)
assert.Contains(t, rec.Header().Get("Access-Control-Allow-Headers"), "If-Match",
"a conditional write must survive preflight")
assert.Contains(t, rec.Header().Get("Access-Control-Allow-Methods"), http.MethodPut,
"the conditional write's method must survive preflight")
}
// TestCORSMatchesAllowAllOtherwise pins the rest of the policy, which is a
// verbatim copy of cors.AllowAll(). Spelling the options out is what let ETag
// be added; it also means a change to the library's defaults no longer reaches
// this API, so the settings that matter are asserted here rather than assumed.
func TestCORSMatchesAllowAllOtherwise(t *testing.T) {
handler := newCORSMiddleware().Handler(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusOK)
}))
req := httptest.NewRequest(http.MethodOptions, "/api/peers", nil)
req.Header.Set("Origin", "https://anywhere.example.com")
req.Header.Set("Access-Control-Request-Method", http.MethodDelete)
rec := httptest.NewRecorder()
handler.ServeHTTP(rec, req)
assert.Equal(t, "*", rec.Header().Get("Access-Control-Allow-Origin"), "any origin must still be allowed")
assert.Empty(t, rec.Header().Get("Access-Control-Allow-Credentials"),
"credentials must stay disallowed — allowing them alongside a wildcard origin would be a real weakening")
assert.Contains(t, rec.Header().Get("Access-Control-Allow-Methods"), http.MethodDelete,
"the full method set must still be allowed")
}

View File

@@ -6,6 +6,8 @@ import (
"encoding/json"
"io"
"net/http"
"strconv"
"strings"
"github.com/netbirdio/netbird/shared/management/http/api"
)
@@ -336,25 +338,35 @@ func (a *AgentNetworkAPI) DeleteBudgetRule(ctx context.Context, ruleID string) e
// to an APIError matchable via IsNotFound rather than fabricating defaults
// the server never stated.
func (a *AgentNetworkAPI) GetSettings(ctx context.Context) (*api.AgentNetworkSettings, error) {
settings, _, err := a.GetSettingsWithETag(ctx)
return settings, err
}
// GetSettingsWithETag is GetSettings, additionally returning the entity-tag
// the server derived for the settings it returned. Hand that validator to
// UpdateSettingsIfMatch or DeleteSettingsIfMatch to make the write conditional
// on nothing having changed in between — the read-modify-write cycle that
// otherwise silently reverts a concurrent change.
func (a *AgentNetworkAPI) GetSettingsWithETag(ctx context.Context) (*api.AgentNetworkSettings, string, error) {
resp, err := a.c.NewRequest(ctx, "GET", "/api/agent-network/settings", nil, nil)
if err != nil {
return nil, err
return nil, "", err
}
if resp.Body != nil {
defer resp.Body.Close()
}
body, err := io.ReadAll(resp.Body)
if err != nil {
return nil, err
return nil, "", err
}
if trimmed := bytes.TrimSpace(body); len(trimmed) == 0 || bytes.Equal(trimmed, []byte("null")) {
return nil, &APIError{StatusCode: http.StatusNotFound, Message: "agent network settings not found"}
return nil, "", &APIError{StatusCode: http.StatusNotFound, Message: "agent network settings not found"}
}
var ret api.AgentNetworkSettings
if err := json.Unmarshal(body, &ret); err != nil {
return nil, err
return nil, "", err
}
return &ret, nil
return &ret, etagFrom(resp), nil
}
// CreateSettings bootstraps the account's Agent Network settings row,
@@ -363,19 +375,30 @@ func (a *AgentNetworkAPI) GetSettings(ctx context.Context) (*api.AgentNetworkSet
// request.Endpoint (self-addressed dedicated endpoint, claimed verbatim) must
// be set. Returns a conflict when the account already has a settings row.
func (a *AgentNetworkAPI) CreateSettings(ctx context.Context, request api.PostApiAgentNetworkSettingsJSONRequestBody) (*api.AgentNetworkSettings, error) {
settings, _, err := a.CreateSettingsWithETag(ctx, request)
return settings, err
}
// CreateSettingsWithETag is CreateSettings, additionally returning the
// entity-tag of the row it bootstrapped, so a client can follow the bootstrap
// with a conditional write without an intervening read.
func (a *AgentNetworkAPI) CreateSettingsWithETag(ctx context.Context, request api.PostApiAgentNetworkSettingsJSONRequestBody) (*api.AgentNetworkSettings, string, error) {
requestBytes, err := json.Marshal(request)
if err != nil {
return nil, err
return nil, "", err
}
resp, err := a.c.NewRequest(ctx, "POST", "/api/agent-network/settings", bytes.NewReader(requestBytes), nil)
if err != nil {
return nil, err
return nil, "", err
}
if resp.Body != nil {
defer resp.Body.Close()
}
ret, err := parseResponse[api.AgentNetworkSettings](resp)
return &ret, err
if err != nil {
return nil, "", err
}
return &ret, etagFrom(resp), nil
}
// UpdateSettings updates the account's Agent Network settings; the request
@@ -385,19 +408,35 @@ func (a *AgentNetworkAPI) CreateSettings(ctx context.Context, request api.PostAp
// a request carrying different values is rejected. Returns not-found until
// the account is bootstrapped.
func (a *AgentNetworkAPI) UpdateSettings(ctx context.Context, request api.PutApiAgentNetworkSettingsJSONRequestBody) (*api.AgentNetworkSettings, error) {
settings, _, err := a.UpdateSettingsIfMatch(ctx, request, "")
return settings, err
}
// UpdateSettingsIfMatch is UpdateSettings made conditional on etag — the
// validator from an earlier read — still being current, and returns the
// validator of the row it wrote. This is what closes the read-modify-write
// window: a settings change made between the read and this write makes the
// request fail with a precondition-failed APIError instead of reverting it.
//
// An empty etag sends no precondition and updates unconditionally, which is
// what UpdateSettings does.
func (a *AgentNetworkAPI) UpdateSettingsIfMatch(ctx context.Context, request api.PutApiAgentNetworkSettingsJSONRequestBody, etag string) (*api.AgentNetworkSettings, string, error) {
requestBytes, err := json.Marshal(request)
if err != nil {
return nil, err
return nil, "", err
}
resp, err := a.c.NewRequest(ctx, "PUT", "/api/agent-network/settings", bytes.NewReader(requestBytes), nil)
resp, err := a.c.newRequest(ctx, "PUT", "/api/agent-network/settings", bytes.NewReader(requestBytes), nil, ifMatchHeader(etag))
if err != nil {
return nil, err
return nil, "", err
}
if resp.Body != nil {
defer resp.Body.Close()
}
ret, err := parseResponse[api.AgentNetworkSettings](resp)
return &ret, err
if err != nil {
return nil, "", err
}
return &ret, etagFrom(resp), nil
}
// DeleteSettings deletes the account's Agent Network settings row, releasing
@@ -405,7 +444,20 @@ func (a *AgentNetworkAPI) UpdateSettings(ctx context.Context, request api.PutApi
// exists for the account or while a proxy is actively serving the endpoint.
// Bootstrapping again afterwards allocates a new endpoint.
func (a *AgentNetworkAPI) DeleteSettings(ctx context.Context) error {
resp, err := a.c.NewRequest(ctx, "DELETE", "/api/agent-network/settings", nil, nil)
return a.DeleteSettingsIfMatch(ctx, "")
}
// DeleteSettingsIfMatch is DeleteSettings made conditional on etag — the
// validator from an earlier read — still being current. Sending it matters
// more here than on update: the server's other two refusals are about state
// (no providers, no serving proxy), so this is the only thing that stops a
// client working from an old read of one row from releasing the endpoint of
// the row that replaced it.
//
// An empty etag sends no precondition and deletes unconditionally, which is
// what DeleteSettings does.
func (a *AgentNetworkAPI) DeleteSettingsIfMatch(ctx context.Context, etag string) error {
resp, err := a.c.newRequest(ctx, "DELETE", "/api/agent-network/settings", nil, nil, ifMatchHeader(etag))
if err != nil {
return err
}
@@ -415,3 +467,20 @@ func (a *AgentNetworkAPI) DeleteSettings(ctx context.Context) error {
return nil
}
// etagFrom returns the bare validator from a response, with the transport's
// quoting stripped so a caller can hand it straight back to an If-Match
// parameter without knowing the wire syntax.
func etagFrom(resp *http.Response) string {
return strings.Trim(resp.Header.Get("ETag"), `"`)
}
// ifMatchHeader renders the precondition headers for a bare validator,
// re-applying the quoting etagFrom stripped. An empty validator yields no
// headers at all — an unconditional request.
func ifMatchHeader(etag string) map[string]string {
if etag == "" {
return nil
}
return map[string]string{"If-Match": strconv.Quote(etag)}
}

View File

@@ -559,3 +559,123 @@ func TestAgentNetwork_DeleteSettings_Guarded(t *testing.T) {
assert.Contains(t, err.Error(), "cannot be deleted")
})
}
// TestAgentNetwork_GetSettings_ETag pins that the validator surfaces to the
// caller with the transport's quoting stripped, so it can be handed straight
// back to a conditional write without the caller knowing the wire syntax.
func TestAgentNetwork_GetSettings_ETag(t *testing.T) {
withMockClient(func(c *rest.Client, mux *http.ServeMux) {
mux.HandleFunc("/api/agent-network/settings", func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("ETag", `"9f86d081884c7d65"`)
retBytes, _ := json.Marshal(testAgentNetworkSettings)
_, err := w.Write(retBytes)
require.NoError(t, err)
})
ret, etag, err := c.AgentNetwork.GetSettingsWithETag(context.Background())
require.NoError(t, err)
assert.Equal(t, testAgentNetworkSettings, *ret)
assert.Equal(t, "9f86d081884c7d65", etag, "the validator must arrive unquoted")
})
}
// TestAgentNetwork_UpdateSettings_IfMatch covers the round trip that makes the
// whole feature usable: a validator taken from a read goes back out quoted on
// the write, and the write's own validator comes back for the next one.
func TestAgentNetwork_UpdateSettings_IfMatch(t *testing.T) {
withMockClient(func(c *rest.Client, mux *http.ServeMux) {
mux.HandleFunc("/api/agent-network/settings", func(w http.ResponseWriter, r *http.Request) {
assert.Equal(t, `"9f86d081884c7d65"`, r.Header.Get("If-Match"),
"the precondition must go out quoted as a strong entity-tag")
w.Header().Set("ETag", `"0011223344556677"`)
retBytes, _ := json.Marshal(testAgentNetworkSettings)
_, err := w.Write(retBytes)
require.NoError(t, err)
})
_, etag, err := c.AgentNetwork.UpdateSettingsIfMatch(context.Background(), api.PutApiAgentNetworkSettingsJSONRequestBody{
Endpoint: "brave-otter.eu.proxy.netbird.io",
ProxyAddress: "eu.proxy.netbird.io",
EnableLogCollection: true,
}, "9f86d081884c7d65")
require.NoError(t, err)
assert.Equal(t, "0011223344556677", etag, "the write must return the new validator")
})
}
// TestAgentNetwork_UpdateSettings_NoPrecondition pins the back-compatible
// path: the plain method sends no If-Match at all, rather than an empty or
// wildcard one, so it stays the unconditional update it has always been.
func TestAgentNetwork_UpdateSettings_NoPrecondition(t *testing.T) {
withMockClient(func(c *rest.Client, mux *http.ServeMux) {
mux.HandleFunc("/api/agent-network/settings", func(w http.ResponseWriter, r *http.Request) {
assert.Empty(t, r.Header.Values("If-Match"), "an unconditional update must send no precondition")
retBytes, _ := json.Marshal(testAgentNetworkSettings)
_, err := w.Write(retBytes)
require.NoError(t, err)
})
_, err := c.AgentNetwork.UpdateSettings(context.Background(), api.PutApiAgentNetworkSettingsJSONRequestBody{
Endpoint: "brave-otter.eu.proxy.netbird.io",
ProxyAddress: "eu.proxy.netbird.io",
EnableLogCollection: true,
})
require.NoError(t, err)
})
}
// TestAgentNetwork_UpdateSettings_StalePrecondition pins how a refused write
// reaches the caller: as an APIError a client can recognise as staleness and
// answer by reading again, rather than as an opaque failure.
func TestAgentNetwork_UpdateSettings_StalePrecondition(t *testing.T) {
withMockClient(func(c *rest.Client, mux *http.ServeMux) {
mux.HandleFunc("/api/agent-network/settings", func(w http.ResponseWriter, r *http.Request) {
retBytes, _ := json.Marshal(util.ErrorResponse{Message: "if-match precondition failed: the settings have changed since they were read; get them again and retry", Code: 412})
w.WriteHeader(412)
_, err := w.Write(retBytes)
require.NoError(t, err)
})
_, _, err := c.AgentNetwork.UpdateSettingsIfMatch(context.Background(), api.PutApiAgentNetworkSettingsJSONRequestBody{
Endpoint: "brave-otter.eu.proxy.netbird.io",
ProxyAddress: "eu.proxy.netbird.io",
EnableLogCollection: true,
}, "9f86d081884c7d65")
require.Error(t, err)
assert.True(t, rest.IsPreconditionFailed(err), "a refused precondition must be recognisable as one")
assert.False(t, rest.IsNotFound(err), "it must not be confused with an unbootstrapped account")
})
}
// TestAgentNetwork_CreateSettings_ETag pins that the bootstrap hands back a
// validator, which is what lets a client follow it with a conditional write
// without an intervening read.
func TestAgentNetwork_CreateSettings_ETag(t *testing.T) {
withMockClient(func(c *rest.Client, mux *http.ServeMux) {
mux.HandleFunc("/api/agent-network/settings", func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("ETag", `"9f86d081884c7d65"`)
retBytes, _ := json.Marshal(testAgentNetworkSettings)
_, err := w.Write(retBytes)
require.NoError(t, err)
})
_, etag, err := c.AgentNetwork.CreateSettingsWithETag(context.Background(), api.PostApiAgentNetworkSettingsJSONRequestBody{
ProxyAddress: ptr("eu.proxy.netbird.io"),
})
require.NoError(t, err)
assert.Equal(t, "9f86d081884c7d65", etag, "the bootstrap must return a validator")
})
}
// TestAgentNetwork_DeleteSettings_IfMatch covers the conditional delete on the
// wire, and that the plain method still sends nothing.
func TestAgentNetwork_DeleteSettings_IfMatch(t *testing.T) {
withMockClient(func(c *rest.Client, mux *http.ServeMux) {
var seen []string
mux.HandleFunc("/api/agent-network/settings", func(w http.ResponseWriter, r *http.Request) {
assert.Equal(t, "DELETE", r.Method)
seen = append(seen, r.Header.Get("If-Match"))
_, err := w.Write([]byte("{}"))
require.NoError(t, err)
})
require.NoError(t, c.AgentNetwork.DeleteSettingsIfMatch(context.Background(), "9f86d081884c7d65"))
require.NoError(t, c.AgentNetwork.DeleteSettings(context.Background()))
assert.Equal(t, []string{`"9f86d081884c7d65"`, ""}, seen,
"the conditional delete must carry the quoted validator and the plain one must carry nothing")
})
}

View File

@@ -31,6 +31,19 @@ func IsNotFound(err error) bool {
return false
}
// IsPreconditionFailed returns true if the error represents a 412 Precondition
// Failed response — an If-Match the server refused, or an endpoint's own
// precondition. A caller that sent a conditional request can use this to tell
// "someone else changed it, read again and retry" apart from a real failure;
// the message distinguishes it from an endpoint's other 412s.
func IsPreconditionFailed(err error) bool {
var apiErr *APIError
if ok := errors.As(err, &apiErr); ok {
return apiErr.StatusCode == http.StatusPreconditionFailed
}
return false
}
// Client Management service HTTP REST API Client
type Client struct {
managementURL string
@@ -218,6 +231,12 @@ func (c *Client) initialize() {
// NewRequest creates and executes new management API request
func (c *Client) NewRequest(ctx context.Context, method, path string, body io.Reader, query map[string]string) (*http.Response, error) {
return c.newRequest(ctx, method, path, body, query, nil)
}
// newRequest is NewRequest with request headers, for the endpoints whose
// contract includes one — conditional requests carrying If-Match.
func (c *Client) newRequest(ctx context.Context, method, path string, body io.Reader, query, headers map[string]string) (*http.Response, error) {
req, err := http.NewRequestWithContext(ctx, method, c.managementURL+path, body)
if err != nil {
return nil, err
@@ -231,6 +250,9 @@ func (c *Client) NewRequest(ctx context.Context, method, path string, body io.Re
if c.userAgent != "" {
req.Header.Set("User-Agent", c.userAgent)
}
for name, value := range headers {
req.Header.Set(name, value)
}
if len(query) != 0 {
q := req.URL.Query()

View File

@@ -6438,6 +6438,15 @@ components:
schema:
type: string
example: cot7r4n3l3vh3qj4qveg
ETag:
description: |
Strong entity-tag identifying the returned representation. Send it back
in `If-Match` on a subsequent write to make that write conditional, so
a change made between the read and the write is refused with `412`
rather than silently overwritten.
schema:
type: string
example: '"9f86d081884c7d65"'
securitySchemes:
BearerAuth:
type: http
@@ -13733,6 +13742,9 @@ paths:
responses:
'200':
description: Agent Network settings for the account
headers:
ETag:
$ref: '#/components/headers/ETag'
content:
application/json:
schema:
@@ -13760,6 +13772,9 @@ paths:
responses:
'200':
description: The freshly bootstrapped Agent Network settings
headers:
ETag:
$ref: '#/components/headers/ETag'
content:
application/json:
schema:
@@ -13778,11 +13793,25 @@ paths:
"$ref": "#/components/responses/internal_error"
put:
summary: Update Agent Network settings
description: Updates the account-level Agent Network settings; the request carries every field, replacing the mutable ones (collection toggles and retention). Returns 404 when the account has no settings row yet — bootstrap it with POST first. The endpoint and proxy address are assigned at bootstrap and immutable; the request must carry them unchanged, and a request carrying different values is rejected.
description: Updates the account-level Agent Network settings; the request carries every field, replacing the mutable ones (collection toggles and retention). Returns 404 when the account has no settings row yet — bootstrap it with POST first. The endpoint and proxy address are assigned at bootstrap and immutable; the request must carry them unchanged, and a request carrying different values is rejected. Supply `If-Match` to make the update conditional; without it the update is unconditional and the last write wins.
tags: [ Agent Network ]
security:
- BearerAuth: [ ]
- TokenAuth: [ ]
parameters:
- name: If-Match
in: header
required: false
description: |
Makes the update conditional on the settings not having changed since
they were read. Send the `ETag` from an earlier `GET`, `POST` or `PUT`,
or `*` to require only that a settings row exists. The precondition is
evaluated against the stored row inside the update's own transaction,
so two clients starting from the same `ETag` cannot both succeed.
Omitting the header leaves the update unconditional.
schema:
type: string
example: '"9f86d081884c7d65"'
requestBody:
description: Settings update request
content:
@@ -13792,6 +13821,9 @@ paths:
responses:
'200':
description: Updated Agent Network settings
headers:
ETag:
$ref: '#/components/headers/ETag'
content:
application/json:
schema:
@@ -13804,17 +13836,34 @@ paths:
"$ref": "#/components/responses/forbidden"
'404':
"$ref": "#/components/responses/not_found"
'412':
description: The `If-Match` precondition failed — the settings changed since they were read. The stored settings are unmodified; read them again and retry.
content: { }
'422':
"$ref": "#/components/responses/validation_failed"
'500':
"$ref": "#/components/responses/internal_error"
delete:
summary: Delete Agent Network settings
description: Deletes the account's Agent Network settings row, releasing the endpoint. Guarded — the delete is refused with 412 while any Agent Network provider exists for the account or while a proxy is actively serving the endpoint. Bootstrapping again after a delete allocates a new endpoint; the released hostname is not reserved.
description: Deletes the account's Agent Network settings row, releasing the endpoint. Guarded — the delete is refused with 412 while any Agent Network provider exists for the account or while a proxy is actively serving the endpoint. Bootstrapping again after a delete allocates a new endpoint; the released hostname is not reserved. Supply `If-Match` to make the delete conditional, which is worth doing here even more than on update — the other two guards are about state rather than staleness, so nothing else stops a client from deleting a row that was replaced since it read one.
tags: [ Agent Network ]
security:
- BearerAuth: [ ]
- TokenAuth: [ ]
parameters:
- name: If-Match
in: header
required: false
description: |
Makes the delete conditional on the settings not having changed since
they were read. Send the `ETag` from an earlier `GET`, `POST` or `PUT`,
or `*` to require only that a settings row exists. The precondition is
evaluated inside the delete's own transaction, ahead of the provider
and serving-proxy guards. Omitting the header leaves the delete
unconditional.
schema:
type: string
example: '"9f86d081884c7d65"'
responses:
'200':
description: Settings deleted
@@ -13825,7 +13874,7 @@ paths:
'404':
"$ref": "#/components/responses/not_found"
'412':
description: Delete refused — Agent Network providers still exist for the account, or a proxy is actively serving the endpoint
description: Delete refused — the `If-Match` precondition failed, or Agent Network providers still exist for the account, or a proxy is actively serving the endpoint. The stored settings are unmodified in every case; the response message distinguishes them.
content: { }
'500':
"$ref": "#/components/responses/internal_error"

View File

@@ -5939,6 +5939,28 @@ type GetApiAgentNetworkAccessLogsParamsSortBy string
// GetApiAgentNetworkAccessLogsParamsSortOrder defines parameters for GetApiAgentNetworkAccessLogs.
type GetApiAgentNetworkAccessLogsParamsSortOrder string
// DeleteApiAgentNetworkSettingsParams defines parameters for DeleteApiAgentNetworkSettings.
type DeleteApiAgentNetworkSettingsParams struct {
// IfMatch Makes the delete conditional on the settings not having changed since
// they were read. Send the `ETag` from an earlier `GET`, `POST` or `PUT`,
// or `*` to require only that a settings row exists. The precondition is
// evaluated inside the delete's own transaction, ahead of the provider
// and serving-proxy guards. Omitting the header leaves the delete
// unconditional.
IfMatch *string `json:"If-Match,omitempty"`
}
// PutApiAgentNetworkSettingsParams defines parameters for PutApiAgentNetworkSettings.
type PutApiAgentNetworkSettingsParams struct {
// IfMatch Makes the update conditional on the settings not having changed since
// they were read. Send the `ETag` from an earlier `GET`, `POST` or `PUT`,
// or `*` to require only that a settings row exists. The precondition is
// evaluated against the stored row inside the update's own transaction,
// so two clients starting from the same `ETag` cannot both succeed.
// Omitting the header leaves the update unconditional.
IfMatch *string `json:"If-Match,omitempty"`
}
// GetApiAgentNetworkUsageOverviewParams defines parameters for GetApiAgentNetworkUsageOverview.
type GetApiAgentNetworkUsageOverviewParams struct {
// Granularity Time bucket width. Defaults to day.

View File

@@ -0,0 +1,103 @@
package util
import (
"net/http"
"slices"
"strconv"
"strings"
)
const (
etagHeader = "ETag"
ifMatchHeader = "If-Match"
// matchAny is the If-Match value that matches any current representation
// of the resource (RFC 9110 §13.1.1).
matchAny = "*"
// weakPrefix marks a weak validator. If-Match is defined in terms of the
// strong comparison function, under which a weak validator never matches.
weakPrefix = "W/"
)
// SetETag writes etag as a strong ETag response header, quoted per RFC 9110.
// The value passed in is the bare validator — callers derive it (typically
// from the type being served) and this applies the wire syntax, so the quoting
// is decided in one place rather than at every handler.
//
// Call it before writing the body: once the response is committed the header
// no longer reaches the client. An empty etag writes no header at all, so a
// caller with nothing to validate against does not have to special-case it.
func SetETag(w http.ResponseWriter, etag string) {
if etag == "" {
return
}
w.Header().Set(etagHeader, strconv.Quote(etag))
}
// Precondition is a parsed If-Match request precondition. The zero value
// matches nothing; a nil *Precondition is an unconditional request and matches
// everything, so a handler can pass the result of IfMatch straight through
// without a presence check.
type Precondition struct {
// tags are the strong entity-tags the client will accept, unquoted.
tags []string
// any records the "*" form, which matches any current representation.
any bool
}
// IfMatch parses the request's If-Match precondition. It returns nil when the
// header is absent — an unconditional request, which is the back-compatible
// default: clients that know nothing of conditional requests keep working.
//
// A header that is present but carries nothing usable — empty, or nothing but
// weak validators — yields a precondition that matches nothing rather than
// nil. Failing closed is the only safe direction: a client that meant to send
// a precondition must not have it silently dropped and its write let through
// unguarded.
func IfMatch(r *http.Request) *Precondition {
values := r.Header.Values(ifMatchHeader)
if len(values) == 0 {
return nil
}
p := &Precondition{}
for _, value := range values {
for raw := range strings.SplitSeq(value, ",") {
candidate := strings.TrimSpace(raw)
switch {
case candidate == "":
// Tolerated rather than rejected: a stray comma changes
// nothing about what the client is willing to accept.
case candidate == matchAny:
p.any = true
case strings.HasPrefix(candidate, weakPrefix):
// Dropped, not unwrapped. If-Match uses strong comparison, so
// a weak validator cannot satisfy it — and unwrapping one into
// a strong tag would quietly grant the match the client's own
// header said it could not have.
default:
p.tags = append(p.tags, strings.Trim(candidate, `"`))
}
}
}
return p
}
// Matches reports whether etag — the bare validator of the resource as it
// currently stands — satisfies the precondition. A nil precondition matches
// everything.
//
// Callers must establish that the resource exists before consulting this: the
// "*" form asks whether there is any current representation, a question only
// the caller can answer, and this reports true for it.
func (p *Precondition) Matches(etag string) bool {
if p == nil {
return true
}
if p.any {
return true
}
return slices.Contains(p.tags, etag)
}

View File

@@ -0,0 +1,161 @@
package util
import (
"net/http"
"net/http/httptest"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
// TestSetETag covers the wire syntax: the bare validator goes in, a quoted
// strong entity-tag comes out. Handlers pass what the type derived, so the
// quoting has to happen here or every handler re-decides it.
func TestSetETag(t *testing.T) {
rec := httptest.NewRecorder()
SetETag(rec, "9f86d081884c7d65")
assert.Equal(t, `"9f86d081884c7d65"`, rec.Header().Get("ETag"),
"the validator must be emitted quoted")
}
// TestSetETagEmpty pins the no-op: a caller with nothing to validate against
// must not emit an empty entity-tag, which would be a validator that every
// later request could match.
func TestSetETagEmpty(t *testing.T) {
rec := httptest.NewRecorder()
SetETag(rec, "")
assert.Empty(t, rec.Header().Values("ETag"), "an empty validator must write no header")
}
// TestSetETagRoundTrip closes the loop between the two halves of the helper:
// what SetETag emits is what IfMatch accepts back. A client echoing the header
// it was given must match, or conditional requests never succeed in practice.
func TestSetETagRoundTrip(t *testing.T) {
const etag = "9f86d081884c7d65"
rec := httptest.NewRecorder()
SetETag(rec, etag)
r := httptest.NewRequest(http.MethodPut, "/", nil)
r.Header.Set("If-Match", rec.Header().Get("ETag"))
assert.True(t, IfMatch(r).Matches(etag), "an echoed ETag header must satisfy the precondition")
}
// TestIfMatchAbsent pins the back-compatibility guarantee: a request with no
// If-Match is unconditional, and the nil precondition it yields matches
// anything so handlers need no presence check.
func TestIfMatchAbsent(t *testing.T) {
p := IfMatch(httptest.NewRequest(http.MethodPut, "/", nil))
require.Nil(t, p, "an absent header must yield no precondition")
assert.True(t, p.Matches("9f86d081884c7d65"), "a nil precondition must match anything")
assert.True(t, p.Matches(""), "a nil precondition must not depend on the validator")
}
// TestIfMatchParsing walks the header forms a client can send. The weak and
// unusable cases are the ones that matter: each must yield a precondition that
// exists and refuses, never one that is absent and waves the write through.
func TestIfMatchParsing(t *testing.T) {
const current = "9f86d081884c7d65"
cases := []struct {
name string
header string
match bool
reason string
}{
{
name: "quoted current validator",
header: `"9f86d081884c7d65"`,
match: true,
reason: "the ordinary conditional request must be honoured",
},
{
name: "unquoted current validator",
header: "9f86d081884c7d65",
match: true,
reason: "a client that omits the quoting means the same thing, and only an exact value can match",
},
{
name: "stale validator",
header: `"0000000000000000"`,
match: false,
reason: "a validator from an earlier read must not match",
},
{
name: "star",
header: "*",
match: true,
reason: "* matches any current representation",
},
{
name: "list containing the current validator",
header: `"0000000000000000", "9f86d081884c7d65"`,
match: true,
reason: "If-Match is a list; any member matching is a match",
},
{
name: "list of stale validators",
header: `"0000000000000000", "1111111111111111"`,
match: false,
reason: "a list none of whose members match must not match",
},
{
name: "surrounding whitespace",
header: ` "9f86d081884c7d65" `,
match: true,
reason: "list whitespace is not part of the entity-tag",
},
{
name: "stray comma",
header: `"9f86d081884c7d65", `,
match: true,
reason: "an empty list element says nothing about what the client accepts",
},
{
name: "weak validator of the current representation",
header: `W/"9f86d081884c7d65"`,
match: false,
reason: "If-Match uses strong comparison, so a weak validator never satisfies it",
},
{
name: "weak validator alongside a strong one",
header: `W/"0000000000000000", "9f86d081884c7d65"`,
match: true,
reason: "dropping the weak member must not discard the rest of the list",
},
{
name: "empty header",
header: "",
match: false,
reason: "a precondition the server cannot make sense of must fail closed, not vanish",
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
r := httptest.NewRequest(http.MethodPut, "/", nil)
r.Header.Set("If-Match", tc.header)
p := IfMatch(r)
require.NotNil(t, p, "a header that was sent must yield a precondition: %s", tc.reason)
assert.Equal(t, tc.match, p.Matches(current), tc.reason)
})
}
}
// TestIfMatchRepeatedHeader covers the same list split across header lines,
// which is semantically identical to the comma form and which a proxy is free
// to produce.
func TestIfMatchRepeatedHeader(t *testing.T) {
r := httptest.NewRequest(http.MethodPut, "/", nil)
r.Header.Add("If-Match", `"0000000000000000"`)
r.Header.Add("If-Match", `"9f86d081884c7d65"`)
assert.True(t, IfMatch(r).Matches("9f86d081884c7d65"),
"entity-tags split across header lines must be read as one list")
}