Skip to content

fix(core): keep the HTTP status of client-safe errors (expired token → 401, not 500) - #727

Merged
d-klotz merged 1 commit into
nextfrom
fix/client-safe-error-status
Oct 26, 2026
Merged

d-klotz merged 1 commit into
nextfrom
fix/client-safe-error-status

Conversation

@d-klotz

@d-klotz d-klotz commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Problem

An expired session token on GET /api/integrations (seen on 2.0.0-next.120 and next.125) answers 500 {"error":"Internal Server Error"} instead of 401. The token repository throws ClientSafeError('Invalid Token: Token is expired', 401), but the express boundary in handlers/app-handler-helpers.js called Boom.boomify(err) without a statusCode, so every non-Boom error, including every ClientSafeError, became 500.

A malformed bearer token had the same outcome: getJSONTokenFromBase64BufferToken threw a SyntaxError from JSON.parse (or a TypeError later for null/{} payloads) → 500.

Fix

  • handlers/app-handler-helpers.js: boomify with { statusCode } only when err.isClientSafe === true and the status is an integer 400–599. Everything else still answers 500 with Internal Server Error. FetchError carries an upstream third-party status and is not client-safe, so it stays 500. A 5xx ClientSafeError keeps its status but its message stays hidden.
  • Mongo/Postgres/DocumentDB token repositories: a token that isn't base64 JSON with an id and token throws ClientSafeError('Invalid Token: Token is malformed', 401).

This brings the express boundary in line with core/create-handler.js, which already returns the status of client-safe errors for non-express Lambdas.

Not changed

  • core/create-handler.js uses error.statusCode || 400 with no range check, and a 5xx ClientSafeError there returns its message. This predates the PR.
  • handlers/routers/db-migration{,.handler}.js keep their own boundaries that always answer 500 (admin only).
  • The legacy token/repositories/token-repository.js (exported, not used by the user repositories) still uses raw JSON.parse.
  • Body-parser errors (statusCode 400, expose: true) still answer 500, because they are not ClientSafeError.

Test plan

  • Expired token ClientSafeError → 401 { error: 'Invalid Token: Token is expired' }, one WARN request_rejected, no request_failed
  • ClientSafeError 403/404 keep their status and message
  • ClientSafeError 503 → 503 Internal Server Error
  • ClientSafeError with statusCode 200/302/600/'401'/'nope' → 500 Internal Server Error
  • FetchError with an upstream 401 → 500
  • Plain Error → 500 with no message leak; Boom errors unchanged (existing tests)
  • Malformed tokens (non-JSON, null, number, {}, missing token) → 401 ClientSafeError on all three repositories; a created token still round-trips
  • npx jest in packages/core: same 7 failing suites as next (env/pre-existing), no new failures

🤖 Generated with Claude Code


Note

Medium Risk
Touches authentication error handling and HTTP status mapping at the Express boundary; behavior change is intentional but affects all Express routes and token parsing.

Overview
Fixes Express HTTP handlers returning 500 for auth failures that should be 401 (and other client-safe 4xx), by teaching the error middleware to preserve statusCode when err.isClientSafe is true and the code is a valid HTTP error (400–599).

The Express boundary in app-handler-helpers.js now uses a toBoom helper instead of plain Boom.boomify(err). 4xx ClientSafeError responses include the safe message and log request_rejected; 5xx client-safe errors keep the status but still return Internal Server Error. Invalid or non-integer status codes and non-client-safe errors (e.g. FetchError with an upstream 401) still answer 500.

Mongo, Postgres, and DocumentDB token repositories no longer let JSON.parse failures bubble as 500s: malformed base64 bearer payloads throw ClientSafeError('Invalid Token: Token is malformed', 401) after shape validation (id + string token).

Tests cover the Express middleware matrix and shared getJSONTokenFromBase64BufferToken behavior across all three repositories.

Reviewed by Cursor Bugbot for commit f8fcc58. Bugbot is set up for automated code reviews on this repo. Configure here.

📦 Published PR as canary version: 2.0.0--canary.727.f8fcc58.0

✨ Test out this PR locally via:

npm install @friggframework/admin-scripts@2.0.0--canary.727.f8fcc58.0
npm install @friggframework/core@2.0.0--canary.727.f8fcc58.0
npm install @friggframework/devtools@2.0.0--canary.727.f8fcc58.0
npm install @friggframework/eslint-config@2.0.0--canary.727.f8fcc58.0
npm install @friggframework/prettier-config@2.0.0--canary.727.f8fcc58.0
npm install @friggframework/schemas@2.0.0--canary.727.f8fcc58.0
npm install @friggframework/serverless-plugin@2.0.0--canary.727.f8fcc58.0
npm install @friggframework/test@2.0.0--canary.727.f8fcc58.0
npm install @friggframework/ui@2.0.0--canary.727.f8fcc58.0
# or 
yarn add @friggframework/admin-scripts@2.0.0--canary.727.f8fcc58.0
yarn add @friggframework/core@2.0.0--canary.727.f8fcc58.0
yarn add @friggframework/devtools@2.0.0--canary.727.f8fcc58.0
yarn add @friggframework/eslint-config@2.0.0--canary.727.f8fcc58.0
yarn add @friggframework/prettier-config@2.0.0--canary.727.f8fcc58.0
yarn add @friggframework/schemas@2.0.0--canary.727.f8fcc58.0
yarn add @friggframework/serverless-plugin@2.0.0--canary.727.f8fcc58.0
yarn add @friggframework/test@2.0.0--canary.727.f8fcc58.0
yarn add @friggframework/ui@2.0.0--canary.727.f8fcc58.0

