fix(docker): keep the digest on a pinned base image, and stop eating the file

src/buddy.tsmodified+6-1

Changes to src/buddy.ts

		⋯1,744 linesShow the 1,744 lines above this hunk@@ -1745,7 +1745,12 @@ export class Buddy {
1745	1745	try {
1746	1746	const { generateDockerfileUpdates } = await import('./utils/dockerfile-parser')
1747	1747	// Pass only the Dockerfile updates for this specific group
1748		- const dockerUpdates = await generateDockerfileUpdates(dockerfileUpdates)
	1748	+ // Passed through so a digest on a private image can be re-resolved
	1749	+ // with the same credentials the scan used to find the tag.
	1750	+ const dockerUpdates = await generateDockerfileUpdates(
	1751	+ dockerfileUpdates,
	1752	+ this.config.registries?.docker,
	1753	+ )
1749	1754	fileUpdates.push(...dockerUpdates)
1750	1755	}
1751	1756	catch (error) {

src/utils/dockerfile-parser.tsmodified+64-11

Changes to src/utils/dockerfile-parser.ts

		⋯111 linesShow the 111 lines above this hunk@@ -112,17 +112,34 @@ export async function parseDockerfile(filePath: string, content: string): Promis
112	112	/**
113	113	* Parse a Docker image reference into name and version components
114	114	*/
115		-function parseImageReference(imageRef: string): { name: string, version: string } | null {
	115	+function parseImageReference(imageRef: string): { name: string, version: string, digest?: string } | null {
116	116	try {
117	117	// Handle different image reference formats:
118	118	// - image:tag
119	119	// - registry/image:tag
120	120	// - registry:port/image:tag
121	121	// - registry/namespace/image:tag
	122	+ // - image:tag@sha256:digest
122	123
123	124	let name: string
124	125	let version: string
125	126
	127	+ // Split the digest off first. Everything below reasons about the *last*
	128	+ // colon, and a digest contributes two of them — so `node:20@sha256:abc`
	129	+ // parsed as the image `node:20@sha256` at version `abc`, which is not an
	130	+ // image, is never found in any registry, and appears on the dependency
	131	+ // dashboard as though it were real.
	132	+ const digestIndex = imageRef.indexOf('@')
	133	+ const digest = digestIndex === -1 ? undefined : imageRef.slice(digestIndex + 1)
	134	+ if (digestIndex !== -1)
	135	+ imageRef = imageRef.slice(0, digestIndex)
	136	+
	137	+ // Pinned by digest alone, with no tag. There is no version to move, and
	138	+ // reporting it as `latest` would put a tag on the dashboard that the file
	139	+ // does not contain.
	140	+ if (digest && !imageRef.includes(':'))
	141	+ return null
	142	+
126	143	if (imageRef.includes(':')) {
127	144	const lastColonIndex = imageRef.lastIndexOf(':')
128	145	const beforeColon = imageRef.substring(0, lastColonIndex)
		⋯17 linesShow the 17 lines above this hunk@@ -146,12 +163,12 @@ function parseImageReference(imageRef: string): { name: string, version: string
146	163	version = 'latest'
147	164	}
148	165
149		- // Skip if version contains variables or is a digest
	166	+ // Skip if version contains variables or is a bare digest
150	167	if (version.includes('$') || version.startsWith('sha256:')) {
151	168	return null
152	169	}
153	170
154		- return { name, version }
	171	+ return { name, version, ...(digest ? { digest } : {}) }
155	172	}
156	173	catch (error) {
157	174	getDefaultLogger().warn(`Failed to parse image reference ${imageRef}:`, error)
		⋯72 linesShow the 72 lines above this hunk@@ -230,7 +247,12 @@ export async function resolveDockerDigest(
230	247	/**
231	248	* Update Dockerfile content with new image versions
232	249	*/
233		-export async function updateDockerfile(filePath: string, content: string, updates: PackageUpdate[]): Promise<string> {
	250	+export async function updateDockerfile(
	251	+ filePath: string,
	252	+ content: string,
	253	+ updates: PackageUpdate[],
	254	+ registries?: DockerRegistryConfig,
	255	+): Promise<string> {
234	256	try {
235	257	if (!isDockerfile(filePath)) {
236	258	getDefaultLogger().info(`⚠️ updateDockerfile: ${filePath} is not a Dockerfile, returning original content`)
		⋯14 linesShow the 14 lines above this hunk@@ -251,8 +273,15 @@ export async function updateDockerfile(filePath: string, content: string, update
251	273	// Create regex to find FROM instructions with this image
252	274	// Handle various formats: FROM image:tag, FROM image:tag as alias
253	275	const escapedImageName = cleanImageName.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')
	276	+ // The tag is captured without its digest, so a pinned reference does not
	277	+ // swallow `@sha256:…` into the version and then lose it on replacement.
	278	+ //
	279	+ // The trailing group is horizontal whitespace only. `\\s` matches a
	280	+ // newline, so with the `m` flag it ran past the end of the line and took
	281	+ // every following line with it — a multi-stage Dockerfile that builds
	282	+ // from the same image twice had both `FROM` lines collapsed into one.
254	283	const fromRegex = new RegExp(
255		- `(FROM\\s+${escapedImageName})(:)([^\\s]+)(\\s.*)?$`,
	284	+ `(FROM\\s+${escapedImageName})(:)([^\\s@]+)(@sha256:[a-f0-9]+)?([ \\t].*)?$`,
256	285	'gim',
257	286	)
258	287
		⋯10 linesShow the 10 lines above this hunk@@ -269,19 +298,40 @@ export async function updateDockerfile(filePath: string, content: string, update
269	298	const fullMatch = match[0]
270	299	const beforeColon = match[1] // "FROM image"
271	300	const colon = match[2] // ":"
272		- const currentVersion = match[3] // "tag"
273		- const afterVersion = match[4] || '' // " as alias" or empty
	301	+ const currentVersion = match[3] // "tag", without any digest
	302	+ const currentDigest = match[4] || '' // "@sha256:…" or empty
	303	+ const afterVersion = match[5] || '' // " as alias" or empty
274	304
275	305	if (shouldRespectVersion(currentVersion)) {
276	306	getDefaultLogger().info(`⚠️ Skipping update for ${cleanImageName} - version "${currentVersion}" should be respected`)
277	307	continue
278	308	}
279	309
	310	+ // A pinned reference has to move both halves together. Writing the new
	311	+ // tag beside the old digest would leave the old image running while
	312	+ // the file claims otherwise, and dropping the digest silently unpins
	313	+ // an image somebody pinned on purpose — both worse than not updating.
	314	+ let digest = currentDigest
	315	+ if (currentDigest) {
	316	+ const resolved = await resolveDockerDigest(cleanImageName, update.newVersion, registries)
	317	+
	318	+ if (!resolved) {
	319	+ getDefaultLogger().warn(
	320	+ `⚠️ Leaving ${cleanImageName} pinned at ${currentVersion}: could not resolve the digest for ${update.newVersion}`,
	321	+ )
	322	+ continue
	323	+ }
	324	+
	325	+ digest = `@${resolved}`
	326	+ }
	327	+
280	328	// Replace with new version
281		- const replacement = `${beforeColon}${colon}${update.newVersion}${afterVersion}`
	329	+ const replacement = `${beforeColon}${colon}${update.newVersion}${digest}${afterVersion}`
282	330	updatedContent = updatedContent.replace(fullMatch, replacement)
283	331
284		- getDefaultLogger().info(`📝 Updated ${cleanImageName}: ${currentVersion} → ${update.newVersion}`)
	332	+ getDefaultLogger().info(
	333	+ `📝 Updated ${cleanImageName}: ${currentVersion} → ${update.newVersion}${digest ? ' (digest re-resolved)' : ''}`,
	334	+ )
285	335	}
286	336	}
287	337
		⋯8 linesShow the 8 lines above this hunk@@ -296,7 +346,10 @@ export async function updateDockerfile(filePath: string, content: string, update
296	346	/**
297	347	* Generate file changes for Dockerfiles
298	348	*/
299		-export async function generateDockerfileUpdates(updates: PackageUpdate[]): Promise<Array<{ path: string, content: string, type: 'update' }>> {
	349	+export async function generateDockerfileUpdates(
	350	+ updates: PackageUpdate[],
	351	+ registries?: DockerRegistryConfig,
	352	+): Promise<Array<{ path: string, content: string, type: 'update' }>> {
300	353	const fileUpdates: Array<{ path: string, content: string, type: 'update' }> = []
301	354
302	355	// Group updates by file
		⋯15 linesShow the 15 lines above this hunk@@ -318,7 +371,7 @@ export async function generateDockerfileUpdates(updates: PackageUpdate[]): Promi
318	371	const fs = await import('node:fs')
319	372	if (fs.existsSync(filePath)) {
320	373	const currentContent = fs.readFileSync(filePath, 'utf-8')
321		- const updatedContent = await updateDockerfile(filePath, currentContent, dockerUpdates)
	374	+ const updatedContent = await updateDockerfile(filePath, currentContent, dockerUpdates, registries)
322	375
323	376	// Only add file update if content actually changed
324	377	if (updatedContent !== currentContent) {

test/dockerfile-digests.test.tsadded+175-0

Changes to test/dockerfile-digests.test.ts

		@@ -0,0 +1,175 @@
	1	+import type { PackageUpdate } from '../src/types'
	2	+import { afterEach, describe, expect, it } from 'bun:test'
	3	+import { parseDockerfile, updateDockerfile } from '../src/utils/dockerfile-parser'
	4	+
	5	+const realFetch = globalThis.fetch
	6	+
	7	+afterEach(() => {
	8	+ globalThis.fetch = realFetch
	9	+})
	10	+
	11	+const DIGEST = 'sha256:aaaa1111bbbb2222cccc3333dddd4444eeee5555ffff6666aaaa7777bbbb8888'
	12	+const NEW_DIGEST = 'sha256:9999888877776666555544443333222211110000ffffeeeeddddccccbbbbaaaa'
	13	+
	14	+function update(overrides: Partial<PackageUpdate> = {}): PackageUpdate {
	15	+ return {
	16	+ name: 'node',
	17	+ currentVersion: '20',
	18	+ newVersion: '22',
	19	+ updateType: 'major',
	20	+ dependencyType: 'docker-image',
	21	+ file: 'Dockerfile',
	22	+ ...overrides,
	23	+ } as PackageUpdate
	24	+}
	25	+
	26	+/** Answer every registry request with a manifest carrying this digest. */
	27	+function registryReturning(digest: string | null): void {
	28	+ globalThis.fetch = (async (input: string) => {
	29	+ if (digest === null)
	30	+ return new Response('not found', { status: 404 })
	31	+
	32	+ if (String(input).includes('/token') || String(input).includes('auth'))
	33	+ return new Response(JSON.stringify({ token: 't' }), { status: 200 })
	34	+
	35	+ return new Response(JSON.stringify({ schemaVersion: 2 }), {
	36	+ status: 200,
	37	+ headers: { 'docker-content-digest': digest, 'content-type': 'application/vnd.oci.image.manifest.v1+json' },
	38	+ })
	39	+ }) as unknown as typeof fetch
	40	+}
	41	+
	42	+/**
	43	+ * `resolveDockerDigest` was written — with a docstring explaining that moving
	44	+ * a tag without moving its digest "would leave the old image running while the
	45	+ * file claims otherwise" — and never called. Two things followed from that.
	46	+ */
	47	+describe('digest-pinned base images', () => {
	48	+ describe('parsing', () => {
	49	+ it('success case - a pinned reference names the image, not the digest', async () => {
	50	+ // `node:20@sha256:abc` was split on the *last* colon, giving the image
	51	+ // `node:20@sha256` at version `abc`. That is not an image, is never
	52	+ // found in any registry, and appears on the dashboard as though real.
	53	+ const parsed = await parseDockerfile('Dockerfile', `FROM node:20@${DIGEST}\n`)
	54	+
	55	+ expect(parsed?.dependencies).toEqual([
	56	+ { name: 'node', currentVersion: '20', type: 'docker-image', file: 'Dockerfile' },
	57	+ ])
	58	+ })
	59	+
	60	+ it('success case - an unpinned reference is unchanged', async () => {
	61	+ const parsed = await parseDockerfile('Dockerfile', 'FROM alpine:3.19\n')
	62	+
	63	+ expect(parsed?.dependencies[0]).toMatchObject({ name: 'alpine', currentVersion: '3.19' })
	64	+ })
	65	+
	66	+ it('edge case - a registry with a port still parses', async () => {
	67	+ const parsed = await parseDockerfile('Dockerfile', `FROM registry.io:5000/team/app:2.1@${DIGEST}\n`)
	68	+
	69	+ expect(parsed?.dependencies[0]).toMatchObject({
	70	+ name: 'registry.io:5000/team/app',
	71	+ currentVersion: '2.1',
	72	+ })
	73	+ })
	74	+
	75	+ it('edge case - a bare digest reference is still skipped', async () => {
	76	+ const parsed = await parseDockerfile('Dockerfile', `FROM node@${DIGEST}\n`)
	77	+
	78	+ expect(parsed?.dependencies).toEqual([])
	79	+ })
	80	+ })
	81	+
	82	+ describe('updating', () => {
	83	+ it('failure case - the pin is never silently dropped', async () => {
	84	+ // The regression: `FROM node:20@sha256:…` became `FROM node:22`, which
	85	+ // unpins an image somebody pinned on purpose.
	86	+ registryReturning(NEW_DIGEST)
	87	+
	88	+ const result = await updateDockerfile('Dockerfile', `FROM node:20@${DIGEST}\n`, [update()])
	89	+
	90	+ expect(result).not.toBe(`FROM node:22\n`)
	91	+ expect(result).toContain('@sha256:')
	92	+ })
	93	+
	94	+ it('success case - the digest moves with the tag', async () => {
	95	+ registryReturning(NEW_DIGEST)
	96	+
	97	+ const result = await updateDockerfile('Dockerfile', `FROM node:20@${DIGEST}\n`, [update()])
	98	+
	99	+ expect(result).toBe(`FROM node:22@${NEW_DIGEST}\n`)
	100	+ })
	101	+
	102	+ it('failure case - an unresolvable digest leaves the line alone', async () => {
	103	+ // Writing the new tag beside the old digest would leave the old image
	104	+ // running while the file claims otherwise — worse than not updating.
	105	+ registryReturning(null)
	106	+
	107	+ const result = await updateDockerfile('Dockerfile', `FROM node:20@${DIGEST}\n`, [update()])
	108	+
	109	+ expect(result).toBe(`FROM node:20@${DIGEST}\n`)
	110	+ })
	111	+
	112	+ it('success case - an unpinned image needs no registry call', async () => {
	113	+ let called = false
	114	+ globalThis.fetch = (async () => {
	115	+ called = true
	116	+ return new Response('{}', { status: 200 })
	117	+ }) as unknown as typeof fetch
	118	+
	119	+ const result = await updateDockerfile('Dockerfile', 'FROM node:20\n', [update()])
	120	+
	121	+ expect(result).toBe('FROM node:22\n')
	122	+ expect(called).toBe(false)
	123	+ })
	124	+
	125	+ it('success case - a stage alias survives the rewrite', async () => {
	126	+ registryReturning(NEW_DIGEST)
	127	+
	128	+ const result = await updateDockerfile('Dockerfile', `FROM node:20@${DIGEST} as build\n`, [update()])
	129	+
	130	+ expect(result).toBe(`FROM node:22@${NEW_DIGEST} as build\n`)
	131	+ })
	132	+
	133	+ it('edge case - a pinned and an unpinned line update independently', async () => {
	134	+ registryReturning(NEW_DIGEST)
	135	+
	136	+ const result = await updateDockerfile(
	137	+ 'Dockerfile',
	138	+ `FROM node:20@${DIGEST}\nFROM node:20 as tools\n`,
	139	+ [update()],
	140	+ )
	141	+
	142	+ expect(result).toContain(`FROM node:22@${NEW_DIGEST}`)
	143	+ expect(result).toContain('FROM node:22 as tools')
	144	+ })
	145	+ })
	146	+})
	147	+
	148	+/**
	149	+ * Pre-existing, and found while fixing the digest handling: the trailing group
	150	+ * was `(\s.*)?$`, and `\s` matches a newline. With the `m` flag the match ran
	151	+ * past the end of its line and took every following line with it.
	152	+ */
	153	+describe('a FROM line stays on its line', () => {
	154	+ it('failure case - a repeated base image does not collapse the file', async () => {
	155	+ // A multi-stage build from the same image twice had both `FROM` lines
	156	+ // replaced by a single one, silently deleting a build stage.
	157	+ const content = 'FROM node:20 as build\nRUN bun install\nFROM node:20 as run\n'
	158	+
	159	+ const result = await updateDockerfile('Dockerfile', content, [update()])
	160	+
	161	+ expect(result.split('\n').filter(line => line.startsWith('FROM'))).toEqual([
	162	+ 'FROM node:22 as build',
	163	+ 'FROM node:22 as run',
	164	+ ])
	165	+ expect(result).toContain('RUN bun install')
	166	+ })
	167	+
	168	+ it('success case - lines between two FROMs are untouched', async () => {
	169	+ const content = 'FROM node:20\nCOPY . .\nFROM alpine:3.19\n'
	170	+
	171	+ const result = await updateDockerfile('Dockerfile', content, [update()])
	172	+
	173	+ expect(result).toBe('FROM node:22\nCOPY . .\nFROM alpine:3.19\n')
	174	+ })
	175	+})