Add permission audit trail — policy-based tool call checking#13
Add permission audit trail — policy-based tool call checking#13Siddhant-K-code merged 6 commits intomainfrom
Conversation
- audit.py: checks every tool_call against .agent-scope.json policy (file read/write glob patterns, command allow/deny list, network deny_all with allow exceptions via fnmatch) - Auto-flags sensitive files (.env, *.pem, .ssh/*, .github/workflows/*) even without a policy file - Exits with code 1 when violations found (CI-friendly) - 26 new tests (198 total passing) Co-authored-by: Ona <no-reply@ona.com>
- _cmd_matches: prefix match now requires space or end-of-string after the pattern, preventing 'curl' from matching 'curling --help' - _audit_event: use 'or' chaining instead of str(args.get(..., ...)) to avoid str(None) == 'None' passing the path guard Co-authored-by: Ona <no-reply@ona.com>
- Policy.load: distinguish json.JSONDecodeError from OSError and write a warning to stderr for both. Previously a malformed policy was silently treated as missing, giving no indication to the user. - _audit_event generic branch: clarify reason string to explain that no policy rule covers arbitrary tool types, not that no policy file exists. Co-authored-by: Ona <no-reply@ona.com>
Review: permission audit trailFour issues found. Two were already fixed upstream in Bug 1 —
|
| # Matching helpers | ||
| # --------------------------------------------------------------------------- | ||
|
|
||
| def _glob_match(path: str, patterns: list[str]) -> bool: |
There was a problem hiding this comment.
Bug: fnmatch does not treat ** as a recursive glob — src/** will not match src/auth.py.
fnmatch.fnmatch uses shell-style patterns where * matches everything except /. So fnmatch.fnmatch("src/auth.py", "src/**") returns False because ** expands to two * wildcards, neither of which matches across path separators.
The test test_glob_pattern asserts _glob_match("src/auth.py", ["src/**"]) is True — this test will fail when run. The filename fallback (fnmatch.fnmatch(name, pat)) doesn't help here because name is "auth.py" and the pattern is "src/**".
Fix options:
- Use
pathlib.PurePath.match()which does support**as a recursive wildcard:Path(path).match(pat) - Or normalise patterns: replace
**with*and accept thatsrc/*only matches one level deep (document this limitation)
pathlib.PurePath.match is stdlib and zero-dependency — recommended.
src/agent_trace/audit.py
Outdated
| args = data.get("arguments", {}) or {} | ||
|
|
||
| # --- File read --- | ||
| if tool_name in ("read", "view"): |
There was a problem hiding this comment.
Logic gap: tool_name == "view" is handled as a file read, but tool_name == "glob" and tool_name == "grep" are not.
Glob and Grep both read the filesystem (directory listings and file contents respectively) and are used by Claude Code. They fall through to the generic else branch and get no_policy regardless of any file read policy. A user who sets files.read.deny: [".env"] would not catch Grep .env or Glob .env*.
At minimum, grep should be treated as a file read (using args.get("pattern") or args.get("path")). glob is trickier since it matches multiple files, but flagging the pattern against deny rules is better than ignoring it.
| )) | ||
|
|
||
| # --- Bash / command execution --- | ||
| elif tool_name == "bash": |
There was a problem hiding this comment.
When a Bash command contains a URL that is denied by network policy, an AuditEntry for the network violation is added, and then the command itself is also evaluated against cmd_allow/cmd_deny. If curl is in cmd_deny, the same command produces two denied entries — one for Network access example.com and one for Ran: curl .... This double-counting inflates the violation count and makes the audit report confusing. Consider skipping the command policy check when a network violation has already been recorded for the same event, or document that this is intentional.
| # Sensitive file patterns (auto-flagged regardless of policy) | ||
| # --------------------------------------------------------------------------- | ||
|
|
||
| SENSITIVE_PATTERNS: list[str] = [ |
There was a problem hiding this comment.
*secret* will match src/secret_manager.py, test_secrets.py, no_secrets_here.py etc. — any file with "secret" anywhere in the name. This will produce a lot of false positives in normal codebases. Consider tightening to secrets.py, *secrets.json, *secrets.yaml or similar, and document the pattern rationale.
Review: PR #13 — Permission audit trailThe overall design is solid: policy loading is defensive (malformed JSON → warning, not crash), the One blocking bug (line 131): Two logic gaps (lines 193, 233):
One nit (line 26): Test gaps:
What's good:
|
…ensitive patterns audit.py: - _glob_match: replace fnmatch with pathlib.PurePath.match() so src/** correctly matches src/auth.py and src/utils/path.py (fnmatch treats ** as two * wildcards and does not match across path separators) - Add Grep and Glob to the file-read branch so file read policy applies to directory listings and content searches, not just Read/View calls - Network+command double-entry: skip command policy check when a network violation has already been recorded for the same event - SENSITIVE_PATTERNS: remove *secret* (too broad — matches secret_manager.py, test_secrets.py, etc.); replace with explicit secrets.json/yaml/toml/yml tests: - test_glob_pattern_nested: src/utils/path.py matches src/** - test_secret_manager_not_sensitive: source files no longer false-positive - test_secrets_json/yaml_is_sensitive: explicit secrets files still caught - TestGrepGlobPolicy: Grep denied path, Glob allowed path, Grep sensitive flag - TestNetworkCommandDoubleEntry: curl to denied URL = 1 entry not 2; curl to allowed host still checked against command policy - TestMalformedPolicy: malformed JSON returns None, audit continues as no-policy Co-authored-by: Ona <no-reply@ona.com>
PurePath.match() with ** is unreliable across Python 3.10-3.13 (the semantics changed in 3.12). Replace with a recursive segment-by-segment matcher (_match_parts) that correctly handles ** as zero-or-more path components on all supported versions. src/** now matches src/auth.py, src/utils/path.py, src/a/b/c/deep.py. Co-authored-by: Ona <no-reply@ona.com>
Co-authored-by: Ona <no-reply@ona.com>
Closes #7
What
Checks every
tool_callin a session against a.agent-scope.jsonpolicy file. Reports allowed actions, policy violations, and auto-flags sensitive file access — even without a policy file.Changes
audit.py(new):Policy— loaded from.agent-scope.jsonviafnmatchglob patternsaudit_session(store, session_id, policy_path)— classifies each tool_call asallowed,denied, orno_policySENSITIVE_PATTERNS— 18 patterns auto-flagged regardless of policy (.env,*.pem,.ssh/*,.github/workflows/*, etc.)format_audit()— structured report with ✅ /Policy file format (
.agent-scope.json):{ "files": { "read": { "allow": ["src/**", "tests/**"], "deny": [".env"] }, "write": { "allow": ["src/**"], "deny": [".github/**"] } }, "commands": { "allow": ["pytest", "uv run", "cat"], "deny": ["curl", "wget", "rm -rf"] }, "network": { "deny_all": true, "allow": ["localhost"] } }CLI:
agent-strace audit [session-id] [--policy <path>]Example output
Tests
26 new tests (198 total passing)