Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ export function apply(ctx: Context, config: Config, chain: ChatChain) {

if (
resolved.constraint.manageMode === 'admin' &&
resolved.constraint.routeMode !== 'personal' &&
!(await checkAdmin(session))
) {
context.message = session.text(
Expand Down
23 changes: 21 additions & 2 deletions packages/core/src/middlewares/system/wipe.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { createLogger } from 'koishi-plugin-chatluna/utils/logger'
import fs from 'fs/promises'
import {
createLegacyTableRetention,
createPassedValidationResult,
dropTableIfExists,
getLegacySchemaSentinel,
getLegacySchemaSentinelDir,
Expand All @@ -15,6 +16,7 @@ import {
readMetaValue,
writeMetaValue
} from '../../migration/validators'
import { BUILTIN_SCHEMA_VERSION } from '../../migration/room_to_conversation'

let logger: Logger

Expand Down Expand Up @@ -76,12 +78,13 @@ export function apply(ctx: Context, _config: Config, chain: ChatChain) {
'chatluna_binding',
'chatluna_constraint',
'chatluna_archive',
'chatluna_acl',
'chatluna_meta'
'chatluna_acl'
]) {
await dropTableIfExists(ctx, table)
}

await ctx.database.remove('chatluna_meta', {})

for (const table of LEGACY_MIGRATION_TABLES) {
await dropTableIfExists(ctx, table)
}
Expand Down Expand Up @@ -115,6 +118,22 @@ export function apply(ctx: Context, _config: Config, chain: ChatChain) {
logger.warn(`wipe: ${e}`)
}

const validated = createPassedValidationResult()
await writeMetaValue(ctx, 'schema_version', BUILTIN_SCHEMA_VERSION)
await writeMetaValue(ctx, 'validation_result', validated)
await writeMetaValue(ctx, 'room_migration_done', true)
await writeMetaValue(ctx, 'message_migration_done', true)
await writeMetaValue(
ctx,
LEGACY_RETENTION_META_KEY,
createLegacyTableRetention('purged')
)
await writeMetaValue(
ctx,
'migration_finished_at',
new Date().toISOString()
)

const sentinelDir = getLegacySchemaSentinelDir(ctx.baseDir)
const sentinelPath = getLegacySchemaSentinel(ctx.baseDir)
await fs.mkdir(sentinelDir, { recursive: true })
Expand Down
7 changes: 4 additions & 3 deletions packages/core/src/migration/legacy_tables.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ export function getLegacySchemaSentinelDir(baseDir: string) {
return path.dirname(getLegacySchemaSentinel(baseDir))
}

export function defineLegacyMigrationTables(ctx: Context) {
if (existsSync(getLegacySchemaSentinel(ctx.baseDir))) {
export function defineLegacyMigrationTables(ctx: Context, force = false) {
if (!force && existsSync(getLegacySchemaSentinel(ctx.baseDir))) {
return
}

Expand Down Expand Up @@ -255,9 +255,10 @@ export function defineLegacyMigrationTables(ctx: Context) {
)
}

function isMissingTableError(error: unknown) {
export function isMissingTableError(error: unknown) {
const message = String(error).toLowerCase()
return (
message.includes('cannot resolve table') ||
message.includes('no such table') ||
message.includes('unknown table') ||
message.includes('not found') ||
Expand Down
132 changes: 128 additions & 4 deletions packages/core/src/migration/room_to_conversation.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { existsSync } from 'fs'
import { createHash, randomUUID } from 'crypto'
import type { Context } from 'koishi'
import type { Config } from '../config'
Expand All @@ -15,13 +16,19 @@
LegacyRoomRecord,
LegacyUserRecord
} from '../services/types'
import { defineLegacyMigrationTables } from './legacy_tables'
import {
defineLegacyMigrationTables,
isMissingTableError,
LEGACY_MIGRATION_TABLES
} from './legacy_tables'
import type { BindingProgress, MessageProgress, RoomProgress } from './types'
import {
aclKey,
createLegacyBindingKey,
createLegacyTableRetention,
createPassedValidationResult,
filterValidRooms,
getLegacySchemaSentinel,
inferLegacyGroupRouteModes,
isComplexRoom,
LEGACY_RETENTION_META_KEY,
Expand All @@ -43,7 +50,13 @@
ctx: Context,
config: Config
) {
defineLegacyMigrationTables(ctx)
const hasSentinel = existsSync(getLegacySchemaSentinel(ctx.baseDir))

if (hasSentinel && !(await hasLegacyMigrationData(ctx))) {
return ensureMigrationValidated(ctx, config)
}

defineLegacyMigrationTables(ctx, hasSentinel)

const result = await readMetaValue<
Awaited<ReturnType<typeof validateRoomMigration>>
Expand All @@ -56,6 +69,7 @@
(await readMetaValue<boolean>(ctx, 'message_migration_done')) ?? false

if (
!hasSentinel &&
schemaVersion >= BUILTIN_SCHEMA_VERSION &&
(result?.passed === true || (roomDone && messageDone))
) {
Expand Down Expand Up @@ -88,6 +102,9 @@
throw new Error('ChatLuna migration validation failed.')
}

// Mark completion before purge so a restart after writing the sentinel
// can resume from the finished state instead of re-reading legacy tables.
await writeMigrationDone(ctx)
await purgeLegacyTables(ctx)
await writeMigrationFinished(ctx)

Expand All @@ -99,67 +116,174 @@
// runRoomToConversationMigration when flags indicate migration is incomplete,
// while runRoomToConversationMigration only calls ensureMigrationValidated when
// flags indicate it IS complete — so the two paths are mutually exclusive.
export async function ensureMigrationValidated(ctx: Context, config: Config) {
const hasSentinel = existsSync(getLegacySchemaSentinel(ctx.baseDir))
const hasLegacyData = hasSentinel
? await hasLegacyMigrationData(ctx)
: false

if (hasSentinel && hasLegacyData) {
ctx.logger.warn(
'Legacy sentinel exists but legacy ChatHub data is still present; continuing migration from legacy tables.'
)
defineLegacyMigrationTables(ctx, true)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This call to defineLegacyMigrationTables is redundant. The hasLegacyMigrationData function, which is called just a few lines above at line 122, already ensures that the legacy tables are defined by calling defineLegacyMigrationTables(ctx, true) internally.

}

const result = await readMetaValue<
Awaited<ReturnType<typeof validateRoomMigration>>
>(ctx, 'validation_result')
const retention = await readMetaValue<{
state?: string
}>(ctx, LEGACY_RETENTION_META_KEY)

if (hasSentinel) {
if (hasLegacyData) {
return runRoomToConversationMigration(ctx, config)
}

if (result?.passed === true) {
await writeMetaValue(ctx, 'schema_version', BUILTIN_SCHEMA_VERSION)
await writeMigrationDone(ctx)
await writeMigrationFinished(ctx)
return result
}

const conversations = (await ctx.database.get(
'chatluna_conversation',
{},
{
limit: 1
}
)) as ConversationRecord[]
const messages = (await ctx.database.get(
'chatluna_message',
{},
{
limit: 1
}
)) as MessageRecord[]
const bindings = (await ctx.database.get(
'chatluna_binding',
{},
{
limit: 1
}
)) as BindingRecord[]
const acl = (await ctx.database.get(
'chatluna_acl',
{},
{
limit: 1
}
)) as ACLRecord[]
Comment on lines +151 to +178
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

These four database queries are executed sequentially, which can be optimized by using Promise.all to fetch the existence of data in parallel. This improves startup performance during the recovery flow.

Suggested change
const conversations = (await ctx.database.get(
'chatluna_conversation',
{},
{
limit: 1
}
)) as ConversationRecord[]
const messages = (await ctx.database.get(
'chatluna_message',
{},
{
limit: 1
}
)) as MessageRecord[]
const bindings = (await ctx.database.get(
'chatluna_binding',
{},
{
limit: 1
}
)) as BindingRecord[]
const acl = (await ctx.database.get(
'chatluna_acl',
{},
{
limit: 1
}
)) as ACLRecord[]
const [conversations, messages, bindings, acl] = (await Promise.all([
ctx.database.get('chatluna_conversation', {}, { limit: 1 }),
ctx.database.get('chatluna_message', {}, { limit: 1 }),
ctx.database.get('chatluna_binding', {}, { limit: 1 }),
ctx.database.get('chatluna_acl', {}, { limit: 1 })
])) as [ConversationRecord[], MessageRecord[], BindingRecord[], ACLRecord[]]

const hasData =
conversations.length > 0 ||
messages.length > 0 ||
bindings.length > 0 ||
acl.length > 0

ctx.logger.warn(
hasData
? 'Legacy sentinel exists and ChatLuna data is present; adopting current ChatLuna state and marking migration finished.'
: 'Legacy sentinel exists and ChatLuna data is empty; treating startup as fresh install.'
)

const validated = createPassedValidationResult()
await writeMetaValue(ctx, 'schema_version', BUILTIN_SCHEMA_VERSION)
await writeMetaValue(ctx, 'validation_result', validated)
await writeMigrationDone(ctx)
await writeMigrationFinished(ctx)
return validated
}

if (result?.passed === true) {
await writeMetaValue(ctx, 'schema_version', BUILTIN_SCHEMA_VERSION)
await writeMigrationDone(ctx)

if (retention?.state !== 'purged') {
await purgeLegacyTables(ctx)
}

await writeMigrationFinished(ctx)
return result
}

const schemaVersion =
(await readMetaValue<number>(ctx, 'schema_version')) ?? 0
const roomDone =
(await readMetaValue<boolean>(ctx, 'room_migration_done')) ?? false
const messageDone =
(await readMetaValue<boolean>(ctx, 'message_migration_done')) ?? false

if (
schemaVersion < BUILTIN_SCHEMA_VERSION ||
roomDone !== true ||
messageDone !== true
) {
// Migration is genuinely incomplete; restart it.
// runRoomToConversationMigration will NOT call back into ensureMigrationValidated
// because it only does so when all done-flags are true, which they are not here.
return runRoomToConversationMigration(ctx, config)
}

if (retention?.state !== 'purged') {
defineLegacyMigrationTables(ctx)
}

const validated = await validateRoomMigration(ctx, config)
await writeMetaValue(ctx, 'validation_result', validated)

if (!validated.passed) {
throw new Error('ChatLuna migration validation failed.')
}

await writeMigrationDone(ctx)
await purgeLegacyTables(ctx)
await writeMigrationFinished(ctx)

return validated
}

Check notice on line 245 in packages/core/src/migration/room_to_conversation.ts

View check run for this annotation

codefactor.io / CodeFactor

packages/core/src/migration/room_to_conversation.ts#L119-L245

Complex Method

async function writeMigrationFinished(ctx: Context) {
await writeMetaValue(ctx, 'migration_finished_at', new Date().toISOString())
async function writeMigrationDone(ctx: Context) {
await writeMetaValue(ctx, 'room_migration_done', true)
await writeMetaValue(ctx, 'message_migration_done', true)
}

async function writeMigrationFinished(ctx: Context) {
await writeMetaValue(
ctx,
LEGACY_RETENTION_META_KEY,
createLegacyTableRetention('purged')
)
await writeMetaValue(ctx, 'migration_finished_at', new Date().toISOString())
}

async function hasLegacyMigrationData(ctx: Context) {
defineLegacyMigrationTables(ctx, true)

for (const table of LEGACY_MIGRATION_TABLES) {
try {
if (
(
await ctx.database.get(
table as never,
{},
{
limit: 1
}
)
).length > 0
) {
return true
}
} catch (error) {
if (!isMissingTableError(error)) {
throw error
}
}
}

return false
}

// (#6) removed redundant inner `done` check — the caller already guards on room_migration_done
Expand Down
38 changes: 37 additions & 1 deletion packages/core/src/migration/validators.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@ import type {
import type { MigrationValidationResult } from './types'
export type { MigrationValidationResult } from './types'
export {
getLegacySchemaSentinel,
dropTableIfExists,
getLegacySchemaSentinelDir,
getLegacySchemaSentinel,
LEGACY_MIGRATION_TABLES,
LEGACY_RETENTION_META_KEY,
LEGACY_RUNTIME_TABLES,
Expand All @@ -30,6 +30,42 @@ export {

const VALIDATION_BATCH_SIZE = 500

export function createPassedValidationResult(): MigrationValidationResult {
return {
passed: true,
checkedAt: new Date().toISOString(),
conversation: {
legacy: 0,
migrated: 0,
matched: true
},
message: {
legacy: 0,
migrated: 0,
matched: true
},
latestMessageId: {
missingConversationIds: [],
matched: true
},
bindingKey: {
inconsistentConversationIds: [],
matched: true
},
binding: {
missingBindingKeys: [],
missingConversationIds: [],
matched: true
},
acl: {
expected: 0,
migrated: 0,
missing: [],
matched: true
}
}
}

export async function validateRoomMigration(ctx: Context, _config: Config) {
ctx.logger.info('Validating built-in ChatLuna migration.')

Expand Down
2 changes: 0 additions & 2 deletions packages/core/src/services/conversation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -336,8 +336,6 @@ export class ConversationService {
}
}

await assertManageAllowed(session, resolved.constraint)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-high high

Removing the assertManageAllowed check here introduces a permission bypass for automatic conversation creation. While the conversation_new middleware now handles this for explicit commands (with a personal route exception), the service layer's ensureActiveConversation is also called during the normal chat flow. If manageMode is set to admin for a non-personal route, regular users will now be able to trigger conversation creation simply by sending a message, which violates the intended administrative restriction.

        if (resolved.constraint.routeMode !== 'personal') {
            await assertManageAllowed(session, resolved.constraint)
        }


if (!resolved.constraint.allowNew) {
throw new Error('Conversation creation is disabled by constraint.')
}
Comment on lines 339 to 341
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Reinstate admin check before implicit conversation creation

Removing the assertManageAllowed call in ensureActiveConversation() opens an authorization bypass in the normal chat path: request_conversation always calls this method, so when no active conversation exists a non-admin user can now create one even if manageMode is admin. This contradicts the admin-only management behavior still enforced in conversation_new for non-personal routes and lets regular users bootstrap shared/custom managed conversations just by sending a message.

Useful? React with 👍 / 👎.

Expand Down
Loading
Loading