Feature/testing cors by alexjavabraz · Pull Request #454 · rsksmart/2wp-api

alexjavabraz · 2026-02-12T20:32:31Z

No description provided.

github-actions · 2026-02-12T20:32:56Z

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

OpenSSF Scorecard

Package	Version	Score	Details

Scanned Files

src/__tests__/unit/api-information.controller.unit.ts

Copilot

Pull request overview

This PR adds CORS configuration and a request security middleware to protect API endpoints with API key and payload hash validation. However, the implementation has several critical bugs that would prevent the security mechanism from working correctly, particularly around salt handling and test validation.

Changes:

Adds CORS configuration with hardcoded staging environment origin
Introduces request security middleware with API key and double-SHA256 payload hash validation
Updates path import to use modern node: prefix convention
Adds unused extraInfo field to ApiInformation model

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 11 comments.

Show a summary per file

File	Description
src/models/api-information.model.ts	Adds unused `extraInfo` field to the ApiInformation model
src/middleware/request-security.middleware.ts	Implements new security middleware with API key and payload hash validation, but has critical bugs with salt handling
src/application.ts	Configures CORS with hardcoded staging URL and registers security middleware; updates path import to node: prefix
src/tests/unit/api-information.controller.unit.ts	Contains formatting issues and unused imports; missing outer describe block
src/tests/acceptance/request-security.acceptance.ts	Adds acceptance tests but with incorrect hash calculations that don't match the middleware implementation

Comments suppressed due to low confidence (2)

src/tests/unit/api-information.controller.unit.ts:12

The 'mockResponse' variable is created but never used. Either remove this unused variable or implement its usage if it was intended for testing purposes.

            const controller = new ApiInformationController();

src/tests/unit/api-information.controller.unit.ts:12

Unused variable mockResponse.

            const controller = new ApiInformationController();

Copilot · 2026-02-12T20:37:54Z

src/middleware/request-security.middleware.ts

+
+export const requestSecurityMiddleware: Middleware = async ({request}, next) => {
+  const expectedApiKey = process.env[API_KEY_ENV_VAR];
+  const rootstockSalt = process.env[SALT_ENV_VAR];


The salt is retrieved from environment variables but not validated. If REQUEST_SALT is undefined, line 83 will concatenate the payload with the string "undefined" instead of failing gracefully. This is a security issue as it weakens the hash validation. Add validation similar to the API key check to ensure REQUEST_SALT is configured.

src/middleware/request-security.middleware.ts

src/application.ts

src/__tests__/unit/api-information.controller.unit.ts

Copilot · 2026-02-12T20:37:55Z

src/__tests__/unit/api-information.controller.unit.ts

-  describe('ApiInformationController (unit)', () => {
+

    describe('getApiInfo()',() => {


The test file is missing the outer 'describe' block that wraps the inner 'describe' for 'getApiInfo()'. Based on other test files in the codebase (like broadcast.controller.unit.ts), the pattern should include a top-level describe block for the controller name. This appears to be a formatting issue introduced by the changes.

src/models/api-information.model.ts

src/__tests__/acceptance/request-security.acceptance.ts

src/application.ts

src/__tests__/acceptance/request-security.acceptance.ts

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 10 comments.

Comments suppressed due to low confidence (1)

src/tests/unit/api-information.controller.unit.ts:20

The describe block wrapper for this test file has been removed. All test files in this codebase should have a describe block that groups related tests. This test file is missing the outer describe block (e.g., 'describe('ApiInformationController (unit)', () => { ... })'). This breaks the test file structure and makes it inconsistent with other unit tests in the codebase.

            const controller = new ApiInformationController();
            const apiInfo    = controller.getApiInformation();
            const version = process.env.npm_package_version;
            
            expect(apiInfo).to.containEql({version: version});
        });
    });

Copilot · 2026-02-12T21:03:31Z

src/application.ts

+      ...options,
+      rest: {
+        cors: {
+          origin: [corsOrigins],


The CORS origin configuration is incorrect. The value is wrapped in an extra array, creating a nested array structure like [[origin1, origin2]] instead of [origin1, origin2]. This will cause CORS to fail since the middleware expects an array of strings, not an array containing an array of strings. Remove the outer brackets wrapping corsOrigins.

Suggested change

origin: [corsOrigins],

origin: corsOrigins,

Copilot · 2026-02-12T21:03:31Z

src/middleware/request-security.middleware.ts

+  if (!providedApiKey) {
+    throw new HttpErrors.Unauthorized();
+  }
+
+  if (!secureStringCompare(providedApiKey, expectedApiKey)) {
+    throw new HttpErrors.Unauthorized();
+  }
+
+  const providedPayloadHash = request.header(PAYLOAD_HASH_HEADER);
+  if (!providedPayloadHash) {
+    throw new HttpErrors.Unauthorized();
+  }
+
+  const normalizedPayloadHash = providedPayloadHash.toLowerCase();
+  const payload = getRequestPayload(request);
+  const expectedPayloadHash = sha256(sha256(payload + rootstockSalt));
+
+  if (!secureStringCompare(normalizedPayloadHash, expectedPayloadHash)) {
+    throw new HttpErrors.Unauthorized();
+  }


The error messages thrown by HttpErrors.Unauthorized() do not match the expectations in the acceptance tests. The tests expect specific error messages like "Missing 'api_key' header" and "Invalid api key", but HttpErrors.Unauthorized() without arguments will use a generic "Unauthorized" message. Add custom error messages to each HttpErrors.Unauthorized() call to match the test expectations.

We're not going to generate specific messages to avoid explaining to potential attackers how to create the header object.

Copilot · 2026-02-12T21:03:32Z

src/middleware/request-security.middleware.ts

+  payload: Uint8Array;
+};
+
+const responseCache = new Map<string, CachedResponse>();


The response cache is implemented as a module-level Map that persists across all requests. This can lead to memory leaks and potential data exposure between different API keys or tenants. Consider implementing one of the following solutions: 1) Use a per-request cache scoped to the API key, 2) Implement proper cache invalidation based on time or size, 3) Use a dedicated caching library with TTL support. The current CACHE_MAX_ENTRIES limit of 1000 provides basic protection but doesn't prevent unbounded growth in long-running applications or address data isolation concerns.

