From 98a58e25d0eb3327d44c3994003c1218382a2b62 Mon Sep 17 00:00:00 2001 From: Matt Rubens <2600+mrubens@users.noreply.github.com> Date: Thu, 10 Sep 2026 12:09:42 -0400 Subject: [PATCH 1/4] fix: pin Modal base image tags to their current digest Modal caches images.fromRegistry(ref) by the ref string, so a mutable tag such as :develop or :latest never gets re-pulled after the first build. Sandboxes silently kept running months-old worker images (and their baked OpenCode CLI), which surfaced as every task failing POST /session with ConfigInvalidError once the generated config used a newer config key. Resolve mutable tags to a digest-pinned reference via the OCI distribution API before handing the image to Modal. New pushes to the tag now produce a new image definition and Modal rebuilds. The lookup is cached per process, probes anonymously (registries answer a rejected Basic header with a placeholder scope), retries the token request anonymously when configured credentials are rejected, and falls back to the tag on any failure so a registry hiccup never blocks sandbox creation. ECR OIDC mode is unchanged. Also include the OpenCode response body in the error thrown for failed harness requests so config rejections are visible in the task error instead of only in the sandbox's harness.log. --- .../lib/harnesses/opencode-server/client.ts | 4 +- .../src/adapters/modal.test.ts | 37 +++ .../compute-providers/src/adapters/modal.ts | 14 +- .../src/modal/registry-digest.test.ts | 294 +++++++++++++++++ .../src/modal/registry-digest.ts | 309 ++++++++++++++++++ 5 files changed, 655 insertions(+), 3 deletions(-) create mode 100644 packages/compute-providers/src/modal/registry-digest.test.ts create mode 100644 packages/compute-providers/src/modal/registry-digest.ts diff --git a/apps/worker/src/sandbox-server/lib/harnesses/opencode-server/client.ts b/apps/worker/src/sandbox-server/lib/harnesses/opencode-server/client.ts index 43e0c38fab..dfe7d46483 100644 --- a/apps/worker/src/sandbox-server/lib/harnesses/opencode-server/client.ts +++ b/apps/worker/src/sandbox-server/lib/harnesses/opencode-server/client.ts @@ -484,7 +484,9 @@ export class OpenCodeServerClient { )}`, ); throw new Error( - `OpenCode request failed method=${method} path=${path} status=${response.status}`, + `OpenCode request failed method=${method} path=${path} status=${response.status}${ + responseText ? ` body=${responseText.slice(0, 300)}` : '' + }`, ); } catch (error) { const elapsedMs = Date.now() - startedAt; diff --git a/packages/compute-providers/src/adapters/modal.test.ts b/packages/compute-providers/src/adapters/modal.test.ts index 890935bdc4..1053247fe4 100644 --- a/packages/compute-providers/src/adapters/modal.test.ts +++ b/packages/compute-providers/src/adapters/modal.test.ts @@ -25,6 +25,14 @@ const { secretFromObjectMock: vi.fn(), })); +const { pinModalBaseImageRefMock } = vi.hoisted(() => ({ + pinModalBaseImageRefMock: vi.fn(), +})); + +vi.mock('../modal/registry-digest', () => ({ + pinModalBaseImageRef: pinModalBaseImageRefMock, +})); + vi.mock('modal', () => { class MockSdkModalClient { public readonly sandboxes = { @@ -62,6 +70,9 @@ describe('ModalClient', () => { beforeEach(() => { vi.clearAllMocks(); + pinModalBaseImageRefMock.mockImplementation( + async ({ ref }: { ref: string }) => ref, + ); imageFromRegistryMock.mockReturnValue({ imageId: 'img-123' }); imageFromAwsEcrMock.mockReturnValue({ imageId: 'img-ecr-123' }); imageFromIdMock.mockResolvedValue({ imageId: 'img-snap-123' }); @@ -247,6 +258,32 @@ describe('ModalClient', () => { MODAL_IMAGE_REF, registrySecret, ); + expect(pinModalBaseImageRefMock).toHaveBeenCalledWith({ + ref: MODAL_IMAGE_REF, + registryUsername: 'ghcr-user', + registryPassword: 'ghcr-token', + }); + }); + + it('builds the sandbox image from the digest-pinned base image ref', async () => { + const pinnedRef = 'ghcr.io/roomote/modal-worker@sha256:' + 'a'.repeat(64); + pinModalBaseImageRefMock.mockResolvedValue(pinnedRef); + sandboxCreateMock.mockResolvedValue({ + sandboxId: 'modal-123', + tunnels: vi.fn().mockResolvedValue({}), + }); + + const client = new ModalClient({ + tokenId: 'token-id', + tokenSecret: 'token-secret', + baseImageRef: MODAL_IMAGE_REF, + }); + + await expect(client.createInstance({})).resolves.toMatchObject({ + instanceId: 'modal-123', + }); + + expect(imageFromRegistryMock).toHaveBeenCalledWith(pinnedRef); }); it('applies sandbox tags after creating a Modal instance', async () => { diff --git a/packages/compute-providers/src/adapters/modal.ts b/packages/compute-providers/src/adapters/modal.ts index 7756b89e65..99241d2484 100644 --- a/packages/compute-providers/src/adapters/modal.ts +++ b/packages/compute-providers/src/adapters/modal.ts @@ -40,6 +40,7 @@ import { toAbortError, throwIfAborted, } from '../modal/abort'; +import { pinModalBaseImageRef } from '../modal/registry-digest'; import { normalizeModalRpcError } from '../modal/rpc-diagnostics'; const DEFAULT_APP_NAME = 'roomote'; @@ -306,12 +307,21 @@ export class ModalClient implements ComputeProviderClient { return this.sdk.images.fromAwsEcr(this.baseImageRef, secret); } + // Modal keys its image cache on the ref string, so a mutable tag such as + // `:develop` would never be re-pulled after the first build. Pin the tag to + // its current digest so a new push produces a new image definition. + const imageRef = await pinModalBaseImageRef({ + ref: this.baseImageRef, + registryUsername: this.config.registryUsername, + registryPassword: this.config.registryPassword, + }); + if (this.imageMode === 'registry-auth') { const secret = await this.getRegistrySecret(); - return this.sdk.images.fromRegistry(this.baseImageRef, secret); + return this.sdk.images.fromRegistry(imageRef, secret); } - return this.sdk.images.fromRegistry(this.baseImageRef); + return this.sdk.images.fromRegistry(imageRef); } private normalizeSandboxTags( diff --git a/packages/compute-providers/src/modal/registry-digest.test.ts b/packages/compute-providers/src/modal/registry-digest.test.ts new file mode 100644 index 0000000000..ef2ed81758 --- /dev/null +++ b/packages/compute-providers/src/modal/registry-digest.test.ts @@ -0,0 +1,294 @@ +import { + clearModalBaseImageDigestCache, + parseImageRef, + pinModalBaseImageRef, + resolveImageRefDigest, +} from './registry-digest'; + +const DIGEST = `sha256:${'f'.repeat(64)}`; + +function response(init: { + status: number; + headers?: Record; + body?: unknown; +}): Response { + return new Response( + init.body === undefined ? null : JSON.stringify(init.body), + { + status: init.status, + headers: init.headers, + }, + ); +} + +describe('parseImageRef', () => { + it('parses registry-qualified refs with tags', () => { + expect(parseImageRef('ghcr.io/roocodeinc/roomote-worker:develop')).toEqual({ + registry: 'ghcr.io', + repository: 'roocodeinc/roomote-worker', + tag: 'develop', + digest: undefined, + }); + }); + + it('defaults the tag to latest', () => { + expect(parseImageRef('ghcr.io/roocodeinc/roomote-worker')?.tag).toBe( + 'latest', + ); + }); + + it('keeps digests and drops the implicit tag', () => { + expect( + parseImageRef(`ghcr.io/roocodeinc/roomote-worker@${DIGEST}`), + ).toEqual({ + registry: 'ghcr.io', + repository: 'roocodeinc/roomote-worker', + tag: undefined, + digest: DIGEST, + }); + }); + + it('maps Docker Hub shorthand to registry-1.docker.io', () => { + expect(parseImageRef('roomote/worker:1.0')).toEqual({ + registry: 'registry-1.docker.io', + repository: 'roomote/worker', + tag: '1.0', + digest: undefined, + }); + expect(parseImageRef('ubuntu:24.04')).toBeNull(); + expect(parseImageRef('docker.io/ubuntu:24.04')?.repository).toBe( + 'library/ubuntu', + ); + }); + + it('handles registries with ports', () => { + expect(parseImageRef('localhost:5000/roomote-worker:local')).toEqual({ + registry: 'localhost:5000', + repository: 'roomote-worker', + tag: 'local', + digest: undefined, + }); + }); + + it('rejects bare local tags and malformed digests', () => { + expect(parseImageRef('roomote-worker:local')).toBeNull(); + expect(parseImageRef('ghcr.io/org/repo@sha256:nope')).toBeNull(); + expect(parseImageRef('')).toBeNull(); + }); +}); + +describe('resolveImageRefDigest', () => { + it('follows the bearer challenge and returns the digest-pinned ref', async () => { + const calls: Array<{ url: string; method: string; auth: string | null }> = + []; + const fetchImpl = vi.fn( + async (input: string | URL | Request, init?: RequestInit) => { + const url = String(input); + const headers = new Headers(init?.headers); + calls.push({ + url, + method: init?.method ?? 'GET', + auth: headers.get('authorization'), + }); + + if (url.startsWith('https://ghcr.io/token')) { + return response({ status: 200, body: { token: 'registry-token' } }); + } + + if (!headers.get('authorization')?.startsWith('Bearer ')) { + return response({ + status: 401, + headers: { + 'www-authenticate': + 'Bearer realm="https://ghcr.io/token",service="ghcr.io",scope="repository:roocodeinc/roomote-worker:pull"', + }, + }); + } + + return response({ + status: 200, + headers: { 'docker-content-digest': DIGEST }, + }); + }, + ) as unknown as typeof fetch; + + await expect( + resolveImageRefDigest({ + ref: 'ghcr.io/roocodeinc/roomote-worker:develop', + registryUsername: 'user', + registryPassword: 'pass', + fetchImpl, + }), + ).resolves.toBe(`ghcr.io/roocodeinc/roomote-worker@${DIGEST}`); + + expect(calls.map((call) => call.method)).toEqual(['HEAD', 'GET', 'HEAD']); + expect(calls[0]?.url).toBe( + 'https://ghcr.io/v2/roocodeinc/roomote-worker/manifests/develop', + ); + expect(calls[0]?.auth).toBeNull(); + expect(calls[1]?.url).toBe( + 'https://ghcr.io/token?service=ghcr.io&scope=repository%3Aroocodeinc%2Froomote-worker%3Apull', + ); + expect(calls[1]?.auth).toBe( + `Basic ${Buffer.from('user:pass').toString('base64')}`, + ); + expect(calls[2]?.auth).toBe('Bearer registry-token'); + }); + + it('retries the token request anonymously when credentials are rejected', async () => { + const tokenAuths: Array = []; + const tokenUrls: string[] = []; + const fetchImpl = vi.fn( + async (input: string | URL | Request, init?: RequestInit) => { + const url = String(input); + const headers = new Headers(init?.headers); + + if (url.startsWith('https://ghcr.io/token')) { + const auth = headers.get('authorization'); + tokenAuths.push(auth); + tokenUrls.push(url); + return auth + ? response({ status: 403, body: { errors: [{ code: 'DENIED' }] } }) + : response({ status: 200, body: { token: 'anon-token' } }); + } + + if (headers.get('authorization') === 'Bearer anon-token') { + return response({ + status: 200, + headers: { 'docker-content-digest': DIGEST }, + }); + } + + return response({ + status: 401, + headers: { + 'www-authenticate': + 'Bearer realm="https://ghcr.io/token",service="ghcr.io",scope="repository:user/image:pull"', + }, + }); + }, + ) as unknown as typeof fetch; + + await expect( + resolveImageRefDigest({ + ref: 'ghcr.io/roocodeinc/roomote-worker:develop', + registryUsername: 'stale-user', + registryPassword: 'expired-token', + fetchImpl, + }), + ).resolves.toBe(`ghcr.io/roocodeinc/roomote-worker@${DIGEST}`); + + expect( + tokenUrls.every((url) => + url.includes('scope=repository%3Aroocodeinc%2Froomote-worker%3Apull'), + ), + ).toBe(true); + expect(tokenAuths).toEqual([ + `Basic ${Buffer.from('stale-user:expired-token').toString('base64')}`, + null, + ]); + }); + + it('returns digest-pinned refs unchanged without touching the registry', async () => { + const fetchImpl = vi.fn() as unknown as typeof fetch; + const ref = `ghcr.io/roocodeinc/roomote-worker@${DIGEST}`; + + await expect(resolveImageRefDigest({ ref, fetchImpl })).resolves.toBe(ref); + expect(fetchImpl).not.toHaveBeenCalled(); + }); + + it('throws when the registry does not return a digest', async () => { + const fetchImpl = vi.fn(async () => + response({ status: 200 }), + ) as unknown as typeof fetch; + + await expect( + resolveImageRefDigest({ + ref: 'ghcr.io/roocodeinc/roomote-worker:develop', + fetchImpl, + }), + ).rejects.toThrow(/Docker-Content-Digest/u); + }); + + it('throws on non-registry-qualified refs', async () => { + await expect( + resolveImageRefDigest({ ref: 'roomote-worker:local' }), + ).rejects.toThrow(/not registry-qualified/u); + }); +}); + +describe('pinModalBaseImageRef', () => { + beforeEach(() => { + clearModalBaseImageDigestCache(); + vi.spyOn(console, 'log').mockImplementation(() => {}); + vi.spyOn(console, 'warn').mockImplementation(() => {}); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('pins mutable tags and caches the lookup', async () => { + const fetchImpl = vi.fn(async () => + response({ + status: 200, + headers: { 'docker-content-digest': DIGEST }, + }), + ) as unknown as typeof fetch; + let now = 1_000; + + const first = await pinModalBaseImageRef({ + ref: 'ghcr.io/roocodeinc/roomote-worker:develop', + fetchImpl, + now: () => now, + }); + const second = await pinModalBaseImageRef({ + ref: 'ghcr.io/roocodeinc/roomote-worker:develop', + fetchImpl, + now: () => now, + }); + + expect(first).toBe(`ghcr.io/roocodeinc/roomote-worker@${DIGEST}`); + expect(second).toBe(first); + expect(fetchImpl).toHaveBeenCalledTimes(1); + + now += 120_000; + await pinModalBaseImageRef({ + ref: 'ghcr.io/roocodeinc/roomote-worker:develop', + fetchImpl, + now: () => now, + }); + expect(fetchImpl).toHaveBeenCalledTimes(2); + }); + + it('falls back to the tag when the registry lookup fails', async () => { + const fetchImpl = vi.fn(async () => { + throw new Error('network down'); + }) as unknown as typeof fetch; + + await expect( + pinModalBaseImageRef({ + ref: 'ghcr.io/roocodeinc/roomote-worker:develop', + fetchImpl, + }), + ).resolves.toBe('ghcr.io/roocodeinc/roomote-worker:develop'); + expect(console.warn).toHaveBeenCalledWith( + expect.stringContaining('Could not resolve base image digest'), + ); + }); + + it('leaves bare local tags and digest refs alone', async () => { + const fetchImpl = vi.fn() as unknown as typeof fetch; + + await expect( + pinModalBaseImageRef({ ref: 'roomote-worker:local', fetchImpl }), + ).resolves.toBe('roomote-worker:local'); + await expect( + pinModalBaseImageRef({ + ref: `ghcr.io/roocodeinc/roomote-worker@${DIGEST}`, + fetchImpl, + }), + ).resolves.toBe(`ghcr.io/roocodeinc/roomote-worker@${DIGEST}`); + expect(fetchImpl).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/compute-providers/src/modal/registry-digest.ts b/packages/compute-providers/src/modal/registry-digest.ts new file mode 100644 index 0000000000..0dbc7b6f08 --- /dev/null +++ b/packages/compute-providers/src/modal/registry-digest.ts @@ -0,0 +1,309 @@ +/** + * Resolves a mutable image tag (`ghcr.io/org/worker:develop`) to a + * digest-pinned reference (`ghcr.io/org/worker@sha256:...`). + * + * Modal caches `images.fromRegistry(ref)` by the ref string, so a mutable tag + * freezes at whatever the registry served the first time it was built. Pinning + * the digest changes the image definition whenever the tag moves, which makes + * Modal pull the new image instead of silently reusing a months-old build. + */ + +const DIGEST_CACHE_TTL_MS = 60_000; + +const MANIFEST_ACCEPT = [ + 'application/vnd.oci.image.index.v1+json', + 'application/vnd.docker.distribution.manifest.list.v2+json', + 'application/vnd.oci.image.manifest.v1+json', + 'application/vnd.docker.distribution.manifest.v2+json', +].join(', '); + +const DIGEST_PATTERN = /^sha256:[a-f0-9]{64}$/u; + +const DOCKER_HUB_REGISTRY = 'registry-1.docker.io'; + +interface ParsedImageRef { + /** Registry host, e.g. `ghcr.io` or `registry-1.docker.io`. */ + registry: string; + /** Repository path without the registry, e.g. `roocodeinc/roomote-worker`. */ + repository: string; + tag: string | undefined; + digest: string | undefined; +} + +/** + * Parses a registry-qualified image reference. Returns `null` for refs that + * are not registry-qualified (bare local tags such as `roomote-worker:local`) + * because those cannot be looked up remotely anyway. + */ +export function parseImageRef(ref: string): ParsedImageRef | null { + const trimmed = ref.trim(); + if (!trimmed) return null; + + let rest = trimmed; + let digest: string | undefined; + const atIndex = rest.indexOf('@'); + if (atIndex !== -1) { + digest = rest.slice(atIndex + 1); + rest = rest.slice(0, atIndex); + if (!DIGEST_PATTERN.test(digest)) return null; + } + + const firstSlash = rest.indexOf('/'); + if (firstSlash === -1) return null; + + const firstSegment = rest.slice(0, firstSlash); + const looksLikeRegistry = + firstSegment.includes('.') || + firstSegment.includes(':') || + firstSegment === 'localhost'; + + let registry: string; + let repositoryWithTag: string; + if (looksLikeRegistry) { + registry = firstSegment; + repositoryWithTag = rest.slice(firstSlash + 1); + } else { + registry = 'docker.io'; + repositoryWithTag = rest; + } + + if (registry === 'docker.io' || registry === 'index.docker.io') { + registry = DOCKER_HUB_REGISTRY; + if (!repositoryWithTag.includes('/')) { + repositoryWithTag = `library/${repositoryWithTag}`; + } + } + + let tag: string | undefined; + let repository = repositoryWithTag; + const lastColon = repositoryWithTag.lastIndexOf(':'); + if (lastColon !== -1 && !repositoryWithTag.slice(lastColon).includes('/')) { + tag = repositoryWithTag.slice(lastColon + 1); + repository = repositoryWithTag.slice(0, lastColon); + } + + if (!repository) return null; + + return { + registry, + repository, + tag: tag || (digest ? undefined : 'latest'), + digest, + }; +} + +function parseWwwAuthenticate( + header: string, +): { realm: string; params: Record } | null { + const match = /^Bearer\s+(.*)$/iu.exec(header.trim()); + if (!match) return null; + + const params: Record = {}; + for (const part of match[1]!.matchAll(/(\w+)="([^"]*)"/gu)) { + params[part[1]!] = part[2]!; + } + + const realm = params.realm; + if (!realm) return null; + delete params.realm; + return { realm, params }; +} + +function basicAuthorization( + username: string | undefined, + password: string | undefined, +): string | undefined { + if (!username || !password) return undefined; + return `Basic ${Buffer.from(`${username}:${password}`, 'utf8').toString('base64')}`; +} + +interface ResolveImageRefDigestOptions { + ref: string; + registryUsername?: string; + registryPassword?: string; + fetchImpl?: typeof fetch; + timeoutMs?: number; +} + +async function fetchBearerToken( + challenge: { realm: string; params: Record }, + repository: string, + options: ResolveImageRefDigestOptions, + fetchImpl: typeof fetch, + signal: AbortSignal, +): Promise { + const url = new URL(challenge.realm); + for (const [key, value] of Object.entries(challenge.params)) { + url.searchParams.set(key, value); + } + // Some registries (ghcr.io) answer a rejected Basic header with a + // placeholder scope, so always ask for the repository we actually need. + url.searchParams.set('scope', `repository:${repository}:pull`); + + const authorization = basicAuthorization( + options.registryUsername, + options.registryPassword, + ); + let response = await fetchImpl(url, { + headers: authorization ? { Authorization: authorization } : {}, + signal, + }); + if (authorization && (response.status === 401 || response.status === 403)) { + // Stale or under-scoped registry credentials must not hide a public + // image: retry the token request anonymously before giving up. + response = await fetchImpl(url, { signal }); + } + if (!response.ok) { + throw new Error( + `registry token request failed with status ${response.status}`, + ); + } + + const body = (await response.json()) as { + token?: unknown; + access_token?: unknown; + }; + const token = + typeof body.token === 'string' + ? body.token + : typeof body.access_token === 'string' + ? body.access_token + : undefined; + if (!token) { + throw new Error('registry token response did not include a token'); + } + return token; +} + +/** + * Looks up the current digest for `ref` via the OCI distribution API and + * returns the digest-pinned reference. Throws when the registry cannot be + * queried; callers decide whether to fall back to the tag. + */ +export async function resolveImageRefDigest( + options: ResolveImageRefDigestOptions, +): Promise { + const parsed = parseImageRef(options.ref); + if (!parsed) { + throw new Error(`image ref "${options.ref}" is not registry-qualified`); + } + if (parsed.digest) { + return options.ref.trim(); + } + + const fetchImpl = options.fetchImpl ?? globalThis.fetch; + const signal = AbortSignal.timeout(options.timeoutMs ?? 10_000); + const manifestUrl = `https://${parsed.registry}/v2/${parsed.repository}/manifests/${parsed.tag}`; + + const head = (authorization?: string) => + fetchImpl(manifestUrl, { + method: 'HEAD', + headers: { + Accept: MANIFEST_ACCEPT, + ...(authorization ? { Authorization: authorization } : {}), + }, + signal, + }); + + // Probe anonymously: the 401 challenge carries the realm and service, and + // credentials (when configured) are presented to the token endpoint. + let response = await head(); + + if (response.status === 401) { + const challengeHeader = response.headers.get('www-authenticate'); + const challenge = challengeHeader + ? parseWwwAuthenticate(challengeHeader) + : null; + if (!challenge) { + throw new Error( + `registry returned 401 for ${manifestUrl} without a bearer challenge`, + ); + } + const token = await fetchBearerToken( + challenge, + parsed.repository, + options, + fetchImpl, + signal, + ); + response = await head(`Bearer ${token}`); + } + + if (!response.ok) { + throw new Error( + `registry manifest lookup failed with status ${response.status} for ${manifestUrl}`, + ); + } + + const digest = response.headers.get('docker-content-digest')?.trim(); + if (!digest || !DIGEST_PATTERN.test(digest)) { + throw new Error( + `registry did not return a usable Docker-Content-Digest for ${manifestUrl}`, + ); + } + + const registry = + parsed.registry === DOCKER_HUB_REGISTRY ? 'docker.io' : parsed.registry; + return `${registry}/${parsed.repository}@${digest}`; +} + +const digestCache = new Map< + string, + { expiresAt: number; promise: Promise } +>(); + +/** + * Pins a Modal base image ref to its current digest so Modal's image cache key + * tracks the tag. Falls back to the original ref (and logs) when the registry + * cannot be queried, so a registry hiccup never blocks sandbox creation. + * Results are cached briefly per process to keep the lookup cheap on the + * per-spawn path while still picking up new pushes within a minute. + */ +export async function pinModalBaseImageRef( + options: ResolveImageRefDigestOptions & { now?: () => number }, +): Promise { + const ref = options.ref.trim(); + const parsed = parseImageRef(ref); + if (!parsed || parsed.digest) { + return ref; + } + + const now = options.now ?? Date.now; + const cacheKey = `${ref} ${options.registryUsername ?? ''}`; + const cached = digestCache.get(cacheKey); + if (cached && cached.expiresAt > now()) { + return cached.promise; + } + + const promise = resolveImageRefDigest(options) + .then((pinned) => { + console.log( + `[ModalClient] Pinned base image ${JSON.stringify({ ref, pinned })}`, + ); + return pinned; + }) + .catch((error) => { + digestCache.delete(cacheKey); + console.warn( + `[ModalClient] Could not resolve base image digest; using tag as-is ${JSON.stringify( + { + ref, + error: error instanceof Error ? error.message : String(error), + }, + )}`, + ); + return ref; + }); + + digestCache.set(cacheKey, { + expiresAt: now() + DIGEST_CACHE_TTL_MS, + promise, + }); + + return promise; +} + +/** Clears the per-process digest cache (tests). */ +export function clearModalBaseImageDigestCache(): void { + digestCache.clear(); +} From c65ae5b1a19262a0188cd894ba1d571e454a5dfe Mon Sep 17 00:00:00 2001 From: Matt Rubens <2600+mrubens@users.noreply.github.com> Date: Thu, 10 Sep 2026 12:15:57 -0400 Subject: [PATCH 2/4] fix: honor Basic registry challenges when resolving image digests Docker Distribution registries secured with htpasswd-style auth answer the manifest probe with a Basic challenge rather than a Bearer one. Retry the manifest HEAD with the configured credentials in that case instead of falling back to the mutable tag, and fail clearly when no credentials are configured. --- .../src/modal/registry-digest.test.ts | 61 +++++++++++++++++++ .../src/modal/registry-digest.ts | 45 +++++++++----- 2 files changed, 91 insertions(+), 15 deletions(-) diff --git a/packages/compute-providers/src/modal/registry-digest.test.ts b/packages/compute-providers/src/modal/registry-digest.test.ts index ef2ed81758..6bcf987738 100644 --- a/packages/compute-providers/src/modal/registry-digest.test.ts +++ b/packages/compute-providers/src/modal/registry-digest.test.ts @@ -189,6 +189,67 @@ describe('resolveImageRefDigest', () => { ]); }); + it('retries the manifest request with Basic credentials when the registry challenges with Basic', async () => { + const calls: Array<{ method: string; auth: string | null }> = []; + const fetchImpl = vi.fn( + async (input: string | URL | Request, init?: RequestInit) => { + const headers = new Headers(init?.headers); + const auth = headers.get('authorization'); + calls.push({ method: init?.method ?? 'GET', auth }); + + if (String(input).includes('/token')) { + throw new Error('token endpoint must not be called for Basic'); + } + + if (auth?.startsWith('Basic ')) { + return response({ + status: 200, + headers: { 'docker-content-digest': DIGEST }, + }); + } + + return response({ + status: 401, + headers: { 'www-authenticate': 'Basic realm="Registry Realm"' }, + }); + }, + ) as unknown as typeof fetch; + + await expect( + resolveImageRefDigest({ + ref: 'registry.example.com/roomote/worker:develop', + registryUsername: 'user', + registryPassword: 'pass', + fetchImpl, + }), + ).resolves.toBe(`registry.example.com/roomote/worker@${DIGEST}`); + + expect(calls).toEqual([ + { method: 'HEAD', auth: null }, + { + method: 'HEAD', + auth: `Basic ${Buffer.from('user:pass').toString('base64')}`, + }, + ]); + }); + + it('throws when the registry challenges with Basic and no credentials are configured', async () => { + const fetchImpl = vi.fn(async () => + response({ + status: 401, + headers: { 'www-authenticate': 'Basic realm="Registry Realm"' }, + }), + ) as unknown as typeof fetch; + + await expect( + resolveImageRefDigest({ + ref: 'registry.example.com/roomote/worker:develop', + fetchImpl, + }), + ).rejects.toThrow(/requires Basic credentials/u); + expect(fetchImpl).toHaveBeenCalledTimes(1); + }); + it('returns digest-pinned refs unchanged without touching the registry', async () => { const fetchImpl = vi.fn() as unknown as typeof fetch; const ref = `ghcr.io/roocodeinc/roomote-worker@${DIGEST}`; diff --git a/packages/compute-providers/src/modal/registry-digest.ts b/packages/compute-providers/src/modal/registry-digest.ts index 0dbc7b6f08..418f66ef86 100644 --- a/packages/compute-providers/src/modal/registry-digest.ts +++ b/packages/compute-providers/src/modal/registry-digest.ts @@ -210,23 +210,38 @@ export async function resolveImageRefDigest( let response = await head(); if (response.status === 401) { - const challengeHeader = response.headers.get('www-authenticate'); - const challenge = challengeHeader - ? parseWwwAuthenticate(challengeHeader) - : null; - if (!challenge) { - throw new Error( - `registry returned 401 for ${manifestUrl} without a bearer challenge`, + const challengeHeader = response.headers.get('www-authenticate') ?? ''; + const scheme = challengeHeader.trim().split(/\s+/u)[0]?.toLowerCase(); + + if (scheme === 'basic') { + // Docker Distribution registries secured with htpasswd-style auth + // challenge with Basic directly; there is no token endpoint to call. + const authorization = basicAuthorization( + options.registryUsername, + options.registryPassword, ); + if (!authorization) { + throw new Error( + `registry requires Basic credentials for ${manifestUrl} and none are configured`, + ); + } + response = await head(authorization); + } else { + const challenge = parseWwwAuthenticate(challengeHeader); + if (!challenge) { + throw new Error( + `registry returned 401 for ${manifestUrl} without a usable challenge`, + ); + } + const token = await fetchBearerToken( + challenge, + parsed.repository, + options, + fetchImpl, + signal, + ); + response = await head(`Bearer ${token}`); } - const token = await fetchBearerToken( - challenge, - parsed.repository, - options, - fetchImpl, - signal, - ); - response = await head(`Bearer ${token}`); } if (!response.ok) { From 03390289a3b4cdc37b5db5f59306cf2c6b154c4b Mon Sep 17 00:00:00 2001 From: Matt Rubens <2600+mrubens@users.noreply.github.com> Date: Thu, 10 Sep 2026 12:39:44 -0400 Subject: [PATCH 3/4] fix: keep last resolved digest and redact OpenCode error bodies Address review findings on the Modal digest pin: - Serve the last successfully resolved digest when a registry refresh fails instead of dropping back to the mutable tag, which would silently re-trigger Modal's stale image cache. The tag is only used when no digest has ever resolved in the process. - Cache failed lookups for 15s so a registry outage costs one timeout per window rather than one per spawn, and thread the sandbox spawn's abort signal into the registry fetches. - Redact the OpenCode response body before it enters the request error message, and render the session-create failure message as a code block in the transcript so JSON bodies do not break markdown. --- .../lib/harnesses/opencode-server/client.ts | 5 +- .../lib/harnesses/opencode-server/harness.ts | 5 +- .../compute-providers/src/adapters/modal.ts | 5 +- .../src/modal/registry-digest.test.ts | 94 +++++++++++++++++++ .../src/modal/registry-digest.ts | 85 ++++++++++++----- 5 files changed, 167 insertions(+), 27 deletions(-) diff --git a/apps/worker/src/sandbox-server/lib/harnesses/opencode-server/client.ts b/apps/worker/src/sandbox-server/lib/harnesses/opencode-server/client.ts index dfe7d46483..02b8551208 100644 --- a/apps/worker/src/sandbox-server/lib/harnesses/opencode-server/client.ts +++ b/apps/worker/src/sandbox-server/lib/harnesses/opencode-server/client.ts @@ -8,6 +8,7 @@ import type { OpenCodeSession, OpenCodeSessionMessage, } from './types'; +import { redactSecrets } from '@roomote/communication/redact-secrets'; // Bound every unary OpenCode HTTP call so a wedged server cannot leave the // worker sitting forever on a bare `fetch` with only the task-wide cancel @@ -485,7 +486,9 @@ export class OpenCodeServerClient { ); throw new Error( `OpenCode request failed method=${method} path=${path} status=${response.status}${ - responseText ? ` body=${responseText.slice(0, 300)}` : '' + responseText + ? ` body=${redactSecrets(responseText.slice(0, 300))}` + : '' }`, ); } catch (error) { diff --git a/apps/worker/src/sandbox-server/lib/harnesses/opencode-server/harness.ts b/apps/worker/src/sandbox-server/lib/harnesses/opencode-server/harness.ts index 79e491b9da..f0718bdaf6 100644 --- a/apps/worker/src/sandbox-server/lib/harnesses/opencode-server/harness.ts +++ b/apps/worker/src/sandbox-server/lib/harnesses/opencode-server/harness.ts @@ -3602,9 +3602,12 @@ export class OpenCodeServerHarness const message = error instanceof Error ? error.message : String(error); const sessionId = 'opencode-session-create-failed'; const timeoutMs = this.client.sessionCreateTimeoutMsValue; + // The request error may carry a slice of the raw OpenCode response body, + // so redact it and render it as code before it reaches the transcript. + const safeMessage = redactSecrets(message).replace(/`/gu, "'"); const userText = message.includes('did not respond within') ? message - : `OpenCode session creation failed before the agent could start.\n\n${message}\n\nOpen the Logs sidebar and inspect harness.log for OpenCode lines (prefixed [opencode-server]).\n\n${formatOpenCodeSessionCreateTimeoutText(timeoutMs)}`; + : `OpenCode session creation failed before the agent could start.\n\n\`\`\`\n${safeMessage}\n\`\`\`\n\nOpen the Logs sidebar and inspect harness.log for OpenCode lines (prefixed [opencode-server]).\n\n${formatOpenCodeSessionCreateTimeoutText(timeoutMs)}`; this.logger.error( `OpenCode initial session create failed; failing the task terminally error=${message}`, diff --git a/packages/compute-providers/src/adapters/modal.ts b/packages/compute-providers/src/adapters/modal.ts index 99241d2484..09aae8d799 100644 --- a/packages/compute-providers/src/adapters/modal.ts +++ b/packages/compute-providers/src/adapters/modal.ts @@ -301,7 +301,7 @@ export class ModalClient implements ComputeProviderClient { return this.resolvedRegistrySecretPromise; } - private async resolveImage(): Promise { + private async resolveImage(signal?: AbortSignal): Promise { if (this.imageMode === 'ecr-oidc') { const secret = await this.getEcrSecret(); return this.sdk.images.fromAwsEcr(this.baseImageRef, secret); @@ -314,6 +314,7 @@ export class ModalClient implements ComputeProviderClient { ref: this.baseImageRef, registryUsername: this.config.registryUsername, registryPassword: this.config.registryPassword, + signal, }); if (this.imageMode === 'registry-auth') { @@ -461,7 +462,7 @@ export class ModalClient implements ComputeProviderClient { } const image = await raceWithAbort({ - promise: this.resolveImage(), + promise: this.resolveImage(input.signal), signal: input.signal, abortMessage: `Resolving Modal image "${this.baseImageRef}" was aborted`, }); diff --git a/packages/compute-providers/src/modal/registry-digest.test.ts b/packages/compute-providers/src/modal/registry-digest.test.ts index 6bcf987738..6f0fcc4313 100644 --- a/packages/compute-providers/src/modal/registry-digest.test.ts +++ b/packages/compute-providers/src/modal/registry-digest.test.ts @@ -338,6 +338,100 @@ describe('pinModalBaseImageRef', () => { ); }); + it('serves the last resolved digest when a refresh fails', async () => { + let fail = false; + const fetchImpl = vi.fn(async () => { + if (fail) throw new Error('network down'); + return response({ + status: 200, + headers: { 'docker-content-digest': DIGEST }, + }); + }) as unknown as typeof fetch; + let now = 1_000; + const pin = () => + pinModalBaseImageRef({ + ref: 'ghcr.io/roocodeinc/roomote-worker:develop', + fetchImpl, + now: () => now, + }); + + await expect(pin()).resolves.toBe( + `ghcr.io/roocodeinc/roomote-worker@${DIGEST}`, + ); + + fail = true; + now += 120_000; + await expect(pin()).resolves.toBe( + `ghcr.io/roocodeinc/roomote-worker@${DIGEST}`, + ); + expect(fetchImpl).toHaveBeenCalledTimes(2); + expect(console.warn).toHaveBeenCalledWith( + expect.stringContaining('using last resolved digest'), + ); + }); + + it('caches failures briefly instead of retrying on every call', async () => { + const fetchImpl = vi.fn(async () => { + throw new Error('network down'); + }) as unknown as typeof fetch; + let now = 1_000; + const pin = () => + pinModalBaseImageRef({ + ref: 'ghcr.io/roocodeinc/roomote-worker:develop', + fetchImpl, + now: () => now, + }); + + await expect(pin()).resolves.toBe( + 'ghcr.io/roocodeinc/roomote-worker:develop', + ); + await expect(pin()).resolves.toBe( + 'ghcr.io/roocodeinc/roomote-worker:develop', + ); + expect(fetchImpl).toHaveBeenCalledTimes(1); + + now += 20_000; + await pin(); + expect(fetchImpl).toHaveBeenCalledTimes(2); + }); + + it('aborts the lookup with the caller signal and does not cache the abort', async () => { + const controller = new AbortController(); + const fetchImpl = vi.fn( + (_input: string | URL | Request, init?: RequestInit) => + new Promise((_resolve, reject) => { + init?.signal?.addEventListener('abort', () => + reject(init.signal?.reason ?? new Error('aborted')), + ); + }), + ) as unknown as typeof fetch; + + const pending = pinModalBaseImageRef({ + ref: 'ghcr.io/roocodeinc/roomote-worker:develop', + fetchImpl, + signal: controller.signal, + }); + controller.abort(); + await expect(pending).resolves.toBe( + 'ghcr.io/roocodeinc/roomote-worker:develop', + ); + expect(console.warn).not.toHaveBeenCalled(); + + const ok = vi.fn(async () => + response({ + status: 200, + headers: { 'docker-content-digest': DIGEST }, + }), + ) as unknown as typeof fetch; + await expect( + pinModalBaseImageRef({ + ref: 'ghcr.io/roocodeinc/roomote-worker:develop', + fetchImpl: ok, + }), + ).resolves.toBe(`ghcr.io/roocodeinc/roomote-worker@${DIGEST}`); + expect(ok).toHaveBeenCalledTimes(1); + }); + it('leaves bare local tags and digest refs alone', async () => { const fetchImpl = vi.fn() as unknown as typeof fetch; diff --git a/packages/compute-providers/src/modal/registry-digest.ts b/packages/compute-providers/src/modal/registry-digest.ts index 418f66ef86..b510dcf186 100644 --- a/packages/compute-providers/src/modal/registry-digest.ts +++ b/packages/compute-providers/src/modal/registry-digest.ts @@ -9,6 +9,11 @@ */ const DIGEST_CACHE_TTL_MS = 60_000; +/** + * How long a failed lookup is remembered before the registry is retried, so a + * registry outage costs one timeout per window instead of one per spawn. + */ +const DIGEST_FAILURE_CACHE_TTL_MS = 15_000; const MANIFEST_ACCEPT = [ 'application/vnd.oci.image.index.v1+json', @@ -123,6 +128,8 @@ interface ResolveImageRefDigestOptions { registryPassword?: string; fetchImpl?: typeof fetch; timeoutMs?: number; + /** Caller abort (for example the sandbox spawn being canceled). */ + signal?: AbortSignal; } async function fetchBearerToken( @@ -192,7 +199,10 @@ export async function resolveImageRefDigest( } const fetchImpl = options.fetchImpl ?? globalThis.fetch; - const signal = AbortSignal.timeout(options.timeoutMs ?? 10_000); + const timeoutSignal = AbortSignal.timeout(options.timeoutMs ?? 10_000); + const signal = options.signal + ? AbortSignal.any([timeoutSignal, options.signal]) + : timeoutSignal; const manifestUrl = `https://${parsed.registry}/v2/${parsed.repository}/manifests/${parsed.tag}`; const head = (authorization?: string) => @@ -262,17 +272,28 @@ export async function resolveImageRefDigest( return `${registry}/${parsed.repository}@${digest}`; } -const digestCache = new Map< - string, - { expiresAt: number; promise: Promise } ->(); +interface DigestCacheEntry { + expiresAt: number; + promise: Promise; + /** + * Most recent successfully resolved digest ref for this key. Served when a + * refresh fails so a registry hiccup never silently drops back to the + * mutable tag (which would re-trigger Modal's stale image cache). + */ + lastPinned: string | undefined; +} + +const digestCache = new Map(); /** * Pins a Modal base image ref to its current digest so Modal's image cache key - * tracks the tag. Falls back to the original ref (and logs) when the registry - * cannot be queried, so a registry hiccup never blocks sandbox creation. - * Results are cached briefly per process to keep the lookup cheap on the - * per-spawn path while still picking up new pushes within a minute. + * tracks the tag. When the registry cannot be queried the last successfully + * resolved digest is reused; only when no digest has ever been resolved in + * this process does it fall back to the original tag (and logs), so a + * registry hiccup never blocks sandbox creation. Results are cached briefly + * per process to keep the lookup cheap on the per-spawn path while still + * picking up new pushes within a minute; failures are cached for a shorter + * window so an outage does not cost a full lookup timeout on every spawn. */ export async function pinModalBaseImageRef( options: ResolveImageRefDigestOptions & { now?: () => number }, @@ -290,32 +311,50 @@ export async function pinModalBaseImageRef( return cached.promise; } - const promise = resolveImageRefDigest(options) + const entry: DigestCacheEntry = { + expiresAt: now() + DIGEST_CACHE_TTL_MS, + promise: Promise.resolve(ref), + lastPinned: cached?.lastPinned, + }; + + entry.promise = resolveImageRefDigest({ ...options, ref }) .then((pinned) => { + entry.lastPinned = pinned; console.log( `[ModalClient] Pinned base image ${JSON.stringify({ ref, pinned })}`, ); return pinned; }) .catch((error) => { - digestCache.delete(cacheKey); + const fallback = entry.lastPinned ?? ref; + const errorMessage = + error instanceof Error ? error.message : String(error); + + if (options.signal?.aborted) { + // The caller gave up; let the next spawn retry immediately. + digestCache.delete(cacheKey); + return fallback; + } + + entry.expiresAt = now() + DIGEST_FAILURE_CACHE_TTL_MS; console.warn( - `[ModalClient] Could not resolve base image digest; using tag as-is ${JSON.stringify( - { - ref, - error: error instanceof Error ? error.message : String(error), - }, - )}`, + `[ModalClient] Could not resolve base image digest; ${ + entry.lastPinned + ? 'using last resolved digest' + : 'using tag as-is (Modal may reuse a stale cached image)' + } ${JSON.stringify({ + ref, + fallback, + retryAfterMs: DIGEST_FAILURE_CACHE_TTL_MS, + error: errorMessage, + })}`, ); - return ref; + return fallback; }); - digestCache.set(cacheKey, { - expiresAt: now() + DIGEST_CACHE_TTL_MS, - promise, - }); + digestCache.set(cacheKey, entry); - return promise; + return entry.promise; } /** Clears the per-process digest cache (tests). */ From 9fd82670dbf2641a9b4e2277b2484e20be34c576 Mon Sep 17 00:00:00 2001 From: Matt Rubens <2600+mrubens@users.noreply.github.com> Date: Thu, 10 Sep 2026 13:18:50 -0400 Subject: [PATCH 4/4] fix: harden Modal digest pinning and cover ECR and broker paths Follow-up review fixes for the Modal base image digest pin: - Refuse to send registry credentials to a token realm that is not HTTPS or not on the registry's own site, so a mirror relaying an upstream challenge cannot redirect them. - Parse WWW-Authenticate leniently: bare (unquoted) parameter values and comma-separated challenge lists, picking the Bearer challenge when present and Basic otherwise. - Skip the registry lookup for immutable release tags (develop-, main-, v*, raw SHAs) so production deployments carry no per-spawn registry dependency. - Pin the base image on the broker backend too, and warn once when an ECR ref uses a mutable tag (ECR digests cannot be resolved from the controller). Log the pinned ref when creating a sandbox. - Replace the hand-rolled TTL cache with lru-cache (already a package dependency) using its fetch coalescing, stale-on-rejection, and an injectable clock; drop the test-only `now` option from the production signature and parse each ref once. - Document MODAL_BASE_IMAGE_REF and the tag resolution behavior. --- apps/docs/environment-variables.mdx | 1 + apps/docs/providers/compute/modal.mdx | 12 + .../src/adapters/modal.test.ts | 3 +- .../compute-providers/src/adapters/modal.ts | 51 +++- .../src/adapters/roomote-broker.test.ts | 43 +++ .../src/adapters/roomote-broker.ts | 13 +- .../src/modal/registry-digest.test.ts | 159 +++++++++- .../src/modal/registry-digest.ts | 278 ++++++++++++------ 8 files changed, 447 insertions(+), 113 deletions(-) diff --git a/apps/docs/environment-variables.mdx b/apps/docs/environment-variables.mdx index f169543d2e..9b02152ce7 100644 --- a/apps/docs/environment-variables.mdx +++ b/apps/docs/environment-variables.mdx @@ -263,6 +263,7 @@ as per-task auth tokens or workspace paths. | `MODAL_ENDPOINT` | Optional | Modal endpoint override. | | `MODAL_ENVIRONMENT` | Optional | Modal environment name. | | `MODAL_APP_NAME` | Optional | Modal app name override. | +| `MODAL_BASE_IMAGE_REF` | Optional | Worker image Modal sandboxes start from. Defaults to the published image for the running release. Mutable tags such as `develop` are resolved to their current digest at launch; ECR refs are used as-is, so pin them. | | `MODAL_REGIONS` | Optional | Comma-separated Modal sandbox placement regions (for example `us` or `us-west`). Unset keeps Modal default placement. | | `MODAL_VM_MEMORY_MIB` | Optional | Memory allocated to Modal VM sandboxes used for nested Docker workloads. Defaults to `8192` MiB. | | `E2B_API_KEY` | E2B | E2B API key. Can also be saved from **Settings > Sandboxes**. | diff --git a/apps/docs/providers/compute/modal.mdx b/apps/docs/providers/compute/modal.mdx index 4391b45789..47b31d8df3 100644 --- a/apps/docs/providers/compute/modal.mdx +++ b/apps/docs/providers/compute/modal.mdx @@ -53,6 +53,18 @@ the task. Roomote starts Docker as the VM sandbox's primary service and requests standard sandbox runtime with 2 CPU cores and 4 GiB of memory. Container builds and services share the task sandbox's CPU and memory. +## Worker image + +Modal sandboxes start from the published Roomote worker image for the running +release. Override it with `MODAL_BASE_IMAGE_REF` to use a fork or registry +mirror. Modal caches images by their reference string, so a mutable tag such as +`develop` or `latest` would never be re-pulled after the first launch. Roomote +resolves mutable tags to their current digest at launch time and reuses the +last resolved digest if the registry is briefly unreachable. Immutable tags +(`develop-`, `main-`, `v*`) and `@sha256` references are used as-is. +ECR references are not resolved because the Roomote server holds no AWS +credentials of its own; pin a digest or enable ECR tag immutability there. + ## Verify setup 1. save Modal credentials diff --git a/packages/compute-providers/src/adapters/modal.test.ts b/packages/compute-providers/src/adapters/modal.test.ts index 1053247fe4..fcb9617cd6 100644 --- a/packages/compute-providers/src/adapters/modal.test.ts +++ b/packages/compute-providers/src/adapters/modal.test.ts @@ -29,7 +29,8 @@ const { pinModalBaseImageRefMock } = vi.hoisted(() => ({ pinModalBaseImageRefMock: vi.fn(), })); -vi.mock('../modal/registry-digest', () => ({ +vi.mock('../modal/registry-digest', async (importOriginal) => ({ + ...(await importOriginal()), pinModalBaseImageRef: pinModalBaseImageRefMock, })); diff --git a/packages/compute-providers/src/adapters/modal.ts b/packages/compute-providers/src/adapters/modal.ts index 09aae8d799..97fc8e32a1 100644 --- a/packages/compute-providers/src/adapters/modal.ts +++ b/packages/compute-providers/src/adapters/modal.ts @@ -40,11 +40,40 @@ import { toAbortError, throwIfAborted, } from '../modal/abort'; -import { pinModalBaseImageRef } from '../modal/registry-digest'; +import { + isImmutableImageTag, + parseImageRef, + pinModalBaseImageRef, +} from '../modal/registry-digest'; import { normalizeModalRpcError } from '../modal/rpc-diagnostics'; const DEFAULT_APP_NAME = 'roomote'; +const warnedUnpinnedEcrRefs = new Set(); + +/** + * ECR digests cannot be resolved from the controller: the OCI token flow does + * not apply, and the controller holds only the OIDC role Modal assumes, not + * AWS credentials of its own for `ecr:GetAuthorizationToken`. Modal keys its + * image cache on the ref string, so a mutable ECR tag will not be re-pulled + * after the first build. Warn once per process so operators know to pin. + */ +function warnUnpinnedEcrBaseImageRef(ref: string): void { + const parsed = parseImageRef(ref); + if (!parsed || parsed.digest || isImmutableImageTag(parsed.tag)) { + return; + } + if (warnedUnpinnedEcrRefs.has(ref)) { + return; + } + warnedUnpinnedEcrRefs.add(ref); + console.warn( + `[ModalClient] ECR base image uses a mutable tag; Modal will not re-pull it after the first build. Pin an @sha256 digest or an immutable tag, or enable ECR tag immutability ${JSON.stringify( + { ref }, + )}`, + ); +} + const DEFAULT_MODAL_WORKDIR = '/sandbox'; const MODAL_VM_DOCKER_COMMAND = [ '/usr/bin/sudo', @@ -301,10 +330,16 @@ export class ModalClient implements ComputeProviderClient { return this.resolvedRegistrySecretPromise; } - private async resolveImage(signal?: AbortSignal): Promise { + private async resolveImage( + signal?: AbortSignal, + ): Promise<{ image: Image; imageRef: string }> { if (this.imageMode === 'ecr-oidc') { + warnUnpinnedEcrBaseImageRef(this.baseImageRef); const secret = await this.getEcrSecret(); - return this.sdk.images.fromAwsEcr(this.baseImageRef, secret); + return { + image: this.sdk.images.fromAwsEcr(this.baseImageRef, secret), + imageRef: this.baseImageRef, + }; } // Modal keys its image cache on the ref string, so a mutable tag such as @@ -319,10 +354,13 @@ export class ModalClient implements ComputeProviderClient { if (this.imageMode === 'registry-auth') { const secret = await this.getRegistrySecret(); - return this.sdk.images.fromRegistry(imageRef, secret); + return { + image: this.sdk.images.fromRegistry(imageRef, secret), + imageRef, + }; } - return this.sdk.images.fromRegistry(imageRef); + return { image: this.sdk.images.fromRegistry(imageRef), imageRef }; } private normalizeSandboxTags( @@ -461,7 +499,7 @@ export class ModalClient implements ComputeProviderClient { throw error; } - const image = await raceWithAbort({ + const { image, imageRef } = await raceWithAbort({ promise: this.resolveImage(input.signal), signal: input.signal, abortMessage: `Resolving Modal image "${this.baseImageRef}" was aborted`, @@ -472,6 +510,7 @@ export class ModalClient implements ComputeProviderClient { try { console.log( `[ModalClient] Creating sandbox... ${JSON.stringify({ + imageRef, encryptedPorts: input.ports, regions: this.config.regions ?? '(default)', cpu: this.config.cpu ?? '(default)', diff --git a/packages/compute-providers/src/adapters/roomote-broker.test.ts b/packages/compute-providers/src/adapters/roomote-broker.test.ts index b2350bedd1..576e4a563a 100644 --- a/packages/compute-providers/src/adapters/roomote-broker.test.ts +++ b/packages/compute-providers/src/adapters/roomote-broker.test.ts @@ -2,6 +2,15 @@ import { createHash, createHmac } from 'node:crypto'; import { BrokerRequestError, RoomoteBrokerClient } from './roomote-broker'; +const { pinModalBaseImageRefMock } = vi.hoisted(() => ({ + pinModalBaseImageRefMock: vi.fn(), +})); + +vi.mock('../modal/registry-digest', async (importOriginal) => ({ + ...(await importOriginal()), + pinModalBaseImageRef: pinModalBaseImageRefMock, +})); + const brokerUrl = 'https://broker.roomote.dev'; const tenantId = '9d137fea-a018-4432-af24-83ce802b4ed2'; const brokerKey = 'rbk_derived-tenant-credential'; @@ -70,6 +79,40 @@ function harness( } describe('RoomoteBrokerClient', () => { + beforeEach(() => { + pinModalBaseImageRefMock.mockImplementation( + async ({ ref }: { ref: string }) => ref, + ); + }); + + it('pins a mutable base image tag before launching a fresh sandbox', async () => { + const pinned = `ghcr.io/roocodeinc/roomote-worker@sha256:${'b'.repeat(64)}`; + pinModalBaseImageRefMock.mockResolvedValue(pinned); + const requests: Array<{ body: unknown }> = []; + const client = new RoomoteBrokerClient({ + brokerUrl: 'http://localhost:4100/compute-broker', + tenantId, + brokerKey, + baseImageRef: 'ghcr.io/roocodeinc/roomote-worker:develop', + fetchImpl: (async ( + _input: string | URL | Request, + init?: RequestInit, + ) => { + requests.push({ body: JSON.parse(String(init?.body ?? 'null')) }); + return jsonResponse({ instanceId: 'sb-2', domains: {} }); + }) as unknown as typeof fetch, + }); + + await client.createInstance({}); + + expect(pinModalBaseImageRefMock).toHaveBeenCalledWith( + expect.objectContaining({ + ref: 'ghcr.io/roocodeinc/roomote-worker:develop', + }), + ); + expect(requests[0]?.body).toMatchObject({ imageRef: pinned }); + }); + it('signs every request with the tenant HMAC scheme', async () => { const { client, requests } = harness(() => jsonResponse({ instances: [] })); diff --git a/packages/compute-providers/src/adapters/roomote-broker.ts b/packages/compute-providers/src/adapters/roomote-broker.ts index 72ea81a8de..e2afc25990 100644 --- a/packages/compute-providers/src/adapters/roomote-broker.ts +++ b/packages/compute-providers/src/adapters/roomote-broker.ts @@ -28,6 +28,7 @@ import type { } from '../types'; import { unsupported } from '../errors'; import { sleepWithSignal, throwIfAborted, toAbortError } from '../modal/abort'; +import { pinModalBaseImageRef } from '../modal/registry-digest'; import { RoomoteBrokerExec } from './roomote-broker-exec'; import { BrokerRequestError, @@ -283,10 +284,16 @@ export class RoomoteBrokerClient implements ComputeProviderClient { throwIfAborted(input.signal); const idempotencyKey = input.idempotencyKey ?? randomUUID(); + // Same mutable-tag pitfall as the direct Modal adapter: the broker keys + // its image cache on the ref string, so pin the tag to its digest first. + const imageRef = sourceSnapshotId + ? undefined + : await pinModalBaseImageRef({ + ref: this.config.baseImageRef, + signal: input.signal, + }); const body = JSON.stringify({ - ...(sourceSnapshotId - ? { snapshotId: sourceSnapshotId } - : { imageRef: this.config.baseImageRef }), + ...(sourceSnapshotId ? { snapshotId: sourceSnapshotId } : { imageRef }), ...(input.ports?.length ? { ports: input.ports } : {}), ...(input.tags && Object.keys(input.tags).length > 0 ? { tags: input.tags } diff --git a/packages/compute-providers/src/modal/registry-digest.test.ts b/packages/compute-providers/src/modal/registry-digest.test.ts index 6f0fcc4313..11789ba405 100644 --- a/packages/compute-providers/src/modal/registry-digest.test.ts +++ b/packages/compute-providers/src/modal/registry-digest.test.ts @@ -1,7 +1,9 @@ import { - clearModalBaseImageDigestCache, + isImmutableImageTag, parseImageRef, + parseWwwAuthenticate, pinModalBaseImageRef, + resetModalBaseImageDigestCache, resolveImageRefDigest, } from './registry-digest'; @@ -77,6 +79,57 @@ describe('parseImageRef', () => { }); }); +describe('parseWwwAuthenticate', () => { + it('parses quoted and bare parameter values', () => { + expect( + parseWwwAuthenticate( + 'Bearer realm=https://r.example/token,service=r.example,scope="repository:a/b:pull"', + ), + ).toEqual([ + { + scheme: 'bearer', + params: { + realm: 'https://r.example/token', + service: 'r.example', + scope: 'repository:a/b:pull', + }, + }, + ]); + }); + + it('splits comma-separated challenge lists', () => { + expect( + parseWwwAuthenticate( + 'Bearer realm="https://a.example/token",service="a", Basic realm="Registry Realm"', + ), + ).toEqual([ + { + scheme: 'bearer', + params: { realm: 'https://a.example/token', service: 'a' }, + }, + { scheme: 'basic', params: { realm: 'Registry Realm' } }, + ]); + }); +}); + +describe('isImmutableImageTag', () => { + it('recognizes release-pipeline tags and commit SHAs', () => { + expect(isImmutableImageTag('develop-0a1b2c3d')).toBe(true); + expect(isImmutableImageTag('main-0a1b2c3d')).toBe(true); + expect(isImmutableImageTag('v1.3.0')).toBe(true); + expect(isImmutableImageTag('v1.3.0-rc.1')).toBe(true); + expect(isImmutableImageTag('a'.repeat(40))).toBe(true); + }); + + it('treats channel aliases and custom tags as mutable', () => { + expect(isImmutableImageTag('develop')).toBe(false); + expect(isImmutableImageTag('main')).toBe(false); + expect(isImmutableImageTag('latest')).toBe(false); + expect(isImmutableImageTag('arm64-real')).toBe(false); + expect(isImmutableImageTag(undefined)).toBe(false); + }); +}); + describe('resolveImageRefDigest', () => { it('follows the bearer challenge and returns the digest-pinned ref', async () => { const calls: Array<{ url: string; method: string; auth: string | null }> = @@ -250,6 +303,78 @@ describe('resolveImageRefDigest', () => { expect(fetchImpl).toHaveBeenCalledTimes(1); }); + it('refuses to send credentials to a token realm on another site', async () => { + const fetchImpl = vi.fn(async (input: string | URL | Request) => { + if (String(input).includes('ghcr.io/token')) { + throw new Error('credentials must not reach the foreign realm'); + } + return response({ + status: 401, + headers: { + 'www-authenticate': + 'Bearer realm="https://ghcr.io/token",service="ghcr.io"', + }, + }); + }) as unknown as typeof fetch; + + await expect( + resolveImageRefDigest({ + ref: 'mirror.example.com/roomote/worker:develop', + registryUsername: 'user', + registryPassword: 'pass', + fetchImpl, + }), + ).rejects.toThrow(/refusing to send registry credentials/u); + expect(fetchImpl).toHaveBeenCalledTimes(1); + }); + + it('refuses plaintext token realms', async () => { + const fetchImpl = vi.fn(async () => + response({ + status: 401, + headers: { + 'www-authenticate': + 'Bearer realm="http://registry.example.com/token",service="registry.example.com"', + }, + }), + ) as unknown as typeof fetch; + + await expect( + resolveImageRefDigest({ + ref: 'registry.example.com/roomote/worker:develop', + fetchImpl, + }), + ).rejects.toThrow(/refusing to request a registry token over http:/u); + }); + + it('accepts a token realm on a sibling host of the same site', async () => { + const fetchImpl = vi.fn( + async (input: string | URL | Request, init?: RequestInit) => { + const url = String(input); + if (url.startsWith('https://auth.docker.io/token')) { + return response({ status: 200, body: { token: 'hub-token' } }); + } + if (new Headers(init?.headers).get('authorization')) { + return response({ + status: 200, + headers: { 'docker-content-digest': DIGEST }, + }); + } + return response({ + status: 401, + headers: { + 'www-authenticate': + 'Bearer realm="https://auth.docker.io/token",service="registry.docker.io"', + }, + }); + }, + ) as unknown as typeof fetch; + + await expect( + resolveImageRefDigest({ ref: 'docker.io/roomote/worker:1.0', fetchImpl }), + ).resolves.toBe(`docker.io/roomote/worker@${DIGEST}`); + }); + it('returns digest-pinned refs unchanged without touching the registry', async () => { const fetchImpl = vi.fn() as unknown as typeof fetch; const ref = `ghcr.io/roocodeinc/roomote-worker@${DIGEST}`; @@ -279,13 +404,17 @@ describe('resolveImageRefDigest', () => { }); describe('pinModalBaseImageRef', () => { + let now = 1_000; + beforeEach(() => { - clearModalBaseImageDigestCache(); + now = 1_000; + resetModalBaseImageDigestCache({ now: () => now }); vi.spyOn(console, 'log').mockImplementation(() => {}); vi.spyOn(console, 'warn').mockImplementation(() => {}); }); afterEach(() => { + resetModalBaseImageDigestCache(); vi.restoreAllMocks(); }); @@ -296,17 +425,14 @@ describe('pinModalBaseImageRef', () => { headers: { 'docker-content-digest': DIGEST }, }), ) as unknown as typeof fetch; - let now = 1_000; const first = await pinModalBaseImageRef({ ref: 'ghcr.io/roocodeinc/roomote-worker:develop', fetchImpl, - now: () => now, }); const second = await pinModalBaseImageRef({ ref: 'ghcr.io/roocodeinc/roomote-worker:develop', fetchImpl, - now: () => now, }); expect(first).toBe(`ghcr.io/roocodeinc/roomote-worker@${DIGEST}`); @@ -317,7 +443,6 @@ describe('pinModalBaseImageRef', () => { await pinModalBaseImageRef({ ref: 'ghcr.io/roocodeinc/roomote-worker:develop', fetchImpl, - now: () => now, }); expect(fetchImpl).toHaveBeenCalledTimes(2); }); @@ -347,12 +472,10 @@ describe('pinModalBaseImageRef', () => { headers: { 'docker-content-digest': DIGEST }, }); }) as unknown as typeof fetch; - let now = 1_000; const pin = () => pinModalBaseImageRef({ ref: 'ghcr.io/roocodeinc/roomote-worker:develop', fetchImpl, - now: () => now, }); await expect(pin()).resolves.toBe( @@ -374,12 +497,10 @@ describe('pinModalBaseImageRef', () => { const fetchImpl = vi.fn(async () => { throw new Error('network down'); }) as unknown as typeof fetch; - let now = 1_000; const pin = () => pinModalBaseImageRef({ ref: 'ghcr.io/roocodeinc/roomote-worker:develop', fetchImpl, - now: () => now, }); await expect(pin()).resolves.toBe( @@ -432,6 +553,24 @@ describe('pinModalBaseImageRef', () => { expect(ok).toHaveBeenCalledTimes(1); }); + it('skips the registry for immutable release tags', async () => { + const fetchImpl = vi.fn() as unknown as typeof fetch; + + await expect( + pinModalBaseImageRef({ + ref: 'ghcr.io/roocodeinc/roomote-worker:develop-0a1b2c3d', + fetchImpl, + }), + ).resolves.toBe('ghcr.io/roocodeinc/roomote-worker:develop-0a1b2c3d'); + await expect( + pinModalBaseImageRef({ + ref: 'ghcr.io/roocodeinc/roomote-worker:v1.3.0', + fetchImpl, + }), + ).resolves.toBe('ghcr.io/roocodeinc/roomote-worker:v1.3.0'); + expect(fetchImpl).not.toHaveBeenCalled(); + }); + it('leaves bare local tags and digest refs alone', async () => { const fetchImpl = vi.fn() as unknown as typeof fetch; diff --git a/packages/compute-providers/src/modal/registry-digest.ts b/packages/compute-providers/src/modal/registry-digest.ts index b510dcf186..9a387e44c5 100644 --- a/packages/compute-providers/src/modal/registry-digest.ts +++ b/packages/compute-providers/src/modal/registry-digest.ts @@ -8,6 +8,8 @@ * Modal pull the new image instead of silently reusing a months-old build. */ +import { LRUCache } from 'lru-cache'; + const DIGEST_CACHE_TTL_MS = 60_000; /** * How long a failed lookup is remembered before the registry is retried, so a @@ -26,6 +28,19 @@ const DIGEST_PATTERN = /^sha256:[a-f0-9]{64}$/u; const DOCKER_HUB_REGISTRY = 'registry-1.docker.io'; +/** + * Tags the release pipeline publishes exactly once and never moves: + * `develop-` / `main-` channel builds, `v*` releases, and raw + * commit SHAs. These never need a registry lookup to stay fresh. + */ +const IMMUTABLE_TAG_PATTERN = + /^(?:(?:develop|main)-[0-9a-f]{7,40}|v\d+\.\d+\.\d+(?:[-+][0-9A-Za-z.-]+)?|[0-9a-f]{40})$/u; + +/** True for tags that are published once and never re-pointed. */ +export function isImmutableImageTag(tag: string | undefined): boolean { + return tag !== undefined && IMMUTABLE_TAG_PATTERN.test(tag); +} + interface ParsedImageRef { /** Registry host, e.g. `ghcr.io` or `registry-1.docker.io`. */ registry: string; @@ -97,21 +112,61 @@ export function parseImageRef(ref: string): ParsedImageRef | null { }; } -function parseWwwAuthenticate( - header: string, -): { realm: string; params: Record } | null { - const match = /^Bearer\s+(.*)$/iu.exec(header.trim()); - if (!match) return null; +interface AuthChallenge { + scheme: string; + params: Record; +} + +/** + * Parses a `WWW-Authenticate` header into its challenges. Handles quoted and + * bare (RFC 7235 token) parameter values and comma-separated challenge lists + * such as `Bearer realm="...",service="...", Basic realm="..."`. Values are + * only used when a challenge matches a scheme we know how to satisfy. + */ +export function parseWwwAuthenticate(header: string): AuthChallenge[] { + const challenges: AuthChallenge[] = []; + let current: AuthChallenge | undefined; - const params: Record = {}; - for (const part of match[1]!.matchAll(/(\w+)="([^"]*)"/gu)) { - params[part[1]!] = part[2]!; + const token = + /([A-Za-z][A-Za-z0-9._~+/-]*)(?:\s*=\s*(?:"((?:[^"\\]|\\.)*)"|([^\s,"]+)))?/gu; + for (const match of header.matchAll(token)) { + const [, name, quoted, bare] = match; + if (quoted === undefined && bare === undefined) { + // A bare word (no `=`) starts a new challenge with that scheme. + current = { scheme: name!.toLowerCase(), params: {} }; + challenges.push(current); + continue; + } + if (!current) continue; + current.params[name!.toLowerCase()] = + quoted !== undefined ? quoted.replace(/\\(.)/gu, '$1') : bare!; } - const realm = params.realm; - if (!realm) return null; - delete params.realm; - return { realm, params }; + return challenges; +} + +function registrySite(host: string): string { + const labels = host.toLowerCase().replace(/:\d+$/u, '').split('.'); + return labels.slice(-2).join('.'); +} + +/** + * Refuses to present registry credentials to a token endpoint the registry + * did not plausibly own: the realm must be HTTPS and share the registry's + * site (`registry-1.docker.io` -> `auth.docker.io` is fine; a mirror relaying + * an upstream `ghcr.io` challenge, or an attacker-controlled realm, is not). + */ +function assertTrustedTokenRealm(realm: URL, registry: string): void { + if (realm.protocol !== 'https:') { + throw new Error( + `refusing to request a registry token over ${realm.protocol} from ${realm.origin}`, + ); + } + if (registrySite(realm.hostname) !== registrySite(registry)) { + throw new Error( + `refusing to send registry credentials for ${registry} to token realm ${realm.origin}`, + ); + } } function basicAuthorization( @@ -133,19 +188,24 @@ interface ResolveImageRefDigestOptions { } async function fetchBearerToken( - challenge: { realm: string; params: Record }, - repository: string, + challenge: AuthChallenge, + parsed: ParsedImageRef, options: ResolveImageRefDigestOptions, fetchImpl: typeof fetch, signal: AbortSignal, ): Promise { - const url = new URL(challenge.realm); - for (const [key, value] of Object.entries(challenge.params)) { + const { realm, scope: _ignoredScope, ...params } = challenge.params; + if (!realm) { + throw new Error('registry Bearer challenge did not include a realm'); + } + const url = new URL(realm); + assertTrustedTokenRealm(url, parsed.registry); + for (const [key, value] of Object.entries(params)) { url.searchParams.set(key, value); } // Some registries (ghcr.io) answer a rejected Basic header with a // placeholder scope, so always ask for the repository we actually need. - url.searchParams.set('scope', `repository:${repository}:pull`); + url.searchParams.set('scope', `repository:${parsed.repository}:pull`); const authorization = basicAuthorization( options.registryUsername, @@ -197,7 +257,13 @@ export async function resolveImageRefDigest( if (parsed.digest) { return options.ref.trim(); } + return resolveParsedImageRefDigest(parsed, options); +} +async function resolveParsedImageRefDigest( + parsed: ParsedImageRef, + options: ResolveImageRefDigestOptions, +): Promise { const fetchImpl = options.fetchImpl ?? globalThis.fetch; const timeoutSignal = AbortSignal.timeout(options.timeoutMs ?? 10_000); const signal = options.signal @@ -220,10 +286,24 @@ export async function resolveImageRefDigest( let response = await head(); if (response.status === 401) { - const challengeHeader = response.headers.get('www-authenticate') ?? ''; - const scheme = challengeHeader.trim().split(/\s+/u)[0]?.toLowerCase(); + const challenges = parseWwwAuthenticate( + response.headers.get('www-authenticate') ?? '', + ); + const bearer = challenges.find( + (challenge) => challenge.scheme === 'bearer' && challenge.params.realm, + ); + const basic = challenges.find((challenge) => challenge.scheme === 'basic'); - if (scheme === 'basic') { + if (bearer) { + const token = await fetchBearerToken( + bearer, + parsed, + options, + fetchImpl, + signal, + ); + response = await head(`Bearer ${token}`); + } else if (basic) { // Docker Distribution registries secured with htpasswd-style auth // challenge with Basic directly; there is no token endpoint to call. const authorization = basicAuthorization( @@ -237,20 +317,9 @@ export async function resolveImageRefDigest( } response = await head(authorization); } else { - const challenge = parseWwwAuthenticate(challengeHeader); - if (!challenge) { - throw new Error( - `registry returned 401 for ${manifestUrl} without a usable challenge`, - ); - } - const token = await fetchBearerToken( - challenge, - parsed.repository, - options, - fetchImpl, - signal, + throw new Error( + `registry returned 401 for ${manifestUrl} without a usable challenge`, ); - response = await head(`Bearer ${token}`); } } @@ -272,92 +341,115 @@ export async function resolveImageRefDigest( return `${registry}/${parsed.repository}@${digest}`; } -interface DigestCacheEntry { - expiresAt: number; - promise: Promise; - /** - * Most recent successfully resolved digest ref for this key. Served when a - * refresh fails so a registry hiccup never silently drops back to the - * mutable tag (which would re-trigger Modal's stale image cache). - */ - lastPinned: string | undefined; +interface DigestFetchContext { + ref: string; + parsed: ParsedImageRef; + options: ResolveImageRefDigestOptions; } -const digestCache = new Map(); +let cacheClock: () => number = Date.now; /** - * Pins a Modal base image ref to its current digest so Modal's image cache key - * tracks the tag. When the registry cannot be queried the last successfully - * resolved digest is reused; only when no digest has ever been resolved in - * this process does it fall back to the original tag (and logs), so a - * registry hiccup never blocks sandbox creation. Results are cached briefly - * per process to keep the lookup cheap on the per-spawn path while still - * picking up new pushes within a minute; failures are cached for a shorter - * window so an outage does not cost a full lookup timeout on every spawn. + * Per-process digest cache keyed by ref + registry user. `fetch()` coalesces + * concurrent lookups for the same key, and a stale (expired) entry is handed + * back to `fetchMethod` so a failed refresh can keep serving the last good + * digest instead of dropping back to the mutable tag. */ -export async function pinModalBaseImageRef( - options: ResolveImageRefDigestOptions & { now?: () => number }, -): Promise { - const ref = options.ref.trim(); - const parsed = parseImageRef(ref); - if (!parsed || parsed.digest) { - return ref; - } - - const now = options.now ?? Date.now; - const cacheKey = `${ref} ${options.registryUsername ?? ''}`; - const cached = digestCache.get(cacheKey); - if (cached && cached.expiresAt > now()) { - return cached.promise; - } - - const entry: DigestCacheEntry = { - expiresAt: now() + DIGEST_CACHE_TTL_MS, - promise: Promise.resolve(ref), - lastPinned: cached?.lastPinned, - }; - - entry.promise = resolveImageRefDigest({ ...options, ref }) - .then((pinned) => { - entry.lastPinned = pinned; +const digestCache = new LRUCache({ + max: 32, + ttl: DIGEST_CACHE_TTL_MS, + // A handful of entries read on the spawn path; skip the perf.now() debounce + // so expiry follows the clock exactly (and the fake clock in tests). + ttlResolution: 0, + perf: { now: () => cacheClock() }, + // A rejected refresh (caller abort) must neither drop the last good digest + // nor fail callers who can be served from it. + noDeleteOnFetchRejection: true, + allowStaleOnFetchRejection: true, + fetchMethod: async ( + _key, + staleValue, + { options: entryOptions, signal, context }, + ) => { + const { ref, parsed, options } = context; + try { + const pinned = await resolveParsedImageRefDigest(parsed, { + ...options, + ref, + signal: options.signal + ? AbortSignal.any([signal, options.signal]) + : signal, + }); console.log( `[ModalClient] Pinned base image ${JSON.stringify({ ref, pinned })}`, ); return pinned; - }) - .catch((error) => { - const fallback = entry.lastPinned ?? ref; - const errorMessage = - error instanceof Error ? error.message : String(error); - + } catch (error) { if (options.signal?.aborted) { - // The caller gave up; let the next spawn retry immediately. - digestCache.delete(cacheKey); - return fallback; + // The caller gave up; keep the stale entry (if any) and let the next + // spawn retry immediately. + throw error; } - entry.expiresAt = now() + DIGEST_FAILURE_CACHE_TTL_MS; + const lastPinned = + staleValue && parseImageRef(staleValue)?.digest + ? staleValue + : undefined; + const fallback = lastPinned ?? ref; + // Remember the failure briefly so an outage costs one lookup timeout + // per window instead of one per spawn. + entryOptions.ttl = DIGEST_FAILURE_CACHE_TTL_MS; console.warn( `[ModalClient] Could not resolve base image digest; ${ - entry.lastPinned + lastPinned ? 'using last resolved digest' : 'using tag as-is (Modal may reuse a stale cached image)' } ${JSON.stringify({ ref, fallback, retryAfterMs: DIGEST_FAILURE_CACHE_TTL_MS, - error: errorMessage, + error: error instanceof Error ? error.message : String(error), })}`, ); return fallback; - }); + } + }, +}); - digestCache.set(cacheKey, entry); +/** + * Pins a Modal base image ref to its current digest so Modal's image cache key + * tracks the tag. Refs that are already digest-pinned, not registry-qualified, + * or carry an immutable release tag are returned unchanged without touching + * the registry. When the registry cannot be queried the last successfully + * resolved digest is reused; only when no digest has ever been resolved in + * this process does it fall back to the original tag (and logs), so a + * registry hiccup never blocks sandbox creation. + */ +export async function pinModalBaseImageRef( + options: ResolveImageRefDigestOptions, +): Promise { + const ref = options.ref.trim(); + const parsed = parseImageRef(ref); + if (!parsed || parsed.digest || isImmutableImageTag(parsed.tag)) { + return ref; + } - return entry.promise; + const cacheKey = `${ref} ${options.registryUsername ?? ''}`; + try { + const pinned = await digestCache.fetch(cacheKey, { + context: { ref, parsed, options }, + }); + return pinned ?? ref; + } catch { + // Only reachable when the caller aborted and nothing was cached yet. + return ref; + } } -/** Clears the per-process digest cache (tests). */ -export function clearModalBaseImageDigestCache(): void { +/** Resets the per-process digest cache, optionally with a fake clock (tests). */ +export function resetModalBaseImageDigestCache(options?: { + now?: () => number; +}): void { + cacheClock = options?.now ?? Date.now; digestCache.clear(); }