fix(messaging): warn when credential redaction rewrites pasteable text

[bolichen97]

Tells the reader when credential redaction rewrote text a channel already delivered, so they do not paste a command that cannot run. Refs #8123, Refs #6189.

Problem

Credential redaction rewrites outbound assistant text on Slack and iMessage without telling the reader. A scheme://user:pass@host connection string arrives as [REDACTED: credential], so a command the assistant wrote is silently altered and fails when pasted. Each site computed a warning list and dropped it into a log the reader never sees:

• slack/handler.py — final response text and thinking text
• slack/interactions.py — _handle_review_approve, which posts an approved draft to the whole channel
• imessage/renderer.py — out, _ = redact_credentials(out), warnings discarded

#8109 has since fixed the same root cause on the dashboard, with a notice row. Slack and iMessage need their own channel-appropriate delivery, which is why they were scoped out of it.

Approach

Redaction is NOT relaxed — a channel is an egress path, so the credential must stay out of it. The missing half is the signal, exactly as in #8109.

• Slack posts one threaded notice per turn after the answer is committed. The answer and the thinking block share a single tally, so a turn never produces two identical warnings.
• Slack review approve posts the same notice, because approving a draft publishes it to the whole channel.
• iMessage sends it as an additional follow-up message, since a sent iMessage cannot be edited.
• The count is read from the placeholder in the text that actually shipped, not from the redactor's warnings list — that list is empty on the streaming path, because each chunk is redacted upstream and re-redacting the assembled text reports nothing. It sums every tag in CREDENTIAL_REDACTION_TAGS so an encoded-credential-only reply is not undercounted.
• Every notice post is best-effort: a failed notice must never turn a delivered answer into a failed turn.

Across the channels the sentence lives once, as messaging.renderer.credential_redaction_notice, beside display_safe — that function keeps the credential out of the channel and this one says it happened. It carries only a count, never secret bytes, and no markup or emoji, because one string ships to platforms that render different dialects or none.

It is not the repo's only spelling, and this description should not claim otherwise: dashboard/chat_runner.py's private _redaction_notice, which #8109 landed while this PR was open, is the same sentence with page-specific phrasing ("before it reached this page", "any command shown above"). Unifying them is a user-visible wording change to a surface that shipped today, on a path this PR does not otherwise touch, so it is named as a follow-up below rather than folded in here.

What changed since the last head

• The security.py half is gone. fix(dashboard): warn when redaction rewrites a credential in chat text #8109 landed the same registry on main (CREDENTIAL_REDACTION_TAGS, _REDACTED_ENCODED_CREDENTIAL_TAG kept deliberately private, pass 2 emitting the constant). Rebasing removed this PR's whole security.py diff, which resolves the First Principles blocker at the root: there is no public alias left to justify. main keeps that tag private on purpose, so exposing it would also have contradicted the module's own stated intent.
• The duplicated notice builder is merged. The two near-identical _credential_redaction_warning copies became the one shared builder, which also removes interactions.py's in-function from kiro_crew.slack.handler import _credential_redaction_warning — a private cross-module import that GPT flagged.
• The ride-along test edit is dropped. The previous head also narrowed main's tag ratchet from _REDACTED_*_TAG to _REDACTED_*_CREDENTIAL_TAG. That loosens a bound this PR does not need, so test_security.py is now byte-identical to main. The observation stands on its own: main's ratchet is broader than the tuple's documented credentials-only scope, so a future non-credential tag would be forced into it. Worth a separate look, not this PR.

Scope, and what stays silent

Deliberately not widened, and no longer overclaimed in this description:

• slack/transport_dispatch.py drives SlackRenderer through the shared redactor in messaging/renderer.py, which discards warnings. That is a live Slack turn path with no notice, so this PR does not close "the last silent Slack egress path".
• Eight sibling renderers stay silent (Discord, Telegram, Teams, Webex, Feishu, Weixin, WeCom, WhatsApp), same discarded-warnings root cause at messaging/driver.py.

