feat: add error logging across services, fix ci script, and silence t… by maximedrn · Pull Request #5 · shreeed-app/multi-party-computation-controller-api

maximedrn · 2026-04-03T21:12:47Z

…est output

Copilot

Pull request overview

This PR introduces broader runtime logging and rate limiting across the NestJS API, adjusts metadata retrieval to use neverthrow result types, and updates CI/test configuration to reduce noisy output.

Changes:

Add request/result logging in controllers/services/processors and improve error logging around failures.
Add global rate limiting via @nestjs/throttler with a custom IP+Bearer-token tracker.
Update test/CI setup (silence Nest logger in Jest; add a combined ci script; update docs/build pipeline ordering).

Reviewed changes

Copilot reviewed 16 out of 17 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
test/jest.setup.ts	Silences NestJS logger output during Jest runs.
jest.config.ts	Registers Jest setup file.
src/tasks/signing/signing.controller.ts	Adds logging around enqueue requests/responses and documents 429.
src/tasks/key-generation/key-generation.dto.ts	Removes stray whitespace in DTO file.
src/tasks/key-generation/key-generation.controller.ts	Adds logging around enqueue requests/responses and documents 429.
src/queue/signing.processor.ts	Adapts to `ResultAsync` metadata retrieval and adds error logging.
src/queue/signing.processor.spec.ts	Updates mocks to return `okAsync(...)` for new `ResultAsync` API.
src/queue/key-generation.processor.ts	Adds error logging for failed gRPC key generation.
src/metadata/metadata.service.ts	Changes `retrieve()` to return `ResultAsync` and replaces `console.error` with Nest logger.
src/main.ts	Logs Swagger URL at startup in non-prod and adds additional bootstrap logging.
src/jobs/jobs.service.ts	Adds logging for unexpected BullMQ job shapes / invalid results.
src/jobs/jobs.controller.ts	Adds request/result debug logging and documents 429.
src/common/throttler/throttler.guard.ts	Introduces custom throttler tracker keyed by IP + Bearer token.
src/common/auth/bearer.guard.ts	Adds warning logs on auth rejection paths.
src/app.module.ts	Enables throttling globally with a custom APP_GUARD and adds ThrottlerModule config.
package.json	Adds `@nestjs/throttler` dependency and a `ci` script.
bun.lock	Locks `@nestjs/throttler` dependency.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-03T21:17:26Z

src/main.ts

