Redact pip/ensurepip stderr before bounding in gateway log warnings

[bolichen97]

Summary

Fixes the unredacted subprocess-stderr logging in src/kiro_crew/dashboard/handlers/memory.py reported in issue #7279. Both pip/ensurepip failure sites logged stderr.decode()[:500] with no redaction, unlike every neighbouring hardened subprocess path. On the faiss-cpu install site this can write a raw private-index-URL credential (e.g. https://<user>:<token>@host/simple) into the gateway log when the index answers 401, because pip's warn_on_401 logs the request URL raw.

Changes

• src/kiro_crew/dashboard/handlers/memory.py

  • Added redact_and_truncate to the existing from kiro_crew.security import ... import.
  • Site ~1048 (faiss-cpu install failure): stderr.decode()[:500] -> redact_and_truncate(stderr.decode(), max_chars=500).
  • Site ~897 (ensurepip bootstrap failure): same conversion.
  • Both now redact the FULL decoded stderr before the 500-char slice, matching the invariant documented on security.redact_and_truncate and the sibling cron paths (cron: apply #4402's tail-slice + redact-before-truncate pattern to run_command_sandboxed and the stdout diagnostic #5547 / fix: redact cron stderr/stdout before truncating, report stderr tail #5574). This prevents a credential straddling the 500-char boundary from surviving as an unredacted fragment.

• test/test_enable_embeddings_faiss.py (new class TestSubprocessStderrRedactedBeforeBound)

  • faiss site: a pip warn_on_401 stderr line carrying https://svc:SECRET123@pypi.invalid/... is asserted redacted in the captured logger.warning record.
  • Boundary-straddle: a secret positioned to start at offset 495 and end at 504 (genuinely straddling the 500-char cut) is asserted not to survive as any SECRET fragment.
  • ensurepip site: URL-shaped secret scrubbed and stderr portion bounded to <=500 chars.

Per the issue's scope note, site 897 has no index-URL credential vector (ensurepip installs from bundled wheels and consults no index); it is converted for consistency of shape on the same code path.

Testing

• Both edited files compile (py_compile OK).
• The core redaction behavior was verified directly against the real kiro_crew.security.redact_and_truncate (importable without heavy deps): the 401-URL case strips the token, and the boundary-straddle case (secret spanning offset 495-504) yields output with no surviving SECRET fragment.

Environment limitation

The full pytest suite for these handler tests imports aiohttp and other runtime deps that are not installed in the sandbox, and the session network mode is 'Repository access only' (INTEGRATIONS_ONLY), which blocks installing them from PyPI. The redaction logic (the substance of the fix) was validated directly since security.redact_and_truncate is importable, but the aiohttp-based HTTP integration wrappers in the new tests could not be executed in-process. Please run pytest test/test_enable_embeddings_faiss.py in an environment with dependencies installed to confirm the wired-up tests pass.

Closes #7279

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed 5015796abc5d86d92e9c793a602d036227722ced — this comment is updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] 5015796

Verdict parsed from the review's SHA-scoped output markers for commit 5015796abc5d86d92e9c793a602d036227722ced.

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

[github-actions]

Design Review (Fable 5) — ✅ PASS

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

The diff is small and targeted: two log sites in memory.py now redact pip/ensurepip stderr before bounding, with tests pinning the straddle case. The fix reuses the existing redact_and_truncate utility (which documents exactly this redact-before-bound invariant), addresses the root cause rather than the symptom, and I found no remaining stderr.decode()[:N] sites for pip-adjacent subprocesses — the two other decode(...)[: N] sites in the repo (voice_reply.py, slack/gateway.py) are non-pip subprocesses without index-URL credentials in their stderr.

Design-Verdict: PASS

Reuses the existing redact-before-bound utility at the exact leak sites, root-cause fix with boundary-straddle tests; no design-level concerns.

Suggestions

• voice_reply.py:364 and slack/gateway.py:9640 still truncate-before-redact subprocess stderr; lower-risk streams, but a follow-up sweeping them onto redact_and_truncate would retire the pattern entirely.

[DESIGN-REVIEWED] 5015796

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of 5015796abc5d86d92e9c793a602d036227722ced and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 5015796

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

[github-actions]

First Principles Review (Fable 5) — 🟡 CONCERNS

Premise-level review of 5015796abc5d86d92e9c793a602d036227722ced — 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 checks are done. The fix uses the existing redact_and_truncate helper (security.py:11600) correctly, but I found one unfixed sibling with the identical vulnerability: the pip fallback in slack/gateway.py:9640 logs pip stderr unredacted, while two other sites in that same file already redact-before-bound.

First-Principles-Verdict: CONCERNS

The fix is real and uses the existing helper, but the same pip-401 credential leak survives unfixed at slack/gateway.py:9640.

What this change ships

Intent: stop pip/ensurepip subprocess stderr from writing private-index credentials into the gateway log (issue #7279) — a FIX.

1. faiss-cpu install failure log now redacts stderr before the 500-char bound — justified (reported defect; untrusted command output).
2. ensurepip failure log gets the same conversion — justified (named in the issue; declared as vectorless hardening of the same path).
3. Three new tests pin redaction, boundary-straddle, and bounding — justified (they pin the invariant the fix restores).

No undeclared items; the diff adds no new public surface, and it correctly reuses security.redact_and_truncate instead of a third hand-rolled spelling.

Watch

• Point patch with one counted unfixed sibling. Grepped \.decode\([^)]*\)\[: under src/kiro_crew/: 2 hits. One is slack/gateway.py:9640, where the core-dep-repair fallback logs fb_err.decode(errors="replace")[:300] — a pip subprocess with exactly the root cause this PR names ("pip's warn_on_401 logs the request URL raw"), in a file that already redact-before-bounds twice (gateway.py:2321-2324, 9890-9893). The description never says this is left; the same three-line conversion closes it. (The other hit, voice_reply.py:364, is piper stderr — no index-URL vector, no action needed.)

[FIRST-PRINCIPLES-REVIEWED] 5015796

[bolichen97]

Closing as superseded by #7283.

Both PRs make the same fix — route pip/ensurepip stderr through redact_and_truncate so
redaction runs over the full decoded stderr before the log bound is applied, instead of
stderr.decode()[:500] truncating first and leaving a credential fragment unredacted. They
touch the same two files and the same two call sites (_ensure_pip_available and the
faiss-cpu install arm of api_memory_enable_embeddings).

#7283 is strictly better on two counts:

• It decodes with errors="replace". This one calls a bare stderr.decode(), which raises
  UnicodeDecodeError on non-UTF-8 pip stderr — i.e. it can turn a logged install failure
  into an unhandled exception on the very path it is hardening.
• It names the bound (_PIP_STDERR_LOG_CHARS = 500) with a comment recording the
  redact-before-bound invariant, rather than repeating a bare max_chars=500 at both sites.
  AGENTS.md's code-style rule is that every limit has an owning module, not a literal in
  business logic.

#7283 is also already fully green on every lane. Two further items would have to be fixed here
for no gain: the title is not conventional-commits shaped, and the Inclusive Language gate
fails on an added line containing "sanity".

Tracking the work on #7283.