diff --git a/GLOSSARY.md b/GLOSSARY.md index f6ab3995..155f7d64 100644 --- a/GLOSSARY.md +++ b/GLOSSARY.md @@ -12,6 +12,7 @@ Shared terms match the skilld CLI glossary (`skilld-dev/skilld/GLOSSARY.md`). Th | Agent | the user's tool | Agent 1—N Agent target | "your agent" | | Repository | GitHub | Repository 1—N Skill | "repository" | | Artifact | skilld.dev API | Skill commit 1—1 Artifact | not shown | +| linked file | Artifact attestation | Artifact 1—N linked file | not shown | | source status | lockfile, skilld.dev API | Artifact 1—1 source status | "Verified", "Unverified", "Local" | | run | skilld CLI | Skill 1—1 run command | "run" | | demo | Skill page, homepage | Skill 1—N demo | "demo" | @@ -262,6 +263,18 @@ Existing route paths, table names, and protocol fields keep their identifiers. **Never:** signature (bare), certificate, receipt. +### linked file + +**Is:** a Skill file the Artifact attestation lists by path, size, and Git blob SHA, without packing its bytes. The skilld CLI reads it from GitHub at the attested commit and checks it against that SHA. Only a CLI that sends `Skilld-Capabilities: linked-files` gets one. See ADR-0013. + +**Never:** external file, remote asset, download, attachment. + +### omitted file + +**Is:** a Skill file left out of an Artifact for a size limit. The `omitted-files` check result names each one. A CLI that reads linked files gets a large file as a linked file instead. + +**Never:** skipped file, excluded file, dropped file. + ### check result **Is:** one named check, its version, its finding, and its outcome for one Artifact. diff --git a/docs/adr/0013-artifact-size-limits-and-linked-files.md b/docs/adr/0013-artifact-size-limits-and-linked-files.md new file mode 100644 index 00000000..176385d7 --- /dev/null +++ b/docs/adr/0013-artifact-size-limits-and-linked-files.md @@ -0,0 +1,114 @@ +# ADR-0013: Artifact size limits and linked files + +Date: 2026-10-07 + +Amends [ADR-0005](0005-v3-cli-harness-artifact-delivery.md). + +## Context + +A public build read every Skill file into memory, then packed a second copy as the archive. +Its limits were 10 MiB an Artifact, 2 MiB a file, and 900 files. +Only the 900 had a recorded reason: one blob read per file, against 1,000 subrequests. + +#487 kept Skills over those limits running by leaving files out. +`latent-spaces/brag` then ran without the four music files its films use. +`oaustegard/claude-skills/tree-sitting` ran without eight of its parsers. + +Each limit now names the constraint that sets it, measured on 2026-10-07. + +## Constraints + +| Constraint | Real limit | Old limit | New limit | +|---|---|---|---| +| Worker memory | 128 MB per isolate, shared with site requests | 10 MiB of files plus their 10 MiB archive, held at once | None from size. A build holds one network chunk, one inflate write of at most 16 KiB, SKILL.md, and the archive when it is at most 16 MiB | +| Worker CPU | 300 s for the queue consumer | Not measured | 1 GiB of archive per pass. Inflating and walking costs 4 to 5 ms per MiB, so about 5 s | +| Wall time | 15 min for the consumer. The skilld CLI waits 60 s for one Resolution | Not measured | A pass stops after the last Skill file. Cold codeload served PostHog/posthog (191 MiB gzipped) in 26 s | +| Subrequests | 1,000 per invocation | 900 files, one blob read each | No file limit. One or two archive reads, and at most 400 single-file reads for files the archive lacks | +| R2 put | 5 GiB in one part | Bytes in memory | A stream of known length in one part. R2 checks it against the SHA-256 the scan found | +| D1 row | 2,000,000 bytes | Not measured | A file list of 448 KiB. The Resolution row holds about 3.34 statements plus the check results | +| Signer | 128 MB per isolate | 10 MiB, read whole | 64 MiB, hashed as it streams | +| skilld CLI 3.0 to 3.6.2 | 64 MiB download, 2,000 files, 6 MiB statement. Before 3.6.2 a download gets 30 s, so 64 MiB needs 2.2 MB/s | 10 MiB, 900 files | 64 MiB and 2,000 files. These two now bind | +| USTAR | A path of at most 256 bytes | Checked after packing | Refused before any byte is read | +| [GitHub](https://github.com) | 100 MiB a file | Not applicable | Linked files of at most 256 MiB in all | + +A private build keeps 10 MiB, 2 MiB a file, and 900 files. +It still reads one blob per request and encrypts the archive in memory. + +## Decision + +### A build streams the Repository archive + +A public build reads `https://codeload.github.com/{owner}/{repository}/tar.gz/{commit}`. +That is one subrequest and spends no REST quota. +It inflates the archive in 16 KiB writes and keeps only the Skill folder's entries. +It stops reading after the last Skill file. + +The Git tree still names every file, its mode, its size, and its blob SHA. +Each file's bytes are hashed as they pass and must match that blob SHA. +The archive is only a byte source, as before. + +A file the archive lacks, or holds with another size, is read from `raw.githubusercontent.com` at the same commit. +Git LFS pointers, `export-ignore`, and `export-subst` produce those cases. +A file whose archive bytes keep their size and fail the digest is read the same way on a second pass. + +Artifact order is the UTF-8 byte order of paths. +It is the order a Git archive lists a tree in. +All 487 archives of the 2026-10-07 sweep listed their files in that order. + +The first pass learns the archive digest and size, the file inventory, and the check results. +An archive of at most 16 MiB stays in memory, and R2 stores it from there. +A larger one is read from GitHub a second time and streams into R2 under the digest from the first pass. +R2 refuses the put if one byte differs. +The store happens before the checks are recorded. +The CLI gives each stage after the checks 15 seconds. + +### Limits + +A public Artifact holds at most 64 MiB and 2,000 files. +A file has no limit of its own. + +When the files exceed a limit, the largest leave the archive first. +For a CLI without linked files, only files the Skill does not read leave, and the `omitted-files` check lists them. +That is the #487 rule at the new limits. +More files than 2,000 leave the same way, so linked files do not change the file count. + +### Linked files + +A skilld CLI that sends `Skilld-Capabilities: linked-files` on `POST /api/v1/resolutions` may get linked files. +The Resolution stores that as `linked_files`, and the request fingerprint names it. + +When the archive would pass 64 MiB, the largest files other than SKILL.md become linked files, up to 256 MiB. +The signed statement lists each one: + +```json +{ + "linkedFiles": [ + { "path": "scripts/KimiXlsx", "mode": 420, "size": 77001601, "gitBlobSha": "<40 hex characters>" } + ] +} +``` + +The CLI verifies the attestation first. +It then reads each linked file from `https://raw.githubusercontent.com/{owner}/{repository}/{commitSha}/{skillPath}/{path}`, each path segment percent-encoded. +It refuses the Skill unless every file has the declared size and Git blob SHA. +It installs the file with the declared mode, beside the archive's files. +The bytes never pass through skilld.dev, and the signature still covers them through the blob SHA. + +The field is present only when it is not empty. +A CLI without linked files never receives it, because it refuses a statement field it does not know. +A build with linked files serves only a CLI that reads them. +A build that left nothing out serves both kinds. + +## Consequences + +`latent-spaces/brag` runs with its music: 289 files, 16.03 MiB, nothing left out. +`tt-a1i/archify` runs whole at 10.36 MiB. +`thedivergentai/gd-agentic-skills/godot-master` runs with 1,728 files. +`thvroyal/kimi-skills/kimi-xlsx` runs its 73 MiB binary as a linked file for a CLI that reads one. + +A first run of a Skill late in a large archive can take longer than the CLI's 60-second wait. +The build still finishes, and the next run reuses it. + +The previous policy `2026-10-07.2` stays signable. This change closes no safety gap. +A ready build under that policy that left nothing out packs the same files, so it is checked again from its bytes. +A stored archive over 16 MiB is never read back whole. It loads from GitHub. diff --git a/docs/arch/README.md b/docs/arch/README.md index 6d9ec8c4..2a72f4ae 100644 --- a/docs/arch/README.md +++ b/docs/arch/README.md @@ -58,6 +58,9 @@ One content hash can have several attestations with different sources or signing The grant must return the requested Resolution's attestation, or deny delivery. Public grants without the header select the newest attestation. Private grants retain Account access checks. +A public build streams the Repository archive from codeload and never holds the whole Skill. +The limits, the constraint behind each, and linked files: [ADR-0013](../adr/0013-artifact-size-limits-and-linked-files.md). + After a contract change in the `skilld-dev/skilld` repository, run `pnpm --filter skilld-sdk generate` and commit the document. Publish the new SDK version from that repository's `release.yml`. Then update this site's exact catalog pin. The route parity test checks the site against the published contract. diff --git a/layers/artifact-delivery/server/api/v1/resolutions/index.post.ts b/layers/artifact-delivery/server/api/v1/resolutions/index.post.ts index ed751037..11d79507 100644 --- a/layers/artifact-delivery/server/api/v1/resolutions/index.post.ts +++ b/layers/artifact-delivery/server/api/v1/resolutions/index.post.ts @@ -12,7 +12,13 @@ import { failResolution, processArtifactBuild } from '../../../utils/build' import { findPrivateRepositoryAccess } from '../../../utils/private-access' import { privateArtifactAccessEnabled } from '../../../utils/private-feature' import { createArtifactBuildDependencies, enqueueArtifactBuild } from '../../../utils/queue' -import { buildAfterResponse, fetchAdmittedSkillIdentity, requestResolution } from '../../../utils/request-resolution' +import { + buildAfterResponse, + CLIENT_CAPABILITIES_HEADER, + fetchAdmittedSkillIdentity, + readsLinkedFiles, + requestResolution, +} from '../../../utils/request-resolution' import { setSkillPageUrlHeader } from '../../../utils/skill-page' import { getResolution } from '../../../utils/state' @@ -65,6 +71,7 @@ export default withArtifactProblems(defineApiHandler({ }, { source: body.source, idempotencyKey, + linkedFiles: readsLinkedFiles(getHeader(event, CLIENT_CAPABILITIES_HEADER)), access: privateAccess._tag === 'allowed' ? { visibility: 'private', diff --git a/layers/artifact-delivery/server/schemas/contracts.ts b/layers/artifact-delivery/server/schemas/contracts.ts index 42c0b07c..da5874ca 100644 --- a/layers/artifact-delivery/server/schemas/contracts.ts +++ b/layers/artifact-delivery/server/schemas/contracts.ts @@ -60,6 +60,18 @@ export const artifactFileSchema = z.object({ sha256: z.string().regex(SHA256_PATTERN), }).strict() +/** + * A Skill file the Artifact does not pack. The skilld CLI reads it from + * GitHub at the attested commit and checks it against its Git blob digest. + * Only a CLI that sends `Skilld-Capabilities: linked-files` gets one. + */ +export const linkedArtifactFileSchema = z.object({ + path: z.string().min(1).max(1024), + mode: z.union([z.literal(420), z.literal(493)]), + size: z.number().int().nonnegative(), + gitBlobSha: z.string().regex(COMMIT_SHA_PATTERN), +}).strict() + export const attestationSignatureSchema = z.object({ algorithm: z.literal('Ed25519'), keyId: z.string().min(1).max(100), @@ -78,6 +90,8 @@ export const artifactAttestationStatementSchema = z.object({ policyVersion: z.string().min(1).max(100), files: z.array(artifactFileSchema).min(1).max(2000), checkResults: z.array(checkResultSchema).min(1).max(100), + /** Absent unless a file is linked: a CLI without linked files refuses the field. */ + linkedFiles: z.array(linkedArtifactFileSchema).min(1).max(2000).optional(), }).strict() export const artifactAttestationSchema = artifactAttestationStatementSchema.extend({ @@ -216,6 +230,7 @@ export type SourceRequest = z.infer export type ResolvedSource = z.infer export type CheckResult = z.infer export type ArtifactFile = z.infer +export type LinkedArtifactFile = z.infer export type ArtifactAttestationStatement = z.infer export type ArtifactAttestation = z.infer export type ResolutionResponse = z.infer diff --git a/layers/artifact-delivery/server/utils/archive-reader.ts b/layers/artifact-delivery/server/utils/archive-reader.ts new file mode 100644 index 00000000..2948e39c --- /dev/null +++ b/layers/artifact-delivery/server/utils/archive-reader.ts @@ -0,0 +1,490 @@ +import type { FileSink, PackedFile, ReadOutcome, SkillFileReader } from './artifact-pack' +import type { SourceRejection } from './github-source' +import { fetchNoRedirect } from './fetch-no-redirect' +import { compareArtifactPaths } from './ustar' + +const ARCHIVE_HOST = 'https://codeload.github.com' +const FILE_HOST = 'https://raw.githubusercontent.com' +/** + * One pass over a Repository archive. Measured 2026-10-07 from a home + * connection: codeload served PostHog/posthog (191 MiB gzipped) in 26 s and + * n8n-io/n8n (54 MiB) in 7 s, about 8 MB/s while it builds the archive. The + * queue consumer may run for 15 minutes. + */ +const ARCHIVE_PASS_TIMEOUT_MS = 5 * 60_000 +/** A file read from GitHub gets 30 seconds plus one second for each MiB. */ +const GITHUB_FILE_BASE_TIMEOUT_MS = 30_000 +/** A pax record or GNU long name holds one path. Anything longer is not an archive GitHub wrote. */ +const MAX_HEADER_RECORD_BYTES = 1024 * 1024 +const BLOCK_SIZE = 512 + +/** + * Files a build may still read from GitHub one by one. Each costs one Worker + * subrequest, and a build has 1,000 for everything. A store that reads the + * files again spends the same reads twice, so one budget covers every pass. + */ +export interface GithubReadBudget { + githubReads: number +} + +export interface ArchiveReport { + reason: 'unavailable' | 'too-large' | 'other-commit' | 'malformed' + detail: string +} + +/** + * Reads the packed files of one Skill from the Repository archive at the + * exact commit, as codeload streams it. + * + * Nothing is buffered beyond one network chunk: entries outside the Skill + * folder are skipped as they pass, each Skill file goes to the sink chunk by + * chunk, and the request is cancelled after the last Skill file. A file the + * archive leaves out, or holds with another size, is read from GitHub at the + * same commit in its place, so the sink still receives Artifact order. The + * sink checks every file against its Git blob digest, so neither host is + * trusted for content. + */ +export function createGithubArchiveReader(input: { + fetch: typeof globalThis.fetch + owner: string + repository: string + commitSha: string + skillPath: string + /** In Artifact order. */ + files: readonly PackedFile[] + /** Uncompressed archive bytes one pass reads at most. */ + maxArchiveBytes: number + budget: GithubReadBudget + gunzip?: (body: ReadableStream) => ReadableStream + report?: (report: ArchiveReport) => void + /** Unix seconds, for the retry time a rate limit names. */ + now?: () => number +}): SkillFileReader { + const now = input.now ?? (() => Math.floor(Date.now() / 1000)) + const gunzip = input.gunzip ?? gunzipStream + const repositoryPath = (path: string) => input.skillPath === '.' ? path : `${input.skillPath}/${path}` + const encodedRepository = `${encodeURIComponent(input.owner)}/${encodeURIComponent(input.repository)}` + + const readFromGithub = async (file: PackedFile, sink: FileSink): Promise => { + if (input.budget.githubReads <= 0) { + return rejection( + 'SOURCE_UNAVAILABLE', + 'The Repository archive left out more Skill files than one build can read one by one.', + [file.path], + ) + } + input.budget.githubReads-- + const url = `${FILE_HOST}/${encodedRepository}/${input.commitSha}/${repositoryPath(file.path).split('/').map(encodeURIComponent).join('/')}` + const sent = await fetchNoRedirect(input.fetch, url, { + headers: { 'User-Agent': 'skilld.dev' }, + signal: AbortSignal.timeout(GITHUB_FILE_BASE_TIMEOUT_MS + Math.ceil(file.size / 1024)), + }).then( + fetched => ({ _tag: 'sent' as const, fetched }), + (error: unknown) => ({ _tag: 'threw' as const, reason: describe(error) }), + ) + if (sent._tag === 'threw') + return rejection('SOURCE_UNAVAILABLE', 'GitHub did not serve a Skill file.', [`${file.path}: ${sent.reason}`]) + if (sent.fetched._tag === 'unexpected-redirect') + return rejection('SOURCE_UNAVAILABLE', 'GitHub redirected a Skill file.', [`${file.path}: ${sent.fetched.status}`]) + const response = sent.fetched.response + if (response.status !== 200 || !response.body) { + await response.body?.cancel() + if (response.status === 429) + return rateLimited('GitHub refused a Skill file: its rate limit is spent.', file.path, retryAt(response, now())) + return rejection('SOURCE_UNAVAILABLE', 'GitHub did not serve a Skill file.', [`${file.path}: ${response.status}`]) + } + await sink.begin(file, 'github') + const reader = response.body.getReader() + let received = 0 + while (true) { + const next = await reader.read().then( + value => ({ _tag: 'chunk' as const, value }), + (error: unknown) => ({ _tag: 'failed' as const, reason: describe(error) }), + ) + if (next._tag === 'failed') + return rejection('SOURCE_UNAVAILABLE', 'GitHub stopped serving a Skill file.', [`${file.path}: ${next.reason}`]) + if (next.value.done) + break + received += next.value.value.byteLength + await sink.chunk(next.value.value) + // The sink refuses extra bytes. Stop reading them too. + if (received > file.size) { + await reader.cancel().catch(() => { + // The answer is already wrong. A body that will not close changes nothing. + }) + break + } + } + const ended = await sink.end() + return ended._tag === 'mismatch' ? ended : null + } + + return async (sink, githubFiles) => { + let next = 0 + /** Reads from GitHub every file that sorts before `path`, or every file left when it is null. */ + const readMissingBefore = async (path: string | null): Promise => { + for (; next < input.files.length; next++) { + const file = input.files[next]! + if (path !== null && compareArtifactPaths(repositoryPath(file.path), path) >= 0) + break + const outcome = await readFromGithub(file, sink) + if (outcome) + return outcome + } + return null + } + + const archive = await openArchive(input, encodedRepository, gunzip, now) + // Reading every file one by one would meet the same limit. + if (archive._tag === 'rate-limited') + return rateLimited('GitHub refused the Repository archive: its rate limit is spent.', '', archive.retryAt) + if (archive._tag === 'unavailable') { + input.report?.({ reason: 'unavailable', detail: archive.reason }) + } + else { + const entries = archive.entries + try { + while (next < input.files.length) { + const entry = await entries.next() + if (entry._tag === 'end') + break + if (entry._tag === 'stopped') { + input.report?.({ reason: entry.reason, detail: entry.detail }) + break + } + if (entry.type !== 'file') { + const skipped = await entries.skip(entry.size) + if (skipped._tag !== 'ok') { + input.report?.({ reason: skipped._tag === 'too-large' ? 'too-large' : 'malformed', detail: skipped.detail }) + break + } + continue + } + const missing = await readMissingBefore(entry.path) + if (missing) + return missing + const file = input.files[next] + if (!file || repositoryPath(file.path) !== entry.path) { + const skipped = await entries.skip(entry.size) + if (skipped._tag !== 'ok') { + input.report?.({ reason: skipped._tag === 'too-large' ? 'too-large' : 'malformed', detail: skipped.detail }) + break + } + continue + } + if (githubFiles.has(file.path) || entry.size !== file.size) { + const skipped = await entries.skip(entry.size) + const outcome = await readFromGithub(file, sink) + if (outcome) + return outcome + next++ + if (skipped._tag !== 'ok') { + input.report?.({ reason: skipped._tag === 'too-large' ? 'too-large' : 'malformed', detail: skipped.detail }) + break + } + continue + } + await sink.begin(file, 'archive') + const forwarded = await entries.forward(entry.size, async bytes => await sink.chunk(bytes)) + const ended = await sink.end() + if (forwarded._tag !== 'ok') { + input.report?.({ reason: forwarded._tag === 'too-large' ? 'too-large' : 'malformed', detail: forwarded.detail }) + // The file arrived short. The next pass reads it from GitHub. + return ended._tag === 'mismatch' + ? ended + : { _tag: 'mismatch', path: file.path, origin: 'archive', reason: forwarded.detail } + } + if (ended._tag === 'mismatch') + return ended + next++ + } + } + finally { + await entries.cancel() + } + } + const rest = await readMissingBefore(null) + return rest ?? { _tag: 'read' } + } +} + +type OpenedArchive + = { _tag: 'open', entries: ArchiveEntries } + | { _tag: 'unavailable', reason: string } + | { _tag: 'rate-limited', retryAt: number | null } + +async function openArchive( + input: { fetch: typeof globalThis.fetch, commitSha: string, maxArchiveBytes: number }, + encodedRepository: string, + gunzip: (body: ReadableStream) => ReadableStream, + now: () => number, +): Promise { + const sent = await fetchNoRedirect(input.fetch, `${ARCHIVE_HOST}/${encodedRepository}/tar.gz/${input.commitSha}`, { + headers: { 'User-Agent': 'skilld.dev' }, + signal: AbortSignal.timeout(ARCHIVE_PASS_TIMEOUT_MS), + }).then( + fetched => ({ _tag: 'sent' as const, fetched }), + (error: unknown) => ({ _tag: 'threw' as const, reason: describe(error) }), + ) + if (sent._tag === 'threw') + return { _tag: 'unavailable', reason: sent.reason } + if (sent.fetched._tag === 'unexpected-redirect') + return { _tag: 'unavailable', reason: `codeload redirected ${sent.fetched.status}` } + const response = sent.fetched.response + if (response.status !== 200 || !response.body) { + await response.body?.cancel() + if (response.status === 429) + return { _tag: 'rate-limited', retryAt: retryAt(response, now()) } + return { _tag: 'unavailable', reason: `codeload returned ${response.status}` } + } + return { _tag: 'open', entries: archiveEntries(gunzip(response.body).getReader(), input.commitSha, input.maxArchiveBytes) } +} + +type ArchiveEntry + = { _tag: 'entry', type: 'file' | 'other', path: string, size: number } + | { _tag: 'end' } + | { _tag: 'stopped', reason: 'too-large' | 'other-commit' | 'malformed', detail: string } + +type BodyRead + = { _tag: 'ok' } + | { _tag: 'too-large' | 'malformed', detail: string } + +interface ArchiveEntries { + /** The next entry with a body still to read: a file, or something to skip. */ + next: () => Promise + /** Passes one entry body to `each` chunk by chunk, then skips its padding. */ + forward: (size: number, each: (bytes: Uint8Array) => Promise) => Promise + skip: (size: number) => Promise + cancel: () => Promise +} + +type Pulled + = { _tag: 'bytes', value: Uint8Array } + | { _tag: 'end' } + | { _tag: 'too-large' | 'malformed', detail: string } + +/** + * The entries of a GitHub tar archive. GitHub writes ustar headers, a pax + * global header that names the commit, pax `x` records for long paths, and + * wraps every path in one directory named for the Repository and commit. + */ +function archiveEntries( + reader: ReadableStreamDefaultReader, + commitSha: string, + maxBytes: number, +): ArchiveEntries { + let carry: Uint8Array = new Uint8Array(0) + let total = 0 + let cancelled = false + + const pull = async (): Promise => { + const next = await reader.read().then( + value => ({ _tag: 'read' as const, value }), + (error: unknown) => ({ _tag: 'failed' as const, detail: describe(error) }), + ) + if (next._tag === 'failed') + return { _tag: 'malformed', detail: next.detail } + if (next.value.done) + return { _tag: 'end' } + total += next.value.value.byteLength + if (total > maxBytes) + return { _tag: 'too-large', detail: `The archive passed ${maxBytes.toLocaleString('en-US')} bytes.` } + return { _tag: 'bytes', value: next.value.value } + } + + /** Exactly `size` bytes, for a header or a pax record. */ + const exact = async (size: number): Promise => { + const parts: Uint8Array[] = [] + let have = 0 + while (have < size) { + if (carry.byteLength === 0) { + const pulled = await pull() + if (pulled._tag === 'end') + return have === 0 ? pulled : { _tag: 'malformed', detail: 'The archive ended inside a header.' } + if (pulled._tag !== 'bytes') + return pulled + carry = pulled.value + } + const take = Math.min(size - have, carry.byteLength) + parts.push(carry.subarray(0, take)) + carry = carry.subarray(take) + have += take + } + if (parts.length === 1) + return { _tag: 'bytes', value: parts[0]! } + const merged = new Uint8Array(size) + let offset = 0 + for (const part of parts) { + merged.set(part, offset) + offset += part.byteLength + } + return { _tag: 'bytes', value: merged } + } + + const forward = async (size: number, each: ((bytes: Uint8Array) => Promise) | null): Promise => { + let left = roundToBlock(size) + let body = size + while (left > 0) { + if (carry.byteLength === 0) { + const pulled = await pull() + if (pulled._tag === 'end') + return { _tag: 'malformed', detail: 'The archive ended inside an entry.' } + if (pulled._tag !== 'bytes') + return pulled + carry = pulled.value + } + const take = Math.min(left, carry.byteLength) + const piece = carry.subarray(0, take) + carry = carry.subarray(take) + left -= take + if (each && body > 0) { + const content = piece.subarray(0, Math.min(body, piece.byteLength)) + body -= content.byteLength + await each(content) + } + } + return { _tag: 'ok' } + } + + return { + async next() { + let longPath: string | null = null + while (true) { + const block = await exact(BLOCK_SIZE) + if (block._tag === 'end') + return { _tag: 'end' } + if (block._tag !== 'bytes') + return { _tag: 'stopped', reason: block._tag, detail: block.detail } + const header = block.value + if (header.every(byte => byte === 0)) + continue + const size = readOctal(header.subarray(124, 136)) + const typeflag = String.fromCharCode(header[156]!) + if (size === null) + return { _tag: 'stopped', reason: 'malformed', detail: 'An archive header has an invalid size.' } + if (typeflag === 'g' || typeflag === 'x' || typeflag === 'L') { + if (size > MAX_HEADER_RECORD_BYTES) + return { _tag: 'stopped', reason: 'malformed', detail: 'An archive header record is too long.' } + const record = await exact(roundToBlock(size)) + if (record._tag === 'end') + return { _tag: 'stopped', reason: 'malformed', detail: 'The archive ended inside a header record.' } + if (record._tag !== 'bytes') + return { _tag: 'stopped', reason: record._tag, detail: record.detail } + const text = new TextDecoder().decode(record.value.subarray(0, size)) + if (typeflag === 'g') { + const comment = paxValue(text, 'comment') + if (comment !== null && comment !== commitSha) + return { _tag: 'stopped', reason: 'other-commit', detail: `The archive names commit ${comment.slice(0, 40)}.` } + } + else if (typeflag === 'x') { + longPath = paxValue(text, 'path') ?? longPath + } + else { + longPath = text.replace(/\0[\s\S]*$/, '') + } + continue + } + const name = longPath ?? headerPath(header) + longPath = null + const separator = name.indexOf('/') + const path = separator === -1 ? '' : name.slice(separator + 1) + return { + _tag: 'entry', + type: (typeflag === '0' || typeflag === '\0') && path !== '' ? 'file' : 'other', + path, + size, + } + } + }, + forward: async (size, each) => await forward(size, each), + skip: async size => await forward(size, null), + async cancel() { + if (cancelled) + return + cancelled = true + // Cancelling is the point of stopping early: the rest is never read. + await reader.cancel().catch(() => { + // The stream already ended or failed, which needs nothing here. + }) + }, + } +} + +/** + * The largest write the inflater takes. Gzip shrinks a run of zeros about + * 1,000 to 1, and an inflater answers each write in full, so one network + * chunk of 1 MiB could arrive as a gigabyte. Writes of 16 KiB cap that at + * about 16 MiB. + */ +const INFLATE_WRITE_BYTES = 16 * 1024 + +function gunzipStream(body: ReadableStream): ReadableStream { + const sliced = body.pipeThrough(new TransformStream({ + transform(chunk, controller) { + for (let offset = 0; offset < chunk.byteLength; offset += INFLATE_WRITE_BYTES) + controller.enqueue(chunk.subarray(offset, offset + INFLATE_WRITE_BYTES)) + }, + })) + // `DecompressionStream` declares its writable side as `BufferSource`, which + // the `ReadableStream` pipe signature refuses. Both sides carry + // `Uint8Array` at runtime. + return sliced.pipeThrough(new DecompressionStream('gzip') as unknown as ReadableWritablePair) +} + +function headerPath(header: Uint8Array): string { + const name = readString(header.subarray(0, 100)) + const prefix = readString(header.subarray(345, 500)) + return prefix ? `${prefix}/${name}` : name +} + +function paxValue(records: string, key: string): string | null { + for (const line of records.split('\n')) { + const match = /^\d+ ([^=]+)=(.*)$/.exec(line) + if (match?.[1] === key) + return match[2]! + } + return null +} + +function readString(bytes: Uint8Array): string { + const end = bytes.indexOf(0) + return new TextDecoder().decode(end === -1 ? bytes : bytes.subarray(0, end)) +} + +function readOctal(bytes: Uint8Array): number | null { + let text = '' + for (const byte of bytes) { + if (byte === 0 || byte === 0x20) + break + text += String.fromCharCode(byte) + } + if (text.length === 0) + return 0 + if (!/^[0-7]+$/.test(text)) + return null + const value = Number.parseInt(text, 8) + return Number.isSafeInteger(value) ? value : null +} + +function roundToBlock(size: number): number { + return Math.ceil(size / BLOCK_SIZE) * BLOCK_SIZE +} + +function describe(error: unknown): string { + return (error instanceof Error ? `${error.name}: ${error.message}` : String(error)).slice(0, 200) +} + +/** Unix seconds a `Retry-After` delay names, or null without one. */ +function retryAt(response: Response, now: number): number | null { + const seconds = Number(response.headers.get('retry-after')) + return Number.isSafeInteger(seconds) && seconds > 0 ? now + seconds : null +} + +function rateLimited(summary: string, path: string, at: number | null): SourceRejection { + const limited = rejection('RATE_LIMITED', summary, path ? [path] : []) + return at === null ? limited : { ...limited, retryAfterSeconds: at } +} + +function rejection(code: SourceRejection['code'], summary: string, findings: string[]): SourceRejection { + return { _tag: 'rejected', code, summary, findings: findings.map(finding => finding.slice(0, 500)) } +} diff --git a/layers/artifact-delivery/server/utils/artifact-pack.ts b/layers/artifact-delivery/server/utils/artifact-pack.ts new file mode 100644 index 00000000..a1b0da61 --- /dev/null +++ b/layers/artifact-delivery/server/utils/artifact-pack.ts @@ -0,0 +1,209 @@ +import type { Hash } from 'node:crypto' +import type { ResolvedSource } from '../schemas/contracts' +import type { ArtifactFileObserver, CheckedArtifactSource } from './checks' +import type { OmittedArtifactFile, SourceRejection } from './github-source' +import { createHash } from 'node:crypto' +import { createArtifactCheckScanner } from './checks' +import { projectedUstarBytes, USTAR_END_BYTES, ustarHeader, ustarPadding } from './ustar' + +/** One file an Artifact packs, as the Git tree names it. */ +export interface PackedFile { + /** The path inside the Skill folder. */ + path: string + mode: 420 | 493 + size: number + gitBlobSha: string +} + +/** + * Where one file's bytes came from: the Repository archive, or GitHub's copy + * of that one blob. Only archive bytes are worth reading again elsewhere. + */ +export type FileOrigin = 'archive' | 'github' + +/** A packed file whose bytes passed their Git digest check. */ +export type FileEnd + = { _tag: 'verified' } + | { _tag: 'mismatch', path: string, origin: FileOrigin, reason: string } + +/** Receives the bytes of each packed file, in Artifact order. */ +export interface FileSink { + begin: (file: PackedFile, origin: FileOrigin) => Promise + chunk: (bytes: Uint8Array) => Promise + end: () => Promise +} + +export type ReadOutcome + = { _tag: 'read' } + | { _tag: 'mismatch', path: string, origin: FileOrigin, reason: string } + | SourceRejection + +/** + * Reads every packed file once, in Artifact order, into the sink. + * + * `readFromGithub` names files whose archive bytes failed their digest. The + * reader reads those from GitHub one by one instead. + */ +export type SkillFileReader = (sink: FileSink, readFromGithub: ReadonlySet) => Promise + +/** + * Writes the deterministic USTAR archive of the files as their bytes stream + * past, and checks each file against its Git blob digest on the way. + * + * The output equals `createDeterministicUstar` over the same files. A file + * that fails its digest ends the read with `mismatch`; the bytes written + * so far are then wrong, and the caller discards them. + */ +export async function packArtifactFiles(input: { + files: readonly PackedFile[] + read: SkillFileReader + readFromGithub: ReadonlySet + write: (bytes: Uint8Array) => Promise + observer?: ArtifactFileObserver +}): Promise { + let next = 0 + let current: { file: PackedFile, origin: FileOrigin, received: number, digest: Hash, overflow: boolean } | null = null + const sink: FileSink = { + async begin(file, origin) { + const expected = input.files[next] + if (current || !expected || expected.path !== file.path) + throw new Error(`An Artifact reader skipped ahead to ${file.path}`) + current = { file: expected, origin, received: 0, digest: createHash('sha1').update(`blob ${expected.size}\0`), overflow: false } + input.observer?.begin(expected) + await input.write(ustarHeader(expected)) + }, + async chunk(bytes) { + if (!current) + throw new Error('An Artifact reader sent bytes outside a file') + if (bytes.byteLength === 0 || current.overflow) + return + const room = current.file.size - current.received + // The archive header already promised the size, so extra bytes never + // reach the output. The file then fails its check at `end`. + const kept = bytes.byteLength > room ? bytes.subarray(0, room) : bytes + current.overflow = bytes.byteLength > room + current.received += kept.byteLength + current.digest.update(kept) + input.observer?.chunk(kept) + if (kept.byteLength > 0) + await input.write(kept) + }, + async end() { + if (!current) + throw new Error('An Artifact reader ended no file') + const { file, origin, received, digest, overflow } = current + current = null + if (overflow || received !== file.size) + return { _tag: 'mismatch', path: file.path, origin, reason: `${received.toLocaleString('en-US')} bytes arrived for a ${file.size.toLocaleString('en-US')} byte blob` } + if (digest.digest('hex') !== file.gitBlobSha) + return { _tag: 'mismatch', path: file.path, origin, reason: 'the bytes failed their Git digest check' } + input.observer?.end() + const padding = ustarPadding(file.size) + if (padding > 0) + await input.write(new Uint8Array(padding)) + next++ + return { _tag: 'verified' } + }, + } + const outcome = await input.read(sink, input.readFromGithub) + if (outcome._tag !== 'read') + return outcome + if (current || next !== input.files.length) + throw new Error(`An Artifact reader stopped after ${next} of ${input.files.length} files`) + await input.write(new Uint8Array(USTAR_END_BYTES)) + return outcome +} + +export interface ScannedArtifact { + _tag: 'scanned' + contentSha256: string + contentBytes: number + checked: CheckedArtifactSource + /** The archive itself, when it is at most `spoolBytes`. Null means a store reads the files again. */ + spool: Uint8Array | null + /** Files read from GitHub one by one, because their archive bytes failed. */ + readFromGithub: ReadonlySet +} + +/** + * The largest archive a scan keeps in memory, so storing it reads nothing + * again. The build before streaming held up to 10 MiB of files and their + * 10 MiB archive at once; this keeps one copy of at most 16 MiB in an + * isolate of 128 MiB that also serves the site. A larger archive streams + * from GitHub a second time, straight into R2. + */ +export const ARTIFACT_SPOOL_BYTES = 16 * 1024 * 1024 + +/** + * The most times a scan reads the files again. Each pass reads from GitHub + * one more file whose archive bytes kept their size and failed their digest, + * as `export-subst` produces. A pass costs one archive request. + */ +const MAX_SCAN_PASSES = 4 + +/** + * Reads the files once to learn everything the build signs: the archive + * digest and size, the file inventory and the check results. Nothing is + * stored. An archive at most `spoolBytes` long is kept, so storing it reads + * nothing again. + */ +export async function scanArtifact(input: { + source: ResolvedSource + files: readonly PackedFile[] + read: SkillFileReader + omitted: OmittedArtifactFile[] + spoolBytes: number +}): Promise { + const contentBytes = projectedUstarBytes(input.files.map(file => file.size)) + const readFromGithub = new Set() + for (let pass = 0; pass < MAX_SCAN_PASSES; pass++) { + const digest = createHash('sha256') + const spool = contentBytes <= input.spoolBytes ? new Uint8Array(contentBytes) : null + let written = 0 + const scanner = createArtifactCheckScanner(input.source) + const outcome = await packArtifactFiles({ + files: input.files, + read: input.read, + readFromGithub, + observer: scanner, + write: async (bytes) => { + digest.update(bytes) + spool?.set(bytes, written) + written += bytes.byteLength + }, + }) + if (outcome._tag === 'rejected') + return outcome + if (outcome._tag === 'mismatch') { + if (outcome.origin === 'github' || readFromGithub.has(outcome.path)) + return digestRejection(outcome.path) + readFromGithub.add(outcome.path) + continue + } + if (written !== contentBytes) + throw new Error(`The Artifact packed to ${written} bytes, not the ${contentBytes} it declared`) + return { + _tag: 'scanned', + contentSha256: digest.digest('hex'), + contentBytes, + checked: scanner.finish(input.omitted), + spool, + readFromGithub, + } + } + return { + _tag: 'rejected', + code: 'SOURCE_UNAVAILABLE', + summary: 'The Repository archive kept serving files that fail their Git digest check.', + findings: [...readFromGithub].slice(0, 100), + } +} + +function digestRejection(path: string): SourceRejection { + return { + _tag: 'rejected', + code: 'INVALID_SOURCE', + summary: 'A Git blob failed its Git digest check.', + findings: [path], + } +} diff --git a/layers/artifact-delivery/server/utils/artifact-storage.ts b/layers/artifact-delivery/server/utils/artifact-storage.ts index d32dbbf6..f516f95a 100644 --- a/layers/artifact-delivery/server/utils/artifact-storage.ts +++ b/layers/artifact-delivery/server/utils/artifact-storage.ts @@ -13,15 +13,7 @@ export async function putImmutableArtifact( const stored = await bucket.put(input.key, input.bytes, { onlyIf: { etagDoesNotMatch: '*' }, sha256: checksumBytes(input.contentSha256), - httpMetadata: { - contentType: 'application/x-tar', - cacheControl: 'public, max-age=31536000, immutable', - contentDisposition: `attachment; filename="${input.contentSha256}.tar"`, - }, - customMetadata: { - contentSha256: input.contentSha256, - format: 'skilld-tar-v1', - }, + ...immutableArtifactOptions(input.contentSha256), }) if (stored) return { _tag: 'stored', key: input.key } @@ -67,3 +59,81 @@ function checksumHex(value: ArrayBuffer | undefined): string | null { function checksumBytes(value: string): Uint8Array { return Uint8Array.from(value.match(/.{2}/g) ?? [], byte => Number.parseInt(byte, 16)) } + +export type StreamedArtifactWrite + = ImmutableArtifactWrite + | { _tag: 'write-failed', failure: Failure } + +/** + * Stores an archive as it is written, never holding it whole. + * + * R2 takes a stream of known length in one part, and checks it against the + * SHA-256 the build already learned from its scan, so a byte that differs + * fails the put. The object keeps the checksum the signer and every reuse + * read back. A multipart upload would carry no whole-object SHA-256. + */ +export async function putImmutableArtifactStream( + bucket: R2Bucket, + input: { + key: string + contentSha256: string + contentBytes: number + /** Writes exactly `contentBytes`, or answers a failure and writes nothing more. */ + write: (write: (bytes: Uint8Array) => Promise) => Promise<{ _tag: 'written' } | { _tag: 'failed', failure: Failure }> + }, + fixedLengthStream: (length: number) => TransformStream = length => new FixedLengthStream(length), +): Promise> { + const expected = { contentSha256: input.contentSha256, contentBytes: input.contentBytes } + if (matchesImmutableArtifact(await bucket.head(input.key), expected)) + return { _tag: 'existing', key: input.key } + const { readable, writable } = fixedLengthStream(input.contentBytes) + const writer = writable.getWriter() + const stored = bucket.put(input.key, readable, { + onlyIf: { etagDoesNotMatch: '*' }, + sha256: checksumBytes(input.contentSha256), + ...immutableArtifactOptions(input.contentSha256), + }).then( + object => ({ _tag: 'put' as const, object }), + (error: unknown) => ({ _tag: 'refused' as const, error }), + ) + const written = await input.write(async (bytes) => { + await writer.write(bytes) + }).then( + outcome => outcome, + (error: unknown) => ({ _tag: 'threw' as const, error }), + ) + if (written._tag !== 'written') { + // An aborted stream ends the put without an object. + await writer.abort(written._tag === 'failed' ? 'write failed' : written.error).catch(() => { + // The put may have ended the stream first. Either way no object exists. + }) + await stored + if (written._tag === 'threw') + throw written.error + return { _tag: 'write-failed', failure: written.failure } + } + await writer.close() + const put = await stored + if (put._tag === 'refused') + throw put.error + if (put.object) + return { _tag: 'stored', key: input.key } + // Another build stored the same key first. + return matchesImmutableArtifact(await bucket.head(input.key), expected) + ? { _tag: 'existing', key: input.key } + : { _tag: 'mutation-rejected', key: input.key } +} + +function immutableArtifactOptions(contentSha256: string): Pick { + return { + httpMetadata: { + contentType: 'application/x-tar', + cacheControl: 'public, max-age=31536000, immutable', + contentDisposition: `attachment; filename="${contentSha256}.tar"`, + }, + customMetadata: { + contentSha256, + format: 'skilld-tar-v1', + }, + } +} diff --git a/layers/artifact-delivery/server/utils/attestation.ts b/layers/artifact-delivery/server/utils/attestation.ts index 762acdcd..06eb7a2b 100644 --- a/layers/artifact-delivery/server/utils/attestation.ts +++ b/layers/artifact-delivery/server/utils/attestation.ts @@ -4,6 +4,7 @@ import type { ArtifactFile, attestationSignatureSchema, CheckResult, + LinkedArtifactFile, ResolvedSource, } from '../schemas/contracts' import type { TrustedRoot } from './trusted-root' @@ -32,6 +33,7 @@ export function createAttestationStatement(input: { contentBytes: number files: ArtifactFile[] checkResults: CheckResult[] + linkedFiles?: LinkedArtifactFile[] }): ArtifactAttestationStatement { return { version: 1, @@ -45,6 +47,8 @@ export function createAttestationStatement(input: { policyVersion: ARTIFACT_POLICY_VERSION, files: input.files, checkResults: input.checkResults, + // An empty list is left out, so the statement stays one every skilld CLI reads. + ...(input.linkedFiles && input.linkedFiles.length > 0 ? { linkedFiles: input.linkedFiles } : {}), } } diff --git a/layers/artifact-delivery/server/utils/build.ts b/layers/artifact-delivery/server/utils/build.ts index 2d3cce45..d5051976 100644 --- a/layers/artifact-delivery/server/utils/build.ts +++ b/layers/artifact-delivery/server/utils/build.ts @@ -2,13 +2,15 @@ import type { ArtifactAttestation, ArtifactFile, CheckResult, + LinkedArtifactFile, ProblemCode, ResolvedSource, SourceRequest, } from '../schemas/contracts' +import type { PackedFile, ReadOutcome, SkillFileReader } from './artifact-pack' import type { ArtifactSigner } from './attestation' import type { CheckedArtifactSource } from './checks' -import type { ArtifactSourceFile, PublicGithubSourceClient, SourceRejection } from './github-source' +import type { PublicGithubSourceClient, SourceRejection } from './github-source' import type { ReadyBuildLookup, ReadyPublicBuild, ResolutionPatch, ResolutionRow } from './state' import type { TrustedRoot } from './trusted-root' import { createWideEvent } from '@harlan-zw/nuxt-wide-events/standalone' @@ -19,7 +21,8 @@ import { resolvedSourceSchema, sourceRequestSchema, } from '../schemas/contracts' -import { artifactR2Key, hasImmutableArtifact, putImmutableArtifact } from './artifact-storage' +import { ARTIFACT_SPOOL_BYTES, packArtifactFiles, scanArtifact } from './artifact-pack' +import { artifactR2Key, hasImmutableArtifact, putImmutableArtifact, putImmutableArtifactStream } from './artifact-storage' import { completeAttestation, createAttestationStatement, @@ -41,7 +44,7 @@ import { publishArtifactRecord, transitionResolution, } from './state' -import { createDeterministicUstar, readDeterministicUstar } from './ustar' +import { readDeterministicUstar } from './ustar' export interface ArtifactBuildDependencies { db: D1Database @@ -77,6 +80,8 @@ export interface ArtifactBuildDependencies { * `artifact-build-reuse` wide event, which the daily check-in can count. */ reportReuse?: (report: ArtifactBuildReuseReport) => void + /** The stream R2 reads a streamed archive from. Workers use `FixedLengthStream`. */ + fixedLengthStream?: (length: number) => TransformStream } /** Why a build did not reuse a ready build of the same commit. */ @@ -87,6 +92,7 @@ export type ReuseMissReason | 'checks-changed' | 'record-mismatch' | 'bytes-missing' + | 'linked-files-differ' export type ArtifactBuildReuseReport = { _tag: 'hit', lookup: ReadyBuildLookup['_tag'], resolutionId: string, reusedFrom: string } @@ -116,6 +122,7 @@ interface ReusableBuild { contentBytes: number files: ArtifactFile[] checkResults: CheckResult[] + linkedFiles: LinkedArtifactFile[] } type ReuseDecision @@ -125,10 +132,21 @@ type ReuseDecision interface LoadedBuild { _tag: 'loaded' source: ResolvedSource - files: ArtifactSourceFile[] checked: CheckedArtifactSource + content: { contentSha256: string, contentBytes: number } + linkedFiles: LinkedArtifactFile[] + bytes: ArtifactBytes } +/** + * Where a loaded build gets the archive it stores: kept from its scan when + * small, written again from GitHub as it streams to R2, or already stored. + */ +type ArtifactBytes + = { _tag: 'spooled', archive: Uint8Array } + | { _tag: 'streamed', files: readonly PackedFile[], read: SkillFileReader, readFromGithub: ReadonlySet } + | { _tag: 'stored', key: string } + interface ReusedBuild { _tag: 'reused' build: ReusableBuild @@ -221,6 +239,15 @@ export async function processArtifactBuild( : rejectionCheckResults(loaded.rejection) if (loaded._tag === 'rejected' && loaded.rejection.code !== 'INVALID_SOURCE') return await failWithRejection(dependencies, row, loaded.rejection) + // A large archive streams from GitHub to R2 for a second time. That + // happens here, before the checks are recorded: the skilld CLI gives a + // Resolution 15 seconds at each stage after its checks pass. + if (loaded._tag === 'loaded' && row.visibility === 'public' && !checksBlockArtifact(checkResults)) { + const stored = await storePublicArtifact(dependencies, loaded) + if (stored._tag === 'rejected') + return await failWithRejection(dependencies, row, stored) + carried = { ...loaded, bytes: { _tag: 'stored', key: stored.key } } + } const advanced = await transitionResolution(dependencies.db, row, 'checking', { checkResultsJson: JSON.stringify(checkResults), }, now) @@ -249,7 +276,7 @@ export async function processArtifactBuild( carried = loaded const content = loaded._tag === 'reused' ? { contentSha256: loaded.build.contentSha256, contentBytes: loaded.build.contentBytes } - : await packagedContent(loaded.files) + : loaded.content const advanced = await transitionResolution(dependencies.db, row, 'signing', { artifactId: `sha256:${content.contentSha256}`, contentSha256: content.contentSha256, @@ -280,6 +307,8 @@ export async function processArtifactBuild( : await stageLoadedStatement(dependencies, row, content, loaded) if (statement._tag === 'content-changed') return await failResolution(dependencies, row, 'INVALID_SOURCE', false) + if (statement._tag === 'rejected') + return await failWithRejection(dependencies, row, statement) const staged = await transitionResolution(dependencies.db, row, 'signing', statement.patch, now) if (staged._tag === 'superseded') return { _tag: 'superseded', resolutionId } @@ -320,9 +349,46 @@ export async function processArtifactBuild( throw new Error('Artifact build exceeded its state transition limit') } -async function packagedContent(files: ArtifactSourceFile[]): Promise<{ contentSha256: string, contentBytes: number }> { - const archive = createDeterministicUstar(files) - return { contentSha256: await digestHex('SHA-256', archive), contentBytes: archive.byteLength } +/** + * Stores a public Artifact under its content address. A small archive is + * already in memory from the scan. A larger one is read from GitHub again and + * written to R2 as it streams; R2 refuses it unless every byte matches the + * digest the scan found. Storing the same archive twice writes nothing. + */ +async function storePublicArtifact( + dependencies: ArtifactBuildDependencies, + loaded: LoadedBuild, +): Promise<{ _tag: 'stored', key: string } | SourceRejection> { + const key = artifactR2Key(loaded.content.contentSha256) + const bytes = loaded.bytes + if (bytes._tag === 'stored') + return { _tag: 'stored', key: bytes.key } + const write = bytes._tag === 'spooled' + ? await putImmutableArtifact(dependencies.bucket, { key, bytes: bytes.archive, contentSha256: loaded.content.contentSha256 }) + : await putImmutableArtifactStream>(dependencies.bucket, { + key, + contentSha256: loaded.content.contentSha256, + contentBytes: loaded.content.contentBytes, + write: async (output) => { + const outcome = await packArtifactFiles({ files: bytes.files, read: bytes.read, readFromGithub: bytes.readFromGithub, write: output }) + return outcome._tag === 'read' ? { _tag: 'written' as const } : { _tag: 'failed' as const, failure: outcome } + }, + }, dependencies.fixedLengthStream) + if (write._tag === 'mutation-rejected') + throw new Error('Immutable Artifact storage rejected changed bytes') + if (write._tag === 'write-failed') { + if (write.failure._tag === 'rejected') + return write.failure + // The scan read the same commit a moment ago. Another answer now is a + // fault upstream, and a new attempt reads it again. + return { + _tag: 'rejected', + code: 'SOURCE_UNAVAILABLE', + summary: 'GitHub served other bytes for a Skill file on the second read.', + findings: [write.failure.path], + } + } + return { _tag: 'stored', key: write.key } } /** Store the loaded bytes, then stage the statement the signer signs. */ @@ -331,37 +397,38 @@ async function stageLoadedStatement( row: ResolutionRow, content: { artifactId: string, contentSha256: string, contentBytes: number }, loaded: LoadedBuild, -): Promise { - const archive = createDeterministicUstar(loaded.files) - const contentSha256 = await digestHex('SHA-256', archive) - if (contentSha256 !== content.contentSha256 || archive.byteLength !== content.contentBytes) +): Promise { + if (loaded.content.contentSha256 !== content.contentSha256 || loaded.content.contentBytes !== content.contentBytes) return { _tag: 'content-changed' } - const write = row.visibility === 'private' - ? await putEncryptedPrivateArtifact(dependencies, row, archive, contentSha256) - : await putImmutableArtifact(dependencies.bucket, { - key: artifactR2Key(contentSha256), - bytes: archive, - contentSha256, - }) - if (write._tag === 'mutation-rejected') - throw new Error('Immutable Artifact storage rejected changed bytes') + let storagePatch: Pick + if (row.visibility === 'private') { + if (loaded.bytes._tag !== 'spooled') + throw new Error('A private Artifact must be packed in memory') + const write = await putEncryptedPrivateArtifact(dependencies, row, loaded.bytes.archive, content.contentSha256) + if (write._tag === 'mutation-rejected') + throw new Error('Immutable Artifact storage rejected changed bytes') + storagePatch = { r2Key: write.key, ...privateArtifactStoragePatch(write) } + } + else { + const stored = await storePublicArtifact(dependencies, loaded) + if (stored._tag === 'rejected') + return stored + storagePatch = { r2Key: stored.key } + } const statement = createAttestationStatement({ artifactId: content.artifactId, createdAt: new Date(row.created_at * 1000).toISOString(), source: loaded.source, - contentSha256, - contentBytes: archive.byteLength, + contentSha256: content.contentSha256, + contentBytes: content.contentBytes, files: loaded.checked.files, checkResults: loaded.checked.checkResults, + linkedFiles: loaded.linkedFiles, }) - const privateStoragePatch = row.visibility === 'private' - ? privateArtifactStoragePatch(write) - : {} return { _tag: 'staged', patch: { - r2Key: write.key, - ...privateStoragePatch, + ...storagePatch, attestationStatementJson: encodeAttestationStatement(statement), }, } @@ -396,6 +463,7 @@ function stageReusedStatement( contentBytes: content.contentBytes, files: build.files, checkResults: build.checkResults, + linkedFiles: build.linkedFiles, }) return { _tag: 'staged', @@ -494,7 +562,9 @@ async function loadStoredBuild( lookup: ReadyBuildLookup, ): Promise { const ready = await findReadyPublicBuild(dependencies.db, lookup) - if (!ready) + // A check reads the stored archive whole, so only one the size a scan + // keeps in memory is checked again. A larger one streams from GitHub. + if (!ready || ready.contentBytes > ARTIFACT_SPOOL_BYTES) return null const attestation = parseStoredAttestation(ready.attestationJson) if ( @@ -525,8 +595,10 @@ async function loadStoredBuild( return { _tag: 'loaded', source: ready.source, - files, checked: await checkArtifactSource(ready.source, files, []), + content: { contentSha256: ready.contentSha256, contentBytes: ready.contentBytes }, + linkedFiles: [], + bytes: { _tag: 'spooled', archive: bytes }, } } @@ -570,7 +642,7 @@ async function decideReuse( lookup: ReadyBuildLookup, fitsRecord: (build: ReusableBuild) => boolean = () => true, ): Promise { - const found = await findReusableBuild(dependencies, lookup) + const found = await findReusableBuild(dependencies, lookup, acceptsLinkedFiles(row)) const decision: ReuseDecision = found._tag === 'hit' && !fitsRecord(found.build) ? { _tag: 'miss', reason: 'record-mismatch' } : found @@ -595,6 +667,7 @@ async function decideReuse( async function findReusableBuild( dependencies: ArtifactBuildDependencies, lookup: ReadyBuildLookup, + linkedFiles: boolean, ): Promise { const ready = await findReadyPublicBuild(dependencies.db, lookup) if (!ready) @@ -608,6 +681,8 @@ async function findReusableBuild( return { _tag: 'miss', reason: 'checks-changed' } if (!attestationMatchesRecord(attestation, ready)) return { _tag: 'miss', reason: 'record-mismatch' } + if (!deliversTo(attestation, linkedFiles)) + return { _tag: 'miss', reason: 'linked-files-differ' } const stored = await hasImmutableArtifact(dependencies.bucket, { key: ready.r2Key, contentSha256: ready.contentSha256, @@ -625,10 +700,29 @@ async function findReusableBuild( contentBytes: ready.contentBytes, files: attestation.files, checkResults: attestation.checkResults, + linkedFiles: attestation.linkedFiles ?? [], }, } } +/** + * Whether a ready build answers a skilld CLI that does, or does not, read + * linked files. A build with linked files serves only a CLI that reads them. + * A build that left no file out serves both, since neither would link or + * omit anything. A build that left files out can only be rebuilt for a CLI + * that reads linked files: it may link some of them. + */ +function deliversTo(attestation: ArtifactAttestation, linkedFiles: boolean): boolean { + if (attestation.linkedFiles) + return linkedFiles + return !linkedFiles || omittedNothing(attestation) +} + +/** Whether the Resolution came from a skilld CLI that reads linked files. */ +function acceptsLinkedFiles(row: ResolutionRow): boolean { + return row.linked_files === 1 +} + /** Whether a stored attestation names exactly the bytes and source of its D1 record. */ function attestationMatchesRecord(attestation: ArtifactAttestation, ready: ReadyPublicBuild): boolean { return attestation.artifactId === ready.artifactId @@ -694,11 +788,31 @@ async function loadAndCheck( const github = await githubForResolution(dependencies, row) if (isSourceRejection(github)) return { _tag: 'rejected', rejection: github } - const loaded = await github.load(source) + const loaded = await github.load(source, { linkedFiles: acceptsLinkedFiles(row) }) if (loaded._tag === 'rejected') return { _tag: 'rejected', rejection: loaded } - const checked = await checkArtifactSource(source, loaded.value.files, loaded.value.omitted) - return { _tag: 'loaded', source, files: loaded.value.files, checked } + const plan = loaded.value + // A private archive is encrypted whole, inside the private limits, so it is + // always kept. A public one is kept only when small. + const scanned = await scanArtifact({ + source, + files: plan.files, + read: plan.read, + omitted: plan.omitted, + spoolBytes: row.visibility === 'private' ? Number.POSITIVE_INFINITY : ARTIFACT_SPOOL_BYTES, + }) + if (scanned._tag === 'rejected') + return { _tag: 'rejected', rejection: scanned } + return { + _tag: 'loaded', + source, + checked: scanned.checked, + content: { contentSha256: scanned.contentSha256, contentBytes: scanned.contentBytes }, + linkedFiles: plan.linked, + bytes: scanned.spool + ? { _tag: 'spooled', archive: scanned.spool } + : { _tag: 'streamed', files: plan.files, read: plan.read, readFromGithub: scanned.readFromGithub }, + } } async function requirePassingSource( diff --git a/layers/artifact-delivery/server/utils/checks.ts b/layers/artifact-delivery/server/utils/checks.ts index ed637bb5..c0d6d5f4 100644 --- a/layers/artifact-delivery/server/utils/checks.ts +++ b/layers/artifact-delivery/server/utils/checks.ts @@ -1,6 +1,7 @@ +import type { Hash } from 'node:crypto' import type { ArtifactFile, CheckResult, ResolvedSource } from '../schemas/contracts' import type { ArtifactSourceFile, OmittedArtifactFile } from './github-source' -import { digestHex } from './encoding' +import { createHash } from 'node:crypto' import { ARTIFACT_POLICY_VERSION } from './state' /** @@ -57,12 +58,9 @@ const CURRENT_ARTIFACT_CHECKS: ArtifactCheckSet = new Map([ */ export const SIGNABLE_ARTIFACT_POLICIES: ReadonlyMap = new Map([ [ARTIFACT_POLICY_VERSION, CURRENT_ARTIFACT_CHECKS], - ['2026-10-07.1', new Map([ - ['path-policy', { version: '1', required: true }], - ['agent-skills-spec', { version: '2026-10-07', required: false }], - ['credential-material', { version: '1', required: true }], - ['executable-files', { version: '1', required: false }], - ])], + // ADR-0013 changed loading and packaging only, so the policy before it + // carries the same checks. + ['2026-10-07.2', CURRENT_ARTIFACT_CHECKS], ]) export interface CheckedArtifactSource { @@ -75,39 +73,163 @@ export async function checkArtifactSource( files: ArtifactSourceFile[], omitted: OmittedArtifactFile[] = [], ): Promise { - const fileInventory = await Promise.all(files.map(async file => ({ - path: file.path, - mode: file.mode, - size: file.bytes.byteLength, - sha256: await digestHex('SHA-256', file.bytes), - } satisfies ArtifactFile))) - const pathFindings = files - .filter(file => !splitUstarPath(file.path)) - .map(file => file.path) - const pathPolicy: CheckResult = pathFindings.length > 0 - ? { - name: 'path-policy', - version: PATH_POLICY_VERSION, - outcome: 'fail', - required: true, - summary: 'A Skill path cannot be represented by the Artifact format.', - findings: pathFindings, + const scanner = createArtifactCheckScanner(source) + for (const file of files) { + scanner.begin({ path: file.path, mode: file.mode, size: file.bytes.byteLength }) + scanner.chunk(file.bytes) + scanner.end() + } + return scanner.finish(omitted) +} + +/** Receives the Skill files one at a time, each as a run of chunks. */ +export interface ArtifactFileObserver { + begin: (file: { path: string, mode: 420 | 493, size: number }) => void + chunk: (bytes: Uint8Array) => void + end: () => void +} + +export interface ArtifactCheckScanner extends ArtifactFileObserver { + finish: (omitted: OmittedArtifactFile[]) => CheckedArtifactSource +} + +/** + * Runs every check over files that stream past once, so a build never holds + * the whole Skill. Only SKILL.md is kept, for its frontmatter. The results + * match {@link checkArtifactSource} over the same bytes, in any chunk size. + */ +export function createArtifactCheckScanner(source: ResolvedSource): ArtifactCheckScanner { + const inventory: ArtifactFile[] = [] + const pathFindings: string[] = [] + const credentialFindings: string[] = [] + const executableFindings: string[] = [] + let skillBytes: Uint8Array[] | null = null + let current: { + path: string + mode: 420 | 493 + size: number + sha256: Hash + credentials: CredentialScanner + skill: Uint8Array[] | null + } | null = null + + return { + begin(file) { + if (current) + throw new Error(`The check scanner is still reading ${current.path}`) + current = { + ...file, + sha256: createHash('sha256'), + credentials: createCredentialScanner(), + skill: file.path === 'SKILL.md' ? [] : null, } - : { - name: 'path-policy', - version: PATH_POLICY_VERSION, - outcome: 'pass', - required: true, + }, + chunk(bytes) { + if (!current) + throw new Error('The check scanner received bytes outside a file') + current.sha256.update(bytes) + current.credentials.chunk(bytes) + current.skill?.push(bytes.slice()) + }, + end() { + if (!current) + throw new Error('The check scanner ended no file') + const file = current + current = null + inventory.push({ path: file.path, mode: file.mode, size: file.size, sha256: file.sha256.digest('hex') }) + if (!splitUstarPath(file.path)) + pathFindings.push(file.path) + if (file.credentials.end()) + credentialFindings.push(`${file.path} contains private key material.`) + if (file.mode === 493) + executableFindings.push(file.path) + if (file.skill) + skillBytes = file.skill + }, + finish(omitted) { + if (current) + throw new Error(`The check scanner did not end ${current.path}`) + const pathPolicy: CheckResult = pathFindings.length > 0 + ? { + name: 'path-policy', + version: PATH_POLICY_VERSION, + outcome: 'fail', + required: true, + summary: `A Skill path cannot be represented by the Artifact format.${listedNote(pathFindings)}`, + findings: boundedFindings(pathFindings), + } + : { + name: 'path-policy', + version: PATH_POLICY_VERSION, + outcome: 'pass', + required: true, + } + const credentialMaterial: CheckResult = credentialFindings.length > 0 + ? { + name: 'credential-material', + version: CREDENTIAL_MATERIAL_VERSION, + outcome: 'fail', + required: true, + summary: `The Skill contains private key material.${listedNote(credentialFindings)}`, + findings: boundedFindings(credentialFindings), + } + : { + name: 'credential-material', + version: CREDENTIAL_MATERIAL_VERSION, + outcome: 'pass', + required: true, + } + const executableFiles: CheckResult = executableFindings.length > 0 + ? { + name: 'executable-files', + version: EXECUTABLE_FILES_VERSION, + outcome: 'warn', + required: false, + summary: `The Skill contains executable files.${listedNote(executableFindings)}`, + findings: boundedFindings(executableFindings), + } + : { + name: 'executable-files', + version: EXECUTABLE_FILES_VERSION, + outcome: 'pass', + required: false, + } + return { + files: inventory, + checkResults: [ + pathPolicy, + agentSkillsSpecResult(source, skillBytes), + credentialMaterial, + executableFiles, + omittedFilesResult(omitted), + ], } + }, + } +} + +/** + * The findings a check result carries. The skilld CLI refuses more than 100, + * or one longer than 500 characters, and a Skill may now hold 2,000 files. + */ +function boundedFindings(findings: string[]): string[] { + return findings.slice(0, MAX_CHECK_FINDINGS).map(finding => finding.slice(0, MAX_CHECK_FINDING_CHARACTERS)) +} + +/** Names how many findings a result leaves unlisted, or nothing when it lists them all. */ +function listedNote(findings: string[]): string { + return findings.length > MAX_CHECK_FINDINGS + ? ` The first ${MAX_CHECK_FINDINGS} of ${findings.length.toLocaleString('en-US')} are listed.` + : '' +} - const skill = files.find(file => file.path === 'SKILL.md') +function agentSkillsSpecResult(source: ResolvedSource, skillChunks: Uint8Array[] | null): CheckResult { const specFindings: string[] = [] - let decodedSkill = '' - if (!skill) { + if (!skillChunks) { specFindings.push('SKILL.md is missing.') } else { - decodedSkill = decodeText(skill.bytes) ?? '' + const decodedSkill = decodeText(concatBytes(skillChunks)) ?? '' if (!decodedSkill) { specFindings.push('SKILL.md must contain UTF-8 text.') } @@ -133,7 +255,7 @@ export async function checkArtifactSource( } } } - const agentSkillsSpec: CheckResult = specFindings.length > 0 + return specFindings.length > 0 ? { name: 'agent-skills-spec', version: AGENT_SKILLS_CHECK_VERSION, @@ -148,52 +270,89 @@ export async function checkArtifactSource( outcome: 'pass', required: false, } +} - const credentialFindings: string[] = [] - for (const file of files) { - const text = decodeText(file.bytes) - if (!text) - continue - if (containsPrivateKey(text)) - credentialFindings.push(`${file.path} contains private key material.`) - } - const credentialMaterial: CheckResult = credentialFindings.length > 0 - ? { - name: 'credential-material', - version: CREDENTIAL_MATERIAL_VERSION, - outcome: 'fail', - required: true, - summary: 'The Skill contains private key material.', - findings: credentialFindings, - } - : { - name: 'credential-material', - version: CREDENTIAL_MATERIAL_VERSION, - outcome: 'pass', - required: true, - } +/** + * Text the credential check reads around a chunk boundary. A key block the + * check finds is at most 16,384 body characters plus its two marker lines, so + * twice that keeps every block whole in one window. + */ +const CREDENTIAL_WINDOW_CHARACTERS = 32 * 1024 - const executableFindings = files.filter(file => file.mode === 493).map(file => file.path) - const executableFiles: CheckResult = executableFindings.length > 0 - ? { - name: 'executable-files', - version: EXECUTABLE_FILES_VERSION, - outcome: 'warn', - required: false, - summary: 'The Skill contains executable files.', - findings: executableFindings, - } - : { - name: 'executable-files', - version: EXECUTABLE_FILES_VERSION, - outcome: 'pass', - required: false, - } +interface CredentialScanner { + chunk: (bytes: Uint8Array) => void + /** True when the file is UTF-8 text that holds a private key. */ + end: () => boolean +} +/** + * The credential check over a file that arrives in chunks. A file that is not + * valid UTF-8 from end to end is not text, so it has no finding, as before. + */ +function createCredentialScanner(): CredentialScanner { + const decoder = new TextDecoder('utf-8', { fatal: true, ignoreBOM: false }) + let text = true + let found = false + let tail = '' + let pending = '' + // Small network reads collect into one piece first, so the window is not + // searched again for every few bytes. + const scan = (piece: string, flush: boolean): void => { + if (found) + return + pending += piece + if (!flush && pending.length < CREDENTIAL_WINDOW_CHARACTERS / 2) + return + const window = tail + pending + pending = '' + if (containsPrivateKey(window)) + found = true + tail = window.slice(-CREDENTIAL_WINDOW_CHARACTERS) + } + const decode = (bytes?: Uint8Array): string | null => { + try { + return bytes ? decoder.decode(bytes, { stream: true }) : decoder.decode() + } + catch { + // Invalid UTF-8 is an expected result: the file is not text. + return null + } + } return { - files: fileInventory, - checkResults: [pathPolicy, agentSkillsSpec, credentialMaterial, executableFiles, omittedFilesResult(omitted)], + chunk(bytes) { + if (!text) + return + const piece = decode(bytes) + if (piece === null) { + text = false + tail = '' + pending = '' + return + } + scan(piece, false) + }, + end() { + if (!text) + return false + const piece = decode() + if (piece === null) + return false + scan(piece, true) + return found + }, + } +} + +function concatBytes(parts: Uint8Array[]): Uint8Array { + if (parts.length === 1) + return parts[0]! + const merged = new Uint8Array(parts.reduce((total, part) => total + part.byteLength, 0)) + let offset = 0 + for (const part of parts) { + merged.set(part, offset) + offset += part.byteLength } + return merged } function omittedFilesResult(omitted: OmittedArtifactFile[]): CheckResult { diff --git a/layers/artifact-delivery/server/utils/github-read-credential.ts b/layers/artifact-delivery/server/utils/github-read-credential.ts index 6398b471..46a49caf 100644 --- a/layers/artifact-delivery/server/utils/github-read-credential.ts +++ b/layers/artifact-delivery/server/utils/github-read-credential.ts @@ -151,7 +151,7 @@ export function withGithubCredential( ): PublicGithubSourceClient { return { resolve: async request => await create(await credential()).resolve(request), - load: async source => await create(await credential()).load(source), + load: async (source, options) => await create(await credential()).load(source, options), } } diff --git a/layers/artifact-delivery/server/utils/github-source.ts b/layers/artifact-delivery/server/utils/github-source.ts index e7f50759..6a5bb0b6 100644 --- a/layers/artifact-delivery/server/utils/github-source.ts +++ b/layers/artifact-delivery/server/utils/github-source.ts @@ -1,8 +1,11 @@ -import type { ProblemCode, ResolvedSource, SourceRequest } from '../schemas/contracts' +import type { LinkedArtifactFile, ProblemCode, ResolvedSource, SourceRequest } from '../schemas/contracts' +import type { ArchiveReport } from './archive-reader' +import type { PackedFile, SkillFileReader } from './artifact-pack' import type { GithubReadTry } from './github-read' -import type { TarballExtraction } from './tarball-source' import { z } from 'zod' import { canonicalSkillFolder, isRegistrySkillPath, slugifySkillName } from '#shared/skill-path' +import { createGithubArchiveReader } from './archive-reader' +import { splitUstarPath } from './checks' import { base64ToBytes, gitBlobShaHex } from './encoding' import { fetchNoRedirect } from './fetch-no-redirect' import { @@ -14,13 +17,18 @@ import { isTruncatedBody, readGithubWithRetry, } from './github-read' -import { extractSkillFilesFromTarball } from './tarball-source' -import { projectedUstarBytes } from './ustar' +import { compareArtifactPaths, projectedUstarBytes } from './ustar' const GITHUB_API = 'https://api.github.com' const GITHUB_API_VERSION = '2026-03-10' const MAX_GITHUB_JSON_BYTES = 8 * 1024 * 1024 -const MAX_TREE_ENTRIES = 2000 +/** + * Entries one Skill folder listing may hold, files and folders together. The + * recursive tree read answers up to 100,000 entries in one request, and the + * file limit below decides what a Skill may pack. It matches the skilld CLI's + * own ceiling for a direct GitHub read. + */ +const MAX_TREE_ENTRIES = 20_000 const MAX_TREE_REQUESTS = 128 /** * The tree reads one search by name may spend. Every read comes out of the @@ -33,34 +41,85 @@ const MAX_NAME_SEARCH_TREE_READS = 64 // time keep a split walk well inside a Worker's 128 MiB. const TREE_WALK_CONCURRENCY = 2 const MAX_SKILL_PATH_SEGMENTS = 64 -// One invocation loads every blob once. With one repository read per load and -// resolve, a handful of tree reads, the D1 state transitions and the R2 write, -// 900 files is the largest ceiling that keeps the worst build inside the -// Worker's 1000-subrequest budget. -const MAX_ARTIFACT_FILES = 900 -const MAX_FILE_BYTES = 2 * 1024 * 1024 -const MAX_ARTIFACT_BYTES = 10 * 1024 * 1024 -// A Repository tarball is one request that costs no REST quota, and it carries -// every file of the Skill. Measured 2026-09-22 on a 33-Skill Repository: 3 -// counted requests and 0.45 s, against 6,525 requests and about 377 s per -// blob. See notes/skilld-tarball-delivery-spike-2026-09-22. -const TARBALL_REQUEST_TIMEOUT_MS = 60_000 + +/** + * The limits one Artifact build applies. ADR-0013 measures the constraint + * behind each one. + */ +export interface ArtifactLimits { + /** The packed archive. */ + maxArchiveBytes: number + /** One file, or null when only the archive limit applies. */ + maxFileBytes: number | null + /** Files the attestation lists, packed and linked together. */ + maxFiles: number + /** Bytes the skilld CLI may read from GitHub as linked files, or null when it cannot. */ + maxLinkedBytes: number | null +} + +const MIB = 1024 * 1024 + +/** + * A public build streams the Repository archive, so neither the Worker's + * memory nor its subrequests depend on the Skill size. + * + * - 64 MiB: the largest Artifact the skilld CLI downloads, from 3.0 on. + * - 2,000 files: the most files the skilld CLI accepts in an attestation. + */ +export const PUBLIC_ARTIFACT_LIMITS: ArtifactLimits = { + maxArchiveBytes: 64 * MIB, + maxFileBytes: null, + maxFiles: 2000, + maxLinkedBytes: null, +} + +/** + * Linked files a skilld CLI that reads them may fetch from GitHub. It holds + * every Skill file in memory before it writes one, and GitHub refuses a file + * over 100 MiB in a Repository. + */ +export const MAX_LINKED_BYTES = 256 * MIB + +/** + * A private build reads one blob per request through the REST API and + * encrypts the archive in memory, so its limits are unchanged: + * + * - 900 files: one blob read each, inside the 1,000 subrequest budget. + * - 2 MiB a file and 10 MiB in all: the blob JSON and the in-memory archive. + */ +export const PRIVATE_ARTIFACT_LIMITS: ArtifactLimits = { + maxArchiveBytes: 10 * MIB, + maxFileBytes: 2 * MIB, + maxFiles: 900, + maxLinkedBytes: null, +} + /** - * The largest Git tree the tarball path will read for, in source bytes. + * The file list a statement may carry. One entry is at most 126 bytes of JSON + * plus its path. * - * The extracted files stay in the isolate until the Artifact is packaged, and - * a Worker has 128 MiB. `MAX_ARTIFACT_BYTES` binds first for every Skill built - * today; this ceiling states the memory budget the byte source itself has to - * respect. + * D1 stores a row of at most 2,000,000 bytes. The Resolution row holds the + * check results, the statement, and the attestation, which repeats the + * statement and adds it again as base64: about 3.34 statements and the check + * results once more. With 100 KB of check results, a 448 KiB file list keeps + * that row near 1.97 MB. */ -export const TARBALL_MAX_TREE_BYTES = 128 * 1024 * 1024 +const MAX_FILE_LIST_BYTES = 448 * 1024 +const FILE_LIST_ENTRY_BYTES = 126 + +/** + * Uncompressed archive bytes one pass reads before it stops. Measured + * 2026-10-07: inflating and walking a GitHub archive costs 4 to 5 ms of CPU + * per MiB, so a full gigabyte is about 5 seconds against the consumer's 300. + * A pass stops after the last Skill file, so most read far less. + */ +export const ARCHIVE_MAX_UNCOMPRESSED_BYTES = 1024 * MIB + /** - * The archive is the whole Repository, whose size no cheap request reveals: - * codeload sends no `content-length` until that exact commit is cached, so the - * only reliable guard is a running count with a hard stop. Measured: the stop - * cancelled a 1.5 GiB archive after 64 MiB, 2.9 s and 390 ms of CPU. + * Files one build may read from GitHub one by one: those the archive left + * out or changed. Each is a subrequest, and a store reads them again. */ -export const TARBALL_MAX_UNCOMPRESSED_BYTES = 256 * 1024 * 1024 +const MAX_GITHUB_FILE_READS = 400 const shaSchema = z.string().regex(/^[a-f0-9]{40}$/) const repositoryResponseSchema = z.object({ @@ -137,8 +196,18 @@ export interface OmittedArtifactFile { export interface LoadedArtifactSource { source: ResolvedSource - files: ArtifactSourceFile[] + /** The files the Artifact packs, in Artifact order. */ + files: PackedFile[] omitted: OmittedArtifactFile[] + /** Files the attestation lists for the skilld CLI to read from GitHub. */ + linked: LinkedArtifactFile[] + /** Streams the packed files' bytes. Each call reads them again. */ + read: SkillFileReader +} + +export interface LoadOptions { + /** The skilld CLI that asked reads linked files. */ + linkedFiles: boolean } export interface SourceRejection { @@ -160,7 +229,11 @@ export type LoadSourceResult export interface PublicGithubSourceClient { resolve: (request: SourceRequest) => Promise - load: (source: ResolvedSource) => Promise + /** + * `options` is required, so a client that wraps another cannot drop it: + * #495's fallback once did, and no CLI got a linked file. + */ + load: (source: ResolvedSource, options: LoadOptions) => Promise } /** One failed GitHub REST read. It never carries a token or a query string. */ @@ -189,6 +262,8 @@ interface GithubClientOptions { * its SHA. Pass it to a public client only, so private trees stay out. */ cache?: GithubObjectCache + /** Inflates the Repository archive. Workers use `DecompressionStream`. */ + gunzip?: (body: ReadableStream) => ReadableStream } /** @@ -643,7 +718,7 @@ export function createGithubSourceClient(options: GithubClientOptions): PublicGi } }, - async load(source) { + async load(source, loadOptions) { const repository = await requestJson( 'repository', `/repos/${encodeURIComponent(source.owner)}/${encodeURIComponent(source.repository)}`, @@ -675,93 +750,46 @@ export function createGithubSourceClient(options: GithubClientOptions): PublicGi const listed = await listTreeBounded(at.owner, at.repository, skillTree) if (listed._tag !== 'listed') return listed - const selected = selectArtifactEntries(listed.entries, source.skillPath) + const limits = expectedVisibility === 'private' + ? PRIVATE_ARTIFACT_LIMITS + : { ...PUBLIC_ARTIFACT_LIMITS, maxLinkedBytes: loadOptions.linkedFiles ? MAX_LINKED_BYTES : null } + const selected = selectArtifactEntries(listed.entries, source.skillPath, limits) if (selected._tag === 'rejected') return selected + const files = selected.entries.map(packedFile) const omitted = selected.omitted.map(entry => ({ path: entry.path, bytes: entry.size, url: githubBlobUrl(source, entry.path), })) + const linked = selected.linked.map(packedFile) - const choice = chooseArtifactByteSource({ - visibility: source.visibility, - treeTruncated: listed.truncated, - totalBlobBytes: selected.entries.reduce((total, entry) => total + entry.size, 0), - }) - if (choice._tag === 'tarball') { - const extracted = await loadFromTarball(source, at, selected.entries) - if (extracted._tag === 'extracted') - return { _tag: 'loaded', value: { source, files: extracted.files, omitted } } - // The blobs API reads one file per request against the same limit. - // Falling back would spend the most quota exactly when none is left. - if (extracted._tag === 'rate-limited') - return rateLimitRejection(extracted.resetAt) - // The tarball is an optimisation, never an authority. Anything it got - // wrong, including a file `.gitattributes export-ignore` removed from - // the archive, falls through to the blobs API, which serves every blob - // the tree names whatever the Repository's export attributes say. - console.warn('Artifact tarball source unusable, reading blobs', { - owner: source.owner, - repository: source.repository, - commitSha: source.commitSha, - skillPath: source.skillPath, - reason: extracted.reason, - findings: extracted.findings.slice(0, 20), - }) + if (expectedVisibility === 'private') { + const fromBlobs = await loadFromBlobs(source, at, selected.entries) + if (fromBlobs._tag !== 'loaded') + return fromBlobs + return { _tag: 'loaded', value: { source, files, omitted, linked, read: bufferedReader(fromBlobs.files) } } } - const fromBlobs = await loadFromBlobs(source, at, selected.entries) - return fromBlobs._tag === 'loaded' ? { _tag: 'loaded', value: { source, files: fromBlobs.files, omitted } } : fromBlobs + // A public build reads the Repository archive from codeload and any file + // it lacks from raw.githubusercontent.com. Neither spends REST quota, and + // each file is checked against the blob digest the tree names. + const read = createGithubArchiveReader({ + fetch: options.fetch, + owner: at.owner, + repository: at.repository, + commitSha: source.commitSha, + skillPath: source.skillPath, + files, + maxArchiveBytes: ARCHIVE_MAX_UNCOMPRESSED_BYTES, + budget: { githubReads: MAX_GITHUB_FILE_READS }, + gunzip: options.gunzip, + report: report => reportArchive(source, report), + now, + }) + return { _tag: 'loaded', value: { source, files, omitted, linked, read } } }, } - async function loadFromTarball( - source: ResolvedSource, - at: RepositoryName, - entries: Array, - ): Promise { - const headers = new Headers({ - 'Accept': 'application/vnd.github+json', - 'User-Agent': 'skilld.dev', - 'X-GitHub-Api-Version': GITHUB_API_VERSION, - }) - if (options.token) - headers.set('Authorization', `Bearer ${options.token}`) - // This request redirects to codeload, so unlike every JSON read it follows - // redirects. The bytes it returns are verified against the tree digests, - // which is what makes an unauthenticated byte host acceptable. - const response = await options.fetch( - `${GITHUB_API}/repos/${encodeURIComponent(at.owner)}/${encodeURIComponent(at.repository)}/tarball/${source.commitSha}`, - { headers, redirect: 'follow', signal: AbortSignal.timeout(TARBALL_REQUEST_TIMEOUT_MS) }, - ).catch((thrown: unknown) => { - // A refused connection or the request timeout lands here. The build has - // a complete second source, so it reports the reason and reads blobs. - return { _tag: 'threw' as const, message: String(thrown) } - }) - if ('_tag' in response) - return { _tag: 'unusable', reason: 'unavailable', findings: [response.message] } - const limited = githubRateLimit(response, now()) - if (limited) { - await response.body?.cancel() - return limited - } - if (!response.ok || !response.body) { - await response.body?.cancel() - return { _tag: 'unusable', reason: 'unavailable', findings: [`GitHub returned ${response.status}`] } - } - return await extractSkillFilesFromTarball({ - body: response.body, - skillPath: source.skillPath, - entries: entries.map(entry => ({ - path: entry.path, - gitBlobSha: entry.sha, - size: entry.size, - mode: artifactFileMode(entry.mode), - })), - maxUncompressedBytes: TARBALL_MAX_UNCOMPRESSED_BYTES, - }) - } - async function loadFromBlobs( source: ResolvedSource, at: RepositoryName, @@ -801,37 +829,36 @@ export function createGithubSourceClient(options: GithubClientOptions): PublicGi } } -export type ArtifactByteSource - = { _tag: 'tarball' } - | { _tag: 'per-blob', reason: 'private-repository' | 'tree-truncated' | 'tree-too-large' } +/** The packed form of one tree entry. */ +function packedFile(entry: SizedEntry): PackedFile { + return { path: entry.path, mode: artifactFileMode(entry.mode), size: entry.size, gitBlobSha: entry.sha } +} + +/** Serves files already in memory, in Artifact order. */ +function bufferedReader(files: ArtifactSourceFile[]): SkillFileReader { + const ordered = [...files].sort((left, right) => compareArtifactPaths(left.path, right.path)) + return async (sink) => { + for (const file of ordered) { + await sink.begin({ path: file.path, mode: file.mode, size: file.bytes.byteLength, gitBlobSha: file.gitBlobSha }, 'github') + await sink.chunk(file.bytes) + const ended = await sink.end() + if (ended._tag === 'mismatch') + return ended + } + return { _tag: 'read' } + } +} -/** - * Chooses the byte source from the one tree read the build already makes. - * - * A private Repository keeps the per-blob path. Its archive URL is a - * pre-signed codeload link that expires five minutes after it is issued, and - * GitHub does not document whether credentials belong on the redirected host. - * Workers `fetch` follows that cross-host redirect itself, so neither question - * has an answer we control or have observed. Public delivery is measured; - * private delivery waits for a real test. - * - * A truncated tree means the Repository is large enough that GitHub would not - * list it in one response, which is the same Repository whose archive is - * expensive to stream. Nothing here reads `content-length`: codeload omits it - * whenever the archive is generated cold, so it cannot gate anything. - */ -export function chooseArtifactByteSource(input: { - visibility: 'public' | 'private' - treeTruncated: boolean - totalBlobBytes: number -}): ArtifactByteSource { - if (input.visibility !== 'public') - return { _tag: 'per-blob', reason: 'private-repository' } - if (input.treeTruncated) - return { _tag: 'per-blob', reason: 'tree-truncated' } - if (input.totalBlobBytes > TARBALL_MAX_TREE_BYTES) - return { _tag: 'per-blob', reason: 'tree-too-large' } - return { _tag: 'tarball' } +/** A Repository archive the build could not use for every file. The build still completes from GitHub. */ +function reportArchive(source: ResolvedSource, report: ArchiveReport): void { + console.warn('Artifact archive read stopped early', { + owner: source.owner, + repository: source.repository, + commitSha: source.commitSha, + skillPath: source.skillPath, + reason: report.reason, + detail: report.detail, + }) } /** @@ -870,22 +897,34 @@ function normalizeRequestedSkillPath(input: string): string | null { type SizedEntry = TreeEntry & { size: number } +interface SelectedEntries { + _tag: 'selected' + /** Packed, in Artifact order. */ + entries: SizedEntry[] + omitted: SizedEntry[] + linked: SizedEntry[] +} + /** - * The blobs one Skill folder packs, the files it leaves out, or the rule that + * The blobs one Skill folder packs, links, and leaves out, or the rule that * refuses them. * - * Every limit counts the Skill folder only. A file the Skill does not read, - * such as music, video, an image, a binary, or a file in an example or test - * folder, is left out when it is over a limit, and the rest of the Skill is - * delivered: first each such file over the one-file limit, then the largest of - * them until the folder fits. Only the files a Skill reads decide a refusal. A - * Skill at the Repository root has the whole Repository as its folder, so its - * README images are left out rather than counted against it. + * Every limit counts the Skill folder only. When a limit binds, files leave + * the archive largest first, so the fewest go: + * + * - A skilld CLI that reads linked files gets the largest files, other than + * SKILL.md, as linked files. It reads each one from GitHub at the commit. + * - Any other CLI gets a file the Skill does not read left out: music, video, + * an image, a binary, or a file in an example or test folder. Only the files + * a Skill reads decide a refusal. + * + * A Skill at the Repository root has the whole Repository as its folder, so + * its README images leave rather than count against it. */ -function selectArtifactEntries(entries: TreeEntry[], skillPath: string): { _tag: 'selected', entries: SizedEntry[], omitted: SizedEntry[] } | SourceRejection { +export function selectArtifactEntries(entries: TreeEntry[], skillPath: string, limits: ArtifactLimits): SelectedEntries | SourceRejection { const findings: string[] = [] const identities = new Map() - const blobs: Array = [] + const blobs: SizedEntry[] = [] for (const entry of entries) { if (!isSafeArtifactPath(entry.path)) findings.push(entry.path) @@ -916,24 +955,63 @@ function selectArtifactEntries(entries: TreeEntry[], skillPath: string): { _tag: const rootNote = skillPath === '.' ? ' The Skill folder is the Repository root, so every file in the Repository counts.' : '' - if (blobs.length > MAX_ARTIFACT_FILES) - return sourceLimitRejection(`${skillFolderLabel(skillPath)} has ${blobs.length} files. The limit is ${MAX_ARTIFACT_FILES}.`) - const oversizedText = blobs.filter(entry => entry.size > MAX_FILE_BYTES && isReadBySkill(entry.path)) - if (oversizedText.length > 0) { - return sourceLimitRejection( - `The file \`${oversizedText[0]!.path}\` is ${mebibytes(oversizedText[0]!.size)}. The limit for one file is ${mebibytes(MAX_FILE_BYTES)}.${rootNote}`, - oversizedText.map(entry => `${entry.path}: ${entry.size.toLocaleString('en-US')} bytes`), - ) + const omitted: SizedEntry[] = [] + const linked: SizedEntry[] = [] + let kept = blobs + + // One file over its own limit. Only a private build has one. + if (limits.maxFileBytes !== null) { + const maxFileBytes = limits.maxFileBytes + const oversizedText = kept.filter(entry => entry.size > maxFileBytes && isReadBySkill(entry.path)) + if (oversizedText.length > 0) { + return sourceLimitRejection( + `The file \`${oversizedText[0]!.path}\` is ${mebibytes(oversizedText[0]!.size)}. The limit for one file is ${mebibytes(maxFileBytes)}.${rootNote}`, + oversizedText.map(entry => `${entry.path}: ${entry.size.toLocaleString('en-US')} bytes`), + ) + } + omitted.push(...kept.filter(entry => entry.size > maxFileBytes)) + kept = kept.filter(entry => entry.size <= maxFileBytes) } - const omitted = blobs.filter(entry => entry.size > MAX_FILE_BYTES) - const kept = blobs.filter(entry => entry.size <= MAX_FILE_BYTES) - // Largest first, so the fewest files go. The path breaks ties, so one - // commit always omits the same files. - for (const media of largestFirst(kept.filter(entry => !isReadBySkill(entry.path)))) { - if (projectedUstarBytes(kept.map(entry => entry.size)) <= MAX_ARTIFACT_BYTES) - break - kept.splice(kept.indexOf(media), 1) - omitted.push(media) + + // Too many files. A linked file still counts, so linking cannot help. + if (kept.length > limits.maxFiles) { + const unread = largestFirst(kept.filter(entry => !isReadBySkill(entry.path))) + const leaving = new Set(unread.slice(0, kept.length - limits.maxFiles)) + omitted.push(...leaving) + kept = kept.filter(entry => !leaving.has(entry)) + if (kept.length > limits.maxFiles) { + return sourceLimitRejection( + `${skillFolderLabel(skillPath)} has ${kept.length.toLocaleString('en-US')} files the Skill reads. The limit is ${limits.maxFiles.toLocaleString('en-US')}.${rootNote}`, + ) + } + } + + // Too many bytes for one archive. + const maxLinkedBytes = limits.maxLinkedBytes + if (projectedUstarBytes(kept.map(entry => entry.size)) > limits.maxArchiveBytes) { + const candidates = maxLinkedBytes === null + ? largestFirst(kept.filter(entry => !isReadBySkill(entry.path))) + : largestFirst(kept.filter(entry => entry.path !== 'SKILL.md')) + let packedBytes = projectedUstarBytes(kept.map(entry => entry.size)) + let linkedBytes = 0 + const leaving = new Set() + for (const entry of candidates) { + if (packedBytes <= limits.maxArchiveBytes) + break + if (maxLinkedBytes !== null && linkedBytes + entry.size <= maxLinkedBytes) { + linked.push(entry) + linkedBytes += entry.size + } + else if (!isReadBySkill(entry.path)) { + omitted.push(entry) + } + else { + continue + } + leaving.add(entry) + packedBytes -= projectedUstarBytes([entry.size]) - projectedUstarBytes([]) + } + kept = kept.filter(entry => !leaving.has(entry)) } // The signer checks `content_bytes` against this same ceiling, and // `content_bytes` is the packed archive rather than the sum of the blobs. A @@ -941,16 +1019,29 @@ function selectArtifactEntries(entries: TreeEntry[], skillPath: string): { _tag: // the R2 write, with the signer's error instead of a named rejection. Guard // the number the signer will actually see. const projectedBytes = projectedUstarBytes(kept.map(entry => entry.size)) - if (projectedBytes > MAX_ARTIFACT_BYTES) { + if (projectedBytes > limits.maxArchiveBytes) { return sourceLimitRejection( - `The files the Skill reads in ${skillFolderLabel(skillPath).replace(/^The /, 'the ')} pack to ${mebibytes(projectedBytes)}. The limit is ${mebibytes(MAX_ARTIFACT_BYTES)}.${rootNote}`, + `The files the Skill reads in ${skillFolderLabel(skillPath).replace(/^The /, 'the ')} pack to ${mebibytes(projectedBytes)}. The limit is ${mebibytes(limits.maxArchiveBytes)}.${rootNote}`, largestFirst(kept).map(entry => `${entry.path}: ${entry.size.toLocaleString('en-US')} bytes`), ) } + // A packed file needs a USTAR header, and the scan writes headers as the + // bytes stream. Refuse such a path before any byte is read. + const unfit = kept.filter(entry => !splitUstarPath(entry.path)).map(entry => entry.path) + if (unfit.length > 0) + return reject('INVALID_SOURCE', 'A Skill path cannot be represented by the Artifact format.', unfit) + const fileListBytes = [...kept, ...linked] + .reduce((total, entry) => total + FILE_LIST_ENTRY_BYTES + new TextEncoder().encode(entry.path).byteLength, 0) + if (fileListBytes > MAX_FILE_LIST_BYTES) { + return sourceLimitRejection( + `The file list of ${skillFolderLabel(skillPath).replace(/^The /, 'the ')} is too long to sign: ${(kept.length + linked.length).toLocaleString('en-US')} files with long paths.${rootNote}`, + ) + } return { _tag: 'selected', - entries: kept.sort((a, b) => comparePath(a.path, b.path)), - omitted: omitted.sort((a, b) => comparePath(a.path, b.path)), + entries: kept.sort((a, b) => compareArtifactPaths(a.path, b.path)), + omitted: omitted.sort((a, b) => compareArtifactPaths(a.path, b.path)), + linked: linked.sort((a, b) => compareArtifactPaths(a.path, b.path)), } } @@ -1088,8 +1179,8 @@ export function storedFilesPassLoadRules(files: ArtifactSourceFile[], skillPath: sha: file.gitBlobSha, size: file.bytes.byteLength, })) - const selected = selectArtifactEntries(entries, skillPath) - return selected._tag === 'selected' && selected.omitted.length === 0 + const selected = selectArtifactEntries(entries, skillPath, PUBLIC_ARTIFACT_LIMITS) + return selected._tag === 'selected' && selected.omitted.length === 0 && selected.linked.length === 0 } function skillFolderLabel(skillPath: string): string { diff --git a/layers/artifact-delivery/server/utils/request-resolution.ts b/layers/artifact-delivery/server/utils/request-resolution.ts index dbe58a7e..32b7cae7 100644 --- a/layers/artifact-delivery/server/utils/request-resolution.ts +++ b/layers/artifact-delivery/server/utils/request-resolution.ts @@ -59,14 +59,18 @@ export async function requestResolution( * read GitHub with that account's own token once the shared quota is spent. */ requesterAccountId?: number | null + /** The skilld CLI that sent the request reads linked files. See ADR-0013. */ + linkedFiles?: boolean }, ): Promise { const accountId = input.access.visibility === 'private' ? input.access.accountId : undefined - const identity = await resolutionRequestIdentity(input.source, input.idempotencyKey, accountId) + // A private build reads blobs and never links a file. + const linkedFiles = input.access.visibility === 'public' && input.linkedFiles === true + const identity = await resolutionRequestIdentity(input.source, input.idempotencyKey, accountId, linkedFiles) const source = input.access.visibility === 'private' ? input.source : await admittedSourceRequest(input.source, dependencies.lookupAdmitted) - const result = await createResolution(dependencies.db, source, identity, dependencies.now(), input.access) + const result = await createResolution(dependencies.db, source, identity, dependencies.now(), input.access, linkedFiles) if (result._tag === 'created' && input.access.visibility === 'public' && input.requesterAccountId) { await recordResolutionRequester(dependencies.db, { resolutionId: result.row.id, @@ -79,6 +83,19 @@ export async function requestResolution( return result } +/** + * The request header a skilld CLI sends to name the Artifact features it + * reads, as a comma-separated list. A CLI that sends none reads none. + */ +export const CLIENT_CAPABILITIES_HEADER = 'skilld-capabilities' + +/** Whether the header names `linked-files`. Unknown names are ignored. */ +export function readsLinkedFiles(header: string | undefined): boolean { + return (header ?? '') + .split(',') + .some(name => name.trim().toLowerCase() === 'linked-files') +} + /** * When the queue takes over a build that ran in its request. A Worker keeps a * request alive 30 s after its response, so a build the runtime stopped diff --git a/layers/artifact-delivery/server/utils/requester-github.ts b/layers/artifact-delivery/server/utils/requester-github.ts index dcd91ba7..43a46ce7 100644 --- a/layers/artifact-delivery/server/utils/requester-github.ts +++ b/layers/artifact-delivery/server/utils/requester-github.ts @@ -109,12 +109,12 @@ export function withRequesterFallback( const client = await ownClient() return client ? await client.resolve(request) : first }, - async load(source) { - const first = await shared.load(source) + async load(source, options) { + const first = await shared.load(source, options) if (first._tag !== 'rejected' || first.code !== 'RATE_LIMITED') return first const client = await ownClient() - return client ? await client.load(source) : first + return client ? await client.load(source, options) : first }, } } diff --git a/layers/artifact-delivery/server/utils/state.ts b/layers/artifact-delivery/server/utils/state.ts index 0556b9aa..5978d1e1 100644 --- a/layers/artifact-delivery/server/utils/state.ts +++ b/layers/artifact-delivery/server/utils/state.ts @@ -26,7 +26,7 @@ import { canonicalJson, digestHex } from './encoding' * The signer also signs the policy before this one, so a deploy window fails * no run. Update `SIGNABLE_ARTIFACT_POLICIES` in `checks.ts` with each bump. */ -export const ARTIFACT_POLICY_VERSION = '2026-10-07.2' +export const ARTIFACT_POLICY_VERSION = '2026-10-07.3' /** * Earlier policies under which every ready build packed the bytes this policy @@ -36,11 +36,17 @@ export const ARTIFACT_POLICY_VERSION = '2026-10-07.2' * A bump that changes only checks adds the version it replaces. A bump that * changes the bytes of a folder an earlier policy accepted empties this set. * + * - `2026-10-07.2`: ADR-0013 streams the archive and raises the limits, so + * it packs folders that policy refused or left files out of. A folder it + * packed whole packs the same files. Artifact order moved from UTF-16 to + * UTF-8 path order, which differs only for a path outside the Basic + * Multilingual Plane; such a stored archive packs differently and fails + * the byte comparison, so it loads from GitHub. * - `2026-10-07.1`: #487 leaves large media out of a folder that policy * refused. Every folder it accepted packs the same files. * - `2026-08-20.1`: #481 changed checks and Skill name resolution only. */ -export const BYTE_COMPATIBLE_POLICY_VERSIONS: ReadonlySet = new Set(['2026-10-07.1', '2026-08-20.1']) +export const BYTE_COMPATIBLE_POLICY_VERSIONS: ReadonlySet = new Set(['2026-10-07.2', '2026-10-07.1', '2026-08-20.1']) export const ACTIVE_BUILD_STATES = [ 'requested', @@ -97,6 +103,8 @@ const resolutionRowSchema = z.object({ error_code: z.string().nullable(), error_retryable: z.union([z.literal(0), z.literal(1)]).nullable(), error_retry_after: z.number().int().positive().nullable().optional(), + /** 1 when the skilld CLI that asked reads linked files. */ + linked_files: z.union([z.literal(0), z.literal(1)]).optional(), created_at: z.number().int(), updated_at: z.number().int(), }) @@ -139,13 +147,15 @@ const transitions: Record = { revoked: [], } -export async function resolutionRequestIdentity(source: SourceRequest, idempotencyKey: string, accountId?: number): Promise<{ +export async function resolutionRequestIdentity(source: SourceRequest, idempotencyKey: string, accountId?: number, linkedFiles = false): Promise<{ keyHash: string fingerprint: string }> { return { keyHash: await digestHex('SHA-256', `${accountId ?? 'public'}\0${idempotencyKey}`), - fingerprint: await digestHex('SHA-256', canonicalJson({ source, accountId: accountId ?? null })), + // The field is left out when false, so a fingerprint from before linked + // files still matches the same request. + fingerprint: await digestHex('SHA-256', canonicalJson({ source, accountId: accountId ?? null, ...(linkedFiles ? { linkedFiles } : {}) })), } } @@ -164,6 +174,7 @@ export async function createResolution( installationId: number repositoryId: number } | { visibility: 'public' } = { visibility: 'public' }, + linkedFiles = false, ): Promise { // A new request key inserts and returns its row in one round trip. A // replayed key inserts nothing, and the lookup below finds its row. @@ -174,10 +185,10 @@ export async function createResolution( id, request_key_hash, request_fingerprint, state, state_version, requested_owner, requested_repository, selector_type, selector_value, ref_type, ref_value, repository_id, visibility, account_id, github_installation_id, - created_at, updated_at + linked_files, created_at, updated_at ) VALUES ( ?1, ?2, ?3, 'requested', 0, ?4, ?5, ?6, ?7, ?8, ?9, - ?10, ?11, ?12, ?13, ?14, ?14 + ?10, ?11, ?12, ?13, ?15, ?14, ?14 ) RETURNING *`, ).bind( @@ -195,6 +206,7 @@ export async function createResolution( access.visibility === 'private' ? access.accountId : null, access.visibility === 'private' ? access.installationId : null, now, + linkedFiles ? 1 : 0, ).first>() if (insert) return { _tag: 'created', row: resolutionRowSchema.parse(insert) } diff --git a/layers/artifact-delivery/server/utils/tarball-source.ts b/layers/artifact-delivery/server/utils/tarball-source.ts deleted file mode 100644 index f39ed367..00000000 --- a/layers/artifact-delivery/server/utils/tarball-source.ts +++ /dev/null @@ -1,335 +0,0 @@ -import type { ArtifactSourceFile } from './github-source' -import { gitBlobShaHex } from './encoding' - -const BLOCK_SIZE = 512 -const REGULAR_FILE_TYPEFLAGS = new Set(['0', '\0']) -const PAX_EXTENDED = 'x' -const PAX_GLOBAL = 'g' -const GNU_LONG_NAME = 'L' -const GNU_LONG_LINK = 'K' - -/** - * One file the Git tree lists for this Skill. The tree is the authority for - * the path, the mode and the bytes: a GitHub tarball widens every file mode to - * 0664 or 0775, so a tar header can never decide whether a file is executable. - */ -export interface TarballWantedFile { - /** Path relative to the Skill directory. */ - path: string - gitBlobSha: string - size: number - mode: 420 | 493 -} - -export type TarballUnusableReason - = 'archive-too-large' - | 'unavailable' - | 'digest-mismatch' - | 'duplicate-path' - | 'malformed-archive' - | 'missing-files' - | 'size-mismatch' - -export type TarballExtraction - = { _tag: 'extracted', files: ArtifactSourceFile[] } - | { _tag: 'unusable', reason: TarballUnusableReason, findings: string[] } - -export interface TarballExtractionRequest { - /** The gzipped tar bytes, as GitHub streams them. */ - body: ReadableStream - /** `.` for a Skill at the Repository root, otherwise the Skill directory. */ - skillPath: string - entries: readonly TarballWantedFile[] - maxUncompressedBytes: number -} - -/** - * Streams a Repository tarball and keeps only the files the Git tree lists for - * one Skill, discarding every other entry as it passes. - * - * Every kept file is verified against its Git blob digest, so the archive is - * an untrusted byte source: nothing it returns can differ from what the Git - * tree already named. Any defect makes the whole archive unusable rather than - * producing a partial Skill, because a partial Skill would be signed. - */ -export async function extractSkillFilesFromTarball( - request: TarballExtractionRequest, -): Promise { - const wanted = new Map(request.entries.map(entry => [entry.path, entry])) - const found = new Map() - // `DecompressionStream` declares its writable side as `BufferSource`, which - // the `ReadableStream` pipe signature refuses. Both sides carry - // `Uint8Array` at runtime. - const gunzip = new DecompressionStream('gzip') as unknown as ReadableWritablePair - const reader = request.body.pipeThrough(gunzip).getReader() - const source = createByteSource(reader, request.maxUncompressedBytes) - - try { - let pendingLongName: string | null = null - while (true) { - const header = await source.exact(BLOCK_SIZE) - if (header._tag !== 'bytes') - return await finish(source, header, found, wanted) - if (isZeroBlock(header.value)) - continue - - const parsed = parseHeader(header.value) - const padded = roundToBlock(parsed.size) - - if (parsed.typeflag === GNU_LONG_NAME || parsed.typeflag === GNU_LONG_LINK) { - const body = await source.exact(padded) - if (body._tag !== 'bytes') - return await finish(source, body, found, wanted) - if (parsed.typeflag === GNU_LONG_NAME) - pendingLongName = readString(body.value.subarray(0, parsed.size)) - continue - } - if (parsed.typeflag === PAX_EXTENDED || parsed.typeflag === PAX_GLOBAL) { - const body = await source.exact(padded) - if (body._tag !== 'bytes') - return await finish(source, body, found, wanted) - if (parsed.typeflag === PAX_EXTENDED) - pendingLongName = readPaxPath(body.value.subarray(0, parsed.size)) ?? pendingLongName - continue - } - - const name = pendingLongName ?? parsed.name - pendingLongName = null - const relative = skillRelativePath(name, request.skillPath) - const want = relative && REGULAR_FILE_TYPEFLAGS.has(parsed.typeflag) - ? wanted.get(relative) - : undefined - if (!want) { - const skipped = await source.skip(padded) - if (skipped._tag !== 'bytes') - return await finish(source, skipped, found, wanted) - continue - } - if (found.has(want.path)) - return await unusable(source, 'duplicate-path', [want.path]) - if (parsed.size !== want.size) - return await unusable(source, 'size-mismatch', [want.path]) - - const body = await source.exact(padded) - if (body._tag !== 'bytes') - return await finish(source, body, found, wanted) - const bytes = new Uint8Array(body.value.subarray(0, want.size)) - const digest = await gitBlobShaHex(bytes) - if (digest !== want.gitBlobSha) - return await unusable(source, 'digest-mismatch', [want.path]) - found.set(want.path, { - path: want.path, - mode: want.mode, - bytes, - gitBlobSha: want.gitBlobSha, - }) - } - } - catch (thrown) { - // A corrupt gzip member or a truncated body throws here. The reason and - // the message travel back to the caller, which falls back to the per-blob - // path rather than failing the build. - await source.cancel() - return { _tag: 'unusable', reason: 'malformed-archive', findings: [String(thrown)] } - } -} - -type SourceRead - = { _tag: 'bytes', value: Uint8Array } - | { _tag: 'end' } - | { _tag: 'too-large' } - | { _tag: 'truncated' } - -interface ByteSource { - exact: (size: number) => Promise - skip: (size: number) => Promise - cancel: () => Promise -} - -function createByteSource( - reader: ReadableStreamDefaultReader, - maxBytes: number, -): ByteSource { - let carry: Uint8Array = new Uint8Array(0) - let ended = false - let total = 0 - let cancelled = false - - const pull = async (): Promise => { - const next = await reader.read() - if (next.done) { - ended = true - return { _tag: 'end' } - } - total += next.value.byteLength - if (total > maxBytes) - return { _tag: 'too-large' } - return { _tag: 'bytes', value: next.value } - } - - const cancel = async (): Promise => { - if (cancelled) - return - cancelled = true - // A cancelled body is the point of the ceiling: the rest is never read. - await reader.cancel().catch(() => { - // The stream is already closed or errored, which needs no handling here. - }) - } - - return { - cancel, - async exact(size) { - if (carry.byteLength >= size) { - const value = carry.subarray(0, size) - carry = carry.subarray(size) - return { _tag: 'bytes', value } - } - const parts: Uint8Array[] = [carry] - let have = carry.byteLength - while (have < size) { - if (ended) - return have === 0 ? { _tag: 'end' } : { _tag: 'truncated' } - const next = await pull() - if (next._tag === 'too-large') - return next - if (next._tag === 'end') - continue - if (next._tag === 'truncated') - return next - parts.push(next.value) - have += next.value.byteLength - } - const merged = new Uint8Array(have) - let offset = 0 - for (const part of parts) { - merged.set(part, offset) - offset += part.byteLength - } - carry = merged.subarray(size) - return { _tag: 'bytes', value: merged.subarray(0, size) } - }, - async skip(size) { - let left = size - while (left > 0) { - if (carry.byteLength > 0) { - const take = Math.min(carry.byteLength, left) - carry = carry.subarray(take) - left -= take - continue - } - if (ended) - return { _tag: 'truncated' } - const next = await pull() - if (next._tag === 'too-large' || next._tag === 'truncated') - return next - if (next._tag === 'end') - continue - carry = next.value - } - return { _tag: 'bytes', value: new Uint8Array(0) } - }, - } -} - -async function finish( - source: ByteSource, - read: SourceRead, - found: Map, - wanted: Map, -): Promise { - if (read._tag === 'too-large') - return await unusable(source, 'archive-too-large', []) - if (read._tag === 'truncated') - return await unusable(source, 'malformed-archive', ['The archive ended inside an entry.']) - const missing = [...wanted.keys()].filter(path => !found.has(path)).sort(comparePath) - if (missing.length > 0) - return await unusable(source, 'missing-files', missing) - await source.cancel() - return { - _tag: 'extracted', - files: [...found.values()].sort((left, right) => comparePath(left.path, right.path)), - } -} - -async function unusable( - source: ByteSource, - reason: TarballUnusableReason, - findings: string[], -): Promise { - await source.cancel() - return { _tag: 'unusable', reason, findings } -} - -interface TarHeader { - name: string - size: number - typeflag: string -} - -function parseHeader(block: Uint8Array): TarHeader { - const name = readString(block.subarray(0, 100)) - const prefix = readString(block.subarray(345, 500)) - return { - name: prefix ? `${prefix}/${name}` : name, - size: readOctal(block.subarray(124, 136)), - typeflag: String.fromCharCode(block[156]!), - } -} - -/** - * GitHub wraps every archive in one directory named for the Repository and the - * short commit, so the first path segment carries no meaning. - */ -function skillRelativePath(path: string, skillPath: string): string | null { - const separator = path.indexOf('/') - if (separator === -1) - return null - const stripped = path.slice(separator + 1) - if (skillPath === '.') - return stripped === '' ? null : stripped - if (!stripped.startsWith(`${skillPath}/`)) - return null - const relative = stripped.slice(skillPath.length + 1) - return relative === '' ? null : relative -} - -function readPaxPath(record: Uint8Array): string | null { - const text = new TextDecoder().decode(record) - for (const line of text.split('\n')) { - const match = /^\d+ path=(.*)$/.exec(line) - if (match) - return match[1]! - } - return null -} - -function readString(bytes: Uint8Array): string { - let end = 0 - while (end < bytes.byteLength && bytes[end] !== 0) - end++ - return new TextDecoder().decode(bytes.subarray(0, end)) -} - -function readOctal(bytes: Uint8Array): number { - let text = '' - for (const byte of bytes) { - if (byte === 0 || byte === 0x20) - break - text += String.fromCharCode(byte) - } - const value = text.length === 0 ? 0 : Number.parseInt(text, 8) - return Number.isSafeInteger(value) && value >= 0 ? value : 0 -} - -function isZeroBlock(block: Uint8Array): boolean { - return block.every(byte => byte === 0) -} - -function roundToBlock(size: number): number { - return Math.ceil(size / BLOCK_SIZE) * BLOCK_SIZE -} - -function comparePath(left: string, right: string): number { - return left < right ? -1 : left > right ? 1 : 0 -} diff --git a/layers/artifact-delivery/server/utils/ustar.ts b/layers/artifact-delivery/server/utils/ustar.ts index e48d264c..9cf1d25d 100644 --- a/layers/artifact-delivery/server/utils/ustar.ts +++ b/layers/artifact-delivery/server/utils/ustar.ts @@ -5,7 +5,7 @@ import { gitBlobShaHex } from './encoding' const BLOCK_SIZE = 512 export function createDeterministicUstar(inputFiles: ArtifactSourceFile[]): Uint8Array { - const files = [...inputFiles].sort((left, right) => left.path < right.path ? -1 : left.path > right.path ? 1 : 0) + const files = [...inputFiles].sort((left, right) => compareArtifactPaths(left.path, right.path)) const byteLength = files.reduce( (total, file) => total + BLOCK_SIZE + roundToBlock(file.bytes.byteLength), BLOCK_SIZE * 2, @@ -13,7 +13,7 @@ export function createDeterministicUstar(inputFiles: ArtifactSourceFile[]): Uint const archive = new Uint8Array(byteLength) let offset = 0 for (const file of files) { - const header = createHeader(file) + const header = ustarHeader({ path: file.path, mode: file.mode, size: file.bytes.byteLength }) archive.set(header, offset) offset += BLOCK_SIZE archive.set(file.bytes, offset) @@ -22,7 +22,29 @@ export function createDeterministicUstar(inputFiles: ArtifactSourceFile[]): Uint return archive } -function createHeader(file: ArtifactSourceFile): Uint8Array { +/** + * The order of files in an Artifact: by the UTF-8 bytes of their paths. + * + * It is the order a Git archive lists a tree in, so a build can stream the + * Repository tarball and write each Skill file as it passes. All 487 + * tarballs of the 2026-10-07 sweep listed their files in this order. It + * differs from JavaScript string order only where a path holds a character + * outside the Basic Multilingual Plane. + */ +export function compareArtifactPaths(left: string, right: string): number { + const length = Math.min(left.length, right.length) + for (let index = 0; index < length;) { + const leftPoint = left.codePointAt(index)! + const rightPoint = right.codePointAt(index)! + if (leftPoint !== rightPoint) + return leftPoint < rightPoint ? -1 : 1 + index += leftPoint > 0xFFFF ? 2 : 1 + } + return left.length === right.length ? 0 : left.length < right.length ? -1 : 1 +} + +/** The 512-byte USTAR header {@link createDeterministicUstar} writes for one file. */ +export function ustarHeader(file: { path: string, mode: 420 | 493, size: number }): Uint8Array { const path = splitUstarPath(file.path) if (!path) throw new Error(`Artifact path does not fit USTAR: ${file.path}`) @@ -31,7 +53,7 @@ function createHeader(file: ArtifactSourceFile): Uint8Array { writeOctal(header, 100, 8, file.mode) writeOctal(header, 108, 8, 0) writeOctal(header, 116, 8, 0) - writeOctal(header, 124, 12, file.bytes.byteLength) + writeOctal(header, 124, 12, file.size) writeOctal(header, 136, 12, 0) header.fill(0x20, 148, 156) header[156] = 0x30 @@ -65,6 +87,14 @@ function roundToBlock(size: number): number { return Math.ceil(size / BLOCK_SIZE) * BLOCK_SIZE } +/** The zero bytes that pad a file of this size to a whole block. */ +export function ustarPadding(size: number): number { + return roundToBlock(size) - size +} + +/** The two zero blocks that close an archive. */ +export const USTAR_END_BYTES = BLOCK_SIZE * 2 + /** * Bytes {@link createDeterministicUstar} will write for files of these sizes. * diff --git a/migrations/0145_artifact_resolution_linked_files.sql b/migrations/0145_artifact_resolution_linked_files.sql new file mode 100644 index 00000000..bfbf1d35 --- /dev/null +++ b/migrations/0145_artifact_resolution_linked_files.sql @@ -0,0 +1,6 @@ +-- 1 when the skilld CLI that asked for the Resolution sent +-- `Skilld-Capabilities: linked-files`. Its Artifact may then list linked files: +-- Skill files it reads from GitHub at the attested commit. Every other CLI +-- refuses that statement field, so its Resolutions keep 0. See ADR-0013. +-- Cull path: drop the column once every supported skilld CLI reads linked files. +ALTER TABLE artifact_resolutions ADD COLUMN linked_files INTEGER NOT NULL DEFAULT 0; diff --git a/test/fixtures/loaded-source.ts b/test/fixtures/loaded-source.ts new file mode 100644 index 00000000..60c0fd71 --- /dev/null +++ b/test/fixtures/loaded-source.ts @@ -0,0 +1,57 @@ +import type { LinkedArtifactFile, ResolvedSource } from '../../layers/artifact-delivery/server/schemas/contracts' +import type { ArtifactSourceFile, LoadedArtifactSource, OmittedArtifactFile, SourceRejection } from '../../layers/artifact-delivery/server/utils/github-source' +import { createHash } from 'node:crypto' +import { scanArtifact } from '../../layers/artifact-delivery/server/utils/artifact-pack' +import { compareArtifactPaths, readDeterministicUstar } from '../../layers/artifact-delivery/server/utils/ustar' + +/** + * What a source client's `load` answers for files already in memory: the + * packed files in Artifact order, and a reader that serves their bytes. The + * Git blob digest is computed from the bytes, as a load verifies it, so a + * fixture may name any placeholder. + */ +export function loadedFromFiles( + source: ResolvedSource, + files: ArtifactSourceFile[], + extra: { omitted?: OmittedArtifactFile[], linked?: LinkedArtifactFile[] } = {}, +): LoadedArtifactSource { + const ordered = [...files] + .sort((left, right) => compareArtifactPaths(left.path, right.path)) + .map(file => ({ ...file, gitBlobSha: createHash('sha1').update(`blob ${file.bytes.byteLength}\0`).update(file.bytes).digest('hex') })) + return { + source, + files: ordered.map(file => ({ path: file.path, mode: file.mode, size: file.bytes.byteLength, gitBlobSha: file.gitBlobSha })), + omitted: extra.omitted ?? [], + linked: extra.linked ?? [], + read: async (sink) => { + for (const file of ordered) { + await sink.begin({ path: file.path, mode: file.mode, size: file.bytes.byteLength, gitBlobSha: file.gitBlobSha }, 'archive') + await sink.chunk(file.bytes) + const ended = await sink.end() + if (ended._tag === 'mismatch') + return ended + } + return { _tag: 'read' } + }, + } +} + +/** + * Reads a loaded source the way a build scans it, and answers the files it + * packed, in Artifact order. + */ +export async function readLoadedFiles(loaded: LoadedArtifactSource): Promise<{ _tag: 'read', files: ArtifactSourceFile[] } | SourceRejection> { + const scanned = await scanArtifact({ + source: loaded.source, + files: loaded.files, + read: loaded.read, + omitted: loaded.omitted, + spoolBytes: Number.POSITIVE_INFINITY, + }) + if (scanned._tag === 'rejected') + return scanned + const files = await readDeterministicUstar(scanned.spool!) + if (!files) + throw new Error('The scan packed an archive it cannot read back') + return { _tag: 'read', files } +} diff --git a/test/fixtures/tar-archive.ts b/test/fixtures/tar-archive.ts index 59e36dc9..b70b8424 100644 --- a/test/fixtures/tar-archive.ts +++ b/test/fixtures/tar-archive.ts @@ -16,8 +16,16 @@ export interface TarFixtureEntry { * Builds a gzipped tar the way codeload does: every path under one top level * directory, file modes widened to 0664 and 0775. */ -export function tarGzFixture(topLevel: string, entries: TarFixtureEntry[]): Uint8Array { +export function tarGzFixture(topLevel: string, entries: TarFixtureEntry[], options: { globalComment?: string } = {}): Uint8Array { const blocks: Uint8Array[] = [] + if (options.globalComment !== undefined) { + // GitHub opens every archive with a pax global header that names the commit. + const record = paxRecord('comment', options.globalComment) + blocks.push(header('pax_global_header', record.byteLength, '0000666', 'g', '')) + const padded = new Uint8Array(Math.ceil(record.byteLength / BLOCK_SIZE) * BLOCK_SIZE) + padded.set(record) + blocks.push(padded) + } for (const entry of entries) { const bytes = entry.bytes ?? new Uint8Array(0) const typeflag = entry.typeflag ?? '0' @@ -56,6 +64,14 @@ export function streamOf(bytes: Uint8Array, chunkSize = 1024): ReadableStream { - it('reads the tarball when the tree is complete and inside the byte ceiling', () => { - expect(chooseArtifactByteSource({ - visibility: 'public', - treeTruncated: false, - totalBlobBytes: TARBALL_MAX_TREE_BYTES, - })).toEqual({ _tag: 'tarball' }) - }) - - it('reads blobs when the tree total passes the byte ceiling by one byte', () => { - expect(chooseArtifactByteSource({ - visibility: 'public', - treeTruncated: false, - totalBlobBytes: TARBALL_MAX_TREE_BYTES + 1, - })).toEqual({ _tag: 'per-blob', reason: 'tree-too-large' }) - }) - - it('reads blobs when the Repository tree came back truncated', () => { - expect(chooseArtifactByteSource({ - visibility: 'public', - treeTruncated: true, - totalBlobBytes: 1024, - })).toEqual({ _tag: 'per-blob', reason: 'tree-truncated' }) - }) - - it('reads blobs for a private Repository, whatever its tree looks like', () => { - expect(chooseArtifactByteSource({ - visibility: 'private', - treeTruncated: false, - totalBlobBytes: 1024, - })).toEqual({ _tag: 'per-blob', reason: 'private-repository' }) - }) -}) - -describe('loading a Skill from the Repository tarball', () => { - it('loads every file from one tarball request and asks for no blobs', async () => { +describe('loading a public Skill from the Repository archive', () => { + it('reads every file from one archive request and asks for no blobs', async () => { const fetchMock = sourceFetch({}) - const client = createPublicGithubSourceClient({ fetch: fetchMock as unknown as typeof fetch }) - const loaded = await client.load(resolvedSource()) + const read = await loadAndRead(fetchMock) - expect(loaded._tag).toBe('loaded') - if (loaded._tag !== 'loaded') + expect(read._tag).toBe('read') + if (read._tag !== 'read') return - expect(loaded.value.files.map(file => file.path)).toEqual(['SKILL.md', 'references/guide.md', 'run.sh']) - expect(new TextDecoder().decode(loaded.value.files[0]!.bytes)).toBe(skillText) + expect(read.files.map(file => file.path)).toEqual(['SKILL.md', 'references/guide.md', 'run.sh']) + expect(new TextDecoder().decode(read.files[0]!.bytes)).toBe(skillText) expect(requestedPaths(fetchMock).filter(url => url.includes('/git/blobs/'))).toEqual([]) - expect(requestedPaths(fetchMock).filter(url => url.includes('/tarball/'))).toHaveLength(1) + expect(requestedPaths(fetchMock).filter(url => url.includes('codeload.github.com'))).toEqual([archiveUrl]) }) - it('takes the executable mode from the tree entry, not the tarball header', async () => { - const client = createPublicGithubSourceClient({ fetch: sourceFetch({}) as unknown as typeof fetch }) - - const loaded = await client.load(resolvedSource()) + it('takes the executable mode from the tree entry, not the archive header', async () => { + const read = await loadAndRead(sourceFetch({})) - expect(loaded._tag).toBe('loaded') - if (loaded._tag !== 'loaded') + expect(read._tag).toBe('read') + if (read._tag !== 'read') return - expect(loaded.value.files.map(file => [file.path, file.mode])).toEqual([ + expect(read.files.map(file => [file.path, file.mode])).toEqual([ ['SKILL.md', 420], ['references/guide.md', 420], ['run.sh', 493], ]) }) - it('falls back to blobs when the tree came back truncated', async () => { + it('reads the archive when the tree came back truncated', async () => { const fetchMock = sourceFetch({ truncated: true }) - const client = createPublicGithubSourceClient({ fetch: fetchMock as unknown as typeof fetch }) - const loaded = await client.load(resolvedSource()) + const read = await loadAndRead(fetchMock) - expect(loaded._tag).toBe('loaded') - if (loaded._tag !== 'loaded') - return - expect(loaded.value.files.map(file => file.path)).toEqual(['SKILL.md', 'references/guide.md', 'run.sh']) - expect(requestedPaths(fetchMock).filter(url => url.includes('/tarball/'))).toEqual([]) - expect(requestedPaths(fetchMock).filter(url => url.includes('/git/blobs/'))).toHaveLength(3) + expect(read._tag).toBe('read') + expect(requestedPaths(fetchMock).filter(url => url.includes('codeload.github.com'))).toHaveLength(1) + expect(requestedPaths(fetchMock).filter(url => url.includes('/git/blobs/'))).toEqual([]) }) - it('falls back to blobs when export-ignore left a tree file out of the tarball', async () => { + it('reads a file export-ignore left out of the archive from GitHub at the commit', async () => { const fetchMock = sourceFetch({ omitFromTarball: ['skills/demo/references/guide.md'] }) - const client = createPublicGithubSourceClient({ fetch: fetchMock as unknown as typeof fetch }) - const loaded = await client.load(resolvedSource()) + const read = await loadAndRead(fetchMock) - expect(loaded._tag).toBe('loaded') - if (loaded._tag !== 'loaded') - return - expect(loaded.value.files.map(file => file.path)).toEqual(['SKILL.md', 'references/guide.md', 'run.sh']) - expect(requestedPaths(fetchMock).filter(url => url.includes('/git/blobs/'))).toHaveLength(3) + expect(read._tag).toBe('read') + expect(requestedPaths(fetchMock).filter(url => url.startsWith(rawBase))).toEqual([`${rawBase}skills/demo/references/guide.md`]) }) - it('falls back to blobs when a tarball file fails its Git digest check', async () => { - const fetchMock = sourceFetch({ rewriteInTarball: { 'skills/demo/references/guide.md': 'tampered\n' } }) - const client = createPublicGithubSourceClient({ fetch: fetchMock as unknown as typeof fetch }) + it('reads a file from GitHub when its archive bytes fail the Git digest check', async () => { + // `export-subst` can rewrite a file in the archive and keep its length. + const fetchMock = sourceFetch({ rewriteInTarball: { 'skills/demo/references/guide.md': 'Read this frst!\n' } }) - const loaded = await client.load(resolvedSource()) + const read = await loadAndRead(fetchMock) - expect(loaded._tag).toBe('loaded') - if (loaded._tag !== 'loaded') + expect(read._tag).toBe('read') + if (read._tag !== 'read') return - expect(new TextDecoder().decode(loaded.value.files[1]!.bytes)).toBe(guideText) - expect(requestedPaths(fetchMock).filter(url => url.includes('/git/blobs/'))).toHaveLength(3) + expect(new TextDecoder().decode(read.files[1]!.bytes)).toBe(guideText) + expect(requestedPaths(fetchMock).filter(url => url.startsWith(rawBase))).toEqual([`${rawBase}skills/demo/references/guide.md`]) }) - it('falls back to blobs when GitHub refuses the tarball request', async () => { + it('reads every file from GitHub when codeload refuses the archive', async () => { const fetchMock = sourceFetch({ tarballStatus: 404 }) - const client = createPublicGithubSourceClient({ fetch: fetchMock as unknown as typeof fetch }) - const loaded = await client.load(resolvedSource()) + const read = await loadAndRead(fetchMock) - expect(loaded._tag).toBe('loaded') - if (loaded._tag !== 'loaded') - return - expect(requestedPaths(fetchMock).filter(url => url.includes('/git/blobs/'))).toHaveLength(3) + expect(read._tag).toBe('read') + expect(requestedPaths(fetchMock).filter(url => url.startsWith(rawBase))).toHaveLength(3) }) - it('packages identical bytes whichever source the files came from', async () => { - const fromTarball = await createPublicGithubSourceClient({ - fetch: sourceFetch({}) as unknown as typeof fetch, - }).load(resolvedSource()) - const fromBlobs = await createPublicGithubSourceClient({ - fetch: sourceFetch({ tarballStatus: 404 }) as unknown as typeof fetch, - }).load(resolvedSource()) - - expect(fromTarball._tag).toBe('loaded') - expect(fromBlobs._tag).toBe('loaded') - if (fromTarball._tag !== 'loaded' || fromBlobs._tag !== 'loaded') + it('packs identical bytes whichever host served the files', async () => { + const fromArchive = await loadAndRead(sourceFetch({})) + const fromGithub = await loadAndRead(sourceFetch({ tarballStatus: 404 })) + + expect(fromArchive._tag).toBe('read') + expect(fromGithub._tag).toBe('read') + if (fromArchive._tag !== 'read' || fromGithub._tag !== 'read') return - expect(createDeterministicUstar(fromTarball.value.files)) - .toEqual(createDeterministicUstar(fromBlobs.value.files)) + expect(createDeterministicUstar(fromArchive.files)).toEqual(createDeterministicUstar(fromGithub.files)) }) - it('asks for no tarball when the Repository is private', async () => { + it('reads blobs and no archive when the Repository is private', async () => { const fetchMock = sourceFetch({ visibility: 'private' }) const client = createGithubSourceClient({ fetch: fetchMock as unknown as typeof fetch, @@ -161,13 +109,13 @@ describe('loading a Skill from the Repository tarball', () => { visibility: 'private', }) - const loaded = await client.load({ ...resolvedSource(), visibility: 'private' }) - - expect(loaded._tag).toBe('loaded') + const loaded = await client.load({ ...resolvedSource(), visibility: 'private' }, { linkedFiles: false }) if (loaded._tag !== 'loaded') - return - expect(loaded.value.files.map(file => file.path)).toEqual(['SKILL.md', 'references/guide.md', 'run.sh']) - expect(requestedPaths(fetchMock).filter(url => url.includes('/tarball/'))).toEqual([]) + throw new Error('The private load failed') + const read = await readLoadedFiles(loaded.value) + + expect(read._tag).toBe('read') + expect(requestedPaths(fetchMock).filter(url => url.includes('codeload') || url.startsWith(rawBase))).toEqual([]) expect(requestedPaths(fetchMock).filter(url => url.includes('/git/blobs/'))).toHaveLength(3) }) @@ -175,18 +123,24 @@ describe('loading a Skill from the Repository tarball', () => { const fetchMock = sourceFetch({ submodule: true }) const client = createPublicGithubSourceClient({ fetch: fetchMock as unknown as typeof fetch }) - const loaded = await client.load(resolvedSource()) + const loaded = await client.load(resolvedSource(), { linkedFiles: false }) expect(loaded).toMatchObject({ _tag: 'rejected', code: 'INVALID_SOURCE', findings: ['vendor/library is a Git submodule'], }) - expect(requestedPaths(fetchMock).filter(url => url.includes('/tarball/'))).toEqual([]) - expect(requestedPaths(fetchMock).filter(url => url.includes('/git/blobs/'))).toEqual([]) + expect(requestedPaths(fetchMock).filter(url => url.includes('codeload') || url.includes('/git/blobs/'))).toEqual([]) }) }) +async function loadAndRead(fetchMock: ReturnType) { + const loaded = await createPublicGithubSourceClient({ fetch: fetchMock as unknown as typeof fetch }).load(resolvedSource(), { linkedFiles: false }) + if (loaded._tag !== 'loaded') + throw new Error(`The load failed: ${loaded.summary}`) + return await readLoadedFiles(loaded.value) +} + interface FetchOptions { visibility?: 'public' | 'private' truncated?: boolean @@ -255,11 +209,15 @@ function sourceFetch(options: FetchOptions) { .map(entry => ({ ...entry, path: entry.path.slice('references/'.length) })), }) } - if (url.includes('/tarball/')) { + if (url === archiveUrl) { if (options.tarballStatus) return new Response('no', { status: options.tarballStatus }) return new Response(archive, { status: 200, headers: { 'content-type': 'application/x-gzip' } }) } + if (url.startsWith(rawBase)) { + const entry = files.find(([path]) => url === `${rawBase}skills/demo/${path}`) + return entry ? new Response(entry[1], { status: 200 }) : new Response(null, { status: 404 }) + } const blobMatch = /\/git\/blobs\/([a-f0-9]{40})$/.exec(url) if (blobMatch) { const entry = files.find(([, text]) => gitBlobSha(text) === blobMatch[1]) diff --git a/test/unit/artifact-archive-reader.test.ts b/test/unit/artifact-archive-reader.test.ts new file mode 100644 index 00000000..e4f02fa6 --- /dev/null +++ b/test/unit/artifact-archive-reader.test.ts @@ -0,0 +1,464 @@ +import type { FileOrigin, FileSink, PackedFile } from '../../layers/artifact-delivery/server/utils/artifact-pack' +import type { TarFixtureEntry } from '../fixtures/tar-archive' +import { createHash, randomBytes } from 'node:crypto' +import { gunzipSync } from 'node:zlib' +import { describe, expect, it } from 'vitest' +import { createGithubArchiveReader } from '../../layers/artifact-delivery/server/utils/archive-reader' +import { tarGzFixture } from '../fixtures/tar-archive' + +const commitSha = '0123456789abcdef0123456789abcdef01234567' +const topLevel = `skills-${commitSha}` +const encoder = new TextEncoder() +const archiveUrl = `https://codeload.github.com/skilld-dev/skills/tar.gz/${commitSha}` +const rawBase = `https://raw.githubusercontent.com/skilld-dev/skills/${commitSha}/` + +const skill = encoder.encode('---\nname: demo\ndescription: Use this Skill for demo work.\n---\n') +const guide = encoder.encode('Read this first.\n') +const track = bytesOfLength(70_000) + +const files: PackedFile[] = [ + packed('SKILL.md', skill), + packed('assets/track.mp3', track), + packed('references/guide.md', guide), +] + +describe('reading Skill files from the Repository archive', () => { + it('passes only the Skill folder, in Artifact order, from one archive request', async () => { + const github = githubFetch({ archive: archiveOf([ + { path: 'README.md', bytes: encoder.encode('# Skills\n') }, + { path: 'skills/demo', typeflag: '5' }, + { path: 'skills/demo/SKILL.md', bytes: skill }, + { path: 'skills/demo/assets/track.mp3', bytes: track }, + { path: 'skills/demo/references/guide.md', bytes: guide }, + { path: 'skills/other/SKILL.md', bytes: skill }, + ]) }) + + const read = await readAll(github.fetch) + + expect(read.outcome).toEqual({ _tag: 'read' }) + expect(read.files).toEqual([ + { path: 'SKILL.md', origin: 'archive', bytes: skill }, + { path: 'assets/track.mp3', origin: 'archive', bytes: track }, + { path: 'references/guide.md', origin: 'archive', bytes: guide }, + ]) + expect(github.requested).toEqual([archiveUrl]) + }) + + it('stops reading the archive after the last Skill file', async () => { + const later = Array.from({ length: 50 }, (_, index) => ({ path: `z/${index}.bin`, bytes: bytesOfLength(20_000) })) + const github = githubFetch({ archive: archiveOf([ + { path: 'skills/demo/SKILL.md', bytes: skill }, + { path: 'skills/demo/assets/track.mp3', bytes: track }, + { path: 'skills/demo/references/guide.md', bytes: guide }, + ...later, + ]) }) + + const read = await readAll(github.fetch) + + expect(read.outcome).toEqual({ _tag: 'read' }) + expect(github.archiveCancelled()).toBe(true) + // The 50 later entries hold about 1 MB. One network chunk past the Skill is read at most. + expect(inflated.pulledBytes).toBeLessThan(200_000) + }) + + it('inflates the archive in small writes, so a file of zeros never arrives at once', async () => { + // 8 MiB of zeros gzips to about 8 KiB: one network chunk could inflate + // to the whole file. The default inflater is the runtime's own. + const zeros = new Uint8Array(8 * 1024 * 1024) + const noise = bytesOfLength(100_000) + const plan = [packed('SKILL.md', skill), packed('assets/blank.bin', zeros), packed('assets/noise.bin', noise)] + // One 100 KB response chunk, as a fast network may deliver. + const archive = archiveOf([ + { path: 'skills/demo/SKILL.md', bytes: skill }, + { path: 'skills/demo/assets/blank.bin', bytes: zeros }, + { path: 'skills/demo/assets/noise.bin', bytes: noise }, + ]) + const largest: number[] = [] + const reader = createGithubArchiveReader({ + fetch: (async () => new Response(archive, { status: 200 })) as unknown as typeof fetch, + owner: 'skilld-dev', + repository: 'skills', + commitSha, + skillPath: 'skills/demo', + files: plan, + maxArchiveBytes: 64 * 1024 * 1024, + budget: { githubReads: 0 }, + }) + let received = 0 + const outcome = await reader({ + begin: async () => {}, + chunk: async (bytes) => { + received += bytes.byteLength + largest.push(bytes.byteLength) + }, + end: async () => ({ _tag: 'verified' }), + }, new Set()) + + expect(outcome).toEqual({ _tag: 'read' }) + expect(received).toBe(skill.byteLength + zeros.byteLength + noise.byteLength) + expect(inputWrites.largest).toBeLessThanOrEqual(16 * 1024) + }) + + it('reads a Skill at the Repository root', async () => { + const github = githubFetch({ archive: archiveOf([ + { path: 'SKILL.md', bytes: skill }, + { path: 'assets/track.mp3', bytes: track }, + { path: 'references/guide.md', bytes: guide }, + ]) }) + + const read = await readAll(github.fetch, { skillPath: '.' }) + + expect(read.files.map(file => file.path)).toEqual(['SKILL.md', 'assets/track.mp3', 'references/guide.md']) + }) + + it('reads a file the archive left out from GitHub at the exact commit, as export-ignore produces', async () => { + const github = githubFetch({ + archive: archiveOf([ + { path: 'skills/demo/SKILL.md', bytes: skill }, + { path: 'skills/demo/references/guide.md', bytes: guide }, + ]), + raw: { 'skills/demo/assets/track.mp3': track }, + }) + + const read = await readAll(github.fetch) + + expect(read.outcome).toEqual({ _tag: 'read' }) + expect(read.files.map(file => [file.path, file.origin])).toEqual([ + ['SKILL.md', 'archive'], + ['assets/track.mp3', 'github'], + ['references/guide.md', 'archive'], + ]) + expect(github.requested).toEqual([archiveUrl, `${rawBase}skills/demo/assets/track.mp3`]) + }) + + it('reads a file from GitHub when the archive holds another size, as Git LFS or export-subst produce', async () => { + const github = githubFetch({ + archive: archiveOf([ + { path: 'skills/demo/SKILL.md', bytes: skill }, + { path: 'skills/demo/assets/track.mp3', bytes: encoder.encode('version https://git-lfs.github.com/spec/v1\n') }, + { path: 'skills/demo/references/guide.md', bytes: guide }, + ]), + raw: { 'skills/demo/assets/track.mp3': track }, + }) + + const read = await readAll(github.fetch) + + expect(read.files.find(file => file.path === 'assets/track.mp3')).toEqual({ path: 'assets/track.mp3', origin: 'github', bytes: track }) + }) + + it('reads the files it is told to from GitHub, and the rest from the archive', async () => { + const github = githubFetch({ + archive: archiveOf([ + { path: 'skills/demo/SKILL.md', bytes: skill }, + { path: 'skills/demo/assets/track.mp3', bytes: track }, + { path: 'skills/demo/references/guide.md', bytes: guide }, + ]), + raw: { 'skills/demo/references/guide.md': guide }, + }) + + const read = await readAll(github.fetch, { readFromGithub: new Set(['references/guide.md']) }) + + expect(read.files.map(file => [file.path, file.origin])).toEqual([ + ['SKILL.md', 'archive'], + ['assets/track.mp3', 'archive'], + ['references/guide.md', 'github'], + ]) + }) + + it('reads every file from GitHub when the archive request fails', async () => { + const github = githubFetch({ + archiveStatus: 504, + raw: { + 'skills/demo/SKILL.md': skill, + 'skills/demo/assets/track.mp3': track, + 'skills/demo/references/guide.md': guide, + }, + }) + + const read = await readAll(github.fetch) + + expect(read.outcome).toEqual({ _tag: 'read' }) + expect(read.files.every(file => file.origin === 'github')).toBe(true) + }) + + it('reads the rest from GitHub when the archive passes its byte ceiling first', async () => { + const github = githubFetch({ + archive: archiveOf([ + { path: 'aaa/huge.bin', bytes: bytesOfLength(200_000) }, + { path: 'skills/demo/SKILL.md', bytes: skill }, + { path: 'skills/demo/assets/track.mp3', bytes: track }, + { path: 'skills/demo/references/guide.md', bytes: guide }, + ]), + raw: { + 'skills/demo/SKILL.md': skill, + 'skills/demo/assets/track.mp3': track, + 'skills/demo/references/guide.md': guide, + }, + }) + + const read = await readAll(github.fetch, { maxArchiveBytes: 100_000 }) + + expect(read.outcome).toEqual({ _tag: 'read' }) + expect(read.files.every(file => file.origin === 'github')).toBe(true) + expect(github.archiveCancelled()).toBe(true) + }) + + it('refuses when the files left to read one by one pass the request budget', async () => { + const github = githubFetch({ + archiveStatus: 503, + raw: { + 'skills/demo/SKILL.md': skill, + 'skills/demo/assets/track.mp3': track, + 'skills/demo/references/guide.md': guide, + }, + }) + + const read = await readAll(github.fetch, { githubReads: 2 }) + + expect(read.outcome).toMatchObject({ _tag: 'rejected', code: 'SOURCE_UNAVAILABLE' }) + expect(github.requested.filter(url => url.startsWith(rawBase))).toHaveLength(2) + }) + + it('stops at a rate-limited archive instead of reading every file', async () => { + const github = githubFetch({ archiveStatus: 429, archiveHeaders: { 'retry-after': '90' }, raw: {} }) + + const read = await readAll(github.fetch) + + expect(read.outcome).toMatchObject({ _tag: 'rejected', code: 'RATE_LIMITED' }) + expect(github.requested).toEqual([archiveUrl]) + }) + + it('names a failed GitHub read as retryable', async () => { + const github = githubFetch({ archiveStatus: 503, raw: {}, rawStatus: 502 }) + + const read = await readAll(github.fetch) + + expect(read.outcome).toMatchObject({ _tag: 'rejected', code: 'SOURCE_UNAVAILABLE' }) + }) + + it('reads every file from GitHub when the archive names another commit', async () => { + const github = githubFetch({ + archive: archiveOf([ + { path: 'skills/demo/SKILL.md', bytes: skill }, + { path: 'skills/demo/assets/track.mp3', bytes: track }, + { path: 'skills/demo/references/guide.md', bytes: guide }, + ], 'f'.repeat(40)), + raw: { + 'skills/demo/SKILL.md': skill, + 'skills/demo/assets/track.mp3': track, + 'skills/demo/references/guide.md': guide, + }, + }) + + const read = await readAll(github.fetch) + + expect(read.files.every(file => file.origin === 'github')).toBe(true) + }) + + it('reports the file in progress when the archive breaks inside it', async () => { + const whole = archiveOf([ + { path: 'skills/demo/SKILL.md', bytes: skill }, + { path: 'skills/demo/assets/track.mp3', bytes: track }, + { path: 'skills/demo/references/guide.md', bytes: guide }, + ]) + const github = githubFetch({ archive: gzipPrefixOf(whole, 40_000) }) + + const read = await readAll(github.fetch) + + expect(read.outcome).toMatchObject({ _tag: 'mismatch', path: 'assets/track.mp3', origin: 'archive' }) + }) +}) + +interface ReadFile { path: string, origin: FileOrigin, bytes: Uint8Array } + +/** The largest write the runtime's inflater received. */ +const inputWrites = { largest: 0 } +const NativeDecompressionStream = globalThis.DecompressionStream +globalThis.DecompressionStream = class extends NativeDecompressionStream { + constructor(format: CompressionFormat) { + super(format) + const writable = this.writable + const writer = writable.getWriter() + writer.releaseLock() + const inner = writable.getWriter.bind(writable) + Object.defineProperty(this, 'writable', { + value: new WritableStream({ + async write(chunk) { + inputWrites.largest = Math.max(inputWrites.largest, (chunk as Uint8Array).byteLength) + const target = inner() + await target.write(chunk) + target.releaseLock() + }, + async close() { + const target = inner() + await target.close() + }, + async abort(reason) { + const target = inner() + await target.abort(reason) + }, + }), + }) + } +} as typeof DecompressionStream + +/** + * Node's `DecompressionStream` reads its whole input before it answers, so it + * cannot show that a read stopped early. This one inflates the gzip bytes + * and serves them on demand, as workerd does, and records whether the reader + * cancelled it. A cut gzip member fails when the reader reaches the cut. + */ +const inflated = { + cancelled: false, + pulledBytes: 0, + stream(body: ReadableStream): ReadableStream { + inflated.cancelled = false + inflated.pulledBytes = 0 + let bytes: Uint8Array | null = null + let failure: Error | null = null + let offset = 0 + return new ReadableStream({ + async pull(controller) { + if (!bytes && !failure) { + const gzipped = concat(await Array.fromAsync(body)) + try { + bytes = new Uint8Array(gunzipSync(gzipped)) + } + catch { + bytes = new Uint8Array(gunzipSync(gzipped, { finishFlush: 2 })) + failure = new Error('unexpected end of file') + } + } + if (offset >= bytes!.byteLength) { + if (failure) + controller.error(failure) + else + controller.close() + return + } + const chunk = bytes!.slice(offset, offset + 4096) + offset += chunk.byteLength + inflated.pulledBytes += chunk.byteLength + controller.enqueue(chunk) + }, + cancel() { + inflated.cancelled = true + }, + }) + }, +} + +async function readAll( + fetcher: typeof fetch, + options: { skillPath?: string, readFromGithub?: ReadonlySet, maxArchiveBytes?: number, githubReads?: number } = {}, +) { + const reader = createGithubArchiveReader({ + fetch: fetcher, + owner: 'skilld-dev', + repository: 'skills', + commitSha, + skillPath: options.skillPath ?? 'skills/demo', + files, + maxArchiveBytes: options.maxArchiveBytes ?? 64 * 1024 * 1024, + budget: { githubReads: options.githubReads ?? 100 }, + gunzip: inflated.stream, + }) + const read: ReadFile[] = [] + let current: { path: string, origin: FileOrigin, parts: Uint8Array[] } | null = null + const sink: FileSink = { + async begin(file, origin) { + current = { path: file.path, origin, parts: [] } + }, + async chunk(bytes) { + current!.parts.push(bytes.slice()) + }, + async end() { + const file = current! + current = null + const bytes = concat(file.parts) + const expected = files.find(candidate => candidate.path === file.path)! + if (bytes.byteLength !== expected.size) + return { _tag: 'mismatch', path: file.path, origin: file.origin, reason: 'size' } + read.push({ path: file.path, origin: file.origin, bytes }) + return { _tag: 'verified' } + }, + } + const outcome = await reader(sink, options.readFromGithub ?? new Set()) + return { outcome, files: read } +} + +function githubFetch(input: { + archive?: Uint8Array + archiveStatus?: number + archiveHeaders?: Record + raw?: Record + rawStatus?: number +}) { + const requested: string[] = [] + let cancelled = false + const fetcher = async (request: string | URL | Request): Promise => { + const url = typeof request === 'string' ? request : request instanceof URL ? request.href : request.url + requested.push(url) + if (url === archiveUrl) { + if (input.archiveStatus) + return new Response(null, { status: input.archiveStatus, headers: input.archiveHeaders }) + const bytes = input.archive! + let offset = 0 + return new Response(new ReadableStream({ + pull(controller) { + if (offset >= bytes.byteLength) { + controller.close() + return + } + controller.enqueue(bytes.slice(offset, offset + 4096)) + offset += 4096 + }, + cancel() { + cancelled = true + }, + }), { status: 200 }) + } + if (url.startsWith(rawBase)) { + if (input.rawStatus) + return new Response(null, { status: input.rawStatus }) + const path = decodeURIComponent(url.slice(rawBase.length)) + const bytes = input.raw?.[path] + return bytes ? new Response(bytes, { status: 200 }) : new Response(null, { status: 404 }) + } + throw new Error(`Unexpected request ${url}`) + } + return { fetch: fetcher as unknown as typeof fetch, requested, archiveCancelled: () => cancelled || inflated.cancelled } +} + +/** A GitHub archive: a pax global header naming the commit, then the entries. */ +function archiveOf(entries: TarFixtureEntry[], commit = commitSha): Uint8Array { + return tarGzFixture(topLevel, entries, { globalComment: commit }) +} + +function gzipPrefixOf(gzipped: Uint8Array, length: number): Uint8Array { + return gzipped.slice(0, length) +} + +function packed(path: string, bytes: Uint8Array): PackedFile { + return { + path, + mode: 420, + size: bytes.byteLength, + gitBlobSha: createHash('sha1').update(`blob ${bytes.byteLength}\0`).update(bytes).digest('hex'), + } +} + +/** Bytes gzip cannot shrink, so a cut archive ends inside the file. */ +function bytesOfLength(length: number): Uint8Array { + return new Uint8Array(randomBytes(length)) +} + +function concat(parts: Uint8Array[]): Uint8Array { + const merged = new Uint8Array(parts.reduce((total, part) => total + part.byteLength, 0)) + let offset = 0 + for (const part of parts) { + merged.set(part, offset) + offset += part.byteLength + } + return merged +} diff --git a/test/unit/artifact-build-reuse.test.ts b/test/unit/artifact-build-reuse.test.ts index b1b7e3d4..40613556 100644 --- a/test/unit/artifact-build-reuse.test.ts +++ b/test/unit/artifact-build-reuse.test.ts @@ -34,6 +34,7 @@ import { transitionResolution, } from '../../layers/artifact-delivery/server/utils/state' import { handleArtifactSignerRequest } from '../../workers/artifact-signer/src/handler' +import { loadedFromFiles } from '../fixtures/loaded-source' import { createSqliteD1 } from './helpers/d1-sqlite' const NOW = 1_787_227_200 @@ -43,6 +44,7 @@ const MIGRATIONS = [ 'migrations/0111_github_app_delivery.sql', 'migrations/0112_private_artifact_keys.sql', 'migrations/0122_artifact_resolution_retry_after.sql', + 'migrations/0145_artifact_resolution_linked_files.sql', 'migrations/0131_artifact_build_reuse.sql', 'migrations/0144_artifact_resolution_requesters.sql', ] @@ -505,7 +507,7 @@ async function createReuseHarness() { function githubServing(resolved: ResolvedSource) { return { resolve: vi.fn(async () => ({ _tag: 'resolved' as const, source: resolved })), - load: vi.fn(async () => ({ _tag: 'loaded' as const, value: { source: resolved, files } })), + load: vi.fn(async () => ({ _tag: 'loaded' as const, value: loadedFromFiles(resolved, files) })), } satisfies PublicGithubSourceClient } @@ -594,7 +596,7 @@ function memoryBucket() { const object = describeObject(key) const stored = objects.get(key) return object && stored - ? { ...object, arrayBuffer: async () => Uint8Array.from(stored.bytes).buffer } + ? { ...object, arrayBuffer: async () => Uint8Array.from(stored.bytes).buffer, body: new Blob([Uint8Array.from(stored.bytes)]).stream() } : null }, } as unknown as R2Bucket diff --git a/test/unit/artifact-client-capabilities.test.ts b/test/unit/artifact-client-capabilities.test.ts new file mode 100644 index 00000000..d834b07d --- /dev/null +++ b/test/unit/artifact-client-capabilities.test.ts @@ -0,0 +1,17 @@ +import { describe, expect, it } from 'vitest' +import { readsLinkedFiles } from '../../layers/artifact-delivery/server/utils/request-resolution' + +describe('the Skilld-Capabilities request header', () => { + it.each([ + ['linked-files', true], + ['Linked-Files', true], + ['streaming, linked-files', true], + [' linked-files ,other', true], + ['linked-file', false], + ['linked-files-v2', false], + ['', false], + [undefined, false], + ])('reads %j as linked files: %s', (header, expected) => { + expect(readsLinkedFiles(header)).toBe(expected) + }) +}) diff --git a/test/unit/artifact-delivery.test.ts b/test/unit/artifact-delivery.test.ts index ea838500..8c7389de 100644 --- a/test/unit/artifact-delivery.test.ts +++ b/test/unit/artifact-delivery.test.ts @@ -31,7 +31,9 @@ import { resolutionRequestIdentity, transitionResolution, } from '../../layers/artifact-delivery/server/utils/state' -import { createDeterministicUstar } from '../../layers/artifact-delivery/server/utils/ustar' +import { compareArtifactPaths, createDeterministicUstar } from '../../layers/artifact-delivery/server/utils/ustar' +import { loadedFromFiles } from '../fixtures/loaded-source' +import { tarGzFixture } from '../fixtures/tar-archive' import { createSqliteD1 } from './helpers/d1-sqlite' const NOW = 1_787_227_200 @@ -42,6 +44,7 @@ const ARTIFACT_MIGRATIONS = [ 'migrations/0112_private_artifact_keys.sql', 'migrations/0122_artifact_resolution_retry_after.sql', 'migrations/0144_artifact_resolution_requesters.sql', + 'migrations/0145_artifact_resolution_linked_files.sql', ] const sourceRequest: SourceRequest = { provider: 'github', @@ -146,10 +149,10 @@ describe('public Artifact delivery', () => { harness.close() }) - it('keeps a 627-file build inside the Worker subrequest budget', async () => { + it('builds 1,500 files from one archive read, with no read per file', async () => { const files: ArtifactSourceFile[] = [ validFiles[0]!, - ...Array.from({ length: 626 }, (_, index) => ({ + ...Array.from({ length: 1499 }, (_, index) => ({ path: `references/entry-${index}.md`, mode: 420 as const, bytes: new TextEncoder().encode(`entry ${index}\n`), @@ -167,7 +170,9 @@ describe('public Artifact delivery', () => { const result = await processArtifactBuild(harness.dependencies, harness.resolutionId) expect(result).toEqual({ _tag: 'ready', resolutionId: harness.resolutionId }) - expect(fetch.mock.calls.length).toBeLessThanOrEqual(1000) + // The Repository, three tree reads, and one archive. + expect(fetch.mock.calls.map(([input]) => String(input)).filter(url => url.includes('codeload.github.com'))).toHaveLength(1) + expect(fetch.mock.calls.length).toBeLessThanOrEqual(10) harness.close() }) @@ -778,7 +783,7 @@ async function createBuildHarness( : resolvedSource const github: PublicGithubSourceClient = githubOverride ?? { resolve: vi.fn(async () => ({ _tag: 'resolved', source: buildSource })), - load: vi.fn(async () => ({ _tag: 'loaded', value: { source: buildSource, files } })), + load: vi.fn(async () => ({ _tag: 'loaded', value: loadedFromFiles(buildSource, files) })), } const put = vi.fn(async () => ({}) as R2Object) const bucket = { @@ -924,16 +929,8 @@ function countingGithubFetch(files: ArtifactSourceFile[]) { })), }) } - const blobSha = url.match(/\/git\/blobs\/([0-9a-f]{40})$/)?.[1] - const blob = blobSha ? blobs.get(blobSha) : undefined - if (blob && blobSha) { - return json({ - sha: blobSha, - size: blob.bytes.byteLength, - encoding: 'base64', - content: Buffer.from(blob.bytes).toString('base64'), - }) - } + if (url === codeloadUrl()) + return new Response(archiveOf(files), { status: 200 }) return json({ message: 'Not Found' }, 404) }) } @@ -981,18 +978,25 @@ function workerdLikeGithubFetch(file: ArtifactSourceFile) { tree: [{ path: file.path, mode: '100644', type: 'blob', sha: blobSha, size: file.bytes.byteLength }], }) } - if (path === `${repo}/git/blobs/${blobSha}`) { - return json({ - sha: blobSha, - size: file.bytes.byteLength, - encoding: 'base64', - content: Buffer.from(file.bytes).toString('base64'), - }) - } + if (String(input) === codeloadUrl()) + return new Response(archiveOf([file]), { status: 200 }) return json({ message: 'Not Found' }, 404) }) } +function codeloadUrl(): string { + return `https://codeload.github.com/${resolvedSource.owner}/${resolvedSource.repository}/tar.gz/${resolvedSource.commitSha}` +} + +/** The Repository archive codeload serves for these Skill files, in Git order. */ +function archiveOf(files: ArtifactSourceFile[]): Uint8Array { + const ordered = [...files].sort((left, right) => compareArtifactPaths(left.path, right.path)) + return tarGzFixture(`${resolvedSource.repository}-${resolvedSource.commitSha}`, ordered.map(file => ({ + path: `${resolvedSource.skillPath}/${file.path}`, + bytes: file.bytes, + })), { globalComment: resolvedSource.commitSha }) +} + function artifactQueueBatch(resolutionId: string, attempts: number) { const ack = vi.fn() const retry = vi.fn() diff --git a/test/unit/artifact-github-reads.test.ts b/test/unit/artifact-github-reads.test.ts index 747b2b7e..9c7f5a83 100644 --- a/test/unit/artifact-github-reads.test.ts +++ b/test/unit/artifact-github-reads.test.ts @@ -41,6 +41,7 @@ const MIGRATIONS = [ 'migrations/0111_github_app_delivery.sql', 'migrations/0112_private_artifact_keys.sql', 'migrations/0122_artifact_resolution_retry_after.sql', + 'migrations/0145_artifact_resolution_linked_files.sql', 'migrations/0131_artifact_build_reuse.sql', ] const REPOSITORY_ID = 123456789 @@ -90,7 +91,7 @@ describe('gitHub reads per run', () => { '/repos/skilld-dev/skills', '/repos/skilld-dev/skills', `/repos/skilld-dev/skills/git/trees/${OTHER_TREE}?recursive=1`, - `/repos/skilld-dev/skills/tarball/${COMMIT}`, + `/skilld-dev/skills/tar.gz/${COMMIT}`, ]) harness.close() }) @@ -138,7 +139,7 @@ describe('a policy bump that changes only checks', () => { const repeat = await harness.run(pinned) expect(repeat.row.state).toBe('ready') - expect(harness.github.paths()).toContain(`/repos/skilld-dev/skills/tarball/${COMMIT}`) + expect(harness.github.paths()).toContain(`/skilld-dev/skills/tar.gz/${COMMIT}`) harness.close() }) @@ -151,7 +152,7 @@ describe('a policy bump that changes only checks', () => { const repeat = await harness.run(pinned) expect(repeat.row.state).toBe('ready') - expect(harness.github.paths()).toContain(`/repos/skilld-dev/skills/tarball/${COMMIT}`) + expect(harness.github.paths()).toContain(`/skilld-dev/skills/tar.gz/${COMMIT}`) harness.close() }) }) @@ -363,7 +364,8 @@ async function fakeGithub() { [`${base}/git/trees/${OTHER_TREE}?recursive=1`, () => json({ sha: OTHER_TREE, truncated: false, tree: [ { path: 'SKILL.md', mode: '100644', type: 'blob', sha: otherBlob, size: OTHER_MD.byteLength }, ] })], - [`${base}/tarball/${COMMIT}`, () => new Response(Uint8Array.from(tarball), { status: 200 })], + // codeload.github.com, which serves the archive without REST quota. + [`/skilld-dev/skills/tar.gz/${COMMIT}`, () => new Response(Uint8Array.from(tarball), { status: 200 })], ]) let requested: string[] = [] const fetch = vi.fn(async (input: RequestInfo | URL) => { @@ -420,7 +422,7 @@ function memoryBucket(): R2Bucket { get: async (key: string) => { const object = describe(key) const stored = objects.get(key) - return object && stored ? { ...object, arrayBuffer: async () => Uint8Array.from(stored.bytes).buffer } : null + return object && stored ? { ...object, arrayBuffer: async () => Uint8Array.from(stored.bytes).buffer, body: new Blob([Uint8Array.from(stored.bytes)]).stream() } : null }, } as unknown as R2Bucket } diff --git a/test/unit/artifact-github-source-scale.test.ts b/test/unit/artifact-github-source-scale.test.ts index fd5770e2..0f181f0b 100644 --- a/test/unit/artifact-github-source-scale.test.ts +++ b/test/unit/artifact-github-source-scale.test.ts @@ -89,7 +89,7 @@ describe('a Repository tree larger than one GitHub response', () => { }) const client = createPublicGithubSourceClient({ fetch: fetchMock as typeof fetch }) - const result = await client.load(resolvedSource('skilld-dev', 'skills')) + const result = await client.load(resolvedSource('skilld-dev', 'skills'), { linkedFiles: false }) expect(result._tag).toBe('loaded') }) @@ -129,7 +129,7 @@ describe('a moved Repository', () => { const fetchMock = movedRepositoryFetch() const client = createPublicGithubSourceClient({ fetch: fetchMock as typeof fetch }) - const result = await client.load(resolvedSource('facebook', 'react', 10270250)) + const result = await client.load(resolvedSource('facebook', 'react', 10270250), { linkedFiles: false }) expect(result._tag).toBe('loaded') const reads = fetchMock.mock.calls.map(call => String(call[0])) @@ -140,7 +140,7 @@ describe('a moved Repository', () => { const fetchMock = movedRepositoryFetch() const client = createPublicGithubSourceClient({ fetch: fetchMock as typeof fetch }) - const result = await client.load(resolvedSource('facebook', 'react', 999)) + const result = await client.load(resolvedSource('facebook', 'react', 999), { linkedFiles: false }) expect(result).toMatchObject({ _tag: 'rejected', code: 'INVALID_SOURCE' }) }) diff --git a/test/unit/artifact-github-source.test.ts b/test/unit/artifact-github-source.test.ts index 42b66679..ddf4dbbf 100644 --- a/test/unit/artifact-github-source.test.ts +++ b/test/unit/artifact-github-source.test.ts @@ -4,6 +4,7 @@ import { createGithubSourceClient, createPublicGithubSourceClient, } from '../../layers/artifact-delivery/server/utils/github-source' +import { readLoadedFiles } from '../fixtures/loaded-source' const commitSha = '0123456789abcdef0123456789abcdef01234567' const rootTreeSha = '89abcdef0123456789abcdef0123456789abcdef' @@ -193,13 +194,15 @@ describe('public GitHub Artifact source', () => { expect(resolution._tag).toBe('resolved') if (resolution._tag !== 'resolved') return - const loaded = await client.load(resolution.source) + const loaded = await client.load(resolution.source, { linkedFiles: false }) - expect(loaded._tag).toBe('loaded') expect(resolution.source).toMatchObject({ commitSha, treeSha: rootTreeSha }) - expect(fetchMock.mock.calls.map(call => String(call[0]))).toContain( - `https://api.github.com/repos/skilld-dev/skills/git/blobs/${skillBlobSha}`, - ) + expect(loaded).toMatchObject({ + _tag: 'loaded', + value: { files: [{ path: 'SKILL.md', mode: 420, size: skillText.length, gitBlobSha: skillBlobSha }] }, + }) + // A load plans the bytes and reads none. A public build never reads a blob through the REST API. + expect(fetchMock.mock.calls.map(call => String(call[0])).filter(url => url.includes('/git/blobs/') || url.includes('codeload'))).toEqual([]) }) it('rejects a commit lookup that returns another identity', async () => { @@ -239,7 +242,7 @@ describe('public GitHub Artifact source', () => { const fetchMock = sourceTreeFetch([blob('SKILL.md', skillBlobSha, skillText.length), unsafe]) const client = createPublicGithubSourceClient({ fetch: fetchMock as typeof fetch }) - const result = await client.load(resolvedSource()) + const result = await client.load(resolvedSource(), { linkedFiles: false }) expect(result).toMatchObject({ _tag: 'rejected', code: 'INVALID_SOURCE' }) expect(fetchMock.mock.calls.some(call => String(call[0]).includes('/git/blobs/'))).toBe(false) @@ -261,7 +264,7 @@ describe('public GitHub Artifact source', () => { }) const client = createPublicGithubSourceClient({ fetch: fetchMock as typeof fetch, token: 'private-token' }) - const result = await client.load(resolvedSource()) + const result = await client.load(resolvedSource(), { linkedFiles: false }) expect(result).toMatchObject({ _tag: 'rejected', code: 'SOURCE_ACCESS_DENIED' }) expect(fetchMock).toHaveBeenCalledTimes(1) @@ -275,7 +278,7 @@ describe('public GitHub Artifact source', () => { ]) const client = createPublicGithubSourceClient({ fetch: fetchMock as typeof fetch }) - const result = await client.load(resolvedSource()) + const result = await client.load(resolvedSource(), { linkedFiles: false }) expect(result).toMatchObject({ _tag: 'rejected', @@ -329,7 +332,7 @@ describe('public GitHub Artifact source', () => { }) const client = createPublicGithubSourceClient({ fetch: fetchMock as typeof fetch }) - const result = await client.load(resolvedSource()) + const result = await client.load(resolvedSource(), { linkedFiles: false }) expect(result._tag).toBe('loaded') if (result._tag === 'loaded') @@ -344,7 +347,7 @@ describe('public GitHub Artifact source', () => { ) const client = createPublicGithubSourceClient({ fetch: fetchMock as typeof fetch }) - const result = await client.load(resolvedSource()) + const result = await client.load(resolvedSource(), { linkedFiles: false }) expect(result).toMatchObject({ _tag: 'rejected', code: 'INVALID_SOURCE' }) expect(fetchMock.mock.calls.some(call => String(call[0]).includes('/git/blobs/'))).toBe(false) @@ -367,7 +370,7 @@ describe('public GitHub Artifact source', () => { }) const client = createPublicGithubSourceClient({ fetch: fetchMock as typeof fetch }) - const result = await client.load(resolvedSource()) + const result = await client.load(resolvedSource(), { linkedFiles: false }) expect(result).toMatchObject({ _tag: 'rejected', code: 'INVALID_SOURCE' }) }) @@ -380,7 +383,7 @@ describe('public GitHub Artifact source', () => { ]) const client = createPublicGithubSourceClient({ fetch: fetchMock as typeof fetch }) - const result = await client.load(resolvedSource()) + const result = await client.load(resolvedSource(), { linkedFiles: false }) expect(result).toMatchObject({ _tag: 'rejected', code: 'INVALID_SOURCE' }) if (result._tag === 'rejected') @@ -389,13 +392,13 @@ describe('public GitHub Artifact source', () => { it('rejects bytes that do not match the commit blob digest', async () => { const changed = skillText.replace('demo work', 'other use') - const fetchMock = sourceTreeFetch( - [blob('SKILL.md', skillBlobSha, changed.length)], - { sha: skillBlobSha, size: changed.length, encoding: 'base64', content: btoa(changed) }, - ) + const fetchMock = sourceTreeFetch([blob('SKILL.md', skillBlobSha, changed.length)], undefined, false, changed) const client = createPublicGithubSourceClient({ fetch: fetchMock as typeof fetch }) - const result = await client.load(resolvedSource()) + const loaded = await client.load(resolvedSource(), { linkedFiles: false }) + if (loaded._tag !== 'loaded') + throw new Error('The load reads no bytes') + const result = await readLoadedFiles(loaded.value) expect(result).toMatchObject({ _tag: 'rejected', @@ -460,9 +463,11 @@ describe('private GitHub Artifact source', () => { }) }) -function sourceTreeFetch(entries: object[], blobResult?: object, truncatedWalk = false) { +function sourceTreeFetch(entries: object[], blobResult?: object, truncatedWalk = false, rawText?: string) { return vi.fn(async (input: RequestInfo | URL) => { const url = String(input) + if (url.startsWith('https://raw.githubusercontent.com/') && rawText !== undefined) + return new Response(rawText, { status: 200 }) if (url.endsWith('/repos/skilld-dev/skills')) return json(publicRepository()) if (url.endsWith(`/git/trees/${rootTreeSha}`)) diff --git a/test/unit/artifact-large-build.test.ts b/test/unit/artifact-large-build.test.ts new file mode 100644 index 00000000..b3642d0c --- /dev/null +++ b/test/unit/artifact-large-build.test.ts @@ -0,0 +1,367 @@ +import type { SourceRequest } from '../../layers/artifact-delivery/server/schemas/contracts' +import type { ArtifactBuildDependencies, ArtifactBuildReuseReport } from '../../layers/artifact-delivery/server/utils/build' +import type { TrustedRoot } from '../../layers/artifact-delivery/server/utils/trusted-root' +import type { ArtifactSignerBindings } from '../../workers/artifact-signer/src/handler' +import type { TarFixtureEntry } from '../fixtures/tar-archive' +import { createHash } from 'node:crypto' +import { describe, expect, it } from 'vitest' +import { artifactAttestationSchema } from '../../layers/artifact-delivery/server/schemas/contracts' +import { ARTIFACT_SPOOL_BYTES } from '../../layers/artifact-delivery/server/utils/artifact-pack' +import { createArtifactSigner } from '../../layers/artifact-delivery/server/utils/attestation' +import { processArtifactBuild } from '../../layers/artifact-delivery/server/utils/build' +import { bytesToBase64Url } from '../../layers/artifact-delivery/server/utils/encoding' +import { createPublicGithubSourceClient } from '../../layers/artifact-delivery/server/utils/github-source' +import { createResolution, getResolution, resolutionRequestIdentity } from '../../layers/artifact-delivery/server/utils/state' +import { compareArtifactPaths, createDeterministicUstar } from '../../layers/artifact-delivery/server/utils/ustar' +import { handleArtifactSignerRequest } from '../../workers/artifact-signer/src/handler' +import { tarGzFixture } from '../fixtures/tar-archive' +import { createSqliteD1 } from './helpers/d1-sqlite' + +/** + * Skills larger than one build may hold in memory, built end to end through + * the real source client, packer, R2 writes, and signer. + */ + +const NOW = 1_787_227_200 +const MIB = 1024 * 1024 +const MIGRATIONS = [ + 'migrations/0017_users.sql', + 'migrations/0110_artifact_delivery.sql', + 'migrations/0111_github_app_delivery.sql', + 'migrations/0112_private_artifact_keys.sql', + 'migrations/0122_artifact_resolution_retry_after.sql', + 'migrations/0131_artifact_build_reuse.sql', + 'migrations/0145_artifact_resolution_linked_files.sql', +] +const REPOSITORY_ID = 987654321 +const COMMIT = '0123456789abcdef0123456789abcdef01234567' +const ROOT_TREE = '1'.repeat(40) +const SKILLS_TREE = '2'.repeat(40) +const BRAG_TREE = '3'.repeat(40) +const encoder = new TextEncoder() +const SKILL_MD = encoder.encode('---\nname: brag\ndescription: Use this Skill to film a brag reel.\n---\n\n# Brag\n') + +const request: SourceRequest = { + provider: 'github', + owner: 'latent-spaces', + repository: 'brag', + selector: { type: 'path', path: 'skills/brag' }, + ref: { type: 'commit', value: COMMIT }, +} + +describe('a Skill over the in-memory size', () => { + it('streams its archive into R2 and signs it, with every music file', async () => { + // latent-spaces/brag packs to 16 MiB with its music. This one passes the + // size a scan keeps, so the store reads the archive a second time. + const music = patterned(ARTIFACT_SPOOL_BYTES + MIB) + const harness = await createHarness([ + ['SKILL.md', SKILL_MD], + ['assets/music/happy-beats.mp3', music], + ['scripts/render.ts', encoder.encode('export const reel = true\n')], + ]) + + const built = await harness.run(false) + + expect(built.outcome).toEqual({ _tag: 'ready', resolutionId: built.row.id }) + const attestation = artifactAttestationSchema.parse(JSON.parse(built.row.attestation_json!)) + expect(attestation.files.map(file => file.path)).toEqual(['SKILL.md', 'assets/music/happy-beats.mp3', 'scripts/render.ts']) + expect(attestation.checkResults.find(check => check.name === 'omitted-files')).toMatchObject({ outcome: 'pass' }) + expect(harness.archiveReads()).toBe(2) + // Compared by digest: a byte-by-byte diff of 17 MiB takes vitest half a minute. + const stored = harness.stored(attestation.contentSha256) + const inMemory = createDeterministicUstar(harness.files.map(([path, bytes]) => ({ path, mode: 420, bytes, gitBlobSha: gitBlobSha(bytes) }))) + expect(sha256(stored!)).toBe(sha256(inMemory)) + expect(sha256(stored!)).toBe(attestation.contentSha256) + harness.close() + }) + + it('fails as retryable, and stores nothing, when the second read serves other bytes', async () => { + const harness = await createHarness([ + ['SKILL.md', SKILL_MD], + ['assets/music/happy-beats.mp3', patterned(ARTIFACT_SPOOL_BYTES + MIB)], + ], { changeOnSecondRead: 'skills/brag/assets/music/happy-beats.mp3' }) + + const built = await harness.run(false) + + expect(built.row).toMatchObject({ state: 'failed', error_code: 'SOURCE_UNAVAILABLE', error_retryable: 1 }) + expect(harness.storedKeys()).toEqual([]) + harness.close() + }) +}) + +describe('a Skill over the 64 MiB archive limit', () => { + // thvroyal/kimi-skills/kimi-xlsx runs a 73 MiB binary. The archive never + // needs its bytes: a CLI either reads it from GitHub or does without it. + const binary: readonly [string, number] = ['scripts/KimiXlsx', 77_001_601] + + it('lists the binary as a linked file for a CLI that reads them', async () => { + const harness = await createHarness([['SKILL.md', SKILL_MD], ['scripts/run.py', encoder.encode('print(1)\n')]], { unpacked: [binary] }) + + const built = await harness.run(true) + + expect(built.row.state).toBe('ready') + const attestation = artifactAttestationSchema.parse(JSON.parse(built.row.attestation_json!)) + expect(attestation.linkedFiles).toEqual([{ path: 'scripts/KimiXlsx', mode: 420, size: 77_001_601, gitBlobSha: harness.unpackedSha('scripts/KimiXlsx') }]) + expect(attestation.files.map(file => file.path)).toEqual(['SKILL.md', 'scripts/run.py']) + expect(attestation.checkResults.find(check => check.name === 'omitted-files')).toMatchObject({ outcome: 'pass' }) + harness.close() + }) + + it('leaves the binary out for any other CLI, and names it', async () => { + const harness = await createHarness([['SKILL.md', SKILL_MD], ['scripts/run.py', encoder.encode('print(1)\n')]], { unpacked: [binary] }) + + const built = await harness.run(false) + + expect(built.row.state).toBe('ready') + const attestation = artifactAttestationSchema.parse(JSON.parse(built.row.attestation_json!)) + expect(attestation).not.toHaveProperty('linkedFiles') + expect(attestation.checkResults.find(check => check.name === 'omitted-files')).toMatchObject({ + outcome: 'warn', + findings: [`scripts/KimiXlsx: 77,001,601 bytes, https://github.com/latent-spaces/brag/blob/${COMMIT}/skills/brag/scripts/KimiXlsx`], + }) + harness.close() + }) + + it('never reuses a build made for the other kind of CLI', async () => { + const harness = await createHarness([['SKILL.md', SKILL_MD]], { unpacked: [binary] }) + const omitting = await harness.run(false) + harness.reports.length = 0 + + const linking = await harness.run(true) + const omittingAgain = await harness.run(false) + + expect(linking.row.state).toBe('ready') + expect(linking.row.attestation_json).not.toBe(omitting.row.attestation_json) + expect(harness.reports).toContainEqual(expect.objectContaining({ _tag: 'miss', reason: 'linked-files-differ' })) + expect(artifactAttestationSchema.parse(JSON.parse(omittingAgain.row.attestation_json!))).not.toHaveProperty('linkedFiles') + harness.close() + }) + + it('reuses one build for both kinds of CLI when nothing is linked or left out', async () => { + const harness = await createHarness([['SKILL.md', SKILL_MD], ['assets/music/happy-beats.mp3', patterned(3 * MIB)]]) + await harness.run(false) + harness.reports.length = 0 + + const linking = await harness.run(true) + + expect(linking.row.state).toBe('ready') + expect(harness.reports).toContainEqual(expect.objectContaining({ _tag: 'hit' })) + expect(harness.archiveReads()).toBe(1) + harness.close() + }) +}) + +type SkillFiles = Array<[string, Uint8Array]> + +async function createHarness( + files: SkillFiles, + options: { unpacked?: ReadonlyArray, changeOnSecondRead?: string } = {}, +) { + const sqlite = createSqliteD1(MIGRATIONS) + const storage = memoryBucket() + const github = fakeGithub(files, options) + const reports: ArtifactBuildReuseReport[] = [] + let clock = NOW + let requests = 0 + const signing = await signingKey('skilld-production-2026-08') + const signerBindings: ArtifactSignerBindings = { + DB: sqlite.db, + PUBLIC_ARTIFACTS: storage.bucket, + PRIVATE_ARTIFACTS: storage.bucket, + ARTIFACT_SIGNING_MAX_AGE_SECONDS: '300', + ...signing.bindings, + } + const signer = createArtifactSigner({ + fetch: async (signed: Request) => await handleArtifactSignerRequest(signed, signerBindings, () => clock), + } as unknown as Fetcher) + const dependencies = (): ArtifactBuildDependencies => ({ + db: sqlite.db, + github: createPublicGithubSourceClient({ fetch: github.fetch, now: () => clock }), + bucket: storage.bucket, + signer, + trustedRoot: signing.trustedRoot, + now: () => clock, + reportReuse: report => reports.push(report), + // Node has no `FixedLengthStream`. R2 reads the same bytes from either. + fixedLengthStream: () => new TransformStream(), + }) + + return { + files, + reports, + archiveReads: () => github.archiveReads(), + stored: (contentSha256: string) => storage.bytes(contentSha256), + storedKeys: () => storage.keys(), + unpackedSha: (path: string) => github.unpackedSha(path), + async run(linkedFiles: boolean) { + requests += 1 + const identity = await resolutionRequestIdentity(request, `large-build-request-${requests}`, undefined, linkedFiles) + const created = await createResolution(sqlite.db, request, identity, clock, { visibility: 'public' }, linkedFiles) + if (created._tag === 'idempotency-conflict') + throw new Error('Test Resolution conflicted') + clock += 1 + const outcome = await processArtifactBuild(dependencies(), created.row.id) + const row = await getResolution(sqlite.db, created.row.id) + return { outcome, row: row! } + }, + close: sqlite.close, + } +} + +/** One public Repository with one Skill folder, served the way GitHub answers. */ +function fakeGithub( + files: SkillFiles, + options: { unpacked?: ReadonlyArray, changeOnSecondRead?: string }, +) { + const unpacked = new Map((options.unpacked ?? []).map(([path]) => [path, createHash('sha1').update(`unpacked ${path}`).digest('hex')])) + const treeEntries = [ + ...files.map(([path, bytes]) => ({ path, mode: '100644', type: 'blob', sha: gitBlobSha(bytes), size: bytes.byteLength })), + ...(options.unpacked ?? []).map(([path, size]) => ({ path, mode: '100644', type: 'blob', sha: unpacked.get(path)!, size })), + ] + const archive = (changed: boolean): Uint8Array => { + const entries: TarFixtureEntry[] = [...files] + .sort(([left], [right]) => compareArtifactPaths(left, right)) + .map(([path, bytes]) => { + const full = `skills/brag/${path}` + return { path: full, bytes: changed && full === options.changeOnSecondRead ? flipFirstByte(bytes) : bytes } + }) + return tarGzFixture(`brag-${COMMIT}`, entries, { globalComment: COMMIT }) + } + let archiveReads = 0 + const base = '/repos/latent-spaces/brag' + const json = (value: unknown, status = 200) => new Response(JSON.stringify(value), { status, headers: { 'content-type': 'application/json' } }) + const fetch = (async (input: RequestInfo | URL) => { + const url = new URL(String(input)) + if (url.hostname === 'codeload.github.com' && url.pathname === `/latent-spaces/brag/tar.gz/${COMMIT}`) { + archiveReads += 1 + return new Response(archive(archiveReads > 1), { status: 200 }) + } + const path = `${url.pathname}${url.search}` + if (path === base) + return json({ id: REPOSITORY_ID, name: 'brag', owner: { login: 'latent-spaces' }, private: false, default_branch: 'main' }) + if (path === `${base}/commits/${COMMIT}`) + return json({ sha: COMMIT, commit: { tree: { sha: ROOT_TREE } } }) + if (path === `${base}/git/trees/${ROOT_TREE}`) + return json({ sha: ROOT_TREE, tree: [{ path: 'skills', mode: '040000', type: 'tree', sha: SKILLS_TREE }] }) + if (path === `${base}/git/trees/${SKILLS_TREE}`) + return json({ sha: SKILLS_TREE, tree: [{ path: 'brag', mode: '040000', type: 'tree', sha: BRAG_TREE }] }) + if (path === `${base}/git/trees/${BRAG_TREE}?recursive=1`) + return json({ sha: BRAG_TREE, truncated: false, tree: treeEntries }) + return json({ message: 'Not Found' }, 404) + }) as unknown as typeof globalThis.fetch + return { + fetch, + archiveReads: () => archiveReads, + unpackedSha: (path: string) => unpacked.get(path)!, + } +} + +/** An R2 bucket in memory that reads streams and checks the SHA-256 it is given, as R2 does. */ +function memoryBucket() { + const objects = new Map }>() + const describe = (key: string) => { + const object = objects.get(key) + if (!object) + return null + return { + key, + size: object.bytes.byteLength, + checksums: { sha256: Uint8Array.from(createHash('sha256').update(object.bytes).digest()).buffer }, + customMetadata: object.customMetadata, + } + } + const bucket = { + put: async (key: string, value: Uint8Array | ReadableStream, options?: R2PutOptions) => { + const bytes = value instanceof ReadableStream ? concat(await Array.fromAsync(value)) : Uint8Array.from(value) + const expected = options?.sha256 ? Buffer.from(options.sha256 as Uint8Array).toString('hex') : null + if (expected && sha256(bytes) !== expected) + throw new Error('put: The SHA-256 checksum you specified did not match what we received.') + if (objects.has(key)) + return null + objects.set(key, { bytes, customMetadata: options?.customMetadata ?? {} }) + return describe(key) + }, + head: async (key: string) => describe(key), + get: async (key: string) => { + const object = describe(key) + const stored = objects.get(key) + if (!object || !stored) + return null + return { + ...object, + body: new Blob([Uint8Array.from(stored.bytes)]).stream(), + arrayBuffer: async () => Uint8Array.from(stored.bytes).buffer, + } + }, + } as unknown as R2Bucket + return { + bucket, + keys: () => [...objects.keys()], + bytes: (contentSha256: string) => [...objects.entries()].find(([key]) => key.includes(contentSha256))?.[1].bytes ?? null, + } +} + +async function signingKey(keyId: string) { + const keyPair = await crypto.subtle.generateKey('Ed25519', true, ['sign', 'verify']) + const publicKey = bytesToBase64Url(new Uint8Array(await crypto.subtle.exportKey('raw', keyPair.publicKey))) + const privateKey = bytesToBase64Url(new Uint8Array(await crypto.subtle.exportKey('pkcs8', keyPair.privateKey))) + const notBefore = new Date((NOW - 60) * 1000).toISOString() + const notAfter = new Date((NOW + 3600) * 1000).toISOString() + const trustedRoot: TrustedRoot = { + version: 1, + rootKeyId: 'skilld-root-2026', + rootPublicKey: publicKey, + keys: [{ + keyId, + algorithm: 'Ed25519', + publicKey, + notBefore, + notAfter, + status: 'active', + statement: bytesToBase64Url(encoder.encode(JSON.stringify({ keyId }))), + rootSignature: 'C'.repeat(86), + }], + fetchedAt: new Date(NOW * 1000).toISOString(), + } + return { + trustedRoot, + bindings: { + ARTIFACT_SIGNING_KEY_ID: keyId, + ARTIFACT_SIGNING_KEY_NOT_BEFORE: notBefore, + ARTIFACT_SIGNING_KEY_NOT_AFTER: notAfter, + ARTIFACT_SIGNING_PRIVATE_KEY_PKCS8: privateKey, + }, + } +} + +/** Bytes that gzip quickly and that no text check reads as a key. */ +function patterned(length: number): Uint8Array { + const bytes = new Uint8Array(length) + for (let index = 0; index < length; index += 4096) + bytes[index] = (index / 4096) % 251 + return bytes +} + +function flipFirstByte(bytes: Uint8Array): Uint8Array { + const changed = Uint8Array.from(bytes) + changed[0] = changed[0]! ^ 0xFF + return changed +} + +function gitBlobSha(bytes: Uint8Array): string { + return createHash('sha1').update(`blob ${bytes.byteLength}\0`).update(bytes).digest('hex') +} + +function sha256(bytes: Uint8Array): string { + return createHash('sha256').update(bytes).digest('hex') +} + +function concat(parts: Uint8Array[]): Uint8Array { + const merged = new Uint8Array(parts.reduce((total, part) => total + part.byteLength, 0)) + let offset = 0 + for (const part of parts) { + merged.set(part, offset) + offset += part.byteLength + } + return merged +} diff --git a/test/unit/artifact-pack.test.ts b/test/unit/artifact-pack.test.ts new file mode 100644 index 00000000..bdb6dda7 --- /dev/null +++ b/test/unit/artifact-pack.test.ts @@ -0,0 +1,231 @@ +import type { ResolvedSource } from '../../layers/artifact-delivery/server/schemas/contracts' +import type { FileSink, PackedFile, SkillFileReader } from '../../layers/artifact-delivery/server/utils/artifact-pack' +import { createHash } from 'node:crypto' +import { describe, expect, it } from 'vitest' +import { packArtifactFiles, scanArtifact } from '../../layers/artifact-delivery/server/utils/artifact-pack' +import { checkArtifactSource } from '../../layers/artifact-delivery/server/utils/checks' +import { compareArtifactPaths, createDeterministicUstar } from '../../layers/artifact-delivery/server/utils/ustar' + +const encoder = new TextEncoder() +const skillText = '---\nname: demo\ndescription: Use this Skill for demo work.\n---\n' + +const source: ResolvedSource = { + provider: 'github', + repositoryId: 1, + owner: 'skilld-dev', + repository: 'skills', + visibility: 'public', + commitSha: '0123456789abcdef0123456789abcdef01234567', + treeSha: '89abcdef0123456789abcdef0123456789abcdef', + skillPath: 'skills/demo', +} + +describe('artifact path order', () => { + it('orders paths by their UTF-8 bytes, as a Git archive lists them', () => { + // UTF-16 code units put U+1F600 (a surrogate pair, 0xD83D) before U+FF21 + // (0xFF21). UTF-8 bytes put it after: F0 9F 98 80 against EF BC A1. + const paths = ['Ａ.md', '\u{1F600}.md', 'a-b/x.md', 'a/y.md', 'a.txt'] + + expect([...paths].sort(compareArtifactPaths)).toEqual(['a-b/x.md', 'a.txt', 'a/y.md', 'Ａ.md', '\u{1F600}.md']) + }) +}) + +describe('streamed Artifact packing', () => { + it('writes the same bytes as the in-memory packer, whatever the chunk size', async () => { + const files = skillFiles({ + 'SKILL.md': skillText, + 'assets/music/track.mp3': bytesOfLength(300 * 1024 + 7), + 'references/guide.md': 'Read this first.\n', + 'run.sh': '#!/usr/bin/env bash\necho demo\n', + }) + + for (const chunkSize of [7, 511, 512, 64 * 1024]) { + const written: Uint8Array[] = [] + const outcome = await packArtifactFiles({ + files: files.map(file => file.packed), + read: memoryReader(files, chunkSize), + readFromGithub: new Set(), + write: async (bytes) => { + written.push(bytes.slice()) + }, + }) + + expect(outcome).toEqual({ _tag: 'read' }) + expect(concat(written)).toEqual(createDeterministicUstar(files.map(file => file.source))) + } + }) + + it('reports a file whose bytes do not match its Git blob digest', async () => { + const files = skillFiles({ 'SKILL.md': skillText, 'references/guide.md': 'Read this first.\n' }) + const tampered = files.map(file => file.packed.path === 'references/guide.md' + ? { ...file, bytes: encoder.encode('Read this last.\n') } + : file) + + const outcome = await packArtifactFiles({ + files: files.map(file => file.packed), + read: memoryReader(tampered, 4), + readFromGithub: new Set(), + write: async () => {}, + }) + + expect(outcome).toMatchObject({ _tag: 'mismatch', path: 'references/guide.md' }) + }) + + it('reports a file that arrives shorter than its tree entry', async () => { + const files = skillFiles({ 'SKILL.md': skillText, 'references/guide.md': 'Read this first.\n' }) + const short = files.map(file => file.packed.path === 'references/guide.md' + ? { ...file, bytes: file.bytes.subarray(0, 4) } + : file) + + const outcome = await packArtifactFiles({ + files: files.map(file => file.packed), + read: memoryReader(short, 64), + readFromGithub: new Set(), + write: async () => {}, + }) + + expect(outcome).toMatchObject({ _tag: 'mismatch', path: 'references/guide.md' }) + }) +}) + +describe('artifact scan', () => { + it('names the content digest and size of the archive it would store, and keeps a small one', async () => { + const files = skillFiles({ 'SKILL.md': skillText, 'references/guide.md': 'Read this first.\n' }) + const archive = createDeterministicUstar(files.map(file => file.source)) + + const scanned = await scanArtifact({ + source, + files: files.map(file => file.packed), + read: memoryReader(files, 3), + omitted: [], + spoolBytes: 1024 * 1024, + }) + + expect(scanned).toMatchObject({ + _tag: 'scanned', + contentSha256: sha256(archive), + contentBytes: archive.byteLength, + }) + if (scanned._tag !== 'scanned') + return + expect(scanned.spool).toEqual(archive) + expect(scanned.checked).toEqual(await checkArtifactSource(source, files.map(file => file.source))) + }) + + it('keeps no copy of an archive over the spool size', async () => { + const files = skillFiles({ 'SKILL.md': skillText, 'assets/clip.mp4': bytesOfLength(4096) }) + + const scanned = await scanArtifact({ + source, + files: files.map(file => file.packed), + read: memoryReader(files, 1000), + omitted: [], + spoolBytes: 2048, + }) + + expect(scanned).toMatchObject({ _tag: 'scanned', spool: null }) + }) + + it('reads a file from GitHub again when the archive bytes fail their digest', async () => { + const files = skillFiles({ 'SKILL.md': skillText, 'references/guide.md': 'Read this first.\n' }) + const asked: Array> = [] + // An `export-subst` file keeps its length and changes its bytes in the + // archive. Only a read from GitHub returns the blob. + const read: SkillFileReader = async (sink, readFromGithub) => { + asked.push(new Set(readFromGithub)) + const served = files.map(file => file.packed.path === 'references/guide.md' && !readFromGithub.has(file.packed.path) + ? { ...file, bytes: encoder.encode('Read this frst!\n') } + : file) + return await memoryReader(served, 5)(sink, readFromGithub) + } + + const scanned = await scanArtifact({ source, files: files.map(file => file.packed), read, omitted: [], spoolBytes: 1024 * 1024 }) + + expect(scanned).toMatchObject({ _tag: 'scanned', readFromGithub: new Set(['references/guide.md']) }) + expect(asked.map(paths => [...paths])).toEqual([[], ['references/guide.md']]) + }) + + it('refuses a file whose GitHub bytes also fail their digest', async () => { + const files = skillFiles({ 'SKILL.md': skillText, 'references/guide.md': 'Read this first.\n' }) + const tampered = files.map(file => file.packed.path === 'references/guide.md' + ? { ...file, bytes: encoder.encode('Read this frst!\n') } + : file) + + const scanned = await scanArtifact({ + source, + files: files.map(file => file.packed), + read: memoryReader(tampered, 5), + omitted: [], + spoolBytes: 1024 * 1024, + }) + + expect(scanned).toMatchObject({ + _tag: 'rejected', + code: 'INVALID_SOURCE', + summary: 'A Git blob failed its Git digest check.', + findings: ['references/guide.md'], + }) + }) +}) + +interface TestFile { + packed: PackedFile + source: { path: string, mode: 420 | 493, bytes: Uint8Array, gitBlobSha: string } + bytes: Uint8Array +} + +function skillFiles(contents: Record): TestFile[] { + return Object.entries(contents) + .map(([path, content]) => { + const bytes = typeof content === 'string' ? encoder.encode(content) : content + const mode = path.endsWith('.sh') ? 493 as const : 420 as const + const gitBlobSha = gitBlobSha1(bytes) + return { + packed: { path, mode, size: bytes.byteLength, gitBlobSha }, + source: { path, mode, bytes, gitBlobSha }, + bytes, + } + }) + .sort((left, right) => compareArtifactPaths(left.packed.path, right.packed.path)) +} + +/** Serves each file in Artifact order, in chunks of one size. */ +function memoryReader(files: Array<{ packed: PackedFile, bytes: Uint8Array }>, chunkSize: number): SkillFileReader { + return async (sink: FileSink, readFromGithub) => { + for (const file of files) { + await sink.begin(file.packed, readFromGithub.has(file.packed.path) ? 'github' : 'archive') + for (let offset = 0; offset < file.bytes.byteLength; offset += chunkSize) + await sink.chunk(file.bytes.subarray(offset, offset + chunkSize)) + const ended = await sink.end() + if (ended._tag === 'mismatch') + return ended + } + return { _tag: 'read' } + } +} + +function bytesOfLength(length: number): Uint8Array { + const bytes = new Uint8Array(length) + for (let index = 0; index < length; index++) + bytes[index] = (index * 31 + 7) % 251 + return bytes +} + +function gitBlobSha1(bytes: Uint8Array): string { + return createHash('sha1').update(`blob ${bytes.byteLength}\0`).update(bytes).digest('hex') +} + +function sha256(bytes: Uint8Array): string { + return createHash('sha256').update(bytes).digest('hex') +} + +function concat(parts: Uint8Array[]): Uint8Array { + const total = parts.reduce((sum, part) => sum + part.byteLength, 0) + const merged = new Uint8Array(total) + let offset = 0 + for (const part of parts) { + merged.set(part, offset) + offset += part.byteLength + } + return merged +} diff --git a/test/unit/artifact-rate-limit.test.ts b/test/unit/artifact-rate-limit.test.ts index 873a817e..4ad62d84 100644 --- a/test/unit/artifact-rate-limit.test.ts +++ b/test/unit/artifact-rate-limit.test.ts @@ -11,6 +11,7 @@ import { } from '../../layers/artifact-delivery/server/utils/github-source' import { ARTIFACT_BUILD_QUEUE_NAME, consumeArtifactBuildBatch } from '../../layers/artifact-delivery/server/utils/queue' import { createResolution, getResolution, presentResolution, resolutionRequestIdentity } from '../../layers/artifact-delivery/server/utils/state' +import { readLoadedFiles } from '../fixtures/loaded-source' import { createSqliteD1 } from './helpers/d1-sqlite' const commitSha = '0123456789abcdef0123456789abcdef01234567' @@ -23,6 +24,7 @@ const ARTIFACT_MIGRATIONS = [ 'migrations/0112_private_artifact_keys.sql', 'migrations/0122_artifact_resolution_retry_after.sql', 'migrations/0144_artifact_resolution_requesters.sql', + 'migrations/0145_artifact_resolution_linked_files.sql', ] const resolutionRequest: SourceRequest = { provider: 'github', @@ -51,12 +53,13 @@ describe('gitHub rate limits as values', () => { }) }) - it('rejects a blob read the same way, part way through a load', async () => { + it('rejects a GitHub file read the same way, part way through a build', async () => { const client = createPublicGithubSourceClient({ + now: () => NOW, fetch: vi.fn(async (input: RequestInfo | URL) => { const url = String(input) - if (url.includes('/git/blobs/')) - return rateLimited() + if (url.startsWith('https://raw.githubusercontent.com/')) + return new Response(null, { status: 429, headers: { 'retry-after': '120' } }) if (url.endsWith('/repos/skilld-dev/skills')) return json({ id: 123, name: 'skills', owner: { login: 'skilld-dev' }, private: false, default_branch: 'main' }) if (url.endsWith('/git/trees/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa')) @@ -70,11 +73,12 @@ describe('gitHub rate limits as values', () => { tree: [{ path: 'SKILL.md', mode: '100644', type: 'blob', sha: 'd'.repeat(40), size: 12 }], }) } + // codeload answers 404, so the build reads the file from GitHub. return json({}, 404) }) as unknown as typeof fetch, }) - const result = await client.load({ + const loaded = await client.load({ provider: 'github', repositoryId: 123, owner: 'skilld-dev', @@ -83,9 +87,11 @@ describe('gitHub rate limits as values', () => { commitSha, treeSha: 'a'.repeat(40), skillPath: 'skills/demo', - }) + }, { linkedFiles: false }) + if (loaded._tag !== 'loaded') + throw new Error('The load needs no file bytes') - expect(result).toMatchObject({ _tag: 'rejected', code: 'RATE_LIMITED', retryAfterSeconds: RESET_AT }) + expect(await readLoadedFiles(loaded.value)).toMatchObject({ _tag: 'rejected', code: 'RATE_LIMITED', retryAfterSeconds: NOW + 120 }) }) it('omits the reset when GitHub sends no usable header', async () => { @@ -170,12 +176,12 @@ describe('gitHub secondary rate limits as values', () => { }) }) - it('stops at a rate-limited tarball instead of reading every blob', async () => { + it('stops at a rate-limited archive instead of reading every file', async () => { const fetchMock = vi.fn(async (input: RequestInfo | URL) => { const url = String(input) - if (url.includes('/tarball/')) { + if (url.includes('codeload.github.com')) { return new Response('{}', { - status: 403, + status: 429, headers: { 'retry-after': '60', 'content-type': 'application/json' }, }) } @@ -201,7 +207,7 @@ describe('gitHub secondary rate limits as values', () => { now: () => NOW, }) - const result = await client.load({ + const loaded = await client.load({ provider: 'github', repositoryId: 123, owner: 'skilld-dev', @@ -210,10 +216,13 @@ describe('gitHub secondary rate limits as values', () => { commitSha, treeSha: 'a'.repeat(40), skillPath: 'skills/demo', - }) + }, { linkedFiles: false }) + + if (loaded._tag !== 'loaded') + throw new Error('The load needs no file bytes') - expect(result).toMatchObject({ _tag: 'rejected', code: 'RATE_LIMITED', retryAfterSeconds: NOW + 60 }) - expect(fetchMock.mock.calls.some(([input]) => String(input).includes('/git/blobs/'))).toBe(false) + expect(await readLoadedFiles(loaded.value)).toMatchObject({ _tag: 'rejected', code: 'RATE_LIMITED', retryAfterSeconds: NOW + 60 }) + expect(fetchMock.mock.calls.some(([input]) => String(input).includes('raw.githubusercontent.com'))).toBe(false) }) }) diff --git a/test/unit/artifact-requester-load-options.test.ts b/test/unit/artifact-requester-load-options.test.ts new file mode 100644 index 00000000..90f7581c --- /dev/null +++ b/test/unit/artifact-requester-load-options.test.ts @@ -0,0 +1,28 @@ +import type { ResolvedSource } from '../../layers/artifact-delivery/server/schemas/contracts' +import type { PublicGithubSourceClient } from '../../layers/artifact-delivery/server/utils/github-source' +import { describe, expect, it, vi } from 'vitest' +import { withRequesterFallback } from '../../layers/artifact-delivery/server/utils/requester-github' + +const source: ResolvedSource = { + provider: 'github', + repositoryId: 1, + owner: 'thvroyal', + repository: 'kimi-skills', + visibility: 'public', + commitSha: '0'.repeat(40), + treeSha: '1'.repeat(40), + skillPath: 'skills/kimi-xlsx', +} + +describe('the requester fallback', () => { + it('passes the linked files choice to both GitHub clients', async () => { + const limited = { _tag: 'rejected' as const, code: 'RATE_LIMITED' as const, summary: 'spent', findings: [] } + const shared = { resolve: vi.fn(), load: vi.fn(async () => limited) } as unknown as PublicGithubSourceClient + const own = { resolve: vi.fn(), load: vi.fn(async () => limited) } as unknown as PublicGithubSourceClient + + await withRequesterFallback(shared, async () => own).load(source, { linkedFiles: true }) + + expect(shared.load).toHaveBeenCalledWith(source, { linkedFiles: true }) + expect(own.load).toHaveBeenCalledWith(source, { linkedFiles: true }) + }) +}) diff --git a/test/unit/artifact-resolution-latency.test.ts b/test/unit/artifact-resolution-latency.test.ts index 41b26aca..05c5b8f3 100644 --- a/test/unit/artifact-resolution-latency.test.ts +++ b/test/unit/artifact-resolution-latency.test.ts @@ -32,6 +32,7 @@ const MIGRATIONS = [ 'migrations/0110_artifact_delivery.sql', 'migrations/0111_github_app_delivery.sql', 'migrations/0122_artifact_resolution_retry_after.sql', + 'migrations/0145_artifact_resolution_linked_files.sql', ] async function requestedRow(): Promise { diff --git a/test/unit/artifact-signer.test.ts b/test/unit/artifact-signer.test.ts index 8a07d2da..a38255f2 100644 --- a/test/unit/artifact-signer.test.ts +++ b/test/unit/artifact-signer.test.ts @@ -173,9 +173,17 @@ describe('artifact signing Worker', () => { fixture.close() }) - it('rejects current check results under the previous policy', async () => { - const [policyVersion] = [...SIGNABLE_ARTIFACT_POLICIES].find(([version]) => version !== ARTIFACT_POLICY_VERSION)! - const fixture = await createSignerFixture({ policyVersion }) + it('rejects check results the previous policy does not carry', async () => { + const [policyVersion, checkSet] = [...SIGNABLE_ARTIFACT_POLICIES].find(([version]) => version !== ARTIFACT_POLICY_VERSION)! + const fixture = await createSignerFixture({ + policyVersion, + checks: [...checkSet].map(([name, check]) => ({ + name, + version: name === 'credential-material' ? 'an-older-version' : check.version, + outcome: 'pass' as const, + required: check.required, + })), + }) const response = await handleArtifactSignerRequest(attestRequest(fixture.artifactId), fixture.bindings, () => NOW) @@ -376,6 +384,7 @@ async function createSignerFixture(options: SignerFixtureOptions = {}) { : { contentSha256: metadataDigest, format: 'skilld-tar-v1' }), }, arrayBuffer: async () => Uint8Array.from(body).buffer, + body: new Blob([Uint8Array.from(body)]).stream(), } as R2ObjectBody }) const bindings = { diff --git a/test/unit/artifact-source-checks.test.ts b/test/unit/artifact-source-checks.test.ts index 707e3787..4eb834b3 100644 --- a/test/unit/artifact-source-checks.test.ts +++ b/test/unit/artifact-source-checks.test.ts @@ -2,7 +2,8 @@ import type { ResolvedSource } from '../../layers/artifact-delivery/server/schem import type { ArtifactSourceFile } from '../../layers/artifact-delivery/server/utils/github-source' import { generateKeyPairSync } from 'node:crypto' import { describe, expect, it } from 'vitest' -import { checkArtifactSource, checksBlockArtifact } from '../../layers/artifact-delivery/server/utils/checks' +import { checkResultSchema } from '../../layers/artifact-delivery/server/schemas/contracts' +import { checkArtifactSource, checksBlockArtifact, createArtifactCheckScanner } from '../../layers/artifact-delivery/server/utils/checks' // Keys are generated per run, so the repository never holds key material. const pkcs8 = generateKeyPairSync('rsa', { modulusLength: 2048 }).privateKey.export({ type: 'pkcs8', format: 'pem' }) as string @@ -56,6 +57,43 @@ describe('credential material check', () => { }) }) +describe('credential material check over streamed bytes', () => { + it.each([1, 7, 1000, 65_536])('finds a key split across %i byte chunks', async (chunkSize) => { + const text = `${'x'.repeat(40_000)}\n${pkcs8}\n${'y'.repeat(40_000)}` + + expect(credentialResult(streamedCheck(file('config/credentials.txt', text), chunkSize))) + .toEqual(credentialResult(await check(file('config/credentials.txt', text)))) + expect(credentialResult(streamedCheck(file('config/credentials.txt', text), chunkSize))).toMatchObject({ outcome: 'fail' }) + }) + + it('reads a file with invalid UTF-8 after a key as binary, as the whole-file check does', async () => { + const bytes = new Uint8Array([...new TextEncoder().encode(pkcs8), 0xFF, 0xFE]) + const binary: ArtifactSourceFile = { path: 'assets/blob.bin', mode: 420, bytes, gitBlobSha: 'a'.repeat(40) } + + expect(credentialResult(streamedCheck(binary, 512))).toMatchObject({ outcome: 'pass' }) + expect(credentialResult(await check(binary))).toMatchObject({ outcome: 'pass' }) + }) +}) + +describe('check results for a Skill of 2,000 files', () => { + it('keeps every result inside the limits the skilld CLI verifies', async () => { + // The old 900 file limit hid this: the schema allows 100 findings of 500 characters. + const files = [ + file('SKILL.md', '---\nname: demo\ndescription: Demo.\n---\n'), + ...Array.from({ length: 150 }, (_, index): ArtifactSourceFile => ({ ...file(`scripts/${'s'.repeat(600)}-${index}.sh`, 'echo\n'), mode: 493 })), + ] + + const checked = await checkArtifactSource(source(), files) + + for (const result of checked.checkResults) + expect(checkResultSchema.safeParse(result).success).toBe(true) + expect(checked.checkResults.find(result => result.name === 'executable-files')).toMatchObject({ + outcome: 'warn', + summary: 'The Skill contains executable files. The first 100 of 150 are listed.', + }) + }) +}) + describe('omitted files check', () => { it('passes when the Artifact holds every file', async () => { const checked = (await checkArtifactSource(source(), [file('SKILL.md', '---\nname: demo\n---\n')], [])).checkResults @@ -104,6 +142,17 @@ async function check(extra: ArtifactSourceFile) { return (await checkArtifactSource(source(), files)).checkResults } +function streamedCheck(extra: ArtifactSourceFile, chunkSize: number) { + const scanner = createArtifactCheckScanner(source()) + for (const entry of [file('SKILL.md', '---\nname: demo\ndescription: Demo.\n---\n'), extra]) { + scanner.begin({ path: entry.path, mode: entry.mode, size: entry.bytes.byteLength }) + for (let offset = 0; offset < entry.bytes.byteLength; offset += chunkSize) + scanner.chunk(entry.bytes.subarray(offset, offset + chunkSize)) + scanner.end() + } + return scanner.finish([]).checkResults +} + function credentialResult(results: Awaited>) { return results.find(result => result.name === 'credential-material') } diff --git a/test/unit/artifact-source-limits.test.ts b/test/unit/artifact-source-limits.test.ts index 5804d9d9..a6c90848 100644 --- a/test/unit/artifact-source-limits.test.ts +++ b/test/unit/artifact-source-limits.test.ts @@ -1,201 +1,276 @@ import type { ArtifactSourceFile } from '../../layers/artifact-delivery/server/utils/github-source' import { createHash } from 'node:crypto' import { describe, expect, it, vi } from 'vitest' -import { createPublicGithubSourceClient } from '../../layers/artifact-delivery/server/utils/github-source' +import { completeAttestation, createAttestationStatement, encodeAttestationStatement } from '../../layers/artifact-delivery/server/utils/attestation' +import { + createPublicGithubSourceClient, + MAX_LINKED_BYTES, + PRIVATE_ARTIFACT_LIMITS, + PUBLIC_ARTIFACT_LIMITS, + selectArtifactEntries, +} from '../../layers/artifact-delivery/server/utils/github-source' import { createDeterministicUstar, projectedUstarBytes } from '../../layers/artifact-delivery/server/utils/ustar' +const MIB = 1024 * 1024 const commitSha = '0123456789abcdef0123456789abcdef01234567' const rootTreeSha = '89abcdef0123456789abcdef0123456789abcdef' const skillsTreeSha = '1111111111111111111111111111111111111111' const skillTreeSha = '2222222222222222222222222222222222222222' const skillText = '---\nname: demo\ndescription: Use this Skill for demo work.\n---\n' const skillBlobSha = gitBlobSha(skillText) +const linking = { ...PUBLIC_ARTIFACT_LIMITS, maxLinkedBytes: MAX_LINKED_BYTES } -describe('artifact source size guards', () => { - it('accepts a Skill with more files than the old 256 ceiling', async () => { - const client = createPublicGithubSourceClient({ - fetch: skillTreeFetch(skillEntries(300)) as unknown as typeof fetch, - }) - - const loaded = await client.load(resolvedSource()) +describe('public Artifact limits', () => { + it('packs music over the old 2 MiB file limit, as latent-spaces/brag needs for its films', async () => { + const loaded = await load([ + ['assets/music/track-1.mp3', 2_107_000], + ['assets/music/track-2.mp3', 3_936_384], + ['scripts/render.ts', 4_000], + ]) - expect(loaded._tag).toBe('loaded') + expect(loaded).toMatchObject({ _tag: 'loaded', value: { omitted: [], linked: [] } }) if (loaded._tag !== 'loaded') return - expect(loaded.value.files).toHaveLength(300) + expect(loaded.value.files.map(file => file.path)).toEqual([ + 'SKILL.md', + 'assets/music/track-1.mp3', + 'assets/music/track-2.mp3', + 'scripts/render.ts', + ]) }) - it('rejects a Skill past the file ceiling by name', async () => { - const client = createPublicGithubSourceClient({ - fetch: skillTreeFetch(skillEntries(901)) as unknown as typeof fetch, - }) + it('packs a Skill that fills the 64 MiB archive', () => { + // 63 files of 1 MiB and SKILL.md pack to 63 MiB and 33 KiB. + const selected = selectArtifactEntries(skillWith(Array.from({ length: 63 }, (_, index) => [`assets/clip-${index}.mp4`, MIB])), 'skills/demo', PUBLIC_ARTIFACT_LIMITS) - const loaded = await client.load(resolvedSource()) + expect(selected).toMatchObject({ _tag: 'selected', omitted: [], linked: [] }) + }) - expect(loaded).toMatchObject({ - _tag: 'rejected', - code: 'INVALID_SOURCE', - summary: 'The Skill folder `skills/demo` has 901 files. The limit is 900.', - }) + it('packs 2,000 files, over the old 900', () => { + const selected = selectArtifactEntries(skillWith(Array.from({ length: 1999 }, (_, index) => [`references/entry-${index}.md`, 100])), 'skills/demo', PUBLIC_ARTIFACT_LIMITS) + + expect(selected).toMatchObject({ _tag: 'selected', omitted: [] }) + if (selected._tag === 'selected') + expect(selected.entries).toHaveLength(2000) }) - it('rejects a Skill whose packaged archive would exceed the ceiling, though its files do not', async () => { - // 900 files of 11,000 bytes is 9,900,000 source bytes, inside the 10 MiB - // ceiling. Each file costs a 512-byte header plus padding to the next - // 512-byte block, so the archive lands at 10,599,424 bytes, outside it. - const entries = Array.from({ length: 900 }, (_, index) => - blob(index === 0 ? 'SKILL.md' : `references/entry-${index}.md`, skillBlobSha, 11_000)) - const client = createPublicGithubSourceClient({ - fetch: skillTreeFetch(entries) as unknown as typeof fetch, - }) + it('leaves files the Skill does not read out, largest first, until 2,000 files remain', () => { + const selected = selectArtifactEntries(skillWith([ + ...Array.from({ length: 1990 }, (_, index) => [`references/entry-${index}.md`, 100] as const), + ...Array.from({ length: 20 }, (_, index) => [`assets/frame-${index}.png`, 1_000 + index] as const), + ]), 'skills/demo', PUBLIC_ARTIFACT_LIMITS) + + expect(selected._tag).toBe('selected') + if (selected._tag !== 'selected') + return + expect(selected.entries).toHaveLength(2000) + expect(selected.omitted.map(entry => entry.path).sort()).toEqual( + Array.from({ length: 11 }, (_, index) => `assets/frame-${19 - index}.png`).sort(), + ) + }) - const loaded = await client.load(resolvedSource()) + it('refuses more files the Skill reads than the CLI accepts', () => { + const selected = selectArtifactEntries(skillWith(Array.from({ length: 2000 }, (_, index) => [`references/entry-${index}.md`, 100])), 'skills/demo', PUBLIC_ARTIFACT_LIMITS) - expect(loaded).toMatchObject({ + expect(selected).toMatchObject({ _tag: 'rejected', code: 'INVALID_SOURCE', - summary: 'The files the Skill reads in the Skill folder `skills/demo` pack to 10.11 MiB. The limit is 10 MiB.', + summary: 'The Skill folder `skills/demo` has 2,001 files the Skill reads. The limit is 2,000.', }) }) - it('names the text file over the one-file limit and its size', async () => { - const entries = [ - blob('SKILL.md', skillBlobSha, skillText.length), - blob('references/api.md', skillBlobSha, 2_153_066), - ] - const client = createPublicGithubSourceClient({ - fetch: skillTreeFetch(entries) as unknown as typeof fetch, + it('leaves the largest media out until the archive fits 64 MiB, for a CLI without linked files', () => { + // thvroyal/kimi-skills/kimi-xlsx: one 73 MiB binary beside its scripts. + const selected = selectArtifactEntries(skillWith([ + ['scripts/KimiXlsx', 77_001_601], + ['scripts/run.py', 4_000], + ]), 'skills/demo', PUBLIC_ARTIFACT_LIMITS) + + expect(selected).toMatchObject({ + _tag: 'selected', + omitted: [{ path: 'scripts/KimiXlsx', size: 77_001_601 }], + linked: [], }) + }) - const loaded = await client.load(resolvedSource()) + it('refuses text the Skill reads that packs over 64 MiB', () => { + const selected = selectArtifactEntries(skillWith(Array.from({ length: 5 }, (_, index) => [`references/part-${index}.md`, 14 * MIB])), 'skills/demo', PUBLIC_ARTIFACT_LIMITS) - expect(loaded).toMatchObject({ + expect(selected).toMatchObject({ _tag: 'rejected', code: 'INVALID_SOURCE', - summary: 'The file `references/api.md` is 2.06 MiB. The limit for one file is 2 MiB.', - findings: ['references/api.md: 2,153,066 bytes'], + summary: 'The files the Skill reads in the Skill folder `skills/demo` pack to 70.01 MiB. The limit is 64 MiB.', }) }) - it('says that a root Skill counts every file in the Repository', async () => { - const entries = [ - blob('SKILL.md', skillBlobSha, skillText.length), - blob('docs/data/catalog.json', skillBlobSha, 2_479_001), - ] - const client = createPublicGithubSourceClient({ - fetch: rootTreeFetch(entries) as unknown as typeof fetch, - }) + it('refuses a file list too long to sign in one D1 row', () => { + // Each path is 238 bytes, the most a USTAR header holds is 256. + const folder = `references/${'d'.repeat(130)}` + const selected = selectArtifactEntries(skillWith(Array.from({ length: 1500 }, (_, index) => [`${folder}/${'n'.repeat(85)}-${String(index).padStart(4, '0')}.md`, 10])), 'skills/demo', PUBLIC_ARTIFACT_LIMITS) + + expect(selected).toMatchObject({ _tag: 'rejected', code: 'INVALID_SOURCE' }) + if (selected._tag === 'rejected') + expect(selected.summary).toMatch(/too long to sign/) + }) - const loaded = await client.load({ ...resolvedSource(), skillPath: '.' }) + it('refuses a path the Artifact format cannot hold before it reads a byte', () => { + // cat-xierluo/legal-skills: a file name of 106 UTF-8 bytes. + const path = `references/case-types/61-${'暂时解除乘坐飞机、高铁限制措施申请'.repeat(2)}书.md` + const selected = selectArtifactEntries(skillWith([[path, 1_000]]), 'skills/demo', PUBLIC_ARTIFACT_LIMITS) - expect(loaded).toMatchObject({ + expect(selected).toMatchObject({ _tag: 'rejected', code: 'INVALID_SOURCE', - summary: 'The file `docs/data/catalog.json` is 2.37 MiB. The limit for one file is 2 MiB. The Skill folder is the Repository root, so every file in the Repository counts.', + summary: 'A Skill path cannot be represented by the Artifact format.', + findings: [path], }) }) -}) -describe('a Skill with media over the size limits', () => { - it('leaves each media file over the one-file limit out and delivers the rest', async () => { - // latent-spaces/brag/brag: its films use four music files of 2.01 to 3.76 MiB. - const served = servedSkill([ - ['assets/music/track-1.mp3', 2_107_000], - ['assets/music/track-2.mp3', 3_936_384], - ['scripts/render.ts', 4_000], - ['scripts/KimiXlsx', 2_200_000], - ]) + it('leaves media out of a root Skill, so the README images never block it', async () => { + const loaded = await load([['docs/assets/interactive-motion.gif', 70 * MIB]], 'root') - const loaded = await createPublicGithubSourceClient({ fetch: served as unknown as typeof fetch }).load(resolvedSource()) + expect(loaded).toMatchObject({ + _tag: 'loaded', + value: { + omitted: [{ + path: 'docs/assets/interactive-motion.gif', + bytes: 70 * MIB, + url: `https://github.com/skilld-dev/skills/blob/${commitSha}/docs/assets/interactive-motion.gif`, + }], + }, + }) + }) +}) - expect(loaded._tag).toBe('loaded') - if (loaded._tag !== 'loaded') +describe('the D1 row a signed Skill fills', () => { + it('stays under 2,000,000 bytes for the longest file list a build accepts', async () => { + // 2,000 files whose paths fill the 448 KiB file list, and 100 KB of check results. + const pathBytes = Math.floor(448 * 1024 / 2000) - 126 - 1 + const entries = skillWith(Array.from({ length: 1999 }, (_, index) => [`r/${String(index).padStart(4, '0')}-${'p'.repeat(pathBytes - 7)}`, 999_999])) + const selected = selectArtifactEntries(entries, 'skills/demo', PUBLIC_ARTIFACT_LIMITS) + expect(selected._tag).toBe('selected') + if (selected._tag !== 'selected') return - expect(loaded.value.files.map(file => file.path)).toEqual(['SKILL.md', 'scripts/render.ts']) - expect(loaded.value.omitted).toEqual([ - omission('assets/music/track-1.mp3', 2_107_000), - omission('assets/music/track-2.mp3', 3_936_384), - omission('scripts/KimiXlsx', 2_200_000), - ]) + const checkResults = Array.from({ length: 5 }, (_, index) => ({ + name: `check-${index}`, + version: '1', + outcome: 'warn' as const, + required: false, + summary: 's'.repeat(400), + findings: Array.from({ length: 40 }, () => 'f'.repeat(500)), + })) + const statement = encodeAttestationStatement(createAttestationStatement({ + artifactId: `sha256:${'a'.repeat(64)}`, + createdAt: new Date(0).toISOString(), + source: resolvedSource(), + contentSha256: 'a'.repeat(64), + contentBytes: 1, + files: selected.entries.map(entry => ({ path: entry.path, mode: 420, size: entry.size, sha256: 'b'.repeat(64) })), + checkResults, + })) + const attestation = JSON.stringify(completeAttestation(statement, { algorithm: 'Ed25519', keyId: 'skilld-production-2026-08', value: 'C'.repeat(86) })) + const row = [JSON.stringify(checkResults), statement, attestation].reduce((total, column) => total + Buffer.byteLength(column), 0) + + expect(Buffer.byteLength(JSON.stringify(checkResults))).toBeGreaterThan(100_000) + expect(row).toBeLessThan(2_000_000) }) +}) - it('leaves the largest media out until the folder fits', async () => { - // tt-a1i/archify packs to 10.36 MiB with its example renders. - const served = servedSkill([ - ['references/guide.md', 1_500_000], - ...Array.from({ length: 6 }, (_, index) => [`examples/render-${index}.png`, 1_600_000 + index * 1_000] as const), - ]) +describe('linked files', () => { + it('links the largest files instead of leaving them out, for a CLI that reads them', async () => { + const loaded = await load([ + ['scripts/KimiXlsx', 77_001_601], + ['scripts/run.py', 4_000], + ], 'skills/demo', true) - const loaded = await createPublicGithubSourceClient({ fetch: served as unknown as typeof fetch }).load(resolvedSource()) + expect(loaded).toMatchObject({ + _tag: 'loaded', + value: { + omitted: [], + linked: [{ path: 'scripts/KimiXlsx', mode: 420, size: 77_001_601 }], + }, + }) + if (loaded._tag === 'loaded') + expect(loaded.value.files.map(file => file.path)).toEqual(['SKILL.md', 'scripts/run.py']) + }) - expect(loaded._tag).toBe('loaded') - if (loaded._tag !== 'loaded') - return - expect(loaded.value.omitted).toEqual([omission('examples/render-5.png', 1_605_000)]) - expect(loaded.value.files.map(file => file.path)).toContain('references/guide.md') - expect(loaded.value.files).toHaveLength(7) + it('links text the Skill reads when that is what keeps the archive over 64 MiB', () => { + const selected = selectArtifactEntries(skillWith(Array.from({ length: 5 }, (_, index) => [`references/part-${index}.md`, 14 * MIB])), 'skills/demo', linking) + + expect(selected._tag).toBe('selected') + // Equal sizes leave in path order, so one commit always links the same file. + if (selected._tag === 'selected') + expect(selected.linked.map(entry => entry.path)).toEqual(['references/part-0.md']) }) - it('leaves example and test files out before it refuses the text a Skill reads', async () => { - // tt-a1i/archify packs to 10.36 MiB, all text: five rendered examples of - // about 760 KB each and its test suite sit beside the scripts it runs. - const served = servedSkill([ - ['assets/template.html', 727_976], - ...Array.from({ length: 5 }, (_, index) => [`scripts/module-${index}.mjs`, 1_200_000] as const), - ...Array.from({ length: 5 }, (_, index) => [`examples/render-${index}.html`, 760_000 + index] as const), - ['test/cli.test.mjs', 201_323], - ]) + it('never links SKILL.md', () => { + const selected = selectArtifactEntries([blob('SKILL.md', skillBlobSha, 70 * MIB)], 'skills/demo', linking) - const loaded = await createPublicGithubSourceClient({ fetch: served as unknown as typeof fetch }).load(resolvedSource()) + expect(selected).toMatchObject({ _tag: 'rejected', code: 'INVALID_SOURCE' }) + }) - expect(loaded._tag).toBe('loaded') - if (loaded._tag !== 'loaded') + it('leaves media out once the linked bytes reach their limit', () => { + const selected = selectArtifactEntries(skillWith([ + ...Array.from({ length: 3 }, (_, index) => [`assets/film-${index}.mp4`, 95 * MIB] as const), + ['assets/poster.png', 30 * MIB], + ]), 'skills/demo', linking) + + expect(selected._tag).toBe('selected') + if (selected._tag !== 'selected') return - expect(loaded.value.omitted.map(file => file.path)).toEqual(['examples/render-4.html']) - expect(loaded.value.files.map(file => file.path)).toContain('assets/template.html') + expect(selected.linked.map(entry => entry.path)).toEqual(['assets/film-0.mp4', 'assets/film-1.mp4']) + expect(selected.omitted.map(entry => entry.path)).toEqual(['assets/film-2.mp4']) + expect(selected.entries.map(entry => entry.path)).toEqual(['SKILL.md', 'assets/poster.png']) }) - it('leaves media out of a root Skill, so the README images never block it', async () => { - const served = servedSkill([['docs/assets/interactive-motion.gif', 2_479_001]], 'root') + it('links nothing when the Skill fits, so every CLI gets the same Artifact', () => { + const selected = selectArtifactEntries(skillWith([['assets/track.mp3', 4 * MIB]]), 'skills/demo', linking) - const loaded = await createPublicGithubSourceClient({ fetch: served as unknown as typeof fetch }) - .load({ ...resolvedSource(), skillPath: '.' }) + expect(selected).toMatchObject({ _tag: 'selected', linked: [], omitted: [] }) + }) +}) - expect(loaded).toMatchObject({ - _tag: 'loaded', - value: { - omitted: [{ - path: 'docs/assets/interactive-motion.gif', - bytes: 2_479_001, - url: `https://github.com/skilld-dev/skills/blob/${commitSha}/docs/assets/interactive-motion.gif`, - }], - }, +describe('private Artifact limits', () => { + it('names the text file over the one-file limit and its size', () => { + const selected = selectArtifactEntries(skillWith([['references/api.md', 2_153_066]]), 'skills/demo', PRIVATE_ARTIFACT_LIMITS) + + expect(selected).toMatchObject({ + _tag: 'rejected', + code: 'INVALID_SOURCE', + summary: 'The file `references/api.md` is 2.06 MiB. The limit for one file is 2 MiB.', + findings: ['references/api.md: 2,153,066 bytes'], }) }) - function omission(path: string, bytes: number) { - return { path, bytes, url: `https://github.com/skilld-dev/skills/blob/${commitSha}/skills/demo/${path}` } - } + it('says that a root Skill counts every file in the Repository', () => { + const selected = selectArtifactEntries(skillWith([['docs/data/catalog.json', 2_479_001]]), '.', PRIVATE_ARTIFACT_LIMITS) - /** A Skill folder whose blobs exist, so a load that keeps a file can read it. */ - function servedSkill(files: ReadonlyArray, at: 'skills/demo' | 'root' = 'skills/demo') { - const contents = new Map([[skillBlobSha, Buffer.from(skillText)]]) - const entries = [blob('SKILL.md', skillBlobSha, skillText.length)] - for (const [index, [path, size]] of files.entries()) { - const bytes = Buffer.alloc(size, index + 1) - const sha = createHash('sha1').update(`blob ${size}\0`).update(bytes).digest('hex') - contents.set(sha, bytes) - entries.push(blob(path, sha, size)) - } - const tree = at === 'root' ? rootTreeFetch(entries) : skillTreeFetch(entries) - return vi.fn(async (input: RequestInfo | URL, init?: RequestInit) => { - const sha = String(input).match(/\/git\/blobs\/([a-f0-9]{40})$/)?.[1] - const bytes = sha ? contents.get(sha) : undefined - if (!sha || !bytes) - return await tree(input, init) - return json({ sha, size: bytes.byteLength, encoding: 'base64', content: bytes.toString('base64') }) + expect(selected).toMatchObject({ + _tag: 'rejected', + summary: 'The file `docs/data/catalog.json` is 2.37 MiB. The limit for one file is 2 MiB. The Skill folder is the Repository root, so every file in the Repository counts.', }) - } + }) + + it('leaves media over 2 MiB out and keeps the 10 MiB archive limit', () => { + const selected = selectArtifactEntries(skillWith([ + ['assets/music/track.mp3', 3_936_384], + ['references/guide.md', 1_500_000], + ...Array.from({ length: 6 }, (_, index) => [`examples/render-${index}.png`, 1_600_000 + index * 1_000] as const), + ]), 'skills/demo', PRIVATE_ARTIFACT_LIMITS) + + expect(selected._tag).toBe('selected') + if (selected._tag === 'selected') + expect(selected.omitted.map(entry => entry.path)).toEqual(['assets/music/track.mp3', 'examples/render-5.png']) + }) + + it('keeps the 900 file limit', () => { + const selected = selectArtifactEntries(skillWith(Array.from({ length: 900 }, (_, index) => [`references/entry-${index}.md`, 100])), 'skills/demo', PRIVATE_ARTIFACT_LIMITS) + + expect(selected).toMatchObject({ + _tag: 'rejected', + summary: 'The Skill folder `skills/demo` has 901 files the Skill reads. The limit is 900.', + }) + }) }) describe('projected archive size', () => { @@ -220,9 +295,19 @@ describe('projected archive size', () => { }) }) -function skillEntries(count: number) { - return Array.from({ length: count }, (_, index) => - blob(index === 0 ? 'SKILL.md' : `references/entry-${index}.md`, skillBlobSha, skillText.length)) +/** Loads a public Skill folder of these sizes. A load reads the tree only. */ +async function load(files: ReadonlyArray, at: 'skills/demo' | 'root' = 'skills/demo', linkedFiles = false) { + const entries = skillWith(files) + const fetch = at === 'root' ? rootTreeFetch(entries) : skillTreeFetch(entries) + const client = createPublicGithubSourceClient({ fetch: fetch as unknown as typeof globalThis.fetch }) + return await client.load(at === 'root' ? { ...resolvedSource(), skillPath: '.' } : resolvedSource(), { linkedFiles }) +} + +function skillWith(files: ReadonlyArray) { + return [ + blob('SKILL.md', skillBlobSha, skillText.length), + ...files.map(([path, size], index) => blob(path, createHash('sha1').update(`${path}\0${index}`).digest('hex'), size)), + ] } function skillTreeFetch(entries: object[]) { @@ -236,8 +321,6 @@ function skillTreeFetch(entries: object[]) { return json({ sha: skillsTreeSha, tree: [tree('demo', skillTreeSha)] }) if (url.endsWith(`/git/trees/${skillTreeSha}?recursive=1`)) return json({ sha: skillTreeSha, tree: entries, truncated: false }) - if (url.includes(`/git/blobs/${skillBlobSha}`)) - return json({ sha: skillBlobSha, size: skillText.length, encoding: 'base64', content: btoa(skillText) }) return json({}, 404) }) } @@ -267,11 +350,11 @@ function resolvedSource() { } function tree(path: string, sha: string) { - return { path, mode: '040000', type: 'tree', sha } + return { path, mode: '040000', type: 'tree' as const, sha } } function blob(path: string, sha: string, size: number) { - return { path, mode: '100644', type: 'blob', sha, size } + return { path, mode: '100644', type: 'blob' as const, sha, size } } function publicRepository() { diff --git a/test/unit/artifact-tarball-extract.test.ts b/test/unit/artifact-tarball-extract.test.ts deleted file mode 100644 index 080b7a75..00000000 --- a/test/unit/artifact-tarball-extract.test.ts +++ /dev/null @@ -1,234 +0,0 @@ -import { createHash } from 'node:crypto' -import { describe, expect, it } from 'vitest' -import { extractSkillFilesFromTarball } from '../../layers/artifact-delivery/server/utils/tarball-source' -import { streamOf, tarGzFixture } from '../fixtures/tar-archive' - -const topLevel = 'skilld-dev-skills-457fed3' -const skillText = '---\nname: demo\ndescription: Use this Skill for demo work.\n---\n' -const scriptText = '#!/usr/bin/env bash\necho demo\n' -const encoder = new TextEncoder() - -describe('tarball Skill extraction', () => { - it('keeps only the Skill directory and strips the top level directory', async () => { - const archive = tarGzFixture(topLevel, [ - { path: 'README.md', bytes: encoder.encode('# unrelated\n') }, - { path: 'skills/other/SKILL.md', bytes: encoder.encode('other\n') }, - { path: 'skills/demo/SKILL.md', bytes: encoder.encode(skillText) }, - { path: 'skills/demo/references/guide.md', bytes: encoder.encode('guide\n') }, - ]) - - const result = await extractSkillFilesFromTarball({ - body: streamOf(archive), - skillPath: 'skills/demo', - entries: [ - wanted('SKILL.md', skillText), - wanted('references/guide.md', 'guide\n'), - ], - maxUncompressedBytes: 1024 * 1024, - }) - - expect(result._tag).toBe('extracted') - if (result._tag !== 'extracted') - return - expect(result.files.map(file => file.path)).toEqual(['SKILL.md', 'references/guide.md']) - expect(new TextDecoder().decode(result.files[0]!.bytes)).toBe(skillText) - }) - - it('reads a Skill at the Repository root', async () => { - const archive = tarGzFixture(topLevel, [ - { path: 'SKILL.md', bytes: encoder.encode(skillText) }, - ]) - - const result = await extractSkillFilesFromTarball({ - body: streamOf(archive), - skillPath: '.', - entries: [wanted('SKILL.md', skillText)], - maxUncompressedBytes: 1024 * 1024, - }) - - expect(result).toMatchObject({ _tag: 'extracted' }) - if (result._tag !== 'extracted') - return - expect(result.files.map(file => file.path)).toEqual(['SKILL.md']) - }) - - it('takes the file mode from the tree entry, not the widened tar header', async () => { - const archive = tarGzFixture(topLevel, [ - { path: 'skills/demo/SKILL.md', bytes: encoder.encode(skillText), mode: '0000664' }, - { path: 'skills/demo/run.sh', bytes: encoder.encode(scriptText), mode: '0000775' }, - ]) - - const result = await extractSkillFilesFromTarball({ - body: streamOf(archive), - skillPath: 'skills/demo', - entries: [ - wanted('SKILL.md', skillText), - { ...wanted('run.sh', scriptText), mode: 493 as const }, - ], - maxUncompressedBytes: 1024 * 1024, - }) - - expect(result._tag).toBe('extracted') - if (result._tag !== 'extracted') - return - expect(result.files.map(file => [file.path, file.mode])).toEqual([ - ['SKILL.md', 420], - ['run.sh', 493], - ]) - }) - - it('refuses an archive missing a file the tree lists, as export-ignore produces', async () => { - const archive = tarGzFixture(topLevel, [ - { path: 'skills/demo/SKILL.md', bytes: encoder.encode(skillText) }, - ]) - - const result = await extractSkillFilesFromTarball({ - body: streamOf(archive), - skillPath: 'skills/demo', - entries: [ - wanted('SKILL.md', skillText), - wanted('tests/case.md', 'case\n'), - ], - maxUncompressedBytes: 1024 * 1024, - }) - - expect(result).toMatchObject({ - _tag: 'unusable', - reason: 'missing-files', - findings: ['tests/case.md'], - }) - }) - - it('refuses a file whose bytes do not match its Git blob digest', async () => { - const archive = tarGzFixture(topLevel, [ - { path: 'skills/demo/SKILL.md', bytes: encoder.encode(skillText) }, - { path: 'skills/demo/notes.md', bytes: encoder.encode('tampered bytes for notes\n') }, - ]) - - const result = await extractSkillFilesFromTarball({ - body: streamOf(archive), - skillPath: 'skills/demo', - entries: [ - wanted('SKILL.md', skillText), - wanted('notes.md', 'original bytes of notes!\n'), - ], - maxUncompressedBytes: 1024 * 1024, - }) - - expect(result).toMatchObject({ - _tag: 'unusable', - reason: 'digest-mismatch', - findings: ['notes.md'], - }) - }) - - it('refuses a file whose size does not match its tree entry', async () => { - const archive = tarGzFixture(topLevel, [ - { path: 'skills/demo/SKILL.md', bytes: encoder.encode(skillText) }, - ]) - - const result = await extractSkillFilesFromTarball({ - body: streamOf(archive), - skillPath: 'skills/demo', - entries: [{ ...wanted('SKILL.md', skillText), size: skillText.length + 10 }], - maxUncompressedBytes: 1024 * 1024, - }) - - expect(result).toMatchObject({ _tag: 'unusable', reason: 'size-mismatch' }) - }) - - it('ignores directory and symbolic link entries inside the Skill path', async () => { - const archive = tarGzFixture(topLevel, [ - { path: 'skills/demo/', typeflag: '5' }, - { path: 'skills/demo/link.md', typeflag: '2', linkname: '../../README.md', mode: '0000777' }, - { path: 'skills/demo/SKILL.md', bytes: encoder.encode(skillText) }, - ]) - - const result = await extractSkillFilesFromTarball({ - body: streamOf(archive), - skillPath: 'skills/demo', - entries: [wanted('SKILL.md', skillText)], - maxUncompressedBytes: 1024 * 1024, - }) - - expect(result._tag).toBe('extracted') - if (result._tag !== 'extracted') - return - expect(result.files.map(file => file.path)).toEqual(['SKILL.md']) - }) - - it('refuses an archive that repeats a Skill path', async () => { - const archive = tarGzFixture(topLevel, [ - { path: 'skills/demo/SKILL.md', bytes: encoder.encode(skillText) }, - { path: 'skills/demo/SKILL.md', bytes: encoder.encode(skillText) }, - ]) - - const result = await extractSkillFilesFromTarball({ - body: streamOf(archive), - skillPath: 'skills/demo', - entries: [wanted('SKILL.md', skillText)], - maxUncompressedBytes: 1024 * 1024, - }) - - expect(result).toMatchObject({ _tag: 'unusable', reason: 'duplicate-path' }) - }) - - it('refuses an archive that passes the uncompressed byte ceiling', async () => { - // Incompressible filler, so the ceiling is reached on bytes rather than on - // a gzip ratio. The ceiling stops the download too: measured in workerd on - // 2026-09-22, a 1.5 GiB archive stopped after 64 MiB, 2.9 s and 390 ms of - // CPU, against 92.9 s to stream it whole. - const archive = tarGzFixture(topLevel, [ - { path: 'other/big.bin', bytes: noise(256 * 1024) }, - { path: 'skills/demo/SKILL.md', bytes: encoder.encode(skillText) }, - ]) - - const result = await extractSkillFilesFromTarball({ - body: streamOf(archive), - skillPath: 'skills/demo', - entries: [wanted('SKILL.md', skillText)], - maxUncompressedBytes: 80 * 1024, - }) - - expect(result).toEqual({ _tag: 'unusable', reason: 'archive-too-large', findings: [] }) - }) - - it('refuses an archive whose gzip bytes are corrupt', async () => { - const archive = tarGzFixture(topLevel, [ - { path: 'skills/demo/SKILL.md', bytes: encoder.encode(skillText) }, - ]) - archive.set([0, 0, 0, 0], 12) - - const result = await extractSkillFilesFromTarball({ - body: streamOf(archive), - skillPath: 'skills/demo', - entries: [wanted('SKILL.md', skillText)], - maxUncompressedBytes: 1024 * 1024, - }) - - expect(result).toMatchObject({ _tag: 'unusable', reason: 'malformed-archive' }) - }) -}) - -function noise(size: number): Uint8Array { - const bytes = new Uint8Array(size) - let state = 0x2545F491 - for (let index = 0; index < size; index++) { - state = (Math.imul(state, 1664525) + 1013904223) | 0 - bytes[index] = (state >>> 24) & 0xFF - } - return bytes -} - -function wanted(path: string, text: string) { - return { - path, - gitBlobSha: gitBlobSha(text), - size: encoder.encode(text).byteLength, - mode: 420 as const, - } -} - -function gitBlobSha(value: string): string { - return createHash('sha1').update(`blob ${Buffer.byteLength(value)}\0${value}`).digest('hex') -} diff --git a/test/unit/private-artifact-delivery.test.ts b/test/unit/private-artifact-delivery.test.ts index 5c154e75..4061d9df 100644 --- a/test/unit/private-artifact-delivery.test.ts +++ b/test/unit/private-artifact-delivery.test.ts @@ -481,6 +481,7 @@ async function createReadyFixture(options: { mutateCiphertext?: boolean } = {}) resolutionId: RESOLUTION_ID, }, arrayBuffer: async () => body.buffer, + body: new Blob([Uint8Array.from(body)]).stream(), } as R2ObjectBody }, } as R2Bucket diff --git a/test/unit/skill-run-sweep.test.ts b/test/unit/skill-run-sweep.test.ts index 417c5ecf..4655bca8 100644 --- a/test/unit/skill-run-sweep.test.ts +++ b/test/unit/skill-run-sweep.test.ts @@ -15,6 +15,7 @@ const MIGRATIONS = [ 'migrations/0111_github_app_delivery.sql', 'migrations/0112_private_artifact_keys.sql', 'migrations/0122_artifact_resolution_retry_after.sql', + 'migrations/0145_artifact_resolution_linked_files.sql', 'migrations/0142_artifact_run_checks.sql', 'migrations/0143_artifact_run_check_streak.sql', 'migrations/0144_artifact_resolution_requesters.sql', diff --git a/workers/artifact-signer/src/handler.ts b/workers/artifact-signer/src/handler.ts index c384a0ca..589f4368 100644 --- a/workers/artifact-signer/src/handler.ts +++ b/workers/artifact-signer/src/handler.ts @@ -1,4 +1,5 @@ import type { ArtifactAttestationStatement, CheckResult } from '../../../layers/artifact-delivery/server/schemas/contracts' +import { createHash } from 'node:crypto' import { z } from 'zod' import { artifactAttestationStatementSchema, @@ -27,7 +28,12 @@ export type ArtifactSignerBindings = Pick { const object = await bucket.get(row.r2_key) - if (!object || !('arrayBuffer' in object)) + if (!object || !('body' in object)) return false if ( object.size !== row.content_bytes @@ -275,16 +281,15 @@ async function verifyArtifactObject(bucket: R2Bucket, row: SigningRow): Promise< ) { return false } - const bytes = new Uint8Array(await object.arrayBuffer()) - return bytes.byteLength === row.content_bytes - && await digestHex('SHA-256', bytes) === row.content_sha256 + const streamed = await streamedSha256(object.body) + return streamed.bytes === row.content_bytes && streamed.sha256 === row.content_sha256 } async function verifyPrivateArtifactObject(bucket: R2Bucket, row: SigningRow): Promise { if (!row.account_id || !row.ciphertext_sha256 || !row.ciphertext_bytes || !row.encryption_key_id) return false const object = await bucket.get(row.r2_key) - if (!object || !('arrayBuffer' in object)) + if (!object || !('body' in object)) return false const accountIdHash = await digestHex('SHA-256', String(row.account_id)) if ( @@ -300,9 +305,23 @@ async function verifyPrivateArtifactObject(bucket: R2Bucket, row: SigningRow): P ) { return false } - const bytes = new Uint8Array(await object.arrayBuffer()) - return bytes.byteLength === row.ciphertext_bytes - && await digestHex('SHA-256', bytes) === row.ciphertext_sha256 + const streamed = await streamedSha256(object.body) + return streamed.bytes === row.ciphertext_bytes && streamed.sha256 === row.ciphertext_sha256 +} + +/** The SHA-256 and length of an object body, read once, chunk by chunk. */ +async function streamedSha256(body: ReadableStream): Promise<{ sha256: string, bytes: number }> { + const hash = createHash('sha256') + const reader = body.getReader() + let bytes = 0 + while (true) { + const next = await reader.read() + if (next.done) + break + bytes += next.value.byteLength + hash.update(next.value) + } + return { sha256: hash.digest('hex'), bytes } } async function loadSigningRow(db: D1Database, resolutionId: string): Promise { diff --git a/workers/artifact-signer/tsconfig.json b/workers/artifact-signer/tsconfig.json index 87083b15..fb8c0037 100644 --- a/workers/artifact-signer/tsconfig.json +++ b/workers/artifact-signer/tsconfig.json @@ -7,7 +7,7 @@ "paths": { "#shared/*": ["../../shared/*"] }, - "types": ["./worker-configuration.d.ts"], + "types": ["./worker-configuration.d.ts", "node"], "allowJs": false, "strict": true, "noFallthroughCasesInSwitch": true,