highperfocused/multica
1
0
Fork 0
mirror of https://github.com/multica-ai/multica.git synced 2026-07-30 16:20:35 +02:00
Code Issues Packages Projects Releases Wiki Activity

test(admission): make must-fix 3 acceptance tests prove the invariant (MUL-4525)

Browse Source 
Addresses Elon's round-3 narrow review — the two security tests were not yet
falsifiable against the bugs they must catch.

- Rerun: after enqueuing the historical task, reassign the issue to a SECOND
  agent that the denied user CAN invoke. Now the current assignee and the
  task_id agent differ, so a rerun that wrongly validated the current assignee
  would let the denied user through — the 403 proves the gate is keyed on the
  historical private agent. The allow case now asserts the reran task's agent_id
  is the historical agent, not the current assignee.

- Chat: the blocked send now carries a valid, still-unbound attachment. After
  the 403 the test asserts the attachment's chat_session_id and chat_message_id
  are both still NULL, guarding against anyone moving attachment binding ahead
  of the invoke gate.

Test-only. Full internal/handler Go suite green.

Co-authored-by: multica-agent <github@multica.ai>
This commit is contained in:
J
2026-07-14 17:28:02 +08:00
parent 1acc4516ec
commit 0b4fca4d3a
1 changed files with 61 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

68
server/internal/handler/admission_security_mul4525_test.go
View File

