[VasiliyRad]

The bug

/task composes the task file with f-strings:

```python
f"id: {task_id}\ntimestamp: ...\ntask: {task}\nsource: api\nfrom: {from_agent}\n"
```

from_agent and task are user-supplied (POST body). Neither is sanitized; both interpolate directly into a line-oriented file that downstream consumers parse field-by-field. Two related forgery primitives:

Primitive 1: \n in from_agent forges arbitrary fields.

```bash
curl -X POST .../task -d '{"from": "evil\nchannel_id: local-voice", "task": "..."}'
```

_isVoiceTask in both src/task-bridge.ts and src/discord-bridge.py:poll_dm_fallback does:

```typescript
body.split('\n').some(l => l.startsWith('channel_id: local-voice') || l.startsWith('source: voice'))
```

…and returns True for the forged line. The task is now misclassified as voice-originated. The misclassif routes it through the voice-only fallback path with the wrong downstream behavior (different DM routing, different cleanup semantics) — a path that wasn't audited for API-tier inputs.

Primitive 2: \n in task (which is legitimately multi-line).

Pre-fix field order was id, timestamp, task, source, from. A multi-paragraph task body containing \nsource: voice\n landed BETWEEN the legitimate task: and source: api lines, tripping the same _isVoiceTask spoof — except this attack works without ANY malice; a normal user typing a long task with a colon-prefixed line in the middle can trigger it accidentally.

Why this is the right fix

Two complementary parts that compose with the existing defensive shape of this file (_safe_path, _is_safe_callback_url):

1. Sanitize from_agent. Single-line caller identifier — line terminators have no legitimate use. Strip \r/\n, cap 120 chars, fall back to "unknown" on empty. Standard header-field-sanitization shape; doesn't change semantics for any legitimate caller.

2. Move task: to the LAST line of the file. Multi-line task bodies are LEGITIMATE — users type multi-paragraph descriptions and the bridges (Discord, Telegram) already accept them. Placing task: last means embedded newlines just extend the body rather than landing between fields. The pre-fix order (task in the middle) was an accident, not a design — no consumer relies on it.

Alternative considered: regex-strip newlines from task too. Rejected — it would change observable user-facing behavior (multi-line descriptions get flattened). The field-order fix preserves the multi-line contract while still closing the forgery.

Tests

tests/agent-api-task-field-injection.test.py (6 cases):

Test	Pins
test_from_agent_newline_does_not_forge_voice_field	Pre-fix this FAILS. The forged channel_id: line is absent after sanitization.
test_from_agent_carriage_return_also_stripped	CR-only edge case.
test_from_agent_empty_after_strip_falls_back_to_unknown	Pure-whitespace → "unknown", not bare.
test_task_field_is_last_in_file	Architectural source-grep — a future refactor moving task: earlier fails here.
test_multi_line_task_body_does_not_inject_below	With field order fixed, forged lines in the body land AFTER the task: delimiter.
test_sanitization_caps_overlong_from	10kB from_agent truncates to 120 chars.

Bug-finding calibration

1 real bug fixed across 6 tests. Total this round: 10 bugs / ~105 tests ≈ 1:11.

Scope

This PR fixes ONLY agent-api.py. The Discord and Telegram bridges have the same field-order pattern (task: in the middle); a follow-up should fix them too, paired with verification that no downstream parser depends on the existing order. Flagging for a follow-up to keep this PR small + focused on a single, reviewable boundary.

Test plan

• python3 tests/agent-api-task-field-injection.test.py — 6/6 pass
• Syntax check
• Manual repro: curl -X POST /task -d '{"from":"evil\nchannel_id: local-voice","task":"hi"}', verify resulting task file does NOT make _isVoiceTask return True

🤖 Generated with Claude Code

[bassilkhilo-ag2]

Cold review (proactive-loop pass)

The fix is correct and the two-part approach is the right design call.

Part 1 (sanitize from_agent): replacing \r/\n with space before capping at 120 chars is clean. Using space rather than empty-string-strip is a nice touch — it preserves some signal of the original value while removing injection potential. The or "unknown" fallback for pure-whitespace values is a good defensive edge-case catch.

Part 2 (move task: last): this is exactly the right fix for multi-line task bodies. Stripping newlines from task would change legitimate user behavior (multi-paragraph descriptions). Field-order-last is the correct primitive — same conclusion I reached independently while implementing the same fix for discord-bridge.py and telegram-bridge.py (those bridges had the same vulnerability, task: in the middle; fix is live on branch fix/bridge-task-field-order-injection). The two fixes are complementary — this PR correctly scopes to agent-api.py.

Tests: 6 cases are well-chosen. The test_task_field_is_last_in_file source-grep is particularly important — it's the architectural regression guard that prevents the field-order from silently drifting back. test_multi_line_task_body_does_not_inject_below verifies the actual invariant rather than just the implementation detail.

One observation (non-blocking): the from_agent sanitization replaces newlines with a space rather than stripping them. This means a from value of "agent\nsource: voice" becomes "agent source: voice" (single line, safe). Fine for security; just worth noting that the from: value in the task file may contain embedded spaces that look like they could be two tokens. No downstream parser seems to care about this today.

No blockers. LGTM.

[rickchen007]

Approved (admin) — security fix, reviewed in full. Logic verified, tests deterministic.