diff --git a/e2e/agentnetwork/management_test.go b/e2e/agentnetwork/management_test.go index f4a821b44..496ef9060 100644 --- a/e2e/agentnetwork/management_test.go +++ b/e2e/agentnetwork/management_test.go @@ -142,14 +142,21 @@ func TestSettingsRoundTrip(t *testing.T) { require.NotEmpty(t, before.Endpoint, "settings must carry the bootstrapped endpoint") require.NotEmpty(t, before.ProxyAddress, "settings must carry the bootstrapped proxy address") + require.NotNil(t, before.AccessLogRetentionDays, "bootstrapped settings must carry a retention") + beforeRetention := *before.AccessLogRetentionDays + flipped, err := srv.UpdateSettings(ctx, api.AgentNetworkSettingsRequest{ EnableLogCollection: !before.EnableLogCollection, EnablePromptCollection: !before.EnablePromptCollection, RedactPii: !before.RedactPii, + AccessLogRetentionDays: beforeRetention, }) require.NoError(t, err, "update settings") assert.Equal(t, !before.EnableLogCollection, flipped.EnableLogCollection, "log collection toggle must flip") assert.Equal(t, !before.EnablePromptCollection, flipped.EnablePromptCollection, "prompt collection toggle must flip") + require.NotNil(t, flipped.AccessLogRetentionDays) + assert.Equal(t, beforeRetention, *flipped.AccessLogRetentionDays, + "retention sent unchanged must round-trip, not reset to the zero value") assert.Equal(t, before.Endpoint, flipped.Endpoint, "endpoint must be immutable across updates") assert.Equal(t, before.ProxyAddress, flipped.ProxyAddress, "proxy address must be immutable across updates") @@ -165,6 +172,7 @@ func TestSettingsRoundTrip(t *testing.T) { EnableLogCollection: before.EnableLogCollection, EnablePromptCollection: before.EnablePromptCollection, RedactPii: before.RedactPii, + AccessLogRetentionDays: beforeRetention, }) require.NoError(t, err, "restore settings") } diff --git a/e2e/agentnetwork/settings_bootstrap_test.go b/e2e/agentnetwork/settings_bootstrap_test.go index be4fc64f0..28c7b32c9 100644 --- a/e2e/agentnetwork/settings_bootstrap_test.go +++ b/e2e/agentnetwork/settings_bootstrap_test.go @@ -57,7 +57,8 @@ func TestSettingsBootstrapViaPost(t *testing.T) { // A PUT has no row to update yet — bootstrap is the explicit POST. _, err = fresh.UpdateSettings(ctx, api.AgentNetworkSettingsRequest{ - EnableLogCollection: true, + EnableLogCollection: true, + AccessLogRetentionDays: 30, }) requireClientError(t, err) @@ -95,8 +96,11 @@ func TestSettingsBootstrapViaPost(t *testing.T) { EnableLogCollection: true, EnablePromptCollection: false, RedactPii: true, + AccessLogRetentionDays: 21, }) require.NoError(t, err, "post-bootstrap update must succeed") + require.NotNil(t, persisted.AccessLogRetentionDays) + assert.Equal(t, 21, *persisted.AccessLogRetentionDays, "retention from the update must apply") assert.Equal(t, bootstrapped.Endpoint, persisted.Endpoint, "endpoint must survive updates untouched") assert.Equal(t, cluster, persisted.ProxyAddress, "proxy address must survive updates untouched") assert.True(t, persisted.EnableLogCollection, "post-bootstrap toggle must apply") diff --git a/management/internals/modules/agentnetwork/handlers/settings_handler_test.go b/management/internals/modules/agentnetwork/handlers/settings_handler_test.go index deefbe2c6..4d8a41f1e 100644 --- a/management/internals/modules/agentnetwork/handlers/settings_handler_test.go +++ b/management/internals/modules/agentnetwork/handlers/settings_handler_test.go @@ -166,10 +166,9 @@ func TestSettingsHandler_PutBeforeBootstrapIs404(t *testing.T) { } // TestSettingsHandler_PutReplacesMutableFields pins the update contract shared -// with the other PUT endpoints: the request replaces every mutable field, so a -// toggle absent from the JSON lands as its zero value rather than being -// preserved. The identity fields are not part of the PUT schema at all, so -// the endpoint and proxy address survive updates by construction. +// with the other PUT endpoints: the request replaces every mutable field, all +// four of which the schema requires. The identity fields are not part of the +// PUT schema at all, so the endpoint and proxy address survive by construction. func TestSettingsHandler_PutReplacesMutableFields(t *testing.T) { f := newAgentNetworkHandlerFixture(t) @@ -181,7 +180,7 @@ func TestSettingsHandler_PutReplacesMutableFields(t *testing.T) { require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &before)) rec = f.do(t, http.MethodPut, "/agent-network/settings", - `{"enable_log_collection": true, "enable_prompt_collection": false, "redact_pii": false}`) + `{"enable_log_collection": true, "enable_prompt_collection": false, "redact_pii": false, "access_log_retention_days": 7}`) require.Equal(t, http.StatusOK, rec.Code, "update PUT must succeed: %s", rec.Body.String()) var got api.AgentNetworkSettings @@ -190,8 +189,36 @@ func TestSettingsHandler_PutReplacesMutableFields(t *testing.T) { assert.False(t, got.EnablePromptCollection, "sent toggle must apply") assert.False(t, got.RedactPii, "sent toggle must apply") require.NotNil(t, got.AccessLogRetentionDays) - assert.Equal(t, 0, *got.AccessLogRetentionDays, - "retention absent from the request must land as the zero value — PUT replaces all mutable fields") + assert.Equal(t, 7, *got.AccessLogRetentionDays, "sent retention must apply") assert.Equal(t, before.Endpoint, got.Endpoint, "endpoint must survive updates untouched") assert.Equal(t, before.ProxyAddress, got.ProxyAddress, "proxy address must survive updates untouched") } + +// TestSettingsHandler_PutOmittedRetentionLandsAsZero documents a residual the +// required-ness of access_log_retention_days does not remove. Marking the field +// required changes the generated client type from *int to int, so a generated +// client cannot omit it — but nothing validates OpenAPI required-ness at +// runtime, so a hand-rolled body without the field still decodes as 0, which +// the API documents as "keep indefinitely". +// +// That is the same latitude the three booleans already have, so it is left +// consistent rather than special-cased. This test exists to make the gap +// explicit: if request validation is ever added, this expectation is what +// changes. +func TestSettingsHandler_PutOmittedRetentionLandsAsZero(t *testing.T) { + f := newAgentNetworkHandlerFixture(t) + + rec := f.do(t, http.MethodPost, "/agent-network/settings", + `{"proxy_address": "eu.proxy.netbird.io", "access_log_retention_days": 14}`) + require.Equal(t, http.StatusOK, rec.Code, "bootstrap POST must succeed: %s", rec.Body.String()) + + rec = f.do(t, http.MethodPut, "/agent-network/settings", + `{"enable_log_collection": true, "enable_prompt_collection": false, "redact_pii": false}`) + require.Equal(t, http.StatusOK, rec.Code, "update PUT must succeed: %s", rec.Body.String()) + + var got api.AgentNetworkSettings + require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &got)) + require.NotNil(t, got.AccessLogRetentionDays) + assert.Equal(t, 0, *got.AccessLogRetentionDays, + "a non-conforming body that omits retention still replaces it with the zero value") +} diff --git a/management/internals/modules/agentnetwork/types/settings.go b/management/internals/modules/agentnetwork/types/settings.go index 839745740..f154131a4 100644 --- a/management/internals/modules/agentnetwork/types/settings.go +++ b/management/internals/modules/agentnetwork/types/settings.go @@ -102,17 +102,22 @@ func (s *Settings) ToAPIResponse() *api.AgentNetworkSettings { return resp } -// FromAPIRequest applies the update request onto the receiver: the mutable -// collection fields are always replaced with the request values. The identity -// fields (Domain, ProxyAddress) are assigned at bootstrap and are not part of -// the update schema at all — immutability by shape, not by rejection. +// FromAPIRequest applies the update request onto the receiver: every mutable +// field is replaced with the request value. The identity fields (Domain, +// ProxyAddress) are assigned at bootstrap and are not part of the update schema +// at all — immutability by shape, not by rejection. +// +// All four fields are required by the schema, so none is presence-sensitive. +// AccessLogRetentionDays in particular must stay required: the caller receives +// a zero-valued Settings, and UpdateSettings copies each field onto the stored +// row unconditionally, so an omitted value would be written as 0 — which the +// API documents as "keep indefinitely". Making retention optional would +// therefore let a client silently maximise log retention by leaving it out. func (s *Settings) FromAPIRequest(req *api.AgentNetworkSettingsRequest) { s.EnableLogCollection = req.EnableLogCollection s.EnablePromptCollection = req.EnablePromptCollection s.RedactPii = req.RedactPii - if req.AccessLogRetentionDays != nil { - s.AccessLogRetentionDays = *req.AccessLogRetentionDays - } + s.AccessLogRetentionDays = req.AccessLogRetentionDays } // FromAPICreateRequest applies the optional collection toggles of a bootstrap diff --git a/shared/management/http/api/openapi.yml b/shared/management/http/api/openapi.yml index ce57a9843..a69dce17c 100644 --- a/shared/management/http/api/openapi.yml +++ b/shared/management/http/api/openapi.yml @@ -6290,6 +6290,7 @@ components: - enable_log_collection - enable_prompt_collection - redact_pii + - access_log_retention_days AgentNetworkBudgetRule: type: object description: Account-level budget rule. A limit-only rule bound to groups and/or users that applies across all policies as a min-wins ceiling. Empty targets means it applies to every caller. diff --git a/shared/management/http/api/types.gen.go b/shared/management/http/api/types.gen.go index 731bb9058..2ef559de7 100644 --- a/shared/management/http/api/types.gen.go +++ b/shared/management/http/api/types.gen.go @@ -2414,7 +2414,7 @@ type AgentNetworkSettingsCreateRequest struct { // AgentNetworkSettingsRequest Account-level Agent Network settings update. The request replaces every mutable field (the collection toggles and retention). The endpoint and proxy address are assigned at bootstrap (POST) and are not part of this schema. type AgentNetworkSettingsRequest struct { // AccessLogRetentionDays Days to retain full access-log rows; older rows are swept. 0 or less means keep indefinitely. - AccessLogRetentionDays *int `json:"access_log_retention_days,omitempty"` + AccessLogRetentionDays int `json:"access_log_retention_days"` // EnableLogCollection Whether per-request access-log entries are collected for this account's agent-network traffic. EnableLogCollection bool `json:"enable_log_collection"`