feat(caretaker): egress cloud run service#28167
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request adds a new automated egress service to the caretaker agent suite. The service acts as a bridge between GCP Pub/Sub and GitHub, allowing the system to react to verified events by performing automated repository operations. It is built as a lightweight Express application optimized for deployment on Google Cloud Run, ensuring secure and reliable execution of GitHub tasks. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
|
📊 PR Size: size/XL
|
There was a problem hiding this comment.
Code Review
This pull request introduces a new Node.js/TypeScript Express-based egress service designed to run on Cloud Run and handle GitHub actions via Pub/Sub push subscriptions. Feedback highlights a critical security vulnerability regarding missing authentication on the POST endpoint, and suggests centralizing environment variable parsing and improving string validation for comments.
| function getRequiredEnvVar(name: string): string { | ||
| const value = process.env[name]; | ||
| if (!value) { | ||
| throw new Error(`Missing required environment variable: ${name}`); | ||
| } | ||
| return value; | ||
| } | ||
|
|
||
| export async function handleEgressEvent(event: EgressEvent): Promise<void> { | ||
| const appId = getRequiredEnvVar('GH_APP_ID'); | ||
| const privateKey = getRequiredEnvVar('GH_PRIVATE_KEY'); | ||
| const installationId = getRequiredEnvVar('GH_INSTALLATION_ID'); | ||
|
|
||
| const octokit = new Octokit({ | ||
| authStrategy: createAppAuth, | ||
| auth: { | ||
| appId: Number(appId), | ||
| privateKey: privateKey.replace(/\\n/g, '\n'), | ||
| installationId: Number(installationId), | ||
| }, | ||
| }); |
There was a problem hiding this comment.
To adhere to the general rules, environment variable parsing and normalization (such as converting to numbers and replacing escaped newlines) should not be scattered or performed repeatedly inside request handlers. Instead, centralize this logic into a dedicated configuration helper. This also prevents redundant parsing on every request and ensures that any configuration errors are handled cleanly.
function getRequiredEnvVar(name: string): string {
const value = process.env[name];
if (!value) {
throw new Error('Missing required environment variable: ' + name);
}
return value;
}
interface GitHubConfig {
appId: number;
privateKey: string;
installationId: number;
}
function getGitHubConfig(): GitHubConfig {
const appId = Number(getRequiredEnvVar('GH_APP_ID'));
const privateKey = getRequiredEnvVar('GH_PRIVATE_KEY').replace(/\\n/g, '\n');
const installationId = Number(getRequiredEnvVar('GH_INSTALLATION_ID'));
if (isNaN(appId)) {
throw new Error('GH_APP_ID must be a valid number');
}
if (isNaN(installationId)) {
throw new Error('GH_INSTALLATION_ID must be a valid number');
}
return { appId, privateKey, installationId };
}
export async function handleEgressEvent(event: EgressEvent): Promise<void> {
const config = getGitHubConfig();
const octokit = new Octokit({
authStrategy: createAppAuth,
auth: {
appId: config.appId,
privateKey: config.privateKey,
installationId: config.installationId,
},
});References
- Do not scatter environment variable normalization (such as trimming or case conversion) across multiple files. If environment variables need to be parsed or sanitized, perform this consistently in a single centralized location.
| case 'COMMENT': | ||
| if (!payload.commentBody) { | ||
| throw new Error('Missing commentBody for COMMENT action'); | ||
| } |
There was a problem hiding this comment.
To prevent whitespace-only values from being accepted, trim the string first and then check for emptiness before executing the action. This aligns with the general rules for validating string parameters from tools.
| case 'COMMENT': | |
| if (!payload.commentBody) { | |
| throw new Error('Missing commentBody for COMMENT action'); | |
| } | |
| case 'COMMENT': | |
| if (!payload.commentBody || payload.commentBody.trim() === '') { | |
| throw new Error('Missing commentBody for COMMENT action'); | |
| } |
References
- When validating string parameters from tools, trim the string first and then check for emptiness to prevent whitespace-only values from being accepted.
Summary
Implements the automated caretaker Egress Cloud Run Service into
tools/caretaker-agent/cloudrun/egress-service/. This service receives verified action event messages pushed via Cloud Pub/Sub (egress-actionstopic) from the Triage Worker and executes automated GitHub operations (such as applying effort labels or posting informational comments) using GitHub App Octokit authentication.Details
src/app.ts,src/server.ts): Provides a lightweight Cloud Run HTTP endpoint to handle GCP Pub/Sub push message delivery, decode Base64 payloads, and return structured health status (/GET) for liveness/readiness probes.src/actions/github.ts): Authenticates via GitHub App credentials (@octokit/auth-app) to performCOMMENTandLABELactions on issues using@octokit/rest.src/app.test.ts,src/actions/github.test.ts): Comprehensive unit and router test suites.How to Validate
Pre-Merge Checklist