Skip to content

Commit 13917df

Browse files
cursor[bot]cursoragentjuliusmarmingecodex
authored
Use idiomatic Effect options for server secret reads (#3110)
Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Julius Marminge <juliusmarminge@users.noreply.github.com> Co-authored-by: Julius Marminge <julius0216@outlook.com> Co-authored-by: codex <codex@users.noreply.github.com>
1 parent 5d4e2fa commit 13917df

13 files changed

Lines changed: 148 additions & 124 deletions

‎apps/server/src/auth/ServerSecretStore.test.ts‎

Lines changed: 23 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
11
import * as NodeServices from "@effect/platform-node/NodeServices";
2-
import { expect, it } from "@effect/vitest";
2+
import { assert, it } from "@effect/vitest";
33
import * as Cause from "effect/Cause";
44
import * as Deferred from "effect/Deferred";
55
import * as Effect from "effect/Effect";
66
import * as FileSystem from "effect/FileSystem";
77
import * as Layer from "effect/Layer";
8+
import * as Option from "effect/Option";
89
import * as Ref from "effect/Ref";
910
import * as PlatformError from "effect/PlatformError";
1011

@@ -145,13 +146,13 @@ const makeConcurrentCreateSecretStoreLayer = () =>
145146
);
146147

147148
it.layer(NodeServices.layer)("ServerSecretStore.layer", (it) => {
148-
it.effect("returns null when a secret file does not exist", () =>
149+
it.effect("returns Option.none when a secret file does not exist", () =>
149150
Effect.gen(function* () {
150151
const secretStore = yield* ServerSecretStore.ServerSecretStore;
151152

152153
const secret = yield* secretStore.get("missing-secret");
153154

154-
expect(secret).toBeNull();
155+
assert.isTrue(Option.isNone(secret));
155156
}).pipe(Effect.provide(makeServerSecretStoreLayer())),
156157
);
157158

@@ -162,7 +163,7 @@ it.layer(NodeServices.layer)("ServerSecretStore.layer", (it) => {
162163
const first = yield* secretStore.getOrCreateRandom("session-signing-key", 32);
163164
const second = yield* secretStore.getOrCreateRandom("session-signing-key", 32);
164165

165-
expect(Array.from(second)).toEqual(Array.from(first));
166+
assert.deepEqual(Array.from(second), Array.from(first));
166167
}).pipe(Effect.provide(makeServerSecretStoreLayer())),
167168
);
168169

@@ -178,10 +179,10 @@ it.layer(NodeServices.layer)("ServerSecretStore.layer", (it) => {
178179
{ concurrency: "unbounded" },
179180
);
180181
const persisted = yield* secretStore.get("session-signing-key");
182+
const persistedBytes = Option.getOrThrow(persisted);
181183

182-
expect(persisted).not.toBeNull();
183-
expect(Array.from(first)).toEqual(Array.from(persisted ?? new Uint8Array()));
184-
expect(Array.from(second)).toEqual(Array.from(persisted ?? new Uint8Array()));
184+
assert.deepEqual(Array.from(first), Array.from(persistedBytes));
185+
assert.deepEqual(Array.from(second), Array.from(persistedBytes));
185186
}).pipe(Effect.provide(makeConcurrentCreateSecretStoreLayer())),
186187
);
187188

@@ -217,10 +218,10 @@ it.layer(NodeServices.layer)("ServerSecretStore.layer", (it) => {
217218

218219
yield* secretStore.set("session-signing-key", Uint8Array.from([1, 2, 3]));
219220

220-
expect(chmodCalls.some((call) => call.mode === 0o700 && call.path.endsWith("/secrets"))).toBe(
221-
true,
221+
assert.isTrue(
222+
chmodCalls.some((call) => call.mode === 0o700 && call.path.endsWith("/secrets")),
222223
);
223-
expect(chmodCalls.filter((call) => call.mode === 0o600).length).toBeGreaterThanOrEqual(2);
224+
assert.isAtLeast(chmodCalls.filter((call) => call.mode === 0o600).length, 2);
224225
}).pipe(Effect.provide(NodeServices.layer)),
225226
);
226227

