Skip to content

Commit 4eec940

Browse files
authored
fix(artifact-delivery): run Skills that ship media or doc keys (#487)
1 parent 6740e29 commit 4eec940

15 files changed

Lines changed: 559 additions & 64 deletions

‎layers/artifact-delivery/server/utils/build.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -569,7 +569,7 @@ async function loadAndCheck(
569569
const loaded = await github.load(source)
570570
if (loaded._tag === 'rejected')
571571
return { _tag: 'rejected', rejection: loaded }
572-
const checked = await checkArtifactSource(source, loaded.value.files)
572+
const checked = await checkArtifactSource(source, loaded.value.files, loaded.value.omitted)
573573
return { _tag: 'loaded', source, files: loaded.value.files, checked }
574574
}
575575

‎layers/artifact-delivery/server/utils/checks.ts‎

Lines changed: 84 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import type { ArtifactFile, CheckResult, ResolvedSource } from '../schemas/contracts'
2-
import type { ArtifactSourceFile } from './github-source'
2+
import type { ArtifactSourceFile, OmittedArtifactFile } from './github-source'
33
import { digestHex } from './encoding'
44
import { ARTIFACT_POLICY_VERSION } from './state'
55

@@ -12,8 +12,24 @@ import { ARTIFACT_POLICY_VERSION } from './state'
1212
const AGENT_SKILLS_CHECK_VERSION = '2026-10-07'
1313
const MAX_DESCRIPTION_CHARACTERS = 1024
1414
const PATH_POLICY_VERSION = '1'
15-
const CREDENTIAL_MATERIAL_VERSION = '1'
15+
/**
16+
* Version 2, 2026-10-07: a finding needs a whole key block, not its first
17+
* line. Version 1 blocked nine Skills for a header alone: a secret scanner's
18+
* pattern table, a `"-----BEGIN PRIVATE KEY-----\n..."` placeholder, and a
19+
* test fixture whose body read `TEST-NOT-A-REAL-KEY`. None held a key.
20+
*/
21+
const CREDENTIAL_MATERIAL_VERSION = '2'
1622
const EXECUTABLE_FILES_VERSION = '1'
23+
/**
24+
* Lists the files left out of the Artifact for a size limit. It is a check
25+
* result, not a new attestation field: the released skilld CLI refuses an
26+
* attestation or a Resolution answer with a field it does not know, and it
27+
* accepts any check result that is not required.
28+
*/
29+
const OMITTED_FILES_VERSION = '1'
30+
/** The skilld CLI refuses a check result with more findings, or a longer one. */
31+
const MAX_CHECK_FINDINGS = 100
32+
const MAX_CHECK_FINDING_CHARACTERS = 500
1733

1834
/** The checks a statement under one policy carries, by check name. */
1935
export type ArtifactCheckSet = ReadonlyMap<string, { version: string, required: boolean }>
@@ -23,6 +39,7 @@ const CURRENT_ARTIFACT_CHECKS: ArtifactCheckSet = new Map([
2339
['agent-skills-spec', { version: AGENT_SKILLS_CHECK_VERSION, required: false }],
2440
['credential-material', { version: CREDENTIAL_MATERIAL_VERSION, required: true }],
2541
['executable-files', { version: EXECUTABLE_FILES_VERSION, required: false }],
42+
['omitted-files', { version: OMITTED_FILES_VERSION, required: false }],
2643
])
2744

2845
/**
@@ -40,9 +57,9 @@ const CURRENT_ARTIFACT_CHECKS: ArtifactCheckSet = new Map([
4057
*/
4158
export const SIGNABLE_ARTIFACT_POLICIES: ReadonlyMap<string, ArtifactCheckSet> = new Map([
4259
[ARTIFACT_POLICY_VERSION, CURRENT_ARTIFACT_CHECKS],
43-
['2026-08-20.1', new Map([
60+
['2026-10-07.1', new Map([
4461
['path-policy', { version: '1', required: true }],
45-
['agent-skills-spec', { version: '2026-08-20', required: true }],
62+
['agent-skills-spec', { version: '2026-10-07', required: false }],
4663
['credential-material', { version: '1', required: true }],
4764
['executable-files', { version: '1', required: false }],
4865
])],
@@ -56,6 +73,7 @@ export interface CheckedArtifactSource {
5673
export async function checkArtifactSource(
5774
source: ResolvedSource,
5875
files: ArtifactSourceFile[],
76+
omitted: OmittedArtifactFile[] = [],
5977
): Promise<CheckedArtifactSource> {
6078
const fileInventory = await Promise.all(files.map(async file => ({
6179
path: file.path,
@@ -136,7 +154,7 @@ export async function checkArtifactSource(
136154
const text = decodeText(file.bytes)
137155
if (!text)
138156
continue
139-
if (/-----BEGIN (?:[A-Z0-9 ]+ )?PRIVATE KEY-----/.test(text))
157+
if (containsPrivateKey(text))
140158
credentialFindings.push(`${file.path} contains private key material.`)
141159
}
142160
const credentialMaterial: CheckResult = credentialFindings.length > 0
@@ -174,7 +192,30 @@ export async function checkArtifactSource(
174192

175193
return {
176194
files: fileInventory,
177-
checkResults: [pathPolicy, agentSkillsSpec, credentialMaterial, executableFiles],
195+
checkResults: [pathPolicy, agentSkillsSpec, credentialMaterial, executableFiles, omittedFilesResult(omitted)],
196+
}
197+
}
198+
199+
function omittedFilesResult(omitted: OmittedArtifactFile[]): CheckResult {
200+
if (omitted.length === 0)
201+
return { name: 'omitted-files', version: OMITTED_FILES_VERSION, outcome: 'pass', required: false }
202+
const count = omitted.length === 1
203+
? '1 file over the size limits was left out of the Artifact.'
204+
: `${omitted.length} files over the size limits were left out of the Artifact.`
205+
const listed = omitted.length > MAX_CHECK_FINDINGS ? ` The first ${MAX_CHECK_FINDINGS} are listed.` : ''
206+
return {
207+
name: 'omitted-files',
208+
version: OMITTED_FILES_VERSION,
209+
outcome: 'warn',
210+
required: false,
211+
summary: count + listed,
212+
findings: omitted.slice(0, MAX_CHECK_FINDINGS).map((file) => {
213+
const finding = `${file.path}: ${file.bytes.toLocaleString('en-US')} bytes, ${file.url}`
214+
// A long path drops the URL first: the path is what a reader needs.
215+
return finding.length <= MAX_CHECK_FINDING_CHARACTERS
216+
? finding
217+
: `${file.path}: ${file.bytes.toLocaleString('en-US')} bytes`.slice(0, MAX_CHECK_FINDING_CHARACTERS)
218+
}),
178219
}
179220
}
180221

@@ -198,6 +239,43 @@ export function checksPermitSigning(checks: CheckResult[], policyVersion: string
198239
&& checks.every(check => !check.required || check.outcome === 'pass')
199240
}
200241

242+
/**
243+
* A PEM private key block: the BEGIN line, a body with no dashes, and the END
244+
* line with the same label. The body bound keeps one match linear; a 4096-bit
245+
* RSA key is about 3,300 characters.
246+
*/
247+
const PRIVATE_KEY_BLOCK = /-----BEGIN ((?:[A-Z0-9]+ )*)PRIVATE KEY-----((?:(?!-----)[\s\S]){0,16384})-----END \1PRIVATE KEY-----/g
248+
/** A line break, real or escaped inside a JSON or code string. */
249+
const BODY_LINE_BREAK = /\r?\n|(?:\\+[rn])+/
250+
/** A legacy PEM header such as `Proc-Type: 4,ENCRYPTED`. */
251+
const PEM_HEADER_LINE = /^[\w-]+:\s/
252+
/**
253+
* Base64 of at least 48 bytes: an Ed25519 PKCS#8 key, the smallest private
254+
* key format, is exactly 64 characters.
255+
*/
256+
const KEY_BODY = /^[A-Z0-9+/]{64,}={0,2}$/i
257+
258+
/**
259+
* True when the text holds a complete private key, encrypted or not.
260+
*
261+
* A placeholder has the BEGIN line but no key: an ellipsis, a bracketed note,
262+
* or a test label in the body never reads as base64. The body may sit in a
263+
* JSON string with escaped line breaks, which is how a leaked service account
264+
* key usually arrives, or be indented inside YAML.
265+
*/
266+
function containsPrivateKey(text: string): boolean {
267+
for (const match of text.matchAll(PRIVATE_KEY_BLOCK)) {
268+
const body = match[2]!
269+
.split(BODY_LINE_BREAK)
270+
.map(line => line.trim())
271+
.filter(line => !PEM_HEADER_LINE.test(line))
272+
.join('')
273+
if (KEY_BODY.test(body))
274+
return true
275+
}
276+
return false
277+
}
278+
201279
function decodeText(bytes: Uint8Array): string | null {
202280
try {
203281
return new TextDecoder('utf-8', { fatal: true, ignoreBOM: false }).decode(bytes)

0 commit comments

Comments
 (0)