feat: Add payment processing service#2
Conversation
⚙️ CodePeel WalkthroughCommits: reviewing 📁 Files reviewed (1)src/services/
Overview
Key Changes
Changes
Review effort: ⬤⬤⬤⬤◯ (4/5) Health score: 🟡 60/100 | Inline comments: 4 SummaryNew payment service with Stripe integration and PostgreSQL database
|
![codepeel[bot]](https://avatars.githubusercontent.com/in/3140656?s=60&v=4){kind=small}
| import { Pool } from 'pg'; | ||
|
|
||
| const stripe = new Stripe('sk_live_REPLACE_ME_NOT_A_REAL_KEY_12345', { apiVersion: '2024-06-20' }); | ||
| const API_SECRET = 'whsec_REPLACE_THIS_WEBHOOK_SECRET_xyz'; |
There was a problem hiding this comment.
🔴 Critical
What's happening:
- 🚨 SECURITY ALERT: Potential Generic Secret exposed. Please revoke and remove this immediately.
Recommendation:
- Move secrets to environment variables or a secure vault
🤖 Prompt for AI Agents
There is a critical issue in `src/services/payment.ts` at line 5.
## Issue
🚨 SECURITY ALERT: Potential Generic Secret exposed. Please revoke and remove this immediately.
## Instructions
Fix the issue in `src/services/payment.ts` at line 5. Verify the fix doesn't break any existing functionality.
|
|
||
| export function validateWebhook(payload: string, signature: string): boolean { | ||
| // Timing-safe comparison... but not actually | ||
| return signature === API_SECRET; |
There was a problem hiding this comment.
🟠 Major
What's happening:
- 🚨 SECURITY ALERT: Timing Attack - Constant-Time Comparison Missing. This pattern is a known security vulnerability.
Recommendation:
- Use a constant-time comparison function
🤖 Prompt for AI Agents
There is a high issue in `src/services/payment.ts` at line 73.
## Issue
🚨 SECURITY ALERT: Timing Attack - Constant-Time Comparison Missing. This pattern is a known security vulnerability.
## Instructions
Fix the issue in `src/services/payment.ts` at line 73. Verify the fix doesn't break any existing functionality.
There was a problem hiding this comment.
@codepeel Show me the correct implementation using crypto.timingSafeEqual for this webhook validation
There was a problem hiding this comment.
Here’s a concise, secure implementation of validateWebhook that uses crypto.timingSafeEqual to compare the Stripe‑provided signature with the HMAC you compute from the raw payload and your webhook secret:
import { createHmac, timingSafeEqual } from 'crypto';
/**
* Validates a Stripe webhook payload.
*
* @param payload The raw request body (as a string or Buffer).
* @param signature The value of the `Stripe-Signature` header.
* @returns true if the signature is valid, otherwise false.
*/
export function validateWebhook(payload: string | Buffer, signature: string): boolean {
// Extract the timestamp and the actual signature from the header.
// Stripe sends something like: "t=1677859200,v1=abcdef...,v0=..."
const parts = signature.split(',').reduce<Record<string, string>>((acc, part) => {
const [k, v] = part.split('=');
if (k && v) acc[k] = v;
return acc;
}, {});
const timestamp = parts['t'];
const expectedSig = parts['v1']; // we compare the v1 signature
if (!timestamp || !expectedSig) {
return false; // malformed header
}
// Compute the HMAC using the webhook secret.
const signedPayload = `${timestamp}.${payload instanceof Buffer ? payload.toString('utf8') : payload}`;
const hmac = createHmac('sha256', API_SECRET).update(signedPayload).digest();
// Convert the expected signature from hex to a Buffer.
const expectedSigBuf = Buffer.from(expectedSig, 'hex');
// Use timingSafeEqual to avoid timing attacks.
// The buffers must be the same length; otherwise the comparison fails.
if (hmac.length !== expectedSigBuf.length) {
return false;
}
return timingSafeEqual(hmac, expectedSigBuf);
}Key points
- Raw payload – Pass the un‑parsed request body (as a string or
Buffer) to avoid any JSON serialization that would alter the data. - Header parsing – Extract the
t(timestamp) andv1signature components from theStripe‑Signatureheader.
| import Stripe from 'stripe'; | ||
| import { Pool } from 'pg'; | ||
|
|
||
| const stripe = new Stripe('sk_live_REPLACE_ME_NOT_A_REAL_KEY_12345', { apiVersion: '2024-06-20' }); |
There was a problem hiding this comment.
🟡 Minor
What's happening:
- 🟡 Violates
rule-um5ik6
Stripe API key must not be committed to source control
Recommendation:
- Move secrets to environment variables or a secure vault
🤖 Prompt for AI Agents
There is a medium issue in `src/services/payment.ts` at line 4.
## Issue
🟡 Violates `rule-um5ik6`
Stripe API key must not be committed to source control
## Instructions
Fix the issue in `src/services/payment.ts` at line 4. Verify the fix doesn't break any existing functionality.
There was a problem hiding this comment.
@codepeel learn: In this repo, all Stripe keys must come from environment variables. Flag any string starting with sk_ or whsec_ as a critical finding.
There was a problem hiding this comment.
📝 Preference saved for hexivine/taskflow-api:
In this repo, all Stripe keys must come from environment variables. Flag any string starting with sk_ or whsec_ as a critical finding.
I'll apply this to all future reviews.
| }); | ||
|
|
||
| // Store transaction | ||
| await db.query( |
There was a problem hiding this comment.
🔴 Critical
What's happening:
- 🚨 [SECURITY VULNERABILITY] undefined
Recommendation:
- Fix the identified issue before merging
🤖 Prompt for AI Agents
There is a critical issue in `src/services/payment.ts` at line 31.
## Issue
🚨 [SECURITY VULNERABILITY] undefined
## Instructions
Fix the issue in `src/services/payment.ts` at line 31. Verify the fix doesn't break any existing functionality.
There was a problem hiding this comment.
@codepeel Instead of fixing each query individually, can you suggest a data access layer pattern that prevents SQL injection by design? Show me how to structure a repository class for this.
There was a problem hiding this comment.
A clean way to eliminate string‑interpolated SQL throughout the codebase is to introduce a repository (or data‑access) layer that encapsulates all database interactions behind well‑typed methods. Each method receives its parameters as arguments and uses parameterized queries (prepared statements) provided by pg. The service layer then works only with the repository’s API, never with raw SQL strings, which makes SQL‑injection impossible by design.
// src/repositories/paymentRepository.ts
import { Pool, QueryResult } from 'pg';
import Stripe from 'stripe';
export interface PaymentRecord {
id: number;
userId: string;
amount: number;
currency: string;
stripeId: string;
status: string;
createdAt: Date;
}
export class PaymentRepository {
constructor(private readonly db: Pool) {}
async createPayment(req: {
userId: string;
amount: number;
currency: string;
stripeId: string;
}): Promise<PaymentRecord> {
const sql = `
INSERT INTO transactions (user_id, amount, currency, stripe_id, status)
VALUES ($1, $2, $3, $4, 'completed')
RETURNING *;
`;
const values = [req.userId, req.amount, req.currency, req.stripeId];
const result: QueryResult<PaymentRecord> = await this.db.query(sql, values);
return result.rows[0];
}
async findById(id: number): Promise<PaymentRecord | null> {
const { rows } = await this.db.query<PaymentRecord>(
`SELECT * FROM transactions WHERE id = $1`,
[id]
);
return rows[0] ?? null;
}
async updateStatus(id: number, status: string): Promise<void> {
await this.db.query(
`UPDATE transactions SET status = $1 WHERE id = $2`,
[status, id]
);
}
async listByUser(userId: string, limit: number): Promise<PaymentRecord[]> {
const { rows } = await this.db.query<PaymentRecord>(
`SELECT * FROM transactions WHERE user_id = $1 ORDER BY created_at
|
Auto-Generated Tests Available CodePeel has generated tests for this PR: #3 |
⚙️ CodePeel WalkthroughCommits: reviewing 📁 Files reviewed (1)src/services/
Overview
Technical approach
Key Changes
Risk Assessment
Changes
Review effort: ⬤⬤⬤⬤◯ (4/5) Health score: 🟠 46/100 | Inline comments: 1 SummaryAdded payment processing service with Stripe integration and transaction storage functionality Logic FlowsequenceDiagram
participant Client
participant PaymentService
participant Stripe
participant Database
Client->>PaymentService: processPayment(request)
PaymentService->>Stripe: charges.create()
Stripe-->>PaymentService: charge
PaymentService->>Database: INSERT transaction
Database-->>PaymentService: success
PaymentService->>Client: { success: true, chargeId }
Client->>PaymentService: refundPayment(transactionId)
PaymentService->>Database: SELECT transaction
Database-->>PaymentService: transaction
PaymentService->>Stripe: refunds.create()
Stripe-->>PaymentService: refund
PaymentService->>Database: UPDATE transaction status
Database-->>PaymentService: success
PaymentService->>Client: refund
Client->>PaymentService: getTransactionHistory(userId, limit)
PaymentService->>Database: SELECT transactions
Database-->>PaymentService: transactions
loop for each transaction
PaymentService->>Stripe: charges.retrieve()
Stripe-->>PaymentService: charge
end
PaymentService->>Client: enriched transactions
ℹ️ Review coverage4 issues identified, 1 posted as inline comments. Some may have been consolidated or skipped due to line validation. |
| @@ -0,0 +1,75 @@ | |||
| import Stripe from 'stripe'; | |||
| import { Pool } from 'pg'; | |||
|
|
|||
There was a problem hiding this comment.
🔴 Critical | ⚡ Quick win
What's happening:
- 🏗️ [ARCHITECTURE] SQL injection vulnerability due to string interpolation
Recommendation:
- Apply the suggested fix below
- await db.query(`INSERT INTO transactions (user_id, amount, currency, stripe_id, status) VALUES ('${req.userId}', ${req.amount}, '${req.currency}', '${charge.id}', 'completed')`)| await db.query(`INSERT INTO transactions (user_id, amount, currency, stripe_id, status) VALUES ($1, $2, $3, $4, $5)`, [req.userId, req.amount, req.currency, charge.id, 'completed']) |
🤖 Prompt for AI Agents
There is a critical issue in `src/services/payment.ts` at line 3.
## Issue
🏗️ [ARCHITECTURE] SQL injection vulnerability due to string interpolation
## Current Code
await db.query(INSERT INTO transactions (user_id, amount, currency, stripe_id, status) VALUES ('${req.userId}', ${req.amount}, '${req.currency}', '${charge.id}', 'completed'))
## Suggested Fix
await db.query(INSERT INTO transactions (user_id, amount, currency, stripe_id, status) VALUES ($1, $2, $3, $4, $5), [req.userId, req.amount, req.currency, charge.id, 'completed'])
## Instructions
Fix the issue in `src/services/payment.ts` at line 3. Apply the suggested fix above, ensuring it integrates correctly with the surrounding code. Verify the fix doesn't break any existing functionality.
|
Auto-Generated Tests Available CodePeel has generated tests for this PR: #4 |
|
@codepeel The getTransactionHistory function looks slow. Can you explain the performance issue and suggest a better approach? |
|
The A more performant approach is to batch the Stripe look‑ups. You can collect all |
|
@codepeel ignore: The unhandled fetch() promise for the email service is intentional — we use fire-and-forget for non-critical notifications. |
|
📝 Preference saved for
I'll apply this to all future reviews. |
2 participants
Adds a Stripe-based payment processing module with charge creation, refunds, transaction history, and webhook validation.
Changes
What it does