Feat/backend global validation pipe#461
Merged
ryzen-xp merged 6 commits intoApr 29, 2026
Merged
Conversation
|
@KevinMB0220 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds class-validator/class-transformer and enables a global NestJS ValidationPipe; augments DTOs with validation decorators; registers AppController/AppService in AppModule; introduces Jest/Supertest test tooling and new e2e tests asserting 400 responses for invalid payloads. Changes
Sequence DiagramsequenceDiagram
participant Client
participant NestApp as "NestJS App"
participant ValPipe as "ValidationPipe"
participant DTO as "DTO (class-validator)"
participant Controller
Client->>NestApp: POST /<endpoint> (payload)
NestApp->>ValPipe: run global ValidationPipe
ValPipe->>DTO: validate & transform payload
alt valid
DTO-->>ValPipe: valid
ValPipe-->>Controller: forward transformed DTO
Controller-->>Client: 200 OK / resource response
else invalid
DTO-->>ValPipe: validation errors
ValPipe-->>Client: 400 Bad Request (validation details)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
backend/test/validation.e2e-spec.ts (1)
30-100: Use valid fixtures for the non-targeted fields.These payloads reuse the same placeholder Stellar address everywhere, but that value does not satisfy the new regex. As a result, each request can raise unrelated validation errors and make the assertions less precise. A valid address fixture would make the suite more stable.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/test/validation.e2e-spec.ts` around lines 30 - 100, Tests reuse an invalid placeholder Stellar address which triggers unrelated validation failures; introduce a single valid fixture (e.g., VALID_STELLAR_ADDRESS) that matches the Stellar address regex and replace every hard-coded 'GABC1234STELLAR...' occurrence in the register and auction tests (the POST to '/resolver/register' and '/auction/1/bid' cases) with that fixture so non-targeted fields pass validation and assertions become precise; add the constant near the top of validation.e2e-spec.ts and use it in the test payloads for username/walletAddress fields.backend/src/vault/dto/vault.dto.ts (1)
32-55: Move these validators to a request DTO if you expect them to run.
AutoPayDtois only used as a GET response today, so these decorators never execute on the current route surface. A dedicated input DTO would make the validation intent explicit.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/vault/dto/vault.dto.ts` around lines 32 - 55, AutoPayDto currently contains validation decorators but is only used as a GET/response DTO so validators never run; extract the input-specific validators into a new request DTO (e.g., CreateAutoPayDto and/or UpdateAutoPayDto) containing the decorated properties (id, recipient, amount, interval, active) and leave AutoPayDto as a plain response DTO (or rename to AutoPayResponseDto); update the controller methods that accept POST/PUT/PATCH payloads to use the new request DTO(s) instead of AutoPayDto so class-validator runs, and keep any ApiProperty metadata in both request and response DTOs as needed.
🤖 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/package.json`:
- Line 13: The npm script "test:debug" in package.json preloads "-r
find-node-modules" which is not declared, causing MODULE_NOT_FOUND; either add
"find-node-modules" to devDependencies (so npm install will provide it) or
remove the "-r find-node-modules" preload from the "test:debug" script; update
package.json accordingly and run npm install to verify the script runs (refer to
the "test:debug" script entry in package.json to locate the change).
In `@backend/src/auction/dto/auction.dto.ts`:
- Around line 31-36: The Swagger ApiProperty example for the bidder field does
not match the Stellar address regex used by the `@Matches` validator; update the
example in the ApiProperty for the bidder property so it is a valid 56-character
Stellar public key (starts with 'G' and uses only A-Z and 2-7) to satisfy the
regex in the Matches decorator (referenced symbol: bidder, decorators:
`@ApiProperty` and `@Matches`(/^G[A-Z2-7]{55}$/)).
In `@backend/src/auth/dto/login.dto.ts`:
- Around line 5-11: The Swagger example for the address field conflicts with its
validator: update the ApiProperty example on the address property so it
satisfies the Matches(/^G[A-Z2-7]{55}$/) constraint — provide a 56-character
string that starts with 'G' and contains only A–Z and digits 2–7 (i.e., change
the example in the ApiProperty decorator for the address field to a value
matching the regex).
In `@backend/src/resolver/dto/resolver.dto.ts`:
- Around line 16-37: Update the ApiProperty examples to match the new validation
regexes: replace the walletAddress example with a valid 56-char Stellar public
key starting with 'G' and containing only A-Z and 2-7 (e.g. a 56-character
string like "G" + 55 valid base32 chars) in the walletAddress property, and
replace the commitment example with a 32-byte hex string prefixed by 0x (64 hex
chars after 0x) for the commitment property so they pass the
`@Matches`(/^G[A-Z2-7]{55}$/) and `@Matches`(/^0x[a-fA-F0-9]{64}$/) validators
respectively; ensure the username example remains valid for
`@Matches`(/^[a-zA-Z0-9_]{3,20}$/).
---
Nitpick comments:
In `@backend/src/vault/dto/vault.dto.ts`:
- Around line 32-55: AutoPayDto currently contains validation decorators but is
only used as a GET/response DTO so validators never run; extract the
input-specific validators into a new request DTO (e.g., CreateAutoPayDto and/or
UpdateAutoPayDto) containing the decorated properties (id, recipient, amount,
interval, active) and leave AutoPayDto as a plain response DTO (or rename to
AutoPayResponseDto); update the controller methods that accept POST/PUT/PATCH
payloads to use the new request DTO(s) instead of AutoPayDto so class-validator
runs, and keep any ApiProperty metadata in both request and response DTOs as
needed.
In `@backend/test/validation.e2e-spec.ts`:
- Around line 30-100: Tests reuse an invalid placeholder Stellar address which
triggers unrelated validation failures; introduce a single valid fixture (e.g.,
VALID_STELLAR_ADDRESS) that matches the Stellar address regex and replace every
hard-coded 'GABC1234STELLAR...' occurrence in the register and auction tests
(the POST to '/resolver/register' and '/auction/1/bid' cases) with that fixture
so non-targeted fields pass validation and assertions become precise; add the
constant near the top of validation.e2e-spec.ts and use it in the test payloads
for username/walletAddress fields.
🪄 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: e779f3ec-ca5e-4074-9fe9-f78a8ed9a77e
⛔ Files ignored due to path filters (1)
backend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (9)
backend/package.jsonbackend/src/app.module.tsbackend/src/auction/dto/auction.dto.tsbackend/src/auth/dto/login.dto.tsbackend/src/main.tsbackend/src/resolver/dto/resolver.dto.tsbackend/src/vault/dto/vault.dto.tsbackend/test/app.e2e-spec.tsbackend/test/validation.e2e-spec.ts
| "test": "jest", | ||
| "test:watch": "jest --watch", | ||
| "test:cov": "jest --coverage", | ||
| "test:debug": "node --inspect-brk -r tsconfig-paths/register -r find-node-modules node_modules/.bin/jest --runInBand", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
node - <<'NODE'
const fs = require('fs');
const pkg = JSON.parse(fs.readFileSync('backend/package.json', 'utf8'));
console.log('declared:', pkg.dependencies?.['find-node-modules'] ?? pkg.devDependencies?.['find-node-modules'] ?? 'missing');
NODE
rg -n '"find-node-modules"' backend/package-lock.json backend/pnpm-lock.yaml backend/yarn.lock 2>/dev/null || true
node -e "require.resolve('find-node-modules'); console.log('resolves')"Repository: Alien-Protocol/Alien-Protocol
Length of output: 906
Add find-node-modules to devDependencies or remove it from the test:debug preload.
The test:debug script preloads -r find-node-modules, but this package is not declared in the manifest and cannot be resolved. On a clean install, npm run test:debug will fail with MODULE_NOT_FOUND.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/package.json` at line 13, The npm script "test:debug" in package.json
preloads "-r find-node-modules" which is not declared, causing MODULE_NOT_FOUND;
either add "find-node-modules" to devDependencies (so npm install will provide
it) or remove the "-r find-node-modules" preload from the "test:debug" script;
update package.json accordingly and run npm install to verify the script runs
(refer to the "test:debug" script entry in package.json to locate the change).
Comment on lines
31
to
36
| @ApiProperty({ description: 'Bidder Stellar address', example: 'GXYZ9876STELLAR1234BIDDERADDRESS' }) | ||
| @IsNotEmpty() | ||
| @Matches(/^G[A-Z2-7]{55}$/, { | ||
| message: 'Invalid Stellar wallet address format', | ||
| }) | ||
| bidder: string; |
There was a problem hiding this comment.
Keep the bid example consistent with the Stellar regex.
bidder's example also fails ^G[A-Z2-7]{55}$, so Swagger is advertising an invalid sample payload here as well.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/src/auction/dto/auction.dto.ts` around lines 31 - 36, The Swagger
ApiProperty example for the bidder field does not match the Stellar address
regex used by the `@Matches` validator; update the example in the ApiProperty for
the bidder property so it is a valid 56-character Stellar public key (starts
with 'G' and uses only A-Z and 2-7) to satisfy the regex in the Matches
decorator (referenced symbol: bidder, decorators: `@ApiProperty` and
`@Matches`(/^G[A-Z2-7]{55}$/)).
Comment on lines
+5
to
+11
| @ApiProperty({ description: 'Stellar wallet address', example: 'GABC1234STELLAR5678WALLETADDRESS' }) | ||
| @IsString() | ||
| @IsNotEmpty() | ||
| @Matches(/^G[A-Z2-7]{55}$/, { | ||
| message: 'Invalid Stellar wallet address format', | ||
| }) | ||
| address: string; |
There was a problem hiding this comment.
Use a Swagger example that satisfies the validator.
GABC1234STELLAR5678WALLETADDRESS does not match ^G[A-Z2-7]{55}$, so the documented sample request is self-contradictory and will fail if copied into a client.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/src/auth/dto/login.dto.ts` around lines 5 - 11, The Swagger example
for the address field conflicts with its validator: update the ApiProperty
example on the address property so it satisfies the Matches(/^G[A-Z2-7]{55}$/)
constraint — provide a 56-character string that starts with 'G' and contains
only A–Z and digits 2–7 (i.e., change the example in the ApiProperty decorator
for the address field to a value matching the regex).
Comment on lines
16
to
+37
| @ApiProperty({ description: 'Username to register (without @)', example: 'alice' }) | ||
| @IsString() | ||
| @IsNotEmpty() | ||
| @Matches(/^[a-zA-Z0-9_]{3,20}$/, { | ||
| message: 'Username must be 3-20 characters long and contain only letters, numbers, and underscores', | ||
| }) | ||
| username: string; | ||
|
|
||
| @ApiProperty({ description: 'Stellar wallet address to link', example: 'GABC1234STELLAR5678WALLETADDRESS' }) | ||
| @IsString() | ||
| @IsNotEmpty() | ||
| @Matches(/^G[A-Z2-7]{55}$/, { | ||
| message: 'Invalid Stellar wallet address format', | ||
| }) | ||
| walletAddress: string; | ||
|
|
||
| @ApiProperty({ description: 'Zero-knowledge commitment hash', example: '0xabc123...' }) | ||
| @IsString() | ||
| @IsNotEmpty() | ||
| @Matches(/^0x[a-fA-F0-9]{64}$/, { | ||
| message: 'Commitment must be a 32-byte hex string prefixed with 0x', | ||
| }) |
There was a problem hiding this comment.
Align the Swagger examples with the new validation rules.
The current walletAddress and commitment examples do not satisfy the regex constraints added below, so the generated docs will mislead API consumers and produce copy-paste payloads that fail validation.
💡 Suggested update
`@ApiProperty`({ description: 'Stellar wallet address to link', example: 'GABC1234STELLAR5678WALLETADDRESS' })
+ `@ApiProperty`({
+ description: 'Stellar wallet address to link',
+ example: `G${'A'.repeat(55)}`,
+ })
@@
- `@ApiProperty`({ description: 'Zero-knowledge commitment hash', example: '0xabc123...' })
+ `@ApiProperty`({
+ description: 'Zero-knowledge commitment hash',
+ example: `0x${'a'.repeat(64)}`,
+ })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/src/resolver/dto/resolver.dto.ts` around lines 16 - 37, Update the
ApiProperty examples to match the new validation regexes: replace the
walletAddress example with a valid 56-char Stellar public key starting with 'G'
and containing only A-Z and 2-7 (e.g. a 56-character string like "G" + 55 valid
base32 chars) in the walletAddress property, and replace the commitment example
with a 32-byte hex string prefixed by 0x (64 hex chars after 0x) for the
commitment property so they pass the `@Matches`(/^G[A-Z2-7]{55}$/) and
`@Matches`(/^0x[a-fA-F0-9]{64}$/) validators respectively; ensure the username
example remains valid for `@Matches`(/^[a-zA-Z0-9_]{3,20}$/).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR addresses issue #441 by implementing a global validation pipe in the backend using NestJS's
ValidationPipe. It ensures that all incoming HTTP request payloads are strictly validated against their Data Transfer Objects (DTOs), automatically stripping out unknown properties and rejecting requests with invalid data.Closes #441
Changes Made
ValidationPipeglobally inmain.tswithwhitelist: true,forbidNonWhitelisted: true, andtransform: true.class-validatorandclass-transformerto the project dependencies, and configured necessary testing tools (jest,supertest) inpackage.json.@IsString(),@IsNotEmpty(),@Matches(),@IsNumberString(), etc.) across multiple domains:RegisterUsernameDto(Resolver Module)PlaceBidDto(Auction Module)AutoPayDto(Vault Module)LoginDto(Auth Module)AppControllerandAppServicetoAppModuleto resolve existing test failures.validation.e2e-spec.ts) specifically to test the validation boundaries (e.g., rejecting invalid formats, missing properties, and extra properties).supertestin existing E2E tests.Acceptance Criteria Met
class-validatorandclass-transformerinstalled.ValidationPipeglobally applied withwhitelist: trueandforbidNonWhitelisted: true.400 Bad Requestresponse.npm run test(andnpm run test:e2e) pass successfully.How to Test
npm installinside thebackend/directory.npm run start:dev./resolver/registerwithcommitment: "invalid-hex"or an unlisted property).400 Bad Requestand an explicit error message detailing the validation failure.npm run test:e2ein thebackend/directory to automatically verify validation rules.Summary by CodeRabbit
New Features
Tests
Chores