ci(security): integrate Bright CI pipeline for security tests and remediation

[bright-security-golf]

Note

Fixed 14 of 18 vulnerabilities.
Please review the fixes before merging.

Fix	Vulnerability	Endpoint	Affected Files	Resolution
✅	[Critical] XPATH Injection	GET /api/partners/partnerLogin	src/partners/partners.service.ts	Sanitize user inputs for XPath queries to prevent injection attacks.
✅	[Critical] Server Side Template Injection	POST /api/render	src/app.controller.ts	Escaped template expressions in user input to prevent code execution.
✅	[Critical] XML External Entity (XXE)	POST /api/metadata	src/app.controller.ts	Disabled external entity expansion and DTD validation in XML parser to prevent XXE attacks.
✅	[Critical] Server Side Javascript Injection	POST /api/process_numbers	src/app.controller.ts	Replaced the use of eval with a safer Function constructor to prevent server-side JavaScript injection.
✅	[Critical] XPATH Injection	GET /api/partners/searchPartners	src/partners/partners.service.ts	Sanitize XPath expressions to prevent injection by removing potentially harmful characters.
✅	[High] Server Side Request Forgery	GET /api/file/google	src/file/file.controller.ts	Added validation to ensure the 'path' parameter starts with the expected base URL for Google Cloud, preventing SSRF attacks.
✅	[High] [BL] ID Enumeration	GET /api/users/id/1	src/users/users.controller.ts	Added authorization check to ensure users can only access their own information by verifying the requester's identity against the requested user ID.
✅	[High] Server Side Request Forgery	GET /api/file/digital_ocean	src/file/file.controller.ts	Added validation to ensure the path starts with the expected base URL for Digital Ocean, preventing SSRF attacks.
✅	[High] Server Side Request Forgery	GET /api/file/azure	src/file/file.controller.ts	Added validation to ensure the path starts with the expected base URL for Azure requests, preventing SSRF attacks.
✅	[High] Server Side Request Forgery	GET /api/file/aws	src/file/file.controller.ts	Added validation to ensure the 'path' parameter in AWS file loading starts with the expected base URL to prevent SSRF.
✅	[High] Cross-Site Scripting	POST /api/metadata	src/app.controller.ts	Applied stronger XSS mitigation by encoding XML content to prevent script execution.
✅	[Medium] Full Path Disclosure	DELETE /api/file	src/file/file.controller.ts, src/file/file.service.ts	Replaced detailed error messages with generic ones to prevent full path disclosure.
✅	[Medium] Secret Tokens Leak	GET /api/secrets	src/app.controller.ts	Secret tokens are now retrieved from environment variables instead of being hardcoded in the source code.
✅	[Medium] Database Error Message Disclosure	GET /api/testimonials/count	src/testimonials/testimonials.service.ts	Replace detailed error messages with a generic error response to prevent information leakage.
❌	[Medium] [BL] Business Constraint Bypass	GET /api/products/latest	src/products/products.service.ts, src/products/products.controller.ts	Attempted fix: Enforce the maximum limit directly in the service layer to ensure it cannot be bypassed by any controller logic.
❌	[Medium] Unvalidated Redirect	GET /api/goto	src/app.controller.ts	Attempted fix: Implemented a strict allowlist and removed query parameters to prevent unvalidated redirects.

Workflow execution details

• ✅ Repository Analysis: TypeScript, NestJS
• ✅ Entrypoints Discovery: 54 entrypoints found
• ✅ Attack Vectors Identification
• ✅ E2E Security Tests Generation
• ✅ E2E Security Tests Execution: 16 vulnerabilities found
• ✅ Cleanup Irrelevant Test Files: 39 test files removed
• ✅ Applying Security Fixes: 16 fixes applied
• ✅ E2E Security Tests Execution: 4 vulnerabilities found
• ✅ Cleanup Irrelevant Test Files: 11 test files removed
• ✅ Applying Security Fixes: 4 fixes applied
• ✅ E2E Security Tests Execution: 2 vulnerabilities found
• ✅ Cleanup Irrelevant Test Files: 2 test files removed
• ✅ Applying Security Fixes: 2 fixes applied
• ✅ E2E Security Tests Execution: 2 vulnerabilities found
• ✅ Cleanup Irrelevant Test Files: 0 test files removed
• ✅ Applying Security Fixes: 2 fixes applied
• ✅ E2E Security Tests Execution: 2 vulnerabilities found
• ✅ Cleanup Irrelevant Test Files: 0 test files removed
• ✅ Applying Security Fixes: 2 fixes applied
• ✅ E2E Security Tests Execution: 2 vulnerabilities found
• ⏭️ Cleanup Irrelevant Test Files: Skipped
• ⏭️ Applying Security Fixes: Skipped
• ⏭️ Workflow Wrap-Up: Skipped

