Skip to content

Commit 122e302

Browse files
joaltorocjoaltoroc
committed
refactor: harden security policies, enforce response types, and integrate XSS sanitization across backend controllers
1 parent c1d9abb commit 122e302

5 files changed

Lines changed: 82 additions & 34 deletions

File tree

‎.agents/AGENTS.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ This file documents workspace-specific rules, patterns, and guidelines that all
88

99
## Security Rules (OWASP Compliance)
1010
* **Secure by Default**: Never disable authorization or validation checks.
11+
* **XSS & Security Vulnerability Prevention**: No code must contain Cross-Site Scripting (XSS), Path Traversal, SQL Injection, or other OWASP vulnerabilities. Unsanitized user inputs or query parameters must NEVER be directly injected into HTML, SVG, metadata, or template responses. All inputs rendered in client-facing outputs must be strictly sanitized or HTML/XML-escaped.
1112
* **Input Validation**:
1213
* Every endpoint receiving user parameters (`username`, `repo`, etc.) must strictly validate them using regular expressions before processing or forwarding.
1314
* Username Regex: `/^[a-z\d](?:[a-z\d]|-(?=[a-z\d])){0,38}$/i`
@@ -37,6 +38,7 @@ This file documents workspace-specific rules, patterns, and guidelines that all
3738
## Code Quality & Sonar Guidelines
3839
* **Avoid Code Duplication**: Do not duplicate common utility functions, helper methods, or business logic (e.g. XML/HTML escaping, URL parsing, custom rate limiting). Consolidate them into reusable modules or helper classes where possible.
3940
* **TypeScript Best Practices**:
41+
* **No Unused Variables or Imports**: Never leave unused variables, parameters, types, functions, or imports (`@typescript-eslint/no-unused-vars`). Clean up all unused symbols before finishing any task.
4042
* **Readonly Members**: Mark all class properties, private fields, and methods that are initialized and never reassigned as `readonly` (e.g. `private readonly handleCardRequest`).
4143
* **Strict Parameter Types**: Ensure all inputs (especially query parameters from Express `req.query`) are strictly type-checked at runtime using `typeof` and validated before passing them to internal functions to avoid type confusion.
4244
* **OWASP & Sonar Compliance**: Keep code clean and free of Sonar issues. Avoid raw `.includes()` checks for security-sensitive domains/referers. Sanitization of user inputs for XSS prevention and validation of dynamic request target hosts to prevent SSRF are required.

‎backend/package.json‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,5 +34,8 @@
3434
"tsconfig-paths": "4.2.0",
3535
"typescript": "6.0.3",
3636
"vitest": "4.1.10"
37+
},
38+
"overrides": {
39+
"find-my-way": "9.7.0"
3740
}
3841
}

‎backend/src/modules/cards/cards.controller.ts‎

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { Controller, Get, Query, Req, Res } from '@nestjs/common';
1+
import { Controller, Get, Req, Res } from '@nestjs/common';
22
import { FastifyRequest, FastifyReply } from 'fastify';
33
import { GetUserStatsCardUseCase } from '@/use-cases/cards/GetUserStatsCardUseCase';
44
import { GetUserLanguagesCardUseCase } from '@/use-cases/cards/GetUserLanguagesCardUseCase';
@@ -91,7 +91,12 @@ export class CardsController {
9191
req: FastifyRequest,
9292
res: FastifyReply,
9393
cardName: string,
94-
executeUseCase: (username: string, theme: string, overrides: Record<string, string>, hitContext?: any) => Promise<string>
94+
executeUseCase: (
95+
username: string,
96+
theme: string,
97+
overrides: Record<string, string>,
98+
hitContext?: any
99+
) => Promise<string>
95100
): Promise<void> {
96101
const query = (req.query as Record<string, any>) || {};
97102
const { username, theme } = query;
@@ -122,8 +127,15 @@ export class CardsController {
122127
.status(200)
123128
.send(svg);
124129
} catch (error: any) {
125-
logger.error(`Error rendering card ${cardName} for user ${username}`, { cardName, username, error });
126-
res.type('image/svg+xml').status(500).send(renderErrorCard(error.message || 'Error al obtener datos'));
130+
logger.error(`Error rendering card ${cardName} for user ${username}`, {
131+
cardName,
132+
username,
133+
error
134+
});
135+
res
136+
.type('image/svg+xml')
137+
.status(500)
138+
.send(renderErrorCard(error.message || 'Error al obtener datos'));
127139
}
128140
}
129141

