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 addresses a critical bug where a2a.Task events could be inadvertently modified after being saved, leading to inconsistent state for consumers. By introducing a deep copy operation, the system now ensures that once a task is processed and saved, its state remains immutable, thereby guaranteeing data integrity and preventing race conditions in concurrent processing environments. Highlights
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
|
There was a problem hiding this comment.
Code Review
This pull request aims to fix a bug where an a2a.Task object could be mutated after being saved by introducing a deep copy of the task event. However, the fix for the in-place mutation is incomplete as the updateArtifact method still performs in-place modifications. Additionally, a high-severity Insecure Direct Object Reference (IDOR) vulnerability was identified in the InMemory task store (a2asrv/taskstore/inmemory.go), where the Get and Update methods lack authorization checks, allowing any authenticated user to read or modify tasks belonging to others if they know the task ID.
| } | ||
| if result2.Task.Status.State != a2a.TaskStateWorking { | ||
| t.Fatalf("previous result state changed to = %q, want = %q", result2.Task.Status.State, a2a.TaskStateWorking) | ||
| } |
There was a problem hiding this comment.
nit: would it make sense to check the original task's status here as well?
There was a problem hiding this comment.
decided such check would be overly paranoid 🙂
🤖 I have created a release *beep* *boop* --- ## [1.0.0](v1.0.0-alpha.3...v1.0.0) (2026-03-17) ### Features * implement the new rest error handling ([#282](#282)) ([a3bda30](a3bda30)) * use v2 suffix for module ID and provide compat support ([#270](#270)) ([dd1b6ba](dd1b6ba)), closes [#250](#250) ### Bug Fixes * a2asrv jsonrpc Content-Type ([#265](#265)) ([2568a46](2568a46)) * bugs before going from alpha ([#279](#279)) ([b1f055c](b1f055c)) * GetTaskRequest nil pointer assignment check ([#258](#258)) ([440bb79](440bb79)) * inject headers into service params ([#277](#277)) ([d33f3bd](d33f3bd)), closes [#275](#275) * propagate cancelation signal using task store ([#272](#272)) ([5e1d462](5e1d462)), closes [#245](#245) * regenerate spec and fix returnImmediately ([#284](#284)) ([2eee0b9](2eee0b9)) * task modified after save ([#266](#266)) ([c15febe](c15febe)) * taskupdater result mutable ([#274](#274)) ([6038d92](6038d92)) * update pushsender ([#256](#256)) ([5f7a594](5f7a594)) * use enum values as in the spec ([#261](#261)) ([eb98981](eb98981)), closes [#251](#251) ### Documentation * **a2asrv:** add Example_* test functions for pkg.go.dev documentation ([#262](#262)) ([7888e37](7888e37)) * add example tests a2a ([#240](#240)) ([4fe08a9](4fe08a9)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
We were capturing a reference to a2a.Task event.
With in-memory consumers this meant that queue readers could potentially see incorrect task state: