Skip to content

idempotency key support implemented #282

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
OlufunbiIK merged 1 commit into from
Mar 25, 2026
Merged

idempotency key support implemented #282

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions backend/migrations/1769950000000-AddIdempotencyKeyToTips.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import { MigrationInterface, QueryRunner, TableColumn, TableIndex } from 'typeorm';

export class AddIdempotencyKeyToTips1769950000000 implements MigrationInterface {
public async up(queryRunner: QueryRunner): Promise<void> {
await queryRunner.addColumn(
'tips',
new TableColumn({
name: 'idempotencyKey',
type: 'varchar',
length: '128',
isNullable: true,
isUnique: true,
}),
);

await queryRunner.createIndex(
'tips',
new TableIndex({
name: 'IDX_tips_idempotencyKey',
columnNames: ['idempotencyKey'],
isUnique: true,
}),
);
}

public async down(queryRunner: QueryRunner): Promise<void> {
await queryRunner.dropIndex('tips', 'IDX_tips_idempotencyKey');
await queryRunner.dropColumn('tips', 'idempotencyKey');
}
}
11 changes: 10 additions & 1 deletion backend/src/tips/create-tips.dto.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { IsUUID, IsOptional, IsString } from 'class-validator';
import { IsUUID, IsOptional, IsString, MaxLength } from 'class-validator';
import { ApiProperty, ApiPropertyOptional } from '@nestjs/swagger';
import { SanitiseAsPlainText } from '../common/utils/sanitise.util';

