diff --git a/config/scripts/generate-skill-bundle-manifest.mjs b/config/scripts/generate-skill-bundle-manifest.mjs index 4ef40d68144..b091d17c5f8 100644 --- a/config/scripts/generate-skill-bundle-manifest.mjs +++ b/config/scripts/generate-skill-bundle-manifest.mjs @@ -139,14 +139,17 @@ async function collectPackageFiles(packageRoot) { if (!fileStat.isFile()) { throw new Error(`Special file is not allowed in a shipped skill: ${manifestPath}`) } - files.push( - describeFile(manifestPath, await readFile(absolutePath), (fileStat.mode & 0o111) !== 0) - ) + // Why: Windows observation cannot see execute bits, so an executable file in + // a shipped skill would misclassify every pristine Windows install as unrecognized. + if ((fileStat.mode & 0o111) !== 0) { + throw new Error(`Executable file is not allowed in a shipped skill: ${manifestPath}`) + } + files.push(describeFile(manifestPath, await readFile(absolutePath), false)) } } await visit(packageRoot) - return files + return sortManifestFiles(files) } function collectGitPackageFiles(ref, name) { @@ -156,7 +159,7 @@ function collectGitPackageFiles(ref, name) { .split('\0') .filter(Boolean) const caseFoldedPaths = new Map() - return output.map((line) => { + const files = output.map((line) => { const match = /^(\d+) (\w+) ([a-f0-9]+)\t(.+)$/.exec(line) if (!match) { throw new Error(`Unexpected git tree entry at ${ref}: ${line}`) @@ -179,6 +182,28 @@ function collectGitPackageFiles(ref, name) { mode === '100755' ) }) + // Why: git ls-tree emits git byte-order, not the canonical walk order. + return sortManifestFiles(files) +} + +// Why: snapshot matching compares files by array index, so every producer — +// working-tree walk, git history, and runtime observation — must emit one +// canonical order. This mirrors the sorted depth-first filesystem walk. +function compareManifestPaths(left, right) { + const leftParts = left.split('/') + const rightParts = right.split('/') + const shared = Math.min(leftParts.length, rightParts.length) + for (let index = 0; index < shared; index += 1) { + const order = leftParts[index].localeCompare(rightParts[index], 'en') + if (order !== 0) { + return order + } + } + return leftParts.length - rightParts.length +} + +function sortManifestFiles(files) { + return [...files].sort((left, right) => compareManifestPaths(left.path, right.path)) } function packageDigest(files) { @@ -273,6 +298,9 @@ function buildReleasedHistory() { async function buildArtifacts(appVersion) { const { registry, mapping } = buildReleasedHistory() + const releasedSnapshotCounts = Object.fromEntries( + Object.entries(registry.skills).map(([name, snapshots]) => [name, snapshots.length]) + ) const skillDirectories = (await readdir(SKILLS_ROOT, { withFileTypes: true })) .filter((entry) => entry.isDirectory()) .map((entry) => entry.name) @@ -312,7 +340,44 @@ async function buildArtifacts(appVersion) { skills: currentSkills }, snapshotRegistry: registry, - releaseMapping: mapping + releaseMapping: mapping, + releasedSnapshotCounts + } +} + +// Why: released snapshots are the detection ground truth for existing installs, +// so a generation-logic change must not rewrite them silently. Only the one +// unreleased working-tree append per skill may change between runs. +function assertReleasedHistoryPreserved(committedRegistry, artifacts) { + if (!committedRegistry || committedRegistry.schemaVersion !== SCHEMA_VERSION) { + return + } + for (const [name, committedSnapshots] of Object.entries(committedRegistry.skills ?? {})) { + const releasedCount = artifacts.releasedSnapshotCounts[name] ?? 0 + const regenerated = artifacts.snapshotRegistry.skills[name] ?? [] + const protectedCount = Math.min(committedSnapshots.length, releasedCount) + for (let index = 0; index < protectedCount; index += 1) { + const committed = committedSnapshots[index] + const rebuilt = regenerated[index] + if ( + !rebuilt || + rebuilt.releaseRevision !== committed.releaseRevision || + rebuilt.packageDigest !== committed.packageDigest + ) { + throw new Error( + `Released snapshot history changed for ${name} at revision ${committed.releaseRevision}. ` + + 'Released snapshots are append-only; a deliberate identity migration must update this check.' + ) + } + } + } +} + +async function readCommittedRegistry() { + try { + return JSON.parse(await readFile(SNAPSHOT_REGISTRY_PATH, 'utf8')) + } catch { + return null } } @@ -358,6 +423,7 @@ async function verifyArtifacts(artifacts) { async function main() { const packageJson = JSON.parse(await readFile(path.join(REPO_ROOT, 'package.json'), 'utf8')) const artifacts = await buildArtifacts(packageJson.version) + assertReleasedHistoryPreserved(await readCommittedRegistry(), artifacts) await (process.argv.includes('--write') ? writeArtifacts : verifyArtifacts)(artifacts) } @@ -369,13 +435,16 @@ if (process.argv[1] && path.resolve(process.argv[1]) === import.meta.filename) { } export { + assertReleasedHistoryPreserved, buildArtifacts, buildReleasedHistory, classifyFile, collectPackageFiles, + describeFile, gitTreeSha, normalizeText, packageDigest, + sortManifestFiles, verifyArtifacts, writeArtifacts } diff --git a/config/scripts/generate-skill-bundle-manifest.test.mjs b/config/scripts/generate-skill-bundle-manifest.test.mjs index 397f495b563..e731fcf10ff 100644 --- a/config/scripts/generate-skill-bundle-manifest.test.mjs +++ b/config/scripts/generate-skill-bundle-manifest.test.mjs @@ -4,11 +4,14 @@ import { tmpdir } from 'node:os' import path from 'node:path' import { afterEach, describe, expect, it } from 'vitest' import { + assertReleasedHistoryPreserved, classifyFile, collectPackageFiles, + describeFile, gitTreeSha, normalizeText, - packageDigest + packageDigest, + sortManifestFiles } from './generate-skill-bundle-manifest.mjs' const temporaryDirectories = [] @@ -39,22 +42,91 @@ describe('skill bundle manifest generator', () => { expect(classifyFile(Buffer.from([0xc3, 0x28]))).toBe('binary') }) - it('uses normalized text identity but exact executable identity', async () => { - const packageRoot = await createPackage() - await writeFile(path.join(packageRoot, 'SKILL.md'), 'line one\r\nline two\r\n') - await writeFile(path.join(packageRoot, 'run.sh'), '#!/bin/sh\necho ok\n') - await chmod(path.join(packageRoot, 'run.sh'), 0o755) - - const files = await collectPackageFiles(packageRoot) - const skillFile = files.find((file) => file.path === 'SKILL.md') - const executable = files.find((file) => file.path === 'run.sh') + it('uses normalized text identity but exact executable identity', () => { + const skillFile = describeFile('SKILL.md', Buffer.from('line one\r\nline two\r\n'), false) + const executable = describeFile('run.sh', Buffer.from('#!/bin/sh\necho ok\n'), true) expect(skillFile.identitySha256).toBe(skillFile.textNormalizedSha256) expect(skillFile.identitySha256).not.toBe(skillFile.exactSha256) expect(executable.identitySha256).toBe(executable.exactSha256) - expect(packageDigest(files)).toMatch(/^[a-f0-9]{64}$/) + expect(packageDigest([skillFile, executable])).toMatch(/^[a-f0-9]{64}$/) }) + it('orders git-history files identically to the filesystem walk', async () => { + const packageRoot = await createPackage() + await mkdir(path.join(packageRoot, 'sub')) + for (const name of ['apple.md', 'sub.md', 'Zebra.md', path.join('sub', 'inner.txt')]) { + await writeFile(path.join(packageRoot, name), `${name}\n`) + } + const walked = await collectPackageFiles(packageRoot) + + // Why: git ls-tree emits [Zebra.md, apple.md, sub.md, sub/inner.txt]; index-based + // snapshot matching requires history and observation to share one order. + const gitOrdered = ['Zebra.md', 'apple.md', 'sub.md', 'sub/inner.txt'].map((manifestPath) => + walked.find((file) => file.path === manifestPath) + ) + + expect(sortManifestFiles(gitOrdered)).toEqual(walked) + expect(packageDigest(sortManifestFiles(gitOrdered))).toBe(packageDigest(walked)) + expect(walked.map((file) => file.path)).toEqual([ + 'apple.md', + 'sub/inner.txt', + 'sub.md', + 'Zebra.md' + ]) + }) + + it('rejects rewrites of released snapshots and allows floating-tail replacement', () => { + const snapshot = (releaseRevision, packageDigest) => ({ releaseRevision, packageDigest }) + const artifacts = { + releasedSnapshotCounts: { 'orca-cli': 2 }, + snapshotRegistry: { + schemaVersion: 1, + skills: { 'orca-cli': [snapshot(1, 'aaa'), snapshot(2, 'bbb'), snapshot(3, 'ccc')] } + } + } + + expect(() => + assertReleasedHistoryPreserved( + { schemaVersion: 1, skills: { 'orca-cli': [snapshot(1, 'aaa'), snapshot(2, 'bbb')] } }, + artifacts + ) + ).not.toThrow() + expect(() => + assertReleasedHistoryPreserved( + { + schemaVersion: 1, + skills: { 'orca-cli': [snapshot(1, 'aaa'), snapshot(2, 'bbb'), snapshot(3, 'stale')] } + }, + artifacts + ) + ).not.toThrow() + expect(() => + assertReleasedHistoryPreserved( + { + schemaVersion: 1, + skills: { 'orca-cli': [snapshot(1, 'aaa'), snapshot(2, 'rewritten')] } + }, + artifacts + ) + ).toThrow('Released snapshot history changed for orca-cli at revision 2') + expect(() => assertReleasedHistoryPreserved(null, artifacts)).not.toThrow() + }) + + it.runIf(process.platform !== 'win32')( + 'rejects executable files in shipped skill packages', + async () => { + const packageRoot = await createPackage() + await writeFile(path.join(packageRoot, 'SKILL.md'), 'skill\n') + await writeFile(path.join(packageRoot, 'run.sh'), '#!/bin/sh\necho ok\n') + await chmod(path.join(packageRoot, 'run.sh'), 0o755) + + await expect(collectPackageFiles(packageRoot)).rejects.toThrow( + 'Executable file is not allowed in a shipped skill: run.sh' + ) + } + ) + it.runIf(process.platform === 'linux')('rejects case-colliding paths', async () => { const packageRoot = await createPackage() await writeFile(path.join(packageRoot, 'SKILL.md'), 'skill')