fix(a2a-server): add missing return after 501 in /tasks/metadata endpoint#24293
fix(a2a-server): add missing return after 501 in /tasks/metadata endpoint#24293garagon wants to merge 2 commits intogoogle-gemini:mainfrom
Conversation
…oint The handler sends a 501 when the task store is not InMemoryTaskStore but does not return. Execution falls through to the try block which sends a second response, triggering ERR_HTTP_HEADERS_SENT and crashing the server. Fixes google-gemini#21729
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 a critical issue where the server would attempt to send multiple HTTP responses when accessing the task metadata endpoint with an unsupported task store. By ensuring the function returns immediately after sending a 501 error, the server avoids subsequent code execution that leads to runtime crashes. 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 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. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request adds a missing return statement to the /tasks/metadata endpoint in the a2a-server package to prevent execution from continuing when an unsupported task store is used. However, a high-severity security issue was identified: the endpoint lacks authentication and authorization, potentially exposing sensitive task metadata to unauthenticated users.
| error: | ||
| 'Listing all task metadata is only supported when using InMemoryTaskStore.', | ||
| }); | ||
| return; |
There was a problem hiding this comment.
The /tasks/metadata endpoint lacks any authentication or authorization checks. While this PR correctly adds a return statement to prevent a server crash (DoS) when using a non-in-memory task store, the endpoint remains publicly accessible. Any unauthenticated user can retrieve metadata for all tasks currently in memory, which may include sensitive information such as workspace paths and agent settings for all users. It is recommended to apply authentication middleware to this route and ensure that users can only access metadata for tasks they own.
There was a problem hiding this comment.
Code Review
This pull request adds a missing return statement to the /tasks/metadata endpoint in the a2a-server package, ensuring that execution stops after an error response is sent. A security review identified a high-severity vulnerability in this same endpoint, noting that it lacks proper authorization checks and could allow users to access sensitive metadata for tasks they do not own.
| error: | ||
| 'Listing all task metadata is only supported when using InMemoryTaskStore.', | ||
| }); | ||
| return; |
There was a problem hiding this comment.
The /tasks/metadata endpoint lacks proper authorization checks. It returns metadata for all tasks currently in the InMemoryTaskStore, which includes sensitive information such as workspacePath and other agent settings. An authenticated user can access this information for all tasks, not just their own, leading to unauthorized information disclosure.
Remediation: Implement an authorization check to ensure that the user requesting the metadata is authorized to view it. Typically, this involves filtering the tasks to only include those owned by the current user or requiring administrative privileges to list all tasks.
There was a problem hiding this comment.
Code Review
This pull request adds a missing return statement in the createApp function within packages/a2a-server/src/http/app.ts. This ensures that execution stops after an error response is sent when the task store is not an InMemoryTaskStore, preventing subsequent code from running unexpectedly. I have no feedback to provide.
|
Please address the gemini comments. |
Problem
GET /tasks/metadatasends a 501 when using a non-InMemory task store (e.g.,GCSTaskStore) but does not return. Execution falls through to thetryblock which attempts to send a second response, triggeringERR_HTTP_HEADERS_SENTand crashing the server.Fix
Add
returnafter the 501 response.Previous attempts (#21730, #21947) were closed for administrative reasons. The bug is still present on main.
Fixes #21729