mirror of
https://github.com/multica-ai/multica.git
synced 2026-08-07 19:17:21 +02:00
fix/codex-patch-apply-payload
4 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Bohan Jiang
|
607209de7c |
fix(avatar): serve avatars through a signed endpoint on private buckets (MUL-5393) (#6088)
* fix(avatar): serve avatars through a signed endpoint on private buckets (MUL-5393) Avatar uploads persisted the raw storage object URL into `avatar_url`. On a deployment whose bucket is private and has no public CDN domain (S3 with Block Public Access, R2, MinIO) that URL is a guaranteed 403 in the browser: ATTACHMENT_DOWNLOAD_MODE only ever applied to the attachment download endpoint, so every user / agent / squad / workspace avatar rendered broken even though the upload itself succeeded. Resolve at read time instead of at upload time. What is persisted stays the durable object reference, so nothing with a TTL is ever written to the database and avatars saved by an older build are fixed without a backfill. What is served is `/api/avatars/<sig>/<key>`, a stable URL the server resolves per request through the deployment's existing storage download policy (presigned redirect, CloudFront-signed redirect, or proxied body). The endpoint is unauthenticated and the HMAC signature is the credential: the session cookie is SameSite=Strict, so an auth-gated URL cannot be a native <img src> from Desktop, a mobile webview, or a split-origin self-hosted web app. The signature covers the storage key and only image extensions resolve, so an avatar_url pointed at a private document cannot launder it into a publicly fetchable URL. Deployments that already work are untouched: a public CDN domain without per-request signing, and the local-disk backend whose /uploads/* route is public, both keep returning the raw URL. Fixes #6024 Co-authored-by: multica-agent <github@multica.ai> * fix(avatar): only publish avatar-class objects through the signed endpoint (MUL-5393) Review found that being able to name a storage object was treated as permission to publish it. `ownedStorageKey` proved only that a URL came from this deployment's storage, and every image-shaped key was then signed — while the avatar update endpoints accepted any raw storage URL. A caller who had seen a private image attachment's URL could submit it as their own avatar, and the unauthenticated endpoint would keep re-signing it indefinitely. A user avatar propagates to every workspace that user belongs to, so the leak crossed workspace boundaries. Add the missing authorization rule: an object is serveable as an avatar only when it is avatar-class — a standalone image upload not attached to an issue, comment, chat session, chat message, or task. The check resolves the backing attachment row from the id UploadFile embeds in the object filename, so it needs no lookup by URL and no new index. It is enforced on both sides. The write side rejects such a value with 403 before anything is stored; the read side re-checks per request, which is what makes the guarantee hold for rows written before this existed and revokes the URL if an object is later bound to a comment or chat. Scope is the `workspaces/` namespace — the only place that can hold content belonging to someone other than whoever is setting the avatar, covering both uploads and channel media ingest. Keys elsewhere (the per-user standalone namespace, or objects an operator placed in the bucket) stay usable, which keeps the documented "an explicit avatar_url is preserved" contract intact. Uploader identity is deliberately not part of the rule: duplicating an agent legitimately reuses the source agent's avatar object, which a different admin may have uploaded. Publishing someone else's unbound image would require knowing its URL, and unbound rows appear in no listing endpoint. Also clamp the 302's cache lifetime to half the signed URL's own TTL (0 -> no-store). ATTACHMENT_DOWNLOAD_URL_TTL takes any positive duration, so the fixed 60s could outlive the target it pointed at on a short-TTL deployment. Co-authored-by: multica-agent <github@multica.ai> --------- Co-authored-by: Bohan-J <bohan@devv.ai> Co-authored-by: multica-agent <github@multica.ai> |
||
|
Bohan Jiang
|
cc5ee169f0 |
fix(agents): allow agent owners to manage their own agent's env (MUL-5438) (#6080)
* fix(agents): let agent owners manage their own agent's env (MUL-5438)
GET/PUT /api/agents/{id}/env admitted only workspace owner/admin, which
made env the one endpoint in the agent permission model that ignored
agent ownership: canManageAgent already lets the owner update/archive,
and canViewAgentSecrets already lets the owner read mcp_config. The
asymmetry was worst on create — POST /api/agents accepts custom_env from
any member and stores that member as owner_id — so a member could write
secrets into their own agent and then never read or rotate them, with
UpdateAgent rejecting custom_env outright and no workaround left.
authorizeAgentEnv now admits a workspace owner/admin OR the agent's own
human owner. The agent-actor rejection still runs first and unchanged:
an agent process is denied even when its backing human owns the target
agent. The owner comparison uses member.UserID rather than the raw
X-User-ID header because agent.owner_id is nullable and uuidToString
renders NULL as "", and canManageAgentEnv rejects an empty owner from
the other side too.
The web Environment tab is gated on the same rule via the existing
canEdit decision, so it stops offering a "Reveal & edit" action that is
a guaranteed 403. The server remains the boundary.
Closes #6076
Fixes: https://github.com/multica-ai/multica/issues/6076
Co-authored-by: multica-agent <github@multica.ai>
* docs(agents): correct stale "owner/admin only" env permission wording
Follow-up to the MUL-5438 permission change: several comments and the
published docs still described the env endpoints as workspace
owner/admin only, which now contradicts the code.
- router.go / agent.go / types/agent.ts: the three sites flagged in
review.
- agents-create.mdx (en/zh/ja/ko): the user-facing callout said reading
values requires a workspace owner or admin. It now names the agent's
own owner first, and spells out that the agent-actor denial holds even
for an agent the same human owns.
- daemon.go / middleware/auth.go: these called the env endpoints
"owner-only" as shorthand for "reject agent actors". That property is
unchanged, but "human-only" is what they actually mean now.
Comments and docs only — no behavior change.
Co-authored-by: multica-agent <github@multica.ai>
---------
Co-authored-by: Bohan-J <bohan@devv.ai>
Co-authored-by: multica-agent <github@multica.ai>
|
||
|
Bohan Jiang
|
270fb6aa73 |
MUL-2792 fix(agent): preserve skills in update/archive/restore response (#3464)
* MUL-2792 fix(agent): preserve skills in update/archive/restore response (#3459) agentToResponse always initialises Skills as []; the mutation handlers relied on the caller to refresh it, but only GetAgent and ListAgents actually did. UpdateAgent / ArchiveAgent / RestoreAgent therefore returned "skills": [] regardless of what the agent_skill junction table contained. The DB write path was never wrong — skills weren't actually deleted — but the misleading response (and its matching agent:status / archived / restored WS broadcast) scared users into manually re-running `agent skills set` and risked scripted clients writing the empty set back as truth. Extract the existing GetAgent skill-reload block into attachAgentSkills and call it from the three buggy handlers. Add regression tests that attach skills, hit each mutation endpoint, and assert both the response and the junction table. Co-authored-by: multica-agent <github@multica.ai> * fix(agent): attach skills before env/template broadcasts (#3459) Two follow-up sites flagged in PR #3464 review that shared the same "agentToResponse zeroes Skills, callers forget to reload" pattern as the mutation handlers: - agent_env.go: the agent:status broadcast after UpdateAgentEnv used a bare agentToResponse, so subscribers saw skills wiped on every env rotation. HTTP body is AgentEnvResponse so the response itself is unaffected, but the WS event still misleads any cache that ingests it. - agent_template.go: CreateAgentFromTemplate attaches imported and extra skills inside the tx, then builds the response/agent:created broadcast without reloading them — so callers (and any client tracking the create event) see the freshly created agent as skill-less despite the template having just imported them. Both call sites now reuse attachAgentSkills introduced for UpdateAgent. Co-authored-by: multica-agent <github@multica.ai> --------- Co-authored-by: J <j@multica.ai> Co-authored-by: multica-agent <github@multica.ai> |
||
|
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>
|