Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -80,3 +80,6 @@ run-milvus-test.sh
# Cloudflare
.wrangler/
.env*.local

# Kilo Telemetry
telemetry-id
24 changes: 24 additions & 0 deletions src/app/api/integrations/github/check-installation/route.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { NextRequest, NextResponse } from 'next/server';
import { findGitHubIntegrationByAccountLogin } from '@/lib/integrations/db/platform-integrations';
import * as Sentry from '@sentry/nextjs';

export async function GET(request: NextRequest) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you give a little more info on:

  1. Why we need to check the installation in the first place
  2. Why we need to check by owner?

Shouldn't an authenticated user be required here? And then we can simply query their own `platform_integrations?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jrf0110 It's to support the kilo github install and kilo github run commands of the CLI, so the CLI needs an endpoint to check if the installation ends to continue with the generation of the .github/workflow files.

try {
const owner = request.nextUrl.searchParams.get('owner');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WARNING: Unauthenticated endpoint can be used to enumerate GitHub App installations

This GET route accepts an arbitrary owner and returns whether an active integration exists. If this is reachable from the browser, it can leak which orgs/users have the GitHub App installed (useful for targeted abuse). Consider requiring an authenticated Kilo session and/or restricting to internal callers, plus rate limiting.

if (!owner) {
return NextResponse.json({ error: 'Missing owner parameter' }, { status: 400 });
}

const integration = await findGitHubIntegrationByAccountLogin(owner);
const installed = !!(integration && integration.platform_installation_id);

return NextResponse.json({ installation: installed });
} catch (error) {
console.error('GitHub installation check failed:', error);
Sentry.captureException(error);
return NextResponse.json(
{ error: error instanceof Error ? error.message : 'Internal server error' },
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WARNING: Returning raw exception messages can leak internals

This response exposes error.message to the caller, which may include sensitive details (upstream errors, stack context, etc.). Prefer a generic error message in the HTTP response and rely on Sentry/server logs for diagnostics.

Suggested change
{ error: error instanceof Error ? error.message : 'Internal server error' },
{ error: 'Internal server error' },

{ status: 500 }
);
}
}
77 changes: 77 additions & 0 deletions src/app/api/integrations/github/exchange-token-with-pat/route.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
import { NextRequest, NextResponse } from 'next/server';
import { Octokit } from '@octokit/rest';
import { findGitHubIntegrationByAccountLogin } from '@/lib/integrations/db/platform-integrations';
import { generateGitHubInstallationToken } from '@/lib/integrations/platforms/github/adapter';
import * as Sentry from '@sentry/nextjs';

export async function POST(request: NextRequest) {
try {
const authHeader = request.headers.get('authorization');
if (!authHeader || !authHeader.startsWith('Bearer ')) {
return NextResponse.json({ error: 'Missing authorization header' }, { status: 400 });
}

const pat = authHeader.substring(7);

let body: { owner?: string; repo?: string };
try {
body = await request.json();
} catch {
return NextResponse.json({ error: 'Invalid JSON body' }, { status: 400 });
}

const { owner, repo } = body;
if (!owner || !repo) {
return NextResponse.json({ error: 'Missing owner or repo in request body' }, { status: 400 });
}

const octokit = new Octokit({ auth: pat });
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CRITICAL: PAT-based token exchange needs stronger guardrails

This endpoint lets any caller who can present any GitHub PAT (with repo access) mint a GitHub App installation token for the owner. That is a large privilege escalation surface if the route is accessible outside a tightly controlled environment. Strongly consider enforcing Kilo-side auth + authorization, restricting to server-to-server usage, and/or limiting which repos/owners/workflows are allowed to exchange.


let repoData;
try {
const response = await octokit.rest.repos.get({ owner, repo });
repoData = response.data;
} catch (error) {
console.error('PAT validation failed:', error);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WARNING: Logging raw Octokit errors may leak sensitive request details

console.error(..., error) can include rich error objects (request metadata, headers) depending on the thrown type. Since this code handles PATs, prefer logging a sanitized subset (e.g., status + message) and rely on Sentry scrubbing for the rest.

return NextResponse.json({ error: 'Invalid PAT or no access to repository' }, { status: 401 });
}

if (!repoData.permissions?.push && !repoData.permissions?.admin) {
return NextResponse.json(
{ error: 'PAT owner does not have write access to repository' },
{ status: 403 }
);
}

const integration = await findGitHubIntegrationByAccountLogin(owner);

if (!integration || !integration.platform_installation_id) {
return NextResponse.json(
{ error: `No GitHub App installation found for owner ${owner}` },
{ status: 404 }
);
}

console.log('PAT token exchange', {
owner,
repo,
installationId: integration.platform_installation_id,
});

const { token } = await generateGitHubInstallationToken(
integration.platform_installation_id,
integration.github_app_type || 'standard',
[`${owner}/${repo}`]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WARNING: Scope installation token using canonical repository name

repositoryNames is built from user-provided owner/repo. Using the canonical name from GitHub (repoData.full_name) avoids casing/canonicalization mismatches and reduces the chance the installation-token request fails unexpectedly.

Suggested change
[`${owner}/${repo}`]
[repoData.full_name]

);

return NextResponse.json({ token });
} catch (error) {
console.error('GitHub token exchange with PAT failed:', error);
Sentry.captureException(error);

return NextResponse.json(
{ error: error instanceof Error ? error.message : 'Internal server error' },
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WARNING: Returning raw exception messages can leak internals

This 500 response exposes error.message to the caller. Prefer a generic message in the response and keep details in Sentry/server logs.

Suggested change
{ error: error instanceof Error ? error.message : 'Internal server error' },
{ error: 'Internal server error' },

{ status: 500 }
);
}
}
89 changes: 89 additions & 0 deletions src/app/api/integrations/github/exchange-token/route.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
import { NextRequest, NextResponse } from 'next/server';
import { verifyGitHubOIDCToken } from '@/lib/integrations/platforms/github/oidc';
import { findGitHubIntegrationByAccountLogin } from '@/lib/integrations/db/platform-integrations';
import { generateGitHubInstallationToken } from '@/lib/integrations/platforms/github/adapter';
import type { PlatformRepository } from '@/lib/integrations/core/types';
import * as Sentry from '@sentry/nextjs';

export async function POST(request: NextRequest) {
try {
const authHeader = request.headers.get('authorization');
if (!authHeader || !authHeader.startsWith('Bearer ')) {
return NextResponse.json({ error: 'Missing authorization header' }, { status: 400 });
}

const oidcToken = authHeader.substring(7);

const payload = await verifyGitHubOIDCToken(oidcToken, 'kilo-github-action');

const repositoryOwner = payload.repository_owner;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WARNING: OIDC payload is used without runtime validation/normalization

payload.repository_owner is assumed to be a valid login string and is used directly for the DB lookup. If the claim is missing/unexpected or differs in casing, this will either throw later or incorrectly return 404. Consider validating required claims (e.g., repository_owner, repository) and normalizing login casing before calling the DB query.

const repositoryOwnerId = payload.repository_owner_id;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WARNING: repository_owner_id type mismatch can cause false "owner mismatch" rejections

GitHub OIDC claims like repository_owner_id are typically numeric in the JWT payload; after jose parsing, this may be a number while integration.platform_account_id is stored as a string. The strict inequality check can then fail even when the IDs represent the same value, returning 403 unexpectedly.

Suggested change
const repositoryOwnerId = payload.repository_owner_id;
const repositoryOwnerId = String(payload.repository_owner_id);

const repository = payload.repository;

const integration = await findGitHubIntegrationByAccountLogin(repositoryOwner);

if (!integration || !integration.platform_installation_id) {
return NextResponse.json(
{ error: `No GitHub App installation found for owner ${repositoryOwner}` },
{ status: 404 }
);
}

if (
integration.platform_account_id &&
integration.platform_account_id !== String(repositoryOwnerId)
) {
Sentry.captureMessage('OIDC token owner ID mismatch', {
level: 'warning',
extra: {
repositoryOwner,
repositoryOwnerId,
integrationAccountId: integration.platform_account_id,
},
});
return NextResponse.json({ error: 'Installation owner mismatch' }, { status: 403 });
}

if (
integration.repository_access === 'selected' &&
integration.repositories &&
Array.isArray(integration.repositories)
) {
const repos = integration.repositories as PlatformRepository[];
const hasAccess = repos.some((r) => r.full_name === repository);
if (!hasAccess) {
return NextResponse.json(
{ error: 'Repository not in installation scope' },
{ status: 403 }
);
}
}

console.log('OIDC token exchange', {
repository,
repositoryOwner,
repositoryOwnerId,
installationId: integration.platform_installation_id,
});

const { token } = await generateGitHubInstallationToken(
integration.platform_installation_id,
integration.github_app_type || 'standard',
[repository]
);

return NextResponse.json({ token });
} catch (error) {
console.error('GitHub token exchange failed:', error);
Sentry.captureException(error);

if (error instanceof Error && error.message.includes('OIDC token verification failed')) {
return NextResponse.json({ error: 'Invalid OIDC token' }, { status: 401 });
}

return NextResponse.json(
{ error: error instanceof Error ? error.message : 'Internal server error' },
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WARNING: Returning raw exception messages can leak internals

This 500 response exposes error.message to the caller. Prefer a generic message in the response and keep details in Sentry/server logs.

Suggested change
{ error: error instanceof Error ? error.message : 'Internal server error' },
{ error: 'Internal server error' },

{ status: 500 }
);
}
}
20 changes: 20 additions & 0 deletions src/lib/integrations/db/platform-integrations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -525,6 +525,26 @@ export async function unsuspendIntegrationForOwner(owner: Owner, platform: strin
.where(and(ownershipCondition, eq(platform_integrations.platform, platform)));
}