Copilot · 2026-02-12T21:03:32Z

src/middleware/request-security.middleware.ts

+  if (responseCache.has(cacheKey)) {
+    responseCache.delete(cacheKey);
+  }
+
+  responseCache.set(cacheKey, cachedResponse);
+
+  if (responseCache.size > CACHE_MAX_ENTRIES) {
+    const firstKey = responseCache.keys().next().value;
+    if (firstKey) {
+      responseCache.delete(firstKey);
+    }
+  }


The cache eviction strategy only removes the first (oldest) entry when the cache exceeds CACHE_MAX_ENTRIES, but then immediately adds a new entry, potentially exceeding the limit again. This implementation allows the cache to grow to CACHE_MAX_ENTRIES + 1 before evicting. Consider evicting the oldest entry before adding the new one, or implement a proper LRU cache that maintains the size limit correctly.

Suggested change

if (responseCache.has(cacheKey)) {

responseCache.delete(cacheKey);

}

responseCache.set(cacheKey, cachedResponse);

if (responseCache.size > CACHE_MAX_ENTRIES) {

const firstKey = responseCache.keys().next().value;

if (firstKey) {

responseCache.delete(firstKey);

}

}

// Evict the oldest entry before adding a new one, ensuring the cache never

// grows beyond CACHE_MAX_ENTRIES, even transiently.

if (!responseCache.has(cacheKey) && responseCache.size >= CACHE_MAX_ENTRIES) {

const firstKey = responseCache.keys().next().value;

if (firstKey !== undefined) {

responseCache.delete(firstKey);

}

}

if (responseCache.has(cacheKey)) {

// Remove existing entry so that re-insertion updates its recency ordering.

responseCache.delete(cacheKey);

}

responseCache.set(cacheKey, cachedResponse);

Copilot · 2026-02-12T21:03:32Z

src/models/api-information.model.ts

+  extraInfo?: string;
 }


The extraInfo property is added to the ApiInformation model but is never used in the codebase. This appears to be an unintentional leftover from development. If this field is not needed, it should be removed.

Suggested change

extraInfo?: string;

}

}

Copilot · 2026-02-12T21:03:32Z

src/__tests__/unit/api-information.controller.unit.ts

  } from '@loopback/testlab';
