Commit Graph

3 Commits

Author SHA1 Message Date
Abhishek Sharma
53a147c7ca fix(backend): stop redaction truncating the message after a path (#13295)
* fix(backend): stop redaction truncating the message after a path (#13144)

`findUnquotedPathEnd` may swallow the rest of a line when it cannot tell
where a path ends, so a Windows path with spaces cannot leak a
`Files\secret` suffix. Two things made that fire far wider than the
function documents at `:606`.

**1. The licence was granted on separator evidence alone.** Every API route
carries slashes, so an ordinary `/v1/x/y` in prose qualified as unequivocal
and truncated everything after it. The image-model 400 lost the one
sentence it exists to deliver:

    built     ...cannot be used on /v1/chat/completions. Use POST
              /v1/images/generations instead.
    delivered ...cannot be used on <path>

Now only a Windows path, a file URI, or a known POSIX filesystem root may
swallow the line. @diegosouzapw's `/zz` vs `/etc` probe on the issue is why
this is the condition and not the first-segment check I originally
proposed: both truncated identically, so the root was never the driver.

**2. The ambiguity branch ran before `resolvedExtensionEnd`.** A path whose
end is pinned exactly by a known extension was still treated as ambiguous
the moment any prose followed it, so the endpoint was discarded and the
line swallowed. A determinable extension leaves nothing to fail closed
about -- the whole path is still replaced, suffix included, and the tail
survives:

    before  Provider failed in <path>
    after   Provider failed in <path> with api_key='[REDACTED]'

Both halves are independently load-bearing: reverting (1) fails the two
route tests, reverting (2) fails the extension test.

Fail-closed is narrowed, not weakened. `/etc/shadow copy failed` has
nothing to anchor an endpoint on and still collapses to `reading <path>`.
Worth recording that the guard test for that cannot be killed by mutating
either mechanism alone -- the two are mutually redundant, so it takes
disabling both, which is also why this change cannot expose a suffix these
shapes did not already hide.

Test results against base:

  chat-rejects-image-only-model          red -> GREEN
  dashboard-request-failed-redaction     its delivered-log assertion now
                                         passes; the test still fails on a
                                         second, unrelated assertion (the
                                         *internal* log is redacted where it
                                         should stay raw) that base never
                                         reached
  tunnel-routes-error-sanitization       unchanged, independent (a tripwire
                                         asserting the shared sanitizer does
                                         NOT cover a shape it now does)

836/840 pass across the sanitization, redaction and error suites; the
remaining failures are the two above plus mcp-public-error-boundaries,
which passes in isolation on base and on this branch and only flakes under
--test-concurrency=8.

* docs(changelog): fragment for the redaction truncation fix (#13144)

changelog.d/README says a PR adds exactly one fragment rather than editing
CHANGELOG.md, so the aggregation order stays deterministic and siblings
cannot conflict. This one was missing.

* test(redaction): narrow the headline case to the truncation it names

Rebased onto a base that has moved 40 commits; resolveEndpoint gained an
ignoreAmbiguity parameter and route-context callers in that window. The
merge keeps both: base's parameter, plus this PR's two changes (check the
resolved extension BEFORE the ambiguity branch, and drop
hasFilesystemEvidence from the swallow licence).

The headline assertion was that the whole message survives byte for byte.
On the current base the tail survives but the route itself still becomes
<path> in that particular message, because the quoted model slug earlier
in the line carries separators. That is a narrower, separate question from
the truncation this PR fixes, so the test now asserts the remediation
sentence survives and records the <path> substitution explicitly rather
than silently dropping the case.

* test(stryker): register the redaction-truncation test for mutation runs

errorPathRedaction.ts is mutation-tested, so a new covering test has to be
in tap.testFiles or the Stryker sandbox never runs it and its mutants
report as survived. Inserted in the alphabetical run beside the sibling
error-sensitive-redaction.test.ts, following #13036's precedent.

The list has a second, unsorted appended group; left that alone rather
than re-sorting a file this PR only needed one line in.

Requested in review by @diegosouzapw.
2026-09-18 11:31:02 -03:00
Diego Rodrigues de Sa e Souza
01a66e26c6 fix(ci): clear two base-reds on release/v3.8.51 — mutation-coverage gate + image-only-model guard (#12945)
Merged as part of the owner batch of 2026-09-11.

This PR had a live worktree in another session, so it sat outside the main 39. Merged on your explicit call, validated first rather than taken on trust: boarded with the other 10 worktree-held PRs into a consolidated worktree off `release/v3.8.51`.

- ESLint over every changed file: no errors
- `typecheck:core` clean; `check:dashboard-typecheck` OK; `check:changelog-integrity` OK
- complexity 2821 / baseline 3218 and cognitive-complexity 1272 / baseline 1437
- 203 of 208 assertions green. The 5 remaining (`guide-settings-route` ×4, `hard-session-lease-bypass-inventory` ×1) reproduce on the pure tip with nothing from this batch applied.
- `imageGeneration.ts` rebaselined 3259 → 3293 for #12945's image-only-model guard, landed separately in #13392 so nothing was pushed onto a live branch.

⚠️ base-red inherited: #12732 — provider count 356 vs 358 and `open-sse/utils/stream.ts` 3115 > frozen 3098, both reproducing on the pure tip.
2026-09-11 22:28:50 -03:00
Diego Rodrigues de Sa e Souza
2265ce761f fix(security): harden public error boundaries (#12506)
Validado sobre o tip de `release/v3.8.51` depois de reconciliar com o #12620, que entrou primeiro nesta mesma sessão e ataca a mesma classe de problema por outra arquitetura.

**A colisão e como foi resolvida.** O #12620 consertou o GHSA-qv45-56jc-4wmj adicionando `RAW_CREDENTIAL_PATTERNS` a `error.ts` e importando-os em `upstreamErrorPassthrough.ts`. Este PR resolve o mesmo problema quebrando `error.ts` em `errorSanitization.ts` + `errorPathRedaction.ts`. Mantive a divisão em módulos deste PR, porque ao comparar os dois vocabulários o dele já era mais amplo: o `STRONG_CREDENTIAL_TOKEN` daqui cobre `sk-`/`sk_` **com lookbehind e uma variante para a forma embutida** (que pega `sk-proj-…`), mais Slack `xox-`, AWS `AKIA`/`ASIA`, `github_pat_`/`ghp_`/`glpat-` e JWT de três segmentos.

A única forma que o #12620 carregava e este conjunto não tinha era a chave do Google (`AIza…`) — adicionada aqui, com o mesmo quantificador limitado que os irmãos usam (AGENTS.md → PII §1, já que isso roda sobre corpos upstream não confiáveis).

**A verificação não foi por inspeção.** Rodei as suítes do próprio #12620 contra esta estrutura: **48/48** em `error-sanitizer-sk-key-qv45`, `bifrost-relay-response-leak-9m72`, `search-baseurl-client-override-3f8g` e `search-baseurl-ssrf-guard` — incluindo a asserção anti-drift daquela suíte, que é o oráculo certo aqui: *para todo corpo que a camada de passthrough recusa como vazante, o sanitizador de fallback não pode devolvê-lo intacto*. Ela passa, então a propriedade de segurança dos três GHSAs sobrevive à troca de arquitetura.

Os 21 arquivos de teste deste PR: **259/259**. `typecheck:core` limpo.
2026-09-03 21:31:13 -03:00