[WIP] Harden training script: credentials validation, error handling, security fixes by EZoni · Pull Request #432 · BLAST-AI-ML/synapse

EZoni · 2026-05-01T23:18:45Z

Overview

Add set -euo pipefail for strict error handling
Validate registry and DB env files exist before sourcing; assert required variables are set
Use --password-stdin for podman-hpc login to avoid exposing password in process list
Add curl -f to fail on HTTP errors and jq -e to fail on null/false output
Quote variable references consistently

Merge first

To do

Split features and bug fixes

Notes

Changes isolated from #428.

…ity fixes - Add `set -euo pipefail` for strict error handling - Validate registry and DB env files exist before sourcing; assert required variables are set - Use `--password-stdin` for podman-hpc login to avoid exposing password in process list - Add `curl -f` to fail on HTTP errors and `jq -e` to fail on null/false output - Quote variable references consistently

EZoni · 2026-05-01T23:19:38Z

@@ -1,4 +1,5 @@
 #!/bin/bash -l
+set -euo pipefail


-e exits on error, -u catches unbound variables, and -o pipefail catches failures mid-pipeline.

EZoni · 2026-05-01T23:21:14Z

+if [ ! -r "${REGISTRY_PROFILE}" ]; then
+    echo "Missing registry credentials file: ${REGISTRY_PROFILE}" >&2
+    echo "Create it with REGISTRY_USER and REGISTRY_PASSWORD for ${REGISTRY_NAME}." >&2
+    exit 1
+fi
+source "${REGISTRY_PROFILE}"  # credential variables: REGISTRY_USER and REGISTRY_PASSWORD
+: "${REGISTRY_USER:?REGISTRY_USER must be set in ${REGISTRY_PROFILE}}"
+: "${REGISTRY_PASSWORD:?REGISTRY_PASSWORD must be set in ${REGISTRY_PROFILE}}"
+
+if [ ! -r "${DB_ENV_FILE}" ]; then
+    echo "Missing container environment file: ${DB_ENV_FILE}" >&2
+    echo "Create it with SF_DB_READONLY_PASSWORD and AM_SC_API_KEY." >&2
+    exit 1
+fi


Failing fast with a clear error message beats a cryptic "unbound variable" or silent auth failure later.

EZoni · 2026-05-01T23:22:10Z

+    exit 1
+fi
+
+printf '%s\n' "${REGISTRY_PASSWORD}" | podman-hpc login --username "${REGISTRY_USER}" --password-stdin "${REGISTRY_NAME}"


Passwords passed as arguments are visible in ps output and shell history; reading from stdin avoids this. Using printf '%s\n' is also slightly better than echo since echo behavior with special characters can be shell-dependent.

EZoni · 2026-05-01T23:22:49Z

 OCI_MANIFEST_TYPE="application/vnd.oci.image.manifest.v1+json"
 #   first we unwarp the provenance information to pick the right image manifest
