mirror of
https://github.com/stablyai/orca.git
synced 2026-10-26 08:02:44 +00:00
fix(skills): canonicalize snapshot file order and guard released history
Historical snapshots kept git ls-tree byte-order while the working-tree walk and runtime observation use the sorted depth-first order, so any future multi-file skill would misclassify older installs as unrecognized and churn spurious registry revisions; all producers now share one canonical order (no digest changes for today's single-file packages). Also rejects executable files from shipped skills (Windows observation cannot see execute bits, which would misclassify pristine Windows installs) and adds an explicit append-only invariant for released snapshots so a generation-logic change cannot rewrite them silently.
This commit is contained in:
@@ -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
|
||||
}
|
||||
|
||||
@@ -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')
|
||||
|
||||
Reference in New Issue
Block a user