singularity/singularity-forge
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 1

fix(gsd): address Codex adversarial review findings

Browse source 
1. Replace ensureDbOpen() with isDbAvailable() in gsd_milestone_status
   so the read-only tool cannot create/migrate the DB as a side effect
2. Wrap all reads in a BEGIN/COMMIT transaction for snapshot consistency
   under concurrent WAL writes
3. Broaden negative regex in guardrail tests to catch sqlite3 with
   flags, relative paths, absolute paths, and quoted paths
This commit is contained in:
Jeremy 2026-04-05 09:56:19 -05:00
parent 4d9eb9ead0
commit 7d74081434
2 changed files with 60 additions and 42 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

93
src/resources/extensions/gsd/bootstrap/query-tools.ts
View file

@ -3,7 +3,6 @@
import { Type } from "@sinclair/typebox";
import type { ExtensionAPI } from "@gsd/pi-coding-agent";
import { ensureDbOpen } from "./dynamic-tools.js";
import { logWarning } from "../workflow-logger.js";
export function registerQueryTools(pi: ExtensionAPI): void {
@ -26,53 +25,69 @@ export function registerQueryTools(pi: ExtensionAPI): void {
}),
async execute(_toolCallId, params, _signal, _onUpdate, _ctx) {
try {
const dbAvailable = await ensureDbOpen();
if (!dbAvailable) {
// Strictly read-only: only use an already-open DB connection.
// Do NOT call ensureDbOpen() — it can create/migrate the DB as a side effect.
const {
isDbAvailable,
getMilestone,
getSliceStatusSummary,
getSliceTaskCounts,
_getAdapter,
} = await import("../gsd-db.js");
if (!isDbAvailable()) {
return {
content: [{ type: "text" as const, text: "Error: GSD database is not available." }],
details: { operation: "milestone_status", error: "db_unavailable" } as any,
};
}
const {
getMilestone,
getSliceStatusSummary,
getSliceTaskCounts,
} = await import("../gsd-db.js");
// Wrap all reads in a single transaction for snapshot consistency.
// SQLite WAL mode guarantees reads within a transaction see a single
// consistent snapshot, preventing torn reads from concurrent writes.
const adapter = _getAdapter()!;
adapter.exec("BEGIN"); // eslint-disable-line -- SQLite exec, not child_process
try {
const milestone = getMilestone(params.milestoneId);
if (!milestone) {
adapter.exec("COMMIT"); // eslint-disable-line
return {
content: [{ type: "text" as const, text: `Milestone ${params.milestoneId} not found in database.` }],
details: { operation: "milestone_status", milestoneId: params.milestoneId, found: false } as any,
};
}
const milestone = getMilestone(params.milestoneId);
if (!milestone) {
return {
content: [{ type: "text" as const, text: `Milestone ${params.milestoneId} not found in database.` }],
details: { operation: "milestone_status", milestoneId: params.milestoneId, found: false } as any,
const sliceStatuses = getSliceStatusSummary(params.milestoneId);
const slices = sliceStatuses.map((s) => {
const counts = getSliceTaskCounts(params.milestoneId, s.id);
return {
id: s.id,
status: s.status,
taskCounts: counts,
};
});
adapter.exec("COMMIT"); // eslint-disable-line
const result = {
milestoneId: milestone.id,
title: milestone.title,
status: milestone.status,
createdAt: milestone.created_at,
completedAt: milestone.completed_at,
sliceCount: slices.length,
slices,
};
return {
content: [{ type: "text" as const, text: JSON.stringify(result, null, 2) }],
details: { operation: "milestone_status", milestoneId: milestone.id, sliceCount: slices.length } as any,
};
} catch (txErr) {
try { adapter.exec("ROLLBACK"); } catch { /* swallow */ } // eslint-disable-line
throw txErr;
}
const sliceStatuses = getSliceStatusSummary(params.milestoneId);
const slices = sliceStatuses.map((s) => {
const counts = getSliceTaskCounts(params.milestoneId, s.id);
return {
id: s.id,
status: s.status,
taskCounts: counts,
};
});
const result = {
milestoneId: milestone.id,
title: milestone.title,
status: milestone.status,
createdAt: milestone.created_at,
completedAt: milestone.completed_at,
sliceCount: slices.length,
slices,
};
return {
content: [{ type: "text" as const, text: JSON.stringify(result, null, 2) }],
details: { operation: "milestone_status", milestoneId: milestone.id, sliceCount: slices.length } as any,
};
} catch (err) {
const msg = err instanceof Error ? err.message : String(err);
logWarning("tool", `gsd_milestone_status tool failed: ${msg}`);

9
src/resources/extensions/gsd/tests/db-access-guardrails.test.ts
View file

@ -80,16 +80,19 @@ test("no prompt file contains an unguarded sqlite3 command invocation", () => {
const line = lines[i];
const trimmed = line.trim();
// Match lines containing sqlite3 or node -e require('better-sqlite3') targeting gsd.db.
// Match lines containing sqlite3 targeting gsd.db in any common form:
// sqlite3 .gsd/gsd.db, sqlite3 ./.gsd/gsd.db, sqlite3 "/path/.gsd/gsd.db",
// sqlite3 -header .gsd/gsd.db, etc.
// Guardrail text that says "Never run" or "Do NOT query" is fine — only flag
// lines where these appear without a surrounding prohibition keyword.
if (/sqlite3\s+\.?\.?gsd\/gsd\.db/.test(trimmed)) {
if (/sqlite3\b.*gsd\.db/.test(trimmed)) {
const context = lines.slice(Math.max(0, i - 3), i + 1).join(" ");
if (!/Never|Do NOT|do not|don't|prohibited|forbidden|never run/i.test(context)) {
violations.push(`${file}:${i + 1} — unguarded sqlite3 command: ${trimmed}`);
}
}
if (/node\s+-e\s+.*require\(.*better-sqlite3/.test(trimmed)) {
// Match node -e with better-sqlite3 require in any quoting style
if (/node\s+-e\s+.*(?:require|import).*better-sqlite3/.test(trimmed)) {
const context = lines.slice(Math.max(0, i - 3), i + 1).join(" ");
if (!/Never|Do NOT|do not|don't|prohibited|forbidden|never run/i.test(context)) {
violations.push(`${file}:${i + 1} — unguarded node -e require command: ${trimmed}`);