Commit e62a2a7
authored
fix(tasks): omit
## Summary
`GET /tasks` (list) returned the full task object, the same
`TaskResponse` the single-task GET returns, so every item included
`params`: the arbitrary, caller-supplied create-time payload. Callers
routinely stash sensitive material there (credentials, tokens, PII).
Because the list is broadly scoped, that turned a single list call into
a bulk read of every task's `params`.
This makes the list a lean summary (`TaskSummary`, no `params`) and
keeps the full record on the single-task fetch. A list response should
carry only what a collection view needs; the full record belongs on the
item fetch.
**Scope:** the list response only. Behavior of `GET /tasks/{id}`, `GET
/tasks/name/{name}`, `POST`/`PATCH`, and the domain/repository layers is
unchanged.
## Endpoint contract (after)
```mermaid
flowchart TD
Client(["Client"])
Client -->|"GET /tasks (list)"| Summary["list of TaskSummary<br/>id, name, status, status_reason,<br/>created_at, updated_at, cleaned_at,<br/>task_metadata, agents"]
Client -->|"GET /tasks/{id} (detail)"| Full["TaskResponse<br/>(all summary fields) + params"]
Summary -. "need params for one task?" .-> Client
Client ==>|"fetch that id"| Full
classDef lean fill:#e8f5e9,stroke:#43a047,color:#1b5e20;
classDef full fill:#fdecea,stroke:#e53935,color:#b71c1c;
class Summary lean;
class Full full;
```
- **List** returns the lean summary for every task, cheap, and safe to
return in bulk.
- **Detail** still returns everything, including `params`, and is
fetched one id at a time (and is subject to the same per-request
authorization as before).
## Schema
`TaskSummary` is a deliberate allowlist. It is a standalone model (not a
subclass of `Task`) so `params` cannot leak in via inheritance, and
because `BaseModel` ignores extra fields, validating a `TaskEntity`
(which still carries `params`) into a `TaskSummary` drops `params` at
serialization.
```mermaid
classDiagram
class TaskSummary {
+id
+name
+status
+status_reason
+created_at
+updated_at
+cleaned_at
+task_metadata
+agents
}
class TaskResponse {
+id
+name
+status
+status_reason
+created_at
+updated_at
+cleaned_at
+task_metadata
+agents
+params
}
note for TaskSummary "list response, omits params"
note for TaskResponse "single-task detail, includes params (may carry credentials or PII)"
```
The two share the same safe fields; only `TaskResponse` exposes
`params`.
## Before / after
```jsonc
// BEFORE - one item in the list response
{
"id": "0c1f8e2a-…",
"name": "…",
"status": "RUNNING",
"created_at": "…", "updated_at": "…", "cleaned_at": null,
"task_metadata": { "display_name": "…" },
"params": {
"auth_token": "…", // whatever the caller stored at create time,
"user_email": "…", // e.g. credentials and PII, returned in bulk
"…": "…"
}
}
// AFTER - same item, lean summary (no params)
{
"id": "0c1f8e2a-…",
"name": "…",
"status": "RUNNING",
"created_at": "…", "updated_at": "…", "cleaned_at": null,
"task_metadata": { "display_name": "…" },
"agents": null
}
```
What changes across a whole list call:
```mermaid
flowchart LR
subgraph Before["Before: GET /tasks"]
direction TB
b["N tasks × full object<br/>N × params (creds/PII)"]
end
subgraph After["After: GET /tasks"]
direction TB
a["N tasks × lean summary<br/>0 × params"]
end
Before --> After
classDef bad fill:#fdecea,stroke:#e53935,color:#b71c1c;
classDef good fill:#e8f5e9,stroke:#43a047,color:#1b5e20;
class Before bad;
class After good;
```
## Field reference
| Field | In list (`TaskSummary`) | In detail (`TaskResponse`) | Notes |
|---|---|---|---|
| `id` | yes | yes | System-assigned task id |
| `name` | yes | yes | Optional, caller-supplied |
| `status` / `status_reason` | yes | yes | Lifecycle state |
| `created_at` / `updated_at` / `cleaned_at` | yes | yes | Timestamps |
| `task_metadata` | yes | yes | Display / routing / filter metadata (the
list can filter on it) |
| `agents` | yes | yes | Populated only with `?relationships=agents` |
| **`params`** | **no** | yes | Arbitrary create-time payload; may carry
credentials/PII |
## Breaking change and migration
Removing `params` from the list is an intentional, security-motivated
breaking change for typed consumers that read `params` off a **list**
item.
```mermaid
flowchart TD
Q{"Reading params off a list item?"}
Q -->|No| OK["No change needed"]
Q -->|Yes| Fix["Fetch the task by id, then read params"]
classDef ok fill:#e8f5e9,stroke:#43a047,color:#1b5e20;
class OK ok;
```
The SDKs are generated from `agentex/openapi.yaml` (Stainless), so the
list-item type regenerates automatically on merge; consumers pick up the
change when they bump the SDK. This PR regenerates and commits
`openapi.yaml` (the `list_tasks` 200 now references `TaskSummary`),
which the `openapi-spec` CI check enforces.
## Consumers touched in this PR
- **Developer UI** (`agentex-ui`): `createTaskName` fell back to
`params.description` for the task label; switched to the task `name`
(`task_metadata.display_name → name → "Unnamed task"`). No other code
reads `params` off a listed task.
## Implementation
- `src/api/schemas/tasks.py`: add `TaskSummary` (allowlist, omits
`params`).
- `src/api/routes/tasks.py`: `list_tasks`
`response_model=list[TaskSummary]`, returns
`TaskSummary.model_validate(...)` per entity. Single-task routes
unchanged.
- `agentex-ui/lib/task-utils.ts` + test: name-derivation fallback.
- `agentex/openapi.yaml`: regenerated.
## Follow-ups / known limitations
- **`task_metadata` is still free-form (`dict[str, Any]`)** and partly
caller-influenced. By convention it holds non-secret
display/routing/filter metadata (and the list can filter on it), so it
stays in the summary. If we want the list to be strict rather than
convention-safe, a follow-up could give `task_metadata` a typed shape so
callers cannot stash sensitive data that the list would surface.
- **Concurrency is out of scope**: unrelated to this change; not touched
here.
## Test plan
```mermaid
flowchart LR
subgraph list["GET /tasks"]
L1["task present in list"]
L2["params NOT in item"]
end
subgraph get["GET /tasks/{id}"]
G1["params present + correct"]
end
list --> get
```
- Integration (`tests/integration/api/tasks/test_tasks_api.py`): list
omits `params` for both populated- and null-`params` tasks; the schema
test asserts `params` absent on list items; single-task GET (by id and
by name) still returns `params`.
- UI unit (`agentex-ui/lib/task-utils.test.ts`): task-name derivation
from `display_name` / `name`.
- `openapi.yaml` regenerated and committed.
- Verified directly that `TaskSummary.model_validate({... "params":
{...}})` drops `params` while `TaskResponse` keeps it; `ruff` clean.
<!-- greptile_comment -->
<h3>Greptile Summary</h3>
This PR fixes an information-disclosure issue where `GET /tasks` (list)
returned the full `TaskResponse` — including `params`, an arbitrary
caller-supplied payload that can hold credentials and PII — for every
task in a single call. The fix introduces a standalone `TaskSummary`
allowlist model that omits `params`, updates the list route's
`response_model`, regenerates the OpenAPI spec, and adapts the UI's
task-name derivation to use `task.name` instead of `params.description`.
- **`TaskSummary`** is a standalone `BaseModel` (not a subclass of
`Task`) so `params` cannot leak via inheritance; Pydantic v2's default
`extra='ignore'` ensures `model_validate` silently drops the field.
- **Single-task routes** (`GET /tasks/{id}`, `GET /tasks/name/{name}`,
lifecycle endpoints) are unchanged and continue to return `params`.
- **UI** now derives the task label from `task_metadata.display_name →
task.name → 'Unnamed task'` instead of the now-absent
`params.description`.
<details><summary><h3>Confidence Score: 5/5</h3></summary>
Safe to merge — the change is a targeted, well-scoped fix that strips a
sensitive field from a bulk endpoint without touching any single-task or
write paths.
The allowlist model is correctly designed as a standalone BaseModel (no
Task inheritance), and Pydantic v2's default extra-field behaviour
ensures params is silently dropped at serialization. The custom
BaseModel in model_utils.py does not set extra='allow', so there is no
bypass path. Single-task routes, lifecycle endpoints, and the domain
layer are untouched. Integration tests verify the before/after contract
for both populated- and null-params tasks, and the UI fallback is
covered by unit tests.
**Files Needing Attention:** No files require special attention.
</details>
<details><summary><h3>Important Files Changed</h3></summary>
| Filename | Overview |
|----------|----------|
| agentex/src/api/schemas/tasks.py | Adds TaskSummary as a standalone
BaseModel allowlist that omits params; correct approach — no inheritance
from Task prevents accidental field leakage. |
| agentex/src/api/routes/tasks.py | list_tasks response_model changed to
list[TaskSummary] and serialization updated; single-task and lifecycle
routes are correctly unchanged. |
| agentex-ui/lib/task-utils.ts | createTaskName fallback updated from
params.description to task.name; correct adaptation to the new
TaskSummary shape. |
| agentex-ui/lib/task-utils.test.ts | Tests updated to cover the new
name-derivation logic (display_name, prefix stripping, task.name
fallback, Unnamed task); coverage looks complete. |
| agentex/tests/integration/api/tasks/test_tasks_api.py | Integration
tests flipped to assert params absent from list items and still present
on single-task GET; covers both populated and null-params tasks. |
| agentex/openapi.yaml | list_tasks 200 response now references
TaskSummary; TaskSummary schema added with correct allowlist fields
matching the Python model. |
</details>
<sub>Reviews (2): Last reviewed commit: ["Merge branch 'main'
into
fix/list-tasks-..."](6977d67)
| [Re-trigger
Greptile](https://app.greptile.com/api/retrigger?id=47039695)</sub>
<!-- /greptile_comment -->params from the list-tasks response (#377)1 parent b5a1d17 commit e62a2a7
6 files changed
Lines changed: 150 additions & 71 deletions
File tree
- agentex-ui/lib
- agentex
- src/api
- routes
- schemas
- tests/integration/api/tasks
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
5 | 5 | | |
6 | 6 | | |
7 | 7 | | |
8 | | - | |
| 8 | + | |
9 | 9 | | |
10 | 10 | | |
11 | | - | |
12 | | - | |
13 | | - | |
14 | | - | |
15 | | - | |
16 | | - | |
17 | | - | |
18 | | - | |
19 | | - | |
20 | | - | |
21 | | - | |
22 | | - | |
23 | | - | |
24 | | - | |
25 | | - | |
26 | | - | |
27 | | - | |
28 | | - | |
| 11 | + | |
| 12 | + | |
29 | 13 | | |
30 | | - | |
| 14 | + | |
31 | 15 | | |
32 | 16 | | |
33 | | - | |
| 17 | + | |
34 | 18 | | |
35 | | - | |
36 | | - | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
37 | 23 | | |
38 | 24 | | |
39 | 25 | | |
40 | | - | |
| 26 | + | |
41 | 27 | | |
42 | 28 | | |
43 | | - | |
| 29 | + | |
44 | 30 | | |
45 | | - | |
46 | | - | |
47 | | - | |
| 31 | + | |
| 32 | + | |
48 | 33 | | |
49 | 34 | | |
50 | | - | |
| 35 | + | |
51 | 36 | | |
52 | 37 | | |
53 | | - | |
| 38 | + | |
54 | 39 | | |
55 | 40 | | |
56 | | - | |
57 | | - | |
58 | | - | |
59 | | - | |
| 41 | + | |
60 | 42 | | |
61 | 43 | | |
62 | 44 | | |
63 | 45 | | |
64 | | - | |
| 46 | + | |
65 | 47 | | |
66 | 48 | | |
67 | | - | |
68 | | - | |
69 | | - | |
70 | | - | |
| 49 | + | |
| 50 | + | |
71 | 51 | | |
72 | 52 | | |
73 | 53 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
20 | 20 | | |
21 | 21 | | |
22 | 22 | | |
23 | | - | |
24 | | - | |
25 | | - | |
26 | | - | |
27 | | - | |
| 23 | + | |
| 24 | + | |
28 | 25 | | |
29 | 26 | | |
30 | 27 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
555 | 555 | | |
556 | 556 | | |
557 | 557 | | |
558 | | - | |
| 558 | + | |
| 559 | + | |
559 | 560 | | |
560 | 561 | | |
561 | 562 | | |
| |||
641 | 642 | | |
642 | 643 | | |
643 | 644 | | |
644 | | - | |
| 645 | + | |
645 | 646 | | |
646 | 647 | | |
647 | 648 | | |
| |||
6845 | 6846 | | |
6846 | 6847 | | |
6847 | 6848 | | |
| 6849 | + | |
| 6850 | + | |
| 6851 | + | |
| 6852 | + | |
| 6853 | + | |
| 6854 | + | |
| 6855 | + | |
| 6856 | + | |
| 6857 | + | |
| 6858 | + | |
| 6859 | + | |
| 6860 | + | |
| 6861 | + | |
| 6862 | + | |
| 6863 | + | |
| 6864 | + | |
| 6865 | + | |
| 6866 | + | |
| 6867 | + | |
| 6868 | + | |
| 6869 | + | |
| 6870 | + | |
| 6871 | + | |
| 6872 | + | |
| 6873 | + | |
| 6874 | + | |
| 6875 | + | |
| 6876 | + | |
| 6877 | + | |
| 6878 | + | |
| 6879 | + | |
| 6880 | + | |
| 6881 | + | |
| 6882 | + | |
| 6883 | + | |
| 6884 | + | |
| 6885 | + | |
| 6886 | + | |
| 6887 | + | |
| 6888 | + | |
| 6889 | + | |
| 6890 | + | |
| 6891 | + | |
| 6892 | + | |
| 6893 | + | |
| 6894 | + | |
| 6895 | + | |
| 6896 | + | |
| 6897 | + | |
| 6898 | + | |
| 6899 | + | |
| 6900 | + | |
| 6901 | + | |
| 6902 | + | |
| 6903 | + | |
| 6904 | + | |
| 6905 | + | |
| 6906 | + | |
| 6907 | + | |
| 6908 | + | |
| 6909 | + | |
| 6910 | + | |
6848 | 6911 | | |
6849 | 6912 | | |
6850 | 6913 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
17 | 17 | | |
18 | 18 | | |
19 | 19 | | |
| 20 | + | |
20 | 21 | | |
21 | 22 | | |
22 | 23 | | |
| |||
73 | 74 | | |
74 | 75 | | |
75 | 76 | | |
76 | | - | |
| 77 | + | |
77 | 78 | | |
78 | | - | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
79 | 83 | | |
80 | 84 | | |
81 | 85 | | |
| |||
143 | 147 | | |
144 | 148 | | |
145 | 149 | | |
146 | | - | |
| 150 | + | |
147 | 151 | | |
148 | 152 | | |
149 | 153 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
74 | 74 | | |
75 | 75 | | |
76 | 76 | | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
77 | 120 | | |
78 | 121 | | |
79 | 122 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
141 | 141 | | |
142 | 142 | | |
143 | 143 | | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
144 | 148 | | |
145 | 149 | | |
146 | 150 | | |
| |||
499 | 503 | | |
500 | 504 | | |
501 | 505 | | |
502 | | - | |
| 506 | + | |
503 | 507 | | |
504 | 508 | | |
505 | | - | |
| 509 | + | |
| 510 | + | |
506 | 511 | | |
507 | 512 | | |
508 | 513 | | |
509 | | - | |
| 514 | + | |
510 | 515 | | |
511 | 516 | | |
512 | 517 | | |
513 | 518 | | |
514 | | - | |
515 | 519 | | |
516 | 520 | | |
517 | 521 | | |
518 | | - | |
519 | | - | |
520 | | - | |
521 | | - | |
522 | | - | |
523 | | - | |
524 | | - | |
525 | | - | |
526 | | - | |
527 | | - | |
| 522 | + | |
| 523 | + | |
528 | 524 | | |
529 | 525 | | |
530 | 526 | | |
| |||
579 | 575 | | |
580 | 576 | | |
581 | 577 | | |
582 | | - | |
| 578 | + | |
583 | 579 | | |
584 | 580 | | |
585 | | - | |
| 581 | + | |
586 | 582 | | |
587 | 583 | | |
588 | 584 | | |
589 | | - | |
| 585 | + | |
590 | 586 | | |
591 | 587 | | |
592 | 588 | | |
593 | | - | |
594 | 589 | | |
595 | 590 | | |
596 | 591 | | |
597 | 592 | | |
598 | | - | |
599 | | - | |
600 | | - | |
601 | | - | |
| 593 | + | |
602 | 594 | | |
603 | 595 | | |
604 | 596 | | |
| |||
0 commit comments