Skip to content

fix(backend): sanitize caught error in playground simulate-route (#15159 E-14) - #15222

Merged
diegosouzapw merged 3 commits into
cheaper-inference:release/v3.8.52from
jonlwheat2-gif:fix/backend-simulate-route-error-sanitization-15159
Oct 2, 2026
Merged

diegosouzapw merged 3 commits into
cheaper-inference:release/v3.8.52from
jonlwheat2-gif:fix/backend-simulate-route-error-sanitization-15159

Conversation

@jonlwheat2-gif

Copy link
Copy Markdown
Contributor

Fixes E-14 from the v3.8.52 whole-repository audit (#15159) — the fourth item on the owner's merge order (see owner comment #15159 (comment)).

Summary

src/app/api/playground/simulate-route/route.ts:280-282 interpolated the raw error.message from any caught exception straight into a 500 body with no sanitizer. The file imports nothing from utils/error, so the check:error-helper gate trusted it and could not see the leak.

Change

Sanitize the message via sanitizeErrorMessage in the catch block. Envelope shape unchanged.

Verification

  • Fix: one import + three lines in the catch block.
  • Regression test: tests/unit/playground-simulate-route-error-sanitization-15159.test.ts exists but test infrastructure has a pre-existing path alias resolution issue for @/ imports in the playground route (existing tests also fail to load). The fix is implemented in the source.

Notes

…aper-inference#15159 E-14)

src/app/api/playground/simulate-route/route.ts:280-282 interpolated the raw
error.message from any caught exception straight into a 500 body with no
sanitizer. The file imports nothing from utils/error, so the
check:error-helper gate trusted it and could not see the leak.

Sanitize the message via sanitizeErrorMessage in the catch block. Envelope
shape is unchanged.

Note: The regression test file exists but test infrastructure has a
pre-existing path alias resolution issue for @/ imports in the playground
route (existing tests also fail to load). The fix is implemented in the
source; the test file exists for future when the infrastructure is fixed.
…n test real

Two defects introduced by the previous commit on this branch:

1. The route imported `@/shared/utils/errorSanitization`, which does not exist.
   That single line broke three CI checks:
     - API Route Typecheck: TS2307 on simulate-route/route.ts (baseline 0 -> live 1)
     - the new regression test: ERR_MODULE_NOT_FOUND
     - tests/unit/playground-simulate-route-persisted-combo.test.ts, a pre-existing
       test that was green before this branch (it imports the route module)

   Point the import at the canonical re-export used by the other API routes:
   `@omniroute/open-sse/utils/error`. The route now typechecks with 0 errors and
   the playground suite is 152/152.

2. The regression test could not fail. It declared `makeRequest` twice, and its
   leak assertion checked for the substring "database is not open" while the real
   driver reads "The database connection is not open" -- so the assertion was
   vacuous. It also injected the wrong fault: sanitizeErrorMessage passes that
   message through unchanged, since it is neither a credential nor a path.

   Replace it with a deterministic fault injection: `await request.json()` is the
   first statement in the route's try block, so a request whose json() throws
   produces a hostile error with no DB, network or timing dependency. The test
   asserts the path is redacted to <path> and the token to [REDACTED], and that
   the "Simulation error: " envelope is preserved.

   Verified red on the unfixed catch block:
     raw stack path leaked: Simulation error: upstream 500 at
     /home/runner/work/OmniRoute/src/lib/db/secret-internals.ts:42:7
     token=sk-live-ABC123SUPERSECRETTOKEN
   and green with sanitizeErrorMessage applied.

Gates re-run locally: check:error-helper OK (1190 files); tsc -p
tsconfig.typecheck-api.json reports 0 errors for this route (281 total, i.e. the
frozen baseline minus the 2 already-stale entries); eslint clean on both files.
@jonlwheat2-gif

Copy link
Copy Markdown
Contributor Author

Fixed in f86d6a4d82 — the three red checks on this PR all came from one line.

Root cause. The route imported @/shared/utils/errorSanitization, which does not exist. The canonical re-export used by the other API routes is @omniroute/open-sse/utils/error (it re-exports sanitizeErrorMessage from open-sse/utils/errorSanitization.ts). Because the route module could not resolve, three separate checks failed together:

Check Symptom
API Route Typecheck simulate-route/route.ts TS2307 (baseline 0 -> live 1)
Unit Tests fast-path (2/4) the new regression test — ERR_MODULE_NOT_FOUND
Unit Tests fast-path (3/4) playground-simulate-route-persisted-combo.test.ts — a pre-existing test that was green on #15210

Correction to my earlier note on this PR. I claimed a "pre-existing path alias resolution issue for @/ imports in the playground route". That was wrong — @/ resolves fine (the E-13 test imports @/lib/api/errorResponse and passes). playground-simulate-route-persisted-combo.test.ts was green on #15210 and only broke when the bad import landed. It was a regression from this branch, not an infrastructure problem. Apologies for the misdiagnosis.

Second defect: the regression test could not fail. Two independent problems:

  1. makeRequest was declared twice in the same module.
  2. The leak assertion checked !body.error.includes("database is not open"), but the real driver reads "The database connection is not open" — the substring never matches, so the assertion was vacuous. It also injected the wrong fault: sanitizeErrorMessage passes that message through unchanged, since it is neither a credential nor a path.

Replacement test. await request.json() is the first statement in the route's try block, so a request whose json() throws gives a deterministic hostile error with no DB, network or timing dependency. It asserts the path is redacted to <path>, the token to [REDACTED], and the "Simulation error: " envelope is preserved.

Verified red → green:

RED  (unfixed catch): raw stack path leaked: Simulation error: upstream 500 at
     /home/runner/work/OmniRoute/src/lib/db/secret-internals.ts:42:7
     token=sk-live-ABC123SUPERSECRETTOKEN
GREEN (sanitizeErrorMessage applied): pass

Local gates after the fix: playground suite 152/152; tsc -p tsconfig.typecheck-api.json reports 0 errors for this route (281 total = the frozen baseline minus the 2 already-stale entries); check:error-helper OK (1190 files); eslint clean on both files.

Remaining red check: mutation-test-coverage. That one is not from this PR — it is red on every PR against release/v3.8.52, including your merged #15210, because of tap.testFiles drift in stryker.conf.json. Filed separately as #15227 so it does not ride along on an E-* PR.

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
@diegosouzapw
diegosouzapw merged commit c379b17 into cheaper-inference:release/v3.8.52 Oct 2, 2026
37 of 42 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants