From e3004ae806d4e9a81e03ff919f50d2d34c37ffe7 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Fri, 10 Apr 2026 13:09:39 -0700 Subject: [PATCH] Harden secret store and resolve catalog overrides (#1891) Co-authored-by: codex --- apps/server/scripts/cli.ts | 26 ++++++++- .../src/auth/Layers/ServerSecretStore.ts | 15 ++--- apps/server/src/git/Layers/GitCore.ts | 2 +- apps/server/src/git/Layers/GitHubCli.ts | 4 +- .../src/provider/Layers/CodexAdapter.test.ts | 7 --- .../src/provider/Layers/CodexAdapter.ts | 15 ++--- .../src/provider/Layers/CodexProvider.ts | 7 +-- apps/server/src/provider/providerSnapshot.ts | 3 +- apps/server/src/terminal/Layers/Manager.ts | 55 ++++++++++--------- .../src/workspace/Layers/WorkspaceEntries.ts | 5 +- apps/server/src/ws.ts | 5 +- apps/web/src/environments/primary/auth.ts | 36 ++++++------ apps/web/src/environments/primary/context.ts | 8 +-- package.json | 6 ++ scripts/build-desktop-artifact.ts | 6 +- scripts/lib/resolve-catalog.ts | 6 +- 16 files changed, 105 insertions(+), 101 deletions(-) diff --git a/apps/server/scripts/cli.ts b/apps/server/scripts/cli.ts index fb8c9fee6faffb85da83f4b45b49d6849273ed9c..299da67faba31b3de2fdced59ee20b0b7bf8869a 100644 --- a/apps/server/scripts/cli.ts +++ b/apps/server/scripts/cli.ts @@ -14,6 +14,22 @@ import { resolveCatalogDependencies } from "../../../scripts/lib/resolve-catalog import rootPackageJson from "../../../package.json" with { type: "json" }; import serverPackageJson from "../package.json" with { type: "json" }; +interface PackageJson { + name: string; + repository: { + type: string; + url: string; + directory: string; + }; + bin: Record; + type: string; + version: string; + engines: Record; + files: string[]; + dependencies: Record; + overrides: Record; +} + class CliError extends Data.TaggedError("CliError")<{ readonly message: string; readonly cause?: unknown; @@ -192,7 +208,7 @@ const publishCmd = Command.make( // Resolve catalog dependencies before any file mutations. If this throws, // acquire fails and no release hook runs, so filesystem must still be untouched. const version = Option.getOrElse(config.appVersion, () => serverPackageJson.version); - const pkg = { + const pkg: PackageJson = { name: serverPackageJson.name, repository: serverPackageJson.repository, bin: serverPackageJson.bin, @@ -200,7 +216,8 @@ const publishCmd = Command.make( version, engines: serverPackageJson.engines, files: serverPackageJson.files, - dependencies: serverPackageJson.dependencies as Record, + dependencies: serverPackageJson.dependencies, + overrides: rootPackageJson.overrides, }; pkg.dependencies = resolveCatalogDependencies( @@ -208,6 +225,11 @@ const publishCmd = Command.make( rootPackageJson.workspaces.catalog, "apps/server dependencies", ); + pkg.overrides = resolveCatalogDependencies( + pkg.overrides, + rootPackageJson.workspaces.catalog, + "root overrides", + ); const original = yield* fs.readFileString(packageJsonPath); yield* fs.writeFileString(backupPath, original); diff --git a/apps/server/src/auth/Layers/ServerSecretStore.ts b/apps/server/src/auth/Layers/ServerSecretStore.ts index a106d15fd59bc7860997108801553729add4ff97..c8acf11babef73f6da45933af4cf1a03c35f9d3b 100644 --- a/apps/server/src/auth/Layers/ServerSecretStore.ts +++ b/apps/server/src/auth/Layers/ServerSecretStore.ts @@ -1,6 +1,6 @@ import * as Crypto from "node:crypto"; -import { Effect, FileSystem, Layer, Path } from "effect"; +import { Effect, FileSystem, Layer, Path, Predicate } from "effect"; import * as PlatformError from "effect/PlatformError"; import { ServerConfig } from "../../config.ts"; @@ -28,17 +28,14 @@ export const makeServerSecretStore = Effect.gen(function* () { const resolveSecretPath = (name: string) => path.join(serverConfig.secretsDir, `${name}.bin`); - const isMissingSecretFileError = (cause: unknown): cause is PlatformError.PlatformError => - cause instanceof PlatformError.PlatformError && cause.reason._tag === "NotFound"; - - const isAlreadyExistsSecretFileError = (cause: unknown): cause is PlatformError.PlatformError => - cause instanceof PlatformError.PlatformError && cause.reason._tag === "AlreadyExists"; + const isPlatformError = (u: unknown): u is PlatformError.PlatformError => + Predicate.isTagged(u, "PlatformError"); const get: ServerSecretStoreShape["get"] = (name) => fileSystem.readFile(resolveSecretPath(name)).pipe( Effect.map((bytes) => Uint8Array.from(bytes)), Effect.catch((cause) => - isMissingSecretFileError(cause) + cause.reason._tag === "NotFound" ? Effect.succeed(null) : Effect.fail( new SecretStoreError({ @@ -108,7 +105,7 @@ export const makeServerSecretStore = Effect.gen(function* () { return create(name, generated).pipe( Effect.as(Uint8Array.from(generated)), Effect.catchTag("SecretStoreError", (error) => - isAlreadyExistsSecretFileError(error.cause) + isPlatformError(error.cause) && error.cause.reason._tag === "AlreadyExists" ? get(name).pipe( Effect.flatMap((created) => created !== null @@ -129,7 +126,7 @@ export const makeServerSecretStore = Effect.gen(function* () { const remove: ServerSecretStoreShape["remove"] = (name) => fileSystem.remove(resolveSecretPath(name)).pipe( Effect.catch((cause) => - isMissingSecretFileError(cause) + cause.reason._tag === "NotFound" ? Effect.void : Effect.fail( new SecretStoreError({ diff --git a/apps/server/src/git/Layers/GitCore.ts b/apps/server/src/git/Layers/GitCore.ts index e5547377b416caab2b7c4a024e6c53941ed9c6f8..62f8405aea31b1ed95245321f80d96c234b60b13 100644 --- a/apps/server/src/git/Layers/GitCore.ts +++ b/apps/server/src/git/Layers/GitCore.ts @@ -2001,7 +2001,7 @@ export const makeGitCore = Effect.fn("makeGitCore")(function* (options?: { "GitCore.removeWorktree", input.cwd, args, - `${commandLabel(args)} failed (cwd: ${input.cwd}): ${error instanceof Error ? error.message : String(error)}`, + `${commandLabel(args)} failed (cwd: ${input.cwd}): ${error.message}`, error, ), ), diff --git a/apps/server/src/git/Layers/GitHubCli.ts b/apps/server/src/git/Layers/GitHubCli.ts index 280679e337cb172c99fe28db86fb312bb1af02f4..ce56e91d53155fb24703a65317c94f804db3205f 100644 --- a/apps/server/src/git/Layers/GitHubCli.ts +++ b/apps/server/src/git/Layers/GitHubCli.ts @@ -1,4 +1,4 @@ -import { Effect, Layer, Schema } from "effect"; +import { Effect, Layer, Schema, SchemaIssue } from "effect"; import { PositiveInt, TrimmedNonEmptyString } from "@t3tools/contracts"; import { runProcess } from "../../processRunner"; @@ -154,7 +154,7 @@ function decodeGitHubJson( (error) => new GitHubCliError({ operation, - detail: error instanceof Error ? `${invalidDetail}: ${error.message}` : invalidDetail, + detail: `${invalidDetail}: ${SchemaIssue.makeFormatterDefault()(error.issue)}`, cause: error, }), ), diff --git a/apps/server/src/provider/Layers/CodexAdapter.test.ts b/apps/server/src/provider/Layers/CodexAdapter.test.ts index 3278aa2105495cb00a463a24fa2cfde9142342ec..c4ee33b7768c696cb211d1c158f665801ff8161f 100644 --- a/apps/server/src/provider/Layers/CodexAdapter.test.ts +++ b/apps/server/src/provider/Layers/CodexAdapter.test.ts @@ -238,14 +238,7 @@ sessionErrorLayer("CodexAdapterLive session errors", (it) => { .pipe(Effect.result); assert.equal(result._tag, "Failure"); - if (result._tag !== "Failure") { - return; - } - assert.equal(result.failure._tag, "ProviderAdapterSessionNotFoundError"); - if (result.failure._tag !== "ProviderAdapterSessionNotFoundError") { - return; - } assert.equal(result.failure.provider, "codex"); assert.equal(result.failure.threadId, "sess-missing"); assert.equal(result.failure.cause instanceof Error, true); diff --git a/apps/server/src/provider/Layers/CodexAdapter.ts b/apps/server/src/provider/Layers/CodexAdapter.ts index cae34eaaf01fa7974bea07a054d3438f6c889fae..60de91b79ac47e301bcd3e1c8c15ba1df4b136b8 100644 --- a/apps/server/src/provider/Layers/CodexAdapter.ts +++ b/apps/server/src/provider/Layers/CodexAdapter.ts @@ -51,18 +51,11 @@ export interface CodexAdapterLiveOptions { readonly nativeEventLogger?: EventNdjsonLogger; } -function toMessage(cause: unknown, fallback: string): string { - if (cause instanceof Error && cause.message.length > 0) { - return cause.message; - } - return fallback; -} - function toSessionError( threadId: ThreadId, cause: unknown, ): ProviderAdapterSessionNotFoundError | ProviderAdapterSessionClosedError | undefined { - const normalized = toMessage(cause, "").toLowerCase(); + const normalized = cause instanceof Error ? cause.message.toLowerCase() : ""; if (normalized.includes("unknown session") || normalized.includes("unknown provider session")) { return new ProviderAdapterSessionNotFoundError({ provider: PROVIDER, @@ -88,7 +81,7 @@ function toRequestError(threadId: ThreadId, method: string, cause: unknown): Pro return new ProviderAdapterRequestError({ provider: PROVIDER, method, - detail: toMessage(cause, `${method} failed`), + detail: cause instanceof Error ? `${method} failed: ${cause.message}` : `${method} failed`, cause, }); } @@ -1427,7 +1420,7 @@ const makeCodexAdapter = Effect.fn("makeCodexAdapter")(function* ( new ProviderAdapterProcessError({ provider: PROVIDER, threadId: input.threadId, - detail: toMessage(cause, "Failed to start Codex adapter session."), + detail: `Failed to start Codex adapter session: ${cause instanceof Error ? cause.message : String(cause)}.`, cause, }), }); @@ -1455,7 +1448,7 @@ const makeCodexAdapter = Effect.fn("makeCodexAdapter")(function* ( new ProviderAdapterRequestError({ provider: PROVIDER, method: "turn/start", - detail: toMessage(cause, "Failed to read attachment file."), + detail: `Failed to read attachment file: ${cause.message}.`, cause, }), ), diff --git a/apps/server/src/provider/Layers/CodexProvider.ts b/apps/server/src/provider/Layers/CodexProvider.ts index 3509fa9257543f15f5621483d29ece12770b2c89..085abe6fe2345d48406cfe631e0226204c56ec19 100644 --- a/apps/server/src/provider/Layers/CodexProvider.ts +++ b/apps/server/src/provider/Layers/CodexProvider.ts @@ -389,7 +389,7 @@ export const checkCodexProviderStatus = Effect.fn("checkCodexProviderStatus")(fu auth: { status: "unknown" }, message: isCommandMissingCause(error) ? "Codex CLI (`codex`) is not installed or not on PATH." - : `Failed to execute Codex CLI health check: ${error instanceof Error ? error.message : String(error)}.`, + : `Failed to execute Codex CLI health check: ${error.message}.`, }, }); } @@ -489,10 +489,7 @@ export const checkCodexProviderStatus = Effect.fn("checkCodexProviderStatus")(fu version: parsedVersion, status: "warning", auth: { status: "unknown" }, - message: - error instanceof Error - ? `Could not verify Codex authentication status: ${error.message}.` - : "Could not verify Codex authentication status.", + message: `Could not verify Codex authentication status: ${error.message}.`, }, }); } diff --git a/apps/server/src/provider/providerSnapshot.ts b/apps/server/src/provider/providerSnapshot.ts index 4c80d78e2081e2308505e0a35911fc4d9a364274..232d2d35826e5b7a07bfeeb852510ce31036f919 100644 --- a/apps/server/src/provider/providerSnapshot.ts +++ b/apps/server/src/provider/providerSnapshot.ts @@ -32,8 +32,7 @@ export function nonEmptyTrimmed(value: string | undefined): string | undefined { return trimmed.length > 0 ? trimmed : undefined; } -export function isCommandMissingCause(error: unknown): boolean { - if (!(error instanceof Error)) return false; +export function isCommandMissingCause(error: Error): boolean { const lower = error.message.toLowerCase(); return lower.includes("enoent") || lower.includes("notfound"); } diff --git a/apps/server/src/terminal/Layers/Manager.ts b/apps/server/src/terminal/Layers/Manager.ts index c6a16b0b496996530ee6d7eb1d90a0ef2ee4cd7f..4bdeba68e16b7ac8df7a08054ce8a7ab3abbcf63 100644 --- a/apps/server/src/terminal/Layers/Manager.ts +++ b/apps/server/src/terminal/Layers/Manager.ts @@ -8,7 +8,6 @@ import { } from "@t3tools/contracts"; import { makeKeyedCoalescingWorker } from "@t3tools/shared/KeyedCoalescingWorker"; import { - Data, Effect, Encoding, Equal, @@ -17,6 +16,7 @@ import { FileSystem, Layer, Option, + Schema, Scope, Semaphore, SynchronizedRef, @@ -54,22 +54,28 @@ const DEFAULT_OPEN_COLS = 120; const DEFAULT_OPEN_ROWS = 30; const TERMINAL_ENV_BLOCKLIST = new Set(["PORT", "ELECTRON_RENDERER_PORT", "ELECTRON_RUN_AS_NODE"]); -type TerminalSubprocessChecker = ( - terminalPid: number, -) => Effect.Effect; +class TerminalSubprocessCheckError extends Schema.TaggedErrorClass()( + "TerminalSubprocessCheckError", + { + message: Schema.String, + cause: Schema.optional(Schema.Defect), + terminalPid: Schema.Number, + command: Schema.Literals(["powershell", "pgrep", "ps"]), + }, +) {} -class TerminalSubprocessCheckError extends Data.TaggedError("TerminalSubprocessCheckError")<{ - readonly message: string; - readonly cause?: unknown; - readonly terminalPid: number; - readonly command: "powershell" | "pgrep" | "ps"; -}> {} +class TerminalProcessSignalError extends Schema.TaggedErrorClass()( + "TerminalProcessSignalError", + { + message: Schema.String, + cause: Schema.optional(Schema.Defect), + signal: Schema.Literals(["SIGTERM", "SIGKILL"]), + }, +) {} -class TerminalProcessSignalError extends Data.TaggedError("TerminalProcessSignalError")<{ - readonly message: string; - readonly cause?: unknown; - readonly signal: "SIGTERM" | "SIGKILL"; -}> {} +interface TerminalSubprocessChecker { + (terminalPid: number): Effect.Effect; +} interface ShellCandidate { shell: string; @@ -271,9 +277,8 @@ function isRetryableShellSpawnError(error: PtySpawnError): boolean { if (current instanceof Error) { messages.push(current.message); - const cause = (current as { cause?: unknown }).cause; - if (cause) { - queue.push(cause); + if (current.cause) { + queue.push(current.cause); } continue; } @@ -876,7 +881,7 @@ export const makeTerminalManagerWithOptions = Effect.fn("makeTerminalManagerWith Effect.logWarning("failed to persist terminal history", { threadId, terminalId, - error: error instanceof Error ? error.message : String(error), + error, }), ), ); @@ -959,7 +964,7 @@ export const makeTerminalManagerWithOptions = Effect.fn("makeTerminalManagerWith Effect.catch((cleanupError) => Effect.logWarning("failed to remove legacy terminal history", { threadId, - error: cleanupError instanceof Error ? cleanupError.message : String(cleanupError), + error: cleanupError, }), ), ); @@ -975,7 +980,7 @@ export const makeTerminalManagerWithOptions = Effect.fn("makeTerminalManagerWith Effect.logWarning("failed to delete terminal history", { threadId, terminalId, - error: error instanceof Error ? error.message : String(error), + error, }), ), ); @@ -985,7 +990,7 @@ export const makeTerminalManagerWithOptions = Effect.fn("makeTerminalManagerWith Effect.logWarning("failed to delete terminal history", { threadId, terminalId, - error: error instanceof Error ? error.message : String(error), + error, }), ), ); @@ -1011,7 +1016,7 @@ export const makeTerminalManagerWithOptions = Effect.fn("makeTerminalManagerWith Effect.catch((error) => Effect.logWarning("failed to delete terminal histories for thread", { threadId, - error: error instanceof Error ? error.message : String(error), + error, }), ), ), @@ -1463,12 +1468,12 @@ export const makeTerminalManagerWithOptions = Effect.fn("makeTerminalManagerWith const terminalPid = session.pid; const hasRunningSubprocess = yield* subprocessChecker(terminalPid).pipe( Effect.map(Option.some), - Effect.catch((error) => + Effect.catch((reason) => Effect.logWarning("failed to check terminal subprocess activity", { threadId: session.threadId, terminalId: session.terminalId, terminalPid, - error: error instanceof Error ? error.message : String(error), + reason, }).pipe(Effect.as(Option.none())), ), ); diff --git a/apps/server/src/workspace/Layers/WorkspaceEntries.ts b/apps/server/src/workspace/Layers/WorkspaceEntries.ts index 542e187457bc560a14ec772cda7ab467b504eeba..783333a49e756fa8bcca7ddc14c198f70dd953f5 100644 --- a/apps/server/src/workspace/Layers/WorkspaceEntries.ts +++ b/apps/server/src/workspace/Layers/WorkspaceEntries.ts @@ -214,9 +214,6 @@ function directoryAncestorsOf(relativePath: string): string[] { return directories; } -const processErrorDetail = (cause: unknown): string => - cause instanceof Error ? cause.message : String(cause); - export const makeWorkspaceEntries = Effect.gen(function* () { const path = yield* Path.Path; const gitOption = yield* Effect.serviceOption(GitCore); @@ -319,7 +316,7 @@ export const makeWorkspaceEntries = Effect.gen(function* () { new WorkspaceEntriesError({ cwd, operation: "workspaceEntries.readDirectoryEntries", - detail: processErrorDetail(cause), + detail: cause instanceof Error ? cause.message : String(cause), cause, }), }).pipe( diff --git a/apps/server/src/ws.ts b/apps/server/src/ws.ts index c7b1fc4804b8e58e8d26a16be3604720810ec637..3ef4a8646970fb0729e572ed1826ae3d9850c904 100644 --- a/apps/server/src/ws.ts +++ b/apps/server/src/ws.ts @@ -313,10 +313,7 @@ const makeWsRpcLayer = (currentSessionId: AuthSessionId) => worktreePath: input.worktreePath, scriptId: input.scriptId, terminalId: input.terminalId, - detail: - error instanceof Error - ? error.message - : "Unknown setup activity dispatch failure.", + detail: error.message, }, ), ), diff --git a/apps/web/src/environments/primary/auth.ts b/apps/web/src/environments/primary/auth.ts index b2b330ce75d437ea4fd1a4031ab2b1fbfdcf48c8..ae2d6bd64dc3a9169ea015569f7c9af4c70cf7c5 100644 --- a/apps/web/src/environments/primary/auth.ts +++ b/apps/web/src/environments/primary/auth.ts @@ -16,6 +16,14 @@ import { } from "../../pairingUrl"; import { resolvePrimaryEnvironmentHttpUrl } from "./target"; +import { Data, Predicate } from "effect"; + +export class BootstrapHttpError extends Data.TaggedError("BootstrapHttpError")<{ + readonly message: string; + readonly status: number; +}> {} +const isBootstrapHttpError = (u: unknown): u is BootstrapHttpError => + Predicate.isTagged(u, "BootstrapHttpError"); export interface ServerPairingLinkRecord { readonly id: string; @@ -87,10 +95,10 @@ export async function fetchSessionState(): Promise { credentials: "include", }); if (!response.ok) { - throw new BootstrapHttpError( - `Failed to load server auth session state (${response.status}).`, - response.status, - ); + throw new BootstrapHttpError({ + message: `Failed to load server auth session state (${response.status}).`, + status: response.status, + }); } return (await response.json()) as AuthSessionState; }); @@ -115,10 +123,10 @@ async function exchangeBootstrapCredential(credential: string): Promise(operation: () => Promise): Promise { const startedAt = Date.now(); while (true) { @@ -182,7 +180,7 @@ function waitForBootstrapRetry(delayMs: number): Promise { } function isTransientBootstrapError(error: unknown): boolean { - if (error instanceof BootstrapHttpError) { + if (isBootstrapHttpError(error)) { return TRANSIENT_BOOTSTRAP_STATUS_CODES.has(error.status); } diff --git a/apps/web/src/environments/primary/context.ts b/apps/web/src/environments/primary/context.ts index df8c5d1dbb9fff3c532dbb09b6533f088d40db4d..dea1683c4d0811bc772f7a6cff76cc7fa11e84bc 100644 --- a/apps/web/src/environments/primary/context.ts +++ b/apps/web/src/environments/primary/context.ts @@ -52,10 +52,10 @@ async function fetchPrimaryEnvironmentDescriptor(): Promise | undefined, - catalog: Record, -): Record { + dependencies: Record | undefined, + catalog: Record, +): Record { if (!dependencies || Object.keys(dependencies).length === 0) { return {}; } diff --git a/scripts/lib/resolve-catalog.ts b/scripts/lib/resolve-catalog.ts index 2946c4a5d9e1fbdc2674a1c5b7b2adc09806ad37..597bd06c24f3ab34a17e740ef36e800ce3b5761c 100644 --- a/scripts/lib/resolve-catalog.ts +++ b/scripts/lib/resolve-catalog.ts @@ -5,10 +5,10 @@ * the concrete version string found in `catalog`. Throws on missing entries. */ export function resolveCatalogDependencies( - dependencies: Record, - catalog: Record, + dependencies: Record, + catalog: Record, label: string, -): Record { +): Record { return Object.fromEntries( Object.entries(dependencies).map(([name, spec]) => { if (typeof spec !== "string" || !spec.startsWith("catalog:")) { -- 2.54.0