From ac893f97bbbc3727beeab4034d689464dc4f2018 Mon Sep 17 00:00:00 2001 From: Yichen Jiang Date: Fri, 17 Jul 2026 10:47:32 +0800 Subject: [PATCH] fix(session-persistence): address permission review --- docs/config-catalog.md | 4 +++- .../session-persistence-sqlite/README.md | 2 +- .../session-persistence-sqlite/src/index.ts | 8 +++++++- .../tests/sqlite.spec.ts | 17 +++++------------ 4 files changed, 16 insertions(+), 15 deletions(-) diff --git a/docs/config-catalog.md b/docs/config-catalog.md index 9474a4877b..49607eb494 100644 --- a/docs/config-catalog.md +++ b/docs/config-catalog.md @@ -605,6 +605,8 @@ export interface Config { * Filesystem path to the SQLite database file. The special value `:memory:` * opens an in-process database (tests). Missing directories and the database * are created with owner-only permissions; existing path modes are preserved. + * Parent directories writable by another principal are outside the backend's + * database-integrity boundary. */ path: string /** @@ -627,7 +629,7 @@ export interface Config { export type JournalMode = 'wal' | 'delete' | 'truncate' | 'persist' ``` -Source: [`packages/session-persistence/session-persistence-sqlite/src/index.ts:48`](../packages/session-persistence/session-persistence-sqlite/src/index.ts) +Source: [`packages/session-persistence/session-persistence-sqlite/src/index.ts:52`](../packages/session-persistence/session-persistence-sqlite/src/index.ts) ## `@deepseek-ai/dsh-session-query` diff --git a/packages/session-persistence/session-persistence-sqlite/README.md b/packages/session-persistence/session-persistence-sqlite/README.md index 1dd480bc38..b5f85bd1a8 100644 --- a/packages/session-persistence/session-persistence-sqlite/README.md +++ b/packages/session-persistence/session-persistence-sqlite/README.md @@ -8,7 +8,7 @@ A SQLite durable session-persistence backend — a second `SessionPersistence` i Each `SessionEvent` maps 1:1 onto a row in an `events` table `(session_id, seq, type, time, data, source_event_seqs, surface_op)` — `data` is the event payload as JSON text, so the row shape is the event verbatim (including `assistant/chunk`, keeping `seq` contiguous). The two `TEXT` columns `source_event_seqs` and `surface_op` are nullable; they store the event's optional surface-metadata fields (see [session surface](../../../docs/rfc/implemented/architecture/2026-06-18-session-surface.md)). Out-of-log metadata (`SessionHeader`) lives in a `sessions` row. A `sessions` row is written only by the first `append` — its existence is the lazy-materialization signal (`list` reports exactly the sessions that have a row), so no separate column is needed. -The repository's Node range supports unflagged `node:sqlite`. The database enables foreign keys and uses the configured journal mode (`wal` by default; use a rollback mode where WAL shared-memory files are unsuitable). On filesystems with POSIX modes, missing directories are created as `0700` and a missing database is exclusively created as `0600` before SQLite opens it, causing new WAL sidecars to inherit owner-only access. Existing directories, database files, and sidecars keep their modes; ordinary filesystem access errors still fail initialization. `PRAGMA user_version` stores the table-layout version; databases with any other version are rejected because this unreleased format has no migrations. +The repository's Node range supports unflagged `node:sqlite`. The database enables foreign keys and uses the configured journal mode (`wal` by default; use a rollback mode where WAL shared-memory files are unsuitable). On filesystems with POSIX modes, missing directories are created as `0700` and a missing database is exclusively created as `0600` before SQLite opens it, causing new WAL sidecars to inherit owner-only access. Existing directories, database files, and sidecars keep their modes; ordinary filesystem access errors still fail initialization. This default prevents incidental exposure through the process umask; it does not protect database integrity when another principal can modify entries in an existing parent directory. `PRAGMA user_version` stores the table-layout version; databases with any other version are rejected because this unreleased format has no migrations. ## Contract semantics over rows diff --git a/packages/session-persistence/session-persistence-sqlite/src/index.ts b/packages/session-persistence/session-persistence-sqlite/src/index.ts index 9f0b7cb7c4..45c05f750b 100644 --- a/packages/session-persistence/session-persistence-sqlite/src/index.ts +++ b/packages/session-persistence/session-persistence-sqlite/src/index.ts @@ -34,7 +34,11 @@ function surfaceBindings(event: SessionEvent): [string | null, string | null] { ] } -/** Create a missing database owner-only while preserving an existing file's mode. */ +/** + * Create a missing database owner-only while preserving an existing file's + * mode. `DatabaseSync` cannot adopt this handle, so a parent directory writable + * by another principal is outside the backend's database-integrity boundary. + */ async function createDatabaseFile(path: string): Promise { try { const handle = await open(path, 'wx', 0o600) @@ -50,6 +54,8 @@ export interface Config { * Filesystem path to the SQLite database file. The special value `:memory:` * opens an in-process database (tests). Missing directories and the database * are created with owner-only permissions; existing path modes are preserved. + * Parent directories writable by another principal are outside the backend's + * database-integrity boundary. */ path: string /** diff --git a/packages/session-persistence/session-persistence-sqlite/tests/sqlite.spec.ts b/packages/session-persistence/session-persistence-sqlite/tests/sqlite.spec.ts index 3606afce67..50d761a39d 100644 --- a/packages/session-persistence/session-persistence-sqlite/tests/sqlite.spec.ts +++ b/packages/session-persistence/session-persistence-sqlite/tests/sqlite.spec.ts @@ -1,7 +1,7 @@ import { afterEach, describe, expect, it } from 'vitest' import { Context } from 'cordis' import { existsSync } from 'node:fs' -import { chmod, mkdtemp, mkdir, rm, stat, writeFile } from 'node:fs/promises' +import { chmod, mkdtemp, rm, stat, writeFile } from 'node:fs/promises' import { tmpdir } from 'node:os' import { dirname, join } from 'node:path' import SessionStore, { SessionId } from '@deepseek-ai/dsh-session' @@ -379,19 +379,12 @@ describe('SessionPersistenceSqlite: edge cases', () => { await fiber.dispose() }) - it('surfaces database pre-creation errors other than an existing file', async () => { - if (process.platform === 'win32') return + it('surfaces database pre-creation errors independently of process privileges', async () => { const path = await freshDbPath() - const blocked = join(dirname(path), 'blocked') - await mkdir(blocked, { mode: 0o500 }) - const b = await backend(join(blocked, 'sessions.db')) + const b = await backend(`${path}\0`) - try { - await expect(b.ctx.sessionPersistence.list()).rejects.toMatchObject({ code: 'EACCES' }) - await b.dispose() - } finally { - await chmod(blocked, 0o700) - } + await expect(b.ctx.sessionPersistence.list()).rejects.toMatchObject({ code: 'ERR_INVALID_ARG_VALUE' }) + await b.dispose() }) it('append rolls back and rethrows when an event INSERT fails inside the transaction', async () => {