mirror of
https://github.com/multica-ai/multica.git
synced 2026-07-05 21:39:54 +02:00
* 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>
266 lines
9.5 KiB
Go
266 lines
9.5 KiB
Go
package middleware
|
|
|
|
import (
|
|
"context"
|
|
"errors"
|
|
"net/http"
|
|
|
|
"github.com/go-chi/chi/v5"
|
|
"github.com/multica-ai/multica/server/internal/util"
|
|
db "github.com/multica-ai/multica/server/pkg/db/generated"
|
|
)
|
|
|
|
// Context keys for workspace-scoped request data.
|
|
type contextKey int
|
|
|
|
const (
|
|
ctxKeyWorkspaceID contextKey = iota
|
|
ctxKeyMember
|
|
)
|
|
|
|
// MemberFromContext returns the workspace member injected by the workspace middleware.
|
|
func MemberFromContext(ctx context.Context) (db.Member, bool) {
|
|
m, ok := ctx.Value(ctxKeyMember).(db.Member)
|
|
return m, ok
|
|
}
|
|
|
|
// WorkspaceIDFromContext returns the workspace ID injected by the workspace middleware.
|
|
func WorkspaceIDFromContext(ctx context.Context) string {
|
|
id, _ := ctx.Value(ctxKeyWorkspaceID).(string)
|
|
return id
|
|
}
|
|
|
|
// SetMemberContext injects workspace ID and member into the context.
|
|
// This is useful for handlers that resolve the workspace from an entity lookup
|
|
// and want to share the member with downstream code.
|
|
func SetMemberContext(ctx context.Context, workspaceID string, member db.Member) context.Context {
|
|
ctx = context.WithValue(ctx, ctxKeyWorkspaceID, workspaceID)
|
|
ctx = context.WithValue(ctx, ctxKeyMember, member)
|
|
return ctx
|
|
}
|
|
|
|
// errWorkspaceNotFound is returned when a slug was provided but doesn't match
|
|
// any workspace. This lets the middleware distinguish "no identifier provided"
|
|
// (400) from "identifier provided but invalid" (404).
|
|
var errWorkspaceNotFound = errors.New("workspace not found")
|
|
|
|
// ResolveWorkspaceIDFromRequest returns the workspace UUID for an HTTP
|
|
// request using the same priority order as the workspace middleware. This is
|
|
// the single source of truth for "which workspace is this request targeting?",
|
|
// shared by middleware-protected routes (via context fast path) and
|
|
// middleware-less routes (e.g. /api/upload-file) that must resolve the slug
|
|
// themselves.
|
|
//
|
|
// Priority:
|
|
// 1. task-token binding (X-Actor-Source == "task_token") — authoritative,
|
|
// server-set, cannot be re-negotiated by the client (MUL-2600)
|
|
// 2. middleware-injected context (fast path for middleware-protected routes)
|
|
// 3. X-Workspace-Slug header → GetWorkspaceBySlug → UUID (post-refactor frontend)
|
|
// 4. ?workspace_slug query → GetWorkspaceBySlug → UUID
|
|
// 5. X-Workspace-ID header (CLI/daemon compat)
|
|
// 6. ?workspace_id query (CLI/daemon compat)
|
|
//
|
|
// Returns "" when no identifier was provided OR a slug was provided but
|
|
// doesn't resolve to any workspace. Callers that need to distinguish "no
|
|
// identifier" (400) from "invalid slug" (404) should use the middleware's
|
|
// internal resolver instead — this helper collapses both cases to "" for
|
|
// simpler handler-level checks.
|
|
func ResolveWorkspaceIDFromRequest(r *http.Request, queries *db.Queries) string {
|
|
// A mat_ task token is bound to exactly one workspace by the token
|
|
// row. Auth middleware writes that workspace into X-Workspace-ID
|
|
// after stripping any client-supplied X-Actor-Source. Any other
|
|
// workspace identifier on the request (slug header/query, ID
|
|
// query, URL param) is the agent trying to widen its blast
|
|
// radius — ignore it.
|
|
if r.Header.Get("X-Actor-Source") == "task_token" {
|
|
return r.Header.Get("X-Workspace-ID")
|
|
}
|
|
if id := WorkspaceIDFromContext(r.Context()); id != "" {
|
|
return id
|
|
}
|
|
if slug := r.Header.Get("X-Workspace-Slug"); slug != "" {
|
|
if ws, err := queries.GetWorkspaceBySlug(r.Context(), slug); err == nil {
|
|
return util.UUIDToString(ws.ID)
|
|
}
|
|
}
|
|
if slug := r.URL.Query().Get("workspace_slug"); slug != "" {
|
|
if ws, err := queries.GetWorkspaceBySlug(r.Context(), slug); err == nil {
|
|
return util.UUIDToString(ws.ID)
|
|
}
|
|
}
|
|
if id := r.Header.Get("X-Workspace-ID"); id != "" {
|
|
return id
|
|
}
|
|
return r.URL.Query().Get("workspace_id")
|
|
}
|
|
|
|
// workspaceResolver extracts a workspace UUID from the request.
|
|
// Returns ("", nil) if no workspace identifier was provided at all.
|
|
// Returns ("", errWorkspaceNotFound) if a slug was provided but doesn't exist.
|
|
// Returns (uuid, nil) on success.
|
|
type workspaceResolver func(r *http.Request) (string, error)
|
|
|
|
// resolveWorkspaceUUID builds a resolver that accepts slug-first identification.
|
|
//
|
|
// Priority:
|
|
// 1. task-token binding (X-Actor-Source == "task_token") — authoritative,
|
|
// server-set; the agent cannot widen its workspace scope by passing a
|
|
// different slug/id (MUL-2600)
|
|
// 2. X-Workspace-Slug header / ?workspace_slug query → GetWorkspaceBySlug → UUID
|
|
// 3. X-Workspace-ID header / ?workspace_id query → UUID directly (CLI/daemon compat)
|
|
//
|
|
// TODO: cache slug→UUID lookup (slug is immutable, safe to cache with short TTL)
|
|
func resolveWorkspaceUUID(queries *db.Queries) workspaceResolver {
|
|
return func(r *http.Request) (string, error) {
|
|
// Task-token-authenticated requests must operate on the
|
|
// token's bound workspace. The auth middleware wrote that ID
|
|
// into X-Workspace-ID; nothing the agent can put on the wire
|
|
// (slug header/query, id query, URL param) can override it.
|
|
if r.Header.Get("X-Actor-Source") == "task_token" {
|
|
id := r.Header.Get("X-Workspace-ID")
|
|
if id == "" {
|
|
return "", errWorkspaceNotFound
|
|
}
|
|
return id, nil
|
|
}
|
|
// Slug path (preferred — frontend sends this after the URL refactor)
|
|
if slug := r.URL.Query().Get("workspace_slug"); slug != "" {
|
|
ws, err := queries.GetWorkspaceBySlug(r.Context(), slug)
|
|
if err != nil {
|
|
return "", errWorkspaceNotFound
|
|
}
|
|
return util.UUIDToString(ws.ID), nil
|
|
}
|
|
if slug := r.Header.Get("X-Workspace-Slug"); slug != "" {
|
|
ws, err := queries.GetWorkspaceBySlug(r.Context(), slug)
|
|
if err != nil {
|
|
return "", errWorkspaceNotFound
|
|
}
|
|
return util.UUIDToString(ws.ID), nil
|
|
}
|
|
// UUID fallback (CLI, daemon, legacy clients)
|
|
if id := r.URL.Query().Get("workspace_id"); id != "" {
|
|
return id, nil
|
|
}
|
|
if id := r.Header.Get("X-Workspace-ID"); id != "" {
|
|
return id, nil
|
|
}
|
|
return "", nil
|
|
}
|
|
}
|
|
|
|
func writeError(w http.ResponseWriter, status int, msg string) {
|
|
w.Header().Set("Content-Type", "application/json")
|
|
w.WriteHeader(status)
|
|
w.Write([]byte(`{"error":"` + msg + `"}`))
|
|
}
|
|
|
|
// RequireWorkspaceMember resolves the workspace from slug (preferred) or UUID
|
|
// (fallback), validates membership, and injects the member and workspace ID
|
|
// into the request context.
|
|
func RequireWorkspaceMember(queries *db.Queries) func(http.Handler) http.Handler {
|
|
return buildMiddleware(queries, resolveWorkspaceUUID(queries), nil)
|
|
}
|
|
|
|
// RequireWorkspaceRole is like RequireWorkspaceMember but additionally checks
|
|
// that the member has one of the specified roles.
|
|
func RequireWorkspaceRole(queries *db.Queries, roles ...string) func(http.Handler) http.Handler {
|
|
return buildMiddleware(queries, resolveWorkspaceUUID(queries), roles)
|
|
}
|
|
|
|
// RequireWorkspaceMemberFromURL resolves the workspace ID from a chi URL
|
|
// parameter, validates membership, and injects into context.
|
|
func RequireWorkspaceMemberFromURL(queries *db.Queries, param string) func(http.Handler) http.Handler {
|
|
return buildMiddleware(queries, func(r *http.Request) (string, error) {
|
|
id := chi.URLParam(r, param)
|
|
if id == "" {
|
|
return "", nil
|
|
}
|
|
return id, nil
|
|
}, nil)
|
|
}
|
|
|
|
// RequireWorkspaceRoleFromURL is like RequireWorkspaceMemberFromURL but
|
|
// additionally checks that the member has one of the specified roles.
|
|
func RequireWorkspaceRoleFromURL(queries *db.Queries, param string, roles ...string) func(http.Handler) http.Handler {
|
|
return buildMiddleware(queries, func(r *http.Request) (string, error) {
|
|
id := chi.URLParam(r, param)
|
|
if id == "" {
|
|
return "", nil
|
|
}
|
|
return id, nil
|
|
}, roles)
|
|
}
|
|
|
|
func buildMiddleware(queries *db.Queries, resolve workspaceResolver, roles []string) func(http.Handler) http.Handler {
|
|
return func(next http.Handler) http.Handler {
|
|
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
|
workspaceID, resolveErr := resolve(r)
|
|
if resolveErr != nil {
|
|
writeError(w, http.StatusNotFound, "workspace not found")
|
|
return
|
|
}
|
|
if workspaceID == "" {
|
|
writeError(w, http.StatusBadRequest, "workspace_id or workspace_slug is required")
|
|
return
|
|
}
|
|
|
|
// Final task-token binding check: even when the workspace
|
|
// was resolved from a chi URL parameter
|
|
// (RequireWorkspaceMemberFromURL), the agent must not be
|
|
// allowed to operate on a workspace other than the one
|
|
// stamped into its task token. This is the catch-all
|
|
// behind resolveWorkspaceUUID's earlier check. MUL-2600.
|
|
if r.Header.Get("X-Actor-Source") == "task_token" {
|
|
bound := r.Header.Get("X-Workspace-ID")
|
|
if bound == "" || workspaceID != bound {
|
|
writeError(w, http.StatusForbidden, "task token is bound to a different workspace")
|
|
return
|
|
}
|
|
}
|
|
|
|
userID := r.Header.Get("X-User-ID")
|
|
if userID == "" {
|
|
writeError(w, http.StatusUnauthorized, "user not authenticated")
|
|
return
|
|
}
|
|
|
|
userUUID, err := util.ParseUUID(userID)
|
|
if err != nil {
|
|
writeError(w, http.StatusUnauthorized, "user not authenticated")
|
|
return
|
|
}
|
|
wsUUID, err := util.ParseUUID(workspaceID)
|
|
if err != nil {
|
|
writeError(w, http.StatusBadRequest, "invalid workspace_id")
|
|
return
|
|
}
|
|
member, err := queries.GetMemberByUserAndWorkspace(r.Context(), db.GetMemberByUserAndWorkspaceParams{
|
|
UserID: userUUID,
|
|
WorkspaceID: wsUUID,
|
|
})
|
|
if err != nil {
|
|
writeError(w, http.StatusNotFound, "workspace not found")
|
|
return
|
|
}
|
|
|
|
if len(roles) > 0 {
|
|
allowed := false
|
|
for _, role := range roles {
|
|
if member.Role == role {
|
|
allowed = true
|
|
break
|
|
}
|
|
}
|
|
if !allowed {
|
|
writeError(w, http.StatusForbidden, "insufficient permissions")
|
|
return
|
|
}
|
|
}
|
|
|
|
ctx := SetMemberContext(r.Context(), workspaceID, member)
|
|
next.ServeHTTP(w, r.WithContext(ctx))
|
|
})
|
|
}
|
|
}
|