Skip to content

Comments

refactor signatures + add private docker registry#2041

Open
alexcos20 wants to merge 6 commits intomainfrom
feature/refactor_signatures
Open

refactor signatures + add private docker registry#2041
alexcos20 wants to merge 6 commits intomainfrom
feature/refactor_signatures

Conversation

@alexcos20
Copy link
Member

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

  • Standardized signature generation: Refactored getSignature() method to accept nonce and command parameters instead of manually constructed signature messages
  • Protocol commands constants: Added PROTOCOL_COMMANDS object in src/@types/Provider.ts to centralize all protocol command strings
  • Updated signature calls: Refactored signature generation in the following methods:
    • encrypt()
    • 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

  • New dependency: Added eciesjs (v0.4.5) for ECIES encryption
  • ECIES encryption utility: Created src/utils/eciesencrypt.ts to encrypt sensitive data using the provider's public key
  • Docker registry auth parameter: Added optional dockerRegistryAuth parameter to initializeCompute() method
  • Type definitions: Added dockerRegistryAuth interface in src/@types/Compute.ts with fields:
    • username?: string
    • password?: string
    • auth?: string
  • Encryption implementation: Docker registry credentials are encrypted using ECIES encryption with the provider node's public key before being sent

🧹 Code Cleanup

  • Removed deprecated method: Deleted computeDelete() method (previously unused/deprecated)
  • Improved type safety: Better separation of concerns with protocol commands as constants

🔧 CI Updates

  • Added NODE_VERSION: pr-1207 environment variable to CI workflow for testing purposes

Docker Registry Authentication Flow

  1. User provides dockerRegistryAuth object with credentials
  2. Provider retrieves node public key via getNodePublicKey()
  3. Credentials are encrypted using ECIES encryption
  4. Encrypted credentials are included in the initializeCompute request payload

@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 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.

@alexcos20
Copy link
Member Author

[ERROR] bug ...

see oceanprotocol/ocean-node@b58383b

@alexcos20 alexcos20 marked this pull request as ready for review February 18, 2026 06:45
@alexcos20 alexcos20 linked an issue Feb 18, 2026 that may be closed by this pull request
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.

Refactor signature message logic

2 participants