From e8afd336a3afe33d22f6ac81783370e81ca3f064 Mon Sep 17 00:00:00 2001 From: scttbnsn <80784472+scttbnsn@users.noreply.github.com> Date: Tue, 4 Aug 2026 14:56:18 -0400 Subject: [PATCH 1/2] =?UTF-8?q?=F0=9F=90=9B=20fix(maturity):=20surface=20s?= =?UTF-8?q?wallowed=20auth=20errors=20and=20honor=20per-container=20thresh?= =?UTF-8?q?old?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Bump getImagePublishedAt failure logging from debug to warn in image-comparison.ts (GHCR/LSCR auth failures were invisible, silently degrading the maturity clock to updateDetectedAt) - getRawUpdateMaturityLevel (container.ts) and getContainerMaturityLevel (maturity-filter.ts) now resolve updatePolicy.maturityMinAgeDays per-container before falling back to the global DD_UI_MATURITY_THRESHOLD_DAYS, matching the actual gate logic in isUpdateSuppressed/isMaturityGatePending Fixes: #604 --- CHANGELOG.md | 1 + app/api/container/maturity-filter.test.ts | 26 ++++++++++ app/api/container/maturity-filter.ts | 16 +++--- app/model/container.test.ts | 51 +++++++++++++++++++ app/model/container.ts | 5 +- ....containers.labels-version-finding.test.ts | 2 +- .../docker/Docker.containers.test.ts | 2 +- .../providers/docker/image-comparison.test.ts | 43 +++++++++++++--- .../providers/docker/image-comparison.ts | 6 +-- 9 files changed, 130 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8cdfe10e4..f3cf36899 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - **WebSocket log streams no longer reject anonymous-auth sessions** ([#636](https://github.com/CodesWhat/drydock/issues/636)). Both WS upgrade paths — the system log stream and the container log stream — gated on `isAuthenticatedSession()` requiring `session.passport.user`, which `passport-anonymous` never sets, so under `DD_ANONYMOUS_AUTH_CONFIRM=true` the log stream WebSocket always rejected the upgrade even though every REST endpoint worked. `isAuthenticatedSession` now also accepts the session when anonymous authentication is the registered mode. +- **Maturity clock: swallowed auth errors surfaced, per-container threshold respected** ([#604](https://github.com/CodesWhat/drydock/issues/604)). `getImagePublishedAt` failures — including GHCR/LSCR 401/403 auth errors — now log at `warn` instead of `debug`, so the maturity gate's silent fallback from the registry `publishedAt` to `updateDetectedAt` is no longer invisible. `getRawUpdateMaturityLevel` (`app/model/container.ts`) and `getContainerMaturityLevel` (`app/api/container/maturity-filter.ts`) now resolve each container's own `updatePolicy.maturityMinAgeDays` before falling back to the global `DD_UI_MATURITY_THRESHOLD_DAYS`, matching the gate's own `isUpdateSuppressed`/`isMaturityGatePending` logic so the hot/mature badge can no longer disagree with the gate in the same API response. - **Container start/stop/restart/rollback return an explicit 501 instead of an ambiguous 404 for agent containers without lifecycle transport** ([#637](https://github.com/CodesWhat/drydock/issues/637)). `POST /:id/start|stop|restart` and `POST /:id/rollback` returned a bare 404 `No docker trigger found for this container` whenever the lookup missed, indistinguishable from "container not found" — for agent-owned containers this was the only signal the UI got. That lookup miss now returns 501 naming the likely cause (the agent's connection typically hasn't advertised `usesControllerDockerTransport`) when `container.agent` is set; non-agent containers still get the existing 404. This complements the native-transport support that shipped in rc.11 via [#651](https://github.com/CodesWhat/drydock/pull/651), which closed #637's core gap — this is the remaining explicit-error half. ### Security diff --git a/app/api/container/maturity-filter.test.ts b/app/api/container/maturity-filter.test.ts index cb2d9bbb0..cd0700da6 100644 --- a/app/api/container/maturity-filter.test.ts +++ b/app/api/container/maturity-filter.test.ts @@ -33,4 +33,30 @@ describe('api/container/maturity-filter', () => { expect(applyContainerMaturityFilter(containers, undefined)).toBe(containers); }); + + test('applyContainerMaturityFilter uses per-container updatePolicy.maturityMinAgeDays over the global threshold on the uncached fallback path', () => { + const tenDaysMs = 10 * 24 * 60 * 60 * 1000; + const containers = [ + { + id: 'c1', + updateAvailable: true, + updateAge: tenDaysMs, + updatePolicy: { maturityMinAgeDays: 30 }, + } as unknown as Container, + { + id: 'c2', + updateAvailable: true, + updateAge: tenDaysMs, + } as unknown as Container, + ]; + + // Default global threshold is 7 days: c2 has no override so it's 'mature'. + // c1 overrides to 30 days, so the same 10-day-old update is still 'hot'. + expect( + applyContainerMaturityFilter(containers, 'hot').map((container) => container.id), + ).toEqual(['c1']); + expect( + applyContainerMaturityFilter(containers, 'mature').map((container) => container.id), + ).toEqual(['c2']); + }); }); diff --git a/app/api/container/maturity-filter.ts b/app/api/container/maturity-filter.ts index 59bdb3103..4cb9ceffc 100644 --- a/app/api/container/maturity-filter.ts +++ b/app/api/container/maturity-filter.ts @@ -28,13 +28,9 @@ function resolveUiMaturityThresholdDays(): number { ); } -function resolveUiMaturityThresholdMs(): number { - return maturityMinAgeDaysToMilliseconds(resolveUiMaturityThresholdDays()); -} - function getContainerMaturityLevel( container: Container, - uiMaturityThresholdMs: number, + uiMaturityThresholdDays: number, ): ContainerMaturityFilter | undefined { const cachedLevel = container.updateMaturityLevel; if (cachedLevel === 'hot' || cachedLevel === 'mature' || cachedLevel === 'established') { @@ -48,7 +44,11 @@ function getContainerMaturityLevel( if (updateAge >= maturityMinAgeDaysToMilliseconds(ESTABLISHED_UPDATE_AGE_DAYS)) { return 'established'; } - return updateAge >= uiMaturityThresholdMs ? 'mature' : 'hot'; + const maturityThresholdDays = resolveMaturityMinAgeDays( + container.updatePolicy?.maturityMinAgeDays, + uiMaturityThresholdDays, + ); + return updateAge >= maturityMinAgeDaysToMilliseconds(maturityThresholdDays) ? 'mature' : 'hot'; } export function applyContainerMaturityFilter( @@ -59,8 +59,8 @@ export function applyContainerMaturityFilter( return containers; } - const uiMaturityThresholdMs = resolveUiMaturityThresholdMs(); + const uiMaturityThresholdDays = resolveUiMaturityThresholdDays(); return containers.filter( - (container) => getContainerMaturityLevel(container, uiMaturityThresholdMs) === maturityFilter, + (container) => getContainerMaturityLevel(container, uiMaturityThresholdDays) === maturityFilter, ); } diff --git a/app/model/container.test.ts b/app/model/container.test.ts index 40710d04a..9c7417a66 100644 --- a/app/model/container.test.ts +++ b/app/model/container.test.ts @@ -1878,6 +1878,57 @@ test('model should use DD_UI_MATURITY_THRESHOLD_DAYS for hot/mature cutoff', asy } }); +test('model should use per-container updatePolicy.maturityMinAgeDays over DD_UI_MATURITY_THRESHOLD_DAYS for hot/mature cutoff', async () => { + const previousThreshold = process.env.DD_UI_MATURITY_THRESHOLD_DAYS; + vi.useFakeTimers(); + try { + process.env.DD_UI_MATURITY_THRESHOLD_DAYS = '3'; + const now = new Date('2026-03-15T12:00:00.000Z'); + vi.setSystemTime(now); + const firstSeenAt = new Date(now.getTime() - daysToMs(10)).toISOString(); + + const containerValidated = container.validate({ + id: 'container-123456789', + name: 'test', + watcher: 'test', + firstSeenAt, + updatePolicy: { maturityMinAgeDays: 30 }, + image: { + id: 'image-123456789', + registry: { + name: 'hub', + url: 'https://hub', + }, + name: 'organization/image', + tag: { + value: '1.0.0', + semver: true, + }, + digest: { + watch: false, + repo: undefined, + }, + architecture: 'arch', + os: 'os', + created: '2021-06-12T05:33:38.440Z', + }, + result: { + tag: '1.0.1', + }, + }); + + expect(containerValidated.updateAge).toBe(daysToMs(10)); + expect(containerValidated.updateMaturityLevel).toBe('hot'); + } finally { + vi.useRealTimers(); + if (previousThreshold === undefined) { + delete process.env.DD_UI_MATURITY_THRESHOLD_DAYS; + } else { + process.env.DD_UI_MATURITY_THRESHOLD_DAYS = previousThreshold; + } + } +}); + test('model should keep updateAvailable when remote tag changes past skipped value', async () => { const containerValidated = container.validate({ id: 'container-123456789', diff --git a/app/model/container.ts b/app/model/container.ts index cfb6e3ed4..0c8586371 100644 --- a/app/model/container.ts +++ b/app/model/container.ts @@ -749,7 +749,10 @@ function getRawUpdateMaturityLevel( return 'established'; } - const maturityThresholdDays = resolveUiMaturityThresholdDays(); + const maturityThresholdDays = resolveMaturityMinAgeDays( + container.updatePolicy?.maturityMinAgeDays, + resolveUiMaturityThresholdDays(), + ); const maturityThresholdMs = maturityMinAgeDaysToMilliseconds(maturityThresholdDays); return updateAge >= maturityThresholdMs ? 'mature' : 'hot'; } diff --git a/app/watchers/providers/docker/Docker.containers.labels-version-finding.test.ts b/app/watchers/providers/docker/Docker.containers.labels-version-finding.test.ts index 01a631b76..f1533dc92 100644 --- a/app/watchers/providers/docker/Docker.containers.labels-version-finding.test.ts +++ b/app/watchers/providers/docker/Docker.containers.labels-version-finding.test.ts @@ -257,7 +257,7 @@ describe('Docker Watcher', () => { const result = await docker.findNewVersion(container, mockLogChild); expect(result).toEqual({ tag: '1.0.0' }); - expect(mockLogChild.debug).toHaveBeenCalledWith( + expect(mockLogChild.warn).toHaveBeenCalledWith( expect.stringContaining('publish date lookup failed'), ); }); diff --git a/app/watchers/providers/docker/Docker.containers.test.ts b/app/watchers/providers/docker/Docker.containers.test.ts index 525f65836..1f83839b9 100644 --- a/app/watchers/providers/docker/Docker.containers.test.ts +++ b/app/watchers/providers/docker/Docker.containers.test.ts @@ -1085,7 +1085,7 @@ describe('Docker Watcher', () => { const result = await docker.findNewVersion(container, mockLogChild); expect(result).toEqual({ tag: '1.0.0' }); - expect(mockLogChild.debug).toHaveBeenCalledWith( + expect(mockLogChild.warn).toHaveBeenCalledWith( expect.stringContaining('publish date lookup failed'), ); }); diff --git a/app/watchers/providers/docker/image-comparison.test.ts b/app/watchers/providers/docker/image-comparison.test.ts index 429ccf6bb..f13fb290d 100644 --- a/app/watchers/providers/docker/image-comparison.test.ts +++ b/app/watchers/providers/docker/image-comparison.test.ts @@ -419,7 +419,7 @@ describe('image-comparison', () => { expect(result.publishedAt).toBeUndefined(); expect(result.publishedAtTrusted).toBeUndefined(); expect(result.digest).toBe('sha256:def456'); - expect(log.debug).toHaveBeenCalledWith(expect.stringContaining('registry timeout')); + expect(log.warn).toHaveBeenCalledWith(expect.stringContaining('registry timeout')); }); test('digest-only with comparisonTag does not set publishedAt when getImagePublishedAt returns non-string', async () => { @@ -808,7 +808,7 @@ describe('image-comparison', () => { expect(result.publishedAtTrusted).toBeUndefined(); }); - test('logs debug and continues when getImagePublishedAt throws', async () => { + test('logs warn and continues when getImagePublishedAt throws', async () => { mockGetState.mockReturnValue({ registry: { hub: { @@ -819,8 +819,8 @@ describe('image-comparison', () => { }, }, }); - const debugFn = vi.fn(); - const log = { error: vi.fn(), warn: vi.fn(), debug: debugFn }; + const warnFn = vi.fn(); + const log = { error: vi.fn(), warn: warnFn, debug: vi.fn() }; const container = { image: { id: 'image-1', @@ -832,10 +832,37 @@ describe('image-comparison', () => { }; const result = await findNewVersion(container as never, log); expect(result.publishedAt).toBeUndefined(); - expect(debugFn).toHaveBeenCalledWith(expect.stringContaining('API error')); + expect(warnFn).toHaveBeenCalledWith(expect.stringContaining('API error')); }); - test('continues silently when getImagePublishedAt throws and logContainer.debug is absent', async () => { + test('logs warn on GHCR/LSCR auth failure so it is not silently swallowed', async () => { + mockGetState.mockReturnValue({ + registry: { + hub: { + getTags: vi.fn().mockResolvedValue(['1.1.0']), + getImageManifestDigest: createManifestLookup(), + normalizeImage: identityNormalizeImage, + getImagePublishedAt: vi.fn().mockRejectedValue(new Error('401 Unauthorized')), + }, + }, + }); + const warnFn = vi.fn(); + const log = { error: vi.fn(), warn: warnFn, debug: vi.fn() }; + const container = { + image: { + id: 'image-1', + registry: { name: 'hub' }, + name: 'library/nginx', + tag: { value: '1.0.0', semver: false }, + digest: { watch: false }, + }, + }; + const result = await findNewVersion(container as never, log); + expect(result.publishedAt).toBeUndefined(); + expect(warnFn).toHaveBeenCalledWith(expect.stringContaining('401 Unauthorized')); + }); + + test('continues silently when getImagePublishedAt throws and logContainer.warn is absent', async () => { mockGetState.mockReturnValue({ registry: { hub: { @@ -846,8 +873,8 @@ describe('image-comparison', () => { }, }, }); - // Intentionally omit debug from log to exercise the false branch of typeof logContainer.debug - const log = { error: vi.fn(), warn: vi.fn() } as unknown as Parameters< + // Intentionally omit warn from log to exercise the false branch of typeof logContainer.warn + const log = { error: vi.fn(), debug: vi.fn() } as unknown as Parameters< typeof findNewVersion >[1]; const container = { diff --git a/app/watchers/providers/docker/image-comparison.ts b/app/watchers/providers/docker/image-comparison.ts index 4d5131431..02b4af95c 100644 --- a/app/watchers/providers/docker/image-comparison.ts +++ b/app/watchers/providers/docker/image-comparison.ts @@ -339,7 +339,7 @@ export async function findNewVersion( } } } catch (error: unknown) { - logContainer.debug(`Remote publish date lookup failed (${getErrorMessage(error)})`); + logContainer.warn(`Remote publish date lookup failed (${getErrorMessage(error)})`); } } else { logContainer.debug('Digest-only image — no registry tag candidate available'); @@ -423,8 +423,8 @@ export async function findNewVersion( } } } catch (error: unknown) { - if (typeof logContainer.debug === 'function') { - logContainer.debug(`Remote publish date lookup failed (${getErrorMessage(error)})`); + if (typeof logContainer.warn === 'function') { + logContainer.warn(`Remote publish date lookup failed (${getErrorMessage(error)})`); } } From f0c853ad6947ee5c492806bce58e15284f8f639f Mon Sep 17 00:00:00 2001 From: scttbnsn <80784472+scttbnsn@users.noreply.github.com> Date: Tue, 4 Aug 2026 17:38:25 -0400 Subject: [PATCH 2/2] =?UTF-8?q?=E2=9C=85=20test(maturity):=20address=20rev?= =?UTF-8?q?iew=20findings=20=E2=80=94=20env=20pinning,=20provider=20fixtur?= =?UTF-8?q?es,=20digest-path=20warn=20guard?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app/api/container/maturity-filter.test.ts | 55 +++++++++++-------- .../providers/docker/image-comparison.test.ts | 23 +++++++- .../providers/docker/image-comparison.ts | 4 +- 3 files changed, 58 insertions(+), 24 deletions(-) diff --git a/app/api/container/maturity-filter.test.ts b/app/api/container/maturity-filter.test.ts index cd0700da6..115e77dda 100644 --- a/app/api/container/maturity-filter.test.ts +++ b/app/api/container/maturity-filter.test.ts @@ -35,28 +35,39 @@ describe('api/container/maturity-filter', () => { }); test('applyContainerMaturityFilter uses per-container updatePolicy.maturityMinAgeDays over the global threshold on the uncached fallback path', () => { - const tenDaysMs = 10 * 24 * 60 * 60 * 1000; - const containers = [ - { - id: 'c1', - updateAvailable: true, - updateAge: tenDaysMs, - updatePolicy: { maturityMinAgeDays: 30 }, - } as unknown as Container, - { - id: 'c2', - updateAvailable: true, - updateAge: tenDaysMs, - } as unknown as Container, - ]; + const previousThreshold = process.env.DD_UI_MATURITY_THRESHOLD_DAYS; + process.env.DD_UI_MATURITY_THRESHOLD_DAYS = '7'; + + try { + const tenDaysMs = 10 * 24 * 60 * 60 * 1000; + const containers = [ + { + id: 'c1', + updateAvailable: true, + updateAge: tenDaysMs, + updatePolicy: { maturityMinAgeDays: 30 }, + } as unknown as Container, + { + id: 'c2', + updateAvailable: true, + updateAge: tenDaysMs, + } as unknown as Container, + ]; - // Default global threshold is 7 days: c2 has no override so it's 'mature'. - // c1 overrides to 30 days, so the same 10-day-old update is still 'hot'. - expect( - applyContainerMaturityFilter(containers, 'hot').map((container) => container.id), - ).toEqual(['c1']); - expect( - applyContainerMaturityFilter(containers, 'mature').map((container) => container.id), - ).toEqual(['c2']); + // Global threshold is pinned to 7 days: c2 has no override so it's 'mature'. + // c1 overrides to 30 days, so the same 10-day-old update is still 'hot'. + expect( + applyContainerMaturityFilter(containers, 'hot').map((container) => container.id), + ).toEqual(['c1']); + expect( + applyContainerMaturityFilter(containers, 'mature').map((container) => container.id), + ).toEqual(['c2']); + } finally { + if (previousThreshold === undefined) { + delete process.env.DD_UI_MATURITY_THRESHOLD_DAYS; + } else { + process.env.DD_UI_MATURITY_THRESHOLD_DAYS = previousThreshold; + } + } }); }); diff --git a/app/watchers/providers/docker/image-comparison.test.ts b/app/watchers/providers/docker/image-comparison.test.ts index f13fb290d..90edf9c50 100644 --- a/app/watchers/providers/docker/image-comparison.test.ts +++ b/app/watchers/providers/docker/image-comparison.test.ts @@ -422,6 +422,27 @@ describe('image-comparison', () => { expect(log.warn).toHaveBeenCalledWith(expect.stringContaining('registry timeout')); }); + test('digest-only with comparisonTag continues when getImagePublishedAt throws and logContainer.warn is absent', async () => { + mockGetState.mockReturnValue({ + registry: { + hub: { + getTags: vi.fn().mockResolvedValue(['latest']), + getImageManifestDigest: createManifestLookup(), + normalizeImage: identityNormalizeImage, + getImagePublishedAt: vi.fn().mockRejectedValue(new Error('timeout')), + }, + }, + }); + const log = { error: vi.fn(), debug: vi.fn() } as unknown as Parameters< + typeof findNewVersion + >[1]; + + const result = await findNewVersion(createDigestOnlyContainer() as never, log); + + expect(result.publishedAt).toBeUndefined(); + expect(result.digest).toBe('sha256:def456'); + }); + test('digest-only with comparisonTag does not set publishedAt when getImagePublishedAt returns non-string', async () => { mockGetState.mockReturnValue({ registry: { @@ -835,7 +856,7 @@ describe('image-comparison', () => { expect(warnFn).toHaveBeenCalledWith(expect.stringContaining('API error')); }); - test('logs warn on GHCR/LSCR auth failure so it is not silently swallowed', async () => { + test('logs warn on registry auth failure so it is not silently swallowed', async () => { mockGetState.mockReturnValue({ registry: { hub: { diff --git a/app/watchers/providers/docker/image-comparison.ts b/app/watchers/providers/docker/image-comparison.ts index 02b4af95c..7db9f23c8 100644 --- a/app/watchers/providers/docker/image-comparison.ts +++ b/app/watchers/providers/docker/image-comparison.ts @@ -339,7 +339,9 @@ export async function findNewVersion( } } } catch (error: unknown) { - logContainer.warn(`Remote publish date lookup failed (${getErrorMessage(error)})`); + if (typeof logContainer.warn === 'function') { + logContainer.warn(`Remote publish date lookup failed (${getErrorMessage(error)})`); + } } } else { logContainer.debug('Digest-only image — no registry tag candidate available');