ducoterra
f4150421bb
**Phase 84 — final verification pass: all green, no defects found** - Verified implementation: `app/core/errors.py` (verbatim lift of sync masker), `app/api/sync.py` alias import, docs-push 502 `detail=sanitize_error(str(exc))`, all five `llm.py` error sites sanitized; new/extended test pins in place - Tests: `uv run pytest` → **1714 passed, 0 failed**; targeted pins (new unit ×2 + integration ×1, existing 502 pin) → 13 passed; sync/git-sources regression → 67 passed - Coverage: `uv run pytest --cov=app --cov-report=term-missing` → **99%** (`app/core/errors.py` 100%, `app/rag/llm.py` 100%) — >90% met - E2E isolation: `uv run pytest tests/e2e/test_smoke.py -v --no-cov` → **3 passed** - Lint/types: `uv run ruff check .` → clean; `uv run pyright` → **0 errors** - Criteria: 502 masks `*****@`/never token + row untouched ✅; LLM base-URL masked, credential-free strings byte-identical ✅; `_CREDS_RE` only in `app/core/errors.py` (working-tree grep) ✅; full gate green ✅; `git diff --stat` limited to the 4 app files + 2 modified test files + 3 phase task files (untracked: new module, new unit test, complete/ dir, reports, audit plan) ✅ - Commit/phase move left to the harness per instructions (task files already in `complete/`) - No deviations; nothing to fix - Next pending phase: **85_mobile_menu_gate_overlap**
58 lines
2.7 KiB
Python
58 lines
2.7 KiB
Python
"""Shared error-string sanitizer (phase 84 — audit SEC-08 / SEC-13).
|
|
|
|
``sanitize_error`` masks ``user:pass@`` userinfo in an error string so
|
|
no error surface ever ships an embedded credential into the UI, a
|
|
response body, or an SSE frame. It is the verbatim lift of the phase-32
|
|
sync masker (same regex, same ``*****@`` replacement — byte-identical
|
|
behavior); ``app/api/sync.py`` keeps the private name ``_sanitize_error``
|
|
as an alias import, and the docs-push 502 detail (SEC-08) and the LLM
|
|
error messages (SEC-13) run through the same function.
|
|
|
|
Audit basis (``.agents/remediation_plan.md``, security audit 2026-09-07):
|
|
|
|
* SEC-08 (Medium) — the docs-push 502 surfaced git's stderr verbatim;
|
|
``BOR_DOCS_REPO`` is documented as "any remote (URL or local path)",
|
|
so an ``https://user:token@host/...`` URL is a normal config shape,
|
|
and a failed push (revoked token, network) echoes the remote URL in
|
|
git's stderr straight into the browser and the logs. The sync path
|
|
already solved exactly this problem — the docs-push and LLM surfaces
|
|
simply never got the treatment.
|
|
* SEC-13 (Low) — the LLM error f-strings interpolated
|
|
``settings.llm_base_url`` raw; a base URL configured with embedded
|
|
credentials would echo into SSE ``error`` frames and logs. Same class
|
|
of leak, same fix.
|
|
|
|
Contract (narrow userinfo-regex only — byte-identical for
|
|
credential-free text):
|
|
|
|
* Only the git/HTTP ``user:pass@`` userinfo shape is rewritten — a run
|
|
of userinfo characters, the separating colon, a second run, and the
|
|
``@``. Ordinary text survives character for character: plain hosts,
|
|
``https://host/...`` URLs without userinfo, prose with a colon +
|
|
space (``fatal: ...``), emails in prose (no colon userinfo run
|
|
before the ``@``). The existing sync / git-sources / LLM error-string
|
|
tests are the tripwire proving the move is behavior-identical.
|
|
* The replacement shape is ``*****@`` — the existing sync copy, so
|
|
every already-pinned masked error stays byte-identical.
|
|
* Idempotent for the masked forms the surfaces produce (sanitizing an
|
|
already-sanitized git/URL failure string is a no-op).
|
|
"""
|
|
from __future__ import annotations
|
|
|
|
import re
|
|
|
|
#: ``user:pass@`` inside any error text (git stderr, endpoint URLs) —
|
|
#: the narrow userinfo run (git/HTTP convention) that carries
|
|
#: credentials; everything else is left untouched.
|
|
_CREDS_RE = re.compile(r"[A-Za-z0-9._~%*-]+:[A-Za-z0-9._~%*-]+@")
|
|
|
|
|
|
def sanitize_error(message: str) -> str:
|
|
"""Mask credentials embedded in an error string (no secrets in the UI).
|
|
|
|
Git's stderr is otherwise surfaced verbatim (the admin needs the
|
|
failing repo and git's reason to fix things) — only the ``user:pass@``
|
|
userinfo shape is rewritten, to ``*****@``.
|
|
"""
|
|
return _CREDS_RE.sub("*****@", message)
|