+      "Bootstrap",
+    );
+    logger.log(
+      `Bearer token: ${configService.clientBearerToken}`,


Avoid logging the configured Bearer token to application logs, even in non-production environments. Logs are often shipped to shared systems and this effectively discloses credentials; prefer omitting it entirely or logging only that a token is configured (or a short fingerprint).

Suggested change

`Bearer token: ${configService.clientBearerToken}`,

`Bearer token configured: ${Boolean(configService.clientBearerToken)}`,

Copilot · 2026-04-03T21:17:27Z

src/tasks/signing/signing.controller.ts

+    this.logger.log(`POST /signing — ${JSON.stringify(dto)}`);
+
+    const result: SigningResponseDto = await this.signingService.enqueue(dto);
+
+    this.logger.log(`Signing job enqueued — ${JSON.stringify(result)}`);


This log statement serializes and emits the entire signing request DTO, which includes the message being signed. That payload can be sensitive and may be large; consider logging only non-sensitive identifiers (e.g., keyIdentifier, message length / hash) and using debug-level logging.

Copilot · 2026-04-03T21:17:27Z

src/tasks/key-generation/key-generation.controller.ts

+    this.logger.log(`POST /key-generation — ${JSON.stringify(dto)}`);
+
+    const result: KeyGenerationResponseDto =
+      await this.keyGenerationService.enqueue(dto);
+
+    this.logger.log(`Key generation job enqueued — ${JSON.stringify(result)}`);


This log statement serializes and emits the full key-generation request DTO. Even if these fields are not secrets, logging full request bodies by default increases log volume and risks accidental disclosure; consider logging only high-level fields (e.g., keyIdentifier, algorithm) and using debug-level logging.

Suggested change

this.logger.log(`POST /key-generation — ${JSON.stringify(dto)}`);

const result: KeyGenerationResponseDto =

await this.keyGenerationService.enqueue(dto);

this.logger.log(`Key generation job enqueued — ${JSON.stringify(result)}`);

this.logger.log("POST /key-generation request received");

const result: KeyGenerationResponseDto =

await this.keyGenerationService.enqueue(dto);

this.logger.log("Key generation job enqueued successfully");

Copilot · 2026-04-03T21:17:27Z

src/common/throttler/throttler.guard.ts

+  protected override async getTracker(request: Request): Promise<string> {
+    const ip: string = request.ip ?? request.socket?.remoteAddress ?? "";
+    const authHeader: string = request.headers?.[Header.AUTHORIZATION] ?? "";
+    const token: string = authHeader.startsWith(AuthScheme.BEARER_PREFIX)
+      ? authHeader.slice(7)
+      : "";
+    return `${ip}:${token}`;
+  }


The throttle key currently includes the raw Bearer token. This stores credentials in the throttler backend (typically Redis) and can leak tokens via key inspection/metrics. Prefer hashing the token (e.g., SHA-256) or using a non-reversible fingerprint before building the tracker key.

Copilot · 2026-04-03T21:17:27Z

src/common/throttler/throttler.guard.ts

+    const authHeader: string = request.headers?.[Header.AUTHORIZATION] ?? "";
+    const token: string = authHeader.startsWith(AuthScheme.BEARER_PREFIX)
+      ? authHeader.slice(7)
+      : "";


request.headers[authorization] can be string | string[] | undefined in Express. Assigning it to string and calling .startsWith() can mis-handle array values, and the Bearer prefix check is currently case-sensitive (unlike BearerGuard). Normalize the header value (handle arrays) and perform a case-insensitive scheme check before extracting the token.

Suggested change

const authHeader: string = request.headers?.[Header.AUTHORIZATION] ?? "";

const token: string = authHeader.startsWith(AuthScheme.BEARER_PREFIX)

? authHeader.slice(7)

: "";

const authHeaderValue = request.headers?.[Header.AUTHORIZATION];

const authHeader: string = Array.isArray(authHeaderValue)

? authHeaderValue[0] ?? ""

: authHeaderValue ?? "";

const bearerPrefix: string = AuthScheme.BEARER_PREFIX;

const token: string =

authHeader.slice(0, bearerPrefix.length).toLowerCase() ===

bearerPrefix.toLowerCase()

? authHeader.slice(bearerPrefix.length)

: "";

Copilot · 2026-04-03T21:17:28Z

src/metadata/metadata.service.ts

+    return ResultAsync.fromPromise(
+      this.redis.get(redisKey).then((serialized) => {
+        if (!serialized) return null;
+        return JSON.parse(serialized) as Metadata;
+      }),
+      (cause) => {
+        this.logger.error(
+          `Corrupted metadata for key identifier '${keyIdentifier}' — value cannot be parsed as JSON`,
+        );
+        return new Error(
+          `Corrupted metadata for '${keyIdentifier}'. ` +
+            `Delete and re-run key generation.`,
+          { cause },
+        );
+      },


The error mapping treats any failure from redis.get(...) (including Redis connectivity/timeouts) as "corrupted metadata" and returns an error instructing users to delete/re-run key generation. Consider distinguishing JSON parse failures from Redis I/O errors so operational incidents don’t surface as misleading corruption errors.

Copilot · 2026-04-03T21:17:28Z

src/jobs/jobs.controller.ts

+    const result: JobStatusResponse =
+      await this.jobsService.getJobStatus(jobId);
+
+    this.logger.debug(`Job status — ${JSON.stringify(result)}`);


This debug log serializes and emits the full job status response, which may include completed job results (e.g., signatures) and failure reasons. Consider redacting sensitive fields and/or logging only jobId/status to avoid leaking data and inflating logs.

Suggested change

this.logger.debug(`Job status — ${JSON.stringify(result)}`);

this.logger.debug(

`Job status retrieved — jobId=${jobId} status=${result.status}`,

);

Copilot · 2026-04-03T21:17:28Z

src/app.module.ts

    AppConfigModule,
+    ThrottlerModule.forRoot([
+      {
+        ttl: 60_000,
+        limit: 100,
+      },
+    ]),


Rate limiting is now enabled globally via ThrottlerModule + APP_GUARD, but there are no tests asserting 429 behavior (e.g., repeated requests from same IP/token). Since the repo already has e2e coverage for auth and endpoints, consider adding an e2e test to ensure throttling works and is keyed as intended.

feat: add error logging across services, fix ci script, and silence t…

1294ba0

…est output

Copilot AI review requested due to automatic review settings April 3, 2026 21:12

Copilot started reviewing on behalf of maximedrn April 3, 2026 21:13 View session

maximedrn merged commit 4049d4d into master Apr 3, 2026
6 checks passed

Copilot AI reviewed Apr 3, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add error logging across services, fix ci script, and silence t…#5

feat: add error logging across services, fix ci script, and silence t…#5
maximedrn merged 1 commit intomasterfrom
dev

maximedrn commented Apr 3, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 3, 2026

Uh oh!

Copilot AI Apr 3, 2026

Uh oh!

Copilot AI Apr 3, 2026

Uh oh!

Copilot AI Apr 3, 2026

Uh oh!

Copilot AI Apr 3, 2026

Uh oh!

Copilot AI Apr 3, 2026

Uh oh!

Copilot AI Apr 3, 2026

Uh oh!

Copilot AI Apr 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	`Bearer token: ${configService.clientBearerToken}`,
	`Bearer token configured: ${Boolean(configService.clientBearerToken)}`,

-    const authHeader: string = request.headers?.[Header.AUTHORIZATION] ?? "";
-    const token: string = authHeader.startsWith(AuthScheme.BEARER_PREFIX)
-      ? authHeader.slice(7)
-      : "";
+    const authHeaderValue = request.headers?.[Header.AUTHORIZATION];
+    const authHeader: string = Array.isArray(authHeaderValue)
+      ? authHeaderValue[0] ?? ""
+      : authHeaderValue ?? "";
+    const bearerPrefix: string = AuthScheme.BEARER_PREFIX;
+    const token: string =
+      authHeader.slice(0, bearerPrefix.length).toLowerCase() ===
+      bearerPrefix.toLowerCase()
+        ? authHeader.slice(bearerPrefix.length)
+        : "";

Conversation

maximedrn commented Apr 3, 2026

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 Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants