[pmoust]

What

The erasure amendment to ADR-2993 (#3101) and the review follow-ups from #3049.

• erase submit --plaintext-matcher KEY=VALUE (repeatable).
  • The value is submitted as written, with no encryption and no fan-out, so data written before a rule can be erased. A warning on stderr names the keys the erasure log will store in the clear.
  • A plaintext matcher on a key with no rule for --signal is refused. For metrics the key is looked up by its sanitized name.
  • A command that mixes a plaintext matcher with a matcher that would be encrypted is refused before anything is written, because no record could match both.
  • A request of plaintext matchers only goes ahead on a bucket with no marker, and writes no marker.
  • The refusal added by IPCrypt: erase submit must not create the policy marker #3109 now points at --plaintext-matcher.
• Status after a fan-out. Submit prints the exact erase status command for each derived id and says the base id stores nothing. docs/deletion-and-gc.md says status takes the derived ids. Resolving a base id inside status would need the policy and matchers, or a new stored object.
• Load.
  • A ruled key on a type = "bytes" logs column now prints a warning at setup.
  • The library load::load* entry points are documented as running with no IPCrypt rules and no marker check, for tests and benchmarks.
• Smaller changes.
  • A conflicting request no longer prints a rerun hint.
  • The fan-out line shows the rule key when it differs from the matcher key.
  • docs/guides/ingest.md no longer says logs load an attributes map.
  • The gc-config test doc comment is back above its own test.
• Configuration guide. A paragraph on write access as an encryption oracle (ADR-2993 write-access amendment, docs(adr-2993): record that write access is an encryption oracle #3113): only flag-configured tokens can split readers from writers by process; durable sys/auth tokens and OIDC cannot.
• The regenerated ravel-cli flag reference and a changelog fragment.

Tests

• Plaintext matcher: submits the raw value and warns. Also covered: refusal on a key with no rule, the mixed-command refusal (which fails if the check is removed), the sanitized metric label, and a plaintext-only request on a bucket with no marker.
• Partial failure and retry: FaultStore fails the second request put. A rerun with the same base id stores all four requests, each with its epoch combination's ciphertext. The test fails with random ids.
• Missing policy flag: a marker holding a policy plus an empty local policy is refused.
• Load scan: covers a classic histogram, a client.ip label through the sanitizer, and the bytes-column warning.

Executor gates: clippy and cargo test -p ravel-cli (441 in the bin suite, 27 erase tests), plus the docs check.

Known limit: the printed status commands carry tenant, signal and id but not the store flags the submit used. The docs say so.

Fixes: #3108
Refs: #2993

[pmoust]

@claude-fleet review

[claude-fleet]

Review task queued.

Task: f32c88c0-bf36-4549-a342-f467182572bf
Commit: 7bab864
Model: claude-opus-5 (effort medium)
Executor: pmoust-mbp
Status: done. Review posted.

[claude-fleet]

Reviewed the full diff, the code behind its claims, and ran the two revert checks.

Request changes: one should_fix finding.

Findings:

• should_fix, services/ravel-cli/src/load/cli.rs:493 — the ruled-bytes-column warning is wired into the logs arm only; a spans mapping has the same silent gap
• nit, services/ravel-cli/src/erase.rs:376 — a metrics --plaintext-matcher written in the rule's dotted spelling passes the rule check but can never match a stored label

Verified:

• A plaintext matcher is appended to every planned request after the plan, so it never fans out and never changes a derived id — erase.rs:480 with derived_request_id hashing only epochs (erase.rs:213-223).
• Plaintext-only proceeds with no marker and writes none: encrypted_keys is empty, which takes the None if ... encrypted_keys.is_empty() arm — ipcrypt.rs:174, plus the stored_marker(...) == None assertion in a_plaintext_matcher_submits_the_raw_value_and_warns.
• A plaintext matcher and a fan-out cannot co-occur, so "added to every request" is always exactly one request: plans.len() > 1 requires an encrypted candidate, which makes encrypted_keys non-empty and hits the mixed refusal first — erase.rs:456-473 against plan_requests' candidate loop (erase.rs:319-347).
• The mixed refusal and the unruled-key refusal both run before plan_requests / the marker check, so "Nothing was written" holds — erase.rs:443-473.
• Metrics look the key up through sanitize_rule_key, logs/spans by the raw key — rule_for, erase.rs:228-237.
• erase status takes --tenant/--signal/--request-id exactly as the printed line spells them, and reports UNKNOWN for an id with no .dreq/.done, which is what docs/deletion-and-gc.md now claims for the base id — main.rs:1606-1613, erase.rs:781.
• The conflict branch downcasts the error submit actually returns (SubmitError::ConflictingRequest.into()) — erase.rs:528-537 against erase.rs:161.
• ingest.md's "only spans load an attributes map": attrs_map_column is declared on both mappings (mapping.rs:118, 780) but read only in spans.rs:157; nothing in the logs loader reads it.
• configuration.md's "tokens carry no ingest or query scope": TokenCapabilities has exactly ddl and ipcrypt — crates/ravel-tenant-resolve/src/lib.rs:54-58.

Checks run:

• cargo test -p ravel-cli --lib erase:: → 27 passed, 0 failed.
• cargo test -p ravel-cli --bin ravel-cli erase → erase_submit_needs_a_matcher_or_a_plaintext_matcher passed.
• Disabled the mixed-matcher refusal condition (erase.rs:456) → a_plaintext_matcher_with_an_encrypted_matcher_is_refused_and_writes_nothing FAILED, 21 others passed. Restored.
• Disabled the SubmitError branch of the error-context arm (erase.rs:530) → a_conflicting_request_in_a_fan_out_gives_no_rerun_hint FAILED, 21 others passed. Restored; git status --porcelain clean apart from an untracked node_modules.

Smaller things:

• ruled_bytes_column_warnings is pub but has only the one in-crate caller; pub(crate) would match run_fold_warning_to next to it.
• The mixed-refusal message does not dedupe repeated --plaintext-matcher keys, while the stderr warning does (erase.rs:461 vs erase.rs:483). Cosmetic only.
• Nothing rejects the same key appearing in both --matcher and --plaintext-matcher when the ordinary one stays plain (empty or non-address value); the predicate then carries the key twice with different values and matches nothing. validate_request checks only non-empty keys (crates/ravel-commit/src/erasure.rs:194-204). This is pre-existing for repeated --matcher and not something this PR introduces.

Not checked:

• docs/reference/ravel-cli-flags.md as regenerated output (I read the two changed rows but did not rerun the generator).
• The claim in configuration.md that a durable sys/auth token resolves on every process reading the bucket, and the per-process OIDC audience argument.
• The histogram additions to ipcrypt_scan_tests.rs were read but not run (the scan suite was outside the test filters I ran).

Not checked:

• docs/reference/ravel-cli-flags.md as generator output (rows read, generator not rerun)
• configuration.md's claims that a durable sys/auth token resolves on every bucket-reading process and that OIDC needs a per-role --oidc-audience
• the classic-histogram cases added to load/ipcrypt_scan_tests.rs were read but not executed

Task f32c88c0-bf36-4549-a342-f467182572bf on pmoust-mbp, model claude-opus-5, effort medium.

[claude-fleet]

should_fix: ruled_bytes_column_warnings is called only in the MappingSection::Logs arm, so a spans mapping with the same hazard warns nothing.
Failure: a spans mapping with [[spans.attribute]] key = "client.ip" column = "raw_ip" type = "bytes" loads under a policy that rules client.ip; the addresses are stored unencrypted and the operator is told nothing. With the identical logs mapping the load now warns.
Evidence: the call site is inside the logs-only tail of run_fold_warning_to (load/cli.rs:493); run_spans (load/cli.rs:755-780) emits no equivalent. SpansMapping carries the same Vec<AttrMap> with a ColType value_type (load/mapping.rs:768-771, 55-62), and the shared encrypt path only touches string values (encrypt_attr, load/columns.rs:245-250), so bytes columns bypass encryption for spans exactly as for logs. docs/guides/ingest.md's new paragraph is also worded as logs-only.
Fix: take the two Vec<AttrMap> rather than &Mapping and call it from the spans arm too.

[claude-fleet]

nit: For metrics, check_plaintext_matchers accepts a key spelled the way the rule is written, but the key is submitted unsanitized, so the request cannot match a stored label.
Failure: erase submit --signal metrics --plaintext-matcher client.ip=192.0.2.1 passes the rule check (rule_for sanitizes to client_ip for the lookup, erase.rs:228-237) and writes a predicate whose key is client.ip, while the loader stores the label as client_ip (load/mapping.rs:588 applies sanitize_label_name). The request is accepted and silently matches nothing.
Evidence: erase.rs:376-397 validates through the sanitized form; erase.rs:480 appends the pair as written; load/mapping.rs:588.
This matches the documented --matcher behaviour ("the matcher key is submitted as written"), so it is pre-existing rather than new, but the new flag's whole purpose is matching stored data and the check is in a position to catch it.

[pmoust]

Round 1: fix round dispatched, along with the opus checkpoint findings: the status command must carry the store and tenancy flags before "erase", and there are new plaintext-only marker tests.

• should_fix, load/cli.rs:493 (spans bytes). The checkpoint reviewer read spans as turning bytes into strings before encryption. The fix round checks the code and either adds the spans warning or adds a test that proves spans encrypt the value.
• nit, erase.rs:376. Kept as is. Remote Write stores dotted label names as sent (ADR-2993 Remote Write amendment), so a plaintext matcher spelled client.ip does match those series. The matcher key is the stored spelling, as for --matcher.

[pmoust]

Round 1 and the opus checkpoint findings are addressed at e42bc67, folded into the single commit.

• Status command (checkpoint). Each printed command now carries the submit's own store and tenancy flags before erase: store kind, S3 endpoint, bucket, region and options, the credentials-file path, and the tenant-hash key file path. Credentials are never printed; when one was passed as a flag, a placeholder takes its place. A test re-parses every printed line and checks it reaches erase status with the same tenant, signal, id, store and key file. docs/deletion-and-gc.md is updated to match.
• should_fix, load/cli.rs:493 (spans bytes). Confirmed. Spans turn a bytes attribute into lowercase hex before the rules run, so it is stored unencrypted. The spans load now warns, a test asserts the warning and the stored hex, and ingest.md covers both signals.
• Plaintext-only tests (checkpoint). A plaintext-only request is refused against a mismatched marker and against a marker read failure, and nothing is written in either case.
• Nits.
  • The mixed-matcher refusal is now tested with and without a marker.
  • A plaintext matcher on an unruled metric key is refused.
  • The configuration-guide paragraph is now consistent: only flag-configured tokens can split readers from writers by process; sys/auth is read by all, gateway and query on a keyed bucket; OIDC needs a separate --oidc-audience per role.
  • The bytes warning wording now says it prints before any data is written.

[pmoust]

@claude-fleet review

[claude-fleet]

Review task queued.

Task: cc82aa7d-2c0a-4923-9366-cc1bcbf0030a
Commit: e42bc67
Model: claude-opus-5 (effort medium)
Executor: rp2
Status: done. Review posted.

[claude-fleet]

Previous round at 7bab864: 2 findings.

Previous finding	Classification	Cited line
services/ravel-cli/src/load/cli.rs:493 (should_fix): ruled_bytes_column_warnings is called only in the MappingSection::Logs arm, so a spans mapping with the same hazard warns nothing. Failure: a spans mapping with [[spans.attribute]] key = "client.ip" column = "raw_ip" type = "bytes" loads under a policy that rules client.ip; the addresses are stored unencrypted and the operator is told nothing. With the identical logs mapping the load now warns. Evidence: the call site is inside the logs-only tail of run_fold_warning_to (load/cli.rs:493); run_spans (load/cli.rs:755-780) emits no equivalent. SpansMapping carries the same Vec<AttrMap> with a ColType value_type (load/mapping.rs:768-771, 55-62), and the shared encrypt path only touches string values (encrypt_attr, load/columns.rs:245-250), so bytes columns bypass encryption for spans exactly as for logs. docs/guides/ingest.md's new paragraph is also worded as logs-only. Fix: take the two Vec<AttrMap> rather than &Mapping and call it from the spans arm too.	fixed	services/ravel-cli/src/load/cli.rs:473
services/ravel-cli/src/erase.rs:376 (nit): For metrics, check_plaintext_matchers accepts a key spelled the way the rule is written, but the key is submitted unsanitized, so the request cannot match a stored label. Failure: erase submit --signal metrics --plaintext-matcher client.ip=192.0.2.1 passes the rule check (rule_for sanitizes to client_ip for the lookup, erase.rs:228-237) and writes a predicate whose key is client.ip, while the loader stores the label as client_ip (load/mapping.rs:588 applies sanitize_label_name). The request is accepted and silently matches nothing. Evidence: erase.rs:376-397 validates through the sanitized form; erase.rs:480 appends the pair as written; load/mapping.rs:588. This matches the documented --matcher behaviour ("the matcher key is submitted as written"), so it is pre-existing rather than new, but the new flag's whole purpose is matching stored data and the check is in a position to catch it.	open	services/ravel-cli/src/erase.rs:376

Re-reviewed at e42bc67. Finding 1 is fixed; finding 2 still stands as a nit.

Approve: no blocking or should_fix finding.

Findings:

• nit (carried over), services/ravel-cli/src/erase.rs:376 — a metrics --plaintext-matcher key is validated sanitized and submitted unsanitized

Verified:

• Finding 1 fixed: ruled_bytes_column_warnings now takes the attribute iterator and is called from the spans arm too, with its own stored-form wording — load/cli.rs:49-65, 473-479, 504-512; the spans case is pinned by a_spans_bytes_column_under_a_ruled_key_is_stored_unencrypted_and_warns (load/ipcrypt_scan_tests.rs:626-707)
• "spans store their lowercase hex, which never parses as an address": span_attr_string hex-encodes bytes before encrypt_str is applied, on both the attribute and the resource-attribute path — load/spans.rs:549-553, 574-587, 475-494
• "Only spans load an attributes map; the logs attrs_map_column is export only": the logs field is write-side only and read nowhere under load/ — load/mapping.rs:112-118; span map entries do go through encrypt_str — load/spans.rs:804
• The fan-out status line carries the submit's top-level flags, and they are top-level flags (not accepted after erase) — main.rs:56-74, 101-117; store.rs:388-446; round-tripped by erase_submit_prints_status_commands_with_the_submit_flags (main.rs:5553-5667), which re-parses the printed line into an erase status with the same store and tenancy args
• Credentials are never echoed; a flag-given credential with its env var unset prints a placeholder — store.rs:433-444, test main.rs:5673-5718
• erase status needs no --ipcrypt-config, so omitting it from the echoed flags is right — main.rs:1623-1633
• "given the base id it reports UNKNOWN" — erase.rs:788
• Conflict errors drop the rerun hint, and SubmitError has only the conflict variant, so no retriable error loses the hint — erase.rs:62-78, 528-545
• Plaintext-only request proceeds with no marker and writes none: encrypted_keys is empty, so check_marker_for_erasure has nothing to refuse — erase.rs:460-487
• The mixed-matcher refusal happens before the marker check and before any write — erase.rs:465-480
• --matcher is required_unless_present = "plaintext_matchers", covered by erase_submit_needs_a_matcher_or_a_plaintext_matcher (main.rs:5468-5500)

Smaller things:

• The PR description's "Known limit: the printed status commands carry tenant, signal and id but not the store flags the submit used. The docs say so." is stale: the code now prints them (erase.rs:570-583) and docs/deletion-and-gc.md:772-787 documents them. Drop that paragraph.
• The <your S3 credential flags> placeholder goes through shell_word (erase.rs:575-578), so it prints single-quoted as '<your S3 credential flags>'. Harmless, but the operator is meant to replace it, and the quotes read as part of the value.
• A key given both as --plaintext-matcher and as a --matcher whose value the policy leaves plaintext (not an address) produces two matchers on the same key with different values in one conjunction, accepted and matching nothing (erase.rs:482-484). Pre-existing for duplicate --matcher keys, narrow, and fails closed.

Not checked:

• No build or test run: this is an aarch64 host and nothing in the review needed a run to settle. CI covers the ravel-cli suite.
• The regenerated docs/reference/ravel-cli-flags.md was read but not regenerated to compare.

Not checked:

• No cargo build or test run (aarch64 host; no finding required one) - CI covers the ravel-cli suite
• docs/reference/ravel-cli-flags.md was read but not regenerated for comparison

Task cc82aa7d-2c0a-4923-9366-cc1bcbf0030a on rp2, model claude-opus-5, effort medium.

[claude-fleet]

nit: Still open since 7bab864. For metrics, check_plaintext_matchers accepts a key spelled the way the rule is written, but the key is submitted unsanitized, so the request cannot match a stored label.
Failure: erase submit --signal metrics --plaintext-matcher client.ip=192.0.2.1 passes the rule check (rule_for sanitizes to client_ip for the lookup, erase.rs:228-237) and writes a predicate whose key is client.ip, while the loader stores the label as client_ip (load/mapping.rs:588 applies sanitize_label_name). The request is accepted and silently matches nothing.
Evidence: erase.rs:376-397 validates through the sanitized form; erase.rs:480 appends the pair as written; load/mapping.rs:588.
This matches the documented --matcher behaviour ("the matcher key is submitted as written"), so it is pre-existing rather than new, but the new flag's whole purpose is matching stored data and the check is in a position to catch it.