Keep rules: wallet tokens ship as sent (0.17.0, #159487)

[jab3z]

Dacian's decision on #159487 (2026-10-01): wallet tokens (Apple Pay, Google Pay) ship in logs exactly as sent. They are single-use, and debugging needs them. Saved-card tokens stay masked.

In 0.16.0, ECSCTX_MASK_VALUE_RULES could only replace a shape with its label. Turning Ottu's WALLET_RULES off would not have shipped the tokens either:

• the token container rule masks every leaf of a token held under token;
• signature and ephemeralPublicKey read as secrets;
• transactionId reads as a payment id;
• Connect's extra secret keys mask signature in bodies.

What this adds

• Keep rules: KeepRule(matches, hints), listed in the same ECSCTX_MASK_VALUE_RULES. The first rule to match wins. A kept value is copied as written, and the walk does not descend into it.

• Floor: nothing is kept under a CVV or SAD key, or inside a CVV/SAD container (in the key walk, and the key right before an object in text).

• Guard: a match is not kept when it holds, at any depth:

  • a key or {name, value} pair that reads as card, CVV or SAD;
  • a whole leaf that is a card number;
  • a key written twice;
  • a Python repr that does not re-render as written.

  So even KeepRule(lambda v: True) cannot ship card data. One exemption: 13-digit epoch milliseconds (from a 1 or a 2, so until 2065), which is Google Pay's keyExpiration.

• Text and bodies: a kept span is set aside behind a [KEPT-…-MASKED] placeholder before any content rule runs, then restored. This applies in mask_by_patterns and redact_body, entity-encoded bodies included.

• Helpers for a service's own masking pass: is_kept(value, key=) and mask_outside_kept(text, mask).

• Ottu contrib, breaking (feat(contrib)!):

  • WALLET_RULES are now strict keep rules: the Apple Pay token and its paymentData, and Google Pay. Exact key sets, version values, and text leaves only.
  • WALLET_SAD_RULES keep 0.16.0's labels, for any service that wants them.
  • Connect and Ottu PG list [*WALLET_RULES], so they flip on upgrade, as intended.

• Fix: configuring value rules clears the strings cached as clean.

• Docs: README "Keep rules", the Ottu paragraph, the redact_body and text-rules paragraphs, CLAUDE.md files and docstrings.

Verification

• Tests: 5765 passed on Python 3.12 and on 3.10 (4744 at 0.16.0). Each group's tests failed first.
• Golden file: golden/masking_0156.json is byte-identical under WALLET_SAD_RULES. Under WALLET_RULES, every golden input without a wallet hint still masks as the file says.
• Review: a task review requested changes (C1 duplicate keys and repr comments, I1, I2, M1), and all were fixed test-first. The scoped re-review approved, after 129,600 mutation-fuzz checks with 0 leaks.
• Bot review (two runs):
  • The split-cache churn is fixed (perf, 21774f4): 100 wallet lines split the rules 305 times, and now once.
  • Two README notes added (docs, a403054): a keep match the guard refuses asks no later rule (a test pins it), and the guard reads card data only, so PII or a credential inside a kept value ships with it.
• Bench (µs per call, v0.16.0 / this branch):
  • Without value rules: within noise.
  • With WALLET_RULES, on lines without a wallet: 1.5–4.6% slower.
  • On a line with a wallet: 651 µs, against 0.16.0's label rules at 555 (before the cache fix).
  • The cache fix alone, on wallet lines (an Apple Pay body, a KPay <udf9>): about 88 µs down to 82.

Parked (follow-up ticket, none blocking)

• M2: in text, a wallet in a list or pair under a CVV key is kept (the key walk floors it).
• M3: distinct kept values inside a hashed credential share one token.
• M4: a placeholder can be forged through an entity-encoded leaf; restore then copies the wallet. No card data or credential ships.
  • Fix it by naming placeholders from a hash of the text: deterministic, so tokens stay stable, and impossible to forge from inside the text.
  • Don't fix it by skipping a leaf that decodes to [KEPT-. A probe shows that ships the leaf's entity-encoded password in clear.
• M5: about 10% of entity-encoded Google Pay tokens are altered under pci. This fails closed.
• M6: a rule with both field_type and keep loads as a keep rule.
• M7 and N4: README wording on cost, and on text-only leaves.
• M8: nothing inside a kept value is labelled.
• N2: a repr inside JSON-encoded text is not kept. This fails closed.
• N3: free text in a wallet's text field ships (the guard is key-, pair- and leaf-based). The README now says so beside the guard; the follow-up is an alphabet check on opaque slots.
• Samsung Pay: its JWE needs a real captured token before a matcher can be pinned.

Release

Minor: 0.17.0. The release note must name the breaking change: Connect and Ottu PG ship wallet tokens as sent on upgrade. The feat(contrib)! commit subject carries it into the changelog. Connect and Ottu PG adopt it next, pinning >=0.17.0. Connect also adopts is_kept / mask_outside_kept in its own pass, and raises its body log cap so a whole Apple Pay token fits.

Refs #159487

🤖 Generated with Claude Code

[jab3z]

@claude review

[claude]

Claude finished @jab3z's task in 25s —— View job

---

Review of #71: keep rules for wallet tokens (0.17.0)

I read the ecsctx/masking/value_rules.py diff in full. I did not read the diffs for patterns.py, filters.py, config.py, contrib/net, or contrib/ottu/masking.py line by line. I could not run the test suite: pytest was blocked by the sandbox permissions. The 5757-pass figure is from the PR description and I haven't checked it.

What I checked

• Design. The keep decision is separate from the guard (holds_card_data) and from the floor, so KeepRule(lambda v: True) still can't ship card data. A keep matcher that raises counts as no match, which is correct on the redact_url and redact_body paths.
• Duplicate keys. json_as_written rejects duplicate keys, and text_ruling degrades KEEP to None when the text names a key twice. This closes the case where the shipped text carries a key that json.loads dropped.
• Epoch-milliseconds exemption. The regex [12][0-9]{12} with fullmatch on the stripped string is tight. Dropping the exemption to bare 13-digit values from a 1 or a 2 is justified by the comment.
• Fail-closed behaviour. Depth over 64, non-JSON objects and unknown types all count as card data, so the guard fails closed.

Findings

1. Rule-split cache churn on the text path (value_rules.py, _split and applicable). applicable() returns a fresh tuple whenever a hint filter drops at least one rule. _split() then stores it under id(rules), and a new tuple per call means a new entry per call. After 64 entries the whole cache is cleared, which also evicts the long-lived configured tuple. The tuple is also kept alive by its entry, so ids can't be reused wrongly, so this is a performance problem and not a correctness one. It only happens with mixed hinted rules, e.g. WALLET_RULES alongside other rules. It would show up as the "line with a wallet" cost in your bench. Cheap fixes:

   • cache applicable per (id(rules), matched-hint-mask); or
   • have text_ruling and label_within split once and filter the already-split keep and label lists, rather than re-splitting the filtered tuple.

2. hints validation. _hints_are_text accepts set and frozenset, which have no order. applicable only needs membership, so this is fine. The docs say "collection of strings", which matches.

3. Parked items. I agree that none of M2–M8 block the release. Two I would move up the follow-up list:

   • M4 (placeholder forgery through an entity-encoded leaf). The result is that restore copies a wallet. That is an integrity weakness in the very mechanism that sets kept spans aside. Add a per-call nonce to the placeholder, or reject input that already contains [KEPT-.
   • N3 (free text in a wallet's text field ships). This is a real PII path by design. Say so in the README's keep-rules section, not only in the follow-up ticket, because a service owner reading the docs will assume the guard covers it.

4. Breaking change on upgrade. Connect and Ottu PG flip from labels to shipped tokens automatically when they list [*WALLET_RULES]. That is the intended decision (#159487), and the PR says so. Make sure the 0.17.0 changelog or release note names it explicitly, since it changes what leaves the process.

Verdict

No blocking correctness problems in what I read. Finding 1 is a small performance fix worth doing before Connect adopts this. I'd confirm CI is green on 3.10 and 3.12 before merging, since I couldn't run it here. I can't approve PRs.
· branch feat/keep-rules

[jab3z]

@claude review

[claude]

Claude finished @jab3z's task in 24s —— View job

---

Review of #71: keep rules for wallet tokens (0.17.0)

I read the full ecsctx/masking/value_rules.py diff this run. I did not read the diffs for patterns.py, filters.py, config.py, contrib/net, contrib/ottu/masking.py or the tests line by line. I did not run the test suite, so I haven't checked the 5757-pass figure or the bench numbers. This repeats my earlier review where it still holds and adds what I confirmed on a second read.

Checked in value_rules.py

• Guard and floor are separate from the matcher. ruling() and label_within() both call holds_card_data before they return KEEP. KeepRule(lambda v: True) therefore can't ship a card key, a card-labelled {name, value} pair, or a whole-leaf PAN.
• Fail-closed paths. Depth above 64, non-JSON objects and unknown types all count as card data. JSON text that names a key twice counts as card data too.
• Duplicate keys. text_ruling degrades KEEP to None when _json_as_read reports the text wasn't parsed as written. What ships is then never narrower than what the guard read.
• Keep matchers that raise. They count as no match, so redact_url and redact_body stay non-raising. Label rules keep their 0.16.0 behaviour.
• Epoch-milliseconds exemption. [12][0-9]{12} with fullmatch on the stripped string is narrow, and the comment explains it.

Findings

1. Split-cache churn on the text path (confirmed in the code).

   • applicable() returns a new tuple whenever a hint filter drops a rule.
   • text_ruling, text_label and label_within then pass that tuple to _split(), which caches it under id(rules).
   • A fresh tuple per call means a fresh entry per call. At 64 entries _splits.clear() runs, which also evicts the long-lived configured tuple.
   • The cost is performance only. The entry holds the tuple, so an id can't be reused wrongly.
   • It happens only with a mix of hinted rules where some don't match the text, such as WALLET_RULES next to other rules. That is probably part of the 651 µs against 555 µs on the wallet line.
   • Fixes:
     • Split once on the configured tuple, then filter its keep and label lists by hint.
     • Or don't cache tuples that applicable() built.

2. A refused keep rule hides later label rules. _first_match picks the first matching rule, and the guard runs after it. If a keep rule matches and the guard refuses it, ruling() returns None and label_within() walks the children. A later label rule that also matches, for example WALLET_SAD_RULES after WALLET_RULES, is never asked. This fails closed, because the value is walked and the key and content rules still apply. It could still surprise someone who lists keep and label rules together and expects a fallback to the label. A docstring line would do, or the call could continue past a refused keep rule.

3. Parked items. None block the release. I would move two up the follow-up list:

   • M4, placeholder forgery through an entity-encoded leaf. Restore then copies a wallet. A per-call nonce in the placeholder, or rejecting input that already holds [KEPT-, would close it.
   • N3, free text in a wallet's text field ships. State this in the README "Keep rules" section as well as in the ticket. A service owner reading the docs will otherwise assume the guard covers it.

4. Upgrade behaviour. Connect and Ottu PG listing [*WALLET_RULES] changes what leaves the process on upgrade, from labels to shipped tokens. This matches the #159487 decision. Please name it explicitly in the 0.17.0 release note.

Verdict

I found no blocking correctness problem in the part I read. Finding 1 is worth fixing before Connect adopts this. Please confirm CI is green on 3.10 and 3.12 before merging. I can't approve PRs.
· branch feat/keep-rules