Skip to content

audit: record focused window title and screenshot hash on every action - #4

Merged
bscott merged 1 commit into
bscott:release/0.5.0from
Mrigbozurike:audit-window-and-hash
Oct 7, 2026
Merged

bscott merged 1 commit into
bscott:release/0.5.0from
Mrigbozurike:audit-window-and-hash

Conversation

@Mrigbozurike

@Mrigbozurike Mrigbozurike commented Sep 10, 2026 •

Copy link
Copy Markdown
Contributor

Why

The audit log said what was asked (input.click 640,400) but not what it was aimed at or which image the agent was looking at. For after-the-fact review of an agent's actions, both matter.

What

  • Screenshot, input, focus and clipboard entries gain window: the focused window's app: title, captured before the action runs so it records the target rather than the result, truncated to 160 characters and passed through the existing sanitize() so a hostile title cannot inject control sequences into the log (test added).
  • Screenshot entries gain screenshot_sha256 of the returned bytes. Every MCP action ends with a fresh screenshot, so every agent tool call leaves at least one hashed line, and a saved image can be matched to the line that produced it.
  • [serve.audit].window_titles = false omits the title (titles can carry document names or mail subjects).
  • rdc audit shows the window in brackets and the first 12 hex digits of the hash; --json has the full fields. Old log files still parse.
  • The window lookup is best-effort: if it fails the request proceeds and the line is written without it.

Testing

  • cargo clippy --all-targets, cargo test on Linux and Windows; new tests cover SHA-256 vectors, focused-window description/truncation, serialisation of the new fields, and sanitisation of a title containing an OSC escape.
  • Only new dependency is sha2.

@bscott

bscott commented Sep 11, 2026

Copy link
Copy Markdown
Owner

Thanks, recording the target window and the screenshot hash is a useful addition and the sanitising and opt-out are the right shape.

Before merge:

  1. Rebase on main: routes.rs changed today (whoami_h and health_h now take State and write audit lines), so this will conflict.
  2. Move the changelog entry from the released 0.3.0 section to Unreleased.
  3. The window lookup runs a full window enumeration before every input action. On macOS and Windows that can add tens of milliseconds per click in a long agent chain. Please measure it, or use a cheaper focused-window lookup.

Also, please sign off the commit (git commit -s --amend).

@Mrigbozurike

Copy link
Copy Markdown
Contributor Author

All three addressed in cb4d036, rebased on current main and signed off:

  1. Rebased. routes.rs merged cleanly with the whoami_h/health_h audit change; the window/hash logic goes through the same audited() helper.
  2. Changelog entry moved to Unreleased.
  3. Lookup cost. Replaced the per-action windows() enumeration with a focused_window() method on Desktop: GetForegroundWindow + one title read + one process-name lookup on Windows, hyprctl -j activewindow (single client, not the whole list) on Hyprland, and a filter over xcap's single CGWindowListCopyWindowInfo call on macOS. It's a provided trait method with a default implementation, so it isn't a new wire/CLI/MCP capability, just a cheaper subset of windows(). The time is logged at debug per request, and rdc doctor now prints it.

Measured on Windows 11 (debug build, 15 top-level windows open):

[info] focused window: 581.8µs
[ok ] screenshot: 1366x768 png 383 KB in 1.3914889s

So roughly 0.6 ms per action against a 1.4 s screenshot. I don't have macOS or Hyprland to measure; rdc doctor will print the number on yours. [serve.audit].window_titles = false remains the off switch.

@bscott

bscott commented Sep 14, 2026

Copy link
Copy Markdown
Owner

Thanks. Rebase, changelog and sign-off are all in, and the Windows and Hyprland lookups are the cheap ones we wanted. Three things before it can go in:

  1. macOS is still the full enumeration. focused_window() falls back to windows(), and in xcap 0.9.8 every title() and app_name() call re-runs CGWindowListCopyWindowInfo, with is_focused() querying NSWorkspace per window. Cheapest fix: check is_focused() first and read title/app only for the hit, or use NSWorkspace.frontmostApplication and filter one CG list by owner pid.
  2. The Hyprland lookup runs hyprctl with Command::output() on the tokio worker with no timeout, on every action now. Please wrap it in spawn_blocking with a short tokio::time::timeout that falls through to None, as the Windows path does.
  3. rdc doctor prints the focused window title unsanitised. Pass it through audit::sanitize.

