diff --git a/packages/storage/src/__tests__/artifact-stores.test.ts b/packages/storage/src/__tests__/artifact-stores.test.ts index d4399dbfe8..a5920ee0b9 100644 --- a/packages/storage/src/__tests__/artifact-stores.test.ts +++ b/packages/storage/src/__tests__/artifact-stores.test.ts @@ -18,11 +18,11 @@ */ import assert from 'node:assert/strict'; -import { mkdir, mkdtemp, rm, stat, writeFile } from 'node:fs/promises'; +import { mkdir, mkdtemp, readFile, rename, rm, stat, symlink, writeFile } from 'node:fs/promises'; import { DatabaseSync } from 'node:sqlite'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; -import { after, describe, test } from 'node:test'; +import { after, describe, test, type TestContext } from 'node:test'; import { authenticateInteractiveArtifactStoreWriter, openInteractiveArtifactStoreForWrite, @@ -309,6 +309,145 @@ describe('interactive artifact store authority', () => { }); }); + test('does not follow a replaced parent directory while reclaiming upgrade residue', async (t) => { + const outsideRoot = await mkdtemp(join(tmpdir(), 'maka-artifact-upgrade-outside-')); + try { + await withInteractiveOwner(async (owner, root, track) => { + const initial = await openInteractiveArtifactStoreForWrite(owner.lease); + initial.close(); + const relativePath = 'session-1/retired-payload.txt'; + const db = new DatabaseSync(join(root, 'runtime.sqlite')); + db.exec(` + DROP TABLE artifact_records; + CREATE TABLE artifact_records ( + storage_key TEXT PRIMARY KEY, artifact_id TEXT NOT NULL, + session_id TEXT NOT NULL, created_at INTEGER NOT NULL CHECK(created_at >= 0), + status TEXT NOT NULL CHECK(status IN ('live', 'deleted')), + relative_path TEXT NOT NULL, record_json TEXT NOT NULL + ); + CREATE UNIQUE INDEX artifact_records_relative_path ON artifact_records(relative_path); + UPDATE operational_schema_migrations SET version = 1 WHERE scope = 'artifact'; + `); + db.prepare('INSERT INTO artifact_records VALUES (?, ?, ?, ?, ?, ?, ?)').run( + 'retired', + 'retired', + 'session-1', + 1, + 'live', + relativePath, + JSON.stringify({ + id: 'retired', + sessionId: 'session-1', + turnId: 'turn-1', + createdAt: 1, + name: 'payload.txt', + kind: 'file', + sizeBytes: 8, + relativePath, + source: 'provider_request_capture', + status: 'live', + }), + ); + db.close(); + + const artifactRoot = join(root, 'artifacts'); + const sessionRoot = join(artifactRoot, 'session-1'); + await mkdir(sessionRoot, { recursive: true }); + await writeFile(join(artifactRoot, relativePath), 'original', 'utf8'); + const store = track(await openInteractiveArtifactStoreForWrite(owner.lease)); + const displacedSessionRoot = join(artifactRoot, 'displaced-session-1'); + await rename(sessionRoot, displacedSessionRoot); + const outsidePath = join(outsideRoot, 'retired-payload.txt'); + await writeFile(outsidePath, 'external', 'utf8'); + if (!(await createSymlinkOrSkip(t, outsideRoot, sessionRoot))) return; + + assert.deepEqual(await store.reclaimUpgradeResidue({ maxPaths: 64 }), { + nextAfter: null, + processedPaths: 1, + failedPaths: 1, + }); + + assert.equal(await readFile(outsidePath, 'utf8'), 'external'); + assert.equal( + await readFile(join(displacedSessionRoot, 'retired-payload.txt'), 'utf8'), + 'original', + ); + assert.deepEqual(readUpgradeOrphanPaths(root), [relativePath]); + }); + } finally { + await rm(outsideRoot, { recursive: true, force: true }); + } + }); + + test('does not reclaim an upgrade orphan path that aliases a live artifact', async (t) => { + await withInteractiveOwner(async (owner, root, track) => { + if (!(await isCaseInsensitiveFilesystem(root))) { + t.skip('requires a case-insensitive filesystem'); + return; + } + + const initial = await openInteractiveArtifactStoreForWrite(owner.lease); + initial.close(); + const liveRelativePath = 'session-1/SHARED-Payload.txt'; + const orphanRelativePath = 'session-1/shared-payload.txt'; + const db = new DatabaseSync(join(root, 'runtime.sqlite')); + db.exec(` + DROP TABLE artifact_records; + CREATE TABLE artifact_records ( + storage_key TEXT PRIMARY KEY, artifact_id TEXT NOT NULL, + session_id TEXT NOT NULL, created_at INTEGER NOT NULL CHECK(created_at >= 0), + status TEXT NOT NULL CHECK(status IN ('live', 'deleted')), + relative_path TEXT NOT NULL, record_json TEXT NOT NULL + ); + CREATE UNIQUE INDEX artifact_records_relative_path ON artifact_records(relative_path); + UPDATE operational_schema_migrations SET version = 1 WHERE scope = 'artifact'; + `); + for (const [storageKey, artifactId, status, relativePath, name] of [ + ['live', 'SHARED', 'live', liveRelativePath, 'Payload.txt'], + ['retired', 'shared', 'deleted', orphanRelativePath, 'payload.txt'], + ] as const) { + db.prepare('INSERT INTO artifact_records VALUES (?, ?, ?, ?, ?, ?, ?)').run( + storageKey, + artifactId, + 'session-1', + 1, + status, + relativePath, + JSON.stringify({ + id: artifactId, + sessionId: 'session-1', + turnId: 'turn-1', + createdAt: 1, + name, + kind: 'file', + sizeBytes: 10, + relativePath, + source: 'user_upload', + status, + }), + ); + } + db.close(); + + await mkdir(join(root, 'artifacts', 'session-1'), { recursive: true }); + await writeFile(join(root, 'artifacts', liveRelativePath), 'live bytes', 'utf8'); + const store = track(await openInteractiveArtifactStoreForWrite(owner.lease)); + assert.deepEqual(readUpgradeOrphanPaths(root), [orphanRelativePath]); + + assert.deepEqual(await store.reclaimUpgradeResidue({ maxPaths: 64 }), { + nextAfter: null, + processedPaths: 1, + failedPaths: 0, + }); + + assert.deepEqual(await store.readTextInSession('session-1', 'SHARED'), { + ok: true, + text: 'live bytes', + }); + assert.deepEqual(readUpgradeOrphanPaths(root), []); + }); + }); + test('requires authentic leases and writer facades', async () => { await assert.rejects( () => @@ -471,3 +610,46 @@ async function withTemporaryRoot( } type TrackArtifactWriter = (writer: T) => T; + +async function createSymlinkOrSkip(t: TestContext, target: string, path: string): Promise { + try { + await symlink(target, path, process.platform === 'win32' ? 'junction' : 'dir'); + return true; + } catch (error) { + const code = (error as { code?: unknown }).code; + if (process.platform === 'win32' && (code === 'EPERM' || code === 'EACCES')) { + t.skip('Windows symlink creation requires elevated privileges or Developer Mode'); + return false; + } + throw error; + } +} + +function readUpgradeOrphanPaths(root: string): string[] { + const database = new DatabaseSync(join(root, 'runtime.sqlite'), { readOnly: true }); + try { + return database + .prepare('SELECT relative_path FROM artifact_upgrade_orphan_paths ORDER BY relative_path') + .all() + .map((row) => (row as { relative_path: string }).relative_path); + } finally { + database.close(); + } +} + +async function isCaseInsensitiveFilesystem(directory: string): Promise { + const probe = join(directory, '.maka-case-sensitivity-probe'); + const alias = join(directory, '.MAKA-CASE-SENSITIVITY-PROBE'); + await writeFile(probe, 'probe', { flag: 'wx' }); + try { + return await stat(alias).then( + () => true, + (error: unknown) => { + if ((error as { code?: unknown }).code === 'ENOENT') return false; + throw error; + }, + ); + } finally { + await rm(probe, { force: true }); + } +} diff --git a/packages/storage/src/artifact-store.ts b/packages/storage/src/artifact-store.ts index 6bb8dea74f..f3da7b016a 100644 --- a/packages/storage/src/artifact-store.ts +++ b/packages/storage/src/artifact-store.ts @@ -506,6 +506,7 @@ class SqliteArtifactStore implements ArtifactAuthorityStore { const directories = new Set(); const discharged: string[] = []; let failedPaths = 0; + let realArtifactRoot: string | undefined; try { for (const relativePath of selected) { if ( @@ -515,10 +516,33 @@ class SqliteArtifactStore implements ArtifactAuthorityStore { discharged.push(relativePath); continue; } - const target = join(this.artifactRoot, relativePath); + const entry = await resolveArtifactRemovalEntry(this.artifactRoot, relativePath); + if (!entry) { + discharged.push(relativePath); + continue; + } + realArtifactRoot ??= await ensureRealDirectory(this.artifactRoot); + if (!isInsideOrSamePath(realArtifactRoot, dirname(entry.unlinkPath))) { + failedPaths += 1; + continue; + } + const artifactIds = new Set([ + ...artifactIdsFromUpgradeOrphanPath(relativePath), + ...artifactIdsFromUpgradeOrphanPath(entry.unlinkPath), + ]); + if ( + artifactIds.size > 0 && + (await this.hasClaimedRemovalIdentityUnlocked( + [...artifactIds], + entry.comparisonIdentity, + )) + ) { + discharged.push(relativePath); + continue; + } try { - await unlink(target); - directories.add(dirname(target)); + await unlink(entry.unlinkPath); + directories.add(dirname(entry.unlinkPath)); } catch (error) { if (!isNotFound(error)) { failedPaths += 1; @@ -539,6 +563,19 @@ class SqliteArtifactStore implements ArtifactAuthorityStore { }); } + private async hasClaimedRemovalIdentityUnlocked( + artifactIds: readonly string[], + comparisonIdentity: string, + ): Promise { + for (const relativePath of this.metadataRepository.readRelativePathsByCaseFoldedArtifactIds( + artifactIds, + )) { + const entry = await resolveArtifactRemovalEntry(this.artifactRoot, relativePath); + if (entry?.comparisonIdentity === comparisonIdentity) return true; + } + return false; + } + private async replayExistingArtifactUnlocked( existing: ArtifactRecord, input: CreateArtifactInput, @@ -1213,6 +1250,20 @@ function symlinkEntryIdentity(entryStat: BigIntStats): string { ].join(':'); } +function artifactIdsFromUpgradeOrphanPath(relativePath: string): readonly string[] { + const name = basename(relativePath); + const artifactIds: string[] = []; + for ( + let separator = name.indexOf('-'); + separator > 0; + separator = name.indexOf('-', separator + 1) + ) { + const artifactId = name.slice(0, separator); + if (isCanonicalArtifactEntityId(artifactId)) artifactIds.push(artifactId); + } + return artifactIds; +} + function isInsideOrSamePath(root: string, target: string): boolean { if (target === root) return true; const rel = relative(root, target); diff --git a/packages/storage/src/sqlite-artifact-metadata.ts b/packages/storage/src/sqlite-artifact-metadata.ts index db128968d9..df4094ef66 100644 --- a/packages/storage/src/sqlite-artifact-metadata.ts +++ b/packages/storage/src/sqlite-artifact-metadata.ts @@ -110,6 +110,19 @@ class SqliteArtifactMetadataRepository { ); } + readRelativePathsByCaseFoldedArtifactIds(artifactIds: readonly string[]): string[] { + this.assertOpen(); + if (artifactIds.length === 0) return []; + const placeholders = artifactIds.map(() => '?').join(', '); + const rows = this.#lease.database + .prepare( + `SELECT relative_path FROM artifact_records + WHERE artifact_id COLLATE NOCASE IN (${placeholders})`, + ) + .all(...artifactIds) as Array<{ relative_path: string }>; + return rows.map((row) => row.relative_path); + } + forgetUpgradeOrphanPaths(relativePaths: readonly string[]): void { this.assertOpen(); this.#lease.transaction('write', () => {