mirror of
https://github.com/siteboon/claudecodeui.git
synced 2026-06-03 02:55:39 +08:00
fix(migrations,projects,clone): normalize legacy schema before writes and harden conflict detection
Why
- Legacy installs can have a sessions table shape that predates provider/custom_name columns. Running migrateLegacySessionNames first caused its INSERT OR REPLACE INTO sessions (...) to target columns that may not exist and fail during startup migration.
- Some upgraded databases had projects.project_id as plain TEXT instead of a real PRIMARY KEY. That breaks assumptions used by id-based lookups and can allow invalid/duplicate identity semantics over time.
- projectsDb.createProjectPath inferred outcomes from
ow.isArchived, but the upsert path always returns the post-update row with isArchived=0, so archived-reactivation and fresh-create could be misclassified.
- git clone accepted user-controlled URLs directly in argv position, so inputs beginning with - could be interpreted as options instead of a repository argument.
What
- Added
ebuildProjectsTableWithPrimaryKeySchema in migrations: detect table shape via getTableInfo('projects'), verify project_id has pk=1, and rebuild when missing.
- Rebuild flow now creates a canonical projects__new table (project_id TEXT PRIMARY KEY), copies rows with transformation, backfills empty ids via SQLITE_UUID_SQL, deduplicates conflicting ids/paths, then swaps tables inside a transaction.
- Replaced the prior ddColumnToTableIfNotExists(...) + UPDATE project_id sequence with PK-aware detection/rebuild logic so legacy DBs converge to the required schema.
- Reordered migration sequence to run
ebuildSessionsTableWithProjectSchema before migrateLegacySessionNames, ensuring sessions is normalized before legacy session_names merge writes execute.
- Updated projectsDb.createProjectPath to generate an ttemptedId before insert, pass it into the prepared statement, and classify outcomes by comparing returned
ow.project_id to ttemptedId (created vs
eactivated_archived), with no-row remaining ctive_conflict.
- Hardened clone execution by inserting -- before clone URL in git argv and rejecting normalized GitHub URLs that start with - in startCloneProject.
Tests
- Added integration coverage for projectsDb.createProjectPath branches: fresh insert, archived reactivation, and active conflict.
- Added clone service test for option-prefixed githubUrl rejection (INVALID_GITHUB_URL).
This commit is contained in:
@@ -102,6 +102,133 @@ const migrateLegacyWorkspaceTableIntoProjects = (db: Database): void => {
|
|||||||
`);
|
`);
|
||||||
};
|
};
|
||||||
|
|
||||||
|
const rebuildProjectsTableWithPrimaryKeySchema = (db: Database): void => {
|
||||||
|
const hasProjectsTable = tableExists(db, 'projects');
|
||||||
|
if (!hasProjectsTable) {
|
||||||
|
db.exec(PROJECTS_TABLE_SCHEMA_SQL);
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
const projectsTableInfo = getTableInfo(db, 'projects');
|
||||||
|
const columnNames = projectsTableInfo.map((column) => column.name);
|
||||||
|
const hasProjectIdPrimaryKey = projectsTableInfo.some(
|
||||||
|
(column) => column.name === 'project_id' && column.pk === 1,
|
||||||
|
);
|
||||||
|
|
||||||
|
if (hasProjectIdPrimaryKey) {
|
||||||
|
addColumnToTableIfNotExists(db, 'projects', columnNames, 'custom_project_name', 'TEXT DEFAULT NULL');
|
||||||
|
addColumnToTableIfNotExists(db, 'projects', columnNames, 'isStarred', 'BOOLEAN DEFAULT 0');
|
||||||
|
addColumnToTableIfNotExists(db, 'projects', columnNames, 'isArchived', 'BOOLEAN DEFAULT 0');
|
||||||
|
db.exec(`
|
||||||
|
UPDATE projects
|
||||||
|
SET project_id = ${SQLITE_UUID_SQL}
|
||||||
|
WHERE project_id IS NULL OR trim(project_id) = ''
|
||||||
|
`);
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
console.log('Running migration: Rebuilding projects table to enforce project_id primary key');
|
||||||
|
|
||||||
|
const projectPathExpression = columnNames.includes('project_path')
|
||||||
|
? 'project_path'
|
||||||
|
: columnNames.includes('workspace_path')
|
||||||
|
? 'workspace_path'
|
||||||
|
: 'NULL';
|
||||||
|
|
||||||
|
const customProjectNameExpression = columnNames.includes('custom_project_name')
|
||||||
|
? 'custom_project_name'
|
||||||
|
: columnNames.includes('custom_workspace_name')
|
||||||
|
? 'custom_workspace_name'
|
||||||
|
: 'NULL';
|
||||||
|
|
||||||
|
const isStarredExpression = columnNames.includes('isStarred') ? 'COALESCE(isStarred, 0)' : '0';
|
||||||
|
|
||||||
|
const isArchivedExpression = columnNames.includes('isArchived') ? 'COALESCE(isArchived, 0)' : '0';
|
||||||
|
|
||||||
|
const projectIdExpression = columnNames.includes('project_id')
|
||||||
|
? `CASE
|
||||||
|
WHEN project_id IS NULL OR trim(project_id) = ''
|
||||||
|
THEN ${SQLITE_UUID_SQL}
|
||||||
|
ELSE project_id
|
||||||
|
END`
|
||||||
|
: SQLITE_UUID_SQL;
|
||||||
|
|
||||||
|
db.exec('PRAGMA foreign_keys = OFF');
|
||||||
|
try {
|
||||||
|
db.exec('BEGIN TRANSACTION');
|
||||||
|
db.exec('DROP TABLE IF EXISTS projects__new');
|
||||||
|
db.exec(`
|
||||||
|
CREATE TABLE projects__new (
|
||||||
|
project_id TEXT PRIMARY KEY NOT NULL,
|
||||||
|
project_path TEXT NOT NULL UNIQUE,
|
||||||
|
custom_project_name TEXT DEFAULT NULL,
|
||||||
|
isStarred BOOLEAN DEFAULT 0,
|
||||||
|
isArchived BOOLEAN DEFAULT 0
|
||||||
|
)
|
||||||
|
`);
|
||||||
|
db.exec(`
|
||||||
|
WITH source_rows AS (
|
||||||
|
SELECT
|
||||||
|
${projectPathExpression} AS project_path,
|
||||||
|
${customProjectNameExpression} AS custom_project_name,
|
||||||
|
${isStarredExpression} AS isStarred,
|
||||||
|
${isArchivedExpression} AS isArchived,
|
||||||
|
${projectIdExpression} AS candidate_project_id,
|
||||||
|
rowid AS source_rowid
|
||||||
|
FROM projects
|
||||||
|
WHERE ${projectPathExpression} IS NOT NULL AND trim(${projectPathExpression}) <> ''
|
||||||
|
),
|
||||||
|
deduped_paths AS (
|
||||||
|
SELECT
|
||||||
|
project_path,
|
||||||
|
custom_project_name,
|
||||||
|
isStarred,
|
||||||
|
isArchived,
|
||||||
|
candidate_project_id,
|
||||||
|
source_rowid,
|
||||||
|
ROW_NUMBER() OVER (PARTITION BY project_path ORDER BY source_rowid) AS project_path_rank
|
||||||
|
FROM source_rows
|
||||||
|
),
|
||||||
|
prepared_rows AS (
|
||||||
|
SELECT
|
||||||
|
CASE
|
||||||
|
WHEN ROW_NUMBER() OVER (PARTITION BY candidate_project_id ORDER BY source_rowid) = 1
|
||||||
|
THEN candidate_project_id
|
||||||
|
ELSE ${SQLITE_UUID_SQL}
|
||||||
|
END AS project_id,
|
||||||
|
project_path,
|
||||||
|
custom_project_name,
|
||||||
|
isStarred,
|
||||||
|
isArchived
|
||||||
|
FROM deduped_paths
|
||||||
|
WHERE project_path_rank = 1
|
||||||
|
)
|
||||||
|
INSERT INTO projects__new (
|
||||||
|
project_id,
|
||||||
|
project_path,
|
||||||
|
custom_project_name,
|
||||||
|
isStarred,
|
||||||
|
isArchived
|
||||||
|
)
|
||||||
|
SELECT
|
||||||
|
project_id,
|
||||||
|
project_path,
|
||||||
|
custom_project_name,
|
||||||
|
isStarred,
|
||||||
|
isArchived
|
||||||
|
FROM prepared_rows
|
||||||
|
`);
|
||||||
|
db.exec('DROP TABLE projects');
|
||||||
|
db.exec('ALTER TABLE projects__new RENAME TO projects');
|
||||||
|
db.exec('COMMIT');
|
||||||
|
} catch (migrationError) {
|
||||||
|
db.exec('ROLLBACK');
|
||||||
|
throw migrationError;
|
||||||
|
} finally {
|
||||||
|
db.exec('PRAGMA foreign_keys = ON');
|
||||||
|
}
|
||||||
|
};
|
||||||
|
|
||||||
const rebuildSessionsTableWithProjectSchema = (db: Database): void => {
|
const rebuildSessionsTableWithProjectSchema = (db: Database): void => {
|
||||||
const hasSessions = tableExists(db, 'sessions');
|
const hasSessions = tableExists(db, 'sessions');
|
||||||
if (!hasSessions) {
|
if (!hasSessions) {
|
||||||
@@ -251,21 +378,11 @@ export const runMigrations = (db: Database) => {
|
|||||||
db.exec('CREATE INDEX IF NOT EXISTS idx_push_subscriptions_user_id ON push_subscriptions(user_id)');
|
db.exec('CREATE INDEX IF NOT EXISTS idx_push_subscriptions_user_id ON push_subscriptions(user_id)');
|
||||||
|
|
||||||
db.exec(PROJECTS_TABLE_SCHEMA_SQL);
|
db.exec(PROJECTS_TABLE_SCHEMA_SQL);
|
||||||
const projectsTableInfo = getTableInfo(db, 'projects');
|
rebuildProjectsTableWithPrimaryKeySchema(db);
|
||||||
const projectColumnNames = projectsTableInfo.map((column) => column.name);
|
|
||||||
addColumnToTableIfNotExists(db, 'projects', projectColumnNames, 'custom_project_name', 'TEXT DEFAULT NULL');
|
|
||||||
addColumnToTableIfNotExists(db, 'projects', projectColumnNames, 'project_id', 'TEXT');
|
|
||||||
addColumnToTableIfNotExists(db, 'projects', projectColumnNames, 'isStarred', 'BOOLEAN DEFAULT 0');
|
|
||||||
addColumnToTableIfNotExists(db, 'projects', projectColumnNames, 'isArchived', 'BOOLEAN DEFAULT 0');
|
|
||||||
db.exec(`
|
|
||||||
UPDATE projects
|
|
||||||
SET project_id = ${SQLITE_UUID_SQL}
|
|
||||||
WHERE project_id IS NULL OR trim(project_id) = ''
|
|
||||||
`);
|
|
||||||
|
|
||||||
migrateLegacyWorkspaceTableIntoProjects(db);
|
migrateLegacyWorkspaceTableIntoProjects(db);
|
||||||
migrateLegacySessionNames(db);
|
|
||||||
rebuildSessionsTableWithProjectSchema(db);
|
rebuildSessionsTableWithProjectSchema(db);
|
||||||
|
migrateLegacySessionNames(db);
|
||||||
ensureProjectsForSessionPaths(db);
|
ensureProjectsForSessionPaths(db);
|
||||||
|
|
||||||
db.exec('CREATE INDEX IF NOT EXISTS idx_session_ids_lookup ON sessions(session_id)');
|
db.exec('CREATE INDEX IF NOT EXISTS idx_session_ids_lookup ON sessions(session_id)');
|
||||||
|
|||||||
@@ -0,0 +1,72 @@
|
|||||||
|
import assert from 'node:assert/strict';
|
||||||
|
import { mkdtemp, rm } from 'node:fs/promises';
|
||||||
|
import { tmpdir } from 'node:os';
|
||||||
|
import path from 'node:path';
|
||||||
|
import test from 'node:test';
|
||||||
|
|
||||||
|
import { closeConnection } from '@/modules/database/connection.js';
|
||||||
|
import { initializeDatabase } from '@/modules/database/init-db.js';
|
||||||
|
import { projectsDb } from '@/modules/database/repositories/projects.db.js';
|
||||||
|
|
||||||
|
async function withIsolatedDatabase(runTest: () => void | Promise<void>): Promise<void> {
|
||||||
|
const previousDatabasePath = process.env.DATABASE_PATH;
|
||||||
|
const tempDirectory = await mkdtemp(path.join(tmpdir(), 'projects-db-'));
|
||||||
|
const databasePath = path.join(tempDirectory, 'auth.db');
|
||||||
|
|
||||||
|
closeConnection();
|
||||||
|
process.env.DATABASE_PATH = databasePath;
|
||||||
|
await initializeDatabase();
|
||||||
|
|
||||||
|
try {
|
||||||
|
await runTest();
|
||||||
|
} finally {
|
||||||
|
closeConnection();
|
||||||
|
if (previousDatabasePath === undefined) {
|
||||||
|
delete process.env.DATABASE_PATH;
|
||||||
|
} else {
|
||||||
|
process.env.DATABASE_PATH = previousDatabasePath;
|
||||||
|
}
|
||||||
|
await rm(tempDirectory, { recursive: true, force: true });
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
test('projectsDb.createProjectPath returns created for fresh paths', async () => {
|
||||||
|
await withIsolatedDatabase(() => {
|
||||||
|
const created = projectsDb.createProjectPath('/workspace/new-project');
|
||||||
|
|
||||||
|
assert.equal(created.outcome, 'created');
|
||||||
|
assert.ok(created.project);
|
||||||
|
assert.equal(created.project?.project_path, '/workspace/new-project');
|
||||||
|
assert.equal(created.project?.isArchived, 0);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
test('projectsDb.createProjectPath returns reactivated_archived for archived duplicates', async () => {
|
||||||
|
await withIsolatedDatabase(() => {
|
||||||
|
const initial = projectsDb.createProjectPath('/workspace/archived-project', 'Archived Project');
|
||||||
|
assert.equal(initial.outcome, 'created');
|
||||||
|
assert.ok(initial.project);
|
||||||
|
|
||||||
|
projectsDb.updateProjectIsArchived('/workspace/archived-project', true);
|
||||||
|
|
||||||
|
const reused = projectsDb.createProjectPath('/workspace/archived-project', 'Renamed Project');
|
||||||
|
assert.equal(reused.outcome, 'reactivated_archived');
|
||||||
|
assert.ok(reused.project);
|
||||||
|
assert.equal(reused.project?.project_id, initial.project?.project_id);
|
||||||
|
assert.equal(reused.project?.isArchived, 0);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
test('projectsDb.createProjectPath returns active_conflict for active duplicates', async () => {
|
||||||
|
await withIsolatedDatabase(() => {
|
||||||
|
const initial = projectsDb.createProjectPath('/workspace/active-project');
|
||||||
|
assert.equal(initial.outcome, 'created');
|
||||||
|
assert.ok(initial.project);
|
||||||
|
|
||||||
|
const conflict = projectsDb.createProjectPath('/workspace/active-project');
|
||||||
|
assert.equal(conflict.outcome, 'active_conflict');
|
||||||
|
assert.ok(conflict.project);
|
||||||
|
assert.equal(conflict.project?.project_id, initial.project?.project_id);
|
||||||
|
assert.equal(conflict.project?.isArchived, 0);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -18,6 +18,7 @@ export const projectsDb = {
|
|||||||
createProjectPath(projectPath: string, customProjectName: string | null = null): CreateProjectPathResult {
|
createProjectPath(projectPath: string, customProjectName: string | null = null): CreateProjectPathResult {
|
||||||
const db = getConnection();
|
const db = getConnection();
|
||||||
const normalizedProjectName = normalizeProjectDisplayName(projectPath, customProjectName);
|
const normalizedProjectName = normalizeProjectDisplayName(projectPath, customProjectName);
|
||||||
|
const attemptedId = randomUUID();
|
||||||
const row = db.prepare(`
|
const row = db.prepare(`
|
||||||
INSERT INTO projects (project_id, project_path, custom_project_name, isArchived)
|
INSERT INTO projects (project_id, project_path, custom_project_name, isArchived)
|
||||||
VALUES (?, ?, ?, 0)
|
VALUES (?, ?, ?, 0)
|
||||||
@@ -25,11 +26,11 @@ export const projectsDb = {
|
|||||||
isArchived = 0
|
isArchived = 0
|
||||||
WHERE projects.isArchived = 1
|
WHERE projects.isArchived = 1
|
||||||
RETURNING project_id, project_path, custom_project_name, isStarred, isArchived
|
RETURNING project_id, project_path, custom_project_name, isStarred, isArchived
|
||||||
`).get(randomUUID(), projectPath, normalizedProjectName) as ProjectRepositoryRow | undefined;
|
`).get(attemptedId, projectPath, normalizedProjectName) as ProjectRepositoryRow | undefined;
|
||||||
|
|
||||||
if (row) {
|
if (row) {
|
||||||
return {
|
return {
|
||||||
outcome: row.isArchived === 1 ? 'reactivated_archived' : 'created',
|
outcome: row.project_id === attemptedId ? 'created' : 'reactivated_archived',
|
||||||
project: row,
|
project: row,
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -125,7 +125,7 @@ const defaultDependencies: CloneProjectDependencies = {
|
|||||||
return tokenRow;
|
return tokenRow;
|
||||||
},
|
},
|
||||||
spawnGitClone: (cloneUrl: string, clonePath: string): GitCloneProcess =>
|
spawnGitClone: (cloneUrl: string, clonePath: string): GitCloneProcess =>
|
||||||
spawn('git', ['clone', '--progress', cloneUrl, clonePath], {
|
spawn('git', ['clone', '--progress', '--', cloneUrl, clonePath], {
|
||||||
stdio: ['ignore', 'pipe', 'pipe'],
|
stdio: ['ignore', 'pipe', 'pipe'],
|
||||||
env: {
|
env: {
|
||||||
...process.env,
|
...process.env,
|
||||||
@@ -167,6 +167,13 @@ export async function startCloneProject(
|
|||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (normalizedGithubUrl.startsWith('-')) {
|
||||||
|
throw new AppError('Invalid githubUrl', {
|
||||||
|
code: 'INVALID_GITHUB_URL',
|
||||||
|
statusCode: 400,
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
const pathValidation = await dependencies.validatePath(normalizedWorkspacePath);
|
const pathValidation = await dependencies.validatePath(normalizedWorkspacePath);
|
||||||
if (!pathValidation.valid || !pathValidation.resolvedPath) {
|
if (!pathValidation.valid || !pathValidation.resolvedPath) {
|
||||||
throw new AppError(pathValidation.error || 'Invalid workspace path', {
|
throw new AppError(pathValidation.error || 'Invalid workspace path', {
|
||||||
|
|||||||
@@ -87,6 +87,29 @@ test('startCloneProject rejects when github URL is missing', async () => {
|
|||||||
);
|
);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test('startCloneProject rejects github URL values that begin with option prefixes', async () => {
|
||||||
|
await assert.rejects(
|
||||||
|
async () =>
|
||||||
|
startCloneProject(
|
||||||
|
{
|
||||||
|
workspacePath: '/workspace/root',
|
||||||
|
githubUrl: '--upload-pack=malicious',
|
||||||
|
userId: 1,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
onProgress: () => undefined,
|
||||||
|
onComplete: () => undefined,
|
||||||
|
},
|
||||||
|
buildDependencies(),
|
||||||
|
),
|
||||||
|
(error: unknown) => {
|
||||||
|
assert.ok(error instanceof AppError);
|
||||||
|
assert.equal(error.code, 'INVALID_GITHUB_URL');
|
||||||
|
return true;
|
||||||
|
},
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
test('startCloneProject rejects when selected github token does not exist', async () => {
|
test('startCloneProject rejects when selected github token does not exist', async () => {
|
||||||
await assert.rejects(
|
await assert.rejects(
|
||||||
async () =>
|
async () =>
|
||||||
|
|||||||
Reference in New Issue
Block a user