From 2977f75129f27f53d3a9531880b2ee55b80b0c62 Mon Sep 17 00:00:00 2001 From: chick Date: Mon, 7 Sep 2026 01:57:47 +0800 Subject: [PATCH] fix: apply security and correctness review findings across both stacks Legacy panel: - upstream body reads now carry their own deadline and a 10 MB byte budget; a stalled modem can no longer hang /api/status fan-out forever nor OOM the proxy (request headers alone had the timeout, bodies had none) - add X-Frame-Options DENY / CSP frame-ancestors none / nosniff; the panel (delete-instance and confirmed-write dialogs) is no longer clickjackable - only send a JSON content-type when the API console request has a body, so payload-less dangerous writes stop failing with 400 and burning the one-use confirmation token - register a form-urlencoded parser (the proxy branch was unreachable) and drop the multipart parser that buffered up to 60 MB before rejecting; bodyLimit drops to 2 MB; framework-level 415 keeps the stable error body - remove /api/sms/send from readable paths: GET bypassed the write confirmation for a send endpoint - /api/instances/:id/login maps upstream failures to a stable 502 instead of leaking raw error text - guard MULTI_SIMADMIN_TIMEOUT_MS parsing (NaN aborted every request); prune dead code (buildClients, cookie expando no-op) Control plane: - deleting a notification channel detaches it from rules instead of leaving dangling ids that made every referencing rule unreadable and silently dropped future notifications; rule reads tolerate unknown ids - startup sweep resets notification_queue rows stranded in 'sending' by a crash (mirrors the sms outbox sweep); terminal outbox rows are pruned on the retention timer - /api/v1/metrics no longer emits operator-assigned node names on the session-free scrape; login limiter map is bounded and pruned; Secure cookie honors the gateway-declared x-forwarded-proto - webhook delivery sets redirect: manual (signed payloads are not replayed) - SMTP envelope sender is validated against CR/LF smuggling - scheduled reboots with delaySeconds != 3 fail fast at the dispatcher with a clear reason instead of burning every retry; contract narrowed to the pinned baseline --- .../scheduled-operation-dispatcher.ts | 8 +++ .../central-notification-service.ts | 50 +++++++++++++++++-- .../notifications/channel-delivery.ts | 2 + .../application/notifications/smtp-sender.ts | 3 ++ .../observability/metrics-service.test.ts | 13 +++-- .../observability/metrics-service.ts | 6 +-- apps/api/src/canary-gateway.ts | 3 ++ apps/api/src/control-plane.ts | 5 ++ .../src/interface/http/console-auth-routes.ts | 23 ++++++++- packages/contracts/src/automation.ts | 4 +- public/app.js | 2 +- server/app.js | 22 ++++++-- server/core.js | 45 +++++++++++++++-- server/proxy/policy.js | 2 +- server/proxy/service.js | 4 +- test/server-core.test.js | 2 +- 16 files changed, 168 insertions(+), 26 deletions(-) diff --git a/apps/api/src/application/automation/scheduled-operation-dispatcher.ts b/apps/api/src/application/automation/scheduled-operation-dispatcher.ts index ca559f5..e3fe1cd 100644 --- a/apps/api/src/application/automation/scheduled-operation-dispatcher.ts +++ b/apps/api/src/application/automation/scheduled-operation-dispatcher.ts @@ -114,6 +114,14 @@ export class ScheduledOperationDispatcher { const schema = operationSchemas[task.operationType]; if (!operationId || !schema) return { outcome: 'needs-attention', reason: 'Unsupported scheduled operation', jobIds: [] }; + // The execution baseline pins delay_seconds to 3; anything else could never + // succeed, so refuse up front instead of exhausting the retry policy. + if (task.operationType === 'reboot-system' && (task.delaySeconds ?? 3) !== 3) + return { + outcome: 'needs-attention', + reason: 'Scheduled reboots only support the pinned 3-second delay', + jobIds: [], + }; const fields = task.operationType === 'reboot-system' ? [ diff --git a/apps/api/src/application/notifications/central-notification-service.ts b/apps/api/src/application/notifications/central-notification-service.ts index bc67164..174a570 100644 --- a/apps/api/src/application/notifications/central-notification-service.ts +++ b/apps/api/src/application/notifications/central-notification-service.ts @@ -559,7 +559,28 @@ export class CentralNotificationService { async deleteChannel(id: string): Promise { const channel = this.getChannel(id); - this.#db.prepare('DELETE FROM notification_channels WHERE id=?').run(id); + this.#db.transaction(() => { + this.#db.prepare('DELETE FROM notification_channels WHERE id=?').run(id); + // Rules keep channel ids in a JSON column, so no FK cascade applies; a + // dangling id would make every referencing rule unreadable and drop + // future notifications on the floor. + const rules = this.#db + .prepare('SELECT id, channel_ids_json FROM notification_rules') + .all() as Array<{ id: string; channel_ids_json: string }>; + for (const rule of rules) { + let ids: unknown; + try { + ids = JSON.parse(String(rule.channel_ids_json)); + } catch { + continue; + } + if (!Array.isArray(ids) || !ids.includes(id)) continue; + const next = JSON.stringify(ids.filter((entry) => entry !== id)); + this.#db + .prepare('UPDATE notification_rules SET channel_ids_json=?, updated_at=? WHERE id=?') + .run(next, this.#now(), rule.id); + } + })(); if (channel.secretReference) await this.#options.store.delete(channel.secretReference).catch(() => false); } @@ -1202,6 +1223,19 @@ export class CentralNotificationService { return { processed: succeeded + failed, succeeded, failed }; } + /** Startup sweep: rows claimed as 'sending' before a crash would never drain again. */ + reconcileInterrupted(): number { + return Number( + this.#db + .prepare( + `UPDATE notification_queue + SET status='pending',updated_at=? + WHERE status='sending'`, + ) + .run(this.#now(), ).changes, + ); + } + retryQueueItem(id: string): void { const result = this.#db .prepare( @@ -1338,7 +1372,12 @@ export class CentralNotificationService { #queueItem(row: Record): NotificationQueueItem { const status = String(row.status) as NotificationQueueItem['status']; const attempts = Number(row.attempts ?? 0); - const payload = record(JSON.parse(String(row.payload_json))) ?? {}; + let payload: Record = {}; + try { + payload = record(JSON.parse(String(row.payload_json))) ?? {}; + } catch { + payload = {}; + } const deliveredAt = typeof row.delivered_at === 'string' && row.delivered_at ? row.delivered_at : undefined; const ruleId = typeof row.rule_id === 'string' && row.rule_id ? row.rule_id : undefined; @@ -1560,7 +1599,12 @@ export class CentralNotificationService { enabled: row.enabled === 1, condition: parseJson(String(row.condition_json), this.#condition.bind(this)), scope: parseJson(String(row.scope_json), this.#scope.bind(this)), - channels: Object.freeze(channelIds.map((id) => this.getChannel(id))), + channels: Object.freeze( + channelIds.flatMap((id) => { + const row = this.#row(`SELECT * FROM notification_channels WHERE id = ?`, id); + return row ? [this.#channel(row)] : []; + }), + ), templates: parseJson(String(row.templates_json), this.#templates.bind(this)), rateLimit: parseJson(String(row.rate_limit_json), validateRateLimit), quietHours: parseJson(String(row.quiet_hours_json), validateQuietHours), diff --git a/apps/api/src/application/notifications/channel-delivery.ts b/apps/api/src/application/notifications/channel-delivery.ts index 0704798..5f9a800 100644 --- a/apps/api/src/application/notifications/channel-delivery.ts +++ b/apps/api/src/application/notifications/channel-delivery.ts @@ -160,6 +160,8 @@ async function defaultRequester(request: HttpRequest, timeoutMs: number): Promis headers: request.headers, ...(request.body === undefined ? {} : { body: request.body }), signal: AbortSignal.timeout(timeoutMs), + // A signed payload must never be replayed to a redirect target. + redirect: 'manual', }); const text = (await response.text()).slice(0, MAX_RESPONSE_BYTES); return { status: response.status, body: text }; diff --git a/apps/api/src/application/notifications/smtp-sender.ts b/apps/api/src/application/notifications/smtp-sender.ts index 7becf3e..b486076 100644 --- a/apps/api/src/application/notifications/smtp-sender.ts +++ b/apps/api/src/application/notifications/smtp-sender.ts @@ -220,6 +220,9 @@ export async function sendEmailNotification( const receivers = splitAddresses(configText(input.config, 'receiver_addresses')); if (!host) return { ok: false, detail: '缺少 SMTP 地址' }; if (!sender) return { ok: false, detail: '缺少发件地址' }; + // Envelope commands are line-framed; reject anything that could break the frame. + if (!/^[^\s@<>,;:"']+@[^\s@<>,;:"']+$/u.test(sender)) + return { ok: false, detail: '发件地址无效' }; if (receivers.length === 0) return { ok: false, detail: '缺少有效的收件地址' }; const port = configNumber(input.config, 'smtp_port', 465); const security = configText(input.config, 'smtp_security') || 'implicit_tls'; diff --git a/apps/api/src/application/observability/metrics-service.test.ts b/apps/api/src/application/observability/metrics-service.test.ts index 9967b75..814ebc1 100644 --- a/apps/api/src/application/observability/metrics-service.test.ts +++ b/apps/api/src/application/observability/metrics-service.test.ts @@ -80,7 +80,9 @@ describe('MetricsService', () => { const text = metrics.render(); expect(text).toContain('multi_simadmin_nodes_total 2'); expect(text).toContain('multi_simadmin_node_enabled{node="node-b"} 0'); - expect(text).toContain('multi_simadmin_node_info{node="node-a",name="机房 A"} 1'); + expect(text).toContain('multi_simadmin_node_info{node="node-a"} 1'); + // Operator-assigned names stay out of the session-free exposition entirely. + expect(text).not.toContain('机房 A'); expect(text).toContain('multi_simadmin_node_probe_total{node="node-a",outcome="success"} 38'); expect(text).toContain('multi_simadmin_node_availability_ratio{node="node-a"} 0.950'); expect(text).toContain('multi_simadmin_sms_outbox{status="failed"} 1'); @@ -108,14 +110,15 @@ describe('MetricsService', () => { expect(text).toContain('multi_simadmin_identity_pending_total 2'); }); - it('escapes label values so a node name cannot forge a series', () => { + it('never emits a node name, so a hostile name cannot forge a series', () => { const { db: database, metrics } = fixture(); seedNode(database, 'node-a', 'evil" name\nwith break'); const text = metrics.render(); - expect(text).toContain('name="evil\\" name\\nwith break"'); - // The injected newline must not become its own sample line. - expect(text.split('\n').filter((line) => line.includes('with break'))).toHaveLength(1); + // Names are excluded from the session-free exposition outright; escaping a + // label is not the right tool for untrusted operator input. + expect(text).not.toContain('evil"'); + expect(text.split('\n').filter((line) => line.includes('with break'))).toHaveLength(0); }); it('emits a valid exposition: help and type precede every series', () => { diff --git a/apps/api/src/application/observability/metrics-service.ts b/apps/api/src/application/observability/metrics-service.ts index 24442bf..1576912 100644 --- a/apps/api/src/application/observability/metrics-service.ts +++ b/apps/api/src/application/observability/metrics-service.ts @@ -77,9 +77,9 @@ export class MetricsService { ]), ); - const info = nodes.map( - (node) => `${PREFIX}_node_info{node="${label(node.id)}",name="${label(node.name)}"} 1`, - ); + // Operator-assigned names can leak hostnames/locations to unauthenticated + // scrapers; the series intentionally stays opaque to node ids. + const info = nodes.map((node) => `${PREFIX}_node_info{node="${label(node.id)}"} 1`); lines.push( ...sample(`${PREFIX}_node_info`, 'Registered nodes and their addresses.', 'gauge', info), ...sample(`${PREFIX}_nodes_total`, 'Number of registered nodes.', 'gauge', [ diff --git a/apps/api/src/canary-gateway.ts b/apps/api/src/canary-gateway.ts index cc70f51..35b9f71 100644 --- a/apps/api/src/canary-gateway.ts +++ b/apps/api/src/canary-gateway.ts @@ -107,6 +107,9 @@ function trustedUpstreamHeaders( delete sanitized[GATEWAY_CLIENT_IP_HEADER]; if (gatewayToken !== undefined) sanitized[GATEWAY_AUTH_HEADER] = gatewayToken; if (clientIp !== undefined) sanitized[GATEWAY_CLIENT_IP_HEADER] = clientIp; + // The browser connection's protocol is the gateway's to declare; the API uses + // it to decide whether session cookies may carry the Secure flag. + sanitized['x-forwarded-proto'] = (headers['x-forwarded-proto'] as string | undefined) ?? 'http'; return sanitized; } diff --git a/apps/api/src/control-plane.ts b/apps/api/src/control-plane.ts index 98dc2b0..854eed3 100644 --- a/apps/api/src/control-plane.ts +++ b/apps/api/src/control-plane.ts @@ -320,6 +320,7 @@ export function buildControlPlaneApp(options: ControlPlaneOptions): ControlPlane ...(options.now ? { now: options.now } : {}), }); smsOutbox.reconcileInterrupted(); + centralNotifications.reconcileInterrupted(); // Removal follows the hub rule the operator already knows: an online device is told to release // its own binding first and a refusal keeps the record, while a device that cannot be reached // at all is forgotten locally. The closure is late-bound because the device action service is @@ -581,6 +582,10 @@ export function buildControlPlaneApp(options: ControlPlaneOptions): ControlPlane void Promise.resolve() .then(() => centralNotifications.pruneLogs()) .catch(() => undefined); + // Delivered and abandoned outbox rows otherwise accumulate forever. + void Promise.resolve() + .then(() => smsOutbox.prune(30 * 24 * 60 * 60 * 1000)) + .catch(() => undefined); }, logPruneIntervalMs) : undefined; // Scheduled component backups: the tick is cheap and only writes when a period is overdue. diff --git a/apps/api/src/interface/http/console-auth-routes.ts b/apps/api/src/interface/http/console-auth-routes.ts index f9c00fc..d2d629b 100644 --- a/apps/api/src/interface/http/console-auth-routes.ts +++ b/apps/api/src/interface/http/console-auth-routes.ts @@ -33,7 +33,8 @@ const cookie = (token: string, secure: boolean): string => `${CONSOLE_SESSION_COOKIE}=${token}; Path=${COOKIE_PATH}; HttpOnly; SameSite=Strict${secure ? '; Secure' : ''}; Max-Age=${MAX_AGE_SECONDS}`; const clearCookie = (secure: boolean): string => `${CONSOLE_SESSION_COOKIE}=; Path=${COOKIE_PATH}; HttpOnly; SameSite=Strict${secure ? '; Secure' : ''}; Max-Age=0`; -const usesSecureCookie = (request: FastifyRequest): boolean => request.protocol === 'https'; +const usesSecureCookie = (request: FastifyRequest): boolean => + request.protocol === 'https' || request.headers['x-forwarded-proto'] === 'https'; const problem = (request: FastifyRequest, status: number, code: string, detail: string) => ({ type: 'about:blank', @@ -83,8 +84,26 @@ const loginSchema = { properties: { password: { type: 'string', minLength: 1, maxLength: 1024 } }, } as const; +const LOGIN_BUCKETS_MAX = 10_000; + export function registerConsoleAuth(app: FastifyInstance, auth: ConsoleAuthService): void { const loginAttempts = new Map(); + const rememberAttempt = (key: string, attempt: { count: number; resetAt: number }): void => { + // Direct-to-API callers can send arbitrary client-ip values; cap the map so + // bucket keys cannot grow without bound. + if (loginAttempts.size >= LOGIN_BUCKETS_MAX) { + const now = Date.now(); + for (const [existing, value] of loginAttempts) { + if (value.resetAt <= now) loginAttempts.delete(existing); + } + while (loginAttempts.size >= LOGIN_BUCKETS_MAX) { + const oldest = loginAttempts.keys().next().value; + if (oldest === undefined) break; + loginAttempts.delete(oldest); + } + } + loginAttempts.set(key, attempt); + }; app.addHook('preHandler', async (request, reply) => { const pathname = request.url.split(/[?#]/u, 1)[0] ?? request.url; if (!auth.isProtectedPath(pathname)) return; @@ -121,7 +140,7 @@ export function registerConsoleAuth(app: FastifyInstance, auth: ConsoleAuthServi attempts && attempts.resetAt > now ? attempts : { count: 0, resetAt: now + LOGIN_WINDOW_MS }; - loginAttempts.set(key, { count: active.count + 1, resetAt: active.resetAt }); + rememberAttempt(key, { count: active.count + 1, resetAt: active.resetAt }); return handle(request, reply, error); } }); diff --git a/packages/contracts/src/automation.ts b/packages/contracts/src/automation.ts index 943ac17..4bdac9a 100644 --- a/packages/contracts/src/automation.ts +++ b/packages/contracts/src/automation.ts @@ -417,10 +417,12 @@ export function parseCreateScheduledTaskRequest(value: unknown): CreateScheduled throw new TypeError('effectiveEndAt must be later than effectiveStartAt'); if (source.enabled !== undefined && typeof source.enabled !== 'boolean') throw new TypeError('enabled must be boolean'); + // The upstream execution baseline pins reboot delay_seconds to 3; accepting a + // wider range here would only produce tasks that can never run. const delaySeconds = source.delaySeconds === undefined ? undefined - : numberInRange(source.delaySeconds, 'delaySeconds', 0, 3_600); + : numberInRange(source.delaySeconds, 'delaySeconds', 3, 3); if (delaySeconds !== undefined && operationType !== 'reboot-system') throw new TypeError('delaySeconds is only valid for reboot-system'); return { diff --git a/public/app.js b/public/app.js index 5e56334..d64a095 100644 --- a/public/app.js +++ b/public/app.js @@ -100,7 +100,7 @@ async function runEndpoint(confirmed=false){ } catch(error){ persistentError(`请求参数无效：${error.message||error}`); return } const {owner,method,path:endpoint,body}=draft, btn=$('runEndpoint'), token=requests.next('api-console'); if(btn.disabled)return btn.dataset.idleLabel='发送请求'; setPending(btn,true,'发送中…'); $('rawOutput').textContent='请求进行中…'; persistentError(''); const started=performance.now() - try{ const init={method}; if(isWriteMethod(method)){ const prepared=await api.prepareConfirmation({instanceId:owner,method,path:endpoint,body}); if(!requests.isCurrent('api-console',token)||owner!==activeId)return; init.headers={'content-type':'application/json','x-confirmation-token':prepared.token}; if(body!==undefined)init.body=JSON.stringify(body) } const res=await api.proxy(owner,endpoint,init), text=await res.text(); if(!requests.isCurrent('api-console',token)||owner!==activeId)return; const elapsed=Math.round(performance.now()-started); outputText=text; try{outputPrettyText=JSON.stringify(JSON.parse(text),null,2)}catch{outputPrettyText=text} $('rawOutput').textContent=`HTTP ${res.status} ${res.statusText} · ${elapsed}ms · ${draft.method} ${draft.path}\n\n${$('outputPretty').getAttribute('aria-pressed')==='true'?outputPrettyText:outputText}`; if(!res.ok)persistentError(`API 请求失败：HTTP ${res.status} ${res.statusText}\n${text}`) }catch(error){ if(requests.isCurrent('api-console',token)&&owner===activeId){$('rawOutput').textContent=error.message||String(error);persistentError(`API 请求失败：${error.message||error}`)} }finally{ if(requests.isCurrent('api-console',token)) setPending(btn,false,'发送中…') } + try{ const init={method}; if(isWriteMethod(method)){ const prepared=await api.prepareConfirmation({instanceId:owner,method,path:endpoint,body}); if(!requests.isCurrent('api-console',token)||owner!==activeId)return; init.headers={'x-confirmation-token':prepared.token}; if(body!==undefined){ init.body=JSON.stringify(body); init.headers['content-type']='application/json' } } const res=await api.proxy(owner,endpoint,init), text=await res.text(); if(!requests.isCurrent('api-console',token)||owner!==activeId)return; const elapsed=Math.round(performance.now()-started); outputText=text; try{outputPrettyText=JSON.stringify(JSON.parse(text),null,2)}catch{outputPrettyText=text} $('rawOutput').textContent=`HTTP ${res.status} ${res.statusText} · ${elapsed}ms · ${draft.method} ${draft.path}\n\n${$('outputPretty').getAttribute('aria-pressed')==='true'?outputPrettyText:outputText}`; if(!res.ok)persistentError(`API 请求失败：HTTP ${res.status} ${res.statusText}\n${text}`) }catch(error){ if(requests.isCurrent('api-console',token)&&owner===activeId){$('rawOutput').textContent=error.message||String(error);persistentError(`API 请求失败：${error.message||error}`)} }finally{ if(requests.isCurrent('api-console',token)) setPending(btn,false,'发送中…') } } async function refreshStatus({silent=false}={}){ const btn=$('refreshBtn'), token=requests.next('fleet-status'); btn.disabled=true; try{const data=await api.status(); if(!requests.isCurrent('fleet-status',token))return; statuses=new Map((data.instances||[]).map(x=>[x.id,x]));lastSuccessfulRefresh=new Date();$('staleBanner').hidden=true;persistentError('');renderHomeCards();if(activeId)renderDetail();if(!silent)showToast(`已刷新 ${statuses.size} 个设备`)}catch(error){if(!requests.isCurrent('fleet-status',token))return;$('staleBanner').textContent=`刷新失败，当前显示上次成功数据。上次成功：${lastSuccessfulRefresh?lastSuccessfulRefresh.toLocaleString():'尚无'}。${error.message}`;$('staleBanner').hidden=false;persistentError(`状态刷新失败：${error.message}`)}finally{if(requests.isCurrent('fleet-status',token))btn.disabled=false} } async function loadConfig(){ const [data,cat]=await Promise.all([api.config(),api.catalog().catch(()=>({groups:[]}))]);instances=data.instances||[];$('configPath').textContent=data.configPath||'config.json';catalog=cat.groups||[];renderEndpointList();const saved=localStorage.getItem('multi-simadmin-active');activeId=saved&&instances.some(x=>x.id===saved)?saved:null;renderHomeCards() } diff --git a/server/app.js b/server/app.js index 2904733..59daf24 100644 --- a/server/app.js +++ b/server/app.js @@ -24,7 +24,7 @@ const CATALOG = [ export async function buildApp({ configStore, clientRegistry, logger = false, staticFiles = true, publicDir = path.join(ROOT, 'public'), reconcileRetryMs = 100, proxyPolicy = defaultProxyPolicy, confirmationOptions = {} } = {}) { if (!configStore || !clientRegistry) throw new TypeError('configStore and clientRegistry are required') - const app = Fastify({ logger, bodyLimit: 60 * 1024 * 1024 }) + const app = Fastify({ logger, bodyLimit: 2 * 1024 * 1024 }) const parseAuthority = raw => { const value = String(raw || '').trim().toLowerCase() if (!value || /[\s/@]/.test(value)) return null @@ -46,6 +46,11 @@ export async function buildApp({ configStore, clientRegistry, logger = false, st const configuredRawHost = String(configStore.snapshot.server.host).replace(/^\[|\]$/g, '').toLowerCase() const configuredHost = isIP(configuredRawHost) === 6 ? '::1' : configuredRawHost const configuredPort = String(configStore.snapshot.server.port) + app.addHook('onRequest', async (_request, reply) => { + reply.header('x-frame-options', 'DENY') + reply.header('content-security-policy', "frame-ancestors 'none'") + reply.header('x-content-type-options', 'nosniff') + }) app.addHook('onRequest', async (request, reply) => { const authority=loopbackAuthority(request.headers.host) const injectDefault = request.headers.host === 'localhost:80' && request.raw.socket?.localPort == null @@ -58,7 +63,14 @@ export async function buildApp({ configStore, clientRegistry, logger = false, st } }) app.addContentTypeParser('application/octet-stream', { parseAs: 'buffer' }, (_request, body, done) => done(null, body)) - app.addContentTypeParser(/^multipart\//, { parseAs: 'buffer' }, (_request, body, done) => done(null, body)) + app.addContentTypeParser('application/x-www-form-urlencoded', { parseAs: 'string' }, (_request, body, done) => done(null, body)) + // Unparsable content types (e.g. multipart) must fail with the same stable body + // the proxy policy would have produced, without buffering the payload first. + app.setErrorHandler((error, _request, reply) => { + if (error?.code === 'FST_ERR_CTP_INVALID_MEDIA_TYPE') return reply.code(415).send({ error: 'unsupported proxy content type' }) + if (error?.code === 'FST_ERR_CTP_EMPTY_JSON_BODY') return reply.code(400).send({ error: 'empty JSON body' }) + reply.send(error) + }) if (staticFiles) await app.register(fastifyStatic, { root: publicDir, prefix: '/' }) const findClient = (id, reply) => { const client = clientRegistry.get(id) @@ -101,7 +113,11 @@ export async function buildApp({ configStore, clientRegistry, logger = false, st app.post('/api/instances/:id/login', async (request, reply) => { const client = findClient(request.params.id, reply); if (!client) return const supplied = Object.hasOwn(request.body || {}, 'password') - return { ...(await client.ensureAuthenticated(supplied ? { credential: request.body.password } : {})), hasSavedPassword: Boolean(configStore.snapshot.instances.find(item => item.id === request.params.id)?.auth?.password) } + try { + return { ...(await client.ensureAuthenticated(supplied ? { credential: request.body.password } : {})), hasSavedPassword: Boolean(configStore.snapshot.instances.find(item => item.id === request.params.id)?.auth?.password) } + } catch (error) { + return reply.code(502).send({ error: 'upstream request failed' }) + } }) app.post('/api/instances/:id/logout', async (request, reply) => { const client = findClient(request.params.id, reply); if (!client) return diff --git a/server/core.js b/server/core.js index 0d78386..249961a 100644 --- a/server/core.js +++ b/server/core.js @@ -1,7 +1,13 @@ export { normalizeBaseUrl, normalizeInstance } from './config/schema.js' -export const DEFAULT_TIMEOUT_MS = Number(process.env.MULTI_SIMADMIN_TIMEOUT_MS || 6000) +function parsePositiveIntEnv(value, fallback) { + const parsed = Number(value) + return Number.isFinite(parsed) && parsed > 0 ? parsed : fallback +} + +export const DEFAULT_TIMEOUT_MS = parsePositiveIntEnv(process.env.MULTI_SIMADMIN_TIMEOUT_MS, 6000) +export const DEFAULT_MAX_BODY_BYTES = parsePositiveIntEnv(process.env.MULTI_SIMADMIN_MAX_BODY_BYTES, 10 * 1024 * 1024) export function redactInstance(instance) { return { id: instance.id, @@ -165,7 +171,6 @@ export function createSimAdminClient(instance, { fetchImpl = fetch, timeoutMs = const headers = new Headers(options.headers || {}) const cookie = jar.header() if (cookie && !headers.has('cookie')) headers.set('cookie', cookie) - if (headers.has('cookie')) headers.cookie = headers.get('cookie') try { const response = await fetchImpl(url, { ...options, headers, signal: controller.signal, redirect: options.redirect || 'manual' }) jar.setFromHeaders(response.headers) @@ -178,7 +183,7 @@ export function createSimAdminClient(instance, { fetchImpl = fetch, timeoutMs = async function fetchJson(endpoint, options = {}) { const startedAt = Date.now() const response = await request(endpoint, { method: 'GET', ...options, headers: { accept: 'application/json,text/plain,*/*', ...(options.headers || {}) } }) - const text = await response.text() + const text = (await readResponseBody(response, { bodyTimeoutMs: options.timeoutMs || timeoutMs })).toString('utf8') let data = null try { data = text ? JSON.parse(text) : null } catch {} return { ok: response.ok, status: response.status, latencyMs: Date.now() - startedAt, data, text: data ? undefined : text.slice(0, 1000), headers: response.headers } @@ -212,6 +217,36 @@ export function createSimAdminClient(instance, { fetchImpl = fetch, timeoutMs = return { instance, jar, request, fetchJson, ensureAuthenticated, clearEphemeralSecret } } -export function buildClients(instances, options = {}) { - return new Map(instances.map(instance => [instance.id, createSimAdminClient(instance, options)])) +// The header timeout ends once headers arrive; body reads need their own +// deadline and a byte budget so a stalled or hostile upstream can neither hang +// status aggregation nor exhaust memory. +export async function readResponseBody(response, { maxBytes = DEFAULT_MAX_BODY_BYTES, bodyTimeoutMs = DEFAULT_TIMEOUT_MS } = {}) { + const reader = response.body?.getReader() + if (!reader) return Buffer.alloc(0) + const chunks = [] + let size = 0 + let failure = null + const timer = setTimeout(() => { + failure = new Error('upstream response body timed out') + reader.cancel(failure).catch(() => {}) + }, bodyTimeoutMs) + try { + while (true) { + const { done, value } = await reader.read() + if (done) break + size += value.byteLength + if (size > maxBytes) { + failure = new Error('upstream response body exceeded the size limit') + throw failure + } + chunks.push(Buffer.from(value)) + } + } finally { + clearTimeout(timer) + } + if (failure) { + await reader.cancel(failure).catch(() => {}) + throw failure + } + return Buffer.concat(chunks) } diff --git a/server/proxy/policy.js b/server/proxy/policy.js index f273a58..b0a2905 100644 --- a/server/proxy/policy.js +++ b/server/proxy/policy.js @@ -2,7 +2,7 @@ const AUTH_PATH = /^\/api\/(?:auth(?:\/|$)|login(?:\/|$)|logout(?:\/|$))/i export const DEFAULT_READ_PATHS = Object.freeze([ '/api/health', '/api/device', '/api/sim', '/api/network', '/api/stats', '/api/connectivity', - '/api/sms/stats', '/api/sms/list', '/api/sms/send', '/api/sms/conversation', + '/api/sms/stats', '/api/sms/list', '/api/sms/conversation', '/api/data', '/api/roaming', '/api/airplane-mode', '/api/radio-mode', '/api/band-lock', '/api/cell-lock', '/api/apn', '/api/cells', '/api/device-network/ddns/status', '/api/device-network/ddns/config', '/api/device-network/wlan/status', '/api/device-network/wlan/profiles', '/api/calls', '/api/call/history', '/api/ims/status', '/api/voicemail/status', diff --git a/server/proxy/service.js b/server/proxy/service.js index 4c36d2e..098bfd8 100644 --- a/server/proxy/service.js +++ b/server/proxy/service.js @@ -1,3 +1,5 @@ +import { readResponseBody } from '../core.js' + const BLOCKED_REQUEST_HEADERS = new Set([ 'connection', 'keep-alive', 'proxy-authenticate', 'proxy-authorization', 'te', 'trailer', 'transfer-encoding', 'upgrade', 'host', 'content-length', 'cookie', 'authorization', @@ -63,5 +65,5 @@ export async function proxyToInstance({ client, request, reply, rest }) { upstream.headers.forEach((value, name) => { if (!BLOCKED_RESPONSE_HEADERS.has(name.toLowerCase())) reply.header(name, value) }) - return reply.send(Buffer.from(await upstream.arrayBuffer())) + return reply.send(await readResponseBody(upstream)) } diff --git a/test/server-core.test.js b/test/server-core.test.js index f62a0f4..ee300fa 100644 --- a/test/server-core.test.js +++ b/test/server-core.test.js @@ -35,7 +35,7 @@ test('client stores simadmin_session from login and sends it on later proxied re assert.equal(auth.authenticated, true) const proxied = await client.fetchJson('/api/device') assert.equal(proxied.status, 200) - assert.match(calls.at(-1).options.headers.cookie, /simadmin_session=abc123/) + assert.match(calls.at(-1).options.headers.get('cookie'), /simadmin_session=abc123/) }) test('snapshot summary exposes device-monitoring fields from stats, sms, data and hardware endpoints', () => {