feat(recovery): make rollback-recovery image lifecycle visible and controllable (#1753)

* feat(recovery): make rollback-recovery image lifecycle visible and controllable

GitHub discussion #1751 asked why Sencho creates sencho-rb/<id>/<service>:hold
images during automatic updates and how to clean them up. That surfaced a real
safety bug alongside the missing visibility: the manual single-image delete
route did not consult the held-image predicate every other deletion path
already honors, so a user could delete a rollback-protected image straight
through the Images tab and silently break automatic recovery for that update.
A short/truncated id also bypassed the predicate's full-id lookup.

Fixes:
- POST /images/delete now resolves the submitted id to its canonical form and
  checks the unified held-image predicate before deleting, returning 409
  IMAGE_HELD_FOR_ROLLBACK for a protected image.
- The Images tab no longer mislabels a protected image as plain "Unused"; a
  fully-synthetic hold image is kept out of the generic inventory entirely and
  surfaced instead in a new Resources -> Rollback tab, with an additive
  "Rollback protected" badge for images that still carry a normal tag too.

New capability:
- Two settings (Deploy Guardrails): superseded-generation retention (days,
  replaces a hardcoded 7) and a cap on retained generations per stack.
- A new Resources -> Rollback tab lists every generation (stack, short id,
  state, retention) with an admin-gated manual release action, including
  releasing the current generation with an explicit warning that automatic
  rollback becomes unavailable until the next successful update. Release is
  a single atomic, server-revalidated transition so a stale UI read can never
  release a row that has since become ineligible.

Also consolidated three near-duplicate implementations of the held-image
predicate (two of which relied on a require() of a sibling .ts file that
silently failed to resolve under the test runner and was never actually
exercised by a real test before this change) into one shared module.

Known follow-up, not fixed here: an orphaned sencho-rb tag whose recovery row
no longer exists (DB restore, node re-add) is invisible in both the Images
and Rollback tabs with no UI path to reclaim it.

* fix(audit): add summary mapping for rollback generation release

* fix(security): sanitize prune target in log sinks and cover release RBAC

Closes two open js/log-injection findings on the system prune route by
applying the same inline sanitizeForLog barrier the rest of the file
already uses. The prune target is validated against an enum by
parsePruneTargets before reaching these sinks, so the findings were false
positives, but the barrier is cheap and removes the standing alerts on a
file this change already touches. Also wraps the generation id in the
release log line for consistency with the stack name beside it.

Adds coverage for gaps a QA pass identified:
- Release endpoint refuses a viewer and a deployer (Admin-only), leaving
  the generation and its artifacts untouched.
- Viewer can still read the generations list, matching the sibling
  Resources routes.
- The predicate the prune routes build reports full-stack rollback holds,
  not just service-scoped ones, and re-reads per call so a hold taken
  between plan and delete still gates the delete.
- After releasing the current generation, no rollback point is claimed
  for the stack through any consumer of the current-generation lookup.
This commit is contained in:
Anso
2026-08-02 21:55:22 -04:00
committed by GitHub
parent 97be019696
commit 41bf075eb0
31 changed files with 1734 additions and 93 deletions
@@ -70,9 +70,13 @@ function sanitizeServiceSlug(name: string): string {
return name.replace(/[^a-zA-Z0-9._-]/g, '-').toLowerCase() || 'svc';
}
/** Same short form used in the opaque rollback tag, so the UI's "Generation" label matches the Docker tag. */
export function shortGenerationId(generationId: string): string {
return generationId.replace(/-/g, '').slice(0, 12);
}
function opaqueRollbackTag(generationId: string, serviceName: string): string {
const short = generationId.replace(/-/g, '').slice(0, 12);
return `sencho-rb/${short}/${sanitizeServiceSlug(serviceName)}:hold`;
return `sencho-rb/${shortGenerationId(generationId)}/${sanitizeServiceSlug(serviceName)}:hold`;
}
function parseServicesJson(raw: string): StackRecoveryServiceCapture[] {
@@ -284,6 +288,8 @@ export class StackUpdateRecoveryService {
updated_at: now,
created_by: createdBy,
artifacts_retired: 0,
released_at: null,
released_by: null,
};
DatabaseService.getInstance().insertStackUpdateRecoveryGeneration(row);
return row;
@@ -329,7 +335,7 @@ export class StackUpdateRecoveryService {
throw new Error('Stack directory escapes compose base');
}
const short = generationId.replace(/-/g, '').slice(0, 12);
const short = shortGenerationId(generationId);
if (!/^[a-f0-9]{12}$/i.test(short)) {
throw new Error('Invalid recovery generation id');
}
@@ -398,6 +404,74 @@ export class StackUpdateRecoveryService {
return ok;
}
/**
* Informational mirror of releaseStackUpdateRecoveryGeneration's WHERE
* clause, for the list endpoint to grey out a row it already knows is
* ineligible. Not authoritative: releaseGeneration revalidates for real.
*/
public isReleaseEligible(row: StackUpdateRecoveryGenerationRow): boolean {
if (row.released_at !== null || row.artifacts_retired !== 0) return false;
if (row.phase !== 'immediate_verified') return false;
if (!['active', 'restored_current', 'superseded'].includes(row.status)) return false;
if (row.health_gate_id) {
const gate = DatabaseService.getInstance().getHealthGateRun(row.node_id, row.stack_name, row.health_gate_id);
if (gate?.status === 'observing') return false;
}
return true;
}
/**
* Operator-initiated release of rollback protection, current generation
* included. The DB transition (releaseStackUpdateRecoveryGeneration)
* atomically revalidates eligibility and clears is_current, which is what
* stops getCurrent()/isRestoredCurrentPinActive() from reporting a released
* row as the live rollback point. Docker tag + override cleanup reuses the
* same idempotent retireGenerationArtifacts() that abandon() already relies
* on, so a mid-cleanup Docker failure leaves artifacts_retired at 0 and is
* retried by the next reconcileIncomplete() sweep rather than silently
* "succeeding" in the UI.
*/
public async releaseGeneration(
id: string,
releasedBy: string | null,
): Promise<
| { ok: true; row: StackUpdateRecoveryGenerationRow; artifactsCleaned: boolean }
| { ok: false; reason: 'not_found' | 'already_released' | 'not_eligible' }
> {
const before = this.get(id);
if (!before) return { ok: false, reason: 'not_found' };
if (before.released_at !== null) return { ok: false, reason: 'already_released' };
const released = DatabaseService.getInstance().releaseStackUpdateRecoveryGeneration(id, releasedBy);
if (!released) return { ok: false, reason: 'not_eligible' };
const row = this.get(id);
if (!row) return { ok: false, reason: 'not_found' };
const artifactsCleaned = await this.retireGenerationArtifacts(row);
const wasCurrent = before.is_current === 1;
try {
DatabaseService.getInstance().addNotificationHistory(row.node_id, {
level: wasCurrent ? 'warning' : 'info',
category: 'rollback_generation_released',
message: wasCurrent
? `${row.stack_name}: current rollback protection released. Automatic rollback is unavailable until the next successful full-stack update.`
: `${row.stack_name}: rollback protection released for generation ${shortGenerationId(row.id)}.`,
timestamp: Date.now(),
stack_name: row.stack_name,
actor_username: releasedBy,
});
} catch (error) {
console.warn(
'[StackUpdateRecovery] Failed to record release activity for %s:',
sanitizeForLog(id),
sanitizeForLog(getErrorMessage(error, 'unknown')),
);
}
return { ok: true, row, artifactsCleaned };
}
public linkHealthGate(id: string, healthGateId: string): void {
DatabaseService.getInstance().linkStackUpdateRecoveryHealthGate(id, healthGateId);
}
@@ -460,22 +534,6 @@ export class StackUpdateRecoveryService {
}
}
/**
* Unified held-image predicate: service-scoped + full-stack holds.
* Fail closed (skip prune) when either lookup fails.
*/
public buildUnifiedHeldImagePredicate(nodeId: number): (imageId: string) => boolean {
// Dynamic require avoids a static cycle with ServiceUpdateRecoveryService.
// eslint-disable-next-line @typescript-eslint/no-require-imports
const { ServiceUpdateRecoveryService } = require('./ServiceUpdateRecoveryService') as typeof import('./ServiceUpdateRecoveryService');
const serviceHeld = ServiceUpdateRecoveryService.getInstance().getHeldImageIds(nodeId);
const stackHeld = this.getHeldImageIds(nodeId);
if (serviceHeld === null || stackHeld === null) {
return () => true;
}
return (imageId: string) => serviceHeld.has(imageId) || stackHeld.has(imageId);
}
/**
* Post-handoff compensation: restore files + pinned up, then probe before
* reporting restored_current / immediate_verified.
@@ -655,7 +713,20 @@ export class StackUpdateRecoveryService {
}
}
if (!tagsOk || !overrideOk) return false;
DatabaseService.getInstance().markStackUpdateRecoveryArtifactsRetired(row.id);
try {
DatabaseService.getInstance().markStackUpdateRecoveryArtifactsRetired(row.id);
} catch (error) {
// Tags/override are already gone at this point; a DB write failure here
// must not surface as "release/abandon failed" to the caller (the
// mutation it asked for already happened). Leave artifacts_retired at 0
// so the next reconcileIncomplete() sweep retries the DB write alone.
console.warn(
'[StackUpdateRecovery] Failed to mark artifacts retired for %s: %s',
sanitizeForLog(row.id),
sanitizeForLog(getErrorMessage(error, 'unknown')),
);
return false;
}
return true;
}
@@ -680,16 +751,38 @@ export class StackUpdateRecoveryService {
});
flagged += 1;
}
let capped = 0;
const maxGenerations = db.getRecoveryMaxGenerations();
if (maxGenerations > 0) {
// The current generation always counts as one of the cap, so the
// superseded budget is one less; it can never itself be evicted here.
const supersededBudget = Math.max(0, maxGenerations - 1);
const byStack = new Map<string, StackUpdateRecoveryGenerationRow[]>();
for (const row of db.listActiveSupersededGenerations()) {
const key = `${row.node_id}:${row.stack_name}`;
const list = byStack.get(key) ?? [];
list.push(row);
byStack.set(key, list);
}
for (const rows of byStack.values()) {
for (const row of rows.slice(supersededBudget)) {
if (row.artifact_expires_at === null || row.artifact_expires_at > now) {
db.updateStackUpdateRecoveryGeneration(row.id, { artifact_expires_at: now });
capped += 1;
}
}
}
}
let retired = 0;
for (const row of db.listStackUpdateRecoveryGenerationsForArtifactRetirement(now)) {
// Never retire an active/current or recovery_required hold target.
if (row.is_current === 1 || row.status === 'recovery_required') continue;
if (await this.retireGenerationArtifacts(row)) retired += 1;
}
if (abandoned > 0 || flagged > 0 || retired > 0) {
if (abandoned > 0 || flagged > 0 || capped > 0 || retired > 0) {
console.log(
`[StackUpdateRecovery] Reconciled ${abandoned} stale candidate(s), `
+ `${flagged} stuck generation(s), retired ${retired} artifact set(s)`,
+ `${flagged} stuck generation(s), ${capped} generation(s) over cap, retired ${retired} artifact set(s)`,
);
}
} catch (error) {