Follow-up from PR #351... correction: from PR #359 (GIS stats publication boundary). A sweep for the same pattern found a parallel exposure in the simulation analytics API, which PR #359 (scoped to GIS per owner decision) does not touch.
Finding
spp_api_v2_simulation accepts client-supplied statistic names and passes them straight to the aggregation engine with no publication allowlist:
spp_api_v2_simulation/schemas/analytics.py:31 — ComputeAggregationRequest.statistics: list[str] | None (client-supplied).spp_api_v2_simulation/routers/analytics.py:~52 — passes request.statistics to the service; the only gate is the aggregation:read scope.spp_api_v2_simulation/services/analytics_api_service.py:23-46 — compute_aggregation(...) forwards statistics to self.env["spp.analytics.service"].sudo().compute_aggregation(scope=..., statistics=statistics, context="api").
As with the GIS path (#359), the aggregation registry (spp_analytics/models/indicator_registry.py _try_statistic_model/_try_variable_model) resolves those names via sudo() against all spp.indicator and all spp.cel.variable records, with no is_published_api / context check. So a client with only aggregation:read can request unpublished indicator names, or raw CEL variable names, and receive their aggregates — bypassing the is_published_api publication boundary (spp.indicator has is_published_api, exposed via get_published_for_context("api")).
Severity: Medium
Same class and impact as #359 (k-anonymity mitigates, publication boundary bypassed for sensitive aggregates), for the API context instead of GIS.
Suggested fix
Mirror #359: in AnalyticsApiService.compute_aggregation (or the router), restrict supplied statistics to the API-published allowlist — env["spp.indicator"].get_published_for_context("api").mapped("name") — dropping unpublished/raw names before calling the aggregation service (drop, not error, to avoid an existence oracle). Consider whether the api context should also cover CEL variables (it currently doesn't publish them directly).
Broader consideration: both #359 and this issue stem from the shared registry resolving any name under sudo regardless of context. A defense-in-depth option is to have compute_aggregation itself honor the context's publication flag; deferred because it is context-agnostic today and shared by multiple callers (needs its own design decision).
Source: swept while implementing #359. Related: #359.
Follow-up from PR #351... correction: from PR #359 (GIS stats publication boundary). A sweep for the same pattern found a parallel exposure in the simulation analytics API, which PR #359 (scoped to GIS per owner decision) does not touch.
Finding
spp_api_v2_simulationaccepts client-supplied statistic names and passes them straight to the aggregation engine with no publication allowlist:spp_api_v2_simulation/schemas/analytics.py:31—ComputeAggregationRequest.statistics: list[str] | None(client-supplied).spp_api_v2_simulation/routers/analytics.py:~52— passesrequest.statisticsto the service; the only gate is theaggregation:readscope.spp_api_v2_simulation/services/analytics_api_service.py:23-46—compute_aggregation(...)forwardsstatisticstoself.env["spp.analytics.service"].sudo().compute_aggregation(scope=..., statistics=statistics, context="api").As with the GIS path (#359), the aggregation registry (
spp_analytics/models/indicator_registry.py_try_statistic_model/_try_variable_model) resolves those names viasudo()against allspp.indicatorand allspp.cel.variablerecords, with nois_published_api/ context check. So a client with onlyaggregation:readcan request unpublished indicator names, or raw CEL variable names, and receive their aggregates — bypassing theis_published_apipublication boundary (spp.indicatorhasis_published_api, exposed viaget_published_for_context("api")).Severity: Medium
Same class and impact as #359 (k-anonymity mitigates, publication boundary bypassed for sensitive aggregates), for the API context instead of GIS.
Suggested fix
Mirror #359: in
AnalyticsApiService.compute_aggregation(or the router), restrict suppliedstatisticsto the API-published allowlist —env["spp.indicator"].get_published_for_context("api").mapped("name")— dropping unpublished/raw names before calling the aggregation service (drop, not error, to avoid an existence oracle). Consider whether theapicontext should also cover CEL variables (it currently doesn't publish them directly).Broader consideration: both #359 and this issue stem from the shared registry resolving any name under sudo regardless of context. A defense-in-depth option is to have
compute_aggregationitself honor the context's publication flag; deferred because it is context-agnostic today and shared by multiple callers (needs its own design decision).Source: swept while implementing #359. Related: #359.