fix: harden auth, validation, and sandboxing (audit #12-#23)#24
Open
LKSNDRTMLKV wants to merge 8 commits into
Open
fix: harden auth, validation, and sandboxing (audit #12-#23)#24LKSNDRTMLKV wants to merge 8 commits into
LKSNDRTMLKV wants to merge 8 commits into
Conversation
This reverts commit 01fa8f6.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Hardening pass across
dpp-engineclosing the open audit findings (#12–#23). Same posture as the core pass: adversarial/malformed input that previously failed open, panicked, or bypassed a check now fails closed with an explicit error, each with a regression test. The fulljust checkgate is green.This PR contains only the audit fixes. An in-progress webhooks feature that happened to be in the working tree is deliberately excluded — see "Scope" below.
Fixes by area
Scope enforcement, end-to-end (#23, #21, #20, #18) — the central finding.
dpp-types(dpp-types: unrecognized API-key scope strings fail open to Admin; retention policy has no lower bound #21): unrecognized API-key scope strings now fail closed toRead(were failing open toAdmin); retention policy gains a lower bound.dpp-dal(dpp-dal: LIKE-pattern injection exposes arbitrary passports; stale compliance-critical columns; fail-open scope default #18): fixed LIKE-pattern injection that exposed arbitrary passports; dropped stale compliance-critical columns from patch writes; made the scope default fail closed; guarded the resubmit path androws_affectedchecks.dpp-vault(dpp-vault: Read-scoped API keys can perform full passport-lifecycle writes #20): every mutation handler (create/update/publish/suspend/archive/eol/transfer + operator) now enforcescan_write(), so Read-scoped keys can no longer perform lifecycle writes.dpp-common (#13) — the request-ID middleware now matches its own
application/problem+jsonerror content-type (it never did), and oversized error bodies pass through with the id in the header instead of being silently emptied.dpp-integrator (#12) — steel/aluminium/tyre importers validate the GTIN mod-10 checksum like the battery importer already did; a malformed vault response is no longer recorded as a successful create.
dpp-identity (#14) — internal mTLS endpoints require a shared-secret proxy binding instead of trusting client-supplied headers alone; key rotation aborts on archive failure instead of failing open; removed a dead per-operator DID route.
dpp-resolver (#15) —
gtin/dppIdare validated at the edge (closes an SSRF / path-traversal surface); the linkset base URL is a fixed constant instead of a tamperableqrCodeUrl-derived value (closes an open redirect).dpp-seal (#16) —
capabilities()reports no sealing support when no QTSP is configured, instead of advertising modes the adapter cannot actually deliver.dpp-plugin-host (#17) — added the missing Wasm table-growth limiter; sandbox-breach metrics are classified from the trap rather than a spoofable message; the in-force compliance gate is applied on the previously-missed output path; unsigned plugins are refused unless explicitly opted in.
dpp-node (#19) — an EU-registry payload that fails validation is no longer sent anyway; the boot-time operator-country lookup surfaces DB errors instead of baking an empty country in for the process lifetime; fixed a token-expiry underflow panic.
odal CLI (#22) — byte-index slicing on untrusted UTF-8 no longer panics (char-safe truncation); the API secret for
odal key useis read from a flag,ODAL_API_SECRET, or a no-echo prompt instead of being accepted as a plaintextargvargument.dpp-evidence verification — also fixes odal-node/dpp-core#25 (its fix lives here, in
dpp-vault): unsigned-but-completed transfer records now fail verification, and a present-but-unresolvablekidreturns an accurate diagnosis instead of silently falling back to the primary key. (GitHub won't auto-close a cross-repo issue on merge — close dpp-core#25 manually.)Scope — webhooks excluded
An unrelated, in-progress webhooks feature (28 modified files + 11 new files) was present in the working tree and is not part of this PR. The commit was assembled to contain only the 42 audit files and verified in isolation —
just checkwas re-run green with the webhooks WIP stashed, so the gate reflects exactly what is committed here. That work continues separately onmain.Deferred follow-ups (not blockers)
test-integrationtier needsDPP_ALLOW_UNSIGNED_PLUGINS=truein the plugin-host feature-gated tests/benches — a consequence of the dpp-plugin-host: Wasm sandbox limiter gap; spoofable sandbox alerts; missing in-force gate on one output path #17 fail-closed change. The fastjust checkgate is unaffected.RegistrySyncPort+RegistrationRequest; left as a follow-up because it interacts with #27's stricter validation (which now rejects an empty operator name/country).Dependency / merge order
Builds against
dpp-corefix/core-auditvia the local[patch.crates-io]. Land the core PR and publishdpp-core0.7.0 first, then this branch can pindpp-* = 0.7.0from the registry and drop the patch.Testing
just checkgreen: fmt-check · clippy--workspace --all-targets -D warnings· debug-check · subjects-check · mod-rs-check ·cargo nextest --workspace· audit.Closes #12, #13, #14, #15, #16, #17, #18, #19, #20, #21, #22, #23