General approach: constrain all filesystem access/deletion to a defined safe root directory on the server, normalize resolved paths, and verify that the final path remains within that root before calling fs APIs. Reject or error out when a user-supplied path points outside the root. This addresses all variants because they all eventually call FileService.getFile (and deleteFile) with user-controlled values.

Concrete fix for this code:

1. In FileService, define a constant safe base directory, for example a subdirectory under process.cwd() (e.g. FILES_ROOT = path.resolve(process.cwd(), 'files')). Since we cannot assume other project config, using process.cwd() plus a fixed subfolder keeps behavior close to current relative-path usage while adding a boundary.

2. Implement a small helper (inside FileService) to resolve a user-supplied path against FILES_ROOT and enforce containment:

   • Use path.resolve(FILES_ROOT, file) to normalize.
   • Optionally call fs.realpathSync or fs.promises.realpath on the resolved path to collapse symlinks (we’ll use the async fs.promises.realpath to stay within async flow).
   • Check that the canonical path starts with FILES_ROOT + path.sep or equals FILES_ROOT. If not, throw an error.

3. Update getFile:

   • Keep the http branch as-is for remote/cloud objects.
   • Remove the branch that trusts absolute paths starting with /, because that allows access anywhere. Instead, treat any non-http input as a logical relative path under FILES_ROOT:
     • Compute const safePath = await this.getSafeLocalPath(file);
     • Use fs.promises.access(safePath, R_OK) and fs.createReadStream(safePath).
   • This preserves functionality for relative paths but removes arbitrary absolute path access.

4. Update deleteFile similarly:

   • Forbid deletion of http/cloud resources as before.
   • Treat any non-http value as a relative path under FILES_ROOT:
     • Compute const safePath = await this.getSafeLocalPath(file);
     • Call fs.promises.unlink(safePath).

5. Error handling: reuse the existing try/catch blocks in FileService; if the path is outside the root, getSafeLocalPath throws an error caught and wrapped into InternalServerErrorException, preserving the controller contracts.

No changes are required in FileController to fix the core issue, because once FileService validates and constrains filesystem paths, all controller variants that call getFile or deleteFile will benefit automatically.

---

To fix this, we need to ensure that filesystem paths derived from user input are constrained to a safe root directory and cannot escape it via .., symlinks, or absolute paths. For URLs (HTTP/HTTPS) we can continue to delegate to CloudProvidersMetaData as today.

The best approach with minimal functional change is:

1. Define a fixed base directory for local file operations, e.g. const LOCAL_ROOT = path.resolve(process.cwd(), ''); (or a more specific subfolder if your app uses one).
2. For any non-HTTP path in getFile and deleteFile:
   • If it is absolute, resolve it relative to LOCAL_ROOT (i.e. disallow arbitrary absolute paths).
   • Use path.resolve(LOCAL_ROOT, userInput) to normalize and join.
   • Optionally, call fs.realpathSync/fs.promises.realpath to resolve symlinks.
   • After resolving, verify the resulting path is still under LOCAL_ROOT with a prefix check (e.g. if (!resolvedPath.startsWith(LOCAL_ROOT + path.sep))).
   • If the check fails, throw a BadRequestException or InternalServerErrorException indicating an invalid path, instead of accessing the filesystem.
