Skip to content

Commit 09c0232

Browse files
committed
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QyoWKK9RwUNxBxmevy9Neb
1 parent b9ac35c commit 09c0232

7 files changed

Lines changed: 140 additions & 76 deletions

File tree

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
---
2+
'@objectstack/cli': patch
3+
'@objectstack/runtime': patch
4+
---
5+
6+
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
7+
8+
Clause-②: no
9+
10+
**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.
11+
12+
**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.
13+
14+
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.

‎packages/cli/src/commands/dev-database-directory.test.ts‎

Lines changed: 48 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,20 @@
3030
// 2. A path through a regular file: refused with `ENOTDIR`, naming the file.
3131
// 3. CONTROL: a creatable unified-default path three levels deep is made.
3232
// 4. CONTROL: any other source is left to the serve child — nothing is made.
33+
// 5. A dangling symlink AS the directory (`OS_HOME/data` pointing nowhere):
34+
// refused at once with `EEXIST`, naming the file and the directory (#24090).
35+
// The walk used to accept any `EEXIST` on a created level, so the link went
36+
// through to the serve child, whose boot failed seconds later with a
37+
// plugin-start error naming neither (measured at `os dev` on `origin/main`
38+
// b9ac35cbf: exit 1 after 8.3 s, "Plugin com.objectstack.auth failed to
39+
// start").
40+
// 6. The answers the fold onto `makeDirectorySync` keeps: a directory that
41+
// already exists, and a live symlink to one, are accepted as they stand; a
42+
// regular file holding the directory's own name is refused with `EEXIST`.
3343

3444
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
3545
import { spawnSync } from 'node:child_process';
36-
import { existsSync, mkdtempSync, rmSync, statSync, writeFileSync } from 'node:fs';
46+
import { existsSync, lstatSync, mkdirSync, mkdtempSync, rmSync, statSync, symlinkSync, writeFileSync } from 'node:fs';
3747
import { tmpdir } from 'node:os';
3848
import { dirname, join, resolve } from 'node:path';
3949
import { fileURLToPath, pathToFileURL } from 'node:url';
@@ -62,11 +72,22 @@ let outcomes: Map<string, Outcome>;
6272
beforeAll(() => {
6373
root = mkdtempSync(join(tmpdir(), 'os-23737-dev-'));
6474
writeFileSync(join(root, 'plain'), '');
75+
mkdirSync(join(root, 'dangling'));
76+
symlinkSync(join(root, 'nowhere'), join(root, 'dangling', 'data'));
77+
mkdirSync(join(root, 'existing', 'data'), { recursive: true });
78+
mkdirSync(join(root, 'live', 'target'), { recursive: true });
79+
symlinkSync(join(root, 'live', 'target'), join(root, 'live', 'data'));
80+
mkdirSync(join(root, 'held'));
81+
writeFileSync(join(root, 'held', 'data'), '');
6582
const cases: Array<{ label: string; url: string; source: string }> = [
6683
...(RUNS_PROC ? [{ label: 'proc', url: `file:${PROC_FILE}`, source: 'unified-default' }] : []),
6784
{ label: 'through-file', url: `file:${join(root, 'plain', 'data', 'objectstack.db')}`, source: 'unified-default' },
6885
{ label: 'creatable', url: `file:${join(root, 'home', 'a', 'data', 'objectstack.db')}`, source: 'unified-default' },
6986
{ label: 'other-source', url: `file:${join(root, 'env', 'a', 'data', 'objectstack.db')}`, source: 'env' },
87+
{ label: 'dangling', url: `file:${join(root, 'dangling', 'data', 'objectstack.db')}`, source: 'unified-default' },
88+
{ label: 'existing', url: `file:${join(root, 'existing', 'data', 'objectstack.db')}`, source: 'unified-default' },
89+
{ label: 'live-link', url: `file:${join(root, 'live', 'data', 'objectstack.db')}`, source: 'unified-default' },
90+
{ label: 'held', url: `file:${join(root, 'held', 'data', 'objectstack.db')}`, source: 'unified-default' },
7091
];
7192
const script = [
7293
'const [subject, json] = process.argv.slice(1);',
@@ -144,3 +165,29 @@ describe('prepareDevDatabaseDirectory — os dev\'s unified-default directory (#
144165
expect(existsSync(join(root, 'env'))).toBe(false);
145166
});
146167
});
168+
169+
describe('prepareDevDatabaseDirectory — a dangling symlink in the path is refused at once (#24090)', () => {
170+
it('refuses a dangling symlink as the directory with EEXIST, naming the file and the directory', () => {
171+
const o = outcomeOf('dangling');
172+
expect(o.outcome).toBe('rejected');
173+
expect(o.code).toBe('EEXIST');
174+
expect(o.message).toContain(`'${join(root, 'dangling', 'data', 'objectstack.db')}'`);
175+
expect(o.message).toContain(`'${join(root, 'dangling', 'data')}'`);
176+
expect(lstatSync(join(root, 'dangling', 'data')).isSymbolicLink()).toBe(true);
177+
expect(existsSync(join(root, 'nowhere'))).toBe(false);
178+
});
179+
180+
it('CONTROL: a directory that already exists, and a live symlink to one, are accepted as they stand', () => {
181+
expect(outcomeOf('existing').outcome).toBe('returned');
182+
expect(statSync(join(root, 'existing', 'data')).isDirectory()).toBe(true);
183+
expect(outcomeOf('live-link').outcome).toBe('returned');
184+
expect(lstatSync(join(root, 'live', 'data')).isSymbolicLink()).toBe(true);
185+
});
186+
187+
it('refuses a regular file holding the directory\'s own name with EEXIST, naming the file', () => {
188+
const o = outcomeOf('held');
189+
expect(o.outcome).toBe('rejected');
190+
expect(o.code).toBe('EEXIST');
191+
expect(o.message).toContain(`'${join(root, 'held', 'data', 'objectstack.db')}'`);
192+
});
193+
});

