Merge checkpoint-idor-fix: v2.7.2 close 2 checkpoint IDOR holes

CHANGELOG.md

@@ -2,6 +2,10 @@
 
 All notable changes per release tag. Most recent on top, ordered by tag creation date (which matches the git history). Tag names follow `vMAJOR.MINOR.PATCH-slug` — the slug describes what shipped, so the tag name alone is enough to recall the batch.
 
+## v2.7.2-checkpoint-idor — 2026-06-01
+
+Closes two IDOR authorization holes in the `v2.7.1-write-edit-robustness` checkpoint routes, flagged by the automated push security review. The `GET /api/sessions/:id/checkpoints?chat_id=` list route scoped its `chat_id` branch by `chat_id` alone — any session's `chat_id` would read its checkpoints; it now joins through `chats` and gates on `chats.session_id` (authoritative; `checkpoints.session_id` is a nullable denormalized hint). The `restoreCheckpoint` scope guard was fail-open — `cp.session_id && cp.session_id !== sessionId` fell through whenever the checkpoint's denormalized `session_id` was null, allowing a cross-session restore (worktree reset + transcript trim) — it now resolves the owning session via the checkpoint's chat and denies on any missing-or-mismatched row. A DB-integration regression covers the exact null-`session_id` cross-session case. Real-world blast radius is small (BooCoder is single-user behind Authelia on loopback), but both are genuine authorization bugs. Coder suite 234 passing (7/7 checkpoint tests incl. the regression against live postgres+git), typecheck clean. Hotfix on `v2.7.1-write-edit-robustness`.
+
 ## v2.7.1-write-edit-robustness — 2026-06-01
 
 Two BooCoder hardening features for local quantized models, algorithm-reimplemented (not vendored) from the cline findings in `boocode_code_review_v2.md` §1 #3/#4. **Fuzzy patch applier:** `edit_file`'s apply path was exact-`.includes`-or-throw + first-occurrence `.replace` (`pending_changes.ts`), so a qwen3.6 whitespace/indentation/unicode drift in `old_string` lost the edit; a new pure `fuzzy-match.ts` (`locateMatch`) now runs an exact → per-line-trim → unicode-canon (curly quotes/dashes/nbsp) → Levenshtein-≥0.66 ladder and returns the real file span, refusing multi-exact matches as ambiguous rather than silently editing the first. `applyOne`/`rewindOne` both use it. **Worktree checkpoints + conversation-trim:** `rewind` only reversed BooCode's own `pending_changes`, blind to what external agents (opencode/goose/qwen/claude) write directly into the session worktree — so a new `checkpoints` table + `checkpoints.ts` shadow-commit (tracked **and** untracked, captured via a temp-index `read-tree`/`add`/`write-tree`/`commit-tree` into a GC-safe `refs/boocode/checkpoints/<id>`) snapshots the worktree before each external-agent turn (hooked into all three dispatcher paths), anchored to the turn's assistant message. A new `POST /api/sessions/:id/checkpoints/:cid/restore` resets the worktree (`reset --hard` + `clean -fd`), trims the transcript past that message, and resets the `(chat,agent)` backend session so files, transcript, and agent context land consistent at the restore point; a per-message "Restore to here" affordance in `CoderMessageList` drives it. Built by three parallel agents over disjoint files; DB-integration testing caught a microsecond-`created_at` self-deletion bug in the later-checkpoint cleanup. Full coder suite 234 passing (incl. 17 fuzzy-match + 6 checkpoint tests), server+coder build + web tsc clean. Builds on `v2.7.0-mit`; openspec `write-edit-robustness`. Live host smoke (dispatcher hook + restore UI end-to-end) still to run.

apps/coder/src/routes/checkpoints.ts

@@ -26,18 +26,24 @@ export function registerCheckpointRoutes(app: FastifyInstance, sql: Sql): void {
         return { error: 'session not found' };
       }
 
