a2aproject / a2aproject/a2a-rs
a2a-server: historyLength, idempotent cancel, and an authorize callback
- Lingua principale
- Rust
- Stelle
- 75
- Fork
- 19
- Merge medio
- 11h 27m
- PR unite (30g)
- 21
Descrizione
`DefaultRequestHandler` still has three interop/security gaps that other SDKs already closed. #123 tried to fix them in one patch. Review on that PR is still open, and the authentication piece as written is not safe to take.
## What is wrong today
1. **`get_task` ignores `historyLength`.** `list_tasks` truncates; `get_task` returns the full history. A client that asks for `historyLength=1` still sees every prior message (information disclosure vs Python/JS/Go).
2. **Negative `historyLength` is not normalized on the list path.** The in-memory store casts `i32` to `usize`, so `-1` wraps and returns full history. `get` and `list` must share one clamp (`<= 0` → empty).
3. **`cancel_task` on a terminal task returns `TASK_NOT_CANCELABLE`.** TypeScript and Go treat cancel-on-terminal as idempotent and return the current task. A retry after a timeout then fails only on Rust.
4. **No authorization hook on handler methods.** `ServiceParams` are passed through and ignored. Any caller can `get_task` / `list_tasks` / `cancel_task` (CWE-862).
## What #123 got right, and what it did not
Keep from #123:
- apply `historyLength` on `get_task`
- treat cancel-on-terminal as success
- opt-in so default deployments do not change
Do not take from #123:
- `require_authentication()` that only checks a non-empty `Authorization` value. `Authorization: arbitrary` would pass. That is a header-presence gate, not authentication, and it does not fix CWE-862.
- a lowercase-only `"authorization"` lookup. SlimRPC copies metadata keys as-is; a valid client can be rejected.
## What the replacement should do
- One shared `historyLength` normalization used by `get_task` and `list_tasks`. Prefer in-place truncate on an owned task (no extra history clone).
- Idempotent `cancel_task` when the loaded task is already terminal.
- An **authorizer callback** on `DefaultRequestHandler`, for example `authorize(&ServiceParams, Option<&task_id>) -> Result<(), A2AError>`. Default is a no-op. The handler must not invent a principal by testing that a header exists.
- Header-key comparison, if an implementation looks at headers, must be case-insensitive.
- `task_id` is reserved for later owner/tenant checks; the first cut only needs the callback seat.
A presence check cannot be that seat. Applications that already validate a token, DID, or session implement the callback with that result.
## Plan
1. Land the three handler fixes in one `a2a-server` PR against this issue.
2. Close #123 in favor of that PR (credit the original `historyLength` / cancel work there).
3. Callers that need real admit implement the callback. They do not call a built-in "require header" helper as authentication.
## Test plan
- [ ] `get_task` with `historyLength=1` / `0` / `-1` matches `list_tasks`
- [ ] cancel on COMPLETED/CANCELED/FAILED returns the current terminal task
- [ ] default handler still accepts requests with empty `ServiceParams`
- [ ] a callback that rejects unauthenticated params is invoked on send, get, list, cancel, subscribe, and push-config methods
- [ ] existing `a2a-server` unit tests stay green
Guida per i contributori
Apri la guida per i contributori
Valutazione
Questa issue non è ancora stata valutata.