Skip to content

fix(enterprise): fail closed on unsupported anonymization config; reject non-finite trust weights - #358

Open
kevincostner17 wants to merge 1 commit into
mainfrom
fix/enterprise-config-honesty
Open

kevincostner17 wants to merge 1 commit into
mainfrom
fix/enterprise-config-honesty

Conversation

@kevincostner17

Copy link
Copy Markdown
Contributor

Summary

#247: EnterpriseConfig.anonymization is accepted but never applied.

  • The bug: EnterpriseConfig.__post_init__ type-checks the field, but no stage of clean_enterprise reads it. Masking is driven only by masking and privacy. A caller who set anonymization=(AnonymizationConfig(strategy="redact"),) got raw PII back, with mask_report=None, privacy_report=None and no warning.
  • The fix fails closed: clean_enterprise raises ValueError when anonymization is non-empty. The message points to masking= (MaskingRule) or privacy= (PIIDetectionConfig with enable_privacy_detection=True).
    • EnterpriseConfig still constructs with the field, so existing config objects keep loading.
    • The field is documented as unsupported in the EnterpriseConfig, AnonymizationConfig and clean_enterprise docstrings.
    • The check runs before any cleaning, whether or not enable_masking is set.
    • The CLI and polars inputs go through clean_enterprise, so they fail the same way. The CLI prints a one-line error and exits 1.
  • Why fail closed rather than warn or leave it:
    • A privacy control that silently does nothing is worse than an explicit error. A user who configured anonymization believes the output is safe to share, and a warning is easy to miss in a pipeline. An error isn't, and the fix is a one-line config change.
    • The change is easy to reverse. Once anonymization is implemented, the check comes out and the same configs start working, with no API break.

#277: TrustScoreWeights accepts NaN and inf.
value < 0 and sum <= 0 are both false for NaN, and +inf passes both, so the trust score came out nan with grade F. Each weight must now be finite (math.isfinite), otherwise ValueError.

Tests

  • tests/test_enterprise_config.py:
    • NaN, +inf and -inf are rejected for all four weights.
    • EnterpriseConfig(anonymization=...) still constructs and type-checks.
  • tests/test_enterprise_interface.py:
    • clean_enterprise raises with enable_masking both True and False.
    • It raises for polars input (skipped if polars isn't installed).
    • The CLI clean path exits non-zero, prints the error to stderr and writes no output file.

Verification

  • ruff check .: all checks passed
  • mypy src/freshdata: no issues in 202 source files
  • pytest -m "not online and not large", Python 3.12: 4256 passed, 6 skipped
  • pytest -m "not online and not large", Python 3.9 / pandas 1.5: 4252 passed, 10 skipped
  • The repro scripts from both issues now raise ValueError.

Closes #247
Closes #277

…non-finite trust weights

EnterpriseConfig.anonymization was accepted and type-checked but no pipeline
stage ever read it, so clean_enterprise returned raw PII with no mask or
privacy report and no warning. clean_enterprise now raises ValueError when the
field is non-empty and points callers at masking= (MaskingRule) or privacy=
(PIIDetectionConfig with enable_privacy_detection=True). EnterpriseConfig still
constructs with the field, and it is documented as unsupported. The CLI builds
its config without this field and goes through clean_enterprise, so it
inherits the guard (reported as a one-line error, exit 1). A privacy control
that silently does nothing is worse than an explicit error; the guard can be
removed once the field is actually implemented.

TrustScoreWeights only checked `value < 0` and a positive sum, both of which
are False for NaN, and +inf passed too; either produced a NaN trust score
graded F. Each weight must now be finite (math.isfinite), otherwise ValueError.

Closes #247
Closes #277
@coderabbitai

coderabbitai Bot commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: c7be8f48-e2fe-4c5b-aa3f-7e6c7c535cf8


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown

FreshData benchmark report — performance

  • freshdata: ?
  • python: ?
  • platform: ?
fixture n_rows n_cols p50 s p95 s peak MB repair % false-repair % preserve % trust monotonic export %

Authored-code reduction (Metric 6)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TrustScoreWeights accepts NaN weights and produces a NaN trust score EnterpriseConfig.anonymization is accepted and type-checked but never applied

1 participant