@@ -197,13 +209,7 @@ export class CardsController {
197209
const cleanTheme = typeof theme === 'string' ? theme : undefined;
198210
const cleanStyle = typeof style === 'string' ? style : undefined;
199211

200-
const svg = renderViewsBadge(
201-
viewsCount,
202-
cleanLabel,
203-
cleanColor,
204-
cleanTheme,
205-
cleanStyle
206-
);
212+
const svg = renderViewsBadge(viewsCount, cleanLabel, cleanColor, cleanTheme, cleanStyle);
207213

208214
res
209215
.type('image/svg+xml')
@@ -214,7 +220,10 @@ export class CardsController {
214220
.send(svg);
215221
} catch (error: any) {
216222
logger.error(`Error in getProfileViews for user ${username}`, { username, error });
217-
res.type('image/svg+xml').status(500).send(renderErrorCard(error.message || 'Error al obtener visitas'));
223+
res
224+
.type('image/svg+xml')
225+
.status(500)
226+
.send(renderErrorCard(error.message || 'Error al obtener visitas'));
218227
}
219228
}
220229

‎backend/src/modules/metrics/metrics.controller.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { Controller, Get, Query, Req, Res } from '@nestjs/common';
1+
import { Controller, Get, Req, Res } from '@nestjs/common';
22
import { FastifyRequest, FastifyReply } from 'fastify';
33
import { TypeORMMetricsRepository } from '@/adapters/repositories/TypeORMMetricsRepository';
44

‎backend/src/modules/tokens/tokens.controller.ts‎

Lines changed: 55 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { RevokeUserTokenUseCase } from '@/use-cases/tokens/RevokeUserTokenUseCas
55
import { PurgeUserDataUseCase } from '@/use-cases/users/PurgeUserDataUseCase';
66
import { GITHUB_USERNAME_REGEX } from '@/domain/entities/Validation';
77
import { logger } from '@/infrastructure/logging/logger';
8+
import { escapeXml } from '@/utils/escape';
89

