From 8fc67cfab6bd2fc068a40f4f35fb0ea6703353df Mon Sep 17 00:00:00 2001 From: Kevin Costner <120246174+kevincostner17@users.noreply.github.com> Date: Tue, 15 Sep 2026 17:32:30 +0530 Subject: [PATCH 01/17] fix(parsers): detect XML encoding and reject DTDs with an expat pass open_safe_xml_binary looked for "]*?\bencoding\s*=\s*([\"'])([A-Za-z][A-Za-z0-9._\-]*)\1" +) + + +def _detect_xml_encoding(data: bytes) -> tuple[str, int]: + """Return ``(codec, bom_length)`` for XML *data* (XML 1.0 Appendix F). + + Raises ``ValueError`` for EBCDIC, which the XML readers do not support. + """ + for bom, codec in _XML_BOMS: + if data.startswith(bom): + return codec, len(bom) + for prefix, codec in _XML_PREFIXES: + if data.startswith(prefix): + return codec, 0 + if data.startswith(_EBCDIC_XML_PREFIX): + raise ValueError("unsupported XML encoding (EBCDIC)") + # A document entity starts with ASCII, so a NUL in either of the first two + # bytes means UTF-16 (expat applies the same rule, e.g. to " bool: + """Whether *data* contains a DTD/entity marker, in its raw bytes or decoded text.""" + lowered = data.lower() + if b" None: + raise ValueError(_DTD_NOT_ALLOWED) + + +def _expat_rejects_dtd(data: bytes) -> None: + """Run expat over *data* and raise ``ValueError`` on any DTD/entity declaration. + + This is the authoritative check: it sees the document exactly as + ``xml.etree.ElementTree`` will (same parser, same encoding detection), so no + encoding can hide a declaration from it. Parse errors are left for the real + XML reader to report, so this never raises ``ExpatError``. + """ + parser = xml.parsers.expat.ParserCreate(None, "}") # ElementTree's configuration + parser.StartDoctypeDeclHandler = _reject_dtd_decl + parser.EntityDeclHandler = _reject_dtd_decl + with contextlib.suppress(xml.parsers.expat.ExpatError, LookupError, UnicodeError): + parser.Parse(data, True) + @dataclass class ParseResult(HtmlReprMixin): @@ -136,7 +223,14 @@ def open_safe_xml_binary( *, max_bytes: int = _MAX_XML_BYTES, ) -> io.BytesIO: - """Return bounded XML bytes with DTD/entity declarations rejected.""" + """Return bounded XML bytes with DTD/entity declarations rejected. + + The document encoding is detected first (byte-order mark, UTF-16/UTF-32 + prefix, or XML declaration) so the marker scan also covers non-UTF-8 + documents, and an expat pass then rejects any DOCTYPE or entity + declaration the XML reader would process. Raises ``ValueError`` for an + oversized, DTD-bearing, or EBCDIC document. + """ stream = self.open_binary(source) try: data = stream.read(max_bytes + 1) @@ -146,7 +240,8 @@ def open_safe_xml_binary( if len(data) > max_bytes: raise ValueError(f"XML input exceeds {max_bytes} bytes") - lowered = data.lower() - if b"' +REF = "&a1;" + + +def _gpx_text(declared: str | None, doctype: str) -> str: + decl = f'' if declared else "" + return f'{decl}{doctype}{REF}' + + +def _sdmx_text(declared: str | None, doctype: str) -> str: + decl = f'' if declared else "" + return ( + f"{decl}{doctype}" + f'' + '' + ) + + +# (id, declared encoding, bytes encoder) +ENCODINGS = [ + ("utf-16-bom", "UTF-16", lambda s: s.encode("utf-16")), + ("utf-16-le-no-bom", "UTF-16", lambda s: s.encode("utf-16-le")), + ("utf-16-be-no-bom", "UTF-16", lambda s: s.encode("utf-16-be")), + ("utf-16-be-bom", "UTF-16", lambda s: b"\xfe\xff" + s.encode("utf-16-be")), + ("utf-32-bom", "UTF-32", lambda s: s.encode("utf-32")), + ("utf-32-le-no-bom", "UTF-32", lambda s: s.encode("utf-32-le")), + ("utf-32-be-no-bom", "UTF-32", lambda s: s.encode("utf-32-be")), + ("utf-8-sig", "UTF-8", lambda s: s.encode("utf-8-sig")), + ("iso-8859-1", "ISO-8859-1", lambda s: s.encode("iso-8859-1")), +] + + +def _max_cell_len(result: ParseResult) -> int: + return max( + (len(str(v)) for df in result.frames.values() for v in df.to_numpy().ravel()), + default=0, + ) + + +def _assert_rejected(result: ParseResult, label: str) -> None: + assert any(f"unsafe {label} XML" in w and "not allowed" in w for w in result.warnings), ( + result.warnings + ) + assert all(df.empty for df in result.frames.values()) + assert _max_cell_len(result) <= len(REF) + + +@pytest.mark.parametrize(("declared", "encode"), [e[1:] for e in ENCODINGS], + ids=[e[0] for e in ENCODINGS]) +def test_gpx_entity_expansion_rejected_in_every_encoding(declared, encode): + doc = encode(_gpx_text(declared, f"")) + _assert_rejected(fd.parse_domain(doc, format="gpx"), "GPX") + + +@pytest.mark.parametrize(("declared", "encode"), [e[1:] for e in ENCODINGS], + ids=[e[0] for e in ENCODINGS]) +def test_sdmx_entity_expansion_rejected_in_every_encoding(declared, encode): + doc = encode(_sdmx_text(declared, f"")) + result = fd.parse_domain(doc, format="sdmx") + _assert_rejected(result, "SDMX") + assert all("audit only" in w for w in result.warnings) + + +@pytest.mark.parametrize("doctype", ["").encode("utf-16") + _assert_rejected(fd.parse_domain(doc, format="gpx"), "GPX") + + +def test_external_system_dtd_without_internal_subset_in_utf16(): + text = _gpx_text("UTF-16", '') + doc = text.replace(REF, "x").encode("utf-16") + _assert_rejected(fd.parse_domain(doc, format="gpx"), "GPX") + + +def test_parameter_entity_only_dtd_in_utf16(): + text = _gpx_text("UTF-16", ']>').replace(REF, "x") + _assert_rejected(fd.parse_domain(text.encode("utf-16-le"), format="gpx"), "GPX") + + +def test_utf16_without_bom_or_declaration(): + # expat reads a NUL in the first two bytes as UTF-16, declaration or not. + for codec in ("utf-16-le", "utf-16-be"): + doc = _gpx_text(None, f"").encode(codec) + _assert_rejected(fd.parse_domain(doc, format="gpx"), "GPX") + leading_space = (" " + _gpx_text(None, f"")).encode(codec) + _assert_rejected(fd.parse_domain(leading_space, format="gpx"), "GPX") + + +def test_utf16_with_trailing_odd_byte_still_rejected(): + doc = _gpx_text("UTF-16", f"").encode("utf-16") + b"\x00" + _assert_rejected(fd.parse_domain(doc, format="gpx"), "GPX") + + +def test_utf16_declaration_over_utf8_bytes(): + with_dtd = _gpx_text("UTF-16", f"") + _assert_rejected(fd.parse_domain(with_dtd.encode("utf-8"), format="gpx"), "GPX") + _assert_rejected(fd.parse_domain(with_dtd, format="gpx"), "GPX") + + without_dtd = _gpx_text("UTF-16", "").replace(REF, "x") + result = fd.parse_domain(without_dtd.encode("utf-8"), format="gpx") + assert all(df.empty for df in result.frames.values()) + assert any("invalid GPX XML" in w for w in result.warnings), result.warnings + + +def test_ebcdic_prefix_is_rejected_as_unsupported(): + doc = ''.encode("cp037") + assert doc.startswith(b"\x4c\x6f\xa7\x94") + result = fd.parse_domain(doc, format="gpx") + assert all(df.empty for df in result.frames.values()) + assert any("unsafe GPX XML" in w and "unsupported XML encoding" in w + for w in result.warnings), result.warnings + + +@pytest.mark.parametrize("codec", ["utf-16", "utf-16-le", "utf-16-be", "utf-8-sig"]) +def test_benign_non_utf8_gpx_still_parses(codec): + text = ( + '' + if codec.startswith("utf-16") else '' + ) + 'Start' + result = fd.parse_domain(text.encode(codec), format="gpx") + assert not any("XML" in w for w in result.warnings), result.warnings + wp = result.frames["waypoints"] + assert len(wp) == 1 + assert wp["name"].iloc[0] == "Start" + assert wp["lat"].iloc[0] == 40.0 + + +def test_benign_utf16_sdmx_still_parses(): + text = _sdmx_text("UTF-16", "").replace(REF, "A") + result = fd.parse_domain(text.encode("utf-16"), format="sdmx") + obs = result.frames["observations"] + assert len(obs) == 1 + assert obs["REF"].iloc[0] == "A" + + +def test_path_and_str_sources(tmp_path: Path): + doc = _gpx_text("UTF-16", f"").encode("utf-16") + path = tmp_path / "evil.gpx" + path.write_bytes(doc) + + _assert_rejected(fd.parse_domain(path, format="gpx"), "GPX") + # clean_domain_file turns an existing str path into a Path. + _assert_rejected(fd.clean_domain_file(str(path), format="gpx"), "GPX") + # A str value passed to parse_domain is document content. + _assert_rejected( + fd.parse_domain(_gpx_text(None, f""), format="gpx"), "GPX" + ) + + benign = tmp_path / "ok.gpx" + benign.write_bytes( + 'ok'.encode("utf-16") + ) + assert len(fd.parse_domain(benign, format="gpx").frames["waypoints"]) == 1 + assert len(fd.clean_domain_file(str(benign), format="gpx").frames["waypoints"]) == 1 + + +@pytest.mark.parametrize( + ("prefix", "expected"), + [ + (b"\x00\x00\xfe\xff<", ("utf-32-be", 4)), + (b"\xff\xfe\x00\x00<", ("utf-32-le", 4)), + (b"\xfe\xff\x00<", ("utf-16-be", 2)), + (b"\xff\xfe<\x00", ("utf-16-le", 2)), + (b"\xef\xbb\xbf<", ("utf-8", 3)), + (b"\x00\x00\x00<", ("utf-32-be", 0)), + (b"<\x00\x00\x00", ("utf-32-le", 0)), + (b"\x00<\x00?", ("utf-16-be", 0)), + (b"<\x00?\x00", ("utf-16-le", 0)), + (b'', ("latin-1", 0)), + (b"", ("utf-8", 0)), + ], + ids=[ + "bom-utf-32-be", "bom-utf-32-le", "bom-utf-16-be", "bom-utf-16-le", "bom-utf-8", + "utf-32-be", "utf-32-le", "utf-16-be", "utf-16-le", "declared", "default", + ], +) +def test_detect_xml_encoding(prefix, expected): + assert _detect_xml_encoding(prefix) == expected + + +def test_expat_pass_is_authoritative_and_never_raises_expat_error(): + doc = _gpx_text("UTF-16", f"").encode("utf-16") + with pytest.raises(ValueError, match="not allowed"): + _expat_rejects_dtd(doc) + with pytest.raises(ValueError, match="not allowed"): + _expat_rejects_dtd(b'') + # Malformed input and unknown encodings are left for ElementTree to report. + _expat_rejects_dtd(b"") + _expat_rejects_dtd(b'') + _expat_rejects_dtd("".encode("utf-16")) + + +def test_safe_xml_binary_returns_the_original_bytes(): + doc = ''.encode("utf-16") + assert GPXParser().open_safe_xml_binary(doc).read() == doc From 081206d6bf80d379052c244f4d373c5b494981eb Mon Sep 17 00:00:00 2001 From: Kevin Costner <120246174+kevincostner17@users.noreply.github.com> Date: Tue, 15 Sep 2026 18:24:03 +0530 Subject: [PATCH 02/17] fix(csv): guard every header level, index labels and axis names against formulas sanitize_csv_formulas rebuilt the columns with _formula_guard applied to each label, but _formula_guard only handles str. With a multi-row header (read_csv_kwargs={"header": [0, 1]}) the labels are tuples, so every header cell was written unchanged, including cells such as =HYPERLINK(...) taken from the input. The rebuild also dropped columns.names, and with to_csv_kwargs={"index": True} index labels and names were written unguarded. The sanitizer now guards each level of MultiIndex columns and index, flat string index labels, and column and index names. Non-string labels are left as they are, and an axis that needs no change keeps its original type. Header aliases passed to the writer are caller-supplied and are documented as not guarded. --- docs/threat-model.md | 6 ++ src/freshdata/_util.py | 40 +++++++-- src/freshdata/api.py | 12 ++- tests/test_csv_formula_sanitize.py | 140 +++++++++++++++++++++++++++++ 4 files changed, 189 insertions(+), 9 deletions(-) diff --git a/docs/threat-model.md b/docs/threat-model.md index 755047b6..bed2c77d 100644 --- a/docs/threat-model.md +++ b/docs/threat-model.md @@ -38,6 +38,12 @@ now safe by default; byte-exact fidelity is the explicit opt-out: | streaming CLI (incl. quarantine export) | sanitize **on** | `--no-sanitize-formulas` | | HTML-report ledger CSV download | sanitize **on** | none (spreadsheet-bound artifact) | +Sanitizing covers every place input text reaches the file: cells, column +labels at every level of a multi-row header (`read_csv_kwargs={"header": +[0, 1]}`), index labels at every level, and column/index names (written with +`index=True`). Header aliases a caller passes to the writer +(`to_csv_kwargs={"header": [...]}`) are caller-supplied and written as given. + JSONL and Parquet are never altered. **Residual risk:** a consumer that opted out and opens the CSV in a spreadsheet re-accepts the injection risk. diff --git a/src/freshdata/_util.py b/src/freshdata/_util.py index 96bfa62c..2b63abee 100644 --- a/src/freshdata/_util.py +++ b/src/freshdata/_util.py @@ -209,12 +209,39 @@ def _formula_guard(value: object) -> object: return value +def _guard_axis(axis: pd.Index) -> pd.Index: + """Formula-guard every label (every level of a MultiIndex) and name of *axis*. + + The axis object is returned unchanged when nothing needs guarding, so + numeric, datetime and categorical axes keep their type. + """ + names = [_formula_guard(n) for n in axis.names] + names_changed = any(g is not n for g, n in zip(names, axis.names)) + if isinstance(axis, pd.MultiIndex): + if any(_formula_guard(v) is not v for level in axis.levels for v in level): + return pd.MultiIndex.from_tuples( + [tuple(_formula_guard(v) for v in label) for label in axis], names=names + ) + elif _is_stringlike_dtype(axis.dtype) or isinstance(axis.dtype, pd.CategoricalDtype): + changed = False + labels: list[object] = [] + for value in axis: + guarded = _formula_guard(value) + changed = changed or guarded is not value + labels.append(guarded) + if changed: + return pd.Index(labels, dtype=object, name=names[0], tupleize_cols=False) + return axis.set_names(names) if names_changed else axis + + def sanitize_csv_formulas(df: pd.DataFrame) -> pd.DataFrame: - """Copy of *df* safe to open in a spreadsheet: string cells (and column - labels) starting with ``= + - @ `` — including after leading - whitespace — are prefixed with ``'`` so they render as text instead of - executing as formulas. Non-string cells (including negative numbers) are - untouched. + """Copy of *df* safe to open in a spreadsheet: string cells, column + labels (every level of a multi-row header), index labels (every level), + and column/index names starting with ``= + - @ `` — including + after leading whitespace — are prefixed with ``'`` so they render as text + instead of executing as formulas. Non-string cells and labels (including + negative numbers) are untouched. Header aliases a caller passes to the + writer (``to_csv(header=[...])``) are not part of *df* and are not guarded. """ out = df.copy() for i, dtype in enumerate(out.dtypes): @@ -223,7 +250,8 @@ def sanitize_csv_formulas(df: pd.DataFrame) -> pd.DataFrame: guarded = column.astype(object).map(_formula_guard) if not guarded.equals(column.astype(object)): out.isetitem(i, guarded) - out.columns = pd.Index([_formula_guard(c) for c in out.columns]) + out.columns = _guard_axis(out.columns) + out.index = _guard_axis(out.index) return out diff --git a/src/freshdata/api.py b/src/freshdata/api.py index f47aba07..febd1cf0 100644 --- a/src/freshdata/api.py +++ b/src/freshdata/api.py @@ -490,11 +490,14 @@ def clean_csv( output_path: Optional path to write the cleaned CSV. sanitize_formulas: - On by default (safe by default): string cells (and column labels) + On by default (safe by default): string cells, column labels + (every level of a multi-row header), index labels and axis names in the **written** file that start with ``= + - @ `` — including after leading whitespace — are prefixed with ``'`` so spreadsheets render them as text instead of executing them (OWASP - CSV-injection guidance). Pass ``sanitize_formulas=False`` for a + CSV-injection guidance). Header aliases passed as + ``to_csv_kwargs={"header": [...]}`` come from the caller and are + written as given. Pass ``sanitize_formulas=False`` for a byte-exact round-trip of cell values; the returned DataFrame is never altered either way. return_report: @@ -579,9 +582,12 @@ def clean_excel( output_path: Optional path to write the cleaned workbook. sanitize_formulas: - On by default (safe by default): string cells (and column labels) + On by default (safe by default): string cells, column labels + (every level of a multi-row header), index labels and axis names in the **written** workbook that start with ``= + - @ `` — including after leading whitespace — are prefixed with ``'``. + Header aliases passed as ``to_excel_kwargs={"header": [...]}`` are + written as given. Without it, a value such as ``=1+1`` is stored as a live formula cell. Pass ``sanitize_formulas=False`` to write values unchanged; the returned DataFrame is never altered either way. diff --git a/tests/test_csv_formula_sanitize.py b/tests/test_csv_formula_sanitize.py index eb6c42b2..1af2c620 100644 --- a/tests/test_csv_formula_sanitize.py +++ b/tests/test_csv_formula_sanitize.py @@ -167,3 +167,143 @@ def test_stream_cli_sanitize_formulas_flag(tmp_path): ) assert rc == 0 assert "'=1+1" not in raw_out.read_text() # explicit opt-out: fidelity + + +# --------------------------------------------------------------------------- # +# Multi-row headers, index labels and axis names +# --------------------------------------------------------------------------- # + +_LIVE_PREFIXES = ("=", "+", "-", "@", "\t", "\r") + + +def _multi_header_input(tmp_path) -> str: + src = tmp_path / "in.csv" + src.write_text('=HYPERLINK("http://example.invalid"),b\n@SUM(A1),c\n1,x\n2,y\n') + return str(src) + + +def test_clean_csv_guards_every_level_of_a_multi_row_header(tmp_path): + out = tmp_path / "out.csv" + fd.clean_csv( + _multi_header_input(tmp_path), output_path=out, verbose=False, + read_csv_kwargs={"header": [0, 1]}, + ) + lines = out.read_text().splitlines() + assert lines[0] == '"\'=HYPERLINK(""http://example.invalid"")",b' + assert lines[1] == "'@SUM(A1),c" + back = pd.read_csv(out, header=[0, 1]) + assert not any(str(v).startswith(_LIVE_PREFIXES) for t in back.columns for v in t) + + +def test_clean_csv_multi_row_header_opt_out_keeps_raw_header(tmp_path): + out = tmp_path / "out.csv" + fd.clean_csv( + _multi_header_input(tmp_path), output_path=out, verbose=False, + read_csv_kwargs={"header": [0, 1]}, sanitize_formulas=False, + ) + lines = out.read_text().splitlines() + assert lines[0] == '"=HYPERLINK(""http://example.invalid"")",b' + assert lines[1] == "@SUM(A1),c" + + +def test_sanitizer_guards_three_level_header_and_leaves_non_str_levels(): + columns = pd.MultiIndex.from_tuples( + [("=a", -1, "+b"), ("ok", 2, "@c"), (" =d", -3, "e")], + names=["=lvl0", 7, "safe"], + ) + df = pd.DataFrame([[1, 2, 3]], columns=columns) + out = sanitize_csv_formulas(df) + assert isinstance(out.columns, pd.MultiIndex) + assert list(out.columns) == [ + ("'=a", -1, "'+b"), ("ok", 2, "'@c"), ("' =d", -3, "e"), + ] + assert list(out.columns.names) == ["'=lvl0", 7, "safe"] + # negative ints in a level are numbers, not formulas + assert list(out.columns.get_level_values(1)) == [-1, 2, -3] + assert out.to_numpy().tolist() == [[1, 2, 3]] + + +def test_sanitizer_guards_index_labels_and_names(): + df = pd.DataFrame( + {"v": [1, 2, 3]}, + index=pd.Index(["=cmd|' /C calc'!A0", "safe", "@SUM(A1)"], name="+idx"), + ) + df.columns.name = "-cols" + out = sanitize_csv_formulas(df) + assert list(out.index) == ["'=cmd|' /C calc'!A0", "safe", "'@SUM(A1)"] + assert out.index.name == "'+idx" + assert out.columns.name == "'-cols" + assert out["v"].tolist() == [1, 2, 3] + + +def test_sanitizer_guards_multiindex_index_levels(): + index = pd.MultiIndex.from_tuples([("=a", 1), ("b", -2)], names=["@k", None]) + out = sanitize_csv_formulas(pd.DataFrame({"v": [1, 2]}, index=index)) + assert list(out.index) == [("'=a", 1), ("b", -2)] + assert list(out.index.names) == ["'@k", None] + + +def test_sanitizer_keeps_numeric_and_datetime_axes_as_is(): + df = pd.DataFrame( + {0: [1, 2]}, index=pd.date_range("2024-01-01", periods=2, name="when") + ) + out = sanitize_csv_formulas(df) + pd.testing.assert_frame_equal(out, df) + assert isinstance(out.index, pd.DatetimeIndex) + ranged = pd.DataFrame({"a": [-1, -2]}) + assert isinstance(sanitize_csv_formulas(ranged).index, pd.RangeIndex) + + +def test_sanitizer_does_not_mutate_input_axes(): + columns = pd.MultiIndex.from_tuples([("=a", "b")], names=["=n", "m"]) + index = pd.Index(["=x"], name="@i") + df = pd.DataFrame([[1]], columns=columns, index=index) + sanitize_csv_formulas(df) + assert list(df.columns) == [("=a", "b")] + assert list(df.columns.names) == ["=n", "m"] + assert list(df.index) == ["=x"] + assert df.index.name == "@i" + + +def test_clean_csv_guards_index_and_names_when_index_written(tmp_path): + src = tmp_path / "in.csv" + src.write_text("=id,name\n=a1,x\n@b2,y\n") + out = tmp_path / "out.csv" + result = fd.clean_csv( + src, output_path=out, verbose=False, + read_csv_kwargs={"index_col": 0}, to_csv_kwargs={"index": True}, + ) + assert out.read_text().splitlines() == ["'=id,name", "'=a1,x", "'@b2,y"] + # the returned frame is not sanitized + assert list(result.index) == ["=a1", "@b2"] + assert result.index.name == "=id" + + +def test_clean_excel_guards_multi_row_header_and_index(tmp_path): + openpyxl = pytest.importorskip("openpyxl") + wb = openpyxl.Workbook() + ws = wb.active + for row in (['=HYPERLINK("http://example.invalid")', "b"], ["@SUM(A1)", "c"], + [1, "x"], [2, "y"]): + ws.append(row) + for row in ws.iter_rows(max_row=2): + for cell in row: + cell.data_type = "s" # header text in the input, not formulas + xlsx_in = tmp_path / "in.xlsx" + wb.save(xlsx_in) + + out = tmp_path / "out.xlsx" + # MultiIndex columns can only be written to Excel with index=True. + fd.clean_excel( + xlsx_in, output_path=out, verbose=False, + read_excel_kwargs={"header": [0, 1]}, to_excel_kwargs={"index": True}, + ) + cells = [ + c for row in openpyxl.load_workbook(out).active.iter_rows() + for c in row if c.value is not None + ] + assert not [c.coordinate for c in cells if c.data_type == "f"] + values = {c.coordinate: c.value for c in cells} + assert values["B1"] == '\'=HYPERLINK("http://example.invalid")' + assert values["B2"] == "'@SUM(A1)" + assert not [v for v in values.values() if str(v).startswith(_LIVE_PREFIXES)] From b3a5a34b08c1fb241c51022fe5cb2ada05d8699d Mon Sep 17 00:00:00 2001 From: Kevin Costner <120246174+kevincostner17@users.noreply.github.com> Date: Tue, 15 Sep 2026 18:43:37 +0530 Subject: [PATCH 03/17] fix(duckdb): spill into a private per-run directory instead of /tmp/freshdata_spill EngineConfig.temp_directory defaulted to the fixed path /tmp/freshdata_spill, which the DuckDB engine created with os.makedirs(exist_ok=True) and handed to DuckDB without checking its owner or mode. Spill files hold rows of the data being cleaned and are written with the process umask, so on a shared host other local users could read them, a directory another user created first was accepted as-is, and concurrent runs collided on DuckDB's fixed file names. temp_directory now defaults to None. The new execution/_spill.py resolves a base directory (an explicit temp_directory, else $FRESHDATA_SPILL_DIR, else the per-user cache directory, falling back to the system temp directory only when that is not writable), creates it with mode 0700, and refuses a base that is not owned by the current user or is group/other-writable without the sticky bit. Each run spills into its own mkdtemp directory under that base, which is removed after the connection closes, or when a returned output_format="duckdb" relation is released. The test suite points FRESHDATA_SPILL_DIR at a pytest temp directory for the whole session so no test writes to the real user cache. --- .gitignore | 1 - docs/backends.md | 16 +- docs/threat-model.md | 17 ++ src/freshdata/execution/_config.py | 7 +- src/freshdata/execution/_spill.py | 131 ++++++++ src/freshdata/execution/backends/_duckdb.py | 63 ++-- tests/conftest.py | 19 ++ tests/test_execution/test_duckdb_spill_dir.py | 282 ++++++++++++++++++ 8 files changed, 515 insertions(+), 21 deletions(-) create mode 100644 src/freshdata/execution/_spill.py create mode 100644 tests/test_execution/test_duckdb_spill_dir.py diff --git a/.gitignore b/.gitignore index 95886417..a37d06cc 100644 --- a/.gitignore +++ b/.gitignore @@ -24,7 +24,6 @@ site/ # Out-of-core engine: benchmark outputs, synthetic data, and DuckDB spill src/freshdata/benchmarks/results/ /tmp/freshdata_bench/ -/tmp/freshdata_spill/ # Benchmark runtime results: raw case files stay local; compact evidence is committed. benchmarks/results/* diff --git a/docs/backends.md b/docs/backends.md index 8be19985..8a95422c 100644 --- a/docs/backends.md +++ b/docs/backends.md @@ -93,10 +93,24 @@ interpolation) can differ from the pandas reference. ```python from freshdata.execution import EngineConfig -cfg = EngineConfig(engine="duckdb", memory_limit_gb=4, temp_directory="/tmp/spill") +import os + +cfg = EngineConfig(engine="duckdb", memory_limit_gb=4, + temp_directory=os.path.expanduser("~/scratch/freshdata-spill")) cfg = EngineConfig(engine="spark", spark_shuffle_partitions=200, output_format="spark") ``` +DuckDB spill files contain rows of the data being cleaned, so each run spills into +its own private (0700) subdirectory, removed when the run's connection closes (for +`output_format="duckdb"`, when the returned relation is released). By default the +subdirectory is created under `$FRESHDATA_SPILL_DIR`, or under the per-user cache +directory (`~/.cache/freshdata/spill` or `$XDG_CACHE_HOME/freshdata/spill` on Linux, +`~/Library/Caches/freshdata/spill` on macOS, `%LOCALAPPDATA%\freshdata\spill` on +Windows), falling back to the system temp directory only when that is not writable. +An explicit `temp_directory` is created with mode 0700 if missing; one that is not +owned by you, or is group/other-writable without the sticky bit, raises +`PermissionError`. + PySpark is an **optional dependency** (`pip install 'freshdata-cleaner[spark]'`) and also needs a JVM at runtime. Importing `freshdata` never imports pyspark. diff --git a/docs/threat-model.md b/docs/threat-model.md index bed2c77d..eed5a1eb 100644 --- a/docs/threat-model.md +++ b/docs/threat-model.md @@ -101,6 +101,23 @@ deterministic path on purpose: a per-run random salt would break the documented reproducibility of `model_context` and its audit fingerprint. This trade-off is tracked as a roadmap item, not silently changed. +### 7. Local temporary/spill files + +When a DuckDB run exceeds `memory_limit_gb`, DuckDB writes intermediate +relation data (rows of the dataset being cleaned) to disk with the process +umask. Each run therefore spills into its own `tempfile.mkdtemp` directory +(mode 0700) that no other local account can list or read, and concurrent runs +never share file names. The directory lives under `EngineConfig.temp_directory` +if set, else `$FRESHDATA_SPILL_DIR`, else the per-user cache directory +(system temp only when that is not writable), and is removed when the run's +connection closes, or when a returned `output_format="duckdb"` relation is +released (`execution/_spill.py`). The base directory must be owned by the +current user and not group/other-writable, or be sticky and owned by the user +or root; otherwise the run raises `PermissionError`. **Residual risk:** a +process killed mid-run (`SIGKILL`, power loss) leaves its private run +directory behind until it is deleted; root and the same user can always read +it. + ## Non-goals - **Not a sandbox.** FreshData reads tabular files; hostile *file formats* diff --git a/src/freshdata/execution/_config.py b/src/freshdata/execution/_config.py index 9bacce1d..ba955385 100644 --- a/src/freshdata/execution/_config.py +++ b/src/freshdata/execution/_config.py @@ -94,7 +94,12 @@ class EngineConfig: #: into order-preserving dedup; freshdata then warns that it materializes. streaming_dedup: bool = True memory_limit_gb: float = 8.0 - temp_directory: str = "/tmp/freshdata_spill" + #: Base directory for DuckDB spill files. Each run spills into its own + #: private (0700) subdirectory, removed when the run's connection closes. + #: ``None`` (default) uses ``$FRESHDATA_SPILL_DIR`` or the per-user cache + #: directory; an explicit directory must be owned by the current user and + #: not group/other-writable (or be sticky, like ``/tmp``), else it raises. + temp_directory: str | None = None polars_n_threads: int | None = None duckdb_threads: int | None = None #: Number of shuffle partitions for the Spark backend (``None`` = Spark default). diff --git a/src/freshdata/execution/_spill.py b/src/freshdata/execution/_spill.py new file mode 100644 index 00000000..9e1ae688 --- /dev/null +++ b/src/freshdata/execution/_spill.py @@ -0,0 +1,131 @@ +"""Private, per-run spill directories for out-of-core engines. + +DuckDB writes intermediate relation data (rows of the dataset being cleaned) to +its ``temp_directory`` once a query exceeds ``memory_limit``. Those files use +fixed names and the process umask, so a directory shared between users or runs +exposes them to other local accounts and makes concurrent runs collide. + +Each run therefore spills into its own ``tempfile.mkdtemp`` directory (mode +0700), created inside a base directory that is checked for ownership and +permissions, and removed when the run's connection closes. The base is: + +1. ``EngineConfig.temp_directory`` when set explicitly; +2. otherwise ``$FRESHDATA_SPILL_DIR`` when set; +3. otherwise the per-user cache directory (``$XDG_CACHE_HOME/freshdata/spill`` + or ``~/.cache/freshdata/spill`` on Linux, ``~/Library/Caches/freshdata/spill`` + on macOS, ``%LOCALAPPDATA%\\freshdata\\spill`` on Windows); +4. and, only when that cache directory cannot be created or written, + ``tempfile.gettempdir()``. +""" + +from __future__ import annotations + +import logging +import os +import shutil +import stat +import sys +import tempfile +from typing import TYPE_CHECKING + +if TYPE_CHECKING: # pragma: no cover - typing only + from ._config import EngineConfig + +log = logging.getLogger("freshdata.execution.spill") + +#: Environment variable that overrides the default spill base directory. +SPILL_DIR_ENV = "FRESHDATA_SPILL_DIR" +_RUN_PREFIX = "freshdata_spill_" + + +class UnsafeSpillDirectoryError(PermissionError): + """A spill base directory other local users could read from or tamper with.""" + + +def _default_cache_base() -> str | None: + """The per-user cache spill directory, or ``None`` if it cannot be resolved.""" + if sys.platform == "win32": + root = os.environ.get("LOCALAPPDATA") or os.path.join( + os.path.expanduser("~"), "AppData", "Local" + ) + elif sys.platform == "darwin": + root = os.path.join(os.path.expanduser("~"), "Library", "Caches") + else: + xdg = os.environ.get("XDG_CACHE_HOME") + # The XDG spec says a relative value is invalid and must be ignored. + root = xdg if xdg and os.path.isabs(xdg) else os.path.join( + os.path.expanduser("~"), ".cache" + ) + if not os.path.isabs(root): # no resolvable home directory + return None + return os.path.join(root, "freshdata", "spill") + + +def _ensure_safe_base(path: str) -> str: + """Create *path* (mode 0700) if needed and check it is safe to spill into. + + On POSIX the directory is accepted when it is owned by the effective user + and not writable by group or other, or when it has the sticky bit (like + ``/tmp``) and is owned by the effective user or root. Anything else raises + :class:`UnsafeSpillDirectoryError` naming the owner and mode. + """ + os.makedirs(path, mode=0o700, exist_ok=True) + geteuid = getattr(os, "geteuid", None) + if os.name != "posix" or geteuid is None: + return path + info = os.stat(path) # follows symlinks: the real target is what gets checked + if not stat.S_ISDIR(info.st_mode): + raise NotADirectoryError(f"spill directory {path!r} is not a directory") + euid = geteuid() + mode = stat.S_IMODE(info.st_mode) + private = info.st_uid == euid and not mode & (stat.S_IWGRP | stat.S_IWOTH) + sticky = bool(mode & stat.S_ISVTX) and info.st_uid in (euid, 0) + if not (private or sticky): + raise UnsafeSpillDirectoryError( + f"refusing to spill into {path!r}: owned by uid {info.st_uid} with mode " + f"{oct(mode)}; a spill directory must be owned by the current user " + f"(uid {euid}) and not group/other-writable, or be a sticky directory " + "such as /tmp. Fix its permissions (chmod 700) or choose another " + "EngineConfig.temp_directory." + ) + return path + + +def _user_spill_base() -> str: + """Resolve and prepare the default spill base directory (see module docstring).""" + override = os.environ.get(SPILL_DIR_ENV) + if override: + return _ensure_safe_base(os.path.expanduser(override)) + base = _default_cache_base() + if base is not None: + try: + # Keep the freshdata cache parent private too, not just the leaf. + os.makedirs(os.path.dirname(base), mode=0o700, exist_ok=True) + base = _ensure_safe_base(base) + if os.access(base, os.W_OK | os.X_OK): + return base + except UnsafeSpillDirectoryError: + raise + except OSError as exc: + log.debug("freshdata spill: cache directory %r unusable (%s)", base, exc) + # The per-run mkdtemp child below is still 0700 inside the temp directory. + return _ensure_safe_base(tempfile.gettempdir()) + + +def create_run_spill_dir(engine_config: EngineConfig) -> str: + """Create and return a private (0700) spill directory for one run. + + The caller removes it with :func:`remove_run_spill_dir` once the engine + connection using it is closed. + """ + explicit = engine_config.temp_directory + if explicit is not None: + base = _ensure_safe_base(os.path.expanduser(os.fspath(explicit))) + else: + base = _user_spill_base() + return tempfile.mkdtemp(prefix=_RUN_PREFIX, dir=base) + + +def remove_run_spill_dir(path: str) -> None: + """Remove a run directory made by :func:`create_run_spill_dir` (best effort).""" + shutil.rmtree(path, ignore_errors=True) diff --git a/src/freshdata/execution/backends/_duckdb.py b/src/freshdata/execution/backends/_duckdb.py index 91cd86de..0b35236e 100644 --- a/src/freshdata/execution/backends/_duckdb.py +++ b/src/freshdata/execution/backends/_duckdb.py @@ -3,7 +3,8 @@ Registers the source (Parquet path read in-place, or an in-memory frame via Arrow) and applies the deterministic representation-repair + structural-reduction subset as a staged SQL pipeline, letting DuckDB stream and spill to -``temp_directory`` under ``memory_limit``. Steps outside that subset fall back to +a private per-run subdirectory of ``temp_directory`` (see ``execution/_spill.py``) +under ``memory_limit``. Steps outside that subset fall back to the pandas pipeline. Materialization is honest and caller-controlled: with the default @@ -17,7 +18,6 @@ import logging import math -import os import time import weakref from typing import TYPE_CHECKING, Any @@ -42,6 +42,7 @@ ) from .._plan import PlanGenerator from .._report import finalize_report, finalize_report_native, init_report, zero_column_frame +from .._spill import create_run_spill_dir, remove_run_spill_dir from ._pandas import materialize_to_pandas if TYPE_CHECKING: # pragma: no cover - typing only @@ -85,15 +86,49 @@ def _number_sql(value: Any) -> str: return f"{number}" if math.isfinite(number) else f"CAST('{number}' AS DOUBLE)" -def _release_native_relation_connection(key: int) -> None: - conn = _NATIVE_RELATION_CONNECTIONS.pop(key, None) - if conn is not None: +def _open_spill_connection(duckdb: Any, engine_config: EngineConfig) -> tuple[Any, str]: + """Connect DuckDB with a private per-run spill directory; return ``(conn, run_dir)``. + + Spill files hold dataset rows, so they never go to a shared or pre-existing + directory (see ``execution/_spill.py``). The directory is removed again if + the connection cannot be opened. + """ + run_dir = create_run_spill_dir(engine_config) + conn_config: dict[str, Any] = { + "memory_limit": f"{engine_config.memory_limit_gb}GB", + "temp_directory": run_dir, + } + if engine_config.duckdb_threads is not None: + conn_config["threads"] = engine_config.duckdb_threads + try: + return duckdb.connect(config=conn_config), run_dir + except BaseException: + remove_run_spill_dir(run_dir) + raise + + +def _close_spill_connection(conn: Any, run_dir: str) -> None: + """Close *conn*, then remove its private spill directory.""" + try: conn.close() + finally: + remove_run_spill_dir(run_dir) + + +def _release_native_relation_connection(key: int) -> None: + entry = _NATIVE_RELATION_CONNECTIONS.pop(key, None) + if entry is not None: + _close_spill_connection(*entry) -def _keep_native_relation_connection_alive(relation: Any, conn: Any) -> None: +def _keep_native_relation_connection_alive(relation: Any, conn: Any, run_dir: str) -> None: + """Keep *conn* (and its spill directory) alive for as long as *relation* is. + + Releasing the relation closes the connection and then removes *run_dir*; + ``weakref.finalize`` also runs this at interpreter exit. + """ key = id(relation) - _NATIVE_RELATION_CONNECTIONS[key] = conn + _NATIVE_RELATION_CONNECTIONS[key] = (conn, run_dir) weakref.finalize(relation, _release_native_relation_connection, key) @@ -124,17 +159,9 @@ def execute( return cleaned, report started = time.perf_counter() - os.makedirs(engine_config.temp_directory, exist_ok=True) - conn_config: dict[str, Any] = { - "memory_limit": f"{engine_config.memory_limit_gb}GB", - "temp_directory": engine_config.temp_directory, - } - if engine_config.duckdb_threads is not None: - conn_config["threads"] = engine_config.duckdb_threads - native = engine_config.output_format in NATIVE_HANDLE_FORMATS - conn = duckdb.connect(config=conn_config) + conn, run_dir = _open_spill_connection(duckdb, engine_config) close_conn = True try: self._register_source(conn, source) @@ -152,7 +179,7 @@ def execute( cleaned = relation # The relation is tied to this connection; keep it open so the # caller can stream from it. Closing here would invalidate it. - _keep_native_relation_connection_alive(cleaned, conn) + _keep_native_relation_connection_alive(cleaned, conn, run_dir) close_conn = False elif engine_config.output_format == "arrow" and not config.semantic_enabled: # Fetch straight into the requested format instead of building a @@ -166,7 +193,7 @@ def execute( cleaned = relation.fetchdf() finally: if close_conn: - conn.close() + _close_spill_connection(conn, run_dir) if native: finalize_report_native(report, started) diff --git a/tests/conftest.py b/tests/conftest.py index 04c42a38..8edfec53 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,3 +1,4 @@ +import os import re from pathlib import Path @@ -129,3 +130,21 @@ def already_clean() -> pd.DataFrame: "c": ["x", "y", "z"], } ) + + +@pytest.fixture(scope="session", autouse=True) +def _isolate_duckdb_spill_dir(tmp_path_factory): + """Keep DuckDB spill directories out of the real per-user cache during tests. + + Session-scoped so it is already in place for module- and session-scoped + fixtures that run the DuckDB engine (a function-scoped fixture is not). + Tests of the default location remove the variable with ``monkeypatch``. + """ + key = "FRESHDATA_SPILL_DIR" + previous = os.environ.get(key) + os.environ[key] = str(tmp_path_factory.mktemp("freshdata-spill")) + yield + if previous is None: + os.environ.pop(key, None) + else: + os.environ[key] = previous diff --git a/tests/test_execution/test_duckdb_spill_dir.py b/tests/test_execution/test_duckdb_spill_dir.py new file mode 100644 index 00000000..e5442092 --- /dev/null +++ b/tests/test_execution/test_duckdb_spill_dir.py @@ -0,0 +1,282 @@ +"""DuckDB spills into a private, per-run directory that is removed afterwards. + +Regression tests for the shared, fixed ``/tmp/freshdata_spill`` default, where +spill files holding dataset rows were readable by other local users and +concurrent runs collided. Every test redirects HOME, XDG_CACHE_HOME and +FRESHDATA_SPILL_DIR into ``tmp_path``; nothing here touches the real /tmp. +""" + +from __future__ import annotations + +import gc +import os +import stat +import sys +import threading + +import pandas as pd +import pytest + +import freshdata as fd +from freshdata.execution import EngineConfig, _spill +from freshdata.execution._spill import ( + SPILL_DIR_ENV, + UnsafeSpillDirectoryError, + _user_spill_base, + create_run_spill_dir, +) + +duckdb = pytest.importorskip("duckdb") + +posix_only = pytest.mark.skipif( + os.name != "posix" or not hasattr(os, "geteuid"), + reason="POSIX ownership and permission bits", +) + + +def _mode(path) -> int: + return stat.S_IMODE(os.stat(path).st_mode) + + +@pytest.fixture +def home(tmp_path, monkeypatch): + """A private HOME/cache layout inside tmp_path, with no spill override.""" + home_dir = tmp_path / "home" + home_dir.mkdir() + monkeypatch.setenv("HOME", str(home_dir)) + monkeypatch.setenv("XDG_CACHE_HOME", str(tmp_path / "xdg-cache")) + monkeypatch.setenv("LOCALAPPDATA", str(tmp_path / "localappdata")) + monkeypatch.delenv(SPILL_DIR_ENV, raising=False) + return tmp_path + + +def _expected_cache_base(root) -> str: + if sys.platform == "win32": + return str(root / "localappdata" / "freshdata" / "spill") + if sys.platform == "darwin": + return str(root / "home" / "Library" / "Caches" / "freshdata" / "spill") + return str(root / "xdg-cache" / "freshdata" / "spill") + + +class _Captured(list): + """Connections seen so far; ``hook`` (if set) runs right after each connect.""" + + hook = None + + +@pytest.fixture +def captured(monkeypatch): + """Record the ``temp_directory`` DuckDB is connected with, while it is live.""" + seen = _Captured() + real_connect = duckdb.connect + + def connect(*args, **kwargs): + conn = real_connect(*args, **kwargs) + config = kwargs.get("config") or {} + if "temp_directory" in config: + path = config["temp_directory"] + seen.append({"path": path, "isdir": os.path.isdir(path), "mode": _mode(path)}) + if seen.hook is not None: + seen.hook() + return conn + + monkeypatch.setattr(duckdb, "connect", connect) + return seen + + +def test_temp_directory_defaults_to_none(): + assert EngineConfig().temp_directory is None + assert EngineConfig(engine="duckdb").temp_directory is None + + +def test_default_run_uses_private_dir_under_user_cache_and_removes_it( + home, captured, small_df, native_config +): + out = fd.clean(small_df.copy(), config=native_config, engine="duckdb") + assert isinstance(out, pd.DataFrame) + + [run] = captured + base = _expected_cache_base(home) + assert os.path.dirname(run["path"]) == base + assert os.path.basename(run["path"]).startswith("freshdata_spill_") + assert run["isdir"] + if os.name == "posix": + assert run["mode"] == 0o700 + assert _mode(base) == 0o700 + assert not os.path.exists(run["path"]) + assert os.listdir(base) == [] + assert _user_spill_base() == base + + +def test_spill_dir_env_override(home, monkeypatch, captured, small_df, native_config): + override = home / "env-spill" + monkeypatch.setenv(SPILL_DIR_ENV, str(override)) + fd.clean(small_df.copy(), config=native_config, engine="duckdb") + [run] = captured + assert os.path.dirname(run["path"]) == str(override) + assert not os.path.exists(run["path"]) + assert not os.path.exists(_expected_cache_base(home)) + + +def test_explicit_temp_directory_gets_a_private_run_subdirectory( + home, captured, small_df, native_config +): + spill = home / "explicit" / "spill" + ec = EngineConfig(engine="duckdb", memory_limit_gb=0.5, temp_directory=str(spill)) + fd.clean(small_df.copy(), config=native_config, engine_config=ec) + [run] = captured + assert os.path.dirname(run["path"]) == str(spill) + if os.name == "posix": + assert run["mode"] == 0o700 + assert _mode(spill) == 0o700 + assert not os.path.exists(run["path"]) + assert spill.is_dir() and os.listdir(spill) == [] + + +def test_parallel_runs_get_distinct_directories(home, captured, small_df, native_config): + barrier = threading.Barrier(2) + captured.hook = lambda: barrier.wait(timeout=60) + errors: list[BaseException] = [] + + def run() -> None: + try: + fd.clean(small_df.copy(), config=native_config, engine="duckdb") + except BaseException as exc: # pragma: no cover - surfaced below + errors.append(exc) + + threads = [threading.Thread(target=run) for _ in range(2)] + for t in threads: + t.start() + for t in threads: + t.join(timeout=120) + assert not errors + paths = [r["path"] for r in captured] + assert len(paths) == 2 and len(set(paths)) == 2 + assert all(r["isdir"] for r in captured) + assert not any(os.path.exists(p) for p in paths) + + +def test_native_relation_keeps_directory_until_released( + home, captured, small_df, native_config +): + ec = EngineConfig(engine="duckdb", output_format="duckdb") + relation = fd.clean(small_df.copy(), config=native_config, engine_config=ec) + [run] = captured + assert os.path.isdir(run["path"]) + assert len(relation.fetchdf()) > 0 + assert os.path.isdir(run["path"]) + + del relation + gc.collect() + assert not os.path.exists(run["path"]) + + +def test_run_directory_removed_when_connect_fails(home, monkeypatch, small_df, native_config): + spill = home / "spill" + + def broken_connect(*args, **kwargs): + raise duckdb.IOException("boom") + + monkeypatch.setattr(duckdb, "connect", broken_connect) + ec = EngineConfig(engine="duckdb", temp_directory=str(spill)) + with pytest.raises(duckdb.IOException): + fd.clean(small_df.copy(), config=native_config, engine_config=ec) + assert os.listdir(spill) == [] + + +@posix_only +def test_world_writable_base_without_sticky_bit_raises(home, small_df, native_config): + spill = home / "shared" + spill.mkdir() + os.chmod(spill, 0o777) + with pytest.raises(PermissionError, match="0o777"): + create_run_spill_dir(EngineConfig(temp_directory=str(spill))) + ec = EngineConfig(engine="duckdb", temp_directory=str(spill)) + with pytest.raises(UnsafeSpillDirectoryError): + fd.clean(small_df.copy(), config=native_config, engine_config=ec) + assert os.listdir(spill) == [] + + +@posix_only +def test_group_writable_base_raises(home): + spill = home / "group" + spill.mkdir() + os.chmod(spill, 0o770) + with pytest.raises(UnsafeSpillDirectoryError): + create_run_spill_dir(EngineConfig(temp_directory=str(spill))) + + +@posix_only +def test_sticky_world_writable_base_is_accepted(home): + spill = home / "sticky" + spill.mkdir() + os.chmod(spill, 0o1777) + run = create_run_spill_dir(EngineConfig(temp_directory=str(spill))) + try: + assert os.path.dirname(run) == str(spill) + assert _mode(run) == 0o700 + finally: + _spill.remove_run_spill_dir(run) + assert not os.path.exists(run) + + +@posix_only +def test_foreign_owner_raises(home, monkeypatch): + spill = home / "foreign" + spill.mkdir(mode=0o700) + real_euid = os.geteuid() + monkeypatch.setattr(os, "geteuid", lambda: real_euid + 4242) + with pytest.raises(UnsafeSpillDirectoryError, match=f"uid {real_euid}"): + create_run_spill_dir(EngineConfig(temp_directory=str(spill))) + assert os.listdir(spill) == [] + + +@posix_only +def test_symlink_to_unsafe_directory_raises(home): + target = home / "target" + target.mkdir() + os.chmod(target, 0o777) + link = home / "link" + link.symlink_to(target, target_is_directory=True) + with pytest.raises(UnsafeSpillDirectoryError): + create_run_spill_dir(EngineConfig(temp_directory=str(link))) + + +@posix_only +def test_unsafe_env_and_default_cache_dirs_raise(home, monkeypatch): + shared = home / "env-shared" + shared.mkdir() + os.chmod(shared, 0o777) + monkeypatch.setenv(SPILL_DIR_ENV, str(shared)) + with pytest.raises(UnsafeSpillDirectoryError): + create_run_spill_dir(EngineConfig()) + + monkeypatch.delenv(SPILL_DIR_ENV) + cache = _expected_cache_base(home) + os.makedirs(cache) + os.chmod(cache, 0o777) + with pytest.raises(UnsafeSpillDirectoryError): + create_run_spill_dir(EngineConfig()) + + +@posix_only +@pytest.mark.skipif(hasattr(os, "geteuid") and os.geteuid() == 0, + reason="root ignores directory permissions") +def test_unwritable_cache_falls_back_to_private_dir_in_temp(home, monkeypatch): + readonly = home / "readonly" + readonly.mkdir() + monkeypatch.setenv("HOME", str(readonly / "home")) + monkeypatch.setenv("XDG_CACHE_HOME", str(readonly / "cache")) + systmp = home / "systmp" + systmp.mkdir(mode=0o700) + monkeypatch.setattr(_spill.tempfile, "gettempdir", lambda: str(systmp)) + os.chmod(readonly, 0o500) + try: + run = create_run_spill_dir(EngineConfig()) + finally: + os.chmod(readonly, 0o700) + try: + assert os.path.dirname(run) == str(systmp) + assert _mode(run) == 0o700 + finally: + _spill.remove_run_spill_dir(run) From 6bc6b83ffa29e2055d813cadb9a4e8681b056f4c Mon Sep 17 00:00:00 2001 From: Kevin Costner <120246174+kevincostner17@users.noreply.github.com> Date: Tue, 15 Sep 2026 17:32:01 +0530 Subject: [PATCH 04/17] fix(contracts): keyed or label-free baseline category labels Baseline categorical frequencies were stored under unkeyed, truncated SHA-1 labels, so anyone holding a baseline JSON could recover low-entropy labels (diagnoses, countries, products) and their exact shares by hashing a guess list. - build_baseline(label_key=...) / $FRESHDATA_BASELINE_KEY: labels become "k:" + HMAC-SHA256 (domain-separated, 128 bits); only a key identifier is stored (label_mode="hmac-sha256"). - Without a key the baseline is label-free (label_mode="rank"): a descending frequency profile, compared by rank for categorical PSI. - compare_to_baseline/monitor_contract take label_key; a keyed baseline with a missing or different key skips categorical PSI and reports a drift.categorical_drift_skipped warning instead of raising. - In-process baselines (raw-frame compare_to_baseline, clean_enterprise inline baseline) use a random per-call key. - Schema bumps to freshdata-baseline-v2; v1 baselines still load, compare via the legacy path and warn that they should be rebuilt. - Threat model section 9 documents persisted baselines. --- docs/threat-model.md | 23 ++ src/freshdata/enterprise/contracts.py | 240 ++++++++++++++++++--- src/freshdata/enterprise/interface.py | 39 +++- tests/test_baseline_label_privacy.py | 289 ++++++++++++++++++++++++++ tests/test_enterprise_contracts.py | 11 +- 5 files changed, 560 insertions(+), 42 deletions(-) create mode 100644 tests/test_baseline_label_privacy.py diff --git a/docs/threat-model.md b/docs/threat-model.md index eed5a1eb..b79dfaeb 100644 --- a/docs/threat-model.md +++ b/docs/threat-model.md @@ -118,6 +118,29 @@ process killed mid-run (`SIGKILL`, power loss) leaves its private run directory behind until it is deleted; root and the same user can always read it. +### 9. Persisted baselines and profiles + +Drift baselines (`fd.build_baseline` / `save_baseline`) are meant to be +committed and shared, so they must not carry the data they summarise. With the +default `include_samples=False` a baseline never stores raw sample values, and +category labels are protected one of two ways: + +- **No key (default): label-free.** Each categorical column stores only its + frequency profile in descending order (`r:0000`, `r:0001`, …). Categorical + PSI still catches shape and cardinality drift, but it cannot see two + categories swapping shares. +- **`label_key=` or `FRESHDATA_BASELINE_KEY`: keyed.** Labels are + HMAC-SHA256 pseudonyms and only a short key identifier is stored. Compare + with the same key; a missing or different key skips categorical PSI with a + `drift.categorical_drift_skipped` warning. Anyone holding the key can confirm + guessed labels, so keep it out of the repository that holds the baseline. + +Baselines built inside one call (`compare_to_baseline(df, other_df)`, the +inline baseline in `clean_enterprise`) use a random key that is never stored. +**Residual risk:** exact category shares are still visible, and +`freshdata-baseline-v1` files (unkeyed SHA-1 labels, reversible by hashing a +guess list) still load with a warning; rebuild and delete them. + ## Non-goals - **Not a sandbox.** FreshData reads tabular files; hostile *file formats* diff --git a/src/freshdata/enterprise/contracts.py b/src/freshdata/enterprise/contracts.py index bd6742c8..979e9496 100644 --- a/src/freshdata/enterprise/contracts.py +++ b/src/freshdata/enterprise/contracts.py @@ -15,9 +15,22 @@ :func:`freshdata.enterprise.metrics.compute_trust_score`. Baselines are persisted as stable, readable JSON tagged with -``"schema_version": "freshdata-baseline-v1"``. By design they never store raw -sample values unless ``include_samples=True`` is passed explicitly, so a -baseline cannot leak PII. +``"schema_version": "freshdata-baseline-v2"``. They never store raw sample +values unless ``include_samples=True`` is passed explicitly. Category labels +are protected in one of two ways: + +* **label-free (default)** — without a key, a categorical column stores only + its descending frequency profile (``"r:0000"``, ``"r:0001"``, …). Categorical + PSI still catches shape and cardinality drift, but a baseline carries no + label identifiers at all. +* **keyed** — with ``label_key=`` (or the ``FRESHDATA_BASELINE_KEY`` + environment variable) labels are ``HMAC-SHA256`` pseudonyms. They cannot be + confirmed by hashing guesses without the key, and the key itself is never + stored (only a short key identifier). Pass the same key to + :func:`compare_to_baseline`. + +Version-1 baselines (unkeyed SHA-1 labels, reversible by dictionary) still load, +with a warning; rebuild them. >>> import freshdata as fd >>> base = fd.build_baseline(trusted_df, name="customers") @@ -30,9 +43,12 @@ import difflib import hashlib +import hmac import json import math +import os import re +import secrets import warnings from dataclasses import dataclass, field from datetime import datetime, timezone @@ -57,7 +73,19 @@ except Exception: # pragma: no cover - defensive FRESHDATA_VERSION = "unknown" -SCHEMA_VERSION = "freshdata-baseline-v1" +SCHEMA_VERSION = "freshdata-baseline-v2" +#: Schema written before keyed / label-free category labels. Still readable. +LEGACY_SCHEMA_VERSION = "freshdata-baseline-v1" +_SUPPORTED_SCHEMA_VERSIONS = (SCHEMA_VERSION, LEGACY_SCHEMA_VERSION) + +#: Environment variable consulted when ``label_key`` is not passed. +BASELINE_KEY_ENV = "FRESHDATA_BASELINE_KEY" +_LABEL_DOMAIN = b"freshdata-baseline-label-v2\x00" +_KEY_ID_DOMAIN = b"freshdata-baseline-key-id" +#: ``ColumnBaseline.metadata["label_mode"]`` values. +_LABEL_RAW = "raw" +_LABEL_RANK = "rank" +_LABEL_HMAC = "hmac-sha256" _Level = Literal["info", "warning", "error"] _Status = Literal["passed", "warned", "failed"] @@ -108,9 +136,53 @@ def _full_precision(value: float | None) -> float | None: return v -def _hash_label(value: str) -> str: - """Stable, non-reversible category label for PII-safe baselines.""" - return "h:" + hashlib.sha1(value.encode("utf-8")).hexdigest()[:16] +def _legacy_sha1_label(value: str) -> str: + """Category label of a v1 baseline: unkeyed, truncated SHA-1. + + Kept only to compare against baselines written before schema v2. These + labels are reversible by hashing a dictionary of guesses; never write them. + """ + return "h:" + hashlib.sha1(value.encode("utf-8")).hexdigest()[:16] # noqa: S324 + + +def _resolve_label_key(label_key: str | bytes | None) -> bytes | None: + """The effective label key: *label_key*, else ``$FRESHDATA_BASELINE_KEY``, else None.""" + if label_key is None: + env = os.environ.get(BASELINE_KEY_ENV) + return env.encode("utf-8") if env else None + key = label_key.encode("utf-8") if isinstance(label_key, str) else bytes(label_key) + if not key: + raise ValueError("label_key must be a non-empty str or bytes") + return key + + +def _keyed_label(key: bytes, value: str) -> str: + """Pseudonymous category label: ``k:`` + HMAC-SHA256 (128 bits, hex).""" + digest = hmac.new(key, _LABEL_DOMAIN + value.encode("utf-8"), hashlib.sha256) + return "k:" + digest.hexdigest()[:32] + + +def _label_key_id(key: bytes) -> str: + """Non-secret identifier of a label key, used to detect a mismatched key.""" + return hmac.new(key, _KEY_ID_DOMAIN, hashlib.sha256).hexdigest()[:16] + + +def _rank_label(rank: int) -> str: + return f"r:{rank:04d}" + + +def _label_mode(cb: ColumnBaseline) -> str | None: + """How *cb*'s category labels are encoded (``None`` = not a categorical profile). + + Columns without a ``label_mode`` predate schema v2: hashed ones use the + legacy SHA-1 labels, the rest are raw. + """ + mode = cb.metadata.get("label_mode") + if mode is not None: + return str(mode) + if "labels_hashed" not in cb.metadata: + return None + return "sha1-legacy" if cb.metadata.get("labels_hashed") else _LABEL_RAW def _normalize_dtype(dtype: str | None) -> str | None: @@ -413,8 +485,11 @@ class DatasetBaseline: metadata: dict[str, Any] = field(default_factory=dict) def to_dict(self) -> dict[str, Any]: + # A baseline loaded from v1 keeps its SHA-1 labels; keep tagging it v1 + # so every later load still warns that it should be rebuilt. + legacy = any(_label_mode(cb) == "sha1-legacy" for cb in self.columns.values()) return { - "schema_version": SCHEMA_VERSION, + "schema_version": LEGACY_SCHEMA_VERSION if legacy else SCHEMA_VERSION, "name": self.name, "version": self.version, "created_at": self.created_at, @@ -430,9 +505,19 @@ def to_dict(self) -> dict[str, Any]: @classmethod def from_dict(cls, d: dict[str, Any]) -> DatasetBaseline: got = d.get("schema_version") - if got != SCHEMA_VERSION: + if got not in _SUPPORTED_SCHEMA_VERSIONS: raise ValueError( - f"unsupported baseline schema_version {got!r}; expected {SCHEMA_VERSION!r}" + f"unsupported baseline schema_version {got!r}; expected one of " + f"{_SUPPORTED_SCHEMA_VERSIONS!r}" + ) + if got == LEGACY_SCHEMA_VERSION: + warnings.warn( + f"baseline {d.get('name')!r} uses schema {LEGACY_SCHEMA_VERSION!r}: its " + "category labels are unkeyed SHA-1 hashes, which are reversible by " + "dictionary; rebuild the baseline with build_baseline() (optionally " + "with label_key=) and delete the old file", + UserWarning, + stacklevel=2, ) contract = d.get("contract") return cls( @@ -752,7 +837,13 @@ def _resolve_label(frame: pd.DataFrame, name: Any) -> Any | None: return matches[0] if matches else None -def _profile_column(series: pd.Series, *, n_rows: int, include_samples: bool) -> ColumnBaseline: +def _profile_column( + series: pd.Series, + *, + n_rows: int, + include_samples: bool, + label_key: bytes | None = None, +) -> ColumnBaseline: name = str(series.name) dtype = str(series.dtype) n_missing = int(series.isna().sum()) @@ -794,16 +885,27 @@ def _profile_column(series: pd.Series, *, n_rows: int, include_samples: bool) -> counts = non_null.astype("string").value_counts() top = counts.head(_MAX_TOP_CATEGORIES) total = int(counts.sum()) - # Category labels can themselves be PII; hash them unless the caller - # opted into raw samples for trusted, non-sensitive reference data. + # Category labels can themselves be PII. Unless the caller opted into + # raw samples for trusted, non-sensitive reference data, store keyed + # HMAC pseudonyms (with a key) or no labels at all (without one). cb.metadata["labels_hashed"] = not include_samples - - def _label(v: Any) -> str: - s = str(v) - return s if include_samples else _hash_label(s) - - cb.top_values = tuple(_label(v) for v in top.index) - cb.frequencies = {_label(k): float(v) / total for k, v in top.items()} if total else {} + if include_samples: + cb.metadata["label_mode"] = _LABEL_RAW + cb.top_values = tuple(str(v) for v in top.index) + cb.frequencies = {str(k): float(v) / total for k, v in top.items()} if total else {} + elif label_key is not None: + cb.metadata["label_mode"] = _LABEL_HMAC + cb.metadata["label_key_id"] = _label_key_id(label_key) + cb.top_values = tuple(_keyed_label(label_key, str(v)) for v in top.index) + cb.frequencies = ( + {_keyed_label(label_key, str(k)): float(v) / total for k, v in top.items()} + if total + else {} + ) + else: + cb.metadata["label_mode"] = _LABEL_RANK + shares = sorted((float(v) / total for v in top.to_numpy()), reverse=True) + cb.frequencies = {_rank_label(i): s for i, s in enumerate(shares)} if total else {} return cb @@ -816,20 +918,35 @@ def build_baseline( trust_score: float | None = None, metadata: dict[str, Any] | None = None, include_samples: bool = False, + label_key: str | bytes | None = None, ) -> DatasetBaseline: """Profile *df* (pandas or polars) into a persistable :class:`DatasetBaseline`. The input frame is never modified. ``include_samples`` defaults to ``False`` so raw values (potential PII) are *not* stored; set it only for trusted, - non-sensitive reference data. + non-sensitive reference data (category labels are then stored raw too). + + Category labels without ``include_samples``: + + * ``label_key`` (or ``$FRESHDATA_BASELINE_KEY``) set — labels are + pseudonymous ``HMAC-SHA256`` values (``label_mode="hmac-sha256"``); only a + short key identifier is stored. Pass the same key to + :func:`compare_to_baseline` for label-aware categorical drift. + * no key — the baseline is label-free (``label_mode="rank"``): each + categorical column stores its frequency profile in descending order and + no labels. Drift detection keeps shape and cardinality changes but cannot + tell that one category swapped places with another of equal share. + + Keep the key secret: anyone holding it can confirm guessed labels. """ frame = to_pandas(df) _require_unique_labels(frame, "build_baseline") + key = None if include_samples else _resolve_label_key(label_key) n_rows = len(frame) columns: dict[str, ColumnBaseline] = {} for col in frame.columns: columns[str(col)] = _profile_column( - frame[col], n_rows=n_rows, include_samples=include_samples + frame[col], n_rows=n_rows, include_samples=include_samples, label_key=key ) return DatasetBaseline( name=name, @@ -967,8 +1084,15 @@ def _psi_numeric(cb: ColumnBaseline, current: pd.Series) -> float | None: return _psi(expected, actual) -def _psi_categorical(cb: ColumnBaseline, current: pd.Series) -> float | None: - """PSI over the baseline top-k categories plus an ``__OTHER__`` bucket.""" +def _psi_categorical( + cb: ColumnBaseline, current: pd.Series, label_key: bytes | None = None +) -> float | None: + """PSI over the baseline top-k categories plus an ``__OTHER__`` bucket. + + A label-free (``rank``) baseline is compared by rank: its descending + top-k shares against the current column's descending top-k shares. A keyed + baseline needs the matching *label_key* (the caller checks it). + """ if not cb.frequencies: return None cats = list(cb.frequencies.keys()) @@ -976,11 +1100,26 @@ def _psi_categorical(cb: ColumnBaseline, current: pd.Series) -> float | None: n = len(cur) if n == 0: return None - if cb.metadata.get("labels_hashed"): - cur = cur.map(lambda v: _hash_label(str(v))) - cur_counts = cur.value_counts(normalize=True) + mode = _label_mode(cb) expected = np.array([cb.frequencies[c] for c in cats] + [0.0], dtype=float) expected[-1] = max(0.0, 1.0 - float(np.sum(expected[:-1]))) + if mode == _LABEL_RANK: + shares = cur.value_counts(normalize=True).to_numpy(dtype=float) + shares = np.sort(shares)[::-1][: len(cats)] + actual_vals = [float(v) for v in shares] + [0.0] * (len(cats) - len(shares)) + actual = np.array(actual_vals + [max(0.0, 1.0 - sum(actual_vals))], dtype=float) + s = expected.sum() + if s: + expected = expected / s + return _psi(expected, actual) + if mode == "sha1-legacy": + cur = cur.map(lambda v: _legacy_sha1_label(str(v))) + elif mode == _LABEL_HMAC: + if label_key is None: + return None + key = label_key + cur = cur.map(lambda v: _keyed_label(key, str(v))) + cur_counts = cur.value_counts(normalize=True) actual_vals = [float(cur_counts.get(c, 0.0)) for c in cats] actual = np.array(actual_vals + [max(0.0, 1.0 - sum(actual_vals))], dtype=float) s = expected.sum() @@ -1191,8 +1330,10 @@ def _check_distribution( current: dict[str, ColumnBaseline], frame: pd.DataFrame, cfg: DriftConfig, + label_key: bytes | None = None, ) -> dict[str, Any]: drift: dict[str, Any] = {} + key_id = _label_key_id(label_key) if label_key is not None else None for col, base in baseline.columns.items(): label = _resolve_label(frame, col) if col in current else None if label is None: @@ -1215,7 +1356,30 @@ def _check_distribution( col_drift["range"] = {"baseline": [base.min, base.max]} elif base.kind == "categorical" and cur.kind == "categorical": if cur.cardinality <= cfg.max_categories_for_categorical_drift: - psi = _psi_categorical(base, series) + if _label_mode(base) == _LABEL_HMAC and ( + key_id is None or key_id != base.metadata.get("label_key_id") + ): + missing = key_id is None + reason = "no label_key was given" if missing else "label_key differs" + code = "missing_label_key" if missing else "label_key_mismatch" + _add( + findings, + "drift.categorical_drift_skipped", + level="warning", + status="warned", + message=( + f"categorical drift not checked: the baseline labels are keyed " + f"and {reason}; pass the key used by build_baseline " + f"(label_key= or ${BASELINE_KEY_ENV})" + ), + column=col, + metric="psi", + details={"reason": code}, + ) + if col_drift: + drift[col] = col_drift + continue + psi = _psi_categorical(base, series, label_key) if psi is not None: col_drift["psi"] = _round(psi) _grade_metric(findings, col, "psi", psi, cfg.psi_warn, cfg.psi_fail, cfg) @@ -1851,9 +2015,16 @@ def compare_to_baseline( trust_score: float | None = None, key: str | list[str] | None = None, event_time: str | None = None, + label_key: str | bytes | None = None, ) -> DriftReport: """Compare *df* against *baseline*; return a :class:`DriftReport`. + ``label_key`` (default ``$FRESHDATA_BASELINE_KEY``) must be the key the + baseline was built with when its category labels are keyed. A missing or + different key does not raise: categorical PSI for those columns is skipped + and a ``drift.categorical_drift_skipped`` warning is reported instead. + Label-free and raw-label baselines need no key. + Read-only: *df* is never mutated. ``contract`` overrides any contract stored in the baseline. ``trust_score`` overrides the computed Data Trust Score for the gate (useful to feed a score already computed elsewhere). @@ -1873,7 +2044,12 @@ def compare_to_baseline( if not isinstance(baseline, DatasetBaseline): baseline_frame = to_pandas(baseline) _require_unique_labels(baseline_frame, "compare_to_baseline") - baseline = build_baseline(baseline_frame, name="baseline") + # Built and compared in this call only: a random key keeps categorical + # drift label-aware without a reusable label hash ever existing. + compare_key: bytes | None = secrets.token_bytes(32) + baseline = build_baseline(baseline_frame, name="baseline", label_key=compare_key) + else: + compare_key = _resolve_label_key(label_key) key_changes = None if key is not None and baseline_frame is not None: @@ -1891,7 +2067,7 @@ def compare_to_baseline( if cfg.enabled: _check_schema(findings, baseline, current, current_order, cfg) _check_statistics(findings, baseline, current, len(frame), cfg) - distribution = _check_distribution(findings, baseline, current, frame, cfg) + distribution = _check_distribution(findings, baseline, current, frame, cfg, compare_key) active_contract = contract or baseline.contract if active_contract is not None: @@ -2034,12 +2210,13 @@ def monitor_contract( drift_config: DriftConfig | None = None, trust_score: float | None = None, return_report: bool = True, + label_key: str | bytes | None = None, ) -> DriftReport | bool: """Convenience monitor: load a baseline and compare *df* against it. Provide either ``baseline_path`` or an in-memory ``baseline``. Returns the full :class:`DriftReport` when ``return_report`` is true, else a pass/fail - boolean. + boolean. ``label_key`` is forwarded to :func:`compare_to_baseline`. """ if baseline is None: if baseline_path is None: @@ -2051,6 +2228,7 @@ def monitor_contract( contract=contract, drift_config=drift_config, trust_score=trust_score, + label_key=label_key, ) return report if return_report else report.passed diff --git a/src/freshdata/enterprise/interface.py b/src/freshdata/enterprise/interface.py index 762ec441..97c3af43 100644 --- a/src/freshdata/enterprise/interface.py +++ b/src/freshdata/enterprise/interface.py @@ -13,6 +13,7 @@ from __future__ import annotations import json +import secrets from dataclasses import dataclass, field from typing import Any @@ -211,6 +212,35 @@ def _gate_and_fold_profile( return resolved, gate, fold_profile_options(resolved, dict(clean_options), gate) +def _drift_against_baseline( + work: Any, + ec: EnterpriseConfig, + baseline: DatasetBaseline | None, + contract: DataContract | None, + trust_score: float, +) -> DriftReport: + """Compare *work* to *baseline*, or to an inline baseline of itself. + + An inline baseline lives only for this call, so it is keyed with a random, + never-stored key: category labels stay comparable but are never reversible. + A caller-supplied baseline takes its key from ``$FRESHDATA_BASELINE_KEY``. + """ + inline_key = secrets.token_bytes(32) if baseline is None else None + base = ( + baseline + if baseline is not None + else build_baseline(work, name="_inline", label_key=inline_key) + ) + return compare_to_baseline( + work, + base, + contract=contract, + drift_config=ec.drift, + trust_score=trust_score, + label_key=inline_key, + ) + + def _resolve_enterprise_config(enterprise: EnterpriseConfig | None) -> EnterpriseConfig: """Return the effective config, failing closed on ``anonymization`` (#247). @@ -357,14 +387,7 @@ def track(rule: str, before: Any, after: Any, count: int, description: str) -> N drift_report: DriftReport | None = None if ec.enable_contracts and (baseline is not None or contract is not None): - base = baseline if baseline is not None else build_baseline(work, name="_inline") - drift_report = compare_to_baseline( - work, - base, - contract=contract, - drift_config=ec.drift, - trust_score=trust_after.overall, - ) + drift_report = _drift_against_baseline(work, ec, baseline, contract, trust_after.overall) quality = QualityReport( trust_before=trust_before, trust_after=trust_after, diff --git a/tests/test_baseline_label_privacy.py b/tests/test_baseline_label_privacy.py new file mode 100644 index 00000000..e31970c2 --- /dev/null +++ b/tests/test_baseline_label_privacy.py @@ -0,0 +1,289 @@ +"""Baseline category labels must not be recoverable without a secret.""" + +from __future__ import annotations + +import hashlib +import json +import re +import warnings + +import pandas as pd +import pytest + +import freshdata as fd +from freshdata.enterprise import EnterpriseConfig, clean_enterprise +from freshdata.enterprise import interface as enterprise_interface +from freshdata.enterprise.contracts import ( + BASELINE_KEY_ENV, + ColumnContract, + DataContract, + DatasetBaseline, + build_baseline, + compare_to_baseline, + load_baseline, + save_baseline, +) + +GUESSES = ["HIV", "diabetes", "depression", "asthma", "cancer"] + + +@pytest.fixture(autouse=True) +def _no_ambient_key(monkeypatch): + monkeypatch.delenv(BASELINE_KEY_ENV, raising=False) + + +def _sha1_label(value: str) -> str: + return "h:" + hashlib.sha1(value.encode()).hexdigest()[:16] + + +def _abc(a: int, b: int, c: int) -> pd.DataFrame: + return pd.DataFrame({"cat": ["a"] * a + ["b"] * b + ["c"] * c}) + + +def _psi_findings(report, column="cat"): + return [f for f in report.findings if f.check_id == "drift.psi" and f.column == column] + + +def _skipped(report): + return [f for f in report.findings if f.check_id == "drift.categorical_drift_skipped"] + + +def test_poc_dictionary_attack_recovers_nothing(tmp_path): + df = pd.DataFrame({"diagnosis": ["HIV", "HIV", "diabetes", "depression"] * 10}) + path = tmp_path / "b.json" + save_baseline(fd.build_baseline(df, name="b"), path) + raw = path.read_text(encoding="utf-8") + stored = json.loads(raw) + freqs = stored["columns"]["diagnosis"]["frequencies"] + + assert {g: freqs[_sha1_label(g)] for g in GUESSES if _sha1_label(g) in freqs} == {} + for guess in GUESSES: + assert guess not in raw + assert hashlib.sha1(guess.encode()).hexdigest()[:16] not in raw + assert "h:" not in raw + assert stored["schema_version"] == "freshdata-baseline-v2" + col = stored["columns"]["diagnosis"] + assert col["metadata"]["label_mode"] == "rank" + assert col["top_values"] == [] + assert freqs == {"r:0000": 0.5, "r:0001": 0.25, "r:0002": 0.25} + + +def test_rank_mode_identical_data_has_no_drift(): + base = build_baseline(_abc(60, 30, 10), name="b") + report = compare_to_baseline(_abc(60, 30, 10), base) + assert report.distribution_drift["cat"]["psi"] == pytest.approx(0.0, abs=1e-6) + assert not _psi_findings(report) + assert not _skipped(report) + assert report.passed + + +def test_rank_mode_flags_a_shape_shift(): + base = build_baseline(_abc(60, 30, 10), name="b") + report = compare_to_baseline(_abc(34, 33, 33), base) + assert _psi_findings(report) + + +def test_rank_mode_cannot_see_a_label_swap(): + # Documented trade-off of the label-free profile. + base = build_baseline(_abc(60, 30, 10), name="b") + assert not _psi_findings(compare_to_baseline(_abc(10, 30, 60), base)) + + +def test_rank_mode_new_categories_count_against_the_profile(): + base = build_baseline(_abc(50, 50, 0), name="b") + current = pd.DataFrame({"cat": [f"v{i % 10}" for i in range(100)]}) + assert _psi_findings(compare_to_baseline(current, base)) + + +def test_keyed_mode_is_label_aware_with_the_same_key(): + base = build_baseline(_abc(60, 30, 10), name="b", label_key="s3cret") + assert not _psi_findings(compare_to_baseline(_abc(60, 30, 10), base, label_key="s3cret")) + swapped = compare_to_baseline(_abc(10, 30, 60), base, label_key="s3cret") + assert _psi_findings(swapped) + assert not _skipped(swapped) + + +def test_keyed_labels_are_hmac_and_key_is_not_stored(tmp_path): + key = "correct horse battery staple" + base = build_baseline(_abc(60, 30, 10), name="b", label_key=key) + col = base.columns["cat"] + assert col.metadata["label_mode"] == "hmac-sha256" + assert col.metadata["labels_hashed"] is True + assert all(re.fullmatch(r"k:[0-9a-f]{32}", v) for v in col.top_values) + assert set(col.frequencies) == set(col.top_values) + + path = tmp_path / "b.json" + save_baseline(base, path) + raw = path.read_text(encoding="utf-8") + assert key not in raw + assert key.encode().hex() not in raw + for value in ("a", "b", "c"): + assert _sha1_label(value) not in raw + + +def test_label_key_id_is_stable_and_key_specific(): + ids = { + build_baseline(_abc(5, 3, 1), name="x", label_key="k1") + .columns["cat"] + .metadata["label_key_id"], + build_baseline(_abc(9, 1, 1), name="y", label_key=b"k1") + .columns["cat"] + .metadata["label_key_id"], + } + assert len(ids) == 1 + other = build_baseline(_abc(5, 3, 1), name="x", label_key="k2") + assert other.columns["cat"].metadata["label_key_id"] not in ids + + +@pytest.mark.parametrize( + ("compare_key", "reason"), + [(None, "missing_label_key"), ("wrong", "label_key_mismatch")], +) +def test_keyed_baseline_without_the_key_skips_categorical_psi(compare_key, reason): + base = build_baseline(_abc(60, 30, 10), name="b", label_key="right") + report = compare_to_baseline(_abc(10, 30, 60), base, label_key=compare_key) + skipped = _skipped(report) + assert len(skipped) == 1 + assert skipped[0].column == "cat" + assert skipped[0].level == "warning" + assert skipped[0].details["reason"] == reason + assert not _psi_findings(report) + assert "psi" not in report.distribution_drift.get("cat", {}) + assert report.passed # a warning, not an error + + +def test_label_key_from_environment(monkeypatch): + monkeypatch.setenv(BASELINE_KEY_ENV, "env-key") + base = build_baseline(_abc(60, 30, 10), name="b") + assert base.columns["cat"].metadata["label_mode"] == "hmac-sha256" + assert _psi_findings(compare_to_baseline(_abc(10, 30, 60), base)) + monkeypatch.delenv(BASELINE_KEY_ENV) + assert _skipped(compare_to_baseline(_abc(10, 30, 60), base)) + + +def test_empty_label_key_is_rejected(): + with pytest.raises(ValueError, match="label_key"): + build_baseline(_abc(1, 1, 1), name="b", label_key="") + + +def _v1_literal() -> dict: + freqs = {_sha1_label("a"): 0.6, _sha1_label("b"): 0.3, _sha1_label("c"): 0.1} + return { + "schema_version": "freshdata-baseline-v1", + "name": "legacy", + "version": "1.0.0", + "created_at": "2026-01-01T00:00:00+00:00", + "freshdata_version": "2.0.0", + "row_count": 100, + "column_order": ["cat"], + "columns": { + "cat": { + "name": "cat", + "dtype": "object", + "missing_ratio": 0.0, + "cardinality": 3, + "n_unique": 3, + "n_rows": 100, + "sample_values": [], + "top_values": list(freqs), + "frequencies": freqs, + "metadata": {"labels_hashed": True}, + } + }, + "contract": None, + "trust_score": None, + "metadata": {}, + } + + +def test_v1_baseline_loads_with_a_warning_and_compares_as_before(tmp_path): + path = tmp_path / "legacy.json" + path.write_text(json.dumps(_v1_literal()), encoding="utf-8") + with pytest.warns(UserWarning, match="reversible by dictionary"): + base = load_baseline(path) + + assert not _psi_findings(compare_to_baseline(_abc(60, 30, 10), base)) + # The legacy comparison is label-aware, as it was. + assert _psi_findings(compare_to_baseline(_abc(10, 30, 60), base)) + # Re-saving keeps the v1 tag, so the next load warns again. + assert base.to_dict()["schema_version"] == "freshdata-baseline-v1" + with pytest.warns(UserWarning, match="rebuild the baseline"): + DatasetBaseline.from_dict(base.to_dict()) + + +def test_unknown_schema_version_still_raises(): + literal = _v1_literal() + literal["schema_version"] = "freshdata-baseline-v9" + with pytest.raises(ValueError, match="unsupported baseline schema_version"): + DatasetBaseline.from_dict(literal) + + +@pytest.mark.parametrize("label_key", [None, "k"]) +def test_v2_round_trip_without_warning(tmp_path, label_key): + df = _abc(60, 30, 10) + base = build_baseline(df, name="b", label_key=label_key) + path = tmp_path / "b.json" + save_baseline(base, path) + with warnings.catch_warnings(): + warnings.simplefilter("error") + loaded = load_baseline(path) + assert loaded.to_dict()["columns"] == base.to_dict()["columns"] + report = compare_to_baseline(df, loaded, label_key=label_key) + assert report.passed + assert not _psi_findings(report) + assert not _skipped(report) + + +def test_include_samples_keeps_raw_labels(): + base = build_baseline(_abc(60, 30, 10), name="b", include_samples=True, label_key="k") + col = base.columns["cat"] + assert col.metadata["label_mode"] == "raw" + assert col.metadata["labels_hashed"] is False + assert "label_key_id" not in col.metadata + assert col.top_values == ("a", "b", "c") + assert col.frequencies == {"a": 0.6, "b": 0.3, "c": 0.1} + assert _psi_findings(compare_to_baseline(_abc(10, 30, 60), base)) + + +def test_clean_enterprise_inline_baseline_uses_an_ephemeral_key(monkeypatch): + seen = [] + real = enterprise_interface.build_baseline + + def spy(*args, **kwargs): + seen.append(kwargs.get("label_key")) + return real(*args, **kwargs) + + monkeypatch.setattr(enterprise_interface, "build_baseline", spy) + df = pd.DataFrame({"cat": ["a"] * 60 + ["b"] * 30 + ["c"] * 10, "n": range(100)}) + contract = DataContract(name="c", columns=(ColumnContract(name="cat"),)) + ec = EnterpriseConfig(enable_contracts=True) + + res = clean_enterprise(df, enterprise=ec, contract=contract) + res2 = clean_enterprise(df, enterprise=ec, contract=contract) + + assert len(seen) == 2 + assert all(isinstance(k, bytes) and len(k) == 32 for k in seen) + assert seen[0] != seen[1] + assert res.drift_report is not None + assert not _skipped(res.drift_report) + assert "psi" in res.drift_report.distribution_drift["cat"] + assert res2.drift_report is not None and not _skipped(res2.drift_report) + + +@pytest.mark.parametrize("label_key", [None, "k"]) +def test_bool_and_nullable_string_columns(label_key): + df = pd.DataFrame( + { + "flag": pd.Series([True, False, True] * 40), + "name": pd.Series(["x", None, "y"] * 40, dtype="string"), + } + ) + base = build_baseline(df, name="b", label_key=label_key) + for col in ("flag", "name"): + expected = "rank" if label_key is None else "hmac-sha256" + assert base.columns[col].metadata["label_mode"] == expected + report = compare_to_baseline(df, base, label_key=label_key) + assert report.passed + assert not _skipped(report) + for col in ("flag", "name"): + assert report.distribution_drift[col]["psi"] == pytest.approx(0.0, abs=1e-6) diff --git a/tests/test_enterprise_contracts.py b/tests/test_enterprise_contracts.py index c943f8c8..e021d683 100644 --- a/tests/test_enterprise_contracts.py +++ b/tests/test_enterprise_contracts.py @@ -51,7 +51,7 @@ def test_build_save_load_round_trip(trusted_df, tmp_path): assert loaded.columns["signup"].kind == "datetime" # schema version is embedded raw = path.read_text() - assert "freshdata-baseline-v1" in raw + assert "freshdata-baseline-v2" in raw def test_baseline_does_not_store_raw_samples_by_default(trusted_df, tmp_path): @@ -225,11 +225,16 @@ def test_numeric_ks_detects_shift(trusted_df): def test_psi_detects_categorical_drift(trusted_df): - base = build_baseline(trusted_df, name="c") + # Categories swap shares (US 60% -> 10%, FR 10% -> 70%): only a keyed, + # label-aware baseline can see that; a label-free one compares ranks. + base = build_baseline(trusted_df, name="c", label_key="k") df2 = trusted_df.copy() rng = np.random.default_rng(2) df2["country"] = rng.choice(["US", "GB", "FR"], len(df2), p=[0.1, 0.2, 0.7]) - report = compare_to_baseline(df2, base) + report = compare_to_baseline(df2, base, label_key="k") + assert any(f.metric == "psi" and f.column == "country" for f in report.findings) + # A raw-frame baseline is keyed in-process, so it stays label-aware too. + report = compare_to_baseline(df2, trusted_df) assert any(f.metric == "psi" and f.column == "country" for f in report.findings) From 092d233a16215a547e153844029faef67c7c0785 Mon Sep 17 00:00:00 2001 From: Kevin Costner <120246174+kevincostner17@users.noreply.github.com> Date: Tue, 15 Sep 2026 17:30:41 +0530 Subject: [PATCH 05/17] fix(privacy): create token vault files owner-only JsonTokenVault and SqliteTokenVault store the plaintext token-to-value mapping, but created their files with the process umask, so under the common 022 umask any local user could read them. - JsonTokenVault opens its file through os.open(O_RDWR | O_CREAT | O_APPEND, 0o600), so a new vault is owner-only from the moment it is created. The in-place locked rewrite design is unchanged. - SqliteTokenVault pre-creates the database file with mode 0600 before sqlite3.connect; SQLite gives journal, WAL and SHM files the same mode. - A missing parent directory is created with mode 0700. - An existing group/other-accessible vault file is used as is and triggers a UserWarning once per instance; its mode is never changed. - Docstrings and threat-model section 8 describe the file modes. --- docs/threat-model.md | 19 +++ src/freshdata/enterprise/privacy.py | 88 ++++++++++- tests/test_token_vault_permissions.py | 205 ++++++++++++++++++++++++++ 3 files changed, 308 insertions(+), 4 deletions(-) create mode 100644 tests/test_token_vault_permissions.py diff --git a/docs/threat-model.md b/docs/threat-model.md index b79dfaeb..e65424e8 100644 --- a/docs/threat-model.md +++ b/docs/threat-model.md @@ -118,6 +118,25 @@ process killed mid-run (`SIGKILL`, power loss) leaves its private run directory behind until it is deleted; root and the same user can always read it. +### 8. Token vault files + +`JsonTokenVault` and `SqliteTokenVault` store the plaintext token-to-value +mapping; so do `MaskingRule(token_vault_path=...)` and policy vaults with +`vault_backend: json` or `sqlite`. Anyone who can read a vault file, or a +backup of it, can reverse every token in it. + +On POSIX, FreshData creates vault files with mode **0600** (owner read/write +only) in the same `os.open` call that creates them, so a new vault is never +readable by other users, whatever the umask. SQLite gives its `-journal`, +`-wal` and `-shm` files the database file's mode. A missing parent directory +is created with mode 0700; only the last path component gets that mode, and +intermediate directories follow the umask. + +An existing vault file keeps its mode. If it is group- or other-accessible, +FreshData emits a `UserWarning` and still uses it; it never changes the mode +of a file it did not create. On Windows, access to vault files follows the +ACLs of the directory. Keep vaults out of shared directories either way. + ### 9. Persisted baselines and profiles Drift baselines (`fd.build_baseline` / `save_baseline`) are meant to be diff --git a/src/freshdata/enterprise/privacy.py b/src/freshdata/enterprise/privacy.py index 84dc740f..5bc545aa 100644 --- a/src/freshdata/enterprise/privacy.py +++ b/src/freshdata/enterprise/privacy.py @@ -632,12 +632,65 @@ def _locked_file(handle: IO[str], *, exclusive: bool) -> Iterator[None]: module.locking(fd, module.LK_UNLCK, 1) +#: Mode for files FreshData creates to hold a token→value mapping. +_PRIVATE_FILE_MODE = 0o600 +#: Mode for a missing vault parent directory (applies to the last component only). +_PRIVATE_DIR_MODE = 0o700 + + +def _open_private_text(path: Path) -> IO[str]: + """Open *path* for reading and appending, creating it owner-only if missing. + + ``os.open`` applies mode 0600 in the same call that creates the file, so a + new vault is never readable by other users, whatever the umask. An existing + file keeps its mode. The handle behaves like ``open(path, "a+")``: every + write appends, so a write after ``seek(0)`` + ``truncate()`` lands at offset 0. + """ + flags = ( + os.O_RDWR + | os.O_CREAT + | os.O_APPEND + | getattr(os, "O_CLOEXEC", 0) + | getattr(os, "O_BINARY", 0) + | getattr(os, "O_NOINHERIT", 0) + ) + fd = os.open(path, flags, _PRIVATE_FILE_MODE) + try: + return os.fdopen(fd, "a+", encoding="utf-8") + except BaseException: + os.close(fd) + raise + + +def _warn_if_shared_vault_file(fd: int, path: Path, *, stacklevel: int) -> None: + """Warn when an open vault file is group- or other-accessible (POSIX only). + + The mode is reported, never changed: the file may be shared on purpose. + """ + if os.name == "nt": + return + mode = os.fstat(fd).st_mode & 0o777 + if mode & 0o077: + warnings.warn( + f"token vault file {str(path)!r} is group/other-accessible ({mode:04o}); " + "it holds the token-to-value mapping, chmod 600 it", + UserWarning, + stacklevel=stacklevel, + ) + + class JsonTokenVault(TokenVault): """A token vault persisted to an explicit JSON file. The file holds the sensitive token→value mapping, so protect it like any secret. Nothing is written until :meth:`put` (or :meth:`save`) is called. + On POSIX the file is created with mode 0600 (owner read/write only), whatever + the umask, and a missing parent directory is created with mode 0700 (only the + last path component; intermediate directories follow the umask). An existing + file keeps its mode; if it is group- or other-accessible a ``UserWarning`` is + emitted once per instance. + Several instances (in one process or in several) may share the same path: writes are merged and serialised. Each write takes an exclusive lock on the file (``fcntl.flock`` on POSIX, ``msvcrt.locking`` on Windows), re-reads the @@ -657,9 +710,17 @@ def __init__(self, path: str | Path) -> None: self._map: dict[str, str] = {} self._stamp: tuple[int, int] | None = None self._lock = threading.RLock() + self._mode_checked = False if self.path.exists(): self._reload() + def _check_mode(self, fd: int) -> None: + # stacklevel 5: warn <- _warn_if_shared_vault_file <- _check_mode + # <- _reload/_write <- public method <- caller. + if not self._mode_checked: + self._mode_checked = True + _warn_if_shared_vault_file(fd, self.path, stacklevel=5) + @staticmethod def _parse(text: str) -> dict[str, str]: return json.loads(text) if text.strip() else {} @@ -680,6 +741,7 @@ def _reload(self) -> None: """Merge the file contents into the in-memory map, under a shared lock.""" try: with open(self.path, encoding="utf-8") as handle: # noqa: SIM117 + self._check_mode(handle.fileno()) with _locked_file(handle, exclusive=False): handle.seek(0) disk = self._parse(handle.read()) @@ -704,8 +766,9 @@ def save(self) -> None: def _write(self, updates: dict[str, str], *, always: bool) -> None: with self._lock: - self.path.parent.mkdir(parents=True, exist_ok=True) - with open(self.path, "a+", encoding="utf-8") as handle: # noqa: SIM117 + self.path.parent.mkdir(parents=True, exist_ok=True, mode=_PRIVATE_DIR_MODE) + with _open_private_text(self.path) as handle: # noqa: SIM117 + self._check_mode(handle.fileno()) with _locked_file(handle, exclusive=True): handle.seek(0) disk = self._parse(handle.read()) @@ -731,6 +794,13 @@ class SqliteTokenVault(TokenVault): example a vault created in the main thread and passed to a worker pool), and a lock serialises every call on it. Other processes sharing the file wait up to 30 seconds for SQLite's own database lock. + + On POSIX the database file is created with mode 0600 (owner read/write only) + before SQLite opens it, whatever the umask; SQLite gives its ``-journal``, + ``-wal`` and ``-shm`` files the database file's mode. A missing parent + directory is created with mode 0700 (only the last path component). An + existing file keeps its mode; if it is group- or other-accessible a + ``UserWarning`` is emitted. """ def __init__(self, path: str | Path) -> None: @@ -738,8 +808,18 @@ def __init__(self, path: str | Path) -> None: self._lock = threading.RLock() self.path = Path(path) - if str(self.path) != ":memory:": - self.path.parent.mkdir(parents=True, exist_ok=True) + if str(self.path) not in (":memory:", "") and not self.path.is_dir(): + self.path.parent.mkdir(parents=True, exist_ok=True, mode=_PRIVATE_DIR_MODE) + # O_RDONLY | O_CREAT creates the file owner-only but still opens an + # existing read-only vault, which SQLite can then open read-only. + fd = os.open( + self.path, os.O_RDONLY | os.O_CREAT | getattr(os, "O_CLOEXEC", 0), + _PRIVATE_FILE_MODE, + ) + try: + _warn_if_shared_vault_file(fd, self.path, stacklevel=3) + finally: + os.close(fd) self._conn = sqlite3.connect(str(self.path), check_same_thread=False, timeout=30.0) self._conn.execute( "CREATE TABLE IF NOT EXISTS tokens (token TEXT PRIMARY KEY, value TEXT NOT NULL)" diff --git a/tests/test_token_vault_permissions.py b/tests/test_token_vault_permissions.py new file mode 100644 index 00000000..87f877bb --- /dev/null +++ b/tests/test_token_vault_permissions.py @@ -0,0 +1,205 @@ +"""Token vault files are created owner-only. + +JsonTokenVault and SqliteTokenVault hold the plaintext token-to-value mapping, so +they must not be readable by other local users under a permissive umask. +""" + +from __future__ import annotations + +import json +import os +import stat +import warnings + +import pandas as pd +import pytest + +from freshdata.enterprise import ( + JsonTokenVault, + MaskingRule, + PrivacyPolicy, + PrivacyRule, + SqliteTokenVault, + anonymize, + apply_privacy_policy, + make_vault, +) + +pytestmark = pytest.mark.skipif(os.name == "nt", reason="POSIX file modes") + +KEY = "unit-test-key" +SSN = "123-45-6789" + + +@pytest.fixture +def umask(): + """Run with umask 022 (the common default); the test may change it. Restored after.""" + previous = os.umask(0o022) + try: + yield os.umask + finally: + os.umask(previous) + + +def _mode(path) -> int: + return stat.S_IMODE(os.stat(path).st_mode) + + +def _make(backend: str, path): + return JsonTokenVault(path) if backend == "json" else SqliteTokenVault(path) + + +def _close(vault) -> None: + close = getattr(vault, "close", None) + if close is not None: + close() + + +def test_poc_anonymize_json_vault_and_sqlite_vault_are_0600(tmp_path, umask): + jp, sp = tmp_path / "vault.json", tmp_path / "vault.db" + rule = MaskingRule( + name="t", columns=("ssn",), strategy="tokenize", reversible=True, key="k", + token_vault_path=str(jp), + ) + anonymize(pd.DataFrame({"ssn": [SSN]}), rules=(rule,)) + sv = SqliteTokenVault(sp) + sv.put("tok_1", SSN) + sv.close() + assert SSN in jp.read_text(encoding="utf-8") + assert {p.name: oct(_mode(p)) for p in (jp, sp)} == { + "vault.json": "0o600", "vault.db": "0o600", + } + + +@pytest.mark.parametrize("backend", ["json", "sqlite"]) +def test_make_vault_creates_owner_only_file(tmp_path, umask, backend): + path = tmp_path / f"vault.{backend}" + vault = make_vault(backend, path=path) + vault.put("tok_1", SSN) + _close(vault) + assert _mode(path) == 0o600 + + +@pytest.mark.parametrize("backend", ["json", "sqlite"]) +def test_policy_vault_path_creates_owner_only_file(tmp_path, umask, backend): + path = tmp_path / f"policy-vault.{backend}" + rule = PrivacyRule(id="ssn", action="tokenize", reversible=True, columns=("ssn",)) + policy = PrivacyPolicy( + name="p", rules=(rule,), key=KEY, vault_backend=backend, vault_path=str(path) + ) + out, report = apply_privacy_policy(pd.DataFrame({"ssn": [SSN]}), policy) + assert out["ssn"].iloc[0].startswith("tok_") + assert report.vault_info["backend"] == backend + assert _mode(path) == 0o600 + + +@pytest.mark.parametrize("backend", ["json", "sqlite"]) +def test_umask_000_still_gives_0600(tmp_path, umask, backend): + umask(0o000) + path = tmp_path / f"vault.{backend}" + vault = _make(backend, path) + vault.put("tok_1", SSN) + _close(vault) + assert _mode(path) == 0o600 + + +def test_json_rewrites_keep_mode_and_content(tmp_path, umask): + path = tmp_path / "vault.json" + vault = JsonTokenVault(path) + vault.put("tok_long", "x" * 200) + vault.put("tok_b", "b") + other = JsonTokenVault(path) + other.put("tok_c", "c") + vault.save() # rewrites in place: truncate, then write at offset 0 + assert json.loads(path.read_text(encoding="utf-8")) == { + "tok_long": "x" * 200, "tok_b": "b", "tok_c": "c", + } + assert _mode(path) == 0o600 + + +def test_sqlite_wal_and_shm_files_inherit_0600(tmp_path, umask): + path = tmp_path / "vault.db" + vault = SqliteTokenVault(path) + try: + vault._conn.execute("PRAGMA journal_mode=WAL") + vault.put("tok_1", SSN) + wal = tmp_path / "vault.db-wal" + assert wal.exists() + assert _mode(wal) == 0o600 + shm = tmp_path / "vault.db-shm" + if shm.exists(): + assert _mode(shm) == 0o600 + finally: + vault.close() + assert _mode(path) == 0o600 + + +@pytest.mark.parametrize("backend", ["json", "sqlite"]) +def test_new_parent_directory_is_0700(tmp_path, umask, backend): + parent = tmp_path / "private" + path = parent / f"vault.{backend}" + vault = _make(backend, path) + vault.put("tok_1", SSN) + _close(vault) + assert _mode(parent) == 0o700 + assert _mode(path) == 0o600 + + +@pytest.mark.parametrize("backend", ["json", "sqlite"]) +def test_fresh_vault_emits_no_warning(tmp_path, umask, backend): + path = tmp_path / f"vault.{backend}" + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + vault = _make(backend, path) + vault.put("tok_1", SSN) + _close(vault) + _close(_make(backend, path)) # reopening an owner-only file is silent too + assert [str(w.message) for w in caught] == [] + + +def test_existing_loose_json_vault_warns_once_and_is_left_unmodified(tmp_path, umask): + path = tmp_path / "vault.json" + path.write_text(json.dumps({"tok_old": "old"}), encoding="utf-8") + os.chmod(path, 0o644) + with pytest.warns(UserWarning, match=r"group/other-accessible \(0644\); .*chmod 600"): + vault = JsonTokenVault(path) + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + vault.put("tok_new", "new") + assert vault.get("tok_old") == "old" + assert caught == [] # one warning per instance + assert _mode(path) == 0o644 + assert json.loads(path.read_text(encoding="utf-8")) == {"tok_old": "old", "tok_new": "new"} + + +def test_existing_loose_json_vault_warns_on_first_write(tmp_path, umask): + path = tmp_path / "vault.json" + vault = JsonTokenVault(path) # file does not exist yet: nothing to check + path.write_text("", encoding="utf-8") + os.chmod(path, 0o640) + with pytest.warns(UserWarning, match=r"group/other-accessible \(0640\)"): + vault.put("tok_1", SSN) + assert _mode(path) == 0o640 + + +def test_existing_loose_sqlite_vault_warns_and_is_left_unmodified(tmp_path, umask): + path = tmp_path / "vault.db" + path.touch() + os.chmod(path, 0o644) + with pytest.warns(UserWarning, match=r"group/other-accessible \(0644\)"): + vault = SqliteTokenVault(path) + try: + vault.put("tok_1", SSN) + assert vault.get("tok_1") == SSN + finally: + vault.close() + assert _mode(path) == 0o644 + + +def test_sqlite_memory_vault_creates_no_file(tmp_path, umask, monkeypatch): + monkeypatch.chdir(tmp_path) + vault = SqliteTokenVault(":memory:") + vault.put("tok_1", SSN) + assert len(vault) == 1 + vault.close() + assert list(tmp_path.iterdir()) == [] From cd5427f2b87aa2c8c89761a703903b902cee8d8b Mon Sep 17 00:00:00 2001 From: Kevin Costner <120246174+kevincostner17@users.noreply.github.com> Date: Tue, 15 Sep 2026 18:30:50 +0530 Subject: [PATCH 06/17] fix(privacy): random per-call key for keyless tokenize, surrogate, fpe and pseudonymize Without a key, tokenize fell back to HMAC("freshdata-default-token-salt", rule name), and surrogate, keyless fpe and the policy pseudonymize action (the GDPR pack default) seeded their HMAC with "freshdata-surrogate". Both constants are in the public source, so anyone holding the output could recompute it for guessed values and recover SSNs, phone numbers and other low-entropy identifiers. - anonymize: a rule without a key (and not reversible) derives its key from a random secret generated once per call. Output is consistent within the call but differs across calls. One EphemeralKeyWarning names the rules, and report.metadata["ephemeral_key_rules"] lists them. Reversible keyless tokenize/fpe still raises. - apply_privacy_policy: keyless pseudonymize uses one random key per call, with the same warning and metadata. Keyless tokenize still raises. - _surrogate_value and _mask_one have no keyless path; the constants are gone. EphemeralKeyWarning is exported from freshdata.enterprise. - Crypto FPE honours visible: only the digits before the last visible characters are encrypted, and a head without digits falls back to the surrogate (#281, part 3). - Docs: MaskingRule strategies and keys, threat-model section 6, pack comments, and migration recipes in docs/compliance.md. Tests and the example that relied on keyless determinism now pass a key. --- docs/compliance.md | 44 +++ docs/threat-model.md | 26 +- examples/privacy_anonymization.py | 17 +- src/freshdata/compliance/packs/ferpa.yaml | 4 + src/freshdata/compliance/packs/gdpr.yaml | 4 + src/freshdata/compliance/packs/hipaa.yaml | 4 + src/freshdata/enterprise/__init__.py | 2 + src/freshdata/enterprise/config.py | 26 +- src/freshdata/enterprise/privacy.py | 90 ++++- src/freshdata/enterprise/privacy_policy.py | 43 ++- tests/test_enterprise_features_edge.py | 5 +- tests/test_enterprise_privacy.py | 3 +- tests/test_privacy_keyless_defaults.py | 378 ++++++++++++++++++++ tests/test_privacy_missing_and_labels.py | 7 +- tests/test_privacy_policy.py | 6 +- tests/test_privacy_policy_classification.py | 2 +- 16 files changed, 622 insertions(+), 39 deletions(-) create mode 100644 tests/test_privacy_keyless_defaults.py diff --git a/docs/compliance.md b/docs/compliance.md index 223cf7ba..af5aae9d 100644 --- a/docs/compliance.md +++ b/docs/compliance.md @@ -127,6 +127,50 @@ Each `FrameworkReport` exposes `framework_key`, `framework_name`, `passed` (bool `audit_entries` or the HIPAA identifier coverage), plus its own `to_dict()`, `to_json()`, and `to_frame()`. +## Pseudonymisation keys {#pseudonymisation-keys} + +The privacy policy engine (`freshdata.enterprise.apply_privacy_policy`) ships +HIPAA, FERPA, PCI and GDPR packs. Their `pseudonymize` rules (the GDPR pack's +default action, the HIPAA date-of-birth rule and the FERPA grade rule) are +keyed, as are the `tokenize`, `surrogate` and `fpe` strategies of +`MaskingRule` in `anonymize` and `clean_enterprise`. + +Pass a secret key for stable, joinable output, preferably from the environment: + +```python +from freshdata.enterprise import PrivacyPolicy, apply_privacy_policy, load_compliance_pack + +policy = PrivacyPolicy( + packs=(load_compliance_pack("gdpr"),), + jurisdiction="EU", + key_env="FRESHDATA_PSEUDONYM_KEY", +) +out, report = apply_privacy_policy(df, policy) +``` + +Without a key, each call uses a random key and emits `EphemeralKeyWarning` +(importable from `freshdata.enterprise`), and +`report.metadata["ephemeral_key_rules"]` lists the rules that used it. The +output cannot be recomputed from FreshData's source, but it changes on every +call, so it cannot be joined across runs. Policy `tokenize` without a key still +raises `ValueError`, as do `reversible=True` `tokenize` / `fpe` masking rules. + +### Reproducing output from before 2.1.0 + +Before 2.1.0 these paths used constants from the public source when no key was +set. Anyone with that output and a list of candidate values can recompute the +pseudonyms, so treat it as reversible and re-pseudonymise it with a secret key. +If you must reproduce the old output for a while (for example, to join against +an existing table during a migration), pass the old constant as an explicit +key. **These keys are public: never use them for new data.** + +| Before 2.1.0 (no key) | Explicit key that reproduces it | +| --- | --- | +| `MaskingRule(name=rule_name, strategy="tokenize")` | `key=hmac.new(b"freshdata-default-token-salt", rule_name.encode(), hashlib.sha256).hexdigest()[:32]` | +| `MaskingRule(strategy="surrogate")` | `key="freshdata-surrogate"` | +| `MaskingRule(strategy="fpe")` | `key="freshdata-surrogate"`, only when `pyffx` is not installed (with `pyffx`, a keyed `fpe` rule uses real FPE) | +| policy `pseudonymize` | `PrivacyPolicy(..., key="freshdata-surrogate")`, only when `pyffx` is not installed (a keyed `pseudonymize` uses FPE when `pyffx` is available) | + ## Errors {#errors} - `ValueError` — an unknown framework key was requested. diff --git a/docs/threat-model.md b/docs/threat-model.md index e65424e8..fe49f001 100644 --- a/docs/threat-model.md +++ b/docs/threat-model.md @@ -92,11 +92,27 @@ string-like columns regardless of detection (see boundary 3). ### 6. Masking tokens -Hash masking is HMAC-SHA256 with a configurable salt. **The default salt is -a public constant** in the source, so tokens are stable across runs and -joinable — and an attacker holding tokens can confirm guesses of -low-cardinality values. Supply your own `salt` on `MaskingRule` when -unlinkability matters. The copilot's internal masking uses the default +Hash masking is HMAC-SHA256 keyed by the rule's `salt`. When `salt` is +empty, `MaskingRule` generates a random salt per rule, so default hash +tokens are not stable across runs and cannot be recomputed from the source. +Set your own `salt` when you need stable, joinable tokens, and keep it +secret: anyone holding the salt and the tokens can confirm guesses of +low-cardinality values. + +`tokenize`, `surrogate` and `fpe` masking rules and the policy +`pseudonymize` action (the GDPR pack's default, also used by the HIPAA +date-of-birth and FERPA grade rules) are keyed by `key` / `key_env`. They +never fall back to a constant from the source. Without a key, each +`anonymize` / `apply_privacy_policy` call uses a random key and emits +`EphemeralKeyWarning`, and `report.metadata["ephemeral_key_rules"]` names +the rules. Output is then consistent within one call but not across calls. +Pass a secret key for stable, joinable pseudonyms; anyone holding that key +and the output can recompute the pseudonym of a guessed value. Before 2.1.0 +these keyless paths used public constants, so keyless output from earlier +releases can be reversed by enumerating candidate values: re-pseudonymise +it with a secret key (see [Compliance](compliance.md#pseudonymisation-keys)). + +The copilot's internal masking uses the default deterministic path on purpose: a per-run random salt would break the documented reproducibility of `model_context` and its audit fingerprint. This trade-off is tracked as a roadmap item, not silently changed. diff --git a/examples/privacy_anonymization.py b/examples/privacy_anonymization.py index 65a33d38..d0a8079c 100644 --- a/examples/privacy_anonymization.py +++ b/examples/privacy_anonymization.py @@ -7,6 +7,8 @@ python examples/privacy_anonymization.py """ +import os + import pandas as pd import freshdata as fd @@ -18,8 +20,16 @@ tokenize_value, ) +#: Keyed strategies read their secret from this environment variable. Without a +#: key they use a random per-call key, so output would change on every run. +KEY_ENV = "FRESHDATA_DEMO_KEY" + def main() -> None: + # A throwaway demo key when none is set (or it is empty); use a real secret + # in production. + if not os.environ.get(KEY_ENV): + os.environ[KEY_ENV] = "DEMO-KEY-DO-NOT-COMMIT" df = pd.DataFrame( { "patient_note": [ @@ -44,7 +54,7 @@ def main() -> None: columns=("email",), strategy="tokenize", reversible=True, - key="DEMO-KEY-DO-NOT-COMMIT", + key_env=KEY_ENV, entity_types=("EMAIL",), ), ) @@ -58,12 +68,13 @@ def main() -> None: # 3) Reversible tokenization round-trip via a vault. vault = InMemoryTokenVault() - token = tokenize_value("j.doe@mail.com", vault, "DEMO-KEY-DO-NOT-COMMIT") + token = tokenize_value("j.doe@mail.com", vault, os.environ[KEY_ENV]) print(f"token={token} -> detokenized={detokenize_value(token, vault)}\n") # 4) Surrogate format-preserving anonymization keeps the shape. fpe_rule = MaskingRule( - name="ssn_fpe", columns=("zip",), strategy="surrogate", preserve_format=True + name="ssn_fpe", columns=("zip",), strategy="surrogate", preserve_format=True, + key_env=KEY_ENV, ) masked, _ = fd.anonymize(df, rules=(fpe_rule,)) print("surrogate zip:", masked["zip"].tolist(), "(same length, not crypto FPE)\n") diff --git a/src/freshdata/compliance/packs/ferpa.yaml b/src/freshdata/compliance/packs/ferpa.yaml index a89c6485..2b4294a8 100644 --- a/src/freshdata/compliance/packs/ferpa.yaml +++ b/src/freshdata/compliance/packs/ferpa.yaml @@ -2,6 +2,10 @@ # # Protects personally identifiable information from a student's education # record: the student ID, parent/guardian contact, and grades/assessment. +# +# pseudonymize is keyed by the policy or rule key / key_env. Without a key each +# call uses a random key and emits EphemeralKeyWarning, so pseudonyms are not +# stable or joinable across runs. name: ferpa jurisdiction: US description: >- diff --git a/src/freshdata/compliance/packs/gdpr.yaml b/src/freshdata/compliance/packs/gdpr.yaml index 5f8c2ae1..09a7fe7c 100644 --- a/src/freshdata/compliance/packs/gdpr.yaml +++ b/src/freshdata/compliance/packs/gdpr.yaml @@ -4,6 +4,10 @@ # (Art. 5(1)(c)): columns that are personal data but not necessary for the # stated purpose are dropped when the policy enables minimisation. Special- # category data (Art. 9) is treated as high risk. +# +# pseudonymize (the default action here) is keyed by the policy or rule key / +# key_env. Without a key each call uses a random key and emits +# EphemeralKeyWarning, so pseudonyms are not stable or joinable across runs. name: gdpr jurisdiction: EU description: >- diff --git a/src/freshdata/compliance/packs/hipaa.yaml b/src/freshdata/compliance/packs/hipaa.yaml index 6b742c6b..c2877dc5 100644 --- a/src/freshdata/compliance/packs/hipaa.yaml +++ b/src/freshdata/compliance/packs/hipaa.yaml @@ -4,6 +4,10 @@ # privacy-policy rules. Jurisdiction is US; pseudonymisation/tokenisation are # permitted because HIPAA expressly contemplates re-identification by a covered # entity that retains the key separately. +# +# pseudonymize is keyed by the policy or rule key / key_env. Without a key each +# call uses a random key and emits EphemeralKeyWarning, so pseudonyms are not +# stable or joinable across runs. name: hipaa jurisdiction: US description: >- diff --git a/src/freshdata/enterprise/__init__.py b/src/freshdata/enterprise/__init__.py index 159d0c0a..355f3853 100644 --- a/src/freshdata/enterprise/__init__.py +++ b/src/freshdata/enterprise/__init__.py @@ -114,6 +114,7 @@ compute_trust_score, ) from .privacy import ( + EphemeralKeyWarning, InMemoryTokenVault, JsonTokenVault, KAnonymityReport, @@ -200,6 +201,7 @@ "SqliteTokenVault", "make_vault", "vault_metadata", + "EphemeralKeyWarning", # privacy policy engine "PrivacyPolicy", "PrivacyRule", diff --git a/src/freshdata/enterprise/config.py b/src/freshdata/enterprise/config.py index 84b111ba..fba3c49c 100644 --- a/src/freshdata/enterprise/config.py +++ b/src/freshdata/enterprise/config.py @@ -68,6 +68,27 @@ class MaskingRule: ``scrub_patterns`` plus any custom ``regexes``. ``drop`` Remove the column entirely. + ``tokenize`` + ``tok_`` plus an HMAC-SHA256 of the value keyed by ``key`` / ``key_env``, + recorded in a token vault (``token_vault_path`` for a JSON file). Equal + inputs map to equal tokens for a given key. + ``surrogate`` + Format-preserving pseudonym (digit count, letter case and separators + kept), keyed by ``key`` / ``key_env``. Not cryptographic FPE. + ``fpe`` + Format-preserving encryption of the digits with ``pyffx`` when it is + installed, else the ``surrogate`` fallback; keyed by ``key`` / + ``key_env``. With ``preserve_format=True`` both modes keep the last + ``visible`` characters, and decryption needs the same ``visible`` split. + + Keys + ---- + ``tokenize``, ``surrogate`` and ``fpe`` never fall back to a constant key. + Without ``key`` / ``key_env``, :func:`~freshdata.enterprise.anonymize` uses a + random key for that call and emits ``EphemeralKeyWarning``: output is + consistent within the call but not across calls. Set a secret key when you + need stable, joinable pseudonyms. ``reversible=True`` on ``tokenize`` / + ``fpe`` requires a key. """ name: str @@ -88,8 +109,9 @@ class MaskingRule: use_context: bool = True #: Opt-in reversibility (only meaningful for ``tokenize``/``fpe``). reversible: bool = False - #: Secret key material for ``tokenize``/``fpe``. Prefer ``key_env`` so the - #: literal never lives in source; raw keys are never written to reports. + #: Secret key material for ``tokenize``/``surrogate``/``fpe``. Prefer ``key_env`` + #: so the literal never lives in source; raw keys are never written to reports. + #: Without a key each ``anonymize`` call uses a random key (see "Keys" above). key: str | None = None #: Name of an environment variable holding the key (takes precedence). key_env: str | None = None diff --git a/src/freshdata/enterprise/privacy.py b/src/freshdata/enterprise/privacy.py index 5bc545aa..82edac22 100644 --- a/src/freshdata/enterprise/privacy.py +++ b/src/freshdata/enterprise/privacy.py @@ -34,6 +34,7 @@ import json import os import re +import secrets import threading import warnings from abc import ABC, abstractmethod @@ -55,6 +56,24 @@ _MAX_EVENTS = 1000 _PREVIEW_LEN = 24 +#: Strategies keyed by ``MaskingRule.key``; without one they use a per-call random key. +_KEYED_STRATEGIES = ("tokenize", "surrogate", "fpe") + + +class EphemeralKeyWarning(UserWarning): + """A keyed masking step ran without a key and used a random per-call key. + + Emitted once per :func:`anonymize` / ``apply_privacy_policy`` call that masks + with ``tokenize``, ``surrogate`` or ``fpe`` rules, or the ``pseudonymize`` + policy action, when no key is configured. The output is consistent within + that call but differs on every call; pass ``key=`` / ``key_env=`` for stable, + joinable pseudonyms. + """ + + +def _ephemeral_key(run_secret: bytes, label: str) -> str: + """Derive a per-rule key from a per-call random secret (never stored or reported).""" + return hmac.new(run_secret, label.encode("utf-8"), hashlib.sha256).hexdigest() def _is_missing_scalar(value: Any) -> bool: @@ -935,7 +954,7 @@ def detokenize_value(token: str, vault: TokenVault, key: str | None = None) -> s def _surrogate_value( value: Any, - key: str | None, + key: str, *, visible: int = 0, preserve_domain: bool = False, @@ -944,13 +963,16 @@ def _surrogate_value( Preserves digit count, alpha case pattern, and separators; optionally keeps the last ``visible`` characters and an email domain. Deterministic per - ``(key, value)`` so equal inputs map to equal surrogates. + ``(key, value)`` so equal inputs map to equal surrogates. ``key`` is required: + a constant fallback key would let anyone recompute surrogates of guessed values. """ + if not key: + raise ValueError("_surrogate_value requires a non-empty key") s = str(value) if preserve_domain and "@" in s: local, _, domain = s.partition("@") return _surrogate_value(local, key, visible=0) + "@" + domain - seed = (key or "freshdata-surrogate").encode("utf-8") + seed = key.encode("utf-8") digest = hmac.new(seed, s.encode("utf-8"), hashlib.sha256).digest() n = len(s) keep_from = n - visible if 0 < visible < n else n @@ -974,18 +996,26 @@ def _fpe_value(value: Any, key: str, *, visible: int = 0) -> tuple[str, str]: """Format-preserving encryption when ``pyffx`` is available, else surrogate. Returns ``(masked, mode)`` where mode flags whether real FPE was used. + + As in the surrogate, ``0 < visible < len(value)`` keeps the last ``visible`` + characters unchanged: only the digits before them are encrypted, as one number + whose length is their count. Decrypting therefore needs the same ``visible`` + split. When that head has no digits the surrogate is used instead. """ + s = str(value) + n = len(s) + keep_from = n - visible if 0 < visible < n else n + head, tail = s[:keep_from], s[keep_from:] try: # pragma: no cover - optional crypto dependency import pyffx - s = str(value) - digits = "".join(c for c in s if c.isdigit()) + digits = "".join(c for c in head if c.isdigit()) if digits and key: cipher = pyffx.Integer(key.encode("utf-8"), length=len(digits)) enc = str(cipher.encrypt(int(digits))).zfill(len(digits)) it = iter(enc) - rebuilt = "".join(next(it) if c.isdigit() else c for c in s) - return rebuilt, "crypto_fpe" + rebuilt = "".join(next(it) if c.isdigit() else c for c in head) + return rebuilt + tail, "crypto_fpe" except Exception: pass return _surrogate_value(value, key, visible=visible), ( @@ -1194,6 +1224,12 @@ def anonymize( With no ``rules`` and no ``detection_config`` there is nothing to apply, and a privacy call that silently returns raw data is a footgun — so it fails closed with a :class:`ValueError` instead of no-opping. + + ``tokenize``, ``surrogate`` and ``fpe`` rules are keyed by ``key`` / + ``key_env``. A rule without a key (and not ``reversible``) uses a random key + generated for this call: equal values get equal output within the call, but + not across calls. Such calls emit one :class:`EphemeralKeyWarning` naming the + rules and record them in ``report.metadata["ephemeral_key_rules"]``. """ if not rules and detection_config is None: raise ValueError( @@ -1227,10 +1263,27 @@ def anonymize( metadata: dict[str, Any] = {} fpe_modes: dict[str, dict[str, int]] = {} + run_secret: bytes | None = None + ephemeral_rules: list[str] = [] for rule in rules: key = _resolve_key(rule) vault = _vault_for(rule) - for column in _resolve_columns(rule, list(frame.columns)): + columns = [c for c in _resolve_columns(rule, list(frame.columns)) if c in frame.columns] + if ( + not key + and columns + and rule.strategy in _KEYED_STRATEGIES + and not (rule.reversible and rule.strategy in ("tokenize", "fpe")) + ): + # Never fall back to a constant key: anyone with the source could then + # recompute the output for guessed values. Reversible rules still + # raise in _apply_rule_column, since a random key cannot be kept. + if run_secret is None: + run_secret = secrets.token_bytes(32) + key = _ephemeral_key(run_secret, f"{rule.strategy}:{rule.name}") + if rule.name not in ephemeral_rules: + ephemeral_rules.append(rule.name) + for column in columns: if column not in frame.columns: continue n, mode_counts = _apply_rule_column( @@ -1252,6 +1305,14 @@ def anonymize( elif modes_used: metadata["fpe_mode"] = "mixed" metadata["fpe_modes"] = fpe_modes + if ephemeral_rules: + metadata["ephemeral_key_rules"] = ephemeral_rules + warnings.warn( + f"no key for masking rule(s) {ephemeral_rules}: using a random per-run key; " + "output is not stable across runs; pass key=/key_env= for stable pseudonyms", + EphemeralKeyWarning, + stacklevel=2, + ) entities_found = 0 if detection_config is not None and detection_config.enabled: @@ -1402,9 +1463,12 @@ def _mask_one( for pattern in _scrub_patterns(rule): scrubbed = re.sub(pattern, rule.placeholder, scrubbed) return scrubbed, None + # tokenize / surrogate / fpe: anonymize always supplies a key (the caller's or + # a random per-call one), so there is deliberately no keyless path here. + if not key: + raise ValueError(f"masking rule {rule.name!r}: {strategy} requires a key") if strategy == "tokenize": - tok_key = key or _hmac_hex("freshdata-default-token-salt", rule.name, 32) - return tokenize_value(original, vault, tok_key, prefix="tok"), None + return tokenize_value(original, vault, key, prefix="tok"), None if strategy == "surrogate": visible = rule.visible if rule.preserve_format else 0 preserve_domain = rule.preserve_format and "@" in original @@ -1413,11 +1477,7 @@ def _mask_one( ) # fpe visible = rule.visible if rule.preserve_format else 0 - if key: - return _fpe_value(original, key, visible=visible) - return _surrogate_value(original, key, visible=visible), ( - "surrogate_format_preserving_not_crypto_fpe" - ) + return _fpe_value(original, key, visible=visible) def _anonymize_detected( diff --git a/src/freshdata/enterprise/privacy_policy.py b/src/freshdata/enterprise/privacy_policy.py index b7c183da..cac64070 100644 --- a/src/freshdata/enterprise/privacy_policy.py +++ b/src/freshdata/enterprise/privacy_policy.py @@ -34,6 +34,8 @@ import json import os import re +import secrets +import warnings from dataclasses import dataclass from enum import Enum from functools import cache @@ -46,6 +48,7 @@ from .config import PIIDetectionConfig from .privacy import ( ENTITY_PATTERNS, + EphemeralKeyWarning, MaskingEvent, PrivacyReport, TokenVault, @@ -714,6 +717,12 @@ def apply_privacy_policy( the policy/rule vault settings build it; a key must come from ``key``/``key_env``. Report previews are redacted unless ``audit_include_pii=True``. + ``pseudonymize`` is keyed by the same keys. Without one it uses a random key + generated for this call (shared by every keyless ``pseudonymize`` column), so + pseudonyms are consistent within the call but not across calls; the call emits + one :class:`~freshdata.enterprise.EphemeralKeyWarning` and lists the rules in + ``report.metadata["ephemeral_key_rules"]``. + Column labels need not be strings. Report entries are keyed by ``str(label)``, so labels must be unique and stay distinct once stringified; otherwise ``ValueError`` is raised. @@ -737,6 +746,8 @@ def apply_privacy_policy( used_vault: TokenVault | None = vault used_backend: str | None = None # None => infer from the vault object's type drop_cols: list[str] = [] + run_key: str | None = None # random key for keyless pseudonymize, this call only + ephemeral_rules: list[str] = [] pack_by_name = {p.name: p for p in policy.packs} @@ -813,6 +824,14 @@ def apply_privacy_policy( new_values: list[Any] = [] changed = 0 tok_vault: TokenVault | None = None + if action is Action.PSEUDONYMIZE and not key: + # Never fall back to a constant key: anyone with the source could + # recompute pseudonyms of guessed values. + if run_key is None: + run_key = secrets.token_hex(32) + label = rule.id if rule is not None else "default_action" + if label not in ephemeral_rules: + ephemeral_rules.append(label) if action is Action.TOKENIZE: # tokenisation always needs a key (deterministic HMAC) and a vault # to record the mapping; reversibility just governs what we advertise. @@ -839,7 +858,8 @@ def apply_privacy_policy( if key: masked, _mode = _fpe(original, key) else: - masked = _surrogate(original, None) + assert run_key is not None # set above for keyless pseudonymize + masked = _surrogate(original, run_key) format_preserving = True else: # REDACT masked = _redact_cell(original, rule, cfg) @@ -866,6 +886,21 @@ def apply_privacy_policy( columns=[labels[c] for c in drop_cols if labels[c] in frame.columns], inplace=True ) + metadata: dict[str, Any] = { + "quarantined_columns": quarantined, + "dropped_columns": drop_cols, + "classification_values_scanned": values_scanned, + } + if ephemeral_rules: + metadata["ephemeral_key_rules"] = ephemeral_rules + warnings.warn( + f"no key for pseudonymize rule(s) {ephemeral_rules}: using a random per-run " + "key; output is not stable across runs; pass key=/key_env= (rule or policy) " + "for stable pseudonyms", + EphemeralKeyWarning, + stacklevel=2, + ) + detected = list(classifications.keys()) trust_dimension = { "sensitive_fields_detected": len(detected), @@ -894,11 +929,7 @@ def apply_privacy_policy( cells_changed=cells_changed, columns_changed=tuple(dict.fromkeys(changed_cols + drop_cols)), events=events, - metadata={ - "quarantined_columns": quarantined, - "dropped_columns": drop_cols, - "classification_values_scanned": values_scanned, - }, + metadata=metadata, policy_name=policy.name, jurisdiction=juris.value, compliance_pack=tuple(sorted(packs_used)), diff --git a/tests/test_enterprise_features_edge.py b/tests/test_enterprise_features_edge.py index 97ec8581..d3113a79 100644 --- a/tests/test_enterprise_features_edge.py +++ b/tests/test_enterprise_features_edge.py @@ -180,7 +180,7 @@ def test_anonymize_partial_and_regex_scrub_and_hash(): rules = ( MaskingRule(name="p", columns=("card",), strategy="partial", visible=4), MaskingRule(name="r", columns=("free",), strategy="regex_scrub", scrub_patterns=("ssn",)), - MaskingRule(name="t", columns=("tok",), strategy="tokenize"), # no key -> default salt + MaskingRule(name="t", columns=("tok",), strategy="tokenize", key="edge-test-key"), ) out, report = anonymize(df, rules=rules) assert out["card"].iloc[0].endswith("1111") @@ -193,7 +193,8 @@ def test_anonymize_partial_and_regex_scrub_and_hash(): def test_surrogate_email_preserves_domain(): df = pd.DataFrame({"email": ["john.doe@company.com"]}) rule = MaskingRule( - name="s", columns=("email",), strategy="surrogate", preserve_format=True + name="s", columns=("email",), strategy="surrogate", preserve_format=True, + key="edge-test-key", ) out, _report = anonymize(df, rules=(rule,)) assert out["email"].iloc[0].endswith("@company.com") diff --git a/tests/test_enterprise_privacy.py b/tests/test_enterprise_privacy.py index b5dea4cc..54ace977 100644 --- a/tests/test_enterprise_privacy.py +++ b/tests/test_enterprise_privacy.py @@ -157,7 +157,8 @@ def test_reversible_without_key_raises(): def test_surrogate_preserves_shape(): df = pd.DataFrame({"ssn": ["123-45-6789"]}) rule = MaskingRule( - name="s", columns=("ssn",), strategy="surrogate", preserve_format=True, visible=4 + name="s", columns=("ssn",), strategy="surrogate", preserve_format=True, visible=4, + key="K", ) out, report = anonymize(df, rules=(rule,)) masked = out["ssn"].iloc[0] diff --git a/tests/test_privacy_keyless_defaults.py b/tests/test_privacy_keyless_defaults.py new file mode 100644 index 00000000..f9642252 --- /dev/null +++ b/tests/test_privacy_keyless_defaults.py @@ -0,0 +1,378 @@ +"""Keyless tokenize / surrogate / fpe / pseudonymize never use constants from the source. + +Without a key, each call uses a random per-call key and emits EphemeralKeyWarning, +so output cannot be recomputed for guessed values. Also #281 part 3: crypto FPE +honours ``visible``. +""" + +from __future__ import annotations + +import hashlib +import hmac +import inspect +import sys +import types +import warnings + +import pandas as pd +import pytest + +from freshdata.enterprise import ( + EphemeralKeyWarning, + InMemoryTokenVault, + MaskingRule, + PrivacyPolicy, + PrivacyRule, + anonymize, + apply_privacy_policy, + load_compliance_pack, + privacy, + privacy_policy, +) + +KEY = "unit-test-key" +KEY_ENV = "FRESHDATA_TEST_KEYLESS_KEY" +SURROGATE_MODE = "surrogate_format_preserving_not_crypto_fpe" +LEGACY_TOKEN_SALT = "freshdata-default-token-salt" +LEGACY_SURROGATE_KEY = "freshdata-surrogate" + + +def _hmac_hex(key: str, value: str, n: int = 16) -> str: + return hmac.new(key.encode(), value.encode(), hashlib.sha256).hexdigest()[:n] + + +def _legacy_surrogate(s: str) -> str: + """The attacker's re-implementation of the pre-2.1.0 keyless surrogate.""" + d = hmac.new(LEGACY_SURROGATE_KEY.encode(), s.encode(), hashlib.sha256).digest() + return "".join( + str(d[i % 32] % 10) + if ch.isdigit() + else ( + chr((65 if ch.isupper() else 97) + d[i % 32] % 26) + if ch.isascii() and ch.isalpha() + else ch + ) + for i, ch in enumerate(s) + ) + + +def _call_warned(call, *args, **kwargs): + """Run *call*, requiring exactly one EphemeralKeyWarning; return (result, message).""" + with pytest.warns(EphemeralKeyWarning) as record: + result = call(*args, **kwargs) + ephemeral = [w for w in record if issubclass(w.category, EphemeralKeyWarning)] + assert len(ephemeral) == 1 + return result, str(ephemeral[0].message) + + +def _call_silent(call, *args, **kwargs): + with warnings.catch_warnings(): + warnings.simplefilter("error", EphemeralKeyWarning) + return call(*args, **kwargs) + + +@pytest.fixture +def no_pyffx(monkeypatch): + monkeypatch.setitem(sys.modules, "pyffx", None) + + +@pytest.fixture +def fake_pyffx(monkeypatch): + """A stand-in ``pyffx`` recording the digit length of every cipher it builds.""" + lengths: list[int] = [] + module = types.ModuleType("pyffx") + + class Integer: + def __init__(self, key, length): + self.length = length + lengths.append(length) + + def encrypt(self, n): + return (n * 7 + 3) % (10**self.length) + + module.Integer = Integer + monkeypatch.setitem(sys.modules, "pyffx", module) + return lengths + + +# -------------------------------------------------------------------------- +# Advisory PoC +# -------------------------------------------------------------------------- + + +def test_poc_keyless_output_is_not_recoverable_from_public_constants(no_pyffx): + (tok, _), _ = _call_warned( + anonymize, + pd.DataFrame({"ssn": ["123-45-6789"]}), + rules=(MaskingRule(name="ssn", columns=("ssn",), strategy="tokenize"),), + ) + (ps, rep), message = _call_warned( + apply_privacy_policy, + pd.DataFrame({"phone": ["+1 555 123 4567"]}), + PrivacyPolicy(packs=(load_compliance_pack("gdpr"),), jurisdiction="EU"), + ) + assert rep.classifications["phone"]["rule_id"] == "gdpr.phone" + assert "gdpr.phone" in message + + k = _hmac_hex(LEGACY_TOKEN_SALT, "ssn", 32) + token = tok["ssn"][0] + assert token.startswith("tok_") + ssn_guesses = (f"123-45-{i:04d}" for i in range(10000)) + assert [c for c in ssn_guesses if "tok_" + _hmac_hex(k, c) == token] == [] + pseudonym = ps["phone"][0] + assert pseudonym != "+1 555 123 4567" + phone_guesses = (f"+1 555 123 {i:04d}" for i in range(10000)) + assert [c for c in phone_guesses if _legacy_surrogate(c) == pseudonym] == [] + + +# -------------------------------------------------------------------------- +# anonymize: random per-call key +# -------------------------------------------------------------------------- + +_VALUES = ["123-45-6789", "123-45-6789", "987-65-4321"] + + +@pytest.mark.parametrize("strategy", ["tokenize", "surrogate", "fpe"]) +def test_keyless_rule_is_consistent_within_a_call_and_differs_across_calls(strategy, no_pyffx): + df = pd.DataFrame({"c": _VALUES}) + rule = MaskingRule(name="r", columns=("c",), strategy=strategy) + (first, report), message = _call_warned(anonymize, df, rules=(rule,)) + (second, _), _ = _call_warned(anonymize, df, rules=(rule,)) + assert first["c"][0] == first["c"][1] + assert first["c"][0] != first["c"][2] + assert first["c"][0] != second["c"][0] + assert all(v not in _VALUES for v in first["c"]) + assert report.metadata["ephemeral_key_rules"] == ["r"] + assert "['r']" in message and "random per-run key" in message + assert "key=/key_env=" in message + + +def test_one_warning_per_call_names_every_keyless_rule(no_pyffx): + df = pd.DataFrame({"a": ["x1"], "b": ["y2"], "c": ["z3"], "d": ["w4"], "e": ["v5"]}) + rules = ( + MaskingRule(name="tok_a", columns=("a",), strategy="tokenize"), + MaskingRule(name="sur_b", columns=("b",), strategy="surrogate"), + MaskingRule(name="fpe_c", columns=("c",), strategy="fpe"), + MaskingRule(name="keyed_d", columns=("d",), strategy="tokenize", key=KEY), + MaskingRule(name="hash_e", columns=("e",), strategy="hash"), + MaskingRule(name="missing", columns=("nope",), strategy="surrogate"), + ) + (_, report), message = _call_warned(anonymize, df, rules=rules) + assert report.metadata["ephemeral_key_rules"] == ["tok_a", "sur_b", "fpe_c"] + assert "keyed_d" not in message and "hash_e" not in message and "missing" not in message + assert report.to_json() # serialisable, and no key material recorded + assert KEY not in report.to_json() + + +@pytest.mark.parametrize("strategy", ["tokenize", "surrogate", "fpe"]) +def test_explicit_key_or_key_env_is_stable_and_silent(strategy, monkeypatch, no_pyffx): + monkeypatch.setenv(KEY_ENV, KEY) + df = pd.DataFrame({"c": _VALUES}) + by_key = MaskingRule(name="r", columns=("c",), strategy=strategy, key=KEY) + by_env = MaskingRule(name="r", columns=("c",), strategy=strategy, key_env=KEY_ENV) + runs = [_call_silent(anonymize, df, rules=(rule,)) for rule in (by_key, by_key, by_env)] + outputs = [list(out["c"]) for out, _ in runs] + assert outputs[0] == outputs[1] == outputs[2] + assert all("ephemeral_key_rules" not in report.metadata for _, report in runs) + + +def test_unset_key_env_uses_a_per_call_key(monkeypatch, no_pyffx): + monkeypatch.delenv(KEY_ENV, raising=False) + rule = MaskingRule(name="r", columns=("c",), strategy="surrogate", key_env=KEY_ENV) + (_, report), _ = _call_warned(anonymize, pd.DataFrame({"c": ["555-0100"]}), rules=(rule,)) + assert report.metadata["ephemeral_key_rules"] == ["r"] + + +@pytest.mark.parametrize("strategy", ["tokenize", "fpe"]) +def test_reversible_keyless_rule_still_raises(strategy): + rule = MaskingRule(name="r", columns=("c",), strategy=strategy, reversible=True) + with warnings.catch_warnings(): + warnings.simplefilter("error", EphemeralKeyWarning) + with pytest.raises(ValueError, match="requires key"): + anonymize(pd.DataFrame({"c": ["123-45-6789"]}), rules=(rule,)) + + +def test_keyless_fpe_with_crypto_uses_the_per_call_key(fake_pyffx): + rule = MaskingRule(name="r", columns=("c",), strategy="fpe") + (out, report), _ = _call_warned(anonymize, pd.DataFrame({"c": ["123-45-6789"]}), rules=(rule,)) + assert report.metadata["fpe_mode"] == "crypto_fpe" + assert report.metadata["ephemeral_key_rules"] == ["r"] + assert [e.reversible for e in report.events] == [False] + assert out["c"][0] == "864-19-7526" # the stub ignores the key + + +def test_no_keyless_code_path_or_constant_remains(): + with pytest.raises(ValueError, match="non-empty key"): + privacy._surrogate_value("123-45-6789", "") + with pytest.raises(ValueError, match="non-empty key"): + privacy._surrogate_value("123-45-6789", None) # type: ignore[arg-type] + for strategy in ("tokenize", "surrogate", "fpe"): + rule = MaskingRule(name="r", columns=("c",), strategy=strategy) + with pytest.raises(ValueError, match="requires a key"): + privacy._mask_one("x", rule, None, InMemoryTokenVault()) + for module in (privacy, privacy_policy): + source = inspect.getsource(module) + assert LEGACY_TOKEN_SALT not in source + assert LEGACY_SURROGATE_KEY not in source + + +# -------------------------------------------------------------------------- +# apply_privacy_policy: keyless pseudonymize +# -------------------------------------------------------------------------- + + +def _pseudonymize_policy(**kwargs) -> PrivacyPolicy: + rules = ( + PrivacyRule(id="pa", action="pseudonymize", columns=("a",)), + PrivacyRule(id="pb", action="pseudonymize", columns=("b",)), + ) + return PrivacyPolicy(name="p", rules=rules, **kwargs) + + +def test_policy_keyless_pseudonymize_uses_one_random_key_per_call(no_pyffx): + df = pd.DataFrame({"a": ["555-0100-2233"], "b": ["555-0100-2233"]}) + (first, report), message = _call_warned(apply_privacy_policy, df, _pseudonymize_policy()) + (second, _), _ = _call_warned(apply_privacy_policy, df, _pseudonymize_policy()) + assert first["a"][0] == first["b"][0] # one key for the whole call + assert first["a"][0] != second["a"][0] + assert first["a"][0] != "555-0100-2233" + assert first["a"][0] != _legacy_surrogate("555-0100-2233") + assert report.metadata["ephemeral_key_rules"] == ["pa", "pb"] + assert "['pa', 'pb']" in message + + +def test_policy_keyed_pseudonymize_is_stable_and_silent(monkeypatch, no_pyffx): + monkeypatch.setenv(KEY_ENV, KEY) + df = pd.DataFrame({"a": ["555-0100-2233"], "b": ["x"]}) + out1, rep1 = _call_silent(apply_privacy_policy, df, _pseudonymize_policy(key=KEY)) + out2, rep2 = _call_silent(apply_privacy_policy, df, _pseudonymize_policy(key_env=KEY_ENV)) + assert out1["a"][0] == out2["a"][0] + assert "ephemeral_key_rules" not in rep1.metadata + assert "ephemeral_key_rules" not in rep2.metadata + + +def test_policy_keyless_tokenize_still_raises(): + rule = PrivacyRule(id="t", action="tokenize", columns=("ssn",)) + with pytest.raises(ValueError, match="tokenize requires a key"): + apply_privacy_policy(pd.DataFrame({"ssn": ["123-45-6789"]}), PrivacyPolicy(rules=(rule,))) + + +@pytest.mark.parametrize( + ("pack", "jurisdiction", "column", "value", "rule_id"), + [ + ("gdpr", "EU", "email", "jane.roe@example.com", "gdpr.email"), + ("gdpr", "EU", "phone", "+1 555 123 4567", "gdpr.phone"), + ("hipaa", "US", "dob", "1980-01-02", "hipaa.dob"), + ("ferpa", "US", "grade", "Midterm 93.5 of 100", "ferpa.grade"), + ], +) +def test_compliance_packs_without_key_do_not_use_the_public_constant( + pack, jurisdiction, column, value, rule_id, no_pyffx +): + policy = PrivacyPolicy(packs=(load_compliance_pack(pack),), jurisdiction=jurisdiction) + df = pd.DataFrame({column: [value]}) + (out, report), message = _call_warned(apply_privacy_policy, df, policy) + assert report.classifications[column]["rule_id"] == rule_id + assert report.metadata["ephemeral_key_rules"] == [rule_id] + assert rule_id in message + assert out[column][0] not in (value, _legacy_surrogate(value)) + + +# -------------------------------------------------------------------------- +# Documented migration recipes reproduce the pre-2.1.0 output +# -------------------------------------------------------------------------- + + +def test_legacy_surrogate_matches_the_advisory_output(): + assert _legacy_surrogate("+1 555 123 4567") == "+1 646 836 3000" + + +def test_migration_recipes_reproduce_legacy_output(no_pyffx): + ssn = "123-45-6789" + df = pd.DataFrame({"ssn": [ssn]}) + + legacy_token_key = hmac.new( + LEGACY_TOKEN_SALT.encode(), b"ssn", hashlib.sha256 + ).hexdigest()[:32] + tok_rule = MaskingRule(name="ssn", columns=("ssn",), strategy="tokenize", key=legacy_token_key) + out, _ = _call_silent(anonymize, df, rules=(tok_rule,)) + assert out["ssn"][0] == "tok_" + _hmac_hex(_hmac_hex(LEGACY_TOKEN_SALT, "ssn", 32), ssn) + + sur_rule = MaskingRule( + name="s", columns=("ssn",), strategy="surrogate", key=LEGACY_SURROGATE_KEY + ) + out, _ = _call_silent(anonymize, df, rules=(sur_rule,)) + assert out["ssn"][0] == _legacy_surrogate(ssn) + + fpe_rule = MaskingRule( + name="f", columns=("ssn",), strategy="fpe", key=LEGACY_SURROGATE_KEY, + preserve_format=True, visible=4, + ) + out, _ = _call_silent(anonymize, df, rules=(fpe_rule,)) + assert out["ssn"][0] == _legacy_surrogate(ssn)[:7] + ssn[7:] + + policy = PrivacyPolicy( + packs=(load_compliance_pack("gdpr"),), jurisdiction="EU", key=LEGACY_SURROGATE_KEY + ) + phone = pd.DataFrame({"phone": ["+1 555 123 4567"]}) + out, _ = _call_silent(apply_privacy_policy, phone, policy) + assert out["phone"][0] == "+1 646 836 3000" + + +# -------------------------------------------------------------------------- +# #281 part 3: crypto FPE honours visible +# -------------------------------------------------------------------------- + +_CARD = "4111-1111-1111-1111" + + +def _fpe_rule(**kwargs) -> MaskingRule: + return MaskingRule(name="c", columns=("cc",), strategy="fpe", key=KEY, **kwargs) + + +def test_issue_281_crypto_fpe_keeps_last_visible_characters(fake_pyffx): + rule = _fpe_rule(preserve_format=True, visible=4) + out, report = _call_silent(anonymize, pd.DataFrame({"cc": [_CARD]}), rules=(rule,)) + assert out["cc"][0] == "8777-7777-7780-1111" + assert fake_pyffx == [12] # only the 12 digits before the visible tail + assert report.metadata == {"fpe_mode": "crypto_fpe"} + + +def test_fpe_value_splits_head_and_tail(fake_pyffx): + assert privacy._fpe_value("123-45-6789", KEY, visible=4) == ("864-18-6789", "crypto_fpe") + assert fake_pyffx == [5] + + +@pytest.mark.parametrize("visible", [19, 30]) +def test_visible_at_least_length_encrypts_every_digit(fake_pyffx, visible): + rule = _fpe_rule(preserve_format=True, visible=visible) + out, _ = _call_silent(anonymize, pd.DataFrame({"cc": [_CARD]}), rules=(rule,)) + assert out["cc"][0] == "8777-7777-7777-7780" + assert fake_pyffx == [16] + + +def test_preserve_format_false_ignores_visible(fake_pyffx): + rule = _fpe_rule(preserve_format=False, visible=4) + out, _ = _call_silent(anonymize, pd.DataFrame({"cc": [_CARD]}), rules=(rule,)) + assert out["cc"][0] == "8777-7777-7777-7780" + assert fake_pyffx == [16] + + +def test_head_without_digits_falls_back_to_surrogate(fake_pyffx): + rule = _fpe_rule(preserve_format=True, visible=4, reversible=True) + out, report = _call_silent(anonymize, pd.DataFrame({"cc": ["abcd-1234"]}), rules=(rule,)) + assert out["cc"][0] == privacy._surrogate_value("abcd-1234", KEY, visible=4) + assert out["cc"][0].endswith("-1234") + assert fake_pyffx == [] + assert report.metadata == {"fpe_mode": SURROGATE_MODE} + assert [e.reversible for e in report.events] == [False] + + +def test_mode_and_reversible_follow_the_mode_actually_used(fake_pyffx): + rule = _fpe_rule(preserve_format=True, visible=4, reversible=True) + df = pd.DataFrame({"cc": [_CARD, "abcd-1234"]}) + out, report = _call_silent(anonymize, df, rules=(rule,)) + assert out["cc"][0] == "8777-7777-7780-1111" + assert report.metadata["fpe_mode"] == "mixed" + assert report.metadata["fpe_modes"] == {"cc": {"crypto_fpe": 1, SURROGATE_MODE: 1}} + assert {e.row: e.reversible for e in report.events} == {0: True, 1: False} diff --git a/tests/test_privacy_missing_and_labels.py b/tests/test_privacy_missing_and_labels.py index 12377f36..c2dcffab 100644 --- a/tests/test_privacy_missing_and_labels.py +++ b/tests/test_privacy_missing_and_labels.py @@ -63,7 +63,7 @@ def no_pyffx(monkeypatch): name="r", columns=("c",), strategy="regex_scrub", scrub_patterns=(), regexes=(r".+",) ), "tokenize": MaskingRule(name="r", columns=("c",), strategy="tokenize", key=KEY), - "surrogate": MaskingRule(name="r", columns=("c",), strategy="surrogate"), + "surrogate": MaskingRule(name="r", columns=("c",), strategy="surrogate", key=KEY), "fpe": MaskingRule(name="r", columns=("c",), strategy="fpe", key=KEY), } @@ -323,7 +323,7 @@ def test_mixed_modes_across_columns_are_counted_per_column(stub_pyffx): df = pd.DataFrame({"a": ["123-45-6789"], "b": ["123-45-6789"]}) rules = ( MaskingRule(name="fa", columns=("a",), strategy="fpe", key="k"), - MaskingRule(name="sb", columns=("b",), strategy="surrogate"), + MaskingRule(name="sb", columns=("b",), strategy="surrogate", key="k"), ) _, report = anonymize(df, rules=rules) assert report.metadata == { @@ -352,7 +352,8 @@ def test_single_mode_report_is_unchanged(no_pyffx): df = pd.DataFrame({"ssn": ["123-45-6789", "987-65-4321"], "acct": ["1234567890", None]}) rules = ( MaskingRule( - name="s", columns=("ssn",), strategy="surrogate", preserve_format=True, visible=4 + name="s", columns=("ssn",), strategy="surrogate", preserve_format=True, visible=4, + key="K", ), MaskingRule(name="f", columns=("acct",), strategy="fpe", key="K", preserve_format=True), MaskingRule(name="d", columns=("ssn",), strategy="drop"), diff --git a/tests/test_privacy_policy.py b/tests/test_privacy_policy.py index 8dab65c1..a950b0f8 100644 --- a/tests/test_privacy_policy.py +++ b/tests/test_privacy_policy.py @@ -15,6 +15,7 @@ from freshdata.enterprise import ( Action, CompliancePack, + EphemeralKeyWarning, Jurisdiction, PrivacyPolicy, PrivacyRule, @@ -341,8 +342,11 @@ def test_load_policy_from_json(tmp_path): assert any(p.name == "gdpr" for p in policy.packs) df = pd.DataFrame({"badge": ["B-1", "B-2"], "email": ["a@x.com", "b@y.com"]}) - out, _ = apply_privacy_policy(df, policy) + # The spec sets no key, so the pack's pseudonymize rule uses a per-call key. + with pytest.warns(EphemeralKeyWarning, match="gdpr.email"): + out, report = apply_privacy_policy(df, policy) assert list(out["badge"]) == ["", ""] # inline rule applied + assert report.metadata["ephemeral_key_rules"] == ["gdpr.email"] def test_load_policy_from_yaml(tmp_path): diff --git a/tests/test_privacy_policy_classification.py b/tests/test_privacy_policy_classification.py index 614eb602..f35d7158 100644 --- a/tests/test_privacy_policy_classification.py +++ b/tests/test_privacy_policy_classification.py @@ -148,7 +148,7 @@ def test_unhashable_cells_are_classified(): def _gdpr_policy(*rules: PrivacyRule, jurisdiction: str = "EU") -> PrivacyPolicy: return PrivacyPolicy( - rules=rules, packs=(load_compliance_pack("gdpr"),), jurisdiction=jurisdiction + rules=rules, packs=(load_compliance_pack("gdpr"),), jurisdiction=jurisdiction, key=KEY ) From 1bf9e365643a5d3fab62fcdbdd55429e4db49a91 Mon Sep 17 00:00:00 2001 From: Kevin Costner <120246174+kevincostner17@users.noreply.github.com> Date: Tue, 15 Sep 2026 18:30:10 +0530 Subject: [PATCH 07/17] fix(learning): mask every detected PII type in privacy='mask' profiles fd.learn(privacy='mask') mapped only a handful of detect_pii entity types to sensitive profile types and silently dropped the rest, so card numbers, IBANs and IP addresses found by the scanner were written into saved profiles as raw value-map literals and examples. - Map CREDIT_CARD, IBAN, IP_ADDRESS, MRN, PATIENT_ID, INSURANCE_ID, DRIVER_LICENSE, ZIP_CODE, GEO_LOCATION and ICD_CODE; any other reported type fails closed to free_text. DATE_OF_BIRTH only counts with a dob/birth column name (new date_of_birth type). - Token-aware column-name hints for card, bank-account, IP and date-of-birth columns (pan/acct must be whole words, so company_name stays unmasked). - freshdata profile audit re-scans stored literals and exits 1 when a profile that claims no raw values holds checksum-valid card numbers or IBANs. - #280: add is_text_dtype() and use it in detect_pii and detection-driven anonymize, so categorical (and Arrow dictionary) text columns are scanned on pandas 1.5 as on pandas 2. --- docs/learning-profiles.md | 16 +- docs/threat-model.md | 11 ++ src/freshdata/_util.py | 30 ++++ src/freshdata/enterprise/cli.py | 3 +- src/freshdata/enterprise/privacy.py | 17 +- src/freshdata/learning/audit.py | 65 +++++++- src/freshdata/learning/privacy.py | 79 ++++++++- src/freshdata/learning/profile.py | 10 +- src/freshdata/learning/types.py | 5 + tests/learning/test_privacy_financial.py | 202 +++++++++++++++++++++++ tests/test_detect_pii_categorical.py | 98 +++++++++++ 11 files changed, 520 insertions(+), 16 deletions(-) create mode 100644 tests/learning/test_privacy_financial.py create mode 100644 tests/test_detect_pii_categorical.py diff --git a/docs/learning-profiles.md b/docs/learning-profiles.md index 466ab400..78e6231b 100644 --- a/docs/learning-profiles.md +++ b/docs/learning-profiles.md @@ -80,8 +80,20 @@ mode, schema, and provenance. ## Privacy By default (`privacy="mask"`), columns detected as sensitive — email, phone, -person name, national ID, address, postal code, or free text — never carry -raw literals in the saved profile: +person name, national ID (including medical record, patient, insurance and +driver's licence numbers), address, postal code, payment card number, bank +account / IBAN, IP address, health code, date of birth, or free text — never +carry raw literals in the saved profile. A column is sensitive when its name +matches a hint (`card_number`, `iban`, `acct`, `dob`, `email`, …; short hints +such as `pan` and `acct` must be a whole `_`-separated word) or when the +enterprise PII scanner finds any PII type in its values. A type without a +specific mapping is treated as free text rather than ignored; a date only +counts as a date of birth when the column name says so. + +`freshdata profile audit` re-scans stored literals and exits `1` when a +profile that claims no raw values holds checksum-valid card numbers or IBANs +(profiles learned before these types were masked). Re-learn such a profile +and delete the old file. - Rule-level evidence (a phone region, a `dayfirst` flag, a sentinel list) carries no literals to begin with and replays normally. diff --git a/docs/threat-model.md b/docs/threat-model.md index fe49f001..2c8c5d96 100644 --- a/docs/threat-model.md +++ b/docs/threat-model.md @@ -176,6 +176,17 @@ inline baseline in `clean_enterprise`) use a random key that is never stored. `freshdata-baseline-v1` files (unkeyed SHA-1 labels, reversible by hashing a guess list) still load with a warning; rebuild and delete them. +Learned `.fdprofile` archives are meant to be saved, diffed and shared. +Under the default `privacy="mask"`, literal value-map entries and examples on +sensitive columns are stored as HMAC tokens. A column is sensitive when its +name matches a hint or when `detect_pii` finds *any* PII type in it — payment +cards, IBANs, IP addresses and health identifiers included; unmapped types +fail closed to free text. `freshdata profile audit` flags checksum-valid card +numbers and IBANs left raw by older versions. **Residual risk:** PII that +neither a name hint nor the regex scanner recognises (for example a bare +account number in a column called `ref`) is stored raw; pass +`protected`/`context` policies or drop such columns before `fd.learn`. + ## Non-goals - **Not a sandbox.** FreshData reads tabular files; hostile *file formats* diff --git a/src/freshdata/_util.py b/src/freshdata/_util.py index 2b63abee..52d784d5 100644 --- a/src/freshdata/_util.py +++ b/src/freshdata/_util.py @@ -163,6 +163,36 @@ def _is_stringlike_dtype(dtype: object) -> bool: ) +def is_text_dtype(dtype: object) -> bool: + """True when *dtype* holds text, identically on pandas 1.5 and 2.x. + + Covers ``object``, ``StringDtype`` (python/pyarrow), ``pd.ArrowDtype`` of + ``string``/``large_string``/``string_view`` or a dictionary of those, and a + ``CategoricalDtype`` whose categories are text. ``is_string_dtype`` is not + used because it answers differently for categoricals across pandas lines. + """ + if isinstance(dtype, pd.CategoricalDtype): + return is_text_dtype(dtype.categories.dtype) + if pd.api.types.is_object_dtype(dtype) or isinstance(dtype, pd.StringDtype): + return True + arrow_dtype_cls = getattr(pd, "ArrowDtype", None) + if arrow_dtype_cls is None or not isinstance(dtype, arrow_dtype_cls): + return False + import pyarrow as pa # noqa: PLC0415 - an ArrowDtype implies pyarrow is installed + + arrow_type = getattr(dtype, "pyarrow_dtype", None) + if arrow_type is None: + return False + if pa.types.is_dictionary(arrow_type): + arrow_type = arrow_type.value_type + is_string_view = getattr(pa.types, "is_string_view", None) + return bool( + pa.types.is_string(arrow_type) + or pa.types.is_large_string(arrow_type) + or (is_string_view is not None and is_string_view(arrow_type)) + ) + + def is_arrow_string_dtype(dtype: object) -> bool: """True for a ``pd.ArrowDtype`` holding strings (pandas >= 2 only). diff --git a/src/freshdata/enterprise/cli.py b/src/freshdata/enterprise/cli.py index c7a4ae03..1899e48e 100644 --- a/src/freshdata/enterprise/cli.py +++ b/src/freshdata/enterprise/cli.py @@ -425,7 +425,8 @@ def _cmd_profile_tools(args: argparse.Namespace) -> int: print(json.dumps(audit.to_dict(), default=str, indent=2)) else: print(audit.render()) - return 0 + # Raw card numbers / IBANs in a profile that claims none: fail the audit. + return 1 if audit.raw_sensitive_literals else 0 if tool == "diff": if len(paths) != 2: print("usage: freshdata profile diff A.fdprofile B.fdprofile") diff --git a/src/freshdata/enterprise/privacy.py b/src/freshdata/enterprise/privacy.py index 82edac22..d7de9ae2 100644 --- a/src/freshdata/enterprise/privacy.py +++ b/src/freshdata/enterprise/privacy.py @@ -45,6 +45,7 @@ import pandas as pd +from .._util import is_text_dtype from ..adapters.polars import from_pandas, to_pandas from .cleaner import _hash_value, _partial_value, _resolve_columns, _scrub_patterns from .config import ( @@ -509,8 +510,10 @@ def _duplicated_labels(frame: pd.DataFrame) -> list[Any]: def detect_pii(df: Any, *, config: PIIDetectionConfig | None = None) -> PIIScanReport: """Scan the text columns of *df* for PII; return a :class:`PIIScanReport`. - Read-only. Only object/string columns are scanned. Raw matched substrings - are redacted in the report unless ``config.redact_samples=False``. + Read-only. Only text columns are scanned: ``object``, ``string`` (python or + pyarrow), Arrow string/dictionary-of-string, and categoricals whose + categories are text, on every supported pandas version. Raw matched + substrings are redacted in the report unless ``config.redact_samples=False``. Raises :class:`ValueError` when *df* has duplicate column labels, because a duplicated label does not identify a single column to scan. @@ -538,7 +541,7 @@ def detect_pii(df: Any, *, config: PIIDetectionConfig | None = None) -> PIIScanR scanned: list[str] = [] for col in frame.columns: series = frame[col] - if series.dtype != object and not pd.api.types.is_string_dtype(series): + if not is_text_dtype(series.dtype): continue scanned.append(str(col)) for row, value in series.items(): @@ -1487,11 +1490,15 @@ def _anonymize_detected( changed_cols: list[str], include_pii: bool, ) -> int: - """Replace detected PII spans in object columns with ````.""" + """Replace detected PII spans in text columns with ````. + + A scrubbed column is written back as ``object`` (a categorical's + categories would otherwise still hold the raw values). + """ n_entities = 0 for col in list(frame.columns): series = frame[col] - if series.dtype != object and not pd.api.types.is_string_dtype(series): + if not is_text_dtype(series.dtype): continue touched = False new_values: list[Any] = [] diff --git a/src/freshdata/learning/audit.py b/src/freshdata/learning/audit.py index d4d510fc..1eae99b2 100644 --- a/src/freshdata/learning/audit.py +++ b/src/freshdata/learning/audit.py @@ -11,12 +11,13 @@ from dataclasses import dataclass, field from typing import TYPE_CHECKING, Any +from .privacy import is_masked_token from .types import DemotionRecord if TYPE_CHECKING: # pragma: no cover from .profile import LearningProfile -__all__ = ["ProfileAudit", "build_audit"] +__all__ = ["ProfileAudit", "build_audit", "find_raw_financial_literals"] @dataclass @@ -44,6 +45,10 @@ class ProfileAudit: holdout_metrics: dict[str, Any] demotions: list[DemotionRecord] = field(default_factory=list) notes: list[str] = field(default_factory=list) + #: Unmasked literals that are checksum-valid card numbers or IBANs in a + #: profile that claims to hold no raw values (``{"where", "column", + #: "kind"}``; never the value itself). Recomputed on every audit. + raw_sensitive_literals: list[dict[str, str]] = field(default_factory=list) def to_dict(self) -> dict[str, Any]: return { @@ -68,6 +73,7 @@ def to_dict(self) -> dict[str, Any]: "holdout_metrics": dict(self.holdout_metrics), "demotions": [d.to_dict() for d in self.demotions], "notes": list(self.notes), + "raw_sensitive_literals": [dict(f) for f in self.raw_sensitive_literals], } @classmethod @@ -94,6 +100,10 @@ def from_dict(cls, data: Mapping[str, Any]) -> ProfileAudit: holdout_metrics=dict(data.get("holdout_metrics", {})), demotions=[DemotionRecord.from_dict(d) for d in data.get("demotions", [])], notes=list(data.get("notes", [])), + raw_sensitive_literals=[ + {str(k): str(v) for k, v in f.items()} + for f in data.get("raw_sensitive_literals", []) + ], ) def render(self) -> str: @@ -141,12 +151,64 @@ def render(self) -> str: lines.append(f" - {demotion.target}: {demotion.outcome} — {demotion.reason}") for note in self.notes: lines.append(f" note: {note}") + if self.raw_sensitive_literals: + places = ", ".join( + f"{f['where']}[{f['column']}] ({f['kind']})" for f in self.raw_sensitive_literals + ) + lines.append( + " ** RAW CARD NUMBERS / IBANS STORED: " + f"{places}. This profile was learned before these types were " + "masked; re-learn it with privacy='mask' and delete this file. **" + ) return "\n".join(lines) def __str__(self) -> str: # pragma: no cover - convenience return self.render() +def _financial_kinds(value: object) -> set[str]: + """``payment_card`` / ``bank_account`` when *value* holds a checksum-valid PAN / IBAN.""" + if not isinstance(value, str) or not value or is_masked_token(value): + return set() + from ..enterprise.config import PIIDetectionConfig # noqa: PLC0415 - heavy import + from ..enterprise.privacy import detect_in_text # noqa: PLC0415 + + cfg = PIIDetectionConfig(entities=("CREDIT_CARD", "IBAN"), use_context=False) + kinds: set[str] = set() + # IBANs are matched upper-case; lower-cased inputs are common in messy data. + for text in {value, value.upper()}: + for entity in detect_in_text(text, config=cfg): + kinds.add("payment_card" if entity.entity_type == "CREDIT_CARD" else "bank_account") + return kinds + + +def find_raw_financial_literals(profile: LearningProfile) -> list[dict[str, str]]: + """Unmasked card-number / IBAN literals in a profile that claims no raw values. + + Profiles learned before card numbers and IBANs were treated as sensitive + can hold them as raw value-map entries and examples. Returns one record per + ``(where, column, kind)``; the literal itself is never included. + """ + if profile.manifest.contains_raw_values: + return [] # declared raw; the audit already warns about it + found: dict[tuple[str, str, str], None] = {} + + def check(where: str, column: str, *values: object) -> None: + for value in values: + for kind in sorted(_financial_kinds(value)): + found[(where, column, kind)] = None + + for column, value_map in profile.value_maps.items(): + for entry in value_map.entries: + if not entry.masked: + check("value_maps", str(column), entry.raw_value, entry.clean_value) + if profile.examples is not None: + for example in profile.examples.examples: + if not example.masked: + check("examples", str(example.column), example.raw_value, example.clean_value) + return [{"where": w, "column": c, "kind": k} for (w, c, k) in found] + + def build_audit(profile: LearningProfile, **extra: Any) -> ProfileAudit: """Compute a fresh audit from an in-memory profile.""" rules_by_family: dict[str, int] = {} @@ -182,4 +244,5 @@ def build_audit(profile: LearningProfile, **extra: Any) -> ProfileAudit: holdout_metrics=dict(extra.get("holdout_metrics", {})), demotions=list(extra.get("demotions", [])), notes=list(extra.get("notes", [])), + raw_sensitive_literals=find_raw_financial_literals(profile), ) diff --git a/src/freshdata/learning/privacy.py b/src/freshdata/learning/privacy.py index 3a03b2cd..0463741b 100644 --- a/src/freshdata/learning/privacy.py +++ b/src/freshdata/learning/privacy.py @@ -1,8 +1,11 @@ """Privacy layer for learned profiles. Under the default ``privacy="mask"`` no raw literal from a sensitive column -(email, phone, person_name, national_id, address, postal_code, free_text) -is ever written into a profile. Rule-level learning (e.g. "this column is +(email, phone, person_name, national_id, address, postal_code, free_text, +payment_card, bank_account, ip_address, health_data, date_of_birth) is ever +written into a profile. Every PII type the enterprise scanner reports makes +a column sensitive; a type without a specific mapping fails closed to +``free_text``. Rule-level learning (e.g. "this column is an email column", "phones are region IN") carries no literals and replays fine; literal value-map entries and examples on sensitive columns are stored as deterministic HMAC tokens — auditable and countable, but not replayable @@ -35,10 +38,37 @@ _MASK_PREFIX = "fdmask" _MASK_RE = re.compile(r"^fdmask:[a-z_]+:[0-9a-f]{16}$") +#: Column-name hints matched as whole ``_``-separated tokens (a hint of several +#: tokens must match that run of tokens), so short fragments such as ``pan`` +#: or ``acct`` do not fire inside ``company_name`` or ``japan_region``. They +#: are checked before the substring hints below. +_TOKEN_HINTS: tuple[tuple[str, str], ...] = ( + ("card_number", "payment_card"), + ("card_no", "payment_card"), + ("cc_number", "payment_card"), + ("credit_card", "payment_card"), + ("debit_card", "payment_card"), + ("pan", "payment_card"), + ("iban", "bank_account"), + ("account_number", "bank_account"), + ("account_no", "bank_account"), + ("acct", "bank_account"), + ("routing_number", "bank_account"), + ("sort_code", "bank_account"), + ("ip_address", "ip_address"), + ("ip_addr", "ip_address"), + ("dob", "date_of_birth"), + ("date_of_birth", "date_of_birth"), +) + #: Column-name fragments mapped to sensitive semantic types. Value-based #: detection (enterprise detect_pii) takes precedence; these catch columns #: whose values are not self-identifying (names, addresses, ids). _NAME_HINTS: tuple[tuple[str, str], ...] = ( + ("creditcard", "payment_card"), + ("cardnumber", "payment_card"), + ("accountnumber", "bank_account"), + ("birth", "date_of_birth"), ("email", "email"), ("e_mail", "email"), ("phone", "phone"), @@ -86,20 +116,61 @@ "SSN": "national_id", "PASSPORT": "national_id", "NATIONAL_ID": "national_id", + "CREDIT_CARD": "payment_card", + "IBAN": "bank_account", + "IP_ADDRESS": "ip_address", + "MRN": "national_id", + "PATIENT_ID": "national_id", + "INSURANCE_ID": "national_id", + "DRIVER_LICENSE": "national_id", + "ZIP_CODE": "postal_code", + "GEO_LOCATION": "address", + "ICD_CODE": "health_data", } +#: Entity types that do not by themselves make a column sensitive. A +#: ``DATE_OF_BIRTH`` match is any date, so it only counts when the column name +#: says so (see :data:`_DOB_NAME_TYPE`). +_IGNORED_ENTITIES = frozenset({"DATE_OF_BIRTH"}) +_DOB_NAME_TYPE = "date_of_birth" +#: Sensitive type for an entity type without a specific mapping (fail closed). +_UNMAPPED_ENTITY_TYPE = "free_text" + _FREE_TEXT_MIN_AVG_LEN = 40.0 _FREE_TEXT_MIN_UNIQUE_RATIO = 0.8 -def _name_hint(column: str) -> str | None: +def _name_tokens(column: str) -> list[str]: lowered = re.sub(r"[^a-z0-9]+", "_", str(column).strip().lower()) + return [t for t in lowered.split("_") if t] + + +def _has_token_run(tokens: list[str], hint: str) -> bool: + want = hint.split("_") + n = len(want) + return any(tokens[i : i + n] == want for i in range(len(tokens) - n + 1)) + + +def _name_hint(column: str) -> str | None: + tokens = _name_tokens(column) + for hint, semantic_type in _TOKEN_HINTS: + if _has_token_run(tokens, hint): + return semantic_type + lowered = "_".join(tokens) for fragment, semantic_type in _NAME_HINTS: if fragment in lowered: return semantic_type return None +def _entity_sensitive_type(entity_type: object, column: str) -> str | None: + """Profile sensitive type for one ``detect_pii`` finding, or None to ignore it.""" + name = str(entity_type).upper() + if name in _IGNORED_ENTITIES: + return _DOB_NAME_TYPE if _name_hint(column) == _DOB_NAME_TYPE else None + return _ENTITY_MAP.get(name, _UNMAPPED_ENTITY_TYPE) + + def _looks_free_text(series: pd.Series) -> bool: non_null = series.dropna() if len(non_null) < 3 or non_null.dtype.kind not in "OU": @@ -123,7 +194,7 @@ def _pii_scan_types(df: pd.DataFrame) -> dict[str, str]: found: dict[str, str] = {} for column, entities in by_column.items(): for entity in entities: - mapped = _ENTITY_MAP.get(str(entity.entity_type).upper()) + mapped = _entity_sensitive_type(entity.entity_type, str(column)) if mapped is not None: found[str(column)] = mapped break diff --git a/src/freshdata/learning/profile.py b/src/freshdata/learning/profile.py index 396f2320..c43058cd 100644 --- a/src/freshdata/learning/profile.py +++ b/src/freshdata/learning/profile.py @@ -264,9 +264,13 @@ def summary(self) -> str: return " | ".join(pieces) def audit(self) -> ProfileAudit: - if self.audit_info is not None: - return self.audit_info - self.audit_info = build_audit(self) + if self.audit_info is None: + self.audit_info = build_audit(self) + else: + # A stored audit predates this check; scan the literals afresh. + from .audit import find_raw_financial_literals # noqa: PLC0415 + + self.audit_info.raw_sensitive_literals = find_raw_financial_literals(self) return self.audit_info def diff(self, other: LearningProfile) -> Any: diff --git a/src/freshdata/learning/types.py b/src/freshdata/learning/types.py index 529d50ec..36b6b3ab 100644 --- a/src/freshdata/learning/types.py +++ b/src/freshdata/learning/types.py @@ -60,6 +60,11 @@ "address", "postal_code", "free_text", + "payment_card", + "bank_account", + "ip_address", + "health_data", + "date_of_birth", } ) diff --git a/tests/learning/test_privacy_financial.py b/tests/learning/test_privacy_financial.py new file mode 100644 index 00000000..b6b44872 --- /dev/null +++ b/tests/learning/test_privacy_financial.py @@ -0,0 +1,202 @@ +"""fd.learn(privacy='mask') must not store raw card numbers, IBANs or IPs.""" + +from __future__ import annotations + +import json +import zipfile +from pathlib import Path + +import pandas as pd +import pytest + +import freshdata as fd +import freshdata.enterprise.privacy as enterprise_privacy +from freshdata.enterprise import cli +from freshdata.enterprise.config import PIIDetectionConfig +from freshdata.learning import privacy as learning_privacy +from freshdata.learning.audit import find_raw_financial_literals +from freshdata.learning.privacy import detect_sensitive_columns, is_masked_token +from freshdata.learning.profile import load_profile +from freshdata.learning.types import SENSITIVE_SEMANTIC_TYPES + +CARDS = ["4111 1111 1111 1111", "5500 0000 0000 0004"] +IBANS = ["DE89370400440532013000", "GB82WEST12345698765432"] +EMAILS = ["asha@gmail.com", "ravi@yahoo.com"] + + +def _saved_text(profile, tmp_path: Path) -> str: + path = tmp_path / "p.fdprofile" + profile.save(path) + with zipfile.ZipFile(path) as z: + return "".join(z.read(n).decode("utf-8") for n in z.namelist()) + + +def test_poc_cards_and_ibans_are_masked(tmp_path): + messy = pd.DataFrame( + { + "card_number": [c + " " for c in CARDS] * 10, + "iban": [i.lower() for i in IBANS] * 10, + "email": [" " + e.upper() for e in EMAILS] * 10, + } + ) + clean = pd.DataFrame( + {"card_number": CARDS * 10, "iban": IBANS * 10, "email": EMAILS * 10} + ) + profile = fd.learn(messy, clean, min_support=2) + text = _saved_text(profile, tmp_path) + + assert [v for v in CARDS + IBANS if v in text] == [] + assert [v for v in EMAILS if v in text] == [] # control + sensitive = profile.audit().sensitive_columns + assert sensitive["card_number"] == "payment_card" + assert sensitive["iban"] == "bank_account" + for column in ("card_number", "iban"): + entries = profile.value_maps[column].entries + assert entries and all(e.masked and is_masked_token(e.raw_value) for e in entries) + assert profile.audit().raw_sensitive_literals == [] + + +def test_ip_addresses_are_masked(tmp_path): + ips = ["10.20.30.40", "192.168.1.77"] + messy = pd.DataFrame({"server": [" " + ip for ip in ips] * 10}) + clean = pd.DataFrame({"server": ips * 10}) + profile = fd.learn(messy, clean, min_support=2) + assert profile.audit().sensitive_columns == {"server": "ip_address"} + text = _saved_text(profile, tmp_path) + assert [ip for ip in ips if ip in text] == [] + + +def test_value_detected_pans_with_mixed_separators(tmp_path): + raw = ["4111-1111-1111-1111", "5500 0000 0000 0004", "4012888888881881"] + messy = pd.DataFrame({"ref": [r + " " for r in raw] * 10}) + clean = pd.DataFrame({"ref": raw * 10}) + profile = fd.learn(messy, clean, min_support=2) + assert profile.audit().sensitive_columns == {"ref": "payment_card"} + text = _saved_text(profile, tmp_path) + assert [r for r in raw if r in text] == [] + + +def test_card_number_name_hint_covers_undetectable_values(tmp_path): + messy = pd.DataFrame({"card_number": ["xxxx-1234 ", "xxxx-9876 "] * 10}) + clean = pd.DataFrame({"card_number": ["xxxx-1234", "xxxx-9876"] * 10}) + profile = fd.learn(messy, clean, min_support=2) + assert profile.audit().sensitive_columns == {"card_number": "payment_card"} + assert "xxxx-1234" not in _saved_text(profile, tmp_path) + + +@pytest.mark.parametrize( + ("column", "expected"), + [ + ("card_number", "payment_card"), + ("Credit Card", "payment_card"), + ("customer_pan", "payment_card"), + ("cardNumber", "payment_card"), + ("IBAN", "bank_account"), + ("acct", "bank_account"), + ("acct_id", "bank_account"), + ("routing_number", "bank_account"), + ("client_ip_address", "ip_address"), + ("dob", "date_of_birth"), + ("birth_date", "date_of_birth"), + ("company_name", None), + ("japan_region", None), + ("panel", None), + ("account_manager", None), + ("status", None), + ], +) +def test_token_aware_name_hints(column, expected): + df = pd.DataFrame({column: ["alpha", "beta", "gamma"]}) + assert detect_sensitive_columns(df).get(column) == expected + + +def test_every_default_pii_type_is_sensitive_and_unknown_fails_closed(): + for entity in PIIDetectionConfig().entities: + mapped = learning_privacy._entity_sensitive_type(entity, "x") + if entity == "DATE_OF_BIRTH": + assert mapped is None + else: + assert mapped in SENSITIVE_SEMANTIC_TYPES, entity + assert learning_privacy._entity_sensitive_type("SOME_NEW_TYPE", "x") == "free_text" + assert learning_privacy._entity_sensitive_type("zip_code", "x") == "postal_code" + assert learning_privacy._entity_sensitive_type("ICD_CODE", "x") == "health_data" + + +def test_dates_count_as_date_of_birth_only_with_a_name_hint(monkeypatch): + dates = ["1990-01-02", "1985-12-31", "2001-07-04"] + df = pd.DataFrame({"visit_date": dates, "patient_dob": dates}) + sensitive = detect_sensitive_columns(df) + assert "visit_date" not in sensitive + assert sensitive["patient_dob"] == "date_of_birth" + + class _Entity: + entity_type = "DATE_OF_BIRTH" + + class _Report: + def by_column(self): + return {"shipped": [_Entity()], "date_of_birth": [_Entity()]} + + monkeypatch.setattr(enterprise_privacy, "detect_pii", lambda df: _Report()) + assert learning_privacy._pii_scan_types(pd.DataFrame()) == { + "date_of_birth": "date_of_birth" + } + + +def test_non_sensitive_columns_still_replay(): + messy = pd.DataFrame( + { + "status": ["Shipped ", "PENDING"] * 10, + "card_number": [c + " " for c in CARDS] * 10, + } + ) + clean = pd.DataFrame({"status": ["shipped", "pending"] * 10, "card_number": CARDS * 10}) + profile = fd.learn(messy, clean, min_support=2) + assert "status" not in profile.audit().sensitive_columns + status_entries = profile.value_maps["status"].replayable_entries() + assert {e.raw_value for e in status_entries} == {"Shipped ", "PENDING"} + assert profile.value_maps["card_number"].replayable_entries() == [] + + +def _legacy_profile(monkeypatch, tmp_path: Path) -> Path: + """A profile as older versions wrote it: raw PANs/IBANs, privacy 'mask'.""" + monkeypatch.setattr(learning_privacy, "_pii_scan_types", lambda df: {}) + messy = pd.DataFrame( + {"ref": [c + " " for c in CARDS] * 10, "code": [i.lower() for i in IBANS] * 10} + ) + clean = pd.DataFrame({"ref": CARDS * 10, "code": IBANS * 10}) + profile = fd.learn(messy, clean, min_support=2) + monkeypatch.undo() + path = tmp_path / "legacy.fdprofile" + profile.save(path) + assert CARDS[0] in _saved_text(profile, tmp_path / "copy") # really raw + return path + + +def test_profile_audit_flags_raw_cards_and_ibans(monkeypatch, tmp_path, capsys): + (tmp_path / "copy").mkdir() + path = _legacy_profile(monkeypatch, tmp_path) + loaded = load_profile(path) + findings = loaded.audit().raw_sensitive_literals + assert {(f["column"], f["kind"]) for f in findings} == { + ("ref", "payment_card"), + ("code", "bank_account"), + } + assert find_raw_financial_literals(loaded) == findings + + assert cli.main(["profile", "audit", str(path)]) == 1 + out = capsys.readouterr().out + assert "RAW CARD NUMBERS / IBANS" in out and "re-learn" in out + assert not [v for v in CARDS + IBANS if v in out or v.lower() in out] + + assert cli.main(["profile", "audit", str(path), "--json"]) == 1 + payload = json.loads(capsys.readouterr().out) + assert len(payload["raw_sensitive_literals"]) == 2 + + +def test_profile_audit_passes_a_masked_profile(tmp_path, capsys): + messy = pd.DataFrame({"card_number": [c + " " for c in CARDS] * 10}) + clean = pd.DataFrame({"card_number": CARDS * 10}) + path = tmp_path / "ok.fdprofile" + fd.learn(messy, clean, min_support=2).save(path) + assert cli.main(["profile", "audit", str(path)]) == 0 + assert "RAW CARD" not in capsys.readouterr().out diff --git a/tests/test_detect_pii_categorical.py b/tests/test_detect_pii_categorical.py new file mode 100644 index 00000000..5d989e70 --- /dev/null +++ b/tests/test_detect_pii_categorical.py @@ -0,0 +1,98 @@ +"""detect_pii / detection-driven anonymize scan categorical text on every pandas line (#280).""" + +from __future__ import annotations + +import zipfile + +import numpy as np +import pandas as pd +import pytest + +import freshdata as fd +from freshdata._util import is_text_dtype +from freshdata.enterprise import PIIDetectionConfig, anonymize, detect_pii + + +def test_issue_poc_categorical_note_is_scanned_and_scrubbed(): + df = pd.DataFrame({"note": pd.Categorical(["mail a@b.com", "ssn 123-45-6789"])}) + assert detect_pii(df).columns_scanned == ("note",) + out, rep = anonymize(df, detection_config=PIIDetectionConfig()) + assert out["note"].tolist() == ["mail ", "ssn "] + assert out["note"].dtype == object + assert rep.entities_found == 2 + assert df["note"].dtype == "category" # input untouched + + +def test_categorical_missing_values_stay_missing(): + df = pd.DataFrame({"note": pd.Categorical(["a@b.com", None, "plain"])}) + out, _ = anonymize(df, detection_config=PIIDetectionConfig()) + values = out["note"].tolist() + assert values[0] == "" + assert pd.isna(values[1]) + assert values[2] == "plain" + + +def test_non_text_categorical_is_not_scanned(): + df = pd.DataFrame( + { + "n": pd.Categorical([4111111111111111, 5500000000000004]), + "when": pd.Categorical(pd.to_datetime(["2020-01-01", "2021-01-01"])), + } + ) + assert detect_pii(df).columns_scanned == () + + +def test_categorical_pan_column_is_masked_by_learn(tmp_path): + cards = ["4111 1111 1111 1111", "5500 0000 0000 0004"] + # The messy side is categorical: that is the frame detect_pii scans. + messy = pd.DataFrame({"ref": pd.Categorical([c + " " for c in cards] * 10)}) + clean = pd.DataFrame({"ref": cards * 10}) + profile = fd.learn(messy, clean, min_support=2) + assert profile.audit().sensitive_columns == {"ref": "payment_card"} + path = tmp_path / "p.fdprofile" + profile.save(path) + with zipfile.ZipFile(path) as z: + text = "".join(z.read(n).decode("utf-8") for n in z.namelist()) + assert [c for c in cards if c in text] == [] + + +@pytest.mark.parametrize( + ("dtype", "expected"), + [ + (np.dtype(object), True), + (pd.StringDtype(), True), + (pd.CategoricalDtype(["a", "b"]), True), + (pd.CategoricalDtype([1, 2]), False), + (np.dtype("int64"), False), + (np.dtype("float64"), False), + (np.dtype("datetime64[ns]"), False), + (pd.Int64Dtype(), False), + (np.dtype(bool), False), + ], +) +def test_is_text_dtype(dtype, expected): + assert is_text_dtype(dtype) is expected + + +def test_is_text_dtype_arrow(): + pa = pytest.importorskip("pyarrow") + arrow_dtype = getattr(pd, "ArrowDtype", None) + if arrow_dtype is None: + pytest.skip("pd.ArrowDtype not available") + assert is_text_dtype(pd.StringDtype("pyarrow")) + assert is_text_dtype(arrow_dtype(pa.string())) + assert is_text_dtype(arrow_dtype(pa.large_string())) + assert is_text_dtype(arrow_dtype(pa.dictionary(pa.int32(), pa.string()))) + assert not is_text_dtype(arrow_dtype(pa.int64())) + assert not is_text_dtype(arrow_dtype(pa.dictionary(pa.int32(), pa.int64()))) + + +def test_arrow_string_column_is_scanned(): + pa = pytest.importorskip("pyarrow") + arrow_dtype = getattr(pd, "ArrowDtype", None) + if arrow_dtype is None: + pytest.skip("pd.ArrowDtype not available") + df = pd.DataFrame({"note": pd.Series(["mail a@b.com", None], dtype=arrow_dtype(pa.string()))}) + rep = detect_pii(df) + assert rep.columns_scanned == ("note",) + assert [e.entity_type for e in rep.entities] == ["EMAIL"] From 6a7447144dc3c11bea6aa431d6bebc25973fda95 Mon Sep 17 00:00:00 2001 From: Kevin Costner <120246174+kevincostner17@users.noreply.github.com> Date: Tue, 15 Sep 2026 18:40:05 +0530 Subject: [PATCH 08/17] fix(enterprise): keep masked column values out of clean_enterprise reports Clustering, semantic validation and core cleaning run before masking, so their reports held pre-masking values: cluster canonical/variant/key values and mappings, validation invalid_samples, and clean_report.coerced_cells (plus the coercion warnings quoting them). to_dict()/to_json() then published raw values of the columns the caller masked. For every column a masking rule selects, or that PII detection changed, the EnterpriseResult now holds redacted report objects: - ClusterResult.redacted_copy(): canonical/variants go through the column's rule (hash/redact/partial/regex_scrub reproduce the data token; tokenize/fpe/surrogate/drop/detection become ""), key becomes "", mapping is emptied, counts kept; new redacted field. - Validation invalid_samples are masked the same way. - coerced_cells originals and the matching warning examples are masked. --- src/freshdata/enterprise/cleaner.py | 28 ++- src/freshdata/enterprise/interface.py | 137 +++++++++++++-- tests/test_enterprise_report_masking.py | 220 ++++++++++++++++++++++++ 3 files changed, 367 insertions(+), 18 deletions(-) create mode 100644 tests/test_enterprise_report_masking.py diff --git a/src/freshdata/enterprise/cleaner.py b/src/freshdata/enterprise/cleaner.py index 030dedae..6a2eede9 100644 --- a/src/freshdata/enterprise/cleaner.py +++ b/src/freshdata/enterprise/cleaner.py @@ -25,7 +25,7 @@ import re from collections import defaultdict from collections.abc import Callable, Mapping, Sequence -from dataclasses import dataclass, field +from dataclasses import dataclass, field, replace from typing import Any import pandas as pd @@ -168,6 +168,10 @@ def _pick_canonical(members: list, policy: str) -> str: return max(members, key=lambda vc: (vc[1], len(vc[0]), vc[0]))[0] # most_frequent +#: Stand-in for a value that must not appear in a report. +REDACTED = "" + + @dataclass(frozen=True) class Cluster: """One merged group: a canonical value and the variants that map to it.""" @@ -198,6 +202,8 @@ class ClusterResult: n_cells_merged: int mapping: dict[str, str] clusters: tuple[Cluster, ...] + #: True when the values were masked/redacted because the column is masked. + redacted: bool = False def to_dict(self) -> dict[str, Any]: return { @@ -206,8 +212,28 @@ def to_dict(self) -> dict[str, Any]: "n_clusters": self.n_clusters, "n_cells_merged": self.n_cells_merged, "clusters": [c.to_dict() for c in self.clusters], + "redacted": self.redacted, } + def redacted_copy(self, mask: Callable[[str], str]) -> ClusterResult: + """A copy safe to report for a masked column. + + ``canonical`` and ``variants`` go through *mask* (the column's masking + rule, so tokens match the masked data), the fingerprint ``key`` is + replaced by ``""`` and the raw ``mapping`` is emptied. Counts + are kept. + """ + clusters = tuple( + replace( + c, + key=REDACTED, + canonical=mask(c.canonical), + variants=tuple(mask(v) for v in c.variants), + ) + for c in self.clusters + ) + return replace(self, mapping={}, clusters=clusters, redacted=True) + def __repr__(self) -> str: return ( f" Callable[[Any], str]: + """How a masked column's values may appear in the report. + + Deterministic rules reproduce the token written to the data (a hashed + column's cluster canonical equals the hashed cell). Token-, surrogate- and + FPE-masked, dropped and detection-scrubbed columns are fully redacted. + """ + if rule is None: + return lambda v: REDACTED + if rule.strategy == "hash": + return lambda v: _hash_value(v, rule.salt, rule.hash_length) + if rule.strategy == "redact": + return lambda v: rule.placeholder + if rule.strategy == "partial": + return lambda v: _partial_value(v, rule.visible, rule.placeholder) + if rule.strategy == "regex_scrub": + patterns = [re.compile(p) for p in _scrub_patterns(rule)] + + def scrub(v: Any) -> str: + text = str(v) + for pattern in patterns: + text = pattern.sub(rule.placeholder, text) + return text + + return scrub + return lambda v: REDACTED + + +def _masked_report_columns(result: EnterpriseResult, ec: EnterpriseConfig) -> dict[str, Any]: + """Column name -> report masker for every report column the masking stage masked.""" + candidates = sorted( + {r.column for r in result.cluster_results} + | set(result.validation_report.columns if result.validation_report else ()) + | {str(c) for c in result.clean_report.coerced_cells} + ) + rules: dict[str, list[MaskingRule]] = {} + for rule in ec.masking: + for column in _resolve_columns(rule, candidates): + rules.setdefault(str(column), []).append(rule) + # One rule reproduces its token; several stacked rules are just redacted. + maskers = {c: _report_masker(rs[0] if len(rs) == 1 else None) for c, rs in rules.items()} + if result.privacy_report is not None: + for column in result.privacy_report.columns_changed: + if column in candidates and column not in maskers: + maskers[column] = _report_masker(None) # detection-scrubbed + return maskers + + +def _redact_masked_values(result: EnterpriseResult, ec: EnterpriseConfig) -> EnterpriseResult: + """Keep raw values of masked columns out of every report on *result*. + + Clustering, semantic validation and core cleaning run before masking, so + their reports hold pre-masking values. For masked columns this replaces, on + the result objects themselves (not only in serialisation), cluster + canonical/variant/key values and mappings, semantic-validation invalid + samples, and ``clean_report.coerced_cells`` originals plus the coercion + warnings that quote them. + """ + use_privacy = ec.enable_privacy_detection and ec.privacy is not None + if not (ec.enable_masking and (ec.masking or use_privacy)): + return result + maskers = _masked_report_columns(result, ec) + if not maskers: + return result + result.cluster_results = [ + r.redacted_copy(maskers[r.column]) if r.column in maskers else r + for r in result.cluster_results + ] + if result.validation_report is not None: + columns = dict(result.validation_report.columns) + for name, cv in columns.items(): + if name in maskers: + samples = tuple(maskers[name](v) for v in cv.invalid_samples) + columns[name] = replace(cv, invalid_samples=samples) + result.validation_report = ValidationReport(columns=columns) + report = result.clean_report + for column, cells in list(report.coerced_cells.items()): + mask = maskers.get(str(column)) + if mask is None: + continue + prefix = f"column '{column}': " + quoted = [(repr(v), repr(mask(v))) for v in list(cells.values())[:3]] + report.coerced_cells[column] = {row: mask(v) for row, v in cells.items()} + for i, warning in enumerate(report.warnings): + if warning.startswith(prefix): + text = warning + for raw, masked in quoted: + text = text.replace(raw, masked) + report.warnings[i] = text + return result + + def _resolve_enterprise_config(enterprise: EnterpriseConfig | None) -> EnterpriseConfig: """Return the effective config, failing closed on ``anonymization`` (#247). @@ -394,19 +493,23 @@ def track(rule: str, before: Any, after: Any, count: int, description: str) -> N clean_report=clean_report, actor=who or "unknown", ) - return EnterpriseResult( - data=work, - trust_before=trust_before, - trust_after=trust_after, - clean_report=clean_report, - quality=quality, - lineage=tracker, - cluster_results=cluster_results, - mask_report=mask_report, - validation_report=validation_report, - fail_under_trust=ec.fail_under_trust, - drift_report=drift_report, - privacy_report=privacy_report, - k_anonymity_report=k_anonymity_report, - entity_resolution_report=entity_resolution_report, + # Clustering, validation and core cleaning saw pre-masking values. + return _redact_masked_values( + EnterpriseResult( + data=work, + trust_before=trust_before, + trust_after=trust_after, + clean_report=clean_report, + quality=quality, + lineage=tracker, + cluster_results=cluster_results, + mask_report=mask_report, + validation_report=validation_report, + fail_under_trust=ec.fail_under_trust, + drift_report=drift_report, + privacy_report=privacy_report, + k_anonymity_report=k_anonymity_report, + entity_resolution_report=entity_resolution_report, + ), + ec, ) diff --git a/tests/test_enterprise_report_masking.py b/tests/test_enterprise_report_masking.py new file mode 100644 index 00000000..aea75ffb --- /dev/null +++ b/tests/test_enterprise_report_masking.py @@ -0,0 +1,220 @@ +"""clean_enterprise reports must not carry raw values of masked columns.""" + +from __future__ import annotations + +import json + +import pandas as pd +import pytest + +from freshdata.enterprise import ( + ClusterConfig, + EnterpriseConfig, + MaskingRule, + PIIDetectionConfig, + SemanticValidatorConfig, + clean_enterprise, +) + +EMAILS = ["john.doe@acme.com", "John.Doe@acme.com", "jane@x.org"] + + +def _frame(n: int = 3) -> pd.DataFrame: + return pd.DataFrame( + { + "email": EMAILS * n, + "city": ["Paris", "paris", "Rome"] * n, + "v": range(3 * n), + } + ) + + +def _leaks(res, values) -> list[str]: + text = res.to_json() + return sorted(v for v in set(values) if v in text) + + +def _email_results(res): + return [r for r in res.cluster_results if r.column == "email"] + + +def test_poc_hashed_column_is_not_in_the_report(): + ec = EnterpriseConfig( + masking=(MaskingRule(name="m", columns=("email",), strategy="hash"),), + enable_clustering=True, + clustering=ClusterConfig(), + ) + res = clean_enterprise(_frame(), enterprise=ec) + assert sorted(set(res.data["email"]) & set(EMAILS)) == [] + assert [e for e in set(EMAILS) if e in json.dumps(res.to_dict()["clusters"])] == [] + assert _leaks(res, EMAILS) == [] + + +@pytest.mark.parametrize( + ("rule_kwargs", "consistent"), + [ + ({"strategy": "hash"}, True), + ({"strategy": "redact"}, True), + ({"strategy": "partial"}, True), + ({"strategy": "tokenize", "key": "k" * 32}, False), + ({"strategy": "surrogate", "key": "k" * 32}, False), + ({"strategy": "drop"}, False), + ], + ids=["hash", "redact", "partial", "tokenize", "surrogate", "drop"], +) +def test_masked_column_clusters_are_masked_on_the_result(rule_kwargs, consistent): + ec = EnterpriseConfig( + masking=(MaskingRule(name="m", columns=("email",), **rule_kwargs),), + enable_clustering=True, + clustering=ClusterConfig(columns=("email",)), + ) + res = clean_enterprise(_frame(), enterprise=ec) + assert _leaks(res, EMAILS) == [] + + results = _email_results(res) + assert results and all(r.redacted and r.mapping == {} for r in results) + clusters = [c for r in results for c in r.clusters] + assert clusters + assert all(c.key == "" for c in clusters) + shown = {c.canonical for c in clusters} | {v for c in clusters for v in c.variants} + assert not shown & set(EMAILS) + if consistent: + # Deterministic rules reproduce the token in the masked data. + assert {c.canonical for c in clusters} <= set(res.data["email"]) + else: + assert shown == {""} + assert all(r.to_dict()["redacted"] is True for r in results) + + +def test_fingerprint_ngram_results_are_all_redacted(): + ec = EnterpriseConfig( + masking=(MaskingRule(name="m", columns=("email",), strategy="hash"),), + enable_clustering=True, + clustering=ClusterConfig(method="fingerprint_ngram"), + ) + res = clean_enterprise(_frame(), enterprise=ec) + results = _email_results(res) + assert {r.method for r in results} == {"fingerprint", "ngram"} + assert all(r.redacted for r in results) + assert _leaks(res, EMAILS) == [] + + +def test_pattern_rule_selects_columns_for_redaction(): + ec = EnterpriseConfig( + masking=(MaskingRule(name="m", pattern="^em", strategy="redact"),), + enable_clustering=True, + clustering=ClusterConfig(), + ) + res = clean_enterprise(_frame(), enterprise=ec) + assert all(r.redacted for r in _email_results(res)) + assert _leaks(res, EMAILS) == [] + + +def test_detection_scrubbed_free_text_column_is_redacted(): + notes = ["contact john.doe@acme.com today", "Contact John.Doe@acme.com today", "no pii here"] + df = pd.DataFrame({"note": notes * 4, "v": range(12)}) + ec = EnterpriseConfig( + enable_privacy_detection=True, + privacy=PIIDetectionConfig(), + enable_clustering=True, + clustering=ClusterConfig(), + ) + res = clean_enterprise(df, enterprise=ec) + assert res.privacy_report is not None and "note" in res.privacy_report.columns_changed + results = [r for r in res.cluster_results if r.column == "note"] + assert results and all(r.redacted for r in results) + assert _leaks(res, ["john.doe@acme.com", "John.Doe@acme.com"]) == [] + + +def test_semantic_validation_samples_are_masked_for_masked_columns(): + ec = EnterpriseConfig( + masking=(MaskingRule(name="m", columns=("email",), strategy="hash"),), + semantic=( + SemanticValidatorConfig(name="nope", columns=("email",), kind="regex", regex="^zzz$"), + SemanticValidatorConfig(name="cities", columns=("city",), kind="reference", + reference=("Rome",)), + ), + ) + res = clean_enterprise(_frame(), enterprise=ec) + email_cv = res.validation_report.columns["email"] + assert email_cv.invalid_samples + assert not set(email_cv.invalid_samples) & set(EMAILS) + assert set(email_cv.invalid_samples) <= set(res.data["email"]) + assert email_cv.n_invalid == 9 # counts kept + # Unmasked columns keep their raw samples. + assert "Paris" in res.validation_report.columns["city"].invalid_samples + assert _leaks(res, EMAILS) == [] + + +def test_unmasked_columns_keep_raw_clusters_and_counts(): + cfg = ClusterConfig() + plain = clean_enterprise( + _frame(), enterprise=EnterpriseConfig(enable_clustering=True, clustering=cfg) + ) + masked = clean_enterprise( + _frame(), + enterprise=EnterpriseConfig( + masking=(MaskingRule(name="m", columns=("email",), strategy="hash"),), + enable_clustering=True, + clustering=cfg, + ), + ) + city = [r for r in masked.cluster_results if r.column == "city"] + assert city and not any(r.redacted for r in city) + assert city[0].mapping + assert {c.canonical for r in city for c in r.clusters} <= {"Paris", "paris"} + assert masked.cells_merged == plain.cells_merged + assert [r.n_clusters for r in masked.cluster_results] == [ + r.n_clusters for r in plain.cluster_results + ] + + +def test_masking_disabled_leaves_clusters_raw(): + ec = EnterpriseConfig( + masking=(MaskingRule(name="m", columns=("email",), strategy="hash"),), + enable_masking=False, + enable_clustering=True, + clustering=ClusterConfig(), + ) + res = clean_enterprise(_frame(), enterprise=ec) + assert not any(r.redacted for r in res.cluster_results) + + +def test_polars_input(): + pl = pytest.importorskip("polars") + ec = EnterpriseConfig( + masking=(MaskingRule(name="m", columns=("email",), strategy="hash"),), + enable_clustering=True, + clustering=ClusterConfig(), + ) + res = clean_enterprise(pl.from_pandas(_frame()), enterprise=ec) + assert isinstance(res.data, pl.DataFrame) + assert all(r.redacted for r in _email_results(res)) + assert _leaks(res, EMAILS) == [] + + +@pytest.mark.parametrize("strategy", ["hash", "drop"]) +def test_coerced_cells_of_masked_date_like_column(strategy): + born = pd.date_range("1970-01-01", periods=99, freq="D").strftime("%Y-%m-%d").tolist() + amount = [f"{i}.5" for i in range(99)] + df = pd.DataFrame( + { + "born": born + ["31/31/XYZZY"], + "amount": amount + ["12abcQWERTY"], + "open": amount + ["7xyzOPEN"], + } + ) + ec = EnterpriseConfig( + masking=(MaskingRule(name="m", columns=("born", "amount"), strategy=strategy),) + ) + res = clean_enterprise(df, enterprise=ec) + coerced = res.clean_report.coerced_cells + assert set(coerced) >= {"born", "amount", "open"} + assert "XYZZY" not in json.dumps(coerced["born"], default=str) + assert "QWERTY" not in json.dumps(coerced["amount"], default=str) + if strategy == "drop": + assert list(coerced["born"].values()) == [""] + assert _leaks(res, ["XYZZY", "QWERTY"]) == [] + assert not [w for w in res.clean_report.warnings if "XYZZY" in w or "QWERTY" in w] + # The unmasked column keeps its reviewable original. + assert list(coerced["open"].values()) == ["7xyzOPEN"] From 13eaedaaae075f2bd714601fbc8478279610dddb Mon Sep 17 00:00:00 2001 From: Kevin Costner <120246174+kevincostner17@users.noreply.github.com> Date: Tue, 15 Sep 2026 18:54:00 +0530 Subject: [PATCH 09/17] fix(report): key the tokens that stand in for sensitive values mask_sensitive_value() made [SENSITIVE:xxxxxxxx] tokens as an unkeyed sha256(repr(v))[:8]. Reports use these tokens in place of values from declared sensitive_columns. Low-entropy values (SSNs, phone numbers, dates of birth, small categories) could be recovered by hashing a guess list and matching the tokens in CleanReport warnings, coerced_cells, semantic action text and metadata, or validate_fields normalized_cells. The token is now a truncated HMAC-SHA256 of repr(v). The key comes from secrets.token_bytes(32), is made once per process on first use (behind a lock), and is never persisted. There is no constant fallback. Token prefix and length are unchanged, and within a run the same value still gives the same token, so records correlate inside one report. Tokens differ between processes. No caller needs tokens to match across processes. The masked memory_key and value_signature are never replayed against raw values. Repair-plan params, which feed decisions_hash, hold the raw proposal, not the token. TruthBench repeats run in one process. No golden files or tests pin a digest. So there is no stable-key parameter. --- docs/threat-model.md | 9 +++ src/freshdata/_util.py | 44 ++++++++++-- src/freshdata/config.py | 7 +- tests/test_mask_sensitive_value.py | 109 +++++++++++++++++++++++++++++ 4 files changed, 162 insertions(+), 7 deletions(-) create mode 100644 tests/test_mask_sensitive_value.py diff --git a/docs/threat-model.md b/docs/threat-model.md index 2c8c5d96..a77e8c67 100644 --- a/docs/threat-model.md +++ b/docs/threat-model.md @@ -117,6 +117,15 @@ deterministic path on purpose: a per-run random salt would break the documented reproducibility of `model_context` and its audit fingerprint. This trade-off is tracked as a roadmap item, not silently changed. +Report stand-ins for declared `sensitive_columns` are a separate case. They +are the `[SENSITIVE:xxxxxxxx]` tokens in `CleanReport` warnings, coerced +cells, semantic action text and metadata, and `validate_fields` +`normalized_cells`. Each one is a truncated HMAC-SHA256 under a random key +made once per process, never a constant. Within one run the same value +gives the same token, so records can be matched up. A guess list of SSNs, +phone numbers or dates cannot be hashed to find a match, and tokens from +different runs are not joinable. + ### 7. Local temporary/spill files When a DuckDB run exceeds `memory_limit_gb`, DuckDB writes intermediate diff --git a/src/freshdata/_util.py b/src/freshdata/_util.py index 52d784d5..ed87a4cc 100644 --- a/src/freshdata/_util.py +++ b/src/freshdata/_util.py @@ -3,7 +3,10 @@ from __future__ import annotations import hashlib +import hmac import math +import secrets +import threading import warnings from fractions import Fraction from typing import Any @@ -285,13 +288,44 @@ def sanitize_csv_formulas(df: pd.DataFrame) -> pd.DataFrame: return out -def mask_sensitive_value(value: object) -> str: - """Deterministic stand-in for a sensitive value in report text. +_SENSITIVE_TOKEN_KEY: bytes | None = None +_SENSITIVE_TOKEN_KEY_LOCK = threading.Lock() + - The short digest lets two mentions of the same value be correlated - without disclosing it; the token never round-trips to the original. +def _sensitive_token_key() -> bytes: + """The per-process secret for :func:`mask_sensitive_value`, made on first use. + + It comes from :func:`secrets.token_bytes` and is never written anywhere. + The lock makes sure concurrent first calls share one key, so one report + never mixes tokens made under two keys. + """ + global _SENSITIVE_TOKEN_KEY # noqa: PLW0603 - lazy process-wide secret + key = _SENSITIVE_TOKEN_KEY + if key is None: + with _SENSITIVE_TOKEN_KEY_LOCK: + if _SENSITIVE_TOKEN_KEY is None: + _SENSITIVE_TOKEN_KEY = secrets.token_bytes(32) + key = _SENSITIVE_TOKEN_KEY + return key + + +def mask_sensitive_value(value: object) -> str: + """Stand-in token for a sensitive value in report text: ``[SENSITIVE:xxxxxxxx]``. + + The 8 hex characters are a truncated HMAC-SHA256 of ``repr(value)``. The key + is a random secret made once per process. Within a process the same value + always gives the same token, so mentions in one report can still be matched + up. Tokens change between processes and never map back to the value. + + Why the key matters: an unkeyed digest of a low-entropy value (an SSN, a + phone number, a date of birth, a small category) can be reversed by hashing + a list of guesses. Without the per-process key, a guess list cannot be + checked against the tokens. No caller needs tokens to match across runs, + so there is no stable-key option and no constant fallback. """ - digest = hashlib.sha256(repr(value).encode("utf-8")).hexdigest()[:8] + digest = hmac.new( + _sensitive_token_key(), repr(value).encode("utf-8"), hashlib.sha256 + ).hexdigest()[:8] return f"[SENSITIVE:{digest}]" diff --git a/src/freshdata/config.py b/src/freshdata/config.py index 81acdcf6..b5a3c285 100644 --- a/src/freshdata/config.py +++ b/src/freshdata/config.py @@ -149,8 +149,11 @@ class CleanConfig: preserve_columns: tuple[str, ...] = () #: Columns whose values are sensitive: the data itself is cleaned as #: normal, but the values never appear verbatim in report text, warning - #: examples, action rationales/metadata, or coerced-cell payloads — a - #: deterministic digest token stands in so records stay correlatable. + #: examples, action rationales/metadata, or coerced-cell payloads. A + #: ``[SENSITIVE:xxxxxxxx]`` token stands in: a truncated HMAC under a + #: per-process random key. Records stay matchable within one run, but a + #: guess list cannot be hashed to recover the value, and tokens differ + #: between runs. sensitive_columns: tuple[str, ...] = () #: The label/target column; never modified by the engine. Columns named #: "target", "label", "y", "outcome", or "class" are detected automatically. diff --git a/tests/test_mask_sensitive_value.py b/tests/test_mask_sensitive_value.py new file mode 100644 index 00000000..fc9651b3 --- /dev/null +++ b/tests/test_mask_sensitive_value.py @@ -0,0 +1,109 @@ +"""Report stand-ins for sensitive values must not be reversible by guessing.""" + +from __future__ import annotations + +import hashlib +import os +import re +import subprocess +import sys +import threading + +import pandas as pd + +import freshdata as fd +from freshdata import _util +from freshdata._util import mask_sensitive_value + +TOKEN_RE = re.compile(r"\[SENSITIVE:[0-9a-f]{8}\]") +SECRET_SSN = "078-05-1120" +COMMON = {"return_report": True, "verbose": False} + + +def _unkeyed_token(value: object) -> str: + """The token scheme used before the fix: ``sha256(repr(v))[:8]``.""" + return f"[SENSITIVE:{hashlib.sha256(repr(value).encode('utf-8')).hexdigest()[:8]}]" + + +def _report_tokens(report) -> set[str]: + blob = repr(report.to_dict()) + repr(report.warnings) + repr(report.coerced_cells) + return set(TOKEN_RE.findall(blob)) + + +def test_dictionary_attack_finds_no_sensitive_value_in_the_report(): + df = pd.DataFrame({"ssn": [str(i) for i in range(19)] + [SECRET_SSN], "id": range(20)}) + _, report = fd.clean(df, sensitive_columns=("ssn",), **COMMON) + + tokens = _report_tokens(report) + assert tokens # the column really was masked; the attack has targets + assert SECRET_SSN not in repr(report.to_dict()) + + # An attacker who knows the format hashes a guess list that includes the + # real value, plus the str() forms callers might pass. + guesses = [f"078-05-{n:04d}" for n in range(1000, 1200)] + [SECRET_SSN] + guesses += [str(i) for i in range(1000)] + dictionary = {_unkeyed_token(g) for g in guesses} + assert _unkeyed_token(SECRET_SSN) in dictionary + assert tokens.isdisjoint(dictionary) + + +def test_same_value_gives_same_token_within_a_process(): + assert mask_sensitive_value(SECRET_SSN) == mask_sensitive_value(SECRET_SSN) + assert mask_sensitive_value(SECRET_SSN) != mask_sensitive_value("078-05-1121") + # repr() is still what gets hashed, so 1 and "1" stay distinct. + assert mask_sensitive_value(1) != mask_sensitive_value("1") + + +def test_same_value_correlates_across_one_report(): + df = pd.DataFrame( + {"ssn": [str(i) for i in range(38)] + [SECRET_SSN, SECRET_SSN], "id": range(40)} + ) + _, report = fd.clean(df, sensitive_columns=("ssn",), **COMMON) + # Both unparseable rows are named in one warning; the matching tokens show + # they are the same value without revealing it. + (warning,) = [w for w in report.warnings if "'ssn'" in w] + assert SECRET_SSN not in warning + assert TOKEN_RE.findall(warning) == [mask_sensitive_value(SECRET_SSN)] * 2 + + +def test_child_process_gives_different_tokens(): + src_root = os.path.dirname(os.path.dirname(os.path.abspath(fd.__file__))) + env = dict(os.environ) + env["PYTHONPATH"] = os.pathsep.join(p for p in (src_root, env.get("PYTHONPATH")) if p) + code = ( + "from freshdata._util import mask_sensitive_value as m; " + f"print(m({SECRET_SSN!r})); print(m({SECRET_SSN!r}))" + ) + out = subprocess.run( + [sys.executable, "-c", code], capture_output=True, text=True, env=env, check=True + ).stdout.split() + assert len(out) == 2 and out[0] == out[1] + assert TOKEN_RE.fullmatch(out[0]) + assert out[0] != mask_sensitive_value(SECRET_SSN) + assert out[0] != _unkeyed_token(SECRET_SSN) + + +def test_token_format_and_length_are_unchanged(): + for value in (SECRET_SSN, 42, None, 3.5, ("a", 1), "", "ü"): + token = mask_sensitive_value(value) + assert TOKEN_RE.fullmatch(token) + assert len(token) == len("[SENSITIVE:]") + 8 == len(_unkeyed_token(value)) + + +def test_concurrent_first_use_shares_one_key(monkeypatch): + monkeypatch.setattr(_util, "_SENSITIVE_TOKEN_KEY", None) + barrier = threading.Barrier(8) + seen: list[str] = [] + + def worker() -> None: + barrier.wait() + seen.append(mask_sensitive_value(SECRET_SSN)) + + threads = [threading.Thread(target=worker) for _ in range(8)] + for t in threads: + t.start() + for t in threads: + t.join() + assert len(seen) == 8 and len(set(seen)) == 1 + key = _util._SENSITIVE_TOKEN_KEY + assert isinstance(key, bytes) and len(key) == 32 From 53eaa9ea34ce6924d53a5cb393317ddb5e9cde3e Mon Sep 17 00:00:00 2001 From: Kevin Costner <120246174+kevincostner17@users.noreply.github.com> Date: Tue, 15 Sep 2026 18:49:59 +0530 Subject: [PATCH 10/17] fix(copilot): mask every non-numeric sample column before model_context In privacy="mask_pii_before_reasoning" the copilot decided which sample columns to hash-mask with a deny-list that recognised only object, StringDtype and CategoricalDtype as string-like. pd.ArrowDtype string, large_string and dictionary columns (the output of pyarrow-backed readers) were not matched, so their raw values went into model_context, the provider prompt, to_json() and the HTML render. Datetime, timedelta and period values were sent raw as well. Replace the deny-list with an allow-list: only numeric and boolean dtypes (numpy, nullable and Arrow-backed, checked through pyarrow on pandas < 2) pass through raw. Every other column, including dtypes the copilot does not recognise, is hash-masked. allow_unmasked_columns still opts a column out, and declared or detected PII columns are always masked. --- docs/ai-copilot.md | 23 +- docs/threat-model.md | 20 +- docs/trust-claims.md | 2 +- src/freshdata/experimental/ai_copilot.py | 60 +++-- tests/test_copilot_sample_dtype_allowlist.py | 231 +++++++++++++++++++ 5 files changed, 300 insertions(+), 36 deletions(-) create mode 100644 tests/test_copilot_sample_dtype_allowlist.py diff --git a/docs/ai-copilot.md b/docs/ai-copilot.md index f0e02fca..5a6d3982 100644 --- a/docs/ai-copilot.md +++ b/docs/ai-copilot.md @@ -43,9 +43,10 @@ Three properties make this different from "ask a chatbot about my data": compiler, value clustering, trust scoring). The same input always produces the same report; it runs in CI with no API key and no network access. - **Privacy-first.** Raw string values never enter `report.model_context` — - the only payload an LLM provider would ever see. Every string-like sample - column is hash-masked first (numeric values pass through as-is), or samples - are omitted entirely with `privacy="schema_only"`. + the only payload an LLM provider would ever see. Every sample column that + is not numeric or boolean is hash-masked first (numeric and boolean values + pass through as-is), or samples are omitted entirely with + `privacy="schema_only"`. - **Actionable.** The output is not advice — it is an ordered plan with a rationale per step, plus a generated freshdata pipeline you can run as-is. (The test suite literally `exec()`s the generated code and asserts the @@ -97,12 +98,16 @@ artifact you would get from `fd.compile_context`. Unknown rules raise a The `privacy` parameter controls what goes into `report.model_context`: - `"mask_pii_before_reasoning"` (default) — includes `sample_rows` sample - rows, but every string-like column is hash-masked first: `must_mask` - columns, columns the PII detector flagged, **and** every other - object/string/categorical column — regex detection cannot see names, - addresses, or free text, so no string value is sent raw. Numeric values - pass through as-is; numeric quasi-identifiers (e.g. exact salary + age) - are the residual risk — drop such columns first or use `"schema_only"`. + rows, but only numeric and boolean columns pass through raw (an + allow-list). Everything else is hash-masked first: `must_mask` columns, + columns the PII detector flagged, **and** every other column — + object/string, Arrow-backed string or dictionary (e.g. from + `read_csv(dtype_backend="pyarrow")`), categorical, bytes, datetime, + timedelta, period, and any dtype the copilot does not recognise. Regex + detection cannot see names, addresses, free text, or dates of birth, so + no such value is sent raw. Numeric quasi-identifiers (e.g. exact salary + + age) are the residual risk — drop such columns first or use + `"schema_only"`. `allow_unmasked_columns=[...]` is an explicit per-column opt-out; it never exempts a declared or detected PII column. - `"schema_only"` — no cell values at all; only column names, dtypes, diff --git a/docs/threat-model.md b/docs/threat-model.md index a77e8c67..da09c452 100644 --- a/docs/threat-model.md +++ b/docs/threat-model.md @@ -53,12 +53,15 @@ opted out and opens the CSV in a spreadsheet re-accepts the injection risk. it is SHA-256 fingerprinted in `report.audit` so you can prove after the fact what was shared. Per privacy mode: -- **`mask_pii_before_reasoning` (default)** — every string-like column - (object / string / categorical) in the sample rows is hash-masked: declared - `must_mask` columns, regex-detected PII columns, and everything else - string-like. This is deliberate defense-in-depth: regex detection cannot - see names, addresses, or free text, so no string value is trusted to be - safe. `allow_unmasked_columns` is an explicit per-column opt-out that +- **`mask_pii_before_reasoning` (default)** — sample values pass through + raw only for numeric and boolean dtypes (an allow-list). Every other + column in the sample rows is hash-masked: declared `must_mask` columns, + regex-detected PII columns, and every object / string / Arrow string or + dictionary / categorical / bytes / datetime / timedelta / period column, + including dtypes the copilot does not recognise. This is deliberate + defense-in-depth: regex detection cannot see names, addresses, free text + or dates of birth, so no non-numeric value is trusted to be safe. + `allow_unmasked_columns` is an explicit per-column opt-out that never exempts a declared or detected PII column and rejects unknown names. Detected-problem details entering `model_context` are value-free in **every** mode (`category_noise` spelling previews stay local). @@ -87,8 +90,9 @@ The dependency-free detector covers EMAIL / PHONE / SSN / credit card / IP. It does **not** detect names, addresses, or free-text PII (install the `privacy` extra for NER via presidio). This is the #1 practitioner gotcha: if you call `anonymize()` with only auto-detected columns, undetected PII -passes through. The copilot does not inherit this gap because it masks all -string-like columns regardless of detection (see boundary 3). +passes through. The copilot does not inherit this gap because it masks every +non-numeric, non-boolean sample column regardless of detection (see +boundary 3). ### 6. Masking tokens diff --git a/docs/trust-claims.md b/docs/trust-claims.md index 1fc621f8..ebaad302 100644 --- a/docs/trust-claims.md +++ b/docs/trust-claims.md @@ -11,7 +11,7 @@ README wording verbatim to named tests, and CI fails if either side drifts. |---|---| | protected columns are never modified | `tests/test_semantic_cleaning.py::test_id_columns_protected`, CleanBench `T2.protected_column_violation_rate` | | nothing happens silently | `tests/test_semantic_cleaning.py::test_assist_records_without_mutating` | -| raw PII never enters the copilot's model context | 4 tests in `tests/test_experimental_ai_copilot.py`, incl. adversarial cases: undeclared string-like columns masked, `category_noise` previews withheld | +| raw PII never enters the copilot's model context | 4 tests in `tests/test_experimental_ai_copilot.py`, incl. adversarial cases: undeclared string-like columns masked, `category_noise` previews withheld; `tests/test_copilot_sample_dtype_allowlist.py` covers every non-numeric dtype (Arrow string / dictionary / list, categorical, bytes, datetime, timedelta, period) across the prompt, `model_context`, JSON, HTML and text sinks | ## Other README / docs claims diff --git a/src/freshdata/experimental/ai_copilot.py b/src/freshdata/experimental/ai_copilot.py index 912edd4e..6051b006 100644 --- a/src/freshdata/experimental/ai_copilot.py +++ b/src/freshdata/experimental/ai_copilot.py @@ -339,7 +339,8 @@ def _build_prompt(goal: str, model_context: dict[str, Any]) -> str: return ( "You are a data-quality assistant. Using ONLY the masked dataset " "context below (schema, aggregate statistics, and sample rows whose " - "string values are hash-masked; numeric values pass through as-is), " + "non-numeric values are hash-masked; numeric and boolean values pass " + "through as-is), " "explain the main data-quality risks and how " "the proposed freshdata cleaning plan addresses them.\n\n" f"User goal: {goal}\n\n" @@ -347,24 +348,45 @@ def _build_prompt(goal: str, model_context: dict[str, Any]) -> str: ) -def _is_stringlike(dtype: object) -> bool: - return pd.api.types.is_object_dtype(dtype) or isinstance( - dtype, (pd.StringDtype, pd.CategoricalDtype) - ) +def _passes_through_raw(dtype: object) -> bool: + """Whether sample values of *dtype* may enter ``model_context`` unmasked. + + An allow-list, so dtypes it does not recognise fail closed: only boolean + and numeric dtypes (numpy, nullable and Arrow-backed) pass. Object, + string, Arrow string/dictionary/list, categorical (even of numbers), + bytes, datetime, timedelta, period and interval columns are all masked. + """ + if isinstance(dtype, pd.CategoricalDtype) or pd.api.types.is_object_dtype(dtype): + return False + pa_type = getattr(dtype, "pyarrow_dtype", None) + if pa_type is not None: + # pandas < 2 does not report Arrow numbers as numeric; ask pyarrow. + import pyarrow.types as pa_types # noqa: PLC0415 - only reachable with pyarrow + + return bool( + pa_types.is_integer(pa_type) + or pa_types.is_floating(pa_type) + or pa_types.is_decimal(pa_type) + or pa_types.is_boolean(pa_type) + ) + try: + return bool(pd.api.types.is_bool_dtype(dtype) or pd.api.types.is_numeric_dtype(dtype)) + except (TypeError, ValueError): + return False def _sample_mask_columns( frame: pd.DataFrame, mask_columns: Sequence[str], allow_unmasked: Sequence[str] ) -> list[str]: """Columns to hash-mask in sample rows: every declared/detected PII column - *and* every string-like column — regex PII detection cannot see names, - addresses, or free text, so string-like columns are unsafe to send raw. - ``allow_unmasked`` exempts specific string-like columns but never a - declared or detected PII column. Numeric columns pass through as-is. + *and* every column whose dtype is not numeric or boolean — regex PII + detection cannot see names, addresses, free text or dates of birth, so + only numeric and boolean values are sent raw. ``allow_unmasked`` exempts + specific columns but never a declared or detected PII column. """ declared = {c for c in mask_columns if c in frame.columns} - stringlike = {c for c in frame.columns if _is_stringlike(frame[c].dtype)} - return sorted(declared | (stringlike - set(allow_unmasked)), key=str) + not_raw = {c for c in frame.columns if not _passes_through_raw(frame[c].dtype)} + return sorted(declared | (not_raw - set(allow_unmasked)), key=str) def _mask_sample( @@ -754,12 +776,14 @@ def analyze_dataset( privacy: ``"mask_pii_before_reasoning"`` (default) includes ``sample_rows`` sample rows in ``report.model_context`` with every declared/detected - PII column *and* every string-like column hash-masked (regex PII - detection cannot see names, addresses, or free text, so string - values are never sent raw). Numeric values pass through as-is — - numeric quasi-identifiers are the residual risk; drop such columns - first or use ``"schema_only"``, which includes no sample rows at - all. + PII column *and* every column that is not numeric or boolean + hash-masked — strings (object, ``string``, Arrow string/dictionary), + categoricals, bytes, datetimes, timedeltas, periods and any dtype the + copilot does not recognise (regex PII detection cannot see names, + addresses, free text or dates of birth, so those values are never + sent raw). Numeric and boolean values pass through as-is — numeric + quasi-identifiers are the residual risk; drop such columns first or + use ``"schema_only"``, which includes no sample rows at all. context_policy: Optional ``{column: rule}`` mapping (rule may also be a list of rules). Supported rules: ``must_mask``, @@ -778,7 +802,7 @@ def analyze_dataset( source_hint: Filename used in the generated ``recommended_code``. allow_unmasked_columns: - Explicit opt-out: string-like columns listed here are sent unmasked + Explicit opt-out: non-numeric columns listed here are sent unmasked in the sample rows. Declared (``must_mask``) and regex-detected PII columns are always masked regardless. Unknown column names raise ``ValueError``. diff --git a/tests/test_copilot_sample_dtype_allowlist.py b/tests/test_copilot_sample_dtype_allowlist.py new file mode 100644 index 00000000..1c87902c --- /dev/null +++ b/tests/test_copilot_sample_dtype_allowlist.py @@ -0,0 +1,231 @@ +"""Copilot sample masking is an allow-list: only numeric and boolean values pass. + +Regression tests for Arrow-backed string columns (and every other non-numeric +dtype) reaching the provider prompt raw because the old check recognised only +object, ``string`` and categorical columns as string-like. +""" + +from __future__ import annotations + +import html +import json +from collections.abc import Callable + +import pandas as pd +import pytest + +from freshdata.experimental.ai_copilot import ( + _mask_sample, + _passes_through_raw, + _sample_mask_columns, + analyze_dataset, +) + +NAMES = ["Alice Johnson", "Bob Smith"] + + +def _pa(): + return pytest.importorskip("pyarrow") + + +def _run_with_prompt(frame: pd.DataFrame, **kwargs): + prompts: list[str] = [] + + def provider(prompt: str) -> str: + prompts.append(prompt) + return "ok" + + with pytest.warns(FutureWarning, match="experimental"): + report = analyze_dataset(frame, provider=provider, **kwargs) + assert len(prompts) == 1 + return report, prompts[0] + + +def _sinks(report, prompt: str) -> dict[str, str]: + rendered = report._repr_html_() + return { + "prompt": prompt, + "model_context": json.dumps(report.model_context, default=str, ensure_ascii=False), + "to_json": report.to_json(), + "html": rendered, + "html_unescaped": html.unescape(rendered), + "str": str(report), + } + + +def _assert_absent(report, prompt: str, raws: list[str]) -> None: + for sink, text in _sinks(report, prompt).items(): + leaked = [raw for raw in raws if raw in text] + assert not leaked, f"raw sample value(s) {leaked} reached {sink}" + + +def _arrow(type_factory: Callable[[object], object]) -> Callable[[], pd.Series]: + def build() -> pd.Series: + pa = _pa() + return pd.Series(pd.array(NAMES, dtype=pd.ArrowDtype(type_factory(pa)))) + + return build + + +def _string_pyarrow() -> pd.Series: + _pa() + return pd.Series(NAMES, dtype="string[pyarrow]") + + +# Each case builds the column and lists raw renderings that must never escape. +MASKED_CASES: dict[str, tuple[Callable[[], pd.Series], list[str]]] = { + "arrow-string": (_arrow(lambda pa: pa.string()), NAMES), + "arrow-large_string": (_arrow(lambda pa: pa.large_string()), NAMES), + "arrow-dictionary": (_arrow(lambda pa: pa.dictionary(pa.int32(), pa.string())), NAMES), + "string[pyarrow]": (_string_pyarrow, NAMES), + "string[python]": (lambda: pd.Series(NAMES, dtype="string"), NAMES), + "categorical": (lambda: pd.Series(pd.Categorical(NAMES)), NAMES), + "object-bytes": (lambda: pd.Series([n.encode() for n in NAMES], dtype=object), NAMES), + "datetime64": ( + lambda: pd.Series(pd.to_datetime(["1980-02-03 04:05:06", "1975-06-07 08:09:10"])), + ["1980-02-03", "1975-06-07"], + ), + "datetime64-tz": ( + lambda: pd.Series(pd.to_datetime(["1980-02-03 04:05:06", "1975-06-07 08:09:10"])).dt + .tz_localize("UTC"), + ["1980-02-03", "1975-06-07"], + ), + "timedelta64": ( + lambda: pd.Series(pd.to_timedelta([1234567, 7654321], unit="s")), + ["14 days 06:56:07", "88 days 14:12:01"], + ), + "period": ( + lambda: pd.Series(pd.period_range("1980-02-03", periods=2, freq="D")), + ["1980-02-03", "1980-02-04"], + ), + "categorical-of-ints": ( + lambda: pd.Series(pd.Categorical([123456789, 987654321])), + ["123456789", "987654321"], + ), +} + + +def test_advisory_poc_arrow_string_column_is_masked() -> None: + pa = _pa() + df = pd.DataFrame({"name": pd.array(NAMES, dtype=pd.ArrowDtype(pa.string())), "v": [1, 2]}) + report, prompt = _run_with_prompt(df) + assert [n for n in NAMES if n in prompt] == [] + assert "name" in report.audit["sample_masked_columns"] + + +@pytest.mark.parametrize("case", sorted(MASKED_CASES)) +def test_non_numeric_dtype_never_reaches_any_sink_raw(case: str) -> None: + build, raws = MASKED_CASES[case] + column = build() + assert not _passes_through_raw(column.dtype) + df = pd.DataFrame({"col": column, "v": [1, 2]}) + report, prompt = _run_with_prompt(df) + assert "col" in report.audit["sample_masked_columns"] + rows = report.model_context["sample_rows_masked"] + assert len(rows) == 2 + for row in rows: + assert isinstance(row["col"], str) + assert len(row["col"]) == 16 and set(row["col"]) <= set("0123456789abcdef") + _assert_absent(report, prompt, raws) + + +def test_arrow_list_of_strings_is_masked_in_sample() -> None: + # ``analyze_dataset`` cannot profile list columns (pandas cannot + # factorize them), so check the sample masking helpers directly. + pa = _pa() + frame = pd.DataFrame( + { + "tags": pd.array([[n] for n in NAMES], dtype=pd.ArrowDtype(pa.list_(pa.string()))), + "v": [1, 2], + } + ) + assert not _passes_through_raw(frame["tags"].dtype) + columns = _sample_mask_columns(frame, [], []) + assert columns == ["tags"] + blob = json.dumps(_mask_sample(frame, columns, 5), default=str) + assert not [n for n in NAMES if n in blob] + + +def _passthrough_frame() -> pd.DataFrame: + return pd.DataFrame( + { + "i64": pd.Series([123456, 654321], dtype="int64"), + "f64": [1.25, 2.5], + "flag": [True, False], + "nullable_int": pd.Series([7, None], dtype="Int64"), + "nullable_bool": pd.Series([True, None], dtype="boolean"), + } + ) + + +def test_numeric_and_bool_columns_still_pass_through() -> None: + frame = _passthrough_frame() + report = analyze_dataset(frame) + assert report.audit["sample_masked_columns"] == [] + rows = report.model_context["sample_rows_masked"] + assert [r["i64"] for r in rows] == [123456, 654321] + assert [r["f64"] for r in rows] == [1.25, 2.5] + assert [r["flag"] for r in rows] == [True, False] + assert [r["nullable_int"] for r in rows] == [7, None] + assert [r["nullable_bool"] for r in rows] == [True, None] + + +def test_arrow_numeric_and_bool_columns_still_pass_through() -> None: + pa = _pa() + frame = pd.DataFrame( + { + "a_int": pd.array([123456, 654321], dtype=pd.ArrowDtype(pa.int64())), + "a_float": pd.array([1.25, 2.5], dtype=pd.ArrowDtype(pa.float64())), + "a_bool": pd.array([True, False], dtype=pd.ArrowDtype(pa.bool_())), + } + ) + for column in frame.columns: + assert _passes_through_raw(frame[column].dtype), column + assert _sample_mask_columns(frame, [], []) == [] + rows = _mask_sample(frame, [], 5) + assert [r["a_int"] for r in rows] == [123456, 654321] + assert [r["a_bool"] for r in rows] == [True, False] + + +@pytest.mark.parametrize( + "dtype", + [ + object, + "string", + pd.CategoricalDtype(["a"]), + pd.CategoricalDtype([1, 2]), + "datetime64[ns]", + pd.DatetimeTZDtype(tz="UTC"), + "timedelta64[ns]", + pd.PeriodDtype("D"), + pd.IntervalDtype("int64"), + ], + ids=str, +) +def test_passes_through_raw_rejects_non_numeric_dtypes(dtype) -> None: + assert not _passes_through_raw(pd.Series([], dtype=dtype).dtype) + + +@pytest.mark.parametrize( + "dtype", ["int8", "uint64", "float32", "complex128", "bool", "Int64", "Float64", "boolean"] +) +def test_passes_through_raw_accepts_numeric_and_bool(dtype) -> None: + assert _passes_through_raw(pd.Series([], dtype=dtype).dtype) + + +def test_declared_numeric_column_is_still_masked() -> None: + df = pd.DataFrame({"ssn": [123456789, 987654321], "v": [1, 2]}) + report, prompt = _run_with_prompt(df, sensitive_columns=["ssn"]) + assert report.audit["sample_masked_columns"] == ["ssn"] + _assert_absent(report, prompt, ["123456789", "987654321"]) + + +def test_allow_unmasked_columns_opts_out_non_string_dtypes() -> None: + df = pd.DataFrame( + {"when": pd.to_datetime(["1980-02-03", "1975-06-07"]), "v": [1, 2]} + ) + masked = analyze_dataset(df) + assert "1980-02-03" not in json.dumps(masked.model_context, default=str) + allowed = analyze_dataset(df, allow_unmasked_columns=("when",)) + assert allowed.audit["sample_masked_columns"] == [] + assert "1980-02-03" in json.dumps(allowed.model_context, default=str) From 0ede98664c787f5995e279fc46bf17c6628a38d5 Mon Sep 17 00:00:00 2001 From: Kevin Costner <120246174+kevincostner17@users.noreply.github.com> Date: Tue, 15 Sep 2026 18:56:07 +0530 Subject: [PATCH 11/17] fix(copilot): mask sample values by column position analyze_dataset matched masking rules and the declared mask set by str(label). A must_mask or sensitive_columns entry whose column label was not a str (an int from read_csv(header=None), a float, a tuple) never matched, so the declared column was dropped from the mask set and its raw values reached model_context and the provider prompt, while the audit still listed it as masked. Default-mode masking of int labels only worked by accident of snake_case name matching. Sample masking now runs on a copy whose columns are renamed __c0, __c1, ... Every mask-set entry is resolved to a position by its real label or str(label), and the masked rows are mapped back to str(label) keys. - Labels that collide once converted to str (0 and "0") raise ValueError. - Unknown sensitive_columns raise ValueError, like allow_unmasked_columns. - Masking fails closed: every non-missing value of a selected column must be a 16-hex hash token, or RuntimeError is raised with no values in the message. The check runs before free-text detection, which now scans only the unmasked columns and so can no longer rewrite a token. --- src/freshdata/experimental/ai_copilot.py | 161 +++++++++++--- tests/test_copilot_positional_masking.py | 210 +++++++++++++++++++ tests/test_copilot_sample_dtype_allowlist.py | 10 +- 3 files changed, 350 insertions(+), 31 deletions(-) create mode 100644 tests/test_copilot_positional_masking.py diff --git a/src/freshdata/experimental/ai_copilot.py b/src/freshdata/experimental/ai_copilot.py index 6051b006..2b9dea1e 100644 --- a/src/freshdata/experimental/ai_copilot.py +++ b/src/freshdata/experimental/ai_copilot.py @@ -65,7 +65,7 @@ from ..enterprise.cleaner import cluster_column from ..enterprise.config import ClusterConfig, MaskingRule from ..enterprise.metrics import TrustScore, compute_trust_score -from ..enterprise.privacy import PIIDetectionConfig, anonymize, detect_pii +from ..enterprise.privacy import PIIDetectionConfig, _is_missing_scalar, anonymize, detect_pii from ..render.mixins import HtmlReprMixin __all__ = [ @@ -375,33 +375,123 @@ def _passes_through_raw(dtype: object) -> bool: return False -def _sample_mask_columns( - frame: pd.DataFrame, mask_columns: Sequence[str], allow_unmasked: Sequence[str] -) -> list[str]: - """Columns to hash-mask in sample rows: every declared/detected PII column - *and* every column whose dtype is not numeric or boolean — regex PII - detection cannot see names, addresses, free text or dates of birth, so - only numeric and boolean values are sent raw. ``allow_unmasked`` exempts - specific columns but never a declared or detected PII column. +def _require_unique_label_strings(labels: Sequence[Any]) -> None: + """Raise unless every column label is unique once converted to ``str``. + + Sample rows, ``context_policy``, ``sensitive_columns`` and the audit all + name columns by ``str(label)``; two labels such as ``0`` and ``"0"`` would + make a column ambiguous, so it could be masked under one name and shown + under the other. + """ + counts: dict[str, int] = {} + for label in labels: + counts[str(label)] = counts.get(str(label), 0) + 1 + colliding = sorted(key for key, n in counts.items() if n > 1) + if colliding: + raise ValueError( + "analyze_dataset requires column labels that stay unique when " + f"converted to str; colliding: {colliding}" + ) + + +def _label_position(labels: Sequence[Any], name: Any) -> int | None: + """Position of the column *name* refers to: the label itself or ``str(label)``.""" + key = str(name) + for position, label in enumerate(labels): + if str(label) == key: + return position + for position, label in enumerate(labels): + try: + if bool(label == name): + return position + except (TypeError, ValueError): + continue + return None + + +def _sample_mask_positions( + frame: pd.DataFrame, mask_columns: Sequence[Any], allow_unmasked: Sequence[Any] +) -> list[int]: + """Positions of the columns to hash-mask in sample rows: every + declared/detected PII column *and* every column whose dtype is not numeric + or boolean — regex PII detection cannot see names, addresses, free text or + dates of birth, so only numeric and boolean values are sent raw. + ``allow_unmasked`` exempts specific columns but never a declared or + detected PII column. Names match a column's label or ``str(label)``. + """ + labels = list(frame.columns) + declared = {p for p in (_label_position(labels, c) for c in mask_columns) if p is not None} + allowed = {p for p in (_label_position(labels, c) for c in allow_unmasked) if p is not None} + not_raw = {i for i, dtype in enumerate(frame.dtypes) if not _passes_through_raw(dtype)} + return sorted(declared | (not_raw - allowed)) + + +#: Hex length of a copilot sample token; the fail-closed check relies on it. +_MASK_HASH_LENGTH = 16 +_MASK_TOKEN = re.compile(rf"[0-9a-f]{{{_MASK_HASH_LENGTH}}}") + + +def _positional_name(position: int) -> str: + return f"__c{position}" + + +def _verify_masked(sample: pd.DataFrame, positions: Sequence[int]) -> None: + """Fail closed unless every non-missing value in *positions* is a hash token. + + The error names only the column position, never a value. """ - declared = {c for c in mask_columns if c in frame.columns} - not_raw = {c for c in frame.columns if not _passes_through_raw(frame[c].dtype)} - return sorted(declared | (not_raw - set(allow_unmasked)), key=str) + for position in positions: + name = _positional_name(position) + values = sample[name] if name in sample.columns else None + if values is None or any( + not _is_missing_scalar(value) + and not (isinstance(value, str) and _MASK_TOKEN.fullmatch(value)) + for value in values + ): + raise RuntimeError( + f"copilot sample masking failed closed: the column at position {position} " + "was not hash-masked, so no model_context was built" + ) def _mask_sample( - frame: pd.DataFrame, columns: Sequence[str], sample_rows: int + frame: pd.DataFrame, positions: Sequence[int], sample_rows: int ) -> list[dict[str, Any]]: - sample = frame.head(sample_rows) - rules = tuple( - MaskingRule(name=f"copilot_mask_{c}", columns=(str(c),), strategy="hash") for c in columns - ) - masked = anonymize( - sample, rules=rules, detection_config=PIIDetectionConfig(), return_report=False - ) + """Mask the first *sample_rows* rows by column position and key them by ``str(label)``. + + Masking runs on a copy whose columns are renamed ``__c0``, ``__c1``, ..., + so a label of any type (int, float, tuple) is masked, and a rule can never + select a different column whose name merely normalises alike. + """ + labels = list(frame.columns) + sample = frame.head(sample_rows).copy() + sample.columns = pd.Index([_positional_name(i) for i in range(len(labels))]) + if positions: + rules = tuple( + MaskingRule( + name=f"copilot_mask_{_positional_name(p)}", + columns=(_positional_name(p),), + strategy="hash", + hash_length=_MASK_HASH_LENGTH, + strict=True, + ) + for p in positions + ) + sample = anonymize(sample, rules=rules, return_report=False) + # Checked before free-text detection, which may rewrite a token that + # happens to look like a card number. + _verify_masked(sample, positions) + masked = set(positions) + raw_names = [_positional_name(i) for i in range(len(labels)) if i not in masked] + if raw_names: + scrubbed = anonymize( + sample[raw_names], detection_config=PIIDetectionConfig(), return_report=False + ) + for name in raw_names: + sample[name] = scrubbed[name] return [ - {str(k): _json_scalar(v) for k, v in record.items()} - for record in masked.to_dict(orient="records") + {str(label): _json_scalar(record[_positional_name(i)]) for i, label in enumerate(labels)} + for record in sample.to_dict(orient="records") ] @@ -806,13 +896,31 @@ def analyze_dataset( in the sample rows. Declared (``must_mask``) and regex-detected PII columns are always masked regardless. Unknown column names raise ``ValueError``. + sensitive_columns: + Columns that are always masked in the sample rows and get a + ``MaskingRule`` in ``recommended_code``, whatever their dtype (for + example an SSN stored as an integer). Unknown column names raise + ``ValueError``. + + Column names in ``context_policy``, ``allow_unmasked_columns`` and + ``sensitive_columns`` match a column's label or ``str(label)``, so + integer, float and tuple labels work. Sample masking is done by column + position and fails closed with ``RuntimeError`` if a selected column was + not masked. Labels that collide once converted to ``str`` (``0`` and + ``"0"``) raise ``ValueError``. """ if privacy not in _PRIVACY_MODES: raise ValueError(f"privacy must be one of {_PRIVACY_MODES}, got {privacy!r}") frame = to_pandas(df) - unknown = [str(c) for c in allow_unmasked_columns if c not in frame.columns] + labels = list(frame.columns) + _require_unique_label_strings(labels) + unknown = [str(c) for c in allow_unmasked_columns if _label_position(labels, c) is None] if unknown: raise ValueError(f"allow_unmasked_columns contains unknown column(s): {unknown}") + sensitive_positions = [_label_position(labels, c) for c in sensitive_columns] + unknown = [str(c) for c, p in zip(sensitive_columns, sensitive_positions) if p is None] + if unknown: + raise ValueError(f"sensitive_columns contains unknown column(s): {unknown}") intent = _parse_context_policy(context_policy) prof = _profile_frame(frame) @@ -837,7 +945,7 @@ def analyze_dataset( # Declared-sensitive columns always join the mask set: pattern-based PII # detection cannot recognise every sensitive token (an internal case ID, # a synthetic SSN), so the caller's declaration is authoritative. - declared_sensitive = [str(c) for c in sensitive_columns if str(c) in df.columns] + declared_sensitive = [str(labels[p]) for p in sensitive_positions if p is not None] mask_for_code = sorted( dict.fromkeys([*intent.mask_columns, *pii_columns, *declared_sensitive]) ) @@ -870,8 +978,9 @@ def analyze_dataset( } sample_mask: list[str] = [] if privacy == "mask_pii_before_reasoning" and sample_rows > 0: - sample_mask = _sample_mask_columns(frame, mask_for_code, allow_unmasked_columns) - model_context["sample_rows_masked"] = _mask_sample(frame, sample_mask, sample_rows) + sample_positions = _sample_mask_positions(frame, mask_for_code, allow_unmasked_columns) + sample_mask = sorted(str(labels[p]) for p in sample_positions) + model_context["sample_rows_masked"] = _mask_sample(frame, sample_positions, sample_rows) # --- optional provider hook (experimental) ---------------------------------- engine = "deterministic-local" diff --git a/tests/test_copilot_positional_masking.py b/tests/test_copilot_positional_masking.py new file mode 100644 index 00000000..825277ea --- /dev/null +++ b/tests/test_copilot_positional_masking.py @@ -0,0 +1,210 @@ +"""Copilot sample masking works by column position, whatever the label type. + +Regression tests for raw sample values reaching the provider prompt when +column labels are not ``str``: masking rules and the declared mask set were +matched by ``str(label)``, so declared ``must_mask`` / ``sensitive_columns`` +entries with int, float or tuple labels were dropped from the mask set. +""" + +from __future__ import annotations + +import html +import json + +import pandas as pd +import pytest + +from freshdata.enterprise import privacy as privacy_module +from freshdata.experimental import ai_copilot +from freshdata.experimental.ai_copilot import analyze_dataset + +NAMES = ("Alice Johnson", "Bob Smith") +SSNS = ("123456789", "987654321") + + +def _run_with_prompt(frame: pd.DataFrame, **kwargs): + prompts: list[str] = [] + + def provider(prompt: str) -> str: + prompts.append(prompt) + return "ok" + + with pytest.warns(FutureWarning, match="experimental"): + report = analyze_dataset(frame, provider=provider, **kwargs) + assert len(prompts) == 1 + return report, prompts[0] + + +def _assert_no_raw(report, prompt: str, raws) -> None: + rendered = report._repr_html_() + sinks = { + "prompt": prompt, + "model_context": json.dumps(report.model_context, default=str, ensure_ascii=False), + "to_json": report.to_json(), + "html": rendered, + "html_unescaped": html.unescape(rendered), + "str": str(report), + } + for sink, text in sinks.items(): + leaked = [raw for raw in raws if raw in text] + assert not leaked, f"raw value(s) {leaked} reached {sink}" + + +def _frame(labels) -> pd.DataFrame: + columns = pd.Index(list(labels), tupleize_cols=False) + return pd.DataFrame( + [[NAMES[0], int(SSNS[0])], [NAMES[1], int(SSNS[1])]], columns=columns + ) + + +def test_advisory_poc_int_labels_default_mode() -> None: + df = pd.DataFrame({0: list(NAMES), 1: [int(s) for s in SSNS]}) + report, prompt = _run_with_prompt(df) + assert [n for n in NAMES if n in prompt] == [] + assert report.audit["sample_masked_columns"] == ["0"] + _assert_no_raw(report, prompt, NAMES) + + +def test_advisory_poc_int_label_must_mask_and_sensitive_columns() -> None: + df = pd.DataFrame({0: list(NAMES), 1: [int(s) for s in SSNS]}) + report, prompt = _run_with_prompt( + df, context_policy={1: "must_mask"}, sensitive_columns=[1] + ) + assert "123456789" not in prompt + assert report.audit["sample_masked_columns"] == ["0", "1"] + assert "1" in report.audit["masked_columns"] + _assert_no_raw(report, prompt, NAMES + SSNS) + + +@pytest.mark.parametrize( + "labels", + [ + (0, 1), + (1.5, 2.5), + (("person", "name"), ("person", "ssn")), + ], + ids=["int", "float", "tuple"], +) +@pytest.mark.parametrize("declare", ["sensitive_columns", "must_mask"]) +def test_declared_numeric_column_is_masked_for_any_label_type(labels, declare) -> None: + df = _frame(labels) + ssn_label = labels[1] + kwargs = ( + {"sensitive_columns": [ssn_label]} + if declare == "sensitive_columns" + else {"context_policy": {ssn_label: "must_mask"}} + ) + report, prompt = _run_with_prompt(df, **kwargs) + assert report.audit["sample_masked_columns"] == sorted(str(label) for label in labels) + rows = report.model_context["sample_rows_masked"] + assert [sorted(row) for row in rows] == [sorted(str(label) for label in labels)] * 2 + _assert_no_raw(report, prompt, NAMES + SSNS) + + +def test_multiindex_columns_are_masked_positionally() -> None: + columns = pd.MultiIndex.from_tuples([("person", "name"), ("person", "ssn")]) + df = pd.DataFrame([[NAMES[0], 123456789], [NAMES[1], 987654321]], columns=columns) + report, prompt = _run_with_prompt(df, sensitive_columns=[("person", "ssn")]) + assert len(report.audit["sample_masked_columns"]) == 2 + _assert_no_raw(report, prompt, NAMES + SSNS) + + +def test_string_form_of_a_non_str_label_is_accepted() -> None: + df = pd.DataFrame({0: list(NAMES), 1: [int(s) for s in SSNS]}) + report, prompt = _run_with_prompt(df, sensitive_columns=["1"]) + assert report.audit["sample_masked_columns"] == ["0", "1"] + _assert_no_raw(report, prompt, SSNS) + + +def test_allow_unmasked_columns_matches_non_str_labels() -> None: + df = pd.DataFrame({0: ["Paris", "Lyon"], 1: [1, 2]}) + for name in (0, "0"): + report = analyze_dataset(df, allow_unmasked_columns=[name]) + assert report.audit["sample_masked_columns"] == [] + assert "Paris" in json.dumps(report.model_context) + + +@pytest.mark.parametrize("privacy", ["mask_pii_before_reasoning", "schema_only"]) +def test_labels_colliding_as_strings_raise(privacy) -> None: + df = pd.DataFrame([[NAMES[0], NAMES[1]]], columns=pd.Index([0, "0"], dtype=object)) + with pytest.raises(ValueError, match="unique when converted to str") as info: + analyze_dataset(df, privacy=privacy) + assert not [n for n in NAMES if n in str(info.value)] + + +def test_unknown_sensitive_columns_raise() -> None: + df = pd.DataFrame({"a": [1, 2], 3: ["x", "y"]}) + with pytest.raises(ValueError, match="sensitive_columns contains unknown column"): + analyze_dataset(df, sensitive_columns=["nope"]) + with pytest.raises(ValueError, match="sensitive_columns contains unknown column"): + analyze_dataset(df, sensitive_columns=[4]) + analyze_dataset(df, sensitive_columns=[3, "a"]) + + +def test_masking_that_changes_nothing_fails_closed(monkeypatch) -> None: + def no_op_anonymize(df, **kwargs): + return df.copy() + + monkeypatch.setattr(ai_copilot, "anonymize", no_op_anonymize) + df = pd.DataFrame({0: list(NAMES), 1: [int(s) for s in SSNS]}) + with pytest.raises(RuntimeError, match="failed closed") as info: + analyze_dataset(df, sensitive_columns=[1]) + message = str(info.value) + assert not [raw for raw in NAMES + SSNS if raw in message] + + +def test_masking_that_skips_one_column_fails_closed(monkeypatch) -> None: + real_anonymize = ai_copilot.anonymize + + def skip_last_rule(df, *, rules=(), **kwargs): + if rules: + rules = rules[:-1] + if not rules: + return df.copy() + return real_anonymize(df, rules=rules, **kwargs) + + monkeypatch.setattr(ai_copilot, "anonymize", skip_last_rule) + df = pd.DataFrame({"name": list(NAMES), "ssn": [int(s) for s in SSNS]}) + with pytest.raises(RuntimeError, match="position 1"): + analyze_dataset(df, sensitive_columns=["ssn"]) + + +def test_missing_values_in_masked_columns_do_not_trip_the_check() -> None: + df = pd.DataFrame( + { + 0: [NAMES[0], None, NAMES[1]], + 1: pd.Series([123456789, None, 987654321], dtype="Int64"), + 2: pd.to_datetime(["1980-02-03", None, "1975-06-07"]), + } + ) + report = analyze_dataset(df, sensitive_columns=[1]) + rows = report.model_context["sample_rows_masked"] + assert [row["1"] is None for row in rows] == [False, True, False] + assert [row["2"] is None for row in rows] == [False, True, False] + + +def test_detection_never_rewrites_a_hash_token(monkeypatch) -> None: + # A token that happens to look like a Luhn-valid card number must stay a + # token: free-text PII detection runs only on unmasked columns. + monkeypatch.setattr( + privacy_module, "_hash_value", lambda value, salt, length: "4111111111111111" + ) + df = pd.DataFrame({"note": list(NAMES), "v": [1, 2]}) + report = analyze_dataset(df) + assert [row["note"] for row in report.model_context["sample_rows_masked"]] == [ + "4111111111111111" + ] * 2 + + +def test_detection_pass_covers_exactly_the_unmasked_columns(monkeypatch) -> None: + real_anonymize = ai_copilot.anonymize + calls: list[tuple[list[str], int, bool]] = [] + + def spy(df, *, rules=(), detection_config=None, **kwargs): + calls.append(([str(c) for c in df.columns], len(rules), detection_config is not None)) + return real_anonymize(df, rules=rules, detection_config=detection_config, **kwargs) + + monkeypatch.setattr(ai_copilot, "anonymize", spy) + df = pd.DataFrame({"city": ["Paris", "Lyon"], "note": list(NAMES), "v": [1, 2]}) + analyze_dataset(df, allow_unmasked_columns=["city"]) + assert calls == [(["__c0", "__c1", "__c2"], 1, False), (["__c0", "__c2"], 0, True)] diff --git a/tests/test_copilot_sample_dtype_allowlist.py b/tests/test_copilot_sample_dtype_allowlist.py index 1c87902c..e6d9c19f 100644 --- a/tests/test_copilot_sample_dtype_allowlist.py +++ b/tests/test_copilot_sample_dtype_allowlist.py @@ -17,7 +17,7 @@ from freshdata.experimental.ai_copilot import ( _mask_sample, _passes_through_raw, - _sample_mask_columns, + _sample_mask_positions, analyze_dataset, ) @@ -140,9 +140,9 @@ def test_arrow_list_of_strings_is_masked_in_sample() -> None: } ) assert not _passes_through_raw(frame["tags"].dtype) - columns = _sample_mask_columns(frame, [], []) - assert columns == ["tags"] - blob = json.dumps(_mask_sample(frame, columns, 5), default=str) + positions = _sample_mask_positions(frame, [], []) + assert positions == [0] + blob = json.dumps(_mask_sample(frame, positions, 5), default=str) assert not [n for n in NAMES if n in blob] @@ -181,7 +181,7 @@ def test_arrow_numeric_and_bool_columns_still_pass_through() -> None: ) for column in frame.columns: assert _passes_through_raw(frame[column].dtype), column - assert _sample_mask_columns(frame, [], []) == [] + assert _sample_mask_positions(frame, [], []) == [] rows = _mask_sample(frame, [], 5) assert [r["a_int"] for r in rows] == [123456, 654321] assert [r["a_bool"] for r in rows] == [True, False] From ac9a1d3a885ddd9e8e3ce70b30b3c7f6fe739d9b Mon Sep 17 00:00:00 2001 From: Kevin Costner <120246174+kevincostner17@users.noreply.github.com> Date: Tue, 15 Sep 2026 19:03:14 +0530 Subject: [PATCH 12/17] feat(copilot): add mask_salt for a reproducible model_context Copilot sample masking built hash rules without a salt, so each rule got a random salt and the masked sample rows and audit["model_context_sha256"] changed on every run, contrary to the documented reproducibility (#288). analyze_dataset(mask_salt=...) derives each column's hash salt as HMAC-SHA256(mask_salt, "copilot-col:"). The same salt reproduces model_context and its fingerprint, and equal values in different columns get different tokens. The default (None) stays a random per-run key. The salt is never written to the report; audit["mask_salt_source"] records "caller" or "per-run-random". An empty or non-str salt raises. The TruthBench copilot adapter pins mask_salt so its sinks are identical across runs and repeats. Unknown-column checks move into a small helper. --- benchmarks/truthbench/surfaces/copilot.py | 9 +- src/freshdata/experimental/ai_copilot.py | 66 +++++++++++--- tests/test_copilot_mask_salt.py | 101 ++++++++++++++++++++++ 3 files changed, 162 insertions(+), 14 deletions(-) create mode 100644 tests/test_copilot_mask_salt.py diff --git a/benchmarks/truthbench/surfaces/copilot.py b/benchmarks/truthbench/surfaces/copilot.py index 1994bc6c..27f689a9 100644 --- a/benchmarks/truthbench/surfaces/copilot.py +++ b/benchmarks/truthbench/surfaces/copilot.py @@ -14,6 +14,10 @@ from ..privacy import SinkScanner from .base import ExceptionDetails, SurfaceAdapter, SurfaceObservation, register_adapter +#: Pinned so masked sample tokens, and so every rendered sink, are identical +#: across runs and repeats instead of depending on a per-run random key. +COPILOT_MASK_SALT = "truthbench-fixed-copilot-mask-salt" + class CopilotAdapter(SurfaceAdapter): """Run the deterministic Copilot path and retain all report sinks safely.""" @@ -35,7 +39,10 @@ def observe(self, fixture: Any, context: Any) -> SurfaceObservation: ) with contextlib.redirect_stdout(stdout), contextlib.redirect_stderr(stderr): report = analyze_dataset( - frame, provider=None, sensitive_columns=sensitive + frame, + provider=None, + sensitive_columns=sensitive, + mask_salt=COPILOT_MASK_SALT, ) # The prompt is constructed exactly as a provider call would see it, # even though this adapter intentionally supplies no provider. diff --git a/src/freshdata/experimental/ai_copilot.py b/src/freshdata/experimental/ai_copilot.py index 2b9dea1e..cf264b45 100644 --- a/src/freshdata/experimental/ai_copilot.py +++ b/src/freshdata/experimental/ai_copilot.py @@ -10,8 +10,10 @@ **Deterministic and offline by default.** The analysis is rule-based and built entirely from freshdata's own primitives (profiling, PII detection, context policies, value clustering, trust scoring). The same input always -produces the same report, no API key or network access is required, and -results are reproducible in CI. +produces the same findings, plan and code, no API key or network access is +required, and results are reproducible in CI. Masked sample tokens use a +per-run key unless ``mask_salt`` is passed, so ``model_context`` and its +fingerprint are reproducible only with a pinned salt. **Privacy-first.** Raw cell values never enter the report's ``model_context`` (the payload an LLM provider *would* see). Samples are @@ -47,9 +49,11 @@ from __future__ import annotations import hashlib +import hmac import json import math import re +import secrets import warnings from collections.abc import Callable, Mapping, Sequence from dataclasses import dataclass, field @@ -409,6 +413,15 @@ def _label_position(labels: Sequence[Any], name: Any) -> int | None: return None +def _resolve_positions(labels: Sequence[Any], names: Sequence[Any], argument: str) -> list[int]: + """Positions of *names* in *labels*; raise ``ValueError`` naming any unknown ones.""" + positions = [_label_position(labels, name) for name in names] + unknown = [str(name) for name, p in zip(names, positions) if p is None] + if unknown: + raise ValueError(f"{argument} contains unknown column(s): {unknown}") + return [p for p in positions if p is not None] + + def _sample_mask_positions( frame: pd.DataFrame, mask_columns: Sequence[Any], allow_unmasked: Sequence[Any] ) -> list[int]: @@ -454,15 +467,27 @@ def _verify_masked(sample: pd.DataFrame, positions: Sequence[int]) -> None: ) +def _column_salt(key: bytes, position: int) -> str: + """Per-column hash salt: ``HMAC-SHA256(key, "copilot-col:")`` as hex.""" + return hmac.new(key, f"copilot-col:{position}".encode(), hashlib.sha256).hexdigest() + + def _mask_sample( - frame: pd.DataFrame, positions: Sequence[int], sample_rows: int + frame: pd.DataFrame, + positions: Sequence[int], + sample_rows: int, + salt_key: bytes | None = None, ) -> list[dict[str, Any]]: """Mask the first *sample_rows* rows by column position and key them by ``str(label)``. Masking runs on a copy whose columns are renamed ``__c0``, ``__c1``, ..., so a label of any type (int, float, tuple) is masked, and a rule can never - select a different column whose name merely normalises alike. + select a different column whose name merely normalises alike. Each column + is salted from *salt_key* and its position, so equal values in different + columns get different tokens. Without *salt_key* a random per-call key is + used. """ + key = secrets.token_bytes(32) if salt_key is None else salt_key labels = list(frame.columns) sample = frame.head(sample_rows).copy() sample.columns = pd.Index([_positional_name(i) for i in range(len(labels))]) @@ -472,6 +497,7 @@ def _mask_sample( name=f"copilot_mask_{_positional_name(p)}", columns=(_positional_name(p),), strategy="hash", + salt=_column_salt(key, p), hash_length=_MASK_HASH_LENGTH, strict=True, ) @@ -849,6 +875,7 @@ def analyze_dataset( source_hint: str = "your_data.csv", allow_unmasked_columns: Sequence[str] = (), sensitive_columns: Sequence[str] = (), + mask_salt: str | None = None, ) -> CopilotReport: """Analyze *df* and return an explainable, privacy-safe :class:`CopilotReport`. @@ -901,6 +928,16 @@ def analyze_dataset( ``MaskingRule`` in ``recommended_code``, whatever their dtype (for example an SSN stored as an integer). Unknown column names raise ``ValueError``. + mask_salt: + Secret that makes the masked sample rows, and so ``model_context`` + and ``audit["model_context_sha256"]``, reproducible across runs. + Each column's hash salt is ``HMAC-SHA256(mask_salt, "copilot-col:")``, + so equal values in different columns get different tokens. By default + (``None``) a random per-run key is used and tokens differ on every + run. Anyone holding the salt can confirm guesses of low-cardinality + values, so keep it secret; it is never written to the report, and + ``audit["mask_salt_source"]`` records only ``"caller"`` or + ``"per-run-random"``. Column names in ``context_policy``, ``allow_unmasked_columns`` and ``sensitive_columns`` match a column's label or ``str(label)``, so @@ -911,16 +948,15 @@ def analyze_dataset( """ if privacy not in _PRIVACY_MODES: raise ValueError(f"privacy must be one of {_PRIVACY_MODES}, got {privacy!r}") + if mask_salt is not None and not isinstance(mask_salt, str): + raise TypeError(f"mask_salt must be a str or None, got {type(mask_salt).__name__}") + if mask_salt == "": + raise ValueError("mask_salt must be a non-empty str, or None for a per-run key") frame = to_pandas(df) labels = list(frame.columns) _require_unique_label_strings(labels) - unknown = [str(c) for c in allow_unmasked_columns if _label_position(labels, c) is None] - if unknown: - raise ValueError(f"allow_unmasked_columns contains unknown column(s): {unknown}") - sensitive_positions = [_label_position(labels, c) for c in sensitive_columns] - unknown = [str(c) for c, p in zip(sensitive_columns, sensitive_positions) if p is None] - if unknown: - raise ValueError(f"sensitive_columns contains unknown column(s): {unknown}") + _resolve_positions(labels, allow_unmasked_columns, "allow_unmasked_columns") + sensitive_positions = _resolve_positions(labels, sensitive_columns, "sensitive_columns") intent = _parse_context_policy(context_policy) prof = _profile_frame(frame) @@ -945,7 +981,7 @@ def analyze_dataset( # Declared-sensitive columns always join the mask set: pattern-based PII # detection cannot recognise every sensitive token (an internal case ID, # a synthetic SSN), so the caller's declaration is authoritative. - declared_sensitive = [str(labels[p]) for p in sensitive_positions if p is not None] + declared_sensitive = [str(labels[p]) for p in sensitive_positions] mask_for_code = sorted( dict.fromkeys([*intent.mask_columns, *pii_columns, *declared_sensitive]) ) @@ -980,7 +1016,10 @@ def analyze_dataset( if privacy == "mask_pii_before_reasoning" and sample_rows > 0: sample_positions = _sample_mask_positions(frame, mask_for_code, allow_unmasked_columns) sample_mask = sorted(str(labels[p]) for p in sample_positions) - model_context["sample_rows_masked"] = _mask_sample(frame, sample_positions, sample_rows) + salt_key = mask_salt.encode("utf-8") if mask_salt is not None else None + model_context["sample_rows_masked"] = _mask_sample( + frame, sample_positions, sample_rows, salt_key + ) # --- optional provider hook (experimental) ---------------------------------- engine = "deterministic-local" @@ -1020,6 +1059,7 @@ def analyze_dataset( "pii_suppressed_date_like": found.pii_suppressed, "masked_columns": mask_for_code, "sample_masked_columns": sample_mask, + "mask_salt_source": "caller" if mask_salt is not None else "per-run-random", "allow_unmasked_columns": sorted(str(c) for c in allow_unmasked_columns), "policy_sentences": list(intent.sentences), "compiled_policy": found.compiled_policy_summary, diff --git a/tests/test_copilot_mask_salt.py b/tests/test_copilot_mask_salt.py new file mode 100644 index 00000000..87752f2c --- /dev/null +++ b/tests/test_copilot_mask_salt.py @@ -0,0 +1,101 @@ +"""``analyze_dataset(mask_salt=...)``: reproducible or per-run sample tokens (#288).""" + +from __future__ import annotations + +import hashlib +import hmac +import html +import json + +import pandas as pd +import pytest + +from freshdata.enterprise.cleaner import _hash_value +from freshdata.experimental.ai_copilot import analyze_dataset + +SALT = "Zq7-copilot-mask-salt-4f1e9b" + + +def _frame() -> pd.DataFrame: + return pd.DataFrame( + { + "name": ["Alice Johnson", "Bob Smith"], + "alias": ["Alice Johnson", "Carol White"], + "v": [1, 2], + } + ) + + +def _tokens(report) -> list[dict]: + return report.model_context["sample_rows_masked"] + + +def test_default_runs_use_a_fresh_key_per_run() -> None: + a, b = analyze_dataset(_frame()), analyze_dataset(_frame()) + assert _tokens(a) != _tokens(b) + assert a.audit["model_context_sha256"] != b.audit["model_context_sha256"] + assert a.audit["mask_salt_source"] == "per-run-random" + + +def test_same_mask_salt_reproduces_model_context_and_fingerprint() -> None: + a = analyze_dataset(_frame(), mask_salt=SALT) + b = analyze_dataset(_frame(), mask_salt=SALT) + assert _tokens(a) == _tokens(b) + assert a.model_context == b.model_context + assert a.audit["model_context_sha256"] == b.audit["model_context_sha256"] + assert a.audit["mask_salt_source"] == "caller" + other = analyze_dataset(_frame(), mask_salt=SALT + "-other") + assert other.audit["model_context_sha256"] != a.audit["model_context_sha256"] + + +def test_tokens_follow_the_documented_per_column_derivation() -> None: + report = analyze_dataset(_frame(), mask_salt=SALT) + for position, column in enumerate(["name", "alias"]): + column_salt = hmac.new( + SALT.encode("utf-8"), f"copilot-col:{position}".encode(), hashlib.sha256 + ).hexdigest() + expected = [_hash_value(v, column_salt, 16) for v in _frame()[column]] + assert [row[column] for row in _tokens(report)] == expected + + +def test_same_value_in_two_columns_gets_different_tokens() -> None: + row = _tokens(analyze_dataset(_frame(), mask_salt=SALT))[0] + assert row["name"] != row["alias"] + + +def test_mask_salt_never_appears_in_any_sink() -> None: + prompts: list[str] = [] + + def provider(prompt: str) -> str: + prompts.append(prompt) + return "ok" + + with pytest.warns(FutureWarning, match="experimental"): + report = analyze_dataset(_frame(), mask_salt=SALT, provider=provider) + rendered = report._repr_html_() + sinks = { + "prompt": prompts[0], + "model_context": json.dumps(report.model_context, default=str), + "audit": json.dumps(report.audit, default=str), + "to_json": report.to_json(), + "html": html.unescape(rendered), + "str": str(report), + "recommended_code": report.recommended_code, + } + for sink, text in sinks.items(): + assert SALT not in text, sink + assert SALT.encode().hex() not in text, sink + + +def test_mask_salt_source_is_recorded_without_sample_rows() -> None: + report = analyze_dataset(_frame(), privacy="schema_only", mask_salt=SALT) + assert "sample_rows_masked" not in report.model_context + assert report.audit["mask_salt_source"] == "caller" + + +@pytest.mark.parametrize( + ("salt", "error"), [("", ValueError), (b"bytes", TypeError), (7, TypeError)] +) +def test_invalid_mask_salt_is_rejected(salt, error) -> None: + with pytest.raises(error, match="mask_salt"): + analyze_dataset(_frame(), mask_salt=salt) From cb9080e3f8c161c211895e9f217e291a6ecbd9e5 Mon Sep 17 00:00:00 2001 From: Kevin Costner <120246174+kevincostner17@users.noreply.github.com> Date: Tue, 15 Sep 2026 19:03:14 +0530 Subject: [PATCH 13/17] docs(copilot): document positional masking and mask_salt - ai-copilot.md: non-str labels, sensitive_columns, fail-closed masking, per-run sample tokens and mask_salt; reproducibility claims now say the findings, plan and code are stable and model_context only with mask_salt. - threat-model.md section 3: positional masking, label collisions, unknown sensitive_columns, fail-closed check, per-run fingerprint; replace the stale CLAIM_REGISTRY reference with the tests that enforce the claim. - threat-model.md section 6: replace the copilot sentences, which described a deterministic path that no longer exists, with the per-run key and mask_salt behaviour. - trust-claims.md: the copilot claim is no longer in the CLAIM_REGISTRY; move it to the other claims table with its tests, and add mask_salt. --- docs/ai-copilot.md | 30 +++++++++++++++++++++++++++--- docs/threat-model.md | 37 ++++++++++++++++++++++++++++--------- docs/trust-claims.md | 10 ++++++---- 3 files changed, 61 insertions(+), 16 deletions(-) diff --git a/docs/ai-copilot.md b/docs/ai-copilot.md index 5a6d3982..6306c66b 100644 --- a/docs/ai-copilot.md +++ b/docs/ai-copilot.md @@ -41,7 +41,10 @@ Three properties make this different from "ask a chatbot about my data": - **Deterministic and offline.** The analysis is rule-based, built from freshdata's own primitives (profiling, PII detection, the context-policy compiler, value clustering, trust scoring). The same input always produces - the same report; it runs in CI with no API key and no network access. + the same findings, plan and code; it runs in CI with no API key and no + network access. Masked sample tokens use a per-run key unless you pass + `mask_salt`, so `model_context` and its fingerprint are reproducible only + with a pinned salt. - **Privacy-first.** Raw string values never enter `report.model_context` — the only payload an LLM provider would ever see. Every sample column that is not numeric or boolean is hash-masked first (numeric and boolean values @@ -109,7 +112,17 @@ The `privacy` parameter controls what goes into `report.model_context`: age) are the residual risk — drop such columns first or use `"schema_only"`. `allow_unmasked_columns=[...]` is an explicit per-column opt-out; it never - exempts a declared or detected PII column. + exempts a declared or detected PII column. `sensitive_columns=[...]` + declares columns that are always masked, whatever their dtype (an SSN + stored as an integer, an internal case ID). + Column names in `context_policy`, `sensitive_columns` and + `allow_unmasked_columns` match a column's label or its `str()` form, so + integer, float and tuple labels (e.g. from `read_csv(header=None)`) work; + unknown `sensitive_columns` / `allow_unmasked_columns` names raise, and so + do labels that collide once converted to `str` (`0` and `"0"`). Masking is + done by column position and fails closed: if a selected column does not + come back hash-masked, `analyze_dataset` raises `RuntimeError` instead of + building `model_context`. - `"schema_only"` — no cell values at all; only column names, dtypes, missing percentages, and aggregate statistics. @@ -128,6 +141,16 @@ Two details worth knowing: - `report.audit["model_context_sha256"]` fingerprints the exact payload a provider would have seen, so you can prove after the fact what was (and was not) shared. +- Masked sample tokens are HMAC-SHA256 hashes with a separate salt per + column, so equal values in two columns get different tokens. By default + the salts come from a random per-run key: the same frame gives different + tokens and a different `model_context_sha256` on every run. Pass + `mask_salt="..."` to derive the salts from your value instead, which makes + `model_context` and its fingerprint reproducible (useful in CI). Treat + that value as a secret, since anyone holding it can confirm guesses of + low-cardinality values; it is never written to the report, and + `report.audit["mask_salt_source"]` records only `"caller"` or + `"per-run-random"`. ## Plugging in an LLM (optional, experimental) @@ -183,7 +206,8 @@ every time. What the copilot (and freshdata underneath it) adds: which spellings are the same category — with severity and evidence; - an audit trail a reviewer can read (`CleanReport` actions with rationale, masked-context SHA, privacy events with HIPAA/GDPR tags); -- reproducibility: the same input produces the same report, plan, and code. +- reproducibility: the same input produces the same findings, plan, and + code, and with `mask_salt` the same `model_context` and fingerprint. ## Limitations and responsible use diff --git a/docs/threat-model.md b/docs/threat-model.md index da09c452..b5f1c5c8 100644 --- a/docs/threat-model.md +++ b/docs/threat-model.md @@ -63,13 +63,28 @@ fact what was shared. Per privacy mode: or dates of birth, so no non-numeric value is trusted to be safe. `allow_unmasked_columns` is an explicit per-column opt-out that never exempts a declared or detected PII column and rejects unknown - names. Detected-problem details entering `model_context` are value-free - in **every** mode (`category_noise` spelling previews stay local). + names. Masking works by column position, so integer, float and tuple + column labels are masked exactly like `str` labels: `must_mask`, + `sensitive_columns` and `allow_unmasked_columns` match a label or its + `str()` form, unknown `sensitive_columns` raise, and labels that collide + once converted to `str` (`0` and `"0"`) raise. Masking fails closed: if a + selected column does not come back as hash tokens, `analyze_dataset` + raises instead of building `model_context`. Detected-problem details + entering `model_context` are value-free in **every** mode + (`category_noise` spelling previews stay local). - **`schema_only`** — no cell values at all. -These guarantees are enforced by adversarial regression tests registered in -the `CLAIM_REGISTRY` (`tests/test_experimental_ai_copilot.py`), which CI -re-verifies against the README wording. +Sample tokens are keyed per run by default, so `model_context` and +`report.audit["model_context_sha256"]` differ between runs on the same +frame; `analyze_dataset(mask_salt=...)` makes them reproducible (see +boundary 6). + +These guarantees are enforced by adversarial regression tests in +`tests/test_experimental_ai_copilot.py`, `tests/test_privacy_adversarial.py`, +`tests/test_copilot_sample_dtype_allowlist.py` and +`tests/test_copilot_positional_masking.py`, which CI runs on every change. +The README no longer states this claim verbatim, so it is not part of the +README `CLAIM_REGISTRY` audit. **Residual risk (by design, documented):** numeric values pass through unmasked. Numeric quasi-identifiers — an exact salary plus age plus a @@ -116,10 +131,14 @@ these keyless paths used public constants, so keyless output from earlier releases can be reversed by enumerating candidate values: re-pseudonymise it with a secret key (see [Compliance](compliance.md#pseudonymisation-keys)). -The copilot's internal masking uses the default -deterministic path on purpose: a per-run random salt would break the -documented reproducibility of `model_context` and its audit fingerprint. -This trade-off is tracked as a roadmap item, not silently changed. +The copilot's sample masking derives a separate salt for each column from a +random per-run key, so masked sample tokens and +`report.audit["model_context_sha256"]` differ between runs on the same +frame. `analyze_dataset(mask_salt=...)` derives the column salts from your +value instead, which makes `model_context` and its fingerprint reproducible. +Treat that value as a secret: anyone holding it can confirm guesses of +low-cardinality sample values. It is never written to the report; +`report.audit["mask_salt_source"]` records only whether one was supplied. Report stand-ins for declared `sensitive_columns` are a separate case. They are the `[SENSITIVE:xxxxxxxx]` tokens in `CleanReport` warnings, coerced diff --git a/docs/trust-claims.md b/docs/trust-claims.md index ebaad302..28494569 100644 --- a/docs/trust-claims.md +++ b/docs/trust-claims.md @@ -1,9 +1,10 @@ # Trust claims — evidence map Every trust-relevant claim FreshData makes, mapped to the thing that proves -it. The three product-defining claims are additionally machine-enforced: the -`CLAIM_REGISTRY` (`benchmarks/cleanbench/reproducibility.py`) pins their -README wording verbatim to named tests, and CI fails if either side drifts. +it. The product-defining claims the README still states verbatim are +additionally machine-enforced: the `CLAIM_REGISTRY` +(`benchmarks/cleanbench/reproducibility.py`) pins their README wording to +named tests, and CI fails if either side drifts. ## Machine-enforced claims (CLAIM_REGISTRY) @@ -11,12 +12,13 @@ README wording verbatim to named tests, and CI fails if either side drifts. |---|---| | protected columns are never modified | `tests/test_semantic_cleaning.py::test_id_columns_protected`, CleanBench `T2.protected_column_violation_rate` | | nothing happens silently | `tests/test_semantic_cleaning.py::test_assist_records_without_mutating` | -| raw PII never enters the copilot's model context | 4 tests in `tests/test_experimental_ai_copilot.py`, incl. adversarial cases: undeclared string-like columns masked, `category_noise` previews withheld; `tests/test_copilot_sample_dtype_allowlist.py` covers every non-numeric dtype (Arrow string / dictionary / list, categorical, bytes, datetime, timedelta, period) across the prompt, `model_context`, JSON, HTML and text sinks | ## Other README / docs claims | Claim | Status | Evidence / boundary | |---|---|---| +| raw PII never enters the copilot's model context | **holds** | not in the `CLAIM_REGISTRY` (the README no longer states it verbatim). Tests: `tests/test_experimental_ai_copilot.py` and `tests/test_privacy_adversarial.py` (undeclared string-like columns masked, `category_noise` previews withheld); `tests/test_copilot_sample_dtype_allowlist.py` (every non-numeric dtype: Arrow string / dictionary / list, categorical, bytes, datetime, timedelta, period); `tests/test_copilot_positional_masking.py` (int, float and tuple labels, fail-closed masking); each across the prompt, `model_context`, JSON, HTML and text sinks. Boundary: numeric and boolean sample values pass through | +| copilot `model_context` fingerprint is reproducible | **holds with `mask_salt`** | `tests/test_copilot_mask_salt.py`; without `mask_salt` sample tokens and `model_context_sha256` are per-run | | 93% coverage gate enforced in CI | **holds** | `--cov-fail-under=93` in pyproject addopts; CI runs it on every PR (recent runs: 93.5%) | | Safe defaults (never imputes identifiers, never touches targets, no blind outlier removal) | **holds** | protected-column guard + role inference tests; `strict=True` escalates ambiguity to errors | | Fully offline; only network call is `fd.models.pull` | **holds** | default test gate is `-m "not online"`; no other network code paths | From 640bad24babec57d360e4aa25ac0f5dc34146649 Mon Sep 17 00:00:00 2001 From: Kevin Costner <120246174+kevincostner17@users.noreply.github.com> Date: Tue, 15 Sep 2026 19:03:14 +0530 Subject: [PATCH 14/17] test(truthbench): pin copilot canary scanning and sink determinism - A canary whose digits sit in one run inside a hex token still flags; the same digits split by letters inside a long leaf do not. - A hash token carrying canary digits is flagged at $.rendered.html on every run. - With the pinned mask_salt the copilot adapter renders identical HTML, prompt and model_context across seeded process randomness, with no leak (3 seeds in the default lane, 25 seeds on every domain under -m large). - An int/float/tuple label and object/string/Arrow/categorical dtype sweep finds no canary in rendered.html, the prompt or model_context. --- .../test_copilot_canary_determinism.py | 204 ++++++++++++++++++ 1 file changed, 204 insertions(+) create mode 100644 tests/truthbench/test_copilot_canary_determinism.py diff --git a/tests/truthbench/test_copilot_canary_determinism.py b/tests/truthbench/test_copilot_canary_determinism.py new file mode 100644 index 00000000..0ba3fc16 --- /dev/null +++ b/tests/truthbench/test_copilot_canary_determinism.py @@ -0,0 +1,204 @@ +"""Copilot TruthBench sinks are deterministic and the canary scanner is precise. + +A TruthBench release gate once reported a digit-only phone canary in the +copilot ``rendered.html`` sink intermittently. Two things could produce that: +the digit-only scanner joining digits from different tokens of one long HTML +leaf, and masked sample tokens that differ on every run. These tests pin both. +""" + +from __future__ import annotations + +import random +import re +import secrets +from datetime import datetime, timezone + +import pandas as pd +import pytest +from benchmarks.truthbench.fixtures import DOMAINS, build_fixture +from benchmarks.truthbench.privacy import SinkScanner +from benchmarks.truthbench.surfaces.copilot import CopilotAdapter + +from freshdata.enterprise import privacy as privacy_module +from freshdata.experimental import ai_copilot +from freshdata.experimental.ai_copilot import analyze_dataset + +PHONE = "555-0110" +PROSE = " ".join(["the quick brown fox jumps over the lazy dog"] * 40) + + +def _education(): + return build_fixture("education") + + +def _phone_canary_id(fixture) -> str: + return next(k for k, v in sorted(fixture.pii_canaries.items()) if v == PHONE) + + +def _digits(text: str) -> str: + return "".join(ch for ch in text if ch.isdigit()) + + +# (a) scanner precision on long leaves ---------------------------------------- + + +def test_contiguous_canary_digits_inside_a_hex_token_flag() -> None: + fixture = _education() + leaf = f"{PROSE} token 3fa5550110c9 {PROSE}" + # One numeric run carries every canary digit, so this is a genuine match + # (the scanner reports whichever normalised variant matches first). + assert any(_digits(PHONE) in run for run in re.findall(r"\d+", leaf)) + leaks = CopilotAdapter().scanner_for(fixture).scan({"html": leaf}) + assert _phone_canary_id(fixture) in {x.canary_id for x in leaks} + + +def test_canary_digits_split_by_letters_in_a_long_leaf_do_not_flag() -> None: + fixture = _education() + leaf = f"{PROSE} 3fa555b01c10 {PROSE}" + # The concatenated digit stream of the leaf does contain the canary; + # only digits within one numeric run may match it. + assert _digits(PHONE) in _digits(leaf) + assert CopilotAdapter().scanner_for(fixture).scan({"html": leaf}) == [] + + +# (b) a token carrying canary digits is caught every time --------------------- + + +def _all_fixture_scanner() -> SinkScanner: + canaries: dict[str, object] = {} + for domain in DOMAINS: + canaries.update(build_fixture(domain).pii_canaries) + return SinkScanner.from_canaries(canaries, key=b"copilot-determinism-test") + + +def test_hash_token_with_canary_digits_is_flagged_deterministically(monkeypatch) -> None: + monkeypatch.setattr( + privacy_module, "_hash_value", lambda value, salt, length: "ab5550110cdef012" + ) + scanner = _all_fixture_scanner() + phone_id = _phone_canary_id(_education()) + runs = [] + for _ in range(2): + observation = CopilotAdapter().observe(build_fixture("crm"), {}) + assert observation.unexpected_exception is None + leaks = scanner.scan(observation.audit_sinks) + runs.append(sorted((x.canary_id, x.variant, x.path) for x in leaks)) + assert runs[0] == runs[1] + paths = {path for canary, _, path in runs[0] if canary == phone_id} + assert "$.rendered.html" in paths + + +# (c) pinned mask_salt: rendered sinks do not depend on process randomness ---- + + +class _FrozenDatetime(datetime): + @classmethod + def now(cls, tz=None): # type: ignore[override] + return datetime(2026, 1, 2, 3, 4, 5, tzinfo=tz or timezone.utc) + + +def _seed_randomness(monkeypatch, seed: int) -> None: + rng = random.Random(seed) + + def token_bytes(nbytes: int | None = None) -> bytes: + n = 32 if nbytes is None else nbytes + return rng.getrandbits(8 * n).to_bytes(n, "big") + + monkeypatch.setattr(secrets, "token_bytes", token_bytes) + monkeypatch.setattr(secrets, "token_hex", lambda nbytes=None: token_bytes(nbytes).hex()) + + +def _rendered_across_seeds(monkeypatch, domain: str, seeds) -> None: + monkeypatch.setattr(ai_copilot, "datetime", _FrozenDatetime) + fixture = build_fixture(domain) + scanner = CopilotAdapter().scanner_for(fixture) + outputs = [] + for seed in seeds: + _seed_randomness(monkeypatch, seed) + observation = CopilotAdapter().observe(fixture, {}) + assert observation.unexpected_exception is None + sinks = observation.audit_sinks + assert scanner.scan(sinks) == [] + outputs.append((sinks["rendered"]["html"], sinks["prompt"], sinks["model_context"])) + assert all(output == outputs[0] for output in outputs[1:]) + + +def test_seeded_randomness_reaches_the_default_key(monkeypatch) -> None: + monkeypatch.setattr(ai_copilot, "datetime", _FrozenDatetime) + frame = _education().frame + htmls = [] + for seed in (11, 12, 11): + _seed_randomness(monkeypatch, seed) + htmls.append(analyze_dataset(frame)._repr_html_()) + assert htmls[0] == htmls[2] + assert htmls[0] != htmls[1] + + +def test_pinned_mask_salt_renders_identically_across_seeds(monkeypatch) -> None: + _rendered_across_seeds(monkeypatch, "education", seeds=(0, 1, 2)) + + +@pytest.mark.large +@pytest.mark.parametrize("domain", DOMAINS) +def test_pinned_mask_salt_renders_identically_across_many_seeds(monkeypatch, domain) -> None: + _rendered_across_seeds(monkeypatch, domain, seeds=range(25)) + + +# (d) label and dtype sweep --------------------------------------------------- + + +def _text_dtypes(): + dtypes = {"object": object, "string": "string", "categorical": "category"} + try: + import pyarrow as pa # noqa: PLC0415 + + dtypes["arrow-string"] = pd.ArrowDtype(pa.string()) + except ImportError: + pass + return dtypes + + +LABELS = { + "int": (0, 1, 2, 3), + "float": (0.5, 1.5, 2.5, 3.5), + "tuple": (("g", "phone"), ("g", "email"), ("g", "notes"), ("g", "n")), +} + + +@pytest.mark.parametrize("labels", sorted(LABELS)) +@pytest.mark.parametrize("dtype", sorted(_text_dtypes())) +def test_label_and_dtype_sweep_finds_no_leak(labels, dtype) -> None: + fixture = _education() + scanner = CopilotAdapter().scanner_for(fixture) + values = sorted(v for v in fixture.pii_canaries.values() if isinstance(v, str)) + assert PHONE in values + others = [v for v in values if v != PHONE][:2] + names = LABELS[labels] + frame = pd.DataFrame( + { + 0: pd.Series([PHONE, "n/a", "unknown"], dtype=_text_dtypes()[dtype]), + 1: pd.Series([others[0], "none", "missing"], dtype=_text_dtypes()[dtype]), + 2: pd.Series([others[1], "blank", "empty"], dtype=_text_dtypes()[dtype]), + 3: [5550110, 1, 2], + } + ) + frame.columns = pd.Index(list(names), tupleize_cols=False) + prompts: list[str] = [] + + def provider(prompt: str) -> str: + prompts.append(prompt) + return "ok" + + with pytest.warns(FutureWarning, match="experimental"): + report = analyze_dataset( + frame, + provider=provider, + sensitive_columns=[names[3]], + mask_salt="sweep-salt", + ) + sinks = { + "rendered": {"html": report._repr_html_()}, + "prompt": prompts[0], + "model_context": report.model_context, + } + assert scanner.scan(sinks) == [] From 50efe91c95c5d583699da61f21be4994df4a89af Mon Sep 17 00:00:00 2001 From: Kevin Costner <120246174+kevincostner17@users.noreply.github.com> Date: Tue, 15 Sep 2026 20:35:42 +0530 Subject: [PATCH 15/17] docs: changelog for the security fixes --- CHANGELOG.md | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ea193dcd..8b0faa66 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,52 @@ adheres to [Semantic Versioning](https://semver.org/). ## [Unreleased] +### Security +- GPX and SDMX parsing detects the document encoding (BOM, UTF-16/UTF-32 + prefixes, XML declaration) and checks with expat before reading, so DTD and + entity declarations are rejected in every encoding. Previously a UTF-16 + document bypassed the check and allowed entity expansion. +- CSV formula sanitising now covers every level of multi-row headers, and index + labels and names, so crafted header cells from the input are no longer written + as live formulas. +- The DuckDB engine no longer spills to the shared `/tmp/freshdata_spill`. + `EngineConfig.temp_directory` defaults to `None`, and each run spills into a + private (0700), per-run directory under the user's cache directory (or + `FRESHDATA_SPILL_DIR`), which is removed afterwards. An explicit + `temp_directory` is checked for ownership and permissions. +- Baseline category labels are no longer unkeyed SHA-1. Without `label_key` a + baseline stores a label-free frequency profile; with `label_key` (or + `FRESHDATA_BASELINE_KEY`) labels are HMAC-SHA256. Baselines are written as + schema `freshdata-baseline-v2`; v1 baselines still load with a warning and + should be rebuilt. +- `JsonTokenVault` and `SqliteTokenVault` create their files owner-only (0600) + at creation time. SQLite journal/WAL files inherit that mode. Existing + group/other-readable vault files trigger a warning. +- `tokenize`, `surrogate` and keyless `fpe` masking rules, and the policy + `pseudonymize` action (the default in the GDPR, HIPAA and FERPA packs), no + longer fall back to public constants when no key is set. They use a random + per-call key and emit `EphemeralKeyWarning`; pass `key=`/`key_env=` for + stable, joinable output. +- `fd.learn(privacy='mask')` treats every PII type `detect_pii` reports (payment + cards, IBANs, IP addresses, health and licence identifiers) as sensitive; + unknown types fail closed. It adds card and bank column-name hints. + `freshdata profile audit` flags raw card numbers and IBANs in existing + profiles. +- `clean_enterprise` reports no longer contain raw values of masked columns: + cluster canonical, variant and key values, semantic-validation invalid + samples, and `clean_report.coerced_cells` originals (and the coercion warnings + quoting them) for masked columns are masked or redacted. The + `[SENSITIVE:xxxxxxxx]` tokens that stand in for declared `sensitive_columns` + values are now a truncated HMAC-SHA256 under a random per-process key instead + of an unkeyed SHA-256; they still match within a run but differ between runs. +- Copilot sample masking uses an allow-list: only numeric and boolean sample + values pass through, so Arrow-backed string, dictionary and other non-numeric + columns (including datetimes) are hash-masked. +- Copilot masks sample values by column position, so integer, float and tuple + column labels no longer bypass masking. `sensitive_columns` and `must_mask` + match non-string labels, unknown `sensitive_columns` raise, labels that + collide as strings raise, and masking fails closed. + ### Added - `fd.clean_excel()`, the Excel companion to `fd.clean_csv()`: reads one sheet, cleans it, and optionally writes the result, with formula sanitization on by @@ -509,6 +555,12 @@ adheres to [Semantic Versioning](https://semver.org/). - The `quarantine` privacy-policy action works on nullable integer, boolean and categorical columns instead of raising `TypeError`; those columns come back as object dtype and missing cells stay missing. +- `detect_pii` and detection-driven `anonymize` scan categorical text columns on + pandas 1.5, as on pandas 2 (#280). +- Crypto FPE honours `visible` (#281). +- `analyze_dataset(mask_salt=...)` makes `model_context` and its fingerprint + reproducible; by default they are per-run, and `audit["mask_salt_source"]` + records which was used (#288). ## [2.0.0] - 2026-07-20 From 4b3cd9a3655069c784ce2245fc0ca590228bfba6 Mon Sep 17 00:00:00 2001 From: Kevin Costner <120246174+kevincostner17@users.noreply.github.com> Date: Tue, 15 Sep 2026 20:42:40 +0530 Subject: [PATCH 16/17] fixup(enterprise): don't apply strict column matching to report redaction Integration fix. The report redaction added for masked columns resolves each masking rule against the report's columns only (cluster, validation and coercion reports) to find which report entries to mask. main's #406 made _resolve_columns raise for listed columns that match nothing when the rule is strict, and CLI --mask rules are strict. A masked column that appears in no report (for example "email" without clustering hits) therefore raised "specifies column(s) not found in dataframe" from clean_enterprise, failing tests/test_enterprise_cli.py on the integrated branch. Pass strict=False for this subset lookup, as privacy.py already does for its duplicated-label lookup. The masking stage still enforces strict against the real frame. Adds regression tests for both sides. --- src/freshdata/enterprise/interface.py | 5 ++++- tests/test_enterprise_report_masking.py | 20 ++++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/src/freshdata/enterprise/interface.py b/src/freshdata/enterprise/interface.py index 15d8fdc8..9d426109 100644 --- a/src/freshdata/enterprise/interface.py +++ b/src/freshdata/enterprise/interface.py @@ -285,7 +285,10 @@ def _masked_report_columns(result: EnterpriseResult, ec: EnterpriseConfig) -> di ) rules: dict[str, list[MaskingRule]] = {} for rule in ec.masking: - for column in _resolve_columns(rule, candidates): + # Only report columns are searched here, so a listed column that is + # absent from them is expected; the masking stage already enforced + # ``strict`` against the frame. Never raise for it. + for column in _resolve_columns(rule, candidates, strict=False): rules.setdefault(str(column), []).append(rule) # One rule reproduces its token; several stacked rules are just redacted. maskers = {c: _report_masker(rs[0] if len(rs) == 1 else None) for c, rs in rules.items()} diff --git a/tests/test_enterprise_report_masking.py b/tests/test_enterprise_report_masking.py index aea75ffb..b868a8da 100644 --- a/tests/test_enterprise_report_masking.py +++ b/tests/test_enterprise_report_masking.py @@ -218,3 +218,23 @@ def test_coerced_cells_of_masked_date_like_column(strategy): assert not [w for w in res.clean_report.warnings if "XYZZY" in w or "QWERTY" in w] # The unmasked column keeps its reviewable original. assert list(coerced["open"].values()) == ["7xyzOPEN"] + + +def test_strict_rule_on_column_absent_from_reports_does_not_raise(): + # "email" is masked but appears in no cluster, validation or coercion + # report; report redaction must not re-apply ``strict`` to that subset. + ec = EnterpriseConfig( + masking=(MaskingRule(name="m", columns=("Email",), strategy="hash", strict=True),), + ) + df = pd.DataFrame({"email": ["a@x.com", "b@y.io"], "v": [1, 2]}) + res = clean_enterprise(df, enterprise=ec) + assert "a@x.com" not in set(res.data["email"]) + assert _leaks(res, ["a@x.com", "b@y.io"]) == [] + + +def test_strict_rule_on_column_missing_from_frame_still_raises(): + ec = EnterpriseConfig( + masking=(MaskingRule(name="m", columns=("nope",), strategy="hash", strict=True),), + ) + with pytest.raises(ValueError, match="not found in dataframe"): + clean_enterprise(pd.DataFrame({"email": ["a@x.com"]}), enterprise=ec) From 25f5a5922fc209df57426c4bec692ea48af14e4d Mon Sep 17 00:00:00 2001 From: Kevin Costner <120246174+kevincostner17@users.noreply.github.com> Date: Tue, 15 Sep 2026 20:36:11 +0530 Subject: [PATCH 17/17] chore(release): 2.1.0 Bump version 2.0.0 -> 2.1.0 and finalize the changelog for release. --- CHANGELOG.md | 2 +- docs/production-readiness.md | 2 +- pyproject.toml | 2 +- src/freshdata/__init__.py | 2 +- uv.lock | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8b0faa66..102c6dcb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ All notable changes to this project are documented here. The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and the project adheres to [Semantic Versioning](https://semver.org/). -## [Unreleased] +## [2.1.0] - 2026-09-15 ### Security - GPX and SDMX parsing detects the document encoding (BOM, UTF-16/UTF-32 diff --git a/docs/production-readiness.md b/docs/production-readiness.md index 8bdb4dc5..1a101728 100644 --- a/docs/production-readiness.md +++ b/docs/production-readiness.md @@ -16,7 +16,7 @@ clean data nobody is watching. Each item links to the relevant guarantee on the ## Install & pin -- [ ] Pin an exact version (`freshdata-cleaner==2.0.0`) and the extras you use +- [ ] Pin an exact version (`freshdata-cleaner==2.1.0`) and the extras you use (`freshdata-cleaner[polars,privacy]`). Cleaning defaults can tighten between minor versions — pinning keeps decisions reproducible. - [ ] Install only the extras you need. The base install has **no** heavy deps; diff --git a/pyproject.toml b/pyproject.toml index 50cad910..72e52101 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -7,7 +7,7 @@ build-backend = "hatchling.build" # distribution MUST remain `freshdata-cleaner`: PyPI rejects `freshdata` as # too similar to the existing `fresh-data` project (see commit 88aa495). name = "freshdata-cleaner" -version = "2.0.0" +version = "2.1.0" description = "Fast, safe, automatic data cleaning for real-world tabular data." readme = "README.md" requires-python = ">=3.9" diff --git a/src/freshdata/__init__.py b/src/freshdata/__init__.py index 12ad20d4..7c2dda65 100644 --- a/src/freshdata/__init__.py +++ b/src/freshdata/__init__.py @@ -112,7 +112,7 @@ ) from .textlint import TextIssue, TextLintReport, lint_text_encoding -__version__ = "2.0.0" +__version__ = "2.1.0" __all__ = [ "Action", diff --git a/uv.lock b/uv.lock index 42044a3f..c125af60 100644 --- a/uv.lock +++ b/uv.lock @@ -2978,7 +2978,7 @@ wheels = [ [[package]] name = "freshdata-cleaner" -version = "2.0.0" +version = "2.1.0" source = { editable = "." } dependencies = [ { name = "numpy", version = "2.0.2", source = { registry = "https://pypi.org/simple" }, marker = "python_full_version < '3.10'" },