fix: security hardening and 2.1.0 release - #415
Merged
Merged
Conversation
open_safe_xml_binary looked for "<!doctype"/"<!entity" in the raw, lowercased bytes. In UTF-16 and UTF-32 documents those markers contain NUL bytes, so the search never matched and ElementTree went on to expand internal entities in GPX and SDMX input. The guard now detects the document encoding (byte-order mark, UTF-16/UTF-32 prefix, or the XML declaration, following XML 1.0 Appendix F; EBCDIC is refused) and scans the decoded text as well as the raw bytes. It then runs an expat pass, configured as ElementTree configures it, whose DOCTYPE and entity-declaration handlers raise, so a declaration is rejected in any encoding expat reads. Parse errors in that pass are left for ElementTree to report. UTF-16 documents without a DTD still parse.
…st 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.
…reshdata_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.
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.
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.
…e 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.
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.
…ports 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 "<redacted>"), key becomes "<redacted>", 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.
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.
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.
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.
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:<position>"). 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.
- 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.
- 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.
…tion 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.
Bump version 2.0.0 -> 2.1.0 and finalize the changelog for release.
Contributor
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
FreshData benchmark report —
|
| fixture | n_rows | n_cols | p50 s | p95 s | peak MB | repair % | false-repair % | preserve % | trust | monotonic | export % |
|---|
Authored-code reduction (Metric 6)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes for ten security advisories, applied on current main, plus the 2.1.0 release.
/tmp/freshdata_spill. Each run uses a private per-run directory (mode 0700) under the user cache dir, orFRESHDATA_SPILL_DIR, and removes it afterwards. Previously, other local users could read spilled rows.EngineConfig.temp_directorydefaults toNone, and an explicit directory is checked for ownership and permissions.label_key/FRESHDATA_BASELINE_KEY.freshdata-baseline-v2; v1 files still load, with a warning.tokenize,surrogate,fpeand the policypseudonymizeaction fell back to public constants, so anyone could recompute outputs for guessed values. They now use a random per-call key.EphemeralKeyWarning; keyless output is no longer stable across calls. Crypto FPE honoursvisible(#281).fd.learn(privacy='mask')treated only some PII types as sensitive, so card numbers, IBANs, IP addresses and health or licence identifiers were stored raw in profiles. Every detected type is now sensitive, unknown types fail closed, and card and bank column-name hints are added.freshdata profile auditflags raw cards and IBANs and exits 1.detect_piiscans categorical text on pandas 1.5 (#280).clean_enterprisereports held raw, pre-masking values of masked columns: cluster canonical values, variants and keys, validation samples, and coerced-cell originals and their warnings. These are now masked or redacted.[SENSITIVE:…]tokens were unkeyed SHA-256 and are now truncated HMAC-SHA256 under a per-process key.<redacted>, and[SENSITIVE:…]tokens differ between runs.model_contextunmasked. It now uses an allow-list, and only numeric and boolean samples pass through.sensitive_columns/must_maskmatch non-string labels. Unknownsensitive_columnsand colliding labels raise, and masking fails closed.analyze_dataset(mask_salt=...)for a reproduciblemodel_context. The default is per-run, recorded inaudit["mask_salt_source"](#288).Migration notes
EphemeralKeyWarning/key=: keylesstokenize/surrogate/fpe/pseudonymizeoutput now changes between calls, including in the GDPR, HIPAA and FERPA packs. Passkey=/key_env=for stable, joinable pseudonyms. Keyless output from earlier releases can be reversed by trying candidate values, so re-pseudonymise it with a secret key.label_key:freshdata-baseline-v2.label_key/FRESHDATA_BASELINE_KEY, compare using the same key. A missing or different key skips categorical PSI and reportsdrift.categorical_drift_skipped.temp_directoryisNone: spill goes to a private per-run directory. An explicittemp_directoryraisesPermissionErrorif it isn't owned by the user, or if it is group- or other-writable without the sticky bit.clean_enterprisereport objects carry tokens or<redacted>for masked columns. Clustermappingis emptied andClusterResult.redactedis set.[SENSITIVE:…]tokens are per-process.model_contextand its fingerprint are per-run unlessmask_saltis given.sensitive_columnsand labels that collide as strings raise.profile auditexit code:freshdata profile auditnow exits 1 when a profile that claims no raw values contains checksum-valid card numbers or IBANs; it used to exit 0. Re-learn the affected profiles.Verification
ruff check .and mypy (src/freshdata): clean.-m "not online and not large"): 6255 passed on Python 3.12, 6226 passed on Python 3.9.make truthbench-prandmake truthbench-release(pandas, polars, duckdb; repeats 2): PASS, 48/48 gates, no baseline changes.uv lock --check, version consistency (pyproject ==__version__== 2.1.0), sdist and wheel build, andtwine check: all pass.Closes #280
Closes #281
Closes #288