@@ -230,10 +231,10 @@ it.layer(NodeServices.layer)("ServerSecretStore.layer", (it) => {
230231

231232
const error = yield* Effect.flip(secretStore.getOrCreateRandom("session-signing-key", 32));
232233

233-
expect(error).toBeInstanceOf(ServerSecretStore.SecretStoreError);
234-
expect(error.message).toContain("Failed to read secret session-signing-key.");
235-
expect(error.cause).toBeInstanceOf(PlatformError.PlatformError);
236-
expect((error.cause as PlatformError.PlatformError).reason._tag).toBe("PermissionDenied");
234+
assert.instanceOf(error, ServerSecretStore.SecretStoreError);
235+
assert.include(error.message, "Failed to read secret session-signing-key.");
236+
assert.instanceOf(error.cause, PlatformError.PlatformError);
237+
assert.equal((error.cause as PlatformError.PlatformError).reason._tag, "PermissionDenied");
237238
}).pipe(Effect.provide(makePermissionDeniedSecretStoreLayer())),
238239
);
239240

@@ -245,10 +246,10 @@ it.layer(NodeServices.layer)("ServerSecretStore.layer", (it) => {
245246
secretStore.set("session-signing-key", Uint8Array.from([1, 2, 3])),
246247
);
247248

248-
expect(error).toBeInstanceOf(ServerSecretStore.SecretStoreError);
249-
expect(error.message).toContain("Failed to persist secret session-signing-key.");
250-
expect(error.cause).toBeInstanceOf(PlatformError.PlatformError);
251-
expect((error.cause as PlatformError.PlatformError).reason._tag).toBe("PermissionDenied");
249+
assert.instanceOf(error, ServerSecretStore.SecretStoreError);
250+
assert.include(error.message, "Failed to persist secret session-signing-key.");
251+
assert.instanceOf(error.cause, PlatformError.PlatformError);
252+
assert.equal((error.cause as PlatformError.PlatformError).reason._tag, "PermissionDenied");
252253
}).pipe(Effect.provide(makeRenameFailureSecretStoreLayer())),
253254
);
254255

@@ -258,10 +259,10 @@ it.layer(NodeServices.layer)("ServerSecretStore.layer", (it) => {
258259

259260
const error = yield* Effect.flip(secretStore.remove("session-signing-key"));
260261

261-
expect(error).toBeInstanceOf(ServerSecretStore.SecretStoreError);
262-
expect(error.message).toContain("Failed to remove secret session-signing-key.");
263-
expect(error.cause).toBeInstanceOf(PlatformError.PlatformError);
264-
expect((error.cause as PlatformError.PlatformError).reason._tag).toBe("PermissionDenied");
262+
assert.instanceOf(error, ServerSecretStore.SecretStoreError);
263+
assert.include(error.message, "Failed to remove secret session-signing-key.");
264+
assert.instanceOf(error.cause, PlatformError.PlatformError);
265+
assert.equal((error.cause as PlatformError.PlatformError).reason._tag, "PermissionDenied");
265266
}).pipe(Effect.provide(makeRemoveFailureSecretStoreLayer())),
266267
);
267268
});

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

Lines changed: 47 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,23 @@
11
import * as Context from "effect/Context";
22
import * as Crypto from "effect/Crypto";
3-
import * as Data from "effect/Data";
43
import * as Effect from "effect/Effect";
54
import * as FileSystem from "effect/FileSystem";
65
import * as Layer from "effect/Layer";
6+
import * as Option from "effect/Option";
77
import * as Path from "effect/Path";
88
import * as Predicate from "effect/Predicate";
99
import * as PlatformError from "effect/PlatformError";
10+
import * as Schema from "effect/Schema";
1011

1112
import { ServerConfig } from "../config.ts";
1213

13-
export class SecretStoreError extends Data.TaggedError("SecretStoreError")<{
14-
readonly message: string;
15-
readonly cause?: unknown;
16-
}> {}
14+
export class SecretStoreError extends Schema.TaggedErrorClass<SecretStoreError>()(
15+
"SecretStoreError",
16+
{
17+
message: Schema.String,
18+
cause: Schema.optional(Schema.Defect()),
19+
},
20+
) {}
1721

1822
const isPlatformError = (value: unknown): value is PlatformError.PlatformError =>
1923
Predicate.isTagged(value, "PlatformError");
@@ -22,7 +26,7 @@ export const isSecretAlreadyExistsError = (error: SecretStoreError): boolean =>
2226
isPlatformError(error.cause) && error.cause.reason._tag === "AlreadyExists";
2327