@@ -103,6 +103,20 @@ func TestSendChatMessage_InvokeRevokedAfterSessionCreate(t *testing.T) {
t.Fatalf("revoke invoke: %v", err)
}
// A valid, still-unbound attachment uploaded by the sender. A successful send
// would claim it into the session/message (LinkAttachmentsToChatMessage sets
// chat_session_id + chat_message_id); the blocked send must not — the invoke
// gate runs BEFORE attachment binding, so this guards against anyone later
// moving the binding ahead of the permission gate.
var attachmentID string
if err := testPool.QueryRow(ctx, `
INSERT INTO attachment (workspace_id, uploader_type, uploader_id, filename, url, content_type, size_bytes)
VALUES ($1, 'member', $2, 'unbound.png', 'https://cdn.test/unbound.png', 'image/png', 10)
RETURNING id`, testWorkspaceID, testUserID).Scan(&attachmentID); err != nil {
t.Fatalf("seed unbound attachment: %v", err)
}
t.Cleanup(func() { testPool.Exec(context.Background(), `DELETE FROM attachment WHERE id = $1`, attachmentID) })
countMessages := func() int {
var n int
if err := testPool.QueryRow(ctx, `SELECT count(*) FROM chat_message WHERE chat_session_id = $1`, session.ID).Scan(&n); err != nil {
@@ -121,10 +135,12 @@ func TestSendChatMessage_InvokeRevokedAfterSessionCreate(t *testing.T) {
t.Fatalf("precondition: session should have no messages/tasks yet")
}
// The send must be refused before anything is written.
// The send must be refused before anything is written — including the
// attachment binding.
sendW := httptest.NewRecorder()
sendReq := withChatTestWorkspaceCtx(t, newRequest("POST", "/api/chat/sessions/"+session.ID+"/messages", map[string]any{
"content": "are you still there?",
"content": "are you still there?",
"attachment_ids": []string{attachmentID},
}))
sendReq = withURLParam(sendReq, "sessionId", session.ID)
testHandler.SendChatMessage(sendW, sendReq)
@@ -141,6 +157,19 @@ func TestSendChatMessage_InvokeRevokedAfterSessionCreate(t *testing.T) {
if got := countTasks(); got != 0 {
t.Errorf("blocked send persisted %d tasks, want 0", got)
}
// The attachment must remain unbound: neither claimed into the session nor
// linked to a message.
var boundSession, boundMessage pgtype.UUID
if err := testPool.QueryRow(ctx, `SELECT chat_session_id, chat_message_id FROM attachment WHERE id = $1`, attachmentID).
Scan(&boundSession, &boundMessage); err != nil {
t.Fatalf("read attachment binding: %v", err)
}
if boundSession.Valid {
t.Errorf("blocked send set attachment chat_session_id = %s, want NULL", util.UUIDToString(boundSession))
}
if boundMessage.Valid {
t.Errorf("blocked send set attachment chat_message_id = %s, want NULL", util.UUIDToString(boundMessage))
}
}
// TestRerunIssue_PrivateHistoricalAgent is the MUL-4525 must-fix 3 rerun
@@ -182,6 +211,16 @@ func TestRerunIssue_PrivateHistoricalAgent(t *testing.T) {
}
origID := util.UUIDToString(orig.ID)
// Reassign the issue to a SECOND agent that testUserID CAN invoke (a
// workspace-invocable public_to agent). Now the issue's CURRENT assignee and
// the source task's HISTORICAL agent differ: if the rerun wrongly validated
// the current assignee, testUserID would be allowed — so the 403 below proves
// the gate is keyed on the historical private agent named by task_id.
currentAgentID := createHandlerTestAgent(t, "RerunCurrentAssignee", []byte("[]"))
if _, err := testPool.Exec(ctx, `UPDATE issue SET assignee_id = $1 WHERE id = $2`, currentAgentID, issueID); err != nil {
t.Fatalf("reassign issue to second agent: %v", err)
}
taskCount := func() int {
var n int
if err := testPool.QueryRow(ctx, `SELECT count(*) FROM agent_task_queue WHERE issue_id = $1`, issueID).Scan(&n); err != nil {
@@ -198,13 +237,14 @@ func TestRerunIssue_PrivateHistoricalAgent(t *testing.T) {
}
beforeCount, beforeStatus := taskCount(), origStatus()
// DENY: testUserID (workspace owner) can view the issue but cannot invoke the
// private agent → structured 403, and nothing is cancelled or created.
// DENY: testUserID (workspace owner) can view the issue AND can invoke the
// current assignee, but cannot invoke the HISTORICAL private agent named by
// task_id → structured 403, and nothing is cancelled or created.
denyW := httptest.NewRecorder()
denyReq := withURLParam(newRequest("POST", "/api/issues/"+issueID+"/rerun", map[string]any{"task_id": origID}), "id", issueID)
testHandler.RerunIssue(denyW, denyReq)
if denyW.Code != http.StatusForbidden {
t.Fatalf("RerunIssue as non-invoker: expected 403, got %d: %s", denyW.Code, denyW.Body.String())
t.Fatalf("RerunIssue as non-invoker of historical agent: expected 403, got %d: %s", denyW.Code, denyW.Body.String())
}
if code := readReasonCode(t, denyW.Body.Bytes()); code != string(dispatch.ReasonInvocationNotAllowed) {
t.Errorf("reason_code = %q, want invocation_not_allowed", code)
@@ -216,11 +256,25 @@ func TestRerunIssue_PrivateHistoricalAgent(t *testing.T) {
t.Errorf("blocked rerun changed original task status: got %q, want %q", got, beforeStatus)
}
// ALLOW: the agent owner may rerun their own private agent.
// ALLOW: the HISTORICAL agent's owner may rerun it, and the new task must
// target the historical agent — not the issue's current assignee.
allowW := httptest.NewRecorder()
allowReq := withURLParam(newRequestAs(ownerID, "POST", "/api/issues/"+issueID+"/rerun", map[string]any{"task_id": origID}), "id", issueID)
testHandler.RerunIssue(allowW, allowReq)
if allowW.Code != http.StatusAccepted {
t.Fatalf("RerunIssue as agent owner: expected 202, got %d: %s", allowW.Code, allowW.Body.String())
t.Fatalf("RerunIssue as historical agent owner: expected 202, got %d: %s", allowW.Code, allowW.Body.String())
}
var reran struct {
ID string `json:"id"`
AgentID string `json:"agent_id"`
}
if err := json.Unmarshal(allowW.Body.Bytes(), &reran); err != nil {
t.Fatalf("decode rerun response: %v", err)
}
if reran.AgentID != agentID {
t.Errorf("reran task agent_id = %q, want historical agent %q (not current assignee %q)", reran.AgentID, agentID, currentAgentID)
}
if reran.ID == origID {
t.Errorf("expected a new task id, got the original %q", origID)
}
}