Add studio.json as an alternative to metadata.json#10061
Add studio.json as an alternative to metadata.json#10061christhompsongoogle merged 4 commits intomainfrom
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 enhances the flexibility of configuration file handling by allowing the system to recognize Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
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 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 counter productive. 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. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request aims to add support for studio.json as an alternative to metadata.json. However, the current implementation introduces a significant logic error in the metadata extraction process. By using forEach with an async callback, the code fails to wait for metadata files to be read, which can lead to security risks such as uploading secrets to the wrong Firebase project if a fallback project ID is used from .firebaserc. This also creates a race condition, preventing metadata from being loaded correctly and ignoring the intended preference for studio.json.
src/firebase_studio/migrate.ts
Outdated
| [metadataPath, studioJsonPath].forEach(async (path) => { | ||
| try { | ||
| const metadataContent = await fs.readFile(path, "utf8"); | ||
| metadata = JSON.parse(metadataContent) as Metadata; | ||
| } catch (err: unknown) { | ||
| logger.debug(`Could not read metadata.json at ${path}: ${err}`); | ||
| } | ||
| }); |
There was a problem hiding this comment.
The extractMetadata function uses a forEach loop with an async callback to read metadata from studio.json or metadata.json. This approach is problematic because forEach does not await the completion of asynchronous operations. As a result, the metadata object remains empty when the code proceeds to determine the projectId on line 127. This can cause the migration process to ignore the project ID specified in the metadata files and potentially fall back to an incorrect or malicious project ID from .firebaserc, leading to sensitive information, such as the GEMINI_API_KEY from .env, being uploaded to the wrong Firebase project. Additionally, the intended preference for studio.json is not guaranteed due to a race condition between the file reads. The variable metadataFile inside the loop also shadows the imported path module, and the debug log message hardcodes "metadata.json" instead of using the variable metadataFile.
| [metadataPath, studioJsonPath].forEach(async (path) => { | |
| try { | |
| const metadataContent = await fs.readFile(path, "utf8"); | |
| metadata = JSON.parse(metadataContent) as Metadata; | |
| } catch (err: unknown) { | |
| logger.debug(`Could not read metadata.json at ${path}: ${err}`); | |
| } | |
| }); | |
| for (const metadataFile of [metadataPath, studioJsonPath]) { | |
| try { | |
| const metadataContent = await fs.readFile(metadataFile, "utf8"); | |
| metadata = JSON.parse(metadataContent) as Metadata; | |
| } catch (err: unknown) { | |
| logger.debug(`Could not read metadata.json at ${metadataFile}: ${err}`); | |
| } | |
| } |
| for (const metadataFile of [metadataPath, studioJsonPath]) { | ||
| try { | ||
| const metadataContent = await fs.readFile(metadataFile, "utf8"); | ||
| metadata = JSON.parse(metadataContent) as Metadata; |
There was a problem hiding this comment.
Can we add a debug log that makes it explicit which file is used? Right now, i could see folks getting confused by this logging.
3 participants
Add studio.json as an alternative