Skip to content

security(dci): deny sensitive parameterized DCI metrics in external predicate searches #235

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jeremi wants to merge 1 commit into
base: 19.0
Choose a base branch
from
+38 −0
Open

security(dci): deny sensitive parameterized DCI metrics in external predicate searches #235

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions spp_dci_server_social/services/search_service.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -26,6 +27,14 @@

_logger = logging.getLogger(__name__)

# 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.
Expand Down Expand Up @@ -352,6 +361,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"]

Expand All @@ -373,6 +384,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"(?<![\w.]){re.escape(accessor)}\s*\("
metric_pattern = rf"(?<![\w.])metric\s*\(\s*(['\"]){re.escape(accessor)}\1"
if re.search(method_pattern, expression) or re.search(metric_pattern, expression):
raise ValueError(_("Predicate searches cannot filter on sensitive DCI metric '%s'.") % accessor)
Comment on lines +396 to +400

@gemini-code-assist gemini-code-assist Bot Jun 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-high high

The current regular expression re.escape(accessor) expects the dots in the accessor (e.g., dr.dci.severity) to be immediately adjacent to the identifiers. However, in CEL (Common Expression Language), optional whitespace around member selection dots (e.g., dr . dci . severity(...)) is syntactically valid and ignored by the lexer/parser. An external caller could easily bypass this security check by inserting spaces around the dots.

To make this validation robust against such bypasses, we can split the accessor by . and join the escaped parts with \s*\.\s* to allow optional whitespace around the dots.

        for accessor in _DCI_PREDICATE_DENIED_METRICS:
            accessor_pattern = r"\s*\.\s*".join(map(re.escape, accessor.split(".")))
            method_pattern = rf"(?<![\w.]){accessor_pattern}\s*\("
            metric_pattern = rf"(?<![\w.])metric\s*\(\s*(['"]){re.escape(accessor)}\1"
            if re.search(method_pattern, expression) or re.search(metric_pattern, expression):
                raise ValueError(_("Predicate searches cannot filter on sensitive DCI metric '%s'.") % accessor)


def _parse_expression(self, expression) -> list:
"""
Parse DCI expression into Odoo domain.
Expand Down
12 changes: 12 additions & 0 deletions spp_dci_server_social/tests/test_search_service_internals.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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',
]
Comment on lines +88 to +93

@gemini-code-assist gemini-code-assist Bot Jun 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Add a test case with spaces around the dots (e.g., dr . dci . severity(...)) to verify that the robust regex validation correctly rejects accessors with whitespace formatting.

        blocked = [
            "dr.dci.severity('Vision') >= 3",
            "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):
Expand Down
Loading