Skip to content

feat: add split comments API and harden short-link authorization#324

Merged
OlufunbiIK merged 1 commit intoOlufunbiIK:mainfrom
Dataguru-tech:feature/split-comments-api
Mar 29, 2026
Merged

feat: add split comments API and harden short-link authorization#324
OlufunbiIK merged 1 commit intoOlufunbiIK:mainfrom
Dataguru-tech:feature/split-comments-api

Conversation

@Dataguru-tech
Copy link
Copy Markdown
Contributor

@Dataguru-tech Dataguru-tech commented Mar 29, 2026

PR 1 — Split Comments API
Add create/list/delete endpoints to SplitCommentsController with pagination, mention event emission, and owner-only delete authorization. Includes unit tests for mention-triggered behavior.

closes #318
PR 2 — Short-Link Hardening
Enforce split membership checks on link generation, validate target participants belong to the split, restrict analytics and delete to split creator only, and fix max-access and expiry enforcement.

closes #320

Summary by CodeRabbit

Release Notes

New Features

  • Added authorization controls for short-link operations (generation, deletion, analytics viewing)
  • Implemented maximum access limits for short links
  • Added comment management: list comments with pagination and delete comments with user ownership validation

Tests

  • Added comprehensive unit tests for split comment functionality

@vercel
Copy link
Copy Markdown

vercel bot commented Mar 29, 2026

@Dataguru-tech is attempting to deploy a commit to the olufunbiik's projects Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 29, 2026

📝 Walkthrough

Walkthrough

This PR implements authorization hardening for short-links and builds out a complete split comments API. It adds authorization methods for short-link operations and participant membership checks, integrates auth validation into short-links service with new expiry and max-access limits, separates split-comment DTO from persistence concerns, and implements controller endpoints for comment creation, listing, and deletion with comprehensive tests.

Changes

Cohort / File(s) Summary
Authorization Service Enhancements
backend/src/auth/services/authorization.service.ts
Added four new public authorization methods: canGenerateShortLink, canDeleteShortLink, canViewShortLinkAnalytics (delegating to split-access/creator checks), and isParticipantInSplit (direct participant query). Simplified existing checks with early-return patterns and quote normalization.
Short-Links Feature
backend/src/short-links/short-links.service.ts, backend/src/short-links/entities/link-access-log.entity.ts
Service now enforces authorization checks for generate/delete/analytics, validates optional targetParticipantId in split, enforces per-link maxAccesses and expiresAt limits, increments access tracking, and logs access attempts. Entity quote normalization from double to single quotes.
Split Comments Implementation
backend/src/split-comments/split-comments.controller.ts, backend/src/split-comments/provider/provider.service.ts, backend/src/split-comments/dto/split-comment.dto.ts, backend/src/split-comments/split-comment.entity.ts, backend/src/split-comments/split-comments.spec.ts
Added HTTP endpoints for POST (create), GET (list with pagination), and DELETE (with user ownership validation) operations. Service now includes listComments and deleteComment methods with ownership verification. DTO decoupled from TypeORM persistence annotations. New comprehensive Jest test suite covering creation, mention-event emission, listing, deletion, and ownership enforcement.

Sequence Diagram

sequenceDiagram
    participant User
    participant ShortLinksController
    participant ShortLinksService
    participant AuthorizationService
    participant Database
    participant LinkAccessLogService

    User->>ShortLinksController: resolve(shortCode)
    ShortLinksController->>ShortLinksService: resolve(shortCode)
    ShortLinksService->>Database: findOne(shortCode)
    Database-->>ShortLinksService: link + split data
    
    ShortLinksService->>ShortLinksService: check expiresAt
    alt Link Expired
        ShortLinksService-->>ShortLinksController: ForbiddenException
        ShortLinksController-->>User: 403 Forbidden
    end
    
    ShortLinksService->>ShortLinksService: check maxAccesses limit
    alt Max Reached
        ShortLinksService-->>ShortLinksController: ForbiddenException
        ShortLinksController-->>User: 403 Forbidden
    end
    
    ShortLinksService->>Database: increment accessCount
    ShortLinksService->>LinkAccessLogService: log access
    LinkAccessLogService->>Database: create LinkAccessLog entry
    
    ShortLinksService-->>ShortLinksController: redirectUrl
    ShortLinksController-->>User: Redirect Response
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Poem

