test(core): add parser and wire-format fuzz targets#1088
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request expands the project's security testing infrastructure by introducing several new structured fuzz targets. These additions focus on validating the robustness of various parser and wire-format implementations, specifically targeting TOML configuration files, capsule manifests, and JSON-based admin/gateway protocols. The changes are designed to improve coverage of security-critical parsing logic while maintaining efficient build times through strategic feature gating. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Inputs flow in a stream of bytes, Testing logic through dark nights. Parsers hold, or break and bend, Security bugs, we now transcend. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces new parser and wire-format fuzz targets for capsule manifests, runtime configurations, admin/kernel JSON messages, and gateway request bodies. The review feedback suggests optimizing the manifest TOML fuzz target by passing the parsing result directly to avoid redundant TOML parsing, and strengthening the configuration TOML fuzz target by asserting that a valid configuration remains valid after a serialization roundtrip.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| if let Ok(manifest) = manifest_result { | ||
| assert!( | ||
| value_result.is_ok(), | ||
| "typed manifest deserialization implies syntactically valid TOML" | ||
| ); | ||
|
|
||
| assert_eq!( | ||
| manifest.effective_ipc_publish_patterns().len(), | ||
| manifest.publishes.len() | ||
| ); | ||
| assert_eq!( | ||
| manifest.effective_ipc_subscribe_patterns().len(), | ||
| manifest.subscribes.len() | ||
| ); | ||
|
|
||
| for publish in manifest.publishes.values() { | ||
| assert_at_most_one_publish_pin(publish); | ||
| } | ||
|
|
||
| for subscribe in manifest.subscribes.values() { | ||
| assert_at_most_one_subscribe_pin(subscribe); | ||
| assert!( | ||
| subscribe.priority.is_none() || subscribe.handler.is_some(), | ||
| "handler-less subscribe entries must not carry priority" | ||
| ); | ||
| } | ||
|
|
||
| for interceptor in manifest.effective_interceptors() { | ||
| let Some(subscribe) = manifest.subscribes.get(&interceptor.event) else { | ||
| panic!("effective interceptor must come from a subscribe entry"); | ||
| }; | ||
| assert!(subscribe.handler.is_some()); | ||
| } | ||
| } else if let Ok(value) = value_result { | ||
| assert_malformed_security_shapes_fail_closed(src, &value); | ||
| } |
There was a problem hiding this comment.
The assert_malformed_security_shapes_fail_closed helper is currently called only when manifest_result is Err. Inside the helper, it re-parses the TOML string src using toml::from_str::<CapsuleManifest>(src).is_err(). Since we already know manifest_result is Err, this re-parsing is completely redundant and always returns true, making the assertion a tautology that wastes CPU cycles.
Instead, we should pass manifest_result.is_err() directly to the helper and run the check whenever value_result is Ok, ensuring that any malformed security shape indeed causes a parsing failure.
if let Ok(value) = &value_result {
assert_malformed_security_shapes_fail_closed(manifest_result.is_err(), value);
}
if let Ok(manifest) = manifest_result {
assert!(
value_result.is_ok(),
"typed manifest deserialization implies syntactically valid TOML"
);
assert_eq!(
manifest.effective_ipc_publish_patterns().len(),
manifest.publishes.len()
);
assert_eq!(
manifest.effective_ipc_subscribe_patterns().len(),
manifest.subscribes.len()
);
for publish in manifest.publishes.values() {
assert_at_most_one_publish_pin(publish);
}
for subscribe in manifest.subscribes.values() {
assert_at_most_one_subscribe_pin(subscribe);
assert!(
subscribe.priority.is_none() || subscribe.handler.is_some(),
"handler-less subscribe entries must not carry priority"
);
}
for interceptor in manifest.effective_interceptors() {
let Some(subscribe) = manifest.subscribes.get(&interceptor.event) else {
panic!("effective interceptor must come from a subscribe entry");
};
assert!(subscribe.handler.is_some());
}
}| fn assert_malformed_security_shapes_fail_closed(src: &str, value: &Value) { | ||
| if section_has_multi_pin(value, "publish") || section_has_multi_pin(value, "subscribe") { | ||
| assert!( | ||
| toml::from_str::<CapsuleManifest>(src).is_err(), | ||
| "ambiguous source pins must fail manifest parsing" | ||
| ); | ||
| } | ||
|
|
||
| if subscribe_has_priority_without_handler(value) { | ||
| assert!( | ||
| toml::from_str::<CapsuleManifest>(src).is_err(), | ||
| "handler-less subscribe priority must fail manifest parsing" | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
Refactor the helper function to accept manifest_is_err directly as a boolean to avoid redundant TOML parsing.
| fn assert_malformed_security_shapes_fail_closed(src: &str, value: &Value) { | |
| if section_has_multi_pin(value, "publish") || section_has_multi_pin(value, "subscribe") { | |
| assert!( | |
| toml::from_str::<CapsuleManifest>(src).is_err(), | |
| "ambiguous source pins must fail manifest parsing" | |
| ); | |
| } | |
| if subscribe_has_priority_without_handler(value) { | |
| assert!( | |
| toml::from_str::<CapsuleManifest>(src).is_err(), | |
| "handler-less subscribe priority must fail manifest parsing" | |
| ); | |
| } | |
| } | |
| fn assert_malformed_security_shapes_fail_closed(manifest_is_err: bool, value: &Value) { | |
| if section_has_multi_pin(value, "publish") || section_has_multi_pin(value, "subscribe") { | |
| assert!( | |
| manifest_is_err, | |
| "ambiguous source pins must fail manifest parsing" | |
| ); | |
| } | |
| if subscribe_has_priority_without_handler(value) { | |
| assert!( | |
| manifest_is_err, | |
| "handler-less subscribe priority must fail manifest parsing" | |
| ); | |
| } | |
| } |
| let serialized = toml::to_string(&config).expect("parsed config must serialize"); | ||
| let roundtrip: Config = | ||
| toml::from_str(&serialized).expect("serialized config must deserialize"); | ||
| let _ = astrid_config::validate::validate(&roundtrip); |
There was a problem hiding this comment.
Instead of discarding the validation result, we can assert a stronger semantic invariant: if the original deserialized config is valid, the roundtripped config must also remain valid. This helps catch any serialization/deserialization bugs that silently corrupt valid configurations.
if astrid_config::validate::validate(&config).is_ok() {
astrid_config::validate::validate(&roundtrip)
.expect("a valid config must remain valid after a serialization roundtrip");
}References
- When a Result is assumed to never be an Err, use .expect() with a descriptive message to enforce this invariant rather than silently ignoring a potential error with if let Ok(...).
There was a problem hiding this comment.
Pull request overview
This PR extends Astrid’s standalone fuzz/ package with Tier 2 structured fuzz targets covering untrusted TOML/JSON parser and wire-format surfaces (config, capsule manifest, kernel/admin messages, and opt-in gateway request DTOs), and updates the fuzz package feature-gating and documentation accordingly.
Changes:
- Add new fuzz targets:
admin_json,config_toml,manifest_toml, and opt-ingateway_request_json. - Expand
fuzz/Cargo.tomlfeature gates so cheap parser/wire targets are enabled by default while capsule/gateway targets remain opt-in. - Update
fuzz/README.mdandCHANGELOG.mdto document the new targets and feature model.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| fuzz/README.md | Documents the new fuzz targets and clarifies which ones are feature-gated. |
| fuzz/fuzz_targets/admin_json.rs | Adds fuzzing for kernel/admin JSON request/response wire types with roundtrip and tag assertions. |
| fuzz/fuzz_targets/config_toml.rs | Adds TOML fuzzing for astrid-config parsing plus serialization roundtrip and validation call. |
| fuzz/fuzz_targets/manifest_toml.rs | Adds TOML fuzzing for Capsule.toml parsing with publish/subscribe invariants and fail-closed shape checks. |
| fuzz/fuzz_targets/gateway_request_json.rs | Adds opt-in fuzzing for gateway request-body DTO deserialization. |
| fuzz/Cargo.toml | Wires new dependencies, default/opt-in feature gates, and new fuzz target bins. |
| CHANGELOG.md | Records the addition of the new parser/wire-format fuzz targets under Added. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if !KNOWN_ADMIN_METHODS.contains(&method) { | ||
| assert!( | ||
| serde_json::from_value::<AdminKernelRequest>(value.clone()).is_err(), | ||
| "unknown admin method {method:?} must not deserialize as an allowed request" | ||
| ); |
| events-target = ["dep:astrid-events"] | ||
| ssrf-target = ["core-target", "dep:astrid-capsule"] | ||
| gateway-target = ["dep:astrid-gateway", "dep:serde", "dep:serde_json"] | ||
| ssrf-target = ["core-target", "capsule-target"] |
| fuzz_target!(|data: &[u8]| { | ||
| try_parse::<astrid_gateway::routes::agent::ApprovalResponseRequest>(data); | ||
| try_parse::<astrid_gateway::routes::agent::ElicitResponseRequest>(data); | ||
| try_parse::<astrid_gateway::routes::agent::PromptRequest>(data); |
Linked Issue
Closes #1087.
Part of #1084.
Summary
Adds the parser and wire-format structured fuzzing slice for untrusted TOML and JSON inputs.
Changes
admin_jsonfuzz coverage for admin/kernel JSON request and response wire formats, including explicit discriminant tags and unknown admin-method fail-closed behavior.config_tomlfuzz coverage for runtime config TOML parsing, serialization roundtrips, and validation entrypoints.manifest_tomlfuzz coverage for capsule manifest TOML parsing and normalized publish/subscribe source-pin and handler invariants.gateway_request_jsonfuzz coverage for public HTTP gateway request-body DTO deserialization.fuzz/Cargo.tomlfeature gates so cheap defaults cover admin/config parsing while capsule and gateway parser targets remain opt-in.fuzz/README.mdandCHANGELOG.mdfor the new fuzz targets.Verification
cargo fmtcargo fmt --manifest-path fuzz/Cargo.tomlcargo metadata --manifest-path fuzz/Cargo.toml --no-deps --format-version 1cargo check --manifest-path fuzz/Cargo.toml --binscargo check --manifest-path fuzz/Cargo.toml --bins --features capsule-target,gateway-targetcargo check --manifest-path fuzz/Cargo.toml --bins --features capabilities-target,ssrf-target,gateway-targetgit diff --check