Comprehensive codebase audit: security, performance, code quality, and CI gaps

Comprehensive Codebase Audit

Full audit of the dev branch covering security, performance, code quality, TODOs, and CI/config gaps. Findings are grouped by severity across 5 categories.

---

Critical (8 items)

Security

S1. Missing signature verification in delete_data endpoint

• provider-node/src/api.rs:456-478
• The admin_signature field is accepted but explicitly ignored (let _ = request.admin_signature). Any user can delete arbitrary bucket data without authentication.
• Fix: Verify signature using the auth module before allowing deletion.

S2. Permissive CORS configuration

• provider-node/src/api.rs:80
• CorsLayer::permissive() allows requests from any origin. Combined with S1, this creates a remotely-exploitable attack vector.
• Fix: Restrict to trusted origins or make configurable.

Performance

P1. Unbounded iteration in on_finalize hook

• pallet/src/lib.rs:62-74
• Iterates over all expired challenges for a block without bounds. An attacker can create many challenges expiring at the same block, stalling the chain. The index as u16 cast also silently truncates at 65536.
• Fix: Cap processing per block (e.g., 100), carry remainder to next block.

CI / Config

C1. Missing try-runtime feature in pallet-drive-registry

• storage-interfaces/file-system/pallet-registry/Cargo.toml
• Pallet doesn't declare the try-runtime feature block. Will break zepter run check feature propagation lint.
• Fix: Add try-runtime feature with proper propagation.

C2. Missing try-runtime feature in pallet-s3-registry

• storage-interfaces/s3/pallet-s3-registry/Cargo.toml
• Same issue as C1.

TODOs blocking functionality

T1. Checkpoint duty query returns empty vec

• provider-node/src/checkpoint_coordinator.rs:329
• get_active_checkpoint_duties() has a TODO and returns Ok(vec![]). Provider-initiated checkpoints are non-functional.

T2. Challenge detection returns empty vec

• provider-node/src/challenge_responder.rs:304
• Challenge storage query is unimplemented, relies on event detection which is also incomplete.

T3. no-admin-left scenario unhandled in agreement removal

• pallet/src/lib.rs:1563
• TODO(no-admin-left) — when removing the last primary provider from a bucket, no admin safeguard exists.

---

High (14 items)

Security

S3. Path traversal in file-system API

• provider-node/src/fs_api.rs:59-61
• Path validation only checks for leading /. Paths like /../../../etc/passwd are not blocked.
• Fix: Reject paths containing .., normalize components.

S4. Unbounded storage iteration in complete_deregister

• pallet/src/lib.rs:~1028-1040
• CheckpointRewards::<T>::iter_prefix(&who).collect() is unbounded. A provider with thousands of reward entries can DoS the extrinsic.
• Fix: Paginated draining or limit bucket participation.

S5. Block number truncation to u32

• pallet/src/lib.rs:3794
• current_block.try_into().unwrap_or(0u32) silently masks overflow. Historical root tracking becomes incorrect on chains with BlockNumber > 2^32.
• Fix: Use saturated_into::<u64>() or return error.

S6. Unchecked division in slashing calculations

• pallet/src/lib.rs:3047, 3850, 2683
• challenge.deposit * challenger_percent / 100 and actual_penalty / 10 use unchecked arithmetic on Balance types.
• Fix: Use checked_mul/checked_div with explicit error.

Performance

P2. O(n) provider search in find_matching_provider

• pallet/src/lib.rs:3450-3507
• Full linear scan of all providers for every auto-match request. At 1000+ providers this is expensive.
• Fix: Add secondary indexes (ProvidersAcceptingPrimary, ProvidersByPriceRange).

P3. Unbounded Challenges storage

• pallet/src/lib.rs:197-202
• #[pallet::unbounded] with no pruning. Challenges accumulate faster than settlement.
• Fix: Auto-prune challenges older than configurable age.

P4. O(n²) nested loop in checkpoint signature verification

• pallet/src/lib.rs:2293-2310, 2382-2407
• For each signature, does primary_providers.iter().position() — O(n) inner search.
• Fix: Build a HashMap of provider→index before the loop.

P5. Repeated sequential storage reads

• pallet/src/lib.rs:1945-1954, 2610-2645
• Multiple ::get() calls for agreement, bucket, and provider info that could be loaded together.
• Fix: Batch reads upfront.

Code Quality

Q1. Silent error swallowing across provider-node

• provider-node/src/replica_sync_coordinator.rs:325, 703 — channel send and event results discarded
• provider-node/src/api.rs:461 — admin_signature unused
• provider-node/src/challenge_responder.rs:298, 511 — storage/event results discarded
• provider-node/src/fs_api.rs:67 — bucket init failures ignored
• provider-node/src/storage/mod.rs:276 — MMR node storage failures ignored
• Fix: Log warnings for non-critical, propagate errors for critical.

Q2. unwrap() in production HTTP handlers

• provider-node/src/fs_api.rs:164,171, provider-node/src/s3_api.rs:164-248, provider-node/src/api.rs:639
• Header parsing and bucket access use unwrap() / .parse().unwrap().
• Fix: Use proper defaults without panicking.

Q3. Missing runtime migrations for new storage items

• pallet/src/lib.rs
• CheckpointConfigs, CheckpointRewards, MemberBuckets storage items have no on_runtime_upgrade() hooks.
• Fix: Add migration code before any mainnet/testnet upgrade.

CI

C3. No security audit in CI

