Skip to content

Commit 5077ff9

Browse files
authored
Fix PII result copy regression (IBM#89)
Signed-off-by: lucarlig <luca.carlig@ibm.com>
1 parent cfe7087 commit 5077ff9

7 files changed

Lines changed: 191 additions & 13 deletions

File tree

Cargo.lock

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

plugins/rust/python-package/pii_filter/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[package]
22
name = "pii_filter"
3-
version = "0.3.1"
3+
version = "0.3.2"
44
edition.workspace = true
55
authors.workspace = true
66
license.workspace = true

plugins/rust/python-package/pii_filter/cpex_pii_filter/plugin-manifest.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
description: "Rust-backed PII detection and masking for prompt arguments, tool inputs, and tool outputs"
22
author: "ContextForge Contributors"
3-
version: "0.3.1"
3+
version: "0.3.2"
44
kind: "cpex_pii_filter.pii_filter.PIIFilterPlugin"
55
available_hooks:
66
- "prompt_pre_fetch"

plugins/rust/python-package/pii_filter/src/detector.rs

Lines changed: 64 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55

66
use log::{debug, warn};
77
use pyo3::prelude::*;
8-
use pyo3::types::{PyAny, PyDict, PyList, PySet, PyString, PyTuple};
8+
use pyo3::types::{PyAny, PyDict, PyList, PyMapping, PySet, PyString, PyTuple};
99
use pyo3_stub_gen::derive::*;
1010
use std::collections::HashMap;
1111

@@ -357,24 +357,28 @@ impl PIIDetectorRust {
357357
}
358358
}
359359

