Skip to content

Keep PII out of default-level WTA logs#261

Open
vanzue wants to merge 3 commits into
mainfrom
dev/vanzue/log-cwd-trace-only
Open

Keep PII out of default-level WTA logs#261
vanzue wants to merge 3 commits into
mainfrom
dev/vanzue/log-cwd-trace-only

Conversation

@vanzue

@vanzue vanzue commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

What

Audit and fix of WTA logs that emitted personal data at default log levels (info! / warn! / error!). The shipped release default filter is info (tools/wta/src/logging.rs), so anything at info+ lands in users' on-disk logs out of the box.

The repo convention is that user/agent content is logged only at trace! (off by default), via *.content targets. This PR brings the stragglers in line.

Findings & fixes

cwd (initial scope). run_delegate, acp_load_session (boot-time + inbound), agents_view resume dispatch, and master new_session logged the full working directory at info/warn. Each default line now logs has_cwd / length; the full path moves to the matching *.content trace!.

Tier 1 — user content / arbitrary files. master forwards fs/read_text_file / fs/write_text_file (the file the agent reads/writes) and the on-disk session title (often derived from the user's prompt) at info!. Default line now logs length + op only; full path / title move to master.content / session_hook.content trace!.

Tier 2 — fixed tool/runtime paths whose only PII is the Windows username. Hooks-installer config/plugin/marketplace/bundle paths, the master-pipe discovery file, and the agent-pane origin record. New crate::logging::redact_user_path() rewrites the %LOCALAPPDATA% / %APPDATA% / %USERPROFILE% prefix to a placeholder, so these stay debuggable at info/warn without leaking the username.

Tier 3 — captured subprocess output. Plugin-CLI stdout / stderr / args move to the agent_hooks.content trace!; only exe + exit status stay at default. (wtcli stdout/stderr reviewed and left as-is — tool diagnostics: pane GUIDs / protocol errors, no user content.)

Out of scope

  • debug!-level logs — dev-only, never the shipped info default (e.g. coordinator_log, acp_log_built_prompt, delegate_with_context).
  • Tier 4 agent command lines (app.rs, client.rs, master) — these are the user-configured agent invocation, not runtime user data.
  • err / error Display fields — diagnostic and essential; not reliably PII.

Verification

cargo build passes (no new warnings). New redact_user_path unit tests + existing logging tests green. No behavior change at WTA_LOG=trace.

Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com

Copilot AI review requested due to automatic review settings June 10, 2026 02:11

Copilot AI 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.

Pull request overview

This PR adjusts WTA delegate logging in tools/wta to avoid emitting the current working directory (cwd) at default log levels, moving that path to the existing trace-gated delegate.content channel to reduce PII exposure in shipped (info) logs.

Changes:

  • In run_delegate, remove cwd from the info! log and include it only in the delegate.content trace! log.
  • In delegate_with_context, remove cwd from the debug! log (leaving cwd only in the delegate.content trace! log alongside the command line).

Comment thread tools/wta/src/main.rs
Comment on lines +1619 to +1623
// Log the prompt length, not the text — the prompt is user content. `cwd`
// is a filesystem path (carries the username / folder names), so it is
// personal data too: keep it on the trace-only content channel.
tracing::info!(prompt_chars = prompt.map(|p| p.chars().count()), agent = agent_cmd, "run_delegate started");
tracing::trace!(target: "delegate.content", prompt = ?prompt, cwd, "run_delegate prompt");

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch — you're right, the delegate logs were not the only default-level cwd leak. Rather than narrow the claim, I followed the second option and gated all of them in fef9dfa:

  • acp_load_session (the two you pointed at, ~2570/2578) plus the inbound load_session handler at ~5422
  • agents_view dispatch_resume / dispatch_resume_in_agent_pane (warn + info, app.rs)
  • master new_session (mod.rs)

Each default-level line now logs has_cwd (bool) instead of the path; the full cwd moved to a sibling trace! on the matching *.content target. The two remaining cwd-in-log sites (coordinator_log, acp_log_built_prompt) are debug! level — dev-only, never the shipped info default — so left as-is. PR description updated.

run_delegate logged cwd at info! level. cwd is a filesystem path carrying the Windows username and folder names, so it is personal data, yet info is the shipped release default. Move cwd onto the existing trace-only delegate.content channel so nothing personal lands in wta-delegate.log at the default filter; the info line keeps only prompt_chars and the agent command name.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@vanzue vanzue force-pushed the dev/vanzue/log-cwd-trace-only branch from 27e904d to 9fa16d7 Compare June 10, 2026 02:17
Addresses Copilot review on PR #261: the delegate path was not the only default-level cwd leak. run_delegate's acp_load_session, app.rs dispatch_resume / dispatch_resume_in_agent_pane (agents_view) and the inbound load_session handler, plus master's new_session, all emitted the full cwd path at info!/warn! — which the shipped info filter writes.

Each default-level line now logs has_cwd (a bool) instead of the path; the full cwd moves to a sibling trace! on the matching *.content target (delegate.content / acp_load_session.content / agents_view.content / master.content), consistent with the repo convention that user/agent content only lands at trace. debug!-level cwd logs (coordinator_log, acp_log_built_prompt) are left as-is since debug is dev-only and never the shipped default.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 10, 2026 02:24

Copilot AI 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.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Full sweep of info/warn/error logs for personal data (the previous commits only covered cwd). Three classes addressed:

Tier 1 (user content / arbitrary files): master forwards fs/read|write_text_file and the on-disk session title at info!. The agent-controlled file path and the title (often derived from the user's prompt) now log only length/op at info; the full value moves to the trace-only master.content / session_hook.content channel.

Tier 2 (fixed tool/runtime paths whose only PII is the Windows username): hooks-installer config/plugin/marketplace/bundle paths, the master-pipe discovery file, and the agent-pane origin record. New crate::logging::redact_user_path() rewrites the %LOCALAPPDATA%/%APPDATA%/%USERPROFILE% prefix to a placeholder, so these stay debuggable at info/warn without leaking the username.

Tier 3 (captured subprocess output): the plugin-CLI stdout/stderr/args move to the agent_hooks.content trace; only exe + exit status stay at default. wtcli stdout/stderr were reviewed and left as-is (tool diagnostics: pane GUIDs / protocol errors, no user content).

Out of scope: debug!-level logs (dev-only) and Tier 4 agent command lines (user-configured, not runtime PII). cargo build + logging unit tests pass.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@vanzue vanzue force-pushed the dev/vanzue/log-cwd-trace-only branch from 90d04ee to ef54b3d Compare June 10, 2026 03:06
Copilot AI review requested due to automatic review settings June 10, 2026 03:06
@vanzue vanzue changed the title Keep delegate cwd path out of default-level logs Keep PII out of default-level WTA logs Jun 10, 2026

Copilot AI 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.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Comment thread tools/wta/src/logging.rs
Comment on lines +61 to +73
if let Some(prefix) = std::env::var_os(var) {
let prefix = prefix.to_string_lossy();
if prefix.is_empty() {
continue;
}
// `get(..len)` is None when `len` is not a char boundary, so this
// never panics on a non-ASCII username.
if let Some(head) = s.get(..prefix.len()) {
if head.eq_ignore_ascii_case(&prefix) {
return format!("{placeholder}{}", &s[prefix.len()..]);
}
}
}
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