3. Use the sanitized path for fs.promises.access, fs.createReadStream, and fs.promises.unlink.
4. For write operations in the controller (saveRawContent snippet around lines 330–336), we should also constrain the destination path similarly: resolve against the same LOCAL_ROOT, check it stays within that root, then access(dirname) and writeFile using the safe path.

Because instructions restrict edits to src/file/file.service.ts and/or src/file/file.controller.ts, we will:

• Add a private helper method in FileService to sanitize local filesystem paths (it can be used by getFile and deleteFile).
• Optionally add a small helper in FileController to sanitize paths used in saveRawContent (if that method exists in the omitted sections and uses user-provided file path; we only have the lower snippet, so we’ll only change what is visible: constrain file before access/writeFile).
• Keep existing behavior for HTTP URLs and cloud-provider-prefixed paths, which don’t hit the filesystem directly.

Concretely:

• In src/file/file.service.ts, introduce private readonly LOCAL_ROOT = path.resolve(process.cwd()); and a private resolveLocalPath(untrusted: string): string:
  • Reject any path that starts with http (already handled) or with .. segments that would traverse outside root.
  • Use path.resolve(this.LOCAL_ROOT, untrusted) and then fs.realpathSync (sync is acceptable for a small single call; otherwise we could use await fs.promises.realpath).
  • Check that the final real path starts with this.LOCAL_ROOT + path.sep (or equals this.LOCAL_ROOT).
• Change getFile:
  • Remove the branch that allows arbitrary absolute / paths.
  • For non-HTTP paths, call this.resolveLocalPath(file) into e.g. const safePath = this.resolveLocalPath(file);, then run access and createReadStream on safePath.
• Change deleteFile similarly: use this.resolveLocalPath(file) and unlink that safe path; remove the explicit absolute path/HTTP handling (or keep those branches but ensure they cannot be abused).
• In src/file/file.controller.ts, in the saveRawContent-like method around lines 330–336, we have:

  await fs.promises.access(path.dirname(file), W_OK);
  await fs.promises.writeFile(file, raw);

  We will:
  • Resolve file against a root (we can reuse process.cwd() here, similar to the service) and ensure it does not escape, using the same normalization and prefix check pattern.
  • Use the sanitized safeFile both in dirname/access and writeFile.

No new external dependencies are required; we will rely on Node’s built-in path and fs modules that are already imported.

---

In general, to fix uncontrolled path usage you must (a) decide on a safe root directory for filesystem operations, (b) normalize any user-supplied relative path against that root using path.resolve, and (c) verify that the resulting absolute path is still inside the root (e.g. by checking startsWith(root + path.sep) or equivalent). If the check fails, reject the request. For absolute paths, you typically should not allow arbitrary user choice at all unless you have a very strict whitelist.

For this codebase, the most targeted, non‑breaking fix is:

1. Introduce a private constant in FileService representing the allowed local root (e.g. LOCAL_ROOT = process.cwd();). This keeps current behavior (using the working directory) but allows for the containment check.
2. In getFile:
   • Disallow arbitrary absolute filesystem paths coming from the client: remove or harden the if (file.startsWith('/')) branch. The safest change without altering high-level behavior is to treat all non-HTTP paths as relative to LOCAL_ROOT, resolve them, and confirm they stay under that root.
   • After path.resolve, compute const resolved = path.resolve(this.LOCAL_ROOT, file); (or similar), and check that resolved starts with this.LOCAL_ROOT (taking care to handle separators so /var/wwwX is not accepted when the root is /var/www).
   • If the check fails, throw an error (which will be mapped to InternalServerErrorException('An error occurred while accessing the file.'); you could alternatively throw a more specific Nest exception, but to minimize functional change we just reuse the existing error pathway).
   • Use resolved in fs.promises.access and fs.createReadStream.
3. In deleteFile:
   • Mirror the same pattern: always resolve against LOCAL_ROOT, check that the resolved path stays under LOCAL_ROOT, and then call fs.promises.unlink on the resolved path only if it passes.
   • Continue rejecting file values that start with / or http, as the current code already does.
4. No changes are required in FileController for this vulnerability, because centralizing validation in FileService (the sink) will address all the listed CodeQL flows, including variants 1a–1e, since they all funnel through getFile.

