Skip to content

Commit 09d9a49

Browse files
author
anqiang.ex
committed
fix(core): start with a database the owner cannot tighten
Since #52150 the database layer chmods `opencode.db` to `0600` before opening it. When the file belongs to another user, which an earlier `sudo opencode` leaves behind, `chmod` fails with `EPERM` even though the file is still readable and writable. The failure aborted the background service, and the CLI only reported `Background service failed to start`, with the dead service still holding its port (#57032). Tightening is best effort: `ENOENT`, `EPERM` and `EACCES` for the database and both sidecars no longer stop startup, while any other failure still surfaces. A refused path is returned to the caller, which logs that the files stay readable by other users and suggests `chown`, so the looser mode is visible instead of silent. The chmod implementation is a parameter so the refusal is testable without a second user: one case asserts a refused chmod is tolerated and reported, another that an unrelated failure still rejects.
1 parent f394d9d commit 09d9a49

2 files changed

Lines changed: 47 additions & 12 deletions

File tree

‎packages/core/src/database/database.ts‎

Lines changed: 25 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -74,27 +74,41 @@ export function layer(options: Options = { path: ":memory:" }) {
7474
const filename = options.path ?? ":memory:"
7575
if (filename === ":memory:") return provide(filename)
7676
const file = isAbsolute(filename) ? filename : join((yield* Global.Service).data, filename)
77-
yield* Effect.promise(() => restrictToOwner(file))
77+
const refused = yield* Effect.promise(() => restrictToOwner(file))
78+
// The files stay readable by other users when they belong to one, so say it instead of leaving the
79+
// looser permissions silent (#57032 review).
80+
if (refused.length > 0)
81+
yield* Effect.logWarning("Database files are readable by other users and cannot be restricted", {
82+
files: refused,
83+
hint: "chown them to the current user to enforce owner-only access",
84+
})
7885
return provide(file)
7986
}),
8087
)
8188
}
8289

8390
// SQLite creates new sidecars with the database's mode, but does not tighten existing sidecars.
8491
// Windows relies on the user profile directory's inherited ACLs instead of POSIX modes.
85-
async function restrictToOwner(filename: string) {
86-
if (process.platform === "win32") return
92+
export async function restrictToOwner(
93+
filename: string,
94+
restrict: (file: string, mode: number) => Promise<void> = chmod,
95+
) {
96+
if (process.platform === "win32") return []
8797
// Opening and closing an existing database can release another connection's POSIX locks.
8898
// Create missing files synchronously so another fiber cannot open one before it is restricted.
8999
if (!existsSync(filename)) closeSync(openSync(filename, "a", 0o600))
90-
await chmod(filename, 0o600)
91-
await Promise.all(
92-
[`${filename}-wal`, `${filename}-shm`].map((file) =>
93-
chmod(file, 0o600).catch((error: NodeJS.ErrnoException) => {
94-
if (error.code !== "ENOENT") throw error
95-
}),
96-
),
97-
)
100+
// Tightening is best effort. A database owned by another user, such as one left behind by an earlier
101+
// `sudo opencode`, is still readable and writable, and failing here stopped startup entirely (#57032).
102+
// Those paths are reported instead so the caller can say the files stay readable by other users.
103+
const refused: string[] = []
104+
const tighten = (file: string) =>
105+
restrict(file, 0o600).catch((error: NodeJS.ErrnoException) => {
106+
if (error.code === "ENOENT") return
107+
if (error.code === "EPERM" || error.code === "EACCES") return void refused.push(file)
108+
throw error
109+
})
110+
await Promise.all([filename, `${filename}-wal`, `${filename}-shm`].map(tighten))
111+
return refused
98112
}
99113

100114
// The database service over an injected SqlClient, for runtimes that receive

‎packages/core/test/database.test.ts‎

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { describe, expect, test } from "bun:test"
22
import { chmod, stat, writeFile } from "node:fs/promises"
33
import path from "path"
44
import { Effect, Layer } from "effect"
5-
import { Database } from "@opencode/core/database/database"
5+
import { Database, restrictToOwner } from "@opencode/core/database/database"
66
import { Global } from "@opencode/util/global"
77
import { tmpdir } from "./fixture/tmpdir"
88

@@ -36,4 +36,25 @@ describe.skipIf(process.platform === "win32")("Database file permissions", () =>
3636

3737
expect(await openModes(filename)).toEqual([0o600, 0o600, 0o600])
3838
})
39+
40+
// A database left behind by an earlier `sudo opencode` is still readable and writable, so a refused
41+
// chmod must not stop startup: it used to abort the whole service (#57032).
42+
test("opens a database it is not allowed to tighten", async () => {
43+
await using tmp = await tmpdir()
44+
const filename = path.join(tmp.path, "opencode.db")
45+
await writeFile(filename, "")
46+
const denied = () => Promise.reject(Object.assign(new Error("EPERM: operation not permitted"), { code: "EPERM" }))
47+
48+
// The refusal is reported so the caller can warn that the files stay readable by other users.
49+
await expect(restrictToOwner(filename, denied)).resolves.toEqual([filename, `${filename}-wal`, `${filename}-shm`])
50+
})
51+
52+
test("surfaces restriction failures that are not about ownership", async () => {
53+
await using tmp = await tmpdir()
54+
const filename = path.join(tmp.path, "opencode.db")
55+
await writeFile(filename, "")
56+
const failed = () => Promise.reject(Object.assign(new Error("EIO: i/o error"), { code: "EIO" }))
57+
58+
await expect(restrictToOwner(filename, failed)).rejects.toThrow("EIO")
59+
})
3960
})

0 commit comments

Comments
 (0)