fix: sanitize error by pawelstepien-da · Pull Request #2269 · canton-network/wallet

pawelstepien-da · 2026-08-12T09:15:29Z

Error handling fixes:

Added global error handler middleware. It catches all errors that propagated through previous middlewares and sanitizes response. Before it would reach express handler which could return HTML error, that could expose sensitive internals.

curl -s -i -X POST http://localhost:3030/api/v0/user \
  -H 'Content-Type: application/json' \
  -d '{"jsonrpc":"2.0","id":1,"method":"listWallets","params":[]}'

Was:

<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="utf-8">
<title>Error</title>
</head>
<body>
<pre>Error: User is not connected<br> &nbsp; &nbsp;at getEthProviderError (<absolute-path>/node_modules/.pnpm/@metamask+rpc-errors@7.0.3_supports-color@10.2.2/node_modules/@metamask/rpc-errors/src/errors.ts:271:10)<br> &nbsp; &nbsp;at Object.unauthorized (<absolute-path>/node_modules/.pnpm/@metamask+rpc-errors@7.0.3_supports-color@10.2.2/node_modules/@metamask/rpc-errors/src/errors.ts:181:12)<br> &nbsp; &nbsp;at assertConnected (<absolute-path>/core/wallet-auth/src/auth-utils.ts:14:30)<br> &nbsp; &nbsp;at StoreSql.assertConnected (<absolute-path>/core/wallet-store-sql/dist/index.js:266:10)<br> &nbsp; &nbsp;at StoreSql.getSession (<absolute-path>/core/wallet-store-sql/dist/index.js:403:23)<br> &nbsp; &nbsp;at &lt;anonymous&gt; (<absolute-path>/wallet-gateway/remote/src/middleware/sessionHandler.ts:43:18)<br> &nbsp; &nbsp;at Layer.handleRequest (<absolute-path>/node_modules/.pnpm/router@2.2.0_supports-color@10.2.2/node_modules/router/lib/layer.js:152:17)<br> &nbsp; &nbsp;at trimPrefix (<absolute-path>/node_modules/.pnpm/router@2.2.0_supports-color@10.2.2/node_modules/router/index.js:342:13)<br> &nbsp; &nbsp;at <absolute-path>/node_modules/.pnpm/router@2.2.0_supports-color@10.2.2/node_modules/router/index.js:297:9<br> &nbsp; &nbsp;at param (<absolute-path>/node_modules/.pnpm/router@2.2.0_supports-color@10.2.2/node_modules/router/index.js:600:14)</pre>
</body>
</html>

Now:
{"jsonrpc":"2.0","id":1,"error":{"code":4100,"message":"No active session found"}}

sessionHandler now checks that there is accessToken attached to request and returns 401 early if not. Before it failed at store level.
Made JSON-RPC errors preserve their custom message. It was getting lost in handleRpcError.

curl -s -X POST http://localhost:3030/api/v0/dapp \
  -H 'Content-Type: application/json' \
  -d '{"jsonrpc":"2.0","id":3,"method":"thisMethodDoesNotExist","params":[]}'

Was:
{"jsonrpc":"2.0","id":null,"error":{"code":-32601}}

Now:
{"jsonrpc":"2.0","id":null,"error":{"code":-32601,"message":"Method thisMethodDoesNotExist not found"}}

Changed sessionHandler and jwtAuth to return auth failures in the JSON-RPC error shape. They previously returned {"error": "<message>"}, which fails ErrorResponse validation in HttpTransport, so clients discarded the real reason and fell back to a generic message derived from the HTTP status.
Stopped attaching the thrown error to error.data. handleRpcError put the raw error object into the response, which could leak sensitive internals. Full error still gets logged.

let response: ErrorResponse = {
    error: {
        ...rpcErrors.internal(),
        message: genericMessage,
        data: error,        // removed
    },
}

Verified by unit test forwards the Error message but not the error object itself

Additional fixes not related to issue:

Fixed middleware not applying properly to endpoints, if dapp or user api was configured to have path that doesn't start with /api.
Renamed userPath to userApiUrl in wallet-gateway-configuration endpoint, because it's actually full url not just path and it's consistent with dappApiUrl.

Signed-off-by: Pawel Stepien <pawel.stepien@digitalasset.com>

…nitize-error Signed-off-by: Pawel Stepien <pawel.stepien@digitalasset.com>

Signed-off-by: Pawel Stepien <pawel.stepien@digitalasset.com>

…nitize-error Signed-off-by: Pawel Stepien <pawel.stepien@digitalasset.com>

mjuchli-da · 2026-08-17T23:34:12Z

+            return
+        }
+
+        res.status(500).json({ error: 'Internal Server Error' })


Is it not possible to tie this to the request somehow?
Or are we relying on the fact that logs should be written and tied to the request id at the place where the error happens?

