Two test-infrastructure findings surfaced while reviewing Codacy output on #57. Neither is a live vulnerability — both are test-only — but both are the kind of thing that gets copied one more time every time someone adds a suite.
1. The Postgres testcontainer harness is duplicated eight times
start_pg() — spin up postgres:17, wait for readiness, sleep 1.5s, CREATE ROLE odal_app, run migrations via the admin URL, reconnect as the app role — is copy-pasted across:
crates/dpp-dal/tests/pg_integration.rscrates/dpp-dal/tests/pg_seal_outbox.rscrates/dpp-node/tests/import_job_store.rscrates/dpp-node/tests/registry_outbox.rscrates/dpp-node/tests/seal_outbox.rscrates/dpp-node/tests/smoke.rscrates/dpp-node/tests/webhook_outbox.rscrates/dpp-vault/tests/helpers/mod.rs
Six predate #57; two were added by it, following the established pattern. This is the main contributor to the duplication metric Codacy reports on PRs touching the Docker tiers.
The cost is not aesthetics: the readiness logic carries a hardcoded sleep(1500ms) that every copy has to get right independently, and a change to the bootstrap sequence (a new role grant, a different image pin) means eight edits or seven silent divergences.
Shape of a fix: a dpp-test-support dev-only crate exposing start_pg() and the passport fixtures. Cross-crate sharing of #[cfg(test)] code needs a real crate — a mod helpers cannot reach across crate boundaries, which is exactly why this got copied.
2. Five tests write a real keystore to a predictable shared-temp path
KeyStore holds Ed25519 private keys. These open one at std::env::temp_dir()/<name>-<uuid>.json, with no cleanup:
crates/dpp-identity/src/handlers/rotate_key.rs:102crates/dpp-identity/src/handlers/verify.rs:94crates/dpp-identity/src/router.rs:82crates/dpp-node/src/infra/ruleset.rs:145crates/dpp-node/tests/smoke.rs:163
Severity is low and worth stating plainly: the passphrases are literals like "test", the keys are throwaway, and nothing production-adjacent reads these paths. It is hygiene, not an exposure. But it is the same finding Codacy raised against the two instances #57 added — those were changed to tempfile::tempdir(), which gives restrictive permissions and removal on drop. These five predate the check and so are invisible to it.
Every run also leaves the file behind, so a long-lived dev machine accumulates them.
Shape of a fix: tempfile::tempdir(), ideally via the shared harness from (1) so there is one place that knows how to make a throwaway keystore.
Sequencing
(1) subsumes (2) if the extracted harness owns keystore creation, so doing them together is cheaper than separately. Neither blocks anything.
Two test-infrastructure findings surfaced while reviewing Codacy output on #57. Neither is a live vulnerability — both are test-only — but both are the kind of thing that gets copied one more time every time someone adds a suite.
1. The Postgres testcontainer harness is duplicated eight times
start_pg()— spin uppostgres:17, wait for readiness, sleep 1.5s,CREATE ROLE odal_app, run migrations via the admin URL, reconnect as the app role — is copy-pasted across:crates/dpp-dal/tests/pg_integration.rscrates/dpp-dal/tests/pg_seal_outbox.rscrates/dpp-node/tests/import_job_store.rscrates/dpp-node/tests/registry_outbox.rscrates/dpp-node/tests/seal_outbox.rscrates/dpp-node/tests/smoke.rscrates/dpp-node/tests/webhook_outbox.rscrates/dpp-vault/tests/helpers/mod.rsSix predate #57; two were added by it, following the established pattern. This is the main contributor to the duplication metric Codacy reports on PRs touching the Docker tiers.
The cost is not aesthetics: the readiness logic carries a hardcoded
sleep(1500ms)that every copy has to get right independently, and a change to the bootstrap sequence (a new role grant, a different image pin) means eight edits or seven silent divergences.Shape of a fix: a
dpp-test-supportdev-only crate exposingstart_pg()and the passport fixtures. Cross-crate sharing of#[cfg(test)]code needs a real crate — amod helperscannot reach across crate boundaries, which is exactly why this got copied.2. Five tests write a real keystore to a predictable shared-temp path
KeyStoreholds Ed25519 private keys. These open one atstd::env::temp_dir()/<name>-<uuid>.json, with no cleanup:crates/dpp-identity/src/handlers/rotate_key.rs:102crates/dpp-identity/src/handlers/verify.rs:94crates/dpp-identity/src/router.rs:82crates/dpp-node/src/infra/ruleset.rs:145crates/dpp-node/tests/smoke.rs:163Severity is low and worth stating plainly: the passphrases are literals like
"test", the keys are throwaway, and nothing production-adjacent reads these paths. It is hygiene, not an exposure. But it is the same finding Codacy raised against the two instances #57 added — those were changed totempfile::tempdir(), which gives restrictive permissions and removal on drop. These five predate the check and so are invisible to it.Every run also leaves the file behind, so a long-lived dev machine accumulates them.
Shape of a fix:
tempfile::tempdir(), ideally via the shared harness from (1) so there is one place that knows how to make a throwaway keystore.Sequencing
(1) subsumes (2) if the extracted harness owns keystore creation, so doing them together is cheaper than separately. Neither blocks anything.