Fix issue when more than 65536 are delivered by RC - #4122
Conversation
|
There was a problem hiding this comment.
Pull request overview
This PR addresses failures when Remote Config delivers an ASM rules_data array that exceeds libddwaf’s per-array entry limit (u16), by splitting large ASM_DATA payloads into multiple smaller WAF configs and tracking/removing the extra chunk configs across updates.
Changes:
- Add per-RC-path tracking (
extra_chunk_paths) so extra WAF chunk configs are removed when the original RC config is removed or replaced. - Split large
rules_data[*].dataarrays into chunked JSON payloads and apply them as multiple WAF configs under derived config paths. - Add unit tests covering “no split”, “split required”, and “no rules_data passthrough” behaviors.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| if i == 0 { | ||
| all_diagnostics.push((rc_path.clone(), diagnostics)); | ||
| last_result = upd_result; | ||
| } | ||
| *waf_changed = true; | ||
| } |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
Suppressed comments (2)
appsec/helper-rust/src/service.rs:634
- Errors from
add_or_update_configare only captured/returned for the first chunk (i == 0). If any later chunk fails to apply, the error is silently ignored and the RC update still reports success, leaving the WAF in a partially-updated state.
let upd_result = self.waf.add_or_update_config(
&chunk_path,
&waf_obj,
Some(&mut diagnostics),
);
appsec/helper-rust/src/service.rs:607
- This branch handles products "ASM_DD", "ASM", and "ASM_DATA", but the added context says "Failed to split ASM_DATA". When a non-ASM_DATA config fails to parse/split, the error message will be misleading; include the product name (or use a product-agnostic message).
let chunks = split_asm_data_if_needed(data)
.with_context(|| format!("Failed to split ASM_DATA for {:?}", rc_path))?;
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0be5d37cb8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Description
Fix: prevent crash when
ASM_DATAIP denylist exceeds 65,535 entrieslibddwaf v2.x uses
uint16_tforWafArraycapacity, which hard-caps any single array at 65,535 entries. When Remote Config delivers anASM_DATApayload whoserules_data[*].dataarray exceeds this limit (e.g. the system-test scenarioAPPSEC_BLOCKING_FULL_DENYLISTwith 65,536 IPs), the helper process panics trying to convert the array length tou16Since the WAF cannot enforce more than 65,535 IP entries regardless, the correct fix is to truncate the array to the limit rather than attempt to register entries the WAF cannot use.
Reviewer checklist