Branch: we moved to release branches (see CONTRIBUTING). This will go into 0.5.0. Please rebase onto release/0.4.0; audit.rs, routes.rs and config.rs conflict because 0.4.0 added host/via fields to Entry, an Entry::sanitized() helper (add window and screenshot_sha256 to its loop), Audit::open(cfg, host), and src/server/tests.rs. Please add a router test there: an /v1/act line carries a sanitised window from a fake desktop with a hostile title, window_titles = false omits it, and the screenshot line's hash matches the body. I will retarget the PR when the release/0.5.0 branch exists.

@bscott
bscott changed the base branch from main to release/0.5.0 September 14, 2026 02:57
@bscott
bscott self-requested a review as a code owner September 14, 2026 02:57
@bscott

bscott commented Sep 14, 2026

Copy link
Copy Markdown
Owner

0.4.0 is released and this PR's base is now release/0.5.0. Please rebase onto it (it is main as of v0.4.0).

@Mrigbozurike

Copy link
Copy Markdown
Contributor Author

All three fixed, rebased onto release/0.5.0, signed off.

  1. macOS no longer enumerates. I read xcap 0.9.8 to check, and it's worse than the review says: ImplWindow stores only a window id, so pid(), app_name(), title() and the rect accessors each re-run CGWindowListCopyWindowInfo over every on-screen window — listing N windows costs O(N) full desktop snapshots. The lookup now asks NSWorkspace.sharedWorkspace().frontmostApplication() once, then reads accessors for that application's first window only (xcap lists front to back, so it is normally the first entry). Cost is independent of window count. If the frontmost application has no window in the on-screen list, its localizedName() is recorded with an empty title rather than nothing. This needs the NSWorkspace feature on objc2-app-kit, which is already a dependency; the binding is a safe fn with no MainThreadMarker, and capture::windows already calls NSWorkspace from spawn_blocking through xcap's is_focused, so the threading is the same as today's.
  2. Hyprland. focused_window now runs in spawn_blocking under a 250 ms tokio::time::timeout; on timeout the line is written without a window and a debug line says so. Hyprland is Copy so the closure owns it. Noted in a comment that a timed-out blocking task is left to finish and its result dropped.
  3. doctor passes the title through audit::sanitize before printing.

Also for the rebase: Entry::sanitized() now covers window and screenshot_sha256, so entries arriving at the viewer over the network are cleaned like every other field, and the screenshot action text is yours.

Router tests added in src/server/tests.rs, per the testing bar:

  • audited_actions_carry_the_focused_window_sanitised — a fake desktop reports an app named Term\x1b]0;pwned\x07 with a title containing a newline and a forged {"outcome":"ok"}; the /v1/act line carries app and title, no escape or newline survives, and exactly one line is written.
  • window_titles_false_omits_the_window — same request with window_titles = false records no window, and the rest of the line is unchanged.
  • screenshot_hash_matches_the_bytes_that_were_sent — the logged screenshot_sha256 equals the SHA-256 of the response body (added a raw-bytes variant of the test call helper so this hashes the actual body), and a non-screenshot route logs no hash.

I can measure Windows only: the lookup is ~0.6 ms there against a ~1.4 s screenshot. rdc doctor prints the number on macOS and Hyprland.

bscott commented Oct 4, 2026

Copy link
Copy Markdown
Owner

Thanks for the updates. CI is green and the router tests are good. Before this goes into 0.5.0:

Please fix

  1. macOS records the wrong window. rdc runs no AppKit run loop, so NSWorkspace.frontmostApplication() keeps returning whatever app was in front when the daemon started. xcap uses activeApplication() for this reason (xcap-0.9.8/src/macos/impl_window.rs). Please do the same.
  2. One timeout for every platform. Only Hyprland has one. Wrap the focused_window() call in audited() in a single short tokio::time::timeout, so macOS, Windows and the X11 fallback are covered too.
  3. rdc audit can panic on the hash. main.rs prints &h[..12], which slices by byte. A non-hex value from a streamed entry panics there. Use clean(h).chars().take(12).
  4. Docs: add window and screenshot_sha256 to the field table in docs/audit.md.

Tests to add. Please apply the patch below. It adds four router tests:

  • every audited desktop route records the window;
  • the window recorded is the one in front before the action;
  • a failed lookup still returns 200 with the hash intact;
  • a denied caller triggers no lookup.

They pass on your current head. Run cargo test and cargo clippy --all-targets -- -D warnings after applying.

