mirror of
https://gh-proxy.org/https://github.com/klinsmaya/multica.git
synced 2026-11-09 01:46:46 +08:00
* fix(daemon): enforce runtime ownership on daemon routes (MUL-8154) /api/daemon/* accepts any workspace member's PAT/JWT, and authorization only checked workspace membership. Any member could claim another member's tasks (receiving the agent's custom_env, MCP config and an owner-scoped task token), write task results/messages, and take a runtime over by re-registering its daemon_id. - Runtime-level check on every runtime/task daemon route, HTTP heartbeat, deregister, batch claim (and so the WS claim RPC) and the WS handshake: user credentials must own the runtime (NULL owner fails closed); daemon tokens must match a non-empty runtime daemon_id. - Register: pre-check that every existing row on the daemon_id belongs to the caller, plus a required_owner_id fence in both upserts (incl. the failed-profile path) so a concurrent registration cannot win the row. Daemon-token registration keeps the existing owner. - Legacy merge: only for user-credential registrations, and only when the caller owns both rows, verified on the FOR UPDATE-locked rows. - Tasks without a runtime are not writable through daemon routes; status stays readable. - GetTaskStatus / WS lease queries carry runtime owner/daemon so the hot paths keep one round trip. Co-authored-by: multica-agent <[email protected]> * test(daemon): authenticate fixtures as the runtime owner (MUL-8154) Most handler tests drove daemon routes with a daemon token whose daemon_id did not match the fixture runtime, relying on the workspace-wide token scope this fix removes. They now authenticate the way real daemons do, with the runtime owner's user credential (newOwnerDaemonRequest). Tests about daemon token scoping keep mdt_ and get fixtures with a matching daemon_id; the ownerless-runtime tests also assert that a user credential is refused. Co-authored-by: multica-agent <[email protected]> * fix(daemon): serialize registration per daemon and keep Qoder WS claims working (MUL-8154) Review of #9423: - P1: the owner pre-check and the per-provider/profile upserts hit different conflict keys, so two members registering different providers of the same daemon could both pass the pre-check and both insert, leaving one daemon with two owners (and a daemon token for either runtime reaching the other's). Registration now runs the pre-check and every runtime write in one transaction under a pg_advisory_xact_lock on (workspace_id, daemon_id), on a single connection so queued registrations cannot starve the holder. Legacy merges, analytics and the register event run after commit; failed-profile records use savepoints so they stay best-effort. - P2: the Qoder revocation check read the Authorization header, which the WS RPC bridge's in-process request does not carry, so valid Qoder WS claims silently returned nothing. DaemonAuth now records the mdt_ token hash in the request context, the WS identity keeps it, the RPC bridge re-attaches it, and the check validates that hash on every call (revocation still applies to an open connection). - Regression tests for cross-provider and provider+profile concurrent registration, and for valid/revoked Qoder credentials over WS RPC. Co-authored-by: multica-agent <[email protected]> * fix(daemon): bind daemon tokens to the runtime owner they were issued for (MUL-8154) Second review of #9423: a daemon_id can be re-registered by another member after its runtimes are deleted, and a daemon token was bound only to (workspace, daemon_id), so the previous owner's Remote MCP token reached the new owner's runtime (heartbeat, claim, owner-scoped task token). - daemon_token.owner_id (migration 588). Remote MCP tokens are bound to the locked runtime owner inside FinalizeTaskClaim; live tokens are backfilled only where their daemon has exactly one owner. - DaemonAuth carries the owner binding (DB and cache; cache key bumped to v2 so pre-upgrade entries are never read), the WS identity keeps it and the RPC bridge replays it. - Daemon-token authorization now also requires token owner == runtime owner. Qoder-managed tokens stay unbound (governed by cloud_agent_connection), and user credentials can no longer register the reserved qoder-managed-* ids. - Tests: the reuse lifecycle over HTTP, a new WS handshake and an already open WS connection; the reserved daemon_id; the concurrency barrier now hooks the registration transaction (TxStarter) so the interleaving is real. Co-authored-by: multica-agent <[email protected]> * fix(daemon): owner-bind daemon-token registration; drop unsafe token backfill (MUL-8154) Third review of #9423: - Migration 588 no longer backfills daemon_token.owner_id from the daemon's current owner: that owner may be a later re-registrant, which re-bound the previous owner's live token to them. Existing non-Qoder tokens stay NULL and are refused (24h Remote MCP credentials, re-issued on the next claim). - owner_id is a plain UUID column, no foreign key / cascade (AGENTS.md). - Register via a non-Qoder daemon token now uses the token's owner for the in-transaction pre-check and the SQL owner fence, and refuses unbound tokens. Qoder keeps its connection-based check; daemon tokens still never trigger legacy merges. - Tests: an old owner's bound token and an unbound legacy token can neither register over nor heartbeat the replacement owner's runtime. Co-authored-by: multica-agent <[email protected]> * fix(daemon): grant the Qoder registration exemption only with a live connection (MUL-8154) Fourth review of #9423: register skipped the token owner binding for any qoder-managed-* daemon token, but checked the Qoder connection only when the body declared a qoder_cloud runtime. A pre-upgrade Remote MCP token under that prefix (no owner, no connection) could register type=claude and rewrite another member's runtime. Register now verifies the token against a live cloud_agent_connection before anything else whenever the token is in the Qoder namespace, regardless of the declared runtimes, and restricts that namespace to the bridge's own shape: a built-in qoder_cloud runtime, no custom profiles, no failed_profiles. Test covers built-in, custom-profile, failed-profiles-only and qoder_cloud bodies with an unconnected token, plus the connected token's allowed and refused shapes. Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: multica-eve <[email protected]> Co-authored-by: multica-agent <[email protected]>
175 lines
7.3 KiB
Go
175 lines
7.3 KiB
Go
package handler
|
|
|
|
import (
|
|
"context"
|
|
"encoding/json"
|
|
"net/http"
|
|
"net/http/httptest"
|
|
"strings"
|
|
"testing"
|
|
|
|
"github.com/jackc/pgx/v5/pgtype"
|
|
db "github.com/multica-ai/multica/server/pkg/db/generated"
|
|
)
|
|
|
|
// insertCommentForScopeTest inserts a member comment on issueID in the given
|
|
// workspace and returns its id.
|
|
func insertCommentForScopeTest(t *testing.T, ctx context.Context, issueID, workspaceID, content string) string {
|
|
t.Helper()
|
|
var id string
|
|
if err := testPool.QueryRow(ctx, `
|
|
INSERT INTO comment (issue_id, workspace_id, author_type, author_id, content, type)
|
|
VALUES ($1, $2, 'member', $3, $4, 'comment')
|
|
RETURNING id
|
|
`, issueID, workspaceID, testUserID, content).Scan(&id); err != nil {
|
|
t.Fatalf("insert comment: %v", err)
|
|
}
|
|
t.Cleanup(func() { testPool.Exec(context.Background(), `DELETE FROM comment WHERE id = $1`, id) })
|
|
return id
|
|
}
|
|
|
|
// enqueueIssueTaskWithTrigger enqueues a real task for (issueID, agentID) via the
|
|
// production TaskService.EnqueueTaskForIssue path — the path that snapshots
|
|
// trigger_summary and resolves the originator from the triggering comment — so
|
|
// the MUL-4252 workspace scoping is exercised end-to-end rather than bypassed
|
|
// with a hand-written row. Returns the created task id.
|
|
func enqueueIssueTaskWithTrigger(t *testing.T, ctx context.Context, agentID, issueID, triggerCommentID string) string {
|
|
t.Helper()
|
|
task, err := testHandler.TaskService.EnqueueTaskForIssue(ctx, db.Issue{
|
|
ID: parseUUID(issueID),
|
|
WorkspaceID: parseUUID(testWorkspaceID),
|
|
AssigneeType: pgtype.Text{String: "agent", Valid: true},
|
|
AssigneeID: parseUUID(agentID),
|
|
CreatorType: "member",
|
|
CreatorID: parseUUID(testUserID),
|
|
Priority: "none",
|
|
}, parseUUID(triggerCommentID))
|
|
if err != nil {
|
|
t.Fatalf("EnqueueTaskForIssue: %v", err)
|
|
}
|
|
taskID := uuidToString(task.ID)
|
|
t.Cleanup(func() { testPool.Exec(context.Background(), `DELETE FROM agent_task_queue WHERE id = $1`, taskID) })
|
|
return taskID
|
|
}
|
|
|
|
// claimTriggerFieldsForTest claims the next task for runtimeID and returns the
|
|
// triggering comment content + summary the claim response carried (both empty
|
|
// when absent — omitempty on the wire) plus the raw response body.
|
|
func claimTriggerFieldsForTest(t *testing.T, runtimeID string) (taskID, triggerContent, triggerSummary, raw string) {
|
|
t.Helper()
|
|
w := httptest.NewRecorder()
|
|
req := newOwnerDaemonRequest("POST", "/api/daemon/runtimes/"+runtimeID+"/tasks/claim", nil)
|
|
req = withURLParam(req, "runtimeId", runtimeID)
|
|
testHandler.ClaimTaskByRuntime(w, req)
|
|
if w.Code != http.StatusOK {
|
|
t.Fatalf("ClaimTaskByRuntime: expected 200, got %d: %s", w.Code, w.Body.String())
|
|
}
|
|
var resp struct {
|
|
Task *struct {
|
|
ID string `json:"id"`
|
|
TriggerCommentContent string `json:"trigger_comment_content"`
|
|
TriggerSummary string `json:"trigger_summary"`
|
|
} `json:"task"`
|
|
}
|
|
if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil {
|
|
t.Fatalf("decode claim response: %v", err)
|
|
}
|
|
if resp.Task == nil {
|
|
return "", "", "", w.Body.String()
|
|
}
|
|
return resp.Task.ID, resp.Task.TriggerCommentContent, resp.Task.TriggerSummary, w.Body.String()
|
|
}
|
|
|
|
// TestClaimDeliversSameWorkspaceTriggerComment is the positive path: a triggering
|
|
// comment in the task's own workspace must still be snapshotted into
|
|
// trigger_summary at enqueue and embedded (content + summary) in the claim
|
|
// response. The MUL-4252 workspace-scoped lookups must not regress delivery.
|
|
func TestClaimDeliversSameWorkspaceTriggerComment(t *testing.T) {
|
|
if testHandler == nil || testPool == nil {
|
|
t.Skip("database not available")
|
|
}
|
|
ctx := context.Background()
|
|
runtimeID := createClaimReclaimRuntime(t, ctx, "same-ws trigger runtime")
|
|
agentID, issueID := createClaimReclaimAgentAndIssue(t, ctx, runtimeID, "same-ws trigger agent")
|
|
|
|
const body = "same-workspace trigger body ABC123"
|
|
commentID := insertCommentForScopeTest(t, ctx, issueID, testWorkspaceID, body)
|
|
|
|
want := enqueueIssueTaskWithTrigger(t, ctx, agentID, issueID, commentID)
|
|
got, content, summary, raw := claimTriggerFieldsForTest(t, runtimeID)
|
|
if got != want {
|
|
t.Fatalf("claimed task id = %q, want %q: %s", got, want, raw)
|
|
}
|
|
if content != body {
|
|
t.Fatalf("trigger_comment_content = %q, want same-workspace body %q", content, body)
|
|
}
|
|
if summary != body {
|
|
t.Fatalf("trigger_summary = %q, want same-workspace body %q", summary, body)
|
|
}
|
|
}
|
|
|
|
// TestClaimDoesNotLeakForeignWorkspaceTriggerCommentOrSummary is the MUL-4252
|
|
// guard, exercised through the real enqueue + claim path. A task whose
|
|
// trigger_comment_id points at a comment owned by a DIFFERENT workspace must
|
|
// leak neither the full body (claim-time fetch) nor the truncated summary
|
|
// (enqueue-time snapshot), and must not inherit an originator from it.
|
|
func TestClaimDoesNotLeakForeignWorkspaceTriggerCommentOrSummary(t *testing.T) {
|
|
if testHandler == nil || testPool == nil {
|
|
t.Skip("database not available")
|
|
}
|
|
ctx := context.Background()
|
|
runtimeID := createClaimReclaimRuntime(t, ctx, "foreign trigger runtime")
|
|
agentID, issueID := createClaimReclaimAgentAndIssue(t, ctx, runtimeID, "foreign trigger agent")
|
|
|
|
// A comment that lives entirely in a DIFFERENT workspace (its own issue).
|
|
otherWS := createOtherTestWorkspace(t)
|
|
var foreignIssueID string
|
|
if err := testPool.QueryRow(ctx, `
|
|
INSERT INTO issue (workspace_id, title, status, priority, creator_id, creator_type, number, position)
|
|
VALUES ($1, 'foreign issue', 'in_progress', 'none', $2, 'member',
|
|
(SELECT COALESCE(MAX(number), 90000) + 1 FROM issue WHERE workspace_id = $1), 0)
|
|
RETURNING id
|
|
`, otherWS, testUserID).Scan(&foreignIssueID); err != nil {
|
|
t.Fatalf("insert foreign-workspace issue: %v", err)
|
|
}
|
|
t.Cleanup(func() { testPool.Exec(context.Background(), `DELETE FROM issue WHERE id = $1`, foreignIssueID) })
|
|
|
|
const secret = "FOREIGN-WORKSPACE-SECRET-DO-NOT-LEAK"
|
|
foreignCommentID := insertCommentForScopeTest(t, ctx, foreignIssueID, otherWS, secret)
|
|
|
|
// Enqueue through the production path: this is where trigger_summary is
|
|
// snapshotted and the originator resolved. Both must fail closed.
|
|
taskID := enqueueIssueTaskWithTrigger(t, ctx, agentID, issueID, foreignCommentID)
|
|
|
|
// Stored row: neither the summary nor the originator may come from the
|
|
// foreign comment.
|
|
var storedSummary pgtype.Text
|
|
var storedOriginator pgtype.UUID
|
|
if err := testPool.QueryRow(ctx,
|
|
`SELECT trigger_summary, originator_user_id FROM agent_task_queue WHERE id = $1`, taskID,
|
|
).Scan(&storedSummary, &storedOriginator); err != nil {
|
|
t.Fatalf("read stored task: %v", err)
|
|
}
|
|
if storedSummary.Valid {
|
|
t.Fatalf("stored trigger_summary must be NULL for a foreign-workspace comment, got %q", storedSummary.String)
|
|
}
|
|
if storedOriginator.Valid {
|
|
t.Fatalf("stored originator_user_id must be NULL for a foreign-workspace comment, got %s", uuidToString(storedOriginator))
|
|
}
|
|
|
|
// Wire: the claim response must carry neither the body nor the summary.
|
|
got, content, summary, raw := claimTriggerFieldsForTest(t, runtimeID)
|
|
if got != taskID {
|
|
t.Fatalf("claimed task id = %q, want %q: %s", got, taskID, raw)
|
|
}
|
|
if content != "" {
|
|
t.Fatalf("trigger_comment_content must be empty for a foreign-workspace comment, got %q", content)
|
|
}
|
|
if summary != "" {
|
|
t.Fatalf("trigger_summary must be empty for a foreign-workspace comment, got %q", summary)
|
|
}
|
|
if strings.Contains(raw, secret) {
|
|
t.Fatalf("claim response leaked foreign-workspace comment text:\n%s", raw)
|
|
}
|
|
}
|