2428
export interface ServerSecretStoreShape {
25-
readonly get: (name: string) => Effect.Effect<Uint8Array | null, SecretStoreError>;
29+
readonly get: (name: string) => Effect.Effect<Option.Option<Uint8Array>, SecretStoreError>;
2630
readonly set: (name: string, value: Uint8Array) => Effect.Effect<void, SecretStoreError>;
2731
readonly create: (name: string, value: Uint8Array) => Effect.Effect<void, SecretStoreError>;
2832
readonly getOrCreateRandom: (
@@ -57,10 +61,10 @@ export const make = Effect.fn("makeServerSecretStore")(function* () {
5761

5862
const get: ServerSecretStoreShape["get"] = (name) =>
5963
fileSystem.readFile(resolveSecretPath(name)).pipe(
60-
Effect.map((bytes) => Uint8Array.from(bytes)),
64+
Effect.map((bytes) => Option.some(Uint8Array.from(bytes))),
6165
Effect.catch((cause) =>
6266
cause.reason._tag === "NotFound"
63-
? Effect.succeed(null)
67+
? Effect.succeed(Option.none())
6468
: Effect.fail(
6569
new SecretStoreError({
6670
message: `Failed to read secret ${name}.`,
@@ -133,41 +137,43 @@ export const make = Effect.fn("makeServerSecretStore")(function* () {
133137

134138
const getOrCreateRandom: ServerSecretStoreShape["getOrCreateRandom"] = (name, bytes) =>
135139
get(name).pipe(
136-
Effect.flatMap((existing) => {
137-
if (existing) {
138-
return Effect.succeed(existing);
139-
}
140-
141-
return crypto.randomBytes(bytes).pipe(
142-
Effect.mapError(
143-
(cause) =>
144-
new SecretStoreError({
145-
message: `Failed to generate random bytes for secret ${name}.`,
146-
cause,
147-
}),
148-
),
149-
Effect.flatMap((generated) =>
150-
create(name, generated).pipe(
151-
Effect.as(Uint8Array.from(generated)),
152-
Effect.catchTag("SecretStoreError", (error) =>
153-
isSecretAlreadyExistsError(error)
154-
? get(name).pipe(
155-
Effect.flatMap((created) =>
156-
created !== null
157-
? Effect.succeed(created)
158-
: Effect.fail(
159-
new SecretStoreError({
160-
message: `Failed to read secret ${name} after concurrent creation.`,
161-
}),
162-
),
163-
),
164-
)
165-
: Effect.fail(error),
140+
Effect.flatMap(
141+
Option.match({
142+
onSome: Effect.succeed,
143+
onNone: () =>
144+
crypto.randomBytes(bytes).pipe(
145+
Effect.mapError(
146+
(cause) =>
147+
new SecretStoreError({
148+
message: `Failed to generate random bytes for secret ${name}.`,
149+
cause,
150+
}),
151+
),
152+
Effect.flatMap((generated) =>
153+
create(name, generated).pipe(
154+
Effect.as(Uint8Array.from(generated)),
155+
Effect.catchTag("SecretStoreError", (error) =>
156+
isSecretAlreadyExistsError(error)
157+
? get(name).pipe(
158+
Effect.flatMap(
159+
Option.match({
160+
onSome: Effect.succeed,
161+
onNone: () =>
162+
Effect.fail(
163+
new SecretStoreError({
164+
message: `Failed to read secret ${name} after concurrent creation.`,
165+
}),
166+
),
167+
}),
168+
),
169+
)
170+
: Effect.fail(error),
171+
),
172+
),
166173
),
167174
),
168-
),
169-
);
170-
}),
175+
}),
176+
),
171177
Effect.withSpan("ServerSecretStore.getOrCreateRandom"),
172178
);
173179

‎apps/server/src/cli/connect.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -385,9 +385,9 @@ const connectStatusCommand = Command.make("status", {
385385
const status: CloudCliStatus = {
386386
desired,
387387
authenticated,
388-
linked: cloudUserId !== null,
389-
cloudUserId: cloudUserId ? bytesToString(cloudUserId) : null,
390-
relayUrl: relayUrl ? bytesToString(relayUrl) : null,
388+
linked: Option.isSome(cloudUserId),
389+
cloudUserId: Option.isSome(cloudUserId) ? bytesToString(cloudUserId.value) : null,
390+
relayUrl: Option.isSome(relayUrl) ? bytesToString(relayUrl.value) : null,
391391
relayClient: executable,
392392
};
393393
yield* Console.log(formatCloudStatus(status, { json: flags.json }));

‎apps/server/src/cloud/CliState.test.ts‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
import * as NodeServices from "@effect/platform-node/NodeServices";
2-
import { expect, it } from "@effect/vitest";
2+
import { assert, it } from "@effect/vitest";
33
import * as Effect from "effect/Effect";
44
import * as Layer from "effect/Layer";
5+
import * as Option from "effect/Option";
56

67
import * as ServerSecretStore from "../auth/ServerSecretStore.ts";
78
import { ServerConfig } from "../config.ts";
@@ -40,18 +41,18 @@ it.layer(NodeServices.layer)("CliState", (it) => {
4041
Effect.gen(function* () {
4142
const secrets = yield* ServerSecretStore.ServerSecretStore;
4243

43-
expect(yield* CliState.readCliDesiredCloudLink).toBe(false);
44+
assert.isFalse(yield* CliState.readCliDesiredCloudLink);
4445
yield* CliState.setCliDesiredCloudLink(true);
45-
expect(yield* CliState.readCliDesiredCloudLink).toBe(true);
46+
assert.isTrue(yield* CliState.readCliDesiredCloudLink);
4647

4748
for (const name of persistedCloudLinkSecrets) {
4849
yield* secrets.set(name, new TextEncoder().encode(name));
4950
}
5051
yield* CliState.clearPersistedCloudLink;
5152

52-
expect(yield* CliState.readCliDesiredCloudLink).toBe(false);
53+
assert.isFalse(yield* CliState.readCliDesiredCloudLink);
5354
for (const name of persistedCloudLinkSecrets) {
54-
expect(yield* secrets.get(name)).toBe(null);
55+
assert.isTrue(Option.isNone(yield* secrets.get(name)));
5556
}
5657
}).pipe(Effect.provide(makeTestLayer())),
5758
);

‎apps/server/src/cloud/CliState.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import * as Effect from "effect/Effect";
2+
import * as Option from "effect/Option";
23

34
import * as ServerSecretStore from "../auth/ServerSecretStore.ts";
45
import {
@@ -17,7 +18,7 @@ const TRUE_BYTES = new TextEncoder().encode("true");
1718

1819
export const readCliDesiredCloudLink = Effect.gen(function* () {
1920
const secrets = yield* ServerSecretStore.ServerSecretStore;
20-
return (yield* secrets.get(CLOUD_CLI_DESIRED_LINK_SECRET)) !== null;
21+
return Option.isSome(yield* secrets.get(CLOUD_CLI_DESIRED_LINK_SECRET));
2122
});
2223

2324
export const setCliDesiredCloudLink = Effect.fn("cloud.cli_state.set_desired")(function* (

‎apps/server/src/cloud/CliTokenManager.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -100,8 +100,8 @@ const make = Effect.gen(function* () {
100100

101101
const read = Effect.fn("cloud.cli_token.read")(function* () {
102102
const encoded = yield* secrets.get(CLOUD_CLI_OAUTH_TOKEN_SECRET);
103-
if (!encoded) return Option.none<PersistedToken>();
104-
return Option.some(yield* decodePersistedToken(bytesToString(encoded)));
103+
if (Option.isNone(encoded)) return Option.none<PersistedToken>();
104+
return Option.some(yield* decodePersistedToken(bytesToString(encoded.value)));
105105
});
106106

107107
const exchangeToken = Effect.fn("cloud.cli_token.exchange")(function* (

‎apps/server/src/cloud/ManagedEndpointRuntime.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,10 @@ function bytesToString(bytes: Uint8Array): string {
2222
const readRuntimeConfig = Effect.gen(function* () {
2323
const secrets = yield* ServerSecretStore.ServerSecretStore;
2424
const bytes = yield* secrets.get(CLOUD_ENDPOINT_RUNTIME_CONFIG);
25-
if (!bytes) {
25+
if (Option.isNone(bytes)) {
2626
return null;
2727
}
28-
return Option.getOrNull(decodeRuntimeConfig(bytesToString(bytes)));
28+
return Option.getOrNull(decodeRuntimeConfig(bytesToString(bytes.value)));
2929
});
3030

3131
export interface CloudManagedEndpointRuntimeShape {

‎apps/server/src/cloud/environmentKeys.test.ts‎

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
import * as NodeServices from "@effect/platform-node/NodeServices";
2-
import { expect, it } from "@effect/vitest";
2+
import { assert, it } from "@effect/vitest";
33
import * as Effect from "effect/Effect";
44
import * as Layer from "effect/Layer";
5+
import * as Option from "effect/Option";
56
import * as PlatformError from "effect/PlatformError";
67

78
import * as ServerSecretStore from "../auth/ServerSecretStore.ts";
@@ -23,10 +24,10 @@ it.layer(NodeServices.layer)("getOrCreateEnvironmentKeyPairFromSecretStore", (it
2324
const first = yield* getOrCreateEnvironmentKeyPairFromSecretStore(secretStore);
2425
const second = yield* getOrCreateEnvironmentKeyPairFromSecretStore(secretStore);
2526

26-
expect(second).toEqual(first);
27-
expect(yield* secretStore.get("cloud-link-ed25519-key-pair")).not.toBeNull();
28-
expect(yield* secretStore.get("cloud-link-ed25519-private-key")).toBeNull();
29-
expect(yield* secretStore.get("cloud-link-ed25519-public-key")).toBeNull();
27+
assert.deepEqual(second, first);
28+
assert.isTrue(Option.isSome(yield* secretStore.get("cloud-link-ed25519-key-pair")));
29+
assert.isTrue(Option.isNone(yield* secretStore.get("cloud-link-ed25519-private-key")));
30+
assert.isTrue(Option.isNone(yield* secretStore.get("cloud-link-ed25519-public-key")));
3031
}).pipe(Effect.provide(makeServerSecretStoreLayer())),
3132
);
3233

@@ -36,11 +37,11 @@ it.layer(NodeServices.layer)("getOrCreateEnvironmentKeyPairFromSecretStore", (it
3637
yield* secretStore.set("cloud-link-ed25519-private-key", new TextEncoder().encode("private"));
3738
yield* secretStore.set("cloud-link-ed25519-public-key", new TextEncoder().encode("public"));
3839

39-
expect(yield* getOrCreateEnvironmentKeyPairFromSecretStore(secretStore)).toEqual({
40+
assert.deepEqual(yield* getOrCreateEnvironmentKeyPairFromSecretStore(secretStore), {
4041
privateKey: "private",
4142
publicKey: "public",
4243
});
43-
expect(yield* secretStore.get("cloud-link-ed25519-key-pair")).not.toBeNull();
44+
assert.isTrue(Option.isSome(yield* secretStore.get("cloud-link-ed25519-key-pair")));
4445
}).pipe(Effect.provide(makeServerSecretStoreLayer())),
4546
);
4647

@@ -53,7 +54,9 @@ it.layer(NodeServices.layer)("getOrCreateEnvironmentKeyPairFromSecretStore", (it
5354
const secretStore = {
5455
get: (name) =>
5556
Effect.sync(() =>
56-
name === "cloud-link-ed25519-key-pair" && createAttempted ? winner : null,
57+
name === "cloud-link-ed25519-key-pair" && createAttempted
58+
? Option.some(winner)
59+
: Option.none(),
5760
),
5861
set: unusedSecretStoreOperation,
5962
create: () =>
@@ -78,7 +81,7 @@ it.layer(NodeServices.layer)("getOrCreateEnvironmentKeyPairFromSecretStore", (it
7881
remove: unusedSecretStoreOperation,
7982
} satisfies ServerSecretStore.ServerSecretStoreShape;
8083

81-
expect(yield* getOrCreateEnvironmentKeyPairFromSecretStore(secretStore)).toEqual({
84+
assert.deepEqual(yield* getOrCreateEnvironmentKeyPairFromSecretStore(secretStore), {
8285
privateKey: "winner-private",
8386
publicKey: "winner-public",
8487
});

0 commit comments

Comments
 (0)