This approach addresses all 6 variants because any path or file string, regardless of which controller method it came from, is forced through the same normalization and containment check inside FileService.

---

In general, to fix this type of issue you must validate or constrain user-controlled paths before using them in filesystem operations. For local filesystem access, the common pattern is: define a fixed root directory for allowed files, resolve the user input relative to that root (normalizing .. segments), and then verify the resulting absolute path is still under the root. Only then should you call fs.access, fs.createReadStream, or fs.unlink. Requests for paths outside the root should be rejected.

The best targeted fix here is to add a single reusable helper method inside FileService that safely resolves and validates local file paths against a chosen root (e.g. process.cwd()), and then use that helper in both getFile and deleteFile for the non-HTTP, non-absolute branch. Specifically:

• Add a private method resolveAndValidateLocalPath(file: string): string in FileService that:
  • Takes the user-provided relative path.
  • Computes const root = process.cwd();
  • Computes const resolvedPath = path.resolve(root, file);
  • Optionally uses fs.realpathSync to resolve symlinks: const realPath = fs.realpathSync(resolvedPath);
  • Ensures realPath.startsWith(root + path.sep) or realPath === root. If the check fails, it throws an error (or BadRequestException / InternalServerErrorException as you prefer).
  • Returns the validated realPath.
• In getFile, replace file = path.resolve(process.cwd(), file); with file = this.resolveAndValidateLocalPath(file);.
• In deleteFile, replace file = path.resolve(process.cwd(), file); with file = this.resolveAndValidateLocalPath(file);.

This keeps existing functionality (still reading/writing under the working directory) but prevents user input from escaping that directory via .. or similar tricks. It also fixes all CodeQL variants focused on the filesystem sink, since they all ultimately flow into getFile and, for deletion, into deleteFile. No changes are required in file.controller.ts for this specific alert, because the controller already routes all these paths through FileService.getFile / deleteFile, and the dangerous behavior is in FileService.

---

In general, the fix is to constrain file operations to a defined safe root directory and verify that any path derived from user input, once normalized, still resides under that root. This is typically done by defining a base directory (e.g., a storage or uploads directory), combining it with the user input using path.resolve, and then checking that the resolved absolute path begins with the base directory path. If the check fails, the operation should be rejected.

For this specific code, the best minimal change is to:

1. Define a constant ROOT_DIR in FileService representing the directory under which deletions are allowed (for example, process.cwd() or a subdirectory of it).
2. When deleting, resolve the user-provided file against ROOT_DIR instead of directly against process.cwd().
3. After resolving, verify that the resulting absolute path starts with ROOT_DIR + path.sep (or exactly equals ROOT_DIR), and throw an error if it does not.
4. Call fs.promises.unlink only on the validated resolved path.

Concretely, in src/file/file.service.ts, add a private readonly ROOT_DIR field (initialized to process.cwd() to preserve behavior while making the constraint explicit). Then, in deleteFile, replace file = path.resolve(process.cwd(), file); with construction of const resolvedPath = path.resolve(this.ROOT_DIR, file);, followed by a check that resolvedPath is inside this.ROOT_DIR. Use resolvedPath in the unlink call instead of the original file. No additional external dependencies are required; the existing path and fs imports are sufficient.

In general, to fix this kind of issue you must ensure the replacement string is actually different from the matched substring and does what the sanitization is supposed to do (e.g., escape, remove, or encode the character). Using \' inside a double-quoted string does not change a single quote; if we want an escaped single quote, we must represent it in a way that is distinguishable in the resulting string (for example, prefixing it with a backslash, or encoding it as an HTML or XML entity).

The minimal fix that preserves existing behavior as much as possible while making sanitizeInput actually sanitize is to escape single quotes by prefixing them with a backslash. In JavaScript/TypeScript source, that means the replacement string should be "\\'": the first backslash escapes the second in the source, and the resulting runtime string is \'. Thus, ' becomes \' in the returned string, which is a common form of escaping for many contexts. Concretely, in src/partners/partners.service.ts, update line 95 from input.replace(/'/g, "\'") to input.replace(/'/g, "\\'"). No additional imports or helper methods are required.