diff --git a/workspace/backend/alembic/versions/040_add_member_display_name.py b/workspace/backend/alembic/versions/040_add_member_display_name.py new file mode 100644 index 000000000..389da9531 --- /dev/null +++ b/workspace/backend/alembic/versions/040_add_member_display_name.py @@ -0,0 +1,28 @@ +# -*- coding: utf-8 -*- +"""Add workspace_members.display_name — user-set label for an agent. + +Revision ID: 040 +Revises: 039 +Create Date: 2026-08-18 + +Any script is allowed (Chinese, emoji, ...). agent_name remains the ASCII +identity used for @mentions, routing and storage keys; UIs fall back to it +when display_name is null. +""" + +from alembic import op +import sqlalchemy as sa + + +revision = "040" +down_revision = "039" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + op.add_column("workspace_members", sa.Column("display_name", sa.Text(), nullable=True)) + + +def downgrade() -> None: + op.drop_column("workspace_members", "display_name") diff --git a/workspace/backend/app/models.py b/workspace/backend/app/models.py index f7bd18ebc..b53577901 100644 --- a/workspace/backend/app/models.py +++ b/workspace/backend/app/models.py @@ -107,6 +107,9 @@ class WorkspaceMember(Base): workspace_id = Column(UUID(as_uuid=False), ForeignKey("workspaces.id", ondelete="CASCADE"), nullable=False) agent_name = Column(Text, nullable=False) + # Free-form label shown in UIs (any script, incl. CJK); agent_name stays + # the ASCII identity used for mentions, routing and storage keys. + display_name = Column(Text, nullable=True) role = Column(Text, default="member") # master | member | observer agent_type = Column(Text, nullable=True) # "claude", "openclaw", etc. server_host = Column(Text, nullable=True) # hostname/IP where agent runs diff --git a/workspace/backend/app/mods/workspace_mod.py b/workspace/backend/app/mods/workspace_mod.py index c67f8d75a..75bd2afdf 100644 --- a/workspace/backend/app/mods/workspace_mod.py +++ b/workspace/backend/app/mods/workspace_mod.py @@ -53,6 +53,8 @@ async def _handle_agent_join(event: Event, ctx: PipelineContext) -> Optional[Eve import uuid as _uuid from app.models import WorkspaceMember + from app import naming + db = ctx.extra["db"] workspace = ctx.extra["workspace"] agent_name = event.payload.get("agent_name") if event.payload else None @@ -60,6 +62,23 @@ async def _handle_agent_join(event: Event, ctx: PipelineContext) -> Optional[Eve logger.warning("workspace_mod: agent.join missing agent_name in payload") return None + # agent_name is inserted verbatim into router prompts and participant + # lists — apply the shared character policy here (post-auth), covering + # /v1/join and raw /v1/events alike. + name_problem = naming.agent_name_problem(agent_name) + if name_problem: + logger.info( + "workspace_mod: refused join in %s — %s", workspace.id, name_problem, + ) + event.metadata["reject_reason"] = "invalid_agent_name" + event.metadata["reject_detail"] = f"Invalid agent name: {name_problem}" + raise EventRejected("workspace_mod", "invalid_agent_name") + + # Take the namespace lock BEFORE reading membership: two concurrent joins + # of the same name could otherwise both see existing=None and collide on + # the primary key instead of the second one rotating the session. + naming.lock_member_namespace(db, workspace.id) + existing = db.execute( select(WorkspaceMember).where( WorkspaceMember.workspace_id == workspace.id, @@ -109,7 +128,31 @@ async def _handle_agent_join(event: Event, ctx: PipelineContext) -> Optional[Eve agent_name, workspace.id, ) else: + # New member: its agent_name enters the shared name/alias namespace, + # so it must not equal another member's display_name. Runs after + # AuthMod, under the namespace lock taken above. + clash = naming.find_alias_clash( + db, workspace.id, agent_name, exclude_agent=agent_name, + ) + if clash: + logger.info( + "workspace_mod: refused join of %s in %s — clashes with display name of %s", + agent_name, workspace.id, clash, + ) + event.metadata["reject_reason"] = "display_name_conflict" + event.metadata["reject_detail"] = ( + f"Agent name '{agent_name}' conflicts with the display name " + f"of member '{clash}'" + ) + raise EventRejected("workspace_mod", "display_name_conflict") + + # Role is caller-supplied (raw /v1/events can claim anything, + # including non-strings that would make the frozenset lookup throw) — + # whitelist it so it can't smuggle text into prompts or grant an + # unknown role. role = event.payload.get("role", "member") + if not isinstance(role, str) or role not in naming.ALLOWED_ROLES: + role = "member" member = WorkspaceMember( workspace_id=workspace.id, agent_name=agent_name, @@ -129,7 +172,7 @@ async def _handle_agent_join(event: Event, ctx: PipelineContext) -> Optional[Eve # Enrich event metadata with resolved info + session_id so the # router returns it to the joining client. - event.metadata["role"] = existing.role if existing else event.payload.get("role", "member") + event.metadata["role"] = existing.role if existing else role event.metadata["network_id"] = str(workspace.id) event.metadata["session_id"] = new_session_id return event @@ -682,6 +725,18 @@ def _master_targets(event, channel, mentions: List[str]) -> List[str]: return [master] +def _prompt_inline(text: str) -> str: + """Flatten user-controlled text for safe inline use in the router prompt. + + Display names and descriptions come from users; control characters or + Unicode line/paragraph separators in them could forge extra + participant/instruction lines. Delegates to the shared Unicode-aware + sanitizer (Cc/Zl/Zp + bidi controls → spaces). + """ + from app.naming import sanitize_inline + return sanitize_inline(text) + + _ROUTER_PROMPT = """\ You are a conversation router for a multi-agent workspace. Decide which \ agent should respond next to the LATEST message. Use judgment — read the \ @@ -836,7 +891,7 @@ async def _route_with_llm( else: label = source text = (payload.get("content") or "")[:500] # Truncate long messages - history_lines.append(f"[{label}] {text}") + history_lines.append(f"[{_prompt_inline(label)}] {text}") history = "\n".join(history_lines) if history_lines else "(no prior messages)" @@ -861,21 +916,31 @@ async def _route_with_llm( if members.get(n) and _member_is_online(members[n]) } candidate_names = [n for n in participant_names if n in online_set] if online_set else participant_names + # Every field below is user-controlled at some entry point (legacy rows + # predate the join-time character policy), so flatten them all — a value + # must never span lines or the prompt structure can be forged. participant_lines = [] for name in candidate_names: m = members.get(name) - role = m.role if m else "member" - desc = m.description if m and m.description else "" - line = f" - {name} (role: {role})" + role = _prompt_inline(str(m.role)) if m and m.role else "member" + desc = _prompt_inline(m.description) if m and m.description else "" + line = f" - {_prompt_inline(name)} (role: {role})" + # Users may address an agent by its display name ("小明, 帮我看下") + # rather than its ASCII agent name — give the router the alias. The + # output contract stays next:. + alias = _prompt_inline(m.display_name) if m and m.display_name else "" + if alias and alias != name: + line += f" (also known as: {alias})" if desc: line += f" — {desc}" participant_lines.append(line) participants_str = "\n".join(participant_lines) if participant_lines else " (none)" - master = channel.master_agent or "(none)" + master = _prompt_inline(channel.master_agent) or "(none)" sender = new_event.source if sender.startswith("openagents:"): sender = sender[len("openagents:"):] + sender = _prompt_inline(sender) content = (new_event.payload or {}).get("content", "")[:500] diff --git a/workspace/backend/app/naming.py b/workspace/backend/app/naming.py new file mode 100644 index 000000000..3cc7fca1a --- /dev/null +++ b/workspace/backend/app/naming.py @@ -0,0 +1,128 @@ +# -*- coding: utf-8 -*- +"""Workspace member naming domain. + +Agent names and display names share ONE namespace per workspace: display +names are routable aliases (the LLM router and the @mention picker resolve +them), so any writer of either field must go through these helpers — the +member PATCH, the agent-join event handler, cloud-agent creation, the OAuth +callback and the Yumi backfill. +""" + +import unicodedata +from typing import Optional + +from sqlalchemy import select + +MAX_DISPLAY_NAME_LENGTH = 64 +MAX_AGENT_NAME_LENGTH = 64 + +# Cc = control chars (covers \n, \r, \t, \x85, DEL and the \x1c-\x1e file +# separators), Zl/Zp = Unicode line/paragraph separators (U+2028, U+2029). +# Together these cover everything str.splitlines() treats as a line break, so +# a display name can never span lines in the router prompt. Cf (format chars) +# stays allowed so emoji ZWJ sequences keep working — except the bidi +# controls, which can visually reorder surrounding text. +_BANNED_CATEGORIES = {"Cc", "Zl", "Zp"} +# The full Unicode Bidi_Control set. +_BIDI_CONTROLS = frozenset( + "\u061c" # ALM + "\u200e\u200f" # LRM RLM + "\u202a\u202b\u202c\u202d\u202e" # LRE RLE PDF LRO RLO + "\u2066\u2067\u2068\u2069" # LRI RLI FSI PDI +) + + +def _unsafe(ch: str) -> bool: + return unicodedata.category(ch) in _BANNED_CATEGORIES or ch in _BIDI_CONTROLS + + +def has_unsafe_chars(text: str) -> bool: + """True if text contains control/line-separator/bidi-control characters.""" + return any(_unsafe(c) for c in text) + + +# Roles an agent may claim on join; anything else downgrades to "member". +ALLOWED_ROLES = frozenset({"master", "member", "observer"}) + + +def agent_name_problem(name: str) -> Optional[str]: + """Reason an agent name can't enter the shared namespace, or None if fine. + + agent_name is inserted verbatim into router prompts and participant + lists, so it gets the same character policy as display names. Called from + every post-auth entry point that can mint a member (join handler, + workspace creation). + """ + if not isinstance(name, str): + # Raw /v1/events payloads are unvalidated JSON — a number or list + # here must be a clean rejection, not an AttributeError 500. + return "agent name must be a string" + if not name or not name.strip(): + return "empty agent name" + if name != name.strip(): + return "leading or trailing whitespace in agent name" + if len(name) > MAX_AGENT_NAME_LENGTH: + return f"agent name longer than {MAX_AGENT_NAME_LENGTH} characters" + if has_unsafe_chars(name): + return "agent name contains control or line-separator characters" + return None + + +def sanitize_inline(text: Optional[str]) -> str: + """Flatten user-controlled text for single-line prompt use. + + Every unsafe character becomes a space, so a crafted display name or + description cannot forge extra participant/instruction lines. + """ + return "".join(" " if _unsafe(c) else c for c in (text or "")).strip() + + +def lock_member_namespace(db, workspace_id) -> None: + """Serialize concurrent namespace writers on the workspace row. + + Every check-then-write of agent_name/display_name must take this lock + first, otherwise two concurrent writers can both pass the clash check and + commit duplicate aliases. SELECT ... FOR UPDATE on PostgreSQL; a no-op on + SQLite, whose single-writer model serializes anyway. + """ + from app.models import Workspace + db.execute( + select(Workspace.id).where(Workspace.id == workspace_id).with_for_update() + ).first() + + +def fold_alias(text: str) -> str: + """Canonical form for namespace comparison. + + NFKC collapses compatibility forms — fullwidth yumi becomes yumi — and + casefold() handles the case pairs lower() misses (ẞ → ss, İ). Plain SQL + lower() does neither, which let visually identical aliases coexist. Both + sides of every namespace comparison must go through this. + """ + return unicodedata.normalize("NFKC", text).casefold() + + +def find_alias_clash(db, workspace_id, name: str, exclude_agent: Optional[str] = None) -> Optional[str]: + """Return the agent_name of a member whose agent_name OR display_name + equals `name` under fold_alias(), or None. `exclude_agent` skips the + member being edited / re-joining itself. + + Comparison happens in Python rather than SQL: the databases' lower() has + no NFKC/casefold, and every caller already holds lock_member_namespace() + over a member list that is small by construction. + """ + from app.models import WorkspaceMember + target = fold_alias(name) + rows = db.execute( + select(WorkspaceMember.agent_name, WorkspaceMember.display_name).where( + WorkspaceMember.workspace_id == workspace_id, + ) + ).all() + for agent_name, display_name in rows: + if exclude_agent is not None and agent_name == exclude_agent: + continue + if fold_alias(agent_name) == target: + return agent_name + if display_name and fold_alias(display_name) == target: + return agent_name + return None diff --git a/workspace/backend/app/routers/cloud_agents.py b/workspace/backend/app/routers/cloud_agents.py index 36a46e81e..241888d78 100644 --- a/workspace/backend/app/routers/cloud_agents.py +++ b/workspace/backend/app/routers/cloud_agents.py @@ -18,8 +18,9 @@ from sqlalchemy import select from sqlalchemy.orm import Session +from app import naming from app.database import get_db -from app.models import CloudAgentConfig, WorkspaceMember +from app.models import CloudAgentConfig, Workspace, WorkspaceMember from app.response import ResponseCode, json_response, success_response from app.routers.network import _resolve_workspace, _verify_workspace_access from app.services.cloud_providers import providers_catalog, validate_provider_model @@ -120,6 +121,10 @@ async def add_cloud_agent( effective_key = body.api_key member_description = f"Cloud agent: {model_info.label} ({body.provider})" + # Namespace lock BEFORE the membership read: a concurrent create/rename + # could otherwise invalidate what we read before we write. + naming.lock_member_namespace(db, workspace.id) + existing = db.execute( select(WorkspaceMember).where( WorkspaceMember.workspace_id == workspace.id, @@ -132,6 +137,19 @@ async def add_cloud_agent( f"Agent '{body.agent_name}' already exists in this workspace", ) + # Names and display-name aliases share one namespace (both are routable in + # the picker and the LLM router) — reject a new agent whose name matches + # another member's display_name. + alias_clash = naming.find_alias_clash( + db, workspace.id, body.agent_name, exclude_agent=body.agent_name, + ) + if alias_clash: + return json_response( + ResponseCode.BAD_REQUEST, + f"Agent name '{body.agent_name}' conflicts with the display name " + f"of member '{alias_clash}'", + ) + if body.provider == "custom" and not body.base_url: return json_response( ResponseCode.BAD_REQUEST, @@ -362,7 +380,9 @@ async def remove_cloud_agent( # Google OAuth — "Sign in with Google" for Gemini # --------------------------------------------------------------------------- +import html as _html import secrets +import time as _time from urllib.parse import urlencode from fastapi.responses import RedirectResponse @@ -371,34 +391,61 @@ async def remove_cloud_agent( _GOOGLE_TOKEN_URL = "https://oauth2.googleapis.com/token" _GOOGLE_SCOPES = "https://www.googleapis.com/auth/generative-language.retriever https://www.googleapis.com/auth/cloud-platform" +# NOTE: process-local — good enough for single-worker deployments, but a +# multi-worker/multi-replica setup needs a shared TTL store or signed states +# (start and callback can land on different processes). Known limitation, +# tracked as a follow-up; predates this feature. _oauth_states: dict[str, dict] = {} +_OAUTH_STATE_TTL_SECONDS = 600 -@router.get("/cloud-agents/google/auth") -async def google_oauth_start( - network: str = Query(...), - agent_name: str = Query("gemini"), - model: str = Query("gemini-3.5-flash"), - x_workspace_token: Optional[str] = Header(None), - authorization: Optional[str] = Header(None), - db: Session = Depends(get_db), -): - """Initiate Google OAuth flow — redirects user to Google consent screen.""" +def _issue_google_auth_url(db, network, agent_name, model, caller_token, authorization): + """Validate the caller and mint a one-time Google consent URL. + + Returns (error_response, None) or (None, url). Shared by the JSON POST + (browser flow: fetch with headers, then navigate) and the header- + authenticated GET redirect. + """ from app.config import config as app_config if not app_config.GOOGLE_OAUTH_CLIENT_ID: - return json_response(ResponseCode.BAD_REQUEST, "Google OAuth not configured on this server") + return json_response(ResponseCode.BAD_REQUEST, "Google OAuth not configured on this server"), None workspace = _resolve_workspace(db, network) if not workspace: - return json_response(ResponseCode.NOT_FOUND, "Network not found") + return json_response(ResponseCode.NOT_FOUND, "Network not found"), None + + # The callback mints a workspace member, so starting the flow requires + # workspace credentials — a state must never be issued to an anonymous + # caller (it used to fall back to the workspace's own token). + if not _verify_workspace_access(workspace, caller_token, authorization): + return json_response(ResponseCode.UNAUTHORIZED, "Invalid workspace credentials"), None + + # Same rules as POST /cloud-agents — validate BEFORE the state is + # written, since the callback trusts everything stored in it. + if not _AGENT_NAME_RE.match(agent_name): + return json_response( + ResponseCode.BAD_REQUEST, + "Agent name must be 3-64 chars, alphanumeric/hyphen/underscore", + ), None + # No model whitelist here on purpose: validate_provider_model accepts any + # model id for known providers (the registry is curated suggestions, not a + # hard restriction) — POST /cloud-agents behaves the same way. + + # Opportunistic prune so abandoned flows don't accumulate forever. + now = _time.time() + for key in [k for k, v in _oauth_states.items() + if now - v.get("created_at", 0) > _OAUTH_STATE_TTL_SECONDS]: + _oauth_states.pop(key, None) state = secrets.token_urlsafe(32) + # No credentials in the state: the callback re-derives everything it + # needs, and possession of the unguessable state IS the authorization. _oauth_states[state] = { "workspace_id": str(workspace.id), - "token": x_workspace_token or workspace.password_hash, "agent_name": agent_name, "model": model, + "created_at": now, } params = { @@ -410,7 +457,59 @@ async def google_oauth_start( "prompt": "consent", "state": state, } - return RedirectResponse(f"{_GOOGLE_AUTH_URL}?{urlencode(params)}") + return None, f"{_GOOGLE_AUTH_URL}?{urlencode(params)}" + + +class GoogleAuthUrlRequest(BaseModel): + network: str + agent_name: str = "gemini" + model: str = "gemini-3.5-flash" + + +@router.post("/cloud-agents/google/auth-url") +async def google_oauth_auth_url( + body: GoogleAuthUrlRequest, + x_workspace_token: Optional[str] = Header(None), + authorization: Optional[str] = Header(None), + db: Session = Depends(get_db), +): + """Mint a one-time Google consent URL for an authenticated caller. + + Browsers can't attach headers to an href navigation, and putting the + workspace token in a query string leaks it to access logs and browser + history — so the frontend fetches this endpoint with headers and then + navigates to the returned URL. + """ + error, url = _issue_google_auth_url( + db, body.network, body.agent_name, body.model, + x_workspace_token, authorization, + ) + if error: + return error + return success_response({"url": url}) + + +@router.get("/cloud-agents/google/auth") +async def google_oauth_start( + network: str = Query(...), + agent_name: str = Query("gemini"), + model: str = Query("gemini-3.5-flash"), + x_workspace_token: Optional[str] = Header(None), + authorization: Optional[str] = Header(None), + db: Session = Depends(get_db), +): + """Initiate Google OAuth flow — redirects to the Google consent screen. + + Header-authenticated only (API clients). Browser flows use + POST /cloud-agents/google/auth-url; a query-string token is deliberately + NOT accepted — it would end up in server logs and browser history. + """ + error, url = _issue_google_auth_url( + db, network, agent_name, model, x_workspace_token, authorization, + ) + if error: + return error + return RedirectResponse(url) @router.get("/cloud-agents/google/callback") @@ -425,12 +524,23 @@ async def google_oauth_callback( from app.config import config as app_config if error: - return _oauth_error_page(f"Google authorization denied: {error}") + # `error` is attacker-reachable (this endpoint is public) — never + # echo it. Map the documented OAuth codes to fixed copy, log the raw + # value for diagnostics. + logger.info("cloud_agents: Google OAuth returned error=%r", error) + message = ( + "Google authorization was denied — no access was granted." + if error == "access_denied" + else "Google authorization failed — please try again." + ) + return _oauth_error_page(message) if not state or state not in _oauth_states: return _oauth_error_page("Invalid OAuth state — please try again") session = _oauth_states.pop(state) + if _time.time() - session.get("created_at", 0) > _OAUTH_STATE_TTL_SECONDS: + return _oauth_error_page("This sign-in link expired — please start again") try: async with httpx.AsyncClient(timeout=30) as http: @@ -460,7 +570,48 @@ async def google_oauth_callback( if not workspace: return _oauth_error_page("Workspace not found") - from app.models import Workspace + # Namespace lock BEFORE the reads, then every guard BEFORE any session + # mutation — bailing out after a db.add() would leave a pending orphan. + naming.lock_member_namespace(db, workspace_id) + + # Ownership is checked on the MEMBER, unconditionally — a stale config + # row must not bypass it, and a removed member of another type must not + # be resurrected as a Google agent. Only a member that already is a + # Google cloud agent may be repaired/reconnected. + existing_member = db.execute( + select(WorkspaceMember).where( + WorkspaceMember.workspace_id == workspace_id, + WorkspaceMember.agent_name == agent_name, + ) + ).scalar_one_or_none() + if existing_member and (existing_member.agent_type or "") != "cloud:google": + logger.warning( + "cloud_agents: OAuth callback refused agent %s in %s — a %s " + "member (status=%s) already owns the name", + agent_name, workspace_id, + existing_member.agent_type, existing_member.status, + ) + return _oauth_error_page( + f"An agent named '{agent_name}' already exists in this " + "workspace. Pick a different name and connect again." + ) + + if existing_member is None: + # Same namespace guard as every other member-creating entry point. + alias_clash = naming.find_alias_clash( + db, workspace_id, agent_name, exclude_agent=agent_name, + ) + if alias_clash: + logger.warning( + "cloud_agents: OAuth callback refused agent %s in %s — " + "clashes with display name of %s", + agent_name, workspace_id, alias_clash, + ) + return _oauth_error_page( + f"The agent name '{agent_name}' conflicts with the display " + f"name of member '{alias_clash}'. Remove or rename that " + "member, then connect again." + ) existing_cfg = db.execute( select(CloudAgentConfig).where( @@ -474,7 +625,7 @@ async def google_oauth_callback( existing_cfg.base_url = f"oauth_refresh:{refresh_token}" if refresh_token else None existing_cfg.model = model else: - cfg = CloudAgentConfig( + db.add(CloudAgentConfig( workspace_id=workspace_id, agent_name=agent_name, provider="google", @@ -482,24 +633,21 @@ async def google_oauth_callback( category="chat", api_key=access_token, base_url=f"oauth_refresh:{refresh_token}" if refresh_token else None, - ) - db.add(cfg) + )) - existing_member = db.execute( - select(WorkspaceMember).where( - WorkspaceMember.workspace_id == workspace_id, - WorkspaceMember.agent_name == agent_name, - ) - ).scalar_one_or_none() - if not existing_member: - db.add(WorkspaceMember( - workspace_id=workspace_id, - agent_name=agent_name, - role="member", - agent_type="cloud:google", - status="online", - description=f"Cloud agent: {model} (Google AI via OAuth)", - )) + if existing_member is None: + db.add(WorkspaceMember( + workspace_id=workspace_id, + agent_name=agent_name, + role="member", + agent_type="cloud:google", + status="online", + description=f"Cloud agent: {model} (Google AI via OAuth)", + )) + elif existing_member.status == "removed": + # Explicit reconnection of a previously-removed Google cloud agent — + # mirrors the POST /cloud-agents reactivation path. + existing_member.status = "online" db.commit() logger.info("cloud_agents: Google OAuth completed for %s in workspace %s", agent_name, workspace_id) @@ -509,6 +657,9 @@ async def google_oauth_callback( def _oauth_success_page(agent_name: str): from fastapi.responses import HTMLResponse + # Escape defensively even though agent_name is validated at start time — + # these pages must never interpolate raw request-derived text. + agent_name = _html.escape(agent_name) return HTMLResponse(f"""Connected!