fix: address PR review comments on CONTRIBUTING.md

Add missing security-critical directories (src/sandbox/, src/orchestrator/) to Track C's list, and clarify that cargo deny check requires deny.toml. Co-Authored-By: Claude Opus 4.6 <[email protected]>
fix: expand CONTRIBUTING.md with setup, workflow, and guidelines
2026-08-30 08:17:53 +00:00 · 2026-03-10 11:15:53 -07:00 · 2026-03-09 23:11:43 -07:00 · 2026-03-09 23:03:27 -07:00
15 changed files with 99 additions and 226 deletions
@@ -0,0 +1,50 @@
 ## Summary
 <!-- 2-5 bullet points: what changed and why -->
 -
 ## Change Type
 <!-- Check one -->
 - [ ] Bug fix
 - [ ] New feature
 - [ ] Refactor
 - [ ] Documentation
 - [ ] CI/Infrastructure
 - [ ] Security
 - [ ] Dependencies
 ## Linked Issue
 <!-- Closes #N, or "None" -->
 ## Validation
 <!-- How did you verify this works? -->
 - [ ] `cargo fmt`
 - [ ] `cargo clippy --all --benches --tests --examples --all-features`
 - [ ] Relevant tests pass: <!-- list specific tests -->
 - [ ] Manual testing: <!-- describe what you tested -->
 ## Security Impact
 <!-- Does this change affect: permissions, network calls, secrets, file access, tool execution, sandbox policy? If yes, describe. If no, write "None". -->
 ## Database Impact
 <!-- Does this add/modify migrations, change schema, or affect both PostgreSQL and libSQL? If yes, describe. If no, write "None". -->
 ## Blast Radius
 <!-- What subsystems does this touch? What could break? -->
 ## Rollback Plan
 <!-- How to revert if this causes problems? For Track C changes, this is mandatory. -->
 ---
 **Review track**: <!-- A (docs/tests/chore) | B (feature/refactor) | C (security/runtime/DB/CI) -->
@@ -1,5 +1,34 @@
 # Contributing
 ## Getting Started
 ```bash
 git clone https://github.com/nearai/ironclaw.git
 cd ironclaw
 ./scripts/dev-setup.sh
 ```
 This installs the Rust toolchain, WASM targets, git hooks, and runs initial checks.
 ## Development Workflow
 ```bash
 cargo fmt                                                    # format
 cargo clippy --all --benches --tests --examples --all-features  # lint (zero warnings)
 cargo test                                                   # unit tests
 cargo test --features integration                            # + PostgreSQL tests
 ```
 ## Code Style
 - Zero clippy warnings policy
 - No `.unwrap()` or `.expect()` in production code (tests are fine)
 - Use `thiserror` for error types, map errors with context
 - Prefer `crate::` for cross-module imports
 - Comments for non-obvious logic only
 See `CLAUDE.md` for full style guidelines.
 ## Feature Parity Requirement
 When your change affects a tracked capability, update `FEATURE_PARITY.md` in the same branch.
@@ -9,3 +38,23 @@ When your change affects a tracked capability, update `FEATURE_PARITY.md` in the
 1. Review the relevant parity rows in `FEATURE_PARITY.md`.
 2. Update status/notes if behavior changed.
 3. Include the `FEATURE_PARITY.md` diff in your commit when applicable.
 ## Review Tracks
 All PRs follow a risk-based review process:
 | Track | Scope | Requirements |
 |-------|-------|-------------|
 | **A** | Docs, tests, chore, dependency bumps | 1 approval + CI green |
 | **B** | Features, refactors, new tools/channels | 1 approval + CI green + test evidence |
 | **C** | Security (`src/safety/`, `src/secrets/`, `src/sandbox/`, `src/orchestrator/`), runtime (`src/agent/`, `src/worker/`), database schema, CI workflows | 2 approvals + rollback plan documented |
 Select the appropriate track in the PR template based on what your changes touch.
 ## Database Changes
 IronClaw uses dual-backend persistence (PostgreSQL + libSQL). All new persistence features must support both backends. See `src/db/CLAUDE.md`.
 ## Adding Dependencies
 Run `cargo deny check` before adding new dependencies to verify license compatibility and check for known advisories (requires `deny.toml`; see the `cargo-deny` CI job).
