T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/signet-cli.ts:263
- Finding
- Concurrent preflights can exceed the monthly payment cap before recording<![CDATA[ ## Vulnerability Details **File Location**: `scripts/signet-cli.ts:263-282` and `scripts/signet-cli.ts:330-365`; the limitation is also acknowledged in `SKILL.md:21-23` and `SKILL.md:128-132` **Vulnerability Type**: Time-of-check to time-of-use race in financial authorization **Risk Level**: High ### Vulnerable Code ```ts const monthSpent = getMonthSpent(policy.currency); const totalAfter = monthSpent + amount; if (totalAfter > policy.maxPerMonth) { appendLedger({ ts: new Date().toISOString(), amount, currency, payee, purpose, status: 'denied', reason: `Would exceed monthly limit (${policy.currency} ${policy.maxPerMonth})`, ...audit, }); return { result: 'DENY', reason: `Would exceed monthly limit of ${policy.currency} ${policy.maxPerMonth} (already spent ${policy.currency} ${monthSpent.toFixed(2)} this month)`, }; } ``` The locked recording operation occurs only after the external payment is expected to have completed: ```ts async function record( amount: number, currency: string, payee: string, purpose: string, idempotencyKey?: string, callerSkill?: string ): Promise<{ ok: true } | { ok: false; error: string }> { return withLock(() => { const policy = loadPolicyFromConfigOrFile(); if (!policy) return { ok: false, error: 'Policy missing or invalid' }; if (currency !== policy.currency) { return { ok: false, error: `Policy currency is ${policy.currency}; request must use ${policy.currency}` }; } const monthSpent = getMonthSpent(policy.currency); if (monthSpent + amount > policy.maxPerMonth) { return { ok: false, error: `Would exceed monthly limit of ${policy.currency} ${policy.maxPerMonth} (already ${policy.currency} ${monthSpent.toFixed(2)} this month)`, }; } if (idempotencyKey && hasIdempotencyKey(idempotencyKey)) { return { ok: true }; // idempotent: already recorded } appendLedger({ ts: new Date().to ...[truncated 2102 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions Implement an atomic authorization and settlement workflow: 1. Add an `authorize` operation that acquires the ledger lock before a payment is made. 2. Under that lock, validate the complete policy, calculate completed spending plus active reservations, and create a reservation with a unique identifier. 3. Require payment-capable callers to possess a valid reservation before transferring funds. 4. Add a `settle` operation that atomically converts the reservation into a completed ledger entry. 5. Add a `release` operation for failed or cancelled payments. 6. Give reservations a bounded expiration time and reclaim expired reservations under the same lock. 7. Bind each reservation to amount, currency, payee, caller, and idempotency key so it cannot authorize a different transaction. 8. Retain preflight only as an advisory early check; do not describe it as definitive cap enforcement. 9. Add concurrency tests in which several processes attempt payments near the monthly cap and verify that only reserved payments proceed. ]]>