Skip to content

Roslyn analyzer: enforce SecretOutputRedactor on all tool output paths #829

@Aaronontheweb

Description

@Aaronontheweb

Problem

The security audit (2026-04-29) found that SecretOutputRedactor.Redact() was only applied inside ShellTool and BackgroundJobExecutionActor, not at the tool execution pipeline level. This meant FileReadTool, WebFetchTool, McpToolAdapter, and all other non-shell tools returned unredacted secrets to the LLM.

We fixed this by adding pipeline-level redaction in DispatchingToolExecutor.ExecuteAsync(), but the gap existed for months without detection. A Roslyn analyzer could enforce this invariant at compile time.

Proposal

Add analyzer rules to the internal analyzer package (see #133, #304):

Rule: Tool output must flow through SecretOutputRedactor

Trigger: Any INetclawTool.ExecuteAsync() return value that reaches DispatchingToolExecutor or SessionToolExecutionPipeline without a SecretOutputRedactor.Redact() call on the path.

Practical approach: Since analyzing inter-method data flow is complex for an incremental generator, consider these simpler enforcement strategies:

  1. Require redaction at the pipeline boundary — Flag if DispatchingToolExecutor.ExecuteAsync() returns a tool result without calling SecretOutputRedactor.Redact() in the same method body. This is a single-method containment check.

  2. Flag direct SecretOutputRedactor.Redact() calls inside individual tools — Now that redaction happens at the pipeline level, per-tool redaction calls (like ShellTool.cs:142) are redundant. The analyzer could warn on these to keep the redaction point centralized and prevent confusion about where redaction happens.

  3. Detect new INetclawTool implementations missing the pattern — If a tool implementation calls SecretOutputRedactor.Redact() directly, warn that this should happen at the pipeline level instead.

Rule: No raw SensitiveString.Value in HTTP responses

Trigger: Usage of .AccessToken.Value or .RefreshToken?.Value (or any SensitiveString.Value access) inside a minimal API endpoint handler or controller action that returns an HTTP response.

Context: The OAuth status endpoint was returning raw provider tokens to all authenticated clients. The fix was to gate on loopback, but a compile-time check would catch future regressions.

Relationship to existing issues

Acceptance criteria

  • At least rule 1 (pipeline-level redaction enforcement) implemented
  • Existing ShellTool.Redact() call either suppressed or removed as part of centralization
  • New tool implementations that bypass the pipeline are flagged
  • CI enforces the rule (warning initially, error after rollout)

Metadata

Metadata

Assignees

No one assigned

    Labels

    securitySecurity-related changes

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions