diff --git a/middleware/src/auth/publicPaths.ts b/middleware/src/auth/publicPaths.ts index bfd7bc89..14b31991 100644 --- a/middleware/src/auth/publicPaths.ts +++ b/middleware/src/auth/publicPaths.ts @@ -30,6 +30,9 @@ export const STATIC_PUBLIC_PATHS: readonly RegExp[] = [ // cookie. Every request is authenticated against the job-token hash in // routes/devRunnerApi.ts — that IS its authentication. /^\/api\/v1\/dev-runner(?:\/|$|\?)/, + // Epic #470 — GitHub redirects finish the dev-platform GitHub-App setup on a + // signed state token / installation ownership check, not on an operator session. + /^\/api\/v1\/dev-platform\/github-app\/(?:callback|setup)(?:\/|$|\?)/, // Plugin-served UI surfaces (`/p//...`), iframed by Teams where // only a Teams SSO token exists. Plugins exposing sensitive data validate // that token themselves. diff --git a/middleware/src/index.ts b/middleware/src/index.ts index afa12b28..d1f5b7f7 100644 --- a/middleware/src/index.ts +++ b/middleware/src/index.ts @@ -120,6 +120,7 @@ import { BuilderTriageLog } from './plugins/builder/builderTriageLog.js'; import { GithubIssueCache } from './plugins/builder/githubIssueCache.js'; import { GithubIssueCreator } from './plugins/builder/githubIssueCreator.js'; import { createGitHubDeviceProvider } from './issues/githubOAuthProvider.js'; +import { DeviceFlowStore } from './issues/deviceFlowStore.js'; import { createIssuesRouter } from './issues/issuesRouter.js'; import { GitHubAppTokenProvider } from './plugins/builder/githubAppAuth.js'; import { UserChoiceCoordinator } from './plugins/builder/userChoiceCoordinator.js'; @@ -175,6 +176,7 @@ import { } from './devplatform/triggers/triggerJobService.js'; import { mintRunnerToken as mintDevRunnerToken } from './devplatform/jobToken.js'; import { DevRetentionRunner } from './devplatform/retention.js'; +import { ManifestFlowStore } from './devplatform/githubApp/manifestFlow.js'; import { OAuthClient } from './auth/oauthClient.js'; import { RefreshStore } from './auth/refreshStore.js'; import { EmailWhitelist } from './auth/whitelist.js'; @@ -218,6 +220,7 @@ import { FileInstalledRegistry } from './plugins/fileInstalledRegistry.js'; import { InstallService } from './plugins/installService.js'; import { registerInstalledPluginTemplates } from './plugins/pluginTemplates.js'; import type { PluginTemplateRegistrar } from './plugins/pluginTemplates.js'; +import { createDevPlatformGithubAppRouter } from './routes/devPlatformGithubApp.js'; import { OAuthBrokerService, PendingFlowStore, @@ -2519,6 +2522,9 @@ async function main(): Promise { // persist a durable queue. The two safety-critical modes (subscription auth, // unsafe-local backend) already refused boot in config.ts if misconfigured. if (config.DEV_PLATFORM_ENABLED && graphPool) { + const devPlatformGithubDeviceProvider = createGitHubDeviceProvider( + config.DEV_PLATFORM_GITHUB_CLIENT_ID ?? config.GITHUB_OAUTH_CLIENT_ID, + ); const shimEntry = fileURLToPath( new URL('../packages/dev-runner-shim/dist/src/index.js', import.meta.url), ); @@ -2618,10 +2624,43 @@ async function main(): Promise { // W4 (spec §2): the Fly Machines backend, present only when a dedicated runner // app is configured (absent ⇒ not registered). ...(flyConfig ? { fly: flyConfig } : {}), + ...(devPlatformGithubDeviceProvider + ? { + deviceFlow: { + provider: devPlatformGithubDeviceProvider, + store: new DeviceFlowStore(), + }, + } + : {}), log: (msg) => console.log(msg), }); mountDevPlatform(app, requireAuth, wiredDevPlatform, (msg) => console.log(msg)); + const devPlatformGithubAppStore = new DevGithubAppStore(graphPool, secretVault); + const devPlatformGithubAppRouter = createDevPlatformGithubAppRouter({ + flowStore: new ManifestFlowStore(), + appStore: devPlatformGithubAppStore, + bindRepoCredential: async (repoId, binding): Promise => { + const boundRepo = await wiredDevPlatform.repoStore.updateRepo(repoId, { + credentialKind: 'github_app', + credentialRef: `github_app:${binding.appRowId}:${binding.installationId}`, + }); + if (!boundRepo) { + throw new Error(`dev-platform repo not found while binding GitHub App credential: ${repoId}`); + } + }, + getRepo: async (repoId): Promise<{ owner: string; name: string } | null> => { + const repo = await wiredDevPlatform.repoStore.getRepo(repoId); + return repo ? { owner: repo.owner, name: repo.name } : null; + }, + publicBaseUrl: config.PUBLIC_BASE_URL, + log: (msg) => console.log(msg), + }); + app.use('/api/v1/admin/dev-platform', requireAuth, devPlatformGithubAppRouter.admin); + console.log('[dev-platform] github-app admin router mounted at /api/v1/admin/dev-platform (requireAuth)'); + app.use('/api/v1/dev-platform', devPlatformGithubAppRouter.public); + console.log('[dev-platform] github-app public router mounted at /api/v1/dev-platform (state-token auth only, no session guard)'); + // Epic #470 W3 — register the chat orchestrator dev-job tools globally on // the native-tool registry (mirrors `requestSelfExtensionTool`). ONE global // registration; the caller is resolved PER CALL from `turnContext.userId` diff --git a/middleware/src/routes/devPlatformRepos.ts b/middleware/src/routes/devPlatformRepos.ts index 730655dc..9e0ea78c 100644 --- a/middleware/src/routes/devPlatformRepos.ts +++ b/middleware/src/routes/devPlatformRepos.ts @@ -83,11 +83,19 @@ export function registerDevPlatformRepoRoutes(router: Router, deps: DevPlatformR // Validate access + capture the default branch (spec §6). const access = await deps.probeRepoAccess({ owner, name, token }); if (!access.ok) { - // Drop the staged device-flow token: it was parked under pending/ - // and, without this, a failed repo add leaves a live credential in the - // vault staging slot with no owning repo to clean it up. - if (kind === 'device_flow') await deps.credentials.clearPending(caller.sub); - throw new DevPlatformError(400, 'devplatform.repo_access_failed', 'could not access the repository with the supplied credential'); + // Keep the staged device-flow token parked under pending/. A probe + // failure is usually fixable WITHOUT re-authorizing — a mistyped owner/name, + // or a private repo the credential cannot yet reach (the OAuth App is not + // approved for the org, or the account lacks access). Dropping the parked + // token here forced a full device-flow re-auth on every retry and surfaced a + // misleading `no_pending_device_flow` on the next attempt. The parked token is + // the operator's own credential, encrypted under their sub; it is overwritten + // by the next connect and consumed by the next successful add. + throw new DevPlatformError( + 400, + 'devplatform.repo_access_failed', + 'could not access the repository with the supplied credential — check the owner/name, and that the credential can reach it (a private repo may need the OAuth App approved for the org, a PAT with repo scope, or the GitHub App installed)', + ); } const credentialKind: DevRepoCredentialKind = kind === 'device_flow' ? 'device_flow' : 'pat'; diff --git a/middleware/test/devplatform/devPlatformDeviceFlowWiring.test.ts b/middleware/test/devplatform/devPlatformDeviceFlowWiring.test.ts new file mode 100644 index 00000000..b2c55adc --- /dev/null +++ b/middleware/test/devplatform/devPlatformDeviceFlowWiring.test.ts @@ -0,0 +1,63 @@ +import assert from 'node:assert/strict'; +import { after, describe, it } from 'node:test'; + +import { DeviceFlowStore } from '../../src/issues/deviceFlowStore.js'; +import type { DevPlatformDeviceFlow } from '../../src/routes/devPlatformShared.js'; +import { authHeaders, makeHarness, postJson } from './devPlatformRoutes.harness.js'; + +/** + * Epic #470 — composition-root wiring regression (GAP 2). + * + * `POST /github/connect/start` 503s with `devplatform.device_flow_unconfigured` + * UNLESS a `deviceFlow` dep is threaded into the router. The bug this fixes was + * exactly that: index.ts called `assembleDevPlatform(...)` with NO `deviceFlow` + * key, so PAT was the only working credential path and the device-flow card + * always 503'd. index.ts now constructs the provider + store and supplies it via + * conditional spread. These tests pin BOTH sides of the gate so a future refactor + * that drops the `deviceFlow` wiring fails loudly here rather than in production. + */ + +function fakeDeviceFlow(): DevPlatformDeviceFlow { + const provider: DevPlatformDeviceFlow['provider'] = { + requestDeviceCode: async () => ({ + // The device_code is the secret half — the route keeps it server-side. + deviceCode: 'dc-secret-stays-server-side', + userCode: 'WXYZ-1234', + verificationUri: 'https://github.com/login/device', + expiresIn: 900, + interval: 5, + }), + pollAccessToken: async () => ({ status: 'pending' }), + fetchUserLogin: async () => 'octocat', + }; + return { provider, store: new DeviceFlowStore() }; +} + +describe('devPlatform device-flow wiring (epic #470 GAP 2)', () => { + it('503s device_flow_unconfigured when no deviceFlow dep is supplied (the pre-fix state)', async () => { + const h = await makeHarness(); + after(() => h.close()); + + const res = await postJson(`${h.baseUrl}/github/connect/start`, authHeaders('alice'), {}); + + assert.equal(res.status, 503); + const body = (await res.json()) as { code?: string }; + assert.equal(body.code, 'devplatform.device_flow_unconfigured'); + }); + + it('returns the device code (never the secret device_code) when deviceFlow is threaded in', async () => { + const h = await makeHarness({ deviceFlow: fakeDeviceFlow() }); + after(() => h.close()); + + const res = await postJson(`${h.baseUrl}/github/connect/start`, authHeaders('alice'), {}); + + assert.equal(res.status, 200); + const body = (await res.json()) as Record; + assert.equal(body.userCode, 'WXYZ-1234'); + assert.equal(body.verificationUri, 'https://github.com/login/device'); + assert.equal(body.expiresIn, 900); + assert.equal(body.interval, 5); + // The secret half must never cross the wire. + assert.equal(body.deviceCode, undefined); + }); +}); diff --git a/middleware/test/devplatform/devPlatformGithubApp.test.ts b/middleware/test/devplatform/devPlatformGithubApp.test.ts index ef5719f3..c79c6b82 100644 --- a/middleware/test/devplatform/devPlatformGithubApp.test.ts +++ b/middleware/test/devplatform/devPlatformGithubApp.test.ts @@ -5,6 +5,7 @@ import { after, describe, it } from 'node:test'; import express, { type RequestHandler } from 'express'; +import { publicPaths } from '../../src/auth/publicPaths.js'; import { createDevPlatformGithubAppRouter, type DevPlatformGithubAppDeps, @@ -155,6 +156,41 @@ async function harness(opts: HarnessOpts = {}) { const authed = (sub: string): Record => ({ 'x-test-sub': sub, 'content-type': 'application/json' }); +async function mountLikeIndex(deps: DevPlatformGithubAppDeps): Promise<{ + admin: string; + publicBase: string; + close: () => Promise; +}> { + const routers = createDevPlatformGithubAppRouter(deps); + const allowlist = publicPaths({ devEndpointsEnabled: false }); + const requireAuth: RequestHandler = (req, res, next) => { + if (allowlist.some((pattern) => pattern.test(req.originalUrl))) { + next(); + return; + } + const sub = req.header('x-test-sub'); + if (!sub) { + res.status(401).json({ code: 'auth.missing', message: 'no session' }); + return; + } + (req as unknown as { session: { sub: string } }).session = { sub }; + next(); + }; + + const app = express(); + app.use('/api', requireAuth, (_req, _res, next) => next()); + app.use('/api/v1/admin/dev-platform', requireAuth, routers.admin); + app.use('/api/v1/dev-platform', routers.public); + const server = app.listen(0); + await new Promise((r) => server.once('listening', r)); + const port = String((server.address() as AddressInfo).port); + return { + admin: `http://127.0.0.1:${port}/api/v1/admin/dev-platform`, + publicBase: `http://127.0.0.1:${port}/api/v1/dev-platform`, + close: () => new Promise((r) => server.close(() => r())), + }; +} + // --------------------------------------------------------------------------- describe('devPlatformGithubApp — manifest/start (admin)', () => { @@ -312,6 +348,35 @@ describe('devPlatformGithubApp — callback (public /bot-api)', () => { }); }); +describe('devPlatformGithubApp — index.ts mount topology', () => { + it('resolves the admin and public routes on the exact prefixes index.ts mounts', async () => { + const state: StoreState = { + apps: new Map(), + installations: new Map(), + secrets: new Map(), + saved: [], + upserts: [], + }; + const mounted = await mountLikeIndex({ + flowStore: new ManifestFlowStore(), + appStore: makeStore(state), + bindRepoCredential: async () => undefined, + getRepo: async () => ({ owner: 'acme', name: 'omadia' }), + publicBaseUrl: 'https://ops.example.com', + fetchImpl: (async () => jsonResponse(404, {})) as unknown as typeof fetch, + }); + after(() => mounted.close()); + + const adminRes = await fetch(`${mounted.admin}/github-apps`, { headers: authed('user-1') }); + assert.equal(adminRes.status, 200, 'the auth-gated admin prefix resolves'); + + const publicRes = await fetch(`${mounted.publicBase}/github-app/setup`, { redirect: 'manual' }); + assert.equal(publicRes.status, 400, 'the public callback/setup prefix bypasses the broad /api auth gate'); + assert.notEqual(publicRes.status, 404, 'the public router is mounted on /api/v1/dev-platform'); + assert.equal(publicRes.headers.get('content-type'), 'text/plain; charset=utf-8'); + }); +}); + describe('devPlatformGithubApp — setup (public /bot-api)', () => { it('verifies the installation belongs to a known App, upserts, and 302s to admin', async () => { const fetchImpl: FetchStub = async (url) => { diff --git a/middleware/test/devplatform/devPlatformRoutes.test.ts b/middleware/test/devplatform/devPlatformRoutes.test.ts index 5d66ffbc..5283f279 100644 --- a/middleware/test/devplatform/devPlatformRoutes.test.ts +++ b/middleware/test/devplatform/devPlatformRoutes.test.ts @@ -124,6 +124,20 @@ describe('devPlatform — POST /repos onboarding', () => { assert.equal(await h.credentials.resolve(created!.id), DEVICE_TOKEN); assert.equal(await h.credentials.resolvePending('alice'), undefined); }); + + it('keeps the staged device-flow token when the probe denies access (retry without re-auth)', async () => { + h = await makeHarness({ probeRepoAccess: async () => ({ ok: false, defaultBranch: '' }) }); + await h.credentials.stashPending('alice', DEVICE_TOKEN); + const res = await postJson(`${h.baseUrl}/repos`, authHeaders('alice'), { + owner: 'o', name: 'r', credential: { kind: 'device_flow' }, + }); + assert.equal(res.status, 400); + assert.equal(((await res.json()) as { code: string }).code, 'devplatform.repo_access_failed'); + // Regression: a probe denial must NOT drop the parked authorization — the + // operator fixes access (org approval, correct name) and retries without + // re-running the whole device flow. Reverting the fix makes this fail. + assert.equal(await h.credentials.resolvePending('alice'), DEVICE_TOKEN); + }); }); describe('devPlatform — POST /jobs brief composition', () => {