diff --git a/packages/core/src/fs-util.ts b/packages/core/src/fs-util.ts index 93a551c9b0a2..ce383beffcf3 100644 --- a/packages/core/src/fs-util.ts +++ b/packages/core/src/fs-util.ts @@ -2,6 +2,7 @@ import { NodeFileSystem } from "@effect/platform-node" import { dirname, isAbsolute, join, relative, resolve as pathResolve, sep } from "path" import { realpathSync } from "fs" import * as NFS from "fs/promises" +import { randomUUID } from "crypto" import { lookup } from "mime-types" import { Context, Effect, FileSystem, Layer, Schema } from "effect" import type { PlatformError } from "effect/PlatformError" @@ -35,6 +36,7 @@ export namespace FSUtil { readonly readFileStringSafe: (path: string) => Effect.Effect readonly readJson: (path: string) => Effect.Effect readonly writeJson: (path: string, data: unknown, mode?: number) => Effect.Effect + readonly writeJsonAtomic: (path: string, data: unknown, mode?: number) => Effect.Effect readonly ensureDir: (path: string) => Effect.Effect readonly writeWithDirs: (path: string, content: string | Uint8Array, mode?: number) => Effect.Effect readonly readDirectoryEntries: (path: string) => Effect.Effect @@ -113,6 +115,18 @@ export namespace FSUtil { if (mode) yield* fs.chmod(path, mode) }) + const writeJsonAtomic = Effect.fn("FileSystem.writeJsonAtomic")(function* (path: string, data: unknown, mode?: number) { + const temporary = `${path}.${process.pid}.${randomUUID()}.tmp` + yield* ensureDir(dirname(path)) + yield* Effect.gen(function* () { + // The mode is applied at create so the temp is never briefly world-readable, and rename + // carries it to the target. A chmod after the rename would follow a symlink planted in + // the window between the two. + yield* fs.writeFileString(temporary, JSON.stringify(data, null, 2), mode ? { flag: "wx", mode } : { flag: "wx" }) + yield* fs.rename(temporary, path) + }).pipe(Effect.ensuring(fs.remove(temporary, { force: true }).pipe(Effect.ignore))) + }) + const ensureDir = Effect.fn("FileSystem.ensureDir")(function* (path: string) { yield* fs.makeDirectory(path, { recursive: true }).pipe( // Bun on Windows can throw EEXIST here despite recursive mode. @@ -207,6 +221,7 @@ export namespace FSUtil { resolve, readJson, writeJson, + writeJsonAtomic, ensureDir, writeWithDirs, findUp, diff --git a/packages/core/test/filesystem/filesystem.test.ts b/packages/core/test/filesystem/filesystem.test.ts index fdce1b447644..99e939720727 100644 --- a/packages/core/test/filesystem/filesystem.test.ts +++ b/packages/core/test/filesystem/filesystem.test.ts @@ -4,6 +4,7 @@ import { LayerNodePlatform } from "@opencode-ai/core/effect/app-node-platform" import { LayerNode } from "@opencode-ai/core/effect/layer-node" import { FSUtil } from "@opencode-ai/core/fs-util" import { testEffect } from "../lib/effect" +import nodeFs from "fs/promises" import path from "path" const live = LayerNode.compile(LayerNode.group([FSUtil.node, LayerNodePlatform.filesystem])) @@ -95,6 +96,23 @@ describe("FSUtil", () => { }) describe("readJson / writeJson", () => { + it( + "writes JSON atomically with the requested mode and cleans up its temporary file", + Effect.gen(function* () { + const fs = yield* FSUtil.Service + const filesys = yield* FileSystem.FileSystem + const tmp = yield* filesys.makeTempDirectoryScoped() + const file = path.join(tmp, "data.json") + + yield* fs.writeJsonAtomic(file, { atomic: true }, 0o600) + + const info = yield* Effect.promise(() => nodeFs.stat(file)) + const entries = yield* Effect.promise(() => nodeFs.readdir(tmp)) + expect(info.mode & 0o777).toBe(0o600) + expect(entries.filter((entry) => /^data\.json\.\d+\..+\.tmp$/.test(entry))).toEqual([]) + }), + ) + it( "round-trips JSON data", Effect.gen(function* () { diff --git a/packages/opencode/src/auth/index.ts b/packages/opencode/src/auth/index.ts index a133e88498d5..07f994b0840e 100644 --- a/packages/opencode/src/auth/index.ts +++ b/packages/opencode/src/auth/index.ts @@ -4,6 +4,7 @@ import { Effect, Layer, Record, Result, Schema, Context } from "effect" import { NonNegativeInt } from "@opencode-ai/core/schema" import { Global } from "@opencode-ai/core/global" import { FSUtil } from "@opencode-ai/core/fs-util" +import { EffectFlock } from "@opencode-ai/core/util/effect-flock" export const OAUTH_DUMMY_KEY = "opencode-oauth-dummy-key" @@ -53,7 +54,14 @@ const layer = Layer.effect( Service, Effect.gen(function* () { const fsys = yield* FSUtil.Service + const flock = yield* EffectFlock.Service const decode = Schema.decodeUnknownOption(Info) + const lockKey = "auth" + + const read = Effect.fn("Auth.read")(function* () { + const data = (yield* fsys.readJson(file).pipe(Effect.orElseSucceed(() => ({})))) as Record + return Record.filterMap(data, (value) => Result.fromOption(decode(value), () => undefined)) + }) const all = Effect.fn("Auth.all")(function* () { if (process.env.OPENCODE_AUTH_CONTENT) { @@ -62,8 +70,7 @@ const layer = Layer.effect( } catch (err) {} } - const data = (yield* fsys.readJson(file).pipe(Effect.orElseSucceed(() => ({})))) as Record - return Record.filterMap(data, (value) => Result.fromOption(decode(value), () => undefined)) + return yield* read() }) const get = Effect.fn("Auth.get")(function* (providerID: string) { @@ -72,26 +79,28 @@ const layer = Layer.effect( const set = Effect.fn("Auth.set")(function* (key: string, info: Info) { const norm = key.replace(/\/+$/, "") - const data = yield* all() - if (norm !== key) delete data[key] - delete data[norm + "/"] - yield* fsys - .writeJson(file, { ...data, [norm]: info }, 0o600) - .pipe(Effect.mapError(fail("Failed to write auth data"))) + yield* Effect.gen(function* () { + const data = yield* read() + if (norm !== key) delete data[key] + delete data[norm + "/"] + yield* fsys.writeJsonAtomic(file, { ...data, [norm]: info }, 0o600) + }).pipe(flock.withLock(lockKey), Effect.mapError(fail("Failed to write auth data"))) }) const remove = Effect.fn("Auth.remove")(function* (key: string) { const norm = key.replace(/\/+$/, "") - const data = yield* all() - delete data[key] - delete data[norm] - yield* fsys.writeJson(file, data, 0o600).pipe(Effect.mapError(fail("Failed to write auth data"))) + yield* Effect.gen(function* () { + const data = yield* read() + delete data[key] + delete data[norm] + yield* fsys.writeJsonAtomic(file, data, 0o600) + }).pipe(flock.withLock(lockKey), Effect.mapError(fail("Failed to write auth data"))) }) return Service.of({ get, all, set, remove }) }), ) -export const node = LayerNode.make({ service: Service, layer: layer, deps: [FSUtil.node] }) +export const node = LayerNode.make({ service: Service, layer: layer, deps: [FSUtil.node, EffectFlock.node] }) export * as Auth from "." diff --git a/packages/opencode/test/auth/auth.test.ts b/packages/opencode/test/auth/auth.test.ts index bb72be66e57c..e722801c0ba7 100644 --- a/packages/opencode/test/auth/auth.test.ts +++ b/packages/opencode/test/auth/auth.test.ts @@ -1,5 +1,8 @@ import { describe, expect } from "bun:test" +import fs from "fs/promises" +import path from "path" import { LayerNode } from "@opencode-ai/core/effect/layer-node" +import { Global } from "@opencode-ai/core/global" import { Effect } from "effect" import { Auth } from "../../src/auth" import { testEffect } from "../lib/effect" @@ -21,6 +24,67 @@ describe("Auth", () => { }), ) + it.instance("set reads the file instead of persisting the auth env snapshot", () => + Effect.gen(function* () { + const auth = yield* Auth.Service + const file = path.join(Global.Path.data, "auth.json") + const previous = process.env.OPENCODE_AUTH_CONTENT + const omitted = "omitted-provider" + + try { + yield* Effect.promise(() => fs.writeFile(file, JSON.stringify({ [omitted]: { type: "api", key: "sk-omitted" } }))) + process.env.OPENCODE_AUTH_CONTENT = JSON.stringify({ "snapshot-provider": { type: "api", key: "sk-snapshot" } }) + + yield* auth.set("written-provider", { type: "api", key: "sk-written" }) + + const onDisk = JSON.parse(yield* Effect.promise(() => fs.readFile(file, "utf8"))) as Record + expect(onDisk[omitted]).toEqual({ type: "api", key: "sk-omitted" }) + expect(onDisk["written-provider"]).toEqual({ type: "api", key: "sk-written" }) + } finally { + if (previous === undefined) delete process.env.OPENCODE_AUTH_CONTENT + else process.env.OPENCODE_AUTH_CONTENT = previous + } + }), + ) + + it.instance("preserves both providers from concurrent writes", () => + Effect.gen(function* () { + const auth = yield* Auth.Service + const file = path.join(Global.Path.data, "auth.json") + + yield* Effect.promise(() => fs.writeFile(file, JSON.stringify({ existing: { type: "api", key: "sk-existing" } }))) + yield* Effect.all( + [ + auth.set("concurrent-a", { type: "api", key: "sk-a" }), + auth.set("concurrent-b", { type: "api", key: "sk-b" }), + ], + { concurrency: "unbounded" }, + ) + + const onDisk = JSON.parse(yield* Effect.promise(() => fs.readFile(file, "utf8"))) as Record + expect(onDisk.existing).toEqual({ type: "api", key: "sk-existing" }) + expect(onDisk["concurrent-a"]).toEqual({ type: "api", key: "sk-a" }) + expect(onDisk["concurrent-b"]).toEqual({ type: "api", key: "sk-b" }) + }), + ) + + it.instance("writes auth.json atomically with mode 0600 and no temporary file", () => + Effect.gen(function* () { + const auth = yield* Auth.Service + const file = path.join(Global.Path.data, "auth.json") + const before = yield* Effect.promise(() => fs.readdir(Global.Path.data)) + + yield* auth.set("atomic-provider", { type: "api", key: "sk-atomic" }) + + const stats = yield* Effect.promise(() => fs.stat(file)) + const after = yield* Effect.promise(() => fs.readdir(Global.Path.data)) + expect(stats.mode & 0o777).toBe(0o600) + expect(after.filter((entry) => /^auth\.json\.\d+\..+\.tmp$/.test(entry))).toEqual( + before.filter((entry) => /^auth\.json\.\d+\..+\.tmp$/.test(entry)), + ) + }), + ) + it.instance("set cleans up pre-existing trailing-slash entry", () => Effect.gen(function* () { const auth = yield* Auth.Service