tests.patch (apply with git apply tests.patch)
diff --git a/src/server/tests.rs b/src/server/tests.rs
index 4f54c88..dfbc1c0 100644
--- a/src/server/tests.rs
+++ b/src/server/tests.rs
@@ -18,6 +18,7 @@ use http_body_util::BodyExt;
 use std::collections::{BTreeSet, HashMap};
 use std::net::{IpAddr, SocketAddr};
 use std::path::PathBuf;
+use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering};
 use std::sync::{Arc, Mutex};
 use tower::ServiceExt;
 
@@ -26,6 +27,10 @@ use tower::ServiceExt;
 struct FakeDesktop {
     actions: Mutex<Vec<String>>,
     windows: Mutex<Vec<Window>>,
+    /// How many times the window list was read; the default `focused_window` goes through it.
+    window_queries: AtomicUsize,
+    /// Make every window lookup fail, as a backend with no window support would.
+    windows_fail: AtomicBool,
 }
 
 impl FakeDesktop {
@@ -65,10 +70,17 @@ impl Desktop for FakeDesktop {
         })
     }
     async fn windows(&self) -> Result<Vec<Window>> {
+        self.window_queries.fetch_add(1, Ordering::SeqCst);
+        if self.windows_fail.load(Ordering::SeqCst) {
+            return Err(RdcError::Unsupported("window listing".into()));
+        }
         Ok(self.windows.lock().unwrap().clone())
     }
     async fn focus(&self, target: WindowTarget) -> Result<()> {
         self.actions.lock().unwrap().push(format!("focus {target:?}"));
+        // Focusing changes which window is in front, so a lookup made after the action would
+        // see the new one.
+        self.focus_a_window_titled("Safari", "after focus");
         Ok(())
     }
     async fn input(&self, action: InputAction) -> Result<()> {
@@ -367,3 +379,85 @@ async fn screenshot_hash_matches_the_bytes_that_were_sent() {
     let clip = audit_lines(&h).into_iter().find(|e| e.path == "/v1/clipboard").unwrap();
     assert_eq!(clip.screenshot_sha256, None);
 }
+
+#[tokio::test]
+async fn every_audited_desktop_route_records_the_window() {
+    let h = harness("window-routes");
+    h.desktop.focus_a_window_titled("Editor", "draft.md");
+    let host = "studio-mac.example.ts.net:7770";
+    let (st, _) = call(&h, ALICE, host, "GET", "/v1/screenshot", None).await;
+    assert_eq!(st, StatusCode::OK);
+    let (st, _) = call(&h, ALICE, host, "GET", "/v1/clipboard", None).await;
+    assert_eq!(st, StatusCode::OK);
+    let (st, _) = call(&h, ALICE, host, "POST", "/v1/act", Some(r#"{"kind":"clipboard_set","text":"x"}"#)).await;
+    assert_eq!(st, StatusCode::OK);
+    let (st, _) = call(&h, ALICE, host, "POST", "/v1/act", Some(CLICK)).await;
+    assert_eq!(st, StatusCode::OK);
+
+    let lines = audit_lines(&h);
+    assert_eq!(lines.len(), 4, "{lines:?}");
+    for e in &lines {
+        let w = e.window.as_deref().unwrap_or_else(|| panic!("no window on {} {:?}", e.path, e.action));
+        assert!(w.contains("Editor") && w.contains("draft.md"), "{w:?}");
+    }
+    // Whoami and health touch no desktop and look nothing up.
+    let before = h.desktop.window_queries.load(Ordering::SeqCst);
+    call(&h, ALICE, host, "GET", "/v1/whoami", None).await;
+    call(&h, ALICE, host, "GET", "/health", None).await;
+    assert_eq!(h.desktop.window_queries.load(Ordering::SeqCst), before);
+}
+
+#[tokio::test]
+async fn the_window_is_the_one_in_front_before_the_action() {
+    let h = harness("window-before");
+    h.desktop.focus_a_window_titled("Terminal", "before focus");
+    let body = r#"{"kind":"focus","by":"app","value":"Safari"}"#;
+    let (st, _) = call(&h, ALICE, "studio-mac.example.ts.net:7770", "POST", "/v1/act", Some(body)).await;
+    assert_eq!(st, StatusCode::OK);
+
+    let act = audit_lines(&h).into_iter().find(|e| e.path == "/v1/act").unwrap();
+    let w = act.window.unwrap();
+    assert!(w.contains("Terminal") && w.contains("before focus"), "recorded the result, not the target: {w:?}");
+}
+
+#[tokio::test]
+async fn a_failed_window_lookup_does_not_fail_the_request() {
+    let h = harness("window-fails");
+    h.desktop.windows_fail.store(true, Ordering::SeqCst);
+    let host = "studio-mac.example.ts.net:7770";
+    let (st, body) = call(&h, ALICE, host, "POST", "/v1/act", Some(CLICK)).await;
+    assert_eq!(st, StatusCode::OK, "{body}");
+    let (st, _) = call(&h, ALICE, host, "GET", "/v1/screenshot", None).await;
+    assert_eq!(st, StatusCode::OK);
+
+    assert_eq!(h.desktop.actions.lock().unwrap().clone(), vec["input.click 10,10 Left x1".to_string()]);
+    let lines = audit_lines(&h);
+    assert_eq!(lines.len(), 2);
+    for e in &lines {
+        assert_eq!(e.outcome, "ok", "{e:?}");
+        assert_eq!(e.window, None, "a failed lookup is recorded as no window");
+    }
+    assert!(lines[1].screenshot_sha256.is_some(), "the hash does not depend on the window lookup");
+}
+
+#[tokio::test]
+async fn denied_requests_never_look_up_the_window() {
+    let h = harness("window-denied");
+    h.desktop.focus_a_window_titled("Mail", "Re: salary review");
+    let host = "studio-mac.example.ts.net:7770";
+    let (st, _) = call(&h, MONITOR, host, "POST", "/v1/act", Some(CLICK)).await;
+    assert_eq!(st, StatusCode::FORBIDDEN);
+    let (st, _) = call(&h, MONITOR, host, "GET", "/v1/clipboard", None).await;
+    assert_eq!(st, StatusCode::FORBIDDEN);
+    let (st, _) = call(&h, STRANGER, host, "GET", "/v1/screenshot", None).await;
+    assert_eq!(st, StatusCode::FORBIDDEN);
+
+    assert_eq!(
+        h.desktop.window_queries.load(Ordering::SeqCst),
+        0,
+        "a refused caller learned nothing about the desktop"
+    );
+    let lines = audit_lines(&h);
+    assert!(!lines.is_empty());
+    assert!(lines.iter().all(|e| e.window.is_none()), "{lines:?}");
+}

I'll check the macOS window tracking on real hardware once the fix is in.


Generated by Claude Code

@Mrigbozurike
Mrigbozurike force-pushed the audit-window-and-hash branch from 006789a to 5f1090a Compare October 5, 2026 07:08
@Mrigbozurike

Copy link
Copy Markdown
Contributor Author

All four fixed, rebased onto release/0.5.0, signed off.

  1. macOS. You're right, and thank you — a daemon with no AppKit run loop would have pinned frontmostApplication() at startup. Rather than call activeApplication() myself, the macOS and X11 path now leads with xcap's is_focused(), which is activeApplication() (impl_window.rs:353), and reads title/app/rect only for the window that answers true. Since xcap lists front to back that normally stops at the first entry, so it keeps the cost fix without rdc holding a deprecated API — and it lets me drop the NSWorkspace feature I had added to objc2-app-kit. Say the word if you would rather see the dictionary lookup written out in rdc.
  2. One timeout. Moved into audited(): a single 250 ms tokio::time::timeout around focused_window() covering macOS, Windows, Hyprland and the X11 fallback. The per-backend timeout is gone; backends keep their spawn_blocking, which is about not sitting on an executor thread.
  3. rdc audit. Now clean(h).chars().take(12). Reproduced the panic first with a streamed entry whose hash was 日本語日本語日本語 — &h[..12] split a 3-byte char — and it prints fine now.
  4. Docs. window and screenshot_sha256 added to the field table in docs/audit.md.

Your four router tests applied as given and pass. 56 tests green on Linux and Windows.

Signed-off-by: mrigbozurike <drbillionbook@gmail.com>
@Mrigbozurike
Mrigbozurike force-pushed the audit-window-and-hash branch from 5f1090a to 7e07e04 Compare October 5, 2026 08:30
bscott added a commit that referenced this pull request Oct 7, 2026
Merge #4 and #3 into release/0.5.0 (conflicts resolved)
@bscott
bscott merged commit be5bbea into bscott:release/0.5.0 Oct 7, 2026
4 checks passed
bscott pushed a commit that referenced this pull request Oct 7, 2026
Bump the version to 0.5.0 and move Unreleased under a dated 0.5.0 section. The root refusal from #5 is listed under Changed as a breaking change; the window-title entry from #4 states that titles are on by default and reach audit streams.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01H6FWzFP6qZXMn5TXipJPRw
Signed-off-by: Brian Scott <claudeai@bscott.email>
This was referenced Oct 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants