fix(core): validate GCP project ID format and prevent alias extraction in memory#27916
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
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 addresses issues where the auto-memory agent incorrectly stored GCP project display names or aliases as project IDs, leading to authentication and API errors. By enforcing strict format validation during setup and refining the agent's extraction logic, the system now ensures that only valid project identifiers are processed, improving overall reliability and preventing downstream API failures. 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/M
|
There was a problem hiding this comment.
Code Review
This pull request introduces stricter validation for Google Cloud Project IDs during setup, adding an InvalidProjectIdFormatError and format checks, alongside corresponding unit tests and system prompt updates. The reviewer feedback recommends refining the validation regex to strictly distinguish between standard GCP project IDs (which must start with a lowercase letter and be 6 to 30 characters long) and domain-scoped project IDs, as well as adding additional test cases to verify these stricter boundaries.
| const validProjectIdRegex = /^[a-z0-9][a-z0-9\-.:]{4,98}[a-z0-9]$/; | ||
| if (!validProjectIdRegex.test(projectId)) { | ||
| throw new InvalidProjectIdFormatError(projectId); | ||
| } |
There was a problem hiding this comment.
The current regular expression /^[a-z0-9][a-z0-9\-.:]{4,98}[a-z0-9]$/ is too permissive for standard GCP project IDs:
- It allows standard project IDs to start with a number (e.g.,
1-project), whereas GCP standard project IDs must start with a lowercase letter. - It allows standard project IDs to be up to 100 characters long, whereas GCP limits standard project IDs to 30 characters.
To prevent confusing API failures (which this PR aims to solve), we should validate standard and domain-scoped project IDs more precisely.
Here is a suggested improvement that separates the validation logic:
- Standard project IDs: 6 to 30 characters, starts with a lowercase letter, ends with a lowercase letter or number, contains only lowercase letters, numbers, and hyphens.
- Domain-scoped project IDs: up to 100 characters, contains a domain prefix followed by a colon and a standard project ID.
const isDomainScoped = projectId.includes(':');
const isValid = isDomainScoped
? projectId.length <= 100 && /^[a-z0-9\-.]+\:[a-z][a-z0-9-]{4,28}[a-z0-9]$/.test(projectId)
: /^[a-z][a-z0-9-]{4,28}[a-z0-9]$/.test(projectId);
if (!isValid) {
throw new InvalidProjectIdFormatError(projectId);
}References
- When handling user input, prefer to be strict and throw an error for invalid or ambiguous cases rather than adding complex logic to support them.
e7dabca to
debed2b
Compare
|
Hi team, I have addressed all the feedback raised by the Gemini Code Assist review:
Could a maintainer please approve the workflow run so the CI checks can execute? Ready for review. Thank you! |
What does this PR do?
Fixes GCP project ID validation to prevent auto-memory from storing invalid display names/aliases, which caused 403 and CONSUMER_INVALID errors in subsequent sessions.
Why this change matters
Prevents confusing API failures for users who have GCP project display names with spaces or uppercase letters.
Breaking changes
None
Summary
This PR resolves issue #27911 where the auto-memory agent incorrectly extracted and stored GCP project display names/aliases (e.g. including spaces or uppercase letters) as actual project IDs. When subsequent sessions or
/authcommands loaded this invalid project ID, all Google Cloud API calls failed with confusing 403 andCONSUMER_INVALIDerrors.To address this, we added early regex-based validation for Google Cloud Project IDs during configuration setup and updated the memory extraction prompt to prevent storing invalid display names/aliases as project IDs.
Details
InvalidProjectIdFormatErrorinsetup.ts./^[a-z][a-z0-9-]{4,28}[a-z0-9]$/(6–30 characters, starting with a lowercase letter, containing only lowercase alphanumeric and hyphens)./^[a-z0-9\-.]+\:[a-z][a-z0-9-]{4,28}[a-z0-9]$/(up to 100 characters with domain prefix followed by colon).skill-extraction-agent.tsto explicitly prohibit the agent from extracting and storing project aliases, display names, or formatted titles as GCP project IDs.setup.test.tscovering standard IDs starting with numbers or exceeding 30 characters being rejected, happy paths (valid and domain-scoped), and error paths (spaces, uppercase, underscores, trailing hyphens).p1andp2) to use valid formats (project-1andproject-2) to keep checking behaviors consistent.Related Issues
Closes #27911
How to Validate
Pre-Merge Checklist