fix: redact stderr before the bound in themes and dev_fleet (#7374)

[CrysisDeu]

Summary

Three subprocess-stderr logging sites redacted credentials AFTER slicing to a fixed bound, so a credential straddling the slice was cut into fragments no redaction regex can match. This is the remainder of the defect class fixed by PR #7316 (deps.py) and PR #7350 (auto_improvement app); the First Principles lane on #7350 named these three sites.

The fix is the established mechanical reorder: feed the redactor the FULL text, apply the bound AFTER redaction.

• src/kiro_crew/dashboard/handlers/themes.py (_clone_github, head cut [:200]): switched to the shared security.redact_and_truncate(text, 200) — the exact idiom PR fix(auto_improvement): redact pip stderr before bounding in install_deps (#7307) #7316 introduced. Both redactors (exfiltration URLs + credentials) still run, now over the full text; the two now-unused imports are swapped for redact_and_truncate.
• src/kiro_crew/apps/builtins/dev_fleet/worktree_ops.py (worktree-removal failure, head cut [:300]): slice moved outside the runtime._redact() call. (The issue named these sites in dev_fleet/server.py; main has since refactored that module into a package, and the sites now live in worktree_ops.py — relocated by pattern per the issue's own instruction.)
• src/kiro_crew/apps/builtins/dev_fleet/worktree_ops.py (rebase-conflict tail, [-200:]): slice moved outside runtime._redact(). A tail cut of already-redacted text can at worst split a redaction marker, never a secret.

Regression tests

Two new test files follow the straddle pattern from #7316/#7350:

• test/test_theme_clone_stderr_redact_before_bound.py — behavioral straddle test for _clone_github (premise-guarded: the secret provably straddles the 200 bound and the @host tail lands beyond it) plus an AST-based structural pin for the module.
• test/test_dev_fleet_stderr_redact_before_bound.py — behavioral straddle test for the rebase-conflict tail cut (the tail window's left edge falls inside the secret, so a raw slice keeps the right half) plus the same structural pin covering both dev_fleet sites.

The structural pin scans direct stderr/stdout slice expressions via AST: a bounded char slice is sanctioned only when applied to the RESULT of a redact call. Mutation-verified: reverting the tail-cut site makes both the behavioral test and the structural pin fail.

No NON_EGRESS_REDACTION_MODULES / _REDACTION_SINKS posture change: all three modules already called a redactor — test/test_security_posture.py passes unchanged.

Testing

• isort, flake8, mypy (1217 files), diff-scoped black gate: all green.
• Full backend pytest: green except failures reproduced identically at pristine base 283625e16 on this host (environmental: home-dir spelling, AF_UNIX path length), none in changed files' modules.
• Targeted modules (themes, dev_fleet, security posture, new tests): 728 passed.

Deferred follow-up (deliberately out of scope)

• The issue's optional suggestion to promote the app-scoped structural sweep to a repo-wide CI scan is DROPPED: the general class (a bounded slice applied before a redact call) still has ~15 pre-existing sibling sites, and — as the First Principles lane correctly pointed out — most live in CORE paths (channel.py, dashboard/chat_runner.py, dashboard/handlers/artifacts.py, mcp_tools/control.py, vector_memory.py, …), not in auto_improvement. Only the auto_improvement subset is covered by open PR fix: redact git stderr before bounding in auto_improvement (#7333) #7350. A repo-wide sweep would red on all of them, so this PR ships only the three subprocess-stderr sites named by issue Slice-before-redact on subprocess stderr: 3 remaining sites outside auto_improvement (themes.py, dev_fleet server.py) #7374 plus straddle tests.
• The core-path sibling set now has a named owner: follow-up issue Slice-before-redact: ~15 core-path sibling sites beyond subprocess stderr (channel, chat_runner, artifacts, mcp_tools, memory) #7390 lists every site with the suggested fix shape. Promote the sweep once that lands.
• Pre-push review also noted auto_improvement/backend/clone_setup.py returns bounded raw stderr with no redaction at all; that file is covered by open PR fix: redact git stderr before bounding in auto_improvement (#7333) #7350.

Pattern harvest

Rule candidate: AST/structural test (shipped in this PR as _find_slice_before_redact_offenders, module-scoped)
Pattern: a bounded char slice applied to a subprocess stderr/stdout expression BEFORE the redaction call (redact*(text[:N]) / redact*(text[-N:]) or a raw stderr...[:N]) — redaction regexes cannot match a credential cut in half. Sanctioned form: slice the RESULT of the redact call, or use security.redact_and_truncate(text, N). Repo-wide promotion deliberately deferred: ~15 core-path sibling sites remain (tracked in follow-up issue #7390) plus the auto_improvement subset on open PR #7350; promote once the baseline is empty.

Closes #7374

[github-actions]

Design Review (Fable 5) — ✅ PASS

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

Design-Verdict: PASS

Established redact-then-bound idiom applied to the three named sites, straddle-tested and structurally pinned, with the sibling class tracked in a follow-up issue.

[DESIGN-REVIEWED] 87d0f27

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed 87d0f2798ba04ee0a3ad79c93f2bfa6c114a1e0d — this comment is updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] 87d0f27

Verdict parsed from the review's SHA-scoped output markers for commit 87d0f2798ba04ee0a3ad79c93f2bfa6c114a1e0d.

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

[github-actions]

First Principles Review (Fable 5) — 🟡 CONCERNS

Premise-level review of 87d0f2798ba04ee0a3ad79c93f2bfa6c114a1e0d — 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 claims verified. The fix sites are real and derived from issue #7374; redact_and_truncate (security.py:14407) and runtime._redact (runtime.py:34) both run the same two redactors, so both fix shapes are coverage-equivalent. The one thing a human should see: the repo now carries two structural scanners for this defect class — the regex app-sweep shipped in auto_improvement/tests/test_stderr_redact_before_bound.py:139 and the new AST scanner — with a third (repo-wide) planned in issue #7390.

First-Principles-Verdict: CONCERNS

Every fix site earns its place; the new AST scanner is a second spelling of the regex sweep already pinning this class in auto_improvement.

What this change ships

Intent: stop a credential that straddles a stderr truncation bound from escaping redaction into user-visible error text — a FIX.

1. Theme-clone failure text can no longer leak a bound-straddling credential — justified (issue Slice-before-redact on subprocess stderr: 3 remaining sites outside auto_improvement (themes.py, dev_fleet server.py) #7374, credential-redaction keep-invariant).
2. Dev-fleet worktree-removal error: redaction now precedes the 300-char cut — justified.
3. Dev-fleet rebase-conflict tail: redaction now precedes the 200-char tail cut — justified.
4. Straddle regression tests pinning both surfaces — justified (mutation-verified per description).
5. AST structural pin for the two modules — declared, but a second spelling of the existing regex sweep.

Watch

• Two structural pins for one class now coexist: the regex sweep (_RAW_SLICE/_SANCTIONED_TAIL, auto_improvement/tests/test_stderr_redact_before_bound.py:139-149, app-scoped) and the AST _find_slice_before_redact_offenders (module-scoped). Count: 2 scanners, already divergent (bound floor 10 vs multi-digit; alias handling). The description's own harvest plans a third, repo-wide. Converge on ONE scanner at the Slice-before-redact: ~15 core-path sibling sites beyond subprocess stderr (channel, chat_runner, artifacts, mcp_tools, memory) #7390 promotion, and say so there.
• ~15 counted core-path siblings stay unfixed — accepted-and-deferred; the description names the level and the owner (Slice-before-redact: ~15 core-path sibling sites beyond subprocess stderr (channel, chat_runner, artifacts, mcp_tools, memory) #7390), so this is declared, not hidden.

Subtractions

• When Slice-before-redact: ~15 core-path sibling sites beyond subprocess stderr (channel, chat_runner, artifacts, mcp_tools, memory) #7390 lands the repo-wide scan, delete the regex sweep in auto_improvement/tests/test_stderr_redact_before_bound.py and the module-scoped pins here — one scanner, one baseline, not three spellings of the same rule.

[FIRST-PRINCIPLES-REVIEWED] 87d0f27

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of 87d0f2798ba04ee0a3ad79c93f2bfa6c114a1e0d and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 87d0f27

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

[CrysisDeu]

🤖 Kiro Crew Auto-Pipeline [operator: CrysisDeu#0c98c3a2]

Disposition for First Principles CONCERNS (1 finding):

Finding: deferred sibling set is mostly core-path, not auto_improvement — ownership claim inaccurate. ACCEPTED — the lane is right. Verified the sibling grep against this checkout: channel.py:766, chat_runner.py:2627, handlers/artifacts.py:610, mcp_tools/control.py:541, vector_memory.py:1821 and the rest are real slice-inside-redact sites with no owner, and PR #7350 covers only the auto_improvement subset. Two corrective actions taken: (1) the PR body's Deferred follow-up and Pattern harvest sections have been rewritten to state the class location accurately, and (2) the core-path sibling set now has a named owner — follow-up issue #7390 lists every site the lane named, the fix shape (security.redact_and_truncate reorder), and the sweep-promotion condition. Scope of this PR is unchanged: the three subprocess-stderr sites named by issue #7374, which remain the highest-value members of the class (authenticated-git stderr is where a credential is most likely to appear).

[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 #7350 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 #7350: KEEP. Deliberate scope split recorded in this PR's own disposition; no file or line contention. Files: src/kiro_crew/dashboard/handlers/themes.py.
• This PR is OVERLAPPING with PR #5603. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #7383: KEEP. Disjoint file, same class; the broad title is not backed by coverage of themes.py or worktree_ops.py. Files: src/kiro_crew/cron_script.py.
• This PR is OVERLAPPING with PR #7142. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #7383: KEEP. Unrelated goals in the same file; the only interaction is possible context-line conflict for the later merge. Files: src/kiro_crew/apps/builtins/dev_fleet/worktree_ops.py. The two independent directions used different labels; the matrix conservatively retains OVERLAPPING for coordination.
• This PR is OVERLAPPING with PR #7316. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #7383: KEEP. Prior art that supplied the helper; it leaves this PR's three sites untouched. Files: src/kiro_crew/security.py.
• This PR is OVERLAPPING with PR #7424. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #7383: KEEP. The merged sibling swept a disjoint module set; this PR's three sites remain broken on main. Post-merge consolidation of the two AST scanners is a cleanup, not a blocker. Files: test/test_core_path_redact_before_bound.py.

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

[CrysisDeu]

Status: review-ready with two documented inherited reds

All checks owned by this PR are green on head f9f9cc5d6b3dcf05feacd4c541c7434b454be484 (52 SUCCESS), both blocking AI review lanes (GPT 5.6, Opus 4.8) and both advisory lanes (Design, First Principles) PASS on this head, and the PR is MERGEABLE with no unanswered review concerns. The two remaining failures are not caused by this diff:

1. Backend Tests (3.12, 3) + Coverage Gate cascade — main-inherited quadratic-regex regression.
test/test_security_regex_linearity.py::test_long_nonshell_line_does_not_blow_up fails identically on main's own CI (run 33824285706 at main head 5df44de2f, 6.08s vs this PR's merge ref 6.03–6.15s). This PR does not touch src/kiro_crew/security.py or that test. The fix is in flight upstream as #8282 (which superseded #8349); once it merges, a rebase will recompute the merge ref and this shard goes green.

2. Dependency Audit / Audit Production Dependencies — repo-wide npm registry outage.
npm audit has timed out after 120s on website/package-lock.json on 15 consecutive attempts since 00:21 UTC (fail-closed by design). Other open PRs (e.g. feat/panel-tab-registry-seam at 02:00 UTC) fail with the exact same error, and this PR does not modify any dependency file. Re-running the job once the registry recovers will clear it.

No action is needed on this branch for either failure; both clear from upstream/external recovery alone.

[CrysisDeu]

Disposition — First Principles CONCERNS (87d0f27)

Finding: two structural scanners for one defect class (regex sweep in auto_improvement/tests/test_stderr_redact_before_bound.py + the AST pin added here), with a repo-wide third planned in #7390.

Disposition: accepted-and-deferred to #7390 — convergence is the promotion step, not this PR.

Rationale:

• This PR's mandate (issue Slice-before-redact on subprocess stderr: 3 remaining sites outside auto_improvement (themes.py, dev_fleet server.py) #7374) is fixing the two remaining slice-before-redact sites outside auto_improvement and pinning those two modules so they cannot regress while the repo-wide harvest is pending. Deleting or refactoring the auto_improvement regex sweep here would be a rider: it touches an app owned by a different scope, and removing an existing guard in the same change that adds a new one briefly widens the unguarded window if this PR is reverted.
• The divergence the review counts (bound floor 10 vs multi-digit, alias handling) is exactly the evidence Slice-before-redact: ~15 core-path sibling sites beyond subprocess stderr (channel, chat_runner, artifacts, mcp_tools, memory) #7390 needs when choosing which scanner semantics to promote. Both stay in place as independent oracles until then.
• Committed at the promotion step (Slice-before-redact: ~15 core-path sibling sites beyond subprocess stderr (channel, chat_runner, artifacts, mcp_tools, memory) #7390): converge on ONE repo-wide scanner and delete both module-scoped pins — the regex sweep in auto_improvement/tests/test_stderr_redact_before_bound.py and the AST _find_slice_before_redact_offenders added here. I have noted this Subtraction on Slice-before-redact: ~15 core-path sibling sites beyond subprocess stderr (channel, chat_runner, artifacts, mcp_tools, memory) #7390 so the owner of that issue inherits the commitment.

No code change in this round; the concern is a sequencing decision, not a defect in the shipped diff.

[bolichen97]

All three sites now feed the redactor the full stderr and bound afterwards: themes._clone_github switches to security.redact_and_truncate(proc.stderr.strip(), 200) (which runs both redactors over the whole text then slices, and was that module's only user of the two removed imports), and the two worktree_ops sites move the [:300] / [-200:] outside runtime._redact(...), so a credential straddling the boundary can no longer survive as an unmatched fragment. The straddle tests are premise-guarded rather than incidental, and the AST pin sanctions a slice only on a redact call's result, which makes reverting either site go red.