Skip to content
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions .github/workflows/conformity-tests.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
name: Conformity Tests

on:
push:
branches: [main, master]
pull_request:
branches: [main, master]
workflow_dispatch:

jobs:
conformity:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4

- uses: actions/setup-python@v5
with:
python-version: "3.11"
Comment on lines +14 to +18

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find workflow actions that are not pinned to a 40-char SHA
rg -nP '^\s*-\s*uses:\s*[^@]+@(?!(?:[0-9a-f]{40})\b).+' .github/workflows

Repository: ctglab/ENEO

Length of output: 345


🏁 Script executed:

cat -n .github/workflows/conformity-tests.yml

Repository: ctglab/ENEO

Length of output: 708


🌐 Web query:

GitHub Actions security best practices pinning commit SHA vs tags

💡 Result:

Pinning GitHub Actions to a full-length commit SHA is a critical security best practice, as it is the only way to ensure that an action is used as an immutable release [1][2]. Why Commit SHA Pinning is Recommended: - Immutability: Git tags and branches are mutable references that can be deleted, moved, or recreated by a repository owner or an attacker to point to different code [3][4][5]. Pinning to a full-length commit SHA guarantees that your workflow executes the exact code you expect every time, preventing "retargeting" attacks where malicious code is silently injected into a previously trusted version [3][6][5]. - Security Policy: GitHub now supports organization-level policies that can mandate full commit SHA pinning [1][7]. When enabled, any workflow referencing an action by a tag or branch will fail, forcing compliance across all repositories in an organization [3][6][7]. - Protection Against Compromise: If an action repository is compromised, pinning to a SHA ensures your pipeline continues to run the last known-good, reviewed code, rather than automatically adopting the attacker's latest malicious commit [4][6]. Best Practices for Implementation: - Use Inline Comments: When pinning to a SHA, include the original version tag as an inline comment (e.g., uses: actions/checkout@3df4ab1 # v4.0.0) [4][6][8]. This maintains human readability and allows automated dependency management tools to correlate the pin with semantic versions [4][9]. - Automate Updates: Manually maintaining SHAs is inefficient. Tools like Renovate (using presets such as helpers:pinGitHubActionDigestsToSemver) or Dependabot can be configured to manage these pins automatically, opening pull requests to update SHAs as new releases are published [4][6][8][10]. - Audit Existing Workflows: Use static analysis tools like CodeQL or Zizmor to identify unpinned actions in your repositories and audit them for migration to SHA pins [6][2]. While pinning to a tag is more convenient, it should only be done if you fully trust the creator and are aware of the risk that the tag's underlying code can change [1]. In professional or secure environments, moving away from tag-based references to SHA-pinned references is the standard for mitigating supply chain risks [7][5].

Citations:


Pin third-party actions to full commit SHAs and disable credential persistence.

Using floating tags (@v4, @v5) exposes the workflow to upstream tag drift and retargeting attacks where malicious code can be silently injected into a previously trusted version. Full commit SHA pinning is the only way to ensure the exact code is executed every time. Additionally, this job does not need persisted checkout credentials.

🔧 Suggested hardening patch
-      - uses: actions/checkout@v4
+      - uses: actions/checkout@<full-commit-sha> # v4
+        with:
+          persist-credentials: false

-      - uses: actions/setup-python@v5
+      - uses: actions/setup-python@<full-commit-sha> # v5
🧰 Tools
🪛 zizmor (1.25.2)

[warning] 14-14: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false

(artipacked)


[error] 14-14: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)

(unpinned-uses)


[error] 16-16: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)

(unpinned-uses)

🤖 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/conformity-tests.yml around lines 14 - 18, Replace the
floating version tags in the GitHub Actions with pinned full commit SHAs to
prevent tag drift and injection attacks. For the actions/checkout@v4 action, pin
it to its full commit SHA and add a configuration to disable credential
persistence by setting the token parameter appropriately. For the
actions/setup-python@v5 action, pin it to its full commit SHA. This ensures the
exact code is executed every time and reduces the attack surface for the
workflow.

Source: Linters/SAST tools


