Skip to content

chore(security): CSO diff audit for #2320 + bound the outcome message - #2365

Merged
vybe merged 1 commit into
devfrom
feature/2320-outcome-cap-and-audit
Aug 21, 2026
Merged

chore(security): CSO diff audit for #2320 + bound the outcome message#2365
vybe merged 1 commit into
devfrom
feature/2320-outcome-cap-and-audit

Conversation

@obasilakis

Copy link
Copy Markdown
Contributor

Why this is a separate PR

#2359 was merged while its /cso --diff audit was still running, so these two changes landed on the branch after the squash and never reached dev. Content-only follow-up — no behaviour change.

The audit

No findings at the ≥8/10 gate. #2359 added no endpoint, no auth-dependency change, no SQL, no dependency, no schema change. Phases 3/4/5/8 were N/A (nothing touched requirements, workflows, Dockerfiles, or skills).

Four load-bearing clean verdicts, each traced to a line rather than assumed:

  • Cross-client leakget_history runs agent_on_rosterdb.get_portal_session(session_id, agent_name, email) (uniform 404) before the outcome read. A foreign session_id 404s and never reaches Redis.
  • Prompt injection — the outcome is Redis-only and never a message row, so _format_history_context cannot replay it. This is precisely why the error-row design was rejected in fix(workspace): a failed turn says so, and Retry follows the billing evidence (#2320) #2359: who = "Client" if role == "user" else "You" would have fed a platform error back to the agent as its own words.
  • XSS>{{ m.error }}</p> is Vue interpolation, auto-escaped. The neighbouring v-html is the pre-existing DOMPurify assistant-content path.
  • Double-bill — only busy and capacity are retryable, both meaning nothing reached the agent. retryable defaults False on the exception and the client tests === true, so it is unprivileged at both ends.

Full report: docs/security-reports/cso-diff-2026-08-21-2320-workspace-failed-turn.md.

The one code change

record_turn_outcome now bounds message at 500 chars, matching _fail_unstarted_execution's cap on the row it writes beside (service.py:1902).

Every producer today is a fixed string, so this changes nothing now. It exists because the two writers bound the same content and were bounding it differently — so a later raise site with a long or foreign-derived detail could put unbounded text into Redis and onto a client without anyone noticing the asymmetry.

Recorded, deliberately NOT changed

The auth category tells an external portal client the agent "has reached its usage limit" — no remediation guidance (the issue rules that operator-only), but it does newly reveal billing posture to an untrusted principal where the prior behaviour was a generic "couldn't respond". A neutral alternative ("The agent is temporarily unavailable. Please try again later.") conveys the same operational fact with no billing signal, at the cost of the honesty the issue asks for. Owner's call — flagged, not decided.

Also carried forward from the #2214 audit: the resume-lock TTL is still not resized with the turn bound. Untouched here, still engine-owned.

Trend worth naming

Third consecutive --diff audit on the Workspace turn path (#2214#2258#2320), third with zero gate-passing findings. All three widened what the client is told while leaving auth alone, and each new field rides the same GET .../history. That read now carries four pieces of turn state, with get_history's session-ownership check as its only boundary — healthy so far, but worth knowing it is load-bearing for whatever is added next.

Test Plan

  • unit/test_2320_… + test_ent286_… + test_2133_…80 passed on this base
  • Portal suites incl. ent#428/ent#364 → 192 passed pre-rebase
  • Full frontend suite → 1122 passed

Refs #2320

🤖 Generated with Claude Code

Audit: no findings at the >=8/10 gate. No new endpoint, no auth-dependency
change, no SQL, no dependency, no schema change. The four load-bearing clean
verdicts are traced to lines rather than assumed: the session-ownership gate
runs before the outcome read; the record is Redis-only so it can never re-enter
an agent's context via `_format_history_context`; `m.error` is interpolated, not
`v-html`; and `retryable` is unprivileged-by-default at both ends.

Applies the one below-gate item: `record_turn_outcome` now bounds `message` at
500 chars, matching `_fail_unstarted_execution`'s cap on the row it writes
beside. Every producer today is a fixed string, so nothing changes now — the two
writers bound the same content and should bound it the same way, so a later
raise site with a long or foreign-derived detail cannot widen it silently.

Recorded and NOT changed: the new `auth` category tells an external client the
agent "has reached its usage limit", which newly reveals billing posture to an
untrusted principal. Carries no remediation guidance (the issue rules that
operator-only), but the wording is an owner's call, not a security fix.

Refs #2320

@vybe vybe left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Validated via /validate-pr: content-only CSO follow-up for #2320 — audit report + a 500-char bound on record_turn_outcome matching _fail_unstarted_execution's cap on the sibling row. Behaviour-neutral today (all producers are fixed strings). Scans clean; head-seed pytest + regression diff green (the red base seed 99999 is a cancelled baseline job, not this PR's code). Refs-not-Fixes is correct — #2320 already closed via #2359.

@vybe
vybe merged commit 75c2ca0 into dev Aug 21, 2026
23 of 24 checks passed
vybe pushed a commit that referenced this pull request Aug 21, 2026
…2365

One conflict: src/backend/main.py router mounts — both sides appended at the
same point (rooms routers from main's ent#443, portal_asks_router from dev's
ent#428). Both kept; both mount before register_enterprise(app).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants