diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index b887f232f..34e3a054d 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -1,674 +1,170 @@ -# Presidio Development Instructions +# Presidio Development & Review Instructions -Presidio is a Python-based data protection and de-identification SDK with multiple components for detecting and anonymizing PII (Personally Identifiable Information) in text and images. +Presidio is a Python-based data protection and de-identification SDK with +multiple components for detecting and anonymizing PII (Personally Identifiable +Information) in text and images. + +Domain-specific rules live in path-scoped instruction files and apply on top of +this file when a change touches the matching paths: + +- `.github/instructions/recognizers.instructions.md` — adding or modifying PII + recognizers. +- `.github/instructions/yaml-config.instructions.md` — the pydantic layer that + translates YAML configuration into Presidio instances. ## Core Philosophy -**Data Privacy is Paramount** - This is a PII detection and anonymization system used in sensitive contexts. Security and correctness are non-negotiable. +**Data privacy is paramount.** This is a PII detection and anonymization system +used in sensitive contexts; security and correctness are non-negotiable. -**Key Principles:** -- **Accuracy First**: False negatives (missed PII) and false positives (incorrect detections) both damage trust -- **Security by Default**: Never log PII values, use non-reversible anonymization, validate all inputs -- **Cross-Component Awareness**: Presidio is a multi-component system - changes ripple across boundaries -- **Stateless Design**: Presidio is designed for scalability - avoid adding unnecessary state -- **Documentation Integrity**: Code and docs must stay synchronized - outdated docs are dangerous +- **Accuracy first**: false negatives (missed PII) and false positives + (incorrect detections) both damage trust. +- **Security by default**: never log PII values, use non-reversible + anonymization, validate all inputs. +- **Presidio is a library**: changes to shared code alter results for users who + wrote no new code. Backward compatibility is the prime directive. +- **Stateless design**: modules that process records are stateless for + scalability — avoid adding state. +- **Cross-component awareness**: Presidio is a multi-component system; changes + ripple across boundaries. +- **Documentation integrity**: code and docs must stay synchronized — outdated + docs are dangerous. ---- +## Backward Compatibility -## Part 1: Implementation Guidelines +Before changing anything outside a brand-new file, the PR description must +state what existing behavior changes. These count as behavior changes even +without a signature change: -Use these guidelines when **generating or writing code** for Presidio. +- Default values on shared base classes (`None` to `[]` changes truthiness for + every subclass). +- Properties on abstract interfaces — custom implementations inherit the new + default and may break. +- Anything altering which entities are returned, or their scores, for text that + previously worked. -### Presidio Architecture Patterns +Prefer additive changes: new parameters get defaults preserving current +behavior; public APIs are never broken without a deprecation path. +**Surface new scoring inputs in explainability.** Anything that changes how a +score is derived (context, negative context, thresholds) must be reflected in +`AnalysisExplanation`, or users cannot tell why a result scored as it did. +**Prefer warnings over exceptions when the caller cannot fix the condition.** +Raising on a configuration a user did not write turns a degraded result into a +hard failure. Where a lookup falls back to a default, add a debug log so the +fallback is discoverable. -**Data Flow (Unidirectional):** -``` -Analyzer (detect PII) → Anonymizer (transform PII) → Output - ↓ -nlp_engine → recognizers → context -``` +**Prefer a property on the base class over a maintained list of class names.** +Lists drift as classes are added, and users installing from PyPI cannot extend +them. -**Design Patterns:** -- **Registry Pattern**: `RecognizerRegistry` for dynamic recognizer management -- **Provider Pattern**: `NlpEngineProvider`, `RecognizerRegistryProvider` for configuration +## Cross-Component Changes -### Implementing New Recognizers +Data flows one way: Analyzer → Anonymizer → Output. Downstream components +(CLI, structured, image-redactor) consume analyzer/anonymizer, never the +reverse. -**1. Choose the Right Base Class:** -```python -from presidio_analyzer import PatternRecognizer, LocalRecognizer, RemoteRecognizer +- Shared data models (`RecognizerResult`, `OperatorConfig`) are contracts — + changes require coordinated updates across all consumers, in the same + changeset. +- Reuse by importing from shared modules, not by copying code across + components. If multiple components need a feature, extract it to a common + location. +- Respect boundaries: a component imports another's public interface, never its + internals (e.g. the anonymizer must not import + `presidio_analyzer.predefined_recognizers`). +- Registry and provider patterns exist to decouple components — bypassing them + creates hidden dependencies. +- Test the complete integration path (unit, integration, and e2e), not just + isolated components. -# For regex-based detection -class MyPatternRecognizer(PatternRecognizer): - pass +## Security & Privacy -# For custom logic (NLP, ML) -class MyCustomRecognizer(LocalRecognizer): - def load(self): ... - def analyze(self, text, entities, nlp_artifacts): ... +Always flag: -# For calling remote services -class MyRemoteRecognizer(RemoteRecognizer): - def analyze(self, text, entities, nlp_artifacts): ... -``` +- **PII leakage in logs, errors, or debug output** — log entity types and + positions, never `entity.text`. +- **Reversible or weak anonymization** — deterministic hashing is reversible + via rainbow tables; use random/unpredictable replacement values that don't + preserve PII characteristics. +- Regex injection: user-provided patterns must be validated before compilation. +- Hardcoded secrets or credentials; unsafe deserialization (pickle, untrusted + models); command injection; path traversal; missing input validation on API + endpoints (including unbounded input sizes). -**2. Predefined Recognizers Location Matters:** -- Country-specific: `presidio-analyzer/presidio_analyzer/predefined_recognizers/country_specific/{country}/` -- Generic patterns: `.../predefined_recognizers/generic/` -- NLP/ML-based: `.../predefined_recognizers/nlp_engine_recognizers/` or `.../ner/` -- Third-party: `.../predefined_recognizers/third_party/` +## Performance -**3. Pattern Design Best Practices:** -```python -# ❌ BAD: Too broad - matches month names as persons -pattern = r"\b[A-Z][a-z]+\b" +- Avoid catastrophic regex backtracking (`(a+)+b` is O(2^n) on `aaaa...b`); + test patterns against long adversarial strings. +- Cache compiled regexes; don't recompile per call. +- Batch NLP processing (`nlp.pipe`) instead of per-text calls; load models + once and reuse. +- Flag O(n²) where O(n) exists, blocking I/O on API paths, and loading entire + datasets into memory. -# ✅ GOOD: Specific pattern with context -PATTERNS = [ - Pattern( - "SSN", - r"\b\d{3}-\d{2}-\d{4}\b", - 0.3 # Low base score, context will boost - ) -] +## Testing Standards -CONTEXT = ["ssn", "social security", "tax id"] -``` +- Test names describe behavior: + `test_when_invalid_checksum_then_no_match`, not `test_case2`. +- Assert exact expected values, not ranges — a range assertion passes even when + the logic producing the value breaks. +- Cover true positives, true negatives, edge cases, and values embedded in + surrounding text; validate exact boundaries, not just types. +- Fix random seeds for non-deterministic NLP/ML tests. +- Test behavior, not implementation details. -**4. Document Pattern Sources:** -```python -""" -Recognizes US Social Security Numbers. +## Documentation -Pattern based on SSA Publication No. 05-10633: -https://www.ssa.gov/history/ssn/geocard.html +When adding features, update all that apply: `docs/supported_entities.md` (new +entity types), `docs/api-docs/api-docs.yml` (API changes), `README.md` (major +features), docstrings (all public classes/methods, reST format — `:param:`, +`:return:`, `:raises:`), and `docs/samples/` (complex features). -Validation uses SSN format rules: AAA-GG-SSSS -- AAA: Area number (001-899, excluding 666) -- GG: Group number (01-99) -- SSSS: Serial number (0001-9999) -""" -``` +Do not update `CHANGELOG.md` in a PR: current-release entries are generated +from merged PRs before each version bump, and per-PR edits create merge +conflicts. -**5. Required Configuration Updates:** -```python -# Update all of these: -# 1. presidio_analyzer/predefined_recognizers/__init__.py -from .country_specific.us.my_recognizer import MyRecognizer -__all__ = [..., "MyRecognizer"] +Terminology: use "threshold", not "cutoff"; use ISO 639-1 language codes in +docs and configuration examples. -# 2. presidio_analyzer/predefined_recognizers/country_specific/us/__init__.py -from .my_recognizer import MyRecognizer -__all__ = [..., "MyRecognizer"] +## Code Review Guidelines -# 3. presidio_analyzer/conf/default_recognizers.yaml -recognizers: - - name: MyRecognizer - supported_languages: ["en"] - type: predefined - enabled: false # Country-specific defaults to false +- Only comment with HIGH CONFIDENCE (>80%) that an issue exists; be concise and + actionable — cite the line and propose the concrete fix. +- Check for existing patterns and helpers in the codebase before suggesting new + approaches. +- Severity: 🔴 security/PII leakage and correctness → 🟡 performance, + cross-component breaks, missing tests → 💡 code quality. -# 4. docs/supported_entities.md (add row to appropriate table) -``` +**Do not flag** (automated tools own these): formatting, line length, import +order (Ruff); type-hint style (`List[str]` vs `list[str]`); style preferences +or speculative abstractions that don't fix bugs or improve accuracy. -**6. Comprehensive Test Coverage:** -```python -@pytest.mark.parametrize("text, expected_len, expected_positions", [ - # True positives - valid formats - ("SSN: 123-45-6789", 1, ((5, 16),)), - ("My SSN is 123-45-6789", 1, ((10, 21),)), - - # True negatives - invalid formats - ("SSN: 000-00-0000", 0, ()), # Invalid area - ("SSN: 666-12-3456", 0, ()), # Excluded area - - # Boundary testing - embedded in text - ("Contact: 123-45-6789 for info", 1, ((9, 20),)), - - # False positive prevention - ("ISBN: 123-45-6789", 0, ()), # Different context -]) -def test_ssn_detection(text, expected_len, expected_positions, recognizer): - results = recognizer.analyze(text, ["US_SSN"]) - assert len(results) == expected_len - for result, (start, end) in zip(results, expected_positions): - assert result.start == start - assert result.end == end -``` +## Repository Context -### Implementing New Anonymizers (Operators) +- **Python** `>=3.10,<3.15` — code must run on every version in range. +- **uv** for dependency management (not pip/Poetry); each package commits a + `uv.lock`. Whenever a package's `pyproject.toml` dependencies change, + regenerate and commit that package's `uv.lock` in the same change + (`cd && uv lock`) — CI installs with `uv sync --locked` and fails + on drift. +- **Ruff** for linting and formatting; **spaCy** as the default NLP engine + (swappable via provider pattern); **Docker** images on + `ghcr.io/data-privacy-stack`. -**1. Implement the Operator Interface:** -```python -from presidio_anonymizer.operators import Operator, OperatorType - -class MyOperator(Operator): - """Custom anonymization operator.""" - - def operate(self, text: str, params: dict = None) -> str: - """Transform the detected PII.""" - # Ensure non-reversible transformation - import uuid - return f"<{params.get('entity_type', 'REDACTED')}_{uuid.uuid4().hex[:8]}>" - - def validate(self, params: dict = None) -> None: - """Validate operator parameters before use.""" - if params and 'entity_type' not in params: - raise ValueError("entity_type is required") - - def operator_name(self) -> str: - return "my_operator" - - def operator_type(self) -> OperatorType: - return OperatorType.Anonymize -``` - -**2. Security Checklist:** -- ✅ **Non-reversible**: Cannot recover original PII from anonymized output -- ✅ **Entropy**: Uses random/unpredictable values (not deterministic hashing) -- ✅ **No PII leakage**: Doesn't preserve PII characteristics (length, format) - -```python -# ❌ BAD: Reversible via rainbow tables -def operate(self, text, params): - return hashlib.md5(text.encode()).hexdigest() - -# ✅ GOOD: Non-reversible with entropy -def operate(self, text, params): - return f"<{params['entity_type']}_{uuid.uuid4().hex[:8]}>" -``` - -**3. Test Anonymization Quality:** -```python -def test_operator_is_non_reversible(): - """Verify same input produces different output.""" - operator = MyOperator() - result1 = operator.operate("John Doe", {"entity_type": "PERSON"}) - result2 = operator.operate("John Doe", {"entity_type": "PERSON"}) - assert result1 != result2 # Different each time - -def test_operator_preserves_structure(): - """Verify anonymized text maintains sentence structure.""" - text = "Email: john@example.com, Phone: 555-1234" - # After anonymization - expected = "Email: , Phone: " - # Structure preserved, PII replaced -``` - -### API Development - -**1. Maintain Backward Compatibility:** -```python -# ❌ BAD: Breaking change -def analyze(text: str, language: str, entities: List[str]): - ... - -# ✅ GOOD: Optional parameter with default -def analyze( - text: str, - language: str, - entities: Optional[List[str]] = None -) -> List[RecognizerResult]: - ... -``` - -**2. Required Updates for API Changes:** ```bash -# 1. Update OpenAPI schema -docs/api-docs/api-docs.yml - -# 2. Add E2E tests -e2e-tests/tests/test_new_endpoint.py - -# 3. Add usage example -docs/samples/python/new_feature_example.ipynb - -``` - -### Cross-Component Changes - -**When modifying shared interfaces:** - -1. **Identify all consumers:** -```python -# RecognizerResult is consumed by: -# - presidio-anonymizer (takes analyzer results) -# - presidio-cli (displays results) -# - presidio-structured (processes tabular data) -# - docs/samples/* (user examples) -``` - -2. **Update all components in same changeset:** -```python -# If adding field to RecognizerResult: -# 1. presidio-analyzer: Add field and populate -# 2. presidio-anonymizer: Handle new field (or ignore safely) -# 3. presidio-cli: Display new field (optional) -# 4. Tests: Update expectations -# 5. Docs: Document new field -# 6. e2e-tests: Add integration test for new field -``` - -3. **Respect component boundaries:** -```python -# ❌ BAD: Anonymizer importing analyzer internals -from presidio_analyzer.predefined_recognizers import UsSsnRecognizer - -# ✅ GOOD: Use public interfaces only -from presidio_analyzer import RecognizerResult -``` - -### Performance Optimization - -**1. Cache Compiled Regexes:** -```python -from functools import lru_cache - -@lru_cache(maxsize=128) -def _compile_pattern(pattern_str: str) -> re.Pattern: - return re.compile(pattern_str, re.IGNORECASE) -``` - -**2. Avoid Catastrophic Backtracking:** -```python -# ❌ BAD: O(2^n) on "aaaa...ab" -pattern = r"(a+)+" - -# ✅ GOOD: Atomic grouping -pattern = r"(?>a+)" -``` - -**3. Batch NLP Processing:** -```python -# ❌ BAD: Process one at a time -for text in texts: - doc = nlp(text) - ... - -# ✅ GOOD: Use spaCy pipe for batching -for doc in nlp.pipe(texts, batch_size=50): - ... -``` - -### Testing Requirements - -**Test Naming Convention:** -```python -# ✅ GOOD: Descriptive, intention-revealing -def test_when_valid_ssn_then_detect_with_correct_boundaries() -def test_when_invalid_checksum_then_no_match() -def test_when_context_missing_then_low_confidence() - -# ❌ BAD: Non-descriptive -def test_ssn_1() -def test_case2() -``` - -### Documentation Requirements - -**1. Required Documentation Updates:** -```markdown -When adding a feature, update ALL of: - -✅ docs/supported_entities.md - For new entity types -✅ docs/api-docs/api-docs.yml - For API changes -✅ README.md - For major features -✅ Docstrings - All public classes/methods -✅ docs/samples/ - Usage examples for complex features -✅ Update docstrings based on the reST docstring format (:param:, :return:, :raises:, :example:) -``` - -Do not update `CHANGELOG.md` in a PR. Before each version bump, changelog -entries for the current release are generated from merged PRs. Per-PR changelog -edits create unnecessary merge conflicts. - -**2. Pattern Source Documentation:** -```python -# In recognizer docstring or comments -""" -Pattern based on Royal Mail PAF specification: -https://www.royalmail.com/find-a-postcode - -UK postcodes follow 6 formats: -- A9 9AA (e.g., M1 1AA) -- A99 9AA (e.g., M60 1NW) -- AA9 9AA (e.g., CR2 6XH) -- AA99 9AA (e.g., DN55 1PT) -- A9A 9AA (e.g., W1A 1HQ) -- AA9A 9AA (e.g., EC1A 1BB) - -Plus special case: GIR 0AA -""" -``` - ---- - -## Part 2: Code Review Guidelines - -Use these guidelines when **reviewing pull requests** for Presidio. - -### Review Philosophy - -* Only comment when you have HIGH CONFIDENCE (>80%) that an issue exists -* Be concise: one sentence per comment when possible -* Focus on actionable feedback, not observations -* Data privacy is paramount - this is a PII detection/anonymization system -* All modules in Presidio which process records are stateless - avoid suggesting stateful solutions -* Presidio is a multi-component system - consider cross-component impacts of changes -* Don't reinvent the wheel - check for existing patterns, functions and best practices in the codebase before suggesting new approaches - -### Review Priorities - -Focus on issues in this order of importance: - -### 🔴 CRITICAL (Always Flag) - -#### 1. Security & Privacy Vulnerabilities - -**PII-Specific Risks:** -- **PII leakage in logs, error messages, or debug output** - Never log detected PII values, only entity types and positions -- **Regex injection vulnerabilities** - User-provided patterns must be validated before compilation -- **Inadequate anonymization** - Reversible transformations, weak masking, deterministic fake data without proper context -- **Side-channel leaks** - Timing attacks revealing PII presence, cache-based information disclosure - -**General Security:** -- Hardcoded secrets, API keys, credentials (especially for NLP model endpoints, cloud services) -- Command injection (especially in CLI component) -- Unsafe deserialization (pickle files, untrusted NLP models) -- Missing input validation on API endpoints (analyzer, anonymizer, image-redactor) -- Path traversal in file operations -- Insecure random number generation for fake data - -#### 2. Correctness & Logic Errors - -**PII Detection Accuracy:** -- **False positives** - Overly broad regex patterns matching non-PII or other entity types with med/high confidence -- **False negatives** - Missing valid cases for a given entity type, especially edge cases or common variations -- **Incorrect entity boundaries** - Off-by-one errors in start/end positions causing malformed anonymization -- **Confidence score miscalculation** - Scores outside [0.0, 1.0], incorrect aggregation of multiple detection methods - -**General Logic:** -- Race conditions in multi-threaded analysis -- Resource leaks (NLP models not released, file handles, network connections) -- Null/None handling in entity detection chains -- Incorrect error handling that silently fails to detect PII -- Adding state where unnecessary (Presidio is designed to be stateless for scalability) - -#### 3. Performance Issues - -**PII Detection Specific:** -- **Inefficient regex patterns** - Catastrophic backtracking (e.g., `(a+)+b` on "aaaa...a") -- **Redundant passes** - Running the same logic multiple times on same text -- **Unbounded batch processing** - Loading entire datasets into memory -- **Missing regex compilation caching** - Recompiling patterns on every call -- **Unnecessary model loads** - Loading the same model multiple times instead of reusing instances - -**General Performance:** -- O(n²) or worse algorithms when O(n) exists -- Blocking I/O on critical API paths -- Missing database indexes for entity result storage -- Inefficient image processing (loading full image when bounding box would suffice) - -### 🟡 IMPORTANT (Flag if Significant) - -#### 4. Cross-Component Alignment & Integration - -**Respect the Natural Data Flow:** -- Presidio follows a unidirectional flow: Analyzer → Anonymizer → Output -- Downstream components (CLI, structured, image-redactor) consume analyzer/anonymizer, never the reverse -- Breaking this flow creates circular dependencies and tight coupling -- Changes should propagate forward through the data pipeline, not backward - -**Module Reuse Guidelines:** -- Reuse code by importing from shared modules, not by copying code across components -- Shared data models (RecognizerResult, OperatorConfig) should be treated as contracts - changes require coordinated updates across all consumers -- When adding functionality, check if it belongs in an existing shared module rather than duplicating logic -- If multiple components need the same feature, extract it to a common location rather than implementing it multiple times -- Backward compatibility is critical when modifying shared modules - ensure existing consumers continue to work without changes - -**Avoid Cross-Component Side Effects:** -- Changes to internal implementation should not affect other components' behavior -- Modifying shared configuration files requires understanding impact on all components that consume them -- Registry and provider patterns exist to decouple components - bypassing them creates hidden dependencies -- Component boundaries must be respected: anonymizer should never import from analyzer internals, only public interfaces -- Providing a solution specific to one component in a shared module instead of providing a general solution that can be used by multiple components creates tight coupling and maintenance challenges - -**When Making Changes Across Components:** -- Identify all components that consume the interface you're modifying -- Update dependent components in the same changeset to maintain system consistency -- Ensure configuration files, API schemas, and documentation stay synchronized -- Test the complete integration path, not just individual components in isolation in unit tests, integration tests, and the e2e test suite -- Communicate changes clearly in the PR description, especially if they affect multiple components or require coordinated - -#### 5. Architecture & Design - -**Presidio Patterns:** -- **Recognizer design violations** - Not inheriting from `EntityRecognizer`, missing `load()` or `analyze()` -- **Operator design violations** - Not implementing `OperatorType` interface correctly -- **Registry pattern misuse** - Bypassing `RecognizerRegistry`, hardcoding recognizer lists -- **Provider pattern violations** - Not following `NlpEngineProvider` or `RecognizerRegistryProvider` patterns -- **Tight coupling** - Recognizers depending on specific NLP engine implementation details - -**General Design:** -- Circular dependencies between modules -- Missing abstraction for third-party service integrations -- Breaking existing public APIs without deprecation warnings -- Inconsistent error handling strategies (mixing exceptions and error codes) - -#### 6. Data Integrity & Validation - -**Input Validation:** -- Missing validation of user-provided entity types -- Accepting arbitrary regex patterns without safety checks -- No length limits on input text (DoS via memory exhaustion) -- Missing validation of parameters -- Unchecked file uploads - -**Output Validation:** -- Confidence scores outside valid range -- Overlapping entity spans not handled correctly -- Missing entity type in anonymization results - -#### 7. Testing Requirements - -**Presidio-Specific Testing:** -- **Missing tests for new recognizers** - Must include: true positives, true negatives, edge cases, false positive scenarios, entity within larger context -- **No validation of entity boundaries** - Tests only check entity type, not exact start/end positions -- **Missing multilingual tests** - Recognizers claiming multi-language support without language-specific tests -- **Anonymization reversibility not tested** - No verification that anonymized data can't be de-anonymized -- **Missing E2E analyzer→anonymizer tests** - Testing components in isolation without integration validation - -**General Testing:** -- Missing tests for critical business logic (PII detection, anonymization) -- Flaky tests due to non-deterministic NLP/ML models (use fixed random seeds) -- Tests that don't validate behavior (checking implementation details instead) -- Missing regex pattern edge cases (empty strings, special characters, unicode) - -#### 8. Documentation Requirements - -**Code-Documentation Consistency:** -- Code changes must be reflected in documentation - outdated docs are misleading and dangerous -- Implementation must not contradict existing documentation - if conflict exists, either update docs or reconsider implementation -- API documentation is auto-generated from docstrings - formatting errors break the build - -**Docstring Quality:** -- All public classes, methods, and functions must have docstrings -- Docstrings must follow consistent format (Args, Returns, Raises, Examples) -- No formatting issues that break API doc generation (malformed RST/Markdown, incorrect indentation) -- Include type information in docstrings when not obvious from type hints - -**Documentation for New Features:** -- New recognizers must document pattern sources - link to official standards, government specifications, or authoritative references -- Complex additions require usage examples in `docs/samples/` - show common use cases, not just API reference -- New entity types must be added to `docs/supported_entities.md` with description and example -- API changes require updates to `docs/api-docs/api-docs.yml` (OpenAPI schema) - -**Pattern Recognizer Documentation:** -- Explain the logic source: "Based on ISO standard X", "Follows format defined by Y government agency" -- Document regex pattern rationale - why specific character classes, lookaheads, or groups are needed -- Include references to validation algorithms (e.g., "Luhn checksum validation per ISO/IEC 7812") -- Note any limitations or known edge cases in the pattern - -### 💡 OPTIONAL (Low Priority) - -#### 9. Code Quality (only if impacts maintainability) - -- Overly complex recognizer logic (>50 lines in `analyze()` method, >3 nesting levels) -- Misleading variable names (e.g., `pattern` for compiled regex, should be `compiled_pattern`) -- Missing docstrings on public recognizer/operator classes -- Incomplete type hints on public APIs (especially `analyze()`, `anonymize()` signatures) - -### What NOT to Flag - -**DO NOT comment on these (handled by automated tools):** - -- ❌ Code formatting, line length, indentation (handled by `ruff format`) -- ❌ Import ordering (handled by `ruff check --select I`) -- ❌ Trailing commas, whitespace (handled by `ruff`) -- ❌ Type hint style preferences (`List[str]` vs `list[str]` - both valid for Python 3.9-3.12 support) - -**DO NOT comment on style preferences that don't affect correctness:** - -- Personal preferences for syntax variations -- Subjective naming when current name is clear in PII context -- Minor refactoring suggestions that don't fix bugs or improve accuracy -- Unnecessary abstractions "for future flexibility" in recognizers - -### Review Examples - -**Be specific and actionable:** -``` -✅ GOOD: "🔴 CRITICAL: Line 45 logs detected PII value. Change logger.info(f'Found: {entity.text}') -to logger.info(f'Found entity type: {entity.entity_type}')" - -❌ BAD: "Don't log PII" -``` - -**Provide context:** -``` -✅ GOOD: "🟡 Important: This regex has catastrophic backtracking on input 'aaaaaa...b' (O(2^n) time). -Use atomic grouping: (?>a+)b or possessive quantifier a++b" - -❌ BAD: "This regex is slow" -``` - -**Differentiate severity:** -- **🔴 CRITICAL** - Security, data leakage, correctness bugs affecting PII detection accuracy -- **🟡 Important** - Performance issues, cross-component breaks, missing tests for new recognizers -- **💡 Suggestion** - Code quality improvements, better error messages, optimization opportunities - -**Acknowledge good practices:** -- Well-tested recognizers with comprehensive edge case coverage -- Proper use of context validation (NLP + regex) -- Good error handling with informative messages -- Performance optimizations (regex caching, batch processing) - ---- - -## Part 3: Repository-Specific Context - -### Technology Stack -- **Python** - Must support all versions -- **uv** - Dependency management and installation (not pip or Poetry). Each package commits a `uv.lock`; `poetry-core` is retained only as the build backend for now. -- **Ruff** - Linting and formatting (replaces flake8, black, isort) -- **spaCy** - Default NLP engine (en_core_web_lg for production), although one can use other NLP engines via provider pattern -- **Docker** - Deployment via GitHub Container Registry (`ghcr.io/data-privacy-stack`) - - -### Critical Files for Cross-Component Changes -- `RecognizerResult` - Shared analyzer output format -- `OperatorConfig` - Anonymizer operator configuration -- `conf/default_recognizers.yaml` - System-wide recognizer registry -- `docs/supported_entities.md` - Public entity type documentation -- API schemas in `docs/api-docs/` - -## Quick Reference Commands - -### Local Development -```bash -# Setup (uv reads the committed uv.lock; --locked fails if it is stale) -cd presidio-analyzer # or presidio-anonymizer, presidio-cli, etc. +# Setup and test (per package) +cd presidio-analyzer uv sync --locked --all-extras --group dev -uv run python -m spacy download en_core_web_lg # For analyzer/CLI only +uv run python -m spacy download en_core_web_lg # analyzer/CLI only +uv run pytest -xvv +uv run ruff check . && uv run ruff format . -# Run tests -uv run pytest -xvv # Stop on first failure with verbose output -uv run pytest tests/test_us_ssn_recognizer.py -k "test_valid" # Specific test - -# Lint -uv run ruff check . -uv run ruff format . +# E2E +docker compose up --build -d && cd e2e-tests && pytest -v ``` -> **Dependency changes:** whenever you edit a package's `pyproject.toml` -> dependencies (add/remove/bump `[project]` deps, extras, or -> `[dependency-groups]`), you MUST regenerate and commit that package's -> `uv.lock` in the same change (`cd && uv lock`). CI installs with -> `uv sync --locked` and fails if `pyproject.toml` and `uv.lock` are out of -> sync, so an updated `pyproject.toml` without its matching `uv.lock` will -> break the build. - -### Docker Testing -```bash -# Quick test with pre-built images -docker pull ghcr.io/data-privacy-stack/presidio-analyzer:latest -docker run -d -p 5002:3000 --name analyzer ghcr.io/data-privacy-stack/presidio-analyzer:latest -curl http://localhost:5002/health - -# Full build from source (takes 15+ minutes) -docker compose up --build -d -``` - -### E2E Testing -```bash -docker-compose up -d # Start all services -cd e2e-tests -python -m venv venv && source venv/bin/activate -pip install -r requirements.txt -pytest -v # Run all E2E tests -``` - -## Common Issues to Watch For - -### Build/Test Issues -- **Stale `uv.lock`** - If `uv sync --locked` fails with "lockfile needs to be updated", run `uv lock` in that package and commit the result. -- **Missing spaCy models** - Download en_core_web_lg before running tests -- **AHDS test skips** - Expected when AHDS_ENDPOINT not set -- **Transformers test failures** - Expected without HuggingFace access in restricted environments - -### Code Issues -- **Logging PII values** - Never log `entity.text`, only `entity.entity_type` -- **Hardcoded language assumptions** - Use `context.language` parameter -- **Missing None checks** - NLP engines return None for empty/invalid text -- **Unbounded regex backtracking** - Test patterns with long strings -- **Confidence score > 1.0** - Validate score normalization logic - -## Documentation Requirements Checklist - -**See section 8 in Review Priorities above for comprehensive documentation guidelines.** - -When adding features, update: -- **docs/supported_entities.md** - For new entity types -- **docs/api-docs/api-docs.yml** - For API changes -- **README.md** - For major features -- **Docstrings** - All public classes and methods (ensure proper formatting for API doc generation) -- **docs/samples/** - Add usage examples for complex new features - -Do not update `CHANGELOG.md` in a PR. Its current-release entries are generated -from merged PRs before each version bump, avoiding conflicts between concurrent -contributions. - -## Reference Documentation - -**Consult these for detailed guidance:** -- **CONTRIBUTING.md** - PR process, CLA, code of conduct -- **docs/development.md** - Build process, testing, CI/CD -- **docs/analyzer/developing_recognizers.md** - Recognizer best practices -- **docs/analyzer/adding_recognizers.md** - Step-by-step recognizer guide -- **docs/anonymizer/adding_operators.md** - Operator development guide - ---- - -**Summary for Code Review**: Prioritize security (PII leakage), correctness (detection accuracy), and performance (regex efficiency). Ensure comprehensive testing for all recognizers. Let automated tools handle formatting. Focus on actionable, specific feedback with concrete fixes. \ No newline at end of file +Reference docs: `CONTRIBUTING.md`, `docs/development.md`, +`docs/analyzer/adding_recognizers.md`, `docs/analyzer/developing_recognizers.md`, +`docs/anonymizer/adding_operators.md`. diff --git a/.github/instructions/recognizers.instructions.md b/.github/instructions/recognizers.instructions.md new file mode 100644 index 000000000..889c65adb --- /dev/null +++ b/.github/instructions/recognizers.instructions.md @@ -0,0 +1,189 @@ +--- +applyTo: "presidio-analyzer/presidio_analyzer/predefined_recognizers/**,presidio-analyzer/presidio_analyzer/conf/default_recognizers.yaml,presidio-analyzer/tests/test_*recognizer*.py,docs/supported_entities.md" +--- + +# Recognizer changes + +Rules for adding or modifying PII recognizers. When reviewing, lead with the +highest-impact gaps in this order: + +1. **Pattern accuracy.** The pattern is as specific as the format allows, the + score is calibrated to the pattern alone, the context words are the right + ones, the checksum is correct where one exists (and none is invented where + it doesn't — `validate_result` must not promote weak matches to 1.0), and + the logic's source is documented, preferably an official specification. +2. **Proper testing.** A configuration-path test through + `RecognizerRegistryProvider` (the load-bearing rule below), exact-score + assertions rather than ranges, a lookalike negative, and + context-enhancement coverage. +3. Construction paths that disagree (direct vs. `add_recognizer()` vs. YAML). +4. Changes to an *existing* recognizer's patterns, scores, or context made as a + side effect of adding a new one — users depend on current detection behavior. +5. Language/country-code mismatch; missing exports, YAML entry, or docs row. + +Give specific, actionable feedback: cite the file and line and propose the +concrete fix. Do not comment on formatting — Ruff and CI own that. + +## The load-bearing rule: test the configuration path + +Most predefined recognizers ship `enabled: false`, so the default test run never +constructs them from configuration. Users, however, reach them exactly one way: +flipping `enabled: true` in a registry YAML. A recognizer that works when built +in Python can still be unreachable — or crash — when enabled in YAML. + +**Every new or changed recognizer needs at least one test that loads it through +`RecognizerRegistryProvider` and asserts detection:** + +```python +def test_recognizer_loads_and_detects_when_enabled_in_yaml(tmp_path): + """Detection must work through the path users actually configure.""" + conf = tmp_path / "recognizers.yaml" + conf.write_text( + """ +supported_languages: + - en +recognizers: + - name: MyRecognizer + supported_languages: + - en + type: predefined + enabled: true + country_code: us +""" + ) + registry = RecognizerRegistryProvider(conf_file=conf).create_recognizer_registry() + analyzer = AnalyzerEngine(registry=registry, nlp_engine=nlp_engine) + + results = analyzer.analyze("Member ID ABC123456", language="en") + + assert [result.entity_type for result in results] == ["MY_ENTITY"] +``` + +This catches, at minimum: + +- Constructor signatures incompatible with the keys the loader passes (`name`, + `supported_entity`, `context`) — e.g. + `TypeError: __init__() got an unexpected keyword argument 'name'`, which takes + down construction of the whole registry the moment the recognizer is enabled. +- Class name typos and missing `__init__.py` exports. +- `country_code` mismatches between the class attribute and the YAML entry. +- Class-level defaults (thresholds, context) that configuration silently discards. +- A recognizer whose declared languages are excluded by the top-level + `supported_languages` filter, which loads nothing and reports no error. + +**Non-English recognizers:** the top-level `supported_languages` key acts as a +global filter and the shipped default is `["en"]`. A recognizer supporting only +`de` will not load from that config — silently. The test config and the PR +description must state the required top-level languages. + +**Construction paths must agree.** Building the recognizer directly, adding it +via `registry.add_recognizer()`, and loading it from configuration must all +produce the same recognizer. Flag defaulting or validation logic applied on one +path but not the others. + +## Placement and naming + +- Country-specific: `predefined_recognizers/country_specific/{country}/`; + generic patterns: `predefined_recognizers/generic/`; NLP/ML-based: + `nlp_engine_recognizers/` or `ner/`; third-party: `third_party/`. +- New country directories use the full lowercase country name (`south_africa`, + `philippines`, `canada`). Some pre-existing directories use short forms + (`us`, `uk`, `thai`); do not imitate them. +- `supported_language` and the YAML `supported_languages` key take **ISO 639-1 + language codes** (`ko` for Korean), not country codes (`kr`). A mismatch + produces a recognizer that never loads, with no error. + +## Pattern scores + +The base score must reflect how much the pattern *alone* narrows the space, +independent of any downstream threshold: + +| Score | Use when | Name the pattern | +| --- | --- | --- | +| 0.05–0.1 | Bare digit or alphanumeric runs, no structure | `"(very weak)"` | +| 0.1–0.3 | Some structure: delimiters, a prefix, a length constraint | `"(weak)"` | +| 0.3–0.5 | Distinctive format, no validation | `"(medium)"` | +| 0.5+ | Distinctive format | `"(strong)"` | + +Compare against existing recognizers before accepting a score +(`UsPassportRecognizer` uses 0.05 for nine bare digits). A 0.3 on a pattern that +also matches `covid19` or `sha256` is overstated. Coincidental matches at a low +score are the mechanism working as designed — a threshold filters them, and +context or validation raises the real ones. + +Suppress low-confidence matches with `score_thresholds`, not by requiring +context: `presidio-structured` has no surrounding text to draw context from. + +## Context words + +`LemmaContextAwareEnhancer` defaults to `context_matching_mode="substring"`, so +short context words fire on unrelated tokens: `member` matches `remember`, +`auth` matches `author` and `OAuth`. Prefer context long enough to be +unambiguous (`member id`, `subscriber`, `prior authorization`). + +Context is prefix-only by default (`context_prefix_count=5`, +`context_suffix_count=0`), so a context word *after* the match does not boost +the score. Tests should cover both placements. + +## Validation and invalidation hooks + +| Hook | Return | Effect on the result | +| --- | --- | --- | +| `validate_result` | `True` | Score replaced with `MAX_SCORE` (1.0) | +| `validate_result` | `False` | Score set to `MIN_SCORE` and result dropped | +| `validate_result` | `None` | Pattern score stands unchanged | +| `invalidate_result` | `True` | Score set to `MIN_SCORE` and result dropped | + +- **`True` is a jump to full confidence, not a nudge.** Ask what fraction of + arbitrary same-shape tokens would pass: a mod-11 check on a 17-character token + passes ~9% of the time, sending ~9% of coincidental matches to 1.0 where no + threshold can filter them. Only promote where the check is genuinely mandatory + for that value. +- **Return `None`, never `False`, when the check does not apply.** `False` means + "definitely not the entity" and discards the result. +- **No checksum is fine.** About 40% of predefined recognizers do not override + `validate_result`; the base score plus a threshold is a valid design. Do not + request an invented validator — but do flag one that promotes weak matches. +- Well-known sample values and reserved ranges belong in `invalidate_result`, + not buried in the regex. + +## Enabled by default or not + +Global (non-country-specific) recognizers — credit card, email, IBAN, IP, URL — +generally ship `enabled: true`, provided their false-positive rate is low. +Country-specific recognizers default to `enabled: false`. In both cases the +deciding question is whether the recognizer can produce **high-confidence false +positives**; shipping enabled requires justification in the PR description and +both of: + +- The base score is calibrated to the pattern's specificity (bands above). +- Nothing promotes a coincidental match to a score the user cannot filter. + +## Test quality + +- **Assert exact scores, not ranges.** `assert 0.5 <= score <= 1.0` still + passes when checksum promotion or context enhancement breaks entirely. Pin it: + `assert result.score == pytest.approx(EntityRecognizer.MAX_SCORE)`. +- **Include a lookalike negative** — a plausible non-PII token of the same shape + (a 17-character order ID for a VIN, a legal citation for a bank account + number) asserted as *not* flagged. This is the actual false-positive surface. +- **Exercise context enhancement.** A recognizer defining `CONTEXT` needs a test + showing the score differs between text with and without a context word. +- **Assert entity boundaries** (exact start/end), not just entity type — and + include values embedded in surrounding text. +- **Use example values the recognizer actually accepts.** Well-known samples + like `123-45-6789` are denylisted by `UsSsnRecognizer`: as a true positive the + test fails, and as a false-positive case it passes for the wrong reason. + +## Required companion updates + +A new recognizer needs all of: + +1. Export in `presidio_analyzer/predefined_recognizers/__init__.py` **and** the + country/category `__init__.py`. +2. An entry in `presidio_analyzer/conf/default_recognizers.yaml` (normally + `enabled: false`). +3. A row in `docs/supported_entities.md`. +4. A docstring citing the pattern's source: the official standard, government + specification, or authoritative reference the format comes from, plus the + validation algorithm if any (e.g. "Luhn checksum per ISO/IEC 7812"). diff --git a/.github/instructions/yaml-config.instructions.md b/.github/instructions/yaml-config.instructions.md new file mode 100644 index 000000000..2602a742a --- /dev/null +++ b/.github/instructions/yaml-config.instructions.md @@ -0,0 +1,108 @@ +--- +applyTo: "presidio-analyzer/presidio_analyzer/input_validation/**,presidio-analyzer/presidio_analyzer/recognizer_registry/**,presidio-analyzer/presidio_analyzer/conf/**,presidio-analyzer/presidio_analyzer/nlp_engine/ner_model_configuration.py,presidio-analyzer/tests/test_yaml_recognizer_models.py,presidio-analyzer/tests/test_recognizer_registry_provider.py,presidio-analyzer/tests/test_configuration_validator.py,presidio-analyzer/tests/test_config_loader.py,presidio-analyzer/tests/test_recognizer_registry.py,presidio-analyzer/tests/test_ner_model_configuration.py" +--- + +# YAML configuration & pydantic validation layer + +Rules for the layer that translates YAML configuration into Presidio instances: +the pydantic models in `presidio_analyzer/input_validation/` +(`yaml_recognizer_models.py`, `schemas.py`), the loaders in +`recognizer_registry/`, and the shipped configs in `conf/`. + +This layer is a public contract. YAML files written by users years ago must keep +parsing, and every field a user can write must actually reach the object it +configures. When reviewing, lead with: + +1. A YAML-reachable field that silently goes nowhere (schema/constructor drift). +2. A change that makes existing YAML files stop parsing or change meaning. +3. A validation failure surfacing as a distant `TypeError` instead of a parse-time + error with an actionable message. + +## Schema/constructor sync + +Every constructor parameter that should be settable from YAML needs a matching +pydantic field. In every contribution, check that constructor parameters and +schema fields have not drifted apart — a mismatch means a value a user writes +in YAML never reaches the object, or reaches it unvalidated. As of today the +consequence is silent: `PredefinedRecognizerConfig` ignores unknown YAML keys, +so a constructor kwarg without a schema field is dropped without any error and +the recognizer falls back to its defaults (the failure +`LangExtractRecognizerConfig` exists to prevent; see its docstring). Even if +that `extra` behavior changes, the no-mismatch rule stands. + +- A recognizer whose constructor takes model-specific kwargs needs a dedicated + config model registered in `CONFIG_MODEL_MAP` (keyed by `class_name` or + `name`), following `HuggingFaceRecognizerConfig` / `GLiNERRecognizerConfig` / + `LangExtractRecognizerConfig`. +- When a PR adds a constructor parameter to a recognizer that already has a + config model, require the matching field in that model — otherwise YAML users + cannot set it and get no error telling them so. + +## `extra` must be a deliberate choice + +- `extra="forbid"` for closed configs (`TextChunkerConfig`, + `RecognizerRegistryConfig`): typos fail fast at parse time with a clear + message. +- `extra="allow"` for pass-through configs whose kwargs flow to a constructor + (HuggingFace, GLiNER, LangExtract). +- Flag a new model that leaves pydantic's default (`extra="ignore"`) without + justification — silent ignoring is almost never the intended behavior. + +## `exclude_none` discipline on kwargs models + +Models whose dump is passed to a constructor override `model_dump` with +`exclude_none=True`, so a field omitted in YAML preserves the constructor +default instead of overriding it with an explicit `None`. Any new pass-through +config model must do the same; flag one that doesn't — it silently clobbers +constructor defaults, which is this layer's sneakiest backward-compatibility +trap. + +## Fail early, with actionable messages + +Validation belongs at parse time, in the pydantic model, phrased so the user +knows how to fix their YAML — not as a distant `TypeError` during registry +construction. House style to hold new code to: + +- Class existence checked at parse time + (`validate_predefined_recognizer_exists` → "Predefined recognizer 'X' not + found"), and custom/predefined name conflicts rejected with the fix spelled + out ("Either use type: 'predefined' or choose a different name"). +- Mutually exclusive fields enforced in a `model_validator` naming both fields + ("Cannot specify both 'supported_language' and 'supported_languages'"), with + an example of the correct form where the fix isn't obvious (see the global + context validator). +- Parameters checked against the selected mode (`TextChunkerConfig` rejects + `max_tokens` on a character chunker by name, listing the allowed fields). +- Prefer warnings over exceptions when the caller cannot fix the condition; + raising on a config the user didn't write turns a degraded result into a hard + failure. + +## Backward compatibility of the schema + +Existing user YAML must keep working. Each of these is a breaking change and +must be called out explicitly in the PR description: + +- A new **required** field, a renamed field, or a removed field. +- A tightened validator that rejects previously-accepted YAML. +- A changed default (including a changed `enabled`, score, or language default). +- Dropping support for the legacy singular forms. `supported_language` / + `supported_languages` and `supported_entity` / `supported_entities` both stay + supported, with mutual exclusivity enforced — do not remove the legacy form. +- Removing accepted input shapes: bare-string recognizer entries and dict + entries with inferred `type` (`patterns`/`deny_list` ⇒ custom) are all valid + today and must remain so. + +## Required tests for changes in this layer + +- **Round-trip through the provider**: load a config through + `RecognizerRegistryProvider` and assert on the constructed registry — not + only on the validated pydantic model. Model-level tests miss dump/loader + drift. +- The shipped `conf/default_recognizers.yaml` must validate against the models; + a change to either side needs `test_recognizer_registry_provider.py` / + `test_yaml_recognizer_models.py` updated in the same PR. +- New validators need both directions tested: valid YAML passes, invalid YAML + fails **with the expected message** (assert on the message — it is part of + the user experience). +- A new schema field needs a test proving the value actually reaches the + constructed object, not just that validation accepts it. diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 000000000..4d7822447 --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,116 @@ +# Presidio — Agent Guidelines + +Presidio is a Python SDK for detecting (presidio-analyzer) and anonymizing +(presidio-anonymizer) PII in text and images, plus CLI, structured-data, and +image-redaction components. It is a widely used **library**: users depend on +current detection behavior, and configuration files written years ago must keep +working. Correctness and backward compatibility outrank cleverness. + +The review-side versions of these rules — which the Copilot PR review agent +also enforces — live in `.github/copilot-instructions.md` and +`.github/instructions/*.instructions.md`. Follow them at authoring time so the +review finds nothing. + +## Working in this repo + +```bash +cd presidio-analyzer # or presidio-anonymizer, presidio-cli, ... +uv sync --locked --all-extras --group dev +uv run python -m spacy download en_core_web_lg # analyzer/CLI only +uv run pytest -xvv +uv run ruff check . && uv run ruff format . +``` + +- Python `>=3.10,<3.15`; code must run on every version in range. +- Dependencies are managed with **uv**, not pip/Poetry. If you touch a + package's `pyproject.toml` dependencies, run `uv lock` in that package and + commit the updated `uv.lock` in the same change — CI fails on drift. +- Do not edit `CHANGELOG.md`; release entries are generated from merged PRs. +- Never log PII values (`entity.text`) — only entity types and positions. +- Modules that process records are stateless; do not add state. +- Terminology: "threshold", not "cutoff"; ISO 639-1 language codes everywhere. + +## Adding a PII recognizer + +The full rulebook — score bands, context-word rules, validation-hook +semantics, the configuration-path test template, and the test-quality bar — +is `.github/instructions/recognizers.instructions.md`. **Read it before +starting**; those rules apply at authoring time, not just in review. The +workflow, in order: + +1. **Place and name it** under `predefined_recognizers/`: full lowercase + country name for new country directories (`south_africa`, not `za`; + don't imitate the pre-existing short forms `us`/`uk`/`thai`), or + `generic/`, `nlp_engine_recognizers/`, `ner/`, `third_party/` as + appropriate. +2. **Use ISO 639-1 language codes** (`ko` for Korean, never `kr`) — a + mismatch loads nothing, silently. +3. **Make the constructor loader-compatible**: accept the YAML loader's + kwargs (`name`, `supported_entity`, `context`, ...) and forward them to + the base class, or the recognizer crashes the whole registry the moment a + user enables it. +4. **Design the pattern for accuracy first** — this is the top review + priority: as specific as the format allows, score calibrated to the + pattern alone, unambiguous context words, the correct checksum if one + exists (and none invented if it doesn't), and the pattern's source + documented in the docstring, preferably an official specification. +5. **Register it everywhere**: exports in `predefined_recognizers/__init__.py` + *and* the country/category `__init__.py`; an entry in + `conf/default_recognizers.yaml` (country-specific ships `enabled: false`); + a row in `docs/supported_entities.md`. +6. **Write the configuration-path test** — the most-missed step and the one + that matters most: enable the recognizer in a YAML config, load it through + `RecognizerRegistryProvider`, and assert detection (template in the + instructions file). Non-English recognizers must set the top-level + `supported_languages` in the test config — it defaults to `["en"]` and + silently filters everything else. + +When **modifying** an existing recognizer: changed patterns, scores, or context +change detection results for existing users — state that in the PR description, +and never change an existing recognizer as a side effect of adding a new one. + +## Changing the YAML configuration layer + +The rulebook for the pydantic models in `presidio_analyzer/input_validation/` +is `.github/instructions/yaml-config.instructions.md` — **read it before +touching the layer**. The short version: + +- Constructor parameters and schema fields must never drift apart: a + YAML-settable kwarg without a matching pydantic field is silently dropped + today. Model-specific kwargs need a dedicated config model registered in + `CONFIG_MODEL_MAP`. +- Choose `extra` deliberately (`forbid` fails fast, `allow` passes through); + pass-through models dump with `exclude_none=True` so YAML omissions keep + constructor defaults. +- Validate at parse time with actionable messages, and never break existing + YAML — legacy singular fields, bare-string entries, and inferred `type` all + stay supported. +- Test through `RecognizerRegistryProvider`, not just the pydantic model: + prove new fields reach the constructed object and assert error *messages* + for invalid YAML. + +## General engineering rules + +- **Declare behavior changes.** Any edit outside a brand-new file needs the PR + description to say what existing behavior changes. Defaults on shared base + classes, properties on abstract interfaces, and anything altering returned + entities or scores all count, even with no signature change. +- **Explainability**: anything that changes how a score is derived must be + reflected in `AnalysisExplanation`. +- **Component boundaries**: data flows Analyzer → Anonymizer → Output; import + public interfaces, never another component's internals; shared models + (`RecognizerResult`, `OperatorConfig`) are contracts — update all consumers + in the same changeset. +- **Anonymizer operators**: non-reversible by default — no deterministic + hashing (rainbow tables), unpredictable replacement values, no preserved PII + characteristics. Deterministic or format-preserving output is sometimes a + hard requirement (e.g. referential integrity across a dataset); support it as + an explicit, documented opt-in, never as the default behavior. +- **Security**: never log PII values; validate untrusted inputs before use + (user-supplied regexes before compilation, file paths, API payloads); no + hardcoded secrets; no unsafe deserialization of untrusted models or pickles. +- **Performance**: no catastrophic regex backtracking (test long adversarial + inputs); cache compiled regexes; batch NLP with `nlp.pipe`. +- **Docs move with code**: `docs/supported_entities.md` for entities, + `docs/api-docs/api-docs.yml` for API changes, reST docstrings on public + APIs, samples for complex features. diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 000000000..08f04451c --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,3 @@ +# Claude Code guidelines for Presidio + +@AGENTS.md