I don't think we can fully rely on logs from all potential places error can come from as a way to correlate error with request. I am inclined to add request info next to the error in logger.error at the top of that handler. But I'm reluctant to just add whole request body and even more so headers to the log, because I'm worried about persisting sensitive information in logs. I'm thinking maybe a subset of request info like:

http method

http url path

json rpc method (if present)

json rpc id (if present)

maybe boolean hasAuthContext, without leaking authContext details like accessToken

What do you think? It could guide us to api and method that caused the unhandled error, but unfortunately not exactly what was in the payload.

mjuchli-da · 2026-08-17T23:35:15Z

+}
+
+// Catches unhandled errors and prevents internal details like stack trace from reaching end user
+export function errorHandler(


Why is this separate from handleRpcError in jsonRpcHandler.ts?

Because jsonRpcHandler wouldn't know about errors thrown anywhere else but controller. Issue this PR originated from reports a runtime error from sessionHandler middleware. When an error is thrown (or next(err) is called) along the journey of request through middlewares, express looks for closest next error middleware (a middleware that has four args: err, req, res, next). We didn't have one, so the error went straight to default express error handler and it responded with a verbose JS error in body.

I think it's a standard pattern to have one general error handler at the end of chain that would catch unexpected errors.

Ok makes sense to me, thanks. I think still think it would be good for @alexmatson-da to have a look here, given that he is the author of jsonRpcHandler

pawelstepien-da added 14 commits August 11, 2026 12:42

middleware catching unhandled errors and returning generic message

bfafec3

Signed-off-by: Pawel Stepien <pawel.stepien@digitalasset.com>

fix user and dapp paths breaking wg when not starting with /api

5f25459

Signed-off-by: Pawel Stepien <pawel.stepien@digitalasset.com>

fix leaking error stack when no access token provided

f46a116

Signed-off-by: Pawel Stepien <pawel.stepien@digitalasset.com>

fix missing error messages

837d784

Signed-off-by: Pawel Stepien <pawel.stepien@digitalasset.com>

hide error messages when unexpected and show when custom error

92c7523

Signed-off-by: Pawel Stepien <pawel.stepien@digitalasset.com>

consistent naming of userApiUrl in /wallet-gateway-config

321ad60

Signed-off-by: Pawel Stepien <pawel.stepien@digitalasset.com>

unit tests

a9500ed

Signed-off-by: Pawel Stepien <pawel.stepien@digitalasset.com>

unit tests

755b293

Signed-off-by: Pawel Stepien <pawel.stepien@digitalasset.com>

Merge remote-tracking branch 'refs/remotes/origin/main' into pawel/sa…

7682934

…nitize-error Signed-off-by: Pawel Stepien <pawel.stepien@digitalasset.com>

unit tests

41b918c

Signed-off-by: Pawel Stepien <pawel.stepien@digitalasset.com>

log full error in jsonRpcHandler while keeping response sanitized

d3082eb

Signed-off-by: Pawel Stepien <pawel.stepien@digitalasset.com>

missed /api/ fix part

6c40e14

Signed-off-by: Pawel Stepien <pawel.stepien@digitalasset.com>

fix req too large becoming 500 instead of 413

a1488e9

Signed-off-by: Pawel Stepien <pawel.stepien@digitalasset.com>

Merge remote-tracking branch 'refs/remotes/origin/main' into pawel/sa…

70d2cff

…nitize-error Signed-off-by: Pawel Stepien <pawel.stepien@digitalasset.com>

pawelstepien-da marked this pull request as ready for review August 13, 2026 08:47

pawelstepien-da requested a review from a team as a code owner August 13, 2026 08:47

mjuchli-da reviewed Aug 17, 2026

View reviewed changes

pawelstepien-da requested a review from alexmatson-da August 21, 2026 17:48

Navigation Menu

Uh oh!

fix: sanitize error - #2269

fix: sanitize error#2269
pawelstepien-da wants to merge 14 commits into
mainfrom
pawel/sanitize-error

pawelstepien-da commented Aug 12, 2026 •

edited

Loading

Uh oh!

mjuchli-da Aug 17, 2026

Uh oh!

pawelstepien-da Aug 19, 2026

Uh oh!

mjuchli-da Aug 17, 2026

Uh oh!

pawelstepien-da Aug 19, 2026 •

edited

Loading

Uh oh!

mjuchli-da Aug 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

pawelstepien-da commented Aug 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mjuchli-da Aug 17, 2026

Choose a reason for hiding this comment

Uh oh!

pawelstepien-da Aug 19, 2026

Choose a reason for hiding this comment

Uh oh!

mjuchli-da Aug 17, 2026

Choose a reason for hiding this comment

Uh oh!

pawelstepien-da Aug 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mjuchli-da Aug 21, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

pawelstepien-da commented Aug 12, 2026 •

edited

Loading

pawelstepien-da Aug 19, 2026 •

edited

Loading