fix(integrations): require account admin for GitHub installs, explicit callback origins

apps/api/src/routes/internal/integrations.http.ts

@@ -117,7 +117,7 @@ export const HttpIntegrationsLive = HttpApiBuilder.group(MapleInternalApi, "inte
 		const trustedCallbackUrl = (path: string) =>
 			Effect.gen(function* () {
 				const origin = resolveRequestOrigin(yield* HttpServerRequest.HttpServerRequest)
-				if (!isTrustedCallbackOrigin(origin, env.MAPLE_APP_BASE_URL)) {
+				if (!isTrustedCallbackOrigin(origin, env)) {
 					return yield* new IntegrationsForbiddenError({
 						message: "Integrations cannot be connected from this host",
 					})

apps/api/src/routes/v2/integrations-chat.http.ts

@@ -118,7 +118,7 @@ export const HttpV2ChatIntegrationsLive = HttpApiBuilder.group(MapleApiV2, "chat
 						// replayed in the token exchange, and the origin is read from a header
 						// a client can set — so an untrusted one would mint an authorize URL
 						// pointing at a host the caller controls.
-						if (!isTrustedCallbackOrigin(origin, env.MAPLE_APP_BASE_URL)) {
+						if (!isTrustedCallbackOrigin(origin, env)) {
 							yield* Effect.logError("Rejected chat install: untrusted callback origin", {
 								origin,
 							})
@@ -152,7 +152,7 @@ export const HttpV2ChatIntegrationsLive = HttpApiBuilder.group(MapleApiV2, "chat
 						// Same reasoning as the install: the origin is read from a header a
 						// client can set, and it is persisted as the state's redirect URI and
 						// replayed in the token exchange.
-						if (!isTrustedCallbackOrigin(origin, env.MAPLE_APP_BASE_URL)) {
+						if (!isTrustedCallbackOrigin(origin, env)) {
 							yield* Effect.logError("Rejected chat account link: untrusted callback origin", {
 								origin,
 							})

apps/api/src/routes/v2/integrations-origin.test.ts

@@ -80,55 +80,73 @@ describe("resolveRequestOrigin (untrusted — reflects client-controlled headers
 })
 
 /**
- * The security boundary for an OAuth callback origin. The resolved origin
- * is embedded in the authorize URL, persisted as `oauth_auth_states.redirectUri`
- * and replayed as `redirect_uri` in the token exchange, so an attacker-chosen
- * host would mint an authorize URL pointing at their own callback. Only hosts in
- * the same family as the trusted `MAPLE_APP_BASE_URL` are accepted; everything
- * else fails closed (the install handler answers 503).
+ * The security boundary for an OAuth callback origin: the resolved origin is
+ * persisted and replayed as `redirect_uri`, so only the deploy's configured
+ * origins (plus its own preview hosts, or local hosts in development) pass.
  */
+const production = {
+	MAPLE_APP_BASE_URL: "https://app.maple.dev",
+	MAPLE_API_BASE_URL: "https://api.maple.dev",
+	MAPLE_ENVIRONMENT: "production",
+}
+const preview = {
+	MAPLE_APP_BASE_URL: "https://app-pr-12.maple.dev",
+	MAPLE_API_BASE_URL: "https://api-pr-12.maple.dev",
+	MAPLE_ENVIRONMENT: "pr-12",
+}
+const development = {
+	MAPLE_APP_BASE_URL: "https://web.localhost",
+	MAPLE_API_BASE_URL: "https://api.localhost",
+	MAPLE_ENVIRONMENT: "development",
+}
+
 describe("isTrustedCallbackOrigin", () => {
-	it("accepts sibling hosts under the same registrable domain, per stage", () => {
-		for (const [origin, appBaseUrl] of [
-			["https://api.maple.dev", "https://app.maple.dev"],
-			["https://api-pr-12.maple.dev", "https://app-pr-12.maple.dev"],
-			// A self-hosted install served over plain http keeps its own scheme.
-			["http://api.maple.internal", "http://app.maple.internal"],
+	it("accepts the configured app and API origins", () => {
+		for (const [origin, config] of [
+			["https://api.maple.dev", production],
+			["https://app.maple.dev", production],
+			["https://api.maple.dev/", production],
+			["http://api.maple.internal", { ...production, MAPLE_API_BASE_URL: "http://api.maple.internal" }],
 		] as const) {
-			assert.isTrue(isTrustedCallbackOrigin(origin, appBaseUrl), `${origin} vs ${appBaseUrl}`)
+			assert.isTrue(isTrustedCallbackOrigin(origin, config), origin)
 		}
 	})
 
-	it("accepts the local-dev family: *.localhost siblings and loopback ports", () => {
-		for (const [origin, appBaseUrl] of [
-			["https://api.localhost", "https://web.localhost"],
-			// Worktree dev hosts nest another label under *.localhost.
-			["https://wt.api.localhost", "https://wt.web.localhost"],
-			// Raw-port dev servers put web and api on different loopback ports.
-			["http://127.0.0.1:3472", "http://127.0.0.1:3471"],
-		] as const) {
-			assert.isTrue(isTrustedCallbackOrigin(origin, appBaseUrl), `${origin} vs ${appBaseUrl}`)
+	it("accepts a preview's own hosts only on that preview deploy", () => {
+		assert.isTrue(isTrustedCallbackOrigin("https://api-pr-12.maple.dev", preview))
+		assert.isTrue(isTrustedCallbackOrigin("https://app-pr-12.maple.dev", preview))
+		assert.isFalse(isTrustedCallbackOrigin("https://api-pr-13.maple.dev", preview))
+		assert.isFalse(isTrustedCallbackOrigin("https://api-pr-12.maple.dev", production))
+	})
+
+	it("accepts local hosts only in development with a local app origin", () => {
+		for (const origin of ["https://api.localhost", "https://wt.api.localhost", "http://127.0.0.1:3472"]) {
+			assert.isTrue(isTrustedCallbackOrigin(origin, development), origin)
+			assert.isFalse(isTrustedCallbackOrigin(origin, production), origin)
 		}
+		assert.isFalse(
+			isTrustedCallbackOrigin("https://api.localhost", {
+				...production,
+				MAPLE_ENVIRONMENT: "development",
+			}),
+		)
 	})
 
-	it("rejects a spoofed forwarded host and every other foreign origin", () => {
-		for (const [origin, appBaseUrl] of [
-			// The spoof case: exactly what `resolveRequestOrigin` hands back for a
-			// request carrying `x-forwarded-host: public.example.com`.
-			["https://public.example.com", "https://app.maple.dev"],
-			["https://evil.dev", "https://app.maple.dev"],
-			// A suffix-only lookalike must not match the trusted parent domain.
-			["https://api.maple.dev.evil.com", "https://app.maple.dev"],
-			// No host at all (relative url, no host headers).
-			["", "https://app.maple.dev"],
-			// Local dev and deployed hosts are separate families in both directions.
-			["https://evil.localhost", "https://app.maple.dev"],
-			["https://api.maple.dev", "http://127.0.0.1:3471"],
-			// A forwarded `http` scheme on an https deployment.
-			["http://api.maple.dev", "https://app.maple.dev"],
-			["http://app.maple.dev", "https://app.maple.dev"],
+	it("rejects sibling lookalikes, spoofed forwarded hosts and scheme downgrades", () => {
+		for (const [origin, config] of [
+			["https://evil.maple.dev", production],
+			["https://app-pr-1.maple.dev", production],
+			["https://evil.maple.dev", preview],
+			["https://public.example.com", production],
+			["https://api.maple.dev.evil.com", production],
+			["https://api.maple.dev:8443", production],
+			["", production],
+			["javascript:alert(1)", production],
+			["http://api.maple.dev", production],
+			["https://evil.localhost", production],
+			["https://api.maple.dev", development],
 		] as const) {
-			assert.isFalse(isTrustedCallbackOrigin(origin, appBaseUrl), `${origin} vs ${appBaseUrl}`)
+			assert.isFalse(isTrustedCallbackOrigin(origin, config), origin)
 		}
 	})
 })

apps/api/src/routes/v2/integrations.http.test.ts

@@ -58,8 +58,8 @@ afterEach(() => cleanupTestDbs(createdDbs))
 
 /**
  * The API and the web app sit on sibling hosts of one registrable domain in
- * every real deployment; `isTrustedCallbackOrigin` is what enforces that, so
- * the harness mirrors it (`api.maple.test` / `app.maple.test`).
+ * every real deployment; `isTrustedCallbackOrigin` allows exactly the configured
+ * origins, so the harness configures both (`api.maple.test` / `app.maple.test`).
  */
 const APP_BASE_URL = "https://app.maple.test"
 const API_HOST = "api.maple.test"
@@ -77,6 +77,8 @@ const testConfig = () =>
 			MAPLE_INGEST_KEY_ENCRYPTION_KEY: Buffer.alloc(32, 1).toString("base64"),
 			MAPLE_INGEST_KEY_LOOKUP_HMAC_KEY: "maple-test-lookup-secret",
 			MAPLE_APP_BASE_URL: APP_BASE_URL,
+			MAPLE_API_BASE_URL: `https://${API_HOST}`,
+			MAPLE_ENVIRONMENT: "production",
 			INTERNAL_SERVICE_TOKEN: "test-internal-token",
 		}),
 	)
@@ -540,6 +542,40 @@ describe("v2 planetscale integration over HTTP", () => {
 		await harness.dispose()
 	})
 
+	it("refuses a connect from a sibling host under the same parent domain", async () => {
+		const harness = makeHarness({})
+		const key = await harness.bootstrapAdminKey()
+		const { status } = await harness.request("POST", "/v2/integrations/planetscale/connect", key.secret, {
+			forwardedHost: "evil.maple.test",
+			body: {},
+		})
+		expect(status).toBe(503)
+		await harness.dispose()
+	})
+
+	it("builds the webhook URL from the configured API origin, not forwarded headers", async () => {
+		const harness = makeHarness({
+			connection: {
+				webhookConfig: () =>
+					Effect.succeed({
+						configured: true,
+						path: "/api/integrations/planetscale/webhook/abc",
+						secret: "s",
+					}),
+			},
+		})
+		const key = await harness.bootstrapAdminKey()
+		const { status, body } = await harness.request(
+			"GET",
+			"/v2/integrations/planetscale/webhook_config",
+			key.secret,
+			{ forwardedHost: "evil.example.com" },
+		)
+		expect(status).toBe(200)
+		expect(body.url).toBe(`https://${API_HOST}/api/integrations/planetscale/webhook/abc`)
+		await harness.dispose()
+	})
+
 	it("treats query_insights and events as reads: an integrations:read key is enough", async () => {
 		const harness = makeHarness({
 			inventory: {

apps/api/src/routes/v2/integrations.http.ts

@@ -59,60 +59,52 @@ export const resolveRequestOrigin = (req: HttpServerRequest.HttpServerRequest):
 	})
 }
 
-const hostnameOf = (url: string): string | null =>
-	Option.match(Option.liftThrowable(() => new URL(url))(), {
-		onNone: () => null,
-		onSome: (parsed) => parsed.hostname.toLowerCase(),
-	})
+const parseUrl = (url: string): Option.Option<URL> => Option.liftThrowable(() => new URL(url))()
 
-const protocolOf = (url: string): string | null =>
-	Option.match(Option.liftThrowable(() => new URL(url))(), {
-		onNone: () => null,
-		onSome: (parsed) => parsed.protocol,
-	})
+const isLocalHostname = (hostname: string) =>
+	hostname === "localhost" ||
+	hostname.endsWith(".localhost") ||
+	hostname.startsWith("127.") ||
+	hostname === "[::1]"
 
-/** `app.maple.dev` → `maple.dev`; a single-label host is its own parent. */
-const parentDomain = (hostname: string): string => {
-	const dot = hostname.indexOf(".")
-	return dot === -1 ? hostname : hostname.slice(dot + 1)
+/** The deploy's own origins, from Env; never from request headers. */
+export interface CallbackOriginConfig {
+	readonly MAPLE_APP_BASE_URL: string
+	readonly MAPLE_API_BASE_URL: string
+	/** `production`, `pr-<n>` or `development`. */
+	readonly MAPLE_ENVIRONMENT: string
 }
 
 /**
- * Whether an origin may be used as an OAuth callback origin.
- *
- * The callback URL is embedded in the authorize URL *and* persisted as
- * `oauth_auth_states.redirectUri`, then replayed as `redirect_uri` in the token
- * exchange — so an attacker-chosen origin mints an authorize URL pointing at a
- * host they control. `resolveRequestOrigin` reads `x-forwarded-host`, which any
- * client can set, so it is only accepted when it belongs to the same host family
- * as the trusted `MAPLE_APP_BASE_URL`:
- *
- *   - every deployed stage puts the web app and the API on sibling hosts under
- *     one registrable domain (`app.maple.dev` / `api.maple.dev`,
- *     `app-pr-<n>` / `api-pr-<n>`), and
- *   - local dev puts them on sibling `*.localhost` hosts (portless proxy) or on
- *     loopback ports.
+ * Whether an origin may be used as an OAuth callback origin. The callback is persisted
+ * and replayed as `redirect_uri`, and `resolveRequestOrigin` reads client-set headers, so
+ * only an explicit allow list passes:
  *
- * Anything else fails closed; the provider's registered-redirect allowlist is
- * then a second line of defense rather than the only one.
+ *   - the configured app and API origins (scheme, host and port all match);
+ *   - on a PR preview (`pr-<n>`), that PR's own `app-pr-<n>` / `api-pr-<n>.maple.dev`;
+ *   - in development with a local app origin, local hosts (`*.localhost`, loopback).
  */
-export const isTrustedCallbackOrigin = (origin: string, appBaseUrl: string): boolean => {
-	const host = hostnameOf(origin)
-	const appHost = hostnameOf(appBaseUrl)
-	if (host === null || appHost === null) return false
-	// Local dev is one family: `*.localhost` (portless proxy) plus loopback IPs
-	// (raw-port dev servers with no portless proxy, where web and api sit on
-	// different loopback ports).
-	const isLocal = (value: string) =>
-		value === "localhost" || value.endsWith(".localhost") || value.startsWith("127.") || value === "[::1]"
-	if (isLocal(host) || isLocal(appHost)) return isLocal(host) && isLocal(appHost)
-	// `x-forwarded-proto` is client-set too: a deployed callback keeps the app's
-	// scheme, so an https deployment never hands out an http redirect.
-	if (protocolOf(origin) !== protocolOf(appBaseUrl)) return false
-	if (host === appHost) return true
-	const parent = parentDomain(host)
-	// Require a dot so a bare TLD (`evil.dev` vs `app.maple.dev`) never matches.
-	return parent.includes(".") && parent === parentDomain(appHost)
+export const isTrustedCallbackOrigin = (origin: string, config: CallbackOriginConfig): boolean => {
+	const parsed = parseUrl(origin)
+	if (Option.isNone(parsed)) return false
+	const candidate = parsed.value
+	if (candidate.protocol !== "https:" && candidate.protocol !== "http:") return false
+	const configured = [config.MAPLE_APP_BASE_URL, config.MAPLE_API_BASE_URL].flatMap((url) =>
+		Option.toArray(parseUrl(url)),
+	)
+	if (configured.some((url) => url.origin === candidate.origin)) return true
+	const pr = /^pr-(\d+)$/.exec(config.MAPLE_ENVIRONMENT)
+	if (pr !== null) {
+		return [`https://app-pr-${pr[1]}.maple.dev`, `https://api-pr-${pr[1]}.maple.dev`].includes(
+			candidate.origin,
+		)
+	}
+	const appIsLocal = configured.some((url) => isLocalHostname(url.hostname))
+	return (
+		config.MAPLE_ENVIRONMENT === "development" &&
+		appIsLocal &&
+		isLocalHostname(candidate.hostname.toLowerCase())
+	)
 }
 
 const toPlanetScaleStatus = (status: PlanetScaleIntegrationStatus): V2PlanetScaleIntegration => ({
@@ -242,7 +234,7 @@ export const HttpV2PlanetScaleIntegrationsLive = HttpApiBuilder.group(
 							// the token exchange, and `resolveRequestOrigin` reads the
 							// client-settable `x-forwarded-host` — so an untrusted origin mints
 							// an authorize URL pointing at a host the caller controls.
-							if (!isTrustedCallbackOrigin(origin, env.MAPLE_APP_BASE_URL)) {
+							if (!isTrustedCallbackOrigin(origin, env)) {
 								yield* Effect.logError(
 									"Rejected PlanetScale connect: untrusted callback origin",
 									{
@@ -379,7 +371,6 @@ export const HttpV2PlanetScaleIntegrationsLive = HttpApiBuilder.group(
 									"Only org admins can read the PlanetScale webhook configuration",
 								),
 							)
-							const req = yield* HttpServerRequest.HttpServerRequest
 							const config = yield* planetscale
 								.webhookConfig(tenant.orgId)
 								.pipe(tapHttpErrors("PlanetScale webhook config failed"))
@@ -392,7 +383,7 @@ export const HttpV2PlanetScaleIntegrationsLive = HttpApiBuilder.group(
 								url:
 									config.path === null
 										? null
-										: `${resolveRequestOrigin(req)}${config.path}`,
+										: `${env.MAPLE_API_BASE_URL.replace(/\/+$/, "")}${config.path}`,
 								secret: config.secret,
 							} satisfies V2PlanetScaleWebhookConfig
 						}),

docs/github-app-setup.md

@@ -75,7 +75,7 @@ This closes the loop after a user installs your app, so Maple can record the ins
 3. Check **Request user authorization (OAuth) during installation**. **Do not skip this. It is a security control.** See the box below.
 4. Leave **Expire user authorization tokens** at its default (checked).
 
-> **Why this is required (confused-deputy guard).** `installation_id` values are small, **enumerable** integers and are not tied to the connect `state`. Without OAuth, anyone who starts a connect flow could submit _someone else's_ `installation_id` and bind another org's private repos to their own Maple org. With OAuth on, GitHub also returns a `code`. Maple exchanges it and calls `GET /user/installations` to confirm the user administers that installation before binding it. GitHub delivers `code`, `installation_id`, and `state` to the Callback URL above. There is no separate Setup URL.
+> **Why this is required (confused-deputy guard).** `installation_id` values are small, **enumerable** integers and are not tied to the connect `state`. Without OAuth, anyone who starts a connect flow could submit _someone else's_ `installation_id` and bind another org's private repos to their own Maple org. With OAuth on, GitHub also returns a `code`. Maple exchanges it, calls `GET /user/installations` to confirm the user can access that installation, then confirms they administer its account (an active org `admin` via `GET /user/memberships/orgs/{org}`, or the personal account itself) before binding it. GitHub delivers `code`, `installation_id`, and `state` to the Callback URL above. There is no separate Setup URL.
 
 ---
 
@@ -114,7 +114,7 @@ On repositories where a Maple admin has switched on the pull request review, Map
 
 > **Upgrading an existing App?** Adding a permission to an App that is already installed makes GitHub ask every installation's admin to accept the new permissions. Until they do, Maple still records each review but cannot post it: the review's row keeps a publish error and nothing appears on the pull request. CI logs behave the same way: until the installation accepts **Actions: Read**, GitHub refuses every job log download with `403 Resource not accessible by integration` and those repositories get traces without logs. To fix it, accept the permission request on GitHub (Settings → Applications → the App → Review request).
 
-3. Leave **Organization permissions** and **Account permissions** entirely at **No access**.
+3. Under **Organization permissions**, set **Members** to **Read-only**. Maple uses it only during connect, to confirm the person connecting an organization installation is an admin of that organization. Without it, connecting an organization is refused. Leave every other organization permission and all **Account permissions** at **No access**.
 
 ---
 
@@ -259,6 +259,7 @@ After setting the variables, **restart the Maple API** so they take effect. Mapl
 | Repository → Checks        | Read and write                                   |
 | Repository → Metadata      | Read-only (mandatory)                            |
 | Repository → Actions       | Read-only (CI job logs)                          |
+| Organization → Members     | Read-only (connect-time admin check)             |
 
 ### Subscribed webhook events