From f7974f514399f711efb321df602c2ae82f02b4c0 Mon Sep 17 00:00:00 2001 From: DanielWalnut <45447813+hetaoBackend@users.noreply.github.com> Date: Fri, 30 Oct 2026 22:47:12 +0800 Subject: [PATCH] fix(local-runtime): stop leaking stack traces through HTTP JSON CodeQL js/stack-trace-exposure on main: json() was a sink for Error-derived payloads. Drop stack keys (and Error instances) in the JSON replacer, stop embedding error.stack in TaskError details, return stable client text for 5xx / feishu questionnaire failures, and keep only short single-line validation messages on 4xx cron responses. --- packages/agent-modules/cron/src/registry.ts | 7 ++++++- .../src/api/host-feishu-card-action.ts | 4 ++-- .../local-runtime/src/api/host-helpers.ts | 15 ++++++++++++- .../local-runtime/src/api/routes/agents.ts | 13 ++++++++---- .../src/background-task/domain.ts | 2 +- packages/local-runtime/src/cron/api.ts | 21 ++++++++++++++++--- 6 files changed, 50 insertions(+), 12 deletions(-) diff --git a/packages/agent-modules/cron/src/registry.ts b/packages/agent-modules/cron/src/registry.ts index b9f368683..324db25cf 100644 --- a/packages/agent-modules/cron/src/registry.ts +++ b/packages/agent-modules/cron/src/registry.ts @@ -619,7 +619,12 @@ export class CronRegistry { this.scheduler.assertSchedulable(config); } catch (err) { if (err instanceof AppError) throw err; - throw new AppError(err instanceof Error ? err.message : String(err), 'VALIDATION_ERROR', 400); + // Keep the scheduler detail on the server; clients get a stable validation code. + logger.warn( + backgroundCtx(), + `[cron] schedule rejected by scheduler: ${err instanceof Error ? err.message : 'unknown'}`, + ); + throw new AppError('Invalid cron schedule', 'VALIDATION_ERROR', 400); } } diff --git a/packages/local-runtime/src/api/host-feishu-card-action.ts b/packages/local-runtime/src/api/host-feishu-card-action.ts index eebc74b30..75bb4ed6f 100644 --- a/packages/local-runtime/src/api/host-feishu-card-action.ts +++ b/packages/local-runtime/src/api/host-feishu-card-action.ts @@ -293,7 +293,7 @@ async function handleFormSubmit(input: { return json({ ok: false, requestId, - error: err instanceof Error ? err.message : String(err), + error: 'QUESTIONNAIRE_SUBMIT_FAILED', toast: { type: 'error', content: '提交失败，请稍后再试' }, }); } @@ -355,7 +355,7 @@ async function handleFormSubmit(input: { return json({ ok: false, requestId, - error: err instanceof Error ? err.message : String(err), + error: 'QUESTIONNAIRE_SUBMIT_FAILED', toast: { type: 'error', content: '提交失败，请稍后再试' }, }); } diff --git a/packages/local-runtime/src/api/host-helpers.ts b/packages/local-runtime/src/api/host-helpers.ts index 8d8624c36..b61ce307e 100644 --- a/packages/local-runtime/src/api/host-helpers.ts +++ b/packages/local-runtime/src/api/host-helpers.ts @@ -387,14 +387,27 @@ export function buildLegacyDaemonProxyPath( } // ─── Response factories ──────────────────────────────────────────────── +/** + * JSON HTTP response. Drops `stack` keys and turns Error values into + * `{ name, message }` so a caller cannot leak stack frames through this sink + * (CodeQL js/stack-trace-exposure). + */ export function json(data: unknown, init?: ResponseInit): Response { - return new Response(JSON.stringify(data), { + return new Response(JSON.stringify(data, publicJsonReplacer), { status: init?.status ?? 200, statusText: init?.statusText, headers: { ...JSON_HEADERS, ...(init?.headers ?? {}) }, }); } +function publicJsonReplacer(key: string, value: unknown): unknown { + if (key === 'stack') return undefined; + if (value instanceof Error) { + return { name: value.name, message: value.message }; + } + return value; +} + export function notFound(pathname: string): Response { return json( { error: `Local runtime route not found: ${pathname}` }, diff --git a/packages/local-runtime/src/api/routes/agents.ts b/packages/local-runtime/src/api/routes/agents.ts index 8e05e414d..83a8224fa 100644 --- a/packages/local-runtime/src/api/routes/agents.ts +++ b/packages/local-runtime/src/api/routes/agents.ts @@ -874,10 +874,15 @@ export class LocalApiAgentRoutes { typeof Object(error).code === "string" ? Object(error).code : "INTERNAL_ERROR"; - return json( - { error: error instanceof Error ? error.message : String(error), code }, - { status }, - ); + // 5xx: generic client text (full error stays in server logs upstream). + // 4xx: keep the typed message the UI already keys on. + const message = + status >= 500 + ? "Internal error" + : error instanceof Error + ? error.message + : "Request failed"; + return json({ error: message, code }, { status }); } } diff --git a/packages/local-runtime/src/background-task/domain.ts b/packages/local-runtime/src/background-task/domain.ts index 4554f22a7..4a979afa7 100644 --- a/packages/local-runtime/src/background-task/domain.ts +++ b/packages/local-runtime/src/background-task/domain.ts @@ -258,10 +258,10 @@ export function normalizeTaskError(error: unknown, fallbackCode = 'TASK_ERROR'): }; } if (error instanceof Error) { + // Stack frames stay on the server log; TaskError is returned over HTTP. return { message: error.message, code: fallbackCode, - ...(error.stack ? { details: { stack: error.stack } } : {}), }; } return { diff --git a/packages/local-runtime/src/cron/api.ts b/packages/local-runtime/src/cron/api.ts index 71fd87ca6..cc7fb84ee 100644 --- a/packages/local-runtime/src/cron/api.ts +++ b/packages/local-runtime/src/cron/api.ts @@ -963,9 +963,16 @@ async function readCronConfigUpdateResponse( } function validationErrorResponse(err: unknown): Response { + // Prefer a short, single-line message from our own validation Errors; never + // forward multi-line / stack-shaped text to the client. + const raw = err instanceof Error ? err.message : ''; + const message = + raw && !raw.includes('\n') && !raw.includes('\r') && raw.length <= 300 + ? raw + : 'Invalid request'; return json( { - error: err instanceof Error ? err.message : String(err), + error: message, code: 'VALIDATION_ERROR', }, { status: 400 }, @@ -1060,8 +1067,10 @@ function validateCronSchedule(config: CronConfig): Response | undefined { function cronErrorResponse(err: unknown): Response { const candidate = err as { statusCode?: unknown; code?: unknown; message?: unknown }; const status = typeof candidate.statusCode === 'number' ? candidate.statusCode : 500; - const message = typeof candidate.message === 'string' ? candidate.message : String(err); + const rawMessage = typeof candidate.message === 'string' ? candidate.message : ''; const code = typeof candidate.code === 'string' ? candidate.code : undefined; + // Duplicate / not-found detection still keys off the server-side message. + const message = rawMessage; // Duplicate detection is independent of the incoming statusCode — see // the same-shaped fix in `contract.ts:toCronContractError`. The // `agent-modules/cron` `AppError` throws with `statusCode = 409` and @@ -1077,9 +1086,15 @@ function cronErrorResponse(err: unknown): Response { // `Cron task already registered: mavis/end-of-workday`. Keeps the same convention as // `packages/local-runtime/src/api/routes/agents.ts` (40901 conflict etc.). const errorCode = isDuplicate ? 40903 : undefined; + const publicMessage = + derivedStatus >= 500 + ? 'Internal error' + : message && !message.includes('\n') && !message.includes('\r') && message.length <= 300 + ? message + : 'Request failed'; return json( { - error: message, + error: publicMessage, ...(code ? { code } : {}), ...(errorCode !== undefined ? { errorCode } : {}), },