From a4c5cfd26d1e3384d6d87d9113b226c077c739c3 Mon Sep 17 00:00:00 2001 From: rcourtman Date: Thu, 25 Jun 2026 20:20:35 +0100 Subject: [PATCH] Fix encoded resource type filters Treat browser-encoded comma-separated resource type lists the same as literal comma filters so Docker and platform pages keep canonical inventory rows. --- .../v6/internal/subsystems/agent-lifecycle.md | 5 ++ .../v6/internal/subsystems/api-contracts.md | 3 ++ .../internal/subsystems/storage-recovery.md | 5 ++ .../internal/subsystems/unified-resources.md | 4 ++ .../__tests__/useUnifiedResources.test.ts | 23 +++++++++ internal/api/contract_test.go | 47 +++++++++++++++++++ internal/api/resources.go | 1 + internal/api/resources_frontend_types_test.go | 32 +++++++++++++ 8 files changed, 120 insertions(+) diff --git a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md index 455cd7e8a6..ef9650270d 100644 --- a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md +++ b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md @@ -801,6 +801,11 @@ surface and no new `internal/api/` lifecycle handler. order instead of inheriting map order or page-local re-sorts, so install and runtime hydration do not present one resource ordering at first load and a different ordering after the first live refresh. + Those same resource-list reads must preserve browser-encoded comma-separated + `/api/resources` type filters. Lifecycle-adjacent consumers that request + agent or Docker host resource rows through standard query builders must + receive the same canonical resource set as literal comma filters instead of + treating `%2C` as part of a resource type token. Those lifecycle-adjacent reads may observe resource API `policyPosture` aggregation as read-only data-governance context, but they must not reinterpret sensitivity, routing, or redaction counts as install diff --git a/docs/release-control/v6/internal/subsystems/api-contracts.md b/docs/release-control/v6/internal/subsystems/api-contracts.md index 9673300935..c85f5102b4 100644 --- a/docs/release-control/v6/internal/subsystems/api-contracts.md +++ b/docs/release-control/v6/internal/subsystems/api-contracts.md @@ -1072,6 +1072,9 @@ payload shape change when the portal presents compact client rows. 59. `internal/agentcapabilities/types.go` shared with `ai-runtime`: the agent capabilities manifest wire type, manifest-owned external-adapter surface tool contract field, capability display title and structured output schema fields, approval-policy vocabulary, capability governance normalization, and tool-governance descriptor shape are both the canonical API payload contract and the AI runtime projection contract for Pulse Assistant and MCP-facing agent tools. 60. `internal/agentcapabilities/workflow_prompt.go` shared with `ai-runtime`: the Pulse Intelligence workflow prompt catalogue, manifest-owned `workflowPrompts` projection, MCP prompt title projection, presentation kind hints, shared resource-context and finding argument vocabulary, Patrol issue-handling capability gating, argument validation, and manifest-gated shared prompt rendering rules are both the AI runtime starter contract for Assistant-compatible surfaces and the canonical API/agent prompt projection contract for MCP-facing clients. 61. `internal/api/access_control_handlers.go` shared with `organization-settings`: RBAC role and user-assignment handlers are both an organization settings control surface and a canonical API payload contract boundary. + The `/api/resources` type filter also treats URL-encoded comma separators + (`%2C`) the same as literal comma-separated type lists, because frontend + consumers build these requests through standard URL query encoders. The shared node setup boundary above owns the guided/manual setup split for PVE/PBS consumers: API Inventory and Host Telemetry Agent setup modes are auto-registration paths, while Token ID/Value fields, Test Connection, diff --git a/docs/release-control/v6/internal/subsystems/storage-recovery.md b/docs/release-control/v6/internal/subsystems/storage-recovery.md index 1718aed012..54cdc2eaaa 100644 --- a/docs/release-control/v6/internal/subsystems/storage-recovery.md +++ b/docs/release-control/v6/internal/subsystems/storage-recovery.md @@ -564,6 +564,11 @@ recovery scope, or a storage/recovery-owned secret source. state payload must publish the same canonical resource types and display labels as `/api/resources` so storage and recovery do not momentarily switch between legacy and canonical infrastructure identities within one session. + Adjacent list surfaces that build `/api/resources` queries with standard + browser encoders must also treat `%2C` separators the same as literal + comma-separated type filters, so storage and recovery consumers do not lose + canonical Docker host, agent, or workload rows when sharing the unified + resource endpoint. That same adjacent API boundary now also owns SSO outbound discovery and metadata fetch trust: storage- and recovery-adjacent surfaces may share `internal/api/sso_outbound.go`, `internal/api/saml_service.go`, and `internal/api/oidc_service.go`, but they must not fork separate metadata/discovery HTTP clients, redirect policies, or credential-file read rules when they depend on shared backend auth helpers. That same adjacent security-status boundary now also owns paid prompt suppression for storage/recovery-adjacent primitives. History and recovery diff --git a/docs/release-control/v6/internal/subsystems/unified-resources.md b/docs/release-control/v6/internal/subsystems/unified-resources.md index f311b692eb..68d5fb9016 100644 --- a/docs/release-control/v6/internal/subsystems/unified-resources.md +++ b/docs/release-control/v6/internal/subsystems/unified-resources.md @@ -512,6 +512,10 @@ container inventory table. runtime workload belongs to both Docker and an owning infrastructure platform. 16. `internal/api/resources.go` shared with `api-contracts`: the unified resource endpoint is both a backend payload contract surface and a unified-resource runtime boundary. + `/api/resources` type filters must accept URL-encoded comma-separated lists + from browser query builders exactly like literal comma separators, so Docker + / Podman runtime pages do not lose `docker-host` inventory while requesting + canonical resource types. App-container Discovery targets are part of that shared payload contract. Backend resources must expose frontend-facing `resourceType=app-container` with the reporting Docker/Podman agent id and the stable container name as diff --git a/frontend-modern/src/hooks/__tests__/useUnifiedResources.test.ts b/frontend-modern/src/hooks/__tests__/useUnifiedResources.test.ts index 4123457444..9e9de416d5 100644 --- a/frontend-modern/src/hooks/__tests__/useUnifiedResources.test.ts +++ b/frontend-modern/src/hooks/__tests__/useUnifiedResources.test.ts @@ -307,6 +307,29 @@ describe('useUnifiedResources', () => { dispose(); }); + it('keeps comma-separated type filters in the browser-encoded resource query shape', async () => { + let dispose = () => {}; + let result: ReturnType | undefined; + createRoot((d) => { + dispose = d; + result = useUnifiedResources({ + query: 'type=agent,docker-host,app-container', + cacheKey: 'docker-overview-contract', + }); + }); + + await flushAsync(); + expect(apiFetchMock).toHaveBeenCalledTimes(1); + expect(apiFetchMock).toHaveBeenNthCalledWith( + 1, + '/api/resources?type=agent%2Cdocker-host%2Capp-container&page=1&limit=100', + { cache: 'no-store' }, + ); + await waitForResourceCount(() => result!.resources().length); + + dispose(); + }); + it('prefers websocket initial hydration over an immediate REST fetch for dashboard snapshots', async () => { setWsConnected(false); setWsInitialDataReceived(false); diff --git a/internal/api/contract_test.go b/internal/api/contract_test.go index 768cfc0e64..33e587bceb 100644 --- a/internal/api/contract_test.go +++ b/internal/api/contract_test.go @@ -11979,6 +11979,53 @@ func TestContract_ResourceListUsesDeterministicNameTieBreakers(t *testing.T) { } } +func TestContract_ResourceListAcceptsBrowserEncodedTypeCSV(t *testing.T) { + now := time.Date(2026, 4, 12, 0, 0, 0, 0, time.UTC) + h := NewResourceHandlers(&config.Config{DataPath: t.TempDir()}) + h.SetStateProvider(resourceUnifiedSeedProvider{ + snapshot: models.StateSnapshot{LastUpdate: now}, + resources: []unifiedresources.Resource{ + {ID: "agent-delly", Type: unifiedresources.ResourceTypeAgent, Name: "delly", Status: unifiedresources.StatusOnline, LastSeen: now}, + {ID: "docker-host-frigate", Type: "docker-host", Name: "frigate", Status: unifiedresources.StatusOnline, LastSeen: now}, + {ID: "container-frigate", Type: unifiedresources.ResourceTypeAppContainer, Name: "frigate", Status: unifiedresources.StatusOnline, LastSeen: now}, + {ID: "vm-ignored", Type: unifiedresources.ResourceTypeVM, Name: "ignored-vm", Status: unifiedresources.StatusOnline, LastSeen: now}, + }, + }) + + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodGet, "/api/resources?type=agent%2Cdocker-host%2Capp-container&page=1&limit=100", nil) + h.HandleListResources(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, body=%s", rec.Code, rec.Body.String()) + } + + var resp ResourcesResponse + if err := json.NewDecoder(rec.Body).Decode(&resp); err != nil { + t.Fatalf("decode response: %v", err) + } + + gotTypes := make(map[unifiedresources.ResourceType]int) + for _, resource := range resp.Data { + gotTypes[resource.Type]++ + } + if len(resp.Data) != 3 { + t.Fatalf("expected 3 encoded-filtered resources, got %d (types=%v)", len(resp.Data), gotTypes) + } + for _, typ := range []unifiedresources.ResourceType{ + unifiedresources.ResourceTypeAgent, + "docker-host", + unifiedresources.ResourceTypeAppContainer, + } { + if gotTypes[typ] != 1 { + t.Fatalf("encoded type filter count for %q = %d, want 1 (types=%v)", typ, gotTypes[typ], gotTypes) + } + } + if gotTypes[unifiedresources.ResourceTypeVM] != 0 { + t.Fatalf("encoded type filter leaked VM resources: %v", gotTypes) + } +} + func TestContract_StateAndResourceListShareCanonicalMockResourceContract(t *testing.T) { setMockModeForTest(t, true) diff --git a/internal/api/resources.go b/internal/api/resources.go index 5282903a03..83080e3a77 100644 --- a/internal/api/resources.go +++ b/internal/api/resources.go @@ -2498,6 +2498,7 @@ func pmgHostname(pmg *unified.PMGData) string { } func splitCSV(raw string) []string { + raw = strings.NewReplacer("%2c", ",", "%2C", ",").Replace(raw) parts := strings.Split(raw, ",") out := make([]string, 0, len(parts)) for _, part := range parts { diff --git a/internal/api/resources_frontend_types_test.go b/internal/api/resources_frontend_types_test.go index 47a02330c4..2d809b218b 100644 --- a/internal/api/resources_frontend_types_test.go +++ b/internal/api/resources_frontend_types_test.go @@ -234,6 +234,11 @@ func TestParseResourceTypesNodeAlias(t *testing.T) { unified.ResourceTypeAgent: {}, unified.ResourceTypeVM: {}, }}, + {name: "url encoded csv from browser query builder", input: "agent%2Cdocker-host%2Capp-container", want: map[unified.ResourceType]struct{}{ + unified.ResourceTypeAgent: {}, + "docker-host": {}, + unified.ResourceTypeAppContainer: {}, + }}, // Mixed case — splitCSV lowercases tokens {name: "mixed case NoDe,VM", input: "NoDe,VM", want: map[unified.ResourceType]struct{}{ unified.ResourceTypeAgent: {}, @@ -456,6 +461,33 @@ func TestResourceListUsesCanonicalContractTypes(t *testing.T) { t.Fatalf("expected docker-host metrics target, got %+v", dockerResp.Data[0].MetricsTarget) } + // Browser URL builders encode comma-separated filters as %2C. The REST + // boundary must treat that as the same canonical type list so platform + // pages do not drop Docker hosts when using URLSearchParams. + encodedDockerRec := httptest.NewRecorder() + encodedDockerReq := httptest.NewRequest(http.MethodGet, "/api/resources?type=agent%2Cdocker-host&page=1&limit=100", nil) + h.HandleListResources(encodedDockerRec, encodedDockerReq) + + if encodedDockerRec.Code != http.StatusOK { + t.Fatalf("encoded docker-host status = %d, body=%s", encodedDockerRec.Code, encodedDockerRec.Body.String()) + } + + var encodedDockerResp ResourcesResponse + if err := json.NewDecoder(encodedDockerRec.Body).Decode(&encodedDockerResp); err != nil { + t.Fatalf("decode encoded docker-host response: %v", err) + } + + encodedTypeSet := make(map[unified.ResourceType]int) + for _, r := range encodedDockerResp.Data { + encodedTypeSet[r.Type]++ + } + if encodedTypeSet["agent"] != 2 { + t.Fatalf("encoded type filter agent count = %d, want 2 (types=%v)", encodedTypeSet["agent"], encodedTypeSet) + } + if encodedTypeSet["docker-host"] != 1 { + t.Fatalf("encoded type filter docker-host count = %d, want 1 (types=%v)", encodedTypeSet["docker-host"], encodedTypeSet) + } + // Test 3: Verify ByType aggregation uses canonical contract names. if resp.Aggregations.ByType["agent"] != 2 { t.Fatalf("aggregations.byType[agent] = %d, want 2 (got byType=%v)", resp.Aggregations.ByType["agent"], resp.Aggregations.ByType)