An earlier version of this description claimed those need a send seam on the Renderer ABC first. That is wrong, and this PR's own design is the counter-example: it stopped consuming the redactor's warnings list and counts placeholders in the text that actually shipped, so the iMessage fix needed no seam and neither would the others. What actually keeps them out is that each is a separate per-platform decision about where the notice belongs — edit in place, follow-up message, or thread reply — which is eight judgements, not one loop. The shared builder added here is the piece each of them consumes.

Named follow-ups

• Unify the dashboard spelling — delete dashboard/chat_runner.py's _redaction_notice and call the shared builder at its one consumer. The dashboard's notice row also renders below the message it describes, so the shared wording holds there; the only cost is changing text that shipped today.
• The eight silent renderers (Discord, Telegram, Teams, Webex, Feishu, Weixin, WeCom, WhatsApp) and slack/transport_dispatch.py, per the paragraph above.
• The tag ratchet in test_security.py, per the note above: main's _REDACTED_*_TAG prefix is broader than the tuple's documented credentials-only scope.

Testing

• test_credential_redaction_notice.py (new) pins the shared builder — singular/plural, no secret bytes, no channel markup — and drives the Slack path end to end through handle_message.
• test_imessage_renderer.py covers iMessage delivery: the follow-up is sent, a clean answer sends none, no secret reaches the wire, and a failing notice does not fail a delivered turn.
• test_review_mode.py covers the review-approve egress, including that redaction is not relaxed and that a failing notice does not abort approve.
• Full targeted set green: 2,477 passed. Contract suites green: capability ledger, security posture, messaging dispatch/transport, options cap, slack render pipeline.
• Gates clean on the new base: black, subprocess-encoding, isort, flake8, mypy (1,281 files), brand, harness-parity, docs-lint.
• Specs updated in the same commit: the messaging/renderer.py row in messaging.md and a Streaming UX bullet in slack-gateway.md.

Pattern harvest

The blocking finding was a new public constant with zero consumers. It did not need arguing, because the constant already existed on main: #8109 fixed the same defect on the dashboard while this PR was open and landed the shared registry as part of it. A rebase deleted the entire contested surface. The failure mode was reaching for a shared constant without first checking whether the sibling fix for the same root cause had already introduced it upstream — which is exactly when it is most likely to exist, since both PRs need the same seam.

The second half is scope hygiene: the same head also carried a ratchet-narrowing test edit and a public alias that the fix did not need. Both were pure ride-along, and both are what the reviewer spent its budget on instead of the fix.

Rule candidate: when a defect has a sibling fix in flight for another surface, rebase onto the base branch before defending any newly added shared constant or helper — the sibling probably landed it, and the finding evaporates instead of needing a rebuttal.

[github-actions]

Design Review (Fable 5) — ✅ PASS

Design-level review of 2bafe3eb43f996efe4b5ede0f5fd7cc6e8e5d0e9 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: PASS

Real silent-corruption harm, fixed at the right seam: one shared sentence, per-site best-effort delivery, scope gaps stated honestly with a follow-up path.

Suggestions

• Hoist the tag tally into a shared helper beside credential_redaction_notice (e.g. count_credential_redactions(text)): the "sum every CREDENTIAL_REDACTION_TAGS tag so encoded-credential-only replies aren't undercounted" invariant is currently restated in three comments at three call sites, and it is exactly the second piece the eight silent sibling renderers will need when the Renderer seam follow-up lands.

[DESIGN-REVIEWED] 2bafe3e

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed 2bafe3eb43f996efe4b5ede0f5fd7cc6e8e5d0e9 — this comment is updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] 2bafe3e

Verdict parsed from the review's SHA-scoped output markers for commit 2bafe3eb43f996efe4b5ede0f5fd7cc6e8e5d0e9.

