fix(automation): maintainer pre-merge review — stale detection, daemon edge cases, parity (F1-F9)

F1: cap the image-status cache TTL at 5m (was 24h) — the cache is keyed by the
    LOCAL imageID, which doesn't change when upstream pushes a new image under the
    same tag, so the 24h TTL hid new images from both the badge and the auto-update
    daemon; a short TTL re-resolves the remote digest within the poll window.
F2: document that the update->rollback guard map is in-memory (restart implication).
F3: skip auto-update for an unnamed container when rollback is on (the endpoint+name
    keyed guard can't record it, so it would loop) — pure skipUnnamedForRollback + test.
F4: wrap the pre-update ContainerInspect in context.WithTimeout(endpointTimeout).
F5: document Reload() does not interrupt an in-flight tick.
F6: floor auto-heal CheckInterval at 1s (mirrors auto-update) + test.
F7: wontfix — migration is currently correct; namespace rework is out of scope.
F8: correct the misleading SSRF/AllowList comment (no filter is applied).
F9: front auto-heal interval floor + test; dedup STALE_TIME; fix invalidation comment.
Also refresh three stale '24h/long-lived cache' comments to match the 5m TTL.

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

api/containerautomation/autoupdate.go

@@ -237,6 +237,19 @@ func (s *Service) stackLookupForEndpoint(endpointID portainer.EndpointID) func(p
 func (s *Service) updateStandalone(cli *dockerclient.Client, endpoint *portainer.Endpoint, c UpdateCandidate, opts updateOptions) {
 	endpointID := int(endpoint.ID)
 
+	// Loop-guard safety: the rolled-back map is keyed by endpoint+name (the only
+	// identifier that survives a recreate). An unnamed container cannot be recorded
+	// (recordRolledBack skips it), so with rollback enabled a container that keeps
+	// failing its health gate would update->rollback every tick with NO suppression.
+	// Skip the unnamed case when rollback is on so it cannot enter that
+	// unsuppressable loop; detection/badge refresh already happened upstream and is
+	// unaffected. (With rollback off there is no rollback to loop, so we proceed.)
+	if skipUnnamedForRollback(opts.rollback, c.Name) {
+		log.Info().Str("container_id", c.ID).Int("endpoint_id", endpointID).
+			Msg("auto-update: skipping unnamed standalone container, rollback is enabled but there is no stable name to key the loop guard")
+		return
+	}
+
 	// Update->rollback loop guard: if this container's update was rolled back
 	// recently and the remote still points at the SAME failed image, skip it until
 	// the cooldown elapses. A genuinely new upstream image (a changed remote digest)
@@ -260,7 +273,12 @@ func (s *Service) updateStandalone(cli *dockerclient.Client, endpoint *portainer
 	var startPeriod time.Duration
 	healthGated := false
 	if opts.rollback {
-		if inspect, err := cli.ContainerInspect(s.baseCtx, c.ID); err != nil {
+		// Bound the inspect like every other engine call so a hung/unreachable engine
+		// cannot block the whole sequential tick until shutdown.
+		inspectCtx, inspectCancel := context.WithTimeout(s.baseCtx, endpointTimeout)
+		inspect, err := cli.ContainerInspect(inspectCtx, c.ID)
+		inspectCancel()
+		if err != nil {
 			log.Warn().Err(err).Str("container_id", c.ID).Int("endpoint_id", endpointID).
 				Msg("auto-update: unable to inspect container before update, proceeding without a health gate")
 		} else {
@@ -355,6 +373,17 @@ func containerName(names []string) string {
 	return strings.TrimPrefix(names[0], "/")
 }
 
+// skipUnnamedForRollback reports whether a standalone update must be skipped
+// because rollback is enabled but the container has no stable name to key the
+// loop guard. The rolled-back map is keyed by endpoint+name (the only identifier
+// that survives a recreate); without a name the guard cannot record a failed
+// target, so a repeatedly-failing update would loop update->rollback every tick
+// with no suppression. When rollback is off there is nothing to loop, so an
+// unnamed container is still allowed to update.
+func skipUnnamedForRollback(rollback bool, name string) bool {
+	return rollback && name == ""
+}
+
 // rollbackKey identifies a standalone container in the rolled-back map by its
 // endpoint and (recreate-stable) name. A recreate assigns a new container ID, so
 // the ID cannot key state across an update; the name is preserved.

api/containerautomation/rollback_test.go

@@ -183,6 +183,28 @@ func TestIsTagReference(t *testing.T) {
 	}
 }
 
+func TestSkipUnnamedForRollback(t *testing.T) {
+	tests := []struct {
+		name     string
+		rollback bool
+		cName    string
+		want     bool
+	}{
+		{name: "rollback on, unnamed -> skip (unsuppressable loop otherwise)", rollback: true, cName: "", want: true},
+		{name: "rollback on, named -> proceed (guard can key it)", rollback: true, cName: "web", want: false},
+		{name: "rollback off, unnamed -> proceed (no rollback to loop)", rollback: false, cName: "", want: false},
+		{name: "rollback off, named -> proceed", rollback: false, cName: "web", want: false},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := skipUnnamedForRollback(tt.rollback, tt.cName); got != tt.want {
+				t.Errorf("skipUnnamedForRollback(%v, %q) = %v, want %v", tt.rollback, tt.cName, got, tt.want)
+			}
+		})
+	}
+}
+
 func TestHasHealthGate(t *testing.T) {
 	tests := []struct {
 		name string

api/containerautomation/service.go

@@ -27,7 +27,8 @@ const (
 	// defaultCheckInterval is used when the configured auto-heal interval is empty or unparseable.
 	defaultCheckInterval = 30 * time.Second
 	// defaultPollInterval is used when the configured auto-update interval is empty or unparseable.
-	// It is conservative (hours) to stay within registry rate limits and rely on the 24h status cache.
+	// It is conservative (hours) to stay within registry rate limits; the image-status cache is
+	// short-lived (keyed by the local imageID), so each poll re-checks the remote digest.
 	defaultPollInterval = 6 * time.Hour
 )
 
@@ -70,6 +71,13 @@ type Service struct {
 	// by endpoint+name, so the auto-update job does not immediately re-pull the
 	// same failed image and roll back again on the next tick (the update->rollback
 	// loop guard, mirroring the auto-heal retries map).
+	//
+	// This state is in-memory only and is NOT persisted: after a Portainer restart
+	// the map is empty, so at most one extra update->rollback cycle per restart is
+	// possible before the guard re-records the failed target. Persisting it would
+	// require a datastore schema (key + digest + timestamp) and is intentionally out
+	// of scope here; the cooldown-bounded single extra cycle is an acceptable
+	// trade-off against that complexity.
 	rolledBack map[string]rolledBackTarget
 }
 
@@ -125,6 +133,13 @@ func (s *Service) Start() {
 // Reload re-applies the current settings: it stops the running jobs and starts
 // fresh ones with the new intervals, or leaves them stopped if disabled. It is
 // safe to call after a settings update.
+//
+// Note: stopping a job unschedules future ticks but does not interrupt a tick
+// already in progress. An in-flight heal/update pass runs to completion on its
+// original (pre-reload) context and is only cancelled by a server shutdown (via
+// baseCtx); the new interval takes effect from the next scheduled tick. The
+// overlap guards (running/updateRunning) and the per-map mutexes keep this safe
+// against data races, so this is a deliberate behavioural nuance, not a bug.
 func (s *Service) Reload() error {
 	s.mu.Lock()
 	defer s.mu.Unlock()

api/docker/images/status.go

@@ -30,12 +30,22 @@ const (
 )
 
 const (
+	// statusCacheTTL bounds how long a computed image status is served from the
+	// statusCache. It is intentionally short (tied to the auto-update poll window),
+	// NOT the previous 24h: the cache key is the LOCAL imageID, which does not
+	// change when upstream pushes a new image under the same tag. A long TTL would
+	// therefore keep serving a stale "updated" status for up to a day, and the
+	// auto-update daemon (which resolves status through this same path) could not
+	// see a freshly-pushed image within its poll interval. A few minutes still
+	// absorbs bursts of badge lookups for the same image while re-checking the
+	// remote digest soon after an upstream push.
+	statusCacheTTL            = 5 * time.Minute
 	errorStatusCacheTTL       = 5 * time.Minute
 	maxConcurrentStatusChecks = 8
 )
 
 var (
-	statusCache       = cache.New(24*time.Hour, 24*time.Hour)
+	statusCache       = cache.New(statusCacheTTL, statusCacheTTL)
 	remoteDigestCache = cache.New(5*time.Second, 5*time.Second)
 	swarmID2NameCache = cache.New(5*time.Second, 5*time.Second)
 )
@@ -134,14 +144,16 @@ func (c *DigestClient) ContainerImageStatus(ctx context.Context, containerID str
 		return Skipped, nil
 	}
 
-	// statusCache is the 24h cache keyed by imageID. Reading it here makes the
-	// long-lived cache effective on the input side for every caller (handler,
-	// ContainersImageStatus, the M4 auto-update job): a hit skips the expensive,
-	// rate-limited remote registry digest lookup below. The container/image
-	// inspects above are local Docker calls and cheap; the registry HEAD is the
-	// part worth avoiding. Only successful statuses are ever written (the error
-	// paths return early without caching), so a hit returns the same value the
-	// full computation would have produced.
+	// statusCache is keyed by the LOCAL imageID and read here so every caller
+	// (handler, ContainersImageStatus, the auto-update job) can skip the expensive,
+	// rate-limited remote registry digest lookup below on a hit; the container/image
+	// inspects above are cheap local Docker calls, the registry HEAD is the part
+	// worth avoiding. The entry TTL is deliberately short (statusCacheTTL): because
+	// the key is the local imageID, a new upstream image pushed under the same tag
+	// leaves the key unchanged, so a long TTL would keep serving a stale "updated"
+	// status (the full computation would now return "outdated") until it expired. A
+	// short TTL re-checks the remote digest within the poll window. Both Outdated
+	// and Skipped are cached too (only the error paths return early without caching).
 	if s, err := CachedResourceImageStatus(imageID); err == nil {
 		return s, nil
 	}

api/docker/images/status_test.go

@@ -2,10 +2,31 @@ package images
 
 import (
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/require"
 )
 
+// TestStatusCacheTTLIsShort is a regression guard for the stale-detection bug: the
+// statusCache is keyed by the LOCAL imageID, which does not change when upstream
+// pushes a new image under the same tag. A long TTL (the previous 24h) would serve
+// a stale "updated" status and hide a freshly-pushed image from both the badge and
+// the auto-update daemon for up to a day. The TTL must stay tied to the poll window
+// (a few minutes), and entries set with the default expiration (0) must actually
+// expire rather than live forever.
+func TestStatusCacheTTLIsShort(t *testing.T) {
+	require.LessOrEqual(t, statusCacheTTL, 10*time.Minute, "status cache TTL must be short, not 24h")
+
+	key := "status-test-ttl-key"
+	CacheResourceImageStatus(key, Updated)
+	defer EvictImageStatus(key)
+
+	_, exp, ok := statusCache.GetWithExpiration(key)
+	require.True(t, ok)
+	require.False(t, exp.IsZero(), "status entries must expire, not live forever")
+	require.LessOrEqual(t, time.Until(exp), statusCacheTTL)
+}
+
 func TestAggregateImageStatus(t *testing.T) {
 	t.Parallel()
 

api/http/handler/docker/containers/image_status.go

@@ -62,8 +62,10 @@ func (handler *Handler) imageStatus(w http.ResponseWriter, r *http.Request) *htt
 	}
 
 	// The detection engine (zlib/CE) routes outbound registry calls through the
-	// RegistryClient, which honors the encrypted credential store and the outbound
-	// SSRF/AllowList. It caches results for 24h and skips digest-pinned/local-only images.
+	// RegistryClient, which honors the encrypted credential store. It caches results
+	// briefly and skips digest-pinned/local-only images. Note: the outbound registry
+	// HEAD (RemoteDigest -> docker.GetDigest) is NOT run through an SSRF/AllowList
+	// filter; this mirrors upstream ContainersImageStatus behaviour.
 	digestClient := images.NewClientWithRegistry(images.NewRegistryClient(handler.dataStore), handler.dockerClientFactory)
 
 	status, err := digestClient.ContainerImageStatus(r.Context(), containerID, endpoint, nodeName)

api/http/handler/settings/settings_update.go

@@ -22,9 +22,16 @@ import (
 	"golang.org/x/oauth2"
 )
 
+// minAutoHealCheckInterval is the lower bound for the auto-heal check interval.
+// A near-zero interval (e.g. 1ms) is valid as a positive duration but would hammer
+// Docker with list/inspect calls for no benefit; keep a sane floor, mirroring the
+// auto-update poll-interval validation.
+const minAutoHealCheckInterval = time.Second
+
 // minAutoUpdatePollInterval is the lower bound for the auto-update poll interval.
 // Polling more often than this hammers registries (rate limits) for no benefit:
-// the image-status cache is long-lived, so a sub-minute interval only adds load.
+// the image-status cache (~5m) bounds detection latency, so a sub-minute interval
+// only adds registry load without resolving new images any faster.
 const minAutoUpdatePollInterval = time.Minute
 
 // minAutoUpdateRollbackTimeout is the lower bound for the health-gate rollback
@@ -142,8 +149,8 @@ func (payload *settingsUpdatePayload) Validate(r *http.Request) error {
 	if payload.ContainerAutomation != nil && payload.ContainerAutomation.AutoHeal != nil {
 		autoHeal := payload.ContainerAutomation.AutoHeal
 		if autoHeal.CheckInterval != nil {
-			if d, err := time.ParseDuration(*autoHeal.CheckInterval); err != nil || d <= 0 {
-				return errors.New("Invalid auto-heal check interval. Must be a positive duration (e.g. 30s)")
+			if d, err := time.ParseDuration(*autoHeal.CheckInterval); err != nil || d < minAutoHealCheckInterval {
+				return errors.New("Invalid auto-heal check interval. Must be a duration of at least 1s (e.g. 30s)")
 			}
 		}
 

api/http/handler/settings/settings_update_test.go

@@ -46,6 +46,46 @@ func TestSettingsUpdatePayloadValidateAutoUpdatePollInterval(t *testing.T) {
 	}
 }
 
+// TestSettingsUpdatePayloadValidateAutoHealCheckInterval covers the auto-heal
+// check-interval floor (F6): durations below minAutoHealCheckInterval (1s), as
+// well as malformed or non-positive durations, must be rejected, mirroring the
+// auto-update poll-interval validation.
+func TestSettingsUpdatePayloadValidateAutoHealCheckInterval(t *testing.T) {
+	cases := []struct {
+		name     string
+		interval string
+		wantErr  bool
+	}{
+		{name: "one millisecond is below the floor", interval: "1ms", wantErr: true},
+		{name: "half a second is below the floor", interval: "500ms", wantErr: true},
+		{name: "exactly one second is allowed", interval: "1s", wantErr: false},
+		{name: "thirty seconds is allowed", interval: "30s", wantErr: false},
+		{name: "zero is rejected", interval: "0s", wantErr: true},
+		{name: "negative is rejected", interval: "-5s", wantErr: true},
+		{name: "unparseable is rejected", interval: "soon", wantErr: true},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			payload := settingsUpdatePayload{
+				ContainerAutomation: &containerAutomationSettingsPayload{
+					AutoHeal: &autoHealSettingsPayload{
+						CheckInterval: strptr(tc.interval),
+					},
+				},
+			}
+
+			err := payload.Validate(httptest.NewRequest("PUT", "/settings", nil))
+			if tc.wantErr && err == nil {
+				t.Errorf("Validate(%q) = nil, want error", tc.interval)
+			}
+			if !tc.wantErr && err != nil {
+				t.Errorf("Validate(%q) = %v, want nil", tc.interval, err)
+			}
+		})
+	}
+}
+
 // TestSettingsUpdatePayloadValidateRollbackTimeout covers the M5 health-gated
 // rollback timeout and its floor (F7): it must be a Go duration of at least
 // minAutoUpdateRollbackTimeout (10s), rejecting near-zero, non-positive and

app/react/docker/containers/ListView/ContainersDatatable/columns/updateAvailable.tsx

@@ -19,7 +19,7 @@ function UpdateAvailableCell({
 }: CellContext<ContainerListViewModel, unknown>) {
   const environmentId = useEnvironmentId();
   // One detection request per visible row is acceptable: the backend caches results
-  // for 24h and the hook keeps a generous client-side staleTime, so re-renders and
+  // for ~5m and the hook keeps a generous client-side staleTime, so re-renders and
   // pagination don't re-hit the registry. The query is non-blocking, so the table
   // renders immediately and badges fill in as statuses resolve.
   const statusQuery = useContainerImageStatus(

app/react/docker/containers/queries/useContainerImageStatus.ts

@@ -30,9 +30,11 @@ export interface ContainerImageStatus {
   Message?: string;
 }
 
-// The backend caches detection results for 24h, so a generous client-side staleTime
-// is enough and avoids hammering the endpoint when many rows are visible at once.
-const STALE_TIME = 5 * 60 * 1000; // 5 minutes
+// Client-side staleTime for image-status badges: long enough to avoid hammering
+// the endpoint when many rows are visible at once, short enough that a freshly
+// pushed upstream image surfaces reasonably soon. Exported as the single source of
+// truth so the bulk-update action reuses the same window instead of redefining it.
+export const STALE_TIME = 5 * 60 * 1000; // 5 minutes
 
 export async function getContainerImageStatus(
   environmentId: EnvironmentId,

app/react/docker/containers/update/useBulkUpdateContainerImages.ts

@@ -7,7 +7,10 @@ import {
   notifyWarning,
 } from '@/portainer/services/notifications';
 
-import { getContainerImageStatus } from '../queries/useContainerImageStatus';
+import {
+  getContainerImageStatus,
+  STALE_TIME,
+} from '../queries/useContainerImageStatus';
 import { queryKeys as containerQueryKeys } from '../queries/query-keys';
 
 import { applyContainerUpdate } from './applyContainerUpdate';
@@ -15,10 +18,6 @@ import { groupContainersForUpdate } from './groupContainersForUpdate';
 import { invalidateContainerUpdateQueries } from './useUpdateContainerImage';
 import { ContainerUpdateContext } from './types';
 
-// Mirror useContainerImageStatus's client-side staleTime so the bulk action
-// reuses cached badge statuses instead of re-hitting the registry per row.
-const STATUS_STALE_TIME = 5 * 60 * 1000;
-
 interface BulkUpdateParams {
   contexts: ContainerUpdateContext[];
   stacks: Stack[];
@@ -66,7 +65,7 @@ async function bulkUpdate(
               context.id,
               context.nodeName
             ),
-          { staleTime: STATUS_STALE_TIME }
+          { staleTime: STALE_TIME }
         )
         .then((status) => status.Status)
         .catch(() => 'error' as const)

app/react/docker/containers/update/useUpdateContainerImage.ts

@@ -13,6 +13,13 @@ import { ContainerUpdateContext } from './types';
  * Refresh the data affected by a container image update: the container itself,
  * its image-status badge (so it flips away from "outdated") and the stacks
  * list (a stack redeploy bumps its deployment info).
+ *
+ * Note: for a stack redeploy this invalidates only the representative container's
+ * badge, not those of its siblings in the same stack — a stack redeploy updates
+ * every container, but only `context` is passed here. The sibling badges refresh
+ * on their next natural refetch (staleTime / window focus) or a manual reload.
+ * They are deliberately not force-invalidated from this shared helper (also used
+ * by the single standalone "Update now") to avoid an endpoint-wide badge refetch.
  */
 export function invalidateContainerUpdateQueries(
   queryClient: QueryClient,

app/react/portainer/settings/SettingsView/AutoHealPanel/validation.test.ts

@@ -0,0 +1,47 @@
+import { validation } from './validation';
+import { Values } from './types';
+
+function validate(values: Values) {
+  return validation()
+    .validate(values, { abortEarly: false })
+    .then(() => undefined)
+    .catch((err: { errors: string[] }) => err.errors);
+}
+
+const base: Values = {
+  enabled: true,
+  checkInterval: '30s',
+  scope: 'labeled',
+};
+
+describe('AutoHealPanel validation', () => {
+  it('accepts a check interval at or above the 1s floor', async () => {
+    expect(await validate({ ...base, checkInterval: '1s' })).toBeUndefined();
+    expect(await validate({ ...base, checkInterval: '30s' })).toBeUndefined();
+    expect(await validate({ ...base, checkInterval: '2m' })).toBeUndefined();
+  });
+
+  it('rejects a check interval below the 1s floor (backend 400s on these)', async () => {
+    expect(await validate({ ...base, checkInterval: '1ms' })).toContain(
+      'Check interval must be at least 1s'
+    );
+    expect(await validate({ ...base, checkInterval: '500ms' })).toContain(
+      'Check interval must be at least 1s'
+    );
+    expect(await validate({ ...base, checkInterval: '0s' })).toContain(
+      'Check interval must be at least 1s'
+    );
+  });
+
+  it('rejects a malformed duration', async () => {
+    expect(await validate({ ...base, checkInterval: 'soon' })).toContain(
+      'Must be a valid duration (e.g. 30s, 1m, 2h)'
+    );
+  });
+
+  it('rejects an empty check interval', async () => {
+    expect(await validate({ ...base, checkInterval: '' })).toContain(
+      'Check interval is required'
+    );
+  });
+});

app/react/portainer/settings/SettingsView/AutoHealPanel/validation.ts

@@ -7,14 +7,63 @@ import { Values } from './types';
 // Matches Go's time.ParseDuration units (e.g. "30s", "1m30s", "2h").
 const durationPattern = /^(\d+(\.\d+)?(ns|us|µs|ms|s|m|h))+$/;
 
+// Lower bound for the check interval, kept in sync with the backend
+// (minAutoHealCheckInterval). A near-zero interval would hammer Docker with
+// list/inspect calls for no benefit, and the backend rejects it (400), so the
+// front-end must reject it too rather than letting a sub-second value through.
+const minCheckIntervalSeconds = 1;
+
+// Seconds per Go duration unit, used to evaluate the configured interval against
+// the floor. Mirrors the units accepted by durationPattern.
+const unitSeconds: Record<string, number> = {
+  ns: 1e-9,
+  us: 1e-6,
+  µs: 1e-6,
+  ms: 1e-3,
+  s: 1,
+  m: 60,
+  h: 3600,
+};
+
+// parseGoDurationSeconds converts a Go-style duration (e.g. "1h30m") to seconds,
+// or returns null when the string is not a well-formed duration.
+function parseGoDurationSeconds(value: string): number | null {
+  if (!durationPattern.test(value)) {
+    return null;
+  }
+
+  let total = 0;
+  const componentPattern = /(\d+(?:\.\d+)?)(ns|us|µs|ms|s|m|h)/g;
+  let match = componentPattern.exec(value);
+  while (match !== null) {
+    total += parseFloat(match[1]) * unitSeconds[match[2]];
+    match = componentPattern.exec(value);
+  }
+
+  return total;
+}
+
 export function validation(): SchemaOf<Values> {
   return object({
     enabled: boolean().default(false),
     checkInterval: string()
       .required('Check interval is required')
-      .matches(
-        durationPattern,
-        'Must be a valid duration (e.g. 30s, 1m, 2h)'
+      .matches(durationPattern, 'Must be a valid duration (e.g. 30s, 1m, 2h)')
+      .test(
+        'min-check-interval',
+        'Check interval must be at least 1s',
+        (value) => {
+          if (!value) {
+            return true; // let required/matches report the error first
+          }
+
+          const seconds = parseGoDurationSeconds(value);
+          if (seconds === null) {
+            return true; // let matches report the format error first
+          }
+
+          return seconds >= minCheckIntervalSeconds;
+        }
       ),
     scope: mixed<AutoHealScope>()
       .oneOf(['labeled', 'all'])

app/react/portainer/settings/SettingsView/AutoUpdatePanel/validation.ts

@@ -9,7 +9,8 @@ const durationPattern = /^(\d+(\.\d+)?(ns|us|µs|ms|s|m|h))+$/;
 
 // Lower bound for the poll interval, kept in sync with the backend
 // (minAutoUpdatePollInterval). Polling more often than this only adds registry
-// load: the image-status cache is long-lived, so a sub-minute interval is wasteful.
+// load: the image-status cache (~5m) bounds detection latency, so a sub-minute
+// interval is wasteful.
 const minPollIntervalSeconds = 60;
 
 // Lower bound for the rollback timeout, kept in sync with the backend