-import { ApiInformationController } from '../../controllers/api-information.controller';
-
-  describe('ApiInformationController (unit)', () => {
+import { ApiInformationController } from '../../controllers/api-information.controller'; 

    describe('getApiInfo()',() => {


The import of createStubInstance and Response type are unused in this test file. These imports should be removed as they were likely added during development but not utilized.

Copilot · 2026-02-12T21:03:33Z

src/application.ts

+    const appOptions: ApplicationConfig = {
+      ...options,
+      rest: {
+        cors: {
+          origin: [corsOrigins],
+          methods: ['GET', 'POST'],
+          allowedHeaders: ['Content-Type', 'api_key', 'x-payload-hash'],
+          credentials: true,
+          maxAge: 86400,
+        },
+      },
+    };


The rest configuration is not properly merged with options.rest. If the caller provides options with a rest property, it will be completely overwritten instead of merged. This could cause unexpected behavior if the application is initialized with custom REST options. Consider using deep merging to properly combine the CORS configuration with any existing REST options from the caller.

Copilot · 2026-02-12T21:03:33Z

src/middleware/request-security.middleware.ts

+  const cacheKey = buildCacheKey(request, normalizedPayloadHash);
+  const cachedResponse = responseCache.get(cacheKey);
+  if (cachedResponse) {
+    response.setHeader('x-payload-hash-cache', 'HIT');
+    if (cachedResponse.type === 'result') {
+      return cachedResponse.result;
+    }
+
+    if (cachedResponse.contentType) {
+      response.setHeader('content-type', cachedResponse.contentType);
+    }
+    response.status(cachedResponse.statusCode).send(Buffer.from(cachedResponse.payload));
+    return undefined;


The cache hit check and cache write operations are not atomic, creating a potential race condition. If two identical requests arrive simultaneously, both could pass the cache check (line 153), execute the downstream handler, and both write to the cache. While this doesn't cause data corruption, it defeats the purpose of the cache for preventing duplicate processing. Consider using a promise-based locking mechanism or a cache library that handles concurrent access properly.

Copilot · 2026-02-12T21:03:33Z

src/__tests__/acceptance/request-security.acceptance.ts

+  before('setupApplication', async () => {
+    originalApiKey = process.env.REQUEST_API_KEY;
+    originalSalt = process.env.REQUEST_SALT;
+    process.env.REQUEST_API_KEY = apiKey;
+    process.env.REQUEST_SALT = requestSalt;
+
+    app = new TwpapiApplication();
+    await app.boot();
+    await app.start();
+
+    client = createClientForHandler(app.requestHandler);
+  });


The test relies on the environment variable CORS_ORIGIN not being set, or being set to an empty value, to use the default origin. However, if CORS_ORIGIN is set in the test environment, the application initialization could fail or use unexpected origins. Consider explicitly setting process.env.CORS_ORIGIN before initializing the app in the before hook to ensure consistent test behavior.

Copilot · 2026-02-12T21:03:33Z

src/middleware/request-security.middleware.ts

+  try {
+    result = await next();
+  } finally {
+    response.write = originalWrite;
+    response.end = originalEnd;
+  }


The response write and end methods are wrapped to capture response chunks, but if an error occurs during request processing (after line 198), the original methods are restored in the finally block. However, if response.write or response.end is called during error handling (e.g., by LoopBack's error handler), the chunks won't be captured. This could lead to incomplete cache entries for error responses. Consider whether error responses should be cached, and if so, ensure the wrapping remains in place during error handling.

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 14 comments.

Copilot · 2026-02-12T21:21:20Z

src/__tests__/acceptance/request-security.acceptance.ts

+      .expect(({body}) => {
+        expect(body.error.message).to.equal("Missing 'api_key' header");
+      });


The test expects the error message "Missing 'api_key' header" (line 69), but based on the previous feedback, the developer intentionally avoids providing specific error messages to attackers. The middleware at lines 130-131 in request-security.middleware.ts throws HttpErrors.Unauthorized() without a custom message, which will produce a generic "Unauthorized" message. Either update the test to expect a generic message, or reconcile this with the security decision documented in the previous feedback.

Copilot · 2026-02-12T21:21:21Z

src/application.ts

+    this.middleware(requestSecurityMiddleware, {
+      group: 'requestSecurity',
+      upstreamGroups: RestMiddlewareGroups.PARSE_PARAMS,
+      downstreamGroups: RestMiddlewareGroups.INVOKE_METHOD,
+    });


The request security middleware is registered globally without any route exclusions, which means it will apply to ALL endpoints including static files (line 58), the API explorer (lines 63-66), and other public endpoints. This requires all requests - including those to static assets and documentation - to provide valid api_key and x-payload-hash headers. Consider either: 1) applying the middleware only to specific routes that need protection, 2) adding a whitelist of paths to exclude from security checks, or 3) documenting that this is intentional if all endpoints should be protected.

Copilot · 2026-02-12T21:21:21Z

src/middleware/request-security.middleware.ts

+
+  const normalizedPayloadHash = providedPayloadHash.toLowerCase();
+  const payload = getRequestPayload(request);
+  const expectedPayloadHash = sha256(sha256(payload + rootstockSalt));


The payload hash is computed as sha256(sha256(payload + salt)), but the salt is simply concatenated to the payload string. If the payload can contain attacker-controlled content, this could potentially allow length extension attacks or other cryptographic issues. Consider using HMAC-SHA256 instead, which is specifically designed for message authentication with a secret key and is more resistant to such attacks.

Copilot · 2026-02-12T21:21:21Z

src/__tests__/acceptance/request-security.acceptance.ts

+  it('replays cached response for repeated /logs payload hash', async () => {
+    const body = {
+      type: 'error',
+      operation: 'peginNative',
+      location: 'frontend register',
+      error: {
+        message: 'wallet timeout',
+        code: 504,
+      },
+    };
+    const payloadHash = hashJsonPayload(body);
+
+    await client
+      .post('/logs')
+      .set('api_key', apiKey)
+      .set('x-payload-hash', payloadHash)
+      .send(body)
+      .expect(200);
+
+    await client
+      .post('/logs')
+      .set('api_key', apiKey)
+      .set('x-payload-hash', payloadHash)
+      .send(body)
+      .expect(200)
+      .expect('x-payload-hash-cache', 'HIT');
+  });


This test attempts to POST to '/logs' endpoint, but the logs.controller.ts has been deleted in this PR. The test will fail unless the logs controller is restored or this test is removed. The existing logs.controller.acceptance.ts file (not part of this PR) also contains 492 lines of tests for the /logs endpoint that will all fail.

Copilot · 2026-02-12T21:21:22Z

src/middleware/request-security.middleware.ts

+    appendChunk(chunk, args[0] as BufferEncoding | undefined);
+    return originalWrite(chunk as never, ...(args as never[]));
+  }) as typeof response.write;
+
+  response.end = ((chunk?: unknown, ...args: unknown[]) => {
+    appendChunk(chunk, args[0] as BufferEncoding | undefined);
+    return originalEnd(chunk as never, ...(args as never[]));


The middleware intercepts response.write and response.end to capture response chunks for caching, but doesn't handle error cases in the write/end callbacks. If originalWrite or originalEnd throw an error, the chunk will be captured but the error won't be propagated correctly. Consider wrapping the originalWrite and originalEnd calls in try-catch blocks or ensuring errors are properly handled.

Suggested change

appendChunk(chunk, args[0] as BufferEncoding | undefined);

return originalWrite(chunk as never, ...(args as never[]));

}) as typeof response.write;

response.end = ((chunk?: unknown, ...args: unknown[]) => {

appendChunk(chunk, args[0] as BufferEncoding | undefined);

return originalEnd(chunk as never, ...(args as never[]));

try {

const writeResult = originalWrite(chunk as never, ...(args as never[]));

appendChunk(chunk, args[0] as BufferEncoding | undefined);

return writeResult;

} catch (error) {

throw error;

}

}) as typeof response.write;

response.end = ((chunk?: unknown, ...args: unknown[]) => {

try {

const endResult = originalEnd(chunk as never, ...(args as never[]));

appendChunk(chunk, args[0] as BufferEncoding | undefined);

return endResult;

} catch (error) {

throw error;

}

Copilot · 2026-02-12T21:21:23Z

src/__tests__/unit/api-information.controller.unit.ts

-  describe('ApiInformationController (unit)', () => {
+import { ApiInformationController } from '../../controllers/api-information.controller'; 

    describe('getApiInfo()',() => {


The outer describe block for 'ApiInformationController (unit)' has been removed, but the nested describe block 'getApiInfo()' remains. This creates invalid test structure where the test cases are not properly wrapped in the controller-level describe block. The outer describe block should be retained.

Copilot · 2026-02-12T21:21:23Z

src/middleware/request-security.middleware.ts

+const storeCachedResponse = (cacheKey: string, cachedResponse: CachedResponse): void => {
+  if (responseCache.has(cacheKey)) {
+    responseCache.delete(cacheKey);
+  }
+
+  responseCache.set(cacheKey, cachedResponse);
+
+  if (responseCache.size > CACHE_MAX_ENTRIES) {
+    const firstKey = responseCache.keys().next().value;
+    if (firstKey) {
+      responseCache.delete(firstKey);
+    }
+  }
+};


The cache eviction strategy only removes a single entry when the cache exceeds CACHE_MAX_ENTRIES (1000). After adding an entry at line 71, if the size is 1001, only one entry is removed, leaving 1000 entries. This is correct. However, the cache will grow unbounded if entries are reinserted (moved to end) via the delete-then-set pattern at lines 67-71. Consider whether the LRU behavior intended here is working correctly, as the Map iteration order is insertion order, not access order.

Copilot · 2026-02-12T21:21:23Z

src/__tests__/acceptance/request-security.acceptance.ts

+      .set('x-payload-hash', requestHash(''))
+      .expect(401)
+      .expect(({body}) => {
+        expect(body.error.message).to.equal('Invalid api key');


The test expects the error message "Invalid api key" (line 80), but the middleware throws HttpErrors.Unauthorized() without a custom message (lines 134-136 in request-security.middleware.ts). This test will fail because the actual error message will be generic "Unauthorized". Either update the test expectations to match the generic error message, or add specific error messages to the middleware (though the previous feedback indicates this is intentionally avoided for security reasons).

Suggested change

expect(body.error.message).to.equal('Invalid api key');

expect(body.error.message).to.equal('Unauthorized');

Copilot · 2026-02-12T21:21:23Z

src/middleware/request-security.middleware.ts

+    throw new HttpErrors.InternalServerError();
+  }
+
+  if (!rootstockSalt) {
+    throw new HttpErrors.InternalServerError();


When the REQUEST_API_KEY or REQUEST_SALT environment variables are missing, the middleware throws a generic InternalServerError without any message. This makes debugging configuration issues difficult. While the previous feedback indicated that specific error messages should not be provided for security headers to avoid helping attackers, environment configuration errors are different - they occur at startup/initialization and should provide clear error messages to help developers identify misconfiguration.

Suggested change

throw new HttpErrors.InternalServerError();

}

if (!rootstockSalt) {

throw new HttpErrors.InternalServerError();

throw new HttpErrors.InternalServerError(

`Missing required environment variable: ${API_KEY_ENV_VAR}`,

);

}

if (!rootstockSalt) {

throw new HttpErrors.InternalServerError(

`Missing required environment variable: ${SALT_ENV_VAR}`,

);

Copilot · 2026-02-12T21:21:24Z

src/middleware/request-security.middleware.ts

+import {createHash, timingSafeEqual} from 'node:crypto';
+import {HttpErrors, Middleware, Request} from '@loopback/rest';
+
+const API_KEY_HEADER = 'api_key';
+const PAYLOAD_HASH_HEADER = 'x-payload-hash';
+const API_KEY_ENV_VAR = 'REQUEST_API_KEY';
+const SALT_ENV_VAR = 'REQUEST_SALT';
+const CACHE_MAX_ENTRIES = 1000;
+
+type CachedResponse = {
+  type: 'result';
+  result: unknown;
+} | {
+  type: 'raw';
+  statusCode: number;
+  contentType?: string;
+  payload: Uint8Array;
+};
+
+const responseCache = new Map<string, CachedResponse>();
+
+const isObject = (value: unknown): value is Record<string, unknown> =>
+  typeof value === 'object' && value !== null && !Array.isArray(value);
+
+const canonicalizeJson = (value: unknown): unknown => {
+  if (Array.isArray(value)) {
+    return value.map(item => canonicalizeJson(item));
+  }
+
+  if (!isObject(value)) {
+    return value;
+  }
+
+  const sortedEntries = Object.keys(value)
+    .sort((left, right) => left.localeCompare(right))
+    .map(key => [key, canonicalizeJson(value[key])] as const);
+
+  return Object.fromEntries(sortedEntries);
+};
+
+const getRequestPayload = (request: Request): string => {
+  const payload = request.body;
+
+  if (payload === undefined || payload === null) {
+    return '';
+  }
+
+  if (typeof payload === 'string') {
+    return payload;
+  }
+
+  if (Buffer.isBuffer(payload)) {
+    return payload.toString('utf8');
+  }
+
+  return JSON.stringify(canonicalizeJson(payload));
+};
+
+const sha256 = (value: string): string => createHash('sha256').update(value).digest('hex');
+
+const buildCacheKey = (request: Request, payloadHash: string): string => {
+  const requestUrl = request.originalUrl ?? request.url;
+  return `${request.method}:${requestUrl}:${payloadHash}`;
+};
+
+const storeCachedResponse = (cacheKey: string, cachedResponse: CachedResponse): void => {
+  if (responseCache.has(cacheKey)) {
+    responseCache.delete(cacheKey);
+  }
+
+  responseCache.set(cacheKey, cachedResponse);
+
+  if (responseCache.size > CACHE_MAX_ENTRIES) {
+    const firstKey = responseCache.keys().next().value;
+    if (firstKey) {
+      responseCache.delete(firstKey);
+    }
+  }
+};
+
+const getContentType = (headerValue: number | string | string[] | undefined): string | undefined => {
+  if (typeof headerValue === 'string') {
+    return headerValue;
+  }
+
+  if (Array.isArray(headerValue)) {
+    return headerValue[0];
+  }
+
+  return undefined;
+};
+
+const concatChunks = (chunks: Uint8Array[]): Uint8Array => {
+  const totalLength = chunks.reduce((sum, chunk) => sum + chunk.length, 0);
+  const merged = new Uint8Array(totalLength);
+  let offset = 0;
+
+  chunks.forEach(chunk => {
+    merged.set(chunk, offset);
+    offset += chunk.length;
+  });
+
+  return merged;
+};
+
+const secureStringCompare = (left: string, right: string): boolean => {
+  const leftBuffer = Buffer.from(left);
+  const rightBuffer = Buffer.from(right);
+
+  if (leftBuffer.length !== rightBuffer.length) {
+    return false;
+  }
+
+  return timingSafeEqual(new Uint8Array(leftBuffer), new Uint8Array(rightBuffer));
+};
+
+export const requestSecurityMiddleware: Middleware = async ({request, response}, next) => {
+  const expectedApiKey = process.env[API_KEY_ENV_VAR];
+  const rootstockSalt = process.env[SALT_ENV_VAR];
+
+  if (!expectedApiKey) {
+    throw new HttpErrors.InternalServerError();
+  }
+
+  if (!rootstockSalt) {
+    throw new HttpErrors.InternalServerError();
+  }
+
+  const providedApiKey = request.header(API_KEY_HEADER);
+  if (!providedApiKey) {
+    throw new HttpErrors.Unauthorized();
+  }
+
+  if (!secureStringCompare(providedApiKey, expectedApiKey)) {
+    throw new HttpErrors.Unauthorized();
+  }
+
+  const providedPayloadHash = request.header(PAYLOAD_HASH_HEADER);
+  if (!providedPayloadHash) {
+    throw new HttpErrors.Unauthorized();
+  }
+
+  const normalizedPayloadHash = providedPayloadHash.toLowerCase();
+  const payload = getRequestPayload(request);
+  const expectedPayloadHash = sha256(sha256(payload + rootstockSalt));
+
+  if (!secureStringCompare(normalizedPayloadHash, expectedPayloadHash)) {
+    throw new HttpErrors.Unauthorized();
+  }
+
+  const cacheKey = buildCacheKey(request, normalizedPayloadHash);
+  const cachedResponse = responseCache.get(cacheKey);
+  if (cachedResponse) {
+    response.setHeader('x-payload-hash-cache', 'HIT');
+    if (cachedResponse.type === 'result') {
+      return cachedResponse.result;
+    }
+
+    if (cachedResponse.contentType) {
+      response.setHeader('content-type', cachedResponse.contentType);
+    }
+    response.status(cachedResponse.statusCode).send(Buffer.from(cachedResponse.payload));
+    return undefined;
+  }
+
+  const responseChunks: Uint8Array[] = [];
+  const originalWrite = response.write.bind(response);
+  const originalEnd = response.end.bind(response);
+
+  const appendChunk = (chunk: unknown, encoding?: BufferEncoding): void => {
+    if (chunk === undefined || chunk === null) {
+      return;
+    }
+
+    if (Buffer.isBuffer(chunk)) {
+      responseChunks.push(new Uint8Array(chunk));
+      return;
+    }
+
+    if (typeof chunk === 'string') {
+      responseChunks.push(new Uint8Array(Buffer.from(chunk, encoding)));
+    }
+  };
+
+  response.write = ((chunk: unknown, ...args: unknown[]) => {
+    appendChunk(chunk, args[0] as BufferEncoding | undefined);
+    return originalWrite(chunk as never, ...(args as never[]));
+  }) as typeof response.write;
+
+  response.end = ((chunk?: unknown, ...args: unknown[]) => {
+    appendChunk(chunk, args[0] as BufferEncoding | undefined);
+    return originalEnd(chunk as never, ...(args as never[]));
+  }) as typeof response.end;
+
+  let result: unknown;
+
+  try {
+    result = await next();
+  } finally {
+    response.write = originalWrite;
+    response.end = originalEnd;
+  }
+
+  if (result === undefined) {
+    const contentType = getContentType(response.getHeader('content-type'));
+
+    storeCachedResponse(cacheKey, {
+      type: 'raw',
+      statusCode: response.statusCode,
+      contentType,
+      payload: concatChunks(responseChunks),
+    });
+
+    return result;
+  }
+
+  storeCachedResponse(cacheKey, {
+    type: 'result',
+    result,
+  });
+
+  return result;
+};


The new environment variables REQUEST_API_KEY, REQUEST_SALT, and CORS_ORIGIN are not documented in the ENV_VARIABLES.md file. These should be added to the environment variables documentation to help developers understand the required configuration.

returning extra info setting cors adding security validations Potential fix for code scanning alert no. 87: Unused variable, import, function or class Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> caching responses and removing error details adding cors configuration dynamically fixing unit tests removing logs collector

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 7 comments.

Copilot · 2026-02-12T23:50:54Z

src/application.ts

+    this.middleware(requestSecurityMiddleware, {
+      group: 'requestSecurity',
+      upstreamGroups: RestMiddlewareGroups.PARSE_PARAMS,
+      downstreamGroups: RestMiddlewareGroups.INVOKE_METHOD,
+    });
+


The middleware is registered unconditionally, but it hard-fails with 500 when REQUEST_API_KEY/REQUEST_SALT are not set. Existing acceptance tests and .env.test don’t define these env vars, so this change will break local/test runs unless every test bootstraps them. Consider enabling this middleware only when the env vars are present (or only in production), or ensure test/bootstrap configuration sets them globally.

Suggested change

this.middleware(requestSecurityMiddleware, {

group: 'requestSecurity',

upstreamGroups: RestMiddlewareGroups.PARSE_PARAMS,

downstreamGroups: RestMiddlewareGroups.INVOKE_METHOD,

});

const hasRequestSecurityEnv =

process.env.REQUEST_API_KEY && process.env.REQUEST_SALT;

if (hasRequestSecurityEnv) {

this.middleware(requestSecurityMiddleware, {

group: 'requestSecurity',

upstreamGroups: RestMiddlewareGroups.PARSE_PARAMS,

downstreamGroups: RestMiddlewareGroups.INVOKE_METHOD,

});

} else if (process.env.NODE_ENV === ENVIRONMENT_PRODUCTION) {

throw new Error(

'REQUEST_API_KEY and REQUEST_SALT environment variables must be set in production to enable request security middleware.',

);

}

Copilot · 2026-02-12T23:50:55Z

src/application.ts

+    if(corsOrigins.length === 0) {
+      throw new Error('CORS_ORIGIN environment variable is set but does not contain any valid origins');
+    }
+
+    if(corsOrigins.includes("*")) {


Lint/style: if(corsOrigins.length === 0) and if(corsOrigins.includes("*")) are likely to fail ESLint/Prettier (missing space after if, and double quotes in a single-quote codebase). Format as if (corsOrigins.length === 0) and use single quotes.

Suggested change

if(corsOrigins.length === 0) {

throw new Error('CORS_ORIGIN environment variable is set but does not contain any valid origins');

}

if(corsOrigins.includes("*")) {

if (corsOrigins.length === 0) {

throw new Error('CORS_ORIGIN environment variable is set but does not contain any valid origins');

}

if (corsOrigins.includes('*')) {

Copilot · 2026-02-12T23:50:55Z

src/middleware/request-security.middleware.ts

+  const cacheKey = buildCacheKey(request, normalizedPayloadHash);
+  const cachedResponse = responseCache.get(cacheKey);
+  if (cachedResponse) {
+    response.setHeader('x-payload-hash-cache', 'HIT');
+    if (cachedResponse.type === 'result') {


This enables response caching for every route keyed only by {method, url, payloadHash}. For dynamic endpoints (e.g., /health, /tx-status/..., /tx-history), repeat requests with the same payload hash will return stale cached data and bypass controller/service logic. Consider restricting caching to a safe allowlist, adding a TTL, or making caching opt-in.

Copilot · 2026-02-12T23:50:55Z

src/__tests__/acceptance/request-security.acceptance.ts

+      .expect(401)
+      .expect(({body}) => {
+        expect(body.error.message).to.equal("Missing 'api_key' header");
+      });


These assertions expect specific 401 error messages, but the middleware intentionally throws HttpErrors.Unauthorized() without details (so the default message will typically be just "Unauthorized"). Update the test to assert only the 401 status (or whatever generic message is returned).

Copilot · 2026-02-12T23:50:55Z

src/__tests__/acceptance/request-security.acceptance.ts

+      .set('x-payload-hash', requestHash(''))
+      .expect(401)
+      .expect(({body}) => {
+        expect(body.error.message).to.equal('Invalid api key');


Same as above: this test asserts a specific "Invalid api key" message, but the middleware does not return detailed auth failure reasons by design. The assertion should be relaxed to status-only (or generic message).

Suggested change

expect(body.error.message).to.equal('Invalid api key');

expect(body).to.have.property('error');

expect(body.error).to.have.property('message');

Copilot · 2026-02-12T23:50:56Z

src/__tests__/acceptance/request-security.acceptance.ts

+      .post('/logs')
+      .set('api_key', apiKey)
+      .set('x-payload-hash', payloadHash)
+      .send(body)
+      .expect(200);
+
+    await client
+      .post('/logs')


This test uses POST /logs, but logs.controller.ts is deleted in this PR and there is no /logs route under src/controllers/. This will 404/405 and won’t exercise caching. Either restore the /logs endpoint or change the test to use an existing POST route.

Suggested change

.post('/logs')

.set('api_key', apiKey)

.set('x-payload-hash', payloadHash)

.send(body)

.expect(200);

await client

.post('/logs')

.post('/api')

.set('api_key', apiKey)

.set('x-payload-hash', payloadHash)

.send(body)

.expect(200);

await client

.post('/api')

Copilot · 2026-02-12T23:50:56Z

src/controllers/api-information.controller.ts

+    // eslint-disable-next-line prefer-destructuring
    const version = packageJson.version;


Avoid disabling prefer-destructuring just to read packageJson.version; this reduces lint signal for minimal benefit. Prefer destructuring (const {version} = packageJson) or turn the rule off at the project level if it’s not useful.

Suggested change

// eslint-disable-next-line prefer-destructuring

const version = packageJson.version;

const {version} = packageJson;

sonarqubecloud · 2026-02-13T00:22:06Z

Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

alexjavabraz self-assigned this Feb 12, 2026

Copilot AI review requested due to automatic review settings February 12, 2026 20:32

Copilot started reviewing on behalf of alexjavabraz February 12, 2026 20:32 View session

github-advanced-security bot found potential problems Feb 12, 2026

View reviewed changes

src/__tests__/unit/api-information.controller.unit.ts Fixed Show fixed Hide fixed

Copilot AI reviewed Feb 12, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings February 12, 2026 20:58

Copilot started reviewing on behalf of alexjavabraz February 12, 2026 20:59 View session

Copilot AI reviewed Feb 12, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings February 12, 2026 21:14

Copilot started reviewing on behalf of alexjavabraz February 12, 2026 21:15 View session

Copilot AI reviewed Feb 12, 2026

View reviewed changes

alexjavabraz added 2 commits February 12, 2026 18:28

removinig unused value

01c634c

alexjavabraz force-pushed the feature/testing-cors branch from 309dea0 to 01c634c Compare February 12, 2026 21:29

adjusting adding url as part of the key for empty endpoints

72a99dd

Copilot AI review requested due to automatic review settings February 12, 2026 23:43

Copilot started reviewing on behalf of alexjavabraz February 12, 2026 23:44 View session

Copilot AI reviewed Feb 12, 2026

View reviewed changes

bumping version

008a0d6

		describe('ApiInformationController (unit)', () => {


		describe('getApiInfo()',() => {

	expect(body.error.message).to.equal('Invalid api key');
	expect(body.error.message).to.equal('Unauthorized');

	expect(body.error.message).to.equal('Invalid api key');
	expect(body).to.have.property('error');
	expect(body.error).to.have.property('message');

		// eslint-disable-next-line prefer-destructuring
		const version = packageJson.version;

	// eslint-disable-next-line prefer-destructuring
	const version = packageJson.version;
	const {version} = packageJson;

Conversation

alexjavabraz commented Feb 12, 2026

Uh oh!

github-actions bot commented Feb 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Dependency Review

OpenSSF Scorecard

Scanned Files

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

alexjavabraz Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 12, 2026

github-actions bot commented Feb 12, 2026 •

edited

Loading