From 973409cb10affaea2388c1ccba3979dca8f7a9e5 Mon Sep 17 00:00:00 2001 From: Jeremi Joslin Date: Wed, 10 Jun 2026 16:13:49 +0700 Subject: [PATCH 1/3] fix(dci): block sensitive metrics in predicates --- .../services/search_service.py | 26 +++++++++++++++++++ .../tests/test_search_service_internals.py | 12 +++++++++ 2 files changed, 38 insertions(+) diff --git a/spp_dci_server_social/services/search_service.py b/spp_dci_server_social/services/search_service.py index 456e63e2..ab77df7d 100644 --- a/spp_dci_server_social/services/search_service.py +++ b/spp_dci_server_social/services/search_service.py @@ -2,6 +2,7 @@ """DCI Social Registry Search Service - Maps Odoo partners to DCI schemas.""" import logging +import re import uuid from datetime import UTC, datetime from typing import Any @@ -34,6 +35,14 @@ "social", } +# Parameterized DCI CEL metrics that expose registry-derived facts which are +# not safe for unauthorised external predicate filtering. DCI Social Registry +# search predicates are a caller-supplied oracle (including total_count), so +# deny these sensitive metrics before compiling the expression. Keep this +# local instead of importing spp_dci_indicators: that addon is optional and is +# not a dependency of the DCI Social Registry server. +_DCI_PREDICATE_DENIED_METRICS = frozenset(("dr.dci.severity", "crvs.dci.has_event")) + class DCISocialSearchService: """Service for DCI Social Registry search operations. @@ -366,6 +375,8 @@ def _parse_predicate(self, predicate) -> list: if not expression or not expression.strip(): return [] + self._validate_external_predicate_expression(expression) + # Use CEL service to compile expression to domain cel_service = self.env["spp.cel.service"] @@ -387,6 +398,21 @@ def _parse_predicate(self, predicate) -> list: return result.get("domain", []) + def _validate_external_predicate_expression(self, expression: str) -> None: + """Reject sensitive DCI metrics in sender-supplied predicates. + + DCI predicate searches return counts and pageable matches, so allowing + callers to filter on raw DCI indicator cache values can disclose the + value by repeated queries even when the value is not present in the + response schema. Internal CEL use can still compile these metrics; this + guard only applies to external Social Registry predicate search. + """ + for accessor in _DCI_PREDICATE_DENIED_METRICS: + method_pattern = rf"(? list: """ Parse DCI expression into Odoo domain. diff --git a/spp_dci_server_social/tests/test_search_service_internals.py b/spp_dci_server_social/tests/test_search_service_internals.py index 3a2fa694..6d94a217 100644 --- a/spp_dci_server_social/tests/test_search_service_internals.py +++ b/spp_dci_server_social/tests/test_search_service_internals.py @@ -84,6 +84,18 @@ def test_parse_predicate_compile_failure_raises(self): self.service._parse_predicate("r.broken ==") self.assertIn("bad syntax", str(ctx.exception)) + def test_parse_predicate_rejects_sensitive_dci_method_metrics(self): + blocked = [ + "dr.dci.severity('Vision') >= 3", + "crvs.dci.has_event('death') == true", + "metric('dr.dci.severity', me, arg='Vision') >= 3", + 'metric("crvs.dci.has_event", me, arg="death") == true', + ] + for expression in blocked: + with self.assertRaises(ValueError) as ctx: + self.service._parse_predicate(expression) + self.assertIn("sensitive DCI metric", str(ctx.exception)) + # --- _to_dci_member ------------------------------------------------------ def test_to_dci_member_with_identifier_and_demographics(self): From 73082f9d6306c7a487ec65c363521abe7fe5b340 Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Fri, 26 Jun 2026 17:24:24 +0800 Subject: [PATCH 2/3] fix(dci): use 19.0 accessor names and harden predicate denylist The denied-metric names predated the 19.0 r.dci.. accessor rename, so on 19.0 the guard matched nothing and the vulnerability stayed open (the test passed against obsolete names). Update the denylist and test to the real accessors (r.dci.dr.severity, r.dci.crvs.has_event). Also harden against the CEL whitespace-dot bypass (e.g. 'r . dci . dr . severity(...)') by allowing optional whitespace around member-selection dots, and cover it in the test. --- spp_dci_server_social/services/search_service.py | 8 ++++++-- .../tests/test_search_service_internals.py | 9 +++++---- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/spp_dci_server_social/services/search_service.py b/spp_dci_server_social/services/search_service.py index ab77df7d..e3c0c5c1 100644 --- a/spp_dci_server_social/services/search_service.py +++ b/spp_dci_server_social/services/search_service.py @@ -41,7 +41,7 @@ # deny these sensitive metrics before compiling the expression. Keep this # local instead of importing spp_dci_indicators: that addon is optional and is # not a dependency of the DCI Social Registry server. -_DCI_PREDICATE_DENIED_METRICS = frozenset(("dr.dci.severity", "crvs.dci.has_event")) +_DCI_PREDICATE_DENIED_METRICS = frozenset(("r.dci.dr.severity", "r.dci.crvs.has_event")) class DCISocialSearchService: @@ -408,7 +408,11 @@ def _validate_external_predicate_expression(self, expression: str) -> None: guard only applies to external Social Registry predicate search. """ for accessor in _DCI_PREDICATE_DENIED_METRICS: - method_pattern = rf"(?= 3", - "crvs.dci.has_event('death') == true", - "metric('dr.dci.severity', me, arg='Vision') >= 3", - 'metric("crvs.dci.has_event", me, arg="death") == true', + "r.dci.dr.severity('Vision') >= 3", + "r . dci . dr . severity('Vision') >= 3", + "r.dci.crvs.has_event('death') == true", + "metric('r.dci.dr.severity', me, arg='Vision') >= 3", + 'metric("r.dci.crvs.has_event", me, arg="death") == true', ] for expression in blocked: with self.assertRaises(ValueError) as ctx: From 4af62a04201ed60f3c00e9064d6b1f3bbcebee36 Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Fri, 26 Jun 2026 18:02:24 +0800 Subject: [PATCH 3/3] fix(dci): deny all DR/CRVS accessors, match bare metric references Expand the predicate denylist beyond the two parameterized methods to the full disability (r.dci.dr.*) and vital/civil-status (r.dci.crvs.*) accessor tier: boolean oracles like r.dci.crvs.is_alive == false leak the same class of private data via count/page queries. Lower-risk r.dci.sr.*/r.dci.ibr.* metrics are intentionally left filterable. Unify the regex so it matches the accessor as a bounded dotted path in any resolving spelling -- bare (r.dci.dr.has_disability), called (r.dci.dr.severity('Vision')), and quoted metric('...') -- since non-parameterized metrics are referenced without parentheses. Boundaries reject prefix/substring matches (severity_score, is_alive_x). Add tests for the boolean forms and a negative test for benign and intentionally-allowed metrics. --- .../services/search_service.py | 51 ++++++++++++++----- .../tests/test_search_service_internals.py | 27 ++++++++++ 2 files changed, 65 insertions(+), 13 deletions(-) diff --git a/spp_dci_server_social/services/search_service.py b/spp_dci_server_social/services/search_service.py index e3c0c5c1..0a7a22db 100644 --- a/spp_dci_server_social/services/search_service.py +++ b/spp_dci_server_social/services/search_service.py @@ -35,13 +35,36 @@ "social", } -# Parameterized DCI CEL metrics that expose registry-derived facts which are -# not safe for unauthorised external predicate filtering. DCI Social Registry -# search predicates are a caller-supplied oracle (including total_count), so -# deny these sensitive metrics before compiling the expression. Keep this -# local instead of importing spp_dci_indicators: that addon is optional and is -# not a dependency of the DCI Social Registry server. -_DCI_PREDICATE_DENIED_METRICS = frozenset(("r.dci.dr.severity", "r.dci.crvs.has_event")) +# DCI CEL metrics that expose registry-derived facts which are not safe for +# unauthorised external predicate filtering. DCI Social Registry search +# predicates are a caller-supplied oracle (including total_count), so a boolean +# metric like r.dci.crvs.is_alive == false discloses the value one query at a +# time even though it never appears in the response schema. Deny these +# sensitive metrics before compiling the expression. +# +# Scope: disability (r.dci.dr.*) and vital/civil-status (r.dci.crvs.*) +# accessors -- the private-data tier this guard protects. Lower-risk Social +# Registry / inter-registry metrics (r.dci.sr.*, r.dci.ibr.*) are intentionally +# left filterable. +# +# Keep this list local instead of importing spp_dci_indicators: that addon is +# optional and is not a dependency of the DCI Social Registry server. +_DCI_PREDICATE_DENIED_METRICS = frozenset( + ( + # Disability registry (functional severity + disability flags) + "r.dci.dr.severity", + "r.dci.dr.has_disability", + "r.dci.dr.assessed", + "r.dci.dr.vision_severe", + "r.dci.dr.hearing_severe", + "r.dci.dr.mobility_severe", + # CRVS (vital events / civil status) + "r.dci.crvs.has_event", + "r.dci.crvs.is_alive", + "r.dci.crvs.birth_verified", + "r.dci.crvs.is_married", + ) +) class DCISocialSearchService: @@ -408,13 +431,15 @@ def _validate_external_predicate_expression(self, expression: str) -> None: guard only applies to external Social Registry predicate search. """ for accessor in _DCI_PREDICATE_DENIED_METRICS: - # CEL ignores whitespace around member-selection dots, so - # `r . dci . dr . severity(...)` is equivalent to the bare form and - # must not slip past the accessor-call check. + # Match the accessor as a dotted member path in any spelling that + # resolves to the metric: bare (r.dci.dr.has_disability), called + # (r.dci.dr.severity('Vision')), or quoted inside metric('...'). + # CEL ignores whitespace around member-selection dots, so allow it + # (`r . dci . dr . severity`). The boundaries reject a denied name + # that is only a prefix/substring of a longer identifier + # (r.dci.dr.severity_score, my_r.dci..., r.dci.crvs.is_alive_x). accessor_pattern = r"\s*\.\s*".join(map(re.escape, accessor.split("."))) - method_pattern = rf"(? list: diff --git a/spp_dci_server_social/tests/test_search_service_internals.py b/spp_dci_server_social/tests/test_search_service_internals.py index ffcf7742..bf94d99c 100644 --- a/spp_dci_server_social/tests/test_search_service_internals.py +++ b/spp_dci_server_social/tests/test_search_service_internals.py @@ -86,17 +86,44 @@ def test_parse_predicate_compile_failure_raises(self): def test_parse_predicate_rejects_sensitive_dci_method_metrics(self): blocked = [ + # Parameterized methods (accessor-call + metric() forms, incl. spaced dots) "r.dci.dr.severity('Vision') >= 3", "r . dci . dr . severity('Vision') >= 3", "r.dci.crvs.has_event('death') == true", "metric('r.dci.dr.severity', me, arg='Vision') >= 3", 'metric("r.dci.crvs.has_event", me, arg="death") == true', + # Non-parameterized disability flags (boolean oracles) + "r.dci.dr.has_disability == true", + "r.dci.dr.vision_severe == true", + "metric('r.dci.dr.mobility_severe', me) == true", + # Non-parameterized CRVS vital/civil status (boolean oracles) + "r.dci.crvs.is_alive == false", + "r.dci.crvs.is_married == true", + "metric('r.dci.crvs.birth_verified', me) == true", ] for expression in blocked: + # Through _parse_predicate so the test also pins that the guard is + # wired in ahead of CEL compilation (it raises before the compiler). with self.assertRaises(ValueError) as ctx: self.service._parse_predicate(expression) self.assertIn("sensitive DCI metric", str(ctx.exception)) + def test_validate_external_predicate_allows_benign_and_lower_risk_metrics(self): + # The guard must not over-match: benign registry predicates, identifiers + # that merely contain a denied name as a substring, and the intentionally + # allowed lower-risk SR/IBR metrics all pass validation untouched. + allowed = [ + "r.gender == 'female'", + "age_years(r.birthdate) >= 18", + "r.dci.dr.severity_score >= 3", # not a call, different accessor + "my_r.dci.dr.severity('Vision') >= 3", # prefixed identifier + "r.dci.sr.household_size >= 5", + "r.dci.ibr.has_duplicate == true", + ] + for expression in allowed: + # Should not raise. + self.service._validate_external_predicate_expression(expression) + # --- _to_dci_member ------------------------------------------------------ def test_to_dci_member_with_identifier_and_demographics(self):