Cloudflare Git Token Service - Add new worker for GitHub token management#19
Cloudflare Git Token Service - Add new worker for GitHub token management#19
Conversation
![kiloconnect[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| with: | ||
| apiToken: ${{ secrets.CLOUDFLARE_API_TOKEN }} | ||
| workingDirectory: cloudflare-git-token-service | ||
| command: deploy --env prod |
There was a problem hiding this comment.
CRITICAL: wrangler deploy --env prod requires an env.prod config, but cloudflare-git-token-service/wrangler.jsonc only defines env.dev.
This workflow will likely fail at deploy time unless you either add env.prod or deploy the default env.
| command: deploy --env prod | |
| command: deploy |
| @@ -0,0 +1,10 @@ | |||
| # GITHUB_APP_ID: The numeric App ID from GitHub App settings | |||
| # GITHUB_APP_PRIVATE_KEY: The PEM-encoded private key (base64 encoded for env var storage) | |||
There was a problem hiding this comment.
WARNING: .dev.vars.example says the private key is base64-encoded, but the runtime code in cloudflare-git-token-service/src/github-token-service.ts expects a PEM string with \\n sequences (it does replace(/\\n/g, "\n")).
If users follow this comment literally and base64-encode the key, token generation will fail.
| # GITHUB_APP_PRIVATE_KEY: The PEM-encoded private key (base64 encoded for env var storage) | |
| # GITHUB_APP_PRIVATE_KEY: The PEM-encoded private key (use \\n for newlines when stored in env vars) |
| } | ||
|
|
||
| // 2. Generate token for the installation (not scoped to specific repo) | ||
| const token = await this.githubService.getToken( |
There was a problem hiding this comment.
WARNING: Token returned by getTokenForRepo is currently installation-scoped (all repos granted to the app install), not repo-scoped.
Given the RPC method is named getTokenForRepo, consider calling GitHubTokenService.getTokenForRepo() with the repo name extracted from params.githubRepo to reduce token blast radius (and help avoid over-privileged tokens if a caller passes an arbitrary owner/repo).
|
|
||
| // Validate orgId is a valid UUID if provided, to prevent database errors | ||
| if (params.orgId !== undefined && !isValidUuid(params.orgId)) { | ||
| throw new Error(`Invalid organization ID format: ${params.orgId}`); |
There was a problem hiding this comment.
WARNING: findInstallationId() returns structured failures, but invalid orgId currently throws, which will surface as a 500 from the RPC call.
If invalid orgId is a user input possibility, consider returning a typed failure reason instead of throwing to keep error handling consistent.
| export function createDatabaseConnection(connectionString: string): Database { | ||
| const pool = new Pool({ | ||
| connectionString, | ||
| max: 100, |
There was a problem hiding this comment.
SUGGESTION: pg.Pool max: 100 is high for a Workers/Hyperdrive context and can easily exhaust DB connections under bursty traffic.
Consider lowering the pool size (and/or making it configurable) to reduce connection pressure.
Code Review SummaryStatus: 6 Issues Found | Recommendation: Address before merge Overview
Fix these issues in Kilo Cloud Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (12 files)
|
| GITHUB_LITE_APP_ID?: string; | ||
| GITHUB_LITE_APP_PRIVATE_KEY?: string; | ||
| }; | ||
|
|
||
| const TokenCacheEntrySchema = z.object({ |
There was a problem hiding this comment.
Seems like TokenCacheEntry, GeneratedToken, and TokenCacheEntrySchema are all the same. Should probably consolidate into a single type inferred from the zod schema:
type Token = z.infer<typeof Token>
const Token = z.object({
token: z.string(),
expiresAt: z.number(),
});
| ${platform_integrations.github_app_type} | ||
| FROM ${platform_integrations} | ||
| -- For org installations, verify user is a member of the org | ||
| LEFT JOIN ${organization_memberships} |
There was a problem hiding this comment.
do we also need to join on kilocode_users and ensure that blocked_reason is null?
eshurakov
force-pushed
the
eshurakov/cloudflare-git-token-service
branch
from
c1ae901 to
0e246fc
Compare
- Implement installation lookup with org membership verification - Support both standard (read/write) and lite (read-only) GitHub App types - Add KV-based token caching with automatic TTL - Include blocked user check to prevent blocked users from getting tokens - Add GitHub Actions workflow for automated deployment
eshurakov
force-pushed
the
eshurakov/cloudflare-git-token-service
branch
from
0e246fc to
443d3b3
Compare
| } | ||
|
|
||
| // Validate githubRepo format (expected: "owner/repo") | ||
| if (!params.githubRepo || !params.githubRepo.includes('/')) { |
There was a problem hiding this comment.
WARNING: githubRepo format validation is too permissive
This currently only checks includes('/'), which means inputs like owner/ (empty repo) or owner/repo/extra will pass and repoOwner will be derived from the first segment anyway. If callers rely on this for input validation, it may lead to surprising lookups.
| if (!params.githubRepo || !params.githubRepo.includes('/')) { | |
| if (!params.githubRepo || !/^[^/]+\/[^/]+$/.test(params.githubRepo)) { |
2 participants
Summary
cloudflare-git-token-serviceCloudflare Worker for centralized GitHub App token management