indifferentketchup/boocode
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(security): scope checkpoint routes to session — close 2 IDORs (v2.7.2)

Browse Source 
Flagged by the automated push security review on v2.7.1.

- GET /checkpoints?chat_id= : the chat_id branch filtered by chat_id alone
  (any session's chat_id read its checkpoints). Now joins chats and gates on
  chats.session_id.
- restoreCheckpoint scope guard was fail-open: `cp.session_id && cp.session_id
  !== sessionId` fell through on a null denormalized session_id, allowing a
  cross-session restore (worktree reset + transcript trim). Now resolves the
  owning session via the checkpoint's chat and denies on missing/mismatch.
- Adds a DB-integration regression for the null-session_id cross-session case.

Both scope authoritatively through chats.session_id (checkpoints.session_id is
a nullable hint). Coder suite 234 passing; 7/7 checkpoint tests (incl. the
regression) against live postgres+git; typecheck clean. Hotfix on v2.7.1.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
indifferentketchup
2026-06-01 12:15:54 +00:00
parent a41a02a62b
commit 9c7d80e2d8
4 changed files with 45 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
CHANGELOG.md
View File

@@ -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.

22
apps/coder/src/routes/checkpoints.ts
View File

@@ -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;
},

16
apps/coder/src/services/__tests__/checkpoints.test.ts
View File

@@ -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}`;
});
});

13
apps/coder/src/services/checkpoints.ts
View File

@@ -159,8 +159,17 @@ export async function restoreCheckpoint(
if (!cp) {
throw new CheckpointNotFoundError('checkpoint not found');
}
if (opts?.sessionId && cp.session_id && cp.session_id !== opts.sessionId) {
throw new CheckpointNotFoundError('checkpoint not in session');
// 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).