/**
* Finds an active GitHub integration by account login (username or org name)
* Uses case-insensitive comparison since GitHub usernames/org names are case-insensitive
*/
export async function findGitHubIntegrationByAccountLogin(accountLogin: string) {
const [integration] = await db
.select()
.from(platform_integrations)
.where(
and(
eq(platform_integrations.platform, PLATFORM.GITHUB),
sql`LOWER(${platform_integrations.platform_account_login}) = LOWER(${accountLogin})`,
eq(platform_integrations.integration_status, INTEGRATION_STATUS.ACTIVE)
)
)
.limit(1);

return integration || null;
}

/**
* Owner-aware upsert for platform integrations
* Supports both user and organization ownership
Expand Down
14 changes: 12 additions & 2 deletions src/lib/integrations/platforms/github/adapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,12 @@ export function verifyGitHubWebhookSignature(
/**
* Generates GitHub App installation token
* @param appType - The type of GitHub App to use (defaults to 'standard')
* @param repositoryNames - Optional list of repository names to scope the token to (e.g., ["owner/repo"])
*/
export async function generateGitHubInstallationToken(
installationId: string,
appType: GitHubAppType = 'standard'
appType: GitHubAppType = 'standard',
repositoryNames?: string[]
): Promise<InstallationToken> {
const credentials = getGitHubAppCredentials(appType);

Expand All @@ -49,7 +51,15 @@ export async function generateGitHubInstallationToken(
installationId,
});

const authResult = await auth({ type: 'installation' });
const authOptions: { type: 'installation'; repositoryNames?: string[] } = {
type: 'installation',
};

if (repositoryNames && repositoryNames.length > 0) {
authOptions.repositoryNames = repositoryNames;
}

const authResult = await auth(authOptions);

return {
token: authResult.token,
Expand Down
49 changes: 49 additions & 0 deletions src/lib/integrations/platforms/github/oidc.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
import { jwtVerify, createRemoteJWKSet } from 'jose';

Check failure on line 1 in src/lib/integrations/platforms/github/oidc.ts

View workflow job for this annotation

GitHub Actions / test

Cannot find module 'jose' or its corresponding type declarations.

const GITHUB_OIDC_ISSUER = 'https://token.actions.githubusercontent.com';
const GITHUB_JWKS_URL = `${GITHUB_OIDC_ISSUER}/.well-known/jwks`;

const jwks = createRemoteJWKSet(new URL(GITHUB_JWKS_URL));

export type GitHubOIDCTokenPayload = {
sub: string;
repository: string;
repository_owner: string;
repository_owner_id: string;
repository_id: string;
repository_visibility: string;
run_id: string;
run_number: string;
run_attempt: string;
actor: string;
actor_id: string;
workflow: string;
ref: string;
ref_type: string;
environment?: string;
job_workflow_ref: string;
iss: string;
aud: string;
exp: number;
iat: number;
jti: string;
};

export async function verifyGitHubOIDCToken(
token: string,
expectedAudience: string
): Promise<GitHubOIDCTokenPayload> {
try {
const { payload } = await jwtVerify(token, jwks, {
issuer: GITHUB_OIDC_ISSUER,
audience: expectedAudience,
});

return payload as GitHubOIDCTokenPayload;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WARNING: Unsafe cast of JWT payload can hide missing/incorrect claims

jwtVerify() returns an untyped payload. Casting with as GitHubOIDCTokenPayload skips runtime validation, so downstream code can assume properties exist when they do not. Consider validating with a schema (e.g., Zod) and returning a typed, validated object.

} catch (error) {
if (error instanceof Error) {
throw new Error(`OIDC token verification failed: ${error.message}`);
}
throw new Error('OIDC token verification failed');
}
}
Loading