feat: initial frontend implementation and backend refinements #3
Conversation
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
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
34 issues found across 64 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="frontend/src/shared/api/client.ts">
<violation number="1" location="frontend/src/shared/api/client.ts:37">
P1: Using `localStorage.clear()` is overly destructive. This clears ALL localStorage data, not just auth-related items. It could wipe out user preferences, application state, or data from other apps on the same domain. Use the same targeted removal approach as lines 31-32.</violation>
</file>
<file name="frontend/src/shared/api/auth.ts">
<violation number="1" location="frontend/src/shared/api/auth.ts:10">
P2: Using `object` as the parameter type defeats TypeScript's type safety. Define specific interfaces for credentials and registration data to ensure proper validation and IDE support.
Example:
```typescript
interface LoginCredentials {
email: string;
password: string;
}
interface RegisterData {
email: string;
password: string;
name?: string;
}
```</violation>
</file>
<file name="frontend/src/features/auth/model/authStore.ts">
<violation number="1" location="frontend/src/features/auth/model/authStore.ts:20">
P1: Storing JWT tokens in localStorage (via Zustand persist) is a security risk. localStorage is accessible to any JavaScript on the page, making tokens vulnerable to XSS attacks.
Consider:
1. Using `sessionStorage` instead (cleared when tab closes) by configuring a custom storage
2. Storing only non-sensitive user info in persist, keeping the token in memory
3. Using httpOnly cookies for token storage on the backend instead</violation>
</file>
<file name="frontend/src/pages/home/ui/Page.tsx">
<violation number="1" location="frontend/src/pages/home/ui/Page.tsx:65">
P2: Remove console statements from production code. These debug logs expose internal details (operator name, organization ID, scan counts) and should be removed or replaced with a proper logging framework that can be disabled in production.</violation>
<violation number="2" location="frontend/src/pages/home/ui/Page.tsx:85">
P2: Replace `alert()` with a proper notification/toast system. Browser alerts block the UI thread and provide poor user experience. The message also references the console, which is not useful for end users.</violation>
<violation number="3" location="frontend/src/pages/home/ui/Page.tsx:187">
P2: Avoid loading resources from external third-party URLs. This texture image should be hosted locally or bundled with the application to ensure reliability and avoid potential security risks from external content.</violation>
</file>
<file name="frontend/src/pages/scans/ui/Page.tsx">
<violation number="1" location="frontend/src/pages/scans/ui/Page.tsx:57">
P1: Splitting translated strings by space breaks i18n for many languages. Word order, spacing, and compound words vary across languages. Instead, use separate translation keys (e.g., `t('scans.mission')` and `t('scans.control')`) or use interpolation with placeholders that can be styled differently.</violation>
<violation number="2" location="frontend/src/pages/scans/ui/Page.tsx:81">
P2: Hardcoded string "Actions" should use the translation function `t()` for consistency with other column headers in the same table.</violation>
<violation number="3" location="frontend/src/pages/scans/ui/Page.tsx:91">
P2: Hardcoded English string should use the translation function for consistency. The loading state already uses `t('common.loading')`.</violation>
<violation number="4" location="frontend/src/pages/scans/ui/Page.tsx:169">
P2: Analytics card labels are hardcoded in English. These should use the `t()` function for translation consistency.</violation>
</file>
<file name="frontend/src/shared/lib/pdf/report-generator.ts">
<violation number="1" location="frontend/src/shared/lib/pdf/report-generator.ts:103">
P2: Date formatting without a specified locale will produce inconsistent results across different user environments. Specify an explicit locale for consistent report output.</violation>
</file>
<file name="backend/src/middleware/auth.ts">
<violation number="1" location="backend/src/middleware/auth.ts:48">
P2: Debug console.error should be removed or replaced with a proper logging framework before merging to production. The comment explicitly notes this is temporary debugging code.</violation>
</file>
<file name="frontend/index.html">
<violation number="1" location="frontend/index.html:7">
P3: Generic placeholder title should be replaced with the actual application name. The title "frontend" is a Vite template default - consider using "DragonSploit" or "DragonSploit Pro" to match your project branding. This affects browser tabs, bookmarks, and SEO.</violation>
</file>
<file name="frontend/src/features/auth/ui/LoginForm.tsx">
<violation number="1" location="frontend/src/features/auth/ui/LoginForm.tsx:36">
P2: Remove `console.log` statement from production code. Debug statements should be removed or replaced with a proper logging service that can be disabled in production.</violation>
</file>
<file name="frontend/src/widgets/layout/DashboardLayout.tsx">
<violation number="1" location="frontend/src/widgets/layout/DashboardLayout.tsx:100">
P2: External resource dependency: Loading a background image from an external URL (`grainy-gradients.vercel.app`) introduces reliability and security risks. Consider hosting this asset locally or using a data URI for this noise pattern.</violation>
</file>
<file name="frontend/src/pages/terminal/ui/Page.tsx">
<violation number="1" location="frontend/src/pages/terminal/ui/Page.tsx:41">
P1: Hardcoded `localhost:3001` URL will fail in non-local environments. Use an environment variable (e.g., `import.meta.env.VITE_WS_URL`) for the WebSocket server URL to support different deployment environments.</violation>
</file>
<file name="backend/src/utils/jwt.ts">
<violation number="1" location="backend/src/utils/jwt.ts:11">
P0: Breaking change: JWT payload claim renamed from `sub` to `id`, but consumers still expect `sub`. Multiple files including `services/kullanici.ts` (lines 107, 133, 138, 152) and `controllers/kurum.ts` (lines 19, 59) access `decoded.sub` or `req.kullanici.sub`. This will cause authentication failures and undefined user IDs at runtime. Additionally, `sub` is the standard JWT claim name for subject per RFC 7519.</violation>
</file>
<file name="frontend/src/app/providers/ProtectedRoute.tsx">
<violation number="1" location="frontend/src/app/providers/ProtectedRoute.tsx:5">
P1: Race condition: The auth store uses Zustand's `persist` middleware which rehydrates asynchronously. On page refresh, `isAuthenticated` will briefly be `false` (the default) before hydration completes, causing authenticated users to be incorrectly redirected to `/login`.
Consider tracking hydration state in the store and showing a loading state until hydration is complete:
```tsx
const isHydrated = useAuthStore((state) => state._hasHydrated);
const isAuthenticated = useAuthStore((state) => state.isAuthenticated);
if (!isHydrated) {
return <LoadingSpinner />; // or null
}
```</violation>
</file>
<file name="frontend/src/pages/vulnerabilities/ui/Page.tsx">
<violation number="1" location="frontend/src/pages/vulnerabilities/ui/Page.tsx:78">
P2: Missing 'INFO' severity in filter buttons. The `VulnerabilitySeverity` type includes 'INFO' but users cannot filter for it.</violation>
<violation number="2" location="frontend/src/pages/vulnerabilities/ui/Page.tsx:129">
P2: When `scan` or `target` is undefined, this renders "undefined (undefined)" to the user. Consider using a fallback or conditional rendering.</violation>
</file>
<file name="frontend/src/shared/lib/i18n/locales/tr.json">
<violation number="1" location="frontend/src/shared/lib/i18n/locales/tr.json:69">
P2: Typo in Turkish translation: "kesif" should be "keşif" (reconnaissance). The Turkish letter "ş" is missing.</violation>
</file>
<file name="frontend/src/features/auth/ui/ForgotCredentialsForm.tsx">
<violation number="1" location="frontend/src/features/auth/ui/ForgotCredentialsForm.tsx:25">
P2: Error message is hardcoded in English instead of using the `t()` translation function. This breaks internationalization for non-English users.</violation>
</file>
<file name="frontend/src/features/language/ui/LanguageSwitcher.tsx">
<violation number="1" location="frontend/src/features/language/ui/LanguageSwitcher.tsx:19">
P2: Add `type="button"` to prevent unintended form submissions. Buttons without an explicit type default to `type="submit"`, which can cause issues if this component is used within a form.</violation>
</file>
<file name="frontend/src/features/auth/ui/RegisterForm.tsx">
<violation number="1" location="frontend/src/features/auth/ui/RegisterForm.tsx:33">
P2: Remove console statements from production code. These log detailed error information including response data which could expose sensitive backend details. Use a proper logging service or remove these debug statements.</violation>
<violation number="2" location="frontend/src/features/auth/ui/RegisterForm.tsx:101">
P2: Password input is missing `minLength={8}` attribute. The UI indicates a minimum of 8 characters but no client-side validation enforces this, allowing shorter passwords to be submitted.</violation>
</file>
<file name="backend/src/middleware/security.ts">
<violation number="1" location="backend/src/middleware/security.ts:16">
P1: Rate limit increased 10x without environment check. The comment indicates this is for development/testing, but without conditional logic like `process.env.NODE_ENV === 'production' ? 100 : 1000`, this weakened limit will apply in production too—enabling brute force and DoS attacks.</violation>
</file>
<file name="frontend/src/pages/settings/ui/Page.tsx">
<violation number="1" location="frontend/src/pages/settings/ui/Page.tsx:26">
P2: Multiple user-facing strings are hardcoded in English while the component supports i18n for other strings. For consistency across the supported locales (en, ar, tr), consider wrapping all visible text in `t()` calls.</violation>
<violation number="2" location="frontend/src/pages/settings/ui/Page.tsx:80">
P2: Splitting translated strings by space is fragile and may break in non-English locales. If the translation has fewer than 2 words, `split(' ')[1]` returns `undefined`. Consider using separate translation keys for the styled portions or a component-based translation approach.</violation>
</file>
<file name="backend/src/controllers/vulnerability.ts">
<violation number="1" location="backend/src/controllers/vulnerability.ts:11">
P2: Unsafe type assertion on query parameter. Express query values can be `string | string[] | undefined`. If multiple values are sent, this becomes an array that passes the truthy check but may cause unexpected behavior. Use proper validation or extract the first element.</violation>
</file>
<file name="backend/src/services/scans.ts">
<violation number="1" location="backend/src/services/scans.ts:29">
P2: Missing validation for `profile` parameter. The function accepts any string, but only 'lightning', 'balanced', and 'deep' are valid values (as shown in the worker processor). User-provided invalid values will silently fall through to a default behavior without feedback. Consider using a union type or adding runtime validation.</violation>
</file>
<file name="backend/src/services/vulnerability.ts">
<violation number="1" location="backend/src/services/vulnerability.ts:4">
P2: Use the existing PrismaClient singleton from `db.ts` instead of creating a new instance. Multiple PrismaClient instances can exhaust database connection pools, especially with hot-reload enabled.</violation>
</file>
<file name="backend/src/index.ts">
<violation number="1" location="backend/src/index.ts:43">
P0: Critical security issue: `origin: true` with `credentials: true` allows any website to make authenticated cross-origin requests. This CORS misconfiguration enables malicious sites to perform actions on behalf of logged-in users. Use an explicit allowlist of trusted origins instead, especially for a security platform handling sensitive data.</violation>
</file>
<file name="backend/src/controllers/scans.ts">
<violation number="1" location="backend/src/controllers/scans.ts:17">
P2: Winston's `log()` method expects arguments in order `(level, message)`, but this passes `(message, level)`. Use `logger.info()` for cleaner code.</violation>
</file>
<file name="frontend/src/entities/vulnerability/model/types.ts">
<violation number="1" location="frontend/src/entities/vulnerability/model/types.ts:6">
P2: Typo: `'XEE'` should be `'XXE'` (XML External Entity). XXE is a well-known OWASP Top 10 vulnerability type, while "XEE" is not a recognized security acronym. This could cause type mismatches with backend data.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
backend/src/utils/jwt.ts
Outdated
|
|
||
| const accessToken = jwt.sign({ sub: user.id, role: user.role }, process.env.JWT_ACCESS_SECRET, { expiresIn: '15m' }); | ||
| const refreshToken = jwt.sign({ sub: user.id, role: user.role }, process.env.JWT_REFRESH_SECRET, { expiresIn: '7d' }); | ||
| const accessToken = jwt.sign({ id: user.id, role: user.role }, process.env.JWT_ACCESS_SECRET, { expiresIn: '15m' }); |
There was a problem hiding this comment.
P0: Breaking change: JWT payload claim renamed from sub to id, but consumers still expect sub. Multiple files including services/kullanici.ts (lines 107, 133, 138, 152) and controllers/kurum.ts (lines 19, 59) access decoded.sub or req.kullanici.sub. This will cause authentication failures and undefined user IDs at runtime. Additionally, sub is the standard JWT claim name for subject per RFC 7519.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/src/utils/jwt.ts, line 11:
<comment>Breaking change: JWT payload claim renamed from `sub` to `id`, but consumers still expect `sub`. Multiple files including `services/kullanici.ts` (lines 107, 133, 138, 152) and `controllers/kurum.ts` (lines 19, 59) access `decoded.sub` or `req.kullanici.sub`. This will cause authentication failures and undefined user IDs at runtime. Additionally, `sub` is the standard JWT claim name for subject per RFC 7519.</comment>
<file context>
@@ -8,8 +8,8 @@ export const signTokens = (user: { id: string; role: Role }) => {
- const accessToken = jwt.sign({ sub: user.id, role: user.role }, process.env.JWT_ACCESS_SECRET, { expiresIn: '15m' });
- const refreshToken = jwt.sign({ sub: user.id, role: user.role }, process.env.JWT_REFRESH_SECRET, { expiresIn: '7d' });
+ const accessToken = jwt.sign({ id: user.id, role: user.role }, process.env.JWT_ACCESS_SECRET, { expiresIn: '15m' });
+ const refreshToken = jwt.sign({ id: user.id, role: user.role }, process.env.JWT_REFRESH_SECRET, { expiresIn: '7d' });
</file context>
Suggested change
| const accessToken = jwt.sign({ id: user.id, role: user.role }, process.env.JWT_ACCESS_SECRET, { expiresIn: '15m' }); | |
| const accessToken = jwt.sign({ sub: user.id, role: user.role }, process.env.JWT_ACCESS_SECRET, { expiresIn: '15m' }); |
backend/src/index.ts
Outdated
|
|
||
| // --- Middlewares --- | ||
| app.use(cors({ | ||
| origin: true, // Allow all origins in dev or specifically localhost |
There was a problem hiding this comment.
P0: Critical security issue: origin: true with credentials: true allows any website to make authenticated cross-origin requests. This CORS misconfiguration enables malicious sites to perform actions on behalf of logged-in users. Use an explicit allowlist of trusted origins instead, especially for a security platform handling sensitive data.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/src/index.ts, line 43:
<comment>Critical security issue: `origin: true` with `credentials: true` allows any website to make authenticated cross-origin requests. This CORS misconfiguration enables malicious sites to perform actions on behalf of logged-in users. Use an explicit allowlist of trusted origins instead, especially for a security platform handling sensitive data.</comment>
<file context>
@@ -37,8 +39,24 @@ async function main() {
// --- Middlewares ---
+ app.use(cors({
+ origin: true, // Allow all origins in dev or specifically localhost
+ credentials: true,
+ methods: ['GET', 'POST', 'PUT', 'DELETE', 'OPTIONS'],
</file context>
Suggested change
| origin: true, // Allow all origins in dev or specifically localhost | |
| origin: process.env.NODE_ENV === 'production' | |
| ? process.env.FRONTEND_URL || 'https://your-production-domain.com' | |
| : ['http://localhost:3000', 'http://localhost:5173'], |
frontend/src/shared/api/client.ts
Outdated
| } | ||
| } catch (e) { | ||
| // Corrupted storage, clear it | ||
| localStorage.clear(); |
There was a problem hiding this comment.
P1: Using localStorage.clear() is overly destructive. This clears ALL localStorage data, not just auth-related items. It could wipe out user preferences, application state, or data from other apps on the same domain. Use the same targeted removal approach as lines 31-32.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/shared/api/client.ts, line 37:
<comment>Using `localStorage.clear()` is overly destructive. This clears ALL localStorage data, not just auth-related items. It could wipe out user preferences, application state, or data from other apps on the same domain. Use the same targeted removal approach as lines 31-32.</comment>
<file context>
@@ -0,0 +1,44 @@
+ }
+ } catch (e) {
+ // Corrupted storage, clear it
+ localStorage.clear();
+ window.location.href = '/login';
+ }
</file context>
Suggested change
| localStorage.clear(); | |
| localStorage.removeItem('dragonsploit-auth-storage'); | |
| localStorage.removeItem('dragonsploit-auth-token'); |
| logout: () => set({ user: null, token: null, isAuthenticated: false }), | ||
| }), | ||
| { | ||
| name: 'dragonsploit-auth-storage', |
There was a problem hiding this comment.
P1: Storing JWT tokens in localStorage (via Zustand persist) is a security risk. localStorage is accessible to any JavaScript on the page, making tokens vulnerable to XSS attacks.
Consider:
- Using
sessionStorageinstead (cleared when tab closes) by configuring a custom storage - Storing only non-sensitive user info in persist, keeping the token in memory
- Using httpOnly cookies for token storage on the backend instead
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/features/auth/model/authStore.ts, line 20:
<comment>Storing JWT tokens in localStorage (via Zustand persist) is a security risk. localStorage is accessible to any JavaScript on the page, making tokens vulnerable to XSS attacks.
Consider:
1. Using `sessionStorage` instead (cleared when tab closes) by configuring a custom storage
2. Storing only non-sensitive user info in persist, keeping the token in memory
3. Using httpOnly cookies for token storage on the backend instead</comment>
<file context>
@@ -0,0 +1,23 @@
+ logout: () => set({ user: null, token: null, isAuthenticated: false }),
+ }),
+ {
+ name: 'dragonsploit-auth-storage',
+ }
+ )
</file context>
Comment on lines
+57
to
+58
| <h1 className="text-4xl font-display font-bold text-white tracking-tighter uppercase"> | ||
| {t('scans.mission_control').split(' ')[0]} <span className="text-cyber-green">{t('scans.mission_control').split(' ')[1]}</span> |
There was a problem hiding this comment.
P1: Splitting translated strings by space breaks i18n for many languages. Word order, spacing, and compound words vary across languages. Instead, use separate translation keys (e.g., t('scans.mission') and t('scans.control')) or use interpolation with placeholders that can be styled differently.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/pages/scans/ui/Page.tsx, line 57:
<comment>Splitting translated strings by space breaks i18n for many languages. Word order, spacing, and compound words vary across languages. Instead, use separate translation keys (e.g., `t('scans.mission')` and `t('scans.control')`) or use interpolation with placeholders that can be styled differently.</comment>
<file context>
@@ -0,0 +1,194 @@
+ <div className="space-y-8 pb-12">
+ <header className="flex justify-between items-end">
+ <div>
+ <h1 className="text-4xl font-display font-bold text-white tracking-tighter uppercase">
+ {t('scans.mission_control').split(' ')[0]} <span className="text-cyber-green">{t('scans.mission_control').split(' ')[1]}</span>
+ </h1>
</file context>
Suggested change
| <h1 className="text-4xl font-display font-bold text-white tracking-tighter uppercase"> | |
| {t('scans.mission_control').split(' ')[0]} <span className="text-cyber-green">{t('scans.mission_control').split(' ')[1]}</span> | |
| <h1 className="text-4xl font-display font-bold text-white tracking-tighter uppercase"> | |
| {t('scans.mission')} <span className="text-cyber-green">{t('scans.control')}</span> |
| export const initiateScan = async ( | ||
| userId: string, | ||
| targetId: string, | ||
| profile: string = 'balanced', |
There was a problem hiding this comment.
P2: Missing validation for profile parameter. The function accepts any string, but only 'lightning', 'balanced', and 'deep' are valid values (as shown in the worker processor). User-provided invalid values will silently fall through to a default behavior without feedback. Consider using a union type or adding runtime validation.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/src/services/scans.ts, line 29:
<comment>Missing validation for `profile` parameter. The function accepts any string, but only 'lightning', 'balanced', and 'deep' are valid values (as shown in the worker processor). User-provided invalid values will silently fall through to a default behavior without feedback. Consider using a union type or adding runtime validation.</comment>
<file context>
@@ -26,6 +26,7 @@ const getTechnologyFingerprint = async (targetUrl: string): Promise<string> => {
export const initiateScan = async (
userId: string,
targetId: string,
+ profile: string = 'balanced',
configurationId?: string
) => {
</file context>
Suggested change
| profile: string = 'balanced', | |
| profile: 'lightning' | 'balanced' | 'deep' = 'balanced', |
| import { PrismaClient } from '@prisma/client'; | ||
| import { ForbiddenError } from '../utils/errors'; | ||
|
|
||
| const prisma = new PrismaClient(); |
There was a problem hiding this comment.
P2: Use the existing PrismaClient singleton from db.ts instead of creating a new instance. Multiple PrismaClient instances can exhaust database connection pools, especially with hot-reload enabled.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/src/services/vulnerability.ts, line 4:
<comment>Use the existing PrismaClient singleton from `db.ts` instead of creating a new instance. Multiple PrismaClient instances can exhaust database connection pools, especially with hot-reload enabled.</comment>
<file context>
@@ -0,0 +1,48 @@
+import { PrismaClient } from '@prisma/client';
+import { ForbiddenError } from '../utils/errors';
+
+const prisma = new PrismaClient();
+
+/**
</file context>
| const scan = await scanService.initiateScan(userId, targetId, configurationId); | ||
| const scan = await scanService.initiateScan(userId, targetId, profile, configurationId); | ||
|
|
||
| logger.log(`OPERATIONS :: Scan sequence initiated for asset ${targetId}`, 'info'); |
There was a problem hiding this comment.
P2: Winston's log() method expects arguments in order (level, message), but this passes (message, level). Use logger.info() for cleaner code.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/src/controllers/scans.ts, line 17:
<comment>Winston's `log()` method expects arguments in order `(level, message)`, but this passes `(message, level)`. Use `logger.info()` for cleaner code.</comment>
<file context>
@@ -8,11 +9,13 @@ export const createScan = async (
- const scan = await scanService.initiateScan(userId, targetId, configurationId);
+ const scan = await scanService.initiateScan(userId, targetId, profile, configurationId);
+ logger.log(`OPERATIONS :: Scan sequence initiated for asset ${targetId}`, 'info');
+
// --- بداية التعديل ---
</file context>
Suggested change
| logger.log(`OPERATIONS :: Scan sequence initiated for asset ${targetId}`, 'info'); | |
| logger.info(`OPERATIONS :: Scan sequence initiated for asset ${targetId}`); |
| export type VulnerabilityType = | ||
| | 'SQL_INJECTION' | ||
| | 'XSS' | ||
| | 'XEE' |
There was a problem hiding this comment.
P2: Typo: 'XEE' should be 'XXE' (XML External Entity). XXE is a well-known OWASP Top 10 vulnerability type, while "XEE" is not a recognized security acronym. This could cause type mismatches with backend data.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/entities/vulnerability/model/types.ts, line 6:
<comment>Typo: `'XEE'` should be `'XXE'` (XML External Entity). XXE is a well-known OWASP Top 10 vulnerability type, while "XEE" is not a recognized security acronym. This could cause type mismatches with backend data.</comment>
<file context>
@@ -0,0 +1,35 @@
+export type VulnerabilityType =
+ | 'SQL_INJECTION'
+ | 'XSS'
+ | 'XEE'
+ | 'IDOR'
+ | 'BROKEN_AUTH'
</file context>
Suggested change
| | 'XEE' | |
| | 'XXE' |
frontend/index.html
Outdated
| <meta charset="UTF-8" /> | ||
| <link rel="icon" type="image/svg+xml" href="/vite.svg" /> | ||
| <meta name="viewport" content="width=device-width, initial-scale=1.0" /> | ||
| <title>frontend</title> |
There was a problem hiding this comment.
P3: Generic placeholder title should be replaced with the actual application name. The title "frontend" is a Vite template default - consider using "DragonSploit" or "DragonSploit Pro" to match your project branding. This affects browser tabs, bookmarks, and SEO.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/index.html, line 7:
<comment>Generic placeholder title should be replaced with the actual application name. The title "frontend" is a Vite template default - consider using "DragonSploit" or "DragonSploit Pro" to match your project branding. This affects browser tabs, bookmarks, and SEO.</comment>
<file context>
@@ -0,0 +1,13 @@
+ <meta charset="UTF-8" />
+ <link rel="icon" type="image/svg+xml" href="/vite.svg" />
+ <meta name="viewport" content="width=device-width, initial-scale=1.0" />
+ <title>frontend</title>
+ </head>
+ <body>
</file context>
Suggested change
| <title>frontend</title> | |
| <title>DragonSploit</title> |
- Security: Fix JWT claim mismatch, restrictive CORS, auth hydration race condition, storage clearing risk, and WebSocket security. - Quality: Implement Prisma singleton, fix logging/types, improve i18n, and remove debug logs. - Infra: Fix Docker/Redis/DB connection issues, improve fingerprinting robustness, and increase timeouts.
There was a problem hiding this comment.
10 issues found across 385 files
Note: This PR contains a large number of files. cubic only reviews up to 75 files per PR, so some files may not have been reviewed.
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="backend/proofs/scans/cmkawahvv0009egsk76e0x8fc/50999045-b732-48cd-948a-4de28a8f4fac/page.html">
<violation number="1" location="backend/proofs/scans/cmkawahvv0009egsk76e0x8fc/50999045-b732-48cd-948a-4de28a8f4fac/page.html:1">
P2: Generated scan proof artifacts should not be committed to version control. The `backend/proofs/` directory contains 53+ scan result directories with auto-generated content. These files bloat the repository and should be:
1. Added to `.gitignore` (e.g., `proofs/` or `proofs/scans/`)
2. Stored externally (database, object storage) if persistence is needed
Consider removing these files and adding the directory to `.gitignore`.</violation>
</file>
<file name="backend/src/utils/logger.ts">
<violation number="1" location="backend/src/utils/logger.ts:16">
P1: WebSocket connections are not authenticated. Any client can connect, join the terminal room, and execute commands. For a security scanning platform, this is a significant vulnerability. Consider adding authentication middleware to verify JWT tokens before allowing socket operations.</violation>
<violation number="2" location="backend/src/utils/logger.ts:37">
P2: Missing input validation on `data.command`. If a client sends malformed data (e.g., `{}` or `{command: null}`), calling `.toLowerCase()` will throw a TypeError and crash the handler.</violation>
</file>
<file name="backend/src/index.ts">
<violation number="1" location="backend/src/index.ts:54">
P2: Redundant OPTIONS request handling. The `cors` middleware already handles preflight OPTIONS requests automatically. This manual handler is either dead code (cors already responded) or could potentially cause issues if it intercepts requests after cors has only set headers but called next(). Remove this manual OPTIONS handling and let the cors middleware manage it.</violation>
</file>
<file name="backend/package.json">
<violation number="1" location="backend/package.json:24">
P2: `@types/socket.io` is unnecessary for `socket.io@^4.8.3` as v4+ bundles its own TypeScript definitions. Additionally, this type definition package should be in `devDependencies` if needed at all, not `dependencies`. The `@types/socket.io@^3.0.1` package provides types for the older v2.x API and may conflict with the bundled v4 types. Consider removing this dependency entirely.</violation>
</file>
<file name="backend/src/services/scans.ts">
<violation number="1" location="backend/src/services/scans.ts:35">
P2: Use `BadRequestError` instead of generic `Error` for input validation failures. This ensures consistent error handling and returns HTTP 400 status to clients. The `BadRequestError` class is already available in the codebase but not imported in this file.</violation>
</file>
<file name="backend/src/controllers/target.ts">
<violation number="1" location="backend/src/controllers/target.ts:24">
P2: User-controlled `name` and `url` are interpolated directly into log messages without sanitization. Since the logger broadcasts to WebSocket clients and console, this enables log injection attacks. Consider sanitizing or encoding user input before logging.</violation>
</file>
<file name="backend/src/middleware/security.ts">
<violation number="1" location="backend/src/middleware/security.ts:16">
P2: Security anti-pattern: rate limit defaults to the less secure option. If `NODE_ENV` is unset or misspelled in production, the app will use 1000 requests instead of 100. Consider inverting the logic to default to the stricter limit.</violation>
</file>
<file name="backend/src/middleware/auth.ts">
<violation number="1" location="backend/src/middleware/auth.ts:44">
P1: Missing algorithm specification in `jwt.verify()` - this is a security vulnerability. Without explicitly specifying the allowed algorithms, the verification may be susceptible to algorithm confusion attacks. Always specify the expected algorithm(s) explicitly.</violation>
</file>
<file name="backend/src/controllers/vulnerability.ts">
<violation number="1" location="backend/src/controllers/vulnerability.ts:12">
P2: Unsafe type assertion on query parameter. Express query params can be objects (e.g., `?organizationId[foo]=bar`), which would bypass validation and produce `"[object Object]"`. Use explicit type checking.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| } | ||
| }); | ||
|
|
||
| this.io.on('connection', (socket) => { |
There was a problem hiding this comment.
P1: WebSocket connections are not authenticated. Any client can connect, join the terminal room, and execute commands. For a security scanning platform, this is a significant vulnerability. Consider adding authentication middleware to verify JWT tokens before allowing socket operations.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/src/utils/logger.ts, line 16:
<comment>WebSocket connections are not authenticated. Any client can connect, join the terminal room, and execute commands. For a security scanning platform, this is a significant vulnerability. Consider adding authentication middleware to verify JWT tokens before allowing socket operations.</comment>
<file context>
@@ -0,0 +1,67 @@
+ }
+ });
+
+ this.io.on('connection', (socket) => {
+ console.log(`[WebSocket] Operator connected: ${socket.id}`);
+
</file context>
| } | ||
|
|
||
| // 3. التحقق من صحة التوكن باستخدام المفتاح السري الصحيح | ||
| jwt.verify(token, secret, (err: any, kullanici: any) => { |
There was a problem hiding this comment.
P1: Missing algorithm specification in jwt.verify() - this is a security vulnerability. Without explicitly specifying the allowed algorithms, the verification may be susceptible to algorithm confusion attacks. Always specify the expected algorithm(s) explicitly.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/src/middleware/auth.ts, line 44:
<comment>Missing algorithm specification in `jwt.verify()` - this is a security vulnerability. Without explicitly specifying the allowed algorithms, the verification may be susceptible to algorithm confusion attacks. Always specify the expected algorithm(s) explicitly.</comment>
<file context>
@@ -0,0 +1,60 @@
+ }
+
+ // 3. التحقق من صحة التوكن باستخدام المفتاح السري الصحيح
+ jwt.verify(token, secret, (err: any, kullanici: any) => {
+ // إذا كان التوكن غير صالح (منتهي الصلاحية، تالف، إلخ)
+ if (err) {
</file context>
Suggested change
| jwt.verify(token, secret, (err: any, kullanici: any) => { | |
| jwt.verify(token, secret, { algorithms: ['HS256'] }, (err: any, kullanici: any) => { |
| @@ -0,0 +1 @@ | |||
| <html><head><meta name="color-scheme" content="light dark"><meta charset="utf-8"></head><body><pre>{"status":"success","data":{"id":1,"name":"Apple Juice (1000ml)","description":"The all-time classic.","price":1.99,"deluxePrice":0.99,"image":"apple_juice.jpg","createdAt":"2026-01-12T06:14:40.837Z","updatedAt":"2026-01-12T06:14:40.837Z","deletedAt":null}}</pre><div class="json-formatter-container"></div></body></html> No newline at end of file | |||
There was a problem hiding this comment.
P2: Generated scan proof artifacts should not be committed to version control. The backend/proofs/ directory contains 53+ scan result directories with auto-generated content. These files bloat the repository and should be:
- Added to
.gitignore(e.g.,proofs/orproofs/scans/) - Stored externally (database, object storage) if persistence is needed
Consider removing these files and adding the directory to .gitignore.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/proofs/scans/cmkawahvv0009egsk76e0x8fc/50999045-b732-48cd-948a-4de28a8f4fac/page.html, line 1:
<comment>Generated scan proof artifacts should not be committed to version control. The `backend/proofs/` directory contains 53+ scan result directories with auto-generated content. These files bloat the repository and should be:
1. Added to `.gitignore` (e.g., `proofs/` or `proofs/scans/`)
2. Stored externally (database, object storage) if persistence is needed
Consider removing these files and adding the directory to `.gitignore`.</comment>
<file context>
@@ -0,0 +1 @@
+<html><head><meta name="color-scheme" content="light dark"><meta charset="utf-8"></head><body><pre>{"status":"success","data":{"id":1,"name":"Apple Juice (1000ml)","description":"The all-time classic.","price":1.99,"deluxePrice":0.99,"image":"apple_juice.jpg","createdAt":"2026-01-12T06:14:40.837Z","updatedAt":"2026-01-12T06:14:40.837Z","deletedAt":null}}</pre><div class="json-formatter-container"></div></body></html>
\ No newline at end of file
</file context>
| } | ||
|
|
||
| private processRemoteCommand(cmd: string) { | ||
| const command = cmd.toLowerCase().trim(); |
There was a problem hiding this comment.
P2: Missing input validation on data.command. If a client sends malformed data (e.g., {} or {command: null}), calling .toLowerCase() will throw a TypeError and crash the handler.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/src/utils/logger.ts, line 37:
<comment>Missing input validation on `data.command`. If a client sends malformed data (e.g., `{}` or `{command: null}`), calling `.toLowerCase()` will throw a TypeError and crash the handler.</comment>
<file context>
@@ -0,0 +1,67 @@
+ }
+
+ private processRemoteCommand(cmd: string) {
+ const command = cmd.toLowerCase().trim();
+
+ if (command === 'status') {
</file context>
Comment on lines
+54
to
+56
| if (req.method === 'OPTIONS') { | ||
| return res.sendStatus(200); | ||
| } |
There was a problem hiding this comment.
P2: Redundant OPTIONS request handling. The cors middleware already handles preflight OPTIONS requests automatically. This manual handler is either dead code (cors already responded) or could potentially cause issues if it intercepts requests after cors has only set headers but called next(). Remove this manual OPTIONS handling and let the cors middleware manage it.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/src/index.ts, line 54:
<comment>Redundant OPTIONS request handling. The `cors` middleware already handles preflight OPTIONS requests automatically. This manual handler is either dead code (cors already responded) or could potentially cause issues if it intercepts requests after cors has only set headers but called next(). Remove this manual OPTIONS handling and let the cors middleware manage it.</comment>
<file context>
@@ -37,8 +39,24 @@ async function main() {
+ // Custom Request Logger
+ app.use((req, res, next) => {
+ console.log(`[${new Date().toISOString()}] ${req.method} ${req.url}`);
+ if (req.method === 'OPTIONS') {
+ return res.sendStatus(200);
+ }
</file context>
Suggested change
| if (req.method === 'OPTIONS') { | |
| return res.sendStatus(200); | |
| } | |
| // CORS middleware handles OPTIONS preflight automatically |
| "@modelcontextprotocol/sdk": "^1.23.0", | ||
| "@prisma/client": "^6.18.0", | ||
| "@types/cors": "^2.8.19", | ||
| "@types/socket.io": "^3.0.1", |
There was a problem hiding this comment.
P2: @types/socket.io is unnecessary for socket.io@^4.8.3 as v4+ bundles its own TypeScript definitions. Additionally, this type definition package should be in devDependencies if needed at all, not dependencies. The @types/socket.io@^3.0.1 package provides types for the older v2.x API and may conflict with the bundled v4 types. Consider removing this dependency entirely.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/package.json, line 24:
<comment>`@types/socket.io` is unnecessary for `socket.io@^4.8.3` as v4+ bundles its own TypeScript definitions. Additionally, this type definition package should be in `devDependencies` if needed at all, not `dependencies`. The `@types/socket.io@^3.0.1` package provides types for the older v2.x API and may conflict with the bundled v4 types. Consider removing this dependency entirely.</comment>
<file context>
@@ -21,6 +21,7 @@
"@modelcontextprotocol/sdk": "^1.23.0",
"@prisma/client": "^6.18.0",
"@types/cors": "^2.8.19",
+ "@types/socket.io": "^3.0.1",
"axios": "^1.12.2",
"bcryptjs": "^3.0.2",
</file context>
| // Validate profile | ||
| const validProfiles = ['lightning', 'balanced', 'deep']; | ||
| if (!validProfiles.includes(profile)) { | ||
| throw new Error(`Invalid scan profile: ${profile}. Must be one of: ${validProfiles.join(', ')}`); |
There was a problem hiding this comment.
P2: Use BadRequestError instead of generic Error for input validation failures. This ensures consistent error handling and returns HTTP 400 status to clients. The BadRequestError class is already available in the codebase but not imported in this file.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/src/services/scans.ts, line 35:
<comment>Use `BadRequestError` instead of generic `Error` for input validation failures. This ensures consistent error handling and returns HTTP 400 status to clients. The `BadRequestError` class is already available in the codebase but not imported in this file.</comment>
<file context>
@@ -26,8 +26,15 @@ const getTechnologyFingerprint = async (targetUrl: string): Promise<string> => {
+ // Validate profile
+ const validProfiles = ['lightning', 'balanced', 'deep'];
+ if (!validProfiles.includes(profile)) {
+ throw new Error(`Invalid scan profile: ${profile}. Must be one of: ${validProfiles.join(', ')}`);
+ }
+
</file context>
| @@ -20,6 +21,7 @@ export const hedefOlustur = async (req: Request, res: Response) => { | |||
| const { name, url, organizationId } = req.body; | |||
| try { | |||
| const yeniHedef = await organizasyonIcinHedefOlustur({ name, url, organizationId, olusturanId }); | |||
| logger.log(`ASSET_DISCOVERY :: New target localized: ${name} (${url})`, 'success'); | |||
There was a problem hiding this comment.
P2: User-controlled name and url are interpolated directly into log messages without sanitization. Since the logger broadcasts to WebSocket clients and console, this enables log injection attacks. Consider sanitizing or encoding user input before logging.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/src/controllers/target.ts, line 24:
<comment>User-controlled `name` and `url` are interpolated directly into log messages without sanitization. Since the logger broadcasts to WebSocket clients and console, this enables log injection attacks. Consider sanitizing or encoding user input before logging.</comment>
<file context>
@@ -20,6 +21,7 @@ export const hedefOlustur = async (req: Request, res: Response) => {
const { name, url, organizationId } = req.body;
try {
const yeniHedef = await organizasyonIcinHedefOlustur({ name, url, organizationId, olusturanId });
+ logger.log(`ASSET_DISCOVERY :: New target localized: ${name} (${url})`, 'success');
return res.status(201).json(yeniHedef);
} catch (hata) {
</file context>
Suggested change
| logger.log(`ASSET_DISCOVERY :: New target localized: ${name} (${url})`, 'success'); | |
| logger.log(`ASSET_DISCOVERY :: New target localized: ${name.replace(/[\r\n]/g, '')} (${url.replace(/[\r\n]/g, '')})`, 'success'); |
| @@ -13,10 +13,10 @@ export const setupSecurity = (app: Express) => { | |||
| // 2. Rate limiting | |||
| const limiter = rateLimit({ | |||
| windowMs: 15 * 60 * 1000, // 15 minutes | |||
| max: 100, // Limit each IP to 100 requests per windowMs | |||
| max: process.env.NODE_ENV === 'production' ? 100 : 1000, // 100 for prod, 1000 for dev | |||
There was a problem hiding this comment.
P2: Security anti-pattern: rate limit defaults to the less secure option. If NODE_ENV is unset or misspelled in production, the app will use 1000 requests instead of 100. Consider inverting the logic to default to the stricter limit.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/src/middleware/security.ts, line 16:
<comment>Security anti-pattern: rate limit defaults to the less secure option. If `NODE_ENV` is unset or misspelled in production, the app will use 1000 requests instead of 100. Consider inverting the logic to default to the stricter limit.</comment>
<file context>
@@ -13,10 +13,10 @@ export const setupSecurity = (app: Express) => {
const limiter = rateLimit({
windowMs: 15 * 60 * 1000, // 15 minutes
- max: 100, // Limit each IP to 100 requests per windowMs
+ max: process.env.NODE_ENV === 'production' ? 100 : 1000, // 100 for prod, 1000 for dev
message: 'Too many requests from this IP, please try again later.',
- standardHeaders: true, // Return rate limit info in the `RateLimit-*` headers
</file context>
Suggested change
| max: process.env.NODE_ENV === 'production' ? 100 : 1000, // 100 for prod, 1000 for dev | |
| max: process.env.NODE_ENV === 'development' ? 1000 : 100, // 1000 for dev, 100 otherwise (safer default) |
| try { | ||
| const userId = req.kullanici!.id; | ||
| const organizationIdParam = req.query.organizationId; | ||
| const organizationId = Array.isArray(organizationIdParam) ? organizationIdParam[0] as string : organizationIdParam as string; |
There was a problem hiding this comment.
P2: Unsafe type assertion on query parameter. Express query params can be objects (e.g., ?organizationId[foo]=bar), which would bypass validation and produce "[object Object]". Use explicit type checking.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/src/controllers/vulnerability.ts, line 12:
<comment>Unsafe type assertion on query parameter. Express query params can be objects (e.g., `?organizationId[foo]=bar`), which would bypass validation and produce `"[object Object]"`. Use explicit type checking.</comment>
<file context>
@@ -0,0 +1,23 @@
+ try {
+ const userId = req.kullanici!.id;
+ const organizationIdParam = req.query.organizationId;
+ const organizationId = Array.isArray(organizationIdParam) ? organizationIdParam[0] as string : organizationIdParam as string;
+
+ if (!organizationId) {
</file context>
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name=".github/workflows/ci.yml">
<violation number="1" location=".github/workflows/ci.yml:30">
P2: Use `npm ci` instead of `npm install` for CI pipelines. `npm ci` ensures reproducible builds by installing exact versions from the lockfile and is faster in CI environments. It will also fail if the lockfile is out of sync with package.json, catching dependency issues early.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
This PR introduces the first version of the frontend application and applies critical refactoring to the backend architecture.
Frontend
I have established the project structure using Feature-Sliced Design (FSD) to ensure long-term scalability.
Backend
Verification
Summary by cubic
Launches the first frontend version (React + Vite, FSD) and tightens backend architecture with standardized JWT, a new vulnerabilities API, and an SQLi false-positive fix. Adds scan profiles, CI for backend and frontend builds, and improves dev experience with hot-reload workers and a new DB check helper.
New Features
Refactors
Written for commit b18dd1d. Summary will update on new commits.