Fix proposal breaking page when building an invalid schema #7467
Conversation
Summary of ChangesHello @jdolle, 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 significantly improves the application's stability and user experience by addressing a critical issue where an invalid schema within a proposal would cause the entire page to become unresponsive. The changes introduce a robust error handling mechanism that catches schema build failures, preventing crashes and allowing the application to manage and potentially display these errors gracefully. Additionally, the update expands the system's ability to normalize and track a broader range of schema change types, laying groundwork for more detailed schema insights. 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. 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
|
🚀 Snapshot Release (
|
| Package | Version | Info |
|---|---|---|
@graphql-hive/apollo |
0.46.0-alpha-20260106214739-c701de4e76a3f2c3f370149646a825070785569a |
npm ↗︎ unpkg ↗︎ |
@graphql-hive/cli |
0.57.0-alpha-20260106214739-c701de4e76a3f2c3f370149646a825070785569a |
npm ↗︎ unpkg ↗︎ |
@graphql-hive/core |
0.19.0-alpha-20260106214739-c701de4e76a3f2c3f370149646a825070785569a |
npm ↗︎ unpkg ↗︎ |
@graphql-hive/envelop |
0.40.1-alpha-20260106214739-c701de4e76a3f2c3f370149646a825070785569a |
npm ↗︎ unpkg ↗︎ |
@graphql-hive/yoga |
0.46.1-alpha-20260106214739-c701de4e76a3f2c3f370149646a825070785569a |
npm ↗︎ unpkg ↗︎ |
hive |
8.14.0-alpha-20260106214739-c701de4e76a3f2c3f370149646a825070785569a |
npm ↗︎ unpkg ↗︎ |
hive-apollo-router-plugin |
2.3.6-alpha-20260106214739-c701de4e76a3f2c3f370149646a825070785569a |
npm ↗︎ unpkg ↗︎ |
hive-console-sdk-rs |
0.2.3-alpha-20260106214739-c701de4e76a3f2c3f370149646a825070785569a |
npm ↗︎ unpkg ↗︎ |
📚 Storybook DeploymentThe latest changes are available as preview in: https://pr-7467.hive-storybook.pages.dev |
💻 Website PreviewThe latest changes are available as preview in: https://pr-7467.hive-landing-page.pages.dev |
|
🐋 This PR was built and pushed to the following Docker images: Targets: Platforms: Image Tag: |
![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 prevent page crashes caused by invalid schemas in proposals by gracefully handling errors with a try-catch block around buildSchema. However, the current implementation introduces a client-side Denial of Service vulnerability, as buildSchema and patchSchema are executed on the main thread with potentially malicious user-provided data, which can lead to browser unresponsiveness. It is recommended to address this DoS vulnerability.
| if (beforeSchema) { | ||
| afterSchema = patchSchema(beforeSchema, allChanges, { | ||
| onError(error, change) { | ||
| if (error instanceof NoopError) { | ||
| ignoredChanges.push({ change, error }); | ||
| } | ||
| conflictingChanges.push({ change, error }); | ||
| return errors.looseErrorHandler(error, change); | ||
| }, | ||
| }); | ||
| } else { | ||
| try { | ||
| afterSchema = buildSchema(proposalVersion.schemaSDL, { | ||
| assumeValid: true, | ||
| assumeValidSDL: true, | ||
| }); | ||
| } catch (e: unknown) { | ||
| console.error(e); | ||
| buildError = e as Error; | ||
| } | ||
| } |
There was a problem hiding this comment.
The buildSchema and patchSchema functions, which are called with user-provided schema data, are vulnerable to a client-side Denial of Service (DoS) attack. Maliciously crafted schema data (e.g., deep recursion or large changes) can cause excessive CPU/memory consumption, blocking the main thread and crashing the browser tab. Although a try-catch block is present for buildSchema, it only prevents an uncaught error from crashing the page, not the DoS itself. To mitigate this, it is strongly recommended to run buildSchema and patchSchema inside a Web Worker to offload these potentially long-running tasks from the main thread. Additionally, within the error handling, consider making the type assertion for e safer by checking if e is an instance of Error before accessing its properties, as e as Error can lead to unexpected behavior if a non-Error value is thrown.
2 participants
Background
If a proposal's schema does not build then it throws an uncaught error, causing the entire page to error up to the error boundary.
Description
This catches the buildSchema error to avoid breaking the entire page.
I encountered this on load when the changed schema is fetched before the latest schema, so I added a loading check prior to the patch/build logic, but also added a try/catch to buildSchema which might be excessive and unnecessary. (The checkschema mutation should always validate the schema so no invalid schema should get through.)