From 174a32ca3d7aaa89e1f1b7718543b1f79342bbce Mon Sep 17 00:00:00 2001 From: "pulse-triage[bot]" <249995291+pulse-triage[bot]@users.noreply.github.com> Date: Mon, 12 Oct 2026 17:50:55 +0100 Subject: [PATCH 1/6] Keep open guest drawers truthful about completed backups Age the existing completed-backup readout and policy tone on the shared passive clock. Show elapsed age and the validated UTC completion time without clipping; keep activity, absence and invalid times separate. Cover default and custom policy boundaries, clock changes, tab round trips and retained completion with connected controls and desktop/phone browser proof. Change-source: pulse-maintainer --- .../subsystems/frontend-primitives.md | 18 ++ .../subsystems/performance-and-scalability.md | 18 ++ .../2ebeb891380553fa.json | 79 ++++++++ .../components/Workloads/GuestDrawer.test.tsx | 20 +- .../Workloads/GuestDrawerOverview.tsx | 23 ++- .../GuestDrawer.backup-clock.test.tsx | 187 ++++++++++++++++++ .../guestDrawerModel.branchcov2.test.ts | 21 +- .../components/Workloads/guestDrawerModel.ts | 12 +- .../Workloads/useGuestDrawerState.ts | 3 + 9 files changed, 356 insertions(+), 25 deletions(-) create mode 100644 frontend-modern/browser-verification/2ebeb891380553fa.json create mode 100644 frontend-modern/src/components/Workloads/__tests__/GuestDrawer.backup-clock.test.tsx diff --git a/docs/release-control/v6/internal/subsystems/frontend-primitives.md b/docs/release-control/v6/internal/subsystems/frontend-primitives.md index 88ca0ec11e..76759c8ba5 100644 --- a/docs/release-control/v6/internal/subsystems/frontend-primitives.md +++ b/docs/release-control/v6/internal/subsystems/frontend-primitives.md @@ -9416,3 +9416,21 @@ provider precedence, stopping/recovery and zero. The local Workloads snapshot adapter/drawer, History and CSS on desktop and phone, with synthetic GET responses. These controls do not establish native restart, thaw, covered-filesystem writes, installed recovery or containing software. + +### Guest backup completion readout (12 October 2026) + +The existing guest Overview's Last completed backup row shows elapsed age +(hours/minutes for recent points, days for older ones), never a rolling-day +Today/Yesterday claim. A validated completion also has visible UTC date/time +in a semantic `time` element with its original ISO timestamp. Invalid, future +or absent times produce no completion element. The row still distinguishes +running activity from completed protection and preserves the policy's tone; +the passive clock may rejudge a previously future timestamp when it becomes +valid, but cannot supply new completion evidence. + +`GuestDrawer.backup-clock.test.tsx` and the connected drawer/model controls +cover these states and update on time alone. Desktop Chromium and touch-phone +WebKit exercise the real policy producer, consuming drawer, shared clock and +existing table evidence. No route, guest check, backup, threshold, collector or +permission changes. This presentation result does not establish thaw, covered +filesystem writes, native restart recovery or containing released software. diff --git a/docs/release-control/v6/internal/subsystems/performance-and-scalability.md b/docs/release-control/v6/internal/subsystems/performance-and-scalability.md index bbda325c7f..836e0e1492 100644 --- a/docs/release-control/v6/internal/subsystems/performance-and-scalability.md +++ b/docs/release-control/v6/internal/subsystems/performance-and-scalability.md @@ -4116,3 +4116,21 @@ notification is introduced. Current snapshot/delta fast paths retain their existing scope; only availability complete-record replacement changes, never other partial provider facets or independent metric updates. This is truthful presentation and scoped invalidation, not a measured whole-fleet CPU saving. + +### Open guest drawers age completed backups passively (12 October 2026) + +`useGuestDrawerState` derives completed-backup age and policy tone from the +existing shared 30-second clock, not just a new guest snapshot. This adds a +clock subscription for the mounted drawer, not a timer per row, network read, +refresh, guest probe or alert evaluation. Disposal releases the subscription. +The supplied completed timestamp is immutable under ticks and backup activity; +only replacement completion evidence changes it. Configured fresh/stale hours +retain their existing inclusive boundaries and unknown/absent distinctions. + +`GuestDrawer.backup-clock.test.tsx` covers passive default/custom policy +crossings, unchanged completion time, tab round trips, later completion, +future/backwards-clock uncertainty and timer disposal. Connected +`GuestDrawer.test.tsx` and `guestDrawerModel.branchcov2.test.ts` retain existing +backup-activity, configured policy and caution/absence controls. Browser proof +uses the real read-only AlertsAPI/store and existing drawer/table badge with +synthetic timestamps, not native backup safety, guest recovery or availability. diff --git a/frontend-modern/browser-verification/2ebeb891380553fa.json b/frontend-modern/browser-verification/2ebeb891380553fa.json new file mode 100644 index 0000000000..c70030850d --- /dev/null +++ b/frontend-modern/browser-verification/2ebeb891380553fa.json @@ -0,0 +1,79 @@ +{ + "version": 1, + "result": "passed", + "base_sha": "0d8ade2b3b8cf73a60eb9efe07534e45f1d7072a", + "changed_paths": [ + "frontend-modern/src/components/Workloads/GuestDrawerOverview.tsx", + "frontend-modern/src/components/Workloads/guestDrawerModel.ts", + "frontend-modern/src/components/Workloads/useGuestDrawerState.ts" + ], + "content_sha256": { + "frontend-modern/src/components/Workloads/useGuestDrawerState.ts": "801aa50aec4c0ec7b801f4f14da60ef9c9a56a886f6df3f509f3fc844ecf4dca", + "frontend-modern/src/components/Workloads/guestDrawerModel.ts": "66ada43fb44fcdbdbbde5640f21ed92d84115f27011a4d49bdcc8b711e0dc088", + "frontend-modern/src/components/Workloads/GuestDrawerOverview.tsx": "f3a446f40d39212d00dcbef049918297f211fdf5b8ee3b96094bf6438cbe17a9" + }, + "routes": [ + "/tmp/backup-clock/index.html (isolated existing production guest drawer/table fixture, no product route added)" + ], + "states": [ + "unchanged VM/LXC completion across a saved one-hour freshness boundary", + "default 24-hour policy covered by connected source controls", + "running/ended activity separate from prior completion", + "new completion then passive minute ageing", + "absent/invalid/future timestamps", + "previous-calendar-day completion" + ], + "interactions": [ + "read synthetic saved policy through actual AlertsAPI/store", + "advance passive browser clock without API requests; both drawer and table reclassify", + "Manage/Overview round trip retains original completed time", + "replace completion evidence without remounting the workload", + "keyboard and touch open existing table evidence; Escape closes and restores focus", + "inspect six final PNG captures with native view_image at original detail" + ], + "viewports": [ + { + "width": 1440, + "height": 900 + }, + { + "width": 390, + "height": 844 + } + ], + "verified_at": "2026-10-12T16:49:58.639554Z", + "proof_scope": "Read-only synthetic alert policy through real AlertsAPI/store, guest drawer state/model/Overview, existing backup table badge and production CSS. Passive Playwright clocks, no native backup/recovery or availability claim.", + "browser_proof": { + "checks": 32, + "loaded_production_hashes": 115, + "captures": 6, + "client_exit": 0, + "playwright": "1.56.1", + "browsers": [ + { + "name": "desktop-light", + "version": "141.0.7390.37", + "viewport": { + "width": 1440, + "height": 900 + } + }, + { + "name": "phone-dark", + "version": "26.0", + "viewport": { + "width": 390, + "height": 844 + } + } + ], + "cleanup": { + "desktop-light-context": "closed", + "desktop-light-browser": "closed", + "phone-dark-context": "closed", + "phone-dark-browser": "closed", + "server": "closed" + }, + "dossier": "RECEIPTS.md / browser/result.json and native-image-review.json" + } +} diff --git a/frontend-modern/src/components/Workloads/GuestDrawer.test.tsx b/frontend-modern/src/components/Workloads/GuestDrawer.test.tsx index a1f14123aa..85201a76e3 100644 --- a/frontend-modern/src/components/Workloads/GuestDrawer.test.tsx +++ b/frontend-modern/src/components/Workloads/GuestDrawer.test.tsx @@ -319,12 +319,12 @@ describe('GuestDrawer', () => { const details = technicalDetails(); expect(details.getByText('Backup time unavailable: invalid timestamp.')).toBeVisible(); expect(details.queryByText('No completed backup found')).not.toBeInTheDocument(); - expect(details.queryByText('Today')).not.toBeInTheDocument(); + expect(details.queryByText('1 hour ago')).not.toBeInTheDocument(); setGuest({ ...guest(), lastBackup: Date.now() + 86_400_000, backupInProgress: true }); expect(details.getByText(/timestamp is in the future/)).toBeVisible(); expect(details.getByText('Running · not completed yet')).toBeVisible(); setGuest({ ...guest(), lastBackup: Date.now() - 3600_000, backupInProgress: false }); - expect(details.getByText('Today')).toBeVisible(); + expect(details.getByText('1 hour ago')).toBeVisible(); expect(details.queryByText(/unavailable/)).not.toBeInTheDocument(); setGuest({ ...guest(), lastBackup: 0 }); expect(details.getByText('No completed backup found')).toBeVisible(); @@ -1011,17 +1011,17 @@ describe('GuestDrawer', () => { vi.setSystemTime(new Date('2026-03-02T12:00:00Z')); }); - it('shows "Today" for a backup from today', () => { + it('shows elapsed hours for a recent backup', () => { const now = new Date('2026-03-02T10:00:00Z').getTime(); render(() => ); expect(screen.getByText('Last completed backup')).toBeInTheDocument(); - expect(technicalDetails().getByText('Today')).toBeInTheDocument(); + expect(technicalDetails().getByText('2 hours ago')).toBeInTheDocument(); }); - it('shows "Yesterday" for a 1-day-old backup', () => { + it('shows elapsed days for a 1-day-old backup', () => { const yesterday = new Date('2026-03-01T12:00:00Z').getTime(); render(() => ); - expect(technicalDetails().getByText('Yesterday')).toBeInTheDocument(); + expect(technicalDetails().getByText('1d ago')).toBeInTheDocument(); }); it('shows "Xd ago" for older backups', () => { @@ -1054,7 +1054,7 @@ describe('GuestDrawer', () => { it('applies green color for recent backups', () => { const now = new Date('2026-03-02T10:00:00Z').getTime(); render(() => ); - const ageCell = technicalDetails().getByText('Today').closest('td'); + const ageCell = technicalDetails().getByText('2 hours ago').closest('td'); expect(ageCell).toHaveClass('text-emerald-700'); }); @@ -1080,7 +1080,7 @@ describe('GuestDrawer', () => { it.each([ ['2026-02-20T12:00:00Z', '10d ago', 'text-amber-700'], - ['2026-03-02T10:00:00Z', 'Today', 'text-emerald-700'], + ['2026-03-02T10:00:00Z', '2 hours ago', 'text-emerald-700'], ])('preserves completed backup age and tone during a new backup (%s)', (date, age, tone) => { solidRender(() => ( { expect(technicalDetails().getByText('No completed backup found')).toBeInTheDocument(); setGuest({ ...guest(), lastBackup: new Date('2026-03-02T10:00:00Z').getTime() }); expect(technicalDetails().queryByText('No completed backup found')).not.toBeInTheDocument(); - expect(technicalDetails().getByText('Today').closest('td')).toHaveClass('text-emerald-700'); + expect(technicalDetails().getByText('2 hours ago').closest('td')).toHaveClass( + 'text-emerald-700', + ); expect(container.querySelector('[data-testid="guest-technical-details"]')).toBe(drawer); }); diff --git a/frontend-modern/src/components/Workloads/GuestDrawerOverview.tsx b/frontend-modern/src/components/Workloads/GuestDrawerOverview.tsx index 319748c473..fcb8d8675f 100644 --- a/frontend-modern/src/components/Workloads/GuestDrawerOverview.tsx +++ b/frontend-modern/src/components/Workloads/GuestDrawerOverview.tsx @@ -26,6 +26,7 @@ import { getGuestDrawerMemoryReading, getGuestDrawerGuestReadPresentation, isGuestDrawerVM, + type GuestDrawerBackupPresentation, } from './guestDrawerModel'; import type { NestedWorkloadContext } from './nestedWorkloadContext'; import { WORKLOAD_ACTION_AGENT_LABEL } from './workloadAgentReadiness'; @@ -59,11 +60,7 @@ interface GuestDrawerOverviewProps { networkInterfaces: NonNullable; nestedWorkloadContext?: NestedWorkloadContext; normalizedTags: string[]; - backupPresentation: { - ageClass: string; - ageLabel: string; - dateLabel: string; - } | null; + backupPresentation: GuestDrawerBackupPresentation | null; diskThresholds?: MetricDisplayThresholds | null; workloadActionAgentTitle: string; parentMemoryTotal?: GuestDrawerProps['parentMemoryTotal']; @@ -143,7 +140,23 @@ export function GuestDrawerOverview(props: GuestDrawerOverviewProps) { // backup. Keep its activity separate from the last completed age. makeDetailRow('Last completed backup', protectionPresentation().label, { tone: protectionPresentation().tone, + layout: 'stacked', wrap: true, + valueContent: ( + <> + + {protectionPresentation().label} + + + + + + ), }), makeDetailRow( 'Backup activity', diff --git a/frontend-modern/src/components/Workloads/__tests__/GuestDrawer.backup-clock.test.tsx b/frontend-modern/src/components/Workloads/__tests__/GuestDrawer.backup-clock.test.tsx new file mode 100644 index 0000000000..afcee678b4 --- /dev/null +++ b/frontend-modern/src/components/Workloads/__tests__/GuestDrawer.backup-clock.test.tsx @@ -0,0 +1,187 @@ +import { cleanup, fireEvent, render, screen, within } from '@solidjs/testing-library'; +import { createSignal } from 'solid-js'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import type { WorkloadGuest } from '@/types/workloads'; +import { RELATIVE_TIME_TICK_MS } from '@/utils/relativeTimeClock'; +import { GuestDrawer } from '../GuestDrawer'; +import { useGuestDrawerState } from '../useGuestDrawerState'; + +const policy = vi.hoisted(() => ({ freshHours: 24, staleHours: 72 })); +const calls = vi.hoisted(() => ({ history: vi.fn() })); +vi.mock('@/stores/alertsActivation', () => ({ + useAlertsActivation: () => ({ + getBackupThresholds: () => policy, + getMetricThresholds: () => null, + }), +})); +vi.mock('@/api/charts', async () => { + const actual = await vi.importActual('@/api/charts'); + return { ...actual, ChartsAPI: { ...actual.ChartsAPI, getMetricsHistory: calls.history } }; +}); +vi.mock('@/api/resourceOperatorState', async () => { + const actual = await vi.importActual( + '@/api/resourceOperatorState', + ); + return { ...actual, getResourceOperatorState: vi.fn().mockResolvedValue(null) }; +}); +vi.mock('@/stores/license', () => ({ + isRangeLocked: () => false, + loadRuntimeCapabilities: vi.fn(), + maxHistoryDays: () => 90, +})); + +const start = Date.parse('2026-10-12T12:00:00Z'); +const hour = 3600_000; +const guest = (type: 'qemu' | 'lxc', lastBackup: number | string): WorkloadGuest => ({ + id: 'lab:pve-a:101', + name: 'backup-clock-fixture', + vmid: 101, + instance: 'lab', + node: 'pve-a', + type, + status: 'running', + cpu: 0.1, + cpus: 1, + memory: { total: 1024, used: 256, free: 768, usage: 25 }, + disk: { total: 1024, used: 256, free: 768, usage: 25 }, + networkIn: 0, + networkOut: 0, + diskRead: 0, + diskWrite: 0, + uptime: 3600, + template: false, + tags: [], + lastBackup, +}); +const details = () => within(screen.getByTestId('guest-technical-details')); +const backupRow = () => details().getByText('Last completed backup').closest('tr')!; +const completion = () => backupRow().querySelector('time'); + +beforeEach(() => { + vi.useFakeTimers(); + vi.setSystemTime(start); + policy.freshHours = 24; + policy.staleHours = 72; + calls.history.mockClear(); +}); +afterEach(() => { + cleanup(); + expect(vi.getTimerCount()).toBe(0); + vi.useRealTimers(); +}); + +describe('mounted completed-backup evidence', () => { + it('invalidates the saved age policy on time alone, independently of new copy or markup', () => { + function StateProbe() { + const state = useGuestDrawerState({ + guest: guest('qemu', start - 24 * hour + 5_000), + onClose: () => {}, + }); + return {state.backupPresentation().ageClass}; + } + render(() => ); + expect(screen.getByTestId('age-policy')).toHaveTextContent('text-green-600'); + vi.advanceTimersByTime(RELATIVE_TIME_TICK_MS); + expect(screen.getByTestId('age-policy')).toHaveTextContent('text-yellow-600'); + }); + it.each(['qemu', 'lxc'] as const)( + 'ages an unchanged %s backup across the default policy boundary', + (type) => { + const timestamp = start - 24 * hour + 5_000; + render(() => {}} />); + const drawer = screen.getByTestId('guest-technical-details'); + const row = backupRow(); + expect(within(row).getByText('23 hours ago').closest('td')).toHaveClass('text-emerald-700'); + const originalTime = completion(); + expect(originalTime).toHaveAttribute('datetime', new Date(timestamp).toISOString()); + expect(originalTime).toHaveTextContent('2026-10-11 12:00:05 UTC'); + vi.advanceTimersByTime(RELATIVE_TIME_TICK_MS); + expect(within(backupRow()).getByText('1d ago').closest('td')).toHaveClass('text-amber-700'); + expect(completion()).toHaveAttribute('datetime', new Date(timestamp).toISOString()); + expect(screen.getByTestId('guest-technical-details')).toBe(drawer); + expect(calls.history).not.toHaveBeenCalled(); + }, + ); + + it.each(['qemu', 'lxc'] as const)( + 'ages an unchanged %s backup under a one-hour policy', + (type) => { + policy.freshHours = 1; + policy.staleHours = 3; + render(() => {}} />); + expect(details().getByText('59 mins ago').closest('td')).toHaveClass('text-emerald-700'); + vi.advanceTimersByTime(RELATIVE_TIME_TICK_MS); + expect(details().getByText('1 hour ago').closest('td')).toHaveClass('text-amber-700'); + expect(calls.history).not.toHaveBeenCalled(); + }, + ); + + it('does not label a previous-calendar-day completion Today', () => { + vi.setSystemTime(Date.parse('2026-10-12T00:30:00Z')); + render(() => {}} />); + expect(details().getByText('1 hour ago')).toBeVisible(); + expect(completion()).toHaveTextContent('2026-10-11 23:30:00 UTC'); + expect(details().queryByText('Today')).not.toBeInTheDocument(); + }); + + it('keeps activity separate and ages while another drawer tab is open', () => { + const timestamp = start - 24 * hour + 5_000; + const [current, setCurrent] = createSignal({ + ...guest('qemu', timestamp), + backupInProgress: true, + }); + render(() => {}} />); + expect(details().getByText('Running · not completed yet')).toBeVisible(); + fireEvent.click(screen.getByRole('tab', { name: 'Manage' })); + vi.advanceTimersByTime(RELATIVE_TIME_TICK_MS); + setCurrent({ ...current(), backupInProgress: false }); + fireEvent.click(screen.getByRole('tab', { name: 'Overview' })); + expect(details().getByText('1d ago').closest('td')).toHaveClass('text-amber-700'); + expect(completion()).toHaveAttribute('datetime', new Date(timestamp).toISOString()); + expect(details().queryByText('Running · not completed yet')).not.toBeInTheDocument(); + setCurrent({ ...current(), lastBackup: start + RELATIVE_TIME_TICK_MS }); + expect(details().getByText('0s ago').closest('td')).toHaveClass('text-emerald-700'); + expect(completion()).toHaveAttribute('datetime', '2026-10-12T12:00:30.000Z'); + vi.advanceTimersByTime(60_000); + expect(details().getByText('1 min ago')).toBeVisible(); + expect(calls.history).not.toHaveBeenCalled(); + }); + + it('rejudges future and backwards-clock times without manufacturing completion', () => { + const timestamp = start + 45_000; + render(() => {}} />); + expect( + details() + .getByText(/timestamp is in the future/) + .closest('td'), + ).toHaveClass('text-amber-700'); + expect(completion()).toBeNull(); + vi.advanceTimersByTime(RELATIVE_TIME_TICK_MS); + expect(completion()).toBeNull(); + vi.advanceTimersByTime(RELATIVE_TIME_TICK_MS); + expect(details().getByText('15s ago').closest('td')).toHaveClass('text-emerald-700'); + expect(completion()).toHaveAttribute('datetime', new Date(timestamp).toISOString()); + vi.setSystemTime(start - hour); + vi.advanceTimersByTime(RELATIVE_TIME_TICK_MS); + expect( + details() + .getByText(/timestamp is in the future/) + .closest('td'), + ).toHaveClass('text-amber-700'); + expect(completion()).toBeNull(); + expect(details().queryByText('No completed backup found')).not.toBeInTheDocument(); + expect(calls.history).not.toHaveBeenCalled(); + }); + + it.each([0, NaN, 'invalid-time'])( + 'does not turn absent or invalid evidence %s into a dated completion', + (timestamp) => { + render(() => {}} />); + const initial = backupRow().textContent; + vi.advanceTimersByTime(60_000); + expect(backupRow()).toHaveTextContent(initial!); + expect(completion()).toBeNull(); + expect(calls.history).not.toHaveBeenCalled(); + }, + ); +}); diff --git a/frontend-modern/src/components/Workloads/__tests__/guestDrawerModel.branchcov2.test.ts b/frontend-modern/src/components/Workloads/__tests__/guestDrawerModel.branchcov2.test.ts index 2ffd5318e2..0e6cb0570a 100644 --- a/frontend-modern/src/components/Workloads/__tests__/guestDrawerModel.branchcov2.test.ts +++ b/frontend-modern/src/components/Workloads/__tests__/guestDrawerModel.branchcov2.test.ts @@ -686,21 +686,25 @@ describe('guestDrawerModel (branch coverage)', () => { const expectPresentation = (days: number) => { const lastBackup = backupNDaysAgo(days); const result = getGuestDrawerBackupPresentation(lastBackup, undefined, now); - expect(result.dateLabel).toBe(new Date(lastBackup).toLocaleDateString()); + expect(result.dateLabel).toBe( + new Date(lastBackup).toISOString().replace('T', ' ').replace('.000Z', ' UTC'), + ); + expect(result.completedAt).toBe(new Date(lastBackup).toISOString()); return result; }; - it('labels day 0 as Today and uses the green (fresh) class', () => { + it('labels a just-completed backup with elapsed time and the green (fresh) class', () => { expect(expectPresentation(0)).toStrictEqual({ ageClass: 'text-green-600 dark:text-green-400', - ageLabel: 'Today', - dateLabel: new Date(backupNDaysAgo(0)).toLocaleDateString(), + ageLabel: '0s ago', + dateLabel: now.toISOString().replace('T', ' ').replace('.000Z', ' UTC'), + completedAt: now.toISOString(), }); }); - it('labels day 1 as Yesterday', () => { + it('labels day 1 as elapsed days', () => { const result = expectPresentation(1); - expect(result.ageLabel).toBe('Yesterday'); + expect(result.ageLabel).toBe('1d ago'); expect(result.ageClass).toBe('text-green-600 dark:text-green-400'); }); @@ -733,7 +737,10 @@ describe('guestDrawerModel (branch coverage)', () => { const result = getGuestDrawerBackupPresentation(lastBackup, undefined, now); expect(result.ageLabel).toBe('10d ago'); expect(result.ageClass).toBe('text-yellow-600 dark:text-yellow-400'); - expect(result.dateLabel).toBe(new Date(lastBackup).toLocaleDateString()); + expect(result.dateLabel).toBe( + new Date(lastBackup).toISOString().replace('T', ' ').replace('.000Z', ' UTC'), + ); + expect(result.completedAt).toBe(new Date(lastBackup).toISOString()); }); }); }); diff --git a/frontend-modern/src/components/Workloads/guestDrawerModel.ts b/frontend-modern/src/components/Workloads/guestDrawerModel.ts index cb06222121..d9b26fdb02 100644 --- a/frontend-modern/src/components/Workloads/guestDrawerModel.ts +++ b/frontend-modern/src/components/Workloads/guestDrawerModel.ts @@ -83,6 +83,7 @@ export interface GuestDrawerBackupPresentation { ageClass: string; ageLabel: string; dateLabel: string; + completedAt?: string; } export const isGuestDrawerVM = (guest: Guest): guest is VM => resolveWorkloadType(guest) === 'vm'; @@ -632,7 +633,7 @@ export const normalizeGuestDrawerTags = (tags: Guest['tags']): string[] => { export const getGuestDrawerBackupPresentation = ( lastBackup: string | number | Date | null | undefined, thresholds?: BackupThresholds, - now: Date = new Date(), + now: number | Date = Date.now(), ): GuestDrawerBackupPresentation => { const timestamp = lastBackup instanceof Date ? lastBackup.getTime() : lastBackup; const info = getBackupInfo(timestamp, thresholds, now); @@ -644,12 +645,15 @@ export const getGuestDrawerBackupPresentation = ( dateLabel: 'Unknown', }; } - const backupDate = new Date(timestamp!); + const completedAt = new Date(timestamp!).toISOString(); const daysSince = Math.floor(info.ageMs / (1000 * 60 * 60 * 24)); return { ageClass: statusPresentation.color, - ageLabel: daysSince === 0 ? 'Today' : daysSince === 1 ? 'Yesterday' : `${daysSince}d ago`, - dateLabel: backupDate.toLocaleDateString(), + // Elapsed age, not a rolling-day calendar claim: a 23-hour-old backup + // need not have completed today. Keep older ages in days, like the table. + ageLabel: daysSince === 0 ? info.ageFormatted : `${daysSince}d ago`, + dateLabel: completedAt.replace('T', ' ').replace(/\.\d{3}Z$/, ' UTC'), + completedAt, }; }; diff --git a/frontend-modern/src/components/Workloads/useGuestDrawerState.ts b/frontend-modern/src/components/Workloads/useGuestDrawerState.ts index 8d0a49be66..e2f262ba83 100644 --- a/frontend-modern/src/components/Workloads/useGuestDrawerState.ts +++ b/frontend-modern/src/components/Workloads/useGuestDrawerState.ts @@ -2,6 +2,7 @@ import { createEffect, createMemo, createSignal } from 'solid-js'; import type { HistoryTimeRange } from '@/api/charts'; import { useAlertsActivation } from '@/stores/alertsActivation'; +import { useRelativeTimeNow } from '@/utils/relativeTimeClock'; import type { DisplayMetricType } from '@/utils/metricThresholds'; import { getCanonicalWorkloadId, @@ -36,6 +37,7 @@ import { export function useGuestDrawerState(props: GuestDrawerProps) { const alertsActivation = useAlertsActivation(); + const now = useRelativeTimeNow(); const [activeTab, setActiveTab] = createSignal('overview'); const [historyRange, setHistoryRange] = createSignal( GUEST_DRAWER_HISTORY_DEFAULT_RANGE, @@ -84,6 +86,7 @@ export function useGuestDrawerState(props: GuestDrawerProps) { getGuestDrawerBackupPresentation( props.guest.lastBackup, alertsActivation.getBackupThresholds(), + now(), ), ); const historyTarget = createMemo(() => getGuestDrawerHistoryTarget(props.guest)); From 40c8f64fe763bec50514a9ec198f4d4cdd1dae4a Mon Sep 17 00:00:00 2001 From: "pulse-triage[bot]" <249995291+pulse-triage[bot]@users.noreply.github.com> Date: Mon, 12 Oct 2026 18:00:00 +0100 Subject: [PATCH 2/6] Keep backup clock fixtures within the workload timestamp type Use parsed numeric completion timestamps and non-finite numeric adverse inputs for the typed WorkloadGuest fixture. Preserve the existing policy, clock, completion, activity and absence assertions; production source and its own-parent browser receipt are unchanged. Change-source: pulse-maintainer --- .../Workloads/__tests__/GuestDrawer.backup-clock.test.tsx | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/frontend-modern/src/components/Workloads/__tests__/GuestDrawer.backup-clock.test.tsx b/frontend-modern/src/components/Workloads/__tests__/GuestDrawer.backup-clock.test.tsx index afcee678b4..44e9fd79d7 100644 --- a/frontend-modern/src/components/Workloads/__tests__/GuestDrawer.backup-clock.test.tsx +++ b/frontend-modern/src/components/Workloads/__tests__/GuestDrawer.backup-clock.test.tsx @@ -32,7 +32,7 @@ vi.mock('@/stores/license', () => ({ const start = Date.parse('2026-10-12T12:00:00Z'); const hour = 3600_000; -const guest = (type: 'qemu' | 'lxc', lastBackup: number | string): WorkloadGuest => ({ +const guest = (type: 'qemu' | 'lxc', lastBackup: number): WorkloadGuest => ({ id: 'lab:pve-a:101', name: 'backup-clock-fixture', vmid: 101, @@ -118,7 +118,9 @@ describe('mounted completed-backup evidence', () => { it('does not label a previous-calendar-day completion Today', () => { vi.setSystemTime(Date.parse('2026-10-12T00:30:00Z')); - render(() => {}} />); + render(() => ( + {}} /> + )); expect(details().getByText('1 hour ago')).toBeVisible(); expect(completion()).toHaveTextContent('2026-10-11 23:30:00 UTC'); expect(details().queryByText('Today')).not.toBeInTheDocument(); @@ -173,7 +175,7 @@ describe('mounted completed-backup evidence', () => { expect(calls.history).not.toHaveBeenCalled(); }); - it.each([0, NaN, 'invalid-time'])( + it.each([0, NaN, Infinity])( 'does not turn absent or invalid evidence %s into a dated completion', (timestamp) => { render(() => {}} />); From 59ff0dabcf84d354cf8a8367626e6f1eec6395d8 Mon Sep 17 00:00:00 2001 From: "pulse-triage[bot]" <249995291+pulse-triage[bot]@users.noreply.github.com> Date: Mon, 12 Oct 2026 18:02:24 +0100 Subject: [PATCH 3/6] Retain SCALE agent recovery until every boot task is retired Disable and independently verify every exact owned middleware activation task before systemd stop and server removal. Close task, unit and process state after the full stop and after task deletion, retaining recovery on refusals or concurrent changes without restarting. Ordinary Linux state directories are not appliance identity. Register all fifteen actual-function/full-caller controls and update both owning contracts in this coherent candidate. Change-source: pulse-maintainer --- .../v6/internal/subsystems/agent-lifecycle.md | 42 ++ .../subsystems/deployment-installability.md | 42 ++ scripts/install.sh | 159 +++++++- scripts/installtests/agent_legacy_teardown.py | 2 +- scripts/installtests/agent_openrc_teardown.py | 2 +- .../installtests/agent_truenas_teardown.py | 377 ++++++++++++++++++ scripts/installtests/agent_uninstall_state.py | 2 +- .../installtests/agent_upstart_teardown.py | 2 +- scripts/installtests/install_sh_test.go | 38 ++ 9 files changed, 653 insertions(+), 13 deletions(-) create mode 100644 scripts/installtests/agent_truenas_teardown.py diff --git a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md index 7193c99f46..d71de6fb80 100644 --- a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md +++ b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md @@ -15,6 +15,48 @@ ## Purpose +### TrueNAS SCALE removal retires every owned middleware boot task before recovery erasure + +TrueNAS SCALE has an independent activation owner: its Init/Shutdown bootstrap +can recreate and enable the systemd unit after a normal disable. A recovered +state directory or an unrelated `/etc/version` alone is not appliance identity. +The uninstall caller selects SCALE only from a bootstrap footprint or vendor +marker. The existing protected/explicit state-removal authority must select the +same state directory and exact bootstrap path; linked, writable or non-regular +bootstrap files refuse before task/service mutation. + +One complete, bounded middleware inventory must identify every exact generated +SCRIPT/POSTINIT task, including duplicates. Unknown ownership, indirect command +activation or another-install bootstrap refuses without deleting unrelated +tasks. The JSON reader rejects duplicate keys/IDs, invalid types, failed/empty/ +diagnostic/incomplete reads, over 512 tasks or over 1 MiB; it returns only owned +numeric IDs and enablement, never task commands, names or credentials. All owned +tasks are disabled with checked calls and independent exact-set readback before +systemd stop. A partial stop/disable stays stopped/disabled: do not re-enable or +automatically restart after a refusal. + +The complete systemd target set and independent kernel collector/bootstrap +process inventory and a final exact disabled-task readback must be quiescent +before same-instance server removal, including task changes during systemd stop. +After remote confirmation, the exact disabled task set is re-read before the first +task deletion. Every owned deletion is checked; final whole-unit/process +quiescence and complete task absence precede credential, stored binary, +bootstrap and state erasure. A changed set, refusal, ignored deletion, new task +or reactivation retains local recovery, even if earlier task/server operations +succeeded. No favourable replacement observation, task recreation or credential +churn is performed. Unrelated tasks survive unchanged. + +`TestInstallSHTrueNASScaleBootTasksRetireBeforeRecoveryErasure` requires all +fifteen `agent_truenas_teardown.py` actual-function/full-caller controls, +including multiple tasks, partial/refused/unknown operations, post-remote +changes, ignored/recreated deletion, process privacy and a real timeout child. +These run against closed middleware/systemd/process models and private ordinary- +user files, not a native appliance. Existing systemd/OpenRC/DSM/rc.d/SysV gates, +transport trust and identity remain. TrueNAS CORE retains its linked rc.d/ +appliance refusal; Unraid/QNAP boot ownership, Windows and signed installed +Linux/PowerShell/native/TLS/cleanup acceptance remain separate. This does not +expand the original removal job into macOS or introduce another dispatcher. + ### DSM Upstart removal retains recovery until automatic activation and the job are absent DSM 6 Upstart jobs (including leftovers on DSM 7) are not classified as SysV. diff --git a/docs/release-control/v6/internal/subsystems/deployment-installability.md b/docs/release-control/v6/internal/subsystems/deployment-installability.md index e71c8a5d55..b6d4667eec 100644 --- a/docs/release-control/v6/internal/subsystems/deployment-installability.md +++ b/docs/release-control/v6/internal/subsystems/deployment-installability.md @@ -16,6 +16,48 @@ ## Purpose +### TrueNAS SCALE removal retires every owned middleware boot task before recovery erasure + +TrueNAS SCALE has an independent activation owner: its Init/Shutdown bootstrap +can recreate and enable the systemd unit after a normal disable. A recovered +state directory or an unrelated `/etc/version` alone is not appliance identity. +The uninstall caller selects SCALE only from a bootstrap footprint or vendor +marker. The existing protected/explicit state-removal authority must select the +same state directory and exact bootstrap path; linked, writable or non-regular +bootstrap files refuse before task/service mutation. + +One complete, bounded middleware inventory must identify every exact generated +SCRIPT/POSTINIT task, including duplicates. Unknown ownership, indirect command +activation or another-install bootstrap refuses without deleting unrelated +tasks. The JSON reader rejects duplicate keys/IDs, invalid types, failed/empty/ +diagnostic/incomplete reads, over 512 tasks or over 1 MiB; it returns only owned +numeric IDs and enablement, never task commands, names or credentials. All owned +tasks are disabled with checked calls and independent exact-set readback before +systemd stop. A partial stop/disable stays stopped/disabled: do not re-enable or +automatically restart after a refusal. + +The complete systemd target set and independent kernel collector/bootstrap +process inventory and a final exact disabled-task readback must be quiescent +before same-instance server removal, including task changes during systemd stop. +After remote confirmation, the exact disabled task set is re-read before the first +task deletion. Every owned deletion is checked; final whole-unit/process +quiescence and complete task absence precede credential, stored binary, +bootstrap and state erasure. A changed set, refusal, ignored deletion, new task +or reactivation retains local recovery, even if earlier task/server operations +succeeded. No favourable replacement observation, task recreation or credential +churn is performed. Unrelated tasks survive unchanged. + +`TestInstallSHTrueNASScaleBootTasksRetireBeforeRecoveryErasure` requires all +fifteen `agent_truenas_teardown.py` actual-function/full-caller controls, +including multiple tasks, partial/refused/unknown operations, post-remote +changes, ignored/recreated deletion, process privacy and a real timeout child. +These run against closed middleware/systemd/process models and private ordinary- +user files, not a native appliance. Existing systemd/OpenRC/DSM/rc.d/SysV gates, +transport trust and identity remain. TrueNAS CORE retains its linked rc.d/ +appliance refusal; Unraid/QNAP boot ownership, Windows and signed installed +Linux/PowerShell/native/TLS/cleanup acceptance remain separate. This does not +expand the original removal job into macOS or introduce another dispatcher. + ### DSM Upstart removal retains recovery until automatic activation and the job are absent DSM 6 Upstart jobs (including leftovers on DSM 7) are not classified as SysV. diff --git a/scripts/install.sh b/scripts/install.sh index 543578ece3..82134ddf26 100755 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -1344,6 +1344,7 @@ teardown_openrc_agent_service() { # cannot establish that after an ignored signal or lost PID file. read_legacy_agent_removal_processes() { local observation="" line="" pid="" parent="" comm="" args="" role="" + local bootstrap="${1:-}" local complete="false" count=0 parents=" " seen=" " local -a rows=() observation=$(LC_ALL=C timeout -k 1 5 ps -ax -o pid= -o ppid= -o comm= -o args= 2>&1 && @@ -1365,6 +1366,7 @@ read_legacy_agent_removal_processes() { elif [[ "${comm##*/}" == daemon && "$args" =~ (^|[[:space:]/])pulse-agent([[:space:]\[]|$) ]]; then role="supervisor" fi + if [[ -n "$bootstrap" && "$args" == *"$bootstrap"* ]]; then role="bootstrap"; fi rows+=("$pid $parent $role ${comm##*/}") count=$((count + 1)) [[ $count -le 65536 ]] || return 1 @@ -1706,6 +1708,130 @@ enable_sysv_agent_service() { log_info "Created rc.d symlinks manually." } +# TrueNAS SCALE has a second activation owner: middleware's boot task can +# recreate and enable the systemd unit even after systemctl disable. Recovered +# state directories alone are not appliance identity (ordinary Linux has them). +truenas_scale_agent_removal_present() { + [[ "$(uname -s)" == Linux ]] || return 1 + [[ -e "${TRUENAS_BOOTSTRAP_SCRIPT:-}" || -L "${TRUENAS_BOOTSTRAP_SCRIPT:-}" || + -f /etc/truenas-version || -d /data/ix-applications || -d /etc/ix-apps.d ]] && return 0 + [[ -f /etc/version ]] && grep -qi truenas /etc/version +} + +read_truenas_agent_removal_tasks() { + local observation="" + observation=$(LC_ALL=C timeout -k 1 5 midclt call initshutdownscript.query 2>&1 && + printf '\n%s' '__pulse_truenas_tasks_complete__') || return 1 + # Parse the complete inventory, including indirect/other-install bootstrap + # references. No task command, name, argument or credential is returned. + python3 -c ' +import json, pathlib, sys +def unique_object(pairs): + result = {} + for key, value in pairs: + if key in result: raise ValueError("duplicate field") + result[key] = value + return result +def reject_constant(value): + raise ValueError("non-JSON constant") +try: + raw = sys.stdin.buffer.read(1048577) + suffix = b"\n__pulse_truenas_tasks_complete__\n" + if len(raw) > 1048576 or not raw.endswith(suffix): raise ValueError("incomplete inventory") + tasks = json.loads(raw[:-len(suffix)].decode("utf-8"), object_pairs_hook=unique_object, + parse_constant=reject_constant) + if not isinstance(tasks, list) or len(tasks) > 512: raise ValueError("unbounded inventory") + expected, seen, owned = sys.argv[1], set(), [] + for task in tasks: + if not isinstance(task, dict): raise ValueError("invalid task") + ident, kind, enabled = task.get("id"), task.get("type"), task.get("enabled") + if type(ident) is not int or not 0 < ident <= 2147483647 or ident in seen: + raise ValueError("invalid identity") + seen.add(ident) + if kind not in ("SCRIPT", "COMMAND") or type(enabled) is not bool: + raise ValueError("unknown activation") + script, command = task.get("script", ""), task.get("command", "") + if script is None: script = "" + if command is None: command = "" + if not isinstance(script, str) or not isinstance(command, str): raise ValueError("invalid activation") + if script == expected: + if (kind != "SCRIPT" or command or task.get("when") != "POSTINIT" or + task.get("comment") != "Pulse Agent Bootstrap" or type(task.get("timeout")) is not int or + task["timeout"] != 30): raise ValueError("unowned bootstrap") + owned.append((ident, enabled)) + elif (pathlib.PurePosixPath(script).name == "bootstrap-pulse-agent.sh" or + "bootstrap-pulse-agent.sh" in command or "pulse-agent" in command): + raise ValueError("indirect or other-install activation") + for ident, enabled in sorted(owned): print(ident, "true" if enabled else "false") + print("complete") +except (ValueError, TypeError, KeyError, UnicodeError): + print("TrueNAS boot-task inventory is incomplete or not owned; recovery retained.", file=sys.stderr) + sys.exit(1) +' "$TRUENAS_BOOTSTRAP_SCRIPT" <<< "$observation" +} + +quiesce_truenas_agent_boot_tasks_for_removal() { + local inventory="" expected="" observed="" ident="" enabled="" mode="" + command -v midclt >/dev/null 2>&1 && command -v python3 >/dev/null 2>&1 && + command -v timeout >/dev/null 2>&1 && command -v systemctl >/dev/null 2>&1 || return 1 + # Do not let a platform marker or an arbitrary saved state path authorise + # erasure of a different appliance install. Keep the existing state owner. + [[ "$TRUENAS_STATE_DIR" == "$STATE_DIR" && + "${STATE_DIR_REMOVAL_AUTHORITY:-}" == "$STATE_DIR" && + "$TRUENAS_BOOTSTRAP_SCRIPT" == "$STATE_DIR/bootstrap-pulse-agent.sh" ]] || return 1 + if [[ -e "$TRUENAS_BOOTSTRAP_SCRIPT" || -L "$TRUENAS_BOOTSTRAP_SCRIPT" ]]; then + mode=$(portable_path_mode "$TRUENAS_BOOTSTRAP_SCRIPT") || return 1 + [[ "$mode" =~ ^[0-7]{3,4}$ ]] && (( (8#$mode & 0022) == 0 )) || return 1 + trusted_lifecycle_regular_file "$TRUENAS_BOOTSTRAP_SCRIPT" "$mode" || return 1 + fi + inventory=$(read_truenas_agent_removal_tasks) || return 1 + while read -r ident enabled; do + [[ "$ident" != complete ]] || continue + if [[ "$enabled" == true ]]; then + LC_ALL=C timeout -k 1 5 midclt call initshutdownscript.update "$ident" '{"enabled":false}' >/dev/null 2>&1 || return 1 + fi + expected+="$ident false"$'\n' + done <<< "$inventory" + expected+=complete + observed=$(read_truenas_agent_removal_tasks) || return 1 + [[ "$observed" == "$expected" ]] || return 1 + printf '%s\n' "$expected" +} + +confirm_truenas_agent_removal_quiescent() { + local unit="" inventory="" observed="" + for unit in "${ACTION_RUNNER_NAME}.service" "${PRIVILEGED_HELPER_NAME}.socket" \ + "${PRIVILEGED_HELPER_NAME}.service" "${AGENT_NAME}.service"; do + confirm_agent_unit_quiescent "$unit" || return 1 + done + inventory=$(read_legacy_agent_removal_processes "$TRUENAS_BOOTSTRAP_SCRIPT") || return 1 + legacy_agent_processes_absent "$inventory" && [[ "$inventory" != *' bootstrap'* ]] || return 1 + # The middleware set can change during the preceding systemd operations. + # Close it again before remote removal/first deletion, not only afterwards. + if [[ $# -gt 0 ]]; then + observed=$(read_truenas_agent_removal_tasks) || return 1 + [[ "$observed" == "$1" ]] || return 1 + fi +} + +teardown_truenas_agent_boot_tasks() { + local expected="$1" observed="" ident="" enabled="" + # No newly discovered task is disabled/deleted after remote removal. A + # changed set, refused read or concurrent activation retains local recovery. + observed=$(read_truenas_agent_removal_tasks) || return 1 + [[ "$observed" == "$expected" ]] || return 1 + confirm_truenas_agent_removal_quiescent "$expected" || return 1 + while read -r ident enabled; do + [[ "$ident" != complete ]] || continue + LC_ALL=C timeout -k 1 5 midclt call initshutdownscript.delete "$ident" >/dev/null 2>&1 || return 1 + done <<< "$expected" + # Close the whole manager/process/task set after the last deletion, while + # every credential, bootstrap, stored binary and service file still exists. + confirm_truenas_agent_removal_quiescent || return 1 + observed=$(read_truenas_agent_removal_tasks) || return 1 + [[ "$observed" == complete ]] +} + write_truenas_bootstrap_script() { local platform="$1" local service_link="" @@ -5999,6 +6125,8 @@ if [[ "$UNINSTALL" == "true" ]]; then local uninstall_has_credential="false" local uninstall_legacy_manager="" local uninstall_upstart="false" + local uninstall_truenas_scale="false" + local uninstall_truenas_tasks="" # Validate saved connection availability before stopping a working install. # Stop every systemd activation source before remote removal, process kills @@ -6009,6 +6137,14 @@ if [[ "$UNINSTALL" == "true" ]]; then fail "The saved Pulse URL is missing; local credentials and services were retained. Restore saved connection state or provide --url for the same trusted Pulse instance and retry uninstall." "$EXIT_MISSING_ARGS" fi fi + # Middleware can recreate and enable the systemd unit at boot. Disable all + # exact owned tasks before stopping services; a partial/refused observation + # retains every recovery file and never restores automatic activation. + if truenas_scale_agent_removal_present; then + uninstall_truenas_scale="true" + uninstall_truenas_tasks=$(quiesce_truenas_agent_boot_tasks_for_removal) || + fail "TrueNAS SCALE removal could not confirm every owned boot task disabled; credentials, bootstrap and service files were retained. Reconcile middleware task ownership/state before retrying; no automatic restart was attempted." "$EXIT_GENERAL" + fi # Detect stale DSM 6 jobs even on a DSM 7 host. Another init manager cannot # establish their absence; missing initctl is an explicit safe refusal. if [[ -d /usr/syno ]] && { command -v initctl >/dev/null 2>&1 || @@ -6037,6 +6173,10 @@ if [[ "$UNINSTALL" == "true" ]]; then quiesce_sysv_agent_for_removal || fail "SysV removal could not confirm stopped process and disabled runlevels; local recovery files were retained. A preceding stop or disable may have succeeded; no automatic restart was attempted." "$EXIT_GENERAL" fi + if [[ "$uninstall_truenas_scale" == true ]]; then + confirm_truenas_agent_removal_quiescent "$uninstall_truenas_tasks" || + fail "TrueNAS SCALE removal could not confirm stopped units, collector and bootstrap processes; local recovery was retained. Reconcile stopped/disabled state before retrying; no automatic restart was attempted." "$EXIT_GENERAL" + fi # A credential-bearing install must durably remove its exact server record # through the same CA/fingerprint/no-proxy transport used by lifecycle @@ -6068,6 +6208,10 @@ if [[ "$UNINSTALL" == "true" ]]; then teardown_upstart_agent_service || fail "DSM Upstart job retirement was not confirmed; local recovery state was retained. The manual override remains after a partial stop. Reconcile the original config or its .removal recovery copy before retrying; no automatic restart was attempted." "$EXIT_GENERAL" fi + if [[ "$uninstall_truenas_scale" == true ]]; then + teardown_truenas_agent_boot_tasks "$uninstall_truenas_tasks" || + fail "TrueNAS SCALE boot-task retirement was not confirmed; local recovery was retained. A preceding task disable/delete or server removal may have succeeded. Reconcile middleware and stopped service state before retrying; no automatic restart was attempted." "$EXIT_GENERAL" + fi # Kill wrapper scripts first: they are watchdogs, so stopping the agent # while its wrapper still loops only races the respawn. No leading path @@ -6181,15 +6325,12 @@ if [[ "$UNINSTALL" == "true" ]]; then fi fi - # TrueNAS SCALE/CORE - if [[ -d "$TRUENAS_STATE_DIR" ]] || [[ -f /etc/truenas-version ]] || [[ -f /etc/version ]]; then - if [[ "$(uname -s)" == "Linux" ]]; then - log_info "Removing TrueNAS SCALE installation..." - teardown_systemd_agent_service - elif [[ "$(uname -s)" == "FreeBSD" ]]; then - log_info "Removing TrueNAS CORE installation..." - # rc.d teardown was confirmed before remote/local recovery erasure. - fi + # SCALE's task retirement was confirmed above, before authorised state + # erasure. A recovered ordinary Linux directory is not a TrueNAS install. + # CORE remains behind its existing rc.d/appliance activation refusal; this + # SCALE step does not claim CORE boot-task or native platform acceptance. + if [[ "$(uname -s)" == "FreeBSD" ]] && { [[ -d "$TRUENAS_STATE_DIR" ]] || [[ -f /etc/truenas-version ]] || [[ -f /etc/version ]]; }; then + log_info "Removing TrueNAS CORE installation..." # Remove Init/Shutdown task if command -v midclt >/dev/null 2>&1 && command -v python3 >/dev/null 2>&1; then TASK_ID=$(midclt call initshutdownscript.query '[["script","=","'"$TRUENAS_BOOTSTRAP_SCRIPT"'"]]' 2>/dev/null | python3 -c "import json,sys; d=json.load(sys.stdin); print(d[0]['id'] if d else '')" 2>/dev/null || echo "") diff --git a/scripts/installtests/agent_legacy_teardown.py b/scripts/installtests/agent_legacy_teardown.py index 54afa1b99c..51dc2a0587 100644 --- a/scripts/installtests/agent_legacy_teardown.py +++ b/scripts/installtests/agent_legacy_teardown.py @@ -17,7 +17,7 @@ import unittest ROOT = Path(__file__).resolve().parents[2] SOURCE = Path(os.environ.get("PULSE_AGENT_LEGACY_SOURCE", ROOT / "scripts/install.sh")) -FUNCTIONS = ("read_legacy_agent_removal_processes", "legacy_agent_processes_absent", +FUNCTIONS = ("truenas_scale_agent_removal_present", "read_legacy_agent_removal_processes", "legacy_agent_processes_absent", "validate_legacy_agent_removal_pidfiles", "read_legacy_agent_removal_status", "read_freebsd_agent_removal_enablement", "quiesce_freebsd_agent_for_removal", "teardown_freebsd_agent_service", "read_sysv_agent_removal_links", diff --git a/scripts/installtests/agent_openrc_teardown.py b/scripts/installtests/agent_openrc_teardown.py index a2cc349ea5..fbc2102ebd 100644 --- a/scripts/installtests/agent_openrc_teardown.py +++ b/scripts/installtests/agent_openrc_teardown.py @@ -21,7 +21,7 @@ import unittest ROOT = Path(__file__).resolve().parents[2] SOURCE = Path(os.environ.get("PULSE_AGENT_OPENRC_SOURCE", ROOT / "scripts/install.sh")) -FUNCTIONS = ("read_openrc_agent_removal_status", "read_openrc_agent_removal_runlevels", +FUNCTIONS = ("truenas_scale_agent_removal_present", "read_openrc_agent_removal_status", "read_openrc_agent_removal_runlevels", "quiesce_openrc_agent_for_removal", "teardown_openrc_agent_service") diff --git a/scripts/installtests/agent_truenas_teardown.py b/scripts/installtests/agent_truenas_teardown.py new file mode 100644 index 0000000000..b5542df5b8 --- /dev/null +++ b/scripts/installtests/agent_truenas_teardown.py @@ -0,0 +1,377 @@ +#!/usr/bin/env python3 +"""TrueNAS SCALE uninstall, using production functions and private manager models. + +Only fixed vendor/service paths are redirected into owned temporary files. +No appliance, root installer, network, credential or native acceptance is used. +The complete uninstall caller runs; middleware/process/systemd are closed models. +""" +import json +import os +from pathlib import Path +import shlex +import stat +import subprocess +import tempfile +import unittest + +ROOT = Path(__file__).resolve().parents[2] +SOURCE = Path(os.environ.get("PULSE_AGENT_TRUENAS_SOURCE", ROOT / "scripts/install.sh")) +FUNCTIONS = ( + "portable_path_uid", "portable_path_mode", "trusted_lifecycle_regular_file", + "read_agent_removal_unit_state", "confirm_agent_unit_quiescent", + "quiesce_agent_units_for_removal", "teardown_systemd_agent_service", + "read_legacy_agent_removal_processes", "legacy_agent_processes_absent", + "truenas_scale_agent_removal_present", "read_truenas_agent_removal_tasks", + "quiesce_truenas_agent_boot_tasks_for_removal", "confirm_truenas_agent_removal_quiescent", + "teardown_truenas_agent_boot_tasks", +) + + +def definitions(source): + selected = [] + for name in FUNCTIONS: + marker = name + "() {" + if marker not in source: # The exact-parent adverse comparison. + continue + if source.count(marker) != 1: + raise ValueError("ambiguous production function " + name) + selected.append(marker + source.split(marker, 1)[1].split("\n}\n", 1)[0] + "\n}\n") + return "\n".join(selected) + + +MODEL = r'''#!/usr/bin/python3 +import json, os, pathlib, sys +root = pathlib.Path(os.environ['FIXTURE']) +path = root / 'model.json' +v = json.loads(path.read_text()) +role, args = pathlib.Path(sys.argv[0]).name, sys.argv[1:] +fault = v['fault'] +def save(): + pending = root / 'model.pending' + pending.write_text(json.dumps(v)); pending.replace(path) +def event(s): + with (root / 'events').open('a') as f: f.write(s + '\n') +remote = (root / 'events').exists() and 'remote:remove' in (root / 'events').read_text() +if role == 'ps': + assert args == ['-ax', '-o', 'pid=', '-o', 'ppid=', '-o', 'comm=', '-o', 'args='], args + event('process:read') + if fault == 'failed-process': sys.exit(1) + if fault == 'empty-process': sys.exit(0) + print('1 0 init /sbin/init') + if fault == 'duplicate-process': print('1 0 init /sbin/init') + if v['active'] == 'active' or fault == 'orphan-agent': + print('100 1 pulse-agent /usr/local/bin/pulse-agent --token-file /synthetic/recovery-only') + if fault == 'live-bootstrap': print('101 1 bash /bin/bash ' + str(root / 'state/bootstrap-pulse-agent.sh')) + if fault == 'diagnostic-process': print('read unavailable', file=sys.stderr) + sys.exit(0) +if role == 'systemctl': + verb = args[0] + if verb == 'daemon-reload': event('unit:reload'); sys.exit(0) + assert len(args) >= 2, args + unit = args[1] + assert unit in ('pulse-agent.service', 'pulse-agent-helper.service', 'pulse-agent-helper.socket', 'pulse-agent-runner.service'), args + if verb == 'show': + assert args[2:] == ['--property=LoadState,ActiveState,UnitFileState'], args + event('unit:read:' + unit) + if fault == 'failed-unit-read': sys.exit(1) + present = unit == 'pulse-agent.service' and (root / 'unit/pulse-agent.service').exists() + if present: + print('LoadState=loaded\nActiveState=' + v['active'] + '\nUnitFileState=' + v['enabled']) + else: print('LoadState=not-found\nActiveState=inactive\nUnitFileState=') + sys.exit(0) + assert verb in ('stop', 'disable'), args + event('unit:' + verb) + if fault == 'refused-stop' and verb == 'stop': sys.exit(1) + if verb == 'stop' and fault != 'ignored-stop': v['active'] = 'inactive' + if verb == 'stop' and fault == 'reactivated-task-on-stop': v['tasks'][0]['enabled'] = True + if verb == 'disable': v['enabled'] = 'disabled' + save(); sys.exit(0) +assert role == 'midclt' and args[0] == 'call', (role, args) +verb = args[1] +if verb == 'initshutdownscript.query': + assert len(args) in (2, 3), args + event('task:query'); v['queries'] += 1 + if fault == 'timeout-query': + save(); (root / 'timed-pid').write_text(str(os.getpid())) + os.execl('/bin/sleep', 'sleep', '30') + if fault == 'failed-query' or (fault == 'failed-after-remote-query' and remote): save(); sys.exit(1) + if fault == 'new-task-after-remote' and remote and not any(t['id'] == 3 for t in v['tasks']): + v['tasks'].append(dict(v['tasks'][0], id=3, enabled=True)) + if fault == 'reactivated-task' and v['updates'] and v['tasks']: v['tasks'][0]['enabled'] = True + if fault == 'recreated-task' and v['deletes'] and not any(t['script'] == str(root / 'state/bootstrap-pulse-agent.sh') for t in v['tasks']): + v['tasks'].append(dict(id=3, type='SCRIPT', script=str(root / 'state/bootstrap-pulse-agent.sh'), command='', when='POSTINIT', enabled=True, timeout=30, comment='Pulse Agent Bootstrap')) + save() + tasks = v['tasks'] + if len(args) == 3: # Preserve the parent's exact filter, not a new graph. + filters = json.loads(args[2]); assert filters[0][:2] == ['script', '='], filters + tasks = [t for t in tasks if t['script'] == filters[0][2]] + if fault == 'empty-query': sys.exit(0) + if fault == 'malformed-query': print('not-json'); sys.exit(0) + if fault == 'object-query': print('{}'); sys.exit(0) + if fault == 'duplicate-field': print('[{"id":1,"id":2}]'); sys.exit(0) + if fault == 'oversized-query': print(' ' * 1048577 + '[]'); sys.exit(0) + if fault == 'oversized-inventory': tasks = [dict(v['tasks'][0], id=i + 1) for i in range(513)] + if fault == 'duplicate-id': tasks = tasks + [tasks[0]] + if fault == 'bool-id': tasks[0]['id'] = True + if fault == 'string-enabled': tasks[0]['enabled'] = 'false' + if fault == 'foreign-owner': tasks[0]['comment'] = 'not owned by Pulse' + if fault == 'wrong-kind': tasks[0]['type'] = 'UNKNOWN' + if fault == 'command-bootstrap': tasks[0].update(type='COMMAND', script='', command='/bin/bash ' + str(root / 'state/bootstrap-pulse-agent.sh')) + if fault == 'other-bootstrap': tasks[0]['script'] = '/other-install/bootstrap-pulse-agent.sh' + if fault == 'diagnostic-query': print('read unavailable', file=sys.stderr) + print(json.dumps(tasks)); sys.exit(0) +if verb == 'initshutdownscript.update': + assert len(args) == 4 and json.loads(args[3]) == {'enabled': False}, args + ident = int(args[2]); event('task:disable:' + str(ident)); v['updates'] += 1 + if fault == 'refused-disable' or (fault == 'partial-disable' and ident == 2): save(); sys.exit(1) + task = next(t for t in v['tasks'] if t['id'] == ident) + if fault != 'ignored-disable': task['enabled'] = False + save(); print(json.dumps(task)); sys.exit(0) +assert verb == 'initshutdownscript.delete' and len(args) == 3, args +ident = int(args[2]); event('task:delete:' + str(ident)); v['deletes'] += 1 +if fault == 'refused-delete' or (fault == 'partial-delete' and ident == 2): save(); sys.exit(1) +if fault != 'ignored-delete': v['tasks'] = [t for t in v['tasks'] if t['id'] != ident] +if fault == 'unit-reactivation-on-delete': v['active'] = 'active' +save(); print('true') +''' + + +class AgentTrueNASTeardown(unittest.TestCase): + source = SOURCE.read_text() + + def exercise(self, fault="", reply="confirmed", bootstrap="regular", tasks="duplicate", + appliance=True, repeat=False, flow="caller"): + with tempfile.TemporaryDirectory() as temporary: + root = Path(temporary) + tools = root / "tools"; tools.mkdir() + for name in ("midclt", "ps", "systemctl"): + file = tools / name; file.write_text(MODEL); file.chmod(0o700) + if fault == "missing-midclt": (tools / "midclt").unlink() + for name in ("state", "helper", "profile", "unit"): + directory = root / name; directory.mkdir(mode=0o700) + if name != "unit": + for file in ("runtime.token", "agent-id", "connection.env", "install.sh"): + (directory / file).write_text("synthetic recovery only\n") + script = root / "state/bootstrap-pulse-agent.sh" + unrelated = root / "unrelated"; unrelated.write_text("do not alter\n") + if bootstrap == "linked": script.symlink_to(unrelated) + elif bootstrap == "directory": script.mkdir() + elif bootstrap != "missing": + script.write_text("#!/bin/bash\n# Pulse Agent Bootstrap for TrueNAS\n") + script.chmod(0o777 if bootstrap == "writable" else 0o755) + vendor = root / "truenas-version" + if appliance: vendor.write_text("TrueNAS-SCALE\n") + (root / "version").write_text("unrelated platform\n") + (root / "unit/pulse-agent.service").write_text("owned service\n") + owned = dict(id=1, type="SCRIPT", script=str(script), command="", when="POSTINIT", + enabled=True, timeout=30, comment="Pulse Agent Bootstrap") + foreign = dict(owned, id=99, script="/unrelated/boot.sh", comment="unrelated task") + selected = [] if tasks == "absent" else [owned] + if tasks == "duplicate": selected.append(dict(owned, id=2)) + if tasks == "disabled": selected[0]["enabled"] = False + selected.append(foreign) + model = dict(fault=fault, tasks=selected, updates=0, deletes=0, queries=0, + active="active", enabled="enabled") + (root / "model.json").write_text(json.dumps(model)) + before = self.snapshot(root) + + def redirect(source): + for path, target in ( + ("/etc/truenas-version", vendor), ("/etc/version", root / "version"), + ("/data/ix-applications", root / "no-ix"), ("/etc/ix-apps.d", root / "no-apps"), + ("/etc/ix.rc.d", root / "no-rc"), ("/usr/syno", root / "no-dsm"), + ("/etc/unraid-version", root / "no-unraid"), + ("/boot/config/plugins/pulse-agent", root / "no-plugin"), + ("/etc/config/qpkg.conf", root / "no-qnap"), + ("/sbin/getcfg", root / "no-getcfg"), + ("/var/log/pulse-agent.log", root / "absent-log"), + ("/etc/systemd/system/${AGENT_NAME}.service", root / "unit/pulse-agent.service"), + ): source = source.replace(path, str(target)) + return source + + values = dict(FIXTURE=root, EVENTS=root / "events", STATE_DIR=root / "state", + INSTALL_DIR=root / "bin", + STATE_DIR_REMOVAL_AUTHORITY=root / "state", + TRUENAS_STATE_DIR=root / "state", TRUENAS_BOOTSTRAP_SCRIPT=script, + PRIVILEGED_HELPER_STATE_DIR=root / "helper", SAFE_PROFILE_STATE_DIR=root / "profile", + PRIVILEGE_SUDOERS_FILE=root / "absent-sudoers", PRIVILEGE_HELPER_DIR=root / "absent-helper", + REPLY=reply, FAULT=fault) + setup = "\n".join(k + "=" + shlex.quote(str(v)) for k, v in values.items()) + shell = r''' +set -euo pipefail +UNINSTALL=true AGENT_NAME=pulse-agent BINARY_NAME=pulse-agent INSTALL_DIR=/usr/local/bin +EXIT_GENERAL=1 EXIT_MISSING_ARGS=16 PULSE_URL=https://same-instance.example.invalid AGENT_ID=synthetic +ACTION_RUNNER_NAME=pulse-agent-runner PRIVILEGED_HELPER_NAME=pulse-agent-helper +log_info() { :; }; log_warn() { :; } +fail() { printf '%s\n' "$1" >&2; exit "$2"; } +uname() { printf 'Linux\n'; } +command() { + case "$*" in + '-v rc-service') return 1 ;; + '-v midclt') [[ "$FAULT" != missing-midclt ]] || return 1 ;; + '-v python3') [[ "$FAULT" != missing-python ]] || return 1 ;; + '-v timeout') [[ "$FAULT" != missing-timeout ]] || return 1 ;; + esac + builtin command "$@" +} +collector_credential_state_present() { [[ -e "$STATE_DIR/runtime.token" ]]; } +uninstall_collector_registration() { printf 'remote:remove\n' >> "$EVENTS"; [[ "$REPLY" == confirmed ]]; } +teardown_action_runner_service() { :; } +teardown_privileged_helper_service() { :; } +find_qnap_state_dir() { :; } +pkill() { printf 'unchecked:process-stop\n' >> "$EVENTS"; } +sleep() { :; } +rm() { + local arg + for arg in "$@"; do [[ "$arg" == -* || "$arg" == "$FIXTURE/"* ]] || return 97; done + printf 'file:remove\n' >> "$EVENTS" + builtin command rm "$@" +} +remove_agent_state_dir() { printf 'state:remove\n' >> "$EVENTS"; rm -rf -- "$STATE_DIR"; } +remove_privileged_helper_state_dir() { rm -rf -- "$PRIVILEGED_HELPER_STATE_DIR"; } +remove_safe_profile_state_dir() { rm -rf -- "$SAFE_PROFILE_STATE_DIR"; } +''' + entry = self.source.split("# --- Uninstall Logic ---\n", 1)[1].split("# --- Validation ---\n", 1)[0] + selected = definitions(self.source) + if flow == "process": entry = 'read_legacy_agent_removal_processes "$TRUENAS_BOOTSTRAP_SCRIPT"\n' + executable = shell + "\n" + setup + "\n" + redirect(selected) + "\nmain() {\n" + redirect(entry) + "}\nmain\n" + env = dict(os.environ, FIXTURE=str(root), PATH=str(tools) + ":" + os.environ["PATH"]) + result = subprocess.run(["bash", "-c", executable], env=env, capture_output=True, text=True, timeout=12) + second = subprocess.run(["bash", "-c", executable], env=env, capture_output=True, text=True, timeout=12) if repeat else None + events = (root / "events").read_text() if (root / "events").exists() else "" + after = self.snapshot(root) + state = json.loads((root / "model.json").read_text()) + timed_stopped = not (root / "timed-pid").exists() or not Path('/proc/' + (root / "timed-pid").read_text()).exists() + return result, second, events.splitlines(), before, after, state, timed_stopped + + @staticmethod + def snapshot(root): + result = {} + for path in root.rglob("*"): + if path.parts[len(root.parts)] not in ("state", "helper", "profile", "unit", "unrelated"): + continue + mode = path.lstat().st_mode + contents = os.readlink(path) if stat.S_ISLNK(mode) else path.read_bytes() if stat.S_ISREG(mode) else None + result[str(path.relative_to(root))] = (mode, contents, path.lstat().st_ino) + return result + + def retained(self, result): + execution, _, events, before, after, _, _ = result + self.assertEqual(execution.returncode, 1, execution.stdout + execution.stderr) + self.assertNotIn('state:remove', events) + for path in before: + if path.startswith(('state', 'helper', 'profile')) or path == 'unrelated': self.assertEqual(after.get(path), before[path], path) + self.assertNotIn('synthetic recovery only', execution.stdout + execution.stderr) + return events + + def test_unavailable_or_untrusted_task_inventory_cannot_remove_recovery(self): + for fault in ('failed-query', 'empty-query', 'malformed-query', 'object-query', 'duplicate-field', + 'diagnostic-query', 'duplicate-id', 'bool-id', 'string-enabled', 'wrong-kind', + 'oversized-query', 'oversized-inventory'): + with self.subTest(fault=fault): + events = self.retained(self.exercise(fault)) + self.assertNotIn('remote:remove', events) + self.assertFalse(any(e.startswith('task:disable:') or e.startswith('task:delete:') for e in events)) + + def test_foreign_or_indirect_bootstrap_is_not_mutated(self): + for fault in ('foreign-owner', 'command-bootstrap', 'other-bootstrap'): + with self.subTest(fault=fault): + events = self.retained(self.exercise(fault)) + self.assertNotIn('remote:remove', events) + self.assertFalse(any(e.startswith('task:disable:') or e.startswith('task:delete:') for e in events)) + + def test_bootstrap_symlink_directory_or_writable_file_refuses_before_mutation(self): + for bootstrap in ('linked', 'directory', 'writable'): + with self.subTest(bootstrap=bootstrap): + events = self.retained(self.exercise(bootstrap=bootstrap)) + self.assertEqual(events, []) + + def test_missing_tools_are_not_local_only_removal_permission(self): + for fault in ('missing-midclt', 'missing-python', 'missing-timeout'): + with self.subTest(fault=fault): + events = self.retained(self.exercise(fault)) + self.assertEqual(events, []) + + def test_every_matching_task_is_disabled_and_independently_observed_before_stop(self): + result = self.exercise() + self.assertEqual(result[0].returncode, 0, result[0].stderr) + events = result[2] + self.assertLess(events.index('task:disable:2'), events.index('unit:stop')) + self.assertEqual(events[:4], ['task:query', 'task:disable:1', 'task:disable:2', 'task:query']) + self.assertLess(events.index('process:read'), events.index('remote:remove')) + + def test_partial_refused_ignored_or_reactivated_disable_retains_recovery(self): + for fault in ('refused-disable', 'partial-disable', 'ignored-disable', 'reactivated-task'): + with self.subTest(fault=fault): + events = self.retained(self.exercise(fault)) + self.assertNotIn('remote:remove', events) + self.assertNotIn('unit:stop', events) + + def test_unit_or_process_uncertainty_keeps_stopped_task_recovery(self): + for fault in ('failed-unit-read', 'refused-stop', 'ignored-stop', 'failed-process', 'empty-process', + 'duplicate-process', 'diagnostic-process', 'orphan-agent', 'live-bootstrap'): + with self.subTest(fault=fault): + events = self.retained(self.exercise(fault)) + self.assertNotIn('remote:remove', events) + + def test_task_activation_during_systemd_stop_refuses_before_remote(self): + result = self.exercise('reactivated-task-on-stop') + events = self.retained(result) + self.assertIn('unit:stop', events) + self.assertNotIn('remote:remove', events) + self.assertFalse(any(e.startswith('task:delete:') for e in events)) + + def test_server_refusal_retains_disabled_tasks_and_all_recovery_without_restart(self): + result = self.exercise(reply='refused') + events = self.retained(result) + self.assertIn('remote:remove', events) + self.assertFalse(any(e.startswith('task:delete:') for e in events)) + self.assertTrue(all(not t['enabled'] for t in result[5]['tasks'] if t['id'] != 99)) + + def test_changed_or_refused_post_remote_inventory_precedes_first_deletion(self): + for fault in ('failed-after-remote-query', 'new-task-after-remote'): + with self.subTest(fault=fault): + events = self.retained(self.exercise(fault)) + self.assertIn('remote:remove', events) + self.assertFalse(any(e.startswith('task:delete:') for e in events)) + + def test_partial_refused_ignored_or_recreated_task_deletion_keeps_recovery(self): + for fault in ('refused-delete', 'partial-delete', 'ignored-delete', 'recreated-task', 'unit-reactivation-on-delete'): + with self.subTest(fault=fault): self.retained(self.exercise(fault)) + + def test_all_owned_tasks_retire_before_state_erasure_and_unrelated_task_survives(self): + result = self.exercise() + self.assertEqual(result[0].returncode, 0, result[0].stderr) + events = result[2] + self.assertLess(events.index('task:delete:2'), events.index('state:remove')) + self.assertIn('task:query', events[events.index('task:delete:2') + 1:events.index('state:remove')]) + self.assertEqual([t['id'] for t in result[5]['tasks']], [99]) + self.assertTrue(result[5]['tasks'][0]['enabled']) + self.assertNotIn('state/runtime.token', result[4]) + self.assertEqual(result[4]['unrelated'], result[3]['unrelated']) + + def test_disabled_absent_missing_bootstrap_and_repeated_removal_are_observed(self): + for tasks, bootstrap in (('disabled', 'regular'), ('absent', 'missing'), ('single', 'missing')): + with self.subTest(tasks=tasks, bootstrap=bootstrap): + result = self.exercise(tasks=tasks, bootstrap=bootstrap, repeat=True) + self.assertEqual(result[0].returncode, 0, result[0].stderr) + self.assertEqual(result[1].returncode, 0, result[1].stderr) + self.assertEqual([t['id'] for t in result[5]['tasks']], [99]) + + def test_ordinary_linux_state_is_not_appliance_evidence(self): + result = self.exercise(bootstrap='missing', tasks='absent', appliance=False) + self.assertEqual(result[0].returncode, 0, result[0].stderr) + self.assertFalse(any(e.startswith('task:') for e in result[2])) + + def test_process_read_exposes_only_roles_and_real_timeout_child_is_stopped(self): + result = self.exercise(fault='live-bootstrap', flow='process') + self.assertEqual(result[0].returncode, 0, result[0].stderr) + self.assertIn('101 1 bootstrap', result[0].stdout) + self.assertNotIn('recovery-only', result[0].stdout + result[0].stderr) + result = self.exercise(fault='timeout-query') + self.retained(result) + self.assertTrue(result[6], 'timed middleware child still exists') + + +if __name__ == '__main__': + unittest.main(verbosity=2) diff --git a/scripts/installtests/agent_uninstall_state.py b/scripts/installtests/agent_uninstall_state.py index 029e6a8fe0..725ac2b4c4 100644 --- a/scripts/installtests/agent_uninstall_state.py +++ b/scripts/installtests/agent_uninstall_state.py @@ -48,7 +48,7 @@ class UnixUninstallState(unittest.TestCase): # Stop before unrelated platform boot/log cleanup; never run it on a host. entry = source.split("# --- Uninstall Logic ---\n", 1)[1].split( " # Remove least-privilege helper artifacts.", 1)[0] + "fi\n" - names = ("collector_credential_state_present", "uninstall_collector_registration", + names = ("truenas_scale_agent_removal_present", "collector_credential_state_present", "uninstall_collector_registration", "remove_authorized_runtime_dir", "remove_agent_state_dir", "remove_privileged_helper_state_dir", "remove_safe_profile_state_dir", "fail") with tempfile.TemporaryDirectory() as temporary: diff --git a/scripts/installtests/agent_upstart_teardown.py b/scripts/installtests/agent_upstart_teardown.py index 376a405435..3b4a6a5d4b 100644 --- a/scripts/installtests/agent_upstart_teardown.py +++ b/scripts/installtests/agent_upstart_teardown.py @@ -16,7 +16,7 @@ import unittest ROOT = Path(__file__).resolve().parents[2] SOURCE = Path(os.environ.get("PULSE_AGENT_UPSTART_SOURCE", ROOT / "scripts/install.sh")) -FUNCTIONS = ("cleanup", "portable_path_uid", "portable_path_mode", "trusted_lifecycle_regular_file", +FUNCTIONS = ("truenas_scale_agent_removal_present", "cleanup", "portable_path_uid", "portable_path_mode", "trusted_lifecycle_regular_file", "read_legacy_agent_removal_processes", "legacy_agent_processes_absent", "read_upstart_agent_removal_status", "upstart_agent_removal_paths_safe", "read_upstart_agent_removal_enablement", "quiesce_upstart_agent_for_removal", diff --git a/scripts/installtests/install_sh_test.go b/scripts/installtests/install_sh_test.go index 490b04be73..928e005506 100644 --- a/scripts/installtests/install_sh_test.go +++ b/scripts/installtests/install_sh_test.go @@ -8501,3 +8501,41 @@ func TestInstallSHUpstartTeardownRequiresObservedQuiescence(t *testing.T) { } t.Logf("%s", output) } + +func TestInstallSHTrueNASScaleBootTasksRetireBeforeRecoveryErasure(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("TrueNAS SCALE shell controls; Windows has its own native SCM gate") + } + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) + defer cancel() + cmd := exec.CommandContext(ctx, "python3", repoFile("scripts", "installtests", "agent_truenas_teardown.py")) + output, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("TrueNAS SCALE boot-task ownership/retirement controls: %v\n%s", err, output) + } + for _, name := range []string{ + "test_unavailable_or_untrusted_task_inventory_cannot_remove_recovery", + "test_foreign_or_indirect_bootstrap_is_not_mutated", + "test_bootstrap_symlink_directory_or_writable_file_refuses_before_mutation", + "test_missing_tools_are_not_local_only_removal_permission", + "test_every_matching_task_is_disabled_and_independently_observed_before_stop", + "test_partial_refused_ignored_or_reactivated_disable_retains_recovery", + "test_unit_or_process_uncertainty_keeps_stopped_task_recovery", + "test_task_activation_during_systemd_stop_refuses_before_remote", + "test_server_refusal_retains_disabled_tasks_and_all_recovery_without_restart", + "test_changed_or_refused_post_remote_inventory_precedes_first_deletion", + "test_partial_refused_ignored_or_recreated_task_deletion_keeps_recovery", + "test_all_owned_tasks_retire_before_state_erasure_and_unrelated_task_survives", + "test_disabled_absent_missing_bootstrap_and_repeated_removal_are_observed", + "test_ordinary_linux_state_is_not_appliance_evidence", + "test_process_read_exposes_only_roles_and_real_timeout_child_is_stopped", + } { + if !strings.Contains(string(output), name+" (__main__.AgentTrueNASTeardown."+name+") ... ok") { + t.Fatalf("TrueNAS SCALE control %s did not pass:\n%s", name, output) + } + } + if !strings.Contains(string(output), "\nRan 15 tests in ") || !strings.Contains(string(output), "\nOK\n") { + t.Fatalf("incomplete TrueNAS SCALE verdict:\n%s", output) + } + t.Logf("%s", output) +} From 9cde497ff3e8c8469a7bad16ca52a92df4310d96 Mon Sep 17 00:00:00 2001 From: "pulse-triage[bot]" <249995291+pulse-triage[bot]@users.noreply.github.com> Date: Mon, 12 Oct 2026 18:06:20 +0100 Subject: [PATCH 4/6] Complete the mounted backup clock workload fixture Supply the required lock and lastSeen fields after committed-source type checking identified the incomplete fixture. Keep both fields fixed so policy-age tests still prove time-only invalidation. Production bytes and browser proof are unchanged. Change-source: pulse-maintainer --- .../Workloads/__tests__/GuestDrawer.backup-clock.test.tsx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/frontend-modern/src/components/Workloads/__tests__/GuestDrawer.backup-clock.test.tsx b/frontend-modern/src/components/Workloads/__tests__/GuestDrawer.backup-clock.test.tsx index 44e9fd79d7..0bf1b7c448 100644 --- a/frontend-modern/src/components/Workloads/__tests__/GuestDrawer.backup-clock.test.tsx +++ b/frontend-modern/src/components/Workloads/__tests__/GuestDrawer.backup-clock.test.tsx @@ -52,6 +52,8 @@ const guest = (type: 'qemu' | 'lxc', lastBackup: number): WorkloadGuest => ({ template: false, tags: [], lastBackup, + lock: '', + lastSeen: new Date(start).toISOString(), }); const details = () => within(screen.getByTestId('guest-technical-details')); const backupRow = () => details().getByText('Last completed backup').closest('tr')!; From 3f63e2021638b2466a2ce7a1318698ac91fb0ea0 Mon Sep 17 00:00:00 2001 From: "pulse-triage[bot]" <249995291+pulse-triage[bot]@users.noreply.github.com> Date: Mon, 12 Oct 2026 18:08:19 +0100 Subject: [PATCH 5/6] Refuse incomplete SCALE task activation targets before removal A missing SCRIPT path or empty COMMAND target cannot establish that a task is unrelated or absent. Retain recovery before any task or service mutation when those fields are unavailable. Add three actual-caller adverse controls to the mandatory sixteen-method hook and both owning contracts. Change-source: pulse-maintainer --- .../v6/internal/subsystems/agent-lifecycle.md | 6 ++++-- .../internal/subsystems/deployment-installability.md | 6 ++++-- scripts/install.sh | 2 ++ scripts/installtests/agent_truenas_teardown.py | 10 ++++++++++ scripts/installtests/install_sh_test.go | 3 ++- 5 files changed, 22 insertions(+), 5 deletions(-) diff --git a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md index d71de6fb80..ddbbb9348e 100644 --- a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md +++ b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md @@ -30,7 +30,9 @@ SCRIPT/POSTINIT task, including duplicates. Unknown ownership, indirect command activation or another-install bootstrap refuses without deleting unrelated tasks. The JSON reader rejects duplicate keys/IDs, invalid types, failed/empty/ diagnostic/incomplete reads, over 512 tasks or over 1 MiB; it returns only owned -numeric IDs and enablement, never task commands, names or credentials. All owned +numeric IDs and enablement, never task commands, names or credentials. A +missing/empty SCRIPT path or COMMAND target is unknown activation, not an +unrelated task or evidence that the owned task is absent. All owned tasks are disabled with checked calls and independent exact-set readback before systemd stop. A partial stop/disable stays stopped/disabled: do not re-enable or automatically restart after a refusal. @@ -47,7 +49,7 @@ succeeded. No favourable replacement observation, task recreation or credential churn is performed. Unrelated tasks survive unchanged. `TestInstallSHTrueNASScaleBootTasksRetireBeforeRecoveryErasure` requires all -fifteen `agent_truenas_teardown.py` actual-function/full-caller controls, +sixteen `agent_truenas_teardown.py` actual-function/full-caller controls, including multiple tasks, partial/refused/unknown operations, post-remote changes, ignored/recreated deletion, process privacy and a real timeout child. These run against closed middleware/systemd/process models and private ordinary- diff --git a/docs/release-control/v6/internal/subsystems/deployment-installability.md b/docs/release-control/v6/internal/subsystems/deployment-installability.md index b6d4667eec..1f61cabe18 100644 --- a/docs/release-control/v6/internal/subsystems/deployment-installability.md +++ b/docs/release-control/v6/internal/subsystems/deployment-installability.md @@ -31,7 +31,9 @@ SCRIPT/POSTINIT task, including duplicates. Unknown ownership, indirect command activation or another-install bootstrap refuses without deleting unrelated tasks. The JSON reader rejects duplicate keys/IDs, invalid types, failed/empty/ diagnostic/incomplete reads, over 512 tasks or over 1 MiB; it returns only owned -numeric IDs and enablement, never task commands, names or credentials. All owned +numeric IDs and enablement, never task commands, names or credentials. A +missing/empty SCRIPT path or COMMAND target is unknown activation, not an +unrelated task or evidence that the owned task is absent. All owned tasks are disabled with checked calls and independent exact-set readback before systemd stop. A partial stop/disable stays stopped/disabled: do not re-enable or automatically restart after a refusal. @@ -48,7 +50,7 @@ succeeded. No favourable replacement observation, task recreation or credential churn is performed. Unrelated tasks survive unchanged. `TestInstallSHTrueNASScaleBootTasksRetireBeforeRecoveryErasure` requires all -fifteen `agent_truenas_teardown.py` actual-function/full-caller controls, +sixteen `agent_truenas_teardown.py` actual-function/full-caller controls, including multiple tasks, partial/refused/unknown operations, post-remote changes, ignored/recreated deletion, process privacy and a real timeout child. These run against closed middleware/systemd/process models and private ordinary- diff --git a/scripts/install.sh b/scripts/install.sh index 82134ddf26..91bf5cb06e 100755 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -1754,6 +1754,8 @@ try: if script is None: script = "" if command is None: command = "" if not isinstance(script, str) or not isinstance(command, str): raise ValueError("invalid activation") + if (kind == "SCRIPT" and not script) or (kind == "COMMAND" and not command): + raise ValueError("missing activation target") if script == expected: if (kind != "SCRIPT" or command or task.get("when") != "POSTINIT" or task.get("comment") != "Pulse Agent Bootstrap" or type(task.get("timeout")) is not int or diff --git a/scripts/installtests/agent_truenas_teardown.py b/scripts/installtests/agent_truenas_teardown.py index b5542df5b8..624ed9293a 100644 --- a/scripts/installtests/agent_truenas_teardown.py +++ b/scripts/installtests/agent_truenas_teardown.py @@ -116,6 +116,9 @@ if verb == 'initshutdownscript.query': if fault == 'string-enabled': tasks[0]['enabled'] = 'false' if fault == 'foreign-owner': tasks[0]['comment'] = 'not owned by Pulse' if fault == 'wrong-kind': tasks[0]['type'] = 'UNKNOWN' + if fault == 'missing-script': tasks[0].pop('script') + if fault == 'null-script': tasks[0]['script'] = None + if fault == 'empty-command': tasks[0].update(type='COMMAND', script='', command='') if fault == 'command-bootstrap': tasks[0].update(type='COMMAND', script='', command='/bin/bash ' + str(root / 'state/bootstrap-pulse-agent.sh')) if fault == 'other-bootstrap': tasks[0]['script'] = '/other-install/bootstrap-pulse-agent.sh' if fault == 'diagnostic-query': print('read unavailable', file=sys.stderr) @@ -280,6 +283,13 @@ remove_safe_profile_state_dir() { rm -rf -- "$SAFE_PROFILE_STATE_DIR"; } self.assertNotIn('remote:remove', events) self.assertFalse(any(e.startswith('task:disable:') or e.startswith('task:delete:') for e in events)) + def test_missing_activation_target_cannot_hide_an_owned_task(self): + for fault in ('missing-script', 'null-script', 'empty-command'): + with self.subTest(fault=fault): + events = self.retained(self.exercise(fault)) + self.assertNotIn('remote:remove', events) + self.assertFalse(any(e.startswith('task:disable:') or e.startswith('task:delete:') for e in events)) + def test_bootstrap_symlink_directory_or_writable_file_refuses_before_mutation(self): for bootstrap in ('linked', 'directory', 'writable'): with self.subTest(bootstrap=bootstrap): diff --git a/scripts/installtests/install_sh_test.go b/scripts/installtests/install_sh_test.go index 928e005506..635f494f41 100644 --- a/scripts/installtests/install_sh_test.go +++ b/scripts/installtests/install_sh_test.go @@ -8516,6 +8516,7 @@ func TestInstallSHTrueNASScaleBootTasksRetireBeforeRecoveryErasure(t *testing.T) for _, name := range []string{ "test_unavailable_or_untrusted_task_inventory_cannot_remove_recovery", "test_foreign_or_indirect_bootstrap_is_not_mutated", + "test_missing_activation_target_cannot_hide_an_owned_task", "test_bootstrap_symlink_directory_or_writable_file_refuses_before_mutation", "test_missing_tools_are_not_local_only_removal_permission", "test_every_matching_task_is_disabled_and_independently_observed_before_stop", @@ -8534,7 +8535,7 @@ func TestInstallSHTrueNASScaleBootTasksRetireBeforeRecoveryErasure(t *testing.T) t.Fatalf("TrueNAS SCALE control %s did not pass:\n%s", name, output) } } - if !strings.Contains(string(output), "\nRan 15 tests in ") || !strings.Contains(string(output), "\nOK\n") { + if !strings.Contains(string(output), "\nRan 16 tests in ") || !strings.Contains(string(output), "\nOK\n") { t.Fatalf("incomplete TrueNAS SCALE verdict:\n%s", output) } t.Logf("%s", output) From 97e4c8eac7caa9c6f6634ac27667ede337645530 Mon Sep 17 00:00:00 2001 From: "pulse-triage[bot]" <249995291+pulse-triage[bot]@users.noreply.github.com> Date: Mon, 12 Oct 2026 18:14:32 +0100 Subject: [PATCH 6/6] Share Proxmox session renewal across concurrent polling reads Publish ticket, CSRF and expiry as one locked snapshot, and bind expiry/401 renewal to the rejected session generation. Concurrent readers share one authentication attempt while retaining their own cancellation and ordinary single-retry behaviour. Keep guest commands single-attempt and token/TLS policy unchanged. Contract-Neutral: Synchronise existing Proxmox password-session renewal without changing API fields, TLS policy or guest-command admission and replay rules. Change-source: pulse-maintainer --- .../monitoring/pve_session_recovery_test.go | 107 ++++++ pkg/proxmox/auth_session_concurrency_test.go | 325 ++++++++++++++++++ pkg/proxmox/client.go | 93 ++++- 3 files changed, 506 insertions(+), 19 deletions(-) create mode 100644 internal/monitoring/pve_session_recovery_test.go create mode 100644 pkg/proxmox/auth_session_concurrency_test.go diff --git a/internal/monitoring/pve_session_recovery_test.go b/internal/monitoring/pve_session_recovery_test.go new file mode 100644 index 0000000000..fc8d0b8ab8 --- /dev/null +++ b/internal/monitoring/pve_session_recovery_test.go @@ -0,0 +1,107 @@ +package monitoring + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "strings" + "sync/atomic" + "testing" + "time" + + "github.com/rcourtman/pulse-go-rewrite/internal/config" + "github.com/rcourtman/pulse-go-rewrite/internal/models" + "github.com/rcourtman/pulse-go-rewrite/internal/unifiedresources" + "github.com/rcourtman/pulse-go-rewrite/pkg/proxmox" +) + +// Exercise the changed password client through the real parallel node poller, +// registry and frontend projection. No native QGA/SSH, backup or database is +// used. The emitted synthetic resources also feed the retained pixel proof. +func TestPVESessionRenewalProducesTrustedNodeReadback(t *testing.T) { + const count = 8 + var authCalls, rejectedStatus, acceptedStatus, agentCalls atomic.Int32 + allOldReads := make(chan struct{}) + ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) + defer cancel() + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + switch { + case strings.HasSuffix(r.URL.Path, "/access/ticket"): + n := authCalls.Add(1) + fmt.Fprintf(w, `{"data":{"ticket":"synthetic-session-%d","CSRFPreventionToken":"synthetic-csrf-%d"}}`, n, n) + case r.URL.Path == "/api2/json/nodes": + nodes := make([]proxmox.Node, count) + for i := range nodes { + nodes[i] = proxmox.Node{Node: fmt.Sprintf("session-node-%02d", i+1), Status: "online", CPU: .05, MaxCPU: 4, MaxMem: 2 << 30, Mem: 1 << 30, MaxDisk: 100 << 30, Disk: 25 << 30, Uptime: 7200} + } + json.NewEncoder(w).Encode(map[string]any{"data": nodes}) + case strings.HasSuffix(r.URL.Path, "/status"): + cookie, err := r.Cookie("PVEAuthCookie") + if err != nil { + t.Error("status missing synthetic session") + w.WriteHeader(401) + return + } + if cookie.Value == "synthetic-session-1" { + if rejectedStatus.Add(1) == count { + close(allOldReads) + } + select { + case <-allOldReads: + case <-ctx.Done(): + return + } + w.WriteHeader(http.StatusUnauthorized) + fmt.Fprint(w, `{"message":"synthetic expired session"}`) + return + } + acceptedStatus.Add(1) + fmt.Fprint(w, `{"data":{"cpu":0.05,"memory":{"total":2147483648,"used":536870912,"free":1073741824,"available":1610612736},"rootfs":{"total":107374182400,"used":26843545600,"free":80530636800},"uptime":7200,"loadavg":[0.1,0.1,0.1],"kversion":"fixture-kernel","pveversion":"pve-manager/9.2.21/fixture","cpuinfo":{"model":"Synthetic CPU","cores":4,"sockets":1,"mhz":"2000"}}}`) + case strings.Contains(r.URL.Path, "/agent/"): + agentCalls.Add(1) + t.Error("node recovery queued guest work") + w.WriteHeader(500) + default: + fmt.Fprint(w, `{"data":[]}`) // ordinary network/apt inventory only + } + })) + defer server.Close() + client, err := proxmox.NewClient(proxmox.ClientConfig{Host: server.URL, User: "synthetic@pam", Password: "synthetic-password", Timeout: 5 * time.Second}) + if err != nil { + t.Fatal(err) + } + rawNodes, err := client.GetNodes(ctx) + if err != nil || len(rawNodes) != count { + t.Fatalf("ordinary node discovery: %v", err) + } + instance := config.PVEInstance{Name: "session-fixture", Host: server.URL, IsCluster: true, ClusterName: "Session recovery"} + m := &Monitor{config: &config.Config{PVEInstances: []config.PVEInstance{instance}}, nodeLastOnline: make(map[string]time.Time)} + nodes, statuses, _ := m.pollPVENodesParallel(ctx, instance.Name, &instance, client, rawNodes, "healthy", nil, false) + if len(nodes) != count || len(statuses) != count || authCalls.Load() != 2 || rejectedStatus.Load() != count || acceptedStatus.Load() != count || agentCalls.Load() != 0 { + t.Fatalf("node recovery counts nodes/statuses/auth/rejected/accepted/guest=%d/%d/%d/%d/%d/%d", len(nodes), len(statuses), authCalls.Load(), rejectedStatus.Load(), acceptedStatus.Load(), agentCalls.Load()) + } + for _, node := range nodes { + if node.Status != "online" || node.ConnectionHealth != "healthy" || !node.Memory.HasKnownUsage() || node.Memory.Usage != 25 || node.Disk.Usage != 25 { + t.Fatalf("session recovery lost trusted ordinary fields: %+v", node) + } + } + registry := unifiedresources.NewRegistry(nil) + registry.IngestSnapshot(models.StateSnapshot{Nodes: nodes}) + resources, _ := convertPresentationResourcesForBroadcast(registry.ListForPresentation()) + if len(resources) != count { + t.Fatal("node projection lost recovered nodes") + } + for _, resource := range resources { + if resource.Memory == nil || resource.Memory.Current != 25 || resource.CPU == nil || resource.CPU.Current != 5 { + t.Fatalf("rendered projection changed cache-aware memory/CPU: %+v", resource) + } + } + payload, err := json.Marshal(map[string]any{"resources": resources, "authentications": authCalls.Load(), "rejectedStatusReads": rejectedStatus.Load(), "acceptedStatusReads": acceptedStatus.Load(), "guestCommands": agentCalls.Load()}) + if err != nil { + t.Fatal(err) + } + t.Logf("PULSE_SESSION_RECOVERY_JSON=%s", payload) +} diff --git a/pkg/proxmox/auth_session_concurrency_test.go b/pkg/proxmox/auth_session_concurrency_test.go new file mode 100644 index 0000000000..cd83e654ec --- /dev/null +++ b/pkg/proxmox/auth_session_concurrency_test.go @@ -0,0 +1,325 @@ +package proxmox + +import ( + "context" + "errors" + "fmt" + "net/http" + "net/http/httptest" + "strconv" + "strings" + "sync" + "sync/atomic" + "testing" + "time" +) + +// All reads have already left with the same rejected session before renewal +// can start. Late 401s must reuse the new session, not invalidate it again. +// The fixture issues synthetic tickets only and never queues guest work. +func TestClientSessionConcurrent401(t *testing.T) { + const readers = 16 + var authentications, oldRequests, newRequests, mismatchedPairs atomic.Int32 + oldArrived := make(chan struct{}) + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if strings.HasSuffix(r.URL.Path, "/access/ticket") { + n := authentications.Add(1) + fmt.Fprintf(w, `{"data":{"ticket":"session-%d","CSRFPreventionToken":"csrf-%d"}}`, n, n) + return + } + cookie, err := r.Cookie("PVEAuthCookie") + if err != nil { + t.Error("request missing synthetic session") + w.WriteHeader(401) + return + } + if cookie.Value == "session-1" { + if oldRequests.Add(1) == readers { + close(oldArrived) + } + select { + case <-oldArrived: + case <-ctx.Done(): + return + } + w.WriteHeader(http.StatusUnauthorized) + fmt.Fprint(w, `{"message":"synthetic session expired"}`) + return + } + if r.Method != http.MethodGet { + n := strings.TrimPrefix(cookie.Value, "session-") + if r.Header.Get("CSRFPreventionToken") != "csrf-"+n { + mismatchedPairs.Add(1) + w.WriteHeader(403) + return + } + } + newRequests.Add(1) + fmt.Fprint(w, `{"data":[{"node":"session-safe-node","status":"online","cpu":0.05,"maxcpu":4,"mem":268435456,"maxmem":2147483648}]}`) + })) + defer server.Close() + c, err := NewClient(ClientConfig{Host: server.URL, User: "synthetic@pam", Password: "synthetic-password", Timeout: 5 * time.Second}) + if err != nil { + t.Fatal(err) + } + defer c.httpClient.CloseIdleConnections() + errors := make(chan error, readers) + var wg sync.WaitGroup + for i := 0; i < readers; i++ { + wg.Add(1) + go func() { + defer wg.Done() + nodes, err := c.GetNodes(ctx) + if err == nil && (len(nodes) != 1 || nodes[0].Node != "session-safe-node" || nodes[0].Status != "online") { + err = fmt.Errorf("renewed session lost ordinary node result") + } + errors <- err + }() + } + wg.Wait() + close(errors) + for err := range errors { + if err != nil { + t.Error(err) + } + } + // Check ticket/CSRF pairing through the same production request path. + for i := 0; i < readers; i++ { + resp, err := c.request(ctx, http.MethodPost, "/synthetic-read", nil) + if err != nil { + t.Error(err) + continue + } + _, err = readResponseBodyLimited(resp.Body) + resp.Body.Close() + if err != nil { + t.Error(err) + } + } + if got := authentications.Load(); got != 2 { + t.Errorf("session-renewal stampede: authentications=%d, want initial plus one renewal", got) + } + if oldRequests.Load() != readers || newRequests.Load() != 2*readers || mismatchedPairs.Load() != 0 { + t.Errorf("old/new/pair-mismatch=%d/%d/%d, want %d/%d/0", oldRequests.Load(), newRequests.Load(), mismatchedPairs.Load(), readers, 2*readers) + } + t.Log("ordinary reads=" + strconv.Itoa(readers) + "; one shared renewal required; coherent ticket/CSRF pairs checked") +} + +// Done signals the exact point at which a caller joins a pending renewal. +// There is no sleep or production-only instrumentation in these controls. +type authJoinContext struct { + context.Context + joined chan struct{} + once sync.Once +} + +func (c *authJoinContext) Done() <-chan struct{} { + c.once.Do(func() { close(c.joined) }) + return c.Context.Done() +} + +func awaitSessionSignal(t *testing.T, signal <-chan struct{}) { + t.Helper() + select { + case <-signal: + case <-time.After(5 * time.Second): + t.Fatal("session fixture barrier did not complete") + } +} + +func expireSessionFixture(c *Client) { + c.authMu.Lock() + c.auth.expiresAt = time.Now().Add(-time.Hour) + c.authMu.Unlock() +} + +func TestClientSessionConcurrentExpiryAndFailureRecovery(t *testing.T) { + for _, failRenewal := range []bool{false, true} { + t.Run(fmt.Sprintf("failure=%t", failRenewal), func(t *testing.T) { + const readers = 8 + entered, finish := make(chan struct{}), make(chan struct{}) + var authCalls, apiCalls atomic.Int32 + var fail atomic.Bool + fail.Store(failRenewal) + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if strings.HasSuffix(r.URL.Path, "/access/ticket") { + n := authCalls.Add(1) + if n == 2 { + close(entered) + select { + case <-finish: + case <-ctx.Done(): + return + } + if fail.Load() { + w.WriteHeader(http.StatusServiceUnavailable) + fmt.Fprint(w, "synthetic renewal unavailable") + return + } + } + fmt.Fprintf(w, `{"data":{"ticket":"session-%d","CSRFPreventionToken":"csrf-%d"}}`, n, n) + return + } + apiCalls.Add(1) + fmt.Fprint(w, `{"data":[{"node":"session-safe-node","status":"online"}]}`) + })) + defer server.Close() + c, err := NewClient(ClientConfig{Host: server.URL, User: "synthetic@pam", Password: "synthetic-password", Timeout: 5 * time.Second}) + if err != nil { + t.Fatal(err) + } + defer c.httpClient.CloseIdleConnections() + expireSessionFixture(c) + done := make(chan error, readers) + go func() { _, err := c.GetNodes(ctx); done <- err }() + awaitSessionSignal(t, entered) + for i := 1; i < readers; i++ { + joined := &authJoinContext{Context: ctx, joined: make(chan struct{})} + go func() { _, err := c.GetNodes(joined); done <- err }() + awaitSessionSignal(t, joined.joined) + } + if authCalls.Load() != 2 || apiCalls.Load() != 0 { + t.Errorf("expired readers bypassed shared renewal: auth/api=%d/%d", authCalls.Load(), apiCalls.Load()) + } + close(finish) + for i := 0; i < readers; i++ { + err := <-done + if failRenewal { + if err == nil || !strings.Contains(err.Error(), "synthetic renewal unavailable") { + t.Errorf("renewal failure not shared: %v", err) + } + } else if err != nil { + t.Error(err) + } + } + if authCalls.Load() != 2 { + t.Error("joined callers retried completed renewal") + } + if failRenewal { + snapshot, _ := c.authSnapshot() + if snapshot.ticket != "session-1" || snapshot.csrfToken != "csrf-1" || !snapshot.expiresAt.Before(time.Now()) { + t.Error("failed renewal published or refreshed the old session") + } + if apiCalls.Load() != 0 { + t.Error("failed authentication admitted API reads") + } + // A new ordinary call may recover; the failed attempt is not + // automatically replayed by any of its joined callers. + fail.Store(false) + if nodes, err := c.GetNodes(ctx); err != nil || len(nodes) != 1 { + t.Errorf("ordinary later recovery failed: %v", err) + } + if authCalls.Load() != 3 || apiCalls.Load() != 1 { + t.Errorf("later recovery counts auth/api=%d/%d", authCalls.Load(), apiCalls.Load()) + } + } else if apiCalls.Load() != readers { + t.Errorf("renewed session lost %d reads", readers-apiCalls.Load()) + } + }) + } +} + +func TestClientSessionRenewalWaiterCancellation(t *testing.T) { + entered, finish := make(chan struct{}), make(chan struct{}) + var authCalls, apiCalls atomic.Int32 + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if strings.HasSuffix(r.URL.Path, "/access/ticket") { + n := authCalls.Add(1) + if n == 2 { + close(entered) + select { + case <-finish: + case <-ctx.Done(): + return + } + } + fmt.Fprintf(w, `{"data":{"ticket":"session-%d","CSRFPreventionToken":"csrf-%d"}}`, n, n) + return + } + apiCalls.Add(1) + fmt.Fprint(w, `{"data":[]}`) + })) + defer server.Close() + c, err := NewClient(ClientConfig{Host: server.URL, User: "synthetic@pam", Password: "synthetic-password", Timeout: 5 * time.Second}) + if err != nil { + t.Fatal(err) + } + defer c.httpClient.CloseIdleConnections() + expireSessionFixture(c) + owner := make(chan error, 1) + go func() { _, err := c.GetNodes(ctx); owner <- err }() + awaitSessionSignal(t, entered) + waiterCtx, cancelWaiter := context.WithCancel(ctx) + defer cancelWaiter() + joined := &authJoinContext{Context: waiterCtx, joined: make(chan struct{})} + waiter := make(chan error, 1) + go func() { _, err := c.GetNodes(joined); waiter <- err }() + awaitSessionSignal(t, joined.joined) + cancelWaiter() + select { + case err := <-waiter: + if !errors.Is(err, context.Canceled) { + t.Errorf("cancelled waiter result: %v", err) + } + case <-time.After(time.Second): + t.Fatal("cancelled waiter remained blocked behind another caller's renewal") + } + if authCalls.Load() != 2 || apiCalls.Load() != 0 { + t.Error("cancelled waiter replayed renewal or admitted a read") + } + select { + case err := <-owner: + t.Fatalf("waiter cancellation ended the owner's renewal: %v", err) + default: + } + close(finish) + if err := <-owner; err != nil { + t.Fatal(err) + } + if _, err := c.GetNodes(ctx); err != nil { + t.Fatal(err) + } + if authCalls.Load() != 2 || apiCalls.Load() != 2 { + t.Errorf("owner/later recovery counts auth/api=%d/%d", authCalls.Load(), apiCalls.Load()) + } +} + +func TestClientSessionTokenAuthDoesNotRenew(t *testing.T) { + var authCalls, apiCalls atomic.Int32 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if strings.HasSuffix(r.URL.Path, "/access/ticket") { + authCalls.Add(1) + w.WriteHeader(500) + return + } + apiCalls.Add(1) + if r.Header.Get("Authorization") != "PVEAPIToken=synthetic@pve!pulse=synthetic-token" || r.Header.Get("Cookie") != "" || r.Header.Get("CSRFPreventionToken") != "" { + t.Error("token read acquired session headers") + } + fmt.Fprint(w, `{"data":[]}`) + })) + defer server.Close() + c, err := NewClient(ClientConfig{Host: server.URL, TokenName: "synthetic@pve!pulse", TokenValue: "synthetic-token", Password: "unused-synthetic-password", Timeout: time.Second}) + if err != nil { + t.Fatal(err) + } + defer c.httpClient.CloseIdleConnections() + done := make(chan error, 16) + for i := 0; i < 16; i++ { + go func() { _, err := c.GetNodes(context.Background()); done <- err }() + } + for i := 0; i < 16; i++ { + if err := <-done; err != nil { + t.Error(err) + } + } + if authCalls.Load() != 0 || apiCalls.Load() != 16 { + t.Errorf("token auth/API calls=%d/%d", authCalls.Load(), apiCalls.Load()) + } +} diff --git a/pkg/proxmox/client.go b/pkg/proxmox/client.go index 47f6a75a43..9613d2cca2 100644 --- a/pkg/proxmox/client.go +++ b/pkg/proxmox/client.go @@ -11,6 +11,7 @@ import ( "net/url" "strconv" "strings" + "sync" "time" "unicode" @@ -255,6 +256,24 @@ type Client struct { httpClient *http.Client auth auth config ClientConfig + + // A polling client is shared by concurrent node and guest reads. Publish + // each ticket/CSRF/expiry tuple atomically and share only session renewal, + // not the subsequent API requests or their caller-owned cancellation. + authMu sync.Mutex + authGeneration uint64 + authInFlight *authRefresh +} + +type authRefresh struct { + done chan struct{} + err error +} + +func (c *Client) authSnapshot() (auth, uint64) { + c.authMu.Lock() + defer c.authMu.Unlock() + return c.auth, c.authGeneration } func (c *Client) apiBaseURL() (*url.URL, error) { @@ -398,16 +417,47 @@ func NewClient(cfg ClientConfig) (*Client, error) { // authenticate performs password-based authentication func (c *Client) authenticate(ctx context.Context) error { - username := c.auth.user + "@" + c.auth.realm - password := c.config.Password + return c.renewAuthentication(ctx, nil) +} - if err := c.authenticateJSON(ctx, username, password); err == nil { - return nil - } else if shouldFallbackToForm(err) { - return c.authenticateForm(ctx, username, password) - } else { +// expected binds expiry or a 401 to the session actually used by that read. +// A late rejection of an old ticket cannot trigger another renewal after a +// concurrent request has already replaced it. Explicit authentication (nil) +// still owns the existing JSON/form fallback and failed-attempt behaviour. +func (c *Client) renewAuthentication(ctx context.Context, expected *uint64) error { + if err := ctx.Err(); err != nil { return err } + c.authMu.Lock() + if expected != nil && c.authGeneration != *expected { + c.authMu.Unlock() + return nil + } + if pending := c.authInFlight; pending != nil { + c.authMu.Unlock() + select { + case <-ctx.Done(): + return ctx.Err() + case <-pending.done: + return pending.err + } + } + pending := &authRefresh{done: make(chan struct{})} + c.authInFlight = pending + username := c.auth.user + "@" + c.auth.realm + c.authMu.Unlock() + + password := c.config.Password + err := c.authenticateJSON(ctx, username, password) + if shouldFallbackToForm(err) { + err = c.authenticateForm(ctx, username, password) + } + c.authMu.Lock() + pending.err = err + c.authInFlight = nil + close(pending.done) + c.authMu.Unlock() + return err } func (c *Client) authenticateJSON(ctx context.Context, username, password string) error { @@ -488,9 +538,12 @@ func (c *Client) handleAuthResponse(resp *http.Response) error { return err } + c.authMu.Lock() c.auth.ticket = result.Data.Ticket c.auth.csrfToken = result.Data.CSRFPreventionToken c.auth.expiresAt = time.Now().Add(2 * time.Hour) // PVE tickets expire after 2 hours + c.authGeneration++ + c.authMu.Unlock() return nil } @@ -528,10 +581,12 @@ func (c *Client) requestWithRetry(ctx context.Context, method, path string, data func (c *Client) requestWithRetryUsingClient(ctx context.Context, method, path string, data url.Values, retriedAfter401 bool, httpClient *http.Client) (*http.Response, error) { // Re-authenticate if needed - if c.config.Password != "" && c.auth.tokenName == "" && time.Now().After(c.auth.expiresAt) { - if err := c.authenticate(ctx); err != nil { + auth, generation := c.authSnapshot() + if c.config.Password != "" && auth.tokenName == "" && time.Now().After(auth.expiresAt) { + if err := c.renewAuthentication(ctx, &generation); err != nil { return nil, fmt.Errorf("re-authentication failed: %w", err) } + auth, generation = c.authSnapshot() } var body io.Reader @@ -554,23 +609,23 @@ func (c *Client) requestWithRetryUsingClient(ctx context.Context, method, path s } // Set authentication - if c.auth.tokenName != "" && c.auth.tokenValue != "" { + if auth.tokenName != "" && auth.tokenValue != "" { // API token authentication authHeader := fmt.Sprintf("PVEAPIToken=%s@%s!%s=%s", - c.auth.user, c.auth.realm, c.auth.tokenName, c.auth.tokenValue) + auth.user, auth.realm, auth.tokenName, auth.tokenValue) req.Header.Set("Authorization", authHeader) // NEVER log the actual token value - only log that we're using token auth maskedHeader := fmt.Sprintf("PVEAPIToken=%s@%s!%s=***", - c.auth.user, c.auth.realm, c.auth.tokenName) + auth.user, auth.realm, auth.tokenName) log.Debug(). Str("authHeader", maskedHeader). Str("url", req.URL.String()). Msg("Setting API token authentication") - } else if c.auth.ticket != "" { + } else if auth.ticket != "" { // Ticket authentication - req.Header.Set("Cookie", "PVEAuthCookie="+c.auth.ticket) - if method != "GET" && c.auth.csrfToken != "" { - req.Header.Set("CSRFPreventionToken", c.auth.csrfToken) + req.Header.Set("Cookie", "PVEAuthCookie="+auth.ticket) + if method != "GET" && auth.csrfToken != "" { + req.Header.Set("CSRFPreventionToken", auth.csrfToken) } } @@ -589,8 +644,8 @@ func (c *Client) requestWithRetryUsingClient(ctx context.Context, method, path s // Password sessions can be invalidated server-side before local expiry. // Retry once after forcing re-authentication on 401. - if resp.StatusCode == http.StatusUnauthorized && !retriedAfter401 && c.config.Password != "" && c.auth.tokenName == "" { - if err := c.authenticate(ctx); err != nil { + if resp.StatusCode == http.StatusUnauthorized && !retriedAfter401 && c.config.Password != "" && auth.tokenName == "" { + if err := c.renewAuthentication(ctx, &generation); err != nil { return nil, fmt.Errorf("re-authentication failed after 401: %w", err) } return c.requestWithRetryUsingClient(ctx, method, path, data, true, httpClient) @@ -600,7 +655,7 @@ func (c *Client) requestWithRetryUsingClient(ctx context.Context, method, path s var apiErr error if resp.StatusCode == 403 && c.config.TokenName != "" { // Special case for 403 with API token - this is usually a permission issue - apiErr = fmt.Errorf("API error 403 (Forbidden): The API token does not have sufficient permissions. Note: In Proxmox GUI, permissions must be set on the USER (not just the token). Please verify the user '%s@%s' has the required permissions", c.auth.user, c.auth.realm) + apiErr = fmt.Errorf("API error 403 (Forbidden): The API token does not have sufficient permissions. Note: In Proxmox GUI, permissions must be set on the USER (not just the token). Please verify the user '%s@%s' has the required permissions", auth.user, auth.realm) } else if resp.StatusCode == 595 { // 595 can mean authentication failed OR trying to access an offline node in a cluster // Check if this is a node-specific endpoint