Skip to content

fix(miner): fail closed when a chat-action handler throws #7041

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
shin-core wants to merge 1 commit into from
+26 −1
Closed

fix(miner): fail closed when a chat-action handler throws #7041

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion packages/loopover-miner/lib/chat-action-dispatch.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,15 @@ export async function dispatchChatAction(request, options = {}) {
return { ok: false, status: "invalid_params", action };
}

const result = await registered.handler(request);
let result;
try {
result = await registered.handler(request);
} catch (error) {
// A handler that throws fails closed with the module's typed result shape (#6989), consistent with the
// paramsValidator catch above -- a distinct "handler_error" status lets a caller tell an execution
// failure apart from a params-validation failure.
return { ok: false, status: "handler_error", action, error: error instanceof Error ? error.message : String(error) };

@superagent-security superagent-security Bot Jul 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2: Handler error messages returned to caller may leak sensitive internal details

Raw handler error messages are returned to the caller, risking internal detail leakage.

Log the error internally and return a generic error code instead of the raw message.

AI prompt 
Check if this security scanner issue is valid. If so, understand the root cause and fix it. If appropriate, update or add tests. Keep the change focused and preserve intended behavior.

<file name="packages/loopover-miner/lib/chat-action-dispatch.js">
<violation number="1" location="packages/loopover-miner/lib/chat-action-dispatch.js:81">
<priority>P2</priority>
<title>Handler error messages returned to caller may leak sensitive internal details</title>
<evidence>The catch block for registered.handler(request) returns `error: error instanceof Error ? error.message : String(error)` directly in the typed result object. If this function is invoked from a REST endpoint, internal error details such as database connection strings, file paths, or secrets could leak to external clients.</evidence>
<recommendation>Log the full error internally and return a generic error identifier (e.g., an error code or trace ID) to the caller instead of the raw error message.</recommendation>
</violation>
</file>

}
return { ok: true, status: "dispatched", action, result };
}

Expand Down
17 changes: 17 additions & 0 deletions test/unit/miner-chat-action-dispatch.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,4 +147,21 @@ describe("dispatchChatAction (#6519)", () => {
const result = await dispatchChatAction({ action: "demo", params: {} }, { env: enabledEnv, registry });
expect(result).toEqual({ ok: false, status: "invalid_params", action: "demo", error: "boom" });
});

it("fails closed with handler_error when the handler throws, not an unhandled rejection (#6989)", async () => {
const registry = registryWith("demo", () => true, () => {
throw new Error("network down");
});
const result = await dispatchChatAction({ action: "demo", params: {} }, { env: enabledEnv, registry });
// Distinct from invalid_params so a caller can tell an execution failure from a validation failure.
expect(result).toEqual({ ok: false, status: "handler_error", action: "demo", error: "network down" });
});

it("stringifies a non-Error thrown by the handler (#6989)", async () => {
const registry = registryWith("demo", () => true, () => {
throw "kaboom";
});
const result = await dispatchChatAction({ action: "demo", params: {} }, { env: enabledEnv, registry });
expect(result).toEqual({ ok: false, status: "handler_error", action: "demo", error: "kaboom" });
});
});