fix(cryptify): canonicalize the default-tier accounting key

[dobby-coder]

Closes #363.

What was wrong

upload_finalize accounted the default-tier rolling limit against the sender email exactly as the uploaded container stored it:

let accounting_key = state.api_key_tenant.as_deref()
    .map(|t| format!("api-key:{}", t))
    .or_else(|| sender.clone());

sender is read verbatim out of the container's public signing policy, and that value is caller-chosen even when the sender is entirely genuine. Policy::derive canonicalizes before it derives the signer's identity, so one signing key for bob@example.com also verifies a container storing Bob@Example.COM, and the Unsealer hands the raw spelling back as pub_id. One identity could therefore mint a fresh 14-day quota bucket per capitalization.

pg-core's Sealer does canonicalize the signing policy before writing it (canonical_signing_key), but that is the honest client's courtesy rather than a gate: a client that skips it produces a container that verifies just the same. The test below builds exactly that container.

The change

Both the get_usage check and the record_upload write now go through one accounting_key helper, so the check and the accrual cannot disagree:

fn accounting_key(
    api_key_tenant: Option<&str>,
    email_attribute: &str,
    sender: Option<&str>,
) -> Option<String> {
    match api_key_tenant {
        Some(tenant) => Some(format!("api-key:{}", tenant)),
        None => sender.map(|s| pg_core::identity::canonicalize(email_attribute, s)),
    }
}

Usage rows already written under a non-canonical spelling keep their own bucket until they age out of the rolling window, which the window clears on its own. No migration is included: the issue did not ask for one, and a backfill would need to merge rows rather than rename them.

Out of scope, and untouched: state.sender stays raw (it is the confirmation-mail recipient and Reply-To), the api-key:<tenant> branch stays as it was (a tenant id is not an identity attribute and has no rule), and no limit, template, locale string or pg-core file changed.

Tests

two_spellings_of_one_sender_share_a_rolling_limit_bucket seeds the bucket to leave room for exactly one container, uploads the capitalized spelling, and requires the lowercase one to be refused. Building the non-canonical container needs a helper, respell_stored_sender, which rewrites the stored spelling inside the header-signature block; the replacement is the same byte length, so every length prefix stays valid and the signed header bytes are untouched. The test reads both containers back through the Unsealer before it counts buckets, so a substitution that landed somewhere inert cannot leave the guard passing over a canonical container. That read verifies the container too, which is the premise itself: the respelled one is still accepted.

accounting_key_canonicalizes_the_sender_spelling is the regression pin #358 asked for. It asserts the key equals canonicalize() of the container value for a capitalized local part, a capitalized domain, an all-caps address and surrounding whitespace, against both the production attribute type and the irma-demo one test deployments configure.

Two smaller tests hold the api-key: branch and the no-tenant-no-sender case in place.

Seen red

With the canonicalization reverted (None => sender.map(|s| s.to_owned())), re-run at fb9abce:

thread 'integration::two_spellings_of_one_sender_share_a_rolling_limit_bucket' panicked at cryptify/src/main.rs:3823:9:
assertion `left == right` failed: the capitalized upload must accrue against the canonical bucket
  left: 4999998525
 right: 5000000000

The server log for that run shows the separate bucket directly:

Rolling limit check for Bob@Example.COM (api_key_tenant=None): used=0 + current=1475 vs limit=5000000000

Dropping that intermediate assertion to reach the end of the test shows the second spelling going through as well:

assertion `left == right` failed: the second spelling must not get a bucket of its own
  left: Status { code: 200 }
 right: Status { code: 413 }

The table test is red on the same revert:

assertion `left == right` failed: pbdf.sidn-pbdf.email.email / "Bob@example.com" must not get a bucket of its own
  left: Some("Bob@example.com")
 right: Some("bob@example.com")

Acceptance check

cargo test -p cryptify --all-targets   190 passed; 0 failed
cargo fmt --all -- --check             clean
cargo clippy -p cryptify --all-targets -- -D warnings   clean
cargo test --workspace                 all targets green

cryptify/CLAUDE.md gains a note on why the accounting key is not the sender value on the wire, so the next reader does not reach past the helper for the raw value.

Part of #338, itself part of #247. Decided in #358.

[dobby-coder]

VERDICT: approve

Rules Dobby 2 — cycle 1, sign-off

One finding, a nit, and it is already fixed on the branch. No bug-severity findings, so no fix loop.

The finding (fixed, not outstanding)

accounting_key's doc comment said the duplicate bucket mints "a fresh 30-day quota". The rolling window is 14 days — ROLLING_WINDOW_SECS = 14 * 24 * 60 * 60 (cryptify/src/store.rs:17), cryptify/CLAUDE.md:136 already said "14d rolling window" four lines above the bullet this PR adds, and cryptify/api-description.yaml:388 says "rolling 14-day upload limit". That comment is where a reader lands to find out what a duplicate bucket costs, so the number there is the one that has to be right.

The same sentence was in the PR description ("mint a fresh 30-day quota bucket per capitalization"), which is not in the diff and would have survived a code-only fix. Both surfaces are corrected:

• cryptify/src/main.rs:927 → bb9b05d (cargo fmt --manifest-path cryptify/Cargo.toml --all -- --check clean; comment-only change)
• the PR body → edited in place, no branch push, no CI rerun

What I checked and did not find

Rule sweep over the rules that plausibly bear on this diff — Rust fmt/clippy gating, regression-test policy, mutation-probe honesty, self-certifying guards, doc/PR claim accuracy, closing keywords, the AEAD/recipient-set family, and cryptify's own finalize and byte-range conventions. Verified against the tree rather than taken from the sweep:

• The fix covers both ends. The get_usage check and the record_upload write now read one helper, so the check and the accrual cannot disagree — which was the shape of the original defect.
• canonicalize is the right call. For an email attribute type it is value.trim().to_lowercase(), and it is total: an attribute type with no rule passes through unchanged, so the irma-demo type test deployments configure is covered by the same tail-keyed rule. The table test pins both types.
• The integration test has teeth and is not self-certifying. respell_stored_sender is read back through the Unsealer before any bucket is counted, so a byte substitution that landed somewhere inert cannot leave the test passing over a canonical container. The quoted "seen red" mutation (None => sender.map(|s| s.to_owned())) is the base branch's expression, and the quoted numbers line up with the test's own assertions (4999998525 vs ROLLING_LIMIT).
• The out-of-scope list holds. state.sender staying raw is correct — it is the confirmation-mail recipient and Reply-To. The api-key:<tenant> branch is unchanged and still wins over the sender.
• No overclaim about already-stored rows. The body says plainly that rows written under a non-canonical spelling keep their bucket until the window clears them, and that no migration is included. That is the qualification this class of fix usually loses.
• Nothing embargoed here. #363 is a public, maintainer-filed issue, and none of the repo's draft advisories covers this topic — so flipping the PR out of draft discloses nothing.

CI on bb9b05d: 63 SUCCESS, 4 SKIPPED, 0 failures. Flipping to ready for review.