From 09c0232dc742339ea6be3a4d2103bedbd96b475a Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 17 Oct 2026 23:01:11 +0000 Subject: [PATCH] fix(cli,runtime): fold the two async database-directory walks onto each package's makeDirectorySync os dev's prepareDevDatabaseDirectory and the standalone host's makeSqliteDatabaseDirectory each carried an async copy of the bounded walk that accepted any EEXIST on a created level. A dangling symlink answers EEXIST to mkdir, so it went through to the driver and the boot failed later in a plugin's start, naming neither the file nor the directory. Both wrappers now call their package's makeDirectorySync, which accepts EEXIST only when a directory stands there; each keeps its own error text. A dangling symlink as the directory is refused at once, naming the file and the directory. Every other measured input class answers as before. Pins: one dangling-symlink case per package, plus the unchanged classes the existing pins did not cover (an existing directory, a live symlink to one, and, for dev, a file holding the directory's name). Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_01QyoWKK9RwUNxBxmevy9Neb --- .changeset/24090-db-dir-dangling-symlink.md | 14 +++++ .../commands/dev-database-directory.test.ts | 49 +++++++++++++++++- packages/cli/src/commands/dev.ts | 42 +++++---------- packages/cli/src/utils/make-directory.ts | 8 +-- packages/runtime/src/make-directory.ts | 10 ++-- ...one-stack-uncreatable-db-directory.test.ts | 42 ++++++++++++++- packages/runtime/src/standalone-stack.ts | 51 ++++++------------- 7 files changed, 140 insertions(+), 76 deletions(-) create mode 100644 .changeset/24090-db-dir-dangling-symlink.md diff --git a/.changeset/24090-db-dir-dangling-symlink.md b/.changeset/24090-db-dir-dangling-symlink.md new file mode 100644 index 0000000000..6b8749af29 --- /dev/null +++ b/.changeset/24090-db-dir-dangling-symlink.md @@ -0,0 +1,14 @@ +--- +'@objectstack/cli': patch +'@objectstack/runtime': patch +--- + +fix(cli,runtime): a dangling symlink standing as the SQLite database's directory is refused at once by `os dev` and `os serve`, naming the database file and the directory, instead of failing the boot later in a plugin's start + +Clause-②: no + +**What was wrong.** `os dev` (for its default database under `OS_HOME/data`) and the standalone host behind `os serve` and `os start` (for a `file:` or `wasm-sqlite://` database URL) make the database file's directory before the driver opens it. Both accepted any `EEXIST` answer while making a missing level. A symbolic link that points nowhere answers `EEXIST` to `mkdir`, so it was taken for a directory, and the boot went on until the driver could not open the file. `os dev` then exited 1 with `Plugin com.objectstack.auth failed to start - rollback complete: The database refused to run this query for object 'sys_organization'`, and `os serve` with `Plugin package-service failed to start - rollback complete: The database refused to run a raw statement`. Neither message named the file, the directory or the link. + +**What an operator sees now.** Both make the directory with the same bounded walk as the other `OS_HOME` paths, which accepts `EEXIST` only when a directory stands there. A dangling link as the directory is refused before the boot starts: `os dev` prints `The default database '…/data/objectstack.db' cannot be created: its directory '…/data' could not be made: EEXIST: …` and exits 1, and `os serve` prints `[StandaloneStack] SQLite database file '…' cannot be created: its directory '…' could not be made: EEXIST: …` and exits 1. Remove the link, or point it at a directory that exists. A dangling link higher up the path was already refused at once, with `ENOENT` at the level below the link; it is now refused with `EEXIST` at the link itself. + +Nothing else changes. A directory that can be made is still made, every missing level of it; a directory that exists, or a symbolic link to one, is used as it stands; and a path the operating system refuses for any other reason (`ENOENT`, `ENOTDIR`, a regular file holding the directory's name) is refused with the same code and text as before. diff --git a/packages/cli/src/commands/dev-database-directory.test.ts b/packages/cli/src/commands/dev-database-directory.test.ts index 892598e0af..8ae83444d6 100644 --- a/packages/cli/src/commands/dev-database-directory.test.ts +++ b/packages/cli/src/commands/dev-database-directory.test.ts @@ -30,10 +30,20 @@ // 2. A path through a regular file: refused with `ENOTDIR`, naming the file. // 3. CONTROL: a creatable unified-default path three levels deep is made. // 4. CONTROL: any other source is left to the serve child — nothing is made. +// 5. A dangling symlink AS the directory (`OS_HOME/data` pointing nowhere): +// refused at once with `EEXIST`, naming the file and the directory (#24090). +// The walk used to accept any `EEXIST` on a created level, so the link went +// through to the serve child, whose boot failed seconds later with a +// plugin-start error naming neither (measured at `os dev` on `origin/main` +// b9ac35cbf: exit 1 after 8.3 s, "Plugin com.objectstack.auth failed to +// start"). +// 6. The answers the fold onto `makeDirectorySync` keeps: a directory that +// already exists, and a live symlink to one, are accepted as they stand; a +// regular file holding the directory's own name is refused with `EEXIST`. import { describe, it, expect, beforeAll, afterAll } from 'vitest'; import { spawnSync } from 'node:child_process'; -import { existsSync, mkdtempSync, rmSync, statSync, writeFileSync } from 'node:fs'; +import { existsSync, lstatSync, mkdirSync, mkdtempSync, rmSync, statSync, symlinkSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { dirname, join, resolve } from 'node:path'; import { fileURLToPath, pathToFileURL } from 'node:url'; @@ -62,11 +72,22 @@ let outcomes: Map; beforeAll(() => { root = mkdtempSync(join(tmpdir(), 'os-23737-dev-')); writeFileSync(join(root, 'plain'), ''); + mkdirSync(join(root, 'dangling')); + symlinkSync(join(root, 'nowhere'), join(root, 'dangling', 'data')); + mkdirSync(join(root, 'existing', 'data'), { recursive: true }); + mkdirSync(join(root, 'live', 'target'), { recursive: true }); + symlinkSync(join(root, 'live', 'target'), join(root, 'live', 'data')); + mkdirSync(join(root, 'held')); + writeFileSync(join(root, 'held', 'data'), ''); const cases: Array<{ label: string; url: string; source: string }> = [ ...(RUNS_PROC ? [{ label: 'proc', url: `file:${PROC_FILE}`, source: 'unified-default' }] : []), { label: 'through-file', url: `file:${join(root, 'plain', 'data', 'objectstack.db')}`, source: 'unified-default' }, { label: 'creatable', url: `file:${join(root, 'home', 'a', 'data', 'objectstack.db')}`, source: 'unified-default' }, { label: 'other-source', url: `file:${join(root, 'env', 'a', 'data', 'objectstack.db')}`, source: 'env' }, + { label: 'dangling', url: `file:${join(root, 'dangling', 'data', 'objectstack.db')}`, source: 'unified-default' }, + { label: 'existing', url: `file:${join(root, 'existing', 'data', 'objectstack.db')}`, source: 'unified-default' }, + { label: 'live-link', url: `file:${join(root, 'live', 'data', 'objectstack.db')}`, source: 'unified-default' }, + { label: 'held', url: `file:${join(root, 'held', 'data', 'objectstack.db')}`, source: 'unified-default' }, ]; const script = [ 'const [subject, json] = process.argv.slice(1);', @@ -144,3 +165,29 @@ describe('prepareDevDatabaseDirectory — os dev\'s unified-default directory (# expect(existsSync(join(root, 'env'))).toBe(false); }); }); + +describe('prepareDevDatabaseDirectory — a dangling symlink in the path is refused at once (#24090)', () => { + it('refuses a dangling symlink as the directory with EEXIST, naming the file and the directory', () => { + const o = outcomeOf('dangling'); + expect(o.outcome).toBe('rejected'); + expect(o.code).toBe('EEXIST'); + expect(o.message).toContain(`'${join(root, 'dangling', 'data', 'objectstack.db')}'`); + expect(o.message).toContain(`'${join(root, 'dangling', 'data')}'`); + expect(lstatSync(join(root, 'dangling', 'data')).isSymbolicLink()).toBe(true); + expect(existsSync(join(root, 'nowhere'))).toBe(false); + }); + + it('CONTROL: a directory that already exists, and a live symlink to one, are accepted as they stand', () => { + expect(outcomeOf('existing').outcome).toBe('returned'); + expect(statSync(join(root, 'existing', 'data')).isDirectory()).toBe(true); + expect(outcomeOf('live-link').outcome).toBe('returned'); + expect(lstatSync(join(root, 'live', 'data')).isSymbolicLink()).toBe(true); + }); + + it('refuses a regular file holding the directory\'s own name with EEXIST, naming the file', () => { + const o = outcomeOf('held'); + expect(o.outcome).toBe('rejected'); + expect(o.code).toBe('EEXIST'); + expect(o.message).toContain(`'${join(root, 'held', 'data', 'objectstack.db')}'`); + }); +}); diff --git a/packages/cli/src/commands/dev.ts b/packages/cli/src/commands/dev.ts index de384bf3e8..17fc74e988 100644 --- a/packages/cli/src/commands/dev.ts +++ b/packages/cli/src/commands/dev.ts @@ -18,6 +18,7 @@ import { } from '../utils/dev-restart.js'; import { isOrganizationOwnershipRequiredMessage, upgradeDevelopmentDatabase } from '../utils/dev-organization-ownership-upgrade.js'; import { childEnvWithResolvedArtifact } from '../utils/internal-artifact-channel.js'; +import { makeDirectorySync } from '../utils/make-directory.js'; // THE artifact precedence, written once (#21501) — shared with `start`, and // with the `serve` child this command spawns. ⛔ No rung of it is restated here. import { @@ -93,7 +94,7 @@ export async function resolveDevDatabase(opts: { /** * Make the directory of `os dev`'s unified-default SQLite database - * (`/data/`), bounded — or reject at once, naming the database + * (`/data/`), bounded — or throw at once, naming the database * file, the directory and the operating system's code (#23737). * * Only the `unified-default` source is prepared here; every other source is @@ -108,40 +109,21 @@ export async function resolveDevDatabase(opts: { * `mkdir("/proc")` in its own process (64,440 failing calls in 25 s) and never * reached its next line. * - * The walk is the one `driver-sql`'s connect and the runtime's standalone host - * run (neither is on a package entry, hence this module-level copy): find the - * nearest level that exists, create the missing ones top-down with a - * NON-recursive `mkdir`, accept `EEXIST` on a created level (a concurrent - * creator), and reject on anything else. + * The walk is this package's {@link makeDirectorySync}: `EEXIST` on a created + * level is accepted only when a directory stands there. A dangling symlink in + * the path answers `EEXIST` to `mkdir` and `ENOENT` to `stat`, so it is refused + * here, at once, naming the link (#24090). Accepting any `EEXIST` let it + * through to the serve child, whose boot then failed seconds later with a + * plugin-start error that named neither the file nor the directory. */ -export async function prepareDevDatabaseDirectory( +export function prepareDevDatabaseDirectory( resolved: Pick, -): Promise { +): void { if (resolved.source !== 'unified-default') return; const filename = resolved.url.replace(/^file:/, ''); const directory = path.resolve(path.dirname(filename)); try { - const missing: string[] = []; - let level = directory; - for (;;) { - try { - if (!(await fs.promises.stat(level)).isDirectory() && missing.length === 0) await fs.promises.mkdir(level); - break; - } catch (e) { - if ((e as { code?: unknown } | null)?.code !== 'ENOENT') throw e; - missing.push(level); - const parent = path.dirname(level); - if (parent === level) break; - level = parent; - } - } - for (const each of missing.reverse()) { - try { - await fs.promises.mkdir(each); - } catch (e) { - if ((e as { code?: unknown } | null)?.code !== 'EEXIST') throw e; - } - } + makeDirectorySync(directory); } catch (e) { const code = (e as { code?: unknown } | null)?.code; const reason = e instanceof Error ? e.message : String(e); @@ -639,7 +621,7 @@ export default class Dev extends Command { } // Bounded, and refused at once when the directory cannot be made (#23737). try { - await prepareDevDatabaseDirectory(resolvedDb); + prepareDevDatabaseDirectory(resolvedDb); } catch (error: any) { printError(error?.message ?? String(error)); console.error(chalk.yellow( diff --git a/packages/cli/src/utils/make-directory.ts b/packages/cli/src/utils/make-directory.ts index 7fc615b967..a4e52da69c 100644 --- a/packages/cli/src/utils/make-directory.ts +++ b/packages/cli/src/utils/make-directory.ts @@ -23,10 +23,10 @@ import { dirname, resolve } from 'node:path'; * * ## The walk * - * The one `driver-sql`'s connect, the runtime's standalone host and `os dev`'s - * database prep already run (#23696, #23737), made synchronous so it can stand - * in for `mkdirSync` where it stood: find the nearest level that exists, then - * create the missing ones top-down. + * The one `driver-sql`'s connect runs (#23696), made synchronous so it can + * stand in for `mkdirSync` where it stood: find the nearest level that exists, + * then create the missing ones top-down. `os dev`'s database prep + * (`prepareDevDatabaseDirectory` in `commands/dev.ts`) runs it too (#24090). * * - `EEXIST` on a created level is a concurrent creator winning the race, and * it is accepted only when what now stands there is a directory, as the diff --git a/packages/runtime/src/make-directory.ts b/packages/runtime/src/make-directory.ts index 496e6dab39..b6544e2a31 100644 --- a/packages/runtime/src/make-directory.ts +++ b/packages/runtime/src/make-directory.ts @@ -24,11 +24,11 @@ import { dirname, resolve } from 'node:path'; * * ## The walk * - * The one `driver-sql`'s connect and this package's standalone host already - * run (#23696, #23737; `makeSqliteDatabaseDirectory` in `standalone-stack.ts` - * is that module's own async copy), made synchronous so it can stand in for - * `mkdirSync` where it stood: find the nearest level that exists, then create - * the missing ones top-down. + * The one `driver-sql`'s connect runs (#23696), made synchronous so it can + * stand in for `mkdirSync` where it stood: find the nearest level that exists, + * then create the missing ones top-down. This package's standalone host + * prepares its SQLite directory with it too (`makeSqliteDatabaseDirectory` in + * `standalone-stack.ts`, #24090). * * - `EEXIST` on a created level is a concurrent creator winning the race, and * it is accepted only when what now stands there is a directory, as the diff --git a/packages/runtime/src/standalone-stack-uncreatable-db-directory.test.ts b/packages/runtime/src/standalone-stack-uncreatable-db-directory.test.ts index 6bd3a651de..86abc1ead4 100644 --- a/packages/runtime/src/standalone-stack-uncreatable-db-directory.test.ts +++ b/packages/runtime/src/standalone-stack-uncreatable-db-directory.test.ts @@ -35,10 +35,19 @@ // 3. CONTROL: a creatable path three levels deep is made, in both arms. // 4. CONTROL: the read-only boot (`sqliteAbsentFile: 'empty-in-memory'`, #6743) // over an absent file still makes no directory at all. +// 5. A dangling symlink AS the directory, both arms: refused at once with +// `EEXIST`, naming the file and the directory (#24090). The host used to +// accept any `EEXIST` on a created level, so the link went through to the +// driver and the boot failed later in a plugin's start, naming neither +// (measured at `os serve` on `origin/main` b9ac35cbf: exit 1 after 2.7 s, +// "Plugin package-service failed to start"). In-process: the operating +// system answers a dangling link at once whatever the walk. +// 6. CONTROL: a directory that already exists, and a live symlink to one, +// are accepted as they stand, in both arms. import { describe, it, expect, beforeEach, afterEach } from 'vitest'; import { spawnSync } from 'node:child_process'; -import { existsSync, mkdtempSync, rmSync, statSync, writeFileSync } from 'node:fs'; +import { existsSync, lstatSync, mkdirSync, mkdtempSync, rmSync, statSync, symlinkSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { dirname, join, resolve } from 'node:path'; import { fileURLToPath, pathToFileURL } from 'node:url'; @@ -213,3 +222,34 @@ describe('createStandaloneStack — a SQLite directory the host cannot make (#23 expect(existsSync(join(root, 'ro'))).toBe(false); }); }); + +describe('createStandaloneStack — a dangling symlink in the SQLite directory\'s path (#24090)', () => { + it('refuses a dangling symlink as the directory at once with EEXIST, in both arms, naming the file and the directory', async () => { + const link = join(root, 'link'); + symlinkSync(join(root, 'nowhere'), link); + + const native = join(link, 'ctl.db'); + expectNamedRefusal(await refusalOf(createStandaloneStack({ databaseUrl: `file:${native}` })), native, link, 'EEXIST'); + + const wasm = join(link, 'wasm.db'); + expectNamedRefusal(await refusalOf(createStandaloneStack({ databaseUrl: `wasm-sqlite://${wasm}` })), wasm, link, 'EEXIST'); + + expect(lstatSync(link).isSymbolicLink()).toBe(true); + expect(existsSync(join(root, 'nowhere'))).toBe(false); + }); + + it('CONTROL: a directory that already exists, and a live symlink to one, are accepted, in both arms', async () => { + const existing = join(root, 'existing'); + mkdirSync(existing); + await createStandaloneStack({ databaseUrl: `file:${join(existing, 'ctl.db')}` }); + await createStandaloneStack({ databaseUrl: `wasm-sqlite://${join(existing, 'wasm.db')}` }); + expect(statSync(existing).isDirectory()).toBe(true); + + const live = join(root, 'live'); + mkdirSync(join(root, 'target')); + symlinkSync(join(root, 'target'), live); + await createStandaloneStack({ databaseUrl: `file:${join(live, 'ctl.db')}` }); + await createStandaloneStack({ databaseUrl: `wasm-sqlite://${join(live, 'wasm.db')}` }); + expect(lstatSync(live).isSymbolicLink()).toBe(true); + }); +}); diff --git a/packages/runtime/src/standalone-stack.ts b/packages/runtime/src/standalone-stack.ts index 6ddd65e0af..8b74041be4 100644 --- a/packages/runtime/src/standalone-stack.ts +++ b/packages/runtime/src/standalone-stack.ts @@ -62,9 +62,8 @@ * builds it like postgres. */ -import { dirname, resolve as resolvePath } from 'node:path'; +import { resolve as resolvePath } from 'node:path'; import { existsSync } from 'node:fs'; -import { mkdir, stat } from 'node:fs/promises'; import { homedir } from 'node:os'; import { z } from 'zod'; import { stampSearchPinyinEnabled } from '@objectstack/types'; @@ -77,6 +76,7 @@ import { } from '@objectstack/spec/data'; import type { IDatasourceDriverFactory } from '@objectstack/service-datasource'; import { loadArtifactBundle, isHttpUrl } from './load-artifact-bundle.js'; +import { makeDirectorySync } from './make-directory.js'; import { loadTursoDriverFactory } from './turso-driver-factory.js'; import { devAutoMigrateConfig } from './dev-auto-migrate.js'; import { @@ -740,40 +740,21 @@ class SqliteDatabaseDirectoryError extends Error { * could fire, and the SQL driver's own bounded refusal (#23696) was never * reached. * - * The walk below is the one `driver-sql`'s connect runs - * (`makeDatabaseDirectory` in `sqlite-database-file.ts`, which that package's - * entry does not export — hence this module-private copy): find the nearest - * level that exists, then create the missing ones top-down with a - * NON-recursive `mkdir`. `EEXIST` on a created level is a concurrent creator - * winning the race and is accepted; any other answer is the refusal, at once. - * A file holding the nearest existing name answers too: `ENOTDIR` from the - * first missing level under it, or `EEXIST` from asking for the directory - * itself. + * The walk is this package's {@link makeDirectorySync}: find the nearest level + * that exists, then create the missing ones top-down with a NON-recursive + * `mkdir`. `EEXIST` on a created level is a concurrent creator winning the + * race, accepted only when a directory stands there; any other answer is the + * refusal, at once. A file holding the nearest existing name answers too: + * `ENOTDIR` from the first missing level under it, or `EEXIST` from asking + * for the directory itself. So does a dangling symlink in the path: `EEXIST` + * at the link (#24090). Accepting any `EEXIST` let it through to the driver, + * and the boot then failed in a plugin's start with an error that named + * neither the file nor the directory. */ -async function makeSqliteDatabaseDirectory(filename: string): Promise { +function makeSqliteDatabaseDirectory(filename: string): void { const directory = resolvePath(filename, '..'); try { - const missing: string[] = []; - let level = directory; - for (;;) { - try { - if (!(await stat(level)).isDirectory() && missing.length === 0) await mkdir(level); - break; - } catch (e) { - if ((e as { code?: unknown } | null)?.code !== 'ENOENT') throw e; - missing.push(level); - const parent = dirname(level); - if (parent === level) break; - level = parent; - } - } - for (const each of missing.reverse()) { - try { - await mkdir(each); - } catch (e) { - if ((e as { code?: unknown } | null)?.code !== 'EEXIST') throw e; - } - } + makeDirectorySync(directory); } catch (e) { throw new SqliteDatabaseDirectoryError(filename, directory, e); } @@ -990,7 +971,7 @@ export async function createStandaloneStack(config?: StandaloneStackConfig): Pro driverId = 'sqlite-wasm'; const filename = sqliteFilenameFromUrl(dbUrl, 'sqlite-wasm'); if (filename !== ':memory:') { - await makeSqliteDatabaseDirectory(filename); + makeSqliteDatabaseDirectory(filename); } driverConfig = { filename }; } else if (dbDriver === 'sqlite') { @@ -1005,7 +986,7 @@ export async function createStandaloneStack(config?: StandaloneStackConfig): Pro // does too, so this costs the mode nothing. Bounded, and refused at once // when the directory cannot be made (#23737, see the helper). if (!(cfg.sqliteAbsentFile === 'empty-in-memory' && !existsSync(filename))) { - await makeSqliteDatabaseDirectory(filename); + makeSqliteDatabaseDirectory(filename); } driverConfig = { filename }; } else {