diff --git a/layers/artifact-delivery/server/utils/build.ts b/layers/artifact-delivery/server/utils/build.ts index d07a4672..2ee950fb 100644 --- a/layers/artifact-delivery/server/utils/build.ts +++ b/layers/artifact-delivery/server/utils/build.ts @@ -569,7 +569,7 @@ async function loadAndCheck( const loaded = await github.load(source) if (loaded._tag === 'rejected') return { _tag: 'rejected', rejection: loaded } - const checked = await checkArtifactSource(source, loaded.value.files) + const checked = await checkArtifactSource(source, loaded.value.files, loaded.value.omitted) return { _tag: 'loaded', source, files: loaded.value.files, checked } } diff --git a/layers/artifact-delivery/server/utils/checks.ts b/layers/artifact-delivery/server/utils/checks.ts index d9e74702..ed637bb5 100644 --- a/layers/artifact-delivery/server/utils/checks.ts +++ b/layers/artifact-delivery/server/utils/checks.ts @@ -1,5 +1,5 @@ import type { ArtifactFile, CheckResult, ResolvedSource } from '../schemas/contracts' -import type { ArtifactSourceFile } from './github-source' +import type { ArtifactSourceFile, OmittedArtifactFile } from './github-source' import { digestHex } from './encoding' import { ARTIFACT_POLICY_VERSION } from './state' @@ -12,8 +12,24 @@ import { ARTIFACT_POLICY_VERSION } from './state' const AGENT_SKILLS_CHECK_VERSION = '2026-10-07' const MAX_DESCRIPTION_CHARACTERS = 1024 const PATH_POLICY_VERSION = '1' -const CREDENTIAL_MATERIAL_VERSION = '1' +/** + * Version 2, 2026-10-07: a finding needs a whole key block, not its first + * line. Version 1 blocked nine Skills for a header alone: a secret scanner's + * pattern table, a `"-----BEGIN PRIVATE KEY-----\n..."` placeholder, and a + * test fixture whose body read `TEST-NOT-A-REAL-KEY`. None held a key. + */ +const CREDENTIAL_MATERIAL_VERSION = '2' const EXECUTABLE_FILES_VERSION = '1' +/** + * Lists the files left out of the Artifact for a size limit. It is a check + * result, not a new attestation field: the released skilld CLI refuses an + * attestation or a Resolution answer with a field it does not know, and it + * accepts any check result that is not required. + */ +const OMITTED_FILES_VERSION = '1' +/** The skilld CLI refuses a check result with more findings, or a longer one. */ +const MAX_CHECK_FINDINGS = 100 +const MAX_CHECK_FINDING_CHARACTERS = 500 /** The checks a statement under one policy carries, by check name. */ export type ArtifactCheckSet = ReadonlyMap @@ -23,6 +39,7 @@ const CURRENT_ARTIFACT_CHECKS: ArtifactCheckSet = new Map([ ['agent-skills-spec', { version: AGENT_SKILLS_CHECK_VERSION, required: false }], ['credential-material', { version: CREDENTIAL_MATERIAL_VERSION, required: true }], ['executable-files', { version: EXECUTABLE_FILES_VERSION, required: false }], + ['omitted-files', { version: OMITTED_FILES_VERSION, required: false }], ]) /** @@ -40,9 +57,9 @@ const CURRENT_ARTIFACT_CHECKS: ArtifactCheckSet = new Map([ */ export const SIGNABLE_ARTIFACT_POLICIES: ReadonlyMap = new Map([ [ARTIFACT_POLICY_VERSION, CURRENT_ARTIFACT_CHECKS], - ['2026-08-20.1', new Map([ + ['2026-10-07.1', new Map([ ['path-policy', { version: '1', required: true }], - ['agent-skills-spec', { version: '2026-08-20', required: true }], + ['agent-skills-spec', { version: '2026-10-07', required: false }], ['credential-material', { version: '1', required: true }], ['executable-files', { version: '1', required: false }], ])], @@ -56,6 +73,7 @@ export interface CheckedArtifactSource { export async function checkArtifactSource( source: ResolvedSource, files: ArtifactSourceFile[], + omitted: OmittedArtifactFile[] = [], ): Promise { const fileInventory = await Promise.all(files.map(async file => ({ path: file.path, @@ -136,7 +154,7 @@ export async function checkArtifactSource( const text = decodeText(file.bytes) if (!text) continue - if (/-----BEGIN (?:[A-Z0-9 ]+ )?PRIVATE KEY-----/.test(text)) + if (containsPrivateKey(text)) credentialFindings.push(`${file.path} contains private key material.`) } const credentialMaterial: CheckResult = credentialFindings.length > 0 @@ -174,7 +192,30 @@ export async function checkArtifactSource( return { files: fileInventory, - checkResults: [pathPolicy, agentSkillsSpec, credentialMaterial, executableFiles], + checkResults: [pathPolicy, agentSkillsSpec, credentialMaterial, executableFiles, omittedFilesResult(omitted)], + } +} + +function omittedFilesResult(omitted: OmittedArtifactFile[]): CheckResult { + if (omitted.length === 0) + return { name: 'omitted-files', version: OMITTED_FILES_VERSION, outcome: 'pass', required: false } + const count = omitted.length === 1 + ? '1 file over the size limits was left out of the Artifact.' + : `${omitted.length} files over the size limits were left out of the Artifact.` + const listed = omitted.length > MAX_CHECK_FINDINGS ? ` The first ${MAX_CHECK_FINDINGS} are listed.` : '' + return { + name: 'omitted-files', + version: OMITTED_FILES_VERSION, + outcome: 'warn', + required: false, + summary: count + listed, + findings: omitted.slice(0, MAX_CHECK_FINDINGS).map((file) => { + const finding = `${file.path}: ${file.bytes.toLocaleString('en-US')} bytes, ${file.url}` + // A long path drops the URL first: the path is what a reader needs. + return finding.length <= MAX_CHECK_FINDING_CHARACTERS + ? finding + : `${file.path}: ${file.bytes.toLocaleString('en-US')} bytes`.slice(0, MAX_CHECK_FINDING_CHARACTERS) + }), } } @@ -198,6 +239,43 @@ export function checksPermitSigning(checks: CheckResult[], policyVersion: string && checks.every(check => !check.required || check.outcome === 'pass') } +/** + * A PEM private key block: the BEGIN line, a body with no dashes, and the END + * line with the same label. The body bound keeps one match linear; a 4096-bit + * RSA key is about 3,300 characters. + */ +const PRIVATE_KEY_BLOCK = /-----BEGIN ((?:[A-Z0-9]+ )*)PRIVATE KEY-----((?:(?!-----)[\s\S]){0,16384})-----END \1PRIVATE KEY-----/g +/** A line break, real or escaped inside a JSON or code string. */ +const BODY_LINE_BREAK = /\r?\n|(?:\\+[rn])+/ +/** A legacy PEM header such as `Proc-Type: 4,ENCRYPTED`. */ +const PEM_HEADER_LINE = /^[\w-]+:\s/ +/** + * Base64 of at least 48 bytes: an Ed25519 PKCS#8 key, the smallest private + * key format, is exactly 64 characters. + */ +const KEY_BODY = /^[A-Z0-9+/]{64,}={0,2}$/i + +/** + * True when the text holds a complete private key, encrypted or not. + * + * A placeholder has the BEGIN line but no key: an ellipsis, a bracketed note, + * or a test label in the body never reads as base64. The body may sit in a + * JSON string with escaped line breaks, which is how a leaked service account + * key usually arrives, or be indented inside YAML. + */ +function containsPrivateKey(text: string): boolean { + for (const match of text.matchAll(PRIVATE_KEY_BLOCK)) { + const body = match[2]! + .split(BODY_LINE_BREAK) + .map(line => line.trim()) + .filter(line => !PEM_HEADER_LINE.test(line)) + .join('') + if (KEY_BODY.test(body)) + return true + } + return false +} + function decodeText(bytes: Uint8Array): string | null { try { return new TextDecoder('utf-8', { fatal: true, ignoreBOM: false }).decode(bytes) diff --git a/layers/artifact-delivery/server/utils/github-source.ts b/layers/artifact-delivery/server/utils/github-source.ts index 6909dcad..70f4d79a 100644 --- a/layers/artifact-delivery/server/utils/github-source.ts +++ b/layers/artifact-delivery/server/utils/github-source.ts @@ -2,7 +2,7 @@ import type { ProblemCode, ResolvedSource, SourceRequest } from '../schemas/cont import type { GithubReadTry } from './github-read' import type { TarballExtraction } from './tarball-source' import { z } from 'zod' -import { canonicalSkillFolder, isRegistrySkillPath } from '#shared/skill-path' +import { canonicalSkillFolder, isRegistrySkillPath, slugifySkillName } from '#shared/skill-path' import { base64ToBytes, gitBlobShaHex } from './encoding' import { fetchNoRedirect } from './fetch-no-redirect' import { @@ -110,9 +110,19 @@ export interface ArtifactSourceFile { gitBlobSha: string } +/** A Skill file left out of the Artifact because it is over a size limit. */ +export interface OmittedArtifactFile { + /** The path inside the Skill folder. */ + path: string + bytes: number + /** The file on GitHub at the Artifact's commit. */ + url: string +} + export interface LoadedArtifactSource { source: ResolvedSource files: ArtifactSourceFile[] + omitted: OmittedArtifactFile[] } export interface SourceRejection { @@ -398,10 +408,16 @@ export function createGithubSourceClient(options: GithubClientOptions): PublicGi [name], ) } + // The registry lists a Skill by its folder name made into a slug, so + // `emailAndPassword` runs as `emailandpassword`. An exact folder name + // still matches for a caller that sends one. const matches = tree.value.tree .filter(entry => entry.type === 'blob' && isRegistrySkillPath(entry.path)) .map(entry => entry.path === 'SKILL.md' ? '.' : entry.path.slice(0, -'/SKILL.md'.length)) - .filter(path => (path === '.' ? repository : path.split('/').at(-1)) === name) + .filter((path) => { + const folderName = path === '.' ? repository : path.split('/').at(-1)! + return folderName === name || slugifySkillName(folderName) === name + }) return canonicalSkillFolder(matches) ?? reject('SOURCE_NOT_FOUND', 'No Skill matched the requested name.', [name]) } @@ -521,6 +537,11 @@ export function createGithubSourceClient(options: GithubClientOptions): PublicGi const selected = selectArtifactEntries(listed.entries, source.skillPath) if (selected._tag === 'rejected') return selected + const omitted = selected.omitted.map(entry => ({ + path: entry.path, + bytes: entry.size, + url: githubBlobUrl(source, entry.path), + })) const choice = chooseArtifactByteSource({ visibility: source.visibility, @@ -530,7 +551,7 @@ export function createGithubSourceClient(options: GithubClientOptions): PublicGi if (choice._tag === 'tarball') { const extracted = await loadFromTarball(source, selected.entries) if (extracted._tag === 'extracted') - return { _tag: 'loaded', value: { source, files: extracted.files } } + return { _tag: 'loaded', value: { source, files: extracted.files, omitted } } // 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 @@ -544,7 +565,8 @@ export function createGithubSourceClient(options: GithubClientOptions): PublicGi findings: extracted.findings.slice(0, 20), }) } - return await loadFromBlobs(source, selected.entries) + const fromBlobs = await loadFromBlobs(source, selected.entries) + return fromBlobs._tag === 'loaded' ? { _tag: 'loaded', value: { source, files: fromBlobs.files, omitted } } : fromBlobs }, } @@ -592,7 +614,7 @@ export function createGithubSourceClient(options: GithubClientOptions): PublicGi async function loadFromBlobs( source: ResolvedSource, entries: Array, - ): Promise { + ): Promise<{ _tag: 'loaded', files: ArtifactSourceFile[] } | SourceRejection> { const files: ArtifactSourceFile[] = [] for (let offset = 0; offset < entries.length; offset += 8) { const batch = entries.slice(offset, offset + 8) @@ -623,7 +645,7 @@ export function createGithubSourceClient(options: GithubClientOptions): PublicGi return rejected files.push(...loaded.filter((item): item is ArtifactSourceFile => !isSourceRejection(item))) } - return { _tag: 'loaded', value: { source, files } } + return { _tag: 'loaded', files } } } @@ -682,14 +704,21 @@ function normalizeRequestedSkillPath(input: string): string | null { return path } +type SizedEntry = TreeEntry & { size: number } + /** - * The blobs one Skill folder packs, or the rule that refuses them. + * The blobs one Skill folder packs, the files it leaves out, or the rule that + * refuses them. * - * Every limit counts the Skill folder only. A Skill at the Repository root has - * the whole Repository as its folder, so each size message says so: a README - * image then counts against the Skill. + * 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. */ -function selectArtifactEntries(entries: TreeEntry[], skillPath: string): { _tag: 'selected', entries: Array } | SourceRejection { +function selectArtifactEntries(entries: TreeEntry[], skillPath: string): { _tag: 'selected', entries: SizedEntry[], omitted: SizedEntry[] } | SourceRejection { const findings: string[] = [] const identities = new Map() const blobs: Array = [] @@ -725,26 +754,161 @@ function selectArtifactEntries(entries: TreeEntry[], skillPath: string): { _tag: : '' if (blobs.length > MAX_ARTIFACT_FILES) return sourceLimitRejection(`${skillFolderLabel(skillPath)} has ${blobs.length} files. The limit is ${MAX_ARTIFACT_FILES}.`) - const oversized = blobs.filter(entry => entry.size > MAX_FILE_BYTES) - if (oversized.length > 0) { + const oversizedText = blobs.filter(entry => entry.size > MAX_FILE_BYTES && isReadBySkill(entry.path)) + if (oversizedText.length > 0) { return sourceLimitRejection( - `The file \`${oversized[0]!.path}\` is ${mebibytes(oversized[0]!.size)}. The limit for one file is ${mebibytes(MAX_FILE_BYTES)}.${rootNote}`, - oversized.map(entry => `${entry.path}: ${entry.size.toLocaleString('en-US')} bytes`), + `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 = 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) + } // The signer checks `content_bytes` against this same ceiling, and // `content_bytes` is the packed archive rather than the sum of the blobs. A // Skill that cleared a source-total check could therefore be refused after // the R2 write, with the signer's error instead of a named rejection. Guard // the number the signer will actually see. - const projectedBytes = projectedUstarBytes(blobs.map(entry => entry.size)) + const projectedBytes = projectedUstarBytes(kept.map(entry => entry.size)) if (projectedBytes > MAX_ARTIFACT_BYTES) { return sourceLimitRejection( - `${skillFolderLabel(skillPath)} packs to ${mebibytes(projectedBytes)}. The limit is ${mebibytes(MAX_ARTIFACT_BYTES)}.${rootNote}`, - largestFirst(blobs).map(entry => `${entry.path}: ${entry.size.toLocaleString('en-US')} bytes`), + `The files the Skill reads in ${skillFolderLabel(skillPath).replace(/^The /, 'the ')} pack to ${mebibytes(projectedBytes)}. The limit is ${mebibytes(MAX_ARTIFACT_BYTES)}.${rootNote}`, + largestFirst(kept).map(entry => `${entry.path}: ${entry.size.toLocaleString('en-US')} bytes`), ) } - return { _tag: 'selected', entries: blobs.sort((a, b) => comparePath(a.path, b.path)) } + return { + _tag: 'selected', + entries: kept.sort((a, b) => comparePath(a.path, b.path)), + omitted: omitted.sort((a, b) => comparePath(a.path, b.path)), + } +} + +/** + * File names a Skill reads as text. Anything else over a size limit, such as + * an image, audio, video, an archive or a binary, can be left out. + */ +const TEXT_EXTENSIONS: ReadonlySet = new Set([ + 'adoc', + 'bash', + 'bat', + 'bib', + 'c', + 'cc', + 'cfg', + 'cjs', + 'conf', + 'cpp', + 'cs', + 'css', + 'csv', + 'cts', + 'dart', + 'env', + 'fish', + 'go', + 'gql', + 'graphql', + 'h', + 'hpp', + 'htm', + 'html', + 'ini', + 'ipynb', + 'java', + 'js', + 'json', + 'json5', + 'jsonc', + 'jsonl', + 'jsx', + 'kt', + 'less', + 'lua', + 'markdown', + 'md', + 'mdc', + 'mdx', + 'mjs', + 'mts', + 'php', + 'pl', + 'prompt', + 'properties', + 'ps1', + 'py', + 'r', + 'rb', + 'rs', + 'rst', + 'sass', + 'scss', + 'sh', + 'sql', + 'svelte', + 'swift', + 'tex', + 'toml', + 'ts', + 'tsv', + 'tsx', + 'txt', + 'vue', + 'xml', + 'yaml', + 'yml', + 'zsh', +]) +const TEXT_FILE_NAMES: ReadonlySet = new Set([ + 'dockerfile', + 'gemfile', + 'license', + 'makefile', + 'procfile', + 'readme', +]) + +/** + * Folders of sample output and tests. A Skill runs its scripts and reads its + * references; it does not read these. tt-a1i/archify keeps five rendered + * examples of about 760 KB each in `examples/`. + */ +const NOT_READ_FOLDERS: ReadonlySet = new Set([ + '__fixtures__', + '__tests__', + 'example', + 'examples', + 'fixtures', + 'sample', + 'samples', + 'test', + 'tests', +]) + +/** True for SKILL.md and for a text file outside the example and test folders. */ +function isReadBySkill(path: string): boolean { + if (path === 'SKILL.md') + return true + const segments = path.split('/') + if (segments.slice(0, -1).some(segment => NOT_READ_FOLDERS.has(segment.toLowerCase()))) + return false + const name = segments.at(-1)!.toLowerCase() + if (TEXT_FILE_NAMES.has(name)) + return true + const dot = name.lastIndexOf('.') + return dot > 0 && TEXT_EXTENSIONS.has(name.slice(dot + 1)) +} + +/** The GitHub page of one Skill file at the Artifact's commit. */ +function githubBlobUrl(source: ResolvedSource, path: string): string { + const repositoryPath = source.skillPath === '.' ? path : `${source.skillPath}/${path}` + return `https://github.com/${encodeURIComponent(source.owner)}/${encodeURIComponent(source.repository)}/blob/${source.commitSha}/${repositoryPath.split('/').map(encodeURIComponent).join('/')}` } function skillFolderLabel(skillPath: string): string { @@ -760,7 +924,7 @@ function mebibytes(bytes: number): string { return `${Number.isInteger(value) ? value : value.toFixed(2)} MiB` } -function largestFirst(entries: Array): Array { +function largestFirst(entries: SizedEntry[]): SizedEntry[] { return [...entries].sort((a, b) => b.size - a.size || comparePath(a.path, b.path)) } diff --git a/layers/artifact-delivery/server/utils/state.ts b/layers/artifact-delivery/server/utils/state.ts index 008b03ff..3602e2be 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.1' +export const ARTIFACT_POLICY_VERSION = '2026-10-07.2' export const ACTIVE_BUILD_STATES = [ 'requested', diff --git a/layers/registry/server/utils/skill-frontmatter.ts b/layers/registry/server/utils/skill-frontmatter.ts index 81d9571f..9651107d 100644 --- a/layers/registry/server/utils/skill-frontmatter.ts +++ b/layers/registry/server/utils/skill-frontmatter.ts @@ -1,4 +1,5 @@ import { parseDocument } from 'yaml' +import { slugifySkillName } from '#shared/skill-path' const FRONTMATTER_RE = /^---\r?\n([\s\S]*?)\r?\n---(?:\r?\n([\s\S]*))?$/ const KEY_LINE_RE = /^([A-Z_][\w-]*):(.*)$/i @@ -142,19 +143,6 @@ export function parseFrontmatter(raw: string): SkillFrontmatter { return fm } -const SLUGIFY_STRIP_RE = /[^a-z0-9-]+/g -const SLUGIFY_DEDUPE_DASH_RE = /-+/g -const SLUGIFY_TRIM_DASH_RE = /^-+|-+$/g - -export function slugifySkillName(s: string): string { - return s - .toLowerCase() - .replace(/\s+/g, '-') - .replace(SLUGIFY_STRIP_RE, '') - .replace(SLUGIFY_DEDUPE_DASH_RE, '-') - .replace(SLUGIFY_TRIM_DASH_RE, '') -} - export interface ParsedSkill { name: string displayName: string diff --git a/scripts/fix-bad-slugs.ts b/scripts/fix-bad-slugs.ts index c3d2122a..f7828c4a 100644 --- a/scripts/fix-bad-slugs.ts +++ b/scripts/fix-bad-slugs.ts @@ -18,7 +18,7 @@ import { execFileSync } from 'node:child_process' import process from 'node:process' -import { slugifySkillName } from '../server/utils/skill-frontmatter' +import { slugifySkillName } from '../shared/skill-path' interface BadRow { owner: string diff --git a/shared/server/skill-mention-verify.ts b/shared/server/skill-mention-verify.ts index b2c0c910..80a056df 100644 --- a/shared/server/skill-mention-verify.ts +++ b/shared/server/skill-mention-verify.ts @@ -31,8 +31,8 @@ import type { GithubBindings } from '#layers/registry/server/utils/github-client' import { getBlobsBatch, getRepoSummary, getTree, GRAPHQL_BATCH_SIZE, hasBody } from '#layers/registry/server/utils/github-client' -import { parseSkillFile, slugifySkillName } from '#layers/registry/server/utils/skill-frontmatter' -import { isRegistrySkillPath, isSkilldCacheSkill } from '#shared/skill-path' +import { parseSkillFile } from '#layers/registry/server/utils/skill-frontmatter' +import { isRegistrySkillPath, isSkilldCacheSkill, slugifySkillName } from '#shared/skill-path' /** Matches `sync-repo.ts`, so this module and the indexer agree on what a skill is. */ const SKILL_FILE_SUFFIX = '/SKILL.md' diff --git a/shared/skill-path.ts b/shared/skill-path.ts index 307fd21f..6f250012 100644 --- a/shared/skill-path.ts +++ b/shared/skill-path.ts @@ -25,6 +25,28 @@ const NON_SKILL_FOLDERS: ReadonlySet = new Set([ 'tests', ]) +const SLUGIFY_STRIP_RE = /[^a-z0-9-]+/g +const SLUGIFY_DEDUPE_DASH_RE = /-+/g +const SLUGIFY_TRIM_DASH_RE = /^-+|-+$/g + +/** + * The registry Skill name for a folder name, or for the Repository name when + * the Skill sits at the root. + * + * The registry admits a Skill under this name, and delivery finds a Skill by + * it, so both read it from this one function. Delivery used to compare the raw + * folder name: `better-auth/emailAndPassword` was listed as `emailandpassword` + * and no run could find it. + */ +export function slugifySkillName(s: string): string { + return s + .toLowerCase() + .replace(/\s+/g, '-') + .replace(SLUGIFY_STRIP_RE, '') + .replace(SLUGIFY_DEDUPE_DASH_RE, '-') + .replace(SLUGIFY_TRIM_DASH_RE, '') +} + /** * True when a Git tree path is a SKILL.md the registry indexes. * diff --git a/test/unit/artifact-delivery.test.ts b/test/unit/artifact-delivery.test.ts index 03069179..6fc5aaea 100644 --- a/test/unit/artifact-delivery.test.ts +++ b/test/unit/artifact-delivery.test.ts @@ -1,7 +1,7 @@ import type { ResolvedSource, SourceRequest } from '../../layers/artifact-delivery/server/schemas/contracts' import type { ArtifactBuildDependencies } from '../../layers/artifact-delivery/server/utils/build' import type { ArtifactSourceFile, PublicGithubSourceClient } from '../../layers/artifact-delivery/server/utils/github-source' -import { createHash } from 'node:crypto' +import { createHash, generateKeyPairSync } from 'node:crypto' import { describe, expect, it, vi } from 'vitest' import { artifactAttestationSchema, @@ -173,7 +173,8 @@ describe('public Artifact delivery', () => { it('blocks failed checks before storage or signing', async () => { const harness = await createBuildHarness([{ ...validFiles[0]!, - bytes: new TextEncoder().encode('---\nname: demo\ndescription: Demo.\n---\n-----BEGIN PRIVATE KEY-----\n'), + // Generated per run, so the repository never holds key material. + bytes: new TextEncoder().encode(`---\nname: demo\ndescription: Demo.\n---\n${generateKeyPairSync('ed25519').privateKey.export({ type: 'pkcs8', format: 'pem' })}`), }]) const result = await processArtifactBuild(harness.dependencies, harness.resolutionId) diff --git a/test/unit/artifact-named-skill.test.ts b/test/unit/artifact-named-skill.test.ts index aaf0c696..85826b80 100644 --- a/test/unit/artifact-named-skill.test.ts +++ b/test/unit/artifact-named-skill.test.ts @@ -58,6 +58,25 @@ describe('named Skill resolution', () => { expect(result).toMatchObject({ _tag: 'resolved', source: { skillPath: '.claude/skills/demo' } }) }) + // The registry names a Skill by its folder name made into a slug. Each case + // is a registry Skill the 2026-10-07 sweep could not find by that name. + it.each([ + ['emailandpassword', 'better-auth/emailAndPassword'], + ['071-dotnet-legacy-api-controllers', 'skills/07.1-dotnet-legacy-api-controllers'], + ['skill-development', '.agents/skills/Skill Development'], + ['ib0dte', '.claude/skills/ib_0dte'], + ])('finds the registry name %s in the folder %s', async (name, folder) => { + const result = await resolveNamed(name, [blob(`${folder}/SKILL.md`)]) + + expect(result).toMatchObject({ _tag: 'resolved', source: { skillPath: folder } }) + }) + + it('finds a root Skill by the registry name of a mixed-case Repository', async () => { + const result = await resolveNamed('agent-skills', [blob('SKILL.md')], false, 'Agent-Skills') + + expect(result).toMatchObject({ _tag: 'resolved', source: { skillPath: '.' } }) + }) + it('asks for a path when GitHub cannot list the whole Repository', async () => { const result = await resolveNamed('demo', [blob('skills/demo/SKILL.md')], true) @@ -69,11 +88,11 @@ describe('named Skill resolution', () => { }) }) -async function resolveNamed(name: string, entries: object[], truncated = false) { +async function resolveNamed(name: string, entries: object[], truncated = false, repositoryName = 'skills') { const fetchMock = vi.fn(async (input: RequestInfo | URL) => { const url = String(input) if (url.endsWith('/repos/skilld-dev/skills')) - return json({ id: 123, name: 'skills', owner: { login: 'skilld-dev' }, private: false, default_branch: 'main' }) + return json({ id: 123, name: repositoryName, owner: { login: 'skilld-dev' }, private: false, default_branch: 'main' }) if (url.endsWith('/git/ref/heads/main')) return json({ ref: 'refs/heads/main', object: { type: 'commit', sha: commitSha } }) if (url.endsWith(`/commits/${commitSha}`)) diff --git a/test/unit/artifact-signer.test.ts b/test/unit/artifact-signer.test.ts index 7749459d..8a07d2da 100644 --- a/test/unit/artifact-signer.test.ts +++ b/test/unit/artifact-signer.test.ts @@ -289,8 +289,9 @@ async function createSignerFixture(options: SignerFixtureOptions = {}) { const checks: CheckResult[] = options.checks ?? [ { name: 'path-policy', version: '1', outcome: options.checkOutcome ?? 'pass', required: true }, { name: 'agent-skills-spec', version: '2026-10-07', outcome: 'pass', required: false }, - { name: 'credential-material', version: '1', outcome: 'pass', required: true }, + { name: 'credential-material', version: '2', outcome: 'pass', required: true }, { name: 'executable-files', version: '1', outcome: 'pass', required: false }, + { name: 'omitted-files', version: '1', outcome: 'pass', required: false }, ] const statement = encodeAttestationStatement({ ...createAttestationStatement({ diff --git a/test/unit/artifact-source-checks.test.ts b/test/unit/artifact-source-checks.test.ts new file mode 100644 index 00000000..707e3787 --- /dev/null +++ b/test/unit/artifact-source-checks.test.ts @@ -0,0 +1,126 @@ +import type { ResolvedSource } from '../../layers/artifact-delivery/server/schemas/contracts' +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' + +// 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 +const pkcs1 = generateKeyPairSync('rsa', { modulusLength: 2048 }).privateKey.export({ type: 'pkcs1', format: 'pem' }) as string +const sec1 = generateKeyPairSync('ec', { namedCurve: 'P-256' }).privateKey.export({ type: 'sec1', format: 'pem' }) as string +const ed25519 = generateKeyPairSync('ed25519').privateKey.export({ type: 'pkcs8', format: 'pem' }) as string +const legacyEncrypted = generateKeyPairSync('rsa', { modulusLength: 2048 }).privateKey.export({ type: 'pkcs1', format: 'pem', cipher: 'aes-256-cbc', passphrase: 'demo' }) as string + +describe('credential material check', () => { + it.each([ + ['a PKCS#8 key', pkcs8], + ['a PKCS#1 RSA key', pkcs1], + ['a SEC1 EC key', sec1], + ['the shortest PKCS#8 key, Ed25519', ed25519], + ['a legacy encrypted key with PEM headers', legacyEncrypted], + ['a key inside a JSON service account file', JSON.stringify({ type: 'service_account', private_key: pkcs8 }, null, 2)], + ['a key indented in a YAML block', `tls:\n key: |\n${pkcs8.split('\n').map(line => ` ${line}`).join('\n')}`], + ])('blocks %s', async (_label, text) => { + const checked = await check(file('config/credentials.txt', text)) + + expect(credentialResult(checked)).toMatchObject({ + outcome: 'fail', + required: true, + findings: ['config/credentials.txt contains private key material.'], + }) + expect(checksBlockArtifact(checked)).toBe(true) + }) + + it('finds a key after a placeholder in the same file', async () => { + const text = `Never commit \`-----BEGIN PRIVATE KEY-----\`.\n\n${pkcs8}` + + expect(credentialResult(await check(file('SKILL.md', text)))).toMatchObject({ outcome: 'fail' }) + }) + + // Each case quotes a Skill the 2026-10-07 sweep blocked. None holds a key. + it.each([ + ['a memory search pattern', 'frida_memory_search(pid, "-----BEGIN RSA PRIVATE KEY-----")'], + ['a truncated JSON value', '"key": "-----BEGIN RSA PRIVATE KEY-----\\n..."'], + ['an ellipsis body', '"key": "-----BEGIN PRIVATE KEY-----\\n...\\n-----END PRIVATE KEY-----"'], + ['a bracketed placeholder body', '-----BEGIN RSA PRIVATE KEY-----\n[Base64 encoded private key]\n-----END RSA PRIVATE KEY-----'], + ['a test fixture body', '"private_key": "-----BEGIN PRIVATE KEY-----\\nTEST-NOT-A-REAL-KEY\\n-----END PRIVATE KEY-----\\n"'], + ['a detection table', '| Private Keys | `-----BEGIN RSA PRIVATE KEY-----`, `-----BEGIN OPENSSH PRIVATE KEY-----` |'], + ['a shell echo', 'sh \'echo "-----BEGIN RSA PRIVATE KEY-----" > /tmp/key.pem\''], + ['prose about the format', 'CryptoKit uses PKCS#8 for private keys (`-----BEGIN PRIVATE KEY-----`).'], + ['a key cut short in documentation', '-----BEGIN PRIVATE KEY-----\nMIIEvQIBADANBgkqhkiG9w0BAQEFAASC...\n-----END PRIVATE KEY-----'], + ])('passes %s', async (_label, text) => { + const checked = await check(file('references/guide.md', text)) + + expect(credentialResult(checked)).toMatchObject({ outcome: 'pass', required: true }) + expect(checksBlockArtifact(checked)).toBe(false) + }) +}) + +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 + + expect(omittedResult(checked)).toEqual({ name: 'omitted-files', version: '1', outcome: 'pass', required: false }) + }) + + it('lists each omitted file with its size and source, and still lets the Skill run', async () => { + const url = `https://github.com/acme/skills/blob/${'0'.repeat(40)}/skills/demo/assets/track.mp3` + const checked = (await checkArtifactSource(source(), [file('SKILL.md', '---\nname: demo\n---\n')], [ + { path: 'assets/track.mp3', bytes: 3_936_384, url }, + ])).checkResults + + expect(omittedResult(checked)).toEqual({ + name: 'omitted-files', + version: '1', + outcome: 'warn', + required: false, + summary: '1 file over the size limits was left out of the Artifact.', + findings: [`assets/track.mp3: 3,936,384 bytes, ${url}`], + }) + expect(checksBlockArtifact(checked)).toBe(false) + }) + + it('keeps every finding inside the limits the skilld CLI verifies', async () => { + const omitted = Array.from({ length: 150 }, (_, index) => ({ + path: `assets/${'a'.repeat(400)}-${index}.png`, + bytes: 3_000_000, + url: `https://github.com/acme/skills/blob/main/assets/${'a'.repeat(400)}-${index}.png`, + })) + const result = omittedResult((await checkArtifactSource(source(), [file('SKILL.md', '---\nname: demo\n---\n')], omitted)).checkResults) + + expect(result?.summary).toBe('150 files over the size limits were left out of the Artifact. The first 100 are listed.') + expect(result?.findings).toHaveLength(100) + expect(result?.findings?.every(finding => finding.length <= 500)).toBe(true) + }) +}) + +function omittedResult(results: Awaited>) { + return results.find(result => result.name === 'omitted-files') +} + +async function check(extra: ArtifactSourceFile) { + const skill = file('SKILL.md', '---\nname: demo\ndescription: Demo.\n---\n') + const files = extra.path === 'SKILL.md' ? [extra] : [skill, extra] + return (await checkArtifactSource(source(), files)).checkResults +} + +function credentialResult(results: Awaited>) { + return results.find(result => result.name === 'credential-material') +} + +function file(path: string, text: string): ArtifactSourceFile { + return { path, mode: 420, bytes: new TextEncoder().encode(text), gitBlobSha: 'a'.repeat(40) } +} + +function source(): ResolvedSource { + return { + provider: 'github', + repositoryId: 1, + owner: 'acme', + repository: 'skills', + visibility: 'public', + commitSha: '0'.repeat(40), + treeSha: '1'.repeat(40), + skillPath: 'skills/demo', + } +} diff --git a/test/unit/artifact-source-limits.test.ts b/test/unit/artifact-source-limits.test.ts index 842c3469..5804d9d9 100644 --- a/test/unit/artifact-source-limits.test.ts +++ b/test/unit/artifact-source-limits.test.ts @@ -54,15 +54,14 @@ describe('artifact source size guards', () => { expect(loaded).toMatchObject({ _tag: 'rejected', code: 'INVALID_SOURCE', - summary: 'The Skill folder `skills/demo` packs to 10.11 MiB. The limit is 10 MiB.', + summary: 'The files the Skill reads in the Skill folder `skills/demo` pack to 10.11 MiB. The limit is 10 MiB.', }) }) - it('names the file over the one-file limit and its size', async () => { - // agiwhitelist/auteur keeps a 2,153,066 byte README image beside its SKILL.md. + it('names the text file over the one-file limit and its size', async () => { const entries = [ blob('SKILL.md', skillBlobSha, skillText.length), - blob('assets/readme/hero-anim.webp', skillBlobSha, 2_153_066), + blob('references/api.md', skillBlobSha, 2_153_066), ] const client = createPublicGithubSourceClient({ fetch: skillTreeFetch(entries) as unknown as typeof fetch, @@ -73,15 +72,15 @@ describe('artifact source size guards', () => { expect(loaded).toMatchObject({ _tag: 'rejected', code: 'INVALID_SOURCE', - summary: 'The file `assets/readme/hero-anim.webp` is 2.06 MiB. The limit for one file is 2 MiB.', - findings: ['assets/readme/hero-anim.webp: 2,153,066 bytes'], + 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'], }) }) it('says that a root Skill counts every file in the Repository', async () => { const entries = [ blob('SKILL.md', skillBlobSha, skillText.length), - blob('docs/assets/interactive-motion.gif', skillBlobSha, 2_479_001), + blob('docs/data/catalog.json', skillBlobSha, 2_479_001), ] const client = createPublicGithubSourceClient({ fetch: rootTreeFetch(entries) as unknown as typeof fetch, @@ -92,11 +91,113 @@ describe('artifact source size guards', () => { expect(loaded).toMatchObject({ _tag: 'rejected', code: 'INVALID_SOURCE', - summary: 'The file `docs/assets/interactive-motion.gif` 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: '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.', }) }) }) +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], + ]) + + const loaded = await createPublicGithubSourceClient({ fetch: served as unknown as typeof fetch }).load(resolvedSource()) + + expect(loaded._tag).toBe('loaded') + if (loaded._tag !== 'loaded') + 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), + ]) + }) + + 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), + ]) + + const loaded = await createPublicGithubSourceClient({ fetch: served as unknown as typeof fetch }).load(resolvedSource()) + + 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('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], + ]) + + const loaded = await createPublicGithubSourceClient({ fetch: served as unknown as typeof fetch }).load(resolvedSource()) + + expect(loaded._tag).toBe('loaded') + if (loaded._tag !== 'loaded') + 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') + }) + + 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') + + const loaded = await createPublicGithubSourceClient({ fetch: served as unknown as typeof fetch }) + .load({ ...resolvedSource(), skillPath: '.' }) + + 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`, + }], + }, + }) + }) + + function omission(path: string, bytes: number) { + return { path, bytes, url: `https://github.com/skilld-dev/skills/blob/${commitSha}/skills/demo/${path}` } + } + + /** 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') }) + }) + } +}) + describe('projected archive size', () => { it.each([ [[10]], diff --git a/test/unit/artifact-spec-check.test.ts b/test/unit/artifact-spec-check.test.ts index 748a1e3c..215cdae3 100644 --- a/test/unit/artifact-spec-check.test.ts +++ b/test/unit/artifact-spec-check.test.ts @@ -60,12 +60,6 @@ describe('agent Skills specification check', () => { expect(specResult(checked.checkResults)).toMatchObject({ outcome: 'pass', required: false }) }) - - it('still blocks a Skill that contains private key material', async () => { - const checked = await check('skills/demo', skillFile('---\nname: demo\ndescription: Demo.\n---\n-----BEGIN PRIVATE KEY-----\n')) - - expect(checksBlockArtifact(checked.checkResults)).toBe(true) - }) }) async function check(skillPath: string, file: ArtifactSourceFile) { diff --git a/test/unit/private-artifact-delivery.test.ts b/test/unit/private-artifact-delivery.test.ts index 75463c75..5c154e75 100644 --- a/test/unit/private-artifact-delivery.test.ts +++ b/test/unit/private-artifact-delivery.test.ts @@ -557,7 +557,8 @@ function requiredChecks() { return [ { name: 'path-policy', version: '1', outcome: 'pass' as const, required: true }, { name: 'agent-skills-spec', version: '2026-10-07', outcome: 'pass' as const, required: false }, - { name: 'credential-material', version: '1', outcome: 'pass' as const, required: true }, + { name: 'credential-material', version: '2', outcome: 'pass' as const, required: true }, { name: 'executable-files', version: '1', outcome: 'pass' as const, required: false }, + { name: 'omitted-files', version: '1', outcome: 'pass' as const, required: false }, ] }