From 205566e1eeba87242bec011d7b3e39f3f2fe447c Mon Sep 17 00:00:00 2001 From: hreyes Date: Thu, 30 Jul 2026 15:03:35 -0600 Subject: [PATCH 1/3] fix(versiones-cras): validar que la URL sembrada sea alcanzable y permitir instalar limpio MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Dos bloqueos que impedían avanzar con la instalación en Linux. 1. La validación de PANEL_PUBLIC_URL solo comparaba que coincidiera con ORIGIN, y eso deja pasar el peor caso: que AMBAS valgan localhost, que es justo lo que produce el compose de desarrollo. Ahí no avisaba nada y la instalación sembraba loopback en el config/.env del destino, donde localhost es el propio destino y el agente acabaría hablando consigo mismo. Ahora se valida lo que de verdad importa —que la URL sea alcanzable desde otra máquina— y solo después el desajuste con ORIGIN. El mensaje también dejaba un consejo peligroso: "comenta PANEL_PUBLIC_URL para que tome ORIGIN" solo vale si ORIGIN sirve para sembrar; con ORIGIN en localhost, seguirlo empeora las cosas. Ese consejo ahora es condicional. De paso se corrige el docstring, que afirmaba que un ORIGIN equivocado se habría manifestado por la protección CSRF de adapter-node. No es cierto: svelte.config.js tiene csrf.checkOrigin en false, así que el Origin de los POST nunca se valida y ese razonamiento llevaba a conclusiones falsas. 2. El formulario fuerza mode='update' en cuanto el servidor tiene versión instalada, y una actualización exige que en la ruta destino ya viva algo. Eso hacía imposible mover una instalación a otra carpeta —por ejemplo al home, para instalar sin privilegios—: abortaba con "no hay una instalación". Se agrega una confirmación explícita siguiendo el molde de platformAck: el ack lleva la RUTA confirmada y no un booleano, así que una casilla marcada deja de valer si después se cambia el destino, y el servidor revalida en vez de confiar en la UI. La casilla advierte además que el agente anterior sigue corriendo: los dos reportarían con el mismo instance_key y se pisarían la carpeta de entrada registrada, que es por donde el panel enruta los respaldos. Co-Authored-By: Claude Opus 5 (1M context) --- src/lib/cras-version.test.ts | 68 +++++++++++++++++++ src/lib/cras-version.ts | 62 +++++++++++++++++ src/routes/versiones-cras/+page.server.ts | 83 +++++++++++++++++++---- src/routes/versiones-cras/+page.svelte | 61 +++++++++++++++++ 4 files changed, 259 insertions(+), 15 deletions(-) diff --git a/src/lib/cras-version.test.ts b/src/lib/cras-version.test.ts index 3b8f4cb..7721d04 100644 --- a/src/lib/cras-version.test.ts +++ b/src/lib/cras-version.test.ts @@ -12,7 +12,9 @@ import { effectivePlatform, installPlatformVerdict, isCrasPlatform, + isCleanInstallConfirmed, isInsideWorkFolder, + unseedablePanelUrlReason, isNewer, isValidVersion, osToPlatform, @@ -260,3 +262,69 @@ describe('isInsideWorkFolder', () => { expect(isInsideWorkFolder('', ['/srv/entrada'])).toBe(false); }); }); + +/** + * URL del panel que se siembra en el agente. + * + * El aviso original solo comparaba que PANEL_PUBLIC_URL y ORIGIN coincidieran, y eso deja pasar + * el peor caso: que ambas valgan localhost, que es justo lo que produce el compose de desarrollo. + * Lo que hay que exigir es que la URL sea alcanzable desde OTRA máquina. + */ +describe('unseedablePanelUrlReason', () => { + it('rechaza loopback en todas sus formas, aunque coincida con ORIGIN', () => { + // Es el hueco que se está cerrando: en el servidor destino, localhost es ese servidor. + expect(unseedablePanelUrlReason('https://localhost:3000')).toMatch(/localhost/); + expect(unseedablePanelUrlReason('http://127.0.0.1:3000')).toMatch(/127\.0\.0\.1/); + expect(unseedablePanelUrlReason('http://127.1.2.3:3000')).not.toBeNull(); + expect(unseedablePanelUrlReason('http://[::1]:3000')).not.toBeNull(); + expect(unseedablePanelUrlReason('http://0.0.0.0:3000')).not.toBeNull(); + }); + + it('acepta una URL alcanzable, con y sin puerto', () => { + expect(unseedablePanelUrlReason('https://cpanel-a24.aduanasoft.com:3000')).toBeNull(); + // Sin puerto también pasa: el problema de esa es el desajuste con ORIGIN, que es un + // aviso distinto. Mezclar los dos haría imposible saber cuál corregir. + expect(unseedablePanelUrlReason('https://cpanel-a24.aduanasoft.com')).toBeNull(); + expect(unseedablePanelUrlReason('http://10.0.0.5:3000')).toBeNull(); + }); + + it('rechaza vacío, basura y protocolos que el agente no habla', () => { + expect(unseedablePanelUrlReason('')).toMatch(/no está configurada/); + expect(unseedablePanelUrlReason(null)).toMatch(/no está configurada/); + expect(unseedablePanelUrlReason('cpanel-a24.aduanasoft.com:3000')).not.toBeNull(); + expect(unseedablePanelUrlReason('ftp://cpanel-a24.aduanasoft.com')).toMatch(/ftp/); + }); +}); + +/** + * Confirmación de instalación limpia en otra ruta. + * + * Sin esto no se puede mover una instalación existente —por ejemplo al home, para instalar sin + * privilegios—: el panel pide `update` en cuanto hay versión instalada y assertExistingInstall + * exige que ya viva algo en la ruta destino. + */ +describe('isCleanInstallConfirmed', () => { + it('solo confirma si el ack es exactamente la ruta pedida', () => { + expect(isCleanInstallConfirmed('/home/srvmid_db/cras', '/home/srvmid_db/cras')).toBe(true); + expect(isCleanInstallConfirmed('/home/srvmid_db/cras', '/opt/cloudrestoreas')).toBe(false); + }); + + it('un ack de otra ruta NO sirve: la casilla no queda encendida si se cambia el destino', () => { + // Es la razón de que el ack lleve la ruta y no un booleano. + expect(isCleanInstallConfirmed('/home/otro/cras', '/home/srvmid_db/cras')).toBe(false); + expect(isCleanInstallConfirmed('/opt/cloudrestoreas', 'true')).toBe(false); + expect(isCleanInstallConfirmed('/opt/cloudrestoreas', 'on')).toBe(false); + }); + + it('tolera el separador final y los espacios, que no cambian la ruta', () => { + expect(isCleanInstallConfirmed('/home/srvmid_db/cras', '/home/srvmid_db/cras/')).toBe(true); + expect(isCleanInstallConfirmed('C:\\Aduanasoft\\CRAS', ' C:\\Aduanasoft\\CRAS\\ ')).toBe(true); + }); + + it('sin ack, o sin ruta pedida, nunca confirma', () => { + expect(isCleanInstallConfirmed('/opt/cloudrestoreas', null)).toBe(false); + expect(isCleanInstallConfirmed('/opt/cloudrestoreas', '')).toBe(false); + expect(isCleanInstallConfirmed('', '')).toBe(false); + expect(isCleanInstallConfirmed(null, null)).toBe(false); + }); +}); diff --git a/src/lib/cras-version.ts b/src/lib/cras-version.ts index 8914083..b2a8b2d 100644 --- a/src/lib/cras-version.ts +++ b/src/lib/cras-version.ts @@ -218,6 +218,68 @@ export function isInsideWorkFolder( }); } +/** Loopback en cualquiera de sus formas. `URL.hostname` devuelve IPv6 entre corchetes. */ +function isLoopbackHost(hostname: string): boolean { + const h = hostname.trim().toLowerCase().replace(/^\[|\]$/g, ''); + return h === 'localhost' || h === '::1' || h === '0.0.0.0' || /^127\./.test(h); +} + +/** + * Motivo por el que una URL del panel NO se puede sembrar en un agente, o null si sirve. + * + * Lo que importa no es que `PANEL_PUBLIC_URL` y `ORIGIN` coincidan, sino que la URL sea + * **alcanzable desde otra máquina**: se escribe en el config/.env del servidor destino, al + * instalar y también al actualizar. Comparar solo que coincidan deja pasar el peor caso — que + * ambas valgan `localhost`, que es lo que produce el compose de desarrollo—, y ahí cada agente + * acabaría hablando consigo mismo en su propio servidor. + */ +export function unseedablePanelUrlReason(url: string | null | undefined): string | null { + const value = String(url ?? '').trim(); + if (!value) return 'no está configurada'; + + let parsed: URL; + try { + parsed = new URL(value); + } catch { + return `no es una URL válida (${value})`; + } + if (parsed.protocol !== 'http:' && parsed.protocol !== 'https:') { + return `usa el protocolo ${parsed.protocol.replace(':', '')} y el agente habla HTTP(S)`; + } + if (isLoopbackHost(parsed.hostname)) { + return `apunta a ${parsed.hostname}, que desde el servidor destino es ese mismo servidor`; + } + return null; +} + +/** Normaliza una ruta para compararla: sin separador final y sin espacios alrededor. */ +function normalizeInstallPath(path: string | null | undefined): string { + return String(path ?? '') + .trim() + .replace(/[/\\]+$/, ''); +} + +/** + * ¿El operador confirmó que quiere una instalación LIMPIA en esta ruta? + * + * Cuando un servidor ya tiene una versión instalada, el panel pide `update`, y una actualización + * exige que en la ruta destino ya viva una instalación (`assertExistingInstall`). Esa guarda es + * correcta —evita crear una segunda instalación con la configuración por omisión y dejar huérfano + * el .env personalizado—, pero bloquea el caso legítimo de mover la instalación a otra ruta, por + * ejemplo al home para instalar sin privilegios. + * + * El ack lleva la RUTA confirmada y no un booleano, igual que `installPlatformVerdict` lleva la + * plataforma: así una casilla que quedó marcada no sigue valiendo si después se cambia la ruta. + */ +export function isCleanInstallConfirmed( + requestedInstallPath: string | null | undefined, + ack: string | null | undefined +): boolean { + const requested = normalizeInstallPath(requestedInstallPath); + if (!requested) return false; + return normalizeInstallPath(ack) === requested; +} + /** Etiqueta legible para la UI. */ export function platformLabel(platform: string | null | undefined): string { if (platform === 'windows') return 'Windows'; diff --git a/src/routes/versiones-cras/+page.server.ts b/src/routes/versiones-cras/+page.server.ts index 6d5e7eb..9301e3b 100644 --- a/src/routes/versiones-cras/+page.server.ts +++ b/src/routes/versiones-cras/+page.server.ts @@ -35,10 +35,13 @@ import { type PrunePlan } from '$lib/server/cras-artifacts'; import { + effectiveInstallPath, installPlatformVerdict, + isCleanInstallConfirmed, isInsideWorkFolder, isSafeInstallPath, - platformLabel + platformLabel, + unseedablePanelUrlReason } from '$lib/cras-version'; import { setCloudRestoreInstallPath } from '$lib/server/controldesk-pg'; import { syncCrasReleasesFromGitea } from '$lib/server/cras-sync'; @@ -77,20 +80,41 @@ function panelApiToken(): string { } /** - * Aviso cuando PANEL_PUBLIC_URL apunta a un origen distinto del que sirve el panel. + * Problemas de la URL del panel que se siembra en el agente. * - * Esta URL se siembra en el config/.env del agente, y el instalador la reescribe en CADA + * Esta URL se escribe en el config/.env del destino, y el instalador la reescribe en CADA * actualización, no solo al instalar. Una URL equivocada no falla al capturarla: rompe el servidor * la próxima vez que se actualice, y de golpe —el agente deja de reportar y de resolver rutas de * restauración—. En producción le faltaba el puerto y el 443 de ese host lo sirve otro servicio, * así que todo lo que pidiera el agente habría caído en un 404 ajeno. * - * Se compara contra ORIGIN porque el navegador ya lo validó: adapter-node rechaza los POST cuyo - * Origin no cuadre, así que si la pantalla responde, ORIGIN es el origen real del panel. + * Se revisan DOS cosas distintas, en este orden: + * + * 1. Que la URL efectiva sea **sembrable** (alcanzable desde otra máquina). Es lo que de verdad + * rompe, y no se puede deducir de que coincida con ORIGIN. + * 2. Que PANEL_PUBLIC_URL y ORIGIN coincidan, como señal de que una de las dos está mal escrita. + * + * Ojo con una tentación: NO se puede suponer que ORIGIN es correcto porque la pantalla responde. + * `svelte.config.js` tiene `csrf.checkOrigin: false`, así que adapter-node no valida el Origin de + * los POST y un ORIGIN equivocado nunca se manifiesta por esa vía. */ -function panelUrlOriginMismatch(): string | null { +function panelUrlProblem(): string | null { const declared = (env.PANEL_PUBLIC_URL || '').trim(); const origin = (env.ORIGIN || '').trim(); + const effective = declared || origin; + + // Primero lo que de verdad rompe: que la URL sembrada no sea alcanzable desde otra máquina. + // Comparar solo que declared y origin coincidan dejaba pasar el peor caso —ambas en + // localhost, que es lo que produce el compose de desarrollo— sin decir nada. + const unseedable = unseedablePanelUrlReason(effective); + if (unseedable) { + const which = declared ? 'PANEL_PUBLIC_URL' : 'ORIGIN'; + return ( + `La URL que se sembrará en el agente (${which}) ${unseedable}. Se escribe en el ` + + 'config/.env del servidor destino al instalar y al actualizar, así que tiene que ser ' + + 'alcanzable DESDE ese servidor.' + ); + } if (!declared || !origin) return null; let declaredUrl: URL; @@ -103,11 +127,18 @@ function panelUrlOriginMismatch(): string | null { } // `host` incluye el puerto, que es justo lo que se omitió en producción. if (declaredUrl.host === originUrl.host) return null; + + // El consejo de "comenta PANEL_PUBLIC_URL y toma ORIGIN" solo vale si ORIGIN sirve para + // sembrar. Darlo a ciegas cuando ORIGIN es loopback empeora las cosas: sembraría localhost. + const originUnseedable = unseedablePanelUrlReason(origin); return ( `PANEL_PUBLIC_URL (${declaredUrl.origin}) no coincide con el origen del panel ` + `(${originUrl.origin}). Es la URL que se siembra en el agente al instalar y al actualizar, ` + - 'así que apuntaría a otro servicio. Corrige el puerto o comenta PANEL_PUBLIC_URL para que ' + - 'tome ORIGIN.' + 'así que apuntaría a otro servicio. ' + + (originUnseedable + ? `Y ORIGIN tampoco sirve para sembrar: ${originUnseedable}. Corrige AMBAS con una URL ` + + 'alcanzable desde los servidores de restauración, incluyendo el puerto.' + : 'Corrige el puerto, o comenta PANEL_PUBLIC_URL para que tome ORIGIN.') ); } @@ -200,9 +231,9 @@ export const load: PageServerLoad = async ({ cookies }) => { 'PANEL_PUBLIC_URL (u ORIGIN) no está configurado: el agente no sabría a qué URL reportar.' ); } - const originMismatch = panelUrlOriginMismatch(); - if (originMismatch) { - configWarnings.push(originMismatch); + const urlProblem = panelUrlProblem(); + if (urlProblem) { + configWarnings.push(urlProblem); } return { @@ -491,7 +522,7 @@ export const actions: Actions = { if (!targetId || !releaseId) { return fail(400, { error: 'Servidor o versión inválidos.' }); } - const mode = modeRaw === 'update' ? 'update' : 'install'; + const requestedMode = modeRaw === 'update' ? 'update' : 'install'; // Lista blanca explícita en vez de una cadena de ternarios: con cuatro modos, el patrón // anterior convertía cualquier valor no reconocido en 'service', que es justo el que // exige privilegios. Un formulario viejo pediría el modo más restrictivo sin quererlo. @@ -561,9 +592,31 @@ export const actions: Actions = { // Se aborta antes de tocar el servidor: esta URL se escribe sobre el config/.env del // destino también al ACTUALIZAR, así que dejarla pasar rompería un agente que hoy sí // reporta. Es más barato corregir el .env del panel que reparar el del servidor. - const originMismatch = panelUrlOriginMismatch(); - if (originMismatch) { - return fail(500, { error: originMismatch }); + const urlProblem = panelUrlProblem(); + if (urlProblem) { + return fail(500, { error: urlProblem }); + } + + // Instalación LIMPIA en otra ruta. Sin esto no se puede mover una instalación existente + // —por ejemplo al home, para instalar sin privilegios—: el formulario pide `update` en + // cuanto hay versión instalada, y una actualización exige que en la ruta destino ya viva + // algo (assertExistingInstall). El ack lleva la RUTA confirmada, no un booleano, así que + // una casilla marcada deja de valer si después se cambia el destino. + const resolvedPath = effectiveInstallPath(target.reported_install_path, target.platform); + const cleanInstall = + requestedMode === 'update' && + isCleanInstallConfirmed(resolvedPath, data.get('clean_install_ack')?.toString()); + const mode = cleanInstall ? 'install' : requestedMode; + if (cleanInstall) { + logger.warn({ + message: 'Instalación limpia confirmada por el operador sobre un servidor con versión previa', + context: { + target: target.name, + install_path: resolvedPath, + previous_version: target.installed_version, + confirmed_by: currentUser.username + } + }); } try { diff --git a/src/routes/versiones-cras/+page.svelte b/src/routes/versiones-cras/+page.svelte index d7a872a..d22b4c5 100644 --- a/src/routes/versiones-cras/+page.svelte +++ b/src/routes/versiones-cras/+page.svelte @@ -56,6 +56,20 @@ let installing = $state(false); /** Confirmación explícita de plataforma cuando el panel no la pudo determinar. */ let platformAck = $state(false); + /** Confirmación de instalar LIMPIO en la ruta elegida, sobre un servidor que ya tiene versión. */ + let cleanInstallAck = $state(false); + + /** + * Ruta donde instalará el panel: la misma que resuelve el servidor con effectiveInstallPath. + * Se muestra en vez del valor crudo porque lo que el operador necesita confirmar es dónde va + * a caer la instalación, no si la columna está en null. + */ + let targetInstallPath = $derived( + installTarget + ? (effectiveInstallPath(installTarget.reported_install_path, installTarget.platform) ?? + '(sin determinar)') + : '' + ); let installOptions = $derived( installTarget ? releasesFor(installTarget.platform) : [] @@ -76,6 +90,7 @@ installTarget = target; installAutostart = 'service'; platformAck = false; + cleanInstallAck = false; // Se preselecciona SOLO si el panel sabe la plataforma del servidor. Sin plataforma no se // adivina: `releasesFor(null)` es TODO el catálogo, ordenado por descubrimiento, así que // el fallback anterior proponía el artefacto más reciente de cualquier plataforma — un @@ -1144,6 +1159,13 @@ name="mode" value={target.installed_version ? 'update' : 'install'} /> + + {/if} + + + {#if target.installed_version} +
+

+ warning_amber + ¿Instalar limpio en otra ruta? +

+

+ Actualizar exige que en {targetInstallPath} ya viva una + instalación. Si vas a mover el agente a otra carpeta —por ejemplo al + home, para instalar sin privilegios— marca esto: se instalará como + nuevo en esa ruta. +

+ + {#if cleanInstallAck} +

+ Ojo: el agente anterior sigue corriendo. Los dos + reportarían con el mismo nombre de instancia y se pisarían la + carpeta de entrada registrada, que es por donde el panel enruta + los respaldos. Detén el viejo en el servidor: + sudo systemctl disable --now cloudrestoreas +

+ {/if} +
+ {/if}
-- 2.49.1 From eda355471d50b26f02d1f655ec35ae2a5551cd9a Mon Sep 17 00:00:00 2001 From: hreyes Date: Thu, 30 Jul 2026 15:40:58 -0600 Subject: [PATCH 2/3] feat(cras-install): actualizar sin privilegios cuando no hay root ni sudo -n MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit En entornos donde no se usa root, ninguna de las dos vías que había servía: la elevación abortaba con 409 y `user-service` exige mover la instalación al home, que en un servidor con el agente ya instalado en /opt pide un paso privilegiado para retirar el unit viejo. Se agrega una tercera vía para ACTUALIZACIONES: dejar el unit como está y solo reemplazar el binario, que es lo único que una actualización necesita. Corre `install.sh --update-in-place` sin prefijo de elevación. Tres precondiciones, comprobadas por SSH ANTES de subir 270 MB, cada una con su propio motivo de rechazo porque cada una tiene un remedio distinto: - El directorio de instalación debe ser escribible por la cuenta SSH. Es el único permiso que hace falta: `install` desvincula el destino antes de crearlo, así que un binario en ejecución no es obstáculo (eso es cosa de `cp`). - El unit debe correr con ese mismo usuario. Si quedó con User=root —alguien instaló desde un `sudo -i`— la cuenta no puede señalizar el proceso. Un User vacío se trata como root, que es lo que hace systemd. - El unit debe tener Restart=always, que es quien vuelve a levantarlo. Sin eso, señalizarlo lo dejaría muerto. Y se rechaza si hay una restauración en curso, aquí y otra vez en el destino. Dos correcciones de robustez en la verificación posterior: - El sello config/.version se sondea en vez de leerse una vez. En esta vía se omite el bootstrap y el sello lo escribe el proceso al reiniciarse, así que durante unos segundos sigue teniendo la versión ANTERIOR: la lectura única convertía una actualización correcta en "reporta 1.0.0, se esperaba 1.1.1". - `systemctl is-active` puede devolver 'activating' justo tras el reinicio, así que la evidencia que manda es el proceso vivo con el binario nuevo. El código 75 (EX_TEMPFAIL) de install.sh se traduce a un 409 con el motivo real —"está restaurando, reintenta"— en vez del 502 genérico que hacía pensar que la instalación se había roto. Co-Authored-By: Claude Opus 5 (1M context) --- src/lib/server/cras-install.test.ts | 104 ++++++++++++++++ src/lib/server/cras-install.ts | 180 ++++++++++++++++++++++++---- 2 files changed, 263 insertions(+), 21 deletions(-) diff --git a/src/lib/server/cras-install.test.ts b/src/lib/server/cras-install.test.ts index de0271b..8cc9b07 100644 --- a/src/lib/server/cras-install.test.ts +++ b/src/lib/server/cras-install.test.ts @@ -9,6 +9,7 @@ import { describe, expect, it, vi } from 'vitest'; import { execRemote, + probeInPlaceUpdate, probeLinuxElevation, psEncoded, shQuote, @@ -294,3 +295,106 @@ describe('probeLinuxElevation', () => { } }); }); + +/** + * Precondiciones de la actualización en sitio sin privilegios. + * + * Cada rechazo tiene un remedio distinto, y por eso no basta un booleano. Lo que se prueba aquí + * son los NO: el sí es trivial y el daño está en aceptar un servidor donde no se puede. + */ +describe('probeInPlaceUpdate', () => { + const PREFIX = '/opt/cloudrestoreas'; + + function sftpFor(responses: Record) { + return fakeSftp((command) => { + for (const [needle, result] of Object.entries(responses)) { + if (command.includes(needle)) return result; + } + return { code: 0, stdout: '' }; + }); + } + + /** Destino ideal: ruta del usuario, unit suyo con Restart=always, sin jobs. */ + const OK = { + 'test -w': { code: 0, stdout: 'si' }, + 'systemctl show': { code: 0, stdout: 'User=srvmid_db\nRestart=always' }, + '/Temp': { code: 0, stdout: '' } + }; + + it('acepta cuando se cumplen las tres condiciones', async () => { + const { sftp } = sftpFor(OK); + expect(await probeInPlaceUpdate(sftp as never, PREFIX, 'srvmid_db')).toMatchObject({ ok: true }); + }); + + it('rechaza si el directorio no es escribible, y dice de quién es', async () => { + // Sin escritura en el DIRECTORIO no se puede reemplazar el binario, y es el único + // permiso que hace falta: `install` desvincula antes de crear. + const { sftp } = sftpFor({ + ...OK, + 'test -w': { code: 1, stdout: 'no' }, + 'stat -c': { code: 0, stdout: 'root' } + }); + const r = await probeInPlaceUpdate(sftp as never, PREFIX, 'srvmid_db'); + expect(r.ok).toBe(false); + expect(r.reason).toContain('root'); + expect(r.reason).toContain(PREFIX); + }); + + it('rechaza si el servicio corre como otro usuario', async () => { + // Si el unit quedó con User=root —alguien instaló desde un `sudo -i`— la cuenta SSH no + // puede señalizar el proceso y toda la vía se cae. + const { sftp } = sftpFor({ + ...OK, + 'systemctl show': { code: 0, stdout: 'User=root\nRestart=always' } + }); + const r = await probeInPlaceUpdate(sftp as never, PREFIX, 'srvmid_db'); + expect(r.ok).toBe(false); + expect(r.reason).toMatch(/corre como 'root'/); + }); + + it('trata el User vacío como root, que es lo que hace systemd', async () => { + const { sftp } = sftpFor({ + ...OK, + 'systemctl show': { code: 0, stdout: 'User=\nRestart=always' } + }); + const r = await probeInPlaceUpdate(sftp as never, PREFIX, 'srvmid_db'); + expect(r.ok).toBe(false); + expect(r.reason).toMatch(/root/); + }); + + it('rechaza sin Restart=always: matarlo lo dejaría muerto', async () => { + for (const restart of ['on-failure', 'no', '']) { + const { sftp } = sftpFor({ + ...OK, + 'systemctl show': { code: 0, stdout: `User=srvmid_db\nRestart=${restart}` } + }); + const r = await probeInPlaceUpdate(sftp as never, PREFIX, 'srvmid_db'); + expect(r.ok, `Restart=${restart}`).toBe(false); + expect(r.reason).toMatch(/Restart=always/); + } + }); + + it('rechaza si hay una restauración en curso, antes de subir 270 MB', async () => { + // El mismo chequeo lo repite install.sh en el destino, pero hacerlo aquí evita la + // transferencia completa para nada. + const { sftp } = sftpFor({ ...OK, '/Temp': { code: 0, stdout: 'job-4212' } }); + const r = await probeInPlaceUpdate(sftp as never, PREFIX, 'srvmid_db'); + expect(r.ok).toBe(false); + expect(r.reason).toMatch(/restauración en curso/); + }); + + it('el motivo del rechazo nunca queda vacío: cada caso tiene su remedio', async () => { + const casos = [ + { ...OK, 'test -w': { code: 1, stdout: 'no' } }, + { ...OK, 'systemctl show': { code: 0, stdout: 'User=otro\nRestart=always' } }, + { ...OK, 'systemctl show': { code: 0, stdout: 'User=srvmid_db\nRestart=no' } }, + { ...OK, '/Temp': { code: 0, stdout: 'job-1' } } + ]; + for (const caso of casos) { + const { sftp } = sftpFor(caso); + const r = await probeInPlaceUpdate(sftp as never, PREFIX, 'srvmid_db'); + expect(r.ok).toBe(false); + expect(r.reason.trim().length).toBeGreaterThan(20); + } + }); +}); diff --git a/src/lib/server/cras-install.ts b/src/lib/server/cras-install.ts index d180259..652510d 100644 --- a/src/lib/server/cras-install.ts +++ b/src/lib/server/cras-install.ts @@ -504,21 +504,37 @@ async function installLinux( // antes de transferir 270 MB, porque fallar después es desperdicio y deja basura en /tmp. const autostartMode = request.autostart ?? 'service'; const privileged = await probeLinuxElevation(sftp); - if (autostartMode !== 'user-service' && !privileged.prefix && privileged.elevation !== 'root') { - throw new InstallError( - 409, - `El usuario '${target.ssh_username}' no puede elevar privilegios en este servidor: ` + - `${privileged.detail} Para instalar en ${installPath} y registrar el servicio de ` + - 'sistema hacen falta. Alternativas: usar una cuenta root, dar NOPASSWD a ese ' + - 'usuario, o elegir el arranque "Servicio de usuario", que instala en el home y no ' + - 'necesita privilegios.' - ); + const canElevate = privileged.elevation === 'root' || privileged.elevation === 'sudo-sin-password'; + + // Sin elevación quedan dos vías antes de rendirse, y ambas evitan pedirle la contraseña a + // sudo. `user-service` instala en el home; la actualización en sitio deja el unit como está + // y solo reemplaza el binario, que es lo único que una actualización necesita de verdad. + let inPlaceUpdate = false; + if (!canElevate && autostartMode !== 'user-service') { + if (request.mode !== 'update') { + throw new InstallError( + 409, + `El usuario '${target.ssh_username}' no puede elevar privilegios: ` + + `${privileged.detail} Una instalación NUEVA en ${installPath} necesita ` + + 'privilegios para registrar el servicio de sistema. Usa el arranque ' + + '"Servicio de usuario", que instala en el home y no los necesita.' + ); + } + const inPlace = await probeInPlaceUpdate(sftp, installPath, target.ssh_username); + if (!inPlace.ok) { + throw new InstallError( + 409, + `No se puede actualizar ${target.name} sin privilegios: ${inPlace.reason}` + ); + } + inPlaceUpdate = true; } await appendInstallStep( runId, 'precondiciones', true, - `${systemEvidence}, privilegios=${privileged.label}` + `${systemEvidence}, privilegios=${privileged.label}` + + (inPlaceUpdate ? ', vía=actualización en sitio sin privilegios' : '') ); // Antes de transferir: si es una actualización, confirmar que ahí VIVE una instalación. @@ -591,7 +607,13 @@ async function installLinux( } const autostart = request.autostart ?? 'service'; - const installerFlags = autostart === 'none' ? '' : ` --${autostart}`; + // En la vía sin privilegios se usa --update-in-place, que no toca /etc ni systemctl: + // solo reemplaza el binario y señaliza al proceso para que Restart=always lo relevante. + const installerFlags = inPlaceUpdate + ? ' --update-in-place' + : autostart === 'none' + ? '' + : ` --${autostart}`; // PREFIX es variable de entorno en install.sh, no un flag. La ruta no es secreta, así // que pasarla por la línea de comandos está bien; el token sí va por archivo. const installCmd = @@ -601,6 +623,17 @@ async function installLinux( await appendInstallStep(runId, 'ejecutar-instalador', true, `install.sh${installerFlags}`); const install = await execRemote(sftp, installCmd, INSTALL_EXEC_TIMEOUT_MS); + // 75 = EX_TEMPFAIL, que install.sh reserva para "hay una restauración en curso". Es un + // conflicto de estado, no un fallo: hay que reintentar, no investigar. Con el 502 genérico + // parecía que la instalación se había roto, y el operador iría a buscar la causa equivocada. + if (install.code === 75) { + throw new InstallError( + 409, + `${target.name} está restaurando un respaldo ahora mismo, así que no se actualizó ` + + 'para no interrumpirlo —eso dejaría ese respaldo vetado y la base en ' + + 'SINGLE_USER—. Reintenta cuando termine.' + ); + } if (install.code !== 0) { throw new InstallError( 502, @@ -609,7 +642,9 @@ async function installLinux( } // --- Verificación del despliegue ------------------------------------- - await verifyLinuxDeployment(sftp, runId, release, privileged, autostart, installPath); + await verifyLinuxDeployment( + sftp, runId, release, privileged, autostart, installPath, inPlaceUpdate + ); } finally { // Limpieza siempre: el panel.env trae el token en claro. Si falla NO se silencia — // significa que el token se quedó en el servidor y alguien tiene que ir a borrarlo. @@ -696,6 +731,87 @@ export async function probeLinuxElevation(sftp: SftpClient): Promise { + const writable = await execRemote( + sftp, + `test -w ${shQuote(installPath)} && echo si || echo no` + ); + if (writable.stdout.trim() !== 'si') { + const owner = await execRemote(sftp, `stat -c '%U' ${shQuote(installPath)} 2>/dev/null`); + return { + ok: false, + reason: + `'${sshUsername}' no puede escribir en ${installPath} (dueño: ` + + `${owner.stdout.trim() || 'desconocido'}), así que no puede reemplazar el binario.` + }; + } + + // `systemctl show` se lee sin privilegios, a diferencia de start/stop. + const unit = await execRemote( + sftp, + 'systemctl show -p User -p Restart cloudrestoreas 2>/dev/null' + ); + const props = new Map( + unit.stdout + .split('\n') + .map((line) => line.trim().split('=')) + .filter((parts) => parts.length === 2) + .map(([k, v]) => [k, v] as const) + ); + // User vacío en systemd significa root: el unit no declaró User=. + const unitUser = props.get('User') || 'root'; + if (unitUser !== sshUsername) { + return { + ok: false, + reason: + `el servicio corre como '${unitUser}' y la sesión es de '${sshUsername}', así que ` + + 'esta cuenta no puede reiniciarlo sin privilegios.' + }; + } + if (props.get('Restart') !== 'always') { + return { + ok: false, + reason: + `el unit tiene Restart=${props.get('Restart') || 'desconocido'}; sin Restart=always ` + + 'nadie volvería a levantar el agente tras actualizarlo.' + }; + } + + // Se mira aquí además de en install.sh para no subir 270 MB en balde. + const busy = await execRemote( + sftp, + `test -d ${shQuote(`${installPath}/Temp`)} && ls -A ${shQuote(`${installPath}/Temp`)} 2>/dev/null | head -1` + ); + if (busy.stdout.trim()) { + return { + ok: false, + reason: + 'hay una restauración en curso. Interrumpirla dejaría ese respaldo vetado para ' + + 'siempre y la base en SINGLE_USER, así que se prefiere esperar. Reintenta luego.' + }; + } + return { ok: true, reason: '' }; +} + async function verifyLinuxDeployment( sftp: SftpClient, runId: number, @@ -705,19 +821,36 @@ async function verifyLinuxDeployment( // Se llama installPath y no `prefix` a propósito: en el cuerpo convive con // `privileged.prefix`, que es el prefijo de ELEVACIÓN. Dos cosas distintas con el mismo // nombre en el mismo alcance es exactamente donde se cuela un bug silencioso. - installPath: string + installPath: string, + inPlaceUpdate = false ): Promise { // El sello config/.version lo escribe el bootstrap del binario; es más confiable que // stdout de --version, sobre todo por paridad con Windows (console=False). - const stamp = await execRemote( - sftp, - `cat ${shQuote(`${installPath}/config/.version`)} 2>/dev/null` - ); - const deployed = stamp.stdout.trim(); + // + // En la vía en sitio se OMITE el bootstrap y el sello lo escribe el proceso al reiniciarse, + // así que durante unos segundos sigue teniendo la versión ANTERIOR. Leerlo una sola vez + // convertía una actualización correcta en un fallo "reporta 1.0.0, se esperaba 1.1.1", con el + // agente ya actualizado y corriendo. Se sondea hasta que coincida, o hasta agotar el margen. + let deployed = ''; + const stampAttempts = inPlaceUpdate ? 15 : 1; + for (let attempt = 0; attempt < stampAttempts; attempt += 1) { + if (attempt > 0) await new Promise((resolve) => setTimeout(resolve, 2000)); + const stamp = await execRemote( + sftp, + `cat ${shQuote(`${installPath}/config/.version`)} 2>/dev/null` + ); + deployed = stamp.stdout.trim(); + // Vacío = todavía no lo escribió, y eso ya se toleraba antes. + if (!deployed || deployed === release.version) break; + } if (deployed && deployed !== release.version) { throw new InstallError( 502, - `El binario desplegado reporta la versión ${deployed}, se esperaba ${release.version}` + `El binario desplegado reporta la versión ${deployed}, se esperaba ${release.version}. ` + + (inPlaceUpdate + ? 'El sello no cambió tras reiniciar el agente, así que el proceso puede seguir ' + + 'ejecutando el binario anterior.' + : '') ); } await appendInstallStep( @@ -733,6 +866,9 @@ async function verifyLinuxDeployment( // usuario, y preguntarlo como root apuntaría al bus equivocado y respondería 'inactive' // sobre un servicio que sí está corriendo. XDG_RUNTIME_DIR va explícito porque un `exec` de // SSH no es una sesión de login y no siempre lo trae. + // En la vía en sitio el agente acaba de reiniciarse: `is-active` puede devolver 'activating' + // durante unos segundos, y eso no es un fallo. install.sh ya confirmó que el proceso volvió, + // así que el proceso vivo es la evidencia que manda. const userMode = autostart === 'user-service'; const active = await execRemote( sftp, @@ -745,7 +881,7 @@ async function verifyLinuxDeployment( // En modo usuario el arranque puede haber quedado por cron (@reboot) en vez de systemd, // cuando el destino no permite lingering. Ahí no hay unit que consultar y el proceso se // comprueba directamente, que es lo que de verdad importa. - if (userMode) { + if (userMode || inPlaceUpdate) { // El ancla ^ es obligatoria: sin ella el `sh -c` que corre este mismo pgrep lleva la // ruta en su propia línea de comandos y haría match consigo mismo, reportando vivo un // agente que no arrancó. @@ -758,7 +894,9 @@ async function verifyLinuxDeployment( runId, 'verificar-servicio', true, - 'proceso vivo (arranque por cron; systemd de usuario no disponible)' + inPlaceUpdate + ? `proceso vivo con el binario nuevo (unit intacto, estado: ${state})` + : 'proceso vivo (arranque por cron; systemd de usuario no disponible)' ); return; } -- 2.49.1 From 3bfe491befe378f0c87bc11745089712332733f3 Mon Sep 17 00:00:00 2001 From: hreyes Date: Thu, 30 Jul 2026 16:16:48 -0600 Subject: [PATCH 3/3] =?UTF-8?q?fix(cras-verify):=20validar=20que=20el=20se?= =?UTF-8?q?rvicio=20pueda=20LEER=20su=20configuraci=C3=B3n,=20y=20loguear?= =?UTF-8?q?=20los=20401?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Dos huecos de diagnóstico que hacían pasar por sano lo que no lo estaba. 1. El check `config` usaba `test -f`, que comprueba existencia y no lectura, así que salía verde exactamente en el caso roto: una instalación con sudo deja config/.env en 0600 de root mientras el unit corre como una cuenta común, que no puede leerlo. El agente no arranca y la pantalla decía "Configuración presente: ok". Ahora se comprueba lectura y, además, el dueño frente al User= del unit — porque la sonda entra con la cuenta SSH, que no siempre es la del servicio, y un `test -r` desde la sesión no responde por el agente. El check se renombra a "Configuración legible por el servicio", que es lo que de verdad mide, y falla nombrando a los dos usuarios para que el remedio sea obvio. El cálculo del bit de lectura octal sale a una función pura probada: 0600 no deja leer a nadie más que al dueño, se mira el bit 4 y no el valor (620 y 611 no son lectura), se ignora el dígito de setuid, y un modo ilegible NO se interpreta como permisivo — asumir lectura cuando stat devuelve '-' sería el error caro. La lectura de propiedades del unit se extrae a un helper compartido con el instalador. Ya hubo una divergencia por copiar esta lógica: la resolución de elevación existía duplicada y las dos pantallas acabaron diciendo cosas distintas del mismo servidor. 2. Ningún rechazo de token de servicio dejaba rastro: los seis endpoints solo logueaban el 500 de "token no configurado". Con el token rotado, todos los agentes quedan mudos —dejan de reportar versión, resolver rutas y registrar resultados— y desde el panel se ve igual que un agente apagado; además el trace_id que se le devuelve al agente no existía del lado servidor, así que era imposible correlacionar. Se agrega un helper que loguea el rechazo con trace_id, ruta e instancia, y distingue "sin header Authorization" de "token no coincide", que son un agente sin configurar y un token rotado: dos problemas con remedios distintos. El token NUNCA se registra, ni un fragmento suyo — es el mismo valor en todos los agentes, así que un prefijo en los logs ya acota el espacio de búsqueda. Hay prueba de eso. Co-Authored-By: Claude Opus 5 (1M context) --- src/lib/server/cras-install.ts | 48 +++++--- src/lib/server/cras-verify.test.ts | 40 ++++++- src/lib/server/cras-verify.ts | 71 +++++++++-- src/lib/server/service-auth.test.ts | 111 ++++++++++++++++++ src/lib/server/service-auth.ts | 33 ++++++ src/routes/api/restore/agent-sync/+server.ts | 8 +- .../api/restore/instance-config/+server.ts | 8 +- src/routes/api/restore/job-result/+server.ts | 8 +- .../api/restore/resolve-route/+server.ts | 10 +- .../api/restore/target-catalog/+server.ts | 8 +- src/routes/api/restore/target-for/+server.ts | 10 +- 11 files changed, 327 insertions(+), 28 deletions(-) create mode 100644 src/lib/server/service-auth.test.ts diff --git a/src/lib/server/cras-install.ts b/src/lib/server/cras-install.ts index 652510d..f1933fb 100644 --- a/src/lib/server/cras-install.ts +++ b/src/lib/server/cras-install.ts @@ -731,6 +731,38 @@ export async function probeLinuxElevation(sftp: SftpClient): Promise> { + const flags = props.map((name) => `-p ${name}`).join(' '); + const out = await execRemote(sftp, `systemctl show ${flags} ${unitName} 2>/dev/null`); + const parsed = new Map(); + for (const line of out.stdout.split('\n')) { + const at = line.indexOf('='); + if (at <= 0) continue; + parsed.set(line.slice(0, at).trim(), line.slice(at + 1).trim()); + } + return parsed; +} + +/** Usuario efectivo de un unit: `User=` vacío es root, igual que lo interpreta systemd. */ +export function unitUserOrRoot(props: Map): string { + return props.get('User') || 'root'; +} + /** * ¿Se puede ACTUALIZAR en su sitio sin privilegios? Devuelve el motivo si no. * @@ -765,20 +797,8 @@ export async function probeInPlaceUpdate( }; } - // `systemctl show` se lee sin privilegios, a diferencia de start/stop. - const unit = await execRemote( - sftp, - 'systemctl show -p User -p Restart cloudrestoreas 2>/dev/null' - ); - const props = new Map( - unit.stdout - .split('\n') - .map((line) => line.trim().split('=')) - .filter((parts) => parts.length === 2) - .map(([k, v]) => [k, v] as const) - ); - // User vacío en systemd significa root: el unit no declaró User=. - const unitUser = props.get('User') || 'root'; + const props = await readUnitProps(sftp, 'cloudrestoreas', ['User', 'Restart']); + const unitUser = unitUserOrRoot(props); if (unitUser !== sshUsername) { return { ok: false, diff --git a/src/lib/server/cras-verify.test.ts b/src/lib/server/cras-verify.test.ts index 2bbf883..fb42403 100644 --- a/src/lib/server/cras-verify.test.ts +++ b/src/lib/server/cras-verify.test.ts @@ -14,7 +14,7 @@ * comando POSIX. */ import { describe, expect, it } from 'vitest'; -import { probeRemoteSystem } from './cras-verify'; +import { posixModeAllowsNonOwnerRead, probeRemoteSystem } from './cras-verify'; /** * Cliente SFTP falso. `responder` decide qué contesta cada comando; se registran todos los @@ -197,3 +197,41 @@ describe('TRIPWIRE: ningún comando POSIX en la rama Windows', () => { } }); }); + +/** + * Bit de lectura del modo POSIX. Decide si un servicio que corre como OTRO usuario puede leer + * config/.env. El caso que motiva esto es el 0600 que deja una instalación con sudo: el agente + * no puede leer su propia configuración y la sonda lo pintaba verde. + */ +describe('posixModeAllowsNonOwnerRead', () => { + it('0600 no deja leer a nadie más que al dueño', () => { + expect(posixModeAllowsNonOwnerRead('600')).toBe(false); + expect(posixModeAllowsNonOwnerRead('0600')).toBe(false); + }); + + it('detecta lectura por grupo y por otros, juntas o por separado', () => { + expect(posixModeAllowsNonOwnerRead('640')).toBe(true); + expect(posixModeAllowsNonOwnerRead('604')).toBe(true); + expect(posixModeAllowsNonOwnerRead('644')).toBe(true); + expect(posixModeAllowsNonOwnerRead('755')).toBe(true); + }); + + it('mira el bit 4, no el valor: 2 y 1 no son lectura', () => { + expect(posixModeAllowsNonOwnerRead('620')).toBe(false); + expect(posixModeAllowsNonOwnerRead('611')).toBe(false); + expect(posixModeAllowsNonOwnerRead('633')).toBe(false); + expect(posixModeAllowsNonOwnerRead('664')).toBe(true); + }); + + it('con 4 dígitos ignora el de setuid/sticky, que no afecta la lectura', () => { + expect(posixModeAllowsNonOwnerRead('4600')).toBe(false); + expect(posixModeAllowsNonOwnerRead('1644')).toBe(true); + }); + + it('un modo ilegible no se interpreta como permisivo', () => { + // stat devuelve '-' cuando el archivo no existe; asumir lectura ahí seria el error caro. + for (const bad of ['-', '', ' ', 'rw-------', '8', '99', '60']) { + expect(posixModeAllowsNonOwnerRead(bad), bad).toBe(false); + } + }); +}); diff --git a/src/lib/server/cras-verify.ts b/src/lib/server/cras-verify.ts index 05640fe..1671dac 100644 --- a/src/lib/server/cras-verify.ts +++ b/src/lib/server/cras-verify.ts @@ -17,7 +17,14 @@ import net from 'node:net'; import SftpClient from 'ssh2-sftp-client'; import { getRestoreTargetSsh, type RestoreTargetSsh } from './controldesk-pg'; -import { execRemote, probeLinuxElevation, psEncoded, shQuote } from './cras-install'; +import { + execRemote, + probeLinuxElevation, + psEncoded, + readUnitProps, + shQuote, + unitUserOrRoot +} from './cras-install'; import { listCrasTargetInventory } from './cras-releases'; import { DEFAULT_INSTALL_PATHS, effectiveInstallPath, type CrasPlatform } from '$lib/cras-version'; import { logger } from './logger'; @@ -497,6 +504,22 @@ interface InspectResult { remediation: Remediation | null; } +/** + * ¿El modo POSIX concede lectura a grupo o a otros? + * + * Se usa para decidir si un usuario que NO es el dueño del archivo puede leerlo. El caso que + * importa es el 0600 que deja una instalación con sudo: solo el dueño, así que un servicio que + * corra como otra cuenta no puede leer su propia configuración. + * + * Acepta 3 o 4 dígitos (el cuarto es el bit de setuid/sticky, que no afecta la lectura). + */ +export function posixModeAllowsNonOwnerRead(mode: string): boolean { + const digits = String(mode ?? '').trim(); + if (!/^[0-7]{3,4}$/.test(digits)) return false; + const [, group, other] = digits.slice(-3); + return (Number(group) & 4) !== 0 || (Number(other) & 4) !== 0; +} + async function inspectLinux( sftp: SftpClient, reportedInstallPath: string | null @@ -588,18 +611,52 @@ async function inspectLinux( ); const deployed = version.stdout.trim(); - const env = await execRemote( + // `test -f` comprobaba EXISTENCIA, no lectura, y por eso este check salía verde justo en el + // caso roto: una instalación hecha con sudo deja config/.env en 0600 de root mientras el unit + // corre como un usuario común, que no puede leerlo. El agente no arranca y la pantalla decía + // "Configuración presente: ok". Se comprueba lectura y, además, quién es el dueño frente al + // usuario del unit — porque la sonda entra con la cuenta SSH, que no siempre es la del + // servicio. + const envProbe = await execRemote( sftp, - `test -f ${shQuote(`${prefix}/config/.env`)} && echo si || echo no`, + [ + `echo "existe=$(test -f ${shQuote(`${prefix}/config/.env`)} && echo si || echo no)"`, + `echo "legible=$(test -r ${shQuote(`${prefix}/config/.env`)} && echo si || echo no)"`, + `echo "dueno=$(stat -c '%U' ${shQuote(`${prefix}/config/.env`)} 2>/dev/null || echo -)"`, + `echo "modo=$(stat -c '%a' ${shQuote(`${prefix}/config/.env`)} 2>/dev/null || echo -)"` + ].join('; '), CHECK_TIMEOUT_MS ); - const hasEnv = env.stdout.trim() === 'si'; + const envInfo = new Map( + envProbe.stdout + .split('\n') + .map((line) => line.trim().split('=')) + .filter((parts) => parts.length === 2) + .map(([k, v]) => [k, v] as const) + ); + const hasEnv = envInfo.get('existe') === 'si'; + const envOwner = envInfo.get('dueno') ?? '-'; + const envMode = envInfo.get('modo') ?? '-'; + + // El unit puede correr como otro usuario que la sesión SSH. Un 0600 solo lo lee su dueño, así + // que si el dueño no es el usuario del servicio, el agente no puede leer su configuración + // aunque la sonda sí pueda. + const unitProps = await readUnitProps(sftp, 'cloudrestoreas', ['User']); + const serviceUser = unitUserOrRoot(unitProps); + const ownerIsService = envOwner === serviceUser; + const serviceCanRead = ownerIsService || posixModeAllowsNonOwnerRead(envMode); + checks.push( check( 'config', - 'Configuración presente', - hasEnv ? 'ok' : 'warn', - hasEnv ? `${prefix}/config/.env` : 'falta config/.env' + 'Configuración legible por el servicio', + !hasEnv ? 'warn' : serviceCanRead ? 'ok' : 'fail', + !hasEnv + ? 'falta config/.env' + : serviceCanRead + ? `${prefix}/config/.env (${envOwner}, ${envMode})` + : `${prefix}/config/.env es de '${envOwner}' en modo ${envMode}, pero el ` + + `servicio corre como '${serviceUser}': el agente no puede leerlo y no arrancará` ) ); diff --git a/src/lib/server/service-auth.test.ts b/src/lib/server/service-auth.test.ts new file mode 100644 index 0000000..744861f --- /dev/null +++ b/src/lib/server/service-auth.test.ts @@ -0,0 +1,111 @@ +/** + * Autenticación servicio-a-servicio de los endpoints que consume CloudRestoreAS. + * + * La prueba que sostiene el resto es la de no-fuga: el rechazo de un token se escribe a los logs + * del contenedor, así que si alguna vez alguien mete el valor recibido "para depurar", el token de + * servicio —que es el MISMO en todos los agentes— acaba en disco y en cualquier agregador. + * + * El resto cubre por qué se loguea: hasta ahora un 401 no dejaba ni una línea, así que un token + * rotado dejaba a todos los agentes mudos y desde el panel se veía igual que un agente apagado. + */ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { env } from '$env/dynamic/private'; + +const { loggerMock } = vi.hoisted(() => ({ + loggerMock: { info: vi.fn(), warn: vi.fn(), error: vi.fn() } +})); +vi.mock('./logger', () => ({ logger: loggerMock })); + +import { checkServiceToken, logServiceTokenRejected } from './service-auth'; + +const TOKEN = 'tok-de-servicio-no-debe-aparecer-en-logs'; + +function req(authorization?: string): Request { + return new Request('https://panel.test/api/restore/target-for?database=X&instance=Mediano', { + headers: authorization ? { authorization } : {} + }); +} + +beforeEach(() => { + env.CLOUDRESTORE_API_TOKEN = TOKEN; + loggerMock.warn.mockClear(); +}); + +afterEach(() => { + // Cadena vacía y no `delete`: es el caso real del compose, que deja la variable definida y + // vacía cuando no se configuró. Y `delete` no tipa, porque la propiedad no es opcional. + env.CLOUDRESTORE_API_TOKEN = ''; + vi.restoreAllMocks(); +}); + +describe('checkServiceToken', () => { + it('acepta el token exacto', () => { + expect(checkServiceToken(req(`Bearer ${TOKEN}`))).toEqual({ ok: true }); + }); + + it('rechaza con 401 sin header, con esquema equivocado, o con token distinto', () => { + for (const header of [undefined, 'Basic abc', `Token ${TOKEN}`, 'Bearer otro-token']) { + expect(checkServiceToken(req(header)), String(header)).toMatchObject({ + ok: false, + status: 401 + }); + } + }); + + it('un token de largo distinto se rechaza sin comparar byte a byte', () => { + // timingSafeEqual lanza si los buffers difieren en largo, así que el guard de largo + // previo no es cosmético: sin él, un token más corto reventaría con una excepción. + expect(() => checkServiceToken(req('Bearer x'))).not.toThrow(); + expect(checkServiceToken(req('Bearer x'))).toMatchObject({ ok: false, status: 401 }); + }); + + it('sin token configurado responde 500, no 401: no se puede autenticar de forma segura', () => { + env.CLOUDRESTORE_API_TOKEN = ''; + expect(checkServiceToken(req(`Bearer ${TOKEN}`))).toMatchObject({ ok: false, status: 500 }); + }); +}); + +describe('logServiceTokenRejected', () => { + it('NUNCA registra el token, ni el esperado ni el recibido', () => { + logServiceTokenRejected({ + traceId: 't-1', + route: 'GET /api/restore/target-for', + request: req('Bearer un-token-que-no-cuadra'), + instanceKey: 'Mediano' + }); + const escrito = JSON.stringify(loggerMock.warn.mock.calls[0][0]); + expect(escrito).not.toContain(TOKEN); + expect(escrito).not.toContain('un-token-que-no-cuadra'); + // Ni fragmentos: un prefijo también acota el espacio de búsqueda. + expect(escrito).not.toContain(TOKEN.slice(0, 8)); + }); + + it('distingue "sin header" de "token no coincide": son remedios distintos', () => { + // Sin header = agente sin configurar. Token que no cuadra = token rotado. + logServiceTokenRejected({ traceId: 't-2', route: 'r', request: req() }); + expect(loggerMock.warn.mock.calls[0][0].context.reason).toMatch(/sin header/i); + + loggerMock.warn.mockClear(); + logServiceTokenRejected({ traceId: 't-3', route: 'r', request: req('Bearer nope') }); + expect(loggerMock.warn.mock.calls[0][0].context.reason).toMatch(/no coincide/i); + }); + + it('lleva trace_id y ruta, que es lo que permite correlacionar con el agente', () => { + logServiceTokenRejected({ + traceId: 't-4', + route: 'POST /api/restore/job-result', + request: req('Bearer nope') + }); + const entry = loggerMock.warn.mock.calls[0][0]; + expect(entry.trace_id).toBe('t-4'); + expect(entry.context.route).toBe('POST /api/restore/job-result'); + }); + + it('normaliza la instancia ausente o en blanco a null, no a cadena vacía', () => { + for (const ik of [undefined, null, '', ' ']) { + loggerMock.warn.mockClear(); + logServiceTokenRejected({ traceId: 't', route: 'r', request: req(), instanceKey: ik }); + expect(loggerMock.warn.mock.calls[0][0].context.instance_key, String(ik)).toBeNull(); + } + }); +}); diff --git a/src/lib/server/service-auth.ts b/src/lib/server/service-auth.ts index ca91933..48302a3 100644 --- a/src/lib/server/service-auth.ts +++ b/src/lib/server/service-auth.ts @@ -8,6 +8,7 @@ */ import { timingSafeEqual } from 'node:crypto'; import { env } from '$env/dynamic/private'; +import { logger } from './logger'; /** * Valida el header Authorization: Bearer contra CLOUDRESTORE_API_TOKEN. @@ -38,3 +39,35 @@ export function checkServiceToken(request: Request): { ok: true } | { ok: false; } return { ok: true }; } + +/** + * Registra un rechazo de token de servicio. + * + * Hasta ahora un 401 no dejaba **ni una línea** en el panel: solo se logueaba el 500 de "token no + * configurado". Con el token rotado, todos los agentes quedan mudos —dejan de reportar versión, de + * resolver rutas y de registrar resultados— y desde el panel eso se ve igual que un agente + * apagado. Peor: el `trace_id` que se le devuelve al agente en el cuerpo del 401 no existía del + * lado del servidor, así que era imposible de correlacionar. + * + * NUNCA se registra el token ni un fragmento suyo. Lo que sirve para diagnosticar es qué ruta se + * rechazó, si venía con header o sin él, y de qué instancia. + */ +export function logServiceTokenRejected(info: { + traceId: string; + route: string; + request: Request; + instanceKey?: string | null; +}): void { + const header = info.request.headers.get('authorization') ?? ''; + logger.warn({ + trace_id: info.traceId, + message: 'Token de servicio rechazado', + context: { + route: info.route, + // Distinguir "no mandó header" de "mandó uno que no cuadra" separa un agente sin + // configurar de un token rotado, que son dos problemas con remedios distintos. + reason: header ? 'token no coincide' : 'sin header Authorization', + instance_key: info.instanceKey?.trim() || null + } + }); +} diff --git a/src/routes/api/restore/agent-sync/+server.ts b/src/routes/api/restore/agent-sync/+server.ts index ff4bc3e..112647a 100644 --- a/src/routes/api/restore/agent-sync/+server.ts +++ b/src/routes/api/restore/agent-sync/+server.ts @@ -12,7 +12,7 @@ */ import { json } from '@sveltejs/kit'; import type { RequestHandler } from './$types'; -import { checkServiceToken } from '$lib/server/service-auth'; +import { checkServiceToken, logServiceTokenRejected } from '$lib/server/service-auth'; import { errorJson, newTraceId } from '$lib/server/api-error'; import { logger } from '$lib/server/logger'; import { GiteaError, isGiteaConfigured } from '$lib/server/gitea-packages'; @@ -27,6 +27,12 @@ export const POST: RequestHandler = async ({ request }) => { logger.error({ trace_id: traceId, message: 'CLOUDRESTORE_API_TOKEN no configurado' }); return errorJson(500, 'Servicio no configurado', traceId); } + logServiceTokenRejected({ + traceId, + route: 'POST /api/restore/agent-sync', + request, + instanceKey: null + }); return errorJson(401, 'Token de servicio ausente o inválido', traceId); } diff --git a/src/routes/api/restore/instance-config/+server.ts b/src/routes/api/restore/instance-config/+server.ts index 0483f5d..8ec3810 100644 --- a/src/routes/api/restore/instance-config/+server.ts +++ b/src/routes/api/restore/instance-config/+server.ts @@ -25,7 +25,7 @@ */ import { json } from '@sveltejs/kit'; import type { RequestHandler } from './$types'; -import { checkServiceToken } from '$lib/server/service-auth'; +import { checkServiceToken, logServiceTokenRejected } from '$lib/server/service-auth'; import { upsertCloudRestoreStatus } from '$lib/server/controldesk-pg'; import { errorJson, newTraceId } from '$lib/server/api-error'; import { logger } from '$lib/server/logger'; @@ -74,6 +74,12 @@ export const POST: RequestHandler = async ({ request }) => { logger.error({ trace_id: traceId, message: 'CLOUDRESTORE_API_TOKEN no configurado' }); return errorJson(500, 'Servicio no configurado', traceId); } + logServiceTokenRejected({ + traceId, + route: 'POST /api/restore/instance-config', + request, + instanceKey: null + }); return errorJson(401, 'Token de servicio ausente o inválido', traceId); } diff --git a/src/routes/api/restore/job-result/+server.ts b/src/routes/api/restore/job-result/+server.ts index c27e039..bb303c7 100644 --- a/src/routes/api/restore/job-result/+server.ts +++ b/src/routes/api/restore/job-result/+server.ts @@ -16,7 +16,7 @@ */ import { json } from '@sveltejs/kit'; import type { RequestHandler } from './$types'; -import { checkServiceToken } from '$lib/server/service-auth'; +import { checkServiceToken, logServiceTokenRejected } from '$lib/server/service-auth'; import { insertRestoreJobLog } from '$lib/server/controldesk-pg'; import { errorJson, newTraceId } from '$lib/server/api-error'; import { logger } from '$lib/server/logger'; @@ -51,6 +51,12 @@ export const POST: RequestHandler = async ({ request }) => { logger.error({ trace_id: traceId, message: 'CLOUDRESTORE_API_TOKEN no configurado' }); return errorJson(500, 'Servicio no configurado', traceId); } + logServiceTokenRejected({ + traceId, + route: 'POST /api/restore/job-result', + request, + instanceKey: null + }); return errorJson(401, 'Token de servicio ausente o inválido', traceId); } diff --git a/src/routes/api/restore/resolve-route/+server.ts b/src/routes/api/restore/resolve-route/+server.ts index 90e6b62..57c8323 100644 --- a/src/routes/api/restore/resolve-route/+server.ts +++ b/src/routes/api/restore/resolve-route/+server.ts @@ -6,7 +6,7 @@ */ import { json } from '@sveltejs/kit'; import type { RequestHandler } from './$types'; -import { checkServiceToken } from '$lib/server/service-auth'; +import { checkServiceToken, logServiceTokenRejected } from '$lib/server/service-auth'; import { resolveRouteForFilename } from '$lib/server/controldesk-pg'; import { errorJson, newTraceId } from '$lib/server/api-error'; import { logger } from '$lib/server/logger'; @@ -20,6 +20,14 @@ export const GET: RequestHandler = async ({ request, url }) => { logger.error({ trace_id: traceId, message: 'CLOUDRESTORE_API_TOKEN no configurado' }); return errorJson(500, 'Servicio no configurado', traceId); } + logServiceTokenRejected({ + traceId, + route: 'GET /api/restore/resolve-route', + request, + // El agente manda su instancia como `instance`; sirve para saber QUÉ servidor + // quedó mudo cuando se rota el token. + instanceKey: url.searchParams.get('instance') + }); return errorJson(401, 'Token de servicio ausente o inválido', traceId); } diff --git a/src/routes/api/restore/target-catalog/+server.ts b/src/routes/api/restore/target-catalog/+server.ts index b439021..0099998 100644 --- a/src/routes/api/restore/target-catalog/+server.ts +++ b/src/routes/api/restore/target-catalog/+server.ts @@ -6,7 +6,7 @@ */ import { json } from '@sveltejs/kit'; import type { RequestHandler } from './$types'; -import { checkServiceToken } from '$lib/server/service-auth'; +import { checkServiceToken, logServiceTokenRejected } from '$lib/server/service-auth'; import { listRestoreTargets } from '$lib/server/controldesk-pg'; import { errorJson, newTraceId } from '$lib/server/api-error'; import { logger } from '$lib/server/logger'; @@ -20,6 +20,12 @@ export const GET: RequestHandler = async ({ request }) => { logger.error({ trace_id: traceId, message: 'CLOUDRESTORE_API_TOKEN no configurado' }); return errorJson(500, 'Servicio no configurado', traceId); } + logServiceTokenRejected({ + traceId, + route: 'GET /api/restore/target-catalog', + request, + instanceKey: null + }); return errorJson(401, 'Token de servicio ausente o inválido', traceId); } diff --git a/src/routes/api/restore/target-for/+server.ts b/src/routes/api/restore/target-for/+server.ts index 7a15232..a572bc0 100644 --- a/src/routes/api/restore/target-for/+server.ts +++ b/src/routes/api/restore/target-for/+server.ts @@ -11,7 +11,7 @@ */ import { json } from '@sveltejs/kit'; import type { RequestHandler } from './$types'; -import { checkServiceToken } from '$lib/server/service-auth'; +import { checkServiceToken, logServiceTokenRejected } from '$lib/server/service-auth'; import { getRestoreTargetForDatabase } from '$lib/server/controldesk-pg'; import { errorJson, newTraceId } from '$lib/server/api-error'; import { logger } from '$lib/server/logger'; @@ -25,6 +25,14 @@ export const GET: RequestHandler = async ({ request, url }) => { logger.error({ trace_id: traceId, message: 'CLOUDRESTORE_API_TOKEN no configurado' }); return errorJson(500, 'Servicio no configurado', traceId); } + logServiceTokenRejected({ + traceId, + route: 'GET /api/restore/target-for', + request, + // El agente manda su instancia como `instance`; sirve para saber QUÉ servidor + // quedó mudo cuando se rota el token. + instanceKey: url.searchParams.get('instance') + }); return errorJson(401, 'Token de servicio ausente o inválido', traceId); } -- 2.49.1