From bd93f49113d33dacc4821d70bb2162ed038c7abf Mon Sep 17 00:00:00 2001
From: hreyes <hreyes@aduanasoft.com.mx>
Date: Fri, 31 Jul 2026 21:49:11 +0000
Subject: [PATCH] fix(cras-install): el panel no puede creerse un exito que no
 ocurrio (#30)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

La bitacora del run 26 tenia la respuesta: la sonda decia que install.ps1 NO
EXISTE y el paso siguiente, "Ejecutar instalador", salia en verde. Comprobado con
PowerShell real por que:

    & 'C:\no-existe.ps1' -Service; exit $LASTEXITCODE   ->   exit 0

$LASTEXITCODE no se fija nunca (no corrio ningun comando nativo), asi que
`exit $null` da 0. El panel leia 0 y asentaba el paso como exitoso. NADA se
instalaba, y por eso seguia viva la 1.1.0: el agente viejo nunca se detuvo porque
el instalador nunca se ejecuto. Esa evidencia falsa mando el diagnostico a otra
parte durante varios runs.

- El script emite un CENTINELA `CRAS-FIN|<codigo>` y el panel lo EXIGE. Sin el, el
  instalador no termino, de lo que de el codigo de salida. Un fallo no es una
  respuesta — la misma leccion que la sonda, ahora en la invocacion.
  Validado con PowerShell real: instalador normal -> CRAS-FIN|0; instalador que
  sale 75 -> CRAS-FIN|75 (la traduccion a "restauracion en curso" sigue viva);
  instalador inexistente -> CRAS-ERROR| y codigo 90 en vez de un 0 silencioso.
- "El instalador no esta" pasa a ser un hecho aparte de "no pude leerlo", y aborta
  con 502. Antes se colapsaba en `desconocido`, el run continuaba sabiendolo ya,
  gastaba el intento entero y acababa culpando al agente de no escribir su sello.
- Se asienta el CONTENIDO del staging tras extraer. Sin eso no se distinguia "el
  instalador no esta", "esta en otra ruta" y "esta pero no se puede leer", que son
  tres arreglos distintos.

Y dos cambios en el endurecimiento del token, cada uno defendible por si solo:

- Se restringe SOLO panel.env, no la carpeta. La restriccion de la carpeta era un
  extra —su motivo declarado, que el archivo heredara una ACL permisiva al
  crearse, ya lo cubre la ACE del propio archivo— y a cambio dejaba la carpeta con
  ACE NO heredables, de modo que lo creado dentro despues podia quedar sin
  permisos utilizables. Ahi se extrae el artefacto: era candidato serio a explicar
  por que el instalador "no existia".
- Por SID y no por nombre de grupo. En un Windows en espanol
  `BUILTIN\Administrators` no resuelve y el icacls falla entero; comprobado.
  S-1-5-32-544 y S-1-5-18 valen en cualquier idioma.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Reviewed-on: https://git.aduanasoft.com/ADUANASOFT/PANEL_BASES_ANEXO24/pulls/30
Co-authored-by: hreyes <hreyes@aduanasoft.com.mx>
Co-committed-by: hreyes <hreyes@aduanasoft.com.mx>
---
 src/lib/server/cras-install.test.ts |  34 +++++++--
 src/lib/server/cras-install.ts      | 104 ++++++++++++++++++++++++----
 2 files changed, 120 insertions(+), 18 deletions(-)

diff --git a/src/lib/server/cras-install.test.ts b/src/lib/server/cras-install.test.ts
index 35916fd..49e127d 100644
--- a/src/lib/server/cras-install.test.ts
+++ b/src/lib/server/cras-install.test.ts
@@ -515,6 +515,29 @@ describe('checkWindowsArtifactUpdateSupport', () => {
         expect(checkWindowsArtifactUpdateSupport('install', SI, '1.1.4', 'Principal')).toBe('');
     });
 
+    it('si el instalador NO ESTÁ, aborta: no hay nada que ejecutar', () => {
+        // Es distinto de "no sé": aquí sí se sabe, y es lo peor posible. Antes se trataba como duda
+        // y el run continuaba, tocaba el servidor y acababa culpando al agente.
+        const falta = { answer: 'sin-archivo', detail: 'no existe C:\\tmp\\install.ps1' } as const;
+        try {
+            checkWindowsArtifactUpdateSupport('update', falta, '1.1.4', 'Principal');
+            throw new Error('se esperaba que lanzara');
+        } catch (e) {
+            const err = e as InstallError;
+            expect(err).toBeInstanceOf(InstallError);
+            expect(err.status).toBe(502);
+            expect(err.message).toContain('C:\\tmp\\install.ps1');
+            expect(err.message).toMatch(/contenido-staging/);
+        }
+    });
+
+    it('y también aborta al instalar limpio: sin instalador no se instala nada', () => {
+        const falta = { answer: 'sin-archivo', detail: 'no existe X' } as const;
+        expect(() =>
+            checkWindowsArtifactUpdateSupport('install', falta, '1.1.4', 'Principal')
+        ).toThrow(InstallError);
+    });
+
     it('NO SABER no bloquea: es la prueba que faltaba y la que dejó 1.1.4 sin poder instalarse', () => {
         // La sonda anterior colapsaba cualquier fallo en "no lo soporta", y sobre esa lectura se
         // decidía un 409. Con un artefacto correcto, eso era un candado permanente.
@@ -582,11 +605,14 @@ describe('probeInstallerUpdateSupport', () => {
         expect((await probeInstallerUpdateSupport(sftp as never, RUTA)).answer).toBe('desconocido');
     });
 
-    it('el archivo que no existe se distingue de un instalador antiguo', async () => {
-        const { sftp } = fakeSftp(() => ({ code: 0, stdout: 'desconocido|no existe el archivo' }));
+    it('el archivo que no existe NO es "desconocido": es un hecho, y el peor', async () => {
+        // Que no haya instalador es definitivo — no hay nada que ejecutar. Colapsarlo en
+        // "desconocido" hacía que el run siguiera adelante sabiéndolo ya, gastara el intento entero
+        // y acabara culpando al agente de no escribir su sello.
+        const { sftp } = fakeSftp(() => ({ code: 0, stdout: 'sin-archivo' }));
         const r = await probeInstallerUpdateSupport(sftp as never, RUTA);
-        expect(r.answer).toBe('desconocido');
-        expect(r.detail).toBe('no existe el archivo');
+        expect(r.answer).toBe('sin-archivo');
+        expect(r.detail).toContain(RUTA);
     });
 
     it('una salida inesperada no se interpreta como respuesta', async () => {
diff --git a/src/lib/server/cras-install.ts b/src/lib/server/cras-install.ts
index 04a8b99..a462204 100644
--- a/src/lib/server/cras-install.ts
+++ b/src/lib/server/cras-install.ts
@@ -1295,7 +1295,13 @@ export async function probeWindowsTask(sftp: SftpClient): Promise<WindowsTaskPro
  * en cuanto gobernó un rechazo, se volvió un candado.
  */
 export interface ProbeAnswer {
-    answer: 'si' | 'no' | 'desconocido';
+    /**
+     * `sin-archivo` está aparte de `desconocido` a propósito: que el instalador no exista **sí** es
+     * una respuesta definitiva, y de las peores —no hay nada que ejecutar—, mientras `desconocido`
+     * es "no pude leerlo". Colapsarlos hacía que el run siguiera adelante sabiendo ya que no había
+     * instalador, y gastara el resto del intento para acabar culpando al agente.
+     */
+    answer: 'si' | 'no' | 'desconocido' | 'sin-archivo';
     /** Por qué no se pudo determinar. Vacío cuando hay respuesta. */
     detail: string;
 }
@@ -1321,7 +1327,7 @@ export async function probeInstallerUpdateSupport(
         sftp,
         psEncoded(
             `$p = '${installerPath}'; ` +
-                "if (-not (Test-Path -LiteralPath $p)) { Write-Output 'desconocido|no existe el archivo'; exit 0 }; " +
+                "if (-not (Test-Path -LiteralPath $p)) { Write-Output 'sin-archivo'; exit 0 }; " +
                 'try { ' +
                 // Select-String y nada más: un cmdlet puro, sin resolución de tipos .NET.
                 // La versión anterior usaba [regex]::IsMatch, que es una llamada estática y por
@@ -1353,6 +1359,9 @@ export async function probeInstallerUpdateSupport(
         };
     }
     if (salida === 'si' || salida === 'no') return { answer: salida, detail: '' };
+    if (salida === 'sin-archivo') {
+        return { answer: 'sin-archivo', detail: `no existe ${installerPath}` };
+    }
     return {
         answer: 'desconocido',
         detail: salida.startsWith('desconocido|') ? salida.slice(12) : truncate(salida)
@@ -1386,6 +1395,19 @@ export function checkWindowsArtifactUpdateSupport(
 ): string {
     if (soporte.answer === 'si') return '';
 
+    // Que el instalador NO ESTÉ no es una duda: es el peor de los hechos posibles, porque no hay
+    // nada que ejecutar. Antes se colapsaba en `desconocido` y el run seguía adelante sabiéndolo ya,
+    // gastaba el resto del intento y acababa culpando al agente de no escribir su sello.
+    if (soporte.answer === 'sin-archivo') {
+        throw new InstallError(
+            502,
+            `El artefacto ${version} se extrajo en ${targetName} pero no dejó el instalador donde ` +
+                `se esperaba: ${soporte.detail}. No hay nada que ` +
+                'ejecutar, así que no se toca el servidor. Revisa el paso "contenido-staging" de ' +
+                'este run para ver qué sí quedó en la carpeta temporal.'
+        );
+    }
+
     // No saber NO es motivo para bloquear. Un fallo de diagnóstico no puede impedir el trabajo: se
     // sigue con el comportamiento anterior y se deja dicho por qué no se pudo determinar, que es el
     // dato que convierte la próxima sorpresa en un diagnóstico de diez segundos.
@@ -1711,19 +1733,26 @@ async function installWindows(
             Buffer.from(panelEnvContents(request, target.name), 'utf8'),
             remoteEnv.replace(/\\/g, '/')
         );
-        // Los permisos POSIX no aplican en NTFS: se restringe el ACL a Administradores y
-        // SYSTEM para que el token no quede legible por cualquier usuario del servidor.
-        // Se restringe el DIRECTORIO además del archivo: New-Item hereda el ACL del padre, y
-        // si el $env:TEMP de la cuenta resuelve a C:\Windows\Temp ese padre es accesible por
-        // todos. Y se verifica el código de salida: un icacls que falla en silencio dejaría el
-        // token legible, que es justo lo que se está tratando de evitar.
+        // Los permisos POSIX no aplican en NTFS: se restringe el ACL a Administradores y SYSTEM
+        // para que el token no quede legible por cualquier usuario del servidor. Se verifica el
+        // código de salida: un icacls que falla en silencio dejaría el token legible, que es justo
+        // lo que se está tratando de evitar.
+        //
+        // Se restringe SOLO el archivo, no la carpeta. La restricción de la carpeta era un extra —su
+        // motivo declarado, que el archivo heredara una ACL permisiva al crearse, ya lo cubre la ACE
+        // del propio archivo— y a cambio dejaba la carpeta con ACE **no heredables**, de modo que
+        // todo lo creado dentro después podía quedar sin permisos utilizables. Ahí se extrae el
+        // artefacto, así que era un candidato serio a explicar por qué el instalador "no existía".
+        //
+        // Y por SID, no por nombre de grupo: en un Windows en español `BUILTIN\Administrators` no
+        // resuelve y el icacls falla entero ("No se efectuó ninguna asignación entre los nombres de
+        // cuenta y los identificadores de seguridad"). Comprobado. S-1-5-32-544 es Administradores y
+        // S-1-5-18 es SYSTEM en cualquier idioma.
         const acl = await execRemote(
             sftp,
             psEncoded(
-                `icacls '${remoteDir}' /inheritance:r ` +
-                    '/grant:r "BUILTIN\\Administrators:(F)" /grant:r "NT AUTHORITY\\SYSTEM:(F)" | Out-Null; ' +
-                    `icacls '${remoteEnv}' /inheritance:r ` +
-                    '/grant:r "BUILTIN\\Administrators:(F)" /grant:r "NT AUTHORITY\\SYSTEM:(F)" | Out-Null'
+                `icacls '${remoteEnv}' /inheritance:r ` +
+                    '/grant:r "*S-1-5-32-544:(F)" /grant:r "*S-1-5-18:(F)" | Out-Null'
             )
         );
         if (acl.code !== 0) {
@@ -1751,6 +1780,25 @@ async function installWindows(
             );
         }
 
+        // Qué dejó realmente la extracción, asentado en la bitácora. Sin esto no se distinguía
+        // "el instalador no está", "está en otra ruta" y "está pero no se puede leer" — y son tres
+        // arreglos distintos. La sonda decía "no existe el archivo" y no había forma de saber cuál
+        // de los tres era.
+        const listado = await execRemote(
+            sftp,
+            psEncoded(
+                `Get-ChildItem -LiteralPath '${remoteDir}' -Force -ErrorAction SilentlyContinue | ` +
+                    'ForEach-Object { $_.Name } | Sort-Object'
+            )
+        );
+        await appendInstallStep(
+            runId,
+            'contenido-staging',
+            true,
+            listado.stdout.trim().split('\n').map((l) => l.trim()).filter(Boolean).join(', ') ||
+                `no se pudo listar ${remoteDir} (código ${listado.code})`
+        );
+
         // Actualizar va por -UpdateInPlace, sea cual sea el arranque configurado: no vuelve a
         // registrar la tarea (así no pisa ajustes que el operador haya hecho sobre ella), se salta
         // el bootstrap —una segunda instancia purgaría el Temp\ de la que está viva— y trae respaldo
@@ -1781,14 +1829,42 @@ async function installWindows(
                 ? ' -Service'
                 : ' -Desktop';
         await appendInstallStep(runId, 'ejecutar-instalador', true, `install.ps1${flag}`);
+        // El script emite un CENTINELA y el panel lo exige.
+        //
+        // Antes se invocaba con `& '<ruta>' …; exit $LASTEXITCODE`, y eso no distingue "corrió y
+        // salió 0" de "no llegó a correr": si la invocación falla —porque el archivo no está, por
+        // ejemplo— `$LASTEXITCODE` no se fija nunca (no corrió ningún comando nativo), así que
+        // `exit $null` da 0. Comprobado con PowerShell real. El panel leía 0, asentaba
+        // "Ejecutar instalador ✓", y el servidor seguía intacto con su agente viejo — durante
+        // varios runs, con esa evidencia falsa mandando el diagnóstico a otra parte.
+        //
+        // Con el centinela, la ausencia de CRAS-FIN significa "no terminó", dé lo que dé el código.
         const install = await execRemote(
             sftp,
             psEncoded(
-                `& '${remoteDir}\\CloudRestoreAS\\install.ps1'${flag} ` +
-                    `-Prefix '${targetPath}' -PanelEnvFile '${remoteEnv}'; exit $LASTEXITCODE`
+                "$ErrorActionPreference = 'Stop'; " +
+                    'try { ' +
+                    `& '${remoteDir}\\CloudRestoreAS\\install.ps1'${flag} ` +
+                    `-Prefix '${targetPath}' -PanelEnvFile '${remoteEnv}'; ` +
+                    '$c = 0; if ($null -ne $LASTEXITCODE) { $c = $LASTEXITCODE } } ' +
+                    "catch { Write-Output ('CRAS-ERROR|' + $_.Exception.Message); exit 90 }; " +
+                    "Write-Output ('CRAS-FIN|' + $c); exit $c"
             ),
             INSTALL_EXEC_TIMEOUT_MS
         );
+
+        const salidaInstalador = `${install.stdout}\n${install.stderr}`;
+        if (!salidaInstalador.includes('CRAS-FIN|')) {
+            const detalle = salidaInstalador.includes('CRAS-ERROR|')
+                ? salidaInstalador.split('CRAS-ERROR|')[1].split('\n')[0].trim()
+                : truncate(install.stderr || install.stdout) || 'sin salida';
+            throw new InstallError(
+                502,
+                `install.ps1 no llegó a terminar en ${target.name}: ${detalle}. El servidor quedó ` +
+                    'como estaba. Revisa el paso "contenido-staging" de este run para ver qué dejó ' +
+                    'la extracción del artefacto.'
+            );
+        }
         // 75 = EX_TEMPFAIL, igual que en install.sh: hay una restauración en curso. Es un
         // conflicto de estado, no un fallo; con el 502 genérico el operador salía a buscar una
         // avería que no existe.