fix(memory): redact pip stderr before logging install failures (#7279)

[bolichen97]

Summary

handlers/memory.py logged pip/ensurepip subprocess stderr into the gateway log unredacted (stderr.decode()[:500]) at two warning sites. On the faiss-cpu install path the child pip inherits the gateway environment, so a private index configured with userinfo credentials reaches pip, and on an auth failure pip writes the raw request URL — including the token — to stderr, which then landed verbatim in the gateway log.

Both sites now route the full decoded stderr through security.redact_and_truncate before logging:

• Redaction runs over the complete text before the length bound (the redact-before-bound invariant from fix(cron): report the terminal stderr, not the leading one #4402 / fix: redact cron stderr/stdout before truncating, report stderr tail #5574), so a credential straddling the old 500-char slice boundary cannot survive as an unredacted fragment.
• The log budget is kept at 500 chars via a named constant _PIP_STDERR_LOG_CHARS.
• stderr.decode(errors="replace") on both touched lines so a non-UTF-8 pip stderr (Windows codepage, mangled index response) cannot raise UnicodeDecodeError on the failure path and wedge _embedding_setup_status at installing_faiss (accepted pre-push Opus advisory; the site's own comments document that wedge class).
• The ensurepip site (no index-URL vector — ensurepip never consults an index) is fixed for shape consistency on the same code path.

Out of scope: the head-vs-tail anchor policy for which 500 chars to keep is tracked in #5555; this PR only introduces the missing redaction.

Tests

test/test_enable_embeddings_faiss.py::TestPipStderrRedaction (4 new tests, all mutation-verified red against the raw-slice code):

• faiss-cpu install failure with a userinfo credential in stderr → logged warning contains [REDACTED: credential], raw token absent (end-to-end through the handler, subprocess mocked).
• ensurepip bootstrap failure, same assertion, direct _ensure_pip_available path.
• credential planted to straddle the 500-char bound → no prefix of the token (3..14 chars) appears in the log; the old head-slice leaked one, and a truncate-then-redact mutation also fails this test because the sliced text loses the @ the userinfo pattern requires.
• non-UTF-8 stderr bytes → no UnicodeDecodeError, warning still logged and redacted.

Local gates (CI-pinned tools): black-baseline, isort, flake8, mypy (1216 files clean), brand-name, harness-parity, subprocess-encoding — all green. Targeted pytest: 23 passed.

Pre-push review dispositions

• GPT lane (gpt-5.6-sol hollow-returned; gpt-5.6-terra fallback): NO BLOCKING FINDINGS, evidence-backed.
• Opus lane (claude-opus-5): NO BLOCKING. Advisory 1 (strict decode() crash on the failure path) accepted and fixed (errors="replace" + regression test). Advisory 2 (redaction now scans the whole stderr buffer, O(n·matches) CPU) no action in-diff: the shape is shared by every existing redact_and_truncate caller, create_subprocess_limited already rlimits the child, and pre-capping the input would re-introduce a bound ahead of redaction — the exact ordering this fix removes.

Closes #7279

Pattern harvest

Rule candidate: semgrep
Pattern: subprocess stderr reaching a logging call without redaction — logger.<level>(..., stderr.decode(...)) (or a sliced variant) not routed through security.redact_and_truncate. This PR fixes two sites; unredacted siblings of the same shape exist elsewhere (e.g. voice_reply.py:364, chat_voice.py:360, apps/routes.py:3236) with lower credential exposure than the pip/index path but the identical mechanism, so a lint that flags stderr.decode flowing into a log call without the redaction shim would catch the class at review time.

Supersedes #7292, which made the same fix but decoded with a bare stderr.decode()
(raises on non-UTF-8 pip stderr) and repeated a bare max_chars=500 at both call sites.

[github-actions]

Design Review (Fable 5) — ✅ PASS

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

Design-Verdict: PASS

Real leak, fixed at the root with the existing shared redact_and_truncate helper — redact-before-bound is the correct ordering, tests pin it.

[DESIGN-REVIEWED] d0c0d82

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

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

Review details

The change routes pip/ensurepip stderr through redact_and_truncate, which I confirmed redacts over the full text before slicing (redact_credentials(redact_exfiltration_urls(text)[0])[0][:max_chars] at security.py:11609). The decode(errors="replace") removes a pre-existing crash risk rather than adding one, and error responses to the user use static strings. No reachable defect on the changed lines.

No findings.

[OPUS-REVIEWED] d0c0d82

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

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

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

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

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] d0c0d82

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

[github-actions]

First Principles Review (Fable 5) — 🟡 CONCERNS

Premise-level review of d0c0d825f483b7f26579ff6202d2762b959f2a44 — 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 evidence gathered. Composing the review.

First-Principles-Verdict: CONCERNS

The fix is real and uses the existing redact_and_truncate mechanism — but one counted sibling pip-stderr leak stays unfixed.

What this change ships

Intent: stop pip's private-index credential landing in the gateway log when a faiss/ensurepip install fails — a FIX.

1. faiss-cpu install-failure log now masks credentials in pip stderr — justified (the reported defect, Unredacted pip stderr in gateway log can leak private-index credentials (handlers/memory.py:1048, :897) #7279).
2. ensurepip bootstrap-failure log gets the same masking — declared; tool output is untrusted external content, boundary-derived.
3. Garbled (non-UTF-8) pip stderr no longer crashes the failure path and wedges enable-embeddings — declared, real crash class.
4. Redaction now scans the full stderr before the 500-char bound — justified; matches redact_and_truncate's documented redact-before-bound invariant (security.py:11600).
5. New constant _PIP_STDERR_LOG_CHARS — 2 real consumers (memory.py:905, 1060); mandated by the code-style "every limit has an owning module" rule.

Watch

• One unfixed sibling of the named root cause (pip inherits gateway env; auth failure echoes the credentialed index URL to stderr). Grepped stderr\.decode across src/ (17 hits) and read every pip-install site: memory.py is fixed here, dep_sync.py's callers redact by design (updates.py:1356), but apps/builtins/auto_improvement/backend/deps.py:113-114 returns pip stderr's tail line sliced [:200] — unredacted AND bound-before-redact, the exact pattern this PR removes. The general fix is the same one-line call; the description's out-of-scope note (Head-anchored stderr bounding siblings: command-cron path (cron_script.py:932) + ~25 repo-wide candidates #5555) does not cover it.

[FIRST-PRINCIPLES-REVIEWED] d0c0d82

[bolichen97]

Disposition — First Principles CONCERNS: accepted-and-deferred, sibling extracted to a tracked issue.

• Unredacted pip stderr tail in auto_improvement/backend/deps.py — verified at
  apps/builtins/auto_improvement/backend/deps.py:113-114, which returns pip stderr's tail line
  [:200] unredacted in the handler's error payload. Same env-inherited pip mechanism as the two
  sites this PR fixes, and bound-before-redact on top of it.

tail = (proc.stderr or "").strip().splitlines()[-1:] or [""]
return {"ok": False, "installed": [], "error": f"pip failed: {tail[0][:200]}"}

Filed as #7307 with the lane's closed enumeration (17 stderr\.decode hits audited; deps.py is
the only remaining pip-stderr site — it evades that grep via text=True) and the same one-line fix
shape. Kept out of this PR because the dispatched scope for #7279 is the two handlers/memory.py
sites plus their tests: deps.py is a different module and a different surface (a returned payload
rendered in the dashboard, not a log line), so it gets its own minimal PR rather than widening this
one.

Thanks to the lane for the genuine remainder — the enumeration is what made it fileable as a
bounded issue.