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

[bright-security-golf]

Note

Fixed 13 of 15 vulnerabilities.
Please review the fixes before merging.

Fix	Vulnerability	Endpoint	Affected Files	Resolution
✅	[Critical] SQL Injection	POST /graphql	src/products/products.resolver.ts, src/products/products.service.ts	Replaced dynamic SQL query with a parameterized query using MikroORM's query builder to prevent SQL injection.
✅	[Critical] XPATH Injection	GET /api/partners/partnerLogin	src/partners/partners.controller.ts, src/partners/partners.service.ts	Sanitize and validate user inputs before constructing XPath queries to prevent injection attacks.
✅	[Critical] XPATH Injection	GET /api/partners/searchPartners	src/partners/partners.service.ts	Added validation for XPath expressions to prevent injection attacks by checking for dangerous patterns.
✅	[Critical] Server Side Template Injection	POST /api/render	src/app.controller.ts	Escaped user input in the renderTemplate method to prevent Server Side Template Injection.
✅	[High] Server Side Request Forgery	GET /api/file/azure	src/file/file.service.ts	Added validation to ensure URL paths do not contain directory traversal sequences, preventing unauthorized access.
✅	[High] Server Side Request Forgery	GET /api/file/google	src/file/cloud.providers.metadata.ts	Restrict server-side requests to known cloud provider metadata URLs only.
✅	[High] Server Side Request Forgery	GET /api/file/digital_ocean	src/file/file.controller.ts, src/file/cloud.providers.metadata.ts	Implement URL validation to ensure only whitelisted base URLs are used for server-side requests.
✅	[High] Server Side Request Forgery	GET /api/file/aws	src/file/file.service.ts	Added URL validation and host whitelisting to prevent 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 email from the request token.
✅	[Medium] Secret Tokens Leak	GET /api/secrets	src/app.controller.ts	Replaced hardcoded secret tokens with environment variables to prevent leaks.
✅	[Medium] GraphQL Introspection	POST /graphql	src/app.module.ts	Disabled GraphQL introspection in the server configuration to prevent schema exposure.
✅	[Medium] GraphQL Introspection	GET /graphql	src/main.ts	Disable GraphQL introspection directly in the main server setup to ensure it is not bypassed.
✅	[Medium] GraphQL Introspection	POST /graphql	src/app.module.ts	Added a context function to the GraphQL module to block introspection queries.
❌	[Medium] [BL] Business Constraint Bypass	GET /api/products/latest	src/products/products.controller.ts, src/products/products.service.ts	Attempted fix: Ensure the maximum limit is enforced at both the controller and service levels, and log warnings when the requested limit exceeds the maximum allowed.

Workflow execution details

• ✅ Repository Analysis: TypeScript, NestJS
• ✅ Entrypoints Discovery: 61 entrypoints found
• ✅ Attack Vectors Identification
• ✅ E2E Security Tests Generation
• ✅ E2E Security Tests Execution: 14 vulnerabilities found
• ✅ Cleanup Irrelevant Test Files: 47 test files removed
• ✅ Applying Security Fixes: 14 fixes applied
• ✅ E2E Security Tests Execution: 3 vulnerabilities found
• ✅ Cleanup Irrelevant Test Files: 10 test files removed
• ✅ Applying Security Fixes: 3 fixes applied
• ✅ E2E Security Tests Execution: 1 vulnerabilities found
• ✅ Cleanup Irrelevant Test Files: 3 test files removed
• ✅ Applying Security Fixes: 1 fixes applied
• ✅ E2E Security Tests Execution: 1 vulnerabilities found
• ✅ Cleanup Irrelevant Test Files: 0 test files removed
• ✅ Applying Security Fixes: 1 fixes applied
• ✅ E2E Security Tests Execution: 1 vulnerabilities found
• ✅ Cleanup Irrelevant Test Files: 0 test files removed
• ✅ Applying Security Fixes: 1 fixes applied
• ✅ E2E Security Tests Execution: 1 vulnerabilities found
• ✅ Cleanup Irrelevant Test Files: 0 test files removed
• ⏭️ Applying Security Fixes: Skipped
• ⏭️ Workflow Wrap-Up: Skipped

In general, to fix this type of issue you must not treat arbitrary user input as a template to be compiled or evaluated. Instead, templates should be static (or at least come from trusted configuration), and user input should only be used as data passed into those templates, where the template engine can safely escape it.

The minimal fix here, without changing the public API shape more than necessary, is:

• Stop compiling user‑provided text as a doT template.
• If all you need is to echo or transform text, just return it (or safely escape it) without using dotT.compile.
• If you really need templating, use a static template string and interpolate user text as data through the template context.

Given the current method name renderTemplate and swagger docs describing “Write your text here / Rendered result”, the least intrusive change is to remove the call to dotT.compile and simply return the text (or a basic transformation), keeping logging behavior. Specifically, in src/app.controller.ts:

• Replace lines 72–80’s body so that, after trimming, you no longer call dotT.compile(escapedText)(). Instead, just return the (possibly sanitized) text, and ensure the function returns a string in all cases.
• Optionally, you can also simplify by removing the ineffective escapedText logic; but to minimize functional change, you can keep it and just not treat it as a template.

No additional imports or methods are needed for this minimal secure fix.

In general, to fix type confusion from HTTP parameters, ensure that any value coming from the request is checked to be of the expected runtime type (here, a string) before applying string methods or using it in security-sensitive logic. Reject or normalize non-string values explicitly.

