From ba0f998ca23824a1f8d248aafe7e1b1412fe7502 Mon Sep 17 00:00:00 2001 From: Rhys Sullivan <39114868+RhysSullivan@users.noreply.github.com> Date: Fri, 17 Jul 2026 21:54:37 -0700 Subject: [PATCH 1/4] Add regression test for secrets data directory --- .../plugins/file-secrets/src/data-dir.test.ts | 118 ++++++++++++++++++ 1 file changed, 118 insertions(+) create mode 100644 packages/plugins/file-secrets/src/data-dir.test.ts diff --git a/packages/plugins/file-secrets/src/data-dir.test.ts b/packages/plugins/file-secrets/src/data-dir.test.ts new file mode 100644 index 000000000..fd98d9e29 --- /dev/null +++ b/packages/plugins/file-secrets/src/data-dir.test.ts @@ -0,0 +1,118 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "@effect/vitest"; +import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { isAbsolute, join, relative } from "node:path"; +import { Effect } from "effect"; + +import { + AuthTemplateSlug, + ConnectionName, + IntegrationSlug, + ProviderKey, + definePlugin, +} from "@executor-js/sdk"; +import { makeTestWorkspaceHarness } from "@executor-js/sdk/testing"; + +import { fileSecretsPlugin } from "./index"; + +const INTEGRATION = IntegrationSlug.make("durable-secrets"); +const CONNECTION = ConnectionName.make("main"); +const TEMPLATE = AuthTemplateSlug.make("apiKey"); +const CREDENTIAL = "secret-token"; + +const connectionFixturePlugin = definePlugin(() => ({ + id: "connectionFixture" as const, + storage: () => ({}), + resolveTools: () => Effect.succeed({ tools: [] }), + extension: (ctx) => ({ + registerIntegration: () => + ctx.core.integrations.register({ + slug: INTEGRATION, + description: "Durable secrets test integration", + config: {}, + }), + resolveCredential: () => + ctx.connections.resolveValue({ + owner: "org", + integration: INTEGRATION, + name: CONNECTION, + }), + }), +}))(); + +const plugins = () => [fileSecretsPlugin(), connectionFixturePlugin] as const; + +describe("file secrets data directory", () => { + let workDir: string; + let dataDir: string; + let firstSandboxDataHome: string; + let recreatedSandboxDataHome: string; + + beforeEach(() => { + workDir = mkdtempSync(join(tmpdir(), "executor-file-secrets-data-dir-")); + dataDir = join(workDir, "executor-data"); + firstSandboxDataHome = join(workDir, "sandbox-a-xdg"); + recreatedSandboxDataHome = join(workDir, "sandbox-b-xdg"); + mkdirSync(dataDir, { recursive: true }); + mkdirSync(firstSandboxDataHome, { recursive: true }); + mkdirSync(recreatedSandboxDataHome, { recursive: true }); + vi.stubEnv("EXECUTOR_DATA_DIR", dataDir); + }); + + afterEach(() => { + vi.unstubAllEnvs(); + rmSync(workDir, { recursive: true, force: true }); + }); + + it.effect("keeps credentials when only EXECUTOR_DATA_DIR survives sandbox recreation", () => + Effect.gen(function* () { + vi.stubEnv("XDG_DATA_HOME", firstSandboxDataHome); + const firstAuthPath = yield* Effect.scoped( + Effect.gen(function* () { + const first = yield* makeTestWorkspaceHarness({ dataDir, plugins: plugins() }); + yield* first.executor.connectionFixture.registerIntegration(); + const connection = yield* first.executor.connections.create({ + owner: "org", + name: CONNECTION, + integration: INTEGRATION, + template: TEMPLATE, + value: CREDENTIAL, + }); + + expect(connection.provider).toBe(ProviderKey.make("file")); + const authPath = first.executor.fileSecrets.filePath; + const pathWithinDataDir = relative(dataDir, authPath); + if (pathWithinDataDir.startsWith("..") || isAbsolute(pathWithinDataDir)) { + expect(authPath).toBe(join(firstSandboxDataHome, "executor", "auth.json")); + } + expect(existsSync(authPath)).toBe(true); + expect(readFileSync(authPath, "utf8")).toContain( + '"connection:org:durable-secrets:main:token": "secret-token"', + ); + expect(existsSync(join(dataDir, "test.db"))).toBe(true); + return authPath; + }), + ); + + vi.stubEnv("XDG_DATA_HOME", recreatedSandboxDataHome); + yield* Effect.scoped( + Effect.gen(function* () { + const recreated = yield* makeTestWorkspaceHarness({ dataDir, plugins: plugins() }); + const connections = yield* recreated.executor.connections.list({ + integration: INTEGRATION, + }); + expect(connections.map((connection) => String(connection.name))).toEqual(["main"]); + + const resolved = yield* recreated.executor.connectionFixture.resolveCredential(); + + // Regression: auth.json follows XDG_DATA_HOME instead of EXECUTOR_DATA_DIR. + // After the fix it must live with test.db under dataDir and survive this home swap. + expect( + resolved, + `credential persisted in ${firstAuthPath} was not available after recreating the sandbox with ${dataDir}`, + ).toBe(CREDENTIAL); + }), + ); + }), + ); +}); From af1905c529494a193cf36921ea89466c497c7582 Mon Sep 17 00:00:00 2001 From: Rhys Sullivan <39114868+RhysSullivan@users.noreply.github.com> Date: Fri, 17 Jul 2026 22:03:41 -0700 Subject: [PATCH 2/4] Store file secrets in executor data directory --- .../plugins/file-secrets/src/index.test.ts | 144 +++++++++++++++ packages/plugins/file-secrets/src/index.ts | 166 +++++++++++++----- 2 files changed, 266 insertions(+), 44 deletions(-) create mode 100644 packages/plugins/file-secrets/src/index.test.ts diff --git a/packages/plugins/file-secrets/src/index.test.ts b/packages/plugins/file-secrets/src/index.test.ts new file mode 100644 index 000000000..7d5c8673d --- /dev/null +++ b/packages/plugins/file-secrets/src/index.test.ts @@ -0,0 +1,144 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "@effect/vitest"; +import { + existsSync, + mkdirSync, + mkdtempSync, + readFileSync, + rmSync, + statSync, + writeFileSync, +} from "node:fs"; +import { tmpdir } from "node:os"; +import { dirname, join } from "node:path"; +import { Effect } from "effect"; + +import { ProviderKey } from "@executor-js/sdk"; +import { makeTestWorkspaceHarness } from "@executor-js/sdk/testing"; + +import { fileSecretsPlugin } from "./index"; + +const FILE_PROVIDER = ProviderKey.make("file"); + +const inspectPlugin = (plugin: ReturnType) => + Effect.scoped( + Effect.gen(function* () { + const workspace = yield* makeTestWorkspaceHarness({ plugins: [plugin] as const }); + const items = yield* workspace.executor.providers.items(FILE_PROVIDER); + return { + filePath: workspace.executor.fileSecrets.filePath, + itemIds: items.map((item) => String(item.id)), + }; + }), + ); + +const writeAuthFile = (filePath: string, contents: string, mode = 0o600): void => { + mkdirSync(dirname(filePath), { recursive: true }); + writeFileSync(filePath, contents, { mode }); +}; + +describe("file secrets auth location", () => { + let workDir: string; + let dataDir: string; + let otherDataDir: string; + let xdgDataHome: string; + let overrideDir: string; + let legacyFilePath: string; + + beforeEach(() => { + workDir = mkdtempSync(join(tmpdir(), "executor-file-secrets-location-")); + dataDir = join(workDir, "data"); + otherDataDir = join(workDir, "other-data"); + xdgDataHome = join(workDir, "xdg"); + overrideDir = join(workDir, "override"); + legacyFilePath = join(xdgDataHome, "executor", "auth.json"); + vi.stubEnv("XDG_DATA_HOME", xdgDataHome); + }); + + afterEach(() => { + vi.unstubAllEnvs(); + rmSync(workDir, { recursive: true, force: true }); + }); + + it.effect("uses auth.json directly under EXECUTOR_DATA_DIR resolved at construction", () => + Effect.gen(function* () { + vi.stubEnv("EXECUTOR_DATA_DIR", dataDir); + const plugin = fileSecretsPlugin(); + vi.stubEnv("EXECUTOR_DATA_DIR", otherDataDir); + + const inspected = yield* inspectPlugin(plugin); + + expect(inspected.filePath).toBe(join(dataDir, "auth.json")); + }), + ); + + it.effect("keeps the XDG location when EXECUTOR_DATA_DIR is unset", () => + Effect.gen(function* () { + vi.stubEnv("EXECUTOR_DATA_DIR", ""); + + const inspected = yield* inspectPlugin(fileSecretsPlugin()); + + expect(inspected.filePath).toBe(legacyFilePath); + }), + ); + + it.effect("gives an explicit directory precedence without migration", () => + Effect.gen(function* () { + vi.stubEnv("EXECUTOR_DATA_DIR", dataDir); + writeAuthFile(legacyFilePath, '{"legacy":"legacy-secret"}'); + + const inspected = yield* inspectPlugin(fileSecretsPlugin({ directory: overrideDir })); + + expect(inspected.filePath).toBe(join(overrideDir, "auth.json")); + expect(inspected.itemIds).toEqual([]); + expect(existsSync(join(dataDir, "auth.json"))).toBe(false); + expect(readFileSync(legacyFilePath, "utf8")).toBe('{"legacy":"legacy-secret"}'); + }), + ); + + it.effect("copies a valid legacy XDG file once and preserves 0600 permissions", () => + Effect.gen(function* () { + vi.stubEnv("EXECUTOR_DATA_DIR", dataDir); + const legacyContents = '{"legacy-token":"legacy-secret"}'; + writeAuthFile(legacyFilePath, legacyContents, 0o644); + + const inspected = yield* inspectPlugin(fileSecretsPlugin()); + const migratedFilePath = join(dataDir, "auth.json"); + + expect(inspected.filePath).toBe(migratedFilePath); + expect(inspected.itemIds).toEqual(["legacy-token"]); + expect(readFileSync(migratedFilePath, "utf8")).toContain('"legacy-token": "legacy-secret"'); + expect(statSync(migratedFilePath).mode & 0o777).toBe(0o600); + expect(readFileSync(legacyFilePath, "utf8")).toBe(legacyContents); + }), + ); + + it.effect("uses an existing data-dir file without merging the legacy file", () => + Effect.gen(function* () { + vi.stubEnv("EXECUTOR_DATA_DIR", dataDir); + const activeFilePath = join(dataDir, "auth.json"); + const activeContents = '{"active-token":"active-secret"}'; + const legacyContents = '{"legacy-token":"legacy-secret"}'; + writeAuthFile(activeFilePath, activeContents); + writeAuthFile(legacyFilePath, legacyContents); + + const inspected = yield* inspectPlugin(fileSecretsPlugin()); + + expect(inspected.itemIds).toEqual(["active-token"]); + expect(readFileSync(activeFilePath, "utf8")).toBe(activeContents); + expect(readFileSync(legacyFilePath, "utf8")).toBe(legacyContents); + }), + ); + + it.effect("leaves the new store empty when the legacy file is corrupt", () => + Effect.gen(function* () { + vi.stubEnv("EXECUTOR_DATA_DIR", dataDir); + writeAuthFile(legacyFilePath, "not-json"); + + const inspected = yield* inspectPlugin(fileSecretsPlugin()); + + expect(inspected.itemIds).toEqual([]); + expect(existsSync(join(dataDir, "auth.json"))).toBe(false); + expect(readFileSync(legacyFilePath, "utf8")).toBe("not-json"); + }), + ); +}); diff --git a/packages/plugins/file-secrets/src/index.ts b/packages/plugins/file-secrets/src/index.ts index de157063d..fb7f2ba9d 100644 --- a/packages/plugins/file-secrets/src/index.ts +++ b/packages/plugins/file-secrets/src/index.ts @@ -11,10 +11,11 @@ import { } from "@executor-js/sdk"; // --------------------------------------------------------------------------- -// XDG data dir resolution +// Auth file location // --------------------------------------------------------------------------- const APP_NAME = "executor"; +const AUTH_FILE_NAME = "auth.json"; export const xdgDataHome = (): string => { if (process.env.XDG_DATA_HOME?.trim()) return process.env.XDG_DATA_HOME.trim(); @@ -28,9 +29,34 @@ export const xdgDataHome = (): string => { return path.join(process.env.HOME || "~", ".local", "share"); }; -const authDir = (overrideDir?: string): string => overrideDir ?? path.join(xdgDataHome(), APP_NAME); +interface AuthLocation { + readonly filePath: string; + readonly legacyFilePath: string | null; +} + +const legacyAuthFilePath = (): string => path.join(xdgDataHome(), APP_NAME, AUTH_FILE_NAME); + +const resolveAuthLocation = (config: FileSecretsPluginConfig | undefined): AuthLocation => { + if (config?.directory !== undefined) { + return { + filePath: path.join(config.directory, AUTH_FILE_NAME), + legacyFilePath: null, + }; + } -const authFilePath = (overrideDir?: string): string => path.join(authDir(overrideDir), "auth.json"); + const dataDir = process.env.EXECUTOR_DATA_DIR?.trim(); + if (dataDir) { + return { + filePath: path.join(dataDir, AUTH_FILE_NAME), + legacyFilePath: legacyAuthFilePath(), + }; + } + + return { + filePath: legacyAuthFilePath(), + legacyFilePath: null, + }; +}; // --------------------------------------------------------------------------- // Schema for the auth file @@ -96,7 +122,10 @@ const writeAll = ( }); } yield* Effect.try({ - try: () => fs.writeFileSync(tmp, JSON.stringify(secrets, null, 2), { mode: 0o600 }), + try: () => { + fs.writeFileSync(tmp, JSON.stringify(secrets, null, 2), { mode: 0o600 }); + fs.chmodSync(tmp, 0o600); + }, catch: toStorageError("Failed to write temporary auth file"), }); yield* Effect.try({ @@ -106,12 +135,30 @@ const writeAll = ( }); }; +const migrateLegacyAuthFile = ({ + filePath, + legacyFilePath, +}: AuthLocation): Effect.Effect => { + if (legacyFilePath === null || fs.existsSync(filePath) || !fs.existsSync(legacyFilePath)) { + return Effect.void; + } + + return readAll(legacyFilePath).pipe( + Effect.matchEffect({ + // A legacy file that cannot be read or decoded must not block startup. + // The active data-dir store remains empty and all later I/O uses it only. + onFailure: () => Effect.void, + onSuccess: (secrets) => writeAll(filePath, secrets), + }), + ); +}; + // --------------------------------------------------------------------------- // Plugin config // --------------------------------------------------------------------------- export interface FileSecretsPluginConfig { - /** Override the directory for auth.json (default: XDG data dir) */ + /** Override the directory for auth.json (default: EXECUTOR_DATA_DIR, then XDG data dir) */ readonly directory?: string; } @@ -119,8 +166,8 @@ export interface FileSecretsPluginConfig { // Plugin extension — public API on executor.fileSecrets // --------------------------------------------------------------------------- -const makeFileSecretsExtension = (options: FileSecretsPluginConfig | undefined) => ({ - filePath: resolveFilePath(options), +const makeFileSecretsExtension = (filePath: string) => ({ + filePath, }); export type FileSecretsExtension = ReturnType; @@ -135,54 +182,85 @@ export type FileSecretsExtension = ReturnType; const FILE_PROVIDER_KEY = ProviderKey.make("file"); -const makeFileProvider = (filePath: string): CredentialProvider => ({ - key: FILE_PROVIDER_KEY, - writable: true, +const makeFileProvider = (location: AuthLocation): CredentialProvider => { + let migrationComplete = false; + const ensureMigration = Effect.suspend(() => { + if (migrationComplete) return Effect.void; + return migrateLegacyAuthFile(location).pipe( + Effect.tap(() => + Effect.sync(() => { + migrationComplete = true; + }), + ), + ); + }); - get: (id: ProviderItemId) => readAll(filePath).pipe(Effect.map((data) => data[id] ?? null)), + return { + key: FILE_PROVIDER_KEY, + writable: true, - has: (id: ProviderItemId) => readAll(filePath).pipe(Effect.map((data) => id in data)), + get: (id: ProviderItemId) => + ensureMigration.pipe( + Effect.andThen(Effect.suspend(() => readAll(location.filePath))), + Effect.map((data) => data[id] ?? null), + ), - set: (id: ProviderItemId, value: string) => - Effect.gen(function* () { - const data = yield* readAll(filePath); - data[id] = value; - yield* writeAll(filePath, data); - }), + has: (id: ProviderItemId) => + ensureMigration.pipe( + Effect.andThen(Effect.suspend(() => readAll(location.filePath))), + Effect.map((data) => id in data), + ), - delete: (id: ProviderItemId) => - Effect.gen(function* () { - const data = yield* readAll(filePath); - if (id in data) { - delete data[id]; - yield* writeAll(filePath, data); - } - }), + set: (id: ProviderItemId, value: string) => + ensureMigration.pipe( + Effect.andThen( + Effect.gen(function* () { + const data = yield* readAll(location.filePath); + data[id] = value; + yield* writeAll(location.filePath, data); + }), + ), + ), - list: () => - readAll(filePath).pipe( - Effect.map((data) => Object.keys(data).map((k) => ({ id: ProviderItemId.make(k), name: k }))), - ), -}); + delete: (id: ProviderItemId) => + ensureMigration.pipe( + Effect.andThen( + Effect.gen(function* () { + const data = yield* readAll(location.filePath); + if (id in data) { + delete data[id]; + yield* writeAll(location.filePath, data); + } + }), + ), + ), + + list: () => + ensureMigration.pipe( + Effect.andThen(Effect.suspend(() => readAll(location.filePath))), + Effect.map((data) => + Object.keys(data).map((k) => ({ id: ProviderItemId.make(k), name: k })), + ), + ), + }; +}; // --------------------------------------------------------------------------- // Plugin definition // -// Compute the file path identically in `extension` (for `filePath`) and -// `credentialProviders` (for the provider's read/write). Both are called once -// per createExecutor. +// Resolve the path once when the configured plugin is constructed. The provider +// performs the one-time migration before its first read or write. // --------------------------------------------------------------------------- -const resolveFilePath = (config: FileSecretsPluginConfig | undefined): string => - authFilePath(config?.directory); +export const fileSecretsPlugin = definePlugin((options?: FileSecretsPluginConfig) => { + const location = resolveAuthLocation(options); -export const fileSecretsPlugin = definePlugin((options?: FileSecretsPluginConfig) => ({ - id: "fileSecrets" as const, - storage: () => ({}), + return { + id: "fileSecrets" as const, + storage: () => ({}), - extension: () => makeFileSecretsExtension(options), + extension: () => makeFileSecretsExtension(location.filePath), - credentialProviders: (): readonly CredentialProvider[] => [ - makeFileProvider(resolveFilePath(options)), - ], -})); + credentialProviders: (): readonly CredentialProvider[] => [makeFileProvider(location)], + }; +}); From 5130fa41f187e6fd7ff51207264e73b70c051856 Mon Sep 17 00:00:00 2001 From: Rhys Sullivan <39114868+RhysSullivan@users.noreply.github.com> Date: Fri, 17 Jul 2026 22:16:23 -0700 Subject: [PATCH 3/4] Harden file secrets migration --- .../plugins/file-secrets/src/data-dir.test.ts | 7 +- .../plugins/file-secrets/src/index.test.ts | 82 +++++++++++++++++-- packages/plugins/file-secrets/src/index.ts | 71 ++++++++++++---- 3 files changed, 128 insertions(+), 32 deletions(-) diff --git a/packages/plugins/file-secrets/src/data-dir.test.ts b/packages/plugins/file-secrets/src/data-dir.test.ts index fd98d9e29..27e501393 100644 --- a/packages/plugins/file-secrets/src/data-dir.test.ts +++ b/packages/plugins/file-secrets/src/data-dir.test.ts @@ -1,7 +1,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "@effect/vitest"; import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync } from "node:fs"; import { tmpdir } from "node:os"; -import { isAbsolute, join, relative } from "node:path"; +import { join } from "node:path"; import { Effect } from "effect"; import { @@ -81,10 +81,7 @@ describe("file secrets data directory", () => { expect(connection.provider).toBe(ProviderKey.make("file")); const authPath = first.executor.fileSecrets.filePath; - const pathWithinDataDir = relative(dataDir, authPath); - if (pathWithinDataDir.startsWith("..") || isAbsolute(pathWithinDataDir)) { - expect(authPath).toBe(join(firstSandboxDataHome, "executor", "auth.json")); - } + expect(authPath).toBe(join(dataDir, "auth.json")); expect(existsSync(authPath)).toBe(true); expect(readFileSync(authPath, "utf8")).toContain( '"connection:org:durable-secrets:main:token": "secret-token"', diff --git a/packages/plugins/file-secrets/src/index.test.ts b/packages/plugins/file-secrets/src/index.test.ts index 7d5c8673d..d33f78431 100644 --- a/packages/plugins/file-secrets/src/index.test.ts +++ b/packages/plugins/file-secrets/src/index.test.ts @@ -4,13 +4,14 @@ import { mkdirSync, mkdtempSync, readFileSync, + readdirSync, rmSync, statSync, writeFileSync, } from "node:fs"; import { tmpdir } from "node:os"; import { dirname, join } from "node:path"; -import { Effect } from "effect"; +import { Effect, Predicate, Result } from "effect"; import { ProviderKey } from "@executor-js/sdk"; import { makeTestWorkspaceHarness } from "@executor-js/sdk/testing"; @@ -130,15 +131,78 @@ describe("file secrets auth location", () => { ); it.effect("leaves the new store empty when the legacy file is corrupt", () => - Effect.gen(function* () { - vi.stubEnv("EXECUTOR_DATA_DIR", dataDir); - writeAuthFile(legacyFilePath, "not-json"); + Effect.scoped( + Effect.gen(function* () { + vi.stubEnv("EXECUTOR_DATA_DIR", dataDir); + writeAuthFile(legacyFilePath, "not-json"); + const workspace = yield* makeTestWorkspaceHarness({ + plugins: [fileSecretsPlugin()] as const, + }); + + const initial = yield* workspace.executor.providers.items(FILE_PROVIDER); + expect(initial).toEqual([]); + expect(existsSync(join(dataDir, "auth.json"))).toBe(false); + expect(readFileSync(legacyFilePath, "utf8")).toBe("not-json"); + + writeAuthFile(legacyFilePath, '{"repaired-token":"repaired-secret"}'); + const afterRepair = yield* workspace.executor.providers.items(FILE_PROVIDER); + expect(afterRepair).toEqual([]); + expect(existsSync(join(dataDir, "auth.json"))).toBe(false); + }), + ), + ); - const inspected = yield* inspectPlugin(fileSecretsPlugin()); + it.effect("retries migration after a legacy read I/O failure", () => + Effect.scoped( + Effect.gen(function* () { + vi.stubEnv("EXECUTOR_DATA_DIR", dataDir); + mkdirSync(legacyFilePath, { recursive: true }); + const workspace = yield* makeTestWorkspaceHarness({ + plugins: [fileSecretsPlugin()] as const, + }); + + const failed = yield* Effect.result(workspace.executor.providers.items(FILE_PROVIDER)); + expect(Result.isFailure(failed)).toBe(true); + if (!Result.isFailure(failed)) return; + expect(Predicate.isTagged("StorageError")(failed.failure)).toBe(true); + + rmSync(legacyFilePath, { recursive: true, force: true }); + writeAuthFile(legacyFilePath, '{"recovered-token":"recovered-secret"}'); + + const recovered = yield* workspace.executor.providers.items(FILE_PROVIDER); + expect(recovered.map((item) => String(item.id))).toEqual(["recovered-token"]); + expect(readFileSync(join(dataDir, "auth.json"), "utf8")).toContain( + '"recovered-token": "recovered-secret"', + ); + }), + ), + ); - expect(inspected.itemIds).toEqual([]); - expect(existsSync(join(dataDir, "auth.json"))).toBe(false); - expect(readFileSync(legacyFilePath, "utf8")).toBe("not-json"); - }), + it.effect("shares one migration across concurrent first provider operations", () => + Effect.scoped( + Effect.gen(function* () { + vi.stubEnv("EXECUTOR_DATA_DIR", dataDir); + const legacyContents = '{"legacy-token":"legacy-secret"}'; + writeAuthFile(legacyFilePath, legacyContents); + const workspace = yield* makeTestWorkspaceHarness({ + plugins: [fileSecretsPlugin()] as const, + }); + + const results = yield* Effect.all( + [ + workspace.executor.providers.items(FILE_PROVIDER), + workspace.executor.providers.items(FILE_PROVIDER), + ], + { concurrency: "unbounded" }, + ); + + expect(results.map((items) => items.map((item) => String(item.id)))).toEqual([ + ["legacy-token"], + ["legacy-token"], + ]); + expect(readdirSync(dataDir)).toEqual(["auth.json"]); + expect(readFileSync(legacyFilePath, "utf8")).toBe(legacyContents); + }), + ), ); }); diff --git a/packages/plugins/file-secrets/src/index.ts b/packages/plugins/file-secrets/src/index.ts index fb7f2ba9d..7b3015286 100644 --- a/packages/plugins/file-secrets/src/index.ts +++ b/packages/plugins/file-secrets/src/index.ts @@ -1,6 +1,7 @@ -import { Effect, Schema } from "effect"; +import { randomUUID } from "node:crypto"; import * as fs from "node:fs"; import * as path from "node:path"; +import { Deferred, Effect, Exit, Schema } from "effect"; import { definePlugin, @@ -71,13 +72,29 @@ const resolveAuthLocation = (config: FileSecretsPluginConfig | undefined): AuthL const FlatAuthFile = Schema.Record(Schema.String, Schema.String); const decodeFlatAuthFile = Schema.decodeUnknownEffect(Schema.fromJsonString(FlatAuthFile)); +class AuthFileDecodeError extends Schema.TaggedErrorClass()( + "AuthFileDecodeError", + { + filePath: Schema.String, + cause: Schema.Defect, + }, + { + description: + "The auth file was readable but its contents were not a valid flat credential map. The caller must surface the failure or explicitly treat malformed legacy data as non-migratable.", + }, +) { + override get message(): string { + return `Failed to parse auth file: ${this.filePath}`; + } +} + // --------------------------------------------------------------------------- // File I/O with restricted permissions // -// These helpers keep real I/O and decode failures in the Effect error -// channel as `StorageError`. Missing files are still treated as an empty -// auth file, but malformed JSON, schema decode failures, and permission -// errors no longer collapse into "empty file". +// These helpers keep real I/O and decode failures distinct internally, then +// project both to `StorageError` at the provider boundary. Missing files are +// still treated as an empty auth file; malformed content and permission errors +// do not collapse into "empty file" during ordinary provider reads. // --------------------------------------------------------------------------- const isFileNotFoundCause = (cause: unknown): cause is NodeJS.ErrnoException => @@ -88,7 +105,9 @@ const toStorageError = (cause: unknown): StorageError => new StorageError({ message, cause }); -const readAll = (filePath: string): Effect.Effect, StorageError> => { +const readAuthFile = ( + filePath: string, +): Effect.Effect, StorageError | AuthFileDecodeError> => { if (!fs.existsSync(filePath)) return Effect.succeed({}); return Effect.try({ try: () => fs.readFileSync(filePath, "utf-8"), @@ -102,18 +121,25 @@ const readAll = (filePath: string): Effect.Effect, Storag raw === "" ? Effect.succeed>({}) : decodeFlatAuthFile(raw).pipe( - Effect.mapError(toStorageError("Failed to parse auth file")), + Effect.mapError((cause) => new AuthFileDecodeError({ filePath, cause })), ), ), ); }; +const readAll = (filePath: string): Effect.Effect, StorageError> => + readAuthFile(filePath).pipe( + Effect.catchTag("AuthFileDecodeError", (cause) => + Effect.fail(new StorageError({ message: cause.message, cause })), + ), + ); + const writeAll = ( filePath: string, secrets: Record, ): Effect.Effect => { const dir = path.dirname(filePath); - const tmp = `${filePath}.tmp`; + const tmp = `${filePath}.${process.pid}.${randomUUID()}.tmp`; return Effect.gen(function* () { if (!fs.existsSync(dir)) { yield* Effect.try({ @@ -124,6 +150,8 @@ const writeAll = ( yield* Effect.try({ try: () => { fs.writeFileSync(tmp, JSON.stringify(secrets, null, 2), { mode: 0o600 }); + // `mode` only applies when the file is created; chmod afterwards covers + // the case where a temporary file with broader permissions already exists. fs.chmodSync(tmp, 0o600); }, catch: toStorageError("Failed to write temporary auth file"), @@ -143,13 +171,11 @@ const migrateLegacyAuthFile = ({ return Effect.void; } - return readAll(legacyFilePath).pipe( - Effect.matchEffect({ - // A legacy file that cannot be read or decoded must not block startup. - // The active data-dir store remains empty and all later I/O uses it only. - onFailure: () => Effect.void, - onSuccess: (secrets) => writeAll(filePath, secrets), - }), + return readAuthFile(legacyFilePath).pipe( + Effect.flatMap((secrets) => writeAll(filePath, secrets)), + // Malformed legacy contents are deliberately skipped. Genuine read I/O + // failures stay visible and leave migration eligible for a later retry. + Effect.catchTag("AuthFileDecodeError", () => Effect.void), ); }; @@ -184,12 +210,21 @@ const FILE_PROVIDER_KEY = ProviderKey.make("file"); const makeFileProvider = (location: AuthLocation): CredentialProvider => { let migrationComplete = false; + let migrationInFlight: Deferred.Deferred | null = null; const ensureMigration = Effect.suspend(() => { if (migrationComplete) return Effect.void; + if (migrationInFlight !== null) return Deferred.await(migrationInFlight); + + // Installing the latch inside Effect.suspend is synchronous, so a peer + // fiber can only observe either no attempt or this fully initialized one. + const latch = Deferred.makeUnsafe(); + migrationInFlight = latch; return migrateLegacyAuthFile(location).pipe( - Effect.tap(() => - Effect.sync(() => { - migrationComplete = true; + Effect.onExit((exit) => + Effect.gen(function* () { + if (Exit.isSuccess(exit)) migrationComplete = true; + yield* Deferred.done(latch, exit); + migrationInFlight = null; }), ), ); From 5eca314abe6c5e5bae1af11cac4e2824b864f7a5 Mon Sep 17 00:00:00 2001 From: Rhys Sullivan <39114868+RhysSullivan@users.noreply.github.com> Date: Fri, 17 Jul 2026 22:58:23 -0700 Subject: [PATCH 4/4] Add changeset --- .changeset/file-secrets-data-dir.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/file-secrets-data-dir.md diff --git a/.changeset/file-secrets-data-dir.md b/.changeset/file-secrets-data-dir.md new file mode 100644 index 000000000..d39a6dd70 --- /dev/null +++ b/.changeset/file-secrets-data-dir.md @@ -0,0 +1,5 @@ +--- +"@executor-js/plugin-file-secrets": patch +--- + +`fileSecretsPlugin()` now stores `auth.json` under `EXECUTOR_DATA_DIR` when that variable is set (an explicit `directory` option still wins; the XDG location remains the fallback when it is unset). Existing secrets in the legacy XDG location are migrated automatically on first use. This keeps all daemon state under one directory, so persisting `EXECUTOR_DATA_DIR` alone preserves credentials across environment recreation.