diff --git a/backend/__tests__/service/teammate-approval.test.js b/backend/__tests__/service/teammate-approval.test.js new file mode 100644 index 0000000000..cd69df8729 --- /dev/null +++ b/backend/__tests__/service/teammate-approval.test.js @@ -0,0 +1,200 @@ +// TASK-512 launch scope: a teammate's agent asks for a write under another +// person's connection, and only the credential owner can approve it. The row +// stores who decided (`decidedBy` + `decidedByRole`) when the decision +// commits, so the grant record (ADR-033 B3/B4) reads it rather than infers it. +process.env.PG_HOST = ''; +process.env.JWT_SECRET = 'teammate-approval-test-only'; +const express = require('express'); +const request = require('supertest'); +const jwt = require('jsonwebtoken'); + +jest.mock('../../services/githubAppService', () => ({ + getPullRequest: jest.fn(async () => ({ head: { sha: 'pinned-head' } })), + mergePullRequest: jest.fn(async () => ({ merged: true, sha: 'merged-head' })), +})); +jest.mock('../../models/ToolCall', () => ({ + create: jest.fn(async () => undefined), + digestArgs: (args) => require('crypto').createHash('sha256').update(JSON.stringify(args || {})).digest('hex'), + reserveBudgetLineage: jest.fn(async () => true), +})); +jest.mock('../../services/agentMessageService', () => ({ + postMessage: jest.fn(async () => ({ success: true, message: { _id: new (require('mongoose').Types.ObjectId)() } })), +})); +jest.mock('../../services/agentEventService', () => ({ enqueue: jest.fn(async () => ({})) })); +jest.mock('../../services/needsYouEmailService', () => ({ sendNeedsYouEmail: jest.fn(async () => undefined) })); +jest.mock('../../config/socket', () => ({ getIO: jest.fn(() => null) })); +const { setupMongoDb, closeMongoDb, clearMongoDb } = require('../utils/testUtils'); +const User = require('../../models/User'); +const Pod = require('../../models/Pod'); +const Integration = require('../../models/Integration'); +const ApprovalAction = require('../../models/ApprovalAction'); +const AttentionItem = require('../../models/AttentionItem'); +const { AgentInstallation } = require('../../models/AgentRegistry'); +const Installable = require('../../models/Installable'); +const { buildGithubToolInstallable } = require('../../services/installable/toolInstallables'); +const grants = require('../../services/roomGrantService'); +const broker = require('../../services/toolBrokerService'); +const approvals = require('../../services/approvalActionService'); +const github = require('../../services/githubAppService'); +const ToolCall = require('../../models/ToolCall'); + +const app = express(); +app.use(express.json()); +app.use('/api/approvals', require('../../routes/approvals')); + +// A owns the GitHub connection. B is A's teammate and owns the agent. +let a; let b; let agent; let + pod; +const human = (name) => User.create({ username: name, email: `${name}@example.test`, password: 'test-password' }); +const bearer = (person) => `Bearer ${jwt.sign({ id: String(person._id), tokenVersion: 0 }, process.env.JWT_SECRET)}`; + +beforeAll(setupMongoDb); +afterAll(closeMongoDb); +beforeEach(async () => { + jest.clearAllMocks(); + await clearMongoDb(); + a = await human('credential-a'); + b = await human('teammate-b'); + agent = await User.create({ + username: 'agent-b', + email: 'agent-b@example.test', + password: 'test-password', + isBot: true, + botType: 'agent', + botMetadata: { agentName: 'agent-b', instanceId: 'default', ownerUserId: String(b._id) }, + }); + pod = await Pod.create({ + name: 'Teammate approval', + type: 'team', + createdBy: a._id, + members: [a._id, b._id, agent._id], + joinPolicy: 'invite-only', + publicRead: false, + }); + await AgentInstallation.create({ + agentName: 'agent-b', + instanceId: 'default', + podId: pod._id, + installedBy: b._id, + version: '1.0.0', + status: 'active', + config: { runtime: { runtimeType: 'hosted' }, hosted: { engine: 'native' } }, + }); + const connection = await Integration.create({ + type: 'github-app', + scope: 'user', + status: 'connected', + createdBy: a._id, + config: { installationId: 'test-install', owner: 'test-owner', repo: 'test-repo' }, + }); + await Installable.create(buildGithubToolInstallable()); + const grant = await grants.mintGrant({ + connectionId: String(connection._id), + installationId: 'test-install', + brokerId: 'test-broker', + target: { kind: 'pod', id: String(pod._id) }, + audience: [String(agent._id)], + tools: ['github.merge_pull_request'], + writeMode: 'write-with-confirm', + expiresAt: new Date(Date.now() + 60000), + }); + await expect(broker.callTool({ + grantId: grant.grantId, + agentUserId: String(agent._id), + agentName: 'agent-b', + instanceId: 'default', + hostedTurn: true, + tool: 'github.merge_pull_request', + args: { pullNumber: 7 }, + })).rejects.toMatchObject({ code: 'approval_required' }); +}); + +const parked = () => ApprovalAction.findOne({ actionType: 'tool_call' }); +const httpDecide = (row, person, decision = 'approved') => request(app) + .post(`/api/approvals/${row._id}/resolve`) + .set('Authorization', bearer(person)) + // Identity fields in the body must not choose who decided. + .send({ + decision, + resolvedBy: String(b._id), + callerUserId: String(b._id), + decidedBy: String(b._id), + decidedByRole: 'decision-owner', + }); + +test('the teammate agent\'s write asks only the credential owner, never the agent owner', async () => { + const row = await parked(); + expect(String(row.ownerUserId)).toBe(String(a._id)); + // B3 records the decider as credential-owner; that holds only while the + // asked human and the connection owner are the same person. + expect(row.toolCall.credentialOwnerId).toBe(String(a._id)); + const queue = await AttentionItem.find({ + 'source.type': 'approval_action', 'source.id': String(row._id), status: 'open', + }).lean(); + expect(queue.map((item) => String(item.recipientUserId))).toEqual([String(a._id)]); + + const asAgentOwner = await httpDecide(row, b); + expect(asAgentOwner.status).toBe(403); + expect(asAgentOwner.body.error).toBe('Only the credential owner can decide this'); + const asAgent = await approvals.resolveApproval({ + approvalId: String(row._id), callerUserId: String(agent._id), decision: 'approved', + }); + expect(asAgent.status).toBe(403); + const pending = await ApprovalAction.findById(row._id); + expect(pending.status).toBe('flagged'); + expect(pending.resolvedBy).toBeUndefined(); + expect(pending.decidedBy).toBeUndefined(); + expect(pending.decidedByRole).toBeUndefined(); + expect(github.mergePullRequest).not.toHaveBeenCalled(); +}); + +test('the credential owner approves the pinned merge once, and the row names them as the decider', async () => { + const row = await parked(); + const result = await httpDecide(row, a); + expect(result.status).toBe(200); + expect(result.body.approval).toMatchObject({ decision: 'approved', toolCallEnding: 'ran' }); + expect(github.mergePullRequest).toHaveBeenCalledTimes(1); + expect(github.mergePullRequest).toHaveBeenCalledWith(expect.objectContaining({ sha: 'pinned-head', pullNumber: 7 })); + + const saved = await ApprovalAction.findById(row._id); + expect(String(saved.resolvedBy)).toBe(String(a._id)); + expect(saved.decidedBy).toBe(String(a._id)); + expect(saved.decidedByRole).toBe('credential-owner'); + expect(saved.toolCall.credentialOwnerId).toBe(String(a._id)); + expect(saved.toolCall.canonicalArgs).toBeUndefined(); + expect(ToolCall.create).toHaveBeenCalledWith(expect.objectContaining({ outcome: 'ok', credentialOwnerId: String(a._id) })); + expect(await AttentionItem.countDocuments({ 'source.id': String(row._id), status: 'open' })).toBe(0); + + expect((await httpDecide(row, a)).status).toBe(409); + expect(github.mergePullRequest).toHaveBeenCalledTimes(1); +}); + +test('the credential owner declining records them without a provider call', async () => { + const row = await parked(); + const result = await httpDecide(row, a, 'declined'); + expect(result.status).toBe(200); + expect(result.body.approval).toMatchObject({ decision: 'declined', toolCallEnding: 'declined' }); + const saved = await ApprovalAction.findById(row._id); + expect(String(saved.resolvedBy)).toBe(String(a._id)); + expect(saved.decidedBy).toBe(String(a._id)); + expect(saved.decidedByRole).toBe('credential-owner'); + expect(saved.toolCall.canonicalArgs).toBeUndefined(); + expect(await AttentionItem.countDocuments({ 'source.id': String(row._id), status: 'open' })).toBe(0); + expect(github.mergePullRequest).not.toHaveBeenCalled(); +}); + +test.each(['sweep', 'late decision'])('an expired parked call names policy as the decider (%s)', async (path) => { + const row = await parked(); + await ApprovalAction.updateOne({ _id: row._id }, { $set: { expiresAt: new Date(Date.now() - 1000) } }); + if (path === 'sweep') { + expect(await approvals.sweepExpiredToolCallApprovals()).toBe(1); + } else { + expect((await httpDecide(row, a)).status).toBe(409); + } + const saved = await ApprovalAction.findById(row._id); + expect(saved.status).toBe('expired'); + expect(saved.decidedBy).toBe('policy'); + expect(saved.decidedByRole).toBe('policy'); + expect(saved.resolvedBy).toBeUndefined(); + expect(github.mergePullRequest).not.toHaveBeenCalled(); +}); diff --git a/backend/models/ApprovalAction.ts b/backend/models/ApprovalAction.ts index 2bf7e090b5..c3a8deef0c 100644 --- a/backend/models/ApprovalAction.ts +++ b/backend/models/ApprovalAction.ts @@ -25,6 +25,9 @@ import mongoose, { Document, Model, Schema, Types } from 'mongoose'; export type ApprovalStatus = 'flagged' | 'resolved' | 'expired' | 'moot'; export type ApprovalDecision = 'approved' | 'declined'; export type ApprovalToolCallEnding = 'ran' | 'failed' | 'refused' | 'declined' | 'expired'; +/** record-v1 `decidedByRole` values a parked call can carry today. The spec + * reserves `decision-owner`; delegated approval is deferred (TASK-512). */ +export type ToolCallDecidedByRole = 'credential-owner' | 'policy'; // Registry of executable action types. Adding an action means adding its // executor in services/approvalExecutors.ts — the enum here only widens in @@ -71,6 +74,13 @@ export interface IApprovalAction extends Document { decision?: ApprovalDecision; resolvedBy?: Types.ObjectId; resolvedAt?: Date; + /** + * tool_call only: who decided the parked call, stored when the decision + * commits so the grant record reads it instead of inferring it (ADR-033 B3/B4). + * A user id with `credential-owner`, or `policy` for both when it expired. + */ + decidedBy?: string; + decidedByRole?: ToolCallDecidedByRole; // ADR-017:201 remains the legacy default. tool_call is terminal on expiry // because it authorizes disclosure to an external provider. decidedAfterExpiry?: boolean; @@ -112,6 +122,8 @@ const ApprovalActionSchema = new Schema( decision: { type: String, enum: ['approved', 'declined'] }, resolvedBy: { type: Schema.Types.ObjectId, ref: 'User' }, resolvedAt: { type: Date }, + decidedBy: { type: String }, + decidedByRole: { type: String, enum: ['credential-owner', 'policy'] }, decidedAfterExpiry: { type: Boolean }, executedAt: { type: Date }, executionResult: { type: Schema.Types.Mixed }, diff --git a/backend/services/approvalActionService.ts b/backend/services/approvalActionService.ts index 1b183db278..354132ee4e 100644 --- a/backend/services/approvalActionService.ts +++ b/backend/services/approvalActionService.ts @@ -805,7 +805,10 @@ export const resolveApproval = async (options: ResolveOptions): Promise