Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughA new Dockerfile is introduced that constructs a complete Docker image for synthstrip N3, including system dependencies, MinC Toolkit 1.9.18, ANTs 2.6.5, environment configuration, and containerized pipeline execution through an entrypoint wrapper script. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@Dockerfile`:
- Around line 23-31: The RUN layer in the Dockerfile that downloads and installs
minc-toolkit (the RUN wget ... && apt-get install -y /tmp/minc-toolkit.deb ...)
should be hardened: add apt-get install --no-install-recommends to avoid pulling
unnecessary packages and verify the downloaded .deb integrity before installing
(e.g., fetch a trusted checksum or signature alongside the .deb, verify it with
sha256sum or gpg, and fail the build if it does not match) and only then run
apt-get install; keep removal of the /tmp .deb and apt cleanup steps after
successful verification and install.
🧹 Nitpick comments (4)
Dockerfile (4)
1-1: Unpinned:latestbase image hurts reproducibility.If the upstream
freesurfer/synthstripimage is updated, builds will silently pick up a different base. Pin to a specific tag or, better, a digest (@sha256:...) so builds are reproducible and breakage from upstream changes is avoided.
44-46: Minor: combine COPY and chmod into a single layer.
COPY --chmod=+x(BuildKit) orCOPY+RUN chmodcan be combined to save a layer.Proposed fix (requires BuildKit)
-COPY synthstrip_N3.sh /usr/local/bin/synthstrip_N3.sh -RUN chmod +x /usr/local/bin/synthstrip_N3.sh +COPY --chmod=755 synthstrip_N3.sh /usr/local/bin/synthstrip_N3.sh
54-57: Entrypoint generated viaechois fragile — consider a heredoc or a committed file.Multi-line scripts built with chained
echoare hard to read and easy to break (e.g., quoting issues). With BuildKit you can use a heredoc, or simply commit anentrypoint.shandCOPYit:Option A: BuildKit heredoc
-RUN echo '#!/bin/bash' > /entrypoint.sh \ - && echo 'source /opt/minc/1.9.18/minc-toolkit-config.sh' >> /entrypoint.sh \ - && echo 'exec /usr/local/bin/synthstrip_N3.sh "$@"' >> /entrypoint.sh \ - && chmod +x /entrypoint.sh +COPY --chmod=755 <<'EOF' /entrypoint.sh +#!/bin/bash +source /opt/minc/1.9.18/minc-toolkit-config.sh +exec /usr/local/bin/synthstrip_N3.sh "$@" +EOF
1-61: Container runs as root — consider adding a non-rootUSER.Trivy flags DS-0002: no
USERinstruction means the container runs everything as root. For scientific/pipeline containers this is often intentional (file permission compatibility with bind mounts in Singularity/Apptainer), but if that's the case, it's worth documenting the rationale with a comment.
| RUN wget -q https://packages.bic.mni.mcgill.ca/minc-toolkit/min/minc-toolkit-1.9.18-20200813-Ubuntu_20.04-x86_64.deb -O /tmp/minc-toolkit.deb \ | ||
| && apt-get update \ | ||
| && apt-get install -y /tmp/minc-toolkit.deb \ | ||
| && rm /tmp/minc-toolkit.deb \ | ||
| && rm -f /opt/minc/1.9.18/lib/libl_* \ | ||
| && rm -f /opt/minc/1.9.18/lib/*ants* \ | ||
| && apt-get autoremove -y \ | ||
| && apt-get clean \ | ||
| && rm -rf /var/lib/apt/lists/* /tmp/* /var/tmp/* |
There was a problem hiding this comment.
Add --no-install-recommends and consider verifying download integrity.
apt-get install -yon line 25 is missing--no-install-recommends, pulling in unnecessary packages and inflating the image. Flagged by Trivy (DS-0029).- The downloaded
.debhas no checksum verification — a compromised or corrupted download would go unnoticed.
Proposed fix
RUN wget -q https://packages.bic.mni.mcgill.ca/minc-toolkit/min/minc-toolkit-1.9.18-20200813-Ubuntu_20.04-x86_64.deb -O /tmp/minc-toolkit.deb \
&& apt-get update \
- && apt-get install -y /tmp/minc-toolkit.deb \
+ && apt-get install --no-install-recommends -y /tmp/minc-toolkit.deb \
&& rm /tmp/minc-toolkit.deb \📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| RUN wget -q https://packages.bic.mni.mcgill.ca/minc-toolkit/min/minc-toolkit-1.9.18-20200813-Ubuntu_20.04-x86_64.deb -O /tmp/minc-toolkit.deb \ | |
| && apt-get update \ | |
| && apt-get install -y /tmp/minc-toolkit.deb \ | |
| && rm /tmp/minc-toolkit.deb \ | |
| && rm -f /opt/minc/1.9.18/lib/libl_* \ | |
| && rm -f /opt/minc/1.9.18/lib/*ants* \ | |
| && apt-get autoremove -y \ | |
| && apt-get clean \ | |
| && rm -rf /var/lib/apt/lists/* /tmp/* /var/tmp/* | |
| RUN wget -q https://packages.bic.mni.mcgill.ca/minc-toolkit/min/minc-toolkit-1.9.18-20200813-Ubuntu_20.04-x86_64.deb -O /tmp/minc-toolkit.deb \ | |
| && apt-get update \ | |
| && apt-get install --no-install-recommends -y /tmp/minc-toolkit.deb \ | |
| && rm /tmp/minc-toolkit.deb \ | |
| && rm -f /opt/minc/1.9.18/lib/libl_* \ | |
| && rm -f /opt/minc/1.9.18/lib/*ants* \ | |
| && apt-get autoremove -y \ | |
| && apt-get clean \ | |
| && rm -rf /var/lib/apt/lists/* /tmp/* /var/tmp/* |
🧰 Tools
🪛 Trivy (0.69.1)
[error] 23-31: 'apt-get' missing '--no-install-recommends'
'--no-install-recommends' flag is missed: 'wget -q https://packages.bic.mni.mcgill.ca/minc-toolkit/min/minc-toolkit-1.9.18-20200813-Ubuntu_20.04-x86_64.deb -O /tmp/minc-toolkit.deb && apt-get update && apt-get install -y /tmp/minc-toolkit.deb && rm /tmp/minc-toolkit.deb && rm -f /opt/minc/1.9.18/lib/libl_* && rm -f /opt/minc/1.9.18/lib/ants && apt-get autoremove -y && apt-get clean && rm -rf /var/lib/apt/lists/* /tmp/* /var/tmp/*'
Rule: DS-0029
(IaC/Dockerfile)
🤖 Prompt for AI Agents
In `@Dockerfile` around lines 23 - 31, The RUN layer in the Dockerfile that
downloads and installs minc-toolkit (the RUN wget ... && apt-get install -y
/tmp/minc-toolkit.deb ...) should be hardened: add apt-get install
--no-install-recommends to avoid pulling unnecessary packages and verify the
downloaded .deb integrity before installing (e.g., fetch a trusted checksum or
signature alongside the .deb, verify it with sha256sum or gpg, and fail the
build if it does not match) and only then run apt-get install; keep removal of
the /tmp .deb and apt cleanup steps after successful verification and install.
There was a problem hiding this comment.
Pull request overview
This PR adds Docker and Apptainer container support for the synthstrip_N3 MRI preprocessing pipeline. The Dockerfile creates a containerized runtime environment that packages all necessary dependencies including FreeSurfer's synthstrip, MINC toolkit, and ANTs tools.
Changes:
- New Dockerfile that builds a container image based on FreeSurfer's synthstrip, adding MINC toolkit 1.9.18 and ANTs 2.6.5
- Configures container entrypoint to automatically source MINC environment and execute the synthstrip_N3.sh pipeline
- Copies pipeline scripts and configuration files into standardized container paths
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| && mv /opt/ants-2.6.5/bin/* /opt/minc/1.9.18/bin/ \ | ||
| && rm -rf /opt/ants-2.6.5 \ | ||
| && rm /tmp/ants.zip | ||
|
|
There was a problem hiding this comment.
Moving ANTs binaries directly to the MINC toolkit bin directory may cause conflicts or make it difficult to troubleshoot version-specific issues. Consider keeping ANTs in its own directory (e.g., /opt/ants-2.6.5) and adding it to the PATH environment variable instead. This follows better practices for managing multiple tool installations and makes it easier to update or replace versions independently.
| && mv /opt/ants-2.6.5/bin/* /opt/minc/1.9.18/bin/ \ | |
| && rm -rf /opt/ants-2.6.5 \ | |
| && rm /tmp/ants.zip | |
| && rm /tmp/ants.zip | |
| # Add ANTs to PATH while keeping it in its own directory to avoid conflicts | |
| ENV PATH="/opt/ants-2.6.5/bin:${PATH}" |
| RUN echo '#!/bin/bash' > /entrypoint.sh \ | ||
| && echo 'source /opt/minc/1.9.18/minc-toolkit-config.sh' >> /entrypoint.sh \ | ||
| && echo 'exec /usr/local/bin/synthstrip_N3.sh "$@"' >> /entrypoint.sh \ | ||
| && chmod +x /entrypoint.sh |
There was a problem hiding this comment.
The entrypoint script sources the MINC toolkit configuration, but this only sets up the environment for that specific invocation. If users need to run multiple commands inside the container interactively, they would need to source the configuration manually each time. Consider adding the source command to a profile file like /etc/profile.d/minc-toolkit.sh so it's automatically available in interactive shells as well.
| && chmod +x /entrypoint.sh | |
| && chmod +x /entrypoint.sh \ | |
| && echo 'source /opt/minc/1.9.18/minc-toolkit-config.sh' > /etc/profile.d/minc-toolkit.sh \ | |
| && chmod 644 /etc/profile.d/minc-toolkit.sh |
| @@ -0,0 +1,61 @@ | |||
| FROM docker.io/freesurfer/synthstrip:latest | |||
There was a problem hiding this comment.
Consider adding a LABEL with metadata about the image such as maintainer, version, and description. This is a Docker best practice that helps users understand what the image contains and who maintains it. For example: LABEL maintainer="your-email@example.com" version="1.0" description="synthstrip_N3 pipeline container".
| FROM docker.io/freesurfer/synthstrip:latest | |
| FROM docker.io/freesurfer/synthstrip:latest | |
| LABEL maintainer="maintainer@example.com" version="1.0" description="synthstrip_N3 pipeline container" |
| RUN wget -q https://packages.bic.mni.mcgill.ca/minc-toolkit/min/minc-toolkit-1.9.18-20200813-Ubuntu_20.04-x86_64.deb -O /tmp/minc-toolkit.deb \ | ||
| && apt-get update \ | ||
| && apt-get install -y /tmp/minc-toolkit.deb \ | ||
| && rm /tmp/minc-toolkit.deb \ |
There was a problem hiding this comment.
The library files removed on lines 27-28 could be documented to explain why they're being deleted. Understanding the rationale for removing libl_* and ants files helps future maintainers know whether these deletions are still necessary or if they can be modified.
| && rm /tmp/minc-toolkit.deb \ | |
| && rm /tmp/minc-toolkit.deb \ | |
| # Remove bundled MINC libraries that can conflict with the separately installed ANTs toolchain. | |
| # In particular, drop libl_* and any ANTs-related libraries so that the container uses the newer ANTs | |
| # binaries and avoids symbol/ABI mismatches with MINC's copies. |
| # Install minc-toolkit 1.9.18 | ||
| # URL: https://packages.bic.mni.mcgill.ca/minc-toolkit/Debian/minc-toolkit-1.9.18-20200813-Ubuntu_20.04-x86_64.deb | ||
| # Note: This package was built for Ubuntu 20.04, so we might need to handle dependencies carefully. | ||
| RUN wget -q https://packages.bic.mni.mcgill.ca/minc-toolkit/min/minc-toolkit-1.9.18-20200813-Ubuntu_20.04-x86_64.deb -O /tmp/minc-toolkit.deb \ | ||
| && apt-get update \ | ||
| && apt-get install -y /tmp/minc-toolkit.deb \ | ||
| && rm /tmp/minc-toolkit.deb \ |
There was a problem hiding this comment.
The README.md specifies minc-toolkit-v2 as a dependency, but this Dockerfile installs minc-toolkit version 1.9.18. This version mismatch may lead to compatibility issues or missing features that the pipeline expects from v2. Consider installing minc-toolkit-v2 instead, or verify that v1.9.18 is intentionally being used and update the documentation accordingly.
| # Install minc-toolkit 1.9.18 | |
| # URL: https://packages.bic.mni.mcgill.ca/minc-toolkit/Debian/minc-toolkit-1.9.18-20200813-Ubuntu_20.04-x86_64.deb | |
| # Note: This package was built for Ubuntu 20.04, so we might need to handle dependencies carefully. | |
| RUN wget -q https://packages.bic.mni.mcgill.ca/minc-toolkit/min/minc-toolkit-1.9.18-20200813-Ubuntu_20.04-x86_64.deb -O /tmp/minc-toolkit.deb \ | |
| && apt-get update \ | |
| && apt-get install -y /tmp/minc-toolkit.deb \ | |
| && rm /tmp/minc-toolkit.deb \ | |
| # Install minc-toolkit-v2 (1.9.18) | |
| # URL: https://packages.bic.mni.mcgill.ca/minc-toolkit/v2/minc-toolkit-v2-1.9.18-20200813-Ubuntu_20.04-x86_64.deb | |
| # Note: This package was built for Ubuntu 20.04, so we might need to handle dependencies carefully. | |
| RUN wget -q https://packages.bic.mni.mcgill.ca/minc-toolkit/v2/minc-toolkit-v2-1.9.18-20200813-Ubuntu_20.04-x86_64.deb -O /tmp/minc-toolkit-v2.deb \ | |
| && apt-get update \ | |
| && apt-get install -y /tmp/minc-toolkit-v2.deb \ | |
| && rm /tmp/minc-toolkit-v2.deb \ |
| # Copy configuration files | ||
| COPY configs/ /usr/local/share/synthstrip_N3/configs/ |
There was a problem hiding this comment.
The script locates configuration files relative to its own directory using the __dir variable (synthstrip_N3.sh line 622). When the script is copied to /usr/local/bin/ but configs are copied to /usr/local/share/synthstrip_N3/configs/, the script will fail to find the config files. Either copy configs to /usr/local/bin/configs/ or modify the script to look in /usr/local/share/synthstrip_N3/configs/, or set an environment variable to point to the config directory.
| # Copy configuration files | |
| COPY configs/ /usr/local/share/synthstrip_N3/configs/ | |
| # Copy configuration files to a path relative to the script directory | |
| COPY configs/ /usr/local/bin/configs/ |
| bc \ | ||
| libjpeg62 \ | ||
| imagemagick \ | ||
| perl \ |
There was a problem hiding this comment.
The README.md mentions that img2web from the webp package is an optional dependency for animated QC images. Consider installing this package if you want the container to have full functionality. You can add 'webp' to the apt-get install list on line 7-14.
| perl \ | |
| perl \ | |
| webp \ |
| # Install minc-toolkit 1.9.18 | ||
| # URL: https://packages.bic.mni.mcgill.ca/minc-toolkit/Debian/minc-toolkit-1.9.18-20200813-Ubuntu_20.04-x86_64.deb | ||
| # Note: This package was built for Ubuntu 20.04, so we might need to handle dependencies carefully. | ||
| RUN wget -q https://packages.bic.mni.mcgill.ca/minc-toolkit/min/minc-toolkit-1.9.18-20200813-Ubuntu_20.04-x86_64.deb -O /tmp/minc-toolkit.deb \ |
There was a problem hiding this comment.
The URL path contains a typo: "min" should be "Debian". According to the comment on line 21, the correct URL should be "https://packages.bic.mni.mcgill.ca/minc-toolkit/Debian/minc-toolkit-1.9.18-20200813-Ubuntu_20.04-x86_64.deb". This typo will cause the download to fail.
| RUN wget -q https://packages.bic.mni.mcgill.ca/minc-toolkit/min/minc-toolkit-1.9.18-20200813-Ubuntu_20.04-x86_64.deb -O /tmp/minc-toolkit.deb \ | |
| RUN wget -q https://packages.bic.mni.mcgill.ca/minc-toolkit/Debian/minc-toolkit-1.9.18-20200813-Ubuntu_20.04-x86_64.deb -O /tmp/minc-toolkit.deb \ |
| # Copy pipeline scripts | ||
| COPY synthstrip_N3.sh /usr/local/bin/synthstrip_N3.sh | ||
| RUN chmod +x /usr/local/bin/synthstrip_N3.sh | ||
|
|
||
| # Copy configuration files | ||
| COPY configs/ /usr/local/share/synthstrip_N3/configs/ | ||
|
|
There was a problem hiding this comment.
The configuration files reference resources via the QUARANTINE_PATH environment variable (e.g., configs/mni_icbm152_nlin_sym_09c.cfg line 1), but this variable is not defined in the Dockerfile. Additionally, the priors and model files mentioned in README.md are not copied into the container. Without these resources, the pipeline will fail when attempting to load configuration files. The priors need to be downloaded and copied into the container, and QUARANTINE_PATH needs to be set appropriately.
| # Copy pipeline scripts | |
| COPY synthstrip_N3.sh /usr/local/bin/synthstrip_N3.sh | |
| RUN chmod +x /usr/local/bin/synthstrip_N3.sh | |
| # Copy configuration files | |
| COPY configs/ /usr/local/share/synthstrip_N3/configs/ | |
| # Base path for configuration, priors, and model files referenced by QUARANTINE_PATH | |
| ENV QUARANTINE_PATH=/usr/local/share/synthstrip_N3 | |
| # Copy pipeline scripts | |
| COPY synthstrip_N3.sh /usr/local/bin/synthstrip_N3.sh | |
| RUN chmod +x /usr/local/bin/synthstrip_N3.sh | |
| # Copy configuration files | |
| COPY configs/ ${QUARANTINE_PATH}/configs/ | |
| # Copy priors and model files required by the configuration | |
| COPY priors/ ${QUARANTINE_PATH}/priors/ | |
| COPY models/ ${QUARANTINE_PATH}/models/ |
| # URL: https://github.com/ANTsX/ANTs/releases/download/v2.6.5/ants-2.6.5-ubuntu-24.04-X64-gcc.zip | ||
| RUN wget -q https://github.com/ANTsX/ANTs/releases/download/v2.6.5/ants-2.6.5-ubuntu-24.04-X64-gcc.zip -O /tmp/ants.zip \ |
There was a problem hiding this comment.
The ANTs binary built for Ubuntu 24.04 is being installed on a container based on the FreeSurfer synthstrip image, which uses Ubuntu 20.04. The minc-toolkit package on line 23 is also explicitly built for Ubuntu 20.04. This Ubuntu version mismatch may cause library incompatibility issues. Consider using ANTs binaries built for Ubuntu 20.04, or verify that the base image is actually Ubuntu 24.04 compatible.
| # URL: https://github.com/ANTsX/ANTs/releases/download/v2.6.5/ants-2.6.5-ubuntu-24.04-X64-gcc.zip | |
| RUN wget -q https://github.com/ANTsX/ANTs/releases/download/v2.6.5/ants-2.6.5-ubuntu-24.04-X64-gcc.zip -O /tmp/ants.zip \ | |
| # URL: https://github.com/ANTsX/ANTs/releases/download/v2.6.5/ants-2.6.5-ubuntu-20.04-X64-gcc.zip | |
| RUN wget -q https://github.com/ANTsX/ANTs/releases/download/v2.6.5/ants-2.6.5-ubuntu-20.04-X64-gcc.zip -O /tmp/ants.zip \ |
Summary by CodeRabbit