refactor: extract safety module into ironclaw_safety crate - #1024
Conversation
Move prompt injection defense, input validation, secret leak detection, and safety policy enforcement into a standalone crate under crates/. The safety module was a leaf dependency with no async, no database, and no other ironclaw traits — only pure computation with pattern matching. SafetyConfig (2 fields) moves into the crate; env-var resolution stays in ironclaw's config module as a free function. src/safety/mod.rs becomes a thin re-export so all existing `crate::safety::*` imports keep working. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add guidance to migrate imports from crate::safety to ironclaw_safety when touching files. Update project structure to reflect crates/ dir. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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 significantly refactors the project's architecture by isolating the safety-related functionalities into a dedicated Rust crate. This move enhances modularity, making the safety components more independent and reusable, as they consist primarily of pure computation without direct dependencies on Highlights
Changelog
Activity
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 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 counter productive. 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. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Pull request overview
Extracts the src/safety/ module into a standalone crates/ironclaw_safety/ crate, replacing the original module with a thin pub use ironclaw_safety::* re-export shim. The SafetyConfig struct moves to the new crate while env-var resolution remains in the main crate.
Changes:
- New
crates/ironclaw_safety/crate with all safety submodules (sanitizer, validator, policy, leak_detector, credential_detect) src/safety/mod.rsreduced to a re-export shim;src/config/safety.rsupdated to use a free function for config resolution- CLAUDE.md and Cargo workspace updated with guidance and dependency wiring
Reviewed changes
Copilot reviewed 9 out of 13 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| crates/ironclaw_safety/Cargo.toml | New crate manifest |
| crates/ironclaw_safety/src/lib.rs | Crate root with SafetyConfig, SafetyLayer, and re-exports |
| crates/ironclaw_safety/src/policy.rs | Moved policy module |
| crates/ironclaw_safety/src/sanitizer.rs | Updated import path |
| crates/ironclaw_safety/src/validator.rs | Moved validator module |
| crates/ironclaw_safety/src/credential_detect.rs | Moved credential detection module |
| crates/ironclaw_safety/src/leak_detector.rs | Updated import paths in tests |
| src/safety/mod.rs | Replaced with re-export shim |
| src/config/safety.rs | Changed from impl method to free function |
| src/config/mod.rs | Updated call site for config resolution |
| Cargo.toml | Added workspace member and dependency |
| Cargo.lock | Updated lockfile |
| CLAUDE.md | Added guidance for extracted crate |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Code Review
This pull request is a well-executed refactoring that extracts the safety module into a new ironclaw_safety crate. The changes are clean, and the use of a re-export shim in src/safety/mod.rs ensures backward compatibility, which is a great approach for a change of this scale. The new crate structure improves modularity and separation of concerns. I have one minor suggestion to improve the performance of XML attribute escaping by avoiding multiple string allocations, aligning with our guidelines on minimizing heap allocations for performance.
| fn escape_xml_attr(s: &str) -> String { | ||
| s.replace('&', "&") | ||
| .replace('"', """) | ||
| .replace('<', "<") | ||
| .replace('>', ">") | ||
| } |
There was a problem hiding this comment.
The current implementation of escape_xml_attr with chained replace calls can be inefficient, as each call might allocate a new String if the character to be replaced is found. A more performant approach is to iterate over the string's characters once and build the new escaped string in a single pass. This avoids intermediate string allocations.
fn escape_xml_attr(s: &str) -> String {
// Pre-allocating with the original string's length is a good starting point.
let mut escaped = String::with_capacity(s.len());
for c in s.chars() {
match c {
'&' => escaped.push_str("&"),
'"' => escaped.push_str("""),
'<' => escaped.push_str("<"),
'>' => escaped.push_str(">"),
_ => escaped.push(c),
}
}
escaped
}References
- To improve performance, avoid unnecessary heap allocations. When processing string parts, build the result in a single pass to prevent intermediate string allocations.
There was a problem hiding this comment.
Fixed in b78c336 — rewrote to single-pass char iteration with String::with_capacity. O(n) with no intermediate allocations.
Split fuzz infrastructure: - crates/ironclaw_safety/fuzz/ — 5 safety-only targets (sanitizer, validator, leak_detector, credential_detect, config_env) depending only on ironclaw_safety for faster builds - fuzz/ — keeps fuzz_tool_params which needs ironclaw::tools Add seed corpus files (51 total) covering each pattern family: sanitizer injection patterns, validator edge cases, leak detector secret formats, credential detect HTTP param shapes. Add new fuzz_credential_detect target exercising params_contain_manual_credentials with arbitrary JSON. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Rewrite escape_xml_attr from chained .replace() to single-pass char iteration (O(n) instead of O(4n) with intermediate allocations). Add version = "0.1.0" to ironclaw_safety path dep to satisfy cargo-deny wildcards = "deny". Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 67 out of 77 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
* refactor: extract safety module into ironclaw_safety crate Move prompt injection defense, input validation, secret leak detection, and safety policy enforcement into a standalone crate under crates/. The safety module was a leaf dependency with no async, no database, and no other ironclaw traits — only pure computation with pattern matching. SafetyConfig (2 fields) moves into the crate; env-var resolution stays in ironclaw's config module as a free function. src/safety/mod.rs becomes a thin re-export so all existing `crate::safety::*` imports keep working. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * docs: update CLAUDE.md for ironclaw_safety crate extraction Add guidance to migrate imports from crate::safety to ironclaw_safety when touching files. Update project structure to reflect crates/ dir. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor: move safety fuzz targets into ironclaw_safety crate Split fuzz infrastructure: - crates/ironclaw_safety/fuzz/ — 5 safety-only targets (sanitizer, validator, leak_detector, credential_detect, config_env) depending only on ironclaw_safety for faster builds - fuzz/ — keeps fuzz_tool_params which needs ironclaw::tools Add seed corpus files (51 total) covering each pattern family: sanitizer injection patterns, validator edge cases, leak detector secret formats, credential detect HTTP param shapes. Add new fuzz_credential_detect target exercising params_contain_manual_credentials with arbitrary JSON. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: address PR review — single-pass XML escaping and versioned path dep Rewrite escape_xml_attr from chained .replace() to single-pass char iteration (O(n) instead of O(4n) with intermediate allocations). Add version = "0.1.0" to ironclaw_safety path dep to satisfy cargo-deny wildcards = "deny". Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
* refactor: extract safety module into ironclaw_safety crate Move prompt injection defense, input validation, secret leak detection, and safety policy enforcement into a standalone crate under crates/. The safety module was a leaf dependency with no async, no database, and no other ironclaw traits — only pure computation with pattern matching. SafetyConfig (2 fields) moves into the crate; env-var resolution stays in ironclaw's config module as a free function. src/safety/mod.rs becomes a thin re-export so all existing `crate::safety::*` imports keep working. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * docs: update CLAUDE.md for ironclaw_safety crate extraction Add guidance to migrate imports from crate::safety to ironclaw_safety when touching files. Update project structure to reflect crates/ dir. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor: move safety fuzz targets into ironclaw_safety crate Split fuzz infrastructure: - crates/ironclaw_safety/fuzz/ — 5 safety-only targets (sanitizer, validator, leak_detector, credential_detect, config_env) depending only on ironclaw_safety for faster builds - fuzz/ — keeps fuzz_tool_params which needs ironclaw::tools Add seed corpus files (51 total) covering each pattern family: sanitizer injection patterns, validator edge cases, leak detector secret formats, credential detect HTTP param shapes. Add new fuzz_credential_detect target exercising params_contain_manual_credentials with arbitrary JSON. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: address PR review — single-pass XML escaping and versioned path dep Rewrite escape_xml_attr from chained .replace() to single-pass char iteration (O(n) instead of O(4n) with intermediate allocations). Add version = "0.1.0" to ironclaw_safety path dep to satisfy cargo-deny wildcards = "deny". Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Summary
src/safety/into a standalonecrates/ironclaw_safety/crate (prompt injection defense, input validation, secret leak detection, policy enforcement)SafetyConfigmoves into the crate; env-var resolution stays in ironclaw's config module as a free function (resolve_safety_config())src/safety/mod.rsbecomes a thinpub use ironclaw_safety::*re-export — all existingcrate::safety::*imports keep workingironclaw_safetywhen touching filesThe safety module was a leaf dependency with no async, no database, and no ironclaw traits — only pure computation with pattern matching. External deps:
regex,aho-corasick,serde_json,url,thiserror,tracing.Test plan
cargo checkpassescargo clippy -p ironclaw_safety --all-targets— zero warningscargo test -p ironclaw_safety— all 81 tests passcargo test -p ironclaw --lib -- safety— re-export workssrc/safety/(fix: built-in time tool call failure #755, fix(safety): stop XML-escaping tool output content in wrap_for_llm #645, LLM-as-Judge semantic tool call evaluation #614, feat(worker): harden reasoning streams and sanitize job event payloads #461, feat(agent): add in-memory reasoning summaries and /reasoning surface #460) resolve conflicts cleanly🤖 Generated with Claude Code