‎packages/cli/src/commands/dev.ts‎

Lines changed: 12 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import {
1818
} from '../utils/dev-restart.js';
1919
import { isOrganizationOwnershipRequiredMessage, upgradeDevelopmentDatabase } from '../utils/dev-organization-ownership-upgrade.js';
2020
import { childEnvWithResolvedArtifact } from '../utils/internal-artifact-channel.js';
21+
import { makeDirectorySync } from '../utils/make-directory.js';
2122
// THE artifact precedence, written once (#21501) — shared with `start`, and
2223
// with the `serve` child this command spawns. ⛔ No rung of it is restated here.
2324
import {
@@ -93,7 +94,7 @@ export async function resolveDevDatabase(opts: {
9394

9495
/**
9596
* Make the directory of `os dev`'s unified-default SQLite database
96-
* (`<state dir>/data/`), bounded — or reject at once, naming the database
97+
* (`<state dir>/data/`), bounded — or throw at once, naming the database
9798
* file, the directory and the operating system's code (#23737).
9899
*
99100
* Only the `unified-default` source is prepared here; every other source is
@@ -108,40 +109,21 @@ export async function resolveDevDatabase(opts: {
108109
* `mkdir("/proc")` in its own process (64,440 failing calls in 25 s) and never
109110
* reached its next line.
110111
*
111-
* The walk is the one `driver-sql`'s connect and the runtime's standalone host
112-
* run (neither is on a package entry, hence this module-level copy): find the
113-
* nearest level that exists, create the missing ones top-down with a
114-
* NON-recursive `mkdir`, accept `EEXIST` on a created level (a concurrent
115-
* creator), and reject on anything else.
112+
* The walk is this package's {@link makeDirectorySync}: `EEXIST` on a created
113+
* level is accepted only when a directory stands there. A dangling symlink in
114+
* the path answers `EEXIST` to `mkdir` and `ENOENT` to `stat`, so it is refused
115+
* here, at once, naming the link (#24090). Accepting any `EEXIST` let it
116+
* through to the serve child, whose boot then failed seconds later with a
117+
* plugin-start error that named neither the file nor the directory.
116118
*/
117-
export async function prepareDevDatabaseDirectory(
119+
export function prepareDevDatabaseDirectory(
118120
resolved: Pick<ResolvedProjectDatabaseUrl, 'url' | 'source'>,
119-
): Promise<void> {
121+
): void {
120122
if (resolved.source !== 'unified-default') return;
121123
const filename = resolved.url.replace(/^file:/, '');
122124
const directory = path.resolve(path.dirname(filename));
123125
try {
124-
const missing: string[] = [];
125-
let level = directory;
126-
for (;;) {
127-
try {
128-
if (!(await fs.promises.stat(level)).isDirectory() && missing.length === 0) await fs.promises.mkdir(level);
129-
break;
130-
} catch (e) {
131-
if ((e as { code?: unknown } | null)?.code !== 'ENOENT') throw e;
132-
missing.push(level);
133-
const parent = path.dirname(level);
134-
if (parent === level) break;
135-
level = parent;
136-
}
137-
}
138-
for (const each of missing.reverse()) {
139-
try {
140-
await fs.promises.mkdir(each);
141-
} catch (e) {
142-
if ((e as { code?: unknown } | null)?.code !== 'EEXIST') throw e;
143-
}
144-
}
126+
makeDirectorySync(directory);
145127
} catch (e) {
146128
const code = (e as { code?: unknown } | null)?.code;
147129
const reason = e instanceof Error ? e.message : String(e);
@@ -639,7 +621,7 @@ export default class Dev extends Command {
639621
}
640622
// Bounded, and refused at once when the directory cannot be made (#23737).
641623
try {
642-
await prepareDevDatabaseDirectory(resolvedDb);
624+
prepareDevDatabaseDirectory(resolvedDb);
643625
} catch (error: any) {
644626
printError(error?.message ?? String(error));
645627
console.error(chalk.yellow(

‎packages/cli/src/utils/make-directory.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,10 @@ import { dirname, resolve } from 'node:path';
2323
*
2424
* ## The walk
2525
*
26-
* The one `driver-sql`'s connect, the runtime's standalone host and `os dev`'s
27-
* database prep already run (#23696, #23737), made synchronous so it can stand
28-
* in for `mkdirSync` where it stood: find the nearest level that exists, then
29-
* create the missing ones top-down.
26+
* The one `driver-sql`'s connect runs (#23696), made synchronous so it can
27+
* stand in for `mkdirSync` where it stood: find the nearest level that exists,
28+
* then create the missing ones top-down. `os dev`'s database prep
29+
* (`prepareDevDatabaseDirectory` in `commands/dev.ts`) runs it too (#24090).
3030
*
3131
* - `EEXIST` on a created level is a concurrent creator winning the race, and
3232
* it is accepted only when what now stands there is a directory, as the

‎packages/runtime/src/make-directory.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,11 @@ import { dirname, resolve } from 'node:path';
2424
*
2525
* ## The walk
2626
*
27-
* The one `driver-sql`'s connect and this package's standalone host already
28-
* run (#23696, #23737; `makeSqliteDatabaseDirectory` in `standalone-stack.ts`
29-
* is that module's own async copy), made synchronous so it can stand in for
30-
* `mkdirSync` where it stood: find the nearest level that exists, then create
31-
* the missing ones top-down.
27+
* The one `driver-sql`'s connect runs (#23696), made synchronous so it can
28+
* stand in for `mkdirSync` where it stood: find the nearest level that exists,
29+
* then create the missing ones top-down. This package's standalone host
30+
* prepares its SQLite directory with it too (`makeSqliteDatabaseDirectory` in
31+
* `standalone-stack.ts`, #24090).
3232
*
3333
* - `EEXIST` on a created level is a concurrent creator winning the race, and
3434
* it is accepted only when what now stands there is a directory, as the

‎packages/runtime/src/standalone-stack-uncreatable-db-directory.test.ts‎

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,10 +35,19 @@
3535
// 3. CONTROL: a creatable path three levels deep is made, in both arms.
3636
// 4. CONTROL: the read-only boot (`sqliteAbsentFile: 'empty-in-memory'`, #6743)
3737
// over an absent file still makes no directory at all.
38+
// 5. A dangling symlink AS the directory, both arms: refused at once with
39+
// `EEXIST`, naming the file and the directory (#24090). The host used to
40+
// accept any `EEXIST` on a created level, so the link went through to the
41+
// driver and the boot failed later in a plugin's start, naming neither
42+
// (measured at `os serve` on `origin/main` b9ac35cbf: exit 1 after 2.7 s,
43+
// "Plugin package-service failed to start"). In-process: the operating
44+
// system answers a dangling link at once whatever the walk.
45+
// 6. CONTROL: a directory that already exists, and a live symlink to one,
46+
// are accepted as they stand, in both arms.
3847

3948
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
4049
import { spawnSync } from 'node:child_process';
41-
import { existsSync, mkdtempSync, rmSync, statSync, writeFileSync } from 'node:fs';
50+
import { existsSync, lstatSync, mkdirSync, mkdtempSync, rmSync, statSync, symlinkSync, writeFileSync } from 'node:fs';
4251
import { tmpdir } from 'node:os';
4352
import { dirname, join, resolve } from 'node:path';
4453
import { fileURLToPath, pathToFileURL } from 'node:url';
@@ -213,3 +222,34 @@ describe('createStandaloneStack — a SQLite directory the host cannot make (#23
213222
expect(existsSync(join(root, 'ro'))).toBe(false);
214223
});
215224
});
225+
226+
describe('createStandaloneStack — a dangling symlink in the SQLite directory\'s path (#24090)', () => {
227+
it('refuses a dangling symlink as the directory at once with EEXIST, in both arms, naming the file and the directory', async () => {
228+
const link = join(root, 'link');
229+
symlinkSync(join(root, 'nowhere'), link);
230+
231+
const native = join(link, 'ctl.db');
232+
expectNamedRefusal(await refusalOf(createStandaloneStack({ databaseUrl: `file:${native}` })), native, link, 'EEXIST');
233+
234+
const wasm = join(link, 'wasm.db');
235+
expectNamedRefusal(await refusalOf(createStandaloneStack({ databaseUrl: `wasm-sqlite://${wasm}` })), wasm, link, 'EEXIST');
236+
237+
expect(lstatSync(link).isSymbolicLink()).toBe(true);
238+
expect(existsSync(join(root, 'nowhere'))).toBe(false);
239+
});
240+
241+
it('CONTROL: a directory that already exists, and a live symlink to one, are accepted, in both arms', async () => {
242+
const existing = join(root, 'existing');
243+
mkdirSync(existing);
244+
await createStandaloneStack({ databaseUrl: `file:${join(existing, 'ctl.db')}` });
245+
await createStandaloneStack({ databaseUrl: `wasm-sqlite://${join(existing, 'wasm.db')}` });
246+
expect(statSync(existing).isDirectory()).toBe(true);
247+
248+
const live = join(root, 'live');
249+
mkdirSync(join(root, 'target'));
250+
symlinkSync(join(root, 'target'), live);
251+
await createStandaloneStack({ databaseUrl: `file:${join(live, 'ctl.db')}` });
252+
await createStandaloneStack({ databaseUrl: `wasm-sqlite://${join(live, 'wasm.db')}` });
253+
expect(lstatSync(live).isSymbolicLink()).toBe(true);
254+
});
255+
});

‎packages/runtime/src/standalone-stack.ts‎

Lines changed: 16 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -62,9 +62,8 @@
6262
* builds it like postgres.
6363
*/
6464

65-
import { dirname, resolve as resolvePath } from 'node:path';
65+
import { resolve as resolvePath } from 'node:path';
6666
import { existsSync } from 'node:fs';
67-
import { mkdir, stat } from 'node:fs/promises';
6867
import { homedir } from 'node:os';
6968
import { z } from 'zod';
7069
import { stampSearchPinyinEnabled } from '@objectstack/types';
@@ -77,6 +76,7 @@ import {
7776
} from '@objectstack/spec/data';
7877
import type { IDatasourceDriverFactory } from '@objectstack/service-datasource';
7978
import { loadArtifactBundle, isHttpUrl } from './load-artifact-bundle.js';
79+
import { makeDirectorySync } from './make-directory.js';
8080
import { loadTursoDriverFactory } from './turso-driver-factory.js';
8181
import { devAutoMigrateConfig } from './dev-auto-migrate.js';
8282
import {
@@ -740,40 +740,21 @@ class SqliteDatabaseDirectoryError extends Error {
740740
* could fire, and the SQL driver's own bounded refusal (#23696) was never
741741
* reached.
742742
*
743-
* The walk below is the one `driver-sql`'s connect runs
744-
* (`makeDatabaseDirectory` in `sqlite-database-file.ts`, which that package's
745-
* entry does not export — hence this module-private copy): find the nearest
746-
* level that exists, then create the missing ones top-down with a
747-
* NON-recursive `mkdir`. `EEXIST` on a created level is a concurrent creator
748-
* winning the race and is accepted; any other answer is the refusal, at once.
749-
* A file holding the nearest existing name answers too: `ENOTDIR` from the
750-
* first missing level under it, or `EEXIST` from asking for the directory
751-
* itself.
743+
* The walk is this package's {@link makeDirectorySync}: find the nearest level
744+
* that exists, then create the missing ones top-down with a NON-recursive
745+
* `mkdir`. `EEXIST` on a created level is a concurrent creator winning the
746+
* race, accepted only when a directory stands there; any other answer is the
747+
* refusal, at once. A file holding the nearest existing name answers too:
748+
* `ENOTDIR` from the first missing level under it, or `EEXIST` from asking
749+
* for the directory itself. So does a dangling symlink in the path: `EEXIST`
750+
* at the link (#24090). Accepting any `EEXIST` let it through to the driver,
751+
* and the boot then failed in a plugin's start with an error that named
752+
* neither the file nor the directory.
752753
*/
753-
async function makeSqliteDatabaseDirectory(filename: string): Promise<void> {
754+
function makeSqliteDatabaseDirectory(filename: string): void {
754755
const directory = resolvePath(filename, '..');
755756
try {
756-
const missing: string[] = [];
757-
let level = directory;
758-
for (;;) {
759-
try {
760-
if (!(await stat(level)).isDirectory() && missing.length === 0) await mkdir(level);
761-
break;
762-
} catch (e) {
763-
if ((e as { code?: unknown } | null)?.code !== 'ENOENT') throw e;
764-
missing.push(level);
765-
const parent = dirname(level);
766-
if (parent === level) break;
767-
level = parent;
768-
}
769-
}
770-
for (const each of missing.reverse()) {
771-
try {
772-
await mkdir(each);
773-
} catch (e) {
774-
if ((e as { code?: unknown } | null)?.code !== 'EEXIST') throw e;
775-
}
776-
}
757+
makeDirectorySync(directory);
777758
} catch (e) {
778759
throw new SqliteDatabaseDirectoryError(filename, directory, e);
779760
}
@@ -990,7 +971,7 @@ export async function createStandaloneStack(config?: StandaloneStackConfig): Pro
990971
driverId = 'sqlite-wasm';
991972
const filename = sqliteFilenameFromUrl(dbUrl, 'sqlite-wasm');
992973
if (filename !== ':memory:') {
993-
await makeSqliteDatabaseDirectory(filename);
974+
makeSqliteDatabaseDirectory(filename);
994975
}
995976
driverConfig = { filename };
996977
} else if (dbDriver === 'sqlite') {
@@ -1005,7 +986,7 @@ export async function createStandaloneStack(config?: StandaloneStackConfig): Pro
1005986
// does too, so this costs the mode nothing. Bounded, and refused at once
1006987
// when the directory cannot be made (#23737, see the helper).
1007988
if (!(cfg.sqliteAbsentFile === 'empty-in-memory' && !existsSync(filename))) {
1008-
await makeSqliteDatabaseDirectory(filename);
989+
makeSqliteDatabaseDirectory(filename);
1009990
}
1010991
driverConfig = { filename };
1011992
} else {

0 commit comments

Comments
 (0)