Skip to content

fix(tests): point the leak guard at the shipping schema - #317

Merged
LKSNDRTMLKV merged 2 commits into
mainfrom
fix/leak-guard-follows-current-version
Sep 19, 2026
Merged

LKSNDRTMLKV merged 2 commits into
mainfrom
fix/leak-guard-follows-current-version

Conversation

@LKSNDRTMLKV

@LKSNDRTMLKV LKSNDRTMLKV commented Sep 17, 2026

Copy link
Copy Markdown
Member

Found by the pre-publish audit for #293, by mutation-testing the gates rather
than running them.

The guard stopped covering what ships

recycler_credential_unlocks_professional_battery_fields is the end-to-end
check that a public view of a battery passport does not carry Annex XIII
point 2 and point 4 content — state of health among them. It built its policy
from a literal:

let policy = ProductGroupAccessPolicy::for_schema_version("battery", "2.6.0")

2.6.0 was current when the test was written. #295 added v2.7.0, and the
literal was not updated. So every leak assertion in that test — including
assert!(public.filtered_data.get("sohMethodology").is_none()) — has been
evaluated against a schema that no longer ships.

Demonstrated rather than argued: flipping sohMethodology's x-disclosure from
restricted to public in v2.7.0 left the test green. After this change
the same mutation fails it.

There is no live leak

I diffed the classification across both versions before claiming anything.
v2.6.0 and v2.7.0 carry the same 15 non-public fields with the same classes
sohMethodology: restricted, stateOfHealth: individual, stateOfHealthPct: individual, and the rest. Nothing is currently misclassified, and nothing has
been exposed.

What was lost is the guard, not the classification. 🚨 This project has already
shipped a state-of-health public leak once, in ≤0.10.0.

The fix, and why it is shaped this way

The policy now reads the catalog's current schema version rather than a
literal, so the guard follows the version it guards and cannot silently age
again. That is the same defect class as the schema $id headers corrected in
#300 — a version literal that stays plausible while becoming wrong — and the
same remedy: derive it.

Scope

Deliberately narrow. Two things I did not do:

  • Other product groups' end-to-end tests may carry the same literal. I found
    this one by mutating battery; a sweep for pinned versions across the test tier
    is worth doing, but it is a different change and should not ride in behind a
    one-line fix.
  • The catalog↔schema parity test already catches a manifest/schema
    divergence
    , and caught my first mutation. It is not a substitute: it compares
    the two records of the classification against each other, so a change applied
    to both passes it. The end-to-end guard is the one that asks what actually
    comes out of the filter.

Summary by CodeRabbit

  • Bug Fixes
    • Updated recycler credential access testing to use the catalog’s current schema version, improving compatibility as schemas evolve.
    • Preserved existing filtering checks for public and legitimate-interest credentials.

@LKSNDRTMLKV LKSNDRTMLKV added review-ready Opt this PR into a CodeRabbit review security Security-relevant issue labels Sep 17, 2026
@LKSNDRTMLKV LKSNDRTMLKV added this to the v0.21.0 milestone Sep 17, 2026
@coderabbitai

coderabbitai Bot commented Sep 17, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: a80e2452-4b74-4895-a569-79f47b11146b

📥 Commits

Reviewing files that changed from the base of the PR and between dcef3df and d9eda6c.

📒 Files selected for processing (1)
  • crates/dpp-tests/tests/battery_end_to_end.rs

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

The recycler credential access test now builds its battery policy from the catalog’s current schema version instead of a fixed 2.6.0 version. Existing public and legitimate-interest filtering assertions remain.

Changes

Battery policy test

Layer / File(s) Summary
Schema-versioned battery policy
crates/dpp-tests/tests/battery_end_to_end.rs
The test reads the registered battery schema version from ProductGroupCatalog and uses it to build the access policy. Existing access filtering assertions remain.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~5 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to d9eda

The test now uses the registered 2.7.0 schema and checks its disclosure classifications without an established merge-blocking risk.

🚥 Pre-merge checks | ✅ 7
✅ Passed checks (7 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Publication Boundary ✅ Passed PASS. The authoritative diff changes one test file and adds only catalog-version lookup code plus comments about schema versions and the leak guard. It introduces no ADR reference, external repository…
Persisted Shape Migration ✅ Passed PASS. The authoritative pull-request diff changes only crates/dpp-tests/tests/battery_end_to_end.rs. It changes the test policy lookup from a hard-coded schema version to `ProductGroupCatalog::new()…
Title check ✅ Passed The title clearly identifies the test fix and states that the leak guard now targets the shipping schema.
Description check ✅ Passed The description clearly explains the outdated schema-version defect, the catalog-derived fix, mutation-testing evidence, current impact, and scope. It does not use every template heading or include th…
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

Comment @coderabbitai help to get the list of available commands.

@LKSNDRTMLKV
LKSNDRTMLKV merged commit 4b3a685 into main Sep 19, 2026
19 checks passed
@LKSNDRTMLKV
LKSNDRTMLKV deleted the fix/leak-guard-follows-current-version branch September 19, 2026 05:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

review-ready Opt this PR into a CodeRabbit review security Security-relevant issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant