fix(backend): resolve critical audit bugs O3, O7, H1, A2, A3, A4, H2c, H7, H8, C2#23
Conversation
📝 WalkthroughWalkthroughThe changes add GraphQL depth validation and trusted-proxy handling, revise authentication lockout and OAuth flows, validate image dimensions, and update stock, guest checkout, and payment error handling. ChangesSecurity and service behavior
Estimated code review effort: 4 (Complex) | ~45 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed: private package registry requires authentication. Disable ESLint in CodeRabbit settings or use public packages. 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. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces several security, validation, and architectural improvements, including GraphQL query depth limiting, image dimension validation, trust proxy configuration for rate limiting, and refactored payment and stock update flows. However, several critical issues were identified in the review: the updated lockout logic in auth-credentials.service.ts introduces a permanent lockout bug for both customers and employees; performing password hashing inside the database transaction in auth-oauth.service.ts can cause connection pool exhaustion; the stock upsert logic in product-stock.service.ts incorrectly prioritizes product-level stock over specific combination variants; and the guest checkout flow in guest-checkout.service.ts poses a security risk by silently merging guest carts with registered customer accounts instead of prompting them to log in.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| if ( | ||
| !customer || | ||
| !customer.active || | ||
| this.isLocked(customer.lockedUntil) || | ||
| (customer.lockedUntil === null && | ||
| customer.failedLoginAttempts >= LOCKOUT_THRESHOLD) | ||
| ) { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
This condition introduces a permanent lockout bug. If customer.lockedUntil is null but customer.failedLoginAttempts is greater than or equal to LOCKOUT_THRESHOLD (which can happen if the threshold is lowered or due to a race condition/interrupted write), the user will be blocked from logging in. Because the password verification is bypassed entirely, the user can never successfully authenticate to trigger clearCustomerFailedLogins, and since lockedUntil is null, the lockout never expires. Reverting to the original check avoids this permanent lockout state.
if (!customer || !customer.active || this.isLocked(customer.lockedUntil)) {
return null;
}| if ( | ||
| !employee || | ||
| !employee.active || | ||
| this.isLocked(employee.lockedUntil) || | ||
| (employee.lockedUntil === null && | ||
| employee.failedLoginAttempts >= LOCKOUT_THRESHOLD) | ||
| ) { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
This condition introduces a permanent lockout bug for employees. If employee.lockedUntil is null but employee.failedLoginAttempts is greater than or equal to LOCKOUT_THRESHOLD, the employee will be permanently blocked from logging in with no way to unlock, as password verification is bypassed and the lockout has no expiration date. Reverting to the original check avoids this permanent lockout state.
if (!employee || !employee.active || this.isLocked(employee.lockedUntil)) {
return null;
}| return this.prisma.$transaction(async (tx) => { | ||
| const customer = await tx.customer.upsert({ | ||
| where: { email: profile.email }, | ||
| update: {}, | ||
| create: { | ||
| email: profile.email, | ||
| password: await this.passwordSecurity.hashWithoutPolicy( | ||
| randomBytes(32).toString('base64url'), | ||
| ), | ||
| firstname: profile.firstname, | ||
| lastname: profile.lastname, | ||
| groupId: group.id, | ||
| }, | ||
| include: { group: true }, | ||
| }); |
There was a problem hiding this comment.
Performing CPU-intensive cryptographic operations like password hashing (this.passwordSecurity.hashWithoutPolicy) inside an interactive database transaction holds the database connection and locks open much longer than necessary. This can easily lead to connection pool exhaustion and transaction timeouts under load. Generating the password hash before starting the transaction resolves this issue.
const password = await this.passwordSecurity.hashWithoutPolicy(
randomBytes(32).toString('base64url'),
);
return this.prisma.$transaction(async (tx) => {
const customer = await tx.customer.upsert({
where: { email: profile.email },
update: {},
create: {
email: profile.email,
password,
firstname: profile.firstname,
lastname: profile.lastname,
groupId: group.id,
},
include: { group: true },
});|
|
||
| return this.prisma.stock.create({ | ||
| data: { | ||
| const where = productId ? { productId } : { combinationId }; |
There was a problem hiding this comment.
When both productId and combinationId are provided, productId takes precedence in the current logic. However, combinationId represents a specific product variant/combination, which is more specific than the general productId. If both are provided, the upsert should target the specific combination's stock rather than the product-level stock. Otherwise, updating a variant's stock will incorrectly overwrite the product-level stock or another variant's stock.
| const where = productId ? { productId } : { combinationId }; | |
| const where = combinationId ? { combinationId } : { productId }; |
| const existingCustomer = await this.prisma.customer.findUnique({ | ||
| where: { email }, | ||
| }); | ||
| if (existingCustomer) { | ||
| return existingCustomer; | ||
| } |
There was a problem hiding this comment.
Allowing guest checkout to silently proceed and merge carts with an existing registered customer account (where isGuest is false) is a security and privacy risk. An unauthenticated guest could view/merge items into a registered user's cart or place unauthorized orders under their account. If the email belongs to a registered customer, the system should throw an error prompting them to log in.
const existingCustomer = await this.prisma.customer.findUnique({
where: { email },
});
if (existingCustomer) {
if (!existingCustomer.isGuest) {
throw new BadRequestException(
'This email is registered to an existing account. Please log in to proceed.',
);
}
return existingCustomer;
}![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
libs/backend/features/payment/src/lib/payment.service.ts (1)
50-60: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winPayment record is created before adapter validation, leaving orphaned records for unsupported providers.
prisma.payment.createat line 50 persists the payment beforethis.adapters.get(provider)at line 60. With the registry now throwingBadRequestExceptionfor unknown providers (per thepayment-adapter.registry.tschange), an unsupported provider will leave an orphaned payment record in the database with a default status and no external transaction. No transaction or cleanup wraps this flow.Move the adapter lookup before the
payment.create, or wrap the create + initiate in a transaction and roll back on failure.🔧 Proposed fix: retrieve adapter before creating payment record
const paymentAmount = expectedAmount; + const adapter = this.adapters.get(provider); + const payment = await this.prisma.payment.create({ data: { orderId, provider: provider.toLowerCase(), amount: paymentAmount, currency, idempotencyKey, }, }); - const adapter = this.adapters.get(provider); const result = await adapter.initiate({ paymentId: payment.id, orderId, amount: paymentAmount, currency, });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@libs/backend/features/payment/src/lib/payment.service.ts` around lines 50 - 60, Move the adapter lookup using this.adapters.get(provider) before prisma.payment.create in the payment creation flow, so unsupported providers are rejected before any payment record is persisted. Preserve the existing adapter usage for valid providers.
🧹 Nitpick comments (1)
libs/backend/features/payment/src/lib/payment.service.ts (1)
42-48: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winFloating-point strict equality for monetary comparison is fragile.
amount !== expectedAmountcompares twofloat64values. A client sending99.99could produce a slightly different representation thanNumber(order.totalTTC), causing a false rejection. Consider comparing in integer cents or using a small epsilon tolerance.♻️ Proposed refactor: compare in integer cents
- const expectedAmount = Number(order.totalTTC); - if (amount !== undefined && amount !== expectedAmount) { + const expectedAmount = Number(order.totalTTC); + const expectedCents = Math.round(expectedAmount * 100); + if (amount !== undefined && Math.round(amount * 100) !== expectedCents) { throw new BadRequestException( `Payment amount must match the order total of ${expectedAmount}`, ); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@libs/backend/features/payment/src/lib/payment.service.ts` around lines 42 - 48, Update the amount validation in the payment service around expectedAmount and paymentAmount to avoid strict floating-point comparison: normalize both the client amount and order total to integer cents before comparing, while preserving the existing optional-amount behavior and BadRequestException message.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/api/src/config/graphql-depth.util.ts`:
- Around line 3-27: Update depthLimit so named fragment spreads are traversed
with the depth accumulated at their spread site, rather than validating fragment
fields independently. Extend the visitor logic around Field and fragment-spread
handling to recursively account for nested fragment selections while preserving
maxDepth reporting through context.reportError.
In `@libs/backend/core/auth/src/lib/services/auth-oauth.service.ts`:
- Around line 69-102: Move the password generation and
passwordSecurity.hashWithoutPolicy call out of the prisma.$transaction callback,
compute the hashed dummy password before starting the transaction, and use that
value in the customer upsert’s create data while preserving the existing
transaction operations.
In `@libs/backend/core/media/src/lib/media.service.ts`:
- Around line 40-49: Update the image-processing catch block in the surrounding
media service method to rethrow existing HttpException instances, including the
BadRequestException dimension checks, before converting unexpected errors to the
generic “Could not process image” response. Preserve the original exception
message and status while retaining the current fallback handling for
non-HttpException errors.
In `@libs/backend/features/order/src/lib/guest-checkout.service.ts`:
- Around line 50-76: Update the duplicate-email handling in the customer
creation flow to reuse existingCustomer only when existingCustomer.isGuest is
true. If the matching customer is registered, raise the service’s established
validation error instead of returning that record; preserve rethrowing unrelated
errors.
---
Outside diff comments:
In `@libs/backend/features/payment/src/lib/payment.service.ts`:
- Around line 50-60: Move the adapter lookup using this.adapters.get(provider)
before prisma.payment.create in the payment creation flow, so unsupported
providers are rejected before any payment record is persisted. Preserve the
existing adapter usage for valid providers.
---
Nitpick comments:
In `@libs/backend/features/payment/src/lib/payment.service.ts`:
- Around line 42-48: Update the amount validation in the payment service around
expectedAmount and paymentAmount to avoid strict floating-point comparison:
normalize both the client amount and order total to integer cents before
comparing, while preserving the existing optional-amount behavior and
BadRequestException message.
🪄 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 Plus
Run ID: 29e4714e-12da-4db3-af19-f200ada46204
📒 Files selected for processing (12)
apps/api/src/app.module.tsapps/api/src/config/graphql-depth.util.tsapps/api/src/main.tslibs/backend/core/auth/src/lib/guards/auth-rate-limit.guard.tslibs/backend/core/auth/src/lib/services/auth-credentials.service.tslibs/backend/core/auth/src/lib/services/auth-oauth.service.tslibs/backend/core/auth/src/lib/services/auth-recovery.service.tslibs/backend/core/media/src/lib/media.service.tslibs/backend/features/catalog/src/lib/product/product-stock.service.tslibs/backend/features/order/src/lib/guest-checkout.service.tslibs/backend/features/payment/src/lib/payment-adapter.registry.tslibs/backend/features/payment/src/lib/payment.service.ts
💤 Files with no reviewable changes (1)
- libs/backend/core/auth/src/lib/guards/auth-rate-limit.guard.ts
| export function depthLimit(maxDepth: number) { | ||
| return (context: ValidationContext): ASTVisitor => { | ||
| let depth = 0; | ||
| return { | ||
| Field: { | ||
| enter(node) { | ||
| depth++; | ||
| if (depth > maxDepth) { | ||
| context.reportError( | ||
| new GraphQLError( | ||
| `Query depth exceeds maximum limit of ${maxDepth}`, | ||
| { | ||
| nodes: [node], | ||
| }, | ||
| ), | ||
| ); | ||
| } | ||
| }, | ||
| leave() { | ||
| depth--; | ||
| }, | ||
| }, | ||
| }; | ||
| }; | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
git ls-files apps/api/src/config/graphql-depth.util.tsRepository: tahiry-dev-29/Swift-Shop
Length of output: 204
🏁 Script executed:
sed -n '1,220p' apps/api/src/config/graphql-depth.util.tsRepository: tahiry-dev-29/Swift-Shop
Length of output: 787
🏁 Script executed:
rg -n "depthLimit\(" apps/api/src -SRepository: tahiry-dev-29/Swift-Shop
Length of output: 324
🌐 Web query:
graphql-js validation visitor fragment definitions traversal fragment spreads depthLimit validation rule semantics
💡 Result:
In graphql-js, the validation process is managed by the validate function, which utilizes a specialized visitor to perform a depth-first traversal of the GraphQL document AST [1][2]. This traversal is orchestrated in parallel across all enabled validation rules [2]. Fragment Definitions and Traversal During validation, the engine does not automatically "inline" fragments into the AST [2]. Instead, it uses the ValidationContext to manage relationships between fragment definitions and fragment spreads [3]. The ValidationContext tracks selections and dependencies, allowing rules to inspect fragment spreads via methods like getFragmentSpreads(selectionSet) [3][4]. When a validation rule needs to analyze a fragment's contents, it typically retrieves the fragment definition by name from the document and inspects its associated selection set [3][4]. TypeInfo, a utility class, is commonly used alongside the visitor to track the schema types corresponding to the current AST nodes during this traversal [5]. DepthLimit Validation Rule There is no standard built-in DepthLimit validation rule in the core graphql-js specification-compliant ruleset [6]. While there is a MaxIntrospectionDepthRule for limiting introspection, standard query depth limiting is often implemented as a custom validation rule [7][6]. Developers frequently implement such rules by creating a visitor that tracks the current depth during traversal, incrementing it when entering a selection set and decrementing it upon leaving, then raising a GraphQLError if a threshold is exceeded [8][7]. Semantics of Validation Traversal The visitor pattern in graphql-js supports enter and leave hooks for specific AST node types [9]. Because the validation process uses visitInParallel, it ensures that all registered validation rules receive the same node during the document-wide traversal, maintaining high performance while allowing rules to maintain their own isolated state [2]. When dealing with fragment spreads, the validation engine maintains a clear separation between the operation's own selection set and the selection sets defined within fragments, relying on the context to resolve these references on-demand [3][4].
Citations:
- 1: https://github.com/graphql/graphql-js/blob/main/src/language/visitor.ts
- 2: https://github.com/graphql/graphql-js/blob/next/src/validation/validate.ts
- 3: https://github.com/graphql/graphql-js/blob/main/src/validation/ValidationContext.ts
- 4: graphql/graphql-js@688a1ee
- 5: https://www.graphql-js.org/api-v17/utilities/
- 6: https://www.graphql-js.org/api-v16/validation/
- 7: [Security] No built-in query depth/complexity limit + alias bombing DoS graphql/graphql-js#4662
- 8: Add optional maxDepth and maxAliases execution options graphql/graphql-js#4666
- 9: https://www.graphql-js.org/api-v16/language/
🏁 Script executed:
node - <<'JS'
try {
const graphql = require('graphql');
console.log('graphql version:', graphql.version || graphql.graphqlVersion || 'unknown');
const { parse, visit } = graphql;
const doc = parse(`
query Q {
user { ...F }
}
fragment F on User {
posts { comments { id } }
}
`);
const events = [];
let depth = 0;
visit(doc, {
Field: {
enter(node) {
depth++;
events.push(['enter', node.name.value, depth]);
},
leave(node) {
events.push(['leave', node.name.value, depth]);
depth--;
}
},
FragmentDefinition: {
enter(node) {
events.push(['frag-enter', node.name.value, depth]);
},
leave(node) {
events.push(['frag-leave', node.name.value, depth]);
}
}
});
console.log(JSON.stringify(events, null, 2));
} catch (e) {
console.log('ERR', e && e.message);
process.exitCode = 0;
}
JSRepository: tahiry-dev-29/Swift-Shop
Length of output: 239
🌐 Web query:
graphql-js visit document fragment definitions visited separately from fragment spreads order validation depth rule
💡 Result:
In graphql-js, the document traversal performed by the visit function is a depth-first search that treats all nodes in the AST as part of a single tree [1]. Fragment definitions and fragment spreads are visited as part of this standard document traversal; they are not inherently "separated" by the visitor logic into distinct phases or execution orders [2][1]. Regarding validation, the order in which specific rules are processed is defined by the specifiedRules array, which establishes a predictable sequence for error reporting [3]. While the visit function itself follows the AST structure (depth-first), individual validation rules, such as NoFragmentCyclesRule or KnownFragmentNamesRule, may implement their own logic to track fragment dependencies and depth [3]. Key points regarding your query: 1. Traversal Order: The visit function performs a depth-first traversal of the entire document AST [1]. There is no separate "fragment definitions" phase; they are visited when the walker encounters them in the tree [1]. 2. Validation Context: Validation rules interact with the document by registering visitors that execute during this traversal [4]. Some advanced features, such as experimental fragment arguments, require initial passes (e.g., using TypeInfo) to index fragment definitions before evaluating spreads to resolve local parameters correctly [5][6]. 3. Depth and Cycles: Rules like NoFragmentCyclesRule and depth-related validation must explicitly maintain state across the traversal to detect circular dependencies or exceed limits, as the standard visit function simply traverses the provided tree [3]. 4. Fragment Spreads: Recent updates in graphql-js have refined how fragment spreads are visited, particularly concerning deferred spreads, to avoid unnecessary re-visitation during traversal [7]. If you are building a custom validation rule that requires knowledge of fragment definitions before evaluating spreads, it is common practice to perform an initial pass over the document to collect and index fragment definitions, or to use TypeInfo to maintain state during the traversal [5][6].
Citations:
- 1: https://www.graphql-js.org/api-v16/language/
- 2: https://github.com/graphql/graphql-js/blob/main/src/language/visitor.ts
- 3: https://github.com/graphql/graphql-js/blob/next/src/validation/specifiedRules.ts
- 4: https://github.com/graphql/graphql-js/blob/main/src/validation/ValidationContext.ts
- 5: https://www.graphql-js.org/api-v17/utilities/
- 6: Enable Fragment Arguments (a form of parameterized fragments) during execution graphql/graphql-js#3152
- 7: avoid re-visiting deferred fragment spreads graphql/graphql-js#4462
Named fragment spreads undercount depth.
This visitor only tracks Field nodes, so selections reached through ...Fragment are measured in isolation instead of at the spread site. A query can push extra nesting into named fragments and exceed maxDepth without tripping this rule. Handle fragment spreads recursively or use a depth-limit library that does.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/api/src/config/graphql-depth.util.ts` around lines 3 - 27, Update
depthLimit so named fragment spreads are traversed with the depth accumulated at
their spread site, rather than validating fragment fields independently. Extend
the visitor logic around Field and fragment-spread handling to recursively
account for nested fragment selections while preserving maxDepth reporting
through context.reportError.
| return this.prisma.$transaction(async (tx) => { | ||
| const customer = await tx.customer.upsert({ | ||
| where: { email: profile.email }, | ||
| update: {}, | ||
| create: { | ||
| email: profile.email, | ||
| password: await this.passwordSecurity.hashWithoutPolicy( | ||
| randomBytes(32).toString('base64url'), | ||
| ), | ||
| firstname: profile.firstname, | ||
| lastname: profile.lastname, | ||
| groupId: group.id, | ||
| }, | ||
| include: { group: true }, | ||
| }); | ||
|
|
||
| await this.prisma.oAuthAccount.create({ | ||
| data: { | ||
| provider: profile.provider, | ||
| providerAccountId: profile.providerAccountId, | ||
| email: profile.email, | ||
| customerId: customer.id, | ||
| }, | ||
| await tx.oAuthAccount.upsert({ | ||
| where: { | ||
| provider_providerAccountId: { | ||
| provider: profile.provider, | ||
| providerAccountId: profile.providerAccountId, | ||
| }, | ||
| }, | ||
| update: {}, | ||
| create: { | ||
| provider: profile.provider, | ||
| providerAccountId: profile.providerAccountId, | ||
| email: profile.email, | ||
| customerId: customer.id, | ||
| }, | ||
| }); | ||
|
|
||
| return customer; | ||
| }); |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
sed -n '1,220p' libs/backend/core/auth/src/lib/services/auth-oauth.service.tsRepository: tahiry-dev-29/Swift-Shop
Length of output: 6311
🏁 Script executed:
sed -n '1,220p' libs/backend/core/auth/src/lib/services/password-security.service.tsRepository: tahiry-dev-29/Swift-Shop
Length of output: 2668
Move the password hash out of the transaction. hashWithoutPolicy() is just argon2.hash(...), so doing it inside this.prisma.$transaction(async (tx) => ...) keeps the interactive transaction open while the CPU-bound hash runs. Compute the dummy password first, then enter the transaction.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@libs/backend/core/auth/src/lib/services/auth-oauth.service.ts` around lines
69 - 102, Move the password generation and passwordSecurity.hashWithoutPolicy
call out of the prisma.$transaction callback, compute the hashed dummy password
before starting the transaction, and use that value in the customer upsert’s
create data while preserving the existing transaction operations.
| if (metadata.width && metadata.width > 4096) { | ||
| throw new BadRequestException( | ||
| `Image width of ${metadata.width}px exceeds maximum allowed size of 4096px`, | ||
| ); | ||
| } | ||
| if (metadata.height && metadata.height > 4096) { | ||
| throw new BadRequestException( | ||
| `Image height of ${metadata.height}px exceeds maximum allowed size of 4096px`, | ||
| ); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Specific dimension error messages are swallowed by the catch block.
The BadRequestException thrown at lines 41-43 and 46-48 is caught by the generic catch at line 84, which replaces it with 'Could not process image'. The image is still rejected (400 response), but the user loses the actionable detail about which dimension exceeded the limit.
Re-throw HttpException subclasses so the original message survives:
🛡️ Proposed fix
} catch (e) {
+ if (e instanceof HttpException) {
+ throw e;
+ }
this.logger.error(`Image processing failed: ${String(e)}`);
throw new BadRequestException('Could not process image');
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@libs/backend/core/media/src/lib/media.service.ts` around lines 40 - 49,
Update the image-processing catch block in the surrounding media service method
to rethrow existing HttpException instances, including the BadRequestException
dimension checks, before converting unexpected errors to the generic “Could not
process image” response. Preserve the original exception message and status
while retaining the current fallback handling for non-HttpException errors.
| try { | ||
| return await this.prisma.customer.create({ | ||
| data: { | ||
| email, | ||
| firstname, | ||
| lastname, | ||
| password: `guest:${randomUUID()}`, | ||
| isGuest: true, | ||
| groupId: group.id, | ||
| }, | ||
| }); | ||
| } catch (e: unknown) { | ||
| if ( | ||
| typeof e === 'object' && | ||
| e !== null && | ||
| 'code' in e && | ||
| (e as { code: string }).code === 'P2002' | ||
| ) { | ||
| const existingCustomer = await this.prisma.customer.findUnique({ | ||
| where: { email }, | ||
| }); | ||
| if (existingCustomer) { | ||
| return existingCustomer; | ||
| } | ||
| } | ||
| throw e; | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check whether the previous version of getOrCreateGuestCustomer checked isGuest,
# and whether downstream code guards against non-guest customers.
# Search for isGuest checks in the guest checkout flow
rg -n 'isGuest' libs/backend/features/order/src/lib/guest-checkout.service.ts
# Check if attachCartToCustomer or createOrderFromCart validate customer type
rg -n 'isGuest|guest' libs/backend/features/order/src/lib/order-creation.service.tsRepository: tahiry-dev-29/Swift-Shop
Length of output: 410
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the relevant files first, then inspect the guest checkout flow and its callers.
ast-grep outline libs/backend/features/order/src/lib/guest-checkout.service.ts --view expanded
ast-grep outline libs/backend/features/order/src/lib/order-creation.service.ts --view expanded
echo '--- guest-checkout.service.ts ---'
cat -n libs/backend/features/order/src/lib/guest-checkout.service.ts | sed -n '1,220p'
echo '--- order-creation.service.ts ---'
cat -n libs/backend/features/order/src/lib/order-creation.service.ts | sed -n '1,260p'Repository: tahiry-dev-29/Swift-Shop
Length of output: 11915
Guard against non-guest accounts here libs/backend/features/order/src/lib/guest-checkout.service.ts:68-73
If email already belongs to a registered customer, this returns their record and lets guest checkout attach the cart to that account. Only reuse the row when existingCustomer?.isGuest is true; otherwise raise a validation error.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@libs/backend/features/order/src/lib/guest-checkout.service.ts` around lines
50 - 76, Update the duplicate-email handling in the customer creation flow to
reuse existingCustomer only when existingCustomer.isGuest is true. If the
matching customer is registered, raise the service’s established validation
error instead of returning that record; preserve rethrowing unrelated errors.
1 participant
Summary
Type
fix
Summary by CodeRabbit
Security
Bug Fixes
Validation