fix(agentnetwork): fold case in the delete guard's proxy-address match

Review follow-ups from #7085:

Proxies declare their cluster address verbatim and Connect stores it
unchanged, while the settings row is normalized lowercase — so on
case-sensitive collations a proxy declaring "GW.Example.com" slipped
past the settings delete guard. Hostnames are case-insensitive per
RFC 4343; the guard query now folds case on both sides, and the guard
test declares its proxy with mixed casing to pin that.

The re-bootstrap test asserted the fresh label differs from the
released one, but nothing guarantees that: the released hostname is
not reserved and a fresh draw may legitimately re-pick it. The
assertions now check the new row's shape (labeled beneath the
requested address, default toggles, persisted) instead of relying on
the RNG not colliding.

The settings bootstrap POST also marks its request body required in
the OpenAPI spec, matching the runtime behavior (a body-less POST is
rejected). The suggested oneOf exactly-one constraint was evaluated
and skipped: oapi-codegen renders it as a json.RawMessage union
wrapper on the generated type, which every caller would have to fight,
and nothing validates request bodies against the schema at runtime —
the handler's 422 stays the enforcement.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Brad Ison
2026-08-10 11:07:15 +02:00
parent 16b42e402c
commit 1a39e50f70
3 changed files with 24 additions and 9 deletions

View File

@@ -320,6 +320,11 @@ func TestSettingsHandler_DeleteBlockedByProviders(t *testing.T) {
// row declaring the endpoint hostname as its cluster address, the dedicated
// shape — the delete is refused with 412. A proxy that has disconnected no
// longer blocks: the guard is about a live serving path, not history.
//
// The proxy declares its address with mixed casing on purpose: Connect
// stores the declared address verbatim while the settings row is normalized
// lowercase, and hostnames are case-insensitive, so the guard must match
// across the casing difference rather than be sidestepped by it.
func TestSettingsHandler_DeleteBlockedByActiveProxy(t *testing.T) {
f := newAgentNetworkHandlerFixture(t)
@@ -332,7 +337,7 @@ func TestSettingsHandler_DeleteBlockedByActiveProxy(t *testing.T) {
proxyRow := &rpproxy.Proxy{
ID: "proxy-guard",
SessionID: "sess-1",
ClusterAddress: endpoint,
ClusterAddress: "GW.Dedicated.Example.Com",
AccountID: &accountID,
LastSeen: now,
ConnectedAt: &now,
@@ -355,16 +360,16 @@ func TestSettingsHandler_DeleteBlockedByActiveProxy(t *testing.T) {
// TestSettingsHandler_DeleteReleasesEndpointForFreshBootstrap pins the
// full-reset semantic that gives replace-on-change clients (e.g. Terraform's
// RequiresReplace) a real path: with both guards clear the delete succeeds,
// the account reads as the defaults again, and a fresh bootstrap allocates a
// new endpoint rather than resurrecting the released one.
// the account reads as the defaults again, and a fresh bootstrap draws a
// fresh label. The released hostname is not reserved — a fresh draw may even
// legitimately re-pick it — so the assertions check the new row's shape, not
// that the label differs.
func TestSettingsHandler_DeleteReleasesEndpointForFreshBootstrap(t *testing.T) {
f := newAgentNetworkHandlerFixture(t)
rec := f.do(t, http.MethodPost, "/agent-network/settings",
`{"proxy_address": "eu.proxy.netbird.io", "enable_prompt_collection": true}`)
require.Equal(t, http.StatusOK, rec.Code, "bootstrap POST must succeed: %s", rec.Body.String())
var first api.AgentNetworkSettings
require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &first))
rec = f.do(t, http.MethodDelete, "/agent-network/settings", "")
require.Equal(t, http.StatusOK, rec.Code,
@@ -381,7 +386,10 @@ func TestSettingsHandler_DeleteReleasesEndpointForFreshBootstrap(t *testing.T) {
require.Equal(t, http.StatusOK, rec.Code, "re-bootstrap after delete must succeed: %s", rec.Body.String())
var second api.AgentNetworkSettings
require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &second))
require.NotEmpty(t, second.Endpoint)
assert.NotEqual(t, first.Endpoint, second.Endpoint,
"re-creating allocates a new endpoint; the released hostname is not reserved")
require.NotEmpty(t, second.Endpoint, "the fresh bootstrap must allocate an endpoint")
assert.True(t, strings.HasSuffix(second.Endpoint, ".eu.proxy.netbird.io"),
"the fresh endpoint must hang beneath the requested proxy address: %s", second.Endpoint)
assert.False(t, second.EnablePromptCollection,
"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")
}

View File

@@ -6345,11 +6345,17 @@ func (s *SqlStore) CountProxiesByAccountID(ctx context.Context, accountID string
// the same connected-within-threshold window as the other active-proxy
// queries. Backs the agent-network settings delete guard: settings cannot be
// deleted while a proxy declares the endpoint hostname as its address.
//
// The comparison folds case on both sides: the caller passes a normalized
// (lowercase) hostname, but proxies declare their cluster address verbatim
// and Connect stores it unchanged, so on case-sensitive collations a proxy
// declaring "GW.Example.com" would otherwise slip past the guard. Hostnames
// are case-insensitive per RFC 4343; the guard must be too.
func (s *SqlStore) HasActiveProxyAtClusterAddress(ctx context.Context, clusterAddress string) (bool, error) {
var count int64
result := s.db.
Model(&proxy.Proxy{}).
Where("cluster_address = ? AND status = ? AND last_seen > ?", clusterAddress, proxy.StatusConnected, time.Now().Add(-proxyActiveThreshold)).
Where("LOWER(cluster_address) = LOWER(?) AND status = ? AND last_seen > ?", clusterAddress, proxy.StatusConnected, time.Now().Add(-proxyActiveThreshold)).
Count(&count)
if result.Error != nil {
log.WithContext(ctx).Errorf("failed to count active proxies at cluster address: %v", result.Error)

View File

@@ -13751,6 +13751,7 @@ paths:
- BearerAuth: [ ]
- TokenAuth: [ ]
requestBody:
required: true
description: Settings bootstrap request
content:
application/json: