Skip to content

Commit 25a1080

Browse files
ualtinokualtinok
committed
fix(memory): rebuild authority guard triggers to durable state-table form (#253)
Durable triggers baked a connection-local UDF (mc_privileged_writer) when the migrating runtime supported scalar UDFs; every non-registering connection then failed guarded writes with 'no such function'. Delete the UDF path entirely and make the guard predicate always the context_privilege_state table check; migration v71 rebuilds existing triggers. Fixes #253.
2 parents cb05b15 + 8b6bc03 commit 25a1080

5 files changed

Lines changed: 278 additions & 53 deletions

File tree

‎packages/plugin/src/features/magic-context/memory/storage-memory.ts‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,4 @@
1-
import {
2-
type Database,
3-
type Statement as PreparedStatement,
4-
registerPrivilegedWriter,
5-
} from "../../../shared/sqlite";
1+
import type { Database, Statement as PreparedStatement } from "../../../shared/sqlite";
62
import { hasMuralCueColumns } from "../mural/storage-mural-cues";
73
import { MEMORY_CATEGORY_ORDER_SQL } from "./constants";
84
import { invalidateMemory, invalidateProject } from "./embedding-cache";
@@ -594,7 +590,6 @@ export class ModuleMemoryAuthorityError extends Error {
594590
}
595591

596592
function assertTsMemoryWriteAllowed(db: Database, projectPath: string): void {
597-
registerPrivilegedWriter(db);
598593
try {
599594
const managed = db
600595
.prepare(

‎packages/plugin/src/features/magic-context/migrations-v71.test.ts‎

Lines changed: 227 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,227 @@
1+
/// <reference types="bun-types" />
2+
3+
import { afterEach, describe, expect, it } from "bun:test";
4+
import { mkdtempSync, rmSync } from "node:fs";
5+
import { tmpdir } from "node:os";
6+
import { join } from "node:path";
7+
import type { Database as DatabaseType } from "../../shared/sqlite";
8+
import { Database, withPrivilegedWriter } from "../../shared/sqlite";
9+
import { closeQuietly } from "../../shared/sqlite-helpers";
10+
import { ensureContextStoreUuid, installAuthorityManagedMarker } from "./context-authority";
11+
import { runMigrations } from "./migrations";
12+
import { initializeDatabase } from "./storage-db";
13+
14+
// The durable predicate every authority guard must carry after v71. Asserted
15+
// byte-for-byte against sqlite_master.sql so a future regression that re-introduces
16+
// a connection-local UDF reference (or drops the guard) is pinned exactly.
17+
const STATE_TABLE_PREDICATE =
18+
"COALESCE((SELECT enabled FROM context_privilege_state WHERE id = 1), 0) = 0";
19+
20+
const GUARD_TRIGGERS = [
21+
"memories_authority_guard_insert",
22+
"memories_authority_guard_update",
23+
"memories_authority_guard_delete",
24+
"notes_authority_guard_insert",
25+
"notes_authority_guard_update",
26+
"notes_authority_guard_delete",
27+
] as const;
28+
29+
const databases: DatabaseType[] = [];
30+
const tempDirs: string[] = [];
31+
32+
function track<T extends DatabaseType>(db: T): T {
33+
databases.push(db);
34+
return db;
35+
}
36+
37+
function freshMigratedDatabase(path = ":memory:"): DatabaseType {
38+
const db = track(new Database(path));
39+
initializeDatabase(db);
40+
runMigrations(db);
41+
return db;
42+
}
43+
44+
/** Drop the v71 record so runMigrations re-applies the trigger rebuild. */
45+
function resetV71(db: DatabaseType): void {
46+
db.prepare("DELETE FROM schema_migrations WHERE version >= 71").run();
47+
}
48+
49+
/**
50+
* Bake the LEGACY trigger shape: the connection-local `mc_privileged_writer()` UDF
51+
* reference that migrations produced when the migrating runtime supported scalar UDFs.
52+
* This reproduces a database migrated under an older UDF-capable runtime, which is the
53+
* exact variant that broke every non-registering connection (issue #253).
54+
*/
55+
function bakeLegacyUdfTriggers(db: DatabaseType): void {
56+
const udf = "mc_privileged_writer() = 0";
57+
db.exec(`
58+
DROP TRIGGER IF EXISTS memories_authority_guard_insert;
59+
DROP TRIGGER IF EXISTS memories_authority_guard_update;
60+
DROP TRIGGER IF EXISTS memories_authority_guard_delete;
61+
CREATE TRIGGER memories_authority_guard_insert
62+
BEFORE INSERT ON memories
63+
WHEN (EXISTS (SELECT 1 FROM authority_managed WHERE project_path = NEW.project_path)
64+
OR EXISTS (SELECT 1 FROM authority_repair_pending WHERE project_path = NEW.project_path))
65+
AND ${udf}
66+
BEGIN SELECT RAISE(ABORT, 'context.db memory writes are managed by the Rust module'); END;
67+
CREATE TRIGGER memories_authority_guard_update
68+
BEFORE UPDATE ON memories
69+
WHEN (EXISTS (SELECT 1 FROM authority_managed WHERE project_path = OLD.project_path)
70+
OR EXISTS (SELECT 1 FROM authority_managed WHERE project_path = NEW.project_path)
71+
OR EXISTS (SELECT 1 FROM authority_repair_pending WHERE project_path = OLD.project_path)
72+
OR EXISTS (SELECT 1 FROM authority_repair_pending WHERE project_path = NEW.project_path))
73+
AND ${udf}
74+
BEGIN SELECT RAISE(ABORT, 'context.db memory writes are managed by the Rust module'); END;
75+
CREATE TRIGGER memories_authority_guard_delete
76+
BEFORE DELETE ON memories
77+
WHEN (EXISTS (SELECT 1 FROM authority_managed WHERE project_path = OLD.project_path)
78+
OR EXISTS (SELECT 1 FROM authority_repair_pending WHERE project_path = OLD.project_path))
79+
AND ${udf}
80+
BEGIN SELECT RAISE(ABORT, 'context.db memory writes are managed by the Rust module'); END;
81+
DROP TRIGGER IF EXISTS notes_authority_guard_insert;
82+
DROP TRIGGER IF EXISTS notes_authority_guard_update;
83+
DROP TRIGGER IF EXISTS notes_authority_guard_delete;
84+
CREATE TRIGGER notes_authority_guard_insert
85+
BEFORE INSERT ON notes
86+
WHEN (EXISTS (SELECT 1 FROM authority_managed WHERE project_path = NEW.project_path)
87+
OR EXISTS (SELECT 1 FROM authority_repair_pending WHERE project_path = NEW.project_path))
88+
AND ${udf}
89+
BEGIN SELECT RAISE(ABORT, 'context.db note writes are managed by the Rust module'); END;
90+
CREATE TRIGGER notes_authority_guard_update
91+
BEFORE UPDATE ON notes
92+
WHEN (EXISTS (SELECT 1 FROM authority_managed WHERE project_path = OLD.project_path)
93+
OR EXISTS (SELECT 1 FROM authority_managed WHERE project_path = NEW.project_path)
94+
OR EXISTS (SELECT 1 FROM authority_repair_pending WHERE project_path = OLD.project_path)
95+
OR EXISTS (SELECT 1 FROM authority_repair_pending WHERE project_path = NEW.project_path))
96+
AND ${udf}
97+
BEGIN SELECT RAISE(ABORT, 'context.db note writes are managed by the Rust module'); END;
98+
CREATE TRIGGER notes_authority_guard_delete
99+
BEFORE DELETE ON notes
100+
WHEN (EXISTS (SELECT 1 FROM authority_managed WHERE project_path = OLD.project_path)
101+
OR EXISTS (SELECT 1 FROM authority_repair_pending WHERE project_path = OLD.project_path))
102+
AND ${udf}
103+
BEGIN SELECT RAISE(ABORT, 'context.db note writes are managed by the Rust module'); END;
104+
`);
105+
}
106+
107+
function triggerSql(db: DatabaseType, name: string): string {
108+
const row = db
109+
.prepare("SELECT sql FROM sqlite_master WHERE type = 'trigger' AND name = ?")
110+
.get(name) as { sql?: string } | null;
111+
if (!row?.sql) throw new Error(`trigger ${name} not found`);
112+
return row.sql;
113+
}
114+
115+
function insertMemory(db: DatabaseType, projectPath: string, content: string): void {
116+
db.prepare(
117+
"INSERT INTO memories (project_path, category, content, normalized_hash, first_seen_at, created_at, updated_at, last_seen_at) VALUES (?, ?, ?, ?, 0, 0, 0, 0)",
118+
).run(projectPath, "CONSTRAINTS", content, `hash-${content}`);
119+
}
120+
121+
afterEach(() => {
122+
for (const db of databases.splice(0)) closeQuietly(db);
123+
for (const dir of tempDirs.splice(0)) rmSync(dir, { recursive: true, force: true });
124+
});
125+
126+
describe("migration v71: authority guards use the durable state-table predicate (issue #253)", () => {
127+
it("rebuilds legacy UDF-form triggers to reference context_privilege_state, not the UDF", () => {
128+
const db = freshMigratedDatabase();
129+
// Simulate a database migrated under an older UDF-capable runtime: roll back the
130+
// v71 record and bake the legacy UDF-reference triggers, then re-run the migration.
131+
resetV71(db);
132+
bakeLegacyUdfTriggers(db);
133+
// Sanity: the fixture really does carry the UDF reference before the migration.
134+
expect(triggerSql(db, "memories_authority_guard_insert")).toContain("mc_privileged_writer");
135+
136+
runMigrations(db);
137+
138+
// Byte-level: every guard now carries the exact durable predicate and no UDF ref.
139+
for (const name of GUARD_TRIGGERS) {
140+
const sql = triggerSql(db, name);
141+
expect(sql).toContain(STATE_TABLE_PREDICATE);
142+
expect(sql).not.toContain("mc_privileged_writer");
143+
}
144+
});
145+
146+
it("is idempotent on a database that already has state-form triggers", () => {
147+
const db = freshMigratedDatabase();
148+
const before = GUARD_TRIGGERS.map((name) => triggerSql(db, name));
149+
resetV71(db);
150+
151+
runMigrations(db);
152+
153+
const after = GUARD_TRIGGERS.map((name) => triggerSql(db, name));
154+
expect(after).toEqual(before);
155+
for (const sql of after) {
156+
expect(sql).toContain(STATE_TABLE_PREDICATE);
157+
expect(sql).not.toContain("mc_privileged_writer");
158+
}
159+
});
160+
161+
it("lets a second, raw connection write to a guarded table without 'no such function'", () => {
162+
// A file-backed DB so two independent connections can open the same schema.
163+
const dir = mkdtempSync(join(tmpdir(), "mc-v71-"));
164+
tempDirs.push(dir);
165+
const dbPath = join(dir, "context.db");
166+
167+
// Connection A: migrate and mark the project module-managed.
168+
const connA = freshMigratedDatabase(dbPath);
169+
const uuid = ensureContextStoreUuid(connA);
170+
installAuthorityManagedMarker(connA, "/project", uuid);
171+
172+
// Connection B: a plain open with NO privilege setup of its own — exactly the
173+
// connection shape that used to fail every guarded write with `no such function`.
174+
const connB = track(new Database(dbPath));
175+
withPrivilegedWriter(connB, () => {
176+
insertMemory(connB, "/project", "written by second connection");
177+
});
178+
179+
const row = connB
180+
.prepare("SELECT content FROM memories WHERE project_path = ?")
181+
.get("/project") as { content: string } | null;
182+
expect(row?.content).toBe("written by second connection");
183+
184+
// The privilege flag is cleared within the same immediate transaction, so once the
185+
// bracket returns no other connection can observe enabled=1.
186+
const enabled = connA
187+
.prepare("SELECT enabled FROM context_privilege_state WHERE id = 1")
188+
.get() as { enabled: number } | null;
189+
expect(enabled?.enabled ?? 0).toBe(0);
190+
});
191+
192+
it("still rejects unprivileged direct writes in both mutation directions", () => {
193+
const db = freshMigratedDatabase();
194+
const uuid = ensureContextStoreUuid(db);
195+
installAuthorityManagedMarker(db, "/project", uuid);
196+
197+
// INSERT direction: an unprivileged insert must be aborted by the guard. If the
198+
// trigger were dropped entirely this would succeed and the assertion would fail.
199+
expect(() => insertMemory(db, "/project", "blocked insert")).toThrow(
200+
"context.db memory writes are managed by the Rust module",
201+
);
202+
203+
// Seed a managed row through the privileged path so DELETE/UPDATE have a target.
204+
withPrivilegedWriter(db, () => insertMemory(db, "/project", "managed row"));
205+
const rowId = (
206+
db
207+
.prepare("SELECT id FROM memories WHERE project_path = ? AND content = ?")
208+
.get("/project", "managed row") as { id: number }
209+
).id;
210+
211+
// DELETE direction.
212+
expect(() => db.prepare("DELETE FROM memories WHERE id = ?").run(rowId)).toThrow(
213+
"context.db memory writes are managed by the Rust module",
214+
);
215+
// UPDATE direction.
216+
expect(() =>
217+
db.prepare("UPDATE memories SET content = ? WHERE id = ?").run("tampered", rowId),
218+
).toThrow("context.db memory writes are managed by the Rust module");
219+
220+
// The row is untouched and, critically, the failure is the managed-write abort —
221+
// never `no such function`.
222+
const surviving = db.prepare("SELECT content FROM memories WHERE id = ?").get(rowId) as {
223+
content: string;
224+
};
225+
expect(surviving.content).toBe("managed row");
226+
});
227+
});

‎packages/plugin/src/features/magic-context/migrations.ts‎

Lines changed: 35 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -121,14 +121,23 @@ function assertForeignKeyIntegrity(db: Database, table?: string): void {
121121
}
122122
}
123123

124-
function authorityPrivilegeCheck(db: Database): string {
125-
const native = db as unknown as {
126-
function?: unknown;
127-
createFunction?: unknown;
128-
};
129-
return typeof native.function === "function" || typeof native.createFunction === "function"
130-
? "mc_privileged_writer() = 0"
131-
: "COALESCE((SELECT enabled FROM context_privilege_state WHERE id = 1), 0) = 0";
124+
/**
125+
* SQL predicate that guards managed-write triggers fire only for UNPRIVILEGED
126+
* writers.
127+
*
128+
* This must ALWAYS be the durable `context_privilege_state` table check, never a
129+
* connection-local scalar UDF. Triggers are DURABLE database artifacts: their SQL
130+
* is stored in the file and re-evaluated on EVERY connection that opens context.db.
131+
* A UDF such as the former `mc_privileged_writer()` only exists on connections that
132+
* registered it, so a trigger baked with that reference fails with `no such function`
133+
* on any connection that did not (older Bun without scalar-UDF support, node:sqlite,
134+
* the dashboard's rusqlite). The state table is ordinary schema every connection can
135+
* read, so the guard works identically everywhere. Privileged writers flip
136+
* `context_privilege_state.enabled` inside their own BEGIN IMMEDIATE transaction
137+
* (see withPrivilegedWriter), which no second connection can observe.
138+
*/
139+
function authorityPrivilegeCheck(): string {
140+
return "COALESCE((SELECT enabled FROM context_privilege_state WHERE id = 1), 0) = 0";
132141
}
133142

134143
function managedAuthorityNoteRow(row: "OLD" | "NEW"): string {
@@ -150,7 +159,7 @@ function managedAuthorityNoteRow(row: "OLD" | "NEW"): string {
150159

151160
/** Install the latest authority fences after historical/recovery migration batches. */
152161
function installLatestAuthorityTriggers(db: Database): void {
153-
const privilegeCheck = authorityPrivilegeCheck(db);
162+
const privilegeCheck = authorityPrivilegeCheck();
154163
if (tableExists(db, "memories")) {
155164
db.exec(`
156165
DROP TRIGGER IF EXISTS memories_authority_guard_insert;
@@ -2669,6 +2678,23 @@ const MIGRATIONS: Migration[] = [
26692678
healMismatchedTierClose(db, "recomp_compartments", false);
26702679
},
26712680
},
2681+
{
2682+
version: 71,
2683+
description:
2684+
"rebuild authority guard triggers to the durable state-table form (issue #253)",
2685+
up(db: Database): void {
2686+
// Earlier migrations baked a connection-local UDF reference
2687+
// (mc_privileged_writer()) into these triggers when the MIGRATING runtime
2688+
// supported scalar UDFs. That reference is only evaluable on connections
2689+
// that registered the UDF, so every other connection opening context.db
2690+
// failed each guarded write with `no such function`. Rebuild all six guards
2691+
// to the durable context_privilege_state predicate. DROP + CREATE is safe on
2692+
// both existing variants (UDF-form and state-form triggers) and on fresh
2693+
// databases. installLatestAuthorityTriggers now emits the state-table form
2694+
// unconditionally.
2695+
installLatestAuthorityTriggers(db);
2696+
},
2697+
},
26722698
];
26732699

26742700
/**

‎packages/plugin/src/features/magic-context/storage-db.ts‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import {
88
} from "../../shared/data-path";
99
import { getErrorMessage } from "../../shared/error-message";
1010
import { log } from "../../shared/logger";
11-
import { Database, registerPrivilegedWriter } from "../../shared/sqlite";
11+
import { Database } from "../../shared/sqlite";
1212
import { closeQuietly } from "../../shared/sqlite-helpers";
1313
import { ensureContextStoreUuid } from "./context-authority";
1414
import { runMigrations, runMigrationsWithRetry } from "./migrations";
@@ -46,7 +46,7 @@ export function getSchemaFenceRejection(): {
4646
return lastSchemaFenceRejection;
4747
}
4848

49-
export const LATEST_SUPPORTED_VERSION = 70;
49+
export const LATEST_SUPPORTED_VERSION = 71;
5050

5151
// chmod is meaningless on Windows (POSIX modes are not honored), so all
5252
// permission tightening is skipped there. mkdir's `mode` is likewise ignored.
@@ -1701,7 +1701,6 @@ export function openDatabase(dbPathOrOptions?: string | OpenDatabaseOptions): Da
17011701
if (!enforceSchemaFence(existing, dbPath, latestSupportedVersion)) {
17021702
return null;
17031703
}
1704-
registerPrivilegedWriter(existing);
17051704
if (!persistenceByDatabase.has(existing)) {
17061705
persistenceByDatabase.set(existing, true);
17071706
}
@@ -1720,7 +1719,6 @@ export function openDatabase(dbPathOrOptions?: string | OpenDatabaseOptions): Da
17201719
ensureSecureStorageDir(dbDir);
17211720

17221721
const db = new Database(dbPath);
1723-
registerPrivilegedWriter(db);
17241722
if (!enforceSchemaFence(db, dbPath, latestSupportedVersion)) {
17251723
closeQuietly(db);
17261724
return null;
@@ -1771,7 +1769,6 @@ export async function openDatabaseAsync(
17711769
ensureSecureStorageDir(dbDir);
17721770

17731771
db = new Database(dbPath);
1774-
registerPrivilegedWriter(db);
17751772
if (!enforceSchemaFence(db, dbPath, latestSupportedVersion)) {
17761773
closeQuietly(db);
17771774
return null;

0 commit comments

Comments
 (0)