batch3 T2 review fix: move PATCH count+bounds check inside sql.begin
A concurrent DELETE between the count read and the transaction could allow an invalid position value to slip in. Mirror the POST fix by validating count + bounds inside the transaction. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -147,20 +147,18 @@ export function registerPaneRoutes(app: FastifyInstance, sql: Sql): void {
|
|||||||
const sid = pane.session_id;
|
const sid = pane.session_id;
|
||||||
const oldPos = pane.position;
|
const oldPos = pane.position;
|
||||||
|
|
||||||
// Validate position if provided
|
|
||||||
if (position !== undefined) {
|
|
||||||
const countRows = await sql<{ n: number }[]>`
|
|
||||||
SELECT COUNT(*)::int AS n FROM session_panes WHERE session_id = ${sid}
|
|
||||||
`;
|
|
||||||
const count = countRows[0]?.n ?? 0;
|
|
||||||
if (position < 0 || position >= count) {
|
|
||||||
reply.code(400);
|
|
||||||
return { error: `position must be between 0 and ${count - 1}` };
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
// Apply position and/or state changes atomically
|
// Apply position and/or state changes atomically
|
||||||
|
let patchError: string | null = null;
|
||||||
await sql.begin(async (tx) => {
|
await sql.begin(async (tx) => {
|
||||||
|
if (position !== undefined) {
|
||||||
|
const countRows = await tx<{ n: number }[]>`
|
||||||
|
SELECT COUNT(*)::int AS n FROM session_panes WHERE session_id = ${sid}
|
||||||
|
`;
|
||||||
|
const count = countRows[0]?.n ?? 0;
|
||||||
|
if (position < 0 || position >= count) {
|
||||||
|
throw `position must be between 0 and ${count - 1}`;
|
||||||
|
}
|
||||||
|
}
|
||||||
if (position !== undefined && position !== oldPos) {
|
if (position !== undefined && position !== oldPos) {
|
||||||
await movePane(tx, req.params.id, sid, oldPos, position);
|
await movePane(tx, req.params.id, sid, oldPos, position);
|
||||||
}
|
}
|
||||||
@@ -170,8 +168,19 @@ export function registerPaneRoutes(app: FastifyInstance, sql: Sql): void {
|
|||||||
WHERE id = ${req.params.id}
|
WHERE id = ${req.params.id}
|
||||||
`;
|
`;
|
||||||
}
|
}
|
||||||
|
}).catch((err: unknown) => {
|
||||||
|
if (typeof err === 'string') {
|
||||||
|
patchError = err;
|
||||||
|
} else {
|
||||||
|
throw err;
|
||||||
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
|
if (patchError !== null) {
|
||||||
|
reply.code(400);
|
||||||
|
return { error: patchError };
|
||||||
|
}
|
||||||
|
|
||||||
const [updated] = await sql<Pane[]>`
|
const [updated] = await sql<Pane[]>`
|
||||||
SELECT id, session_id, position, kind, state, created_at
|
SELECT id, session_id, position, kind, state, created_at
|
||||||
FROM session_panes WHERE id = ${req.params.id}
|
FROM session_panes WHERE id = ${req.params.id}
|
||||||
|
|||||||
Reference in New Issue
Block a user