Skip to content

Commit 2533d79

Browse files
juliusmarmingecodex
andcommitted
Harden secret store and resolve catalog overrides (#1891)
Co-authored-by: codex <codex@users.noreply.github.com>
1 parent f899961 commit 2533d79

16 files changed

Lines changed: 106 additions & 102 deletions

File tree

apps/server/scripts/cli.ts

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,22 @@ import { resolveCatalogDependencies } from "../../../scripts/lib/resolve-catalog
1414
import rootPackageJson from "../../../package.json" with { type: "json" };
1515
import serverPackageJson from "../package.json" with { type: "json" };
1616

17+
interface PackageJson {
18+
name: string;
19+
repository: {
20+
type: string;
21+
url: string;
22+
directory: string;
23+
};
24+
bin: Record<string, string>;
25+
type: string;
26+
version: string;
27+
engines: Record<string, string>;
28+
files: string[];
29+
dependencies: Record<string, string>;
30+
overrides: Record<string, string>;
31+
}
32+
1733
class CliError extends Data.TaggedError("CliError")<{
1834
readonly message: string;
1935
readonly cause?: unknown;
@@ -192,22 +208,28 @@ const publishCmd = Command.make(
192208
// Resolve catalog dependencies before any file mutations. If this throws,
193209
// acquire fails and no release hook runs, so filesystem must still be untouched.
194210
const version = Option.getOrElse(config.appVersion, () => serverPackageJson.version);
195-
const pkg = {
211+
const pkg: PackageJson = {
196212
name: serverPackageJson.name,
197213
repository: serverPackageJson.repository,
198214
bin: serverPackageJson.bin,
199215
type: serverPackageJson.type,
200216
version,
201217
engines: serverPackageJson.engines,
202218
files: serverPackageJson.files,
203-
dependencies: serverPackageJson.dependencies as Record<string, unknown>,
219+
dependencies: serverPackageJson.dependencies,
220+
overrides: rootPackageJson.overrides,
204221
};
205222

206223
pkg.dependencies = resolveCatalogDependencies(
207224
pkg.dependencies,
208225
rootPackageJson.workspaces.catalog,
209226
"apps/server dependencies",
210227
);
228+
pkg.overrides = resolveCatalogDependencies(
229+
pkg.overrides,
230+
rootPackageJson.workspaces.catalog,
231+
"root overrides",
232+
);
211233

212234
const original = yield* fs.readFileString(packageJsonPath);
213235
yield* fs.writeFileString(backupPath, original);

apps/server/src/auth/Layers/ServerSecretStore.ts

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import * as Crypto from "node:crypto";
22

3-
import { Effect, FileSystem, Layer, Path } from "effect";
3+
import { Effect, FileSystem, Layer, Path, Predicate } from "effect";
44
import * as PlatformError from "effect/PlatformError";
55

66
import { ServerConfig } from "../../config.ts";
@@ -28,17 +28,14 @@ export const makeServerSecretStore = Effect.gen(function* () {
2828

2929
const resolveSecretPath = (name: string) => path.join(serverConfig.secretsDir, `${name}.bin`);
3030

31-
const isMissingSecretFileError = (cause: unknown): cause is PlatformError.PlatformError =>
32-
cause instanceof PlatformError.PlatformError && cause.reason._tag === "NotFound";
33-
34-
const isAlreadyExistsSecretFileError = (cause: unknown): cause is PlatformError.PlatformError =>
35-
cause instanceof PlatformError.PlatformError && cause.reason._tag === "AlreadyExists";
31+
const isPlatformError = (u: unknown): u is PlatformError.PlatformError =>
32+
Predicate.isTagged(u, "PlatformError");
3633

3734
const get: ServerSecretStoreShape["get"] = (name) =>
3835
fileSystem.readFile(resolveSecretPath(name)).pipe(
3936
Effect.map((bytes) => Uint8Array.from(bytes)),
4037
Effect.catch((cause) =>
41-
isMissingSecretFileError(cause)
38+
cause.reason._tag === "NotFound"
4239
? Effect.succeed(null)
4340
: Effect.fail(
4441
new SecretStoreError({
@@ -108,7 +105,7 @@ export const makeServerSecretStore = Effect.gen(function* () {
108105
return create(name, generated).pipe(
109106
Effect.as(Uint8Array.from(generated)),
110107
Effect.catchTag("SecretStoreError", (error) =>
111-
isAlreadyExistsSecretFileError(error.cause)
108+
isPlatformError(error.cause) && error.cause.reason._tag === "AlreadyExists"
112109
? get(name).pipe(
113110
Effect.flatMap((created) =>
114111
created !== null
@@ -129,7 +126,7 @@ export const makeServerSecretStore = Effect.gen(function* () {
129126
const remove: ServerSecretStoreShape["remove"] = (name) =>
130127
fileSystem.remove(resolveSecretPath(name)).pipe(
131128
Effect.catch((cause) =>
132-
isMissingSecretFileError(cause)
129+
cause.reason._tag === "NotFound"
133130
? Effect.void
134131
: Effect.fail(
135132
new SecretStoreError({

apps/server/src/git/Layers/GitCore.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2001,7 +2001,7 @@ export const makeGitCore = Effect.fn("makeGitCore")(function* (options?: {
20012001
"GitCore.removeWorktree",
20022002
input.cwd,
20032003
args,
2004-
`${commandLabel(args)} failed (cwd: ${input.cwd}): ${error instanceof Error ? error.message : String(error)}`,
2004+
`${commandLabel(args)} failed (cwd: ${input.cwd}): ${error.message}`,
20052005
error,
20062006
),
20072007
),

apps/server/src/git/Layers/GitHubCli.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { Effect, Layer, Schema } from "effect";
1+
import { Effect, Layer, Schema, SchemaIssue } from "effect";
22
import { PositiveInt, TrimmedNonEmptyString } from "@t3tools/contracts";
33

44
import { runProcess } from "../../processRunner";
@@ -154,7 +154,7 @@ function decodeGitHubJson<S extends Schema.Top>(
154154
(error) =>
155155
new GitHubCliError({
156156
operation,
157-
detail: error instanceof Error ? `${invalidDetail}: ${error.message}` : invalidDetail,
157+
detail: `${invalidDetail}: ${SchemaIssue.makeFormatterDefault()(error.issue)}`,
158158
cause: error,
159159
}),
160160
),

apps/server/src/provider/Layers/CodexAdapter.test.ts

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -238,14 +238,7 @@ sessionErrorLayer("CodexAdapterLive session errors", (it) => {
238238
.pipe(Effect.result);
239239

240240
assert.equal(result._tag, "Failure");
241-
if (result._tag !== "Failure") {
242-
return;
243-
}
244-
245241
assert.equal(result.failure._tag, "ProviderAdapterSessionNotFoundError");
246-
if (result.failure._tag !== "ProviderAdapterSessionNotFoundError") {
247-
return;
248-
}
249242
assert.equal(result.failure.provider, "codex");
250243
assert.equal(result.failure.threadId, "sess-missing");
251244
assert.equal(result.failure.cause instanceof Error, true);

apps/server/src/provider/Layers/CodexAdapter.ts

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -51,18 +51,11 @@ export interface CodexAdapterLiveOptions {
5151
readonly nativeEventLogger?: EventNdjsonLogger;
5252
}
5353

54-
function toMessage(cause: unknown, fallback: string): string {
55-
if (cause instanceof Error && cause.message.length > 0) {
56-
return cause.message;
57-
}
58-
return fallback;
59-
}
60-
6154
function toSessionError(
6255
threadId: ThreadId,
6356
cause: unknown,
6457
): ProviderAdapterSessionNotFoundError | ProviderAdapterSessionClosedError | undefined {
65-
const normalized = toMessage(cause, "").toLowerCase();
58+
const normalized = cause instanceof Error ? cause.message.toLowerCase() : "";
6659
if (normalized.includes("unknown session") || normalized.includes("unknown provider session")) {
6760
return new ProviderAdapterSessionNotFoundError({
6861
provider: PROVIDER,
@@ -88,7 +81,7 @@ function toRequestError(threadId: ThreadId, method: string, cause: unknown): Pro
8881
return new ProviderAdapterRequestError({
8982
provider: PROVIDER,
9083
method,
91-
detail: toMessage(cause, `${method} failed`),
84+
detail: cause instanceof Error ? `${method} failed: ${cause.message}` : `${method} failed`,
9285
cause,
9386
});
9487
}
@@ -1427,7 +1420,7 @@ const makeCodexAdapter = Effect.fn("makeCodexAdapter")(function* (
14271420
new ProviderAdapterProcessError({
14281421
provider: PROVIDER,
14291422
threadId: input.threadId,
1430-
detail: toMessage(cause, "Failed to start Codex adapter session."),
1423+
detail: `Failed to start Codex adapter session: ${cause instanceof Error ? cause.message : String(cause)}.`,
14311424
cause,
14321425
}),
14331426
});
@@ -1455,7 +1448,7 @@ const makeCodexAdapter = Effect.fn("makeCodexAdapter")(function* (
14551448
new ProviderAdapterRequestError({
14561449
provider: PROVIDER,
14571450
method: "turn/start",
1458-
detail: toMessage(cause, "Failed to read attachment file."),
1451+
detail: `Failed to read attachment file: ${cause.message}.`,
14591452
cause,
14601453
}),
14611454
),

apps/server/src/provider/Layers/CodexProvider.ts

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -389,7 +389,7 @@ export const checkCodexProviderStatus = Effect.fn("checkCodexProviderStatus")(fu
389389
auth: { status: "unknown" },
390390
message: isCommandMissingCause(error)
391391
? "Codex CLI (`codex`) is not installed or not on PATH."
392-
: `Failed to execute Codex CLI health check: ${error instanceof Error ? error.message : String(error)}.`,
392+
: `Failed to execute Codex CLI health check: ${error.message}.`,
393393
},
394394
});
395395
}
@@ -489,10 +489,7 @@ export const checkCodexProviderStatus = Effect.fn("checkCodexProviderStatus")(fu
489489
version: parsedVersion,
490490
status: "warning",
491491
auth: { status: "unknown" },
492-
message:
493-
error instanceof Error
494-
? `Could not verify Codex authentication status: ${error.message}.`
495-
: "Could not verify Codex authentication status.",
492+
message: `Could not verify Codex authentication status: ${error.message}.`,
496493
},
497494
});
498495
}

apps/server/src/provider/providerSnapshot.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,7 @@ export function nonEmptyTrimmed(value: string | undefined): string | undefined {
3232
return trimmed.length > 0 ? trimmed : undefined;
3333
}
3434

35-
export function isCommandMissingCause(error: unknown): boolean {
36-
if (!(error instanceof Error)) return false;
35+
export function isCommandMissingCause(error: Error): boolean {
3736
const lower = error.message.toLowerCase();
3837
return lower.includes("enoent") || lower.includes("notfound");
3938
}

apps/server/src/terminal/Layers/Manager.ts

Lines changed: 31 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import {
88
} from "@t3tools/contracts";
99
import { makeKeyedCoalescingWorker } from "@t3tools/shared/KeyedCoalescingWorker";
1010
import {
11-
Data,
1211
Effect,
1312
Encoding,
1413
Equal,
@@ -17,6 +16,7 @@ import {
1716
FileSystem,
1817
Layer,
1918
Option,
19+
Schema,
2020
Scope,
2121
Semaphore,
2222
SynchronizedRef,
@@ -54,22 +54,28 @@ const DEFAULT_OPEN_COLS = 120;
5454
const DEFAULT_OPEN_ROWS = 30;
5555
const TERMINAL_ENV_BLOCKLIST = new Set(["PORT", "ELECTRON_RENDERER_PORT", "ELECTRON_RUN_AS_NODE"]);
5656

57-
type TerminalSubprocessChecker = (
58-
terminalPid: number,
59-
) => Effect.Effect<boolean, TerminalSubprocessCheckError>;
60-
61-
class TerminalSubprocessCheckError extends Data.TaggedError("TerminalSubprocessCheckError")<{
62-
readonly message: string;
63-
readonly cause?: unknown;
64-
readonly terminalPid: number;
65-
readonly command: "powershell" | "pgrep" | "ps";
66-
}> {}
57+
class TerminalSubprocessCheckError extends Schema.TaggedErrorClass<TerminalSubprocessCheckError>()(
58+
"TerminalSubprocessCheckError",
59+
{
60+
message: Schema.String,
61+
cause: Schema.optional(Schema.Defect),
62+
terminalPid: Schema.Number,
63+
command: Schema.Literals(["powershell", "pgrep", "ps"]),
64+
},
65+
) {}
66+
67+
class TerminalProcessSignalError extends Schema.TaggedErrorClass<TerminalProcessSignalError>()(
68+
"TerminalProcessSignalError",
69+
{
70+
message: Schema.String,
71+
cause: Schema.optional(Schema.Defect),
72+
signal: Schema.Literals(["SIGTERM", "SIGKILL"]),
73+
},
74+
) {}
6775

68-
class TerminalProcessSignalError extends Data.TaggedError("TerminalProcessSignalError")<{
69-
readonly message: string;
70-
readonly cause?: unknown;
71-
readonly signal: "SIGTERM" | "SIGKILL";
72-
}> {}
76+
interface TerminalSubprocessChecker {
77+
(terminalPid: number): Effect.Effect<boolean, TerminalSubprocessCheckError>;
78+
}
7379

7480
interface ShellCandidate {
7581
shell: string;
@@ -271,9 +277,8 @@ function isRetryableShellSpawnError(error: PtySpawnError): boolean {
271277

272278
if (current instanceof Error) {
273279
messages.push(current.message);
274-
const cause = (current as { cause?: unknown }).cause;
275-
if (cause) {
276-
queue.push(cause);
280+
if (current.cause) {
281+
queue.push(current.cause);
277282
}
278283
continue;
279284
}
@@ -876,7 +881,7 @@ export const makeTerminalManagerWithOptions = Effect.fn("makeTerminalManagerWith
876881
Effect.logWarning("failed to persist terminal history", {
877882
threadId,
878883
terminalId,
879-
error: error instanceof Error ? error.message : String(error),
884+
error,
880885
}),
881886
),
882887
);
@@ -959,7 +964,7 @@ export const makeTerminalManagerWithOptions = Effect.fn("makeTerminalManagerWith
959964
Effect.catch((cleanupError) =>
960965
Effect.logWarning("failed to remove legacy terminal history", {
961966
threadId,
962-
error: cleanupError instanceof Error ? cleanupError.message : String(cleanupError),
967+
error: cleanupError,
963968
}),
964969
),
965970
);
@@ -975,7 +980,7 @@ export const makeTerminalManagerWithOptions = Effect.fn("makeTerminalManagerWith
975980
Effect.logWarning("failed to delete terminal history", {
976981
threadId,
977982
terminalId,
978-
error: error instanceof Error ? error.message : String(error),
983+
error,
979984
}),
980985
),
981986
);
@@ -985,7 +990,7 @@ export const makeTerminalManagerWithOptions = Effect.fn("makeTerminalManagerWith
985990
Effect.logWarning("failed to delete terminal history", {
986991
threadId,
987992
terminalId,
988-
error: error instanceof Error ? error.message : String(error),
993+
error,
989994
}),
990995
),
991996
);
@@ -1011,7 +1016,7 @@ export const makeTerminalManagerWithOptions = Effect.fn("makeTerminalManagerWith
10111016
Effect.catch((error) =>
10121017
Effect.logWarning("failed to delete terminal histories for thread", {
10131018
threadId,
1014-
error: error instanceof Error ? error.message : String(error),
1019+
error,
10151020
}),
10161021
),
10171022
),
@@ -1463,12 +1468,12 @@ export const makeTerminalManagerWithOptions = Effect.fn("makeTerminalManagerWith
14631468
const terminalPid = session.pid;
14641469
const hasRunningSubprocess = yield* subprocessChecker(terminalPid).pipe(
14651470
Effect.map(Option.some),
1466-
Effect.catch((error) =>
1471+
Effect.catch((reason) =>
14671472
Effect.logWarning("failed to check terminal subprocess activity", {
14681473
threadId: session.threadId,
14691474
terminalId: session.terminalId,
14701475
terminalPid,
1471-
error: error instanceof Error ? error.message : String(error),
1476+
reason,
14721477
}).pipe(Effect.as(Option.none<boolean>())),
14731478
),
14741479
);

apps/server/src/workspace/Layers/WorkspaceEntries.ts

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -214,9 +214,6 @@ function directoryAncestorsOf(relativePath: string): string[] {
214214
return directories;
215215
}
216216

217-
const processErrorDetail = (cause: unknown): string =>
218-
cause instanceof Error ? cause.message : String(cause);
219-
220217
export const makeWorkspaceEntries = Effect.gen(function* () {
221218
const path = yield* Path.Path;
222219
const gitOption = yield* Effect.serviceOption(GitCore);
@@ -319,7 +316,7 @@ export const makeWorkspaceEntries = Effect.gen(function* () {
319316
new WorkspaceEntriesError({
320317
cwd,
321318
operation: "workspaceEntries.readDirectoryEntries",
322-
detail: processErrorDetail(cause),
319+
detail: cause instanceof Error ? cause.message : String(cause),
323320
cause,
324321
}),
325322
}).pipe(

0 commit comments

Comments
 (0)