Skip to content

Commit 6aeab33

Browse files
tracycamalfonso-magic-context
authored andcommitted
plugin: harden storage permissions to 0700/0600
Cherry-picked from PR #143 (parts 1+2 of 3; part 3 superseded by the finer-grained stripUnsafeProjectConfigFields already on master). Resolved import conflicts against the NODE_ENV=test isolation backstop. context.db holds raw conversation history. The storage dir + models cache dir are created/chmod'd 0700 and context.db/-wal/-shm chmod'd 0600. Best-effort with try/catch, skipped on win32. Co-authored-by: Alfonso [Magic Context] <288211368+alfonso-magic-context@users.noreply.github.com>
1 parent 8027bc7 commit 6aeab33

3 files changed

Lines changed: 94 additions & 7 deletions

File tree

packages/plugin/src/features/magic-context/memory/embedding-local.ts

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { mkdirSync } from "node:fs";
1+
import { chmodSync, mkdirSync } from "node:fs";
22
import { open, stat, unlink, writeFile } from "node:fs/promises";
33
import { dirname, join } from "node:path";
44
import { pathToFileURL } from "node:url";
@@ -455,7 +455,17 @@ export class LocalEmbeddingProvider implements EmbeddingProvider {
455455
// or buffer" failures. Using our own storage dir survives plugin updates too.
456456
const modelCacheDir = join(getMagicContextStorageDir(), "models");
457457
try {
458-
mkdirSync(modelCacheDir, { recursive: true });
458+
// Owner-only: the cache lives under the same storage tree as
459+
// memories/history. mkdir's mode is masked by umask, so chmod
460+
// afterwards (no-op on Windows, where POSIX modes are ignored).
461+
mkdirSync(modelCacheDir, { recursive: true, mode: 0o700 });
462+
if (process.platform !== "win32") {
463+
try {
464+
chmodSync(modelCacheDir, 0o700);
465+
} catch {
466+
// Non-fatal — leave default perms if chmod is rejected.
467+
}
468+
}
459469
env.cacheDir = modelCacheDir;
460470
} catch {
461471
// Non-fatal — fall back to library default if we can't create the dir

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

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,17 @@
11
/// <reference types="bun-types" />
22

33
import { afterEach, describe, expect, it } from "bun:test";
4-
import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs";
4+
import {
5+
existsSync,
6+
mkdirSync,
7+
mkdtempSync,
8+
readFileSync,
9+
rmSync,
10+
statSync,
11+
writeFileSync,
12+
} from "node:fs";
513
import { homedir, tmpdir } from "node:os";
6-
import { join } from "node:path";
14+
import { dirname, join } from "node:path";
715
import { Database } from "../../shared/sqlite";
816
import { closeQuietly } from "../../shared/sqlite-helpers";
917
import {
@@ -63,6 +71,26 @@ describe("storage-db", () => {
6371
expect(isDatabasePersisted(db)).toBe(true);
6472
});
6573

74+
it("#when called first time #then restricts storage dir to 0o700 and DB files to 0o600", () => {
75+
// POSIX-only: chmod is a no-op on Windows (modes are not honored).
76+
if (process.platform === "win32") return;
77+
const dataHome = useTempDataHome("storage-db-perms-");
78+
79+
openDatabase();
80+
81+
const dbPath = resolveDbPath(dataHome);
82+
const dbDir = dirname(dbPath);
83+
// Low 9 permission bits only (mask off file-type/setuid bits).
84+
expect(statSync(dbDir).mode & 0o777).toBe(0o700);
85+
expect(statSync(dbPath).mode & 0o777).toBe(0o600);
86+
for (const suffix of ["-wal", "-shm"]) {
87+
const sidecar = `${dbPath}${suffix}`;
88+
if (existsSync(sidecar)) {
89+
expect(statSync(sidecar).mode & 0o777).toBe(0o600);
90+
}
91+
}
92+
});
93+
6694
it("#when called first time #then creates required tables", () => {
6795
useTempDataHome("storage-db-tables-");
6896

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

Lines changed: 52 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { copyFileSync, cpSync, existsSync, mkdirSync, mkdtempSync } from "node:fs";
1+
import { chmodSync, copyFileSync, cpSync, existsSync, mkdirSync, mkdtempSync } from "node:fs";
22
import { tmpdir } from "node:os";
33
import { dirname, join } from "node:path";
44
import {
@@ -40,6 +40,52 @@ export function getSchemaFenceRejection(): {
4040

4141
export const LATEST_SUPPORTED_VERSION = 41;
4242

43+
// chmod is meaningless on Windows (POSIX modes are not honored), so all
44+
// permission tightening is skipped there. mkdir's `mode` is likewise ignored.
45+
const PERMISSIONS_ENFORCEABLE = process.platform !== "win32";
46+
47+
/**
48+
* Create `dir` (recursively) owner-only and tighten an existing dir to 0o700.
49+
*
50+
* The storage tree holds project memories, raw conversation history, and
51+
* embeddings. Created with the default umask these can be group/world-readable,
52+
* leaking that content to other local users. We create with mode 0o700 and
53+
* additionally chmod (mkdir's `mode` is masked by umask and a no-op when the
54+
* dir already exists). Best-effort: a chmod failure is logged, not fatal.
55+
*/
56+
function ensureSecureStorageDir(dir: string): void {
57+
mkdirSync(dir, { recursive: true, mode: 0o700 });
58+
if (!PERMISSIONS_ENFORCEABLE) return;
59+
try {
60+
chmodSync(dir, 0o700);
61+
} catch (error) {
62+
log(
63+
`[magic-context] could not restrict storage dir permissions on ${dir}: ${getErrorMessage(error)}`,
64+
);
65+
}
66+
}
67+
68+
/**
69+
* Restrict the SQLite DB file and its WAL/SHM sidecars to owner-only (0o600).
70+
* They are created with the process umask, which can be group/world-readable;
71+
* since they hold the same durable state as the storage dir, tighten them once
72+
* they exist. Best-effort per file.
73+
*/
74+
function restrictDatabaseFilePermissions(dbPath: string): void {
75+
if (!PERMISSIONS_ENFORCEABLE) return;
76+
for (const suffix of ["", "-wal", "-shm"]) {
77+
const file = `${dbPath}${suffix}`;
78+
if (!existsSync(file)) continue;
79+
try {
80+
chmodSync(file, 0o600);
81+
} catch (error) {
82+
log(
83+
`[magic-context] could not restrict DB file permissions on ${file}: ${getErrorMessage(error)}`,
84+
);
85+
}
86+
}
87+
}
88+
4389
export interface OpenDatabaseOptions {
4490
dbPath?: string;
4591
latestSupportedVersion?: number;
@@ -153,7 +199,7 @@ function migrateLegacyStorageIfNeeded(targetDbPath: string, targetDbDir: string)
153199
log(
154200
`[magic-context] migrating legacy plugin storage: ${legacyDir} -> ${targetDbDir} (legacy left in place as backup)`,
155201
);
156-
mkdirSync(targetDbDir, { recursive: true });
202+
ensureSecureStorageDir(targetDbDir);
157203

158204
// Fold the legacy WAL into the main DB FIRST so the copied target is one
159205
// crash-consistent file. Copying .db/-wal/-shm as three separate files is
@@ -1455,7 +1501,7 @@ export function openDatabase(dbPathOrOptions?: string | OpenDatabaseOptions): Da
14551501
if (!explicitDbPath) {
14561502
migrateLegacyStorageIfNeeded(dbPath, dbDir);
14571503
}
1458-
mkdirSync(dbDir, { recursive: true });
1504+
ensureSecureStorageDir(dbDir);
14591505

14601506
const db = new Database(dbPath);
14611507
if (!enforceSchemaFence(db, dbPath, latestSupportedVersion)) {
@@ -1507,6 +1553,9 @@ export function openDatabase(dbPathOrOptions?: string | OpenDatabaseOptions): Da
15071553
// sidebar regression report.
15081554
setToolDefinitionDatabase(db);
15091555
loadToolDefinitionMeasurements(db);
1556+
// Tighten the DB + WAL/SHM sidecars to owner-only now that WAL mode has
1557+
// created the sidecars; best-effort, never fatal.
1558+
restrictDatabaseFilePermissions(dbPath);
15101559
databases.set(dbPath, db);
15111560
pathByDatabase.set(db, dbPath);
15121561
persistenceByDatabase.set(db, true);

0 commit comments

Comments
 (0)