360-
// Handle dictionaries
361-
if let Ok(dict) = data.cast::<PyDict>() {
362-
let mut entries: Vec<(Py<PyAny>, Py<PyAny>)> = Vec::with_capacity(dict.len());
360+
// Handle mappings through the Python protocol. CPEX isolation wraps
361+
// dicts in copy-on-write dict subclasses whose visible entries are not
362+
// stored in the underlying PyDict table.
363+
if let Ok(mapping) = data.cast::<PyMapping>() {
364+
let mapping_len = mapping.len()?;
365+
let mut entries: Vec<(Py<PyAny>, Py<PyAny>)> = Vec::with_capacity(mapping_len);
363366
let mut all_detections = HashMap::new();
364-
if dict.len() > self.config.max_collection_items {
367+
if mapping_len > self.config.max_collection_items {
365368
warn!(
366369
"Rejected nested mapping at path '{}' because size {} exceeds max {}",
367-
path,
368-
dict.len(),
369-
self.config.max_collection_items
370+
path, mapping_len, self.config.max_collection_items
370371
);
371372
return Err(PyErr::new::<pyo3::exceptions::PyValueError, _>(format!(
372373
"Nested mapping exceeds maximum size of {} items",
373374
self.config.max_collection_items
374375
)));
375376
}
376377

377-
for (key, value) in dict.iter() {
378+
for item in mapping.items()?.iter() {
379+
let item = item.cast::<PyTuple>()?;
380+
let key = item.get_item(0)?;
381+
let value = item.get_item(1)?;
378382
let key_str = key.str()?.to_string_lossy().into_owned();
379383
let new_path = if path.is_empty() {
380384
key_str.clone()
@@ -1659,6 +1663,57 @@ class ConfigModel:
16591663
});
16601664
}
16611665

1666+
#[test]
1667+
fn test_process_nested_mapping_allows_collection_limit_boundary() {
1668+
Python::initialize();
1669+
Python::attach(|py| {
1670+
let config = PyDict::new(py);
1671+
config.set_item("detect_email", true).unwrap();
1672+
config.set_item("max_collection_items", 1).unwrap();
1673+
1674+
let detector = PIIDetectorRust::new(&config.into_any()).unwrap();
1675+
let data = PyDict::new(py);
1676+
data.set_item("email", "alice@example.com").unwrap();
1677+
1678+
let (modified, new_data, _) =
1679+
detector.process_nested(py, &data.into_any(), "").unwrap();
1680+
1681+
assert!(modified);
1682+
assert_eq!(
1683+
new_data
1684+
.bind(py)
1685+
.cast::<PyDict>()
1686+
.unwrap()
1687+
.get_item("email")
1688+
.unwrap()
1689+
.unwrap()
1690+
.extract::<String>()
1691+
.unwrap(),
1692+
"[REDACTED]"
1693+
);
1694+
});
1695+
}
1696+
1697+
#[test]
1698+
fn test_process_nested_mapping_rejects_over_collection_limit() {
1699+
Python::initialize();
1700+
Python::attach(|py| {
1701+
let config = PyDict::new(py);
1702+
config.set_item("detect_email", true).unwrap();
1703+
config.set_item("max_collection_items", 1).unwrap();
1704+
1705+
let detector = PIIDetectorRust::new(&config.into_any()).unwrap();
1706+
let data = PyDict::new(py);
1707+
data.set_item("first", "alice@example.com").unwrap();
1708+
data.set_item("second", "bob@example.com").unwrap();
1709+
1710+
let err = detector
1711+
.process_nested(py, &data.into_any(), "")
1712+
.unwrap_err();
1713+
assert!(err.is_instance_of::<pyo3::exceptions::PyValueError>(py));
1714+
});
1715+
}
1716+
16621717
#[test]
16631718
fn test_detects_plus_prefixed_international_phone_number() {
16641719
let config = PIIConfig {

plugins/tests/conftest.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,18 @@
4040

4141
cpex = types.ModuleType("cpex")
4242
framework = types.ModuleType("cpex.framework")
43+
hooks = types.ModuleType("cpex.framework.hooks")
44+
policies = types.ModuleType("cpex.framework.hooks.policies")
45+
memory = types.ModuleType("cpex.framework.memory")
4346

4447
framework.__dict__.update(plugin_hooks.__dict__)
48+
policies.HookPayloadPolicy = plugin_hooks.HookPayloadPolicy
49+
policies.apply_policy = plugin_hooks.apply_policy
50+
memory.wrap_payload_for_isolation = plugin_hooks.wrap_payload_for_isolation
4551
sys.modules["cpex"] = cpex
4652
sys.modules["cpex.framework"] = framework
53+
sys.modules["cpex.framework.hooks"] = hooks
54+
sys.modules["cpex.framework.hooks.policies"] = policies
55+
sys.modules["cpex.framework.memory"] = memory
4756
sys.modules["cpex.framework.models"] = plugin_hooks
4857
sys.modules["cpex.framework.settings"] = plugin_hooks

plugins/tests/pii_filter/test_integration.py

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313
ToolPostInvokePayload,
1414
ToolPreInvokePayload,
1515
)
16+
from cpex.framework.hooks.policies import HookPayloadPolicy, apply_policy
17+
from cpex.framework.memory import wrap_payload_for_isolation
1618
from cpex.framework.models import GlobalContext
1719

1820
from cpex_pii_filter.pii_filter import PIIDetectorRust, PIIFilterPlugin
@@ -314,6 +316,59 @@ async def test_tool_post_invoke_returns_copied_payload_for_frozen_models():
314316
assert result.modified_payload.result["contact"] == "[REDACTED]"
315317

316318

319+
@pytest.mark.asyncio
320+
async def test_tool_post_invoke_returns_new_nested_result_for_mcp_content():
321+
plugin = PIIFilterPlugin(_make_config())
322+
payload = ToolPostInvokePayload(
323+
name="search",
324+
result={
325+
"content": [
326+
{
327+
"type": "text",
328+
"text": "Contact alice@example.com",
329+
}
330+
],
331+
"isError": False,
332+
},
333+
)
334+
335+
result = await plugin.tool_post_invoke(payload, _make_context())
336+
337+
assert result.modified_payload is not None
338+
assert result.modified_payload is not payload
339+
assert result.modified_payload.result is not payload.result
340+
assert result.modified_payload.result["content"] is not payload.result["content"]
341+
assert result.modified_payload.result["content"][0] is not payload.result["content"][0]
342+
assert payload.result["content"][0]["text"] == "Contact alice@example.com"
343+
assert result.modified_payload.result["content"][0]["text"] == "Contact [REDACTED]"
344+
345+
346+
@pytest.mark.asyncio
347+
async def test_tool_post_invoke_survives_cpex_policy_with_isolated_payload():
348+
plugin = PIIFilterPlugin(_make_config())
349+
payload = ToolPostInvokePayload(
350+
name="search",
351+
result={
352+
"content": [{"type": "text", "text": "Contact alice@example.com"}],
353+
"isError": False,
354+
},
355+
)
356+
plugin_input = wrap_payload_for_isolation(payload)
357+
358+
result = await plugin.tool_post_invoke(plugin_input, _make_context())
359+
360+
assert result.modified_payload is not None
361+
filtered = apply_policy(
362+
plugin_input,
363+
result.modified_payload,
364+
HookPayloadPolicy(writable_fields=frozenset({"result"})),
365+
apply_to=payload,
366+
)
367+
assert filtered is not None
368+
assert payload.result["content"][0]["text"] == "Contact alice@example.com"
369+
assert filtered.result["content"][0]["text"] == "Contact [REDACTED]"
370+
371+
317372
@pytest.mark.asyncio
318373
async def test_tool_post_invoke_blocks_when_configured():
319374
plugin = PIIFilterPlugin(_make_config(block_on_detection=True))

plugins/tests/plugin_hooks.py

Lines changed: 60 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,70 @@
33
from __future__ import annotations
44

55
import importlib
6-
from dataclasses import dataclass, field
6+
from dataclasses import dataclass, field, fields, is_dataclass
77
from enum import Enum
88
from typing import Any
99

1010

11+
@dataclass(frozen=True)
12+
class HookPayloadPolicy:
13+
writable_fields: frozenset[str]
14+
15+
16+
class CopyOnWriteDict(dict):
17+
def __init__(self, original: dict[str, Any]) -> None:
18+
super().__init__()
19+
self._original = original
20+
21+
def __getitem__(self, key: Any) -> Any:
22+
return super().__getitem__(key) if key in self else self._original[key]
23+
24+
def __iter__(self):
25+
return iter(self._original)
26+
27+
def __len__(self) -> int:
28+
return len(self._original)
29+
30+
def items(self):
31+
return ((key, self[key]) for key in self)
32+
33+
def copy(self) -> dict:
34+
return dict(self.items())
35+
36+
37+
def wrap_payload_for_isolation(payload: Any) -> Any:
38+
if not is_dataclass(payload):
39+
return payload
40+
updates = {}
41+
for item in fields(payload):
42+
value = getattr(payload, item.name)
43+
updates[item.name] = CopyOnWriteDict(value) if isinstance(value, dict) else value
44+
return type(payload)(**updates)
45+
46+
47+
def apply_policy(
48+
original: Any,
49+
modified: Any,
50+
policy: HookPayloadPolicy,
51+
*,
52+
apply_to: Any | None = None,
53+
) -> Any | None:
54+
target = apply_to if apply_to is not None else original
55+
updates = {}
56+
for item in fields(modified):
57+
old_value = getattr(original, item.name)
58+
new_value = getattr(modified, item.name)
59+
if new_value == old_value:
60+
continue
61+
if item.name in policy.writable_fields:
62+
updates[item.name] = new_value
63+
if not updates:
64+
return None
65+
values = {item.name: getattr(target, item.name) for item in fields(target)}
66+
values.update(updates)
67+
return type(target)(**values)
68+
69+
1170
class PromptHookType(str, Enum):
1271
PROMPT_PRE_FETCH = "prompt_pre_fetch"
1372
PROMPT_POST_FETCH = "prompt_post_fetch"

0 commit comments

Comments
 (0)