Repository navigation
Fixes #34386: Recognize complete card, URL and IP spans - #34674
stout-yeoman wants to merge 14 commits into
Conversation
|
Hi there 👋 Thanks for your contribution! The OpenMetadata team will review the PR shortly! Once it has been labeled as Let us know if you need any help! |
|
Hi there 👋 Thanks for your contribution! The OpenMetadata team will review the PR shortly! Once it has been labeled as Let us know if you need any help! |
|
Hi there 👋 Thanks for your contribution! The OpenMetadata team will review the PR shortly! Once it has been labeled as Let us know if you need any help! |
🚦 Removed from the merge queue —
|
🚦 Removed from the merge queue —
|
🚦 Removed from the merge queue —
|
🚦 Removed from the merge queue —
|
Code Review ✅ Approved 5 closed / 5 findings🔴 High risk · Changing credit-card, URL, and IP recognition could misclassify catalog data, leaving sensitive values untagged or applying sensitive tags to operational identifiers. Adds complete card, URL, and IP span detection to the Presidio PII recognizers, with exact original-text offsets and handling for grouped card formats and IPv6 boundaries. Resolved: IPs following non-CIDR paths outside http(s) URLs, IPv4 after Open: a valid card is still dropped when a non-card number sits beside it separated by a space or dash (e.g. ✅ 5 closed✅ Bug: IP dropped when followed by a non-CIDR path outside http(s) URLs
✅ Edge Case: IPv4 after
|
| Compact |
|
Was this helpful? React with 👍 / 👎 | Powered by Gitar — free for open source
|



Describe your changes:
Fixes #34386
Detect complete credit-card, URL and IP candidates with spans anchored to the original input. Embedded cards previously disappeared or returned offsets into sanitized text; URL and IPv6 matches could return incomplete spans.
The card follow-up recovers complete compact, spaced or dashed cards after a calendar-valid
YYYY-MM-DDdate separated by whitespace, or a case-insensitiveQty/Quantityinteger field. For example,2024-01-15 4111111111111111andQty 2 4111 1111 1111 1111recover the card span. Complete 19-digit candidates take priority. Both4111111111111111 123and4111 1111 1111 1111 123remain rejected. Existing Diners 4-6-4, repeated-space and separate-year behavior remains supported.Type of change:
High-level design:
Configured Presidio adapters in
presidio_utils.pyvalidate complete bounded candidates without changing thresholds or classification policy.ipaddress, retains IPv6 boundaries, and validates port/CIDR suffixes while returning the address span.CreditCardRecognizeridentity, its actual stored ID, score and pattern evidence, matching the upstream attribution contract.No schema or migration changes. Supported Luhn-valid card-shaped operational values still receive protective Sensitive tags. The numeric-prefix decision and rejected alternatives are recorded in
docs/decisions/2026-10-15-card-prefix-recovery-requires-explicit-fields.md. Expiry-date recovery and additional URL/IP edge cases are outside this follow-up.Tests:
Use cases covered
Exact original card/URL/IP spans, canonical and grouped cards (including Diners and 19 digits), consistent/repeated separators, invalid checksums and extra digits, phone collisions, explicit numeric prefixes and their negative controls, separate years, complete URL punctuation/authority/port handling, IPv4/IPv6/CIDR, and configured TagProcessor attribution.
Unit tests
At
512b065669e5889415ea46b631b9ed91624d9b97:cd ingestion ../env/bin/python -m pytest tests/unit/pii tests/unit/metadata/pii --cov=metadata.pii.algorithms.presidio_utils --cov-branch --cov-report=term-missing -q792 passed; 96% utility coverage including branches. Updated regression files:
tests/unit/pii/algorithms/test_presidio_utils.py,test_presidio_recognizer_factory.py, andtests/unit/metadata/pii/test_tag_processor_integration.py. Focused red/green tests preceded the full run. Scoped Ruff/format and configured basedpyright passed (0 errors, 0 warnings); applicable pre-commit checks andgit diff --checkpassed. Independent review found no actionable issues.Frozen replay: 3,104 existing cases, zero changes to spans, scores, names or pattern evidence after the numeric-prefix follow-up. Earlier corrective replay covered English plus Spanish/Italian/Polish card cases and a held-back English supplement; those historical results are superseded for current unit coverage by the run above.
Backend integration tests
Not applicable: no Java, REST endpoint, schema, migration, seed or pom changes.
python3 .github/scripts/plan_local_java_tests.py --check-branchpassed; no affected Java tests.Ingestion integration tests
Updated
ingestion/tests/integration/auto_classification/databases/{init.sql,test_tag_processor.py}. Ran the complete affected database test file against real PostgreSQL and OpenMetadata: 2 passed at512b065669. Used the normal repository fixtures/credentials with only the local server endpoint adapted to port 18585. Persisted labels verify configured ID/name, score 1.0 and matched patterns. The baseline reproduced the old subclass-name assertion failure before the correction.The gated
tests/integration/auto_classification/test_model_provisioning.pycheck reported 1 skipped, because no candidate wheel was supplied; packaging/model provisioning code is unchanged. No clean-wheel certification is claimed.Playwright (UI) tests
Shared validation uses local combined commit
d84bd7b9204de848334bce46f7e033ccefb6f804: the production union of exact published card head512b065669and identifier head0cf68d9f51, plus32 local preservation/control tests. UI sources and lockfiles are unchanged by either PR. A matching freshly built backend, production UI bundle, real managed Airflow runner and isolated MySQL/Metabase/Kafka/schema-registry fixtures resolve the earlier missing-team-type/runner prerequisites.The current combined impact planner selected 37 specs /616 tests. Full original execution against the Vite development UI completed 603 passed,13 failed,0 skipped. Original failures and traces are retained. Ten failures were traced to shared global settings interference/precondition conflicts (domain search, generic entity domain/glossary helpers and the logical test suite); three bulk-edit variants stalled before export in development mode.
Unchanged failed cases were rerun sequentially against the production UI with their normal settings preconditions. All13 original failing test IDs/projects now pass: Entity9/9 (including one extra control), Logical1/1, Domain3/3, Bulk3/3, plus setup2/2. Production bulk success supports the development lifecycle explanation. This is a full original run plus controlled reruns, not a claim that one uninterrupted616-case run was green. No assertion or product source was weakened. All six AutoPilot connectors and the real Incident workflow passed in the original run. A separate local tooling repair isolates global-setting writers and restores settings; it is outside both PII PRs.
Additional shared regression validation
TestCaseEvaluationScope; no full current-schema downstream certification is claimed.Manual testing performed
metadataCLI point to the isolated checkout at512b065669.metadata ingest -c ...andmetadata classify -c ...using the shipped TagProcessor.UI screen recording:
Not applicable: backend ingestion bug fixes, with no UI feature or UI source changes. The normal ingestion/classification CLI was exercised as described above.
Checklist:
Fixes <issue-number>: <short explanation>and the issue is linked above.Merge coordination
This PR shares
presidio_utils.py, database fixtures and processor tests with #34672. Merge sequentially, reconcile/rebase the remaining PR and rerun the combined fixture and configured TagProcessor checks. Complete internal-domain acceptance remains owned by #34569.