diff --git a/REVIEW.md b/REVIEW.md index 1294734..07419f8 100644 --- a/REVIEW.md +++ b/REVIEW.md @@ -670,7 +670,7 @@ OpenClaw's own supported commands and both verified against a real OpenClaw 2026 | Repair | Does | Verified by | Risk | |---|---|---|---| | `auto-update-enabled-warning` | `openclaw config set update.auto.enabled false` | reads the key back through `config get` | low | -| `gateway-loopback-no-auth` | sets `gateway.auth.mode` to `token`, then `doctor --fix --generate-gateway-token` | reads the mode back; never reads the token itself | medium | +| `gateway-loopback-no-auth` | reuses or generates a token, then sets token mode | verifies mode and token presence without recording token material; client usability remains an external OpenClaw semantic | medium | Repairable findings went from 1 to 3. On a real install, broken deliberately: diff --git a/cli/adapters/openclaw.js b/cli/adapters/openclaw.js index 0a868cb..22dec75 100644 --- a/cli/adapters/openclaw.js +++ b/cli/adapters/openclaw.js @@ -490,11 +490,50 @@ export function createOpenClawAdapter({ * * Repairs verify against this rather than parsing openclaw.json: it is the value OpenClaw * resolves, and it keeps repair evidence to a single key instead of a whole config blob. - * Returns '' when the key is unset or the call fails — callers must not read that as false. + * An empty value can be a successful "unset" result. Callers must use `ok` to distinguish + * that from an invocation failure. */ async configGet(key, options = {}) { - if (typeof key !== 'string' || !/^[A-Za-z0-9_.-]{1,128}$/.test(key)) return ''; - return processText(await invoke(['config', 'get', key], options)); + if (typeof key !== 'string' || !/^[A-Za-z0-9_.-]{1,128}$/.test(key)) { + return Object.freeze({ + ok: false, + value: '', + status: null, + errorSummary: 'invalid config key', + }); + } + const result = await invoke(['config', 'get', key], options); + const ok = result.status === 0 + && result.errorCode == null + && result.errorSummary == null + && result.signal == null + && !result.timedOut + && !result.aborted + && !result.outputLimitExceeded + && !result.stdoutTruncated + && !result.stderrTruncated; + return Object.freeze({ + ok, + value: ok ? String(result.stdout || '').trim() : '', + status: result.status, + errorSummary: ok + ? null + : result.errorSummary || `openclaw config get exited with status ${result.status}`, + }); + }, + + /** + * Check whether a config key has a non-empty value without returning that value to callers. + * This is the only repair-facing primitive allowed for secret-bearing config keys. + */ + async configHasValue(key, options = {}) { + const read = await this.configGet(key, options); + return Object.freeze({ + ok: read.ok, + present: read.ok ? read.value.trim().length > 0 : false, + status: read.status, + errorSummary: read.errorSummary, + }); }, /** Set one config key. Values are passed as literal argv, never through a shell. */ @@ -504,6 +543,14 @@ export function createOpenClawAdapter({ } return invoke(['config', 'set', key, String(value)], options); }, + + /** Remove one config key through OpenClaw itself. */ + async configUnset(key, options = {}) { + if (typeof key !== 'string' || !/^[A-Za-z0-9_.-]{1,128}$/.test(key)) { + return Object.freeze({ status: 1, errorSummary: 'invalid config key' }); + } + return invoke(['config', 'unset', key], options); + }, /** * PIDs that plausibly belong to a running gateway *server*. * diff --git a/cli/core/repair-catalog.js b/cli/core/repair-catalog.js index 4475ac1..979dc9d 100644 --- a/cli/core/repair-catalog.js +++ b/cli/core/repair-catalog.js @@ -12,6 +12,34 @@ // which itself only ever spawns argv arrays (shell: false). ctx.wait is an injectable delay hook // so tests can drive apply -> verify without real timers. +import { applyFailureReason } from './repair-engine.js'; + +/** + * Preserve the process adapter's complete terminal verdict without retaining command output or + * raw Error objects in repair/session state. The repair engine must see every failure marker; + * projecting only `status` lets a status-zero timeout, abort, signal, or truncated result pass. + */ +function terminalResult(result, details = {}) { + const source = result && typeof result === 'object' && !Array.isArray(result) ? result : {}; + const flag = (field) => (Object.hasOwn(source, field) ? source[field] : false); + const projected = { + status: source.status, + signal: source.signal ?? null, + timedOut: flag('timedOut'), + aborted: flag('aborted'), + stdoutTruncated: flag('stdoutTruncated'), + stderrTruncated: flag('stderrTruncated'), + outputLimitExceeded: flag('outputLimitExceeded'), + errorCode: source.errorCode ?? null, + errorSummary: source.errorSummary ?? null, + error: source.error == null ? null : true, + }; + for (const field of ['partial', 'partiallyApplied']) { + if (Object.hasOwn(source, field)) projected[field] = source[field]; + } + return Object.freeze({ ...projected, ...details }); +} + /** * Is the gateway actually up? * @@ -78,12 +106,7 @@ const gatewayNotRunning = Object.freeze({ async apply(ctx) { const { openclaw } = ctx; const result = await openclaw.invoke(['gateway', 'restart'], { timeoutMs: 60_000 }); - return Object.freeze({ - status: result.status, - timedOut: result.timedOut, - errorSummary: result.errorSummary, - stdout: result.stdout, - }); + return terminalResult(result); }, async verify(ctx) { @@ -108,6 +131,11 @@ function configFlag(value) { return null; } +function configValue(read) { + if (!read || read.ok !== true || typeof read.value !== 'string') return null; + return read.value; +} + /** * A repair that flips one boolean OpenClaw config key to `target`. * @@ -128,10 +156,15 @@ function configToggleRepair({ id, key, target, title, description, blockedReason risk, async preflight(ctx) { - const current = await ctx.openclaw.configGet(key, { timeoutMs: 10_000 }); + const read = await ctx.openclaw.configGet(key, { timeoutMs: 10_000 }); + const current = configValue(read); const flag = configFlag(current); if (flag === null) { - return Object.freeze({ ok: false, reason: 'config_state_unknown', evidence: { key, current } }); + return Object.freeze({ + ok: false, + reason: 'config_state_unknown', + evidence: { key, current: current ?? '', errorSummary: read?.errorSummary ?? null }, + }); } if (flag === target) { return Object.freeze({ ok: false, reason: blockedReason, evidence: { key, current } }); @@ -152,25 +185,43 @@ function configToggleRepair({ id, key, target, title, description, blockedReason async apply(ctx) { const result = await ctx.openclaw.configSet(key, targetText, { timeoutMs: 30_000 }); - return Object.freeze({ - status: result.status, - timedOut: result.timedOut, - errorSummary: result.errorSummary, + const failure = applyFailureReason(result); + return terminalResult(result, { + changed: failure === null ? true : 'unknown', + changes: failure === null + ? Object.freeze([Object.freeze({ type: 'config', key, before: previousText, after: targetText })]) + : Object.freeze([]), }); }, async verify(ctx) { - const current = await ctx.openclaw.configGet(key, { timeoutMs: 10_000 }); - return Object.freeze({ ok: configFlag(current) === target, evidence: { key, current } }); + const read = await ctx.openclaw.configGet(key, { timeoutMs: 10_000 }); + const current = configValue(read); + return Object.freeze({ + ok: configFlag(current) === target, + evidence: { key, current: current ?? '', errorSummary: read?.errorSummary ?? null }, + }); }, async rollback(ctx) { const result = await ctx.openclaw.configSet(key, previousText, { timeoutMs: 30_000 }); + if (result.status !== 0) { + return Object.freeze({ + rolledBack: false, + note: `Could not restore ${key}; check \`openclaw config get ${key}\`.`, + }); + } + const read = await ctx.openclaw.configGet(key, { timeoutMs: 10_000 }); + const current = configValue(read); + const restored = configFlag(current) === !target; return Object.freeze({ - rolledBack: result.status === 0, - note: result.status === 0 + rolledBack: restored, + note: restored ? `Restored ${key} to ${previousText}.` - : `Could not restore ${key}; check \`openclaw config get ${key}\`.`, + : read?.ok === true + ? `Rollback command completed but ${key} was not restored to ${previousText}.` + : `Rollback command completed but ${key} could not be read back: ` + + `${read?.errorSummary ?? 'unknown read failure'}.`, }); }, }); @@ -213,64 +264,193 @@ const gatewayLoopbackNoAuth = Object.freeze({ id: 'gateway-loopback-no-auth', title: 'Require a token on the gateway', description: - 'The gateway accepts unauthenticated connections. This switches auth to token mode and has ' - + 'OpenClaw generate one. Clients will need that token to connect afterwards.', + 'The gateway accepts unauthenticated connections. This switches auth to token mode, reuses ' + + 'an existing token or has OpenClaw generate one when missing, and verifies token presence ' + + 'without recording the token. Clients will need that token to connect afterwards.', // Medium, not low: existing clients stop working until they carry the new token. risk: 'medium', async preflight(ctx) { - const mode = await ctx.openclaw.configGet('gateway.auth.mode', { timeoutMs: 10_000 }); - const current = String(mode || '').trim().toLowerCase(); + const read = await ctx.openclaw.configGet('gateway.auth.mode', { timeoutMs: 10_000 }); + const mode = configValue(read); + const current = String(mode ?? '').trim().toLowerCase(); + if (mode === null || current === '') { + return Object.freeze({ + ok: false, + reason: 'config_state_unknown', + evidence: { mode: current, errorSummary: read?.errorSummary ?? null }, + }); + } if (current === 'token' || current === 'password' || current === 'trusted-proxy') { return Object.freeze({ ok: false, reason: 'gateway_auth_already_enabled', evidence: { mode: current } }); } - return Object.freeze({ ok: true, evidence: { mode: current || '(unset)' } }); + if (current !== 'none') { + return Object.freeze({ ok: false, reason: 'config_state_unknown', evidence: { mode: current } }); + } + return Object.freeze({ ok: true, evidence: { mode: current } }); }, async preview() { return Object.freeze({ steps: Object.freeze([ + 'Check whether gateway.auth.token is present without recording its value.', 'Set gateway.auth.mode to token through the OpenClaw CLI (argv, no shell).', - 'Run `openclaw doctor --fix --generate-gateway-token` so OpenClaw generates the token.', - 'Read gateway.auth.mode back to confirm token auth is active.', + 'If no token exists, run `openclaw doctor --fix --generate-gateway-token` so OpenClaw generates one.', + 'Read gateway.auth.mode and token presence back to confirm token auth is usable.', 'Restart the gateway yourself for it to take effect; existing clients need the new token.', ]), - summary: 'openclaw config set gateway.auth.mode token + doctor --generate-gateway-token', + summary: 'verify/generate gateway token + openclaw config set gateway.auth.mode token', }); }, async apply(ctx) { + const changes = []; + const tokenBefore = await ctx.openclaw.configHasValue( + 'gateway.auth.token', + { timeoutMs: 10_000 }, + ); + if (!tokenBefore.ok) { + return terminalResult(tokenBefore, { + stage: 'read-token-state', + changed: false, + changes: Object.freeze(changes), + errorSummary: tokenBefore.errorSummary || 'could not determine gateway token presence', + }); + } const set = await ctx.openclaw.configSet('gateway.auth.mode', 'token', { timeoutMs: 30_000 }); - if (set.status !== 0) { - return Object.freeze({ status: set.status, stage: 'set-mode', errorSummary: set.errorSummary }); + if (applyFailureReason(set) !== null) { + return terminalResult(set, { + stage: 'set-mode', + changed: 'unknown', + changes: Object.freeze(changes), + tokenPreviouslyPresent: tokenBefore.present, + tokenMayHaveChanged: false, + }); } - const generated = await ctx.openclaw.invoke( - ['doctor', '--fix', '--generate-gateway-token'], - { timeoutMs: 120_000 }, - ); - return Object.freeze({ - status: generated.status, + changes.push(Object.freeze({ + type: 'config', + key: 'gateway.auth.mode', + before: 'none', + after: 'token', + })); + if (tokenBefore.present) { + return terminalResult(set, { + stage: 'set-mode', + changed: true, + changes: Object.freeze(changes), + tokenPreviouslyPresent: true, + tokenMayHaveChanged: false, + }); + } + let generated; + try { + generated = await ctx.openclaw.invoke( + ['doctor', '--fix', '--generate-gateway-token'], + { timeoutMs: 120_000 }, + ); + } catch (error) { + return terminalResult(null, { + stage: 'generate-token', + changed: true, + changes: Object.freeze(changes), + tokenPreviouslyPresent: false, + tokenMayHaveChanged: true, + errorSummary: error.message, + error: true, + }); + } + return terminalResult(generated, { stage: 'generate-token', - timedOut: generated.timedOut, - errorSummary: generated.errorSummary, + changed: true, + changes: Object.freeze(changes), + tokenPreviouslyPresent: false, + tokenMayHaveChanged: true, }); }, + // Presence is the strongest safe local assertion: a client authentication round trip would + // require handling token material, so end-to-end token usability remains an external OpenClaw + // semantic rather than evidence retained by ClawFix. async verify(ctx) { - // Evidence is the mode only — never read the token itself into a repair record. - const mode = String(await ctx.openclaw.configGet('gateway.auth.mode', { timeoutMs: 10_000 })).trim(); - return Object.freeze({ ok: mode.toLowerCase() === 'token', evidence: { mode } }); + const [modeRead, tokenState] = await Promise.all([ + ctx.openclaw.configGet('gateway.auth.mode', { timeoutMs: 10_000 }), + ctx.openclaw.configHasValue('gateway.auth.token', { timeoutMs: 10_000 }), + ]); + const mode = String(configValue(modeRead) ?? '').trim(); + return Object.freeze({ + ok: modeRead?.ok === true + && mode.toLowerCase() === 'token' + && tokenState.ok === true + && tokenState.present === true, + evidence: { + mode, + tokenPresent: tokenState.ok === true ? tokenState.present : null, + errorSummary: modeRead?.errorSummary ?? tokenState.errorSummary ?? null, + }, + }); }, async rollback(ctx, { applyResult } = {}) { - if (applyResult?.stage === 'set-mode') { + if (!applyResult?.changed) { return Object.freeze({ rolledBack: false, note: 'Auth mode was never changed.' }); } + const setMode = await ctx.openclaw.configSet( + 'gateway.auth.mode', + 'none', + { timeoutMs: 30_000 }, + ); + if (setMode.status !== 0) { + return Object.freeze({ + rolledBack: false, + note: 'Could not restore gateway.auth.mode; inspect it before restarting the gateway.', + }); + } + const modeRead = await ctx.openclaw.configGet( + 'gateway.auth.mode', + { timeoutMs: 10_000 }, + ); + const mode = String(configValue(modeRead) ?? '').trim().toLowerCase(); + if (modeRead?.ok !== true || mode !== 'none') { + return Object.freeze({ + rolledBack: false, + note: modeRead?.ok === true + ? `Rollback command completed but gateway.auth.mode is ${mode || '(empty)'}, not none.` + : 'Rollback command completed but gateway.auth.mode could not be read back: ' + + `${modeRead?.errorSummary ?? 'unknown read failure'}.`, + }); + } + + if (applyResult.tokenPreviouslyPresent === false && applyResult.tokenMayHaveChanged) { + const unset = await ctx.openclaw.configUnset( + 'gateway.auth.token', + { timeoutMs: 30_000 }, + ); + if (unset.status !== 0) { + return Object.freeze({ + rolledBack: false, + note: 'Restored gateway.auth.mode to none, but could not remove the token this repair may have generated.', + }); + } + const tokenState = await ctx.openclaw.configHasValue( + 'gateway.auth.token', + { timeoutMs: 10_000 }, + ); + if (!tokenState.ok || tokenState.present) { + return Object.freeze({ + rolledBack: false, + note: tokenState.ok + ? 'Restored gateway.auth.mode to none, but the generated token is still present.' + : 'Restored gateway.auth.mode to none, but token absence could not be verified: ' + + `${tokenState.errorSummary ?? 'unknown read failure'}.`, + }); + } + } + return Object.freeze({ - rolledBack: false, - note: 'Gateway auth was switched to token mode. To undo it deliberately, run ' - + '`openclaw config set gateway.auth.mode none` — that returns the gateway to accepting ' - + 'unauthenticated connections.', + rolledBack: true, + note: applyResult.tokenPreviouslyPresent === false && applyResult.tokenMayHaveChanged + ? 'Restored gateway.auth.mode to none and verified the generated token was removed.' + : 'Restored gateway.auth.mode to its previous value, none.', }); }, }); diff --git a/cli/core/repair-engine.js b/cli/core/repair-engine.js index 01e00c8..a0f8a70 100644 --- a/cli/core/repair-engine.js +++ b/cli/core/repair-engine.js @@ -22,14 +22,64 @@ function defaultRandomToken() { } /** Rollback is best-effort cleanup — a throw here must never mask the apply/verify outcome. */ -async function safeRollback(entry, ctx, applyResult) { +async function safeRollback(entry, ctx, applyResult, preflight) { try { - return await entry.rollback(ctx, { applyResult }); + return await entry.rollback(ctx, { applyResult, preflight }); } catch (error) { return Object.freeze({ rolledBack: false, note: `rollback failed: ${error.message}` }); } } +function safeResultText(value, fallback) { + try { + const text = String(value) + .replace(/[\u0000-\u001f\u007f-\u009f]/g, ' ') + .trim(); + return text ? text.slice(0, 200) : fallback; + } catch { + return fallback; + } +} + +export function applyFailureReason(result) { + if (!result || typeof result !== 'object' || Array.isArray(result)) { + return 'adapter returned no structured result'; + } + + const terminalFlags = [ + ['timedOut', 'command timed out'], + ['aborted', 'command was aborted'], + ['outputLimitExceeded', 'command output limit was exceeded'], + ['stdoutTruncated', 'command stdout was truncated'], + ['stderrTruncated', 'command stderr was truncated'], + ['partial', 'command reported a partial apply'], + ['partiallyApplied', 'command reported a partial apply'], + ]; + for (const [field, message] of terminalFlags) { + if (Object.hasOwn(result, field) && typeof result[field] !== 'boolean') { + return `adapter returned invalid ${field} metadata`; + } + if (result[field] === true) return message; + } + + if (result.signal != null) { + return `command terminated by signal ${safeResultText(result.signal, 'unknown')}`; + } + if (result.error != null) { + return `adapter error: ${safeResultText(result.error?.message || result.error, 'unknown error')}`; + } + if (result.errorCode != null) { + return `adapter error code ${safeResultText(result.errorCode, 'unknown')}`; + } + const errorSummary = result.errorSummary == null ? '' : safeResultText(result.errorSummary, ''); + if (errorSummary) { + return errorSummary; + } + if (result.changed === 'unknown') return 'adapter could not determine whether it changed state'; + if (result.status !== 0) return `status ${safeResultText(result.status, 'unknown')}`; + return null; +} + function stableFingerprintInput(finding, revision) { return JSON.stringify({ revision, @@ -160,7 +210,36 @@ export function createRepairEngine({ catalog = {}, now = () => Date.now(), rando try { applyResult = await entry.apply(ctx); } catch (error) { - return Object.freeze({ status: 'error', error: error.message, plan, preview }); + applyResult = Object.freeze({ + status: null, + changed: 'unknown', + changes: Object.freeze([]), + errorSummary: error.message, + }); + const rollback = await safeRollback(entry, ctx, applyResult, preflight); + return Object.freeze({ + status: 'error', + error: `apply failed: ${error.message}`, + plan, + preview, + applyResult, + rollback, + }); + } + + const applyFailure = applyFailureReason(applyResult); + if (applyFailure) { + // A failed/ambiguous process result may still have changed state. Roll back every returned + // failure rather than trusting an optional `changed` flag supplied by the failing adapter. + const rollback = await safeRollback(entry, ctx, applyResult, preflight); + return Object.freeze({ + status: 'error', + error: `apply failed: ${applyFailure}`, + plan, + preview, + applyResult, + rollback, + }); } // Past this point the repair has run. Every remaining failure must still be reported as a @@ -170,7 +249,7 @@ export function createRepairEngine({ catalog = {}, now = () => Date.now(), rando try { verify = await entry.verify(ctx); } catch (error) { - const rollback = await safeRollback(entry, ctx, applyResult); + const rollback = await safeRollback(entry, ctx, applyResult, preflight); return Object.freeze({ status: 'verify_failed', plan, @@ -182,7 +261,7 @@ export function createRepairEngine({ catalog = {}, now = () => Date.now(), rando } if (!verify.ok) { - const rollback = await safeRollback(entry, ctx, applyResult); + const rollback = await safeRollback(entry, ctx, applyResult, preflight); return Object.freeze({ status: 'verify_failed', plan, preview, applyResult, verify, rollback }); } diff --git a/cli/interfaces/plain.js b/cli/interfaces/plain.js index 87cc840..52e4579 100755 --- a/cli/interfaces/plain.js +++ b/cli/interfaces/plain.js @@ -419,9 +419,9 @@ async function applyCatalogRepair(issue, rl, session) { }); if (result.status === 'applied') { - console.log(` ${c.green('✅')} Gateway restarted and verified.`); + console.log(` ${c.green('✅')} ${plan.title} applied and verified.`); } else if (result.status === 'verify_failed') { - console.log(` ${c.yellow('⚠️')} Restart ran, but the gateway is still unavailable.`); + console.log(` ${c.yellow('⚠️')} Repair ran, but verification failed.`); } else if (result.status === 'blocked') { console.log(` ${c.dim('ℹ️')} Repair no longer needed: ${result.reason}`); } else if (result.status === 'rejected') { @@ -494,14 +494,16 @@ async function applyBuiltinFix(issue, builtinFix, rl, scanFn) { console.log(` ${c.green('✅')} ${change}`); } - // Restart if needed + // Restart if needed. A failed restart is a failed verification, never a soft success. + let restartVerified = true; if (builtinFix.needsRestart) { process.stdout.write(` ${c.blue('🔄')} Restarting gateway...`); - const ok = tryGatewayRestart(); - console.log(ok ? ` ${c.green('✅')}` : ` ${c.yellow('⚠️ may need manual restart')}`); + restartVerified = tryGatewayRestart(); + console.log(restartVerified ? ` ${c.green('✅')}` : ` ${c.yellow('⚠️ restart failed')}`); } - // Re-scan to verify + // Re-scan to verify. Legacy fixes are never reported as applied without this evidence. + let status = 'unverified'; if (scanFn) { process.stdout.write(` ${c.blue('🔍')} Re-scanning...`); const scanResult = await scanFn(); @@ -509,18 +511,26 @@ async function applyBuiltinFix(issue, builtinFix, rl, scanFn) { const allAfter = mergeIssues(scanResult.issues, scanResult.serverIssues); const stillPresent = allAfter.some(candidate => candidate.id === issue.id); - if (stillPresent) { - console.log(` ${c.yellow('⚠️ issue may persist until gateway fully restarts')}`); + if (stillPresent || !restartVerified) { + status = 'verify_failed'; + console.log(` ${c.yellow('⚠️ verification failed')}`); } else { + status = 'applied'; console.log(` ${c.green('✅ Issue resolved!')}`); } } else { console.log(` ${c.dim('skipped')}`); } + } else { + console.log(` ${c.yellow('⚠️')} Verification unavailable; repair is not marked applied.`); } console.log(''); - return { applied: true }; + return { + status, + applied: status === 'applied', + backupPath, + }; } catch (err) { console.log(` ${c.red('❌')} Error: ${err.message}`); @@ -528,104 +538,40 @@ async function applyBuiltinFix(issue, builtinFix, rl, scanFn) { console.log(` ${c.dim(`Rollback available: cp ${backupPath} ${CONFIG_PATH}`)}`); } console.log(''); - return { error: err.message }; + return { status: 'error', applied: false, error: err.message, backupPath }; } } /** - * Apply all fixable issues at once with single backup and single restart + * Batch mutation is intentionally disabled. Each repair must keep its own approval, transaction, + * rollback, and verification boundary; the old batch path bypassed the catalog repair engine and + * counted attempted legacy mutations as applied. */ -async function applyAllFixes(issues, serverIssues, rl, scanFn) { +async function applyAllFixes(issues, serverIssues) { const allIssues = mergeIssues(issues, serverIssues); - const fixable = allIssues.filter(i => BUILTIN_FIXES[i.repairId] && !BUILTIN_FIXES[i.repairId].informational); + const fixable = allIssues.filter(issue => ( + repairCatalog[issue.repairId] + || (BUILTIN_FIXES[issue.repairId] && !BUILTIN_FIXES[issue.repairId].informational) + )); if (fixable.length === 0) { console.log(c.dim(' No auto-fixable issues found.')); - return null; + return { status: 'blocked', reason: 'no_fixable_issues', total: 0 }; } console.log(''); - console.log(c.bold(` Fix plan (${fixable.length} issues):`)); + console.log(c.yellow(' Batch repair is disabled: each repair requires individual approval and verification.')); + console.log(c.dim(' Apply repairs one at a time:')); for (const issue of fixable) { - const fix = BUILTIN_FIXES[issue.repairId]; - const risk = fix.risk === 'low' ? c.green('low') : c.yellow(fix.risk); - console.log(` ${c.blue('🔧')} [${risk}] ${issue.title || issue.text}`); - console.log(` ${c.dim(fix.description)}`); - } - - const skipped = allIssues.filter(i => BUILTIN_FIXES[i.repairId]?.informational); - if (skipped.length) { - console.log(''); - for (const issue of skipped) { - console.log(` ${c.dim(`ℹ️ [SKIP] ${issue.title || issue.text} — informational`)}`); - } + const index = allIssues.indexOf(issue) + 1; + console.log(` ${c.blue(`fix ${index}`)} ${issue.title || issue.text}`); } - - const noFix = allIssues.filter(i => !BUILTIN_FIXES[i.repairId] && !i.fix); - if (noFix.length) { - console.log(''); - for (const issue of noFix) { - console.log(` ${c.dim(`❓ [MANUAL] ${issue.title || issue.text} — ask AI for help`)}`); - } - } - console.log(''); - const answer = await new Promise(resolve => { - rl.question(` ${c.yellow(`Apply ${fixable.length} fix(es)?`)} [Y/n] `, resolve); - }); - - if (answer.trim() && !/^y(es)?$/i.test(answer.trim())) { - console.log(c.dim(' Cancelled.')); - console.log(''); - return null; - } - - // Single backup - const backupPath = await backupConfig(); - console.log(` ${c.green('✅')} Config backed up → ${c.dim(backupPath.split('/').pop())}`); - - // Read config once - let config = await readConfig(); - let needsRestart = false; - let applied = 0; - - for (const issue of fixable) { - const fix = BUILTIN_FIXES[issue.repairId]; - try { - const result = await fix.apply(config); - for (const change of result.changes) { - console.log(` ${c.green('✅')} ${change}`); - } - if (fix.needsRestart) needsRestart = true; - applied++; - } catch (err) { - console.log(` ${c.red('❌')} ${issue.title || issue.text}: ${err.message}`); - } - } - - // Write config once - await safeWriteConfig(config); - console.log(` ${c.green('✅')} Config saved`); - - // Restart once - if (needsRestart) { - process.stdout.write(` ${c.blue('🔄')} Restarting gateway...`); - const ok = tryGatewayRestart(); - console.log(ok ? ` ${c.green('✅')}` : ` ${c.yellow('⚠️ may need manual restart')}`); - } - - // Re-scan - if (scanFn) { - process.stdout.write(` ${c.blue('🔍')} Re-scanning...`); - await scanFn(); - console.log(` ${c.green('done')}`); - } - - console.log(''); - console.log(c.green(` ✅ ${applied}/${fixable.length} fix(es) applied.`)); - if (backupPath) console.log(c.dim(` Rollback: cp ${backupPath} ${CONFIG_PATH}`)); - console.log(''); - return { applied, total: fixable.length }; + return { + status: 'blocked', + reason: 'individual_approval_required', + total: fixable.length, + }; } // ============================================================ diff --git a/test/openclaw-adapter.test.js b/test/openclaw-adapter.test.js index 0bdf9cd..c198ddf 100644 --- a/test/openclaw-adapter.test.js +++ b/test/openclaw-adapter.test.js @@ -176,6 +176,111 @@ test('version and gatewayStatus construct immutable argv and bounded process opt assert.equal(calls[1].options.maxStderrBytes, 256 * 1024); }); +test('configGet distinguishes an empty value from a failed invocation', async () => { + const results = [ + Object.freeze({ status: 0, stdout: '', stderr: '' }), + Object.freeze({ status: 1, stdout: '', stderr: 'cannot read config' }), + ]; + const processAdapter = { + async run() { + return results.shift(); + }, + }; + const fs = { + async access() {}, + async stat() { return { isFile: () => true }; }, + }; + const adapter = createOpenClawAdapter({ + env: { PATH: '/tools' }, + fs, + platform: 'linux', + processAdapter, + }); + + assert.deepEqual(await adapter.configGet('gateway.auth.mode'), { + ok: true, + value: '', + status: 0, + errorSummary: null, + }); + const failed = await adapter.configGet('gateway.auth.mode'); + assert.equal(failed.ok, false); + assert.equal(failed.value, ''); + assert.equal(failed.status, 1); + assert.match(failed.errorSummary, /status 1/); + assert.equal(Object.isFrozen(failed), true); +}); + +test('configHasValue reports only presence and configUnset uses literal argv', async () => { + const calls = []; + const processAdapter = { + async run(_executable, argv) { + calls.push(argv); + if (argv[1] === 'get') { + return Object.freeze({ status: 0, stdout: 'super-secret-token\n', stderr: '' }); + } + return Object.freeze({ status: 0, stdout: '', stderr: '' }); + }, + }; + const fs = { + async access() {}, + async stat() { return { isFile: () => true }; }, + }; + const adapter = createOpenClawAdapter({ + env: { PATH: '/tools' }, + fs, + platform: 'linux', + processAdapter, + }); + + const presence = await adapter.configHasValue('gateway.auth.token'); + assert.deepEqual(presence, { + ok: true, + present: true, + status: 0, + errorSummary: null, + }); + assert.equal(JSON.stringify(presence).includes('super-secret-token'), false); + + const unset = await adapter.configUnset('gateway.auth.token'); + assert.equal(unset.status, 0); + assert.deepEqual(calls, [ + ['config', 'get', 'gateway.auth.token'], + ['config', 'unset', 'gateway.auth.token'], + ]); +}); + +test('configHasValue fails closed without returning failed stdout', async () => { + const adapter = createOpenClawAdapter({ + env: { PATH: '/tools' }, + fs: { + async access() {}, + async stat() { return { isFile: () => true }; }, + }, + platform: 'linux', + processAdapter: { + async run() { + return Object.freeze({ status: 1, stdout: 'must-not-escape', stderr: 'failed' }); + }, + }, + }); + + const presence = await adapter.configHasValue('gateway.auth.token'); + assert.equal(presence.ok, false); + assert.equal(presence.present, false); + assert.equal(JSON.stringify(presence).includes('must-not-escape'), false); +}); + +test('configGet rejects invalid keys without invoking OpenClaw', async () => { + const adapter = createOpenClawAdapter(); + assert.deepEqual(await adapter.configGet('bad key'), { + ok: false, + value: '', + status: null, + errorSummary: 'invalid config key', + }); +}); + test('runtime collectors pass hostile values as literal argv and parse Linux service evidence', async () => { const calls = []; const processAdapter = { diff --git a/test/repair-catalog.test.js b/test/repair-catalog.test.js index e00a2a7..29540b7 100644 --- a/test/repair-catalog.test.js +++ b/test/repair-catalog.test.js @@ -2,6 +2,7 @@ import test from 'node:test'; import assert from 'node:assert/strict'; import { repairCatalog } from '../cli/core/repair-catalog.js'; +import { createRepairEngine } from '../cli/core/repair-engine.js'; function fakeOpenClaw({ statusText = '', pid = '', invokeResult } = {}) { const calls = []; @@ -66,6 +67,90 @@ test('apply invokes the OpenClaw adapter with an argv array, never a shell strin } }); +test('gateway restart apply preserves every terminal-failure field for the engine', async () => { + const terminalFailure = { + status: 0, + signal: 'SIGTERM', + timedOut: true, + aborted: true, + stdoutTruncated: true, + stderrTruncated: true, + outputLimitExceeded: true, + errorCode: 'ETIMEDOUT', + errorSummary: 'bounded failure', + error: new Error('private detail'), + }; + const entry = repairCatalog['gateway-not-running']; + const applied = await entry.apply({ + openclaw: { invoke: async () => terminalFailure }, + }); + + assert.deepEqual(applied, { + status: 0, + signal: 'SIGTERM', + timedOut: true, + aborted: true, + stdoutTruncated: true, + stderrTruncated: true, + outputLimitExceeded: true, + errorCode: 'ETIMEDOUT', + errorSummary: 'bounded failure', + error: true, + }); +}); + +test('gateway restart rejects explicit invalid terminal markers before verification', async (t) => { + for (const marker of [ + 'timedOut', + 'aborted', + 'stdoutTruncated', + 'stderrTruncated', + 'outputLimitExceeded', + 'partial', + 'partiallyApplied', + ]) { + for (const [label, value] of [['null', null], ['undefined', undefined]]) { + await t.test(`${marker}=${label}`, async () => { + let listeningChecks = 0; + const ctx = { + openclaw: { + async gatewayStatusText() { return 'not running'; }, + async gatewayProcesses() { return ''; }, + async gatewayListening() { + listeningChecks += 1; + return listeningChecks > 1; + }, + async invoke() { return { status: 0, [marker]: value }; }, + }, + wait: async () => {}, + }; + const finding = { + id: `gateway-${marker}`, + title: 'Gateway is not running', + severity: 'medium', + repairable: true, + repairId: 'gateway-not-running', + evidence: {}, + }; + const engine = createRepairEngine({ catalog: repairCatalog }); + const plan = engine.createPlan({ finding, revision: 'rev-invalid-marker' }); + + const result = await engine.applyPlan({ + planId: plan.planId, + approvalToken: plan.approvalToken, + revision: 'rev-invalid-marker', + finding, + ctx, + }); + + assert.equal(result.status, 'error'); + assert.match(result.error, new RegExp(`invalid ${marker} metadata`)); + assert.equal(listeningChecks, 1, 'verification must not run after ambiguous apply metadata'); + }); + } + } +}); + test('verify uses live runtime evidence (process/port), not any title comparison', async () => { const entry = repairCatalog['gateway-not-running']; const ctx = { @@ -166,21 +251,77 @@ test('without a port probe the filtered PID evidence is used', async () => { // auto-update-enabled-warning // ============================================================ -function configCtx(values, { setStatus = 0, invokeStatus = 0 } = {}) { +function configCtx(values, { + setStatus = 0, + setResult = null, + invokeStatus = 0, + invokeResult = null, + invokeThrows = false, + invokeCreatesToken = true, + invokeCreatesTokenOnFailure = false, + setNoOpKeys = [], + unsetStatus = 0, + unsetNoOpKeys = [], + unreadableKeys = [], + unreadableOnGetCounts = {}, +} = {}) { const store = { ...values }; const calls = []; + const getCounts = new Map(); return { calls, store, ctx: { openclaw: { - async configGet(key) { calls.push(['get', key]); return store[key] ?? ''; }, + async configGet(key) { + calls.push(['get', key]); + const count = (getCounts.get(key) ?? 0) + 1; + getCounts.set(key, count); + if (unreadableKeys.includes(key) + || unreadableOnGetCounts[key]?.includes(count)) { + return { ok: false, value: '', status: 1, errorSummary: 'read failed' }; + } + return { ok: true, value: store[key] ?? '', status: 0, errorSummary: null }; + }, + async configHasValue(key) { + calls.push(['has', key]); + const count = (getCounts.get(key) ?? 0) + 1; + getCounts.set(key, count); + if (unreadableKeys.includes(key) + || unreadableOnGetCounts[key]?.includes(count)) { + return { ok: false, present: false, status: 1, errorSummary: 'read failed' }; + } + return { + ok: true, + present: String(store[key] ?? '').trim().length > 0, + status: 0, + errorSummary: null, + }; + }, async configSet(key, value) { calls.push(['set', key, value]); - if (setStatus === 0) store[key] = String(value); - return { status: setStatus }; + const result = setResult ?? { status: setStatus }; + if (result.status === 0 && !setNoOpKeys.includes(key)) store[key] = String(value); + return result; + }, + async configUnset(key) { + calls.push(['unset', key]); + if (unsetStatus === 0 && !unsetNoOpKeys.includes(key)) delete store[key]; + return { status: unsetStatus }; + }, + async invoke(argv) { + calls.push(['invoke', argv.join(' ')]); + if (invokeThrows) throw new Error('token generation crashed'); + const result = invokeResult ?? { + status: invokeStatus, + errorSummary: invokeStatus === 0 ? null : 'token generation failed', + }; + if ((result.status === 0 && invokeCreatesToken) + || (result.status !== 0 && invokeCreatesTokenOnFailure)) { + store['gateway.auth.token'] = 'generated-secret'; + } + return result; }, - async invoke(argv) { calls.push(['invoke', argv.join(' ')]); return { status: invokeStatus }; }, async gatewayStatusText() { return ''; }, async gatewayProcesses() { return ''; }, async gatewayListening() { return false; }, @@ -245,18 +386,76 @@ test('gateway auth repair sets token mode and has OpenClaw generate the token', const applied = await entry.apply(ctx); assert.equal(applied.status, 0); assert.equal(store['gateway.auth.mode'], 'token'); + assert.equal(store['gateway.auth.token'], 'generated-secret'); assert.deepEqual(calls.at(-1), ['invoke', 'doctor --fix --generate-gateway-token']); assert.equal((await entry.verify(ctx)).ok, true); }); -test('gateway auth repair never reads the token into its evidence', async () => { +test('gateway auth repair verifies token presence without exposing its value', async () => { const entry = repairCatalog['gateway-loopback-no-auth']; - const { ctx, calls } = configCtx({ 'gateway.auth.mode': 'none' }); + const { ctx } = configCtx({ 'gateway.auth.mode': 'none' }); await entry.apply(ctx); const verify = await entry.verify(ctx); - assert.equal(JSON.stringify(verify).includes('token.'), false); - assert.equal(calls.some(([, key]) => String(key).includes('auth.token')), false); + assert.equal(verify.ok, true); + assert.equal(verify.evidence.tokenPresent, true); + assert.equal(JSON.stringify(verify).includes('generated-secret'), false); +}); + +test('gateway auth repair reuses an existing token without invoking the generator', async () => { + const entry = repairCatalog['gateway-loopback-no-auth']; + const { ctx, calls, store } = configCtx({ + 'gateway.auth.mode': 'none', + 'gateway.auth.token': 'existing-secret', + }); + + const applied = await entry.apply(ctx); + assert.equal(applied.status, 0); + assert.equal(applied.tokenPreviouslyPresent, true); + assert.equal(calls.some(([verb]) => verb === 'invoke'), false); + assert.equal(store['gateway.auth.token'], 'existing-secret'); + assert.equal((await entry.verify(ctx)).ok, true); +}); + +test('gateway auth set-mode stage does not erase an ambiguous status-zero result', async () => { + const entry = repairCatalog['gateway-loopback-no-auth']; + const { ctx, calls } = configCtx( + { + 'gateway.auth.mode': 'none', + 'gateway.auth.token': 'existing-secret', + }, + { setResult: { status: 0, timedOut: true } }, + ); + + const applied = await entry.apply(ctx); + assert.equal(applied.status, 0); + assert.equal(applied.timedOut, true); + assert.equal(applied.stage, 'set-mode'); + assert.equal(applied.changed, 'unknown'); + assert.equal(calls.some(([verb]) => verb === 'invoke'), false); +}); + +test('gateway auth token-generation stage preserves terminal-failure metadata', async () => { + const entry = repairCatalog['gateway-loopback-no-auth']; + const { ctx } = configCtx( + { 'gateway.auth.mode': 'none' }, + { + invokeCreatesToken: false, + invokeResult: { + status: 0, + signal: 'SIGKILL', + stderrTruncated: true, + outputLimitExceeded: true, + }, + }, + ); + + const applied = await entry.apply(ctx); + assert.equal(applied.status, 0); + assert.equal(applied.signal, 'SIGKILL'); + assert.equal(applied.stderrTruncated, true); + assert.equal(applied.outputLimitExceeded, true); + assert.equal(applied.stage, 'generate-token'); }); test('gateway auth repair is blocked when auth is already required', async () => { @@ -269,6 +468,128 @@ test('gateway auth repair is blocked when auth is already required', async () => } }); +test('gateway auth repair blocks when the current mode cannot be read', async () => { + const entry = repairCatalog['gateway-loopback-no-auth']; + const { ctx, calls } = configCtx({}, { unreadableKeys: ['gateway.auth.mode'] }); + const preflight = await entry.preflight(ctx); + + assert.equal(preflight.ok, false); + assert.equal(preflight.reason, 'config_state_unknown'); + assert.match(preflight.evidence.errorSummary, /read failed/); + assert.equal(calls.some(([verb]) => verb === 'set'), false); +}); + +test('gateway auth repair fails verification when doctor exits zero without a token', async () => { + const entry = repairCatalog['gateway-loopback-no-auth']; + const { ctx } = configCtx( + { 'gateway.auth.mode': 'none' }, + { invokeCreatesToken: false }, + ); + + const applied = await entry.apply(ctx); + assert.equal(applied.status, 0); + assert.equal((await entry.verify(ctx)).ok, false); +}); + +test('gateway auth repair removes a partially generated token during rollback', async () => { + const entry = repairCatalog['gateway-loopback-no-auth']; + const { ctx, store } = configCtx( + { 'gateway.auth.mode': 'none' }, + { invokeStatus: 1, invokeCreatesTokenOnFailure: true }, + ); + const preflight = await entry.preflight(ctx); + const applied = await entry.apply(ctx); + + assert.equal(applied.status, 1); + assert.equal(applied.changed, true); + assert.equal(applied.stage, 'generate-token'); + assert.deepEqual(applied.changes, [{ + type: 'config', + key: 'gateway.auth.mode', + before: 'none', + after: 'token', + }]); + assert.equal(store['gateway.auth.mode'], 'token'); + assert.equal(store['gateway.auth.token'], 'generated-secret'); + + const rollback = await entry.rollback(ctx, { applyResult: applied, preflight }); + assert.equal(rollback.rolledBack, true); + assert.equal(store['gateway.auth.mode'], 'none'); + assert.equal('gateway.auth.token' in store, false); +}); + +test('gateway auth rollback rejects a status-zero mode no-op', async () => { + const entry = repairCatalog['gateway-loopback-no-auth']; + const { ctx, store } = configCtx( + { 'gateway.auth.mode': 'token', 'gateway.auth.token': 'generated-secret' }, + { setNoOpKeys: ['gateway.auth.mode'] }, + ); + const rollback = await entry.rollback(ctx, { + applyResult: { + changed: true, + tokenPreviouslyPresent: false, + tokenMayHaveChanged: true, + }, + }); + + assert.equal(rollback.rolledBack, false); + assert.equal(store['gateway.auth.mode'], 'token'); + assert.equal(store['gateway.auth.token'], 'generated-secret'); +}); + +test('gateway auth rollback rejects an unreadable post-rollback mode', async () => { + const entry = repairCatalog['gateway-loopback-no-auth']; + const { ctx } = configCtx( + { 'gateway.auth.mode': 'token' }, + { unreadableOnGetCounts: { 'gateway.auth.mode': [1] } }, + ); + const rollback = await entry.rollback(ctx, { + applyResult: { changed: true, tokenPreviouslyPresent: true, tokenMayHaveChanged: false }, + }); + + assert.equal(rollback.rolledBack, false); + assert.match(rollback.note, /could not be read back/i); +}); + +test('gateway auth rollback rejects a status-zero token-unset no-op', async () => { + const entry = repairCatalog['gateway-loopback-no-auth']; + const { ctx, store } = configCtx( + { 'gateway.auth.mode': 'token', 'gateway.auth.token': 'generated-secret' }, + { unsetNoOpKeys: ['gateway.auth.token'] }, + ); + const rollback = await entry.rollback(ctx, { + applyResult: { + changed: true, + tokenPreviouslyPresent: false, + tokenMayHaveChanged: true, + }, + }); + + assert.equal(rollback.rolledBack, false); + assert.equal(store['gateway.auth.mode'], 'none'); + assert.equal(store['gateway.auth.token'], 'generated-secret'); +}); + +test('gateway auth rollback rejects unreadable token state after removal', async () => { + const entry = repairCatalog['gateway-loopback-no-auth']; + const { ctx, store } = configCtx( + { 'gateway.auth.mode': 'token', 'gateway.auth.token': 'generated-secret' }, + { unreadableOnGetCounts: { 'gateway.auth.token': [1] } }, + ); + const rollback = await entry.rollback(ctx, { + applyResult: { + changed: true, + tokenPreviouslyPresent: false, + tokenMayHaveChanged: true, + }, + }); + + assert.equal(rollback.rolledBack, false); + assert.equal(store['gateway.auth.mode'], 'none'); + assert.equal('gateway.auth.token' in store, false); + assert.match(rollback.note, /absence could not be verified/i); +}); + test('gateway auth repair carries medium risk and says clients need the new token', async () => { const entry = repairCatalog['gateway-loopback-no-auth']; assert.equal(entry.risk, 'medium'); @@ -305,6 +626,27 @@ for (const toggle of TOGGLES) { assert.equal((await entry.verify(ctx)).ok, true); }); + test(`${toggle.id}: preserves terminal-failure metadata from config set`, async () => { + const entry = repairCatalog[toggle.id]; + const { ctx } = configCtx( + { [toggle.key]: toggle.from }, + { + setResult: { + status: 0, + aborted: true, + stdoutTruncated: true, + errorCode: 'ABORT_ERR', + }, + }, + ); + + const applied = await entry.apply(ctx); + assert.equal(applied.status, 0); + assert.equal(applied.aborted, true); + assert.equal(applied.stdoutTruncated, true); + assert.equal(applied.errorCode, 'ABORT_ERR'); + }); + test(`${toggle.id}: is blocked when the key is already correct`, async () => { const entry = repairCatalog[toggle.id]; const { ctx } = configCtx({ [toggle.key]: toggle.to }); @@ -334,6 +676,28 @@ for (const toggle of TOGGLES) { assert.equal(store[toggle.key], toggle.from); }); + test(`${toggle.id}: rollback rejects a status-zero setter no-op`, async () => { + const entry = repairCatalog[toggle.id]; + const { ctx, store } = configCtx( + { [toggle.key]: toggle.to }, + { setNoOpKeys: [toggle.key] }, + ); + const rollback = await entry.rollback(ctx); + assert.equal(rollback.rolledBack, false); + assert.equal(store[toggle.key], toggle.to); + }); + + test(`${toggle.id}: rollback rejects an unreadable restored value`, async () => { + const entry = repairCatalog[toggle.id]; + const { ctx } = configCtx( + { [toggle.key]: toggle.to }, + { unreadableOnGetCounts: { [toggle.key]: [1] } }, + ); + const rollback = await entry.rollback(ctx); + assert.equal(rollback.rolledBack, false); + assert.match(rollback.note, /could not be read back/i); + }); + test(`${toggle.id}: preview names the exact command and has no side effects`, async () => { const entry = repairCatalog[toggle.id]; const { ctx, calls } = configCtx({ [toggle.key]: toggle.from }); diff --git a/test/repair-engine.test.js b/test/repair-engine.test.js index 9417055..baa4d75 100644 --- a/test/repair-engine.test.js +++ b/test/repair-engine.test.js @@ -13,7 +13,9 @@ function gatewayFinding(overrides = {}) { return finding; } -function fakeCatalogEntry({ preflightOk = true, verifyOk = true } = {}) { +function fakeCatalogEntry(options = {}) { + const { preflightOk = true, verifyOk = true } = options; + const applyResult = Object.hasOwn(options, 'applyResult') ? options.applyResult : { status: 0 }; const calls = []; return { calls, @@ -30,7 +32,7 @@ function fakeCatalogEntry({ preflightOk = true, verifyOk = true } = {}) { }, async apply() { calls.push('apply'); - return { status: 0 }; + return applyResult; }, async verify() { calls.push('verify'); @@ -238,6 +240,19 @@ test('the real fix command routes catalog repairs through the repair engine befo assert.match(source, /const catalogRepair = repairCatalog\[issue\.repairId\]/); assert.match(source, /if \(catalogRepair\) \{\s*await applyCatalogRepair\(issue, rl, session\)/); assert.match(source, /revision: result\.revision/); + + const batchStart = source.indexOf('async function applyAllFixes'); + const batchEnd = source.indexOf('// Diagnostic core compatibility bridge', batchStart); + const batchSource = source.slice(batchStart, batchEnd); + assert.match(batchSource, /individual approval and verification/i); + assert.doesNotMatch(batchSource, /\.apply\(/, 'fix all must not bypass catalog transactions'); + + const legacyStart = source.indexOf('async function applyBuiltinFix'); + const legacyEnd = source.indexOf('async function applyAllFixes', legacyStart); + const legacyBody = source.slice(legacyStart, batchStart); + assert.doesNotMatch(legacyBody, /return \{ applied: true \}/); + assert.match(legacyBody, /status = 'verify_failed'/); + assert.match(legacyBody, /let status = 'unverified'/); }); // ============================================================ @@ -327,6 +342,98 @@ test('a throwing rollback does not mask the verify failure', async () => { assert.match(outcome.rollback.note, /rollback failed: rollback exploded/); }); +test('a throwing apply attempts rollback and preserves a partial-change outcome', async () => { + const finding = gatewayFinding(); + const entry = fakeCatalogEntry(); + entry.apply = async () => { + entry.calls.push('apply'); + throw new Error('second step crashed'); + }; + const engine = createRepairEngine({ catalog: { 'gateway-not-running': entry } }); + const plan = engine.createPlan({ finding, revision: 'rev-1' }); + + const outcome = await engine.applyPlan({ + planId: plan.planId, + approvalToken: plan.approvalToken, + revision: 'rev-1', + finding, + ctx: {}, + }); + + assert.equal(outcome.status, 'error'); + assert.match(outcome.error, /apply failed: second step crashed/); + assert.equal(outcome.applyResult.changed, 'unknown'); + assert.deepEqual(entry.calls, ['preflight', 'preview', 'apply', 'rollback']); + assert.ok(outcome.rollback); +}); + +test('a failed apply result rolls back recorded changes and never verifies', async () => { + const finding = gatewayFinding(); + const entry = fakeCatalogEntry(); + entry.apply = async () => { + entry.calls.push('apply'); + return { + status: 1, + changed: true, + changes: [{ type: 'config', key: 'gateway.auth.mode', before: 'none', after: 'token' }], + errorSummary: 'second step failed', + }; + }; + const engine = createRepairEngine({ catalog: { 'gateway-not-running': entry } }); + const plan = engine.createPlan({ finding, revision: 'rev-1' }); + + const outcome = await engine.applyPlan({ + planId: plan.planId, + approvalToken: plan.approvalToken, + revision: 'rev-1', + finding, + ctx: {}, + }); + + assert.equal(outcome.status, 'error'); + assert.match(outcome.error, /apply failed: second step failed/); + assert.deepEqual(entry.calls, ['preflight', 'preview', 'apply', 'rollback']); + assert.equal(entry.calls.includes('verify'), false); +}); + +const terminalApplyFailures = [ + ['nonzero status', { status: 1 }], + ['null status', { status: null }], + ['string status', { status: '0' }], + ['timed out', { status: 0, timedOut: true }], + ['aborted', { status: 0, aborted: true }], + ['terminated by signal', { status: 0, signal: 'SIGKILL' }], + ['stdout truncated', { status: 0, stdoutTruncated: true }], + ['stderr truncated', { status: 0, stderrTruncated: true }], + ['output limit exceeded', { status: 0, outputLimitExceeded: true }], + ['error summary present', { status: 0, errorSummary: 'spawn was not clean' }], + ['error code present', { status: 0, errorCode: 'EIO' }], + ['error object present', { status: 0, error: new Error('adapter failure') }], + ['missing result', undefined], +]; + +for (const [scenario, applyResult] of terminalApplyFailures) { + test(`applyPlan treats ${scenario} as failure, rolls back, and never verifies`, async () => { + const finding = gatewayFinding(); + const entry = fakeCatalogEntry({ applyResult }); + const engine = createRepairEngine({ catalog: { 'gateway-not-running': entry } }); + const plan = engine.createPlan({ finding, revision: 'rev-1' }); + + const outcome = await engine.applyPlan({ + planId: plan.planId, + approvalToken: plan.approvalToken, + revision: 'rev-1', + finding, + ctx: {}, + }); + + assert.equal(outcome.status, 'error', scenario); + assert.match(outcome.error, /apply failed:/, scenario); + assert.equal(outcome.applyResult, applyResult, scenario); + assert.deepEqual(entry.calls, ['preflight', 'preview', 'apply', 'rollback'], scenario); + }); +} + test('a throwing preflight reports an error outcome instead of rejecting', async () => { const finding = gatewayFinding(); const entry = fakeCatalogEntry();