fix(compliance): stop the phone PII pattern matching inside opaque identifiers

[cryptoxdog]

Problem

tests/test_handlers.py::test_match_returns_structure fails intermittently — it passes in isolation, so it reads like a flake:

tests/test_handlers.py:116: in test_match_returns_structure
    assert "query_id" in result
E   AssertionError: assert 'query_id' in {'candidates': [], 'execution_time_ms': 3.96,
    'match_direction': 'buyer_to_seller', 'scoring_meta': {...}, ...}

It is not a flake. handle_match builds query_id as q_ + uuid4().hex[:12], and PIICategory.PHONE was the only entry in _PII_PATTERNS without boundary guards, so any 10-digit run inside that opaque token matched the phone regex. ComplianceEngine.redact_response then removed the field outright — PIIHandler.redact pops the key rather than masking it — so the match response silently lost query_id.

Measured over 200,000 generated ids: 1.572% were classified as a phone number. The assertion was right; the engine was dropping the key on roughly 1 in 64 match requests.

Fix

Added (?<[0-9A-Za-z_]) / (?[0-9A-Za-z_]) guards to the phone pattern so a digit run cannot match inside a longer alphanumeric token.

Alternatives rejected:

• \b anchors (what SSN and IP_ADDRESS use) — unusable here. The phone pattern may start with + or (, which are not word characters, so a leading \b breaks +1 (555) 123-4567. SSN and IP can use \b only because they start with a digit.
• Changing query_id's format — cosmetic. It leaves every other opaque identifier misclassifiable.
• A redaction allowlist — new mechanism for a one-line defect, and a second place where PII policy lives.

Fixing the detector keeps _PII_PATTERNS the single source of truth for value-based PII.

Risk

• Low — additive, reversible, no data or contract change

Blast radius: PII value-pattern detection only. Phone detection by field name (_PII_FIELD_HINTS) is untouched, and redact_response is reachable only from handle_match (engine/handlers.py:641), so no other response path was affected.
Rollback: revert the single regex change; the regression tests revert with it.

Evidence

False positives eliminated, real detection retained:

$ python -c "... 200k generated query_ids through PIIHandler.detect_pii ..."
before: 3144/200000 = 1.572%   after: 0/200000
real phone still detected: ['phone']          # 555-123-4567 by value
phone-by-name still detected: ['phone']       # contact.phone by field name

Deterministic before/after on the same input (q_5c9382647927):

AFTER FIX  -> detect: []            redact keeps query_id: True
BEFORE FIX -> detect: [('query_id', 'phone')]   redact keeps query_id: False

Full suite, with a working Docker daemon so nothing was skipped for a missing service:

$ PYTHONPATH=. pytest tests/          # exit 0
2099 collected — 2033 passed, 0 failed, 10 skipped, 56 xfailed

$ PYTHONPATH=. pytest tests/integration/
50 collected — 50 passed, 0 skipped, in 46.93s   # live neo4j:5.18-enterprise testcontainer

$ ruff check . && ruff format --check .          # passed, 363 files
$ mypy engine/ --ignore-missing-imports          # no issues, 132 files
$ python tools/contract_scanner.py               # no violations
$ python tools/audit_harness.py                  # HARNESS PASSED

The 10 remaining skips are all pre-existing and none are environment-caused: 2 × openapi-spec-validator absent (not declared in requirements-dev.txt or requirements-ci.txt, so CI skips identically), 1 × shared-models.yaml not generated, 1 × documented @pytest.mark.skip from PR #62, 6 × guarded on unimplemented loader/generator methods. No test was skipped, weakened, or xfailed to reach green.

Gates

• Regression test added that fails without this fix
• No secrets, tokens, or customer data in code, tests, fixtures, or logs
• semgrep clean, or findings triaged below — semgrep, gitleaks and bandit all PASS on the changed files
• New IAM / workflow permissions are least privilege and enumerated — no IAM or workflow change
• Third-party actions pinned to a full commit SHA — no workflow change
• Public interface change is documented and versioned — no public interface change; this restores the documented response shape rather than altering it
• Observability exists for the new path — no new path; existing redaction logging is unchanged

Reviewer focus

The lookaround guards are the whole change — please sanity-check them against phone formats this repo cares about. Four are pinned by test_detect_phone_by_pattern_still_matches_real_numbers, and detection by field name is unaffected either way.

One deliberate narrowing: a number immediately followed by an alphanumeric, such as an inline extension 5551234567x123, is no longer matched by value. That is the same guard that stops opaque ids matching, and such a field is still caught by name if it is called phone.

Audited and found clean, so not changed here: the engine's other opaque identifier, enrichment_idempotency_key → ceg:enrich:<tenant>:<entity>:<16 hex>, measured 0/100,000 detections after this change.

Out of scope, reported not fixed: make cypher-lint is documented in AGENTS.md but has no Makefile target.

Note on scope: this branch originally also carried the fix for tests/unit/test_gate_egress.py::test_request_enrichment_fails_closed_without_gate_url, the other failure on main. Open PR #264 already contains that correction, byte-identical in the block it touches plus an extra assertion, so the duplicate was reverted here rather than contested — that failure is #264's to land, and it is the one red test expected on this PR until #264 merges.

Changes by intent

Modified

• engine/compliance/pii.py — guard the phone value-pattern so a digit run inside a longer token is not PII; this is the defect
• tests/compliance/test_hipaa.py — pin both directions: opaque query_id-shaped tokens are not PII, and four real phone formats still are. Without the second test the fix could silently degrade into a weakened detector

Files touched

pending — the bot fills this in on push

🤖 Generated with Claude Code

https://claude.ai/code/session_01Nwd3DgnqkpEuaBMPKYLhF3

---

Generated by Claude Code

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[github-actions]

✅ PR reviewable size is within recommended limits

[github-actions]

L9 Audit Harness Report

• Generated: 2026-09-19T01:41:29.354774+00:00
• Repo root: /home/runner/work/Cognitive.Engine.Graphs/Cognitive.Engine.Graphs
• Overall result: ✅ PASSED
• Exit code: 0

Step Results

Step	Status	Exit Code	Notes
Architecture Audit	✅ Passed	0
Spec Coverage	✅ Passed	0
Contract Wiring	✅ Passed	0

Architecture Audit Findings

Severity	Count
🔴 CRITICAL	0
🟠 HIGH	0
🟡 MEDIUM	17
🔵 LOW	0

See artifacts/audit_report.md for full details.

Spec Coverage

• ✅ Implemented: 37
• ⚠️ Partial: 9
• ❌ Missing: 0
• Total features: 46

Category	Implemented	Partial	Missing	Total
gates	10	0	0	10
scoring	7	0	0	7
v1.1_node	2	0	0	2
v1.1_edge	2	0	0	2
v1.1_action	0	2	0	2
v1.1_scoring	1	1	0	2
action_handler	0	6	0	6
gds_algorithm	5	0	0	5
research_pattern	10	0	0	10

See artifacts/coverage_report.md for full details.

Next Steps

All checks passed. Safe to merge.

[Copilot]

Copilot review overview

🟢 Approval recommended

The regex guard change is narrowly scoped, directly addresses the verified failure mode, and is backed by targeted regression tests for both false-positive prevention and true-positive retention.

Review effort: Lite
Findings: None

What changed in this PR

This PR fixes an intermittent loss of query_id in match responses by preventing the phone-number value regex from matching 10-digit runs embedded inside opaque alphanumeric identifiers (e.g., q_<12 hex>), which previously caused response redaction to drop the field.

Changes:

• Added alphanumeric/underscore lookaround guards to the PIICategory.PHONE regex to avoid matches inside longer tokens.
• Added regression coverage ensuring query_id-shaped opaque tokens are not detected as phone PII.
• Added coverage to ensure common real phone formats are still detected by value-pattern matching.

File	Description
engine/​compliance/​pii.py	Tightens the phone PII value regex with lookaround guards to avoid false positives in opaque identifiers.
tests/​compliance/​test_hipaa.py	Adds regression tests for the opaque-identifier false positive and for continued detection of real phone formats.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[cryptoxdog]

CI status on be15809 — two red checks, neither this PR's

1. Test Suite — failing, and it is the failure this PR deliberately does not carry

1 failed, 1973 passed, 37 skipped, 56 xfailed
FAILED tests/unit/test_gate_egress.py::test_request_enrichment_fails_closed_without_gate_url

That test is red on main today, independently of this PR — it was one of the two failures found when this work started. The other one, tests/test_handlers.py::test_match_returns_structure, is what this PR fixes, and it passes here.

A fix for the failing test exists and I have read it: #264, which corrects the same assertion. I am not porting it into this PR, and that is a deliberate call rather than a wait:

• This branch originally did carry that fix. The repo's own pre-push overlap gate blocked publication, naming tests/unit/test_gate_egress.py as textually conflicting with fix(test): repair the fail-closed enrichment assertion that is red on main #264 and Add IdeaOS portfolio graph domain and hydration seam #262.
• fix(test): repair the fail-closed enrichment assertion that is red on main #264's correction is byte-identical to mine in the block it touches, and additionally asserts the idempotency key on the SDK-error path — it is the better change.
• Carrying a duplicate would only create an add/add conflict for whichever of the two landed second. So I reverted it here (be15809) and left ownership of that path with fix(test): repair the fail-closed enrichment assertion that is red on main #264.

Merging #264 turns this check green with no change to this PR. If reviewers would rather see this PR self-contained, say so and I will stack it on #264's head instead.

2. OpenSSF Scorecard — infrastructure, red on main as well

The job dies before it does any analysis:

✅ Set up job
❌ Pull gcr.io/openssf/scorecard-action:v2.4.0
⏭️ Checkout Repository        (skipped)
⏭️ Run OpenSSF Scorecard Analysis (skipped)

The container image pull fails; checkout and the scan never run. This cannot be affected by a two-file Python diff, and the last five supply-chain.yml runs on main all failed the same way — including on 9c8e3cf, this PR's own base.

I re-ran the failed job once. It failed identically, at the same step, so it is not a flake. No fix for it exists to port: the remedy would be bumping the pinned ossf/scorecard-action SHA in .github/workflows/supply-chain.yml, which is unrelated to this diff and belongs in its own PR (it also touches the workflow-pin gates). I have not widened this PR to do it.

Everything else

Green, including Lint & Format (Ruff + MyPy), Security Scanning, Semgrep Policy Check, Secrets Scan (GitGuardian), CodeQL, Scan for Contract Violations, Verify L9_META Headers, SonarCloud, and the L9 audit harness (✅ PASSED, 0 critical / 0 high).

One note on coverage: CI reports 37 skipped against my local 10, because the hosted runner has no Docker and skips tests/integration/. Those 50 integration tests were run locally against a live neo4j:5.18-enterprise testcontainer for this change — 50 passed, 0 skipped — so the Docker-dependent path is covered even though CI does not exercise it.

---

Generated by Claude Code

[sonarqubecloud]

Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[cryptoxdog]

Correction: the enrichment fix is now ported into this PR (568493b)

My earlier comment said I was standing down on tests/unit/test_gate_egress.py and leaving it to #264. That was the wrong call and this supersedes it.

Two things changed my mind:

1. It was not one red check, it was seven. That single assertion was failing Test Suite, Pre-commit Hooks (via its pytest hook), Coverage (Codecov), Quality Gate, Baseline Ratchet / Required Tests, Baseline Ratchet / Ratchet Verdict — which recorded it as an unledgered finding — and the merge-blocking CI Gate rollup. That is not a cosmetic red mark to explain away; the PR could not merge.

2. The conflict was self-inflicted. #262 and #264 make byte-identical changes to this hunk. My first attempt rewrote the same assertion with an explanatory comment inside the dict, and that textual difference is what made it a real add/add conflict. The overlap gate was right to block it.

Porting the exact hunk both PRs carry — no edits of my own — three-way merges cleanly against both. The gate now agrees:

NOTE: overlap with PR #264 (claude/cognitive-engine-graphs-bootstrap-grag6c) is disjoint hunks — proceeding
NOTE: overlap with PR #262 (feat/idea-portfolio-domain) is disjoint hunks — proceeding
PASS: overlapping files merge cleanly (disjoint hunks)

It no-ops the moment main carries the change from whichever of #262/#264 lands first, so this creates no work for either.

Status

Check	Was	Now
Test Suite, Pre-commit Hooks, Coverage, Quality Gate, Baseline Ratchet, CI Gate	red on this one assertion	expected green on 568493b
OpenSSF Scorecard	red	still red — unchanged, see below

OpenSSF Scorecard stands as described earlier: it dies at Pull gcr.io/openssf/scorecard-action:v2.4.0 before checkout or any analysis, the last five supply-chain.yml runs on main failed identically including on this PR's base 9c8e3cf, and the one re-run I spent reproduced it at the same step. Not a flake, not this diff, and the remedy (bumping the pinned action SHA) belongs in its own PR rather than widening this one.

---

Generated by Claude Code