diff --git a/layers/artifact-delivery/server/utils/build.ts b/layers/artifact-delivery/server/utils/build.ts index 7bdc5ecc..9d45f9e1 100644 --- a/layers/artifact-delivery/server/utils/build.ts +++ b/layers/artifact-delivery/server/utils/build.ts @@ -46,6 +46,7 @@ import { LEADING_BUILD_FRESH_SECONDS, parseCheckResults, publishArtifactRecord, + shareReusedAttestation, transitionResolution, } from './state' import { readDeterministicUstar } from './ustar' @@ -311,6 +312,21 @@ export async function processArtifactBuild( if (loaded._tag === 'rejected') return await failWithRejection(dependencies, row, loaded.rejection) carried = loaded + if (loaded._tag === 'reused' && !reusedContentChanged(content, loaded.build)) { + // A reused build serves the attestation of the build it reused and + // stores no copy. If that attestation is gone, it signs its own. + const shared = await shareReusedAttestation(dependencies.db, row, { + attestationResolutionId: loaded.build.resolutionId, + r2Key: artifactR2Key(content.contentSha256), + now, + }) + if (shared._tag === 'superseded') + return { _tag: 'superseded', resolutionId } + if (shared._tag === 'advanced') { + row = shared.row + continue + } + } const statement = loaded._tag === 'reused' ? stageReusedStatement(row, content, loaded.build) : await stageLoadedStatement(dependencies, row, content, loaded) @@ -343,11 +359,12 @@ export async function processArtifactBuild( } if (row.state === 'publishing') { - const attestation = artifactAttestationSchema.parse(JSON.parse(row.attestation_json ?? 'null')) const published = await publishArtifactRecord(dependencies.db, { row, checks: parseCheckResults(row.check_results_json), - attestation, + attestation: row.attestation_resolution_id + ? { _tag: 'shared' } + : { _tag: 'signed', attestation: artifactAttestationSchema.parse(JSON.parse(row.attestation_json ?? 'null')) }, now, }) return published._tag === 'superseded' @@ -444,7 +461,9 @@ async function stageLoadedStatement( } /** - * Stage a new statement over bytes that R2 already holds. + * Stage a new statement over bytes that R2 already holds. A reused build + * comes here only when the build it reused no longer holds its attestation; + * otherwise it serves that one (see `shareReusedAttestation`). * * The statement names this Resolution's source and creation time, with the * files and check results of the reused build. The signer then signs it @@ -457,13 +476,8 @@ function stageReusedStatement( content: { artifactId: string, contentSha256: string, contentBytes: number }, build: ReusableBuild, ): StagedStatement { - if ( - build.artifactId !== content.artifactId - || build.contentSha256 !== content.contentSha256 - || build.contentBytes !== content.contentBytes - ) { + if (reusedContentChanged(content, build)) return { _tag: 'content-changed' } - } const statement = createAttestationStatement({ artifactId: content.artifactId, createdAt: new Date(row.created_at * 1000).toISOString(), @@ -483,6 +497,16 @@ function stageReusedStatement( } } +/** Whether a reused build holds other bytes than this Resolution recorded. */ +function reusedContentChanged( + content: { artifactId: string, contentSha256: string, contentBytes: number }, + build: ReusableBuild, +): boolean { + return build.artifactId !== content.artifactId + || build.contentSha256 !== content.contentSha256 + || build.contentBytes !== content.contentBytes +} + /** * A pinned public request, before any GitHub read: reuse a ready build of its * commit, check stored bytes again, or wait for an earlier build of it. diff --git a/layers/artifact-delivery/server/utils/grant.ts b/layers/artifact-delivery/server/utils/grant.ts index 5d73bf12..ce7636b9 100644 --- a/layers/artifact-delivery/server/utils/grant.ts +++ b/layers/artifact-delivery/server/utils/grant.ts @@ -50,6 +50,8 @@ export interface PublicGrantDependencies { /** * Every grant reads current D1 check results before it reveals cached bytes. + * A named Resolution that reused a build is granted under its own state and + * checks, with the attestation of the build it names. * The public R2 URL is immutable. A cached object cannot create a fresh grant. */ export async function createPublicArtifactGrant( @@ -60,13 +62,13 @@ export async function createPublicArtifactGrant( const rowStatement = dependencies.db.prepare( `SELECT a.id, a.content_sha256, a.content_bytes, a.r2_key, a.delivery_status, - aa.resolution_id, aa.attestation_json, + r.id AS resolution_id, aa.attestation_json, r.state AS resolution_state FROM artifacts a JOIN artifact_attestations aa ON aa.artifact_id = a.id - JOIN artifact_resolutions r ON r.id = aa.resolution_id + JOIN artifact_resolutions r ON r.id = COALESCE(?2, aa.resolution_id) WHERE a.id = ?1 - AND (?2 IS NULL OR aa.resolution_id = ?2) + AND aa.resolution_id = COALESCE(r.attestation_resolution_id, r.id) ORDER BY aa.created_at DESC, aa.resolution_id DESC LIMIT 1`, ).bind(artifactId, resolutionId ?? null) diff --git a/layers/artifact-delivery/server/utils/resolution-retention.ts b/layers/artifact-delivery/server/utils/resolution-retention.ts index 930f5380..e4b2c550 100644 --- a/layers/artifact-delivery/server/utils/resolution-retention.ts +++ b/layers/artifact-delivery/server/utils/resolution-retention.ts @@ -24,9 +24,12 @@ export const RESOLUTION_PRUNE_BATCH = 500 * - a run check still reads it; * - a download grant names it; * - it is ready and holds the newest attestation of its Artifact, which build - * reuse copies and the Artifact routes serve. + * reuse reads and the Artifact routes serve; + * - a reused build names it, because that build serves its attestation. * - * Deleting one cascades to its attestation, check results, and requester rows. + * A reused build holds no attestation of its own, so it goes once the window + * passes. Deleting one cascades to its attestation, check results, and + * requester rows. Migration 0161 indexes the reused-build lookup. * The batch prune filters by `idx_artifact_resolutions_state`, so its scan reads * only settled rows older than the window. */ @@ -40,8 +43,13 @@ const UNNEEDED = ` UNION SELECT resolution_id FROM artifact_download_grants ) + AND NOT EXISTS ( + SELECT 1 FROM artifact_resolutions reused + WHERE reused.attestation_resolution_id = r.id + ) AND ( r.state <> 'ready' + OR r.attestation_resolution_id IS NOT NULL OR EXISTS ( SELECT 1 FROM artifact_attestations own diff --git a/layers/artifact-delivery/server/utils/state.ts b/layers/artifact-delivery/server/utils/state.ts index 135a6bdf..74ab8ba9 100644 --- a/layers/artifact-delivery/server/utils/state.ts +++ b/layers/artifact-delivery/server/utils/state.ts @@ -97,6 +97,13 @@ const resolutionRowSchema = z.object({ check_results_json: z.string().nullable(), attestation_statement_json: z.string().nullable(), attestation_json: z.string().nullable(), + /** + * A reused build stores no attestation. It names the Resolution that signed + * the one it serves. See migration 0161. + */ + attestation_resolution_id: z.string().nullable().optional(), + /** The attestation of `attestation_resolution_id`, from {@link RESOLUTION_SELECT}. */ + shared_attestation_json: z.string().nullable().optional(), visibility: z.enum(['public', 'private']), account_id: z.number().int().positive().nullable(), github_installation_id: z.number().int().positive().safe().nullable(), @@ -226,16 +233,24 @@ export async function createResolution( : { _tag: 'idempotency-conflict' } } +/** + * A Resolution row with the attestation it serves. A reused build reads the + * attestation of the Resolution it names. + */ +const RESOLUTION_SELECT = `SELECT r.*, shared.attestation_json AS shared_attestation_json + FROM artifact_resolutions r + LEFT JOIN artifact_resolutions shared ON shared.id = r.attestation_resolution_id` + async function findResolutionByRequestKey(db: D1Database, keyHash: string): Promise { const row = await db.prepare( - 'SELECT * FROM artifact_resolutions WHERE request_key_hash = ?1 LIMIT 1', + `${RESOLUTION_SELECT} WHERE r.request_key_hash = ?1 LIMIT 1`, ).bind(keyHash).first>() return row ? resolutionRowSchema.parse(row) : null } export async function getResolution(db: D1Database, resolutionId: string): Promise { const row = await db.prepare( - 'SELECT * FROM artifact_resolutions WHERE id = ?1 LIMIT 1', + `${RESOLUTION_SELECT} WHERE r.id = ?1 LIMIT 1`, ).bind(resolutionId).first>() return row ? resolutionRowSchema.parse(row) : null } @@ -313,6 +328,7 @@ export type ReadyBuildLookup /** A ready public build as D1 stores it. Nothing here verifies its attestation. */ export interface ReadyPublicBuild { + /** The Resolution that signed {@link attestationJson}. A reused build names it. */ resolutionId: string source: ResolvedSource artifactId: string @@ -334,7 +350,8 @@ const readyPublicBuildRowSchema = z.object({ content_sha256: z.string(), content_bytes: z.number().int().positive(), r2_key: z.string(), - attestation_json: z.string(), + attestation_resolution_id: z.string().uuid(), + attestation_json: z.string().nullable(), }) /** @@ -355,8 +372,12 @@ export async function findReadyPublicBuild( if (!value) return null const row = readyPublicBuildRowSchema.parse(value) + // A reused build whose signed Resolution is gone serves nothing. The prune + // keeps every Resolution a reused build names, so this is a fault. + if (row.attestation_json === null) + return null return { - resolutionId: row.id, + resolutionId: row.attestation_resolution_id, source: resolvedSourceSchema.parse({ provider: 'github', repositoryId: row.repository_id, @@ -386,9 +407,12 @@ export async function findReadyPublicBuild( // instead of every ready row of the commit. const READY_PUBLIC_BUILD_SELECT = `SELECT r.id, r.repository_id, r.resolved_owner, r.resolved_repository, r.commit_sha, r.tree_sha, r.skill_path, r.artifact_id, r.content_sha256, - r.content_bytes, r.r2_key, r.attestation_json + r.content_bytes, r.r2_key, + COALESCE(r.attestation_resolution_id, r.id) AS attestation_resolution_id, + COALESCE(r.attestation_json, shared.attestation_json) AS attestation_json FROM artifact_resolutions r JOIN artifacts a ON a.id = r.artifact_id + LEFT JOIN artifact_resolutions shared ON shared.id = r.attestation_resolution_id WHERE r.state = 'ready' AND r.visibility = 'public' AND a.delivery_status = 'available'` const NEWEST_READY_BUILD = 'ORDER BY r.created_at DESC, r.id DESC LIMIT 1' @@ -520,18 +544,59 @@ export function presentResolution(row: ResolutionRow): ResolutionResponse { artifact: { artifactId: row.artifact_id, visibility: row.visibility, - attestation: parseAttestation(row.attestation_json), + attestation: parseAttestation(row.attestation_json ?? row.shared_attestation_json ?? null), }, } } +/** + * The attestation a published Resolution serves: one it signed, or the one + * of the build it reused, which it does not store again. + */ +export type PublishedAttestation + = { _tag: 'signed', attestation: ArtifactAttestation } + | { _tag: 'shared' } + export interface PublishArtifactInput { row: ResolutionRow checks: CheckResult[] - attestation: ArtifactAttestation + attestation: PublishedAttestation now: number } +/** + * Move a signing Resolution that reuses a ready public build to `publishing`, + * naming the Resolution that signed the attestation it serves. It stores no + * attestation and asks the signer for none. + * + * The write applies only while the named Resolution still holds its + * attestation, so a prune between the reuse decision and this write cannot + * leave the Resolution naming nothing. Once written, the prune keeps the named + * Resolution. `attestation-missing` means the build must sign its own. + */ +export async function shareReusedAttestation( + db: D1Database, + row: ResolutionRow, + input: { attestationResolutionId: string, r2Key: string, now: number }, +): Promise<{ _tag: 'advanced', row: ResolutionRow } | { _tag: 'superseded' | 'attestation-missing' }> { + if (row.state !== 'signing') + throw new Error(`Invalid Artifact build transition: ${row.state} to publishing`) + const advanced = await db.prepare( + `UPDATE artifact_resolutions + SET state = 'publishing', state_version = state_version + 1, updated_at = ?1, + r2_key = ?2, attestation_resolution_id = ?3 + WHERE id = ?4 AND state = 'signing' AND state_version = ?5 + AND EXISTS (SELECT 1 FROM artifact_attestations WHERE resolution_id = ?3) + RETURNING *`, + ).bind(input.now, input.r2Key, input.attestationResolutionId, row.id, row.state_version).first>() + if (advanced) + return { _tag: 'advanced', row: resolutionRowSchema.parse(advanced) } + const current = await getResolution(db, row.id) + return current?.state === 'signing' && current.state_version === row.state_version + ? { _tag: 'attestation-missing' } + : { _tag: 'superseded' } +} + export async function publishArtifactRecord( db: D1Database, input: PublishArtifactInput, @@ -553,8 +618,10 @@ export async function publishArtifactRecord( } } + if (attestation._tag === 'shared' && (row.visibility !== 'public' || !row.attestation_resolution_id)) + throw new Error('Only a reused public build serves a shared attestation') const artifactStatements: D1PreparedStatement[] = row.visibility === 'private' - ? privateArtifactPublishStatements(db, row, attestation, now) + ? privateArtifactPublishStatements(db, row, signedAttestation(attestation), now) : publicArtifactPublishStatements(db, row, attestation, now) const statements: D1PreparedStatement[] = [ ...artifactStatements, @@ -586,23 +653,32 @@ export async function publishArtifactRecord( return { _tag: 'published', row: resolutionRowSchema.parse(published) } } +function signedAttestation(attestation: PublishedAttestation): ArtifactAttestation { + if (attestation._tag !== 'signed') + throw new Error('A private Artifact signs its own attestation') + return attestation.attestation +} + function publicArtifactPublishStatements( db: D1Database, row: ResolutionRow, - attestation: ArtifactAttestation, + attestation: PublishedAttestation, now: number, ): D1PreparedStatement[] { + const artifact = db.prepare( + `INSERT OR IGNORE INTO artifacts ( + id, content_sha256, content_bytes, format, r2_key, delivery_status, created_at, updated_at + ) VALUES (?1, ?2, ?3, 'skilld-tar-v1', ?4, 'available', ?5, ?5)`, + ).bind(row.artifact_id!, row.content_sha256!, row.content_bytes!, row.r2_key!, now) + if (attestation._tag === 'shared') + return [artifact] return [ - db.prepare( - `INSERT OR IGNORE INTO artifacts ( - id, content_sha256, content_bytes, format, r2_key, delivery_status, created_at, updated_at - ) VALUES (?1, ?2, ?3, 'skilld-tar-v1', ?4, 'available', ?5, ?5)`, - ).bind(row.artifact_id!, row.content_sha256!, row.content_bytes!, row.r2_key!, now), + artifact, db.prepare( `INSERT OR REPLACE INTO artifact_attestations ( resolution_id, artifact_id, attestation_json, created_at ) VALUES (?1, ?2, ?3, ?4)`, - ).bind(row.id, row.artifact_id!, JSON.stringify(attestation), now), + ).bind(row.id, row.artifact_id!, JSON.stringify(attestation.attestation), now), ] } diff --git a/migrations/0161_artifact_resolution_shared_attestation.sql b/migrations/0161_artifact_resolution_shared_attestation.sql new file mode 100644 index 00000000..d8df6de3 --- /dev/null +++ b/migrations/0161_artifact_resolution_shared_attestation.sql @@ -0,0 +1,17 @@ +-- The Resolution whose attestation a reused build serves. +-- +-- A build that reuses a ready public build used to sign a new statement and +-- store another copy of the same attestation: about 33 KB on the row and +-- 23 KB in `artifact_attestations`. In a 12-hour sample on 2026-11-12, 1,967 +-- ready Resolutions covered 142 Artifacts. A reused build now stores no +-- attestation. It names the Resolution that holds one, and readers serve that. +-- +-- It always names a Resolution that signed its own attestation, never another +-- reused build. NULL for every build that signed its own. +ALTER TABLE artifact_resolutions ADD COLUMN attestation_resolution_id TEXT; + +-- The prune keeps a Resolution while a reused build names it. Only reused +-- builds enter this index. +CREATE INDEX IF NOT EXISTS idx_artifact_resolutions_attestation_resolution + ON artifact_resolutions(attestation_resolution_id) + WHERE attestation_resolution_id IS NOT NULL; diff --git a/test/unit/artifact-build-reuse.test.ts b/test/unit/artifact-build-reuse.test.ts index aa8123b0..d30c5f14 100644 --- a/test/unit/artifact-build-reuse.test.ts +++ b/test/unit/artifact-build-reuse.test.ts @@ -42,6 +42,7 @@ const NOW = 1_787_227_200 const MIGRATIONS = [ 'migrations/0017_users.sql', 'migrations/0110_artifact_delivery.sql', + 'migrations/0161_artifact_resolution_shared_attestation.sql', 'migrations/0111_github_app_delivery.sql', 'migrations/0112_private_artifact_keys.sql', 'migrations/0122_artifact_resolution_retry_after.sql', @@ -97,8 +98,7 @@ describe('reusing a ready public build', () => { expect(await verifyArtifactAttestation(attestation, harness.trustedRoot(), harness.now())).toBe(true) expect(response.artifact.artifactId).toBe(first.row.artifact_id) expect(attestation.source).toEqual(source) - expect(attestation.createdAt).toBe(new Date(repeat.row.created_at * 1000).toISOString()) - expect(attestation.createdAt).not.toBe(parseReady(first.row).createdAt) + expect(attestation).toEqual(parseReady(first.row)) expect(harness.reports).toContainEqual({ _tag: 'hit', lookup: 'pinned', @@ -403,6 +403,109 @@ describe('reusing a ready public build', () => { }) }) +describe('a reused build stores no attestation of its own', () => { + it('names the build it reused and serves that attestation', async () => { + const harness = await createReuseHarness() + const first = await harness.build(pinned, githubServing(source)) + harness.advanceClock(120) + + const repeat = await harness.build(pinned, githubDown()) + + expect(repeat.row.state).toBe('ready') + expect(repeat.row.attestation_json).toBeNull() + expect(repeat.row.attestation_statement_json).toBeNull() + expect(repeat.row.attestation_resolution_id).toBe(first.row.id) + expect(harness.raw.prepare('SELECT resolution_id FROM artifact_attestations').all()) + .toEqual([{ resolution_id: first.row.id }]) + expect(presentResolution(repeat.row)).toEqual({ ...presentResolution(first.row), resolutionId: repeat.row.id }) + harness.close() + }) + + it('names the build that signed, never another reused build', async () => { + const harness = await createReuseHarness() + const first = await harness.build(pinned, githubServing(source)) + harness.advanceClock(60) + await harness.build(pinned, githubDown()) + harness.advanceClock(60) + + const third = await harness.build(onBranch, githubServing(source)) + + expect(third.row.attestation_resolution_id).toBe(first.row.id) + expect(presentResolution(third.row)).toEqual({ ...presentResolution(first.row), resolutionId: third.row.id }) + harness.close() + }) + + it('answers a replayed request key with the attestation it reused', async () => { + const harness = await createReuseHarness() + const first = await harness.build(pinned, githubServing(source)) + harness.advanceClock(120) + const repeat = await harness.build(pinned, githubDown()) + const identity = await resolutionRequestIdentity(pinned, 'reuse-test-request-2') + + const replay = await createResolution(harness.db, pinned, identity, harness.now()) + + if (replay._tag !== 'existing') + throw new Error('The replay created a new Resolution') + expect(replay.row.id).toBe(repeat.row.id) + expect(presentResolution(replay.row)).toEqual({ ...presentResolution(first.row), resolutionId: repeat.row.id }) + harness.close() + }) + + it('grants the reused build with the attestation it serves, under its own checks', async () => { + const harness = await createReuseHarness() + const first = await harness.build(pinned, githubServing(source)) + harness.advanceClock(120) + const repeat = await harness.build(pinned, githubDown()) + const grantFor = async (resolutionId: string) => await createPublicArtifactGrant({ + db: harness.db, + trustedRoot: harness.trustedRoot(), + publicBaseUrl: 'https://artifacts.skilld.dev', + now: harness.now(), + }, repeat.row.artifact_id!, resolutionId) + + const granted = await grantFor(repeat.row.id) + + if (granted._tag !== 'granted') + throw new Error(`The reused build was not granted: ${JSON.stringify(granted)}`) + expect(granted.grant.attestation).toEqual(parseReady(first.row)) + harness.raw.prepare(`UPDATE artifact_check_results SET outcome = 'fail', required = 1 WHERE resolution_id = ?`).run(repeat.row.id) + expect(await grantFor(repeat.row.id)).toEqual({ _tag: 'denied', code: 'CHECK_BLOCKED' }) + expect((await grantFor(first.row.id))._tag).toBe('granted') + harness.close() + }) + + it('signs its own attestation when the build it reused lost its attestation', async () => { + const harness = await createReuseHarness() + const first = await harness.build(pinned, githubServing(source)) + harness.raw.prepare('DELETE FROM artifact_attestations WHERE resolution_id = ?').run(first.row.id) + harness.advanceClock(120) + + const repeat = await harness.build(pinned, githubDown()) + + expect(repeat.row.state).toBe('ready') + expect(repeat.row.attestation_resolution_id).toBeNull() + const attestation = parseReady(repeat.row) + expect(attestation.createdAt).toBe(new Date(repeat.row.created_at * 1000).toISOString()) + expect(await verifyArtifactAttestation(attestation, harness.trustedRoot(), harness.now())).toBe(true) + expect(harness.raw.prepare('SELECT resolution_id FROM artifact_attestations').all()) + .toEqual([{ resolution_id: repeat.row.id }]) + harness.close() + }) + + it('finds the newest ready build through a reused one, with the attestation it names', async () => { + const harness = await createReuseHarness() + const first = await harness.build(pinned, githubServing(source)) + harness.advanceClock(60) + await harness.build(pinned, githubDown()) + + const found = await findReadyPublicBuild(harness.db, { _tag: 'resolved', source }) + + expect(found?.resolutionId).toBe(first.row.id) + expect(found?.attestationJson).toBe(first.row.attestation_json) + harness.close() + }) +}) + describe('the pinned reuse lookup seeks', () => { // Production read every ready row of the commit per pinned lookup: the only // ready-public index led with columns the pinned lookup cannot all supply. @@ -818,7 +921,7 @@ describe('round trips to D1 for one build', () => { harness.close() }) - it('a reused build reaches ready in 11 round trips', async () => { + it('a reused build reaches ready in 10 round trips', async () => { const harness = await createReuseHarness() await harness.build(pinned, githubServing(source)) const resolutionId = await harness.request(pinned) @@ -827,7 +930,7 @@ describe('round trips to D1 for one build', () => { const outcome = await processArtifactBuild({ ...harness.dependencies(githubDown()), db: counted.db }, resolutionId) expect(outcome._tag).toBe('ready') - expect(counted.roundTrips()).toBe(11) + expect(counted.roundTrips()).toBe(10) harness.close() }) }) @@ -853,7 +956,8 @@ describe('round trips to D1 for one Resolution request', () => { const replay = await createResolution(counted.db, pinned, identity, harness.now()) - expect(replay).toEqual({ _tag: 'existing', row: first._tag === 'created' ? first.row : undefined }) + // A lookup also reads the attestation a reused build names, none here. + expect(replay).toEqual({ _tag: 'existing', row: first._tag === 'created' ? { ...first.row, shared_attestation_json: null } : undefined }) expect(counted.roundTrips()).toBe(2) harness.close() }) diff --git a/test/unit/artifact-delivery.test.ts b/test/unit/artifact-delivery.test.ts index 8c7389de..4ffd06bb 100644 --- a/test/unit/artifact-delivery.test.ts +++ b/test/unit/artifact-delivery.test.ts @@ -40,6 +40,7 @@ const NOW = 1_787_227_200 const ARTIFACT_MIGRATIONS = [ 'migrations/0017_users.sql', 'migrations/0110_artifact_delivery.sql', + 'migrations/0161_artifact_resolution_shared_attestation.sql', 'migrations/0111_github_app_delivery.sql', 'migrations/0112_private_artifact_keys.sql', 'migrations/0122_artifact_resolution_retry_after.sql', diff --git a/test/unit/artifact-github-reads.test.ts b/test/unit/artifact-github-reads.test.ts index 9c7f5a83..22bed78f 100644 --- a/test/unit/artifact-github-reads.test.ts +++ b/test/unit/artifact-github-reads.test.ts @@ -38,6 +38,7 @@ const NOW = 1_787_227_200 const MIGRATIONS = [ 'migrations/0017_users.sql', 'migrations/0110_artifact_delivery.sql', + 'migrations/0161_artifact_resolution_shared_attestation.sql', 'migrations/0111_github_app_delivery.sql', 'migrations/0112_private_artifact_keys.sql', 'migrations/0122_artifact_resolution_retry_after.sql', diff --git a/test/unit/artifact-large-build.test.ts b/test/unit/artifact-large-build.test.ts index b3642d0c..8fe22d03 100644 --- a/test/unit/artifact-large-build.test.ts +++ b/test/unit/artifact-large-build.test.ts @@ -27,6 +27,7 @@ const MIB = 1024 * 1024 const MIGRATIONS = [ 'migrations/0017_users.sql', 'migrations/0110_artifact_delivery.sql', + 'migrations/0161_artifact_resolution_shared_attestation.sql', 'migrations/0111_github_app_delivery.sql', 'migrations/0112_private_artifact_keys.sql', 'migrations/0122_artifact_resolution_retry_after.sql', diff --git a/test/unit/artifact-rate-limit.test.ts b/test/unit/artifact-rate-limit.test.ts index 46ffe350..f3389b91 100644 --- a/test/unit/artifact-rate-limit.test.ts +++ b/test/unit/artifact-rate-limit.test.ts @@ -20,6 +20,7 @@ const NOW = 1_787_227_200 const ARTIFACT_MIGRATIONS = [ 'migrations/0017_users.sql', 'migrations/0110_artifact_delivery.sql', + 'migrations/0161_artifact_resolution_shared_attestation.sql', 'migrations/0111_github_app_delivery.sql', 'migrations/0112_private_artifact_keys.sql', 'migrations/0122_artifact_resolution_retry_after.sql', diff --git a/test/unit/artifact-resolution-latency.test.ts b/test/unit/artifact-resolution-latency.test.ts index 05c5b8f3..3d4af30b 100644 --- a/test/unit/artifact-resolution-latency.test.ts +++ b/test/unit/artifact-resolution-latency.test.ts @@ -30,6 +30,7 @@ const request: SourceRequest = { const MIGRATIONS = [ 'migrations/0017_users.sql', 'migrations/0110_artifact_delivery.sql', + 'migrations/0161_artifact_resolution_shared_attestation.sql', 'migrations/0111_github_app_delivery.sql', 'migrations/0122_artifact_resolution_retry_after.sql', 'migrations/0145_artifact_resolution_linked_files.sql', diff --git a/test/unit/artifact-served-resolution.test.ts b/test/unit/artifact-served-resolution.test.ts index df731333..2c7b3aad 100644 --- a/test/unit/artifact-served-resolution.test.ts +++ b/test/unit/artifact-served-resolution.test.ts @@ -27,6 +27,7 @@ const NOW = 1_787_227_200 const MIGRATIONS = [ 'migrations/0017_users.sql', 'migrations/0110_artifact_delivery.sql', + 'migrations/0161_artifact_resolution_shared_attestation.sql', 'migrations/0111_github_app_delivery.sql', 'migrations/0112_private_artifact_keys.sql', 'migrations/0122_artifact_resolution_retry_after.sql', diff --git a/test/unit/artifact-signer.test.ts b/test/unit/artifact-signer.test.ts index 201a1360..0b644419 100644 --- a/test/unit/artifact-signer.test.ts +++ b/test/unit/artifact-signer.test.ts @@ -21,6 +21,7 @@ const NOW = 1_787_227_200 const ARTIFACT_MIGRATIONS = [ 'migrations/0017_users.sql', 'migrations/0110_artifact_delivery.sql', + 'migrations/0161_artifact_resolution_shared_attestation.sql', 'migrations/0111_github_app_delivery.sql', 'migrations/0112_private_artifact_keys.sql', ] diff --git a/test/unit/private-artifact-delivery.test.ts b/test/unit/private-artifact-delivery.test.ts index f5c909cc..befbd63e 100644 --- a/test/unit/private-artifact-delivery.test.ts +++ b/test/unit/private-artifact-delivery.test.ts @@ -373,6 +373,7 @@ async function createFixture(options: { maximumQueries?: number } = {}) { const sqlite = createSqliteD1([ 'migrations/0017_users.sql', 'migrations/0110_artifact_delivery.sql', + 'migrations/0161_artifact_resolution_shared_attestation.sql', 'migrations/0111_github_app_delivery.sql', 'migrations/0112_private_artifact_keys.sql', ], options) diff --git a/test/unit/private-github-app-routes.test.ts b/test/unit/private-github-app-routes.test.ts index 98b15862..8b5e2a26 100644 --- a/test/unit/private-github-app-routes.test.ts +++ b/test/unit/private-github-app-routes.test.ts @@ -26,6 +26,7 @@ describe('opt-in GitHub App connection routes', () => { fixture = createSqliteD1([ 'migrations/0017_users.sql', 'migrations/0110_artifact_delivery.sql', + 'migrations/0161_artifact_resolution_shared_attestation.sql', 'migrations/0111_github_app_delivery.sql', 'migrations/0112_private_artifact_keys.sql', ]) diff --git a/test/unit/private-github-app.test.ts b/test/unit/private-github-app.test.ts index e0dd9d7b..762f5366 100644 --- a/test/unit/private-github-app.test.ts +++ b/test/unit/private-github-app.test.ts @@ -127,6 +127,7 @@ describe('private GitHub App access', () => { const fixture = createSqliteD1([ 'migrations/0017_users.sql', 'migrations/0110_artifact_delivery.sql', + 'migrations/0161_artifact_resolution_shared_attestation.sql', 'migrations/0111_github_app_delivery.sql', 'migrations/0112_private_artifact_keys.sql', ]) @@ -193,6 +194,7 @@ describe('private GitHub App access', () => { const fixture = createSqliteD1([ 'migrations/0017_users.sql', 'migrations/0110_artifact_delivery.sql', + 'migrations/0161_artifact_resolution_shared_attestation.sql', 'migrations/0111_github_app_delivery.sql', 'migrations/0112_private_artifact_keys.sql', ]) diff --git a/test/unit/resolution-retention.test.ts b/test/unit/resolution-retention.test.ts index 381db9c6..aba5abae 100644 --- a/test/unit/resolution-retention.test.ts +++ b/test/unit/resolution-retention.test.ts @@ -1,7 +1,7 @@ // @vitest-environment node import type { SqliteD1 } from './helpers/d1-sqlite' import { afterEach, describe, expect, it } from 'vitest' -import { pruneSettledResolutions, RESOLUTION_RETENTION_SECONDS } from '../../layers/artifact-delivery/server/utils/resolution-retention' +import { deleteUnneededResolution, pruneSettledResolutions, RESOLUTION_RETENTION_SECONDS } from '../../layers/artifact-delivery/server/utils/resolution-retention' import { allMigrations, createSqliteD1 } from './helpers/d1-sqlite' const NOW = 1_795_000_000 @@ -17,6 +17,8 @@ interface Seed { at: number state?: 'ready' | 'blocked' | 'failed' | 'fetching' visibility?: 'public' | 'private' + /** A reused build: it names this Resolution and stores no attestation. */ + reuses?: string } function seed(rows: Seed[]) { @@ -27,17 +29,19 @@ function seed(rows: Seed[]) { const state = row.state ?? 'ready' sqlite.raw.prepare(`INSERT INTO artifact_resolutions ( id, request_fingerprint, state, visibility, requested_owner, requested_repository, - selector_type, selector_value, artifact_id, attestation_json, created_at, updated_at) - VALUES (?, ?, ?, ?, 'acme', 'skills', 'named-skill', 'demo', ?, '{}', ?, ?)`).run( + selector_type, selector_value, artifact_id, attestation_json, attestation_resolution_id, created_at, updated_at) + VALUES (?, ?, ?, ?, 'acme', 'skills', 'named-skill', 'demo', ?, ?, ?, ?, ?)`).run( row.id, `fp-${row.id}`, state, row.visibility ?? 'public', state === 'ready' ? ARTIFACT : null, + row.reuses ? null : '{}', + row.reuses ?? null, row.at, row.at, ) - if (state === 'ready') { + if (state === 'ready' && !row.reuses) { sqlite.raw.prepare(`INSERT INTO artifact_attestations (resolution_id, artifact_id, attestation_json, created_at) VALUES (?, ?, '{}', ?)`).run(row.id, ARTIFACT, row.at) } @@ -79,6 +83,46 @@ describe('pruneSettledResolutions', () => { expect(remaining()).toEqual(['building', 'checked', 'newest', 'private', 'recent-failure']) }) + it('drops an old reused build, then the build it named once nothing names it', async () => { + seed([ + { id: 'r1', at: OLD - 30 }, + { id: 'reused', at: OLD - 20, reuses: 'r1' }, + { id: 'r2', at: OLD - 10 }, + ]) + + expect(await pruneSettledResolutions(sqlite.db, { now: NOW })).toBe(1) + expect(remaining()).toEqual(['r1', 'r2']) + + expect(await pruneSettledResolutions(sqlite.db, { now: NOW })).toBe(1) + expect(remaining()).toEqual(['r2']) + }) + + it('keeps a build that a recent or building Resolution names', async () => { + seed([ + { id: 'r1', at: OLD - 30 }, + { id: 'r2', at: OLD - 20 }, + { id: 'r3', at: OLD - 10 }, + { id: 'recent', at: NOW - 60, reuses: 'r1' }, + { id: 'publishing', at: OLD, state: 'fetching', reuses: 'r2' }, + ]) + + expect(await pruneSettledResolutions(sqlite.db, { now: NOW })).toBe(0) + expect(remaining()).toEqual(['publishing', 'r1', 'r2', 'r3', 'recent']) + }) + + it('deletes a replaced check now, unless a reused build names it', async () => { + seed([ + { id: 'named', at: NOW - 60 }, + { id: 'reused', at: NOW - 30, reuses: 'named' }, + { id: 'newest', at: NOW - 10 }, + ]) + + expect(await deleteUnneededResolution(sqlite.db, 'named')).toBe(false) + expect(await deleteUnneededResolution(sqlite.db, 'reused')).toBe(true) + expect(await deleteUnneededResolution(sqlite.db, 'named')).toBe(true) + expect(remaining()).toEqual(['newest']) + }) + it('deletes at most one batch per call', async () => { seed([ { id: 'f1', at: OLD, state: 'failed' }, diff --git a/test/unit/skill-run-sweep.test.ts b/test/unit/skill-run-sweep.test.ts index 496807d7..81c6ae0a 100644 --- a/test/unit/skill-run-sweep.test.ts +++ b/test/unit/skill-run-sweep.test.ts @@ -13,6 +13,7 @@ import { createSqliteD1 } from './helpers/d1-sqlite' const MIGRATIONS = [ 'migrations/0017_users.sql', 'migrations/0110_artifact_delivery.sql', + 'migrations/0161_artifact_resolution_shared_attestation.sql', 'migrations/0111_github_app_delivery.sql', 'migrations/0112_private_artifact_keys.sql', 'migrations/0122_artifact_resolution_retry_after.sql',