Skip to content

feat(audit): surface subtool audit - #1641

Open
kurtisc wants to merge 4 commits into
mainfrom
kurtisc/audit-subtools
Open

feat(audit): surface subtool audit#1641
kurtisc wants to merge 4 commits into
mainfrom
kurtisc/audit-subtools

Conversation

@kurtisc

@kurtisc kurtisc commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Linked Issue

Closes #1574

Summary

Records a per-session rollup of tool sandbox command-policy decisions, surfaced in nono audit list, nono audit show --json and nono inspect.

Test Plan

Run steps manually:

S=$(mktemp -d)
mkdir -p "$S/config/nono/profiles" "$S/ws"
printf 'public data\n' > "$S/ws/allowed.txt"
printf 'top secret\n'  > "$S/ws/secret.txt"
cat > "$S/config/nono/profiles/toolsum.json" <<EOF
{
  "meta": { "name": "toolsum" },
  "filesystem": { "allow": ["$S/ws"] },
  "network": { "block": true },
  "command_policies": { "commands": { "cat": {
    "executable": "/bin/cat",
    "from": { "session": {
      "sandbox": { "fs_read": ["$S/ws"] },
      "invocation_policy": {
        "default": "allow",
        "deny": [{ "argv": { "contains": ["secret.txt"] }, "reason": "secrets are not readable" }]
      }
    }}
  }}}
}
EOF

export XDG_CONFIG_HOME="$S/config"
export XDG_STATE_HOME="$HOME/.nono-repro/state"
N=./target/debug/nono

$N run --profile toolsum --no-rollback --workdir "$S/ws" -- \
  sh -c 'cat allowed.txt; cat secret.txt'
$N audit list        # tools: cat (1 allowed, 1 denied)
$N audit list --no-tools    # line suppressed

Checklist

  • An issue exists and is linked above
  • All commits are signed-off, using DCO
  • All new code follows the project's coding standards (CLAUDE.md) and is covered by tests
  • Public-facing changes are paired with documentation updates

@github-actions

github-actions Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

PR Review Summary

Size

Metric Value
Lines added +2054
Lines removed -240
Total changed 2294
Classification Large (> 300 lines)

Affected crates

  • crates/nono (core library) — careful review required. This is the security-critical sandbox primitive. A bug here bypasses OS-level isolation for every downstream user.
  • crates/nono-cli — CLI changes. Verify argument parsing, flag documentation, and UX behaviour across supported platforms.
  • bindings/c (C FFI) — ABI changes can silently break C callers. Confirm header and symbol compatibility.

Blast radius — Contained

This PR touches: source code


Updated automatically on each push to this PR.

Adds a closed outcome classification for command policy decisions, typed
at the CLI emitters that author the vocabulary, and folds terminal
decisions into a per-session rollup held in session metadata. Only
terminal decisions count, so an invocation gate followed by its terminal
decision records a single invocation.

Covers the rollup with the session digest, so an edited summary no
longer matches the digest committed to the ledger. The field is skipped
while unset, keeping the digests of metadata written before it existed
unchanged.

Refs: #1574
Signed-off-by: Kurtis Charnock <kurtis@nolabs.ai>
Carries the recorder's command policy rollup into the session metadata
written when a supervised run exits, on both the rollback and
audit-only paths.

Denormalising the rollup at finalize keeps later readers off the event
log, so listing sessions costs the same whether a run mediated one tool
call or millions.

Refs: #1574
Signed-off-by: Kurtis Charnock <kurtis@nolabs.ai>
Prints a per-session rollup of mediated commands under each entry in
`nono audit list`, naming the commands and highlighting denials, and
exposes the same rollup through `--json`. A tool-sandbox decision is
now visible without guessing which session to run `audit show` against.

Sessions that mediated nothing print no extra line, and `--no-tools`
suppresses the rollup. A session whose mediation never reached a
terminal decision reports that rather than reading as untouched.

Refs: #1574
Signed-off-by: Kurtis Charnock <kurtis@nolabs.ai>
Adds a mediated-command rollup to `nono inspect`, recomputed from the
audit event log because session metadata carries one only once a session
finalizes. `[live]` marks counts that are not final, and a log ending
mid-record is reported rather than passed off as complete. Decision
strings fold through a frozen classification, so an old log summarizes
as it was written, and the session directory is confirmed to resolve
inside an audit root before it is read.

Folds the sandbox runtime event's tool_sandbox_active flag into the
rollup, so a session that configures mediation produces a summary even
when no mediated command is ever invoked. `nono audit list` renders that
as `tools: active, no invocations`, separating a session that mediated
nothing from one that ran without mediation at all.

The flag comes from a sandbox runtime event recorded on Linux only, so
a macOS session reports mediation inactive and is recognisable only by
recorded decisions.

Refs: #1574
Signed-off-by: Kurtis Charnock <kurtis@nolabs.ai>
@kurtisc
kurtisc force-pushed the kurtisc/audit-subtools branch from e813ff7 to 897fb11 Compare August 14, 2026 13:22
@kurtisc
kurtisc marked this pull request as ready for review August 14, 2026 13:30

@lukehinds lukehinds left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Solid work @kurtisc , just a couple of bits I noticed, but worth verifying my assumptions are correct - the BufReader switch, you might well have a valid reason for that.

Comment on lines -97 to -99
let reader = BufReader::new(file);
let mut events = Vec::new();
for (index, line) in reader.lines().enumerate() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason for dropping BufReader and using read_to_string?

The advantage with BufReader, it reads chunks and streams, which has a lower memory footprint, compared to read_to_string which reads the whole file into mem first, then iterates over its lines. I guess on most systems that might be ok, but some audit-events.ndjson may get big over time if not pruned.

Comment thread crates/nono/src/audit.rs
self.command_policy_summary.observe_mediation_active();
}
self.append_event(AuditEventPayload::SandboxRuntime { event })
}

@lukehinds lukehinds Aug 14, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe worth a switcheroo on these;

self.append_event(...)?;
self.command_policy_summary.observe_mediation_active()

If there is any sort of recoverable failure in self.append_event, it stops the in-memory summary being written into session.json during the finalization later on

  1. Summary state changes in memory.
  2. writes to audit-events.ndjson fails.
  3. The session continues and later finalizes successfully.
  4. session.json contains a summary claiming an event/state that the audit log does not contain.

This might be not be a valid review though, if self.append_event is solid around safe failing - but even still it might be good from a defensive point of view.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

nono audit list should surface nested tool-sandbox activity

2 participants