refactor signatures + add private docker registry#2041
refactor signatures + add private docker registry#2041
Conversation
|
/run-security-scan |
alexcos20
left a comment
There was a problem hiding this comment.
AI automated code review (Gemini 3).
Overall risk: medium
Summary:
This pull request significantly refactors signature generation across Aquarius and Provider services, centralizing command strings and signing logic. It also introduces support for ECIES encryption of sensitive data like Docker registry authentication for compute jobs. However, there are critical incomplete implementations and removals that need addressing.
Comments:
• [ERROR][bug] The getNodePublicKey method is called by initializeCompute when dockerRegistryAuth is provided (L595). However, it currently contains a // TODO: Implement this function comment. This means the Docker registry authentication feature will not function correctly, as the public key required for ECIES encryption will not be retrieved. This must be implemented or the PR should clearly state that dockerRegistryAuth is not fully functional until this is resolved.
• [WARNING][other] The computeDelete method has been entirely removed from the Provider class. This is a significant functional change. Could you please clarify if this functionality is no longer needed, has been moved, or if there's an alternative method for deleting compute jobs now?
• [INFO][other] The ci.yml file adds NODE_VERSION: pr-1207 as an environment variable for the start_ocean.sh script. Could you explain the purpose of this specific NODE_VERSION and if it's a temporary measure related to this PR, or a permanent addition for the CI setup?
• [INFO][other] sleep(100) has been added before generating and invalidating auth tokens in the integration tests. While this might resolve immediate test failures, it often indicates a potential race condition or timing issue in the underlying code. Could you elaborate on why this delay was necessary? Is there an underlying asynchronous operation that isn't being properly awaited, or is this expected behavior due to network/service startup?
• [INFO][style] The private getAuthorization helper function within Aquarius only returns the authToken if signerOrAuthToken is a string. Its name might be slightly misleading as it doesn't 'get' authorization in a general sense, but rather extracts an auth token if present. Consider renaming it to something more specific like getAuthTokenIfProvided to better reflect its singular purpose.
• [INFO][security] The addition of eciesjs for ECIES encryption is a valuable security enhancement. Could you confirm that this library has been reviewed for cryptographic best practices or if there are any known considerations regarding its use in a production environment?
• [INFO][other] The refactoring to use PROTOCOL_COMMANDS for signature messages and unifying the signRequest logic in SignatureUtils.ts is a significant improvement for consistency, maintainability, and security across the codebase. Great work on standardizing these patterns.
|
Refactor Signatures and Add Private Docker Registry Support
see oceanprotocol/ocean-node#1207 and oceanprotocol/ocean-node#1197
Summary
This PR refactors the signature generation mechanism across Provider service methods and adds support for private Docker registry authentication in compute jobs. The changes standardize how signatures are constructed and provide a secure way to pass Docker registry credentials to compute providers.
Changes
🔐 Signature Refactoring
getSignature()method to acceptnonceandcommandparameters instead of manually constructed signature messagesPROTOCOL_COMMANDSobject insrc/@types/Provider.tsto centralize all protocol command stringsencrypt()download()initializeCompute()computeStart()freeComputeStart()computeGetStreamableLogs()computeStop()computeGetResult()The new signature format follows the pattern:
address + nonce + command, providing better consistency and maintainability.🐳 Private Docker Registry Support
eciesjs(v0.4.5) for ECIES encryptionsrc/utils/eciesencrypt.tsto encrypt sensitive data using the provider's public keydockerRegistryAuthparameter toinitializeCompute()methoddockerRegistryAuthinterface insrc/@types/Compute.tswith fields:username?: stringpassword?: stringauth?: string🧹 Code Cleanup
computeDelete()method (previously unused/deprecated)🔧 CI Updates
NODE_VERSION: pr-1207environment variable to CI workflow for testing purposesDocker Registry Authentication Flow
dockerRegistryAuthobject with credentialsgetNodePublicKey()initializeComputerequest payload