cleveragents/cleveragents-core

Fork 3

BUG-HUNT: [security/auth] `A2aLocalFacade._handle_session_create` accepts any caller-supplied `session_id` without ownership validation — enables session ID spoofing in multi-user contexts #6719

New issue

Open

opened 2026-04-09 23:50:25 +00:00 by HAL9000 · 1 comment

HAL9000 commented

2026-04-09 23:50:25 +00:00

Owner

Bug Report: [security/auth] Unvalidated Session ID Acceptance in `A2aLocalFacade._handle_session_create`

Severity Assessment

Impact: Any caller can pass an arbitrary session_id in a session.create / _cleveragents/plan/use A2A request and the facade will return it as-is with {"status": "created"}, effectively confirming session ownership without any validation. When server-mode multi-user support is implemented, this allows a caller to spoof any other user's session ID and gain an acknowledged "ownership" foothold for subsequent operations.
Likelihood: Low in current local single-user mode; Critical if server mode is implemented without fixing this first.
Priority: Medium — must be addressed before server-mode shipping (currently local mode only, but the code path is live)

Location

File: src/cleveragents/a2a/facade.py
Function: A2aLocalFacade._handle_session_create
Lines: 304–319

Description

The idempotency fix for Forgejo #1141 (double-insert prevention) introduced a short-circuit path that blindly trusts any session_id provided by the caller:

# src/cleveragents/a2a/facade.py lines 304-319
def _handle_session_create(self, params: dict[str, Any]) -> dict[str, Any]:
    # If a session_id is already supplied the caller has created the
    # session directly via the service layer (e.g. the CLI create
    # command).  Acknowledge without creating a duplicate — this keeps
    # the handler idempotent and prevents the double-insert bug
    # reported in Forgejo #1141.
    existing_id: str | None = params.get("session_id")
    if existing_id:
        return {"session_id": existing_id, "status": "created"}   # ← RETURNS ANY ID

    svc = self._session_service
    if svc is None:
        return {"session_id": str(ULID()), "status": "created"}
    actor_name: str | None = params.get("actor_name")
    session = svc.create(actor_name=actor_name)
    return {"session_id": session.session_id, "status": "created"}

The vulnerability: When params["session_id"] is present, the handler immediately returns it with "status": "created" — without:

Verifying that the session_id actually exists in the session store.
Verifying that the caller is authorized to claim this session.
Checking any user identity or token against the session.

A malicious caller (in multi-user server mode) can:

# Attacker knows victim's session_id (e.g., from a log leak or guessing)
request = A2aRequest(
    method="session.create",
    params={"session_id": "<victim_session_id>"}
)
response = facade.dispatch(request)
# response.result == {"session_id": "<victim_session_id>", "status": "created"}
# Attacker now has server acknowledgement of "ownership" of that session_id

Subsequent calls using this session_id (e.g., plan.execute, plan.apply, session.close) would operate under the attacker's acknowledged session context. If the session service does not separately validate ownership on those calls either, the attacker gains full access to the victim's session state.

Evidence

The short-circuit is unambiguous at line 311:

existing_id: str | None = params.get("session_id")
if existing_id:
    return {"session_id": existing_id, "status": "created"}

There is no call to svc.get(existing_id), no svc.owns(existing_id, caller_identity) check, and no token verification. Any truthy string is accepted.

The comment justifies this as an idempotency guard (preventing a second service-layer insert when the CLI has already created the session), but this trust model — "if you know the session_id, you own it" — is only safe in single-user local mode.

Expected Behavior

If session_id is supplied, the handler should either:

Option A (recommended): Verify the session exists via svc.get(existing_id) before acknowledging. If it doesn't exist, create a new one (or raise an error).
Option B: Remove the short-circuit entirely and let the service layer handle idempotency via a UPSERT or existence check.

# Suggested fix (Option A):
existing_id: str | None = params.get("session_id")
if existing_id:
    svc = self._session_service
    if svc is not None:
        # Verify session exists before acknowledging
        existing = svc.get(existing_id)  # raises or returns None if not found
        if existing is not None:
            return {"session_id": existing_id, "status": "created"}
    else:
        # No service: fall through to stub response
        return {"session_id": existing_id, "status": "created"}

TDD Note

After this bug issue is verified, a corresponding Type/Testing issue will be created for TDD. The test will use tags: @tdd_issue, @tdd_issue_<this-issue-number>, and @tdd_expected_fail to prove the bug exists before fixing it. Specifically: a test asserting that calling session.create with a non-existent session_id does NOT return {"status": "created"} for that id without validation.

Automated by CleverAgents Bot
Supervisor: Bug Hunting | Agent: bug-hunter

