feat(tools): browser-export redaction + bounded oversized-read preview - #50042
feat(tools): browser-export redaction + bounded oversized-read preview#50042arminanton wants to merge 1 commit into
Conversation
alt-glitch
added
type/security
teknium1
left a comment
teknium1
left a comment
There was a problem hiding this comment.
Thanks for targeting a real file-read redaction gap. The browser-export portion is still distinct from the related CDP guard in #54198, but this draft needs a current-main salvage rather than a direct application.
Problems
tools/file_tools.py:1010sets a 1,000-character minimum preview. Withfile_read_max_chars=50(covered by this PR's test), the result exceeds the configured cap; the test does not assertlen(result["content"]).tools/file_tools.py:117uses generic redaction rather thanfile_read=True. Current main's file-content contract intentionally emits non-reusable sentinels for prefix credentials (agent/redact.py:544-547;tests/agent/test_redact.py:936-973).- Current main already supersedes the oversized-read change:
tools/file_tools.py:1253-1280and1379-1414provide line-boundary truncation plusnext_offsetin both read paths via25f0cecf5e21bfe87aa27ffe8a8cb8734af2b1ca.
Suggested changes
- Keep current truncation/continuation behavior and reapply only browser-export redaction on top of it.
- Preserve the
file_read=Truenon-reusable-sentinel guarantee and add cap-boundary coverage.
Automated hermes-sweeper review.
| total_lines = result_dict.get("total_lines", "unknown") | ||
| return json.dumps({ | ||
| "error": ( | ||
| preview_chars = max(1_000, min(max_chars - 1_000, _OVERSIZED_READ_PREVIEW_CHARS)) |
There was a problem hiding this comment.
This minimum defeats small configured limits: with file_read_max_chars=50, this evaluates to 1,000, and the appended marker makes the returned content larger still. Bound the complete returned content to max_chars and add an assertion for the existing 50-character configuration test.
| if not content: | ||
| return content | ||
| if _looks_like_sensitive_browser_export(path): | ||
| content = redact_sensitive_text(content, force=True, code_file=False) |
There was a problem hiding this comment.
Please preserve the current file-content redaction contract when salvaging this: file_read=True emits non-reusable sentinels for recognized prefixes, whereas this generic call retains a head/tail mask. Keep the browser-specific opaque-value pass, but do not regress that file-read guarantee.
teknium1
added
sweeper:risk-security-boundary
a54e706 to
6f60b75
Compare
|
Rebased onto current Preview honors the configured cap (sweeper): the 1,000-character preview floor no longer exceeds
Kept scoped to file-read/browser-export redaction (no overlap with the CDP guard in #54198). |
read_file now scrubs browser cookie/localStorage/sessionStorage/IndexedDB export dumps via _BROWSER_EXPORT_NAME_RE: an extra opaque-value pass masks JSON "value" fields and the Netscape TSV cookie column that the built-in code-file patterns intentionally skip. Redaction uses file_read=True so prefix credentials become the non-reusable sentinel, preserving main's file-content contract. The oversized-read change from the original draft is dropped: current main already provides line-boundary truncation with next_offset via _truncate_to_char_budget.
6f60b75 to
2991a3f
Compare
3 participants
Hardens
read_file: (1) browser-storage exports (cookie/localStorage/IndexedDB/conversation DBs etc., matched by_BROWSER_EXPORT_NAME_RE) are redacted on read to avoid leaking captured secrets into context; (2) oversized reads return a bounded_OVERSIZED_READ_PREVIEW_CHARSpreview instead of erroring.tools/file_tools.py+ 2 tests. Draft for review.