fix(security): stop error-path redaction from swallowing a shielded route hint (#6457)

#12506's unquoted-path ambiguity resolution failed closed across the
*entire remainder* of a message once it saw an unshielded route-looking
span (no HTTP-method context) followed by ambiguous prose and then a
second, legitimately route-shielded reference (e.g. "POST /v1/foo").
resolveEndpoint() only distinguished "ambiguous" from "resolved", so
hitting that second absolute-looking span always re-triggered the
fail-closed value.length branch via hasFilesystemEvidence, deleting
the shielded route and everything after it instead of just redacting
the first, unshielded span.

This broke POST /v1/chat/completions' image-only-model guard (#6457):
its 400 body's second sentence ("Use POST /v1/images/generations
instead.") was being erased by sanitizeErrorMessage(), so
tests/unit/chat-rejects-image-only-model.test.ts failed release-green
on release/v3.8.51.

resolveEndpoint() now takes an ignoreAmbiguity flag; when the next
absolute-looking span is itself route-context-shielded per the
existing hasRouteContextBefore() check, its mere presence no longer
forces a value.length swallow. Added a direct regression test in
tests/unit/error-message-sanitization.test.ts pinning the exact
pattern down at the sanitizeErrorMessage level.
This commit is contained in:
diegosouzapw
2026-09-07 09:01:51 -03:00
parent de9a71f858
commit ff16cd034b
2 changed files with 23 additions and 3 deletions

View File

@@ -464,8 +464,8 @@ function findUnquotedPathEnd(
let hasFilesystemEvidence = false;
let hasUnresolvedFragments = false;
const resolveEndpoint = (): number => {
if (hasUnresolvedFragments) {
const resolveEndpoint = (ignoreAmbiguity = false): number => {
if (hasUnresolvedFragments && !ignoreAmbiguity) {
return failClosedAmbiguity || hasFilesystemEvidence ? value.length : -1;
}
if (resolvedExtensionEnd >= 0) return resolvedExtensionEnd;
@@ -530,7 +530,12 @@ function findUnquotedPathEnd(
while (nextTokenStart < value.length && isWhitespace(value[nextTokenStart])) nextTokenStart++;
if (nextTokenStart >= value.length) return resolveEndpoint();
if (isSyntacticallyAbsolutePathAt(value, nextTokenStart)) {
const endpoint = resolveEndpoint();
// A route-shielded upcoming span (e.g. "POST /v1/foo") is never
// filesystem-sensitive by design — see hasRouteContextBefore. Its mere
// presence must not force ambiguous prose in between (like "Use POST")
// to fail closed and swallow past it into the shielded route and
// beyond; resolve with whatever evidence was already gathered instead.
const endpoint = resolveEndpoint(hasRouteContextBefore(value, nextTokenStart));
if (endpoint >= 0) return endpoint;
return acceptEndpointBeforeAnotherAbsolute ? lastPathTokenEnd : -1;
}

View File

@@ -239,6 +239,21 @@ test("sanitizeErrorMessage replaces absolute paths with <path>", async () => {
assert.ok(out2.includes("<path>"));
});
test("sanitizeErrorMessage does not swallow a shielded route hint that follows an earlier redacted path (#6457)", async () => {
// Regression: an unshielded route-looking span ("on /v1/chat/completions")
// followed by ambiguous prose ("Use POST") used to make the unquoted-path
// scanner fail closed all the way to the end of the string, deleting a
// second, legitimately-shielded route reference ("POST /v1/images/...")
// and everything after it instead of just redacting the first span.
const { sanitizeErrorMessage } = await import("../../open-sse/utils/error.ts");
const input =
"Model 'x' is an image-generation model and cannot be used on /v1/chat/completions. Use POST /v1/images/generations instead.";
const out = sanitizeErrorMessage(input);
assert.match(out, /\/v1\/images\/generations/, "shielded route hint must survive");
assert.match(out, /instead\.$/, "text after the shielded route hint must not be dropped");
assert.ok(out.includes("<path>"), "the earlier unshielded route span is still redacted");
});
test("sanitizeErrorMessage handles non-string inputs safely", async () => {
const { sanitizeErrorMessage } = await import("../../open-sse/utils/error.ts");
assert.equal(sanitizeErrorMessage(undefined), "");