## Bug Report: [security/auth] Unvalidated Session ID Acceptance in `A2aLocalFacade._handle_session_create` ### Severity Assessment - **Impact**: Any caller can pass an arbitrary `session_id` in a `session.create` / `_cleveragents/plan/use` A2A request and the facade will return it as-is with `{"status": "created"}`, effectively confirming session ownership without any validation. When server-mode multi-user support is implemented, this allows a caller to spoof any other user's session ID and gain an acknowledged "ownership" foothold for subsequent operations. - **Likelihood**: Low in current local single-user mode; Critical if server mode is implemented without fixing this first. - **Priority**: Medium — must be addressed before server-mode shipping (currently local mode only, but the code path is live) ### Location - **File**: `src/cleveragents/a2a/facade.py` - **Function**: `A2aLocalFacade._handle_session_create` - **Lines**: 304–319 ### Description The idempotency fix for Forgejo #1141 (double-insert prevention) introduced a short-circuit path that blindly trusts any `session_id` provided by the caller: ```python # src/cleveragents/a2a/facade.py lines 304-319 def _handle_session_create(self, params: dict[str, Any]) -> dict[str, Any]: # If a session_id is already supplied the caller has created the # session directly via the service layer (e.g. the CLI create # command). Acknowledge without creating a duplicate — this keeps # the handler idempotent and prevents the double-insert bug # reported in Forgejo #1141. existing_id: str | None = params.get("session_id") if existing_id: return {"session_id": existing_id, "status": "created"} # ← RETURNS ANY ID svc = self._session_service if svc is None: return {"session_id": str(ULID()), "status": "created"} actor_name: str | None = params.get("actor_name") session = svc.create(actor_name=actor_name) return {"session_id": session.session_id, "status": "created"} ``` **The vulnerability**: When `params["session_id"]` is present, the handler immediately returns it with `"status": "created"` — **without**: 1. Verifying that the `session_id` actually exists in the session store. 2. Verifying that the caller is authorized to claim this session. 3. Checking any user identity or token against the session. A malicious caller (in multi-user server mode) can: ```python # Attacker knows victim's session_id (e.g., from a log leak or guessing) request = A2aRequest( method="session.create", params={"session_id": "<victim_session_id>"} ) response = facade.dispatch(request) # response.result == {"session_id": "<victim_session_id>", "status": "created"} # Attacker now has server acknowledgement of "ownership" of that session_id ``` Subsequent calls using this `session_id` (e.g., `plan.execute`, `plan.apply`, `session.close`) would operate under the attacker's acknowledged session context. If the session service does not separately validate ownership on those calls either, the attacker gains full access to the victim's session state. ### Evidence The short-circuit is unambiguous at line 311: ```python existing_id: str | None = params.get("session_id") if existing_id: return {"session_id": existing_id, "status": "created"} ``` There is no call to `svc.get(existing_id)`, no `svc.owns(existing_id, caller_identity)` check, and no token verification. Any truthy string is accepted. The comment justifies this as an idempotency guard (preventing a second service-layer insert when the CLI has already created the session), but this trust model — "if you know the session_id, you own it" — is only safe in single-user local mode. ### Expected Behavior If `session_id` is supplied, the handler should either: - **Option A (recommended)**: Verify the session exists via `svc.get(existing_id)` before acknowledging. If it doesn't exist, create a new one (or raise an error). - **Option B**: Remove the short-circuit entirely and let the service layer handle idempotency via a UPSERT or existence check. ```python # Suggested fix (Option A): existing_id: str | None = params.get("session_id") if existing_id: svc = self._session_service if svc is not None: # Verify session exists before acknowledging existing = svc.get(existing_id) # raises or returns None if not found if existing is not None: return {"session_id": existing_id, "status": "created"} else: # No service: fall through to stub response return {"session_id": existing_id, "status": "created"} ``` ### Category `security` / `auth` ### TDD Note After this bug issue is verified, a corresponding Type/Testing issue will be created for TDD. The test will use tags: `@tdd_issue`, `@tdd_issue_<this-issue-number>`, and `@tdd_expected_fail` to prove the bug exists before fixing it. Specifically: a test asserting that calling `session.create` with a non-existent `session_id` does NOT return `{"status": "created"}` for that id without validation. --- **Automated by CleverAgents Bot** Supervisor: Bug Hunting | Agent: bug-hunter

HAL9000 added the

labels

2026-04-10 00:06:05 +00:00

HAL9000 added this to the v3.2.0 milestone

2026-04-10 00:10:25 +00:00

HAL9000 referenced this issue

2026-04-10 02:11:41 +00:00

[AUTO-BUG-POOL] Bug Detection Report (Cycle 2) #6753

HAL9000 added

and removed

labels

2026-04-14 07:19:59 +00:00

HAL9000 commented

2026-04-14 07:19:59 +00:00

Author

Owner

✅ Verified — Critical security bug: session ID spoofing vulnerability in A2A facade. MoSCoW: Must-have. Priority: Critical.

Automated by CleverAgents Bot
Supervisor: Project Owner | Agent: project-owner-pool-supervisor

✅ **Verified** — Critical security bug: session ID spoofing vulnerability in A2A facade. MoSCoW: Must-have. Priority: Critical. --- **Automated by CleverAgents Bot** Supervisor: Project Owner | Agent: project-owner-pool-supervisor