fix(persistence): bound RDB stream/count allocations against untrusted lengths

[pilotspacex-byte]

Closes the remaining untrusted-length gaps in the native RDB decode path (src/persistence/rdb.rs), same class as #266.

Gaps fixed (present identically in read_entry and read_entry_zero_copy — literal duplicates):

• group_count / pel_count / consumer_count / pending_count in the TYPE_STREAM branch had no validate_count guard. All sibling counts (hash/list/set/zset/entry/field) were already guarded.

Min-bytes-per-item traced from write_entry's serialization layout (groups 28, PEL 36, consumers 16, pending 16) so no valid file is ever rejected — proven by a fully-populated round-trip control test.

Tests: 4 DoS-rejection tests (crafted u32::MAX counts with zero backing bytes, asserted Err from BOTH decode paths) + 1 overcounting-safety control. 18/18 passing; clippy -D warnings clean on both feature sets; fmt clean.

Note: today these counts grow maps incrementally (no eager with_capacity), so this is defense-in-depth + pattern consistency rather than a live OOM — but it removes a latent trap for future refactors.

Follow-up (not this PR): rdb.rs is over the 1500-line guideline (~2038 lines) and needs a module split.

Summary by CodeRabbit

• Bug Fixes
  • Strengthened stream data parsing to reject malformed or suspiciously large count values.
  • Prevented potential denial-of-service issues caused by crafted stream data.
  • Confirmed valid stream consumer groups continue to load successfully.

[qodo-code-review]

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

[coderabbitai]

📝 Walkthrough

Walkthrough

RDB TYPE_STREAM decoding now validates group, PEL, consumer, and pending counts against remaining bytes in both parsing paths. Tests cover malformed count fields and a valid minimal consumer-group stream, with the change documented in the changelog.

Changes

RDB stream hardening

Layer / File(s)	Summary
Stream count validation src/persistence/rdb.rs	Both read_entry_zero_copy and read_entry validate untrusted group, PEL, consumer, and pending counts using minimum structural byte bounds before parsing.
Malformed and valid stream coverage src/persistence/rdb.rs, CHANGELOG.md	Tests reject crafted count-lie payloads through both decoder paths, accept a valid minimal stream, and document the hardening. של

Estimated code review effort: 3 (Moderate) | ~20 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description covers the summary well, but it omits the template's Checklist and Performance Impact sections and doesn't use the required headings.	Add the required Summary, Checklist, Performance Impact, and Notes headings, and fill in the checklist items plus a clear performance impact statement.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: hardening RDB stream/count handling against untrusted lengths.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/rdb-stream-count-dos

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

🧹 Nitpick comments (2)

src/persistence/rdb.rs (2)

2033-2035: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider verifying parsed contents in the zero-copy control assertion.

The read_entry path in the control test verifies group count, PEL length, consumer count, and pending count. The zero-copy path only asserts is_ok(), so a parsing bug that returns Ok with wrong data would go undetected.

♻️ Suggested: mirror the read_entry assertions for the zero-copy path

     // Same body must also be accepted by the zero-copy variant.
     let mut cursor2 = Cursor::new(&entry_bytes[..]);
-    assert!(read_entry_zero_copy(&mut cursor2, TYPE_STREAM, 0, false).is_ok());
+    let (_key2, entry2) = read_entry_zero_copy(&mut cursor2, TYPE_STREAM, 0, false)
+        .expect("legit stream must not be rejected by zero-copy path");
+    match entry2.value.as_redis_value() {
+        RedisValueRef::Stream(stream) => {
+            assert_eq!(stream.groups.len(), 1);
+            let group = stream.groups.get(&Bytes::from_static(b"g")).unwrap();
+            assert_eq!(group.pel.len(), 1);
+            assert_eq!(group.consumers.len(), 1);
+            assert_eq!(
+                group.consumers.get(&Bytes::from_static(b"c")).unwrap().pending.len(),
+                1
+            );
+        }
+        _ => panic!("Expected Stream"),
+    }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/persistence/rdb.rs` around lines 2033 - 2035, Update the zero-copy
control assertion using read_entry_zero_copy to retain and inspect the parsed
result, then mirror the existing read_entry checks for group count, PEL length,
consumer count, and pending count instead of asserting only is_ok().

---

1979-2036: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoff

Verify file length against the 1500-line guideline.

This file appears to exceed 1500 lines (tests end at line 2037). As per coding guidelines, no single .rs file should exceed 1500 lines; larger modules should be split into submodules. Consider extracting the test module into a separate file (e.g., rdb_tests.rs or tests/rdb.rs).

As per coding guidelines: "No single .rs file should exceed 1500 lines; split larger modules into submodules."

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/persistence/rdb.rs` around lines 1979 - 2036, src/persistence/rdb.rs
exceeds the 1500-line guideline because its tests are embedded in the
implementation file. Extract the test module, including
test_stream_consumer_group_round_trip_not_rejected and related tests, into a
dedicated submodule such as rdb_tests.rs or tests/rdb.rs, then wire it into the
module with the appropriate cfg(test) declaration and preserve access to
required helpers and private items.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/persistence/rdb.rs`:
- Around line 2033-2035: Update the zero-copy control assertion using
read_entry_zero_copy to retain and inspect the parsed result, then mirror the
existing read_entry checks for group count, PEL length, consumer count, and
pending count instead of asserting only is_ok().
- Around line 1979-2036: src/persistence/rdb.rs exceeds the 1500-line guideline
because its tests are embedded in the implementation file. Extract the test
module, including test_stream_consumer_group_round_trip_not_rejected and related
tests, into a dedicated submodule such as rdb_tests.rs or tests/rdb.rs, then
wire it into the module with the appropriate cfg(test) declaration and preserve
access to required helpers and private items.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b8574f98-5d12-442a-94cd-de11cd6cbf4b

📥 Commits

Reviewing files that changed from the base of the PR and between 02ceae9 and b5af8ad.

📒 Files selected for processing (2)

• CHANGELOG.md
• src/persistence/rdb.rs