-MANIFEST_INDEX=$(curl -s \
+MANIFEST_INDEX=$(curl -fsS \


-f makes curl return a non-zero exit code on HTTP errors (4xx/5xx), and -S shows error messages even in silent mode. Without -f, a 401 or 404 would silently return an error document and let the script continue.

EZoni · 2026-05-01T23:22:55Z

 fi
 #   then we download the manifest of the image and parse out the config digest SHA
-REMOTE_SHA=$(curl -s \
+REMOTE_SHA=$(curl -fsS \


-f makes curl return a non-zero exit code on HTTP errors (4xx/5xx), and -S shows error messages even in silent mode. Without -f, a 401 or 404 would silently return an error document and let the script continue.

EZoni · 2026-05-01T23:23:58Z

    -u "${REGISTRY_USER}:${REGISTRY_PASSWORD}" \
    "https://${REGISTRY_NAME}/v2/${IMAGE_NAME}/manifests/${MANIFEST_INDEX}" | \
-    jq -r '.config.digest')
+    jq -er '.config.digest')


jq -er instead of -r adds -e, which makes jq exit non-zero if the output is false or null. Combined with set -e and curl -f, this closes a gap where a malformed or unexpected JSON response could let a null digest pass through and cause a misleading "IDs are different" message or worse.

EZoni · 2026-05-01T23:24:32Z

    -v /etc/localtime:/etc/localtime \
    -v /global/cfs/cdirs/m558/superfacility/model_training/config.yaml:/app/ml/config.yaml \
-    --env-file $HOME/db-podman.profile \
+    --env-file "${DB_ENV_FILE}" \


Quoting ${DB_ENV_FILE} in --env-file is a minor but correct fix for paths with spaces.

Copilot

Pull request overview

Hardens the ml/training_pm.sbatch Slurm training script by enabling strict bash mode, validating required credential/env files before use, and reducing credential exposure during registry login.

Changes:

Enable strict error handling via set -euo pipefail.
Validate registry credential and container env files exist; assert required registry variables are set.
Improve robustness of registry/manifest fetching with curl -f/jq -e and switch registry login to --password-stdin.

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

+set -euo pipefail



EZoni · 2026-05-01T23:32:34Z

+MANIFEST_INDEX=$(curl -fsS \
    -H "Accept: ${OCI_INDEX_TYPE}" \
    -u "${REGISTRY_USER}:${REGISTRY_PASSWORD}" \
    "https://${REGISTRY_NAME}/v2/${IMAGE_NAME}/manifests/${IMAGE_VERSION}" | \


This was pre-existing and is unrelated to the changes in this PR.

EZoni · 2026-05-01T23:40:53Z

+MANIFEST_INDEX=$(curl -fsS \
    -H "Accept: ${OCI_INDEX_TYPE}" \
    -u "${REGISTRY_USER}:${REGISTRY_PASSWORD}" \
    "https://${REGISTRY_NAME}/v2/${IMAGE_NAME}/manifests/${IMAGE_VERSION}" | \
-    jq -r '.manifests[] | select(.annotations["vnd.docker.reference.type"] != "attestation-manifest") | .digest' || echo "NOOCI")
+    jq -er '.manifests[] | select(.annotations["vnd.docker.reference.type"] != "attestation-manifest") | .digest' || echo "NOOCI")
 #   fallback for old Docker builders without OCI index


The original .manifests[] would cause jq to error if the manifests key was absent from the JSON response. For example, when the registry returns a plain Docker V2 manifest instead of an OCI index. The // [] fallback means jq gracefully produces no output rather than erroring, which allows the || echo "NOOCI" fallback to trigger correctly and the script to continue to the Docker V2 code path.

Suggested change

MANIFEST_INDEX=$(curl -fsS \

-H "Accept: ${OCI_INDEX_TYPE}" \

-u "${REGISTRY_USER}:${REGISTRY_PASSWORD}" \

"https://${REGISTRY_NAME}/v2/${IMAGE_NAME}/manifests/${IMAGE_VERSION}" | \

jq -r '.manifests[] | select(.annotations["vnd.docker.reference.type"] != "attestation-manifest") | .digest' || echo "NOOCI")

jq -er '.manifests[] | select(.annotations["vnd.docker.reference.type"] != "attestation-manifest") | .digest' || echo "NOOCI")

# fallback for old Docker builders without OCI index

MANIFEST_INDEX=$(curl -fsS \

-H "Accept: ${OCI_INDEX_TYPE}" \

-u "${REGISTRY_USER}:${REGISTRY_PASSWORD}" \

"https://${REGISTRY_NAME}/v2/${IMAGE_NAME}/manifests/${IMAGE_VERSION}" | \

jq -er '(.manifests // [])[] | select(.annotations["vnd.docker.reference.type"] != "attestation-manifest") | .digest' || echo "NOOCI")

# fallback for old Docker builders without OCI index

    --rm -it ${REGISTRY_NAME}/${IMAGE_NAME}:${IMAGE_VERSION} \
    python -u /app/ml/train_model.py --config_file /app/ml/config.yaml --model ${model}


EZoni added the ml Changes related to the ML models label May 1, 2026

EZoni mentioned this pull request May 1, 2026

[WIP] Support AmSC IRI API (deprecated) #428

Open

5 tasks

EZoni commented May 1, 2026

View reviewed changes

EZoni requested a review from Copilot May 1, 2026 23:25

Copilot started reviewing on behalf of EZoni May 1, 2026 23:25 View session

Copilot AI reviewed May 1, 2026

View reviewed changes

Add FIXME comment to training batch script

c81cb53

This was referenced May 8, 2026

[WIP] Support AmSC IRI API #439

Open

Read password from stdin for podman-hpc login #444

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Harden training script: credentials validation, error handling, security fixes#432

[WIP] Harden training script: credentials validation, error handling, security fixes#432
EZoni wants to merge 2 commits into
BLAST-AI-ML:mainfrom
EZoni:harden_training_script

EZoni commented May 1, 2026 •

edited

Loading

Uh oh!

EZoni May 1, 2026

Uh oh!

EZoni May 1, 2026

Uh oh!

EZoni May 1, 2026

Uh oh!

EZoni May 1, 2026

Uh oh!

EZoni May 1, 2026

Uh oh!

EZoni May 1, 2026

Uh oh!

EZoni May 1, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

EZoni May 1, 2026

Uh oh!

EZoni May 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		--rm -it ${REGISTRY_NAME}/${IMAGE_NAME}:${IMAGE_VERSION} \
		python -u /app/ml/train_model.py --config_file /app/ml/config.yaml --model ${model}

Conversation

EZoni commented May 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Overview

Merge first

To do

Notes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

EZoni commented May 1, 2026 •

edited

Loading