fix(security): redact database URI passwords and mutable reasoning

[sdyckjq-lab]

What does this PR do?

Refs #43666.

This PR closes the remaining persistence-boundary gaps that can be fixed without changing provider replay data:

• redact passwords in common database URIs, including dialect+driver forms;
• redact mutable assistant reasoning before non-streaming callbacks, conversation history, and state DB persistence;
• keep mutable reasoning separate from provider-owned replay fields across normal, streaming, adapter, and MoA paths.

This is intentionally a partial fix for #43666, so it uses Refs rather than Fixes. It does not claim that state.db can never contain secrets.

Related Issue

• Refs Redaction gaps at the persistence boundary: tool output file dumps, compaction blocks, DB URIs (split from #43083) #43666
• fix(compression): apply strict redaction at every compaction text boundary #69294 already owns the compaction-summary hardening. This PR no longer changes compaction behavior.

Type of Change

• 🐛 Bug fix (non-breaking change that fixes an issue)
• ✨ New feature (non-breaking change that adds functionality)
• 🔒 Security fix
• 📝 Documentation update
• ✅ Tests (adding or improving test coverage)
• ♻️ Refactor (no behavior change)
• 🎯 New skill (bundled or hub)

Changes Made

Database URI passwords

• Cover PostgreSQL, MySQL/MySQLX, MariaDB, MongoDB, MSSQL, Oracle, ClickHouse, CockroachDB, Snowflake, Trino, DB2, Redis, and AMQP URI families, including driver-qualified schemes.
• Handle empty usernames and punctuation commonly found in URI passwords.
• Bound matching so a passwordless URI cannot consume a later email address from another quoted field.
• Keep the scan safe on very large input.

Mutable reasoning

• Redact ordinary assistant reasoning before it reaches persisted history or non-streaming reasoning callbacks.
• Make Gemini native, Copilot ACP, and Bedrock expose ordinary reasoning as mutable text rather than provider replay content.
• Preserve provenance when streams return reasoning, reasoning_content, or both, including partial-stream fallback and Bedrock used as an MoA aggregator.
• Extract deeply nested reasoning summaries iteratively so large or deeply nested responses do not hit recursion limits or degrade quadratically.

Replay boundary kept unchanged

The following remain unmodified because providers may require exact replay:

• every reasoning_details item, including unsigned text blocks;
• provider-native reasoning_content;
• signatures, encrypted content, and redacted_thinking material;
• tool-call arguments.

Live streaming reasoning display is also unchanged. If secret redaction is disabled, this PR intentionally provides no storage protection. Existing databases are not rewritten.

How to Test

1. Run the focused redaction and persistence tests, including a real SQLite round trip and the optional JSON persistence path.
2. Run the affected reasoning, streaming, Gemini, Copilot, Bedrock, MoA, and provider-parity tests.
3. Run Ruff on all changed Python files and git diff --check.

Local result on the refreshed head: 392 passed, 9 skipped. The database-redaction suite alone passes 113 tests. Ruff and git diff --check pass. Stress probes process 100,000 reasoning fragments in about 0.06 seconds and a one-million-character URI candidate in about 0.1 seconds on the test machine.

Checklist

Code

• I've read the Contributing Guide
• My commit messages follow Conventional Commits
• I searched for existing PRs and accounted for fix(compression): apply strict redaction at every compaction text boundary #69294
• My PR contains only changes related to this fix
• I've run the entire repository test suite locally as one command (the affected suites pass locally; GitHub CI covers the full matrix)
• I've added tests for the changed behavior and its negative cases
• I've tested on macOS; GitHub CI covers the project matrix

Documentation & Housekeeping

• Documentation update: N/A, no user-facing configuration or workflow changed
• cli-config.yaml.example: N/A, no config keys changed
• CONTRIBUTING.md / AGENTS.md: N/A, no contributor workflow changed
• Cross-platform impact considered; the implementation uses existing Python paths and CI covers the matrix
• Tool descriptions/schemas: N/A, no tool contract changed

Residual Risk

• Provider replay fields may still contain plaintext secrets by design because mutating them can break later requests.
• Ambiguous, unquoted database-like text is handled with a safety-first match; structural password characters should be percent-encoded.
• This PR does not clean historical records and does not protect storage when redaction is disabled.

Co-authored-by: Kiro 有点Yes 246816394+sdyckjq-lab@users.noreply.github.com

[tonydwb]

Code Review Summary

Verdict: Approved

Security: Close persistence-boundary redaction gaps (#43666)

• Three distinct gaps closed:
  1. DB URIs: postgresql+psycopg://... and other SQLAlchemy-style dialect+driver schemes now caught by _DB_CONNSTR_RE. Previously only bare postgres://, mysql://, etc. matched.
  2. Reasoning fields: reasoning, reasoning_content, and reasoning_details now redacted at message construction before entering history/persistence. Previously only tool output and assistant content were redacted.
  3. Compaction blocks: summarizer input and static fallback summary both verified to contain no plaintext secrets.
• Opaque fields preserved: signature, encrypted_content, data, and encrypted type items in reasoning_details are returned byte-exact to avoid breaking provider signature checks. This is a documented tradeoff (signed thinking blocks can contain secrets that won't be redacted).
• Copy-on-write: _redact_reasoning_detail() returns a new dict rather than mutating the input, preserving the raw API response for tool execution in the same turn.
• Comprehensive tests (299 lines new test file + additions): test_signed_detail_preserved_byte_exact, test_raw_api_detail_dict_not_mutated, test_redaction_disabled_is_noop, test_stored_bytes_equal_replayed_bytes, and the SQLAlchemy dialect+driver parametrized suite.
• No regression risk: all changes are additive or defensive.

---

Reviewed by Hermes Agent

[liuhao1024]

Verification Review — looks solid ✅

Reviewed the persistence-boundary redaction diff end-to-end. Key observations:

1. Copy-on-write semantics — _redact_reasoning_detail() creates a new dict only when redaction actually changes a value; the original API response object is never mutated. Verified the test_raw_api_detail_dict_not_mutated test confirms this.

2. Opaque key preservation — Items with signature, encrypted_content, or data are returned untouched, preserving byte-exact replay for provider signature checks. The encrypted type-string check is a good secondary guard.

3. Reasoning field coverage — The three persistence paths (reasoning, reasoning_content, reasoning_details) are all redacted at message construction time in build_assistant_message, matching the defense-in-depth intent from Redaction gaps at the persistence boundary: tool output file dumps, compaction blocks, DB URIs (split from #43083) #43666.

4. DB connstr regex expansion — The +driver segment and empty-username coverage (redis://:password@host) close real gaps in the previous pattern.

5. Test suite — 299 lines covering reasoning fields, opaque preservation, tool-call args, state.db byte-level verification, and compaction. The stored_bytes_equal_replayed_bytes test is particularly valuable for the stored == replayed invariant.

No findings.

[sdyckjq-lab]

Thanks for the careful verification — I really appreciate you checking the copy-on-write behavior and the stored-bytes==replayed-bytes invariant. That was exactly the boundary I was trying to keep tight here.

[sdyckjq-lab]

@alt-glitch thanks for triaging and labeling this. CI is green, the PR is mergeable, and there is external verification on the diff.

Since this is scoped to #43666, I would appreciate a maintainer pass when you have bandwidth, especially on whether the fix matches the persistence-boundary constraints from the issue. Happy to adjust if you would prefer the scope split or narrowed further.

[egilewski]

Recommendation: approve.

I reviewed this security PR against current GitHub main b6c7ebf028d8434270c9f446edb668c76485025e, PR base acd7932c0fc5f98331f01af6073115e6cf3ccf9d, and PR head 61908862a28ad3b38d2d881bc9ec13a800f85fcb.

Validation:

• git rev-list --left-right --count refs/remotes/upstream/main...refs/remotes/upstream/pr/43940: 244 1; git merge-tree --write-tree refs/remotes/upstream/main refs/remotes/upstream/pr/43940: 5828999c11942eeeb59dc701e10a9455ec3bf01a; git diff --check refs/remotes/upstream/main...refs/remotes/upstream/pr/43940: passed. A no-commit merge of the PR head into current main also succeeded cleanly.
• Synthetic persistence-boundary probe: current main reproduced the leak (redact_sensitive_text, reasoning, reasoning_content, unsigned reasoning_details, and the synthetic state.db scan all still contained the test password). The same probe on the current-main replay of this PR removed the password from those redacted sinks and left the synthetic state.db scan clean, while preserving a signed reasoning detail byte-exact.
• python -B -m pytest -q tests/agent/test_redact.py tests/agent/test_persistence_redaction.py -p no:cacheprovider: 105 passed.
• python -B -m pytest -q tests/run_agent/test_run_agent.py -k 'reasoning or extra_content' -p no:cacheprovider: 40 passed, 337 deselected.
• python -B -m pytest -q tests/run_agent/test_provider_parity.py -k 'reasoning or extra_content or thought_signature or encrypted_content' -p no:cacheprovider: 24 passed, 69 deselected.
• python -B -m py_compile agent/redact.py agent/chat_completion_helpers.py tests/agent/test_redact.py tests/agent/test_persistence_redaction.py: passed.
• CodeRabbit clean-pass review on the uncommitted current-main replay completed with no findings.

Finding: I did not find a blocker in the reviewed scope. The patch covers the reproduced DB URI/userinfo leak plus the plain assistant reasoning persistence paths, keeps the stored-bytes==replayed-bytes invariant, and preserves opaque provider-signed/encrypted reasoning material as required.

Signed: GPT-5.5-xhigh in Codex

[sdyckjq-lab]

@egilewski thank you for the careful review and for independently replaying this against current main. I really appreciate you checking both the reproduced leak and the signed/opaque reasoning preservation path — that validation is very helpful for this security-sensitive change.

[sdyckjq-lab]

@teknium1 since you opened #43666, I’d appreciate your view on whether #43940 matches the persistence-boundary constraints you described. I’ve kept it scoped to the three gaps from the issue; CI is green after rebasing today, and the PR is mergeable. There has also been independent community validation on the diff.

Happy to narrow or adjust if you’d prefer a different shape for the fix.

[egilewski]

Recommendation: approve

I reviewed this security-mode rebase against current GitHub main bee13817f06995cf690ee4e4aafed956be78ab69, PR base aca11c227eb7e8b2f53f6e130d6e922455a573c1, and PR head 859dd1ebc4385555cb49572045d5f606047db21d.

Validation:

• GitHub checks on PR head: all reported checks passed, with only expected skipped jobs.
• git merge-tree --write-tree refs/remotes/upstream/main refs/remotes/upstream/pr/43940: passed with tree f6524fdb50c91ebc761351d5649d6e834ceb73f2.
• git diff --check refs/remotes/upstream/main...refs/remotes/upstream/pr/43940: passed.
• Current-main probe: redact_sensitive_text("postgresql+psycopg://postgres:honchorulez@...") and assistant reasoning persistence both retained the plaintext password, reproducing the #43666 persistence-boundary gap.
• Same probe on PR head: both the DB URI and assistant reasoning text redacted the password to ***.
• python -B -m pytest -q -p no:cacheprovider tests/agent/test_redact.py tests/agent/test_persistence_redaction.py: passed (105 passed).
• python -B -m pytest -q -p no:cacheprovider tests/run_agent/test_run_agent.py -k 'reasoning or extra_content': passed (40 passed, 338 deselected).
• python -B -m pytest -q -p no:cacheprovider tests/run_agent/test_provider_parity.py -k 'reasoning or extra_content or thought_signature or encrypted_content': passed (24 passed, 69 deselected).
• ruff check agent/redact.py agent/chat_completion_helpers.py tests/agent/test_redact.py tests/agent/test_persistence_redaction.py: passed.

Finding:
I did not find a blocker in the reviewed scope. The rebase still closes the DB URI and reasoning-field persistence gaps while preserving opaque provider replay material.

CodeRabbit ran successfully and reported one test-strength suggestion: changing the Gemini extra_content assertion from equality to identity. I checked the implementation and did not treat that as blocking because the replay contract is byte/value preservation, and the code can legitimately call model_dump() for provider objects while still preserving the serialized bytes. The raw-dict path currently preserves identity as well.

Signed: GPT-5.5-xhigh in Codex

[sdyckjq-lab]

Rebased this onto current main and resolved the conflicts.

The branch now follows #54136's direction: tool-call arguments remain preserved verbatim in replayed history, with builder + state DB round-trip tests covering that behavior.

The remaining scope is narrowed to the #43666 persistence-boundary gaps this PR still owns: DB URI dialect+driver password coverage and safe-to-mutate reasoning-field redaction before assistant messages enter stored/replayed history or reasoning display callbacks. Signed/encrypted provider replay material remains unmodified, and #49556's broader compaction-hardening scope remains separate.

Local validation included the focused redaction/persistence tests, reasoning + extra_content regressions, provider parity coverage, ruff, and a multi-pass /review sweep.

[sdyckjq-lab]

@teknium1 @alt-glitch quick update: rebased this PR onto current main, preserved #54136’s tool-call argument replay behavior, kept #49556’s broader compaction hardening separate, and fixed the CI flake that surfaced in the compression concurrency test. All required checks and Docker jobs are now green. Ready for another look when you have bandwidth.

[egilewski]

looks mergeable

Security evidence:

• trust boundary: model/provider output and tool output cross into display callbacks, conversation history, state DB persistence, and later provider replay.
• source/sink/invariant: DB URI userinfo and mutable assistant reasoning text must be redacted before persistence/display; tool-call arguments and signed/encrypted reasoning provider material must stay replay-stable.
• current-main reproduction: the run-root probe reproduced plaintext DB URI, reasoning, reasoning_content, and reasoning_details leaks on current main; tool-call arguments were preserved.
• PR-head or patch-replay validation: the same probe on PR head redacted DB URI, reasoning, reasoning_content, and unsigned reasoning_details text while preserving tool-call arguments; merge-tree and diff-check against current GitHub main passed.
• positive/negative cases: focused tests cover dialect+driver DB URIs, empty-username DB URIs, host:port false positives, web URL userinfo carveout, redaction-disabled no-op behavior, opaque signed/encrypted reasoning_details preservation, state DB replay equality, and tool-call argument byte preservation.
• residual bypass search: inspected builder, streaming reasoning callback, reasoning extraction, DB regex, and compaction summary paths; no blocker found in the reviewed scope, with tool-call argument storage intentionally out of scope.
• reviewer validation: CodeRabbit completed with four major suggestions; I verified them and did not treat them as blockers because the data payload recursion is intentionally outside the plain-text field contract, nested multi-key extraction is a non-security cleanup, and the surrogate/model_dump observations are pre-existing hardening gaps rather than regressions in this boundary fix.

I reproduced the original persistence-boundary leak on current main and validated the PR head with focused probes plus the redaction/persistence, run_agent reasoning, provider-parity, compression, and ruff checks.

Signed: GPT-5.5-xhigh in Codex

[teknium1]

Thanks for the narrowly scoped persistence-boundary fix. I found no verified blocker in the reviewed scope.

Current main still stores raw extracted reasoning in agent/chat_completion_helpers.py:1107-1120 and preserves raw reasoning_details at agent/chat_completion_helpers.py:1160-1175; hermes_state.py:3715-3767 serializes those fields and hermes_state.py:4398-4410 restores them for replay. Current _DB_CONNSTR_RE at agent/redact.py:232-235 also lacks a dialect+driver segment. The PR's stated boundary—redact mutable text while retaining signed/encrypted replay material—is consistent with the existing replay invariant documented by tests/run_agent/test_provider_parity.py:961-985.

GitHub currently reports the PR mergeable and CLEAN. This is an automated hermes-sweeper review.

[sdyckjq-lab]

Rebased #43940 onto current main and kept it focused on #43666's persistence boundary. The refreshed candidate removes the incomplete per-delta streaming callback hook and preserves provider-native reasoning_content, tool-call arguments, and signed/encrypted replay material. Current-base checks cover the exact disabled-redaction no-op, positive and negative DB URI cases, nested reasoning persistence/replay, and raw provider-input immutability. All required GitHub checks and both Docker builds are green.

[sdyckjq-lab]

Refreshed this PR from current main and narrowed it after #69294 merged.

• fix(compression): apply strict redaction at every compaction text boundary #69294 now owns the compaction-summary hardening.
• fix(security): redact database URI passwords and mutable reasoning #43940 now only covers database URI password redaction and mutable assistant reasoning before persistence or non-streaming callbacks.
• All provider replay carriers (reasoning_details, provider-native reasoning_content, signatures, encrypted/redacted material) and tool-call arguments remain unchanged.
• The fresh review found and fixed Bedrock-as-MoA provenance, a cross-field URI false positive, missing common URI families, and deep/large-input performance cases.
• Local affected suites pass with 392 passed and 9 skipped. Current GitHub CI, all eight Python slices, Desktop E2E, and both Docker builds are green.

The PR description now reflects the exact boundary and residual risks. Ready for a fresh maintainer review.