security(gis): apply k-anonymity suppression to spatial query beneficiary counts - #366
Draft
gonzalesedwin1123 wants to merge 5 commits into
Draft
security(gis): apply k-anonymity suppression to spatial query beneficiary counts#366gonzalesedwin1123 wants to merge 5 commits into
gonzalesedwin1123 wants to merge 5 commits into
Conversation
Authenticated GIS statistics endpoints returned total_count = len(registrant_ids) with no k-anonymity/suppression, while the aggregate statistics were suppressed. A client with gis:read or statistics:read could send tiny polygons or proximity buffers (or grid/batch queries) to learn whether beneficiaries live at a precise location or in a small area — location/presence disclosure. Fix at the SpatialQueryService choke point: every registrant count it emits (single query, batch per-geometry items, batch summary, and proximity) is now passed through k-anonymity suppression using the caller's access-rule threshold (the same value the aggregation engine applies to statistics; falls back to the privacy service default). Any count below the threshold — including a genuinely empty area — is reported as total_count = 0 with a new count_suppressed flag, so small and empty results are indistinguishable and the flag itself cannot be used as a presence oracle. - spp_analytics: expose spp.analytics.service.get_effective_k_threshold() so the GIS service reuses the caller's threshold rather than a hardcoded value. - spp_api_v2_gis: _get_k_threshold() + _suppress_count() helpers; count_suppressed added to the four response schemas. No schema/data change → no migration. statistics.py's total_count counts published indicator definitions (catalog metadata), not registrants — not a leak. Refs: GIS count-leak report. Follow-ups (differencing residuals) to be filed.
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
Member
Author
|
Differencing-residual follow-ups filed: #367 (batch: per-geometry + summary counts differenceable across attacker-chosen tiles) and #368 (proximity |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 19.0 #366 +/- ##
==========================================
- Coverage 74.28% 71.22% -3.06%
==========================================
Files 372 175 -197
Lines 25385 14920 -10465
==========================================
- Hits 18857 10627 -8230
+ Misses 6528 4293 -2235
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Applied verbatim from CI's oca-gen-addon-readme output for the HISTORY fragments added in this branch.
Adversarial staff review found the count-flooring alone did NOT close the
presence oracle: an empty region returned statistics={}, access_level=None,
computed_at=None, query_method=area_fallback while a 1..(k-1)-person region
returned populated statistics, access_level=aggregate, a real computed_at, and
query_method=coordinates. A client could read computed_at (or the others) as a
binary presence oracle for an attacker-chosen location despite total_count=0.
Fix: _apply_suppression() now returns a canonical suppressed response for any
count in [0,k) — floors total_count to 0 AND blanks every people-correlated
field (statistics, access_level, from_cache, computed_at, query_method,
areas_matched) to fixed values, so an empty region and a small one are
byte-identical. Applied at all query/proximity/batch sites and the summary.
Request echoes (radius_km, relation, reference_points_count, geometries_queried,
id) are preserved. Schema query_method doc notes the 'suppressed' value.
Tests strengthened to assert the canonical form (empty == small, byte-identical)
and adjusted the proximity area-fallback tests for the new behavior. 197 pass.
Member
Author
|
Adversarial staff-engineer review done — one must-fix applied, one follow-up filed, one nit noted.
Re-verified: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addresses the reported issue "GIS queries leak exact beneficiary counts."
Problem
The authenticated GIS statistics endpoints (
/gis/query/statistics,.../batch,/gis/query/proximity) returnedtotal_count = len(registrant_ids)with no k-anonymity/suppression, even though the aggregate statistics are suppressed by the analytics engine. A client with onlygis:readorstatistics:readcould send very small polygons/buffers (or repeated grid/batch queries) and readtotal_countto learn whether one or more beneficiaries live at a precise location or in a small area — location/presence disclosure — while the actual statistics would be suppressed.Root cause (all on
19.0):services/spatial_query_service.pysetstotal_count = len(registrant_ids)at every result site; the routers strip the raw IDs (spatial_query.py:69,proximity.py:66) but keep the exact count. The count never passed throughspp.metric.privacy.Fix
Suppress every registrant count at the
SpatialQueryServicechoke point (covers single, batch per-geometry, batch summary, and proximity in one place), reusing the existing privacy machinery — no new threshold invented:spp_analytics: exposespp.analytics.service.get_effective_k_threshold()so the GIS service uses the caller's access-rule k-anonymity threshold (the same value the aggregation engine applies to statistics), falling back tospp.metric.privacy.DEFAULT_K_THRESHOLD(5).spp_api_v2_gis:_get_k_threshold()+_suppress_count(); a newcount_suppressed: boolfield on the four response schemas.Any count in
[0, k)— including a genuinely empty area — is reported astotal_count = 0withcount_suppressed = True. Merging true-zero into the suppressed band is deliberate: if empty→falseand 1..k−1→true, the flag itself would disclose presence (≥1) at an attacker-chosen location — the exact leak. Counts ofkor more pass through unchanged.routers/statistics.py'stotal_countcounts published indicator definitions (catalog metadata), not registrants — verified not a leak, left unchanged. No schema/data change → no migration.Tests
spp_analytics:get_effective_k_threshold()default + reads-rule (252 pass).spp_api_v2_gis:_suppress_countband (0/1/4 → suppressed; 5/123 → passthrough),_get_k_threshold(default + rule), and end-to-end empty-area suppression for single/batch/proximity (197 pass).test_within_returns_registrants_in_nearby_areas): its 2-registrant near area is below k, so the exact count is now suppressed to 0 — assertion updated to verify suppression; matching is still proven viaregistrant_ids(strengthened, not weakened).Follow-ups (differencing residuals, to be filed)
v1 suppresses each count independently below k, closing the primary "tiny polygon/buffer → presence" leak. Residual differencing attacks (hard against attacker-chosen geometries) to track separately: (a) batch — visible per-geometry counts + visible summary; (b) proximity
beyond= total −within.Draft — awaiting human security review before undraft/merge.