- name: Install test dependencies
run: pip install -r tests/requirements.txt

- name: Run conformity tests
run: pytest tests/ -v -m "not network"
3 changes: 2 additions & 1 deletion .test/common.smk
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,10 @@
import pandas as pd
import os
import glob
from pathlib import Path
from snakemake.utils import min_version

min_version("5.9.1")
min_version("8.0.0")


configfile: "config/config.yaml"
Expand Down
4 changes: 2 additions & 2 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@ RUN wget https://downloads.iedb.org/tools/mhci/3.1.6/IEDB_MHC_I-3.1.6.tar.gz &&
RUN micromamba install -n base -y \
-c bioconda -c conda-forge \
python=3.10 \
bedtools bcftools tabix samtools pip scipy pandas bionumpy cyvcf2 numpy toml pyyaml \
bedtools bcftools fastp tabix samtools pip scipy pandas bionumpy cyvcf2 numpy toml pyyaml \
&& micromamba clean --all --yes
# bind netmhcpan
ENV PATH="$MAMBA_ROOT_PREFIX/bin:$PATH:/opt/iedb/mhc_i/method/netmhcpan-4.1-executable/netmhcpan_4_1_executable/"
# Explicitly ensure the ARG is set for any subsequent RUN commands in this build stage
ARG MAMBA_DOCKERFILE_ACTIVATE=1
WORKDIR /opt
WORKDIR /opt
7 changes: 7 additions & 0 deletions config/config_main.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ datadirs:
bams: bams
expression: expression_data
index_folder: genome_index
qc_reports: qc_reports
logs:
align: log/align
annotate_variants: log/annotate_variants
Expand Down Expand Up @@ -39,6 +40,11 @@ params:
deepvariant:
threads: 4
extra: "split_skip_reads=true,channels=''"
fastp:
threads: 6
extra: "-q 20 -u 20 -l 50 -y 20 -x -g -3 -e 30 --detect_adapter_for_pe"
sortmerna:
threads: 8
gatk:
RAM: 20
extra:
Expand Down Expand Up @@ -93,6 +99,7 @@ params:
Frameshift: workflow/utils/vep_plugins/Frameshift.pm
Wildtype: workflow/utils/vep_plugins/Wildtype.pm
resources:
sortmerna_db: path/to/smr_v4.3_default_db.fasta
dbsnps: path/to/dbsnps_withAF.vcf.gz
deepvariant_rna_model: path/to/deepvariant_rna_model
genome: path/to/GRCh38_GIABv3_no_alt_analysis_set_maskedGRC_decoys_MAP2K3_KMT2C_KCNJ18.fasta
Expand Down
3 changes: 3 additions & 0 deletions pytest.ini
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
[pytest]
markers =
network: marks tests requiring network access (deselect with '-m "not network"')
33 changes: 30 additions & 3 deletions setup/download_res.py
Original file line number Diff line number Diff line change
Expand Up @@ -204,8 +204,7 @@ def create_sequence_dictionary(fasta_file):
"""
Create index and sequence dictionary for a FASTA file using samtools
"""
dict_file = f"{''.join(fasta_file.split('.')[:-1])}.dict"
fasta_file.replace(".fa", ".dict").replace(".fasta", ".dict")
dict_file = f"{''.join(fasta_file.split('.')[:-1])}.dict"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Use a suffix-safe path builder for the .dict filename.

Line [207] removes all dots from the full path (''.join(fasta_file.split('.')[:-1])), so names like GRCh38.primary.fa become GRCh38primary.dict. This can produce the wrong dictionary path.

Suggested fix
-    dict_file = f"{''.join(fasta_file.split('.')[:-1])}.dict"
+    dict_file = str(Path(fasta_file).with_suffix(".dict"))
🤖 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 `@setup/download_res.py` at line 207, The dict_file assignment removes all dots
from the fasta filename instead of just the final extension, causing files like
GRCh38.primary.fa to become GRCh38primary.dict instead of GRCh38.primary.dict.
Replace the problematic string split logic with a suffix-safe approach using
either pathlib.Path().stem or by splitting only the rightmost dot using
rsplit('.', 1), then append the .dict extension properly to preserve the full
path with all intermediate dots intact.

