use privateRegistry per job#1197
use privateRegistry per job#1197alexcos20 wants to merge 6 commits intofeature/docker_registry_authfrom
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 introduces a new feature allowing users to provide per-job encrypted Docker registry authentication credentials for compute jobs. This enhances flexibility and security by enabling different users to utilize various private registries or specific credentials for their compute operations, overriding node-level configurations. The implementation involves:
- Adding
encryptedDockerRegistryAuthparameter to relevant API endpoints and internal command structures. - Updating the compute engine to decrypt, validate, and utilize these credentials for Docker image manifest retrieval and pulling.
- Storing the encrypted credentials in the
DBComputeJobrecord. - Extensive documentation of the feature in
docs/env.md, detailing encryption, schema, usage, and behavior.
Overall, the feature is well-designed, addressing security by encrypting sensitive information with ECIES and validating its format. The documentation is thorough and crucial for user adoption.
Comments:
• [INFO][style] The description for encryptedDockerRegistryAuth is Ecies encrypted docker auth schema for image. While technically correct, it could be slightly more descriptive for clarity, e.g., ECIES encrypted Docker registry authentication credentials (hex string) to better indicate its purpose and format at a glance.
• [INFO][other] The addition of this comprehensive documentation section for per-job Docker registry authentication is excellent. It clearly explains the 'why', 'how', and 'what' of the feature, which is critical for developers and users. The examples, encryption process, behavior, and error handling sections are particularly well-detailed and helpful.
• [INFO][security] Storing the encryptedDockerRegistryAuth directly in DBComputeJob is appropriate, ensuring that the credentials remain encrypted at rest and are only decrypted by the node when needed for image operations. This is a good security practice.
• [INFO][security] The validation logic for encryptedDockerRegistryAuth here, including decryption, JSON parsing, and schema validation using DockerRegistryAuthSchema.safeParse, is very robust. This prevents malformed or invalid credentials from being processed, protecting the system and providing clear error messages to the user.
• [INFO][security] Omitting encryptedDockerRegistryAuth from the ComputeJob object when converting from DBComputeJob is a good security measure, as these are sensitive credentials that should not be exposed in external API responses.
• [WARNING][bug] While existing tests are updated to pass null for encryptedDockerRegistryAuth, there are no new dedicated integration tests covering the encryptedDockerRegistryAuth functionality itself.
Given this is a security-sensitive feature involving encryption/decryption, validation, and conditional override of node-level settings, it's crucial to add tests for:
- Successful compute job start with valid
encryptedDockerRegistryAuth. - Failed compute job start with invalid (e.g., malformed, incorrectly encrypted)
encryptedDockerRegistryAuth. - Ensuring
encryptedDockerRegistryAuthtakes precedence over node-levelDOCKER_REGISTRY_AUTHSwhen both are present.
Without these, there's a risk of regressions or unexpected behavior not being caught.
|
/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 introduces a significant new feature allowing users to provide ECIES-encrypted Docker registry authentication credentials on a per-compute-job basis. This enhances flexibility and security by enabling compute jobs to pull images from private registries without relying solely on node-level configurations. The implementation includes client-side encryption, server-side decryption and robust validation using a Zod schema, and integration across the compute job lifecycle (initialization, starting, image pulling, and job record storage). Comprehensive documentation in docs/env.md and extensive integration tests are also included, which is excellent.
Comments:
• [INFO][style] The description for encryptedDockerRegistryAuth in the API docs is quite brief. While docs/env.md provides excellent detail, it would be helpful to briefly mention the expected decrypted JSON structure or directly refer to the docs/env.md for full details here, e.g., 'ECIES encrypted hex string of Docker auth JSON (see env.md for schema).'
• [WARNING][security] The decryption and JSON.parse logic for encryptedDockerRegistryAuth is duplicated in getDockerManifest, pullDockerImage, and the command handlers (initialize.ts, startCompute.ts). While correctly handled with try...catch in all places, consider refactoring this into a private utility method within C2DEngineDocker (or a more general utility) to reduce repetition and ensure consistent error handling and logging for decryption/parsing failures. This helps maintainability and security best practices.
• [INFO][other] Storing the encryptedDockerRegistryAuth directly in the DBComputeJob object means these encrypted credentials will persist in the database. While they are encrypted, it's a good practice to ensure that:
- Database access controls are sufficiently stringent.
- The lifecycle of these stored credentials is clear (e.g., how long they persist after job completion, if they are ever purged).
This is an architectural decision, and it seems intentional, but it's important to be aware of the implications for data retention and security policies.
• [INFO][security] The error messageInvalid encryptedDockerRegistryAuth: failed to parse JSON - ${error?.message || String(error)}is good for debugging but might leak too much internal detail iferror.messagecontains sensitive information or a lengthy stack trace from the underlying decryption/parsing library. For production, consider a slightly more generic message likeInvalid encryptedDockerRegistryAuth: decryption or JSON parsing failed.to prevent information disclosure, while still logging the full error internally.
• [INFO][security] Similar toinitialize.ts, the error message here could potentially leak internal details. Consider a more generic user-facing message while maintaining detailed internal logging.
• [INFO][other] The integration test suite forencryptedDockerRegistryAuthis exceptionally thorough, covering valid/invalid cases, different auth formats, and malformed input. This is commendable and significantly increases confidence in the feature's robustness and security. Good job!
|
#1198 should be merged here, before squash and merge |
Fixes #738
Private Docker Registries with Per-Job Authentication
In addition to node-level registry authentication via
DOCKER_REGISTRY_AUTHS, you can provide encrypted Docker registry authentication credentials on a per-job basis. This allows different users to use different private registries or credentials for their compute jobs.Overview
The
encryptedDockerRegistryAuthparameter allows you to securely provide Docker registry credentials that are:authstring ORusername+password)Encryption Format
The
encryptedDockerRegistryAuthmust be:Auth Schema Format:
The decrypted JSON must follow this structure:
{ "username": "myuser", "password": "mypassword" }OR
{ "auth": "base64-encoded-username:password" }OR (all fields present)
{ "username": "myuser", "password": "mypassword", "auth": "base64-encoded-username:password" }Validation Rules:
authstring must be provided (non-empty), ORusernameANDpasswordmust be provided (both non-empty)Usage Examples
1. Paid Compute Start (
POST /api/services/compute){ "command": "startCompute", "consumerAddress": "0x...", "signature": "...", "nonce": "123", "environment": "0x...", "algorithm": { "meta": { "container": { "image": "registry.example.com/myorg/myimage:latest" } } }, "datasets": [], "payment": { ... }, "encryptedDockerRegistryAuth": "0xdeadbeef..." // ECIES encrypted hex string }2. Free Compute Start (
POST /api/services/freeCompute){ "command": "freeStartCompute", "consumerAddress": "0x...", "signature": "...", "nonce": "123", "environment": "0x...", "algorithm": { "meta": { "container": { "image": "ghcr.io/myorg/myimage:latest" } } }, "datasets": [], "encryptedDockerRegistryAuth": "0xdeadbeef..." // ECIES encrypted hex string }3. Initialize Compute
The
initializecommand acceptsencryptedDockerRegistryAuthas part of the command payload, as it validates the image{ "command": "initialize", "datasets": [...], "algorithm": { "meta": { "container": { "image": "registry.gitlab.com/myorg/myimage:latest" } } }, "environment": "0x...", "payment": { ... }, "consumerAddress": "0x...", "maxJobDuration": 3600, "encryptedDockerRegistryAuth": "0xdeadbeef..." // ECIES encrypted hex string }Encryption Process
To create
encryptedDockerRegistryAuth, you need to:Prepare the auth JSON object:
{ "username": "myuser", "password": "mypassword" }Get the node's public key (available via the node's API or P2P interface)
Encrypt the JSON string using ECIES with the node's public key
Hex-encode the encrypted result
Behavior
encryptedDockerRegistryAuthis provided, it takes precedence over node-levelDOCKER_REGISTRY_AUTHSconfiguration for that specific jobError Handling
If
encryptedDockerRegistryAuthis invalid, you'll receive an error:Invalid encryptedDockerRegistryAuth: failed to parse JSON - [error message]Invalid encryptedDockerRegistryAuth: Either 'auth' must be provided, or both 'username' and 'password' must be providedNotes
The
encryptedDockerRegistryAuthparameter is optional. If not provided, the node will useDOCKER_REGISTRY_AUTHSconfiguration or attempt unauthenticated accessThe registry URL in the Docker image reference must match the registry you're authenticating to
For Docker Hub, use
registry-1.docker.ioas the registry URLCredentials are stored encrypted in the job record and decrypted only when needed for image operations