For this code, the best minimal fix is:

1. Add a helper method to normalize the query parameter to a string, or reject non-string inputs. Since the code already uses a validation method, we can enhance isValidXPath to first verify the type.
2. Specifically, change isValidXPath so that it:
   • Returns false if xpath is not a string (e.g., is an array or any other type).
   • Only applies .includes when the type is confirmed as string.
3. Optionally, you can add a similar type guard in queryPartnersRaw before logging and before calling escapeXPathValue/includes, but because queryPartnersRaw already relies on isValidXPath for rejection, updating isValidXPath is sufficient without changing observable behavior (invalid inputs already result in an error).

Concretely:

• In src/partners/partners.controller.ts, modify the isValidXPath implementation (lines 163–167) to:

  private isValidXPath(xpath: string): boolean {
    if (typeof xpath !== 'string') {
      return false;
    }
    return !xpath.includes('//') && !xpath.includes("'");
  }

This ensures that when CodeQL follows tainted data into includes, the value is guaranteed to be a string, eliminating the type confusion risk while preserving existing behavior (non-string values simply cause the same error path as other invalid XPath expressions).

In general terms, fix the problem by ensuring that any user-controlled HTTP parameter that is assumed to be a string is either (a) checked at runtime to confirm it is a string (and rejected otherwise), or (b) safely coerced into a string in a well-defined way before using string methods on it. This prevents arrays (or other types) from being treated as strings in a way that could bypass validation or sanitization.

For this specific code, the best fix with minimal functional change is:

1. Add a small helper that takes an arbitrary value (potentially string, string[] or undefined) and either:
   • returns it as a string if it is a string,
   • rejects it (throws an error) if it is not a string.
2. Use this helper at the beginning of queryPartnersRaw and searchPartners to normalize/validate the query parameters before they are used.
3. Keep the existing XPath validation and escaping logic, but only run it on validated strings.

Concretely, within src/partners/partners.controller.ts:

• Add a private method like private ensureStringParam(param: unknown, name: string): string at the bottom of the controller (or near the other helpers). It should:
  • Check typeof param === 'string'.
  • If not, throw an HttpException with HttpStatus.BAD_REQUEST, indicating an invalid parameter type.
• In queryPartnersRaw, immediately convert the xpath parameter using this helper, e.g. const xpathStr = this.ensureStringParam(xpath, 'xpath'); and then use xpathStr instead of xpath in isValidXPath and in the call to partnersService.getPartnersProperties.
• In searchPartners, similarly normalize keyword with ensureStringParam and use the result for escapeXPathValue and logging.

This keeps behavior unchanged for valid string inputs, but safely rejects array or other non-string values instead of letting them slip through type confusion.

In general, to fix this kind of issue you must ensure that any value derived from an HTTP parameter is validated or normalized to the expected primitive type before using methods like includes, indexOf, or concatenation. For query parameters, that means checking typeof value === 'string' (and possibly Array.isArray) and either rejecting non-string input or converting it to a safe canonical string.

For this codebase, the minimal, non‑breaking fix is to harden the isValidXPath function in src/partners/partners.service.ts so that it only applies .includes when the argument is actually a string. If the value is not a string (e.g., an array), we should treat it as invalid and return false. This protects both the service’s internal validation and the controller’s this.isValidXPath(xpath) call (the controller snippet references such a method, and given the shared pattern, it's reasonable to use the same runtime checks there, but per your constraints I will only modify the shown code in partners.service.ts). Concretely, we will:

• Update isValidXPath(xpath: string): boolean in PartnersService to:
  • First check if (typeof xpath !== 'string') return false;.
  • Then perform the existing substring checks.
• This ensures that if the client sends ?xpath=a&xpath=b (so Nest gives an array), isValidXPath will safely return false instead of calling Array.prototype.includes with different semantics.

No additional methods or external libraries are required; we just add a simple runtime type guard in the existing function.

General fix: enforce that all sanitizer logic (here, isValidXPath) and any downstream uses treat the user-controlled xpath parameter as a string at runtime, rejecting or normalizing array values. That means adding explicit runtime type checks (or coercions) before calling string methods such as .includes, and ensuring the controller does not pass anything but a proper string into the service.

Best concrete fix with minimal behavior change:

1. In PartnersController.queryPartnersRaw, add a type check on the xpath query parameter to ensure it is a string; if it is an array or anything else, fail fast with a 400 Bad Request. This prevents array values from ever reaching the service.
2. In PartnersService.isValidXPath, add a typeof check to only operate on strings and explicitly reject non-string inputs. This directly addresses the CodeQL warning on line 96, and hardens the service even if other callers are added later.
3. Optionally (but still minimal), refine the parameter type of isValidXPath to string | string[] in TypeScript if desired; however, the safer and simpler approach here is to keep it as string and add a runtime guard. Since CodeQL’s complaint is about actual runtime behavior, the essential part is the runtime guard.

Concretely:

• In src/partners/partners.controller.ts, inside queryPartnersRaw, before logging and validation, check typeof xpath !== 'string' and throw an HttpException with HttpStatus.BAD_REQUEST if that’s the case.
• In src/partners/partners.service.ts, in isValidXPath, first check if (typeof xpath !== 'string') { return false; } (or throw), then perform the .includes checks. This ensures .includes is never called on an array or other non-string value.

No new methods or imports are required beyond the existing ones.

---