Repository navigation
docs(adr-2993): erase data written before a rule, and never create the marker on erase - #3101
Conversation
…e marker on erase Data written before an IPCrypt rule took effect still holds raw addresses, and erase submit with the policy set encrypts every ruled matcher, so it cannot reach that data. An opt-in --plaintext-matcher submits a value as written and warns that the erasure log will hold it. erase submit also stops creating sys/ipcrypt/policy: an absent marker with a non-empty policy means no data was written under it, so it refuses instead of reporting a zero-record erasure. Refs: #2993 Signed-off-by: Panagiotis Moustafellos <pmoust@nofire.ai>
|
@claude-fleet review |
|
Review task queued. Task: fbe01815-4b54-4685-9cdc-03d4a6c91966 |
There was a problem hiding this comment.
Request changes: two should_fix findings, both about the amendment's claims against text it does not touch.
The amendment is well scoped and the marker checks pass (check-amendment-integrity.sh clean). Three problems: one sentence of the new text contradicts decision 8 one paragraph above it, decision 9's "First write" rule is now narrowed with no pointer or amendment-supersedes marker on it, and the new --plaintext-matcher leaves the mixed-matcher case unspecified in exactly the shape decision 8 elsewhere refuses.
Findings:
- should_fix, docs/adrs/2993-ipcrypt-attribute-pseudonymization.md:738 — "the one exception to the rule that an erasure request never records the raw value" is not true of decision 8
- should_fix, docs/adrs/2993-ipcrypt-attribute-pseudonymization.md:741 — decision 9's "First write" bullet still says every non-empty-policy process creates the marker, with no pointer and no
amendment-supersedes - nit, docs/adrs/2993-ipcrypt-attribute-pseudonymization.md:731 — a plaintext matcher mixed with an encrypted matcher in one command is a conjunction that matches nothing; the amendment does not say whether that is refused
- nit, docs/adrs/2993-ipcrypt-attribute-pseudonymization.md:365 — the decision 8 pointer says "never creates the marker" where the operative new behaviour is a refusal
Verified:
- The
amendment-appliesmarker's two named sections both carry the pointer text: Decision at 364-365 and 397-398, Consequences at 639-640 — and the guard agrees. - "Creating the marker would also make every ingesting process without the policy refuse at its next start" matches decision 9's startup check, "a marker whose fingerprint differs from the process's own, an empty policy included: the process refuses" — 432-434.
- "An absent marker with an empty local policy proceeds as before" matches 430-431.
- "no epoch fan-out of its own" is consistent with the epoch-combination cap, which counts only rule-carrying matchers — 371-382.
- The description's "Inline pointers go in decision 8 and in the Consequences paragraph about older data" matches the diff: two in decision 8, one in Consequences.
- No other doc or source file mentions
--plaintext-matcheror the ipcrypt marker's creation rule, so nothing else goes stale with this text; the code lands in the #3093 follow-up as the description says.
Checks run:
scripts/guards/check-amendment-integrity.sh→ clean, 359 amendments over 189 filesscripts/guards/check-adr-band-sources.sh→ cleanscripts/guards/check-claim-single-source.sh→ clean
Smaller things:
- The warning names only plaintext-matcher keys, but by decision 8 an ordinary matcher on a no-rule key, and a non-address value, also land raw in the erasure log with no warning. If the warning is meant to cover raw values in the log, it is narrower than the hazard.
- The heading says "erasure of data written before a rule", which covers the first bullet only; the marker gap is a separate decision riding under it.
Task fbe01815-4b54-4685-9cdc-03d4a6c91966 on rp2, model claude-opus-5, effort medium.
| refused, because an ordinary matcher already does that. An operator | ||
| who wants both the older raw data and the newer encrypted data erased | ||
| submits one command with the plaintext matcher and one without. This | ||
| is the one exception to the rule that an erasure request never records |
There was a problem hiding this comment.
should_fix: "This is the one exception to the rule that an erasure request never records the raw value" is not true of decision 8 as written, and the parenthetical added at line 397-398 propagates the same claim into the Decision.
Failure: a reader implementing or auditing the erasure log concludes that only a --plaintext-matcher can put a raw value there, and skips the other two paths. Decision 8 opens with "When a tenant has a rule for a matcher's key, ravel-cli erase submit encrypts that matcher's value" (356-358), so an ordinary matcher on a key with no rule is submitted raw -- this amendment says so itself two sentences earlier ("refused, because an ordinary matcher already does that", 734-735) -- and the Non-addresses bullet submits a non-address value unchanged (393-394). Both record the raw value in the log.
Evidence: lines 356-358, 393-394, 734-735, 737-739, 396-398.
Fix: say it is the one exception for a value that has a rule, or state the other paths and drop "one".
| is the one exception to the rule that an erasure request never records | ||
| the raw value. | ||
| - **erase submit never creates the marker.** Unlike a data writer, | ||
| `erase submit` does not create `sys/ipcrypt/policy` when it is absent. |
There was a problem hiding this comment.
should_fix: Decision 9's "First write" bullet is narrowed by this amendment but carries neither an inline pointer nor an amendment-supersedes marker.
Failure: line 419-421 reads "A process whose local policy is non-empty and that finds no marker creates it with CreateIfAbsent", unqualified. erase submit is enrolled in decision 9 explicitly ("ravel-cli erase submit reads it too (decision 8)", 427), so an implementer working from decision 9 writes the create path for it, which is the behaviour this amendment exists to forbid. The amendment-applies marker is section-level, so the guard passes while the contradicted sentence stands.
Evidence: lines 419-421 and 427 against 740-750; docs/adrs/README.md:258-266 requires a retired wording to be qualified where it survives.
Fix: add the pointer to the "First write" bullet ("the erasure amendment of 2026-10-19 below excepts erase submit"), and declare it with an amendment-supersedes: phrase=... marker so the guard holds it.
| rule, so it cannot reach that data. `erase submit` therefore takes a | ||
| repeatable `--plaintext-matcher KEY=VALUE`. A plaintext matcher is | ||
| submitted exactly as written, with no encryption and no epoch fan-out | ||
| of its own, alongside the other matchers. Every request carrying one |
There was a problem hiding this comment.
nit: The amendment says a plaintext matcher is submitted "alongside the other matchers" but does not say what happens when a command carries both a plaintext matcher and a rule-carrying one.
Failure: erase submit --plaintext-matcher client.address=203.0.113.7 --matcher host.ip=198.51.100.9 submits a conjunction of one raw and one encrypted value. Since a plaintext matcher is only allowed on a key that has a rule (734-735), a record written before the rules holds raw values for both keys and misses on the encrypted one, and a record written after holds ciphertext for both and misses on the plaintext one: no record can match, and the command reports a successful erasure of zero records. That is the outcome decision 8's Policy bullet and the "One request per epoch" bullet ("two ciphertexts of one address in the same request would match no record", 371-373) both exist to prevent.
Evidence: lines 371-373, 729-737.
Fix: say whether mixing is refused (consistent with the epoch rule) or merely warned about.
| otherwise submit the raw address, match nothing, and report a | ||
| successful erasure of zero records. | ||
| successful erasure of zero records. (The erasure amendment of | ||
| 2026-10-19 below adds that erase submit never creates the marker.) |
There was a problem hiding this comment.
nit: The pointer tells a decision 8 reader what erase submit no longer does, not what it now does.
Failure: the Policy bullet it sits in enumerates the refusal conditions ("refuses on the same conditions as decision 9: a fingerprint mismatch ... or a failed read"), and the amendment adds a third one that is in neither list. A reader who stops at the pointer learns the marker is not created and may assume the command proceeds.
Evidence: lines 359-365 against 740-750.
Fix: "(the erasure amendment of 2026-10-19 below adds that an absent marker with a non-empty policy is refused rather than created.)"
A plaintext matcher combined with an encrypted one is a conjunction no record can match, so the command refuses it. The exception sentence now covers only values whose key has a rule, decision 9's first-write rule points at the erase exception, and the decision 8 pointer names the refusal. Refs: #2993 Signed-off-by: Panagiotis Moustafellos <pmoust@nofire.ai>
|
Round 1 is addressed in the new head.
|
|
@claude-fleet review |
|
Review task queued. Task: 0becfb88-4482-43b0-bf1a-1993f4f34869 |
There was a problem hiding this comment.
Previous round at 12e27ea: 4 findings.
| Previous finding | Classification | Cited line |
|---|---|---|
docs/adrs/2993-ipcrypt-attribute-pseudonymization.md:738 (should_fix): "This is the one exception to the rule that an erasure request never records the raw value" is not true of decision 8 as written, and the parenthetical added at line 397-398 propagates the same claim into the Decision. Failure: a reader implementing or auditing the erasure log concludes that only a --plaintext-matcher can put a raw value there, and skips the other two paths. Decision 8 opens with "When a tenant has a rule for a matcher's key, ravel-cli erase submit encrypts that matcher's value" (356-358), so an ordinary matcher on a key with no rule is submitted raw -- this amendment says so itself two sentences earlier ("refused, because an ordinary matcher already does that", 734-735) -- and the Non-addresses bullet submits a non-address value unchanged (393-394). Both record the raw value in the log. Evidence: lines 356-358, 393-394, 734-735, 737-739, 396-398. Fix: say it is the one exception for a value that has a rule, or state the other paths and drop "one". |
open | docs/adrs/2993-ipcrypt-attribute-pseudonymization.md:398 |
docs/adrs/2993-ipcrypt-attribute-pseudonymization.md:741 (should_fix): Decision 9's "First write" bullet is narrowed by this amendment but carries neither an inline pointer nor an amendment-supersedes marker. Failure: line 419-421 reads "A process whose local policy is non-empty and that finds no marker creates it with CreateIfAbsent", unqualified. erase submit is enrolled in decision 9 explicitly ("ravel-cli erase submit reads it too (decision 8)", 427), so an implementer working from decision 9 writes the create path for it, which is the behaviour this amendment exists to forbid. The amendment-applies marker is section-level, so the guard passes while the contradicted sentence stands. Evidence: lines 419-421 and 427 against 740-750; docs/adrs/README.md:258-266 requires a retired wording to be qualified where it survives. Fix: add the pointer to the "First write" bullet ("the erasure amendment of 2026-10-19 below excepts erase submit"), and declare it with an amendment-supersedes: phrase=... marker so the guard holds it. |
fixed | docs/adrs/2993-ipcrypt-attribute-pseudonymization.md:421 |
docs/adrs/2993-ipcrypt-attribute-pseudonymization.md:731 (nit): The amendment says a plaintext matcher is submitted "alongside the other matchers" but does not say what happens when a command carries both a plaintext matcher and a rule-carrying one. Failure: erase submit --plaintext-matcher client.address=203.0.113.7 --matcher host.ip=198.51.100.9 submits a conjunction of one raw and one encrypted value. Since a plaintext matcher is only allowed on a key that has a rule (734-735), a record written before the rules holds raw values for both keys and misses on the encrypted one, and a record written after holds ciphertext for both and misses on the plaintext one: no record can match, and the command reports a successful erasure of zero records. That is the outcome decision 8's Policy bullet and the "One request per epoch" bullet ("two ciphertexts of one address in the same request would match no record", 371-373) both exist to prevent. Evidence: lines 371-373, 729-737. Fix: say whether mixing is refused (consistent with the epoch rule) or merely warned about. |
fixed | docs/adrs/2993-ipcrypt-attribute-pseudonymization.md:739 |
docs/adrs/2993-ipcrypt-attribute-pseudonymization.md:365 (nit): The pointer tells a decision 8 reader what erase submit no longer does, not what it now does. Failure: the Policy bullet it sits in enumerates the refusal conditions ("refuses on the same conditions as decision 9: a fingerprint mismatch ... or a failed read"), and the amendment adds a third one that is in neither list. A reader who stops at the pointer learns the marker is not created and may assume the command proceeds. Evidence: lines 359-365 against 740-750. Fix: "(the erasure amendment of 2026-10-19 below adds that an absent marker with a non-empty policy is refused rather than created.)" |
fixed | docs/adrs/2993-ipcrypt-attribute-pseudonymization.md:365 |
Re-review of the amendment at 04ef0e7. The three editorial findings from the last round are addressed; one is still partly standing, and the new "refuses instead" text contradicts the remedy it offers.
Findings:
- should_fix, docs/adrs/2993-ipcrypt-attribute-pseudonymization.md:757 — the refusal condition as stated also refuses the remedy its own message recommends
- should_fix (carried, narrowed), docs/adrs/2993-ipcrypt-attribute-pseudonymization.md:398 — the Decision's parenthetical still names the plaintext matcher as the only raw-value path, which the amendment itself contradicts at 746-748
Previous findings:
- 1: open, but only at line 396-399 now. The amendment body fix ("For a value whose key has a rule, this is the one way a raw value reaches the erasure log; matchers on keys with no rule, and values that are not addresses, are recorded as written, as before", 745-748) resolves the half I quoted at 737-739. The parenthetical added to the Decision sentence is the surviving half.
- 2: fixed. Inline pointer on the "First write" bullet (421-422) plus the
amendment-supersedesmarker at 723;check-amendment-integrity.shexits 0, so the phrase is qualified where it survives. - 3: fixed. 738-743 refuses the mixed command and gives the conjunction reason.
- 4: fixed. 364-366 now states the added refusal rather than only what no longer happens.
Verified:
- The
amendment-supersedesphrase "finds no marker creates it with" occurs only at 421, where the pointer is on the match's own line —docs/adrs/README.md:265-269requires exactly that window. sections="Decision|Consequences"both carry the pointer: 364-366 and 641-642.- The mixing refusal is consistent with the "One request per epoch" rationale at 371-373 (conjunction, no record matches).
Checks run:
scripts/guards/check-amendment-integrity.sh→ exit 0, "clean (359 amendment(s) over 189 file(s))"
Smaller things:
- 421 is 87 columns, against this file's ~72-column wrap; 739-740 ("of its own. Every request carrying one") leaves a short line from the edit. Cosmetic.
Not checked:
- The code this amendment describes: no
--plaintext-matcherimplementation exists in the tree yet (the PR says it lands in a follow-up to #3093), so only the document's internal consistency was reviewed. - Issues #3049, #3093 and #2993 and the maintainer decision of 2026-10-19 cited in the description are not reachable from this clone.
Task 0becfb88-4482-43b0-bf1a-1993f4f34869 on rp1, model claude-opus-5, effort medium.
|
|
||
| The request records the encrypted value, never the raw one: the | ||
| erasure log is itself retained data. | ||
| erasure log is itself retained data (except for a plaintext matcher the |
There was a problem hiding this comment.
should_fix: Still open since 12e27ea. "This is the one exception to the rule that an erasure request never records the raw value" is not true of decision 8 as written, and the parenthetical added at line 397-398 propagates the same claim into the Decision.
Failure: a reader implementing or auditing the erasure log concludes that only a --plaintext-matcher can put a raw value there, and skips the other two paths. Decision 8 opens with "When a tenant has a rule for a matcher's key, ravel-cli erase submit encrypts that matcher's value" (356-358), so an ordinary matcher on a key with no rule is submitted raw -- this amendment says so itself two sentences earlier ("refused, because an ordinary matcher already does that", 734-735) -- and the Non-addresses bullet submits a non-address value unchanged (393-394). Both record the raw value in the log.
Evidence: lines 356-358, 393-394, 734-735, 737-739, 396-398.
Fix: say it is the one exception for a value that has a rule, or state the other paths and drop "one".
| without the policy refuse at its next start. erase submit refuses | ||
| instead, and its message says that the bucket's data was written | ||
| without the policy, so the erasure should be submitted without | ||
| `--ipcrypt-config`, or with plaintext matchers. An absent marker with |
There was a problem hiding this comment.
should_fix: The refusal is stated on the policy alone, so it also refuses the second remedy its own error message names.
Failure: with an absent marker and a non-empty local policy, erase submit --ipcrypt-config cfg --plaintext-matcher client.address=203.0.113.7 is refused by this bullet (and by the unconditional pointer at 364-366, "an absent marker with a non-empty policy is refused rather than created"), yet the message tells the operator to submit "with plaintext matchers". Dropping --ipcrypt-config instead does not rescue it: with an empty policy no key has a rule, and 737-738 refuses a plaintext matcher on a key with no rule. So an implementer following this text refuses both spellings of the remedy, and data written before the rules cannot be erased at all in the absent-marker case.
Evidence: lines 752-758 against 364-366 and 737-738; the bullet's own rationale at 754-756 is about encrypted matchers ("every encrypted matcher would match nothing"), which a plaintext-only request has none of.
Fix: condition the refusal on the request carrying at least one matcher that would be encrypted, and say that a plaintext-only request proceeds with the policy loaded.
|
|
||
| The request records the encrypted value, never the raw one: the | ||
| erasure log is itself retained data. | ||
| erasure log is itself retained data (except for a plaintext matcher the |
There was a problem hiding this comment.
should_fix: The Decision still says the request records "the encrypted value, never the raw one" with the plaintext matcher as the single exception, which the amendment itself contradicts.
Failure: an auditor reading decision 8 concludes the erasure log holds a raw value only when an operator passed --plaintext-matcher, and does not audit the other two paths. Decision 8 encrypts only matchers whose key has a rule (356-358), and the Non-addresses bullet submits a non-address value unchanged (393-394); the amendment says so too ("matchers on keys with no rule, and values that are not addresses, are recorded as written, as before", 746-748).
Evidence: lines 396-399 against 356-358, 393-394 and 746-748.
Fix: narrow the parenthetical the same way the amendment did, e.g. "the request records the encrypted value rather than the raw one for every matcher whose key has a rule, except a plaintext matcher the operator names explicitly".
…rker A plaintext-only request on a bucket with no marker proceeds with the policy loaded and writes no marker, so data written before the policy can still be erased. The decision 8 parenthetical names the raw-value exception only for matchers whose key has a rule. Refs: #2993 Signed-off-by: Panagiotis Moustafellos <pmoust@nofire.ai>
|
Round 2 is addressed in the new head.
|
|
@claude-fleet review |
|
Review task queued. Task: d7071c4a-c1ff-4cb1-9960-b8642b25ceb2 |
There was a problem hiding this comment.
Previous round at 04ef0e7: 6 findings.
| Previous finding | Classification | Cited line |
|---|---|---|
docs/adrs/2993-ipcrypt-attribute-pseudonymization.md:398 (should_fix): "This is the one exception to the rule that an erasure request never records the raw value" is not true of decision 8 as written, and the parenthetical added at line 397-398 propagates the same claim into the Decision. Failure: a reader implementing or auditing the erasure log concludes that only a --plaintext-matcher can put a raw value there, and skips the other two paths. Decision 8 opens with "When a tenant has a rule for a matcher's key, ravel-cli erase submit encrypts that matcher's value" (356-358), so an ordinary matcher on a key with no rule is submitted raw -- this amendment says so itself two sentences earlier ("refused, because an ordinary matcher already does that", 734-735) -- and the Non-addresses bullet submits a non-address value unchanged (393-394). Both record the raw value in the log. Evidence: lines 356-358, 393-394, 734-735, 737-739, 396-398. Fix: say it is the one exception for a value that has a rule, or state the other paths and drop "one". |
open | docs/adrs/2993-ipcrypt-attribute-pseudonymization.md:398 |
docs/adrs/2993-ipcrypt-attribute-pseudonymization.md:421 (should_fix): Decision 9's "First write" bullet is narrowed by this amendment but carries neither an inline pointer nor an amendment-supersedes marker. Failure: line 419-421 reads "A process whose local policy is non-empty and that finds no marker creates it with CreateIfAbsent", unqualified. erase submit is enrolled in decision 9 explicitly ("ravel-cli erase submit reads it too (decision 8)", 427), so an implementer working from decision 9 writes the create path for it, which is the behaviour this amendment exists to forbid. The amendment-applies marker is section-level, so the guard passes while the contradicted sentence stands. Evidence: lines 419-421 and 427 against 740-750; docs/adrs/README.md:258-266 requires a retired wording to be qualified where it survives. Fix: add the pointer to the "First write" bullet ("the erasure amendment of 2026-10-19 below excepts erase submit"), and declare it with an amendment-supersedes: phrase=... marker so the guard holds it. |
fixed | docs/adrs/2993-ipcrypt-attribute-pseudonymization.md:422 |
docs/adrs/2993-ipcrypt-attribute-pseudonymization.md:739 (nit): The amendment says a plaintext matcher is submitted "alongside the other matchers" but does not say what happens when a command carries both a plaintext matcher and a rule-carrying one. Failure: erase submit --plaintext-matcher client.address=203.0.113.7 --matcher host.ip=198.51.100.9 submits a conjunction of one raw and one encrypted value. Since a plaintext matcher is only allowed on a key that has a rule (734-735), a record written before the rules holds raw values for both keys and misses on the encrypted one, and a record written after holds ciphertext for both and misses on the plaintext one: no record can match, and the command reports a successful erasure of zero records. That is the outcome decision 8's Policy bullet and the "One request per epoch" bullet ("two ciphertexts of one address in the same request would match no record", 371-373) both exist to prevent. Evidence: lines 371-373, 729-737. Fix: say whether mixing is refused (consistent with the epoch rule) or merely warned about. |
fixed | docs/adrs/2993-ipcrypt-attribute-pseudonymization.md:739 |
docs/adrs/2993-ipcrypt-attribute-pseudonymization.md:365 (nit): The pointer tells a decision 8 reader what erase submit no longer does, not what it now does. Failure: the Policy bullet it sits in enumerates the refusal conditions ("refuses on the same conditions as decision 9: a fingerprint mismatch ... or a failed read"), and the amendment adds a third one that is in neither list. A reader who stops at the pointer learns the marker is not created and may assume the command proceeds. Evidence: lines 359-365 against 740-750. Fix: "(the erasure amendment of 2026-10-19 below adds that an absent marker with a non-empty policy is refused rather than created.)" |
fixed | docs/adrs/2993-ipcrypt-attribute-pseudonymization.md:364 |
docs/adrs/2993-ipcrypt-attribute-pseudonymization.md:757 (should_fix): The refusal is stated on the policy alone, so it also refuses the second remedy its own error message names. Failure: with an absent marker and a non-empty local policy, erase submit --ipcrypt-config cfg --plaintext-matcher client.address=203.0.113.7 is refused by this bullet (and by the unconditional pointer at 364-366, "an absent marker with a non-empty policy is refused rather than created"), yet the message tells the operator to submit "with plaintext matchers". Dropping --ipcrypt-config instead does not rescue it: with an empty policy no key has a rule, and 737-738 refuses a plaintext matcher on a key with no rule. So an implementer following this text refuses both spellings of the remedy, and data written before the rules cannot be erased at all in the absent-marker case. Evidence: lines 752-758 against 364-366 and 737-738; the bullet's own rationale at 754-756 is about encrypted matchers ("every encrypted matcher would match nothing"), which a plaintext-only request has none of. Fix: condition the refusal on the request carrying at least one matcher that would be encrypted, and say that a plaintext-only request proceeds with the policy loaded. |
fixed | docs/adrs/2993-ipcrypt-attribute-pseudonymization.md:756 |
docs/adrs/2993-ipcrypt-attribute-pseudonymization.md:398 (should_fix): The Decision still says the request records "the encrypted value, never the raw one" with the plaintext matcher as the single exception, which the amendment itself contradicts. Failure: an auditor reading decision 8 concludes the erasure log holds a raw value only when an operator passed --plaintext-matcher, and does not audit the other two paths. Decision 8 encrypts only matchers whose key has a rule (356-358), and the Non-addresses bullet submits a non-address value unchanged (393-394); the amendment says so too ("matchers on keys with no rule, and values that are not addresses, are recorded as written, as before", 746-748). Evidence: lines 396-399 against 356-358, 393-394 and 746-748. Fix: narrow the parenthetical the same way the amendment did, e.g. "the request records the encrypted value rather than the raw one for every matcher whose key has a rule, except a plaintext matcher the operator names explicitly". |
open | docs/adrs/2993-ipcrypt-attribute-pseudonymization.md:398 |
No longer carried to the next round, settled in rounds 2 and 3:
- docs/adrs/2993-ipcrypt-attribute-pseudonymization.md:421 (should_fix): Decision 9's "First write" bullet is narrowed by this amendment but carries neither an inline pointer nor an
amendment-supersedesmarker. Failure: line 419-421 reads "A process whose local policy is non-empty and that finds no marker creates it withCreateIfAbsent", unqualified.erase submitis enrolled in decision 9 explicitly ("ravel-cli erase submitreads it too (decision 8)", 427), so an implementer working from decision 9 writes the create path for it, which is the behaviour this amendment exists to forbid. Theamendment-appliesmarker is section-level, so the guard passes while the contradicted sentence stands. Evidence: lines 419-421 and 427 against 740-750;docs/adrs/README.md:258-266requires a retired wording to be qualified where it survives. Fix: add the pointer to the "First write" bullet ("the erasure amendment of 2026-10-19 below exceptserase submit"), and declare it with anamendment-supersedes: phrase=...marker so the guard holds it. - docs/adrs/2993-ipcrypt-attribute-pseudonymization.md:739 (nit): The amendment says a plaintext matcher is submitted "alongside the other matchers" but does not say what happens when a command carries both a plaintext matcher and a rule-carrying one. Failure:
erase submit --plaintext-matcher client.address=203.0.113.7 --matcher host.ip=198.51.100.9submits a conjunction of one raw and one encrypted value. Since a plaintext matcher is only allowed on a key that has a rule (734-735), a record written before the rules holds raw values for both keys and misses on the encrypted one, and a record written after holds ciphertext for both and misses on the plaintext one: no record can match, and the command reports a successful erasure of zero records. That is the outcome decision 8's Policy bullet and the "One request per epoch" bullet ("two ciphertexts of one address in the same request would match no record", 371-373) both exist to prevent. Evidence: lines 371-373, 729-737. Fix: say whether mixing is refused (consistent with the epoch rule) or merely warned about. - docs/adrs/2993-ipcrypt-attribute-pseudonymization.md:365 (nit): The pointer tells a decision 8 reader what
erase submitno longer does, not what it now does. Failure: the Policy bullet it sits in enumerates the refusal conditions ("refuses on the same conditions as decision 9: a fingerprint mismatch ... or a failed read"), and the amendment adds a third one that is in neither list. A reader who stops at the pointer learns the marker is not created and may assume the command proceeds. Evidence: lines 359-365 against 740-750. Fix: "(the erasure amendment of 2026-10-19 below adds that an absent marker with a non-empty policy is refused rather than created.)"
Re-review of the two commits since 04ef0e7. The three nit/pointer findings and the over-broad refusal are fixed; one earlier finding survives in a narrower form, and the amendment's second bullet describes behaviour the already-merged erase submit code contradicts with no pointer to the change that will make it true.
Request changes: one previous finding still open (narrowed), one new should_fix.
Findings:
- open (narrowed), docs/adrs/2993-ipcrypt-attribute-pseudonymization.md:398 — previous findings 1 and 6: the "key has a rule" qualifier still overstates, because a rule-keyed value that is not an address is recorded raw
- should_fix, docs/adrs/2993-ipcrypt-attribute-pseudonymization.md:749 — the "never creates the marker" bullet reads as current behaviour, but shipped
erase submitdoes create it, and the amendment names no implementing change
Previous findings:
- 1 and 6: still open, but only for one case now. Line 398-400 reads "this holds for every matcher whose key has a rule", and amendment line 746 says "For a value whose key has a rule, this is the one way a raw value reaches the erasure log". The Non-addresses bullet (394-395) and the amendment's own next clause (747-748) both carve out non-address values, and that carve-out applies to rule-keyed matchers too:
plan_requestspushesMatcherValues::Plain(value)whenencryptreturnsRenderOutcome::Unparsed(services/ravel-cli/src/erase.rs:266-271, 281-283). So a matcher on a rule key whose value does not parse as an address records raw, which both sentences exclude. New version of the finding: narrow 398-400 and 746 to "every matcher whose key has a rule and whose value is an address", or drop "one way" and point at the Non-addresses bullet. - 2: fixed. Inline pointer at 422-423 plus
amendment-supersedesat 724;check-amendment-integrity.shexits 0, and the phrase occurs nowhere else in docs/. - 3: fixed at 739-746, with the one-command-each remedy spelled out.
- 4: fixed at 364-366.
- 5: fixed at 755-761; the refusal is now conditioned on at least one matcher that would be encrypted, and the plaintext-only request is stated to proceed with the policy loaded and no marker written. This is better than the fix I proposed, since it also says what happens to a request whose matchers are all on keys with no rule.
Verified:
- The
amendment-appliessections exist exactly once each and carry the pointer: "## Decision" (110), "## Consequences" (570), pointers at 364-366, 398-400, 641-643. - The mixing refusal matches the rationale the "One request per epoch" bullet gives (372-374 against 740-746).
- The plaintext-matcher remedy named in the absent-marker message is reachable: with the policy loaded, keys have rules, so 738-739 does not refuse it (755-761 against 738-739).
Checks run:
scripts/guards/check-amendment-integrity.sh-> exit 0, clean (359 amendments over 189 files)grep -rn "finds no marker creates it" docs/-> only line 422 and the marker at 724
Smaller things:
- 364-366: "an absent marker with a non-empty policy is never created, and refuses a request that would encrypt a matcher" makes the marker the subject of "refuses". Say "... is never created, and a request that would encrypt a matcher is refused".
- 735: "Every request carrying one" sits alone on a short line; the paragraph wraps unevenly there.
Not checked:
- The follow-up implementation of
--plaintext-matcher(not in this tree or on main). - Whether
docs/deletion-and-gc.md's erasure note (#3049) needs the plaintext matcher documented; it is out of this PR's scope and mentions no IPCrypt matcher handling today.
Not checked:
- The follow-up implementation of --plaintext-matcher, which does not exist in this tree or on origin/main.
- Whether docs/deletion-and-gc.md's erasure note needs a plaintext-matcher mention; out of this PR's scope.
Task d7071c4a-c1ff-4cb1-9960-b8642b25ceb2 on pmoust-mbp, model claude-opus-5, effort medium.
|
|
||
| The request records the encrypted value, never the raw one: the | ||
| erasure log is itself retained data. | ||
| erasure log is itself retained data (this holds for every matcher whose |
There was a problem hiding this comment.
should_fix: Still open since 04ef0e7. "This is the one exception to the rule that an erasure request never records the raw value" is not true of decision 8 as written, and the parenthetical added at line 397-398 propagates the same claim into the Decision.
Failure: a reader implementing or auditing the erasure log concludes that only a --plaintext-matcher can put a raw value there, and skips the other two paths. Decision 8 opens with "When a tenant has a rule for a matcher's key, ravel-cli erase submit encrypts that matcher's value" (356-358), so an ordinary matcher on a key with no rule is submitted raw -- this amendment says so itself two sentences earlier ("refused, because an ordinary matcher already does that", 734-735) -- and the Non-addresses bullet submits a non-address value unchanged (393-394). Both record the raw value in the log.
Evidence: lines 356-358, 393-394, 734-735, 737-739, 396-398.
Fix: say it is the one exception for a value that has a rule, or state the other paths and drop "one".
|
|
||
| The request records the encrypted value, never the raw one: the | ||
| erasure log is itself retained data. | ||
| erasure log is itself retained data (this holds for every matcher whose |
There was a problem hiding this comment.
should_fix: Still open since 04ef0e7. The Decision still says the request records "the encrypted value, never the raw one" with the plaintext matcher as the single exception, which the amendment itself contradicts.
Failure: an auditor reading decision 8 concludes the erasure log holds a raw value only when an operator passed --plaintext-matcher, and does not audit the other two paths. Decision 8 encrypts only matchers whose key has a rule (356-358), and the Non-addresses bullet submits a non-address value unchanged (393-394); the amendment says so too ("matchers on keys with no rule, and values that are not addresses, are recorded as written, as before", 746-748).
Evidence: lines 396-399 against 356-358, 393-394 and 746-748.
Fix: narrow the parenthetical the same way the amendment did, e.g. "the request records the encrypted value rather than the raw one for every matcher whose key has a rule, except a plaintext matcher the operator names explicitly".
| one without. For a value whose key has a rule, this is the one way a | ||
| raw value reaches the erasure log; matchers on keys with no rule, and | ||
| values that are not addresses, are recorded as written, as before. | ||
| - **erase submit never creates the marker.** Unlike a data writer, |
There was a problem hiding this comment.
should_fix: The bullet states "erase submit does not create sys/ipcrypt/policy when it is absent" in the present tense, but the merged implementation does create it, and the amendment names no change that will make the sentence true.
Failure: an operator or auditor reading the ADR concludes today's binary leaves the marker alone with an absent marker and a non-empty policy, so they run erase submit --ipcrypt-config cfg against such a bucket; the marker is created and every ingesting process without the policy refuses at its next start, which is exactly the outcome this bullet exists to prevent.
Evidence: services/ravel-cli/src/erase.rs:383-385 calls policy.check_marker_logged(...) on the erase path, and services/ravel-cli/src/ipcrypt.rs:195-201 reports CheckOutcome::Created by printing "created sys/ipcrypt/policy ..."; crates/ravel-ipcrypt/src/marker.rs:97 is the PutOptions::create_if_absent() that writes it. Both are on origin/main (37e63ae, "feat(cli): apply the IPCrypt policy to load and erase").
Fix: do what the Remote Write amendment does at lines 717-719 and name where the code lands ("the code change lands in the follow-up to #3093"), so the bullet reads as a decision rather than as a description of the shipped command.
|
Round 3 is addressed in the new head.
The 3 review rounds are used up, so merging this head is the maintainer's call. |
What
An amendment to ADR-2993 with two changes to erasure (decision 8).
erase submit --plaintext-matcher KEY=VALUE, which can be repeated, submits a value exactly as written, so data written before a rule took effect can be erased. Any request carrying one records the raw value in the erasure log, so the command warns and names the keys. A plaintext matcher on a key with no rule is refused. This is the one exception to "an erasure request never records the raw value".Inline pointers go in decision 8 and in the Consequences paragraph about older data.
Why
The checkpoint review of #3093 (#3049) found both. The maintainer chose the opt-in plaintext matcher on 2026-10-19. The code lands in a follow-up to #3093.
Refs: #2993
The maintainer approved this text on 2026-10-19, including the review-round changes (mixed matchers refused; only encrypting requests refused on an absent marker; the exception narrowed to address values under a ruled key; the pointer to #3108). The maintainer also approved merging without a 4th review.