Version

Published prerelease version: v2.0.0-next.126

Changelog

🐛 Bug Fix

  • @friggframework/core
    • fix(core): keep the HTTP status of client-safe errors (expired token → 401, not 500) #727 (@d-klotz)
  • @friggframework/admin-scripts, @friggframework/core, @friggframework/devtools, @friggframework/eslint-config, @friggframework/prettier-config, @friggframework/schemas, @friggframework/serverless-plugin, @friggframework/test, @friggframework/ui
    • fix(core): only run user actions through the integration actions route, 404 for foreign integrations #726 (@d-klotz)

Authors: 1

…→ 401, not 500)

The express error boundary boomified every non-Boom error without a
statusCode, so a ClientSafeError such as "Invalid Token: Token is expired"
(401) answered 500. Keep the status for client-safe errors with a 4xx/5xx
statusCode; every other error, including upstream FetchError statuses,
still answers 500.

A malformed bearer token (not base64 JSON, or JSON without id/token) now
throws ClientSafeError('Invalid Token: Token is malformed', 401) instead of
a SyntaxError/TypeError that answered 500.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@d-klotz d-klotz added release Create a release when this pr is merged prerelease This change is available in a prerelease. labels Oct 26, 2026
@netlify

netlify Bot commented Oct 26, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for friggframework-org canceled.

Name Link
🔨 Latest commit f8fcc58
🔍 Latest deploy log https://app.netlify.com/projects/friggframework-org/deploys/6adf979af4b08fa60f3af992

@sonarqubecloud

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
26.2% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Comment on lines +266 to +271
if (
!['string', 'number'].includes(typeof jsonToken?.id) ||
typeof jsonToken?.token !== 'string'
) {
throw new ClientSafeError('Invalid Token: Token is malformed', 401);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟣 pre-existing, not blocking: pre-existing, partial fix: a bearer token whose id has the wrong shape for the database still gets a 500 instead of the promised 401. The new check accepts any string or number id, but validateAndGetToken cannot handle every such value. On Postgres, _convertId throws a plain Error for id: "abc". On Mongo, Prisma rejects a number or a non-ObjectId string. Fix: turn every id the adapter cannot look up into ClientSafeError('Invalid Token: Token is malformed', 401) (Postgres: integer-parsable; Mongo: 24-hex ObjectId string), which covers the 2 sites: token-repository-postgres.js:90 and token-repository-mongo.js:58. [also at: packages/core/token/repositories/token-repository-mongo.js:225 - pre-existing. A bearer token whose decoded id is a string or number but not a valid id for the adapter still gets a 500 on Mongo and Postgres.]
A small fix can ride a push you are already making; otherwise a short reply is enough.

Why this was flagged

The trigger is a client sending Authorization: Bearer &lt;base64 of {"id":"abc","token":"x"}&gt; (or {"id":123,...} on Mongo). The new check at token-repository-postgres.js:266-271 lets it through because typeof id is 'string'. Then validateAndGetToken calls _convertId (token-repository-postgres.js:32-34), which throws new Error('Invalid ID: abc cannot be converted to integer'). That error is not client-safe, so toBoom makes it a 500 'Internal Server Error' and logs frigg.http.request_failed at ERROR. On Mongo, prisma.token.findUnique({ where: { id: tokenObj.id } }) (token-repository-mongo.js:57-58) throws a Prisma validation or malformed-ObjectID error for a numeric or non-hex id, with the same 500 result. The base branch also returned 500 here. The PR says malformed tokens now get 401, but the shape check is looser than what each adapter's lookup accepts.

Verification: pre-existing. The new check at token-repository-postgres.js:266-271 only tests !&#91;'string', 'number'].includes(typeof jsonToken?.id), so "abc" passes it. _convertId (lines 30-37) does parseInt and then throws. That is a plain Error, not a ClientSafeError, so the new toBoom falls back to 500. The base commit already returned 500 for these inputs, so nothing gets worse.

An automated safety screen replaced some characters, such as angle brackets, in the code quoted in this comment with HTML escape codes.

@d-klotz
d-klotz merged commit 3d88399 into next Oct 26, 2026
16 of 17 checks passed
@d-klotz
d-klotz deleted the fix/client-safe-error-status branch October 26, 2026 18:16
@seanspeaks

Copy link
Copy Markdown
Contributor

🚀 PR was released in v2.0.0-next.126 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

prerelease This change is available in a prerelease. release Create a release when this pr is merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants