Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Fixed

- **The maturity soak clock no longer resets when a container is recreated.** `getResultSignature` (`app/store/container.ts`) decided whether a recreated container inherits its stashed `updateDetectedAt` / `firstSeenAt` / `maturityGatePendingSince`, and it included `created` even when a digest was present. Registries don't all derive `created` from the digest, so it could differ between the pre-delete snapshot and the post-recreate rescan without the update candidate changing โ€” discarding the stash and silently restarting the soak right after an update landed, which is exactly when clock continuity matters. Same defect class as [#565](https://github.com/CodesWhat/drydock/issues/565), surviving in the recreate path that [#568](https://github.com/CodesWhat/drydock/pull/568) never touched.
- **Notification dedup no longer fires a duplicate `once: true` notification when a manual recheck bypasses the registry poll cache.** `computeResultHash` (`app/store/notification-history.ts`) hashed `suggestedTag` and `created` alongside `tag`/`digest`/`updateKind`, so a recheck that returned a drifted `created`/`suggestedTag` for the same candidate produced a different hash, made `hasAlreadyNotifiedForResult` report "not yet notified", and re-fired notifications that were supposed to be one-shot. The hash now derives from the same candidate-identity definition PR #568 established for the maturity soak (`tag` + `digest`, with `created` participating only when no digest is available), via a single shared helper (`getCandidateIdentityFields` in `app/model/container.ts`) now used by `hasCandidateIdentityChanged`, `getResultSignature`, and `computeResultHash` alike โ€” previously three independently hand-rolled comparators that disagreed with each other. Upgrading may produce one extra notification for updates already in flight, since history persisted under the old hash formula won't match the new one; this is a one-time false positive, not a regression.
- **A security scan finishing no longer reverts an update the watcher detected while it was running.** `runBulkScan` (`app/api/container/bulk-security.ts`) and `persistAndBroadcast` (`app/api/container/security.ts`) both captured a container snapshot before starting the scan, then wrote that whole snapshot back with only `security` changed. Vulnerability scans run for seconds to minutes, so a watcher poll during that window could legitimately detect a new update and have `result`, `updateAvailable`, `updateDetectedAt`, and `maturityGatePendingSince` silently rolled back by the scan's write-back. The next poll re-detected the same update and stamped a fresh timestamp, discarding the elapsed soak time. Both paths now re-fetch the live record at write-back time and merge only `security` onto it, matching what the scan scheduler already did, and skip the write entirely if the container is gone.
- **Containers with no available update are no longer ranked or filtered as if they had one.** `getContainerUpdateAge` (`app/api/container/update-age.ts`) fell back to `firstSeenAt` / `publishedAt` / `updateDetectedAt` without checking `updateAvailable`, so a container gated by `maturityMode` could still appear under `?maturity=mature`, `?maturity=hot`, and `?sort=age` while its own `updateEligibility` blockers in the same response said it was blocked.

## [1.6.0-rc.6] โ€” 2026-07-26