🐰 Authorization walls protect the warren well,
Split comments bloom in every earthen cell,
Short links bound by time and access delight,
Hop hop—the burrow's secured just right! 🔐

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the two main features added: split comments API and short-link authorization hardening, matching the changeset's primary objectives.
Linked Issues check ✅ Passed The PR fully implements all coding requirements from both linked issues: split comments endpoints with authorization and pagination, short-link hardening with membership checks, max-access/expiry enforcement, and comprehensive tests.
Out of Scope Changes check ✅ Passed All code changes directly support the two linked issues; quote normalization and minor refactoring are incidental improvements aligned with existing code standards.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 9

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
backend/src/short-links/short-links.service.ts (1)

134-163: ⚠️ Potential issue | 🔴 Critical

Critical: analytics() controller method missing userId parameter.

The controller method (lines 43-46) does not inject the request object to access the user ID, but the service method (line 135) requires userId as the second parameter. Update the controller method to accept @Req() req: AuthRequest and pass req.user.id to the service call, following the same pattern as the resolve() method.

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

In `@backend/src/short-links/short-links.service.ts` around lines 134 - 163, The
controller action that calls ShortLinksService.analytics is missing injection of
the request user ID; update the controller method (the analytics controller
method that currently calls shortLinksService.analytics(shortCode)) to accept
`@Req`() req: AuthRequest and pass req.user.id as the second argument to
ShortLinksService.analytics(shortCode, req.user.id), mirroring the pattern used
by the resolve() method so the service's analytics(shortCode: string, userId:
string) receives the required userId for authorization.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@backend/src/short-links/short-links.service.ts`:
- Line 83: Add a nullable/optional maxAccesses number field to the
GenerateLinkDto class so dto.maxAccesses can be populated; inside the
GenerateLinkDto definition add a property named maxAccesses with `@IsOptional`(),
`@IsNumber`(), and `@Min`(1) decorators and ensure the corresponding validator
decorators are imported (IsOptional, IsNumber, Min) so the short-links service’s
assignment maxAccesses: dto.maxAccesses ?? null becomes reachable and validated.
- Line 172: The thrown NotFoundException is using an unquoted identifier (if
(!link) throw new NotFoundException(Short link not found);) which causes a
syntax error; fix it by changing the constructor argument to a proper string
literal (e.g., 'Short link not found' or "Short link not found") where
NotFoundException is used in the short-links service method that performs the
link lookup so the throw reads new NotFoundException('Short link not found').
- Around line 128-131: The returned redirectUrl is using invalid string
interpolation (missing template literal backticks) and omits the split
identifier; update the object returned in the method in short-links.service.ts
so redirectUrl is built as a template literal combining process.env.FRONTEND_URL
with the splits path and the split id (e.g., include link.split.id), while
leaving link.linkType unchanged; ensure you use backticks for the template
literal so the environment variable and link.split.id are interpolated
correctly.
- Around line 91-94: The assignment that constructs the short link object (in
short-links.service.ts) has a malformed template literal for the frontend URL
(the line setting url: ${process.env.FRONTEND_URL}/l/, and a similar line later
around the buildSep0007Uri usage at line ~129) which causes a syntax error; fix
both by wrapping the template expression in backticks so the url becomes a valid
template literal (e.g., use `...` around ${process.env.FRONTEND_URL}/l/) while
leaving the existing buildSep0007Uri(dto.splitId) and expiresAt: expiry
unchanged.
- Around line 186-188: The string returned by buildSep0007Uri is unquoted and
currently a placeholder, causing a syntax error; fix by returning a properly
quoted URI (use a string or template literal) and build the URI with real values
or parameters (populate destination and memo) — update the
buildSep0007Uri(splitId: string) implementation to construct and return a valid
"web+stellar:pay?destination=...&memo=..." string (e.g., interpolate splitId or
accept/marshal destination/memo inputs) so the function no longer returns an
unquoted literal or empty fields.
- Around line 165-184: Controller's remove handler is missing the request user
extraction, so service.remove(shortCode, userId) is called with undefined;
update the controller's remove(...) method to accept `@Req`() req: AuthRequest
(matching the pattern in generate() and resolve()), and pass req.user.id as the
second argument to the ShortLinksService.remove(shortCode, req.user.id); ensure
the handler signature and any imports match the existing AuthRequest usage.

In `@backend/src/split-comments/split-comments.controller.ts`:
- Around line 18-21: Remove the fallback 'test-user' and require an
authenticated user in the controller methods (e.g., create(...) and the
list/fetch method) by throwing or returning 401/403 when req.user is missing;
then verify the caller is a member of the target split before calling
SplitCommentService (e.g., call an authorization helper or
SplitMembershipService to confirm membership for the splitId) and only then
delegate to SplitCommentService.createComment or
SplitCommentService.listComments; ensure the controller passes the authenticated
user id (req.user.id) to the service and rejects requests that fail membership
checks.
- Around line 24-30: The controller's list handler currently passes unchecked
Number(page)/Number(limit) to splitCommentService.listComments, allowing NaN, 0,
negatives or huge values; update the list method to parse page and limit with
Number and Math.floor, validate that page and limit are finite integers >= 1,
throw a BadRequestException for invalid inputs, and cap limit to a reasonable
MAX_LIMIT constant before calling splitCommentService.listComments (e.g., const
MAX_LIMIT = 100); reference the list method in split-comments.controller.ts and
the splitCommentService.listComments call when making this change.

In `@backend/src/split-comments/split-comments.spec.ts`:
- Line 23: The Jest spec's top-level describe('SplitCommentService', ...) is not
properly closed — replace the final solitary closing brace with a proper closure
for the describe call so the file ends with "});". Locate the trailing "}" at
the end of the file (the one meant to close the describe block) and change it to
close both the block and the describe invocation; also verify there are matching
closing braces/parentheses for any nested it/test or beforeEach/afterEach blocks
in the file (e.g., the describe and any nested functions) so the test file
parses correctly.

---

Outside diff comments:
In `@backend/src/short-links/short-links.service.ts`:
- Around line 134-163: The controller action that calls
ShortLinksService.analytics is missing injection of the request user ID; update
the controller method (the analytics controller method that currently calls
shortLinksService.analytics(shortCode)) to accept `@Req`() req: AuthRequest and
pass req.user.id as the second argument to
ShortLinksService.analytics(shortCode, req.user.id), mirroring the pattern used
by the resolve() method so the service's analytics(shortCode: string, userId:
string) receives the required userId for authorization.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e7e52575-6664-4200-b1b3-8ca917d4c510

📥 Commits

Reviewing files that changed from the base of the PR and between c9db068 and b2a15dc.

⛔ Files ignored due to path filters (1)
  • backend/package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (8)
  • backend/src/auth/services/authorization.service.ts
  • backend/src/short-links/entities/link-access-log.entity.ts
  • backend/src/short-links/short-links.service.ts
  • backend/src/split-comments/dto/split-comment.dto.ts
  • backend/src/split-comments/provider/provider.service.ts
  • backend/src/split-comments/split-comment.entity.ts
  • backend/src/split-comments/split-comments.controller.ts
  • backend/src/split-comments/split-comments.spec.ts
💤 Files with no reviewable changes (1)
  • backend/src/split-comments/dto/split-comment.dto.ts

? ({ id: dto.targetParticipantId } as any)
: null,
expiresAt: expiry,
maxAccesses: dto.maxAccesses ?? null,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

maxAccesses feature is unreachable — DTO is missing the field.

Per backend/src/short-links/dto/generate-link.dto.ts, the DTO does not declare a maxAccesses property. This means dto.maxAccesses is always undefined, and the max-access limit feature cannot be used through the API.

Add the field to GenerateLinkDto:

// In backend/src/short-links/dto/generate-link.dto.ts
`@IsOptional`()
`@IsNumber`()
`@Min`(1)
maxAccesses?: number;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@backend/src/short-links/short-links.service.ts` at line 83, Add a
nullable/optional maxAccesses number field to the GenerateLinkDto class so
dto.maxAccesses can be populated; inside the GenerateLinkDto definition add a
property named maxAccesses with `@IsOptional`(), `@IsNumber`(), and `@Min`(1)
decorators and ensure the corresponding validator decorators are imported
(IsOptional, IsNumber, Min) so the short-links service’s assignment maxAccesses:
dto.maxAccesses ?? null becomes reachable and validated.

