Skip to content

Conversation

@plexoos
Copy link
Member

@plexoos plexoos commented Nov 26, 2025

No description provided.

Copilot AI review requested due to automatic review settings December 3, 2025 20:37
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR parameterizes Docker builds by introducing build arguments for OS, CUDA version, and component versions, enabling the CI pipeline to build multiple configurations through an extended GitHub Actions matrix strategy.

Key Changes:

  • Dockerfile now accepts build arguments (OS, CUDA_VERSION, OPTIX_VERSION, GEANT4_VERSION, CMAKE_VERSION) for flexible image builds
  • GitHub workflow expands from single "release/develop" targets to a matrix of 36 combinations (3 OS × 4 CUDA versions × other parameters)
  • Base image changed from nvcr.io/nvidia/cuda to nvidia/cuda registry

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.

File Description
Dockerfile Parameterized OS and component versions using ARG directives; changed base image registry and converted DEBIAN_FRONTEND to ENV
.github/workflows/build-pull-request.yaml Extended build matrix to test multiple OS/CUDA combinations with exclude/include rules; added build-args passthrough

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

cuda_version: [11.8.0, 12.5.1, 12.9.1, 13.0.2]
optix_version: [7.7.0]
geant4_version: [11.3.2]
cmake_version: [3.22.1]
Copy link

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

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

The CMake version 3.22.1 in the workflow doesn't match the default version 4.1.2 specified in the Dockerfile. Since CMake 4.1.2 doesn't exist (CMake is currently in the 3.x series), this inconsistency needs to be resolved. Update the Dockerfile's default CMAKE_VERSION to match this valid version (3.22.1).

Copilot uses AI. Check for mistakes.
ARG CUDA_VERSION=12.4.0

ARG DEBIAN_FRONTEND=noninteractive
FROM nvidia/cuda:${CUDA_VERSION}-devel-${OS} AS base
Copy link

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

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

