Fix organization pickup and add DIANN 2.5.1#22
Conversation
🐛 pickup organization automatically + 2.5.1
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
📝 WalkthroughWalkthroughAdds DIANN 2.5.1: workflow path detection, CHG_251 build flag, DIANN matrix entry and jq selection support; updates diann-2.5.0 tagging to use repository owner; includes a new Dockerfile to build diann-2.5.1 image. ChangesDIA-NN 2.5.1 Build Support
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/quantms-containers.yml (1)
64-74:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMissing
CHG_251env mapping blocks DIANN 2.5.1 conditional builds.
--arg c251 "${CHG_251:-false}"(Line 98) depends onCHG_251, butCHG_251is never set in the step env. As a result,CHG_251always defaults tofalse, sodiann-2.5.1won’t be selected on push/pull_request changes.Proposed fix
- name: Build matrices id: set-matrix env: EVENT: ${{ github.event_name }} + CHG_251: ${{ steps.filter.outputs.diann_2_5_1 }} CHG_250: ${{ steps.filter.outputs.diann_2_5_0 }} CHG_232: ${{ steps.filter.outputs.diann_2_3_2 }} CHG_220: ${{ steps.filter.outputs.diann_2_2_0 }}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/quantms-containers.yml around lines 64 - 74, Add the missing CHG_251 environment mapping in the workflow env block so the DIANN 2.5.1 conditional can be honored; specifically, add a line like CHG_251: ${{ steps.filter.outputs.diann_2_5_1 }} to the env list (so the --arg c251 "${CHG_251:-false}" usage later will receive the correct value) ensuring the output key matches the filter step’s output name (diann_2_5_1).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In @.github/workflows/quantms-containers.yml:
- Around line 64-74: Add the missing CHG_251 environment mapping in the workflow
env block so the DIANN 2.5.1 conditional can be honored; specifically, add a
line like CHG_251: ${{ steps.filter.outputs.diann_2_5_1 }} to the env list (so
the --arg c251 "${CHG_251:-false}" usage later will receive the correct value)
ensuring the output key matches the filter step’s output name (diann_2_5_1).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c802784a-aaf0-4b6b-b6ca-b7c385bd5ddd
📒 Files selected for processing (1)
.github/workflows/quantms-containers.yml
it needs to be updated in many places...
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
diann-2.5.1/Dockerfile (1)
54-57:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRun the container as a non-root user.
diann-2.5.1/DockerfilesetsWORKDIR /data/but contains noUSERinstruction, so the container will run as the default user (typically root), weakening isolation.🛡️ Suggested hardening diff
-WORKDIR /data/ +RUN useradd --create-home --shell /bin/bash diann +WORKDIR /data/ +RUN chown -R diann:diann /data +USER diann🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@diann-2.5.1/Dockerfile` around lines 54 - 57, The Dockerfile currently sets WORKDIR /data/ but never switches from the default root user; create a non-root user and switch to it by adding user creation and a USER instruction (e.g., add a dedicated user/group, chown /data to that user, and then set USER) so the container runs as non-root; reference the existing WORKDIR /data/ to ensure ownership is set correctly before switching users and ensure any subsequent RUN/CMD steps are compatible with the new user.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@diann-2.5.1/Dockerfile`:
- Around line 39-41: The Dockerfile currently downloads and unzips
DIA-NN-2.5.1-Academia-Linux.zip in the RUN line (the wget/unzip/rm sequence)
without verifying integrity; update this step to verify the archive before
unzipping by fetching a pinned expected SHA256 (or GPG signature) from a trusted
source and comparing it to the downloaded file, failing the build on mismatch.
Concretely, modify the RUN that invokes wget/unzip/rm to (a) download the
expected checksum or signature from a trusted URL you control or upstream, (b)
compute and compare the SHA256 of DIA-NN-2.5.1-Academia-Linux.zip (or verify the
signature) and exit non‑zero if it doesn't match, and only then unzip and remove
the archive so that the sequence around wget/unzip/rm enforces integrity.
---
Outside diff comments:
In `@diann-2.5.1/Dockerfile`:
- Around line 54-57: The Dockerfile currently sets WORKDIR /data/ but never
switches from the default root user; create a non-root user and switch to it by
adding user creation and a USER instruction (e.g., add a dedicated user/group,
chown /data to that user, and then set USER) so the container runs as non-root;
reference the existing WORKDIR /data/ to ensure ownership is set correctly
before switching users and ensure any subsequent RUN/CMD steps are compatible
with the new user.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9a574750-26ae-42ea-aca4-fc12e9ea2d0f
📒 Files selected for processing (2)
.github/workflows/quantms-containers.ymldiann-2.5.1/Dockerfile
| RUN wget https://github.com/vdemichev/DiaNN/releases/download/2.0/DIA-NN-2.5.1-Academia-Linux.zip && \ | ||
| unzip DIA-NN-2.5.1-Academia-Linux.zip -d /usr/ && \ | ||
| rm DIA-NN-2.5.1-Academia-Linux.zip |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does the official DIA-NN 2.5.1 Linux release publish a SHA256 checksum for DIA-NN-2.5.1-Academia-Linux.zip? If yes, provide the checksum and source URL.
💡 Result:
No, the official DIA-NN release documentation and GitHub repository do not publish a SHA256 checksum for the DIA-NN-2.5.1-Academia-Linux.zip file [1][2][3]. The official source for DIA-NN releases is the GitHub repository, which lists files for download without providing accompanying checksum verification data [1].
Citations:
- 1: https://github.com/vdemichev/DiaNN/releases
- 2: http://www.github.com/vdemichev/DiaNN
- 3: https://github.com/vdemichev/diann
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show Dockerfile section around the download
sed -n '1,120p' diann-2.5.1/Dockerfile | nl -ba | sed -n '25,80p'
# Confirm whether a non-root USER is set anywhere in the file
rg -n --no-heading -S '^\s*USER\s+' diann-2.5.1/Dockerfile || true
rg -n --no-heading -S 'USER\s' diann-2.5.1/Dockerfile || true
# Compare with diann-2.5.0 if present
if [ -f diann-2.5.0/Dockerfile ]; then
echo "---- diann-2.5.0/Dockerfile (relevant section) ----"
sed -n '1,120p' diann-2.5.0/Dockerfile | nl -ba | sed -n '25,80p'
rg -n --no-heading -S '^\s*USER\s+' diann-2.5.0/Dockerfile || true
else
echo "---- diann-2.5.0/Dockerfile not found ----"
fiRepository: bigbio/quantms-containers
Length of output: 112
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== diann-2.5.1/Dockerfile existence =="
ls -la diann-2.5.1/Dockerfile
echo "== Show Dockerfile lines 30-70 with line numbers (via awk) =="
awk 'NR>=25 && NR<=75 {printf "%5d\t%s\n", NR, $0}' diann-2.5.1/Dockerfile
echo "== Search for USER directives in diann-2.5.1/Dockerfile =="
rg -n --no-heading '^\s*USER\s+' diann-2.5.1/Dockerfile || true
echo "== Search for useradd/groupadd/adduser in diann-2.5.1/Dockerfile =="
rg -n --no-heading '(useradd|groupadd|adduser|USER\s+\w+)' diann-2.5.1/Dockerfile || true
if [ -f diann-2.5.0/Dockerfile ]; then
echo "== diann-2.5.0/Dockerfile existence yes =="
awk 'NR>=25 && NR<=75 {printf "%5d\t%s\n", NR, $0}' diann-2.5.0/Dockerfile
echo "== Search for USER directives in diann-2.5.0/Dockerfile =="
rg -n --no-heading '^\s*USER\s+' diann-2.5.0/Dockerfile || true
else
echo "== diann-2.5.0/Dockerfile not found =="
fiRepository: bigbio/quantms-containers
Length of output: 2883
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== diann-2.5.1 Dockerfile: first 40 lines =="
awk 'NR<=40 {printf "%5d\t%s\n", NR, $0}' diann-2.5.1/Dockerfile
echo "== diann-2.5.1 Dockerfile: last 40 lines =="
wc -l diann-2.5.1/Dockerfile
awk '{ln=NR} NR> (FNR>1?0:0) { }' diann-2.5.1/Dockerfile >/dev/null 2>&1 || true
awk 'NR>=max(1,NR0)' diann-2.5.1/Dockerfile >/dev/null 2>&1 || true
lines=$(wc -l < diann-2.5.1/Dockerfile)
start=$((lines>40 ? lines-39 : 1))
awk -v s="$start" 'NR>=s {printf "%5d\t%s\n", NR, $0}' diann-2.5.1/Dockerfile
echo "== diann-2.5.0 Dockerfile: first 40 lines =="
awk 'NR<=40 {printf "%5d\t%s\n", NR, $0}' diann-2.5.0/Dockerfile
echo "== diann-2.5.0 Dockerfile: last 40 lines =="
lines0=$(wc -l < diann-2.5.0/Dockerfile)
start0=$((lines0>40 ? lines0-39 : 1))
awk -v s="$start0" 'NR>=s {printf "%5d\t%s\n", NR, $0}' diann-2.5.0/DockerfileRepository: bigbio/quantms-containers
Length of output: 6963
Add integrity verification for the downloaded DIA-NN archive.
Lines 39-41 download and unzip DIA-NN-2.5.1-Academia-Linux.zip without checksum/signature validation. The upstream GitHub release asset does not publish a SHA256 for this ZIP, so the build needs a pinned expected hash (or signature) from a trusted source.
🔒 Suggested hardening diff
# Download and install DIA-NN
-RUN wget https://github.com/vdemichev/DiaNN/releases/download/2.0/DIA-NN-2.5.1-Academia-Linux.zip && \
- unzip DIA-NN-2.5.1-Academia-Linux.zip -d /usr/ && \
- rm DIA-NN-2.5.1-Academia-Linux.zip
+ARG DIANN_URL="https://github.com/vdemichev/DiaNN/releases/download/2.0/DIA-NN-2.5.1-Academia-Linux.zip"
+ARG DIANN_SHA256="<official_sha256_here>"
+RUN wget "${DIANN_URL}" -O /tmp/diann.zip && \
+ echo "${DIANN_SHA256} /tmp/diann.zip" | sha256sum -c - && \
+ unzip /tmp/diann.zip -d /usr/ && \
+ rm /tmp/diann.zip🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@diann-2.5.1/Dockerfile` around lines 39 - 41, The Dockerfile currently
downloads and unzips DIA-NN-2.5.1-Academia-Linux.zip in the RUN line (the
wget/unzip/rm sequence) without verifying integrity; update this step to verify
the archive before unzipping by fetching a pinned expected SHA256 (or GPG
signature) from a trusted source and comparing it to the downloaded file,
failing the build on mismatch. Concretely, modify the RUN that invokes
wget/unzip/rm to (a) download the expected checksum or signature from a trusted
URL you control or upstream, (b) compute and compare the SHA256 of
DIA-NN-2.5.1-Academia-Linux.zip (or verify the signature) and exit non‑zero if
it doesn't match, and only then unzip and remove the archive so that the
sequence around wget/unzip/rm enforces integrity.
2 participants
This pull request updates the GitHub Actions workflow for container builds to add support for the new
diann-2.5.1version. The changes ensure that the workflow can detect changes to the new version, build its container, and handle its related tags and change tracking.Support for diann-2.5.1 version:
diann_2_5_1in the workflow to detect changes in thediann-2.5.1directory or workflow files, enabling conditional execution of build steps for this new version.DIANN_ALLJSON structure to include an entry fordiann-2.5.1, specifying its context, image tag, SIF file, extra tags, and change tracking variable. The latest tag is now associated withdiann-2.5.1.CHG_251change tracking variable, ensuring that changes todiann-2.5.1trigger the correct build logic in the workflow.Summary by CodeRabbit