Comment on lines +91 to 94
url: ${process.env.FRONTEND_URL}/l/,
sep0007: this.buildSep0007Uri(dto.splitId),
expiresAt: expiry,
};
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Critical: Missing template literal backticks causes syntax error and build failure.

The pipeline is failing because this line is missing backticks for the template literal. The same issue exists on line 129.

🐛 Proposed fix
     return {
       shortCode,
-      url: ${process.env.FRONTEND_URL}/l/,
+      url: `${process.env.FRONTEND_URL}/l/${shortCode}`,
       sep0007: this.buildSep0007Uri(dto.splitId),
       expiresAt: expiry,
     };
📝 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
url: ${process.env.FRONTEND_URL}/l/,
sep0007: this.buildSep0007Uri(dto.splitId),
expiresAt: expiry,
};
return {
shortCode,
url: `${process.env.FRONTEND_URL}/l/${shortCode}`,
sep0007: this.buildSep0007Uri(dto.splitId),
expiresAt: expiry,
};
🧰 Tools
🪛 Biome (2.4.9)

[error] 91-91: expected , but instead found {

(parse)


[error] 91-91: Expected an expression but instead found ','.

(parse)


[error] 92-92: Expected a semicolon or an implicit semicolon after a statement, but found none

(parse)

🪛 GitHub Actions: CI

[error] 91-91: ESLint parsing error in file. Parsing error: ',' expected

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

In `@backend/src/short-links/short-links.service.ts` around lines 91 - 94, The
assignment that constructs the short link object (in short-links.service.ts) has
a malformed template literal for the frontend URL (the line setting url:
${process.env.FRONTEND_URL}/l/, and a similar line later around the
buildSep0007Uri usage at line ~129) which causes a syntax error; fix both by
wrapping the template expression in backticks so the url becomes a valid
template literal (e.g., use `...` around ${process.env.FRONTEND_URL}/l/) while
leaving the existing buildSep0007Uri(dto.splitId) and expiresAt: expiry
unchanged.

Comment on lines 128 to 131
return {
redirectUrl: `${process.env.FRONTEND_URL}/splits/${link.split.id}`,
redirectUrl: ${process.env.FRONTEND_URL}/splits/,
linkType: link.linkType,
};
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Critical: Missing template literal backticks in redirectUrl.

Same syntax error as line 91. Additionally, the URL appears incomplete — it should probably include link.split.id or similar.

🐛 Proposed fix
     return {
-      redirectUrl: ${process.env.FRONTEND_URL}/splits/,
+      redirectUrl: `${process.env.FRONTEND_URL}/splits/${link.split.id}`,
       linkType: link.linkType,
     };
📝 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
return {
redirectUrl: `${process.env.FRONTEND_URL}/splits/${link.split.id}`,
redirectUrl: ${process.env.FRONTEND_URL}/splits/,
linkType: link.linkType,
};
return {
redirectUrl: `${process.env.FRONTEND_URL}/splits/${link.split.id}`,
linkType: link.linkType,
};
🧰 Tools
🪛 Biome (2.4.9)

[error] 129-129: expected , but instead found {

(parse)


[error] 129-129: Expected an expression but instead found ','.

(parse)


[error] 130-130: Expected a semicolon or an implicit semicolon after a statement, but found none

(parse)


[error] 128-130: Illegal return statement outside of a function

(parse)


[error] 130-130: Expected a statement but instead found ': link.linkType,'.

(parse)

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

In `@backend/src/short-links/short-links.service.ts` around lines 128 - 131, The
returned redirectUrl is using invalid string interpolation (missing template
literal backticks) and omits the split identifier; update the object returned in
the method in short-links.service.ts so redirectUrl is built as a template
literal combining process.env.FRONTEND_URL with the splits path and the split id
(e.g., include link.split.id), while leaving link.linkType unchanged; ensure you
use backticks for the template literal so the environment variable and
link.split.id are interpolated correctly.

Comment on lines +165 to 184
// ? Delete - only split creator can delete
async remove(shortCode: string, userId: string): Promise<void> {
const link = await this.shortLinkRepo.findOne({
where: { shortCode },
relations: ['split'],
});

if (!link) throw new NotFoundException(Short link not found);

// Only split creator can delete links
const canDelete = await this.authorizationService.canDeleteShortLink(
userId,
link.split.id,
);
if (!canDelete) {
throw new ForbiddenException('Only the split creator can delete links');
}

await this.shortLinkRepo.delete({ shortCode });
}
Copy link
Copy Markdown

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
# Verify controller method signature mismatch
rg -n -A5 'remove\(`@Param`' backend/src/short-links/

Repository: OlufunbiIK/StellarSplit

Length of output: 367


🏁 Script executed:

rg -n -B3 -A10 'remove\(`@Param`\("shortCode"\)' backend/src/short-links/short-links.controller.ts

Repository: OlufunbiIK/StellarSplit

Length of output: 205


🏁 Script executed:

cat -n backend/src/short-links/short-links.controller.ts | head -60

Repository: OlufunbiIK/StellarSplit

Length of output: 1986


Critical: Controller does not pass required userId parameter to remove().

The service method requires userId (line 166), but the controller at short-links.controller.ts:54-57 calls this.service.remove(code) without passing the user ID. The remove() method is missing the @Req() req: AuthRequest parameter that other methods in this controller use to extract the user ID from the request. This will cause the authorization check to fail since userId will be undefined.

Add @Req() req: AuthRequest parameter and pass req.user.id to the service method, following the pattern used in generate() and resolve() methods.

🧰 Tools
🪛 Biome (2.4.9)

[error] 166-166: Expected a semicolon or an implicit semicolon after a statement, but found none

(parse)


[error] 166-166: expected , but instead found :

(parse)


[error] 166-166: Expected a semicolon or an implicit semicolon after a statement, but found none

(parse)


[error] 172-172: expected , but instead found link

(parse)


[error] 172-172: expected , but instead found not

(parse)


[error] 172-172: expected , but instead found found

(parse)

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

In `@backend/src/short-links/short-links.service.ts` around lines 165 - 184,
Controller's remove handler is missing the request user extraction, so
service.remove(shortCode, userId) is called with undefined; update the
controller's remove(...) method to accept `@Req`() req: AuthRequest (matching the
pattern in generate() and resolve()), and pass req.user.id as the second
argument to the ShortLinksService.remove(shortCode, req.user.id); ensure the
handler signature and any imports match the existing AuthRequest usage.

relations: ['split'],
});