False positive or not applicable? A repository writer can comment:
/ai-review override fable 2bafe3eb43f996efe4b5ede0f5fd7cc6e8e5d0e9: <one-sentence reason>

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of 2bafe3eb43f996efe4b5ede0f5fd7cc6e8e5d0e9 and found no blocking issues.

This comment is updated in place on each push.

Review details

FINDING -- src/kiro_crew/slack/handler.py:4011 -- "thinking_mrkdwn" counts placeholders truncated from the 600-character preview, causing a false notice -> Fix: count tags in thinking_block after condensing.
[GPT-REVIEWED] 2bafe3e

False positive or not applicable? A repository writer can comment:
/ai-review override gpt 2bafe3eb43f996efe4b5ede0f5fd7cc6e8e5d0e9: <one-sentence reason>

[github-actions]

First Principles Review (Fable 5) — 🟡 CONCERNS

Premise-level review of 2bafe3eb43f996efe4b5ede0f5fd7cc6e8e5d0e9 — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All counts verified. The one substantive finding: the notice sentence now lives in two places — the PR's new shared builder and the dashboard's pre-existing private _redaction_notice from #8109, which this PR left standing while claiming "the sentence lives once."

First-Principles-Verdict: CONCERNS

"The sentence lives once" is false: dashboard/chat_runner.py:3121 _redaction_notice is the same sentence, and this PR's builder is its second spelling.

What this change ships

Intent: tell a Slack/iMessage reader when credential redaction rewrote a command they may paste (issue #6189 / #8123). FIX — the signal half of a reported defect, redaction unchanged.

1. Slack posts one threaded security notice after a redacted answer — justified
2. Answer and thinking share one tally, one notice per turn — justified
3. Approving a review draft posts the same notice — justified (public egress)
4. iMessage sends the notice as a follow-up message — justified
5. New public builder messaging.renderer.credential_redaction_notice — 3 consumers counted; duplicate of dashboard/chat_runner.py:3121
6. Spec rows in messaging.md and slack-gateway.md — mandated (same-commit spec rule)

Watch

• Description: "one sentence cannot drift into per-channel spellings" — it already has two. _redaction_notice (chat_runner.py:3121) differs only in "in this message … before it reached this page" vs "in the message above"; both render below the text they describe. Grep redaction placeholder builders: 2.
• Deferral rationale for the 8 silent renderers ("Both need a send seam on the Renderer ABC so the driver can hand a renderer its own warnings") is contradicted by this PR's own design, which abandons warnings and counts tags in delivered text — the iMessage fix needed no seam. Siblings counted: 9 renderer files under src/kiro_crew/*/renderer.py besides imessage/messaging. Deferring 8 channel edits is fine; the ABC-seam premise is not what blocks them.

Subtractions

• Delete _redaction_notice (dashboard/chat_runner.py:3121-3147) and call credential_redaction_notice at its one consumer (chat_runner.py:3265); the dashboard notice row also sits below its message, so the shared wording holds there.

[FIRST-PRINCIPLES-REVIEWED] 2bafe3e

[bolichen97]

Open PR relationship audit

This is a consolidated, point-in-time code-level audit note. It compares complete merge-base diffs and current/merged code; it does not treat a shared topic as duplication or partial coverage as completion.

Relationship findings

• PR #7960 is OVERLAPPING relative to this PR. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #7960: CONTINUE_DEVELOPMENT. Code-near in the same sink module but materially different behavior, and already reconciled — 7960 still merges cleanly over it. Files: src/kiro_crew/messaging/renderer.py. The two independent directions used different labels; the matrix conservatively retains OVERLAPPING for coordination.
• PR #8291 is OVERLAPPING relative to this PR. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #8291: MERGE_DISCUSSION. Sibling application of the same notice pattern on a different surface and a different rewriter; no coverage of PR #8291's behavior and no code contact. Files: src/kiro_crew/messaging/renderer.py. The two independent directions used different labels; the matrix conservatively retains OVERLAPPING for coordination.

No PR, Issue, label, branch, or review state was changed by the relationship-note portion of this audit.