Skip to content

Commit 6d7a1f9

Browse files
lishaduckampagent
lishaduck
and
ampagent
authored
fix(grove): preserve URL environment values as strings (#388)
Amp-Thread-ID: https://ampcode.com/threads/T-019fe8a7-ff8e-73ac-8ca3-52b6a5d3cb64 Co-authored-by: Amp <amp@ampcode.com>
1 parent af39dd0 commit 6d7a1f9

3 files changed

Lines changed: 66 additions & 95 deletions

File tree

‎apps/grove/src/env.ts‎

Lines changed: 30 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -4,119 +4,89 @@ import { Schema } from "effect";
44

55
const authSecret = Schema.String.check(Schema.isMinLength(32));
66

7-
const parseUrl = (value: string) => {
8-
try {
9-
return new URL(value);
10-
} catch {
11-
return undefined;
12-
}
13-
};
14-
const safeUrl = (value: string, isDev: boolean, originOnly: boolean) => {
15-
const url = parseUrl(value);
16-
if (!url || url.username || url.password || url.search || url.hash) return false;
17-
if (originOnly && url.pathname !== "/") return false;
18-
if (url.protocol === "https:") return true;
19-
return (
20-
isDev &&
21-
url.protocol === "http:" &&
22-
(url.hostname === "localhost" || url.hostname === "127.0.0.1" || url.hostname === "[::1]")
23-
);
24-
};
25-
const authUrl = (isDev: boolean) =>
26-
Schema.NonEmptyString.check(
27-
Schema.makeFilter(
28-
(value) => safeUrl(value, isDev, true) || "expected a canonical HTTPS origin",
29-
),
30-
);
31-
const oidcIssuer = (isDev: boolean) =>
32-
Schema.NonEmptyString.check(
33-
Schema.makeFilter(
34-
(value) => safeUrl(value, isDev, false) || "expected a canonical HTTPS issuer",
35-
),
36-
);
37-
38-
export const authUrlSchema = (isBuilding: boolean, isDev: boolean) => {
39-
const schema = authUrl(isDev);
40-
return (isBuilding ? Schema.optional(schema) : schema) as typeof schema;
41-
};
7+
const safeUrl = Schema.NonEmptyString.check(
8+
Schema.makeFilter((value) => {
9+
if (!URL.canParse(value)) return false;
4210

43-
export const oidcIssuerSchema = (isBuilding: boolean, isDev: boolean) => {
44-
const schema = oidcIssuer(isDev);
45-
return (isBuilding ? Schema.optional(schema) : schema) as typeof schema;
46-
};
11+
const url = new URL(value);
12+
if (url.username || url.password || url.search || url.hash) return false;
13+
if (url.protocol === "https:") return true;
14+
return (
15+
dev &&
16+
url.protocol === "http:" &&
17+
(url.hostname === "localhost" || url.hostname === "127.0.0.1" || url.hostname === "[::1]")
18+
);
19+
}),
20+
);
21+
const authUrl = safeUrl.check(
22+
Schema.makeFilter(
23+
(value) => URL.parse(value)?.pathname === "/" || "expected a canonical HTTPS origin",
24+
),
25+
);
4726

48-
export const mcpResourceSchema = (isBuilding: boolean, isDev: boolean) => {
49-
const schema = authUrl(isDev);
50-
return (isBuilding ? Schema.optional(schema) : schema) as typeof schema;
51-
};
27+
export const authUrlSchema = building ? Schema.optional(authUrl) : authUrl;
28+
export const oidcIssuerSchema = building ? Schema.optional(safeUrl) : safeUrl;
29+
export const mcpResourceSchema = building ? Schema.optional(authUrl) : authUrl;
5230

5331
export const variables = defineEnvVars({
5432
BETTER_AUTH_SECRET: {
5533
public: false,
5634
static: false,
5735
description: "Secret used to sign and encrypt Grove authentication state.",
58-
schema: Schema.toStandardSchemaV1(
59-
(building ? Schema.optional(authSecret) : authSecret) as typeof authSecret,
60-
),
36+
schema: Schema.toStandardSchemaV1(building ? Schema.optional(authSecret) : authSecret),
6137
},
6238
BETTER_AUTH_URL: {
6339
public: false,
6440
static: false,
6541
description: "Canonical public URL used for Grove authentication callbacks.",
66-
schema: Schema.toStandardSchemaV1(authUrlSchema(building, dev)),
42+
schema: Schema.toStandardSchemaV1(authUrlSchema),
6743
},
6844
DATABASE_URL: {
6945
public: false,
7046
static: false,
7147
description: "PostgreSQL connection URL for Grove's durable state.",
7248
schema: Schema.toStandardSchemaV1(
73-
(building
74-
? Schema.optional(Schema.NonEmptyString)
75-
: Schema.NonEmptyString) as typeof Schema.NonEmptyString,
49+
building ? Schema.optional(Schema.NonEmptyString) : Schema.NonEmptyString,
7650
),
7751
},
7852
GROVE_OIDC_CLIENT_ID: {
7953
public: false,
8054
static: false,
8155
description: "OIDC client ID used only for Grove browser login.",
8256
schema: Schema.toStandardSchemaV1(
83-
(building
84-
? Schema.optional(Schema.NonEmptyString)
85-
: Schema.NonEmptyString) as typeof Schema.NonEmptyString,
57+
building ? Schema.optional(Schema.NonEmptyString) : Schema.NonEmptyString,
8658
),
8759
},
8860
GROVE_OIDC_CLIENT_SECRET: {
8961
public: false,
9062
static: false,
9163
description: "OIDC client secret used only for Grove browser login.",
9264
schema: Schema.toStandardSchemaV1(
93-
(building
94-
? Schema.optional(Schema.NonEmptyString)
95-
: Schema.NonEmptyString) as typeof Schema.NonEmptyString,
65+
building ? Schema.optional(Schema.NonEmptyString) : Schema.NonEmptyString,
9666
),
9767
},
9868
GROVE_OIDC_ISSUER: {
9969
public: false,
10070
static: false,
10171
description: "Pinned OIDC issuer used for Grove browser login and discovery.",
102-
schema: Schema.toStandardSchemaV1(oidcIssuerSchema(building, dev)),
72+
schema: Schema.toStandardSchemaV1(oidcIssuerSchema),
10373
},
10474
GROVE_MCP_ISSUER: {
10575
public: false,
10676
static: false,
10777
description: "Pinned authorization-server issuer for MCP bearer tokens.",
108-
schema: Schema.toStandardSchemaV1(oidcIssuerSchema(building, dev)),
78+
schema: Schema.toStandardSchemaV1(oidcIssuerSchema),
10979
},
11080
GROVE_MCP_JWKS_URL: {
11181
public: false,
11282
static: false,
11383
description: "JWKS endpoint for validating MCP bearer tokens.",
114-
schema: Schema.toStandardSchemaV1(oidcIssuerSchema(building, dev)),
84+
schema: Schema.toStandardSchemaV1(oidcIssuerSchema),
11585
},
11686
GROVE_MCP_RESOURCE: {
11787
public: false,
11888
static: false,
11989
description: "Canonical origin of Grove's MCP protected resource.",
120-
schema: Schema.toStandardSchemaV1(mcpResourceSchema(building, dev)),
90+
schema: Schema.toStandardSchemaV1(mcpResourceSchema),
12191
},
12292
});

‎apps/grove/src/lib/server/runtime.ts‎

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -15,16 +15,20 @@ import { AuthenticationRequired } from "./authorization";
1515
import { SproutDatabaseError, SproutNotFound, SproutService } from "./sprouts/service";
1616

1717
function makeRuntime() {
18-
const GroveServicesLayer = building
19-
? Layer.merge(GroveAuthBuildLayer, SproutServiceBuildLayer)
20-
: Layer.merge(GroveAuthLayer, SproutServiceLayer).pipe(
21-
Layer.provide(
22-
PgClient.layer({
23-
url: Redacted.make(DATABASE_URL),
24-
maxConnections: 5,
25-
}),
26-
),
27-
);
18+
let GroveServicesLayer;
19+
if (building) {
20+
GroveServicesLayer = Layer.merge(GroveAuthBuildLayer, SproutServiceBuildLayer);
21+
} else {
22+
if (!DATABASE_URL) throw new Error("DATABASE_URL is required at runtime");
23+
GroveServicesLayer = Layer.merge(GroveAuthLayer, SproutServiceLayer).pipe(
24+
Layer.provide(
25+
PgClient.layer({
26+
url: Redacted.make(DATABASE_URL),
27+
maxConnections: 5,
28+
}),
29+
),
30+
);
31+
}
2832

2933
return makeEffectSvelteKitRuntime(Layer.orDie(GroveServicesLayer), {
3034
mapFailure: (failure) => {

‎apps/grove/test/env.test.ts‎

Lines changed: 22 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { building, dev } from "$app/env";
12
import { Exit, Schema } from "effect";
23
import { describe, expect, it } from "vitest";
34

@@ -7,38 +8,34 @@ const accepts = (schema: Schema.ConstraintDecoder<unknown>, value: unknown) =>
78
Exit.isSuccess(Schema.decodeUnknownExit(schema)(value));
89

910
describe("Grove environment", () => {
10-
it("requires a canonical HTTPS auth URL in production", () => {
11-
const production = authUrlSchema(false, false);
11+
it("requires a canonical auth URL", () => {
12+
const value = "https://grove.example.com";
1213

13-
expect(accepts(production, undefined)).toBe(false);
14-
expect(accepts(production, "http://grove.example.com")).toBe(false);
15-
expect(accepts(production, "https://grove.example.com")).toBe(true);
16-
expect(accepts(production, "https://grove.example.com/path")).toBe(false);
17-
expect(accepts(production, "https://user@grove.example.com")).toBe(false);
18-
expect(accepts(production, "https://grove.example.com?callback=evil")).toBe(false);
14+
expect(accepts(authUrlSchema, undefined)).toBe(building);
15+
expect(accepts(authUrlSchema, "http://grove.example.com")).toBe(false);
16+
expect(Schema.decodeUnknownSync(authUrlSchema)(value)).toBe(value);
17+
expect(accepts(authUrlSchema, "https://grove.example.com/path")).toBe(false);
18+
expect(accepts(authUrlSchema, "https://user@grove.example.com")).toBe(false);
19+
expect(accepts(authUrlSchema, "https://grove.example.com?callback=evil")).toBe(false);
20+
expect(accepts(authUrlSchema, "not a URL")).toBe(false);
1921
});
2022

21-
it("allows an optional HTTP auth URL in local development and build", () => {
22-
expect(accepts(authUrlSchema(false, true), "http://localhost:5173")).toBe(true);
23-
expect(accepts(authUrlSchema(false, true), "http://example.com")).toBe(false);
24-
expect(accepts(authUrlSchema(true, false), undefined)).toBe(true);
23+
it("only allows HTTP on localhost in development", () => {
24+
expect(accepts(authUrlSchema, "http://localhost:5173")).toBe(dev);
25+
expect(accepts(authUrlSchema, "http://example.com")).toBe(false);
2526
});
2627

27-
it("requires an HTTPS OIDC issuer outside local development", () => {
28-
expect(accepts(oidcIssuerSchema(false, false), "http://identity.example.com")).toBe(false);
29-
expect(
30-
accepts(oidcIssuerSchema(false, false), "https://identity.example.com/realm/grove"),
31-
).toBe(true);
32-
expect(accepts(oidcIssuerSchema(false, true), "http://localhost:8080/realms/grove")).toBe(true);
33-
expect(
34-
accepts(oidcIssuerSchema(false, false), "https://identity.example.com/realm/grove#other"),
35-
).toBe(false);
28+
it("requires a safe OIDC issuer", () => {
29+
expect(accepts(oidcIssuerSchema, "http://identity.example.com")).toBe(false);
30+
expect(accepts(oidcIssuerSchema, "https://identity.example.com/realm/grove")).toBe(true);
31+
expect(accepts(oidcIssuerSchema, "http://localhost:8080/realms/grove")).toBe(dev);
32+
expect(accepts(oidcIssuerSchema, "https://identity.example.com/realm/grove#other")).toBe(false);
3633
});
3734

3835
it("requires an HTTPS origin for the canonical MCP resource", () => {
39-
expect(accepts(mcpResourceSchema(false, false), "https://grove.example")).toBe(true);
40-
expect(accepts(mcpResourceSchema(false, false), "http://grove.example")).toBe(false);
41-
expect(accepts(mcpResourceSchema(false, false), "https://grove.example/mcp")).toBe(false);
42-
expect(accepts(mcpResourceSchema(false, true), "http://localhost:5173")).toBe(true);
36+
expect(accepts(mcpResourceSchema, "https://grove.example")).toBe(true);
37+
expect(accepts(mcpResourceSchema, "http://grove.example")).toBe(false);
38+
expect(accepts(mcpResourceSchema, "https://grove.example/mcp")).toBe(false);
39+
expect(accepts(mcpResourceSchema, "http://localhost:5173")).toBe(dev);
4340
});
4441
});

0 commit comments

Comments
 (0)