if (!link) throw new NotFoundException(Short link not found);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Critical: Missing quotes around error message string.

This will cause a syntax error.

🐛 Proposed fix
-    if (!link) throw new NotFoundException(Short link not found);
+    if (!link) throw new NotFoundException('Short link not found');
📝 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
if (!link) throw new NotFoundException(Short link not found);
if (!link) throw new NotFoundException('Short link not found');
🧰 Tools
🪛 Biome (2.4.9)

[error] 172-172: expected , but instead found link

(parse)


[error] 172-172: expected , but instead found not

(parse)


[error] 172-172: expected , but instead found found

(parse)

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

In `@backend/src/short-links/short-links.service.ts` at line 172, The thrown
NotFoundException is using an unquoted identifier (if (!link) throw new
NotFoundException(Short link not found);) which causes a syntax error; fix it by
changing the constructor argument to a proper string literal (e.g., 'Short link
not found' or "Short link not found") where NotFoundException is used in the
short-links service method that performs the link lookup so the throw reads new
NotFoundException('Short link not found').

Comment on lines 186 to 188
private buildSep0007Uri(splitId: string): string {
return `web+stellar:pay?destination=${process.env.PLATFORM_WALLET}&memo=${splitId}`;
return web+stellar:pay?destination=&memo=;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Critical: Missing quotes around SEP-0007 URI string.

The URI string is not quoted, causing a syntax error. Also, the URI appears to be a placeholder with empty destination and memo values — verify this is intentional.

🐛 Proposed fix
   private buildSep0007Uri(splitId: string): string {
-    return web+stellar:pay?destination=&memo=;
+    return `web+stellar:pay?destination=${process.env.PLATFORM_WALLET}&memo=${splitId}`;
   }
📝 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
private buildSep0007Uri(splitId: string): string {
return `web+stellar:pay?destination=${process.env.PLATFORM_WALLET}&memo=${splitId}`;
return web+stellar:pay?destination=&memo=;
}
private buildSep0007Uri(splitId: string): string {
return `web+stellar:pay?destination=${process.env.PLATFORM_WALLET}&memo=${splitId}`;
}
🧰 Tools
🪛 Biome (2.4.9)

[error] 186-186: Illegal use of reserved keyword private as an identifier in strict mode

(parse)


[error] 186-186: Expected a semicolon or an implicit semicolon after a statement, but found none

(parse)


[error] 186-186: expected , but instead found :

(parse)


[error] 186-186: Expected a semicolon or an implicit semicolon after a statement, but found none

(parse)


[error] 187-187: Expected a semicolon or an implicit semicolon after a statement, but found none

(parse)


[error] 187-187: Illegal return statement outside of a function

(parse)


[error] 187-187: Expected a statement but instead found ':pay?destination=&memo='.

(parse)

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

In `@backend/src/short-links/short-links.service.ts` around lines 186 - 188, The
string returned by buildSep0007Uri is unquoted and currently a placeholder,
causing a syntax error; fix by returning a properly quoted URI (use a string or
template literal) and build the URI with real values or parameters (populate
destination and memo) — update the buildSep0007Uri(splitId: string)
implementation to construct and return a valid
"web+stellar:pay?destination=...&memo=..." string (e.g., interpolate splitId or
accept/marshal destination/memo inputs) so the function no longer returns an
unquoted literal or empty fields.

Comment on lines +18 to +21
@Post()
async create(@Body() dto: CreateSplitCommentDto, @Req() req: any) {
const userId = req.user?.id ?? 'test-user';
return this.splitCommentService.createComment(userId, dto);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Require authenticated split membership on these routes.

Lines 20 and 35 default missing auth to 'test-user', and Lines 24-30 never even carry caller identity for the list path. That means this controller can write under a shared identity and return any split thread by splitId without verifying that the caller belongs to that split. Fail closed when req.user is missing, and authorize the caller against the target split before delegating to SplitCommentService.

Also applies to: 24-30, 33-36

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

In `@backend/src/split-comments/split-comments.controller.ts` around lines 18 -
21, Remove the fallback 'test-user' and require an authenticated user in the
controller methods (e.g., create(...) and the list/fetch method) by throwing or
returning 401/403 when req.user is missing; then verify the caller is a member
of the target split before calling SplitCommentService (e.g., call an
authorization helper or SplitMembershipService to confirm membership for the
splitId) and only then delegate to SplitCommentService.createComment or
SplitCommentService.listComments; ensure the controller passes the authenticated
user id (req.user.id) to the service and rejects requests that fail membership
checks.

Comment on lines +24 to +30
@Get(':splitId')
async list(
@Param('splitId') splitId: string,
@Query('page') page = 1,
@Query('limit') limit = 10,
) {
return this.splitCommentService.listComments(splitId, Number(page), Number(limit));
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Reject invalid and unbounded pagination values.

Number(page) and Number(limit) accept NaN, 0, negatives, and arbitrarily large values. Those go straight into skip/take, which can produce invalid offsets or very expensive reads. Validate both inputs and cap limit before calling the service.

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

In `@backend/src/split-comments/split-comments.controller.ts` around lines 24 -
30, The controller's list handler currently passes unchecked
Number(page)/Number(limit) to splitCommentService.listComments, allowing NaN, 0,
negatives or huge values; update the list method to parse page and limit with
Number and Math.floor, validate that page and limit are finite integers >= 1,
throw a BadRequestException for invalid inputs, and cap limit to a reasonable
MAX_LIMIT constant before calling splitCommentService.listComments (e.g., const
MAX_LIMIT = 100); reference the list method in split-comments.controller.ts and
the splitCommentService.listComments call when making this change.

emit: jest.fn(),
};

describe('SplitCommentService', () => {
Copy link
Copy Markdown

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
# Verify the last non-empty line of the spec closes the Jest suite.
awk 'NF{last=$0} END{print "Last non-empty line:", last}' backend/src/split-comments/split-comments.spec.ts

Repository: OlufunbiIK/StellarSplit

Length of output: 89


Close the describe block; this Jest spec does not parse.

The describe block opened at line 23 is not properly closed. The file ends with } instead of }); at line 90, causing a parse error.

Proposed fix
   it('should throw error if user tries to delete someone elses comment', async () => {
     mockRepo.findOne.mockResolvedValue({ id: 'c1', userId: 'u1' });
 
     await expect(service.deleteComment('c1', 'u2')).rejects.toThrow('You can only delete your own comments');
   });
-}
+});
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@backend/src/split-comments/split-comments.spec.ts` at line 23, The Jest
spec's top-level describe('SplitCommentService', ...) is not properly closed —
replace the final solitary closing brace with a proper closure for the describe
call so the file ends with "});". Locate the trailing "}" at the end of the file
(the one meant to close the describe block) and change it to close both the
block and the describe invocation; also verify there are matching closing
braces/parentheses for any nested it/test or beforeEach/afterEach blocks in the
file (e.g., the describe and any nested functions) so the test file parses
correctly.

@OlufunbiIK OlufunbiIK merged commit e3a89fa into OlufunbiIK:main Mar 29, 2026
4 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Harden Short-Link Authorization, Expiry, and Analytics Rules Build a Real Split Comments API With Mentions and Membership Checks

2 participants