Expand Down Expand Up @@ -26,4 +26,13 @@ export class CreateTipDto {
@IsString()
@SanitiseAsPlainText()
message?: string;

@ApiPropertyOptional({
description: 'Client-supplied idempotency key (UUID or opaque string, max 128 chars). Re-submitting with the same key returns the original tip instead of creating a duplicate.',
example: '550e8400-e29b-41d4-a716-446655440099',
})
@IsOptional()
@IsString()
@MaxLength(128)
idempotencyKey?: string;
}
4 changes: 4 additions & 0 deletions backend/src/tips/entities/tip.entity.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,10 @@ export class Tip {
@Column({ length: 64, unique: true })
stellarTxHash: string;

@Column({ length: 128, nullable: true, unique: true })
@Index()
idempotencyKey?: string;

@Column({ length: 56 })
senderAddress: string;

Expand Down
11 changes: 10 additions & 1 deletion backend/src/tips/tips.controller.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,11 @@ export class TipsController {
description: 'User ID of the tipper',
required: true,
})
@ApiHeader({
name: 'Idempotency-Key',
description: 'Optional client-generated key (UUID recommended). Re-submitting with the same key returns the original tip without creating a duplicate.',
required: false,
})
@ApiResponse({
status: HttpStatus.CREATED,
description: 'Tip successfully created',
Expand All @@ -56,11 +61,15 @@ export class TipsController {
async create(
@Body(ModerateMessagePipe) createTipDto: CreateTipDto,
@Headers('x-user-id') userId: string,
@Headers('idempotency-key') idempotencyKeyHeader?: string,
): Promise<Tip> {
if (!userId) {
throw new BadRequestException('User ID header (x-user-id) is required');
}
// Simple validation, in real app use AuthGuard
// Header takes precedence over body field; merge into DTO
if (idempotencyKeyHeader) {
createTipDto.idempotencyKey = idempotencyKeyHeader;
}
Comment on lines +69 to +72
Copy link

@coderabbitai coderabbitai bot Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Header value bypasses DTO validation.

The idempotencyKeyHeader is assigned to createTipDto.idempotencyKey after the DTO has already been validated by the ValidationPipe. This means the header value bypasses @MaxLength(128) validation—a header longer than 128 characters will pass through and could cause a database error or truncation.

Suggested fix: Add explicit validation 
     // Header takes precedence over body field; merge into DTO
     if (idempotencyKeyHeader) {
+      if (idempotencyKeyHeader.length > 128) {
+        throw new BadRequestException('Idempotency-Key header must not exceed 128 characters');
+      }
       createTipDto.idempotencyKey = idempotencyKeyHeader;
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Header takes precedence over body field; merge into DTO
if (idempotencyKeyHeader) {
createTipDto.idempotencyKey = idempotencyKeyHeader;
}
// Header takes precedence over body field; merge into DTO
if (idempotencyKeyHeader) {
if (idempotencyKeyHeader.length > 128) {
throw new BadRequestException('Idempotency-Key header must not exceed 128 characters');
}
createTipDto.idempotencyKey = idempotencyKeyHeader;
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@backend/src/tips/tips.controller.ts` around lines 69 - 72, The
idempotencyKeyHeader is being assigned to createTipDto.idempotencyKey after the
DTO has already passed ValidationPipe, so the header bypasses the
`@MaxLength`(128) check; fix by validating the header before or as part of DTO
validation: either set createTipDto.idempotencyKey from idempotencyKeyHeader
prior to running ValidationPipe (so class-validator enforces `@MaxLength`(128)) or
perform an explicit check on idempotencyKeyHeader (e.g., if
(idempotencyKeyHeader.length > 128) throw new BadRequestException(...)) before
assigning it to createTipDto; reference createTipDto, idempotencyKeyHeader,
ValidationPipe, and the `@MaxLength`(128) constraint when making the change.

return this.tipsService.create(userId, createTipDto);
}

Expand Down
16 changes: 15 additions & 1 deletion backend/src/tips/tips.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,20 @@ export class TipsService {
) {}

async create(userId: string, createTipDto: CreateTipDto): Promise<Tip> {
const { artistId, trackId, stellarTxHash, message } = createTipDto;
const { artistId, trackId, stellarTxHash, message, idempotencyKey } = createTipDto;
Comment on lines 67 to +68
Copy link

@coderabbitai coderabbitai bot Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if fromUser is set anywhere during tip creation
ast-grep --pattern 'fromUser: $_'

Repository: OlufunbiIK/tip-tune

Length of output: 45

🏁 Script executed:

fd -e "\.ts$" tips | grep -E "(entity|service)" | head -20

Repository: OlufunbiIK/tip-tune

Length of output: 45

🏁 Script executed:

cat -n backend/src/tips/tips.entity.ts | head -100

Repository: OlufunbiIK/tip-tune

Length of output: 126

🏁 Script executed:

cat -n backend/src/tips/tips.service.ts | sed -n '60,90p'

Repository: OlufunbiIK/tip-tune

Length of output: 1363

🏁 Script executed:

cat -n backend/src/tips/tips.service.ts | sed -n '160,185p'

Repository: OlufunbiIK/tip-tune

Length of output: 985

🏁 Script executed:

find . -type f -name "*entity*" | grep -i tip

Repository: OlufunbiIK/tip-tune

Length of output: 152

🏁 Script executed:

find . -type f -name "*.ts" | xargs grep -l "fromUser" | head -10

Repository: OlufunbiIK/tip-tune

Length of output: 426

🏁 Script executed:

rg "class Tip" -A 30

Repository: OlufunbiIK/tip-tune

Length of output: 21374

🏁 Script executed:

cat -n backend/src/tips/entities/tip.entity.ts

Repository: OlufunbiIK/tip-tune

Length of output: 4776

Set fromUser field during tip creation.

The Tip entity has a required fromUser field (line 70), but it's not being set in the tipRepository.create() call (lines 168-183). The userId parameter is currently only used to fetch the user and set senderAddress, leaving fromUser undefined. Add fromUser: userId to the repository create call.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@backend/src/tips/tips.service.ts` around lines 67 - 68, The Tip entity's
required fromUser field is missing when creating a new Tip in the create method;
update the tipRepository.create(...) call inside the async create(userId:
string, createTipDto: CreateTipDto): Promise<Tip> function to include fromUser:
userId (alongside senderAddress, artistId, trackId, stellarTxHash, message,
idempotencyKey) so the created Tip has the required fromUser relation populated.


// --- Idempotency key check: replay the original response if key already seen ---
if (idempotencyKey) {
const existing = await this.tipRepository.findOne({
where: { idempotencyKey },
});
if (existing) {
this.logger.log(
`Idempotency replay for key=${idempotencyKey}, tipId=${existing.id}`,
);
return existing;
}
Comment on lines +70 to +80
Copy link

@coderabbitai coderabbitai bot Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Security: Idempotency replay leaks tip data across users.

The lookup queries only by idempotencyKey without verifying ownership. If User A creates a tip with key "abc", and User B later submits a request with the same key, User B receives User A's tip record—exposing senderAddress, amount, message, and other sensitive fields.

Either scope the idempotency key per-user (requires schema change to a compound unique constraint on (fromUser, idempotencyKey)) or validate ownership before returning the cached response.

Minimal fix: Validate ownership before replay 
     if (idempotencyKey) {
       const existing = await this.tipRepository.findOne({
         where: { idempotencyKey },
       });
       if (existing) {
+        // Verify the cached tip belongs to the requesting user
+        if (existing.fromUser !== userId) {
+          throw new ConflictException(
+            'Idempotency key is already in use by another request',
+          );
+        }
         this.logger.log(
           `Idempotency replay for key=${idempotencyKey}, tipId=${existing.id}`,
         );
         return existing;
       }
     }
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@backend/src/tips/tips.service.ts` around lines 70 - 80, The idempotency
lookup returns any record matching idempotencyKey and can leak tip data across
users; update the logic in the idempotency block (the call to
this.tipRepository.findOne and the subsequent use of existing/return existing)
to verify ownership before replaying: either include the current user's
identifier in the findOne query (e.g., where: { idempotencyKey, fromUser:
currentUserId }) or, if you can't change the query, check existing.fromUser ===
currentUserId and only log/return existing when they match; alternatively plan a
schema change to enforce a compound unique constraint on (fromUser,
idempotencyKey) to make the key per-user.

}
Comment on lines +71 to +81
Copy link

@coderabbitai coderabbitai bot Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Race condition: Concurrent requests with the same new key cause unhandled exception.

Two concurrent requests with the same (new) idempotencyKey can both pass the findOne check (TOCTOU), then both attempt to insert. One succeeds; the other throws a QueryFailedError (unique constraint violation) that bubbles up as a 500 error instead of returning the already-created tip.

Wrap the save in a try-catch to handle the duplicate key error gracefully.

Suggested fix: Catch unique constraint violation and retry lookup 
-    const savedTip = await this.tipRepository.save(newTip);
+    let savedTip: Tip;
+    try {
+      savedTip = await this.tipRepository.save(newTip);
+    } catch (error: any) {
+      // Handle race condition: another request inserted the same idempotencyKey
+      if (
+        idempotencyKey &&
+        error.code === '23505' // PostgreSQL unique violation
+      ) {
+        const existing = await this.tipRepository.findOne({
+          where: { idempotencyKey },
+        });
+        if (existing && existing.fromUser === userId) {
+          this.logger.log(
+            `Idempotency replay (race recovery) for key=${idempotencyKey}, tipId=${existing.id}`,
+          );
+          return existing;
+        }
+        throw new ConflictException('Idempotency key collision');
+      }
+      throw error;
+    }

Also applies to: 182-185

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@backend/src/tips/tips.service.ts` around lines 71 - 81, Concurrent requests
using the same idempotencyKey can both pass the tipRepository.findOne check and
then one insert will succeed while the other throws a QueryFailedError (unique
constraint violation); modify the create/save path that persists the tip (where
you call this.tipRepository.save / insert after checking idempotencyKey) to wrap
the save in a try-catch, detect the DB unique-constraint error (QueryFailedError
or specific SQL error code), and on that error re-query tipRepository.findOne({
where: { idempotencyKey } }) and return the existing record instead of letting
the exception bubble; apply the same try-catch pattern to the other similar
block referenced at lines 182-185.


const existingTip = await this.tipRepository.findOne({
where: { stellarTxHash },
Expand Down Expand Up @@ -166,6 +179,7 @@ export class TipsService {
status: TipStatus.VERIFIED,
verifiedAt: new Date(),
stellarTimestamp: new Date(txDetails.created_at),
...(idempotencyKey ? { idempotencyKey } : {}),
});

const savedTip = await this.tipRepository.save(newTip);
Expand Down
Loading