@@ -14,7 +14,6 @@ exclude = [
    "tools-src/google-slides",
    "tools-src/slack",
    "tools-src/telegram",
    "fuzz",
 ]
 [package]
@@ -1,40 +0,0 @@
 [package]
 name = "ironclaw-fuzz"
 version = "0.0.0"
 publish = false
 edition = "2021"
 [package.metadata]
 cargo-fuzz = true
 [dependencies]
 libfuzzer-sys = "0.4"
 serde_json = "1"
 [dependencies.ironclaw]
 path = ".."
 [[bin]]
 name = "fuzz_safety_sanitizer"
 path = "fuzz_targets/fuzz_safety_sanitizer.rs"
 doc = false
 [[bin]]
 name = "fuzz_safety_validator"
 path = "fuzz_targets/fuzz_safety_validator.rs"
 doc = false
 [[bin]]
 name = "fuzz_leak_detector"
 path = "fuzz_targets/fuzz_leak_detector.rs"
 doc = false
 [[bin]]
 name = "fuzz_tool_params"
 path = "fuzz_targets/fuzz_tool_params.rs"
 doc = false
 [[bin]]
 name = "fuzz_config_env"
 path = "fuzz_targets/fuzz_config_env.rs"
 doc = false
@@ -1,43 +0,0 @@
 # IronClaw Fuzz Targets
 Fuzz testing for security-critical input parsing paths using [cargo-fuzz](https://github.com/rust-fuzz/cargo-fuzz) (libFuzzer).
 ## Targets
 | Target | What it exercises |
 |--------|-------------------|
 | `fuzz_safety_sanitizer` | Prompt injection pattern detection (Aho-Corasick + regex) |
 | `fuzz_safety_validator` | Input validation (length, encoding, forbidden patterns) |
 | `fuzz_leak_detector` | Secret leak detection (API keys, tokens, credentials) |
 | `fuzz_tool_params` | Tool parameter and schema JSON validation |
 | `fuzz_config_env` | Combined safety primitives (sanitize, validate, leak detect) |
 ## Setup
 ```bash
 cargo install cargo-fuzz
 rustup install nightly
 ```
 ## Running
 ```bash
 # Run a specific target (runs until stopped or crash found)
 cargo +nightly fuzz run fuzz_safety_sanitizer
 # Run with a time limit (5 minutes)
 cargo +nightly fuzz run fuzz_leak_detector -- -max_total_time=300
 # Run all targets for 60 seconds each
 for target in fuzz_safety_sanitizer fuzz_safety_validator fuzz_leak_detector fuzz_tool_params fuzz_config_env; do
    echo "==> $target"
    cargo +nightly fuzz run "$target" -- -max_total_time=60
 done
 ```
 ## Adding New Targets
 1. Create `fuzz/fuzz_targets/fuzz_<name>.rs` following the existing pattern
 2. Add a `[[bin]]` entry in `fuzz/Cargo.toml`
 3. Create `fuzz/corpus/fuzz_<name>/` for seed inputs
 4. Exercise real IronClaw code paths, not just generic serde
@@ -1,44 +0,0 @@
 #![no_main]
 use libfuzzer_sys::fuzz_target;
 use std::sync::LazyLock;
 use ironclaw::safety::{LeakDetector, Sanitizer, Validator};
 static SANITIZER: LazyLock<Sanitizer> = LazyLock::new(Sanitizer::new);
 static VALIDATOR: LazyLock<Validator> = LazyLock::new(Validator::new);
 static LEAK_DETECTOR: LazyLock<LeakDetector> = LazyLock::new(LeakDetector::new);
 fuzz_target!(|data: &[u8]| {
    if let Ok(input) = std::str::from_utf8(data) {
        // Exercise Sanitizer: detect and neutralize prompt injection attempts.
        let sanitized = SANITIZER.sanitize(input);
        // If no modification occurred, content must equal input.
        if !sanitized.was_modified {
            assert_eq!(sanitized.content, input);
        }
        // Exercise Validator: input validation (length, encoding, patterns).
        let result = VALIDATOR.validate(input);
        // ValidationResult must always be well-formed: if valid, no errors.
        if result.is_valid {
            assert!(
                result.errors.is_empty(),
                "valid result should have no errors"
            );
        }
        // Exercise LeakDetector: secret detection (API keys, tokens, etc.).
        let scan = LEAK_DETECTOR.scan(input);
        // scan_and_clean must not panic and must return valid UTF-8.
        let cleaned = LEAK_DETECTOR.scan_and_clean(input);
        // If scan found no matches, scan_and_clean should return the input unchanged.
        if scan.matches.is_empty() {
            if let Ok(ref clean_str) = cleaned {
                assert_eq!(
                    clean_str, input,
                    "scan_and_clean changed content despite no matches"
                );
            }
        }
    }
 });
@@ -1,25 +0,0 @@
 #![no_main]
 use libfuzzer_sys::fuzz_target;
 use std::sync::LazyLock;
 use ironclaw::safety::LeakDetector;
 static DETECTOR: LazyLock<LeakDetector> = LazyLock::new(LeakDetector::new);
 fuzz_target!(|data: &[u8]| {
    if let Ok(s) = std::str::from_utf8(data) {
        // Exercise scan path
        let result = DETECTOR.scan(s);
        // Invariant: if should_block, there must be matches
        if result.should_block {
            assert!(!result.matches.is_empty());
        }
        // Invariant: match locations must be valid
        for m in &result.matches {
            assert!(m.location.end <= s.len());
        }
        // Exercise scan_and_clean path
        let _ = DETECTOR.scan_and_clean(s);
    }
 });
@@ -1,25 +0,0 @@
 #![no_main]
 use libfuzzer_sys::fuzz_target;
 use std::sync::LazyLock;
 use ironclaw::safety::Sanitizer;
 static SANITIZER: LazyLock<Sanitizer> = LazyLock::new(Sanitizer::new);
 fuzz_target!(|data: &[u8]| {
    if let Ok(s) = std::str::from_utf8(data) {
        // Exercise the main sanitization path
        let result = SANITIZER.sanitize(s);
        // Verify invariant: warnings should have valid ranges
        for w in &result.warnings {
            assert!(w.location.end <= s.len());
        }
        // Verify invariant: critical severity triggers modification
        let has_critical = result.warnings.iter().any(|w| {
            w.severity == ironclaw::safety::Severity::Critical
        });
        if has_critical {
            assert!(result.was_modified);
        }
    }
 });
@@ -1,23 +0,0 @@
 #![no_main]
 use libfuzzer_sys::fuzz_target;
 use std::sync::LazyLock;
 use ironclaw::safety::Validator;
 static VALIDATOR: LazyLock<Validator> = LazyLock::new(Validator::new);
 fuzz_target!(|data: &[u8]| {
    if let Ok(s) = std::str::from_utf8(data) {
        // Exercise input validation
        let result = VALIDATOR.validate(s);
        // Invariant: empty input is always invalid
        if s.is_empty() {
            assert!(!result.is_valid);
        }
        // Exercise tool parameter validation with arbitrary JSON
        if let Ok(value) = serde_json::from_str::<serde_json::Value>(s) {
            let _ = VALIDATOR.validate_tool_params(&value);
        }
    }
 });
@@ -1,25 +0,0 @@
 #![no_main]
 use libfuzzer_sys::fuzz_target;
 use std::sync::LazyLock;
 use ironclaw::safety::Validator;
 use ironclaw::tools::validate_tool_schema;
 static VALIDATOR: LazyLock<Validator> = LazyLock::new(Validator::new);
 fuzz_target!(|data: &[u8]| {
    if let Ok(s) = std::str::from_utf8(data) {
        // Try parsing as JSON and validating as tool parameters
        if let Ok(value) = serde_json::from_str::<serde_json::Value>(s) {
            // Exercise Validator::validate_tool_params with arbitrary JSON
            let result = VALIDATOR.validate_tool_params(&value);
            // Invariant: result should always be well-formed
            if !result.is_valid {
                assert!(!result.errors.is_empty());
            }
            // Exercise validate_tool_schema with arbitrary JSON as a schema
            let _ = validate_tool_schema(&value, "fuzz");
        }
    }
 });