• No cargo audit, no secret scanning, no SAST in any workflow.
• Fix: Add cargo audit --deny warnings + secret scanning job.

C4. Missing precompile unit tests in CI

• .github/workflows/check.yml
• pallet-storage-provider-precompile, pallet-drive-registry-precompile, pallet-s3-registry-precompile have no dedicated test job.
• Fix: Add cargo test -p <precompile> to CI.

C5. No try-runtime test coverage in CI

• .github/workflows/check.yml
• Builds with try-runtime feature but never runs tests with it. Migration bugs won't surface until live.
• Fix: Add cargo test --workspace --features try-runtime.

---

Medium (16 items)

Security

S7. Unvalidated max_keys in S3 list objects

• provider-node/src/s3_api.rs:293
• Defaults to 1000 with no upper bound. Client can request millions.
• Fix: std::cmp::min(max_keys, 10_000).

S8. Integer overflow in challenge index as u16

• pallet/src/lib.rs:69
• >65535 challenges per deadline causes ID collisions.
• Fix: Use u32 or enforce limit.

S9. Insufficient public key validation

• pallet/src/lib.rs:878-883
• Only checks length (32/33/64), not actual key validity.

Performance

P6. No per-connection rate limiting

• provider-node/src/api.rs:78
• 256 MB body limit with no streaming/rate limiting per connection.

P7. Missing request-level timeouts in replica sync

• provider-node/src/replica_sync.rs:45-51
• HTTP requests have no individual timeout.

P8. Unbounded membership cache growth

• provider-node/src/auth.rs:50-104
• DashMap<u64, CachedMembership> has no size limit or LRU eviction.

P9. Vec allocation without capacity

• provider-node/src/api.rs:523
• Vec::new() then push in loop. Use Vec::with_capacity(request.hashes.len()).

P10. Sequential RocksDB deserialization on every stats() call

• provider-node/src/storage/disk.rs:128-150
• iter_buckets() deserializes all bucket states on every call.
• Fix: Cache stats summary, update incrementally.

Code Quality

Q4. Missing rustdoc on public APIs

• client/src/checkpoint.rs — ProviderHealthHistory, CheckpointManager, CommitmentCollection methods
• client/src/base.rs — ClientError missing Timeout/SignatureVerification/Crypto variants

Q5. Hardcoded magic numbers

• provider-node/src/replica_sync_coordinator.rs:37-46 — poll interval 12s, timeout 300s, max concurrent 3
• client/src/checkpoint.rs:70 — consensus threshold 51%
• Fix: Move to config file or environment.

Q6. Incomplete upload_replicated() stub

• client/src/storage_user.rs:115-138
• API exists but logs "Would replicate" without doing anything. Misleads users.
• Fix: Either implement or mark #[doc(hidden)] / remove.

Q7. String-based account IDs instead of typed

• client/src/discovery.rs:68, client/src/admin.rs:21
• pub account: String instead of AccountId32. SS58 prefix mismatch causes comparison bugs.

Q8. Inconsistent error handling patterns

• Mix of structured errors (Error::InvalidHash), string errors (#[error("Node not found: {0}")]), and silently discarded results across modules.

CI / Config

C6. Layer 1 unit tests missing from main CI

• file-system-primitives, s3-primitives, pallet-drive-registry, pallet-s3-registry only tested via integration tests.
• Fix: Add cargo test -p <crate> to check workflow.

C7. serde_json default-features inconsistency

• Workspace Cargo.toml:133 declares default-features = false, but runtime/Cargo.toml overrides with default-features = true.

C8. Hardcoded port assumptions

• justfile:22-30 — ports 9900, 2222, 3333 hardcoded with no conflict detection.

---

Low (10 items)

Performance

• P11. No graceful shutdown handler in provider node (provider-node/src/command.rs:119-125)
• P12. Weight accuracy for provider_checkpoint may drift (pallet/src/weights.rs:500-510)
• P13. No retry/backoff logic in checkpoint/replica sync coordinators

Code Quality

• Q9. expect() in genesis config (pallet/src/lib.rs:273) — chain fails to boot with cryptic panic
• Q10. Content-defined chunking documented but unimplemented (client/src/base.rs:213-223)
• Q11. S3 list_objects pagination unimplemented (storage-interfaces/s3/client/src/substrate.rs:457)
• Q12. HTTP header parsing uses nested unwrap() fallbacks that hide bad input (provider-node/src/s3_api.rs:164-172)
• Q13. Duplicated HTTP header parsing logic between fs_api.rs and s3_api.rs
• Q14. Mixed builder patterns — with_dev_signer(self) vs set_dev_signer(&mut self) (client/src/base.rs:120-151)

CI / Config

• C9. scripts/runtimes-matrix.json has "TODO: update WSS" placeholders (lines 9, 28)
• C10. Single zombienet collator doesn't test multi-collator consensus scenarios

---

Summary

Severity	Security	Performance	Code Quality	CI/Config	TODOs	Total
Critical	2	1	0	2	3	8
High	4	4	3	3	0	14
Medium	3	5	5	3	0	16
Low	0	3	5	2	0	10
Total	9	13	13	10	3	48

Suggested Priority

1. Immediate (blocks production safety): S1, S2, P1, S3, S4, T1, T2, T3
2. Before testnet upgrade: S5, S6, Q3, C1, C2
3. Next sprint: P2-P5, Q1, Q2, C3-C6
4. Backlog: Everything else