Skip to content

fix(azure_pipeline): github head validation#2681

Open
james-matthews1 wants to merge 1 commit intomasterfrom
DTSPO-28735-fixing-validation
Open

fix(azure_pipeline): github head validation#2681
james-matthews1 wants to merge 1 commit intomasterfrom
DTSPO-28735-fixing-validation

Conversation

@james-matthews1
Copy link
Copy Markdown
Contributor

@james-matthews1 james-matthews1 commented Dec 1, 2025

Jira link

https://tools.hmcts.net/jira/browse/DTSPO-28735

Change description

Added GitHub HEAD validation

Testing done

Security Vulnerability Assessment

CVE Suppression: Are there any CVEs present in the codebase (either newly introduced or pre-existing) that are being intentionally suppressed or ignored by this commit?

  • Yes
  • No

Checklist

  • commit messages are meaningful and follow good commit message guidelines
  • README and other documentation has been updated / added (if needed)
  • tests have been updated / new tests has been added (if needed)
  • Does this PR introduce a breaking change

Link to Terraform Plan

https://tfplan-viewer.hmcts.net/azure-platform-terraform/2681

🤖AEP PR SUMMARY🤖

  • azure_pipeline.yaml
    🔧 Added support for GitHub repository HEAD validation during pipeline precheck stage:
    • Fetches latest commit SHA for the target branch using multiple methods.
    • Compares latest SHA with current run SHA to determine if HEAD validation passes.
    • Sets pipeline action to "apply" if SHAs match, else defaults to "plan".
      🛠️ Refined logs and error handling for branch resolution failures.
      🔄 Changed message for unsupported repository providers from "Non-Azure repository" to "Unsupported repository provider".

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Dec 1, 2025

Code Quality

  1. Excessive Use of Blank Echo Statements:

    • The addition of multiple echo \"\" lines seems unnecessary and clutters the script. They don't provide value. Consider removing these lines for better readability and maintainability.

    Example:
    diff

    • echo ""
    • echo ""
      ...
    could be removed entirely.
    
    
  2. Repetitive Code for Fetch and SHA Resolution:

    • The repeated logic for resolving the latest SHA for GitHub can be refactored into a reusable function. This reduces redundancy and improves maintainability.

    Example:

    resolve_latest_sha() {
        local branch=$1
        local repo_url=$2
        local latest_sha=$(git ls-remote origin \"refs/heads/${branch}\" 2>/dev/null | awk '{print $1}')
        if [[ -z \"${latest_sha}\" ]]; then
            latest_sha=$(git rev-parse \"origin/${branch}\" 2>/dev/null || echo \"\")
        fi
        if [[ -z \"${latest_sha}\" ]]; then
            latest_sha=$(git ls-remote \"${repo_url}\" \"refs/heads/${branch}\" 2>/dev/null | awk '{print $1}')
        fi
        echo \"${latest_sha}\"
    }

    Replace:

    LATEST_SHA=$(git ls-remote origin \"refs/heads/${BRANCH}\" 2>/dev/null | awk '{print $1}') 
    if [[ -z \"${LATEST_SHA}\" ]]; then 
        LATEST_SHA=$(git rev-parse \"origin/${BRANCH}\" 2>/dev/null || echo \"\") 
    fi 
    if [[ -z \"${LATEST_SHA}\" ]]; then 
        LATEST_SHA=$(git ls-remote \"${REPO_URL}\" \"refs/heads/${BRANCH}\" 2>/dev/null | awk '{print $1}') 
    fi

    With:

    LATEST_SHA=$(resolve_latest_sha \"${BRANCH}\" \"${REPO_URL}\")
  3. Unnecessary exit 0 Statements:

    • Multiple exit 0 statements toward the end of conditional branches may terminate the script prematurely. Review whether this is intentional or if these can be replaced by explicit setting of finalAction.

    Example:

    exit 0

    If unnecessary, remove or consolidate.

Best Practices

  1. Error Logging for Unsuccessful Commands:

    • Currently, git fetch, git ls-remote, and git rev-parse suppress STDERR with 2>/dev/null. This hides errors that could be valuable for debugging. Use proper error logging or conditional checking after these commands.

    Example:

    git fetch origin \"${BRANCH}\" --depth=1 2>/dev/null || { 
        echo \"Error: Failed to fetch branch '${BRANCH}'\"; 
        exit 1; 
    }
  2. Consistent Handling for Unsupported Providers:

    • Your message for unsupported repository providers (Unsupported repository provider detected) is inconsistent with the rest of the script that uses structured error format.
    • Example:
    - echo \"Unsupported repository provider detected (${REPO_PROVIDER}). Skipping HEAD validation for ${SRC_BRANCH_REF}.\"

    Update to:

    echo \"Error: Unsupported repository provider '${REPO_PROVIDER}' detected. Skipping HEAD validation for branch '${SRC_BRANCH_REF}'.\"
  3. Protect Against Unset Variables:

    • BRANCH, RUN_SHA, etc. assume values are always set. Add set -u at the top of the script to error out on unset variables.

    Example:

    set -euo pipefail

Security

  1. Hardening Against Injection:

    • Ensure variables like BRANCH, REPO_URL, RUN_SHA, etc. are sanitized before use, as they could originate from untrusted sources (e.g., environment variables).
    • Example:
    if [[ \"${BRANCH}\" =~ [^a-zA-Z0-9/_-] ]]; then
        echo \"Error: Unsafe branch name detected (${BRANCH})\"
        exit 1
    fi
  2. Minimize Depth in Git Fetch:

    • Specify --depth=1 when using git fetch is good, but confirm it meets your use case. If more history is required, adjust depth accordingly or omit for full fetch.

Cost and Carbon Usage

  1. Optimize Git Operations (Minor impact on cost and carbon):

    • Reduce unnecessary fetches in cases where the branch SHA is already known, saving pipeline runtime and compute resources.

    Example:

    if [[ \"${LATEST_SHA}\" == \"${RUN_SHA}\" ]]; then
        echo \"HEAD validation passed (GitHub).\"
        return
    fi

Estimated Cost and Carbon Impact:

  • Optimizing git fetch and reducing repetitions may save up to 0.5-1 seconds per pipeline run, depending on the repository size. This can cumulatively save compute costs over time, though the direct impact is minor.
  • Cost impact: ~£0 for moderate-sized workflows.
  • Carbon impact: Negligible for small pipelines but accumulates for thousands of runs, especially if using shared cloud resources.

Summary

  1. Remove redundant echo \"\" statements.
  2. Refactor repeated SHA resolution logic into a function.
  3. Add proper error logging and output consistency.
  4. Validate and sanitize inputs such as BRANCH and REPO_URL.
  5. Consolidate redundant exit 0 statements.
  6. Set set -euo pipefail for better error handling and reduce potential script vulnerabilities.

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