index_file = fasta_file + ".fai"
for file in [dict_file, index_file]:
if os.path.isfile(file):
Expand All @@ -229,6 +228,32 @@ def download_deepvariant_model_files(urls: list, outfolder: str):
return destpath


def download_sortmerna_db(url, keep_file, outfolder):
"""
Download SortMeRNA database, extract only the needed file, and cleanup.
"""
dest_file = os.path.join(outfolder, keep_file)
if os.path.isfile(dest_file):
logging.info(f"{keep_file} already exists. Skipping.")
return dest_file

tar_filename = url.split("/")[-1]
tar_path = os.path.join(outfolder, tar_filename)

logging.info(f"Downloading SortMeRNA database from {url}")
if not os.path.isfile(tar_path):
run_command(["wget", "-c", url, "-P", outfolder])

logging.info(f"Extracting {keep_file} from archive")
run_command(["tar", "-xzf", tar_path, "-C", outfolder, f"--wildcards", f"*/{keep_file}", "--strip-components=1"])

Comment on lines +247 to +249

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Confirm the offending f-string
rg -n 'f"--wildcards"' setup/download_res.py

Repository: ctglab/ENEO

Length of output: 176


🏁 Script executed:

#!/bin/bash
# Let's also check the actual line to see the full context
if [ -f setup/download_res.py ]; then
  sed -n '248,250p' setup/download_res.py
fi

Repository: ctglab/ENEO

Length of output: 230


Remove the unnecessary f-string to satisfy Ruff F541.

f"--wildcards" on line 249 has no placeholders and triggers a lint error. The adjacent f"*/{keep_file}" should remain since it contains a placeholder.

🧹 Minimal fix
-    run_command(["tar", "-xzf", tar_path, "-C", outfolder, f"--wildcards", f"*/{keep_file}", "--strip-components=1"])
+    run_command(["tar", "-xzf", tar_path, "-C", outfolder, "--wildcards", f"*/{keep_file}", "--strip-components=1"])
🧰 Tools
🪛 Ruff (0.15.2)

[error] 249-249: f-string without any placeholders

Remove extraneous f prefix

(F541)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@setup/download_res.py` around lines 248 - 250, The lint error F541 is caused
by an unnecessary f-string in the run_command call; update the arguments passed
to run_command (the call that includes ["tar", "-xzf", tar_path, "-C",
outfolder, f"--wildcards", f"*/{keep_file}", "--strip-components=1"]) by
replacing f"--wildcards" with a plain string "--wildcards" while keeping
f"*/{keep_file}" unchanged; this change should be made where the run_command
invocation is defined/used in download_res.py.

logging.info("Cleaning up archive")
if os.path.isfile(tar_path):
os.remove(tar_path)

return dest_file
Comment on lines +231 to +254

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Honor --dry-run and validate extraction output.

The new SortMeRNA downloader ignores dry-run and may update config even if extraction didn’t actually produce keep_file. Please pass args.dry_run into the helper, skip download/extract when dry, and verify the extracted file exists before returning.

🛠️ Suggested fix
-def download_sortmerna_db(url, keep_file, outfolder):
+def download_sortmerna_db(url, keep_file, outfolder, dry=False):
@@
-    logging.info(f"Downloading SortMeRNA database from {url}")
-    if not os.path.isfile(tar_path):
-        run_command(["wget", "-c", url, "-P", outfolder])
+    logging.info(f"Downloading SortMeRNA database from {url}")
+    if dry:
+        logging.info("Dry-run enabled; skipping download/extraction.")
+        return dest_file
+    if not os.path.isfile(tar_path):
+        run_command(["wget", "-c", url, "-P", outfolder])
@@
-    run_command(["tar", "-xzf", tar_path, "-C", outfolder, f"--wildcards", f"*/{keep_file}", "--strip-components=1"])
+    run_command(["tar", "-xzf", tar_path, "-C", outfolder, "--wildcards", f"*/{keep_file}", "--strip-components=1"])
+    if not os.path.isfile(dest_file):
+        raise FileNotFoundError(f"Expected {dest_file} after extraction")
@@
-        elif ftype == "sortmerna":
-            path = download_sortmerna_db(res_entry['url'], res_entry['keep_file'], outfolder)
+        elif ftype == "sortmerna":
+            path = download_sortmerna_db(res_entry['url'], res_entry['keep_file'], outfolder, args.dry_run)

Also applies to: 314-315

🧰 Tools
🪛 Ruff (0.15.2)

[error] 249-249: f-string without any placeholders

Remove extraneous f prefix

(F541)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@setup/download_res.py` around lines 232 - 255, The download_sortmerna_db
helper currently ignores dry-run and returns dest_file even if extraction
failed; update its signature to accept a dry_run flag (e.g., add parameter
dry_run or pass args.dry_run into download_sortmerna_db), branch early to skip
wget/tar when dry_run is True, and before returning ensure the expected file
(dest_file) actually exists on disk—if extraction failed, raise or return
None/raise an exception so callers don’t update config; also apply the same
dry-run propagation/validation to the other call site mentioned (lines ~314-315)
that invokes this helper.



