T09 · Insecure Skill Coding Practices
Error
- Location
- runtime/src/mcp-server.ts:343
- Finding
- Client-controlled confirmation flag allows outbound call safeguards to be bypassed<![CDATA[ ## Vulnerability Details **File Location**: `runtime/src/mcp-server.ts:343-474` **Vulnerability Type**: Authorization and confirmation-state bypass **Risk Level**: High ### Vulnerable Code ```ts case 'make_call': { let { to, name: toName, objective, confirmed } = args as { to?: string; name?: string; objective: string; confirmed?: boolean }; if (!OUTBOUND_CALLS_ENABLED) { return { content: [{ type: 'text', text: 'Outbound calling is disabled because AMBER_ENABLE_OUTBOUND_CALLS=false is set. Remove it or set AMBER_ENABLE_OUTBOUND_CALLS=true, then restart Amber.', }], isError: true, }; } // Resolve name → phone number via contacts cache if (!to && toName) { const cachePath = path.join(__dirname, '..', 'contacts-cache.json'); if (!fs.existsSync(cachePath)) { return { content: [{ type: 'text', text: 'Contacts cache not found. Run `npm run sync-contacts` first.' }], isError: true }; } const cache = JSON.parse(fs.readFileSync(cachePath, 'utf8')); const contacts: any[] = cache.contacts || []; const q = toName.toLowerCase(); const matches = contacts.filter((c: any) => `${c.firstName} ${c.lastName}`.toLowerCase().includes(q) || c.firstName?.toLowerCase().includes(q) || c.lastName?.toLowerCase().includes(q) || c.nickname?.toLowerCase().includes(q) ).filter((c: any) => c.phones?.length > 0 || c.phone); if (matches.length === 0) { return { content: [{ type: 'text', text: `No contact found named "${toName}" with a phone number.` }], isError: true }; } if (matches.length > 1) { const names = matches.map((c: any) => `${c.firstName} ${c.lastName}`.trim() + ` (${(c.phones?.[0]?.number || c.phone)})` ).join('\n'); return { content: [{ type: 'text', text: ...[truncated 5439 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions Replace the caller-supplied Boolean confirmation model with a server-enforced, two-phase approval workflow: 1. **Reject first-request confirmation** - Do not accept `confirmed: true` unless the server has already created a matching pending-call proposal. - Treat the Boolean as informational rather than authoritative. 2. **Create server-side pending-call state** - During the preview phase, store the normalized destination, resolved contact identity, objective, requester identity, session identifier, creation time, and a cryptographically random nonce. - Return only the opaque proposal identifier or nonce to the client. 3. **Bind approval to immutable parameters** - Require the confirmation operation to reference the pending proposal. - Dial using the destination and objective stored by the server rather than accepting replacement values from the confirmation request. - Reject approval if any call parameters differ from the previewed proposal. 4. **Require operator-controlled authorization** - Prefer an approval action performed through a trusted operator interface rather than allowing the same autonomous agent to preview and approve its own action. - Associate the approval with an authenticated operator identity. 5. **Expire and consume approvals** - Give proposals a short expiration period. - Make approval tokens single-use. - Delete or invalidate the proposal after approval, rejection, timeout, or call initiation. 6. **Preserve contact restrictions** - If policy requires calls to verified contacts, enforce that restriction regardless of the confirmation value. - Implement a separate, explicit operator override for unrecognized numbers. 7. **Add authorization and audit logging** - Record the proposal, operator approval identity, destination, objective hash, timestamps, and final result. - Avoid placing unnecessary call content or credentials in logs. 8. **Add regression tes ...[truncated 341 chars]