mirror of
https://github.com/hellodigua/ChatLab.git
synced 2026-10-25 17:54:34 +08:00
fix(mcp): secure live read-only queries and retain tool hints
This commit is contained in:
1 parent
d42e1d1bfb
commit
1a214a5a9e
11 files changed
+668
-64
No files matched your search
@@ -51,7 +51,9 @@ If you install the package globally, you can also use:
|
||||
|
||||
If you configured a custom ChatLab data directory in `~/.chatlab/config.toml`, the MCP server will use that configuration automatically.
|
||||
|
||||
The server is read-only. It opens existing ChatLab session databases and does not modify chat data.
|
||||
MCP tools use native read-only SQLite connections and short read transactions. They do not modify logical chat records, database schema, or the data-directory compatibility marker, and they never automatically migrate sessions. A session with an older schema must first be upgraded explicitly by a writable ChatLab runtime.
|
||||
|
||||
**Live SQLite caveat:** WAL-mode reads may create/update `-wal` or `-shm` coordination files beside the database. This mode does **not** guarantee an unchanged filesystem directory, nor universal support for read-only mounts or databases requiring recovery. MCP query tools declare `readOnlyHint: true` and `destructiveHint: false` because they do not intentionally change _logical chat data_, perform migrations, or update compatibility metadata. Whether SQLite's auxiliary WAL/SHM coordination is compatible with the MCP annotation's broader "does not modify its environment" wording is a product/contract question for the upstream maintainers; the annotation is not a filesystem-sandbox guarantee. Queries do not create full temporary copies of chat databases.
|
||||
|
||||
## Available Capabilities
|
||||
|
||||
|
||||
@@ -6,7 +6,7 @@ import test from 'node:test'
|
||||
import { Client } from '@modelcontextprotocol/sdk/client/index.js'
|
||||
import { InMemoryTransport } from '@modelcontextprotocol/sdk/inMemory.js'
|
||||
import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'
|
||||
import { CURRENT_SCHEMA_VERSION } from '@openchatlab/core'
|
||||
import { CHAT_DB_TABLES, CURRENT_SCHEMA_VERSION } from '@openchatlab/core'
|
||||
import { DatabaseManager, NodePathProvider, readDataDirCompatibilityMeta } from '@openchatlab/node-runtime'
|
||||
import { startMcpServer } from './server'
|
||||
import { openTestSqliteDatabase } from '../../../tests/helpers/sqlite.mts'
|
||||
@@ -218,3 +218,69 @@ test('MCP tools and resources refuse invalid SQLite schema versions before SQL d
|
||||
await client.close()
|
||||
}
|
||||
})
|
||||
|
||||
test('real MCP tools and resources read checkpointed WAL without logical data/marker mutation', async (t) => {
|
||||
const root = fs.mkdtempSync(path.join(os.tmpdir(), 'chatlab-mcp-wal-live-'))
|
||||
t.after(() => fs.rmSync(root, { recursive: true, force: true }))
|
||||
const dataDir = path.join(root, 'data')
|
||||
const dbDir = path.join(dataDir, 'databases')
|
||||
fs.mkdirSync(dbDir, { recursive: true })
|
||||
const dbPath = path.join(dbDir, 'wal.db')
|
||||
const fixture = openTestSqliteDatabase(dbPath)
|
||||
try {
|
||||
fixture.pragma('journal_mode = WAL')
|
||||
fixture.exec(CHAT_DB_TABLES)
|
||||
fixture.exec(`
|
||||
INSERT INTO meta (name, platform, type, imported_at) VALUES ('WAL live', 'qq', 'group', 1000);
|
||||
INSERT INTO member (platform_id, account_name) VALUES ('alice', 'Alice');
|
||||
INSERT INTO message (sender_id, ts, type, content) VALUES (1, 1000, 0, 'WAL query works');
|
||||
`)
|
||||
} finally {
|
||||
fixture.close()
|
||||
}
|
||||
const dbManager = new DatabaseManager(new NodePathProvider(dataDir), {
|
||||
runtime: { version: '0.37.6', kind: 'mcp' },
|
||||
})
|
||||
t.after(() => dbManager.closeAll())
|
||||
dbManager.raiseCurrentChatDbCompatibilityGate()
|
||||
const original = fs.readFileSync(dbPath)
|
||||
const metaPath = path.join(dataDir, '.chatlab-meta.json')
|
||||
const marker = fs.readFileSync(metaPath)
|
||||
|
||||
const client = new Client({ name: 'wal-live-test', version: '1.0.0' })
|
||||
const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair()
|
||||
const connect = McpServer.prototype.connect
|
||||
t.mock.method(McpServer.prototype, 'connect', function (this: McpServer) {
|
||||
t.after(() => this.close())
|
||||
return connect.call(this, serverTransport)
|
||||
})
|
||||
await startMcpServer({ version: '0.37.6', dbManager })
|
||||
await client.connect(clientTransport)
|
||||
try {
|
||||
for (const [name, args] of [
|
||||
['chatlab_list_sessions', {}],
|
||||
['chatlab_get_session_info', { session_id: 'wal' }],
|
||||
['chatlab_get_recent_messages', { session_id: 'wal' }],
|
||||
['chatlab_execute_sql', { session_id: 'wal', sql: 'SELECT content FROM message' }],
|
||||
] as const) {
|
||||
const result = await client.callTool({ name, arguments: { ...args, format: 'json' } })
|
||||
assert.equal(result.isError, undefined, `${name}: ${JSON.stringify(result.content)}`)
|
||||
}
|
||||
for (const uri of ['chatlab://sessions', 'chatlab://sessions/wal/meta', 'chatlab://sessions/wal/schema']) {
|
||||
const resource = await client.readResource({ uri })
|
||||
assert.ok(resource.contents.length > 0, uri)
|
||||
}
|
||||
// Native readonly WAL readers may create coordination sidecars, but may
|
||||
// not modify logical chat records, DB schema or compatibility metadata.
|
||||
assert.deepEqual(fs.readFileSync(dbPath), original)
|
||||
assert.deepEqual(fs.readFileSync(metaPath), marker)
|
||||
const readonlyDb = openTestSqliteDatabase(dbPath, { readonly: true })
|
||||
try {
|
||||
assert.deepEqual(readonlyDb.prepare('SELECT content FROM message').all(), [{ content: 'WAL query works' }])
|
||||
} finally {
|
||||
readonlyDb.close()
|
||||
}
|
||||
} finally {
|
||||
await client.close()
|
||||
}
|
||||
})
|
||||
@@ -0,0 +1,163 @@
|
||||
import assert from 'node:assert/strict'
|
||||
import fs from 'node:fs'
|
||||
import os from 'node:os'
|
||||
import path from 'node:path'
|
||||
import test from 'node:test'
|
||||
import Database from 'better-sqlite3'
|
||||
import { Client } from '@modelcontextprotocol/sdk/client/index.js'
|
||||
import { InMemoryTransport } from '@modelcontextprotocol/sdk/inMemory.js'
|
||||
import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'
|
||||
import { CHAT_DB_TABLES } from '@openchatlab/core'
|
||||
import { DatabaseManager, NodePathProvider } from '@openchatlab/node-runtime'
|
||||
import { startMcpServer } from './server'
|
||||
|
||||
/** Full MCP protocol tests for the two trust boundaries identified in PR review. */
|
||||
test('MCP confines session IDs to real ChatLab databases and rejects SQL prepare side effects', async (t) => {
|
||||
const root = fs.mkdtempSync(path.join(os.tmpdir(), 'chatlab-mcp-safety-'))
|
||||
t.after(() => fs.rmSync(root, { recursive: true, force: true }))
|
||||
const dataDir = path.join(root, 'data')
|
||||
const dbDir = path.join(dataDir, 'databases')
|
||||
fs.mkdirSync(dbDir, { recursive: true })
|
||||
const dbPath = path.join(dbDir, 'valid.db')
|
||||
const db = new Database(dbPath)
|
||||
try {
|
||||
db.pragma('journal_mode=WAL')
|
||||
db.exec(CHAT_DB_TABLES)
|
||||
db.exec(`
|
||||
INSERT INTO meta(name, platform, type, imported_at)
|
||||
VALUES('Valid chat', 'qq', 'group', 1000);
|
||||
INSERT INTO member(platform_id, account_name) VALUES('p1', 'Alice');
|
||||
INSERT INTO message(sender_id, ts, type, content) VALUES(1, 1000, 0, 'allowed chat content');
|
||||
`)
|
||||
} finally {
|
||||
db.close()
|
||||
}
|
||||
|
||||
const privateDbPath = path.join(dataDir, 'private.db')
|
||||
const privateDb = new Database(privateDbPath)
|
||||
try {
|
||||
privateDb.exec("CREATE TABLE secrets(value TEXT); INSERT INTO secrets VALUES('SYNTHETIC_OUTSIDE_SESSION_SECRET');")
|
||||
} finally {
|
||||
privateDb.close()
|
||||
}
|
||||
const foreignDbPath = path.join(dbDir, 'foreign.db')
|
||||
const foreignDb = new Database(foreignDbPath)
|
||||
try {
|
||||
foreignDb.exec("CREATE TABLE secrets(value TEXT); INSERT INTO secrets VALUES('NON_CHATLAB_SECRET');")
|
||||
} finally {
|
||||
foreignDb.close()
|
||||
}
|
||||
|
||||
const manager = new DatabaseManager(new NodePathProvider(dataDir), {
|
||||
runtime: { version: '0.37.6', kind: 'mcp' },
|
||||
})
|
||||
manager.raiseCurrentChatDbCompatibilityGate()
|
||||
t.after(() => manager.closeAll())
|
||||
assert.deepEqual(manager.listSessionIdsMcpReadonly(), ['valid'])
|
||||
// File symlinks require elevated privilege on some Windows test runners.
|
||||
// Exercise the symlink boundary on POSIX; Windows-specific path checks still run.
|
||||
if (process.platform !== 'win32') fs.symlinkSync(privateDbPath, path.join(dbDir, 'linked.db'))
|
||||
const markerPath = path.join(dataDir, '.chatlab-meta.json')
|
||||
const beforeMarker = fs.readFileSync(markerPath)
|
||||
const beforeDb = fs.readFileSync(dbPath)
|
||||
|
||||
const client = new Client({ name: 'mcp-safety-client', version: '1.0.0' })
|
||||
const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair()
|
||||
const connect = McpServer.prototype.connect
|
||||
t.mock.method(McpServer.prototype, 'connect', function (this: McpServer) {
|
||||
t.after(() => this.close())
|
||||
return connect.call(this, serverTransport)
|
||||
})
|
||||
await startMcpServer({ version: '0.37.6', dbManager: manager })
|
||||
await client.connect(clientTransport)
|
||||
try {
|
||||
for (const sessionId of [
|
||||
'../private',
|
||||
'..\\private',
|
||||
'C:\\outside',
|
||||
'/tmp/outside',
|
||||
'name:stream',
|
||||
'bad\u0000id',
|
||||
'.',
|
||||
'..',
|
||||
'linked',
|
||||
'foreign',
|
||||
]) {
|
||||
const result = await client.callTool({
|
||||
name: 'chatlab_execute_sql',
|
||||
arguments: { session_id: sessionId, sql: 'SELECT * FROM secrets', format: 'json' },
|
||||
})
|
||||
assert.equal(result.isError, true, `${JSON.stringify(sessionId)} escaped session boundary`)
|
||||
assert.doesNotMatch(JSON.stringify(result.content), /SYNTHETIC_OUTSIDE_SESSION_SECRET|NON_CHATLAB_SECRET/)
|
||||
}
|
||||
for (const uri of [
|
||||
'chatlab://sessions/..%2Fprivate/meta',
|
||||
'chatlab://sessions/..%5Cprivate/schema',
|
||||
'chatlab://sessions/linked/meta',
|
||||
'chatlab://sessions/foreign/schema',
|
||||
]) {
|
||||
let body: string
|
||||
try {
|
||||
const result = await client.readResource({ uri })
|
||||
body = JSON.stringify(result.contents)
|
||||
} catch (error) {
|
||||
body = String(error)
|
||||
}
|
||||
assert.doesNotMatch(body, /SYNTHETIC_OUTSIDE_SESSION_SECRET|NON_CHATLAB_SECRET/, uri)
|
||||
assert.match(body, /Session not found|Invalid MCP session|Not a ChatLab|not found|Invalid Request/i, uri)
|
||||
}
|
||||
|
||||
const check = async (sql: string) =>
|
||||
client.callTool({ name: 'chatlab_execute_sql', arguments: { session_id: 'valid', sql, format: 'json' } })
|
||||
// Query a second SQLite connection to prove that prepare() itself did not
|
||||
// change the process-global temp_store_directory setting.
|
||||
const probe = new Database(':memory:')
|
||||
const beforeTempDirectory = probe.pragma('temp_store_directory')
|
||||
try {
|
||||
const pragma = await check(`PRAGMA temp_store_directory='${root.replace(/'/g, "''")}' -- LIMIT`)
|
||||
assert.equal(pragma.isError, true)
|
||||
assert.match(JSON.stringify(pragma.content), /Only SELECT or WITH/i)
|
||||
assert.deepEqual(probe.pragma('temp_store_directory'), beforeTempDirectory)
|
||||
} finally {
|
||||
probe.close()
|
||||
}
|
||||
for (const sql of [
|
||||
'ATTACH DATABASE ":memory:" AS extra',
|
||||
'EXPLAIN PRAGMA temp_store_directory',
|
||||
'BEGIN',
|
||||
'/* comment */ PRAGMA writable_schema=ON',
|
||||
]) {
|
||||
const result = await check(sql)
|
||||
assert.equal(result.isError, true, sql)
|
||||
assert.match(JSON.stringify(result.content), /Only SELECT or WITH/i, sql)
|
||||
}
|
||||
// WITH is allowed lexically, but SQLite's native readonly flag must
|
||||
// still reject DML hidden behind the CTE entry point.
|
||||
const withWrite = await check(
|
||||
"WITH c(x) AS (SELECT 1) UPDATE message SET content='TAMPERED' WHERE id IN (SELECT x FROM c)"
|
||||
)
|
||||
assert.match(JSON.stringify(withWrite.content), /read-only statements/i)
|
||||
|
||||
const validSql = [
|
||||
'SELECT content FROM message',
|
||||
'-- comment\n SELECT content FROM message',
|
||||
'/* leading */\nWITH c(x) AS (SELECT 7) SELECT x FROM c',
|
||||
]
|
||||
for (const sql of validSql) {
|
||||
const result = await check(sql)
|
||||
assert.equal(result.isError, undefined, `${sql}: ${JSON.stringify(result.content)}`)
|
||||
}
|
||||
assert.deepEqual(fs.readFileSync(dbPath), beforeDb)
|
||||
assert.deepEqual(fs.readFileSync(markerPath), beforeMarker)
|
||||
const other = new Database(privateDbPath, { readonly: true })
|
||||
try {
|
||||
assert.deepEqual(other.prepare('SELECT value FROM secrets').all(), [
|
||||
{ value: 'SYNTHETIC_OUTSIDE_SESSION_SECRET' },
|
||||
])
|
||||
} finally {
|
||||
other.close()
|
||||
}
|
||||
} finally {
|
||||
await client.close()
|
||||
}
|
||||
})
|
||||
@@ -17,7 +17,7 @@ import {
|
||||
} from '@openchatlab/node-runtime'
|
||||
import { registerTools } from './server'
|
||||
|
||||
test('advertises every ChatLab MCP query tool as read-only', async () => {
|
||||
test('advertises logical ChatLab MCP queries as read-only', async () => {
|
||||
const server = new McpServer({ name: 'chatlab-test', version: '0.0.0' })
|
||||
const client = new Client({ name: 'chatlab-test-client', version: '0.0.0' })
|
||||
const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair()
|
||||
@@ -49,6 +49,36 @@ test('advertises every ChatLab MCP query tool as read-only', async () => {
|
||||
}
|
||||
})
|
||||
|
||||
test('rejects unsafe raw MCP SQL before opening a database', async () => {
|
||||
const server = new McpServer({ name: 'chatlab-sql-precheck', version: '0.0.0' })
|
||||
const client = new Client({ name: 'chatlab-sql-precheck-client', version: '0.0.0' })
|
||||
const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair()
|
||||
let databaseOpenCalls = 0
|
||||
|
||||
try {
|
||||
registerTools(server, {
|
||||
listSessionIdsMcpReadonly: () => [],
|
||||
openMcpReadonly: () => {
|
||||
databaseOpenCalls++
|
||||
throw new Error('Unsafe SQL should never open a database')
|
||||
},
|
||||
})
|
||||
await server.connect(serverTransport)
|
||||
await client.connect(clientTransport)
|
||||
|
||||
const result = await client.callTool({
|
||||
name: 'chatlab_execute_sql',
|
||||
arguments: { session_id: 'missing', sql: "PRAGMA temp_store_directory='/tmp'" },
|
||||
})
|
||||
assert.equal(result.isError, true)
|
||||
assert.match(JSON.stringify(result.content), /Only SELECT or WITH queries/i)
|
||||
assert.equal(databaseOpenCalls, 0)
|
||||
} finally {
|
||||
await client.close()
|
||||
await server.close()
|
||||
}
|
||||
})
|
||||
|
||||
test('MCP rejects schema v5 before dispatch without migrating or raising the compatibility gate', async (t) => {
|
||||
const root = fs.mkdtempSync(path.join(os.tmpdir(), 'chatlab-mcp-readonly-'))
|
||||
t.after(() => fs.rmSync(root, { recursive: true, force: true }))
|
||||
|
||||
@@ -9,15 +9,19 @@ import { McpServer, ResourceTemplate } from '@modelcontextprotocol/sdk/server/mc
|
||||
import { StdioServerTransport } from '@modelcontextprotocol/sdk/server/stdio.js'
|
||||
import { z } from 'zod'
|
||||
import { getSessionMeta, getSessionOverview, getDatabaseSchema } from '@openchatlab/core'
|
||||
import type { DatabaseAdapter } from '@openchatlab/core'
|
||||
import { MCP_TOOL_REGISTRY, CoreDataProvider, getLocalizedToolMetadata } from '@openchatlab/tools'
|
||||
import type { SessionListContext } from '@openchatlab/tools/src/definitions/sessions'
|
||||
import type { McpDatabaseManager, McpServerOptions } from './types'
|
||||
import { jsonSchemaToZod } from './schema'
|
||||
import { formatToolResultAsText } from './format'
|
||||
import { assertMcpSqlReadOnly } from './sql-guard'
|
||||
|
||||
const MCP_TOOL_PREFIX = 'chatlab_'
|
||||
|
||||
// These tools only read logical chat data, never migrate sessions or modify
|
||||
// the compatibility marker. SQLite's ordinary WAL/SHM coordination can still
|
||||
// change auxiliary filesystem files; that contract distinction is documented
|
||||
// explicitly and left for upstream maintainer approval.
|
||||
const READ_ONLY_TOOL_ANNOTATIONS = {
|
||||
readOnlyHint: true,
|
||||
destructiveHint: false,
|
||||
@@ -53,28 +57,19 @@ export function registerTools(server: McpServer, dbManager: McpDatabaseManager,
|
||||
{ description, inputSchema: zodShape, annotations: READ_ONLY_TOOL_ANNOTATIONS },
|
||||
async (params) => {
|
||||
const format = params.format as string | undefined
|
||||
// The shared list handler opens multiple databases; close every connection
|
||||
// after it returns, including when a query throws.
|
||||
const openedDbs: DatabaseAdapter[] = []
|
||||
try {
|
||||
const context: SessionListContext = {
|
||||
db: null as any,
|
||||
sessionId: '',
|
||||
listSessionIds: () => dbManager.listSessionIdsMcpReadonly(),
|
||||
openDb: (id) => {
|
||||
const db = dbManager.openMcpReadonly(id)
|
||||
if (db) openedDbs.push(db)
|
||||
return db
|
||||
},
|
||||
}
|
||||
const toolParams = { ...params } as Record<string, unknown>
|
||||
delete toolParams.format
|
||||
const result = await tool.handler(toolParams, context)
|
||||
const text = applyFormat(result.content, format, tool.name)
|
||||
return { content: [{ type: 'text' as const, text }] }
|
||||
} finally {
|
||||
for (const db of openedDbs) db.close()
|
||||
const context: SessionListContext = {
|
||||
db: null as any,
|
||||
sessionId: '',
|
||||
listSessionIds: () => dbManager.listSessionIdsMcpReadonly(),
|
||||
openDb: (id) => dbManager.openMcpReadonly(id),
|
||||
// Close each independent session transaction before opening the next.
|
||||
closeDb: (db) => db.close(),
|
||||
}
|
||||
const toolParams = { ...params } as Record<string, unknown>
|
||||
delete toolParams.format
|
||||
const result = await tool.handler(toolParams, context)
|
||||
const text = applyFormat(result.content, format, tool.name)
|
||||
return { content: [{ type: 'text' as const, text }] }
|
||||
}
|
||||
)
|
||||
continue
|
||||
@@ -92,6 +87,12 @@ export function registerTools(server: McpServer, dbManager: McpDatabaseManager,
|
||||
async (params) => {
|
||||
const sessionId = params.session_id as string
|
||||
const format = params.format as string | undefined
|
||||
// Reject SQL that can have prepare-time side effects before even opening
|
||||
// the SQLite database, which might itself create WAL/SHM sidecars.
|
||||
const rawSql = (params as Record<string, unknown>).sql
|
||||
if ((tool.name === 'execute_sql' || tool.name === 'render_chart') && typeof rawSql === 'string') {
|
||||
assertMcpSqlReadOnly(rawSql)
|
||||
}
|
||||
const db = dbManager.openMcpReadonly(sessionId)
|
||||
if (!db) {
|
||||
return {
|
||||
@@ -104,7 +105,6 @@ export function registerTools(server: McpServer, dbManager: McpDatabaseManager,
|
||||
const toolParams = { ...params } as Record<string, unknown>
|
||||
delete toolParams.session_id
|
||||
delete toolParams.format
|
||||
|
||||
const result = await tool.handler(toolParams, { db, sessionId, dataProvider: new CoreDataProvider(db) })
|
||||
const text = applyFormat(result.content, format, tool.name)
|
||||
return { content: [{ type: 'text' as const, text }] }
|
||||
|
||||
@@ -0,0 +1,106 @@
|
||||
import assert from 'node:assert/strict'
|
||||
import fs from 'node:fs'
|
||||
import path from 'node:path'
|
||||
import test from 'node:test'
|
||||
import { Client } from '@modelcontextprotocol/sdk/client/index.js'
|
||||
import { InMemoryTransport } from '@modelcontextprotocol/sdk/inMemory.js'
|
||||
import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'
|
||||
import { CHAT_DB_TABLES } from '@openchatlab/core'
|
||||
import type { DatabaseAdapter } from '@openchatlab/core'
|
||||
import { DatabaseManager, NodePathProvider } from '@openchatlab/node-runtime'
|
||||
import { startMcpServer } from './server'
|
||||
import { openTestSqliteDatabase } from '../../../tests/helpers/sqlite.mts'
|
||||
|
||||
test('MCP list_sessions closes each read transaction before opening the next one', async (t) => {
|
||||
const root = fs.mkdtempSync(path.join(process.cwd(), '.chatlab-mcp-list-lifecycle-'))
|
||||
t.after(() => fs.rmSync(root, { recursive: true, force: true }))
|
||||
const dataDir = path.join(root, 'data')
|
||||
const dbDir = path.join(dataDir, 'databases')
|
||||
fs.mkdirSync(dbDir, { recursive: true })
|
||||
|
||||
const firstPath = path.join(dbDir, 'first.db')
|
||||
const fixture = openTestSqliteDatabase(firstPath)
|
||||
try {
|
||||
fixture.pragma('journal_mode = WAL')
|
||||
fixture.exec(CHAT_DB_TABLES)
|
||||
fixture.exec(`
|
||||
INSERT INTO meta (name, platform, type, imported_at)
|
||||
VALUES ('Session One', 'qq', 'group', 1000);
|
||||
`)
|
||||
} finally {
|
||||
fixture.close()
|
||||
}
|
||||
const secondPath = path.join(dbDir, 'second.db')
|
||||
fs.copyFileSync(firstPath, secondPath)
|
||||
const second = openTestSqliteDatabase(secondPath)
|
||||
try {
|
||||
second.exec("UPDATE meta SET name = 'Session Two'")
|
||||
} finally {
|
||||
second.close()
|
||||
}
|
||||
|
||||
const pathProvider = new NodePathProvider(dataDir)
|
||||
const manager = new DatabaseManager(pathProvider, { runtime: { version: '0.37.6', kind: 'mcp' } })
|
||||
t.after(() => manager.closeAll())
|
||||
manager.raiseCurrentChatDbCompatibilityGate()
|
||||
const markerPath = path.join(dataDir, '.chatlab-meta.json')
|
||||
const beforeMarker = fs.readFileSync(markerPath)
|
||||
|
||||
let activeConnections = 0
|
||||
let peakConnections = 0
|
||||
let sessionsOpened = 0
|
||||
let sessionsClosed = 0
|
||||
const trackedManager = {
|
||||
listSessionIdsMcpReadonly: () => manager.listSessionIdsMcpReadonly(),
|
||||
openMcpReadonly(sessionId: string): DatabaseAdapter | null {
|
||||
const db = manager.openMcpReadonly(sessionId)
|
||||
if (!db) return null
|
||||
activeConnections++
|
||||
sessionsOpened++
|
||||
peakConnections = Math.max(peakConnections, activeConnections)
|
||||
return {
|
||||
readonly: db.readonly,
|
||||
exec: (sql) => db.exec(sql),
|
||||
prepare: (sql) => db.prepare(sql),
|
||||
transaction: (fn) => db.transaction(fn),
|
||||
pragma: (pragma) => db.pragma(pragma),
|
||||
close() {
|
||||
db.close()
|
||||
activeConnections--
|
||||
sessionsClosed++
|
||||
},
|
||||
}
|
||||
},
|
||||
}
|
||||
|
||||
const client = new Client({ name: 'session-list-lifetime-test-client', version: '1.0.0' })
|
||||
const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair()
|
||||
const connect = McpServer.prototype.connect
|
||||
t.mock.method(McpServer.prototype, 'connect', function (this: McpServer) {
|
||||
t.after(() => this.close())
|
||||
return connect.call(this, serverTransport)
|
||||
})
|
||||
|
||||
await startMcpServer({ version: '0.37.6', dbManager: trackedManager })
|
||||
await client.connect(clientTransport)
|
||||
try {
|
||||
const result = await client.callTool({ name: 'chatlab_list_sessions', arguments: { format: 'json' } })
|
||||
assert.equal(result.isError, undefined, JSON.stringify(result.content))
|
||||
const parsed = JSON.parse((result.content as Array<{ text: string }>)[0].text)
|
||||
assert.equal(parsed.total, 2)
|
||||
assert.deepEqual(parsed.sessions.map((s: { id: string }) => s.id).sort(), ['first', 'second'])
|
||||
assert.equal(peakConnections, 1, 'a second session connection opened before the previous one was closed')
|
||||
assert.equal(activeConnections, 0)
|
||||
assert.equal(sessionsOpened, 2)
|
||||
assert.equal(sessionsClosed, 2)
|
||||
|
||||
const resource = await client.readResource({ uri: 'chatlab://sessions' })
|
||||
assert.match(JSON.stringify(resource.contents), /Session One/)
|
||||
assert.match(JSON.stringify(resource.contents), /Session Two/)
|
||||
assert.ok(fs.readdirSync(dbDir).includes('first.db'))
|
||||
assert.ok(fs.readdirSync(dbDir).includes('second.db'))
|
||||
assert.deepEqual(fs.readFileSync(markerPath), beforeMarker)
|
||||
} finally {
|
||||
await client.close()
|
||||
}
|
||||
})
|
||||
@@ -0,0 +1,27 @@
|
||||
/**
|
||||
* MCP-only guard for user-provided SQL. SQLite can change global settings
|
||||
* during prepare() (e.g. some PRAGMAs), before stmt.readonly can be checked.
|
||||
* This lexical precheck is deliberately narrow, not a replacement for SQLite's
|
||||
* native single-statement/read-only verification in the shared SQL executor.
|
||||
*/
|
||||
export function assertMcpSqlReadOnly(sql: string): void {
|
||||
let pos = 0
|
||||
while (pos < sql.length) {
|
||||
while (pos < sql.length && /\s/.test(sql[pos])) pos++
|
||||
if (sql.startsWith('--', pos)) {
|
||||
const end = sql.indexOf('\n', pos + 2)
|
||||
pos = end < 0 ? sql.length : end + 1
|
||||
continue
|
||||
}
|
||||
if (sql.startsWith('/*', pos)) {
|
||||
const end = sql.indexOf('*/', pos + 2)
|
||||
if (end < 0) throw new Error('Only SELECT or WITH queries are allowed in MCP')
|
||||
pos = end + 2
|
||||
continue
|
||||
}
|
||||
break
|
||||
}
|
||||
if (!/^(SELECT|WITH)\b/i.test(sql.slice(pos))) {
|
||||
throw new Error('Only SELECT or WITH queries are allowed in MCP')
|
||||
}
|
||||
}
|
||||
@@ -4,7 +4,7 @@ import os from 'node:os'
|
||||
import path from 'node:path'
|
||||
import test from 'node:test'
|
||||
import Database from 'better-sqlite3'
|
||||
import type { PathProvider } from '@openchatlab/core'
|
||||
import type { DatabaseAdapter, PathProvider } from '@openchatlab/core'
|
||||
import { CHAT_DB_SCHEMA, CURRENT_SCHEMA_VERSION, getSessionInfo } from '@openchatlab/core'
|
||||
import { DataDirCompatibilityError, readDataDirCompatibilityMeta } from './data-dir-compat'
|
||||
import { DatabaseManager, listDatabaseCandidateIds } from './database-manager'
|
||||
@@ -1477,3 +1477,114 @@ test('MCP schema preflight rejects invalid SQLite version values without affecti
|
||||
}
|
||||
}
|
||||
})
|
||||
|
||||
test('MCP live WAL reads use one committed SQLite snapshot across multiple SELECTs', () => {
|
||||
const root = makeTempDir()
|
||||
const provider = createPathProvider(root)
|
||||
fs.mkdirSync(provider.getDatabaseDir(), { recursive: true })
|
||||
const dbPath = path.join(provider.getDatabaseDir(), 'concurrent.db')
|
||||
const writer = new Database(dbPath, { nativeBinding })
|
||||
const manager = new DatabaseManager(provider, {
|
||||
nativeBinding,
|
||||
runtime: { version: '0.37.6', kind: 'mcp' },
|
||||
})
|
||||
try {
|
||||
writer.pragma('journal_mode=WAL')
|
||||
writer.exec(CHAT_DB_SCHEMA)
|
||||
writer.exec(`
|
||||
INSERT INTO meta(name, platform, type, imported_at) VALUES('Concurrent', 'qq', 'group', 1000);
|
||||
INSERT INTO member(platform_id, account_name) VALUES('p1', 'Alice');
|
||||
INSERT INTO message(sender_id, ts, type, content) VALUES(1, 1000, 0, 'committed earlier');
|
||||
`)
|
||||
manager.raiseCurrentChatDbCompatibilityGate()
|
||||
const markerPath = path.join(provider.getUserDataDir(), '.chatlab-meta.json')
|
||||
const beforeMarker = fs.readFileSync(markerPath)
|
||||
const count = (db: DatabaseAdapter) => (db.prepare('SELECT count(*) as n FROM message').get() as { n: number }).n
|
||||
const first = manager.openMcpReadonly('concurrent')
|
||||
assert.ok(first)
|
||||
try {
|
||||
assert.equal(first.readonly, true)
|
||||
assert.equal(count(first), 1)
|
||||
writer.exec("INSERT INTO message(sender_id, ts, type, content) VALUES(1, 1001, 0, 'committed later')")
|
||||
// The version check and both queries must see the same transaction view.
|
||||
assert.equal(count(first), 1)
|
||||
const second = manager.openMcpReadonly('concurrent')
|
||||
assert.ok(second)
|
||||
try {
|
||||
assert.equal(count(second), 2)
|
||||
} finally {
|
||||
second.close()
|
||||
}
|
||||
assert.equal(count(first), 1)
|
||||
} finally {
|
||||
first.close()
|
||||
}
|
||||
assert.equal((writer.pragma('wal_checkpoint(TRUNCATE)') as Array<{ busy: number }>)[0].busy, 0)
|
||||
assert.deepEqual(fs.readFileSync(markerPath), beforeMarker)
|
||||
} finally {
|
||||
manager.closeAll()
|
||||
writer.close()
|
||||
}
|
||||
})
|
||||
|
||||
test('MCP does not return uncommitted changes from a rollback-journal writer', () => {
|
||||
const root = makeTempDir()
|
||||
const provider = createPathProvider(root)
|
||||
fs.mkdirSync(provider.getDatabaseDir(), { recursive: true })
|
||||
const dbPath = path.join(provider.getDatabaseDir(), 'rollback.db')
|
||||
const writer = new Database(dbPath, { nativeBinding })
|
||||
const manager = new DatabaseManager(provider, {
|
||||
nativeBinding,
|
||||
runtime: { version: '0.37.6', kind: 'mcp' },
|
||||
})
|
||||
try {
|
||||
writer.exec(CHAT_DB_SCHEMA)
|
||||
writer.exec(`
|
||||
INSERT INTO meta(name, platform, type, imported_at) VALUES('Rollback', 'qq', 'group', 1000);
|
||||
INSERT INTO member(platform_id, account_name) VALUES('p1', 'Alice');
|
||||
`)
|
||||
writer.pragma('journal_mode=DELETE')
|
||||
writer.pragma('cache_size=10')
|
||||
writer.pragma('cache_spill=ON')
|
||||
const insert = writer.prepare('INSERT INTO message(sender_id, ts, type, content) VALUES(1, ?, 0, ?)')
|
||||
writer.transaction(() => {
|
||||
for (let i = 0; i < 90; i++) insert.run(i, 'committed '.padEnd(1600, 'a'))
|
||||
})()
|
||||
manager.raiseCurrentChatDbCompatibilityGate()
|
||||
writer.exec('BEGIN IMMEDIATE')
|
||||
try {
|
||||
writer.exec("UPDATE message SET content = 'UNCOMMITTED ' || content")
|
||||
assert.equal(fs.existsSync(dbPath + '-journal'), true)
|
||||
try {
|
||||
const reader = manager.openMcpReadonly('rollback')
|
||||
if (reader) {
|
||||
try {
|
||||
const uncommitted = reader
|
||||
.prepare("SELECT count(*) AS n FROM message WHERE content LIKE 'UNCOMMITTED%'")
|
||||
.get() as { n: number }
|
||||
assert.equal(uncommitted.n, 0, 'a live MCP reader must never expose uncommitted rollback pages')
|
||||
} finally {
|
||||
reader.close()
|
||||
}
|
||||
}
|
||||
} catch (error) {
|
||||
assert.match(String(error), /BUSY|locked/i, 'only a SQLite lock error is acceptable')
|
||||
}
|
||||
} finally {
|
||||
writer.exec('ROLLBACK')
|
||||
}
|
||||
const clean = manager.openMcpReadonly('rollback')
|
||||
assert.ok(clean)
|
||||
try {
|
||||
assert.equal(
|
||||
(clean.prepare("SELECT count(*) AS n FROM message WHERE content LIKE 'UNCOMMITTED%'").get() as { n: number }).n,
|
||||
0
|
||||
)
|
||||
} finally {
|
||||
clean.close()
|
||||
}
|
||||
} finally {
|
||||
manager.closeAll()
|
||||
writer.close()
|
||||
}
|
||||
})
|
||||
@@ -64,6 +64,14 @@ export function listDatabaseCandidateIds(databaseDir: string): string[] {
|
||||
.map((fileName) => fileName.slice(0, -3))
|
||||
}
|
||||
|
||||
/** A non-ChatLab SQLite file may be skipped during discovery, not opened directly. */
|
||||
class NotChatLabSessionError extends Error {
|
||||
constructor(sessionId: string) {
|
||||
super(`Not a ChatLab session database: ${sessionId}`)
|
||||
this.name = 'NotChatLabSessionError'
|
||||
}
|
||||
}
|
||||
|
||||
export class DatabaseManager {
|
||||
private cache = new Map<string, DatabaseAdapter>()
|
||||
private nativeBinding?: string
|
||||
@@ -144,41 +152,89 @@ export class DatabaseManager {
|
||||
}
|
||||
|
||||
/**
|
||||
* MCP-only strict read access. Never migrate or repair data. Older schema
|
||||
* versions require an explicit upgrade through a writable ChatLab runtime.
|
||||
* Validate MCP session paths without tightening the shared CLI/desktop APIs.
|
||||
* Reject out-of-scope files, directory traversal, symlinks and non-files.
|
||||
*/
|
||||
private resolveMcpSessionPath(sessionId: string): string | null {
|
||||
if (
|
||||
!sessionId ||
|
||||
sessionId === '.' ||
|
||||
sessionId === '..' ||
|
||||
/[\\/:]/.test(sessionId) ||
|
||||
[...sessionId].some((character) => character.charCodeAt(0) < 32) ||
|
||||
/[. ]$/.test(sessionId) ||
|
||||
path.isAbsolute(sessionId) ||
|
||||
path.win32.isAbsolute(sessionId)
|
||||
) {
|
||||
throw new Error(`Invalid MCP session ID: ${JSON.stringify(sessionId)}`)
|
||||
}
|
||||
|
||||
const databaseDir = this.pathProvider.getDatabaseDir()
|
||||
const dbPath = path.join(databaseDir, `${sessionId}.db`)
|
||||
let entry: fs.Stats
|
||||
try {
|
||||
entry = fs.lstatSync(dbPath)
|
||||
} catch (error) {
|
||||
if ((error as NodeJS.ErrnoException).code === 'ENOENT') return null
|
||||
throw error
|
||||
}
|
||||
if (!entry.isFile() || entry.isSymbolicLink()) {
|
||||
throw new Error(`Invalid MCP session database path: ${sessionId}`)
|
||||
}
|
||||
|
||||
// A configured database root may itself be a symlink. Resolve both sides
|
||||
// and verify the real target is the expected direct child of that root.
|
||||
const root = fs.realpathSync(databaseDir)
|
||||
const actual = fs.realpathSync(dbPath)
|
||||
if (path.relative(root, actual) !== `${sessionId}.db`) {
|
||||
throw new Error(`MCP session database is outside the configured directory: ${sessionId}`)
|
||||
}
|
||||
return dbPath
|
||||
}
|
||||
|
||||
/**
|
||||
* MCP-only live SQLite read. No migrations or marker updates, and no
|
||||
* full-file snapshot: the native read transaction isolates committed data.
|
||||
* Normal WAL coordination may create/update -wal or -shm sidecars.
|
||||
*/
|
||||
openMcpReadonly(sessionId: string): DatabaseAdapter | null {
|
||||
const db = this.openReadonly(sessionId)
|
||||
if (!db) return null
|
||||
this.assertCompatible()
|
||||
const dbPath = this.resolveMcpSessionPath(sessionId)
|
||||
if (!dbPath) return null
|
||||
const db = openBetterSqliteDatabase(dbPath, { readonly: true, nativeBinding: this.nativeBinding })
|
||||
|
||||
try {
|
||||
if (isChatSessionDb(db)) {
|
||||
const schemaVersion = getSchemaVersion(db)
|
||||
// SQLite INTEGER-affinity columns may actually contain TEXT, REAL or
|
||||
// BLOB values. A type assertion cannot protect numeric comparisons
|
||||
// from coercing invalid values (e.g. nonnumeric text to NaN).
|
||||
if (!Number.isSafeInteger(schemaVersion) || schemaVersion < 0) {
|
||||
throw new Error(
|
||||
`ChatLab session ${sessionId} has an invalid schema version. ` +
|
||||
'Cannot safely serve this database through read-only MCP tools.'
|
||||
)
|
||||
}
|
||||
// The runtime cannot understand a future schema or safely repair its
|
||||
// compatibility metadata; report the version mismatch before the gate.
|
||||
if (schemaVersion > CURRENT_SCHEMA_VERSION) {
|
||||
throw new Error(
|
||||
`ChatLab session ${sessionId} uses schema v${schemaVersion}, but read-only MCP tools require ` +
|
||||
`schema v${CURRENT_SCHEMA_VERSION}. Upgrade the ChatLab runtime to one that supports this newer schema.`
|
||||
)
|
||||
}
|
||||
// BEGIN is deferred; the first schema query fixes the SQLite read view.
|
||||
// Closing this one-request connection releases the read transaction.
|
||||
db.exec('BEGIN')
|
||||
if (!isChatSessionDb(db)) {
|
||||
throw new NotChatLabSessionError(sessionId)
|
||||
}
|
||||
const schemaVersion = getSchemaVersion(db)
|
||||
// SQLite INTEGER-affinity columns may actually contain TEXT, REAL or
|
||||
// BLOB values. A type assertion cannot protect numeric comparisons
|
||||
// from coercing invalid values (e.g. nonnumeric text to NaN).
|
||||
if (!Number.isSafeInteger(schemaVersion) || schemaVersion < 0) {
|
||||
throw new Error(
|
||||
`ChatLab session ${sessionId} has an invalid schema version. ` +
|
||||
'Cannot safely serve this database through read-only MCP tools.'
|
||||
)
|
||||
}
|
||||
// The runtime cannot understand a future schema or safely repair its
|
||||
// compatibility metadata; report the version mismatch before the gate.
|
||||
if (schemaVersion > CURRENT_SCHEMA_VERSION) {
|
||||
throw new Error(
|
||||
`ChatLab session ${sessionId} uses schema v${schemaVersion}, but read-only MCP tools require ` +
|
||||
`schema v${CURRENT_SCHEMA_VERSION}. Upgrade the ChatLab runtime to one that supports this newer schema.`
|
||||
)
|
||||
}
|
||||
|
||||
this.assertReadonlyCompatibilityGate(schemaVersion)
|
||||
if (schemaVersion < CURRENT_SCHEMA_VERSION) {
|
||||
throw new Error(
|
||||
`ChatLab session ${sessionId} uses schema v${schemaVersion}, but read-only MCP tools require ` +
|
||||
`schema v${CURRENT_SCHEMA_VERSION}. Open the session with the current writable ChatLab runtime to migrate it first.`
|
||||
)
|
||||
}
|
||||
this.assertReadonlyCompatibilityGate(schemaVersion)
|
||||
if (schemaVersion < CURRENT_SCHEMA_VERSION) {
|
||||
throw new Error(
|
||||
`ChatLab session ${sessionId} uses schema v${schemaVersion}, but read-only MCP tools require ` +
|
||||
`schema v${CURRENT_SCHEMA_VERSION}. Open the session with the current writable ChatLab runtime to migrate it first.`
|
||||
)
|
||||
}
|
||||
return db
|
||||
} catch (error) {
|
||||
@@ -287,10 +343,18 @@ export class DatabaseManager {
|
||||
this.assertCompatible()
|
||||
|
||||
return listDatabaseCandidateIds(this.pathProvider.getDatabaseDir()).filter((sessionId) => {
|
||||
const db = this.openMcpReadonly(sessionId)
|
||||
let db: DatabaseAdapter | null
|
||||
try {
|
||||
db = this.openMcpReadonly(sessionId)
|
||||
} catch (error) {
|
||||
// Discovery may ignore proven non-ChatLab databases, but must not
|
||||
// silently swallow damaged databases or schema/gate/permission errors.
|
||||
if (error instanceof NotChatLabSessionError) return false
|
||||
throw error
|
||||
}
|
||||
if (!db) return false
|
||||
try {
|
||||
return isChatSessionDb(db)
|
||||
return true // openMcpReadonly has already validated its type and schema.
|
||||
} finally {
|
||||
db.close()
|
||||
}
|
||||
|
||||
@@ -0,0 +1,29 @@
|
||||
import assert from 'node:assert/strict'
|
||||
import test from 'node:test'
|
||||
import type { DatabaseAdapter } from '@openchatlab/core'
|
||||
import { sessionsListTool, type SessionListContext } from './sessions'
|
||||
|
||||
test('list_sessions closes each per-session database even when metadata query throws', () => {
|
||||
let closeCount = 0
|
||||
const fakeDb: DatabaseAdapter = {
|
||||
readonly: true,
|
||||
prepare: () => {
|
||||
throw new Error('synthetic database query failure')
|
||||
},
|
||||
pragma: () => [],
|
||||
exec: () => undefined,
|
||||
transaction: (fn) => fn(),
|
||||
close: () => {
|
||||
closeCount++
|
||||
},
|
||||
}
|
||||
const context: SessionListContext = {
|
||||
db: fakeDb,
|
||||
sessionId: '',
|
||||
listSessionIds: () => ['one'],
|
||||
openDb: () => fakeDb,
|
||||
closeDb: (db) => db.close(),
|
||||
}
|
||||
assert.throws(() => sessionsListTool.handler({}, context), /synthetic database query failure/)
|
||||
assert.equal(closeCount, 1)
|
||||
})
|
||||
@@ -22,6 +22,8 @@ const inputSchema: JsonSchema = {
|
||||
export interface SessionListContext extends ToolExecutionContext {
|
||||
listSessionIds: () => string[]
|
||||
openDb: (sessionId: string) => DatabaseAdapter | null
|
||||
/** Optional lifecycle hook for uncached per-session connections (e.g. MCP read transactions). */
|
||||
closeDb?: (db: DatabaseAdapter) => void
|
||||
}
|
||||
|
||||
function handler(_params: Record<string, unknown>, context: SessionListContext): ToolResult {
|
||||
@@ -32,11 +34,15 @@ function handler(_params: Record<string, unknown>, context: SessionListContext):
|
||||
.map((id) => {
|
||||
const db = context.openDb(id)
|
||||
if (!db) return null
|
||||
const meta = getSessionMeta(db)
|
||||
if (!meta) return null
|
||||
if (keyword && !meta.name.toLowerCase().includes(keyword)) return null
|
||||
const overview = getSessionOverview(db)
|
||||
return { id, ...meta, ...overview }
|
||||
try {
|
||||
const meta = getSessionMeta(db)
|
||||
if (!meta) return null
|
||||
if (keyword && !meta.name.toLowerCase().includes(keyword)) return null
|
||||
const overview = getSessionOverview(db)
|
||||
return { id, ...meta, ...overview }
|
||||
} finally {
|
||||
context.closeDb?.(db)
|
||||
}
|
||||
})
|
||||
.filter(Boolean)
|
||||
|
||||
|
||||
Reference in new issue
Block a user