def convert_REDI(bed_url, bed_output, drop_intermediate=True):
if os.path.isfile(bed_output):
logging.info(f"{bed_output} already exists.")
Expand Down Expand Up @@ -261,7 +286,7 @@ def main(args):
if name not in resources and not os.path.isfile(existing_path):
logging.error(f"{name} missing in resources and not in repo.")
continue
if os.path.isfile(existing_path):
if os.path.isfile(existing_path) or os.path.isdir(existing_path):
logging.info(f"{name} already exists. Skipping.")
continue
res_entry = resources.get(name)
Expand All @@ -285,6 +310,8 @@ def main(args):
path = decompress_file(download_resource(res_entry, outfolder, args.dry_run))
elif ftype == "model":
path = download_deepvariant_model_files(res_entry['url'], outfolder)
elif ftype == "sortmerna":
path = download_sortmerna_db(res_entry['url'], res_entry['keep_file'], outfolder)
else:
logging.warning(f"Unknown filetype for {name} as its {ftype}. Skipping.")
continue
Expand Down
5 changes: 5 additions & 0 deletions setup/resources.json
Original file line number Diff line number Diff line change
Expand Up @@ -47,5 +47,10 @@
"https://storage.googleapis.com/deepvariant/models/DeepVariant/1.4.0/DeepVariant-inception_v3-1.4.0+data-rnaseq_standard/model.ckpt.index",
"https://storage.googleapis.com/deepvariant/models/DeepVariant/1.4.0/DeepVariant-inception_v3-1.4.0+data-rnaseq_standard/model.ckpt.meta"
]
},
"sortmerna_db": {
"filetype": "sortmerna",
"url": "https://github.com/biocore/sortmerna/releases/download/v4.3.4/database.tar.gz",
"keep_file": "smr_v4.3_default_db.fasta"
}
}
Empty file added tests/__init__.py
Empty file.
27 changes: 27 additions & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import json
import pathlib

import pytest
import yaml

ROOT = pathlib.Path(__file__).parent.parent


@pytest.fixture(scope="session")
def root():
return ROOT


@pytest.fixture(scope="session")
def resources_json():
return json.loads((ROOT / "setup" / "resources.json").read_text())


@pytest.fixture(scope="session")
def config():
return yaml.safe_load((ROOT / "config" / "config_main.yaml").read_text())


@pytest.fixture(scope="session")
def rule_files():
return list((ROOT / "workflow" / "rules").glob("*.smk"))
3 changes: 3 additions & 0 deletions tests/requirements.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
pytest>=7.0
pyyaml>=6.0
requests>=2.28
66 changes: 66 additions & 0 deletions tests/test_config_conformity.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
"""
Validate config/config_main.yaml structure and cross-references with resources.json.
"""
import pathlib

import pytest
import yaml

ROOT = pathlib.Path(__file__).parent.parent