910
function extractBearerToken(req: FastifyRequest, bodyToken?: string): string | undefined {
1011
const authHeader = req.headers['authorization'];
@@ -36,30 +37,42 @@ export class TokensController {
3637
const { username, token, consentAccepted } = body || {};
3738

3839
if (!username || typeof username !== 'string' || !GITHUB_USERNAME_REGEX.test(username)) {
39-
res.status(400).send({ error: 'Usuario de GitHub inválido.' });
40+
res.type('application/json').status(400).send({ error: 'Usuario de GitHub inválido.' });
4041
return;
4142
}
4243

4344
if (!token || typeof token !== 'string' || token.trim() === '') {
44-
res.status(400).send({ error: 'Token de GitHub no proporcionado.' });
45+
res.type('application/json').status(400).send({ error: 'Token de GitHub no proporcionado.' });
4546
return;
4647
}
4748

4849
if (consentAccepted !== true) {
49-
res.status(400).send({ error: 'Debes aceptar los términos y condiciones de almacenamiento de datos.' });
50+
res
51+
.type('application/json')
52+
.status(400)
53+
.send({ error: 'Debes aceptar los términos y condiciones de almacenamiento de datos.' });
5054
return;
5155
}
5256

5357
try {
5458
const ip = req.ip || '';
5559
const userAgent = (req.headers['user-agent'] as string) || '';
5660

57-
const result = await this.registerUseCase.execute(username, token, consentAccepted, ip, userAgent);
61+
const result = await this.registerUseCase.execute(
62+
username,
63+
token,
64+
consentAccepted,
65+
ip,
66+
userAgent
67+
);
5868
logger.info(`Token registered successfully for user ${username}`, { username });
59-
res.status(200).send(result);
69+
res.type('application/json').status(200).send(result);
6070
} catch (error: any) {
6171
logger.error(`Error registering token for user ${username}`, { username, error });
62-
res.status(500).send({ error: error.message || 'Error interno del servidor al registrar el token.' });
72+
const safeErrorMessage = escapeXml(
73+
error?.message || 'Error interno del servidor al registrar el token.'
74+
);
75+
res.type('application/json').status(500).send({ error: safeErrorMessage });
6376
}
6477
}
6578

@@ -73,22 +86,30 @@ export class TokensController {
7386
const providedToken = extractBearerToken(req, bodyToken);
7487

7588
if (!username || typeof username !== 'string' || !GITHUB_USERNAME_REGEX.test(username)) {
76-
res.status(400).send({ error: 'Usuario de GitHub inválido.' });
89+
res.type('application/json').status(400).send({ error: 'Usuario de GitHub inválido.' });
7790
return;
7891
}
7992

8093
if (!providedToken || providedToken.trim() === '') {
81-
res.status(400).send({ error: 'Se requiere proveer un token de GitHub válido para confirmar tu identidad.' });
94+
res
95+
.type('application/json')
96+
.status(400)
97+
.send({
98+
error: 'Se requiere proveer un token de GitHub válido para confirmar tu identidad.'
99+
});
82100
return;
83101
}
84102

85103
try {
86104
const result = await this.revokeUseCase.execute(username, providedToken);
87105
logger.info(`Token revoked successfully for user ${username}`, { username });
88-
res.status(200).send(result);
106+
res.type('application/json').status(200).send(result);
89107
} catch (error: any) {
90108
logger.error(`Error revoking token for user ${username}`, { username, error });
91-
res.status(500).send({ error: error.message || 'Error interno del servidor al revocar el token.' });
109+
const safeErrorMessage = escapeXml(
110+
error?.message || 'Error interno del servidor al revocar el token.'
111+
);
112+
res.type('application/json').status(500).send({ error: safeErrorMessage });
92113
}
93114
}
94115

@@ -102,13 +123,14 @@ export class TokensController {
102123
const providedToken = extractBearerToken(req, bodyToken);
103124

104125
if (!username || typeof username !== 'string' || !GITHUB_USERNAME_REGEX.test(username)) {
105-
res.status(400).send({ error: 'Usuario de GitHub inválido.' });
126+
res.type('application/json').status(400).send({ error: 'Usuario de GitHub inválido.' });
106127
return;
107128
}
108129

109130
if (!providedToken || providedToken.trim() === '') {
110-
res.status(400).send({
111-
error: 'Se requiere proveer tu token de GitHub válido para confirmar y autorizar la purga de datos.'
131+
res.type('application/json').status(400).send({
132+
error:
133+
'Se requiere proveer tu token de GitHub válido para confirmar y autorizar la purga de datos.'
112134
});
113135
return;
114136
}
@@ -123,30 +145,42 @@ export class TokensController {
123145
});
124146

125147
if (!profileRes.ok) {
126-
res.status(401).send({ error: 'El token de GitHub provisto no es válido o ha expirado.' });
148+
res
149+
.type('application/json')
150+
.status(401)
151+
.send({ error: 'El token de GitHub provisto no es válido o ha expirado.' });
127152
return;
128153
}
129154

130155
const githubUser = (await profileRes.json()) as { login: string };
131156
const tokenOwner = githubUser.login;
132157

133158
if (tokenOwner.toLowerCase() !== username.toLowerCase()) {
134-
res.status(403).send({
135-
error: `Acceso denegado. El token proporcionado pertenece al usuario '${tokenOwner}', pero estás intentando purgar los datos de '${username}'.`
136-
});
159+
const safeOwner = escapeXml(tokenOwner);
160+
const safeUsername = escapeXml(username);
161+
res
162+
.type('application/json')
163+
.status(403)
164+
.send({
165+
error: `Acceso denegado. El token proporcionado pertenece al usuario '${safeOwner}', pero estás intentando purgar los datos de '${safeUsername}'.`
166+
});
137167
return;
138168
}
139169

140170
await this.purgeUseCase.execute(username);
141171
logger.info(`GDPR data purge completed for user ${username}`, { username });
142172

143-
res.status(200).send({
144-
message: 'Todos tus datos (token, historial, métricas de uso y logs) han sido eliminados de forma definitiva.'
173+
res.type('application/json').status(200).send({
174+
message:
175+
'Todos tus datos (token, historial, métricas de uso y logs) han sido eliminados de forma definitiva.'
145176
});
146177
} catch (error: any) {
147178
logger.error(`Error purging data for user ${username}`, { username, error });
148-
res.status(500).send({
149-
error: error.message || 'Error interno del servidor al procesar la purga de datos.'
179+
const safeErrorMessage = escapeXml(
180+
error?.message || 'Error interno del servidor al procesar la purga de datos.'
181+
);
182+
res.type('application/json').status(500).send({
183+
error: safeErrorMessage
150184
});
151185
}
152186
}

0 commit comments

Comments
 (0)