Repository navigation
fix(guardrails): fail closed on unknown-root paths in ambiguous prose (#14110) - #14445
Merged
Merged
Conversation
…#14110) PR #13295 narrowed findUnquotedPathEnd's ambiguity fallback to failClosedAmbiguity only, but the redactUnquotedAbsolutePathSpans call site only set that flag for Windows/file-URI/known-POSIX-root candidates. An unquoted POSIX path whose root is not in POSIX_FILESYSTEM_ROOTS (e.g. /custom/internal in a container), followed by ambiguous prose, fell through unredacted -- regressing HR#12 (never leak err.message/paths). Per owner decision on #14110 (fail-closed), extend mustFailClosed in redactUnquotedAbsolutePathSpans to also cover an unknown-root POSIX candidate (isPosixPath) once findUnquotedPathEnd returns -1. This intentionally also fail-closes a bare, unanchored API route in prose (route- and filesystem- shaped candidates are lexically indistinguishable with no second anchor) -- the #13144 guarantee that a route immediately followed by another absolute- path span keeps its trailing prose is unaffected and re-asserted as a co-located regression test. Reactivates the #14110-SUSPENDED assertion in error-public-boundaries-hardening.fixture.ts and folds in the proven repro.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #14110
Root cause
PR #13295 changed
errorPathRedaction.ts's ambiguity branch fromfailClosedAmbiguity || hasFilesystemEvidencetofailClosedAmbiguityonly,and the call-site
mustFailClosedatredactUnquotedAbsolutePathSpansonlycovered Windows/file-URI/known-POSIX-root candidates. For an unquoted POSIX
path whose root is not in
POSIX_FILESYSTEM_ROOTS(e.g./custom/...,/data/...in a container) followed by ambiguous prose,findUnquotedPathEndreturned
-1and the fallback left the whole path in the sanitized errormessage in clear text instead of redacting it to
<path>— regressingHR#12 (never leak
err.message/paths).Owner decision
The issue explicitly framed this as non-delegable between (a) fail-closed on
unknown-root ambiguity, or (b) redact-token-preserve-prose with a stronger
route/path discriminator. Owner decision (2026-09-21, recorded in the
plan-file frontmatter): (a) fail-closed — extend
mustFailClosedinredactUnquotedAbsolutePathSpans(open-sse/utils/errorPathRedaction.ts) toalso cover an unknown-root POSIX candidate once
findUnquotedPathEndreturns-1.This intentionally also fail-closes a bare, unanchored API route in prose
(route-shaped and filesystem-shaped candidates are lexically indistinguishable
with no second anchor) — the accepted trade-off, since HR#12 makes a possible
path leak worse than over-redacting a route. The narrower #13144 guarantee —
a route immediately followed by another absolute-path span (e.g.
"... Use POST /v1/y instead.") keeps its trailing prose — is unaffected andre-asserted as a co-located regression test.
Fix
open-sse/utils/errorPathRedaction.ts:mustFailClosedinredactUnquotedAbsolutePathSpansnow also includesisPosixPath(thealready-computed unknown-root POSIX candidate flag), so an unresolved
ambiguous tail after such a candidate fails closed instead of falling through
to clear text.
Regression test
New:
tests/unit/issue-14110-unknown-root-path-exposed.test.tsRED (pre-fix,
git apply -Ron the fix):GREEN (post-fix): both assertions in that file pass — the unknown-root path is
redacted, and the #13144 trailing-prose guarantee is unaffected.
Also reactivated the
#14110-SUSPENDED assertion intests/unit/fixtures/error-public-boundaries-hardening.fixture.ts(added byPR #14101), which documents the exact same case inline.
Gates run
node --import tsx/esm --teston the touched/new files (30 assertions) — passnode --import tsx/esm --teston the full error-sanitization test family (12files, 121 assertions covering routes/tunnels/skills/MITM/provider-validation
error boundaries) — pass, no regressions
npm run typecheck:core— exit 0node scripts/check/check-open-sse-typecheck.mjs— 0 errors, within frozen baselinenpx eslint --suppressions-location config/quality/eslint-suppressions.json <changed files>— exit 0node scripts/check/check-file-size.mjs— OK, no file over its frozen capnode scripts/check/check-complexity-ratchets.mjs --base-ref origin/release/v3.8.51— OK, 0 new violations (base: 5 cyclomatic / 4 cognitive, unchanged)node scripts/check/check-changelog-integrity.mjs— OKExisting tests
Ran every test file referencing
errorPathRedaction/redactErrorPaths/sanitizeErrorMessagein the error-sanitization family. One existing test
(
tests/unit/error-path-redaction-route-truncation-13144.test.ts) had its"separator evidence alone does not license swallowing the line" assertion
updated: it encoded the pre-#14110 contract (a bare unanchored route in prose
survives verbatim), which the owner's fail-closed decision explicitly
supersedes. The sibling test in the same file (the actual #13144 regression
guard — a route followed by trailing prose with another absolute-path
anchor) was left untouched and still passes, so #13144 is not regressed.
Notes for the auditor
This PR resumes a previous worker's session that was cut mid-run by a usage
limit (no commits existed yet, work was uncommitted). I audited the diff it
left against the plan-file and the owner's binding decision before committing
it as-is — no scope changes were needed; the diff matched the plan exactly
(fail-closed via
isPosixPath, reactivated suspended assertion, new + alignedtests).
Plan-file:
_tasks/pipeline/bugs/2-implementing/14110-13295-exposes-unknown-root-filesystem-paths-in-clear-text-when.plan.md