REQUIRED_TOP_LEVEL_KEYS = {
"OUTPUT_FOLDER", "TEMP_DIR", "datadirs", "params", "resources", "execution_mode"
}
VALID_EXECUTION_MODES = {"full", "CI"}
REQUIRED_PARAM_SECTIONS = {
"BQSR", "deepvariant", "fastp", "gatk", "MarkDuplicates", "pMHC",
"STAR", "SplitNCigarReads", "salmon", "samtools", "strelka2", "t1k", "vcfanno", "vep",
}


def test_config_parses():
data = yaml.safe_load((ROOT / "config" / "config_main.yaml").read_text())
assert isinstance(data, dict) and len(data) > 0


def test_required_top_level_keys(config):
missing = REQUIRED_TOP_LEVEL_KEYS - set(config.keys())
assert not missing, f"Missing top-level keys: {missing}"


def test_execution_mode_is_valid(config):
mode = config.get("execution_mode")
assert mode in VALID_EXECUTION_MODES, (
f"execution_mode '{mode}' is not one of {VALID_EXECUTION_MODES}"
)


def test_params_sections_present(config):
params = config.get("params", {})
missing = REQUIRED_PARAM_SECTIONS - set(params.keys())
assert not missing, f"Missing params sections: {missing}"


def test_all_downloadable_resources_in_config(config, resources_json):
config_resources = set(config.get("resources", {}).keys())
json_resources = set(resources_json.keys())
missing = json_resources - config_resources
assert not missing, (
f"Resources defined in resources.json but absent from config.resources: {missing}"
)


def test_datadirs_has_logs_section(config):
assert "logs" in config.get("datadirs", {}), "datadirs is missing 'logs' section"


def test_in_repo_resources_exist(config):
"""Resources whose paths start with 'workflow/' must exist on disk."""
resources = config.get("resources", {})
missing = []
for name, path in resources.items():
if isinstance(path, str) and path.startswith("workflow/"):
full_path = ROOT / path
if not full_path.exists():
missing.append(f"{name}: {path}")
assert not missing, "In-repo resources missing from disk:\n" + "\n".join(missing)
59 changes: 59 additions & 0 deletions tests/test_resources_manifest.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
"""
Validate setup/resources.json structure and completeness.
"""
import json
import pathlib

import pytest

ROOT = pathlib.Path(__file__).parent.parent
RESOURCES_PATH = ROOT / "setup" / "resources.json"

VALID_FILETYPES = {"vcf", "fasta", "gtf", "table", "archive", "model", "sortmerna"}


def test_resources_json_parses():
data = json.loads(RESOURCES_PATH.read_text())
assert isinstance(data, dict) and len(data) > 0


def test_each_entry_has_required_fields(resources_json):
for name, entry in resources_json.items():
assert "filetype" in entry, f"{name}: missing 'filetype'"
assert "url" in entry, f"{name}: missing 'url'"


def test_filetypes_are_known(resources_json):
for name, entry in resources_json.items():
ft = entry["filetype"].lower()
assert ft in VALID_FILETYPES, f"{name}: unknown filetype '{ft}'"


def test_urls_are_nonempty_strings_or_lists(resources_json):
for name, entry in resources_json.items():
url = entry["url"]
if isinstance(url, list):
assert len(url) > 0, f"{name}: url list is empty"
for u in url:
assert isinstance(u, str) and u.strip(), (
f"{name}: url list contains empty or non-string entry"
)
else:
assert isinstance(url, str) and url.strip(), (
f"{name}: url is empty or not a string"
)


def test_sortmerna_entries_have_keep_file(resources_json):
for name, entry in resources_json.items():
if entry["filetype"].lower() == "sortmerna":
assert "keep_file" in entry, f"{name}: sortmerna entry missing 'keep_file'"
assert entry["keep_file"].strip(), f"{name}: keep_file is empty"


def test_model_entries_have_list_of_urls(resources_json):
for name, entry in resources_json.items():
if entry["filetype"].lower() == "model":
assert isinstance(entry["url"], list), (
f"{name}: model filetype should have a list of urls"
)
Loading
Loading