Skip to content

secure docker#1291

Open
alexcos20 wants to merge 3 commits intomainfrom
feature/compute_engine_optimize
Open

secure docker#1291
alexcos20 wants to merge 3 commits intomainfrom
feature/compute_engine_optimize

Conversation

@alexcos20
Copy link
Member

@alexcos20 alexcos20 commented Mar 24, 2026

Summary

This PR strengthens C2D runtime isolation defaults

Why

  • Improve baseline container hardening for C2D algorithm execution.

Main changes

1) C2D Docker hardening defaults

Updated src/components/c2d/compute_engine_docker.ts:

  • Set HostConfig.NetworkMode = 'none' for C2D algorithm containers -> containers DO NOT HAVE NETWORK ACCESS
  • Enforce HostConfig.CapDrop = ['ALL'] by default.

2) Improve build process

  • Set a timeout of 5 minutes
  • Limit cpu & ram usage
  • Improve stream handling

@alexcos20
Copy link
Member Author

/run-security-scan

alexcos20

This comment was marked as outdated.

@alexcos20 alexcos20 marked this pull request as ready for review March 24, 2026 15:32
@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: low

Summary:
This pull request introduces significant improvements to the C2DEngineDocker component, primarily enhancing security, resource management, and reliability during Docker image build processes. Key changes include: enforcing NetworkMode: 'none' and dropping all container capabilities by default for enhanced security; applying resource limits (memory, CPU shares) to image builds to prevent resource exhaustion; implementing a 5-minute timeout for image builds using AbortController to improve robustness; and refining error handling and stream management for Docker build processes, including proper cleanup of timers and event listeners.

Comments:
• [INFO][security] Setting NetworkMode: 'none' for the container is an excellent security hardening measure. It ensures that the compute container does not have network access unless explicitly configured otherwise, significantly reducing the attack surface for untrusted code.
• [INFO][security] Defaulting CapDrop to ['ALL'] is a strong security practice, adhering to the principle of least privilege. This ensures that the container starts with no elevated capabilities, and only explicitly added capabilities (via advancedConfig.CapAdd) would be available. This is a significant improvement.
• [INFO][security] This loop correctly appends any additional CapDrop configurations from advancedConfig to the default ['ALL']. While adding capabilities to drop when ALL is already dropped might seem redundant, the logic ensures that ['ALL'] is always enforced as a baseline, which is a stronger security posture compared to the previous implementation where advancedConfig.CapDrop would override ['ALL'].
• [INFO][reliability] Introducing AbortController and a timeout for image builds is a great improvement for reliability. This prevents build processes from hanging indefinitely and consuming resources, enhancing the stability of the compute engine.
• [INFO][performance] Applying resource limits (memory, memswap, cpushares, cpuquota) directly to the Docker image build process is an excellent addition. This prevents a single build from monopolizing host resources, improving overall system stability and performance. The chosen values seem reasonable as a starting point, but could be configurable if specific builds require more or less.
• [INFO][security] Setting nocache: true during image builds is a strong security measure to prevent cache poisoning attacks. While it might slightly increase build times, it ensures that each build starts from a clean slate, enhancing the integrity and security of the resulting image. This is a good trade-off for security-sensitive environments.
• [INFO][style] Adding more robust checks (text && text.stream && typeof text.stream === 'string') before attempting to trim and append text.stream is a good defensive programming practice. It makes the log parsing more resilient to unexpected or malformed Docker build output.
• [INFO][reliability] The refactoring of the Promise-based build stream handling, including the settled flag, detachBuildLog, and finish helper, significantly improves the robustness and correctness of event listener management and prevents double-resolution/rejection issues. Handling both end and close events for HTTP streams is particularly good for reliability.
• [INFO][reliability] Explicitly checking for ABORT_ERR or AbortError in the catch block allows for specific handling of timeout-related failures, providing clearer error messages and better diagnostic capabilities for timed-out builds.
• [INFO][performance] Clearing the timer in a finally block is crucial for preventing memory leaks and ensuring that the timeout is always canceled, regardless of whether the build succeeded, failed, or was aborted. This is excellent resource management.

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