Răsfoiți Sursa

fix(session-persistence): address permission review

Yichen Jiang 1 lună în urmă
părinte
comite
ac893f97bb

+ 3 - 1
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`
 

+ 1 - 1
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
 

+ 7 - 1
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<void> {
   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
   /**

+ 6 - 13
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'))
-
-    try {
-      await expect(b.ctx.sessionPersistence.list()).rejects.toMatchObject({ code: 'EACCES' })
-      await b.dispose()
-    } finally {
-      await chmod(blocked, 0o700)
-    }
+    const b = await backend(`${path}\0`)
+
+    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 () => {