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]>
301 lines
10 KiB
Go
301 lines
10 KiB
Go
package handler
|
|
|
|
import (
|
|
"context"
|
|
"encoding/json"
|
|
"net/http"
|
|
"net/http/httptest"
|
|
"strings"
|
|
"testing"
|
|
|
|
"github.com/go-chi/chi/v5"
|
|
"github.com/google/uuid"
|
|
"github.com/multica-ai/multica/server/internal/util"
|
|
db "github.com/multica-ai/multica/server/pkg/db/generated"
|
|
)
|
|
|
|
// seedNULTask creates an agent with one running task, ready to be driven
|
|
// through a terminal daemon callback.
|
|
func seedNULTask(t *testing.T, label string) (agentID, taskID string) {
|
|
t.Helper()
|
|
ctx := context.Background()
|
|
|
|
if err := testPool.QueryRow(ctx, `
|
|
INSERT INTO agent (workspace_id, name, description, runtime_mode, runtime_config,
|
|
runtime_id, visibility, max_concurrent_tasks, owner_id,
|
|
instructions, custom_env, custom_args)
|
|
VALUES ($1, $2, '', 'cloud', '{}'::jsonb, $3, 'private', 1, $4, '', '{}'::jsonb, '[]'::jsonb)
|
|
RETURNING id`, testWorkspaceID, label, handlerTestRuntimeID(t), testUserID).Scan(&agentID); err != nil {
|
|
t.Fatalf("seed agent: %v", err)
|
|
}
|
|
t.Cleanup(func() {
|
|
testPool.Exec(context.Background(), `DELETE FROM agent WHERE id = $1`, agentID)
|
|
})
|
|
|
|
// The task needs an issue: requireDaemonTaskAccess resolves the workspace
|
|
// through it, and a task with no issue / chat / autopilot link 404s before
|
|
// the handler ever reads the body.
|
|
var issueID string
|
|
if err := testPool.QueryRow(ctx, `
|
|
INSERT INTO issue (workspace_id, title, status, priority, creator_id, creator_type, number, position)
|
|
VALUES ($1, $2, 'in_progress', 'none', $3, 'member',
|
|
(SELECT COALESCE(MAX(number), 0) + 1 FROM issue WHERE workspace_id = $1), 0)
|
|
RETURNING id`, testWorkspaceID, label+" fixture", testUserID).Scan(&issueID); err != nil {
|
|
t.Fatalf("seed issue: %v", err)
|
|
}
|
|
t.Cleanup(func() {
|
|
testPool.Exec(context.Background(), `DELETE FROM issue WHERE id = $1`, issueID)
|
|
})
|
|
|
|
if err := testPool.QueryRow(ctx, `
|
|
INSERT INTO agent_task_queue (agent_id, runtime_id, issue_id, status, priority, started_at)
|
|
VALUES ($1, $2, $3, 'running', 0, now())
|
|
RETURNING id`, agentID, handlerTestRuntimeID(t), issueID).Scan(&taskID); err != nil {
|
|
t.Fatalf("seed running task: %v", err)
|
|
}
|
|
t.Cleanup(func() {
|
|
testPool.Exec(context.Background(), `DELETE FROM agent_task_queue WHERE id = $1`, taskID)
|
|
})
|
|
return agentID, taskID
|
|
}
|
|
|
|
// daemonTaskRequest builds a daemon-authenticated request with taskId bound as
|
|
// a chi URL param, the way the router does in production.
|
|
func daemonTaskRequest(t *testing.T, path, taskID string, body any) *http.Request {
|
|
t.Helper()
|
|
req := newOwnerDaemonRequest("POST", path, body)
|
|
rctx := chi.NewRouteContext()
|
|
rctx.URLParams.Add("taskId", taskID)
|
|
return req.WithContext(context.WithValue(req.Context(), chi.RouteCtxKey, rctx))
|
|
}
|
|
|
|
// TestCompleteTaskCallbackWithNULSucceeds is the /complete half of GH #7098,
|
|
// driven through the real handler.
|
|
//
|
|
// The NUL travels the way the report describes: encoding/json escapes it on the
|
|
// wire and the decoder rebuilds it, so the handler sees a genuine 0x00 the way
|
|
// a daemon would deliver it. Going through CompleteTask (rather than calling
|
|
// the sanitizer by hand) is what makes this a regression test for the WIRING —
|
|
// delete the sanitize call in the handler and this fails.
|
|
func TestCompleteTaskCallbackWithNULSucceeds(t *testing.T) {
|
|
ctx := context.Background()
|
|
_, taskID := seedNULTask(t, "nul-complete-agent")
|
|
|
|
w := httptest.NewRecorder()
|
|
req := daemonTaskRequest(t, "/api/daemon/tasks/"+taskID+"/complete", taskID, map[string]any{
|
|
"output": "done\x00 summary text",
|
|
"work_dir": "/tmp/work\x00dir",
|
|
"durable_work_dir": "/Users/dev/pro\x00ject",
|
|
"session_id": "sess-nul-complete",
|
|
})
|
|
|
|
testHandler.CompleteTask(w, req)
|
|
if w.Code != http.StatusOK {
|
|
t.Fatalf("CompleteTask returned %d, want 200: %s", w.Code, w.Body.String())
|
|
}
|
|
|
|
var (
|
|
status string
|
|
completedAt *string
|
|
storedJSON []byte
|
|
)
|
|
if err := testPool.QueryRow(ctx, `
|
|
SELECT status, completed_at::text, result
|
|
FROM agent_task_queue WHERE id = $1`, taskID).Scan(&status, &completedAt, &storedJSON); err != nil {
|
|
t.Fatalf("read task: %v", err)
|
|
}
|
|
if status != "completed" {
|
|
t.Fatalf("status = %q, want completed", status)
|
|
}
|
|
if completedAt == nil {
|
|
t.Fatal("completed_at is NULL, want a terminal timestamp")
|
|
}
|
|
|
|
var stored TaskCompleteRequest
|
|
if err := json.Unmarshal(storedJSON, &stored); err != nil {
|
|
t.Fatalf("decode stored result: %v", err)
|
|
}
|
|
// Removing rather than rejecting: the readable text survives.
|
|
if stored.Output != "done summary text" {
|
|
t.Fatalf("stored output = %q, want %q", stored.Output, "done summary text")
|
|
}
|
|
if stored.WorkDir != "/tmp/workdir" {
|
|
t.Fatalf("stored work_dir = %q, want %q", stored.WorkDir, "/tmp/workdir")
|
|
}
|
|
if stored.DurableWorkDir != "/Users/dev/project" {
|
|
t.Fatalf("stored durable_work_dir = %q, want %q", stored.DurableWorkDir, "/Users/dev/project")
|
|
}
|
|
}
|
|
|
|
// TestReportTaskMessagesCallbackWithNULSucceeds covers the /messages entry,
|
|
// again through the real handler.
|
|
//
|
|
// Three poisoned places at once, because they fail for three different reasons:
|
|
// type and output are TEXT columns, and input is JSONB where the offending byte
|
|
// sits two levels down inside a tool's arguments while every top-level string
|
|
// is clean.
|
|
func TestReportTaskMessagesCallbackWithNULSucceeds(t *testing.T) {
|
|
ctx := context.Background()
|
|
_, taskID := seedNULTask(t, "nul-messages-agent")
|
|
|
|
w := httptest.NewRecorder()
|
|
req := daemonTaskRequest(t, "/api/daemon/tasks/"+taskID+"/messages", taskID, map[string]any{
|
|
"messages": []any{
|
|
map[string]any{
|
|
"seq": 1,
|
|
"type": "tool_use\x00",
|
|
"tool": "Bash\x00",
|
|
"content": "reading\x00 the binary",
|
|
"output": "ELF\x00\x00binary",
|
|
"input": map[string]any{
|
|
"command": "cat",
|
|
"args": []any{"-n", "build/app.bin"},
|
|
"result": map[string]any{
|
|
"stdout": "ELF\x00\x00binary",
|
|
},
|
|
},
|
|
},
|
|
},
|
|
})
|
|
|
|
testHandler.ReportTaskMessages(w, req)
|
|
if w.Code != http.StatusOK {
|
|
t.Fatalf("ReportTaskMessages returned %d, want 200: %s", w.Code, w.Body.String())
|
|
}
|
|
|
|
var (
|
|
msgType string
|
|
msgTool string
|
|
msgContent string
|
|
msgOutput string
|
|
msgInput []byte
|
|
)
|
|
if err := testPool.QueryRow(ctx, `
|
|
SELECT type, COALESCE(tool, ''), COALESCE(content, ''), COALESCE(output, ''), input
|
|
FROM task_message WHERE task_id = $1 AND seq = 1`, taskID).
|
|
Scan(&msgType, &msgTool, &msgContent, &msgOutput, &msgInput); err != nil {
|
|
t.Fatalf("read persisted task message: %v", err)
|
|
}
|
|
|
|
if msgType != "tool_use" {
|
|
t.Fatalf("stored type = %q, want %q", msgType, "tool_use")
|
|
}
|
|
if msgTool != "Bash" {
|
|
t.Fatalf("stored tool = %q, want %q", msgTool, "Bash")
|
|
}
|
|
if msgContent != "reading the binary" {
|
|
t.Fatalf("stored content = %q, want %q", msgContent, "reading the binary")
|
|
}
|
|
if msgOutput != "ELFbinary" {
|
|
t.Fatalf("stored output = %q, want %q", msgOutput, "ELFbinary")
|
|
}
|
|
|
|
var storedInput struct {
|
|
Result struct {
|
|
Stdout string `json:"stdout"`
|
|
} `json:"result"`
|
|
}
|
|
if err := json.Unmarshal(msgInput, &storedInput); err != nil {
|
|
t.Fatalf("decode stored input: %v", err)
|
|
}
|
|
if storedInput.Result.Stdout != "ELFbinary" {
|
|
t.Fatalf("stored nested stdout = %q, want %q", storedInput.Result.Stdout, "ELFbinary")
|
|
}
|
|
}
|
|
|
|
func TestReportTaskMessagesCreatesUUIDv7(t *testing.T) {
|
|
ctx := context.Background()
|
|
_, taskID := seedNULTask(t, "uuidv7-messages-agent")
|
|
|
|
w := httptest.NewRecorder()
|
|
req := daemonTaskRequest(t, "/api/daemon/tasks/"+taskID+"/messages", taskID, map[string]any{
|
|
"messages": []any{
|
|
map[string]any{
|
|
"seq": 1,
|
|
"type": "text",
|
|
"content": "UUIDv7 task message",
|
|
},
|
|
},
|
|
})
|
|
|
|
testHandler.ReportTaskMessages(w, req)
|
|
if w.Code != http.StatusOK {
|
|
t.Fatalf("ReportTaskMessages returned %d, want 200: %s", w.Code, w.Body.String())
|
|
}
|
|
|
|
var rawID string
|
|
if err := testPool.QueryRow(ctx, `
|
|
SELECT id::text
|
|
FROM task_message WHERE task_id = $1 AND seq = 1`, taskID).Scan(&rawID); err != nil {
|
|
t.Fatalf("read persisted task message id: %v", err)
|
|
}
|
|
messageID, err := uuid.Parse(rawID)
|
|
if err != nil {
|
|
t.Fatalf("parse task message id %q: %v", rawID, err)
|
|
}
|
|
if got := messageID.Version(); got != 7 {
|
|
t.Fatalf("task message UUID version = %d, want 7", got)
|
|
}
|
|
}
|
|
|
|
// TestPostgresRejectsUnsanitizedTaskPayloads is the control for both tests
|
|
// above: it proves the database really does refuse these payloads, so the
|
|
// handler tests are demonstrating a fix rather than a no-op. If PostgreSQL ever
|
|
// starts accepting them, this fails and the guards can be reconsidered.
|
|
func TestPostgresRejectsUnsanitizedTaskPayloads(t *testing.T) {
|
|
ctx := context.Background()
|
|
_, taskID := seedNULTask(t, "nul-control-agent")
|
|
|
|
t.Run("complete result JSONB", func(t *testing.T) {
|
|
raw, err := json.Marshal(TaskCompleteRequest{Output: "done\x00 summary"})
|
|
if err != nil {
|
|
t.Fatalf("marshal: %v", err)
|
|
}
|
|
if !strings.Contains(string(raw), "\\u0000") {
|
|
t.Fatalf("premise broken: payload carries no NUL escape: %s", raw)
|
|
}
|
|
if _, err := testHandler.Queries.CompleteAgentTask(ctx, db.CompleteAgentTaskParams{
|
|
ID: util.MustParseUUID(taskID),
|
|
Result: raw,
|
|
}); err == nil {
|
|
t.Fatal("PostgreSQL accepted a JSONB payload containing a NUL escape")
|
|
}
|
|
|
|
// The rejected write rolled back — which is exactly the reported wedge.
|
|
var status string
|
|
if err := testPool.QueryRow(ctx, `SELECT status FROM agent_task_queue WHERE id = $1`, taskID).Scan(&status); err != nil {
|
|
t.Fatalf("read task: %v", err)
|
|
}
|
|
if status != "running" {
|
|
t.Fatalf("status after rejected write = %q, want running", status)
|
|
}
|
|
})
|
|
|
|
t.Run("task message input JSONB", func(t *testing.T) {
|
|
raw, err := json.Marshal(map[string]any{
|
|
"result": map[string]any{"stdout": "ELF\x00binary"},
|
|
})
|
|
if err != nil {
|
|
t.Fatalf("marshal: %v", err)
|
|
}
|
|
if _, err := testHandler.Queries.CreateTaskMessage(ctx, db.CreateTaskMessageParams{
|
|
TaskID: util.MustParseUUID(taskID),
|
|
Seq: 99,
|
|
Type: "tool_use",
|
|
Input: raw,
|
|
}); err == nil {
|
|
t.Fatal("PostgreSQL accepted nested JSONB containing a NUL escape")
|
|
}
|
|
})
|
|
|
|
t.Run("task message type TEXT", func(t *testing.T) {
|
|
if _, err := testHandler.Queries.CreateTaskMessage(ctx, db.CreateTaskMessageParams{
|
|
TaskID: util.MustParseUUID(taskID),
|
|
Seq: 98,
|
|
Type: "tool_use\x00",
|
|
}); err == nil {
|
|
t.Fatal("PostgreSQL accepted a TEXT value containing a NUL")
|
|
}
|
|
})
|
|
}
|