From 9bd1b6e58202aed9458901b89f35281a7ad0a3c7 Mon Sep 17 00:00:00 2001 From: rcourtman <8825017+rcourtman@users.noreply.github.com> Date: Wed, 7 Oct 2026 20:52:31 +0100 Subject: [PATCH] Let only POST narrate single-resource reports with Pulse Assistant GET /api/admin/reports/generate resolved the tenant's AI narrator, so a PDF requested with GET made a paid provider call and appended a usage event to the AI cost ledger. GET passes the demo-mode guard and the CSRF check, and a SameSite=Lax session cookie rides a cross-site top-level GET navigation, so a link could spend a signed-in admin's AI budget. POST with a JSON body, decoded with the generate-multi rules, is now the canonical single-resource transport and the only one that may use the AI narrator. The settings UI sends it, following a new performanceReport.singleResourceMethod catalog field; a catalog without the field, and the legacy fallback catalog, keep the GET transport that older backends accept. GET stays available with the same query parameters but narrates deterministically, printing a note in place of the "Configure Pulse Assistant" tip, and other methods get 405 with Allow: GET, POST. GET also stops resolving the AI service at all: it reads Patrol findings through a new ExistingAIService lookup, because GetAIService constructs a tenant service on first use and that can list provider models or start background discovery. The router's reporting wiring moves into wireReportingAIResolvers so a test can drive it. Keeping GET rather than answering 405 avoids breaking documented API scripts (MSP.md pointed providers at it) for no security gain: CSV is never narrated and GET no longer reaches the provider. --- docs/API.md | 11 +- docs/MSP.md | 6 +- .../v6/internal/subsystems/agent-lifecycle.md | 10 ++ .../v6/internal/subsystems/ai-runtime.md | 12 ++ .../v6/internal/subsystems/api-contracts.md | 26 +++ .../subsystems/deployment-installability.md | 8 + .../subsystems/frontend-primitives.md | 7 + .../internal/subsystems/security-privacy.md | 12 ++ .../internal/subsystems/storage-recovery.md | 13 ++ .../dad3b270f160419c.json | 42 +++++ frontend-modern/public/docs/API.md | 11 +- frontend-modern/public/docs/MSP.md | 6 +- .../__tests__/ReportingPanel.test.tsx | 1 + .../reportingCatalogModel.branchcov2.test.ts | 14 ++ .../__tests__/reportingCatalogModel.test.ts | 2 + .../__tests__/reportingPanelModel.test.ts | 59 ++++++- .../__tests__/useReportingPanelState.test.ts | 43 +++++ .../Settings/reportingCatalogModel.ts | 8 + .../Settings/reportingPanelModel.ts | 35 +++- .../src/utils/__tests__/docsLinks.test.ts | 21 +++ internal/api/ai_handlers.go | 19 ++ internal/api/ai_handlers_more_test.go | 30 ++++ internal/api/contract_test.go | 164 ++++++++++++++++++ internal/api/metrics_reporting_handlers.go | 145 +++++++++++++--- internal/api/reporting_handlers_test.go | 43 ++++- internal/api/router.go | 72 +++++--- pkg/reporting/catalog.go | 3 + 27 files changed, 739 insertions(+), 84 deletions(-) create mode 100644 frontend-modern/browser-verification/dad3b270f160419c.json diff --git a/docs/API.md b/docs/API.md index 296d4cc116..50b8809acd 100644 --- a/docs/API.md +++ b/docs/API.md @@ -836,9 +836,12 @@ delivery for all resources. - `GET /api/admin/reports/catalog` (admin, `settings:read`) - Returns the canonical reporting catalog for the settings surface, including locked-state teaser copy, enabled-surface guidance copy, performance-report options, canonical single-report filename subject, canonical fallback filename date style, and the nested VM inventory export definition. - Metadata route: readable without the `advanced_reporting` feature so locked admin surfaces can render the same reporting definition before upsell. -- `GET /api/admin/reports/generate` (admin, `settings:read`) - - Query params: `format` (pdf/csv, default `pdf`), `resourceType`, `resourceId`, `metricType` (optional), `start`/`end` (RFC3339, optional; defaults to last 24h), `title` (optional) +- `POST /api/admin/reports/generate` (admin, `settings:read`) + - Body fields: `format` (pdf/csv, default `pdf`), `resourceType`, `resourceId`, `metricType` (optional), `start`/`end` (RFC3339, optional; defaults to last 24h), `title` (optional) - If `title` is omitted, the backend applies the canonical default title for that resource report. + - When Pulse Assistant is configured, the configured AI provider narrates a PDF's executive summary; each narration counts against the AI budget and appears in AI usage. If the call fails, times out or the budget is spent, the PDF falls back to the deterministic summary. CSV output is never narrated. The settings UI uses this method. +- `GET /api/admin/reports/generate` (admin, `settings:read`) + - Takes the same fields as query params and returns the same report, except that it never calls an AI provider: a PDF carries the deterministic summary and a note saying AI narration needs `POST`. Other methods return `405` with `Allow: GET, POST`. - `POST /api/admin/reports/generate-multi` (admin, `settings:read`) - Body fields: `resources` (1-50 entries of `{resourceType,resourceId}`), `format`, `metricType` (optional), `start`/`end` (RFC3339, optional; defaults to last 24h), `title` (optional) - If `title` is omitted, the backend applies the canonical default fleet report title. @@ -851,8 +854,8 @@ Validation and limits: - Malformed `start`/`end` values, `end` not strictly after `start`, or report windows over 366 days return `400 invalid_time_range`. - `metricType` must match `[a-zA-Z0-9._:-]+` and be <= 64 chars, otherwise `400 invalid_metric_type`. - `title` must be <= 256 chars, otherwise `400 invalid_title`. -- Multi-report body max size is 1MB; oversized payloads return `400 body_too_large`. -- Multi-report bodies reject trailing payload and unknown JSON fields with `400 invalid_body`. +- Report body max size is 1MB; oversized payloads return `400 body_too_large`. +- Report bodies reject trailing payload and unknown JSON fields with `400 invalid_body`. - VM inventory export only accepts `csv`. Common reporting error codes: diff --git a/docs/MSP.md b/docs/MSP.md index 7561f520c6..9302b66a1d 100644 --- a/docs/MSP.md +++ b/docs/MSP.md @@ -260,10 +260,12 @@ Each client runtime (or organization) generates its own reports, scoped to that client's resources: - **UI**: Settings → Data & Reports. -- **API**: `GET /api/admin/reports/generate` (single resource) and +- **API**: `POST /api/admin/reports/generate` (single resource) and `POST /api/admin/reports/generate-multi` (up to 50 resources per report), returning PDF or CSV. In shared-process mode, scope with `X-Pulse-Org-ID` - or an org-bound token. + or an org-bound token. `GET /api/admin/reports/generate` with query + parameters still returns a single-resource report, but never adds the + Pulse Assistant narrative. - **Schedules**: `GET`/`POST /api/admin/reports/schedules`, `PUT`/`DELETE /api/admin/reports/schedules/{id}`, and `POST /api/admin/reports/schedules/{id}/run`. Schedules can target explicit diff --git a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md index a2ee20031e..d78de6b180 100644 --- a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md +++ b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md @@ -7639,6 +7639,16 @@ bounded and classified temperature nodes, and leaves the host report intact when the helper is absent or incompatible. It does not treat Windows ACPI thermal zones as hardware sensor evidence, accept remote provider locations, or grant Pulse hardware-control authority. +### Single-resource report AI narration requires POST + +`internal/api/metrics_reporting_handlers.go`, `internal/api/router.go` and +`internal/api/ai_handlers.go` changed only how `/api/admin/reports/generate` +reads its fields (a `POST` JSON body or `GET` query parameters), which of those +may use the tenant's AI narrator (`POST` only), and how a `GET` finds Patrol +findings (`ExistingAIService`, which never constructs a tenant AI service). No +agent registration, enrolment, install, update, removal or report ingest path +changed. + ### Agent profile adoption is reported as a count only The usage telemetry snapshot reads `LoadAgentProfiles` and reports diff --git a/docs/release-control/v6/internal/subsystems/ai-runtime.md b/docs/release-control/v6/internal/subsystems/ai-runtime.md index 9ab54a9d35..fb573ab97a 100644 --- a/docs/release-control/v6/internal/subsystems/ai-runtime.md +++ b/docs/release-control/v6/internal/subsystems/ai-runtime.md @@ -7510,6 +7510,18 @@ prompt budget. Reporting is therefore an additive consumer of AI runtime, not a new ownership boundary, and the narrator/findings surfaces inherit the same governance the rest of the canonical AI runtime already enforces. +Report narration is caller-requested spend, so on the single-resource HTTP +route only `POST /api/admin/reports/generate` reaches it (fleet reports, from +`POST .../generate-multi` or a report schedule, use the fleet narrator). The +reporting narrator resolver goes through `AISettingsHandler.GetAIService`, +which constructs a tenant service on first use and can then list provider +models or start background discovery. A GET single-resource report never calls +that resolver: +it narrates deterministically and reads Patrol findings through +`AISettingsHandler.ExistingAIService`, which returns only a service that is +already running and never constructs one +(`TestAISettingsHandler_ExistingAIServiceNeverConstructs`, and through the +router's real wiring `TestContract_SingleReportGETDoesNotConstructTenantAIService`). The same canonical AI runtime now also owns the fleet-level report narrative through `report_fleet_narrator.go`. `Service` implements diff --git a/docs/release-control/v6/internal/subsystems/api-contracts.md b/docs/release-control/v6/internal/subsystems/api-contracts.md index 3015b71efb..b00b8c50a6 100644 --- a/docs/release-control/v6/internal/subsystems/api-contracts.md +++ b/docs/release-control/v6/internal/subsystems/api-contracts.md @@ -9650,6 +9650,32 @@ narrator must fail closed: nil provider, parse failure, timeout, or empty response causes the engine to fall back to the heuristic narrative without surfacing the AI failure to the caller, so reporting is never blocked by AI availability. +Only `POST /api/admin/reports/generate` may hand the tenant's AI narrator to +the engine, because a narrated PDF makes a paid provider call and appends a +`cost.UsageEvent` to the cost ledger. The single-resource handler accepts +`POST` with a JSON body (the `generate-multi` decoding rules: 1MB cap, unknown +fields and trailing payload rejected, through `decodeReportingRequestBody`) and +`GET` with the same fields as query parameters, and answers any other method +with `405` and `Allow: GET, POST`. GET never calls `resolveNarrator`, whose +`GetAIService` can construct a tenant AI service (and with it list provider +models or start background discovery): it narrates through `getReportNarrator`, +the heuristic summary whose `Disclaimer` replaces the "Configure Pulse +Assistant" tip with a note that AI narration needs POST, and reads Patrol +findings through `SetExistingFindingsResolver`, which `router.go` backs with +`AISettingsHandler.ExistingAIService` so only an already running service is +used. GET passes the demo-mode guard and the CSRF check, and a SameSite=Lax +session cookie rides a cross-site top-level GET navigation, so a link must not +be able to spend AI budget. The reporting catalog advertises the transport as +`performanceReport.singleResourceMethod` (`POST`), and the settings UI follows +it. `generate-multi` and the manual schedule run endpoint +(`POST /api/admin/reports/schedules/{id}/run`) were already POST-only; due +schedules run from the server's scheduler, not an HTTP request. +`TestContract_SingleReportAINarrationRequiresPOST` pins the rule on a real +engine: GET, HEAD, PUT and DELETE make no narrator call and no AI service +resolution, while POST makes one of each. +`TestContract_SingleReportGETDoesNotConstructTenantAIService` drives +`Router.wireReportingAIResolvers` with an uncached tenant: GET constructs no +tenant AI service and POST resolves one. Report alerts keep the alert engine's resolution. The node, VM and container enrichers in `internal/api/metrics_reporting_handlers.go` build every row through `reportActiveAlertInfo` and `reportResolvedAlertInfo`, diff --git a/docs/release-control/v6/internal/subsystems/deployment-installability.md b/docs/release-control/v6/internal/subsystems/deployment-installability.md index 3af74e0266..14ef915a65 100644 --- a/docs/release-control/v6/internal/subsystems/deployment-installability.md +++ b/docs/release-control/v6/internal/subsystems/deployment-installability.md @@ -6101,6 +6101,14 @@ summaries whose scope, format, and attachments the server fixes. The delivery surface (`/api/admin/reports/schedules`) and tenant-local storage are unchanged. +### Provider docs name POST for single-resource reports + +`docs/MSP.md` and its shipped copy now give `POST /api/admin/reports/generate` +for per-client single-resource reports and say a `GET` with query parameters +still returns the report without the Pulse Assistant narrative, matching the +server, which lets only `POST` narrate. Install, upgrade and provider bundle +delivery are unchanged. + ### Convergence log-reader compatibility (5 September 2026) Scheduled reconciliation run 33939926237 failed when `gh api` refused terminal diff --git a/docs/release-control/v6/internal/subsystems/frontend-primitives.md b/docs/release-control/v6/internal/subsystems/frontend-primitives.md index fd73a5370c..ba4fa40da4 100644 --- a/docs/release-control/v6/internal/subsystems/frontend-primitives.md +++ b/docs/release-control/v6/internal/subsystems/frontend-primitives.md @@ -4828,6 +4828,13 @@ performance-report transport (`/api/reporting` and `/api/reporting/generate-mult so the reporting panel does not go dead on mixed-version installs, but that compatibility path is intentionally report-only and must not invent the newer catalog-owned VM inventory export surface. +`reportingPanelModel.ts` picks the single-resource transport from the catalog's +`performanceReport.singleResourceMethod`: `POST` sends the request fields as a +JSON body, which is the only transport the server lets narrate a PDF with Pulse +Assistant, and `GET` sends the same fields as query parameters. +`reportingCatalogModel.ts` reads a missing field as `GET`, the only method a +backend that predates the field accepts, and the legacy compatibility catalog +names `GET` for `/api/reporting`. Fleet reports stay `POST` on every backend. `ReportingPanel.tsx` must therefore treat `vmInventoryExport` as optional when it renders a governed reporting catalog. A legacy compatibility catalog with no inventory export still owns a valid enabled reporting surface and must continue diff --git a/docs/release-control/v6/internal/subsystems/security-privacy.md b/docs/release-control/v6/internal/subsystems/security-privacy.md index 1524faa42c..bdc77ad4f1 100644 --- a/docs/release-control/v6/internal/subsystems/security-privacy.md +++ b/docs/release-control/v6/internal/subsystems/security-privacy.md @@ -3025,6 +3025,18 @@ Removing the disconnected recorder does not alter alert, action approval or operator authority. An unrelated organization receives no default archive fallback. +### Report GETs cannot spend AI budget + +`Router.wireReportingAIResolvers` in `internal/api/router.go` wires a second +reporting resolver, `SetExistingFindingsResolver`, backed by +`AISettingsHandler.ExistingAIService`. +A `GET /api/admin/reports/generate` uses only that resolver and the +deterministic narrator, so it makes no AI provider call, writes no cost-ledger +entry and never constructs a tenant AI service. GET passes the demo-mode guard +and the CSRF check, and a SameSite=Lax session cookie rides a cross-site +top-level navigation, so AI narration of a single-resource report now requires +a CSRF-checked `POST`. No authentication, scope or session rule changed. + ### Node setup action v7 compatibility Workflow consumers pin `actions/setup-node` to diff --git a/docs/release-control/v6/internal/subsystems/storage-recovery.md b/docs/release-control/v6/internal/subsystems/storage-recovery.md index d1199c9224..075774c72d 100644 --- a/docs/release-control/v6/internal/subsystems/storage-recovery.md +++ b/docs/release-control/v6/internal/subsystems/storage-recovery.md @@ -6247,6 +6247,19 @@ is in `applyLicensedFeatureConfigSnapshot` (`pkg/server/telemetry_licensed_features.go`), pinned by `TestApplyLicensedFeatureConfigSnapshot_CountsScheduledReportingAndProfiles`. +### Single-resource report AI narration requires POST + +`internal/api/metrics_reporting_handlers.go` now accepts `POST` with a JSON +body for `/api/admin/reports/generate` and lets only that method use the +tenant's AI narrator; `GET` keeps working with query parameters and the +deterministic summary. A narrated PDF records the same AI cost-ledger entry as +before, and only a `POST` can now produce one. A `GET` reads Patrol findings +only from an AI service that is already running +(`AISettingsHandler.ExistingAIService`), so it no longer constructs a tenant AI +service, whose construction opens that tenant's SQLite resource export store. +Report content, backup and recovery enrichment, and tenant storage are +otherwise unchanged. + ### Audit-read activity is a bounded, content-free local history `audit_read_activity.json` follows the same shape as the existing external-agent diff --git a/frontend-modern/browser-verification/dad3b270f160419c.json b/frontend-modern/browser-verification/dad3b270f160419c.json new file mode 100644 index 0000000000..5cee659013 --- /dev/null +++ b/frontend-modern/browser-verification/dad3b270f160419c.json @@ -0,0 +1,42 @@ +{ + "version": 1, + "base_sha": "77d21197927a670076a74e87634e73ea999a566a", + "verified_at": "2026-10-07T19:05:00Z", + "result": "passed", + "changed_paths": [ + "frontend-modern/src/components/Settings/reportingCatalogModel.ts", + "frontend-modern/src/components/Settings/reportingPanelModel.ts" + ], + "content_sha256": { + "frontend-modern/src/components/Settings/reportingCatalogModel.ts": "fc3ab83f49ac62aaf7ef97b415a0bff1c4e445ca17771efc18304295e94e3df0", + "frontend-modern/src/components/Settings/reportingPanelModel.ts": "e4366a8c50ea897dcb4907b4e488a2c5f758e710d7a9be0d5fe0d3e047e2d9c5" + }, + "routes": [ + "/settings/support/reporting", + "/api/admin/reports/generate" + ], + "viewports": [ + { + "width": 1280, + "height": 800 + }, + { + "width": 390, + "height": 844 + } + ], + "states": [ + "Pro dev licence active, Pulse Assistant enabled on a local stub OpenAI-compatible provider that logs each call, real admin session login (no admin bypass)", + "Data & Reports panel with one mock agent selected, then two", + "Downloaded single-resource PDF opened: POST copy carries the stub AI narrative and provenance footer; GET copy carries the deterministic summary and the new GET note in place of the Configure Pulse Assistant tip", + "AI usage summary after the run: report_narrative recorded only for the two UI POST downloads" + ], + "interactions": [ + "Generate Report (1 resource) as PDF at 1280x800 and 390x844: one POST /api/admin/reports/generate with a JSON body, 200 application/pdf, download saved, stub provider called once each", + "Generate Report (1 resource) as CSV at both widths: POST, 200 text/csv, stub provider not called", + "Generate Report (2 resources) as PDF at both widths: POST /api/admin/reports/generate-multi unchanged, 200 application/pdf", + "Top-level browser navigation to GET /api/admin/reports/generate?format=pdf with the session cookie: PDF downloaded, stub provider not called", + "HEAD, PUT, DELETE and PATCH return 405 with Allow GET, POST; a session POST without the CSRF header returns 403", + "No console errors on the reporting panel at either width" + ] +} diff --git a/frontend-modern/public/docs/API.md b/frontend-modern/public/docs/API.md index 296d4cc116..50b8809acd 100644 --- a/frontend-modern/public/docs/API.md +++ b/frontend-modern/public/docs/API.md @@ -836,9 +836,12 @@ delivery for all resources. - `GET /api/admin/reports/catalog` (admin, `settings:read`) - Returns the canonical reporting catalog for the settings surface, including locked-state teaser copy, enabled-surface guidance copy, performance-report options, canonical single-report filename subject, canonical fallback filename date style, and the nested VM inventory export definition. - Metadata route: readable without the `advanced_reporting` feature so locked admin surfaces can render the same reporting definition before upsell. -- `GET /api/admin/reports/generate` (admin, `settings:read`) - - Query params: `format` (pdf/csv, default `pdf`), `resourceType`, `resourceId`, `metricType` (optional), `start`/`end` (RFC3339, optional; defaults to last 24h), `title` (optional) +- `POST /api/admin/reports/generate` (admin, `settings:read`) + - Body fields: `format` (pdf/csv, default `pdf`), `resourceType`, `resourceId`, `metricType` (optional), `start`/`end` (RFC3339, optional; defaults to last 24h), `title` (optional) - If `title` is omitted, the backend applies the canonical default title for that resource report. + - When Pulse Assistant is configured, the configured AI provider narrates a PDF's executive summary; each narration counts against the AI budget and appears in AI usage. If the call fails, times out or the budget is spent, the PDF falls back to the deterministic summary. CSV output is never narrated. The settings UI uses this method. +- `GET /api/admin/reports/generate` (admin, `settings:read`) + - Takes the same fields as query params and returns the same report, except that it never calls an AI provider: a PDF carries the deterministic summary and a note saying AI narration needs `POST`. Other methods return `405` with `Allow: GET, POST`. - `POST /api/admin/reports/generate-multi` (admin, `settings:read`) - Body fields: `resources` (1-50 entries of `{resourceType,resourceId}`), `format`, `metricType` (optional), `start`/`end` (RFC3339, optional; defaults to last 24h), `title` (optional) - If `title` is omitted, the backend applies the canonical default fleet report title. @@ -851,8 +854,8 @@ Validation and limits: - Malformed `start`/`end` values, `end` not strictly after `start`, or report windows over 366 days return `400 invalid_time_range`. - `metricType` must match `[a-zA-Z0-9._:-]+` and be <= 64 chars, otherwise `400 invalid_metric_type`. - `title` must be <= 256 chars, otherwise `400 invalid_title`. -- Multi-report body max size is 1MB; oversized payloads return `400 body_too_large`. -- Multi-report bodies reject trailing payload and unknown JSON fields with `400 invalid_body`. +- Report body max size is 1MB; oversized payloads return `400 body_too_large`. +- Report bodies reject trailing payload and unknown JSON fields with `400 invalid_body`. - VM inventory export only accepts `csv`. Common reporting error codes: diff --git a/frontend-modern/public/docs/MSP.md b/frontend-modern/public/docs/MSP.md index 7561f520c6..9302b66a1d 100644 --- a/frontend-modern/public/docs/MSP.md +++ b/frontend-modern/public/docs/MSP.md @@ -260,10 +260,12 @@ Each client runtime (or organization) generates its own reports, scoped to that client's resources: - **UI**: Settings → Data & Reports. -- **API**: `GET /api/admin/reports/generate` (single resource) and +- **API**: `POST /api/admin/reports/generate` (single resource) and `POST /api/admin/reports/generate-multi` (up to 50 resources per report), returning PDF or CSV. In shared-process mode, scope with `X-Pulse-Org-ID` - or an org-bound token. + or an org-bound token. `GET /api/admin/reports/generate` with query + parameters still returns a single-resource report, but never adds the + Pulse Assistant narrative. - **Schedules**: `GET`/`POST /api/admin/reports/schedules`, `PUT`/`DELETE /api/admin/reports/schedules/{id}`, and `POST /api/admin/reports/schedules/{id}/run`. Schedules can target explicit diff --git a/frontend-modern/src/components/Settings/__tests__/ReportingPanel.test.tsx b/frontend-modern/src/components/Settings/__tests__/ReportingPanel.test.tsx index cf86494145..f947e2968c 100644 --- a/frontend-modern/src/components/Settings/__tests__/ReportingPanel.test.tsx +++ b/frontend-modern/src/components/Settings/__tests__/ReportingPanel.test.tsx @@ -35,6 +35,7 @@ const baseCatalog = { title: 'Performance Reports', description: 'Historical performance reporting', singleResourceEndpoint: '/api/admin/reports/generate', + singleResourceMethod: 'POST', multiResourceEndpoint: '/api/admin/reports/generate-multi', singleFilenamePrefix: 'report', singleFilenameSubject: 'resource_id', diff --git a/frontend-modern/src/components/Settings/__tests__/reportingCatalogModel.branchcov2.test.ts b/frontend-modern/src/components/Settings/__tests__/reportingCatalogModel.branchcov2.test.ts index b451dc65fd..1b38e0882f 100644 --- a/frontend-modern/src/components/Settings/__tests__/reportingCatalogModel.branchcov2.test.ts +++ b/frontend-modern/src/components/Settings/__tests__/reportingCatalogModel.branchcov2.test.ts @@ -42,6 +42,7 @@ const basePerformanceReport = () => ({ title: 'Performance Reports', description: 'Historical performance reporting', singleResourceEndpoint: '/api/admin/reports/generate', + singleResourceMethod: 'POST', multiResourceEndpoint: '/api/admin/reports/generate-multi', singleFilenamePrefix: 'report', singleFilenameSubject: 'resource_id', @@ -453,6 +454,19 @@ describe('parseReportingPerformanceReportDefinition (via catalog.performanceRepo }); }); + it('throws when singleResourceMethod is neither GET nor POST', () => { + expectInvalid({ + ...baseCatalog(), + performanceReport: { ...basePerformanceReport(), singleResourceMethod: 'PUT' }, + }); + }); + + it('reads a missing singleResourceMethod as GET, the only method older backends accept', () => { + const { singleResourceMethod: _omitted, ...withoutMethod } = basePerformanceReport(); + const parsed = parseReportingCatalog({ ...baseCatalog(), performanceReport: withoutMethod }); + expect(parsed.performanceReport.singleResourceMethod).toBe('GET'); + }); + it('throws when multiResourceEndpoint is not a string', () => { expectInvalid({ ...baseCatalog(), diff --git a/frontend-modern/src/components/Settings/__tests__/reportingCatalogModel.test.ts b/frontend-modern/src/components/Settings/__tests__/reportingCatalogModel.test.ts index 97add3f9ae..2b1d289a3c 100644 --- a/frontend-modern/src/components/Settings/__tests__/reportingCatalogModel.test.ts +++ b/frontend-modern/src/components/Settings/__tests__/reportingCatalogModel.test.ts @@ -22,6 +22,7 @@ const baseCatalogPayload = { title: 'Performance Reports', description: 'Historical performance reporting', singleResourceEndpoint: '/api/admin/reports/generate', + singleResourceMethod: 'POST', multiResourceEndpoint: '/api/admin/reports/generate-multi', singleFilenamePrefix: 'report', singleFilenameSubject: 'resource_id', @@ -74,6 +75,7 @@ describe('reporting catalog model', () => { const catalog = buildLegacyReportingCatalogFallback(); expect(catalog.performanceReport.singleResourceEndpoint).toBe('/api/reporting'); + expect(catalog.performanceReport.singleResourceMethod).toBe('GET'); expect(catalog.performanceReport.multiResourceEndpoint).toBe('/api/reporting/generate-multi'); expect(catalog.vmInventoryExport).toBeNull(); }); diff --git a/frontend-modern/src/components/Settings/__tests__/reportingPanelModel.test.ts b/frontend-modern/src/components/Settings/__tests__/reportingPanelModel.test.ts index a058281e36..708b9fe8cb 100644 --- a/frontend-modern/src/components/Settings/__tests__/reportingPanelModel.test.ts +++ b/frontend-modern/src/components/Settings/__tests__/reportingPanelModel.test.ts @@ -14,6 +14,7 @@ const performanceDefinition: ReportingPerformanceReportDefinition = { title: 'Performance Reports', description: 'Historical performance reporting', singleResourceEndpoint: '/api/admin/reports/generate', + singleResourceMethod: 'POST', multiResourceEndpoint: '/api/admin/reports/generate-multi', singleFilenamePrefix: 'report', singleFilenameSubject: 'resource_id', @@ -75,12 +76,52 @@ describe('reporting panel model', () => { ); expect(request.filename).toBe('report-agent-1-20260320.pdf'); + // POST, never GET: only POST may narrate the PDF with Pulse Assistant. + expect(request.request.url).toBe('/api/admin/reports/generate'); + expect(request.request.init).toMatchObject({ + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + }); + expect(JSON.parse(String(request.request.init?.body))).toEqual({ + resourceType: 'agent', + resourceId: 'agent-1', + format: 'pdf', + start: '2026-03-19T12:34:56.000Z', + end: now.toISOString(), + metricType: 'cpu', + }); + }); + + it('keeps the query-string GET transport for backends whose catalog names GET', () => { + const now = new Date('2026-03-20T12:34:56.000Z'); + const request = buildReportingRequest( + { + end: now.toISOString(), + format: 'csv', + metricType: 'cpu', + now, + resources: [{ id: 'vm-1', type: 'vm', name: 'vm-a' }], + start: '2026-03-19T12:34:56.000Z', + title: '', + }, + { + ...performanceDefinition, + singleResourceEndpoint: '/api/reporting', + singleResourceMethod: 'GET', + }, + ); + expect(request.request.init).toBeUndefined(); - expect(request.request.url).toContain('/api/admin/reports/generate?'); - expect(request.request.url).toContain('resourceType=agent'); - expect(request.request.url).toContain('resourceId=agent-1'); - expect(request.request.url).toContain('metricType=cpu'); - expect(request.request.url).not.toContain('title='); + const url = new URL(request.request.url, 'http://pulse.local'); + expect(url.pathname).toBe('/api/reporting'); + expect(Object.fromEntries(url.searchParams)).toEqual({ + resourceType: 'vm', + resourceId: 'vm-1', + format: 'csv', + start: '2026-03-19T12:34:56.000Z', + end: now.toISOString(), + metricType: 'cpu', + }); }); it('builds a fleet reporting request body and filename', () => { @@ -158,9 +199,9 @@ describe('reporting panel model', () => { }, ); - expect(request.request.url).not.toContain('title='); - expect(request.request.url).not.toContain('metricType='); - expect(request.request.url).not.toContain('Custom+fleet+title'); + const body = JSON.parse(String(request.request.init?.body)); + expect(body).not.toHaveProperty('title'); + expect(body).not.toHaveProperty('metricType'); }); it('passes through explicit custom titles without inventing defaults', () => { @@ -186,7 +227,7 @@ describe('reporting panel model', () => { performanceDefinition, ); - expect(request.request.url).toContain('title=Custom+report+title'); + expect(JSON.parse(String(request.request.init?.body)).title).toBe('Custom report title'); }); it('derives canonical range starts from the selected preset', () => { diff --git a/frontend-modern/src/components/Settings/__tests__/useReportingPanelState.test.ts b/frontend-modern/src/components/Settings/__tests__/useReportingPanelState.test.ts index aaff7556e7..5a3caa8bfc 100644 --- a/frontend-modern/src/components/Settings/__tests__/useReportingPanelState.test.ts +++ b/frontend-modern/src/components/Settings/__tests__/useReportingPanelState.test.ts @@ -28,6 +28,7 @@ const catalogPayload = { title: 'Performance Reports', description: 'Historical performance reporting', singleResourceEndpoint: '/api/admin/reports/generate', + singleResourceMethod: 'POST', multiResourceEndpoint: '/api/admin/reports/generate-multi', singleFilenamePrefix: 'report', singleFilenameSubject: 'resource_id', @@ -478,6 +479,48 @@ describe('useReportingPanelState', () => { dispose(); }); + it('posts a single-resource report as a JSON body through apiFetch', async () => { + apiFetchMock.mockImplementation((url: string) => { + if (url === '/api/admin/reports/generate') { + return Promise.resolve(new Response('%PDF', { status: 200 })); + } + if (url === '/api/admin/reports/schedules') { + return Promise.resolve(jsonResponse(schedulesPayload)); + } + return Promise.resolve(jsonResponse(catalogPayload)); + }); + const { createObjectURL, revokeObjectURL } = URL; + URL.createObjectURL = vi.fn(() => 'blob:report'); + URL.revokeObjectURL = vi.fn(); + try { + const { hookState, dispose } = mountHook(); + await flushAsync(); + + hookState.setSelectedResources([{ id: 'vm-1', type: 'vm', name: 'vm-a' }]); + await hookState.handleGenerate(); + + const generate = apiFetchMock.mock.calls.filter(([url]) => + String(url).startsWith('/api/admin/reports/generate'), + ); + expect(generate).toHaveLength(1); + expect(generate[0][0]).toBe('/api/admin/reports/generate'); + expect(generate[0][1]).toMatchObject({ + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + }); + expect(JSON.parse(String(generate[0][1].body))).toMatchObject({ + resourceType: 'vm', + resourceId: 'vm-1', + format: catalogPayload.performanceReport.defaultFormat, + }); + expect(showWarningMock).not.toHaveBeenCalled(); + dispose(); + } finally { + URL.createObjectURL = createObjectURL; + URL.revokeObjectURL = revokeObjectURL; + } + }); + it('saves a Patrol weekly summary without a resource scope', async () => { const savedSchedule = { id: 'digest-1', diff --git a/frontend-modern/src/components/Settings/reportingCatalogModel.ts b/frontend-modern/src/components/Settings/reportingCatalogModel.ts index 52749a55de..33b73fafa6 100644 --- a/frontend-modern/src/components/Settings/reportingCatalogModel.ts +++ b/frontend-modern/src/components/Settings/reportingCatalogModel.ts @@ -6,6 +6,7 @@ import { export type ReportingFormat = 'pdf' | 'csv'; export type ReportingFilenameDateStyle = 'utc_yyyymmdd'; export type ReportingFilenameSubject = 'resource_id'; +export type ReportingSingleResourceMethod = 'GET' | 'POST'; export interface ReportingFormatDefinition { value: ReportingFormat; @@ -24,6 +25,7 @@ export interface ReportingPerformanceReportDefinition { title: string; description: string; singleResourceEndpoint: string; + singleResourceMethod: ReportingSingleResourceMethod; multiResourceEndpoint: string; singleFilenamePrefix: string; singleFilenameSubject: ReportingFilenameSubject; @@ -85,6 +87,7 @@ export function buildLegacyReportingCatalogFallback(): ReportingCatalog { description: 'Generate PDF summaries or CSV metric exports from historical monitoring data for one or more selected resources.', singleResourceEndpoint: '/api/reporting', + singleResourceMethod: 'GET', multiResourceEndpoint: '/api/reporting/generate-multi', singleFilenamePrefix: 'report', singleFilenameSubject: 'resource_id', @@ -176,6 +179,9 @@ function parseReportingPerformanceReportDefinition( typeof candidate.title !== 'string' || typeof candidate.description !== 'string' || typeof candidate.singleResourceEndpoint !== 'string' || + (candidate.singleResourceMethod !== undefined && + candidate.singleResourceMethod !== 'GET' && + candidate.singleResourceMethod !== 'POST') || typeof candidate.multiResourceEndpoint !== 'string' || typeof candidate.singleFilenamePrefix !== 'string' || candidate.singleFilenameSubject !== 'resource_id' || @@ -211,6 +217,8 @@ function parseReportingPerformanceReportDefinition( title: candidate.title, description: candidate.description, singleResourceEndpoint: candidate.singleResourceEndpoint, + // Backends that predate the field accept only GET. + singleResourceMethod: candidate.singleResourceMethod ?? 'GET', multiResourceEndpoint: candidate.multiResourceEndpoint, singleFilenamePrefix: candidate.singleFilenamePrefix, singleFilenameSubject: candidate.singleFilenameSubject, diff --git a/frontend-modern/src/components/Settings/reportingPanelModel.ts b/frontend-modern/src/components/Settings/reportingPanelModel.ts index fbaa61d26a..45a6671e68 100644 --- a/frontend-modern/src/components/Settings/reportingPanelModel.ts +++ b/frontend-modern/src/components/Settings/reportingPanelModel.ts @@ -96,6 +96,7 @@ export function buildReportingRequest( | 'singleFilenamePrefix' | 'singleFilenameSubject' | 'singleResourceEndpoint' + | 'singleResourceMethod' | 'supportsCustomTitle' | 'supportsMetricFilter' >, @@ -105,23 +106,41 @@ export function buildReportingRequest( if (context.resources.length === 1) { const resource = context.resources[0]; - const params = new URLSearchParams({ + const fields = { resourceType: toReportingResourceType(resource.type), resourceId: resource.id, format: context.format, start: context.start, end: context.end, - }); + title: customTitle || undefined, + metricType: metricType || undefined, + }; + const filename = buildReportingFilename(context.format, resource.id, context.now, definition); - if (metricType) { - params.append('metricType', metricType); - } - if (customTitle) { - params.append('title', customTitle); + // Only POST may narrate a PDF with Pulse Assistant; the catalog names + // GET only for backends that accept nothing else. + if (definition.singleResourceMethod === 'POST') { + return { + filename, + request: { + url: definition.singleResourceEndpoint, + init: { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify(fields), + }, + }, + }; } + const params = new URLSearchParams(); + for (const [key, value] of Object.entries(fields)) { + if (value) { + params.append(key, value); + } + } return { - filename: buildReportingFilename(context.format, resource.id, context.now, definition), + filename, request: { url: `${definition.singleResourceEndpoint}?${params.toString()}`, }, diff --git a/frontend-modern/src/utils/__tests__/docsLinks.test.ts b/frontend-modern/src/utils/__tests__/docsLinks.test.ts index 0ccae13f89..c37c693e09 100644 --- a/frontend-modern/src/utils/__tests__/docsLinks.test.ts +++ b/frontend-modern/src/utils/__tests__/docsLinks.test.ts @@ -566,6 +566,27 @@ describe('docsLinks', () => { expect(apiReference).not.toContain('Mobile Remote Access'); }); + it('documents POST as the only report transport that uses Pulse Assistant', () => { + const handler = readFileSync( + path.join(repoRoot, 'internal', 'api', 'metrics_reporting_handlers.go'), + 'utf8', + ); + expect(handler).toContain( + 'if r.Method == http.MethodPost {\n\t\tnarrator, _, findings = h.resolveNarrator(r.Context())', + ); + expect(handler).toContain('req.Narrator = getReportNarrator{}'); + for (const guide of ['API.md', 'MSP.md']) { + const rootDoc = readFileSync(path.join(repoRoot, 'docs', guide), 'utf8'); + expect(readFileSync(path.join(frontendRoot, 'public', 'docs', guide), 'utf8')).toBe(rootDoc); + expect(rootDoc).toContain('`POST /api/admin/reports/generate` ('); + } + const apiReference = readFileSync(path.join(repoRoot, 'docs', 'API.md'), 'utf8'); + expect(apiReference).toContain('it never calls an AI provider'); + expect(apiReference).toContain('`405` with `Allow: GET, POST`'); + const msp = readFileSync(path.join(repoRoot, 'docs', 'MSP.md'), 'utf8'); + expect(msp).not.toContain('`GET /api/admin/reports/generate` (single resource)'); + }); + it('ships credential-free Proxmox setup instructions and a separate token prompt', () => { const apiReference = readFileSync(path.join(repoRoot, 'docs', 'API.md'), 'utf8'); const shipped = readFileSync(path.join(frontendRoot, 'public', 'docs', 'API.md'), 'utf8'); diff --git a/internal/api/ai_handlers.go b/internal/api/ai_handlers.go index 62ee996eff..0986b24b6d 100644 --- a/internal/api/ai_handlers.go +++ b/internal/api/ai_handlers.go @@ -385,6 +385,25 @@ func (h *AISettingsHandler) loadAIConfig(ctx context.Context) (*config.AIConfig, } // GetAIService returns the underlying AI service +// ExistingAIService returns the org's AI service only when one already exists +// and never constructs one. GetAIService builds and configures a tenant service +// on first use, which can list provider models and start background discovery, +// so read paths that must not reach an AI provider use this instead. +func (h *AISettingsHandler) ExistingAIService(ctx context.Context) *ai.Service { + if h == nil { + return nil + } + mtPersistence, mtMonitor, _, _, _, _ := h.stateRefs() + defaultAIService := h.providerSnapshot().defaultAIService + orgID := GetOrgID(ctx) + if orgID == "default" || orgID == "" || (mtPersistence == nil && mtMonitor == nil) { + return defaultAIService + } + h.aiServicesMu.RLock() + defer h.aiServicesMu.RUnlock() + return h.aiServices[orgID] +} + func (h *AISettingsHandler) GetAIService(ctx context.Context) *ai.Service { mtPersistence, mtMonitor, _, _, _, _ := h.stateRefs() providers := h.providerSnapshot() diff --git a/internal/api/ai_handlers_more_test.go b/internal/api/ai_handlers_more_test.go index 52339f4893..8b38b132ff 100644 --- a/internal/api/ai_handlers_more_test.go +++ b/internal/api/ai_handlers_more_test.go @@ -303,6 +303,36 @@ func TestAISettingsHandler_GetAIService_MultiTenantProviders(t *testing.T) { } } +// Report GETs read Patrol findings through ExistingAIService, so it must never +// construct a tenant service: construction can list provider models and start +// background discovery. +func TestAISettingsHandler_ExistingAIServiceNeverConstructs(t *testing.T) { + handler := NewAISettingsHandler(config.NewMultiTenantPersistence(t.TempDir()), nil, nil) + t.Cleanup(handler.StopServices) + tenant := context.WithValue(context.Background(), OrgIDContextKey, "tenant-1") + + if svc := handler.ExistingAIService(tenant); svc != nil { + t.Fatalf("expected no service before the tenant's first GetAIService, got %p", svc) + } + handler.aiServicesMu.RLock() + built := len(handler.aiServices) + handler.aiServicesMu.RUnlock() + if built != 0 { + t.Fatalf("ExistingAIService constructed %d tenant services, want 0", built) + } + + created := handler.GetAIService(tenant) + if created == nil { + t.Fatal("expected GetAIService to construct the tenant service") + } + if got := handler.ExistingAIService(tenant); got != created { + t.Fatalf("expected the running tenant service %p, got %p", created, got) + } + if got, want := handler.ExistingAIService(context.Background()), handler.GetAIService(context.Background()); got != want { + t.Fatalf("expected the default service %p, got %p", want, got) + } +} + func TestAISettingsHandler_GetAIService_UsesTenantReadState(t *testing.T) { tmp := t.TempDir() mtp := config.NewMultiTenantPersistence(tmp) diff --git a/internal/api/contract_test.go b/internal/api/contract_test.go index 953ed0fba2..d0bebe25b3 100644 --- a/internal/api/contract_test.go +++ b/internal/api/contract_test.go @@ -5081,6 +5081,7 @@ func TestContract_ReportingCatalogJSONSnapshot(t *testing.T) { "title":"Performance Reports", "description":"Generate PDF summaries or CSV metric exports from historical monitoring data for one or more selected resources.", "singleResourceEndpoint":"/api/admin/reports/generate", + "singleResourceMethod":"POST", "multiResourceEndpoint":"/api/admin/reports/generate-multi", "singleFilenamePrefix":"report", "singleFilenameSubject":"resource_id", @@ -5227,6 +5228,169 @@ func TestContract_PerformanceReportTransportUsesCatalogDefinition(t *testing.T) if engine.lastReq.MetricType != "cpu" || engine.lastReq.Title != "Node report" { t.Fatalf("expected trimmed canonical optional fields, got %+v", engine.lastReq) } + + post := httptest.NewRequest( + http.MethodPost, + "/api/reporting", + strings.NewReader(`{"resourceType":"node","resourceId":"node-1","metricType":" cpu ","title":" Node report "}`), + ) + rec = httptest.NewRecorder() + handler.HandleGenerateReport(rec, post) + if rec.Code != http.StatusOK { + t.Fatalf("expected POST 200, got %d body=%s", rec.Code, rec.Body.String()) + } + if engine.lastReq.ResourceID != "node-1" || engine.lastReq.Format != definition.DefaultFormat || + engine.lastReq.MetricType != "cpu" || engine.lastReq.Title != "Node report" { + t.Fatalf("expected POST body to carry the same canonical fields as GET, got %+v", engine.lastReq) + } +} + +type recordingReportEngine struct { + *reporting.ReportEngine + requests []reporting.MetricReportRequest +} + +func (e *recordingReportEngine) Generate(req reporting.MetricReportRequest) ([]byte, string, error) { + e.requests = append(e.requests, req) + return e.ReportEngine.Generate(req) +} + +type countingReportNarrator struct{ calls atomic.Int32 } + +func (n *countingReportNarrator) Narrate(context.Context, reporting.NarrativeInput) (reporting.Narrative, error) { + n.calls.Add(1) + return reporting.Narrative{Source: reporting.NarrativeSourceAI, HealthStatus: "HEALTHY", HealthMessage: "narrated"}, nil +} + +// A PDF single-resource report may be narrated by the tenant's Pulse +// Assistant, a paid provider call recorded in the cost ledger. GET passes the +// demo-mode guard and the CSRF check, and SameSite=Lax session cookies ride +// cross-site top-level GET navigations, so only POST, the settings UI +// transport, may reach the AI narrator. GET still returns the report, with +// the deterministic summary and a note in place of the "Configure Pulse +// Assistant" tip. +func TestContract_SingleReportAINarrationRequiresPOST(t *testing.T) { + store, err := metrics.NewStore(metrics.DefaultConfig(t.TempDir())) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = store.Close() }) + engine := &recordingReportEngine{ReportEngine: reporting.NewReportEngine(reporting.EngineConfig{MetricsStore: store})} + original := reporting.GetEngine() + reporting.SetEngine(engine) + t.Cleanup(func() { reporting.SetEngine(original) }) + + // The narrator resolver stands for GetAIService, which can construct a + // tenant AI service; GET must use only the existing-service findings path. + narrator := &countingReportNarrator{} + var narratorResolves, findingsResolves atomic.Int32 + handler := NewReportingHandlers(nil, nil) + handler.SetNarratorResolver(func(context.Context) (reporting.Narrator, reporting.FleetNarrator, reporting.FindingsProvider) { + narratorResolves.Add(1) + return narrator, nil, nil + }) + handler.SetExistingFindingsResolver(func(context.Context) reporting.FindingsProvider { + findingsResolves.Add(1) + return nil + }) + generate := func(req *http.Request) *httptest.ResponseRecorder { + rec := httptest.NewRecorder() + handler.HandleGenerateReport(rec, req) + return rec + } + const query = "/api/admin/reports/generate?format=pdf&resourceType=node&resourceId=node-1" + + rec := generate(httptest.NewRequest(http.MethodGet, query, nil)) + if rec.Code != http.StatusOK || rec.Header().Get("Content-Type") != "application/pdf" { + t.Fatalf("GET report: status %d, content type %q, body=%s", rec.Code, rec.Header().Get("Content-Type"), rec.Body.String()) + } + if got := narrator.calls.Load(); got != 0 { + t.Fatalf("GET report called the AI narrator %d times, want 0", got) + } + narrative, err := engine.NarrativeFor(engine.requests[len(engine.requests)-1]) + if err != nil { + t.Fatalf("GET report narrative: %v", err) + } + if narrative.Source != reporting.NarrativeSourceHeuristic || narrative.Disclaimer != reportGETNarrativeNote { + t.Fatalf("GET report narrative = source %q disclaimer %q, want the deterministic summary with the GET note", narrative.Source, narrative.Disclaimer) + } + + for _, method := range []string{http.MethodHead, http.MethodPut, http.MethodDelete} { + rec := generate(httptest.NewRequest(method, query, nil)) + if rec.Code != http.StatusMethodNotAllowed || rec.Header().Get("Allow") != "GET, POST" { + t.Fatalf("%s report: status %d, Allow %q, want 405 with Allow GET, POST", method, rec.Code, rec.Header().Get("Allow")) + } + } + if got := narrator.calls.Load(); got != 0 { + t.Fatalf("GET, HEAD, PUT and DELETE called the AI narrator %d times, want 0", got) + } + if got := narratorResolves.Load(); got != 0 { + t.Fatalf("GET, HEAD, PUT and DELETE resolved the AI service %d times, want 0", got) + } + if got := findingsResolves.Load(); got != 1 { + t.Fatalf("GET looked up existing Patrol findings %d times, want 1", got) + } + + rec = generate(httptest.NewRequest( + http.MethodPost, + "/api/admin/reports/generate", + strings.NewReader(`{"format":"pdf","resourceType":"node","resourceId":"node-1"}`), + )) + if rec.Code != http.StatusOK || rec.Header().Get("Content-Type") != "application/pdf" { + t.Fatalf("POST report: status %d, content type %q, body=%s", rec.Code, rec.Header().Get("Content-Type"), rec.Body.String()) + } + if got := narrator.calls.Load(); got != 1 { + t.Fatalf("POST report called the AI narrator %d times, want 1", got) + } + if got := narratorResolves.Load(); got != 1 { + t.Fatalf("POST report resolved the AI service %d times, want 1", got) + } +} + +// The router's real resolvers: a GET report for a tenant whose AI service is +// not running must not construct one, because construction can list provider +// models and start background discovery. POST resolves it as before. +func TestContract_SingleReportGETDoesNotConstructTenantAIService(t *testing.T) { + engine := &stubReportingEngine{data: []byte("report"), contentType: "application/pdf"} + original := reporting.GetEngine() + reporting.SetEngine(engine) + t.Cleanup(func() { reporting.SetEngine(original) }) + + router := &Router{ + aiSettingsHandler: NewAISettingsHandler(config.NewMultiTenantPersistence(t.TempDir()), nil, nil), + reportingHandlers: NewReportingHandlers(nil, nil), + } + router.wireReportingAIResolvers() + t.Cleanup(router.aiSettingsHandler.StopServices) + tenantServices := func() int { + router.aiSettingsHandler.aiServicesMu.RLock() + defer router.aiSettingsHandler.aiServicesMu.RUnlock() + return len(router.aiSettingsHandler.aiServices) + } + generate := func(req *http.Request) *httptest.ResponseRecorder { + rec := httptest.NewRecorder() + router.reportingHandlers.HandleGenerateReport(rec, req.WithContext(context.WithValue(req.Context(), OrgIDContextKey, "tenant-1"))) + return rec + } + + rec := generate(httptest.NewRequest(http.MethodGet, "/api/admin/reports/generate?format=pdf&resourceType=node&resourceId=node-1", nil)) + if rec.Code != http.StatusOK { + t.Fatalf("GET report: status %d, body=%s", rec.Code, rec.Body.String()) + } + if got := tenantServices(); got != 0 { + t.Fatalf("GET report constructed %d tenant AI services, want 0", got) + } + if _, ok := engine.lastReq.Narrator.(getReportNarrator); !ok || engine.lastReq.FindingsProvider != nil { + t.Fatalf("GET report narrator %T, findings %v; want the deterministic narrator and no findings without a running service", engine.lastReq.Narrator, engine.lastReq.FindingsProvider) + } + + rec = generate(httptest.NewRequest(http.MethodPost, "/api/admin/reports/generate", strings.NewReader(`{"format":"pdf","resourceType":"node","resourceId":"node-1"}`))) + if rec.Code != http.StatusOK { + t.Fatalf("POST report: status %d, body=%s", rec.Code, rec.Body.String()) + } + if got := tenantServices(); got != 1 { + t.Fatalf("POST report resolved %d tenant AI services, want 1", got) + } } // Normalization keeps a disk's last-known temperature when the current diff --git a/internal/api/metrics_reporting_handlers.go b/internal/api/metrics_reporting_handlers.go index 08e4c7b240..f918817170 100644 --- a/internal/api/metrics_reporting_handlers.go +++ b/internal/api/metrics_reporting_handlers.go @@ -7,6 +7,7 @@ import ( "fmt" "io" "net/http" + "net/url" "os" "regexp" "strings" @@ -99,6 +100,7 @@ type ReportingHandlers struct { mtMonitor *monitoring.MultiTenantMonitor recoveryManager *recoverymanager.Manager narratorResolver func(ctx context.Context) (reporting.Narrator, reporting.FleetNarrator, reporting.FindingsProvider) + existingFindingsResolver func(ctx context.Context) reporting.FindingsProvider settingsStore reportingSystemSettingsStore scheduleRunMu sync.Mutex commercialLicenseResolver func(ctx context.Context) *licenseService @@ -168,6 +170,23 @@ func (h *ReportingHandlers) resolveNarrator(ctx context.Context) (reporting.Narr return h.narratorResolver(ctx) } +// SetExistingFindingsResolver wires the Patrol findings lookup that GET reports +// use. Unlike the narrator resolver it must return only an AI service that is +// already running, never construct one. +func (h *ReportingHandlers) SetExistingFindingsResolver(resolver func(ctx context.Context) reporting.FindingsProvider) { + if h == nil { + return + } + h.existingFindingsResolver = resolver +} + +func (h *ReportingHandlers) resolveExistingFindings(ctx context.Context) reporting.FindingsProvider { + if h == nil || h.existingFindingsResolver == nil { + return nil + } + return h.existingFindingsResolver(ctx) +} + func (h *ReportingHandlers) resolveReportBranding(ctx context.Context) reporting.ReportBranding { service := getLicenseServiceForContext(ctx) entitled := service != nil && service.HasFeature(featureWhiteLabelValue) @@ -300,28 +319,33 @@ func normalizePerformanceReportTimeRange( return start, end, nil } -func decodeMultiReportRequestBody(w http.ResponseWriter, r *http.Request) (multiReportRequestBody, bool) { +// decodeReportingRequestBody strictly decodes one JSON report request body: +// at most 1MB, no unknown fields, no trailing payload. +func decodeReportingRequestBody[T any](w http.ResponseWriter, r *http.Request) (T, bool) { r.Body = http.MaxBytesReader(w, r.Body, reportingMultiReportBodyMax) - var body multiReportRequestBody + var body, zero T decoder := json.NewDecoder(r.Body) decoder.DisallowUnknownFields() - if err := decoder.Decode(&body); err != nil { + err := decoder.Decode(&body) + if err == nil { + // Anything after the value is invalid, and reading past it can also + // hit the size cap. + if trailing := decoder.Decode(&struct{}{}); trailing != io.EOF { + err = errors.New("trailing payload") + if trailing != nil { + err = trailing + } + } + } + if err != nil { var maxBytesErr *http.MaxBytesError if errors.As(err, &maxBytesErr) { writeErrorResponse(w, http.StatusBadRequest, "body_too_large", "Request body must be 1MB or less", nil) - return multiReportRequestBody{}, false + return zero, false } writeErrorResponse(w, http.StatusBadRequest, "invalid_body", "Invalid request body", nil) - return multiReportRequestBody{}, false - } - if decoder.More() { - writeErrorResponse(w, http.StatusBadRequest, "invalid_body", "Invalid request body", nil) - return multiReportRequestBody{}, false - } - if err := decoder.Decode(&struct{}{}); err != io.EOF { - writeErrorResponse(w, http.StatusBadRequest, "invalid_body", "Invalid request body", nil) - return multiReportRequestBody{}, false + return zero, false } return body, true @@ -539,9 +563,67 @@ func (h *ReportingHandlers) listBackupsForReport(ctx context.Context, orgID stri return out } -// HandleGenerateReport generates a report +// singleReportRequestBody carries the single-resource report fields. POST +// sends them as a JSON body; GET sends the same names as query parameters. +type singleReportRequestBody struct { + ResourceType string `json:"resourceType"` + ResourceID string `json:"resourceId"` + Format string `json:"format"` + Start string `json:"start"` + End string `json:"end"` + Title string `json:"title"` + MetricType string `json:"metricType"` +} + +func singleReportRequestFromQuery(q url.Values) singleReportRequestBody { + return singleReportRequestBody{ + ResourceType: q.Get("resourceType"), + ResourceID: q.Get("resourceId"), + Format: q.Get("format"), + Start: q.Get("start"), + End: q.Get("end"), + Title: q.Get("title"), + MetricType: q.Get("metricType"), + } +} + +// reportGETNarrativeNote takes the place of the PDF's "Configure Pulse +// Assistant" tip on a GET report, which stays deterministic whatever the +// Assistant settings. +const reportGETNarrativeNote = "Tip: This report was requested with GET, which never uses Pulse Assistant. Generate it from Settings, or request it with POST, to include AI narration when Pulse Assistant is configured." + +// getReportNarrator narrates GET single-resource reports with the +// deterministic summary and never calls an AI provider. +type getReportNarrator struct{} + +func (getReportNarrator) Narrate(ctx context.Context, in reporting.NarrativeInput) (reporting.Narrative, error) { + out, err := reporting.HeuristicNarrator{}.Narrate(ctx, in) + out.Source = reporting.NarrativeSourceHeuristic + out.Disclaimer = reportGETNarrativeNote + return out, err +} + +// HandleGenerateReport generates a single-resource report. POST with a JSON +// body is the canonical transport, used by the settings UI, and the only one +// that may narrate a PDF with the tenant's Pulse Assistant, which spends AI +// budget and records usage in the cost ledger. GET takes the same fields as +// query parameters and always narrates deterministically: GET passes the +// demo-mode guard and the CSRF check, and SameSite=Lax session cookies ride +// cross-site top-level GET navigations, so a link must not be able to spend +// that budget. func (h *ReportingHandlers) HandleGenerateReport(w http.ResponseWriter, r *http.Request) { - if r.Method != http.MethodGet { + var params singleReportRequestBody + switch r.Method { + case http.MethodGet: + params = singleReportRequestFromQuery(r.URL.Query()) + case http.MethodPost: + body, ok := decodeReportingRequestBody[singleReportRequestBody](w, r) + if !ok { + return + } + params = body + default: + w.Header().Set("Allow", "GET, POST") http.Error(w, "Method not allowed", http.StatusMethodNotAllowed) return } @@ -553,15 +635,14 @@ func (h *ReportingHandlers) HandleGenerateReport(w http.ResponseWriter, r *http. } definition := performanceReportDefinition() - q := r.URL.Query() - format, err := normalizePerformanceReportFormat(q.Get("format"), definition) + format, err := normalizePerformanceReportFormat(params.Format, definition) if err != nil { writeErrorResponse(w, http.StatusBadRequest, "invalid_format", err.Error(), nil) return } - resourceTypeRaw := q.Get("resourceType") - resourceID := q.Get("resourceId") + resourceTypeRaw := params.ResourceType + resourceID := params.ResourceID if resourceTypeRaw == "" || resourceID == "" { writeErrorResponse(w, http.StatusBadRequest, "missing_params", "resourceType and resourceId are required", nil) return @@ -584,8 +665,8 @@ func (h *ReportingHandlers) HandleGenerateReport(w http.ResponseWriter, r *http. metricType, title, err := normalizePerformanceReportOptionalFields( definition, - q.Get("metricType"), - q.Get("title"), + params.MetricType, + params.Title, ) if err != nil { code := "invalid_metric_type" @@ -599,8 +680,8 @@ func (h *ReportingHandlers) HandleGenerateReport(w http.ResponseWriter, r *http. start, end, err := normalizePerformanceReportTimeRange( definition, - q.Get("start"), - q.Get("end"), + params.Start, + params.End, time.Now(), ) if err != nil { @@ -628,9 +709,18 @@ func (h *ReportingHandlers) HandleGenerateReport(w http.ResponseWriter, r *http. // Wire the per-tenant AI narrator and Patrol findings provider when // configured. Both are nil-safe at the engine layer; absence falls // back to the heuristic narrator with no findings section. Single- - // resource reports do not use the FleetNarrator. - narrator, _, findings := h.resolveNarrator(r.Context()) - req.Narrator = narrator + // resource reports do not use the FleetNarrator. Only POST resolves the + // AI service; GET narrates deterministically and reads findings only + // from an AI service that is already running. + var narrator reporting.Narrator + var findings reporting.FindingsProvider + if r.Method == http.MethodPost { + narrator, _, findings = h.resolveNarrator(r.Context()) + req.Narrator = narrator + } else { + findings = h.resolveExistingFindings(r.Context()) + req.Narrator = getReportNarrator{} + } req.FindingsProvider = findings data, contentType, err := engine.Generate(req) @@ -647,6 +737,7 @@ func (h *ReportingHandlers) HandleGenerateReport(w http.ResponseWriter, r *http. log.Info(). Str("event", "reporting.single.generated"). Str("org_id", orgID). + Str("method", r.Method). Str("resource_type", resourceType). Str("metric_type", metricType). Str("format", string(format)). @@ -1187,7 +1278,7 @@ func (h *ReportingHandlers) HandleGenerateMultiReport(w http.ResponseWriter, r * http.Error(w, "Method not allowed", http.StatusMethodNotAllowed) return } - body, ok := decodeMultiReportRequestBody(w, r) + body, ok := decodeReportingRequestBody[multiReportRequestBody](w, r) if !ok { return } diff --git a/internal/api/reporting_handlers_test.go b/internal/api/reporting_handlers_test.go index 00851e4a56..f5adb84e9d 100644 --- a/internal/api/reporting_handlers_test.go +++ b/internal/api/reporting_handlers_test.go @@ -75,7 +75,7 @@ func (p reportBrandLicenseProvider) Service(context.Context) *pkglicensing.Servi func TestReportingHandlers_MethodNotAllowed(t *testing.T) { handler := NewReportingHandlers(nil, nil) - req := httptest.NewRequest(http.MethodPost, "/api/reporting", nil) + req := httptest.NewRequest(http.MethodPut, "/api/reporting", nil) rr := httptest.NewRecorder() handler.HandleGenerateReport(rr, req) @@ -83,6 +83,47 @@ func TestReportingHandlers_MethodNotAllowed(t *testing.T) { if rr.Code != http.StatusMethodNotAllowed { t.Fatalf("expected status %d, got %d", http.StatusMethodNotAllowed, rr.Code) } + if got := rr.Header().Get("Allow"); got != "GET, POST" { + t.Fatalf("expected Allow GET, POST, got %q", got) + } +} + +func TestReportingHandlers_GenerateReportRejectsInvalidPOSTBody(t *testing.T) { + engine := &stubReportingEngine{data: []byte("report"), contentType: "application/pdf"} + original := reporting.GetEngine() + reporting.SetEngine(engine) + t.Cleanup(func() { reporting.SetEngine(original) }) + + handler := NewReportingHandlers(nil, nil) + for name, body := range map[string]string{ + "empty": ``, + "unknown field": `{"resourceType":"node","resourceId":"node-1","narrate":true}`, + "trailing value": `{"resourceType":"node","resourceId":"node-1"} {}`, + "query shape": `resourceType=node&resourceId=node-1`, + } { + req := httptest.NewRequest(http.MethodPost, "/api/reporting", strings.NewReader(body)) + rr := httptest.NewRecorder() + handler.HandleGenerateReport(rr, req) + if rr.Code != http.StatusBadRequest || !strings.Contains(rr.Body.String(), "invalid_body") { + t.Fatalf("%s: expected 400 invalid_body, got %d body=%s", name, rr.Code, rr.Body.String()) + } + } + + req := httptest.NewRequest(http.MethodPost, "/api/reporting?resourceType=node&resourceId=node-1", strings.NewReader(`{}`)) + rr := httptest.NewRecorder() + handler.HandleGenerateReport(rr, req) + if rr.Code != http.StatusBadRequest || !strings.Contains(rr.Body.String(), "missing_params") { + t.Fatalf("POST must read fields from the body, not the query: got %d body=%s", rr.Code, rr.Body.String()) + } + + // A valid value followed by padding past the cap is too large, not malformed. + padded := `{"resourceType":"node","resourceId":"node-1"}` + strings.Repeat(" ", reportingMultiReportBodyMax) + req = httptest.NewRequest(http.MethodPost, "/api/reporting", strings.NewReader(padded)) + rr = httptest.NewRecorder() + handler.HandleGenerateReport(rr, req) + if rr.Code != http.StatusBadRequest || !strings.Contains(rr.Body.String(), "body_too_large") { + t.Fatalf("expected 400 body_too_large for an oversized body, got %d body=%s", rr.Code, rr.Body.String()) + } } func TestReportingHandlers_EngineUnavailable(t *testing.T) { diff --git a/internal/api/router.go b/internal/api/router.go index 6f77a32878..60dcd7343d 100644 --- a/internal/api/router.go +++ b/internal/api/router.go @@ -805,33 +805,7 @@ func (r *Router) setupRoutes() { // tenant services are refreshed when Router.SetMonitor replaces a monitor. r.configureMetadataProviderFactory() - // Wire the per-tenant AI narrator, fleet narrator, and Patrol - // findings provider into reporting. The AI service implements all - // three interfaces; when not configured for the tenant the engine - // falls back to the heuristic narrators with no findings section. - if r.reportingHandlers != nil { - settings := r.aiSettingsHandler - r.reportingHandlers.SetPatrolDigestResolver(func(ctx context.Context, days int) (ai.PatrolDigest, bool) { - if settings == nil { - return ai.PatrolDigest{}, false - } - return settings.BuildPatrolDigest(ctx, days) - }) - r.reportingHandlers.SetNarratorResolver(func(ctx context.Context) (reporting.Narrator, reporting.FleetNarrator, reporting.FindingsProvider) { - if settings == nil { - return nil, nil, nil - } - svc := settings.GetAIService(ctx) - if svc == nil { - return nil, nil, nil - } - cfg := svc.GetAIConfig() - if cfg == nil || !cfg.Enabled { - return nil, nil, nil - } - return svc, svc, svc - }) - } + r.wireReportingAIResolvers() // AI chat handler r.aiHandler = NewAIHandler(r.multiTenant, r.mtMonitor, r.agentExecServer) @@ -1031,6 +1005,50 @@ func (r *Router) setupRoutes() { // See issue #334 - ServeMux redirects empty path to "./" which breaks reverse proxies } +// wireReportingAIResolvers connects reporting to the per-tenant AI service. +func (r *Router) wireReportingAIResolvers() { + // Wire the per-tenant AI narrator, fleet narrator, and Patrol + // findings provider into reporting. The AI service implements all + // three interfaces; when not configured for the tenant the engine + // falls back to the heuristic narrators with no findings section. + if r.reportingHandlers != nil { + settings := r.aiSettingsHandler + r.reportingHandlers.SetPatrolDigestResolver(func(ctx context.Context, days int) (ai.PatrolDigest, bool) { + if settings == nil { + return ai.PatrolDigest{}, false + } + return settings.BuildPatrolDigest(ctx, days) + }) + r.reportingHandlers.SetNarratorResolver(func(ctx context.Context) (reporting.Narrator, reporting.FleetNarrator, reporting.FindingsProvider) { + if settings == nil { + return nil, nil, nil + } + svc := settings.GetAIService(ctx) + if svc == nil { + return nil, nil, nil + } + cfg := svc.GetAIConfig() + if cfg == nil || !cfg.Enabled { + return nil, nil, nil + } + return svc, svc, svc + }) + r.reportingHandlers.SetExistingFindingsResolver(func(ctx context.Context) reporting.FindingsProvider { + if settings == nil { + return nil + } + svc := settings.ExistingAIService(ctx) + if svc == nil { + return nil + } + if cfg := svc.GetAIConfig(); cfg == nil || !cfg.Enabled { + return nil + } + return svc + }) + } +} + // CleanupTenant removes all per-tenant resources (RBAC, AI, License) for a deleted org. func (r *Router) CleanupTenant(ctx context.Context, orgID string) error { orgID = strings.TrimSpace(orgID) diff --git a/pkg/reporting/catalog.go b/pkg/reporting/catalog.go index 3820d821c2..fcf287cc09 100644 --- a/pkg/reporting/catalog.go +++ b/pkg/reporting/catalog.go @@ -2,6 +2,7 @@ package reporting import ( "fmt" + "net/http" "strings" "time" ) @@ -33,6 +34,7 @@ type PerformanceReportDefinition struct { Title string `json:"title"` Description string `json:"description"` SingleResourceEndpoint string `json:"singleResourceEndpoint"` + SingleResourceMethod string `json:"singleResourceMethod"` MultiResourceEndpoint string `json:"multiResourceEndpoint"` SingleFilenamePrefix string `json:"singleFilenamePrefix"` SingleFilenameSubject string `json:"singleFilenameSubject"` @@ -150,6 +152,7 @@ func DescribePerformanceReport() PerformanceReportDefinition { Title: "Performance Reports", Description: "Generate PDF summaries or CSV metric exports from historical monitoring data for one or more selected resources.", SingleResourceEndpoint: "/api/admin/reports/generate", + SingleResourceMethod: http.MethodPost, MultiResourceEndpoint: "/api/admin/reports/generate-multi", SingleFilenamePrefix: "report", SingleFilenameSubject: FilenameSubjectResourceID,