Skip to content

refactor fileinfo route & handler#1265

Merged
alexcos20 merged 1 commit intomainfrom
feature/refactor_fileinfo
Mar 17, 2026
Merged

refactor fileinfo route & handler#1265
alexcos20 merged 1 commit intomainfrom
feature/refactor_fileinfo

Conversation

@alexcos20
Copy link
Member

@alexcos20 alexcos20 commented Mar 17, 2026

Changes proposed in this PR:

  • refactor fileInfo httpRoute & handler to always use StorageObjects
  • move checks from httpRoute to handler
  • removed hardcoded values

@alexcos20
Copy link
Member Author

/run-security-scan

Copy link
Member Author

@alexcos20 alexcos20 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI automated code review (Gemini 3).

Overall risk: medium

Summary:
This pull request refactors the FileInfoCommand and EncryptFileCommand to use a unified StorageObject type, simplifying the internal representation of various file storage methods (URL, IPFS, Arweave, FTP). The FileInfoHttpRequest type has also been updated to be more flexible, allowing direct StorageObject input or did/serviceId based requests. A key change involves the relocation and modification of input validation logic for file info requests. Basic checks for did format and FileObjectType enum are now handled in the core FileInfoHandler. However, a more comprehensive validation function previously present in httpRoutes/fileInfo.ts has been removed, replaced by a much simpler check, leading to potential gaps in input validation at the API entry point.

Comments:
• [ERROR][security] The removal of the comprehensive validateFileInfoRequest function without fully replicating its checks in FileInfoHandler introduces a significant security and stability risk. For example, requests for type: 'ipfs' without a hash, or type: 'url' without a url, will now pass the initial HTTP validation (if (!hasType && !hasDid) is too permissive), potentially leading to errors or unexpected behavior downstream. It's crucial to ensure all necessary input validations are performed at the API entry point or the command handler to prevent malformed requests from being processed effectively.
• [WARNING][security] The new validation check if (!hasType && !hasDid) is insufficient. It only ensures that either type or did is present, but it does not validate the completeness of the StorageObject itself. For instance, if type: 'ipfs' is provided, a hash is still required. This check should either be expanded to cover all necessary StorageObject properties based on type, or the FileInfoHandler must be guaranteed to handle these missing parameters gracefully and securely (e.g., by consistently returning 400 Bad Request).
• [WARNING][security] While moving the did format and FileObjectType enum validation to the core handler is a good step towards centralization, the handler currently lacks detailed validation for the specific properties required by each FileObjectType. For example, it does not check for the presence of hash when type is 'ipfs', url for 'url' or 'ftp', or transactionId for 'arweave'. Without these checks, the handler might receive incomplete StorageObject instances, leading to downstream errors. Consider adding these specific property validations here to ensure robust input validation at the command processing level, regardless of the command's origin.
• [INFO][style] Changing FileInfoHttpRequest to a type alias using a union of StorageObject and a did/serviceId object is a good improvement. It clearly reflects the two primary ways a file info request can be structured and enhances type safety and flexibility in the application.

@alexcos20
Copy link
Member Author

[ERROR][security] The removal of the comprehensive validateFileInfoRequest function without fully replicating its checks in FileInfoHandler introduces a significant security and stability risk. For example, requests for type: 'ipfs' without a hash, or type: 'url' without a url, will now pass the initial HTTP validation (if (!hasType && !hasDid) is too permissive), potentially leading to errors or unexpected behavior downstream. It's crucial to ensure all necessary input validations are performed at the API entry point or the command handler to prevent malformed requests from being processed effectively.

Checks are now done in handler when calling const storage = Storage.getStorageClass(task.file, config). This performs all required security checks

Copy link
Collaborator

@AdriGeorge AdriGeorge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, tested and worked as expecting

@alexcos20 alexcos20 merged commit 0348651 into main Mar 17, 2026
25 of 27 checks passed
@alexcos20 alexcos20 deleted the feature/refactor_fileinfo branch March 17, 2026 12:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants