feat(audit): integrate audit log recording into adapter layer#194
feat(audit): integrate audit log recording into adapter layer#194fohte merged 3 commits intofohte/audit-logfrom
Conversation
Add `audit_metadata()` and `is_auditable()` methods to the `Endpoint` trait and implement them for `ExecAdapter` (exec) and `ClaudeCodeHookAdapter` (hook). `CheckAdapter` remains non-auditable. Record audit log entries in `run_with_options()` after command evaluation but before action dispatch. Compound commands include per-sub-command evaluation results as `sub_evaluations`. Dry-run mode and non-auditable endpoints skip recording. Write failures are reported to stderr without affecting the exit code (fail-open). 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 integrates a comprehensive audit logging mechanism into the core command evaluation flow. It enables detailed recording of command executions and their outcomes, providing crucial visibility into system activity. The changes ensure that auditable actions are logged with relevant metadata, while non-auditable or dry-run operations are appropriately excluded, enhancing security and operational transparency without impacting core functionality on failure. Highlights
Changelog
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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
|
|
||
| #[rstest] | ||
| fn audit_log_written_for_auditable_endpoint_on_match() { | ||
| let audit_dir = tempfile::TempDir::new().unwrap(); |
There was a problem hiding this comment.
🟡 CLAUDE.md violation: repeated tempfile::TempDir::new().unwrap() setup across 7 tests not extracted into #[fixture]
The CLAUDE.md rule "Use #[fixture] for shared test setup" states: "Do not repeat the same setup code across tests. Extract into #[fixture]." The let audit_dir = tempfile::TempDir::new().unwrap(); pattern is repeated identically in 7 test functions (lines 1155, 1181, 1200, 1222, 1244, 1263, 1305). The same codebase already follows the fixture pattern correctly in src/audit/writer.rs:76-79 with #[fixture] fn audit_dir() -> TempDir { TempDir::new().unwrap() }. The adapter mod tests should follow the same convention.
Prompt for agents
In src/adapter/mod.rs, in the #[cfg(test)] mod tests block, add a #[fixture] for audit_dir (similar to the existing one in src/audit/writer.rs:76-79):
#[fixture]
fn audit_dir() -> tempfile::TempDir {
tempfile::TempDir::new().unwrap()
}
Then update each of the 7 test functions that currently have `let audit_dir = tempfile::TempDir::new().unwrap();` to instead accept audit_dir as a parameter. The affected test functions are:
- audit_log_written_for_auditable_endpoint_on_match (line 1154)
- audit_log_not_written_for_non_auditable_endpoint (line 1180)
- audit_log_not_written_in_dry_run_mode (line 1199)
- audit_log_not_written_when_disabled (line 1221)
- audit_log_records_deny_action (line 1243)
- audit_log_records_default_action_for_no_match (line 1262)
- audit_log_records_compound_sub_evaluations (line 1304)
Also add `use rstest::fixture;` to the existing imports if not already present.
For example, change:
#[rstest]
fn audit_log_written_for_auditable_endpoint_on_match() {
let audit_dir = tempfile::TempDir::new().unwrap();
To:
#[rstest]
fn audit_log_written_for_auditable_endpoint_on_match(audit_dir: tempfile::TempDir) {
Was this helpful? React with 👍 or 👎 to provide feedback.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## fohte/audit-log #194 +/- ##
===================================================
+ Coverage 89.83% 90.09% +0.26%
===================================================
Files 34 34
Lines 6885 7039 +154
===================================================
+ Hits 6185 6342 +157
+ Misses 700 697 -3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Return per-sub-command evaluation results from `evaluate_compound` via `CompoundEvalResult::sub_results` to avoid re-evaluating sub-commands for audit logging. Extract `read_single_audit_entry` helper and `#[fixture] audit_dir` to reduce test duplication. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
| let entries: Vec<_> = std::fs::read_dir(audit_dir.path()).unwrap().collect(); | ||
| assert!( | ||
| entries.is_empty(), | ||
| "no audit log should be written for non-auditable endpoint" | ||
| ); |
There was a problem hiding this comment.
🟡 CLAUDE.md violation: repeated assertion chain in 3+ tests not extracted into helper
The CLAUDE.md rule "Extract repeated assertions into helper functions" states: "If the same assertion chain appears in 3+ tests, extract it into a helper." The assertion chain let entries: Vec<_> = std::fs::read_dir(audit_dir.path()).unwrap().collect(); assert!(entries.is_empty(), ...); appears identically in exactly 3 tests: audit_log_not_written_for_non_auditable_endpoint (line 1188), audit_log_not_written_in_dry_run_mode (line 1209), and audit_log_not_written_when_disabled (line 1230). This should be extracted into a helper function like assert_no_audit_entries(audit_dir: &TempDir), similar to how read_single_audit_entry was already extracted as a helper for the positive case.
Prompt for agents
In src/adapter/mod.rs, in the test module (around line 1149 where read_single_audit_entry is defined), add a helper function:
fn assert_no_audit_entries(audit_dir: &TempDir) {
let entries: Vec<_> = std::fs::read_dir(audit_dir.path()).unwrap().collect();
assert!(entries.is_empty(), "expected no audit log files, but found {}", entries.len());
}
Then replace the 3 occurrences of the duplicated assertion chain:
1. Lines 1188-1192 in audit_log_not_written_for_non_auditable_endpoint
2. Lines 1209-1213 in audit_log_not_written_in_dry_run_mode
3. Lines 1230-1234 in audit_log_not_written_when_disabled
Each should be replaced with: assert_no_audit_entries(&audit_dir);
Was this helpful? React with 👍 or 👎 to provide feedback.
Replace inline assertion patterns with the extracted helper function for consistency and reduced duplication. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 participant
Why
What
audit_metadata()andis_auditable()methods to theEndpointtrait, allowing each adapter to provide audit log metadatarun_with_options()after command evaluation (exec/hook only; dry-run and check are excluded)