diff --git a/src/routes/billing.ts b/src/routes/billing.ts index 2d78e40c..98c15910 100644 --- a/src/routes/billing.ts +++ b/src/routes/billing.ts @@ -315,7 +315,7 @@ router.get( const requestId = requireString(req.params.requestId, "requestId"); const billingService = getBillingService(req); - const result = await billingService.getByRequestId(requestId); + const result = await billingService.getByRequestId(requestId, user.id); if (!result) { next( diff --git a/src/routes/billing/deduct.test.ts b/src/routes/billing/deduct.test.ts index 8550f03d..bfbe6fb5 100644 --- a/src/routes/billing/deduct.test.ts +++ b/src/routes/billing/deduct.test.ts @@ -222,10 +222,33 @@ describe('POST /api/billing/deduct - developerId validation', () => { expect(deducted.status).toBe(200); expect(lookup.status).toBe(200); expect(fakeService.deduct).toHaveBeenCalledTimes(1); - expect(fakeService.getByRequestId).toHaveBeenCalledWith('req_1'); + expect(fakeService.getByRequestId).toHaveBeenCalledWith('req_1', 'user_123'); expect(app.locals.billingService).toBe(fakeService); }); + it('passes the authenticated user id into the owner-scoped lookup', async () => { + const fakeService = { + deduct: jest.fn(), + getByRequestId: jest.fn().mockResolvedValue(null), + }; + const app = buildApp( + { query: jest.fn() } as unknown as Pool, + fakeService as unknown as BillingService, + ); + + const res = await request(app) + .get('/api/billing/deduct/request/req_foreign') + .set('Authorization', `Bearer ${makeToken('user_other')}`); + + // A non-owned request id resolves to the same 404 as a missing one, so the + // route never reveals that another user's charge exists. + expect(res.status).toBe(404); + expect(fakeService.getByRequestId).toHaveBeenCalledWith( + 'req_foreign', + 'user_other', + ); + }); + it('creates the billing client only once when the app starts', async () => { const fakeClient: jest.Mocked = { getBalance: jest.fn().mockResolvedValue({ balance: '0' }), diff --git a/src/routes/billing/deduct.ts b/src/routes/billing/deduct.ts index 892cdfc2..4399cfe3 100644 --- a/src/routes/billing/deduct.ts +++ b/src/routes/billing/deduct.ts @@ -222,7 +222,7 @@ router.get( const requestId = requireString(req.params.requestId, "requestId"); const billingService = getBillingService(req); - const result = await billingService.getByRequestId(requestId); + const result = await billingService.getByRequestId(requestId, user.id); if (!result) { next( diff --git a/src/services/billing.test.ts b/src/services/billing.test.ts index 04bdade9..614599f8 100644 --- a/src/services/billing.test.ts +++ b/src/services/billing.test.ts @@ -593,13 +593,40 @@ describe('BillingService.getByRequestId', () => { const soroban = createMockSorobanClient(); const svc = new BillingService(pool, soroban.client, { retryDelaysMs: [] }); - const result = await svc.getByRequestId('req_existing'); + const result = await svc.getByRequestId('req_existing', 'user_1'); assert.ok(result !== null); assert.equal(result?.usageEventId, '123'); assert.equal(result?.stellarTxHash, 'tx_abc'); }); + test('scopes the lookup to the owning user and returns null for a foreign request', async () => { + const calls: unknown[][] = []; + const pool = { + query: async (...args: unknown[]) => { + calls.push(args); + const [, params] = args as [string, unknown[]]; + // Mirror the owner-scoped WHERE clause: only the owner's row matches. + return makeQr( + params[1] === 'user_1' ? [{ id: 123, stellar_tx_hash: 'tx_abc' }] : [], + ); + }, + } as unknown as Pool; + + const soroban = createMockSorobanClient(); + const svc = new BillingService(pool, soroban.client, { retryDelaysMs: [] }); + + const owned = await svc.getByRequestId('req_shared', 'user_1'); + assert.ok(owned !== null); + + const foreign = await svc.getByRequestId('req_shared', 'user_2'); + assert.equal(foreign, null); + + const [sql, params] = calls[0] as [string, unknown[]]; + assert.ok(sql.includes('user_id = $2')); + assert.deepEqual(params, ['req_shared', 'user_1']); + }); + test('returns null when request_id is absent', async () => { const pool = { query: async () => makeQr(), @@ -608,7 +635,7 @@ describe('BillingService.getByRequestId', () => { const soroban = createMockSorobanClient(); const svc = new BillingService(pool, soroban.client, { retryDelaysMs: [] }); - const result = await svc.getByRequestId('req_missing'); + const result = await svc.getByRequestId('req_missing', 'user_1'); assert.equal(result, null); }); @@ -621,7 +648,7 @@ describe('BillingService.getByRequestId', () => { const soroban = createMockSorobanClient(); const svc = new BillingService(pool, soroban.client, { retryDelaysMs: [] }); - const result = await svc.getByRequestId('req_failed_soroban'); + const result = await svc.getByRequestId('req_failed_soroban', 'user_1'); assert.ok(result !== null); assert.equal(result?.success, false); diff --git a/src/services/billing.ts b/src/services/billing.ts index 292daa22..eece6298 100644 --- a/src/services/billing.ts +++ b/src/services/billing.ts @@ -544,7 +544,10 @@ export class BillingService { } // --- Idempotency precheck: return early if request has already been processed --- - const existing = await this.getByRequestId(request.requestId); + const existing = await this.getByRequestId( + request.requestId, + request.userId, + ); if (existing) { return { ...existing, @@ -919,15 +922,27 @@ export class BillingService { }; } - async getByRequestId(requestId: string): Promise { + /** + * Look up a previously recorded usage event by its request id. + * + * The lookup is owner-scoped: only the row whose `user_id` matches `userId` + * is ever returned, so callers cannot enumerate or read another user's + * charges and transaction hashes via a guessed/predictable request id. + * A non-owned request id resolves to `null` — the same result as a missing + * request id — so existence is not leaked. + */ + async getByRequestId( + requestId: string, + userId: string, + ): Promise { const result = await this.pool.query<{ id: string; stellar_tx_hash: string | null; }>( `SELECT id, stellar_tx_hash FROM usage_events - WHERE request_id = $1`, - [requestId], + WHERE request_id = $1 AND user_id = $2`, + [requestId, userId], ); if (result.rows.length === 0) return null;