* feat(shell): add Low/Medium/High risk levels for graduated approval (#172)
- Add `RiskLevel` enum (Low/Medium/High, Ord-comparable) to `tool.rs`
and re-export from `tools/mod.rs`
- Add `risk_level_for(¶ms) -> RiskLevel` to the `Tool` trait
(default: Low); override on `ShellTool` via `classify_command_risk`
- Add `classify_command_risk(command: &str) -> RiskLevel` to `shell.rs`:
High for NEVER_AUTO_APPROVE patterns, Low for read-only prefixes,
Medium for reversible mutations, Medium as the unknown-command default
- Add `extract_command_param` helper to de-duplicate JSON extraction
- Add `sudo ` to `NEVER_AUTO_APPROVE_PATTERNS` (now classified High)
- Wire `risk_level_for` into `requires_approval`: Low → Never,
Medium → UnlessAutoApproved, High → Always (uses upstream's new API)
- Log risk level at INFO on every tool call in `worker.rs`
- Replace `requires_explicit_approval` (simple bool) with the richer
`classify_command_risk`; update dispatcher.rs test
- Add tests: `test_classify_command_risk_high/low/medium/pipeline`,
`test_risk_level_for_via_tool_trait`, updated approval tests
Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
* style: apply cargo fmt to shell.rs and dispatcher.rs
Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
* fix(shell): fix pipeline risk aggregation and word-boundary matching
Address reviewer feedback:
- `classify_command_risk` now iterates ALL pipeline segments and takes
the maximum risk, so `echo hello | cargo build` → Medium instead of
the previous (wrong) Low
- Replace `starts_with` with `matches_command_pattern`: single-word
patterns use exact first-token comparison so `lsblk` no longer
matches `ls`, `makeself` no longer matches `make`, etc.; multi-word
patterns (e.g. `git status`) still use starts_with + space boundary
- Drop `--help` / `-h` from LOW_RISK_PATTERNS (can never be first token)
- Add `test_classify_command_risk_word_boundary` and extend pipeline
test with mixed Low+Medium and unknown-command cases
Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
* fix(shell): move sed/awk/find from Low to Medium risk
`sed -i`, `awk -i inplace`, and `find -delete`/`find -exec rm` can all
modify or delete files. Classifying these as Low (auto-approve) was
unsafe. Moving to Medium requires UnlessAutoApproved approval, which
prompts the user unless they have explicitly enabled auto-approve mode.
Fixes review feedback from zmanian on PR #368.
Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
* fix(shell): update test to use classify_command_risk after requires_explicit_approval removal
The rebase brought in upstream commits that removed requires_explicit_approval.
Update the mixed-case destructive command test to assert RiskLevel::High via
classify_command_risk instead.
Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
* fix(shell): use word-boundary matching for High-risk patterns to prevent false positives
The NEVER_AUTO_APPROVE_PATTERNS check used `contains()` on the full command
string, causing false positives: `makeshutdownscript` matched `shutdown`,
`nftables-config` matched `nft`, and `passwdqc-check` matched `passwd`.
Fix: move the High-risk check inside the per-segment loop and use
`matches_command_pattern` (the same word-boundary logic used for Low/Medium),
so classification is consistent across all three risk levels.
Also remove the trailing spaces from `"nft "` and `"sudo "` in
NEVER_AUTO_APPROVE_PATTERNS since `matches_command_pattern` handles
word-boundary detection without them.
Adds three regression tests for the false-positive cases.
Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
* fix(shell): address zmanian review — redirect safety + explicit git push pattern
Two issues from zmanian's CHANGES_REQUESTED review on PR #368:
1. **Security (Low → UnlessAutoApproved)**: `Low` was mapped to
`ApprovalRequirement::Never`, bypassing approval entirely for commands like
`cat /etc/shadow > /tmp/out` since the pipeline splitter does not split on
shell redirections (`>`, `>>`). Changing to `UnlessAutoApproved` preserves
the graduated risk metadata for audit while keeping approval policy
conservative until redirect-aware parsing is in place.
2. **Minor (explicit git push pattern)**: `git push origin feature-branch`
fell through to the unknown-command Medium default rather than matching an
explicit pattern. Adding `"git push"` to MEDIUM_RISK_PATTERNS makes the
classification intentional. Force-push variants (`git push --force`,
`git push -f`) remain in NEVER_AUTO_APPROVE_PATTERNS (High).
Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
* test(shell): add regression tests for redirect bypass and git push pattern fixes
Two regression tests for the fixes in the previous commit:
1. `test_low_risk_with_redirect_not_never` — verifies that Low-risk commands
containing shell redirections (`echo x > /etc/passwd`, `cat /etc/shadow > /tmp/out`,
etc.) return `UnlessAutoApproved`, not `Never`. Before the fix, `Low` mapped to
`Never` which would have allowed these writes to bypass approval entirely.
2. `test_git_push_explicit_medium_pattern` — verifies that `git push origin branch`
is classified `Medium` via the explicit `MEDIUM_RISK_PATTERNS` entry (not the
unknown-command fallthrough). Force variants (`--force`, `-f`) remain `High`.
Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
* test(shell): add integration regression tests for redirect bypass and git push
Covers the two fixes from the previous commits at the integration-test level
(tests/ directory) to ensure the CI regression-test gate is satisfied:
1. `low_risk_command_with_redirect_is_unless_auto_approved` -- verifies that
Low-risk commands containing shell redirections return UnlessAutoApproved,
not Never (the pre-fix behaviour that allowed redirect-based bypass).
2. `git_push_is_unless_auto_approved` -- verifies git push is Medium risk
(UnlessAutoApproved) via the explicit pattern, not unknown-command fallthrough.
3. `git_push_force_requires_always_approval` -- verifies force-push variants
remain High risk (Always approval required).
Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
* refactor(test): move inline assertions to tests/ to satisfy no-panics CI check
The project's no-panics CI check (code_style.yml) scans src/**/*.rs for
assert_eq!/assert_ne!/.unwrap() in added lines. Moving classify_command_risk
tests to tests/shell_risk_regression.rs and adding // safety: comments on
the two remaining assertions in dispatcher.rs eliminates all false positives.
- Remove test_classify_command_risk_* and related functions from shell.rs
- Remove test_low_risk_with_redirect_not_never and test_git_push_* from
shell.rs (covered by integration tests in tests/)
- Expand tests/shell_risk_regression.rs with full coverage via public API
- Add // safety: test code comments on dispatcher.rs assert lines
Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
* fix(shell): address review findings — force-with-lease, test runners, Display
- Add `git push --force-with-lease` to NEVER_AUTO_APPROVE_PATTERNS — the
word-boundary matching in matches_command_pattern would not match it
against the existing `git push --force` pattern (next char is `-`, not
space), causing it to fall through to Medium instead of High.
- Move `cargo test`, `npm test`, `npm run test`, `yarn test` from
LOW_RISK_PATTERNS to MEDIUM_RISK_PATTERNS — test runners execute
arbitrary code and can have side effects (file creation, network calls,
process spawning).
- Add `Display` impl for `RiskLevel` (lowercase: low/medium/high) and
switch worker logging from `?risk` (Debug) to `%risk` (Display) for
cleaner audit logs.
- Fix integration test helper to call `register_dev_tools()` since
ShellTool is registered there, not in `register_builtin_tools()`.
Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
---------
Co-authored-by: Claude Sonnet 4.6 <[email protected]>
Co-authored-by: [email protected] <[email protected]>
* fix: add debug_assert invariant guards to critical code paths (closes#1215)
Add three debug_assert! calls to catch impossible-in-correct-code states
early in debug builds without affecting release performance:
- execute_tool_with_safety: assert tool_name is non-empty at entry
- JobContext::transition_to: assert state machine transition is valid
- CircuitBreakerProvider::record_success: assert circuit is not Open
(check_allowed() must gate all calls before record_success())
Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
* test: add regression test for empty tool name invariant guard
Covers the debug_assert!(!tool_name.is_empty()) added in execute_tool_with_safety.
Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
---------
Co-authored-by: Claude Sonnet 4.6 <[email protected]>
* fix(llm): complete response cache — set_model invalidation, stats logging, sync mutex
# Conflicts:
# src/llm/response_cache.rs
* fix(llm): address response cache review comments
- Add total_hit_count AtomicU64 that is never decremented on eviction;
maybe_log_stats now uses this counter so hit_rate_pct stays accurate
under high eviction pressure
- Log cache stats before returning on provider error so milestone
intervals (every 100 requests) are never silently skipped
- Add tracing-test dev-dep and three new tests: total_hits_survives_eviction,
stats_logged_at_request_100, stats_logged_on_provider_error_at_interval
- Update PR description to reflect actual set_model() behavior (key
isolation, not cache clear)
Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
---------
Co-authored-by: Claude Sonnet 4.6 <[email protected]>
* fix: allow OAuth callback to work on remote servers via OAUTH_CALLBACK_HOST
Fixes#186.
The OAuth callback URL was hardcoded to `http://127.0.0.1:9876` in two
places (NEAR AI login and MCP server auth). On a remote server this URL
is unreachable from the user's browser, making authentication impossible.
Changes:
- Add `callback_host()` to `oauth_defaults` that reads `OAUTH_CALLBACK_HOST`
(default: `127.0.0.1`)
- Update `bind_callback_listener()` to bind to `0.0.0.0` when a non-loopback
host is configured, so the port is reachable from outside the machine
- Update `session.rs` and `mcp/auth.rs` to use `callback_host()` instead
of hardcoded `127.0.0.1` / `localhost`
Usage on a remote server:
export OAUTH_CALLBACK_HOST=<your-server-ip>
ironclaw login
* fix: address PR review comments for OAuth callback security
* fix: address serrrfirat review comments on PR #212
---------
Co-authored-by: firat.sertgoz <[email protected]>
- Add docs/LLM_PROVIDERS.md with setup instructions for all supported providers
- Expand .env.example with Together AI and Fireworks AI example configs
- Add "Alternative LLM Providers" section to README with quickstart snippet
Co-authored-by: Claude Sonnet 4.6 <[email protected]>
Co-authored-by: firat.sertgoz <[email protected]>
* feat: wire memory hygiene into heartbeat loop (#166)
* refactor: address PR review comments for hygiene wiring
* style: fix fmt import ordering and clippy too_many_arguments warning
* fix: update heartbeat integration test to pass HygieneConfig argument
HeartbeatRunner::new() now requires a HygieneConfig as its second
argument after the hygiene wiring refactor. Pass the default config
in the integration test.
Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
---------
Co-authored-by: Claude Sonnet 4.6 <[email protected]>
Co-authored-by: Illia Polosukhin <[email protected]>