+      // Scope authoritatively through chats.session_id (always set) — NOT the
+      // denormalized checkpoints.session_id (nullable). The chat_id branch must
+      // still be session-gated or it's an IDOR (any session's chat_id reads its
+      // checkpoints).
       const rows = chatId
         ? await sql<{ id: string; chat_id: string; message_id: string | null; label: string | null; created_at: Date }[]>`
-            SELECT id, chat_id, message_id, label, created_at
-            FROM checkpoints
-            WHERE chat_id = ${chatId}
-            ORDER BY created_at
+            SELECT cp.id, cp.chat_id, cp.message_id, cp.label, cp.created_at
+            FROM checkpoints cp
+            JOIN chats c ON c.id = cp.chat_id
+            WHERE cp.chat_id = ${chatId} AND c.session_id = ${sessionId}
+            ORDER BY cp.created_at
           `
         : await sql<{ id: string; chat_id: string; message_id: string | null; label: string | null; created_at: Date }[]>`
-            SELECT id, chat_id, message_id, label, created_at
-            FROM checkpoints
-            WHERE session_id = ${sessionId}
-            ORDER BY created_at
+            SELECT cp.id, cp.chat_id, cp.message_id, cp.label, cp.created_at
+            FROM checkpoints cp
+            JOIN chats c ON c.id = cp.chat_id
+            WHERE c.session_id = ${sessionId}
+            ORDER BY cp.created_at
           `;
       return rows;
     },

apps/coder/src/services/__tests__/checkpoints.test.ts

@@ -233,4 +233,20 @@ describe.runIf(!!process.env.DATABASE_URL)('checkpoint create + restore (DB + gi
     ).rejects.toBeInstanceOf(CheckpointNotFoundError);
     await sql`DELETE FROM checkpoints WHERE id = ${cp!.id}`;
   });
+
+  it('restoreCheckpoint denies a NULL-session_id checkpoint from another session (no fail-open IDOR)', async () => {
+    // Regression for the fail-open authorization bug: a checkpoint row whose
+    // denormalized session_id is NULL must STILL be scoped via its chat's owning
+    // session (chats.session_id), not skipped. The old guard `cp.session_id &&
+    // cp.session_id !== sessionId` fell through on NULL → cross-session restore.
+    const [row] = await sql<{ id: string }[]>`
+      INSERT INTO checkpoints (chat_id, session_id, message_id, commit_sha)
+      VALUES (${chatId}, NULL, NULL, 'deadbeef')
+      RETURNING id
+    `;
+    await expect(
+      restoreCheckpoint(sql, row!.id, { sessionId: '22222222-2222-2222-2222-222222222222' }),
+    ).rejects.toBeInstanceOf(CheckpointNotFoundError);
+    await sql`DELETE FROM checkpoints WHERE id = ${row!.id}`;
+  });
 });

apps/coder/src/services/checkpoints.ts

@@ -159,9 +159,18 @@ export async function restoreCheckpoint(
   if (!cp) {
     throw new CheckpointNotFoundError('checkpoint not found');
   }
-  if (opts?.sessionId && cp.session_id && cp.session_id !== opts.sessionId) {
+  // Authorization scope (fail-safe): the checkpoint's chat must belong to the
+  // requested session. cp.session_id is a denormalized hint that may be null, so
+  // gating on it directly fails open — resolve the owning session via chats
+  // (authoritative; chat_id is NOT NULL) and deny on any mismatch or missing row.
+  if (opts?.sessionId) {
+    const [owner] = await sql<{ session_id: string | null }[]>`
+      SELECT session_id FROM chats WHERE id = ${cp.chat_id}
+    `;
+    if (!owner || owner.session_id !== opts.sessionId) {
       throw new CheckpointNotFoundError('checkpoint not in session');
     }
+  }
 
   // 2. Resolve the worktree path (by worktree_id, else the session's active one).
   let worktreePath: string | null = null;