The base image has changed from nvcr.io/nvidia/cuda (NVIDIA's official container registry) to nvidia/cuda (Docker Hub). While both registries host NVIDIA CUDA images, this change may affect image availability, update frequency, or require authentication changes. Ensure this is intentional and that the nvidia/cuda repository on Docker Hub supports all the specified CUDA versions (11.8.0, 12.5.1, 12.9.1) and OS combinations (ubuntu22.04, ubuntu24.04, rockylinux9).

Suggested change
FROM nvidia/cuda:${CUDA_VERSION}-devel-${OS} AS base
FROM nvcr.io/nvidia/cuda:${CUDA_VERSION}-devel-${OS} AS base

Copilot uses AI. Check for mistakes.
Comment on lines 50 to 51
- os: [ubuntu22.04, rockylinux9]
cuda_version: 11.8.0
Copy link

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

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

The matrix exclude syntax is incorrect. The os and cuda_version fields should not be arrays in the exclude section. Each exclusion should be a separate item. For example:

exclude:
  - os: ubuntu22.04
    cuda_version: 11.8.0
  - os: rockylinux9
    cuda_version: 11.8.0

This will cause the workflow to fail or not exclude the intended combinations.

Suggested change
- os: [ubuntu22.04, rockylinux9]
cuda_version: 11.8.0
- os: ubuntu22.04
cuda_version: 11.8.0
- os: rockylinux9
cuda_version: 11.8.0

Copilot uses AI. Check for mistakes.
&& rm -rf /var/lib/apt/lists/*

RUN mkdir -p /opt/cmake/src && curl -sL https://github.com/Kitware/CMake/releases/download/v4.1.2/cmake-4.1.2.tar.gz | tar -xz --strip-components 1 -C /opt/cmake/src \
RUN mkdir -p /opt/cmake/src && curl -sL https://github.com/Kitware/CMake/releases/download/v${CMAKE_VERSION}/cmake-${CMAKE_VERSION}.tar.gz | tar -xz --strip-components 1 -C /opt/cmake/src \
Copy link

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

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

curl | tar downloads and extracts external archives (CMake, Geant4, plog, optix) over HTTPS without any signature or checksum verification. An attacker compromising DNS/CA or the upstream repo could serve a malicious tarball that gets executed during the build, resulting in a supply-chain compromise. Verify integrity before extraction, e.g., download detached signatures and verify with GPG or check a published SHA256 digest prior to tar -xz, for example:

curl -sL -o cmake.tar.gz https://github.com/Kitware/CMake/releases/download/v${CMAKE_VERSION}/cmake-${CMAKE_VERSION}.tar.gz
curl -sL -o cmake.tar.gz.sha256 https://github.com/Kitware/CMake/releases/download/v${CMAKE_VERSION}/cmake-${CMAKE_VERSION}.tar.gz.sha256
sha256sum -c cmake.tar.gz.sha256
tar -xz --strip-components 1 -C /opt/cmake/src -f cmake.tar.gz

Copilot uses AI. Check for mistakes.
Comment on lines 32 to 35
RUN mkdir -p /opt/geant4/src && curl -sL https://github.com/Geant4/geant4/archive/refs/tags/v${GEANT4_VERSION}.tar.gz | tar -xz --strip-components 1 -C /opt/geant4/src \
&& cmake -S /opt/geant4/src -B /opt/geant4/build -DGEANT4_USE_OPENGL_X11=ON -DGEANT4_USE_QT=ON -DGEANT4_USE_GDML=ON -DGEANT4_INSTALL_DATA=ON -DGEANT4_BUILD_MULTITHREADED=ON \
&& cmake --build /opt/geant4/build --parallel --target install \
&& rm -fr /opt/geant4
Copy link

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

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

curl | tar fetches and extracts Geant4 sources without verifying signatures or checksums. A malicious or tampered archive would be executed during the build (cmake --build), enabling a supply-chain attack. Mitigate by verifying a trusted SHA256 or GPG signature before extraction, then use tar -f on the verified file.

Suggested change
RUN mkdir -p /opt/geant4/src && curl -sL https://github.com/Geant4/geant4/archive/refs/tags/v${GEANT4_VERSION}.tar.gz | tar -xz --strip-components 1 -C /opt/geant4/src \
&& cmake -S /opt/geant4/src -B /opt/geant4/build -DGEANT4_USE_OPENGL_X11=ON -DGEANT4_USE_QT=ON -DGEANT4_USE_GDML=ON -DGEANT4_INSTALL_DATA=ON -DGEANT4_BUILD_MULTITHREADED=ON \
&& cmake --build /opt/geant4/build --parallel --target install \
&& rm -fr /opt/geant4
RUN set -eux; \
mkdir -p /opt/geant4/src; \
curl -sL -o /tmp/geant4.tar.gz https://github.com/Geant4/geant4/archive/refs/tags/v${GEANT4_VERSION}.tar.gz; \
curl -sL -o /tmp/geant4.tar.gz.sha256 https://github.com/Geant4/geant4/releases/download/v${GEANT4_VERSION}/SHA256SUMS; \
grep "geant4-${GEANT4_VERSION}.tar.gz" /tmp/geant4.tar.gz.sha256 | sha256sum -c --ignore-missing -; \
tar -xz --strip-components 1 -C /opt/geant4/src -f /tmp/geant4.tar.gz; \
rm /tmp/geant4.tar.gz /tmp/geant4.tar.gz.sha256; \
cmake -S /opt/geant4/src -B /opt/geant4/build -DGEANT4_USE_OPENGL_X11=ON -DGEANT4_USE_QT=ON -DGEANT4_USE_GDML=ON -DGEANT4_INSTALL_DATA=ON -DGEANT4_BUILD_MULTITHREADED=ON; \
cmake --build /opt/geant4/build --parallel --target install; \
rm -fr /opt/geant4

Copilot uses AI. Check for mistakes.
&& rm -fr /opt/plog

RUN mkdir -p /opt/optix && curl -sL https://github.com/NVIDIA/optix-dev/archive/refs/tags/v7.7.0.tar.gz | tar -xz --strip-components 1 -C /opt/optix
RUN mkdir -p /opt/optix && curl -sL https://github.com/NVIDIA/optix-dev/archive/refs/tags/v${OPTIX_VERSION}.tar.gz | tar -xz --strip-components 1 -C /opt/optix
Copy link

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

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

curl | tar downloads and extracts optix sources without any checksum or signature verification. A tampered archive could inject malicious code into the image during build. Fix by downloading to a file and validating a trusted SHA256 or GPG signature before extraction, then run tar -f on the verified file.

Copilot uses AI. Check for mistakes.
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