### Added
Expand Down
8 changes: 8 additions & 0 deletions app/api/container.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1724,6 +1724,14 @@ describe('Container Router', () => {
return res;
}

beforeEach(() => {
// Write-back re-fetches the current store record via getContainerRaw
// rather than trusting the pre-scan snapshot (see security.ts
// persistAndBroadcast). None of these tests simulate a concurrent
// watcher poll, so mirror whatever getContainer was set up to return.
storeContainer.getContainerRaw.mockImplementation(() => storeContainer.getContainer());
});

test('should return 404 when container not found', async () => {
storeContainer.getContainer.mockReturnValue(undefined);
const res = await callScanContainer('missing');
Expand Down
1 change: 1 addition & 0 deletions app/api/container.ts
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,7 @@ const bulkSecurityHandlers = createBulkSecurityHandlers({
storeContainer: {
getAllContainers: () => storeContainer.getContainers({}),
getContainer: (id) => storeContainer.getContainer(id),
getContainerRaw: (id) => storeContainer.getContainerRaw(id),
updateContainer: (c) => storeContainer.updateContainer(c),
},
getSecurityConfiguration,
Expand Down
73 changes: 73 additions & 0 deletions app/api/container/bulk-security.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ function createHarness(options: { containers?: Record<string, unknown>[] } = {})
const storeContainer = {
getAllContainers: vi.fn(() => [...containers]),
getContainer: vi.fn((id: string) => containers.find((c) => c.id === id)),
getContainerRaw: vi.fn((id: string) => containers.find((c) => c.id === id)),
updateContainer: vi.fn((c: any) => {
const idx = containers.findIndex((existing) => existing.id === c.id);
if (idx >= 0) {
Expand Down Expand Up @@ -875,6 +876,78 @@ describe('api/container/bulk-security', () => {

expect(harness.deps.updateDigestScanCache).toHaveBeenCalledWith('sha256:abc', scan, '');
});

test('does not revert a concurrent watcher update detected while the scan was in flight', async () => {
const initialContainer = createContainer({
id: 'c1',
name: 'nginx',
updateAvailable: true,
result: { tag: 'a', digest: 'sha256:AAA' },
updateDetectedAt: 'T0',
});
const harness = createHarness({ containers: [initialContainer] });

let releaseScan: ((scan: ReturnType<typeof createScanResult>) => void) | undefined;
const deferredScan = new Promise<ReturnType<typeof createScanResult>>((resolve) => {
releaseScan = resolve;
});
harness.deps.scanImageForVulnerabilities.mockReturnValue(deferredScan);

await callScanAll(harness.handlers);

// Let the background scan get past the pre-scan snapshot capture and
// into the (still-pending) scan.
await vi.waitFor(() => {
expect(harness.deps.scanImageForVulnerabilities).toHaveBeenCalled();
});

// Simulate a concurrent watcher poll that legitimately detected a
// newer update and wrote it directly into the store while the scan
// was still in flight.
const liveRecord = {
...initialContainer,
result: { tag: 'b', digest: 'sha256:BBB' },
updateDetectedAt: 'T1',
};
harness.storeContainer.getContainerRaw.mockReturnValue(liveRecord);

const scan = createScanResult();
releaseScan?.(scan);
await waitForCycleComplete(harness.deps);

expect(harness.storeContainer.updateContainer).toHaveBeenCalledTimes(1);
const persisted = harness.storeContainer.updateContainer.mock.calls[0][0];
expect(persisted.result).toEqual({ tag: 'b', digest: 'sha256:BBB' });
expect(persisted.updateDetectedAt).toBe('T1');
expect(persisted.security.scan).toEqual(scan);
});

test('skips the write when the container vanished while the scan was in flight', async () => {
const harness = createHarness({
containers: [{ id: 'c1', name: 'nginx' }],
});

let releaseScan: ((scan: ReturnType<typeof createScanResult>) => void) | undefined;
const deferredScan = new Promise<ReturnType<typeof createScanResult>>((resolve) => {
releaseScan = resolve;
});
harness.deps.scanImageForVulnerabilities.mockReturnValue(deferredScan);

await callScanAll(harness.handlers);

await vi.waitFor(() => {
expect(harness.deps.scanImageForVulnerabilities).toHaveBeenCalled();
});

// Container was removed (or recreated under a new id) mid-scan.
harness.storeContainer.getContainerRaw.mockReturnValue(undefined);

releaseScan?.(createScanResult());
await waitForCycleComplete(harness.deps);

expect(harness.storeContainer.updateContainer).not.toHaveBeenCalled();
expect(harness.deps.broadcastScanCompleted).toHaveBeenCalledWith('c1', 'passed');
});
});

describe('rate limiter wiring', () => {
Expand Down
27 changes: 20 additions & 7 deletions app/api/container/bulk-security.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ interface BulkSecurityAlertPayload {
interface BulkSecurityStoreApi {
getAllContainers: () => Container[];
getContainer: (id: string) => Container | undefined;
getContainerRaw: (id: string) => Container | undefined;
updateContainer: (container: Container) => Container;
}

Expand Down Expand Up @@ -179,13 +180,25 @@ async function runBulkScan(
scannedCount += 1;

try {
deps.storeContainer.updateContainer({
...container,
security: {
...(container.security || {}),
scan: scanResult,
},
});
// Re-fetch the current store record at write-back time rather than
// spreading the stale `container` snapshot captured when the scan
// was queued. The scan can take seconds to minutes, so by the time
// we write back, a watcher poll may have legitimately advanced
// `result`/`updateAvailable`/`updateDetectedAt`/`maturityGatePendingSince`.
// Spreading the stale snapshot would silently revert those fields
// and reset the maturity soak clock. If the record is gone
// (container removed or recreated under a new id) skip the write
// rather than resurrecting a zombie record.
const current = deps.storeContainer.getContainerRaw(containerId);
if (current) {
deps.storeContainer.updateContainer({
...current,
security: {
...(current.security || {}),
scan: scanResult,
},
});
}
} catch (persistErr: unknown) {
deps.log.info(
`Bulk scan persistence failed for container ${containerId} (${deps.getErrorMessage(persistErr)})`,
Expand Down
101 changes: 98 additions & 3 deletions app/api/container/crud.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -684,9 +684,9 @@ describe('api/container/crud', () => {
test('supports sort=age and returns oldest updates first', () => {
const harness = createHarness({
containers: [
createContainer({ id: 'c1', updateAge: 2_000 }),
createContainer({ id: 'c2', updateAge: 8_000 }),
createContainer({ id: 'c3', updateAge: 4_000 }),
createContainer({ id: 'c1', updateAvailable: true, updateAge: 2_000 }),
createContainer({ id: 'c2', updateAvailable: true, updateAge: 8_000 }),
createContainer({ id: 'c3', updateAvailable: true, updateAge: 4_000 }),
],
});

Expand Down Expand Up @@ -1328,19 +1328,23 @@ describe('api/container/crud', () => {
containers: [
createContainer({
id: 'c-min',
updateAvailable: true,
firstSeenAt: '2026-03-05T00:00:00.000Z',
result: { publishedAt: '2026-03-12T00:00:00.000Z' },
}),
createContainer({
id: 'c-first',
updateAvailable: true,
firstSeenAt: '2026-03-07T00:00:00.000Z',
}),
createContainer({
id: 'c-published',
updateAvailable: true,
result: { publishedAt: '2026-03-09T00:00:00.000Z' },
}),
createContainer({
id: 'c-detected',
updateAvailable: true,
updateDetectedAt: '2026-03-11T00:00:00.000Z',
}),
createContainer({
Expand Down Expand Up @@ -1376,14 +1380,17 @@ describe('api/container/crud', () => {
containers: [
createContainer({
id: 'c-hot',
updateAvailable: true,
firstSeenAt: '2026-03-14T00:00:00.000Z',
}),
createContainer({
id: 'c-mature',
updateAvailable: true,
result: { publishedAt: '2026-03-07T00:00:00.000Z' },
}),
createContainer({
id: 'c-established',
updateAvailable: true,
updateDetectedAt: '2026-02-01T00:00:00.000Z',
}),
],
Expand Down Expand Up @@ -1425,6 +1432,7 @@ describe('api/container/crud', () => {
}),
createContainer({
id: 'c-hot',
updateAvailable: true,
firstSeenAt: '2026-03-14T00:00:00.000Z',
}),
],
Expand All @@ -1439,6 +1447,93 @@ describe('api/container/crud', () => {
}
});

test('excludes maturity-gated containers (updateAvailable: false) from the age fallback', () => {
vi.useFakeTimers();
vi.setSystemTime(new Date('2026-03-15T00:00:00.000Z'));

try {
const harness = createHarness({
containers: [
createContainer({
id: 'c-gated',
updateAvailable: false,
firstSeenAt: '2026-01-01T00:00:00.000Z',
}),
createContainer({
id: 'c-hot',
updateAvailable: true,
firstSeenAt: '2026-03-14T00:00:00.000Z',
}),
],
});

// A container with no available update must never surface an age,
// so it cannot be caught by a maturity filter it was never gated
// into, nor ranked by ?sort=age as if it had a real pending update.
const maturityRes = callGetContainers(harness.handlers, { maturity: 'hot' });
expect(
maturityRes.json.mock.calls[0][0].data.map((container: { id: string }) => container.id),
).toEqual(['c-hot']);

const establishedRes = callGetContainers(harness.handlers, { maturity: 'established' });
expect(establishedRes.json.mock.calls[0][0].data).toEqual([]);

const sortRes = callGetContainers(harness.handlers, { sort: 'age' });
expect(
sortRes.json.mock.calls[0][0].data.map((container: { id: string }) => container.id),
).toEqual(['c-hot', 'c-gated']);
} finally {
vi.useRealTimers();
}
});

test('excludes gated containers carrying a materialized updateAge from the age fallback', () => {
vi.useFakeTimers();
vi.setSystemTime(new Date('2026-03-15T00:00:00.000Z'));

try {
// buildFallbackContainerReport (watchers/providers/docker/docker-helpers.ts)
// spreads the container, which turns the self-gating `updateAge` getter into
// a plain frozen number, and only then sets updateAvailable = false. That
// shape reaches this code with a stale finite age attached to a container
// that has no available update, so the availability gate has to run before
// the cached age is read rather than after it.
const harness = createHarness({
containers: [
createContainer({
id: 'c-gated-cached-age',
updateAvailable: false,
updateAge: 73 * 24 * 3600 * 1000,
firstSeenAt: '2026-01-01T00:00:00.000Z',
}),
createContainer({
id: 'c-hot',
updateAvailable: true,
firstSeenAt: '2026-03-14T00:00:00.000Z',
}),
],
});

const establishedRes = callGetContainers(harness.handlers, { maturity: 'established' });
expect(establishedRes.json.mock.calls[0][0].data).toEqual([]);

const matureRes = callGetContainers(harness.handlers, { maturity: 'mature' });
expect(matureRes.json.mock.calls[0][0].data).toEqual([]);

const hotRes = callGetContainers(harness.handlers, { maturity: 'hot' });
expect(
hotRes.json.mock.calls[0][0].data.map((container: { id: string }) => container.id),
).toEqual(['c-hot']);

const sortRes = callGetContainers(harness.handlers, { sort: 'age' });
expect(
sortRes.json.mock.calls[0][0].data.map((container: { id: string }) => container.id),
).toEqual(['c-hot', 'c-gated-cached-age']);
} finally {
vi.useRealTimers();
}
});

test('uses watcher/name/id fallbacks when sorting by age with equal ages', () => {
const harness = createHarness({
containers: [
Expand Down
1 change: 1 addition & 0 deletions app/api/container/filters.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ describe('api/container/filters', () => {
const containers = Array.from({ length: 12 }, (_, index) => ({
id: `c${index + 1}`,
name: `container-${index + 1}`,
updateAvailable: true,
firstSeenAt: `2024-01-${String(index + 1).padStart(2, '0')}T00:00:00.000Z`,
}));

Expand Down
14 changes: 11 additions & 3 deletions app/api/container/maturity-filter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,17 @@ describe('api/container/maturity-filter', () => {

test('applyContainerMaturityFilter returns only hot containers', () => {
const containers = [
{ id: 'c1', updateAge: 60_000 } as unknown as Container,
{ id: 'c2', updateAge: 9 * 24 * 60 * 60 * 1000 } as unknown as Container,
{ id: 'c3', updateAge: 35 * 24 * 60 * 60 * 1000 } as unknown as Container,
{ id: 'c1', updateAvailable: true, updateAge: 60_000 } as unknown as Container,
{
id: 'c2',
updateAvailable: true,
updateAge: 9 * 24 * 60 * 60 * 1000,
} as unknown as Container,
{
id: 'c3',
updateAvailable: true,
updateAge: 35 * 24 * 60 * 60 * 1000,
} as unknown as Container,
];

const filtered = applyContainerMaturityFilter(containers, 'hot');
Expand Down
Loading
Loading