Skip to content

fix: avoid numeric PII false positives in prices and search results - #143

Merged
Mr-Lucky merged 4 commits into
mainfrom
fix/numeric-pii-false-positives
Oct 14, 2026
Merged

Mr-Lucky merged 4 commits into
mainfrom
fix/numeric-pii-false-positives

Conversation

@Mr-Lucky

Copy link
Copy Markdown
Contributor

Summary

Avoid numeric PII false positives in decimal prices and technical search-result fields.

Add context-aware filtering for decimals and known technical numeric fields while preserving detection and redaction for actual national IDs and bank account numbers. Add regression coverage for WebSearch, PostToolUse, and PostToolBatch flows.

Type

  • Bug fix
  • New feature / detection rule
  • Refactoring
  • Documentation

Testing

  • npm run build passes
  • npm test passes (886 tests)
  • Manually tested the change

Targeted verification passed:

  • node --test dist/tests/privacy-numeric-context.test.js
  • 4 tests passed

The full suite could not complete in the current sandbox because unrelated Cloud, Gateway, and daemon tests require localhost listeners and failed with EPERM.

Related Issues

Closes #

@github-actions

github-actions Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

AgentGuard PR Review

This patch introduces several security and correctness regressions around the new public-search exception and related redaction paths.

  1. severity: critical — src/runtime/protect.ts / src/dsh/runtime.ts / src/runtime/public-search.ts

    • What can go wrong: The new “public search result” provenance is trusted from mutable runtime metadata and in-memory matching, and then used to suppress PII blocking/redaction. For Claude and DSH paths, an attacker-controlled tool response or malformed hook payload can potentially mark content as publicSearchSensitive = false / claudeWebSearchSensitive = false and bypass PII egress protections for numeric identifiers.
    • Fix: Make the exception depend only on a strict, host-validated result boundary with schema-checked tool name/phase, and never on fields carried in untrusted raw input or mutable metadata alone. Verify the result provenance server-side before suppressing PII rules, and default to strict blocking if provenance is missing or ambiguous.
  2. severity: high — src/runtime/redaction.ts and src/scanner/rules/privacy.ts

    • What can go wrong: skipBareNumericIdentifiers disables redaction for broad classes of numeric identifiers inside WebSearch results, including structured outputs, while only partially re-checking some labeled cases. This can leak real card/account/national-id values embedded in search results, especially when they are not in the exact label patterns the patch knows about.
    • Fix: Narrow the exemption to a minimal, explicitly enumerated set of verified public-search fields and result shapes, and continue redacting any value that matches validated PII patterns unless it is positively proven to be non-PII by schema and provenance checks.
  3. severity: high — src/runtime/protect.ts hunk around redactVerifiedClaudeToolOutput / redactStructuredClaudeOutput

    • What can go wrong: The new structured redaction logic skips redaction recursively for WebSearch content unless keys match SENSITIVE_IDENTIFIER_FIELD. This can leave nested PII in arbitrary substructures unredacted if the field names do not match the regex, and it also treats some credential-like fields inconsistently across object levels.
    • Fix: Redact based on value semantics recursively for all nested fields, not just selected keys. Preserve an allowlist for explicitly safe public-search metadata, but keep recursive PII scanning enabled for all other nested content.
  4. severity: medium — src/runtime/protect.ts (visibleContentNeedsProtection / containsSensitiveContent)

    • What can go wrong: containsSensitiveContent now depends on optional metadata flags (claudeWebSearchSensitive, claudeBatchSensitive) that are only conditionally set. If those flags are absent due to an unhandled code path, the logic falls back to redactText(action.input) !== action.input, which can miss cases where the input was already partially normalized/redacted or where the sensitive data is only present in tool result payloads not reflected in action.input.
    • Fix: Derive sensitivity from a single authoritative scan over the full action payload plus verified tool response, rather than mixing optional flags and string-diff heuristics. Treat missing provenance as sensitive by default.
  5. severity: medium — plugins/hermes/bridge.py / plugins/hermes/plugin.py

    • What can go wrong: The Hermes bridge forwards arbitrary caller-supplied result/tool_result/output into tool_response for post_tool_call. Because this new field is used to establish public-search provenance, a plugin caller can inject a fake result boundary and influence downstream privacy decisions.
    • Fix: Only accept result payloads from the host’s actual tool callback path, and reject or ignore externally supplied result/tool_result values unless they are validated as originating from the native tool execution boundary.

@Mr-Lucky
Mr-Lucky merged commit 9196e93 into main Oct 14, 2026
4 checks passed
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