2 Commits

Author	SHA1	Message	Date
Bohan Jiang	13f74e651a	feat(agents): remove custom_env from agent resources, add audited env endpoint (MUL-2600) (#3209) ... * feat(agents): remove custom_env from agent resources, add audited env endpoint (MUL-2600) The agent resource shape (list / get / create / update / archive / restore responses + WebSocket events) no longer carries `custom_env` values. Reads/writes of env now flow exclusively through a dedicated `/api/agents/{id}/env` endpoint that is owner/admin-only, rejects agent-actor sessions, applies a "****" sentinel preserve guard on PUT, and writes a persistent audit row per reveal/update. Why - `multica agent list --output json` historically returned plaintext `custom_env` for owner/admin callers (the redaction gate gave only members the masked map). Any agent token running on the workspace inherits its owner's role and could read every other agent's secrets just by listing. - Patching list/get redaction alone (PR #3175 direction) left symmetric leaks via mutation responses, WS events, the "reveal" path itself (no actor-aware auth), and a `****` overwrite footgun on UpdateAgent. What changed - Backend: drop `custom_env` from AgentResponse; add coarse `has_custom_env` + `custom_env_key_count`. Strip env handling from UpdateAgent (silently ignored if sent). Keep CreateAgent's custom_env acceptance. - Backend: new GET/PUT `/api/agents/{id}/env` handlers in `internal/handler/agent_env.go`: - resolveActor → 403 for agent actors (closes the lateral-movement path). - Owner/admin role gate via existing helper. - PUT honours value == "****" as "preserve existing value". - Both write to `activity_log` with `agent_env_revealed` / `agent_env_updated` actions. Audit details record key names only, never values. - Daemon claim path (`ClaimAgentTask`) unchanged — `TaskAgentData` still carries plaintext env for runtime injection. - SQL: new `UpdateAgentCustomEnv` query; sqlc regenerated (v1.31.1). - CLI: new `multica agent env get|set` subcommands. `--custom-env*` flags removed from `multica agent update`; the no-fields error now points to the new path. - Frontend: drop env fields from `Agent` + `UpdateAgentRequest`; add `getAgentEnv` / `updateAgentEnv` client methods; rewrite env-tab to show "N variables configured" + explicit "Reveal & edit" button, fetching values only on intentional reveal. - Locales: parity-safe additions to en + zh-Hans. - Docs: agents-create.{mdx,zh.mdx} reflect the new threat model and endpoint. - Mobile: schema drops `custom_env` / `custom_env_redacted`, adds metadata fields. Tests - Handler tests pinned the new invariants: no env in list/get responses, owner reveal happy-path + audit row, agent-actor 403, `****` sentinel preserves real values, UpdateAgent silently ignores `custom_env`, pure `mergeAgentEnv` cases. - CLI tests pivot to the new flag surface: `agent update` MUST NOT expose the env flags; `agent env set` MUST expose --custom-env-stdin/--custom-env-file. - Frontend test fixtures updated; pnpm typecheck / test / lint pass cleanly. This is a breaking API change. Scripts that read `custom_env` from `/api/agents` must migrate to `GET /api/agents/{id}/env`. Co-authored-by: multica-agent <github@multica.ai> * fix(agents): close actor-spoofing + audit fail-closed in env endpoints (MUL-2600) Addresses Elon's review of #3209: * Mint a task-scoped `mat_` token per claim, bound to (agent, task, workspace, owner). Daemon injects it into the agent process in place of its own credential. Auth middleware authoritatively rebuilds X-User-ID / X-Agent-ID / X-Task-ID from the token row and sets X-Actor-Source=task_token; that header is server-set only — incoming values are stripped before any auth branch runs. resolveActor honors the header so an agent that strips X-Agent-ID / X-Task-ID still resolves as actor=agent. * GetAgentEnv / UpdateAgentEnv are now fail-closed on audit-log failures: GET refuses to return plaintext, PUT persists inside the same tx as the audit row so they commit/roll back together. * PUT /api/agents/{id} returns 400 when the body carries custom_env instead of silently dropping it — directs callers to the audited env endpoint. * Agent actors never see mcp_config, even when the underlying member is owner/admin; mutation broadcasts go through a redaction shim so WS subscribers don't pick it up either. * Fix backend test that asserted dense JSON (jsonb::text renders whitespace) and frontend test that assumed a unique "Test User" match. Co-authored-by: multica-agent <github@multica.ai> * fix(agents): close residual MUL-2600 gaps from review (MUL-2600) Migration 108 FK now correctly references agent_task_queue(id) instead of the non-existent agent_task table; the previous name blocked CI backend migrations. Task-token-authenticated requests can no longer be re-routed at a different workspace by passing workspace_slug / workspace_id / ?workspace_id / a URL workspace param. ResolveWorkspaceIDFromRequest and resolveWorkspaceUUID both short-circuit on X-Actor-Source=task_token and return only the token-bound X-Workspace-ID; buildMiddleware adds a defence-in-depth 403 if any URL-resolved workspace disagrees with the token binding. mcp_config no longer leaks back to agent actors through UpdateAgent / CreateAgent / ArchiveAgent / RestoreAgent HTTP responses — the same redactAgentResponseForActor helper that GetAgent/ListAgents use is now applied to mutation responses too. WS broadcasts were already redacted via broadcastAgentResponse. FailTask and every TaskService cancel path (CancelTask / CancelTasksForIssue / CancelTasksForAgent / CancelTasksByTriggerComment / BroadcastCancelledTasks) now eagerly DeleteTaskTokensByTask so the mat_ token's 24h window doesn't outlive a terminated task. Failure is non-fatal — the FK cascade and expiry remain durable guards. Doc-only: clarify that PUT /api/agents/{id} now hard-rejects bodies that carry custom_env (was previously "silently ignores"). Tests: - middleware: TestResolveWorkspaceIDFromRequest gains a task_token case asserting client-supplied slug/id/query cannot override the bound workspace. - handler: TestUpdateAgent_RedactsMcpConfigForAgentActor and TestUpdateAgent_KeepsMcpConfigForMemberActor pin the mutation- response redaction contract per actor type. Co-authored-by: multica-agent <github@multica.ai> * fix(agents): match redacted mcp_config as JSON null, not Go nil (MUL-2600) `AgentResponse.McpConfig` is `json.RawMessage` without `omitempty`, so the redacted response serialises as `"mcp_config": null`. On decode, `json.RawMessage` keeps the literal bytes `null` rather than collapsing to Go nil, which made the assertion fire on a non-leak. The product contract (field always present, distinguished from "no config" via `mcp_config_redacted`) is intentional, so adjust the test to check for "no secret-bearing content" instead of weakening the contract via `omitempty`. Co-authored-by: multica-agent <github@multica.ai> --------- Co-authored-by: multica-agent <github@multica.ai>	2026-05-25 18:42:48 +08:00
Naiyuan Qing	f0f3cb5c3a	fix(server): resolve X-Workspace-Slug in middleware-less handlers (#1165) ... Problem ------- The v2 workspace URL refactor (#1141) switched the frontend from sending X-Workspace-ID (UUID) to X-Workspace-Slug. The workspace middleware was updated to accept the slug and translate it via GetWorkspaceBySlug. But the handler package maintained a PARALLEL resolver (`resolveWorkspaceID` in handler.go) used by endpoints that sit outside the workspace middleware — and that resolver was never updated. It only checked context / ?workspace_id / X-Workspace-ID, never the slug. /api/upload-file is the one production route that hit the broken path: it's user-scoped (not behind workspace middleware) because it also serves avatar uploads (no workspace). Post-refactor requests from the frontend arrived with only X-Workspace-Slug; the handler resolver returned "", the code fell into the "no workspace context" branch, and every file upload since v2 landed in S3 with no corresponding DB attachment row — files orphaned, invisible to the UI. Root cause is structural: two resolvers doing the same job, written independently, diverged silently when one was updated. Fix --- Collapse to a single shared helper. middleware.ResolveWorkspaceIDFromRequest is the new canonical resolver; both the middleware's internal `resolveWorkspaceUUID` (for middleware gating) and the handler-side `(h *Handler).resolveWorkspaceID` (promoted from a package function) now delegate to it. Priority order matches what the middleware has had since v2: context > X-Workspace-Slug header > ?workspace_slug query > X-Workspace-ID header > ?workspace_id query. Impact analysis --------------- 47 call sites of the old `resolveWorkspaceID(r)` are renamed to `h.resolveWorkspaceID(r)`. 46 of them sit behind workspace middleware, so they hit the context fast path and see zero behavior change. The one caller that actually gains capability is UploadFile — which now correctly recognizes slug requests and creates DB attachment rows. Tests ----- - New table-driven unit test for ResolveWorkspaceIDFromRequest covers all priority levels and the unknown-slug fallback. - Regression tests for UploadFile: once with X-Workspace-Slug only (the broken path), once with X-Workspace-ID only (legacy CLI/daemon compat path). Both assert that a DB attachment row is created. - Full Go test suite passes; typecheck + pnpm test unaffected. Plan ---- See docs/plans/2026-04-16-unify-workspace-identity-resolver.md for the full first-principles writeup. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>	2026-04-16 18:01:56 +08:00