mirror of
https://github.com/multica-ai/multica.git
synced 2026-08-06 01:50:14 +02:00
agent/lambda/llm-env-plumbing
6 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Multica Eve
|
999e9f93c7 |
fix(codex): record file edit payload for patch_apply events (#6158)
* fix(redact): scrub secrets nested inside tool input maps and slices
InputMap only passed top-level string values through Text and documented
non-string values as "preserved as-is". Any secret one level down reached
the database and the WebSocket broadcast untouched:
flat -> [REDACTED ...] (scrubbed)
nested -> [map[diff:token=ghp_... path:a.go]] (leaked verbatim)
This is a prerequisite for recording structured file-edit payloads. Codex
reports an edit as changes[]{path, diff, content}, and the legacy protocol
reports a deletion as the whole outgoing file — so without this, deleting a
.env would persist its full contents in cleartext.
redactValue now walks the composite shapes json.Unmarshal produces, plus
[]string and map[string]string for argv-style inputs. Composites are copied
rather than scrubbed in place, because the caller keeps using the map it
passed in.
Nesting depth comes from daemon-supplied JSON, so the walk is bounded at 32
levels; a pathologically nested payload would otherwise recurse until the
stack blows. Hitting the bound yields a placeholder rather than the raw
value, keeping the fail-safe direction.
Verified: the five new tests each fail against the previous top-level-only
implementation and pass now; full ./pkg/redact suite green.
Co-authored-by: multica-agent <github@multica.ai>
* fix(codex): record file edit payload for patch_apply events
Both Codex protocol paths recorded a file edit as a bare call ID and set no
payload, so a run that edited six files left six blank, unexpandable rows in
the transcript. The same task on Claude or Grok showed a readable diff, and
the branch Codex pushed was the only surviving record of what it changed
(GH #6157).
The omission was specific to this one tool, not to the adapter: the
exec_command handlers directly above already captured command and output.
Both paths are fixed, since the protocol is sniffed at runtime. Their wire
shapes differ more than they appear, and the normalizer reconciles that:
- legacy patch_apply_begin/end carry map[path]FileChange, internally tagged
on `type`, where add/delete hold whole-file `content` and only update
holds a `unified_diff` plus `move_path`. There is no diff for every case,
so the normalized form keeps diff and content as alternatives.
- v2 fileChange items carry an ordered array of {path, kind, diff} where
`kind` is an object, not a string — reading it as a string silently
yields "" and loses the add/delete/update distinction.
- status spellings differ too: legacy is snake_case, v2 is camelCase and
adds inProgress. Both normalize onto one vocabulary, and a legacy event
predating `status` falls back to its `success` bool.
Legacy map iteration is sorted by path so a replayed event does not reshuffle
the file list.
Completion events now also produce a non-empty output (status, file count,
and any apply_patch stdout/stderr), because an empty output renders as an
unexpandable blank row just like a missing input.
Anything unrecognised — absent, wrongly typed, or malformed changes — returns
no payload, preserving exactly the previous degradation rather than risking
the transcript.
Total diff/content bytes are bounded at 64 KiB with UTF-8-safe truncation,
recording `truncated` and `original_bytes`; paths and kinds always survive,
since they are what a reviewer needs when the body is gone. The bound is
deliberately scoped to this new payload: other providers stream tool inputs
through unbounded, and clamping them here would silently truncate
transcripts that render correctly today. Unifying the limit at the
persistence boundary is left as a follow-up.
Verified: the new tests reproduce the reported symptom (Input:map[],
Output:"") against the previous call sites and pass now; ./pkg/agent and
./pkg/redact green, go vet and gofmt clean.
Co-authored-by: multica-agent <github@multica.ai>
* feat(transcript): render Codex multi-file patch payloads as diffs
The presenter identified an edit by input shape — a top-level file_path plus
old_string/new_string or content — which is Claude's and Grok's shape. Codex
records one patch_apply covering several files as changes[], so even with the
payload now populated it fell through to pretty JSON instead of a diff.
A new `patch` detail kind carries one entry per file, since collapsing them
into a single body would lose which change belongs where. Each file reuses the
existing single-file surfaces, so all bodies behave alike inside the
virtualized list.
Codex hands over a ready-made unified diff, so parseUnifiedDiff maps it onto
diff rows rather than recomputing one — there is no before/after pair to
compare, and reconstructing both sides from the diff just to diff them again
would be circular. Hunk headers become `gap` rows, which is what they denote:
skipped unchanged content.
A deletion renders as all-removals rather than as a whole-file write, because
the legacy protocol reports it as the outgoing file's content and a green
"+N" gutter would state the opposite of what happened.
The collapsed row needed its own fix: with no single path field, the summary
fell through the preference chain and came back empty. It now reads as the
first path plus "+N more".
Anything that is not this shape still falls back to pretty JSON, so a payload
this presenter does not understand stays readable.
Verified: 17 new tests (43 in the presenter suite) pass; repo typecheck and
lint clean. The one failing views test, layout/sidebar-resize, fails
identically on an untouched checkout.
Co-authored-by: multica-agent <github@multica.ai>
* fix(codex): route v2 add/delete payloads as content, not diff
Addresses review on #6158.
Upstream's format_file_change_diff only produces a unified diff for `update`.
For `add` and `delete` it returns the whole file's contents under the same
`diff` field, and for a moved `update` it appends a trailing
"\n\nMoved to: <path>" line:
FileChange::Add { content } => content.clone(),
FileChange::Delete { content } => content.clone(),
FileChange::Update { unified_diff, move_path } => ...
(codex-rs/app-server-protocol/src/protocol/item_builders.rs, rust-v0.145.0)
Recording that as a diff mislabels every line of an added or deleted file as
context, and actively inverts any line whose content begins with '+' or '-' —
so an added file containing "-minus lead" rendered as a deletion. The payload
is now routed by `kind` rather than by field name, and the "Moved to:"
sentence is stripped since move_path already carries the destination.
The previous v2 tests hid this by using a fixture the real protocol never
emits (an `add` carrying "@@ ... +package main"). They now use upstream's
shape, plus cases for delete, an empty add, and an add whose contents look
like diff headers.
Empty bodies are also kept on both paths: presence of the field, not its
non-emptiness, decides whether a body was reported, so an empty added file
renders as an empty body instead of "no content reported".
Verified: the new assertions fail against the previous normalizer — where an
`add` came through as {"diff": "package main\n"} — and pass now.
Co-authored-by: multica-agent <github@multica.ai>
* fix(transcript): stop treating header-like content lines as file headers
Addresses review on #6158.
parseUnifiedDiff matched "---" / "+++" / "diff --git" / "index " at any
position, so a changed line whose *content* starts with a dash or plus was
silently discarded:
parseUnifiedDiff("@@ -1 +1 @@\n--- old markdown\n+++ new markdown\n")
// before: [{ kind: "gap", ... }] — both changed lines gone
A removal of "-- old markdown" is spelled "--- old markdown" on the wire, so
this hit Markdown rules, embedded patches, and comment banners.
File headers only exist ahead of the first hunk, so they are only recognised
there; once inside a hunk every line is parsed strictly by its first
character.
Also localizes the multi-file summary count, which was hardcoded English and
so leaked into the zh-Hans / ja / ko transcript rows. The presenter owns no
React and no i18n by design, so the phrasing is injected by the caller rather
than imported here, keeping the module unit-testable in isolation; the English
form remains the fallback. The three Chinese/Japanese/Korean truncation
strings now use "..." to match the English source they translate.
Verified: both new parser assertions fail against the previous
strip-anywhere behaviour and pass now; 47 presenter tests green, repo
typecheck and lint clean.
Co-authored-by: multica-agent <github@multica.ai>
* fix(daemon): redact nested tool input before it leaves the daemon
Addresses review on #6158.
Recursive redaction ran only in the server's ingest handler. The daemon built
the new nested edit payload and sent msg.Input verbatim, so a daemon that
self-updated ahead of the server — or one talking to a server mid-rollout —
would ship whole-file edit contents to a peer that does not scrub nested
values yet. The legacy protocol reports a deletion as the whole outgoing file,
so that window covered a deleted .env in cleartext.
Ordering three commits inside one PR is not a deployment barrier, and daemon
and server upgrade independently. Deployment order is not a control we have,
so the sending side is now safe on its own; the server keeps redacting on
ingest as the second line of defence.
Scoped to Input, which is the field this PR newly fills with file contents.
Content and Output are plain strings already redacted server-side, and
changing their daemon-side handling would be unrelated to this fix.
Verified: the new daemon test asserts the nested token is masked in the
reported batch while the change metadata survives. It fails without this
change, reporting the full GITHUB_TOKEN= line on the wire, and passes with
it; ./internal/daemon green.
Co-authored-by: multica-agent <github@multica.ai>
* fix(transcript): correct the Chinese multi-file patch count semantics
Addresses review on #6158.
The summary is handed the number of files *beyond* the named one, but the
Chinese phrasing stated a total: "a.go 等 2 个文件" reads as two files including
a.go, so a three-file patch under-reported by one. English hides the
distinction ("+2 more"), which is why it survived the first pass.
Rewords zh-Hans to "另有 N 个文件". Japanese (他) and Korean (외) already read
as "besides", so their wording is unchanged.
Also renames the interpolation variable from `count` to `extra`, for two
reasons. i18next treats `count` as the plural selector — this very namespace
relies on that for events_one/events_other — so a plain number had no business
borrowing it. And the name is what a translator reads: `extra` cannot be
mistaken for a total the way `count` was.
Guards the whole bug class rather than just this string: a locale test asserts
every locale interpolates {{path}} and {{extra}} and never the reserved
{{count}}, and a presenter test pins that the injected number is the count of
additional files, not the total.
Verified: both new locale assertions fail against the reverted string and pass
now; rendering the real locale strings for a three-file patch yields "+2 more",
"另有 2 个文件", "他 2 件", "외 2개". 53 target tests pass, repo typecheck clean,
views lint back to its pre-existing 16 warnings and 0 errors.
Co-authored-by: multica-agent <github@multica.ai>
* fix(transcript): put the patch surface on the type scale
Addresses review on #6158.
The patch surface wrote text-[10px] / text-[11px] / text-[10px], copied from
the sibling transcript surfaces as they looked when this branch started. Since
then MUL-5451 (#6136) introduced a role-named type scale and migrated those
same siblings to text-micro, so these three call sites were the only remaining
arbitrary sizes — and the type-scale guard reports them precisely.
All three become text-micro. That matches the analogues they were copied from
now that those have moved: the FileWriteSurface line-count row, the
DiffDetailSurface header row, and the ToolDetailSurface body. It is also the
only correct target, since micro (11px) is the smallest step the scale defines
— there is nothing at 10px to map to.
Merges origin/main so the guard runs here rather than only in CI.
Verified: apps/web app/type-scale.test.ts 13/13 (it listed exactly these three
lines before), no `text-[` left in the file, repo typecheck clean, views lint
unchanged at 16 pre-existing warnings and 0 errors.
Co-authored-by: multica-agent <github@multica.ai>
* fix(codex): redact patch bodies before applying the size budget
Addresses review on #6158.
The adapter sized and truncated the normalized changes, and redaction only ran
later — in the daemon before sending, then again in the server on ingest. That
order loses secrets that straddle the budget.
The PEM rule needs both markers to match:
-----BEGIN[A-Z\s]*PRIVATE KEY-----.*?-----END[A-Z\s]*PRIVATE KEY-----
So a 70 KB private key whose BEGIN sits inside the first 64 KiB and whose END
falls past the cut stops matching once truncated. Neither later pass can
recognise what truncation already broke, so the marker and 64 KiB of key
material reach the database and the WebSocket broadcast. Measured on the
previous code:
stored bytes 65536 | BEGIN marker present | key body present | placeholder absent
Redaction now runs first, and the budget measures the redacted bodies — which
is also the honest measurement, since those are what actually gets stored and
redaction usually shrinks them (that key collapses to 23 bytes, so no trimming
is needed at all). `original_bytes` still reports the pre-redaction size so the
reader sees how large the real patch was. The daemon and server passes stay as
defence in depth; redaction is idempotent, so running three times is safe and
that is now asserted.
Note for callers: codexPatchInput no longer trims its argument in place, because
redaction copies first. Two existing tests were asserting on the caller's
original slice and had silently become vacuous; they now read the returned
payload, and one pins the no-mutation contract. The delete fixture in the
diff-vs-content routing test was also a credential-shaped string, which now
redacts — it is plain text so that test keeps testing routing.
Verified: the new boundary test fails on the previous order, reporting the
surviving BEGIN marker and key material, and passes now. go test ./pkg/agent
./pkg/redact ./internal/daemon green; execenv ByteIdentical green; go vet and
gofmt clean.
Co-authored-by: multica-agent <github@multica.ai>
---------
Co-authored-by: Eve <eve@multica-ai.local>
Co-authored-by: multica-agent <github@multica.ai>
|
||
|
YOMXXX
|
c599f47ba4 |
fix(redact): cover GitHub fine-grained PATs and Google API keys (#4678)
The primary server redactor now covers github_pat_ and standard AIza keys, but the client transcript safety net does not. Add matching client patterns and regression coverage so previously stored or otherwise unredacted transcript values are masked at display time. Also make the fixed-length Google key rule redact a key ending in '-' while preserving its trailing delimiter. Keep the 39-character format exact to avoid broadening false positives. Co-authored-by: multica-agent <github@multica.ai> |
||
|
Rusty Raven
|
5f767d671a |
fix(redact): cover credential formats that leaked unredacted (#5274)
The GitHub-token rule only matched classic tokens (ghp_/gho_/ghu_/ghs_/ghr_), even though its comment claims to cover fine-grained tokens. Add coverage for GitHub fine-grained PATs (github_pat_), Slack app-level (xapp-) / config (xoxe-), Google API keys (AIza...), and Stripe live secret/restricted keys (sk_live_/rk_live_). Publishable Stripe keys (pk_live_) are intentionally NOT redacted (public). Adds regression tests for every new shape, including a positive test that pk_live_ stays unredacted. |
||
|
LinYushen
|
cb68669c73 |
feat(composio): gate MCP apps behind feature flag (#4876)
* feat(composio): server-side connect flow + connections REST (Notion MVP) (MUL-3720) (#4608)
* feat(composio): server-side connect flow + connections REST (Notion MVP) (MUL-3720)
Compose the merged server/pkg/composio SDK into a user-facing connection
manager: signed-state connect handshake, local user_composio_connection
mirror, idempotent disconnect, and a per-user MCP session helper (not yet
wired into task dispatch).
- migration 127_user_composio_connection (no FK/cascade, per DB rules)
- sqlc queries: upsert (idempotent on user_id+connected_account_id), list
active, owner-scoped get, mark revoked
- internal/integrations/composio: signed HMAC-SHA256 state, BeginConnect,
CompleteCallback (idempotent upsert), ListConnections, Disconnect
(upstream 404 = idempotent success), CreateMCPSession (no-op when empty,
pins connected_accounts per toolkit), CallbackRedirect
- REST handlers under /api/integrations/composio (user-scoped, 503 when
COMPOSIO_API_KEY unset): connect/init, callback (302), connections list,
delete
- router wiring gated by COMPOSIO_API_KEY; COMPOSIO_AUTH_CONFIGS_JSON maps
toolkit->auth_config (MVP: notion); state secret from COMPOSIO_STATE_SECRET
or derived from JWT_SECRET; callback base from COMPOSIO_CALLBACK_BASE_URL
or MULTICA_PUBLIC_URL
- tests: state (expire/tamper/wrong-secret), service (mapping, callback
idempotency, non-success, disconnect owner/404 idempotency, MCP pin),
handlers (httptest), redact regression for Bearer mcp_ tokens
MVP scope: Notion only; no task-dispatch overlay, sharing, or webhook
event handling (later stages).
Co-authored-by: multica-agent <github@multica.ai>
* fix(composio): bind callback account to user + idempotent revoked disconnect (MUL-3720)
Address PR 4608 review (CHANGES_REQUESTED):
- callback: verify connected_account_id with Composio before mirroring it.
The signed state only proved user/toolkit/exp, so a valid state paired with
a tampered connected_account_id would be written verbatim. CompleteCallback
now calls ListConnectedAccounts and fails closed (ErrAccountVerification)
unless the account belongs to the state's user (composio_user_id == multica
user id) and was created under the toolkit's auth config. No row is written
on mismatch / unknown account / upstream error.
- disconnect: short-circuit to a no-op when the local row is already revoked,
before touching upstream. Previously a second DELETE re-hit Composio and a
non-404 upstream error surfaced as a 502, breaking the 204-idempotent
contract.
- CreateMCPSession: document the v1 single-active-connection-per-(user,toolkit)
constraint and make duplicate selection deterministic (newest-wins, rows are
connected_at DESC) instead of order-dependent map overwrite. Stage 3 owns the
real single-account-enforcement vs multi-account-shape decision.
Tests: tampered/wrong-auth-config/unknown-account callback rejection, revoked-row
disconnect no-op (asserts upstream not re-hit). composio pkg 85% coverage; all
green.
Co-authored-by: multica-agent <github@multica.ai>
* feat(composio): list all toolkits + dynamic auth-config resolution (MUL-3720)
Yushen's follow-up to the Notion MVP: surface the full Composio toolkit
catalog, render it in Settings, and drop the static env mapping in favor of
dynamic auth-config discovery.
Config correctness (per Composio docs):
- Remove COMPOSIO_AUTH_CONFIGS_JSON entirely. The toolkit→auth_config mapping
is now resolved at request time from the project's /auth_configs (cached,
5-min TTL), so enabling a toolkit is a dashboard action, not a redeploy.
- Do NOT add COMPOSIO_PROJECT_ID. The project API key (x-api-key) authenticates
to exactly one project; the project is resolved from the key. Only org-level
endpoints use x-org-api-key, which this integration never calls.
Backend:
- SDK: server/pkg/composio/auth_configs.go — ListAuthConfigs (toolkit_slug,
is_composio_managed, show_disabled, limit, cursor).
- service: dynamic resolver (authConfigMap cache; betterAuthConfig prefers a
custom/white-label config over Composio-managed, newest wins); BeginConnect
and CompleteCallback resolve via it; ListToolkits fetches the full catalog
(paginated, capped) annotated with connectable = has an enabled auth config,
connectable-first ordering.
- handler + route: GET /api/integrations/composio/toolkits (user-scoped, 503
when COMPOSIO_API_KEY unset) returning slug/name/logo/category/connectable.
Frontend:
- core: ComposioToolkit/ComposioConnection types, api client methods, and
composio query options (@multica/core/composio).
- views: Settings → Integrations now has a Composio section rendering every
toolkit as a card with search. Connect is gated on `connectable`;
non-connectable toolkits show a muted "not configured" hint instead of a
dead button. Connected toolkits show a badge + Disconnect (with confirm).
- i18n: composio block added to en/zh-Hans/ja/ko settings.
Tests: SDK + service (dynamic resolution, custom-over-managed preference,
connectable flag, resolver-error soft-degrade) and handler toolkits endpoint;
composio pkg 85.7% coverage. go build/vet/gofmt clean; core+views typecheck,
core+views lint, and core tests (691) all green.
Co-authored-by: multica-agent <github@multica.ai>
* fix(composio): close cross-toolkit callback fail-open by signing auth_config_id into state (MUL-3720)
Re-review blocker: CompleteCallback resolved the toolkit's auth config at
callback time and ignored a resolve error/empty result, while
verifyAccountOwnership skipped the auth-config comparison when the expected
value was empty. A user could then pass another toolkit's connected_account_id
into this toolkit's callback — the owner check passed and it was written under
the wrong toolkit_slug/account binding.
Fix: the auth_config_id is already resolved in BeginConnect (before the state
is signed), so sign it into the state and compare it exactly at callback. No
re-resolve, no fail-open. verifyAccountOwnership now fails closed when the
expected auth config is empty (rejects instead of skipping) and requires an
exact match — closing the cross-toolkit binding gap.
Tests: state round-trips auth_config_id; BeginConnect signs it; callback
rejects wrong/cross-toolkit auth config and an empty (no-mapping) auth config
fails closed. composio pkg 85.2% coverage, all green.
Frontend (non-blocking): the Composio settings tab now surfaces an error when
the connections query fails instead of silently rendering everything as
unconnected.
Co-authored-by: multica-agent <github@multica.ai>
* fix(composio): hide Settings section entirely when integration unconfigured (MUL-3720)
Decision (option 2, hide-then-merge): don't show a card that leaks the internal
COMPOSIO_API_KEY env-var name to every end user. IntegrationsTab now gates the
whole Composio section (heading + body) on the toolkits query — a 503 means the
key is unset, so the section is withheld instead of rendering the not-configured
card. Admin-only setup guidance is a later, role-gated affordance.
Removed the notConfigured card (and now-unused ApiError import) from
ComposioTab; it only mounts when configured. views typecheck + lint clean.
Co-authored-by: multica-agent <github@multica.ai>
---------
Co-authored-by: multica-agent <github@multica.ai>
* feat(composio): Stage 2 frontend polish — callback toast, last_used & expired UI, e2e (MUL-3718) (#4688)
* feat(composio): callback toast + refresh, last_used & expired UI, e2e (MUL-3718)
Co-authored-by: multica-agent <github@multica.ai>
* fix(composio): real callback redirect route + StrictMode-safe toast dedup (MUL-3718 review)
Co-authored-by: multica-agent <github@multica.ai>
---------
Co-authored-by: multica-agent <github@multica.ai>
* fix(composio): callback endpoint should not require Multica auth (MUL-3843) (#4709)
* fix(composio): move OAuth callback out of the Auth group (MUL-3843)
Composio 302-redirects the browser to /api/integrations/composio/callback
at the end of the OAuth flow, but PR #4608 mounted it inside the cookie-auth
middleware group. When the session cookie is absent (expired session,
SameSite=Strict / Safari ITP, private window, self-hosted callback subdomain)
the Auth middleware returned a hard 401 and a JSON blob instead of the
settings redirect, breaking the flow.
Identity never came from the cookie anyway: it is carried by the HMAC-signed
state param that CompleteCallback verifies (signature, expiry, replay) and
cross-checked by verifyAccountOwnership; h.Composio == nil still 503s. So the
callback is registered alongside the other public OAuth/webhook routes; the
other four composio endpoints stay session-gated.
Refs MUL-3843, MUL-3715.
Co-authored-by: multica-agent <github@multica.ai>
* fix(composio): correct stale callback routing comments (MUL-3843)
The package header and ComposioCallback doc comments still described the
callback as sitting under the Auth middleware group. After the route was
moved out (this PR), update both to state it is a public route whose identity
comes from the signed state — addressing review nit from 张大彪.
Refs MUL-3843.
Co-authored-by: multica-agent <github@multica.ai>
---------
Co-authored-by: multica-agent <github@multica.ai>
* feat(composio): inject MCP overlay into agent runtime at task dispatch (MUL-3721) (#4704)
Stage 3 of the Composio epic. Wires the per-user Composio MCP session into
every agent task so the agent process sees the initiator's connected tools
without any prompt-time plumbing.
Server side
- Migration 128 adds agent_task_queue.runtime_mcp_overlay JSONB plus a
BEFORE-UPDATE trigger that wipes the column on any transition into a
terminal status (completed / failed / cancelled). A trigger is the single
source of truth — future queries that flip status cannot bypass it.
- composio.Service.BuildTaskOverlay(userID) reuses CreateMCPSession and
emits the Claude-style { mcpServers: { composio: { type: http, url,
headers } } } shape the daemon's existing sidecar generators consume.
Returns (nil, nil) on zero active connections so we never burn a
Composio session for a user with nothing to call.
- TaskService grows a Composio ComposioOverlayBuilder seam, wired in
router.go after composiointeg.NewService succeeds. Five enqueue paths
(issue / mention / quick-create / chat / auto-retry) attach the overlay
after CreateAgentTask returns and before the daemon is notified — so
every claim reads a settled row, with no second daemon hop. Best-effort:
a builder failure logs and proceeds with no overlay.
- resolveInitiatorFromTriggerComment derives the initiator user from the
trigger comment when it was authored by a member. Agent-authored
triggers are not treated as initiators (their connected-apps view is
empty by construction).
Daemon side
- handler/daemon.go claim path merges task.runtime_mcp_overlay onto
agent.mcp_config via mergeMCPOverlay before populating
TaskAgentData.McpConfig. Overlay wins on server-name collisions
because it carries the live user-scoped session URL. Errors fall back
to the agent config unchanged — a bad overlay must not surprise-disable
saved MCP tools. The existing execenv sidecar generators (cursor /
codex / openclaw / opencode / hermes-kiro) need no changes: they keep
consuming the merged result through TaskAgentData.McpConfig.
Tests
- 9 merge cases (mcp_overlay_test): both-nil short-circuit, agent-only
pass-through, overlay-only canonicalization, two-side merge, name
collision (overlay wins), top-level key preservation, malformed agent
fallback, malformed overlay fallback, non-object server rejection.
- 4 dispatch cases (composio): zero-connections returns nil without
CreateSession, happy-path emits the right shape with the right user
id, empty-URL defensive branch, SDK error surfacing.
- 4 TaskService helper cases: nil Composio is a no-op (Queries-safe),
invalid initiator does not call the builder, nil overlay skips the
UPDATE, builder error swallowed without panic.
- Migration 128 verified to roll up + down + up cleanly against the test
database.
Out of scope (deferred): assignment-triggered enqueue paths with no
trigger comment get no overlay attached today (no initiator UUID flows
through enqueueIssueTask in that case). Retry paths recompute the overlay
fresh from the parent's initiator_user_id instead of inheriting the bearer
from the parent row, so a stale token can never resurface on a retry.
Co-authored-by: Eve <eve@multica.ai>
Co-authored-by: multica-agent <github@multica.ai>
* feat(composio): per-agent allowlist + originator-scoped MCP overlay (MUL-3869) (#4736)
* feat(composio): per-agent allowlist + originator-scoped MCP overlay (MUL-3869)
Stage 3.1 of the Composio epic (MUL-3721 parent). PR #4704 wired in the
runtime_mcp_overlay column and a per-task dispatch hook; this change
inverts the default from "all-on" to opt-in and locks the overlay to the
agent owner's own connected apps:
- Agents carry composio_toolkit_allowlist TEXT[]. NULL or [] => no MCP.
Owner-only read/write; non-owner GET/PUT silently redacts/drops the
field (same shape as mcp_config).
- agent_task_queue carries originator_user_id UUID. Set from the
top-of-chain HUMAN at every enqueue path:
* issue/mention comment by member -> author_id
* issue/mention comment by agent -> inherit via comment.source_task_id
-> parent task originator_user_id
* quick-create -> requester_id
* chat -> initiator_user_id
* retry -> SQL-inherited from parent row
* autopilot -> NULL (system-driven)
- BuildTaskOverlay (composio dispatch) now takes (ctx, originatorUserID,
agent) and short-circuits on five gates: invalid originator,
originator != agent.owner_id, empty allowlist, empty intersection of
allowlist ∩ active connections, defensive empty session URL. Composio
CreateSession is called with BOTH `toolkits.slugs` (the intersection)
AND `connected_accounts` (the pinned account ids), narrowing the
tool-router twice.
- The originator-vs-owner gate closes the agent-fanout privacy hole: any
workspace member who can @-mention a public agent used to project the
owner's connected apps into their run. Now the overlay only mounts
when the human at the top of the chain IS the agent owner.
Tests:
- dispatch_test.go covers all 5 gates plus uppercase/whitespace slug
normalisation.
- task_runtime_mcp_overlay_test.go covers the no-op gates of the new
applyRuntimeMCPOverlay signature.
- agent_composio_allowlist_test.go (handler): owner roundtrip
(list/empty/null), workspace-admin silent-drop, owner-only GET
visibility, pure normaliseComposioToolkitAllowlist.
- resolve_originator_test.go (service, DB-backed): member-authored,
agent-authored inherits via comment.source_task_id, invalid id.
Migration 129 up/down/up verified against docker postgres.
Co-authored-by: multica-agent <github@multica.ai>
* chore(composio): gofmt + regenerate sqlc with v1.31.1 (MUL-3869 review nits)
Co-authored-by: multica-agent <github@multica.ai>
---------
Co-authored-by: Eve <eve@multica-ai.local>
Co-authored-by: multica-agent <github@multica.ai>
* fix(composio): accept nested connected account auth config
* feat(views): creator-only MCP tab for per-agent Composio allowlist (MUL-3870) (#4743)
Stage 3.2 frontend on top of the Stage 3.1 backend (MUL-3869,
|
||
|
Jiayuan
|
a00485cf13 |
feat(security): redact sensitive information in agent live output
Server-side (primary): Apply redact.Text/InputMap on task message content, output, and input fields before DB persistence and WebSocket broadcast. Extended redact package with GitLab tokens, JWTs, connection strings, and PASSWORD/SECRET/TOKEN env var patterns. Frontend (fallback): redactSecrets utility mirrors server patterns, applied in buildTimeline and ToolCallRow render as a safety net. |
||
|
Jiayuan
|
0491350f1b |
feat(security): add agent output redaction and private agent assignment enforcement
- Add redact package to detect and mask secrets (AWS keys, private keys, API tokens, bearer tokens, credentials, home paths) in agent output before posting as comments in TaskService - Enforce agent visibility on issue assignment: private agents can only be assigned by their owner or workspace admins - Add visibility picker (workspace/private) to CreateAgentDialog, default to private - Grey out unassignable private agents in the assignee picker with lock icon indicator |