diff --git a/apps/server/src/modules/storage/backends/disk/blob-store.test.ts b/apps/server/src/modules/storage/backends/disk/blob-store.test.ts index 7d287192..f60e8b92 100644 --- a/apps/server/src/modules/storage/backends/disk/blob-store.test.ts +++ b/apps/server/src/modules/storage/backends/disk/blob-store.test.ts @@ -3,10 +3,13 @@ import { existsSync, + mkdirSync, mkdtempSync, readFileSync, readdirSync, rmSync, + symlinkSync, + writeFileSync, } from 'node:fs'; import { tmpdir } from 'node:os'; import path from 'node:path'; @@ -133,4 +136,26 @@ describe('DiskBlobStore temp file hygiene', () => { rmSync(otherRoot, { recursive: true, force: true }); } }); + + it.skipIf(process.platform === 'win32')( + 'refuses to delete blobs through a symlinked scope root', + async () => { + const target = mkdtempSync(path.join(tmpdir(), 'huabu-blob-outside-')); + const artifact = path.join(target, '.artifacts', 'keep.bin'); + mkdirSync(path.dirname(artifact), { recursive: true }); + writeFileSync(artifact, 'bytes'); + symlinkSync(target, path.join(root, 'symlink-canvas'), 'dir'); + + try { + const scope = new DiskBlobStore().scope({ + kind: 'canvas', + canvasId: 'symlink-canvas', + }); + await expect(scope.deleteAll()).rejects.toThrow(/symbolic link/i); + expect(existsSync(artifact)).toBe(true); + } finally { + rmSync(target, { recursive: true, force: true }); + } + }, + ); }); diff --git a/apps/server/src/modules/storage/backends/disk/blob-store.ts b/apps/server/src/modules/storage/backends/disk/blob-store.ts index 88dd8eb6..efb3b459 100644 --- a/apps/server/src/modules/storage/backends/disk/blob-store.ts +++ b/apps/server/src/modules/storage/backends/disk/blob-store.ts @@ -14,7 +14,12 @@ */ import { randomUUID } from 'node:crypto'; -import { createReadStream, createWriteStream } from 'node:fs'; +import { + createReadStream, + createWriteStream, + existsSync, + lstatSync, +} from 'node:fs'; import { mkdir, readdir, @@ -26,7 +31,12 @@ import { import path from 'node:path'; import { pipeline } from 'node:stream/promises'; -import { artifactsDir } from './layout.js'; +import { destructiveCanvasDirName } from './canvas-dirs.js'; +import { + artifactsDir, + ARTIFACTS_DIR_NAME, + SPACE_JSON_FILENAME, +} from './layout.js'; import { renameOverWithRetry } from '../../../../utils/fs.js'; import { getWorkspacePath } from '../../../workspace.js'; import { createBlobLease, normalizeBlobName } from '../../ports/blob.js'; @@ -94,6 +104,43 @@ class DiskBlobScope implements BlobScope { return scopeDir(this.#ref); } + #resolveDeleteDir(): string | null { + const active = path.resolve(getWorkspacePath()); + if (active !== this.#workspacePath) { + throw new Error( + `DiskBlobScope(${this.#ref.canvasId}) belongs to an inactive workspace. ` + + `Resolve a fresh scope after workspace activation.`, + ); + } + const target = destructiveCanvasDirName(this.#ref.canvasId); + if (target === null) return null; + const root = path.resolve(this.#workspacePath, target.filename); + const resolved = path.join(root, ARTIFACTS_DIR_NAME); + if (!resolved.startsWith(`${this.#workspacePath}${path.sep}`)) { + throw new Error( + `Blob scope escapes the active Workspace: "${this.#ref.canvasId}"`, + ); + } + try { + if (lstatSync(root).isSymbolicLink()) { + throw new Error( + `Refusing to delete blobs through a symbolic link: "${root}"`, + ); + } + } catch (error) { + if (!isMissing(error)) throw error; + } + // A Space may have appeared in a previously orphaned directory after the + // fresh scan. Refuse before the first await instead of deleting its blobs. + if ( + target.kind === 'orphan' && + existsSync(path.join(root, SPACE_JSON_FILENAME)) + ) { + return null; + } + return resolved; + } + async #headAt(dir: string, name: string): Promise { const safe = normalizeBlobName(name); try { @@ -229,7 +276,9 @@ class DiskBlobScope implements BlobScope { } async deleteAll(): Promise { - await rm(this.#resolveDir(), { recursive: true, force: true }); + const dir = this.#resolveDeleteDir(); + if (dir === null) return; + await rm(dir, { recursive: true, force: true }); } } diff --git a/apps/server/src/modules/storage/backends/disk/canvas-dirs.ts b/apps/server/src/modules/storage/backends/disk/canvas-dirs.ts index 1780e106..2fcb1f03 100644 --- a/apps/server/src/modules/storage/backends/disk/canvas-dirs.ts +++ b/apps/server/src/modules/storage/backends/disk/canvas-dirs.ts @@ -10,7 +10,11 @@ import { existsSync, readdirSync, renameSync, statSync } from 'node:fs'; import path from 'node:path'; -import { SPACE_JSON_FILENAME, WORLD_CANVAS_DIR_NAME } from './layout.js'; +import { + SPACE_JSON_FILENAME, + WORLD_CANVAS_DIR_NAME, + WORKSPACE_SETTING_DIR_NAME, +} from './layout.js'; import { NameIndex, type NameIndexResult } from './name-index.js'; import { readJsonStrict, sanitizeId } from '../../../../utils/fs.js'; import { @@ -149,6 +153,43 @@ export function canvasDirName(canvasId: string): string { return index.get(canvasId)?.filename ?? canvasId; } +/** + * Resolve a directory for destructive blob cleanup. + * + * Missing stable ids may still own blobs in the legacy id-named directory, + * but that fallback must never alias another Space or a Workspace-owned + * directory. Destructive callers get a fresh scan so a stale name index + * cannot authorize the wrong target. + */ +export type DestructiveCanvasDirTarget = + | { readonly kind: 'owned'; readonly filename: string } + | { readonly kind: 'orphan'; readonly filename: string }; + +export function destructiveCanvasDirName( + canvasId: string, +): DestructiveCanvasDirTarget | null { + const safeId = sanitizeId(canvasId, 'canvasId'); + scanWorkspace(); + if (worldEntry?.id === safeId) { + return { kind: 'owned', filename: WORLD_CANVAS_DIR_NAME }; + } + + const owned = index.get(safeId); + if (owned) return { kind: 'owned', filename: owned.filename }; + // NameIndex's secondary key is the normalized on-disk filename, not the + // display title. A hit here means the fallback belongs to another Space. + if (index.findByName(safeId)) return null; + + const normalized = normalizeForCompare(safeId); + if ( + normalized === normalizeForCompare(WORLD_CANVAS_DIR_NAME) || + normalized === normalizeForCompare(WORKSPACE_SETTING_DIR_NAME) + ) { + return null; + } + return { kind: 'orphan', filename: safeId }; +} + /** Ordinary user-visible Spaces only. */ export function listCanvasDirEntries(): CanvasDirEntry[] { ensureScanned(); diff --git a/apps/server/src/modules/storage/backends/disk/canvas-dirs.world.test.ts b/apps/server/src/modules/storage/backends/disk/canvas-dirs.world.test.ts index e71c32fd..3b9bb2b1 100644 --- a/apps/server/src/modules/storage/backends/disk/canvas-dirs.world.test.ts +++ b/apps/server/src/modules/storage/backends/disk/canvas-dirs.world.test.ts @@ -21,6 +21,7 @@ vi.mock('../../../workspace.js', () => ({ import { canvasDirName, + destructiveCanvasDirName, getWorldCanvasId, isWorldCanvasId, listAllCanvasDirEntries, @@ -119,4 +120,36 @@ describe('World canvas directory indexing', () => { 'World canvas is missing or malformed', ); }); + + it('resolves destructive fallbacks by stable ownership and disk filename', () => { + writeCanvas( + workspaceState.path, + 'Alias_Victim', + 'canvas-alias', + 'Alias/Victim', + ); + + expect(destructiveCanvasDirName('canvas-alias')).toEqual({ + kind: 'owned', + filename: 'Alias_Victim', + }); + expect(destructiveCanvasDirName('Alias_Victim')).toBeNull(); + expect(destructiveCanvasDirName('SETTING')).toBeNull(); + expect(destructiveCanvasDirName('canvas-orphan')).toEqual({ + kind: 'orphan', + filename: 'canvas-orphan', + }); + }); + + it('refreshes a warm index before authorizing a destructive fallback', () => { + expect(listCanvasDirEntries()).toHaveLength(1); + writeCanvas( + workspaceState.path, + 'FreshAlias', + 'canvas-fresh', + 'FreshAlias', + ); + + expect(destructiveCanvasDirName('FreshAlias')).toBeNull(); + }); }); diff --git a/apps/server/src/modules/storage/backends/disk/layout.ts b/apps/server/src/modules/storage/backends/disk/layout.ts index 35afbdbd..d9d3168a 100644 --- a/apps/server/src/modules/storage/backends/disk/layout.ts +++ b/apps/server/src/modules/storage/backends/disk/layout.ts @@ -56,6 +56,7 @@ export function canvasRoot(canvasId: string): string { */ export const SPACE_JSON_FILENAME = 'space.json'; export const WORLD_CANVAS_DIR_NAME = '.world'; +export const WORKSPACE_SETTING_DIR_NAME = 'setting'; export function canvasJsonPath(canvasId: string): string { return path.join(canvasRoot(canvasId), SPACE_JSON_FILENAME); diff --git a/apps/server/src/modules/storage/backends/disk/legacy/canvas-store.ts b/apps/server/src/modules/storage/backends/disk/legacy/canvas-store.ts index e852553d..acb7431e 100644 --- a/apps/server/src/modules/storage/backends/disk/legacy/canvas-store.ts +++ b/apps/server/src/modules/storage/backends/disk/legacy/canvas-store.ts @@ -63,6 +63,7 @@ import { eventsPath, nodeFilePath, nodesDir, + SPACE_JSON_FILENAME, } from '../layout.js'; import { NameIndex } from '../name-index.js'; import { readValidCanvasFile } from '../space-record-validation.js'; @@ -1515,11 +1516,16 @@ export class CanvasStore { /** Recursively delete the entire canvas directory. */ destroy(): boolean { this.assertActiveWorkspace(); + refreshCanvasDirIndex(); if (isWorldCanvasId(this.canvasId)) { throw new Error('World canvas cannot be deleted'); } const root = canvasRoot(this.canvasId); - if (!existsSync(root)) { + const record = readValidCanvasFile( + path.join(root, SPACE_JSON_FILENAME), + this.canvasId, + ); + if (record === null) { unregisterCanvasDir(this.canvasId); this.invalidateNodeIndex(); clearSpaceNodeTombstones(this.#workspacePath, this.canvasId); diff --git a/apps/server/src/modules/storage/backends/disk/space-repository.test.ts b/apps/server/src/modules/storage/backends/disk/space-repository.test.ts index 9f19aff1..767284c9 100644 --- a/apps/server/src/modules/storage/backends/disk/space-repository.test.ts +++ b/apps/server/src/modules/storage/backends/disk/space-repository.test.ts @@ -257,6 +257,31 @@ describe('DiskSpaceRepository membership', () => { await expect(held.worldId()).rejects.toThrow(/inactive workspace/i); await expect(new DiskSpaceRepository().worldId()).resolves.toBe('world-b'); }); + + it('releases deletion admission when a queued session resumes in another Workspace', async () => { + const firstRoot = makeWorkspace('huabu-space-delete-release-a-'); + seedWorld(firstRoot, 'world-a'); + seedSpace(firstRoot, 'canvas-a', 'Alpha'); + const spaces = new DiskSpaceRepository(); + const first = await spaces.beginDelete({ canvasId: 'canvas-a' }); + if (!first.ok) throw new Error('Expected ordinary Space deletion session'); + + const queued = spaces.beginDelete({ canvasId: 'canvas-a' }); + await Promise.resolve(); + const secondRoot = makeWorkspace('huabu-space-delete-release-b-'); + seedWorld(secondRoot, 'world-b'); + await first.session.abort(); + + await expect(queued).rejects.toThrow(/inactive workspace/i); + + workspaceState.path = firstRoot; + resetStorageCache(); + refreshCanvasDirIndex(); + const retry = await spaces.beginDelete({ canvasId: 'canvas-a' }); + if (!retry.ok) + throw new Error('Expected deletion admission to be released'); + await retry.session.abort(); + }); }); describe('DiskSpaceRepository lifecycle', () => { diff --git a/apps/server/src/modules/storage/backends/disk/space-repository.ts b/apps/server/src/modules/storage/backends/disk/space-repository.ts index ea8d3045..4b691485 100644 --- a/apps/server/src/modules/storage/backends/disk/space-repository.ts +++ b/apps/server/src/modules/storage/backends/disk/space-repository.ts @@ -29,6 +29,7 @@ import { forgetCanvasStore, getCanvasStore, } from './legacy/canvas-store-cache.js'; +import { clearSpaceNodeTombstones } from './legacy/node-tombstones.js'; import { withSpaceDirHandlesReleased } from './space-dir-handles.js'; import { readValidCanvasFile } from './space-record-validation.js'; import { readDiskSpaceRecord } from './space-record.js'; @@ -160,11 +161,22 @@ export class DiskSpaceRepository implements SpaceRepository { return { ok: false, reason: 'world-forbidden' }; } - const store = getCanvasStore(canvasId); const release = await beginSpaceDeleteAdmission( this.#workspacePath, canvasId, ); + let store: ReturnType | null; + try { + this.#assertActiveWorkspace(); + refreshCanvasDirIndex(); + const existed = listAllCanvasDirEntries().some( + (entry) => entry.id === canvasId, + ); + store = existed ? getCanvasStore(canvasId) : null; + } catch (error) { + release(); + throw error; + } let state: 'open' | 'finishing' | 'closed' = 'open'; const close = (): void => { if (state === 'closed') return; @@ -178,6 +190,11 @@ export class DiskSpaceRepository implements SpaceRepository { } state = 'finishing'; try { + if (!store) { + forgetCanvasStore(canvasId); + clearSpaceNodeTombstones(this.#workspacePath, canvasId); + return { ok: false as const, reason: 'not-found' as const }; + } const deleted = await withSpaceDirHandlesReleased(canvasId, () => store.destroy(), ); diff --git a/apps/server/src/modules/storage/compatibility/delete-canvas.test.ts b/apps/server/src/modules/storage/compatibility/delete-canvas.test.ts index 74118247..5e8c1f2d 100644 --- a/apps/server/src/modules/storage/compatibility/delete-canvas.test.ts +++ b/apps/server/src/modules/storage/compatibility/delete-canvas.test.ts @@ -255,6 +255,69 @@ describe('deleteSpace composition', () => { }); }); + it.each(['cold', 'warm'] as const)( + 'does not resolve an absent id through another Space directory (%s index)', + async (indexState) => { + const victimRoot = path.join(workspaceState.path, 'AliasVictim'); + const victimRecord = path.join(victimRoot, 'space.json'); + const victimArtifact = path.join(victimRoot, '.artifacts', 'keep-me.bin'); + writeCanvas('AliasVictim', 'canvas-victim', 'AliasVictim'); + mkdirSync(path.dirname(victimArtifact), { recursive: true }); + writeFileSync(victimArtifact, 'bytes'); + if (indexState === 'warm') { + await new DiskStructuredStore().spaces().list(); + } else { + refreshCanvasDirIndex(); + } + + await expect(deleteSpace('AliasVictim')).resolves.toEqual({ + ok: false, + reason: 'not-found', + }); + + expect(existsSync(victimRecord)).toBe(true); + expect(existsSync(victimArtifact)).toBe(true); + }, + ); + + it('does not delete the workspace setting directory for an absent id', async () => { + const settingFile = path.join(workspaceState.path, 'setting', 'user.md'); + const settingArtifact = path.join( + workspaceState.path, + 'setting', + '.artifacts', + 'keep-me.bin', + ); + mkdirSync(path.dirname(settingArtifact), { recursive: true }); + writeFileSync(settingFile, 'preference'); + writeFileSync(settingArtifact, 'bytes'); + + await expect(deleteSpace('setting')).resolves.toEqual({ + ok: false, + reason: 'not-found', + }); + + expect(existsSync(settingFile)).toBe(true); + expect(existsSync(settingArtifact)).toBe(true); + }); + + it('sweeps orphan blobs without treating their directory as a Space', async () => { + const orphanRoot = path.join(workspaceState.path, 'canvas-orphan'); + const orphanArtifact = path.join(orphanRoot, '.artifacts', 'orphan.bin'); + const unrelatedFile = path.join(orphanRoot, 'unrelated.txt'); + mkdirSync(path.dirname(orphanArtifact), { recursive: true }); + writeFileSync(orphanArtifact, 'bytes'); + writeFileSync(unrelatedFile, 'keep'); + + await expect(deleteSpace('canvas-orphan')).resolves.toEqual({ + ok: false, + reason: 'not-found', + }); + + expect(existsSync(orphanArtifact)).toBe(false); + expect(existsSync(unrelatedFile)).toBe(true); + }); + it('refuses the World canvas without touching its blobs', async () => { mkdirSync(path.dirname(artifactPath('canvas-world', 'art_w.png')), { recursive: true, diff --git a/docs/architecture/canvas-storage.md b/docs/architecture/canvas-storage.md index 87e3e596..05b1db5f 100644 --- a/docs/architecture/canvas-storage.md +++ b/docs/architecture/canvas-storage.md @@ -98,7 +98,7 @@ The Disk structured adapter and compatibility facade resolve the same cached leg Canvas persistence DTOs and the write coordinator live under `modules/canvas/`; Workspace identity, durable membership, and Disk locators live under `modules/storage/`, while active-Workspace lifecycle and boot migrations remain under `modules/workspace/`; generic filesystem and Markdown codecs live under `utils/`. `canvasRoot()` validates the identifier and then verifies that the resolved Space directory remains a strict descendant of the active Workspace before any downstream Disk operation receives it. `module-boundaries.test.ts` enforces the storage dependency direction and prevents new consumers of the forwarding shims. -Space deletion is serialized against composed blob puts by a writer-preferring admission coordinator and holds an active-Workspace lease across blob cleanup and structured destruction. `beginDelete()` acquires the exclusive session before blob I/O; `finish()` removes structured state, while `abort()` releases the fence without doing so. Blobs are swept before structure so a failed sweep can be retried while the Space record still names them. Puts already admitted may finish; a put queued behind a successful deletion rechecks existence and fails without recreating blobs, while a failed blob sweep leaves the record available for retry. Mutations through existing Space handles and repositories reject while deletion is active or queued; reads remain available for cleanup. Residual direct-filesystem capabilities such as ZIP import, RFS upload/delete, and external-note claim are outside this repository fence and remain blockers for a non-Disk profile. +Space deletion is serialized against composed blob puts by a writer-preferring admission coordinator and holds an active-Workspace lease across blob cleanup and structured destruction. `beginDelete()` acquires the exclusive session before blob I/O; `finish()` removes structured state, while `abort()` releases the fence without doing so. Blobs are swept before structure so a failed sweep can be retried while the Space record still names them. A missing stable id may sweep an orphan `.artifacts/` directory only when its id-named fallback does not alias an indexed Space, `.world`, or `setting`; it never authorizes recursive removal of the fallback directory itself. Puts already admitted may finish; a put queued behind a successful deletion rechecks existence and fails without recreating blobs, while a failed blob sweep leaves the record available for retry. Mutations through existing Space handles and repositories reject while deletion is active or queued; reads remain available for cleanup. Residual direct-filesystem capabilities such as ZIP import, RFS upload/delete, and external-note claim are outside this repository fence and remain blockers for a non-Disk profile. Retained Disk Space repository and handle instances, blob scopes, and legacy `CanvasStore` instances reject use after the active Workspace changes. Each `spaces()` call returns a fresh Workspace-bound handle and each read rescans current Disk state. The Workspace-qualified LRU is cleared and rebuilt on the next lookup after a switch. The delete-session contract covers overlapping operations through one configured backend instance. Disk realizes it with the shared process-local coordinator; it is not a multi-process transaction or distributed lock, and a SQL adapter must supply an equivalent backend-instance fence using its own mechanisms.