feat: add endpoint to report false Blockaid scan results (COR-636) by Fbartoli · Pull Request #2815 · safe-global/safe-client-gateway

Fbartoli · 2025-12-01T11:41:57Z

Add POST /v1/chains/:chainId/security/:safeAddress/report-false-result endpoint
Support reporting FALSE_POSITIVE and FALSE_NEGATIVE scan results
Use request_id from original scan response to identify the transaction
Add reportTransaction method to BlockaidApi and ThreatAnalysisService
Handle errors gracefully with logging and success: false response
Add chain ID to Blockaid chain name mapping (for future use)
Add comprehensive unit tests
Add reading the request id from the header

src/modules/safe-shield/entities/dtos/report-false-result.dto.ts

src/modules/safe-shield/entities/dtos/threat-analysis.dto.ts

src/modules/safe-shield/entities/dtos/report-false-result.dto.ts

src/modules/safe-shield/safe-shield.service.ts

src/modules/safe-shield/threat-analysis/threat-analysis.service.ts

PooyaRaki · 2025-12-02T09:16:54Z

src/modules/safe-shield/threat-analysis/blockaid/blockaid-api.interface.ts

+ * Extended TransactionScanResponse that includes the request_id from x-request-id header.
+ */
+export type TransactionScanResponseWithRequestId = TransactionScanResponse & {
+  request_id: string | null;


I would make it string | undefined unless we explicitly need to send null to Blockaid.

fixed, not necessary to send null

PooyaRaki · 2025-12-02T09:18:39Z

src/modules/safe-shield/threat-analysis/blockaid/blockaid-api.interface.ts

+/**
+ * Extended TransactionScanResponse that includes the request_id from x-request-id header.
+ */
+export type TransactionScanResponseWithRequestId = TransactionScanResponse & {


Nit: Since it's in the interface file I would make it an interface.

maybe it would make sense to extract it to a separate entity or types file?

PooyaRaki · 2025-12-02T09:19:43Z

src/modules/safe-shield/safe-shield.service.spec.ts

+
+  describe('reportFalseResult', () => {
+    const mockReportRequest = {
+      event: 'FALSE_POSITIVE' as const,


Can we remove as const from all the parameters here?

PooyaRaki · 2025-12-02T09:22:10Z

src/modules/safe-shield/safe-shield.service.spec.ts

+
+      expect(result).toEqual({ success: false });
+      expect(mockLoggingService.error).toHaveBeenCalledWith(
+        `Failed to report false result for request_id ${mockReportRequest.request_id}: Blockaid API error`,


Nit: What if there’s an error on our end that prevents us from calling the Blockaid endpoint? In that case, the error message wouldn’t be entirely accurate.

error message updated

PooyaRaki · 2025-12-02T09:25:03Z

src/modules/safe-shield/threat-analysis/threat-analysis.service.spec.ts

      );
    });

+    it('should handle null request_id from header', async () => {


request_id is not null here it's undefined.

LucieFaire

looks great! Some small comments

LucieFaire · 2025-12-02T12:20:43Z

src/modules/safe-shield/threat-analysis/blockaid/blockaid-api.interface.ts

+/**
+ * Extended TransactionScanResponse that includes the request_id from x-request-id header.
+ */
+export type TransactionScanResponseWithRequestId = TransactionScanResponse & {


maybe it would make sense to extract it to a separate entity or types file?

src/modules/safe-shield/entities/dtos/report-false-result.dto.ts

src/modules/safe-shield/threat-analysis/threat-analysis.service.ts

src/modules/safe-shield/safe-shield.controller.ts

src/modules/safe-shield/threat-analysis/blockaid/blockaid-api.interface.ts

- Add POST /v1/chains/:chainId/security/:safeAddress/report-false-result endpoint - Support reporting FALSE_POSITIVE and FALSE_NEGATIVE scan results - Use request_id from original scan response to identify the transaction - Add reportTransaction method to BlockaidApi and ThreatAnalysisService - Handle errors gracefully with logging and success: false response - Add chain ID to Blockaid chain name mapping (for future use) - Add comprehensive unit tests

- Extract request_id from x-request-id header in Blockaid API response - Extend TransactionScanResponse with request_id field - Include request_id in ThreatAnalysisResponse for reporting false positives

- Refactor ReportEvent to follow ContractStatus pattern with const array - Add comprehensive documentation for enum types and DTOs - Explain 1000 char limit is CGW-imposed safeguard - Remove underscore prefixes from controller params - Add comment explaining params are for URL validation

…reatAnalysis services - Change request_id type from string | null to string | undefined in various services and DTOs. - Update logging messages to use 'warn' instead of 'error' for reporting failures in SafeShieldService. - Adjust unit tests to reflect changes in request_id handling and logging behavior. - Ensure consistency in request_id usage across the threat analysis module.

…ndefined request_id - Modify the mock response in the ThreatAnalysisService unit tests to set request_id as undefined instead of null, ensuring consistency with recent refactoring changes in request_id handling.

- Change the event parameter type in reportTransaction method across ThreatAnalysisService, IBlockaidApi, and BlockaidApi from string literals to ReportEvent type for improved type safety and consistency. - Add BLOCKAID_REQUEST_ID_HEADER constant for better readability and maintainability in Blockaid API service.

…2815) * feat: add endpoint to report false Blockaid scan results (COR-636) - Add POST /v1/chains/:chainId/security/:safeAddress/report-false-result endpoint - Support reporting FALSE_POSITIVE and FALSE_NEGATIVE scan results - Use request_id from original scan response to identify the transaction - Add reportTransaction method to BlockaidApi and ThreatAnalysisService - Handle errors gracefully with logging and success: false response - Add chain ID to Blockaid chain name mapping (for future use) - Add comprehensive unit tests * feat: add request_id to threat analysis response - Extract request_id from x-request-id header in Blockaid API response - Extend TransactionScanResponse with request_id field - Include request_id in ThreatAnalysisResponse for reporting false positives * refactor: address PR review comments for COR-636 - Refactor ReportEvent to follow ContractStatus pattern with const array - Add comprehensive documentation for enum types and DTOs - Explain 1000 char limit is CGW-imposed safeguard - Remove underscore prefixes from controller params - Add comment explaining params are for URL validation * refactor: update request_id handling and logging in SafeShield and ThreatAnalysis services - Change request_id type from string | null to string | undefined in various services and DTOs. - Update logging messages to use 'warn' instead of 'error' for reporting failures in SafeShieldService. - Adjust unit tests to reflect changes in request_id handling and logging behavior. - Ensure consistency in request_id usage across the threat analysis module. * test: update mock response in ThreatAnalysisService tests to handle undefined request_id - Modify the mock response in the ThreatAnalysisService unit tests to set request_id as undefined instead of null, ensuring consistency with recent refactoring changes in request_id handling. * refactor: update reportTransaction method to use ReportEvent type - Change the event parameter type in reportTransaction method across ThreatAnalysisService, IBlockaidApi, and BlockaidApi from string literals to ReportEvent type for improved type safety and consistency. - Add BLOCKAID_REQUEST_ID_HEADER constant for better readability and maintainability in Blockaid API service. * chore: trigger CI --------- Co-authored-by: SafeFB <fb@SafeFBs-MBP.fritz.box> Co-authored-by: SafeFB <fb@SafeFBs-MacBook-Pro.fritz.box>

Fbartoli self-assigned this Dec 1, 2025

Fbartoli requested a review from a team as a code owner December 1, 2025 11:41

Fbartoli force-pushed the feature/cor-636-report-false-blockaid-result branch from 713ddce to a38b991 Compare December 1, 2025 11:44

Fbartoli marked this pull request as draft December 1, 2025 11:55

Fbartoli marked this pull request as ready for review December 1, 2025 13:36

PooyaRaki reviewed Dec 2, 2025

View reviewed changes

LucieFaire reviewed Dec 2, 2025

View reviewed changes

LucieFaire reviewed Dec 3, 2025

View reviewed changes

src/modules/safe-shield/threat-analysis/blockaid/blockaid-api.interface.ts Outdated Show resolved Hide resolved

LucieFaire approved these changes Dec 3, 2025

View reviewed changes

Fbartoli enabled auto-merge (squash) December 3, 2025 13:04

Fbartoli mentioned this pull request Dec 3, 2025

feat: Add Report False Result feature for Blockaid threat analysis (COR-43) safe-global/safe-wallet-monorepo#6775

Closed

SafeFB and others added 7 commits December 3, 2025 15:34

feat: add request_id to threat analysis response

08b1545

- Extract request_id from x-request-id header in Blockaid API response - Extend TransactionScanResponse with request_id field - Include request_id in ThreatAnalysisResponse for reporting false positives

chore: trigger CI

ea89619

Fbartoli force-pushed the feature/cor-636-report-false-blockaid-result branch from a1ec9b0 to ea89619 Compare December 3, 2025 14:35

Fbartoli disabled auto-merge December 3, 2025 14:37

Fbartoli enabled auto-merge (squash) December 3, 2025 14:37

Fbartoli merged commit 55573d1 into main Dec 3, 2025
14 of 16 checks passed

Fbartoli deleted the feature/cor-636-report-false-blockaid-result branch December 3, 2025 14:42

This was referenced Dec 3, 2025

Revert "feat: add endpoint to report false Blockaid scan results (COR-636)" #2826

Merged

Revert "Revert "feat: add endpoint to report false Blockaid scan results (COR-636)"" #2827

Merged

Comments

Conversation

Fbartoli commented Dec 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LucieFaire left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Fbartoli commented Dec 1, 2025 •

edited

Loading