From 3079e7a218dcb1e25373e0ca73b43d8782ee5906 Mon Sep 17 00:00:00 2001 From: shreyasNaik0101 Date: Sat, 4 Oct 2025 15:25:30 +0530 Subject: [PATCH 1/3] fix(ci): Use merge-base for correct target validation --- .../workflows/validate_modified_targets.yml | 28 +++++++++++++------ 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/.github/workflows/validate_modified_targets.yml b/.github/workflows/validate_modified_targets.yml index de0240907..3e5fde311 100644 --- a/.github/workflows/validate_modified_targets.yml +++ b/.github/workflows/validate_modified_targets.yml @@ -14,33 +14,43 @@ jobs: contents: read pull-requests: write steps: - - name: Checkout repository + - name: Checkout PR branch uses: actions/checkout@v5 with: - ref: ${{ github.base_ref }} - fetch-depth: 1 + # Check out the actual PR code, not the base branch + ref: ${{ github.event.pull_request.head.sha }} + # Fetch all history so we can find the common ancestor (merge-base) + fetch-depth: 0 - name: Set up Python uses: actions/setup-python@v6 with: - python-version: '3.13' + python-version: "3.13" - name: Install Poetry uses: abatilo/actions-poetry@v4 with: - poetry-version: 'latest' + poetry-version: "latest" - name: Install dependencies run: | poetry install --no-interaction --with dev - - name: Drop in place updated manifest from base + - name: Prepare JSON versions for comparison run: | - cp sherlock_project/resources/data.json data.json.base - git fetch origin pull/${{ github.event.pull_request.number }}/head:pr --depth=1 - git show pr:sherlock_project/resources/data.json > sherlock_project/resources/data.json + # Fetch the target branch to ensure we can compare against it + git fetch origin ${{ github.base_ref }} + + # Find the exact commit where this branch split from the target branch + MERGE_BASE=$(git merge-base origin/${{ github.base_ref }} HEAD) + echo "Comparing HEAD against merge-base commit: $MERGE_BASE" + + # Copy the version of the file from the current PR branch (HEAD) cp sherlock_project/resources/data.json data.json.head + # Extract the version of the file from the merge-base commit + git show $MERGE_BASE:sherlock_project/resources/data.json > data.json.base + - name: Discover modified targets id: discover-modified run: | From 4d00884d8c9689bce722ef64fb5e0a5bb4238f8c Mon Sep 17 00:00:00 2001 From: shreyasNaik0101 Date: Sun, 5 Oct 2025 03:00:21 +0530 Subject: [PATCH 2/3] fix(ci): Implement secure diff logic per feedback --- .../workflows/validate_modified_targets.yml | 40 ++++++++++++------- 1 file changed, 25 insertions(+), 15 deletions(-) diff --git a/.github/workflows/validate_modified_targets.yml b/.github/workflows/validate_modified_targets.yml index 3e5fde311..4738ae2b1 100644 --- a/.github/workflows/validate_modified_targets.yml +++ b/.github/workflows/validate_modified_targets.yml @@ -14,13 +14,11 @@ jobs: contents: read pull-requests: write steps: - - name: Checkout PR branch + - name: Checkout repository uses: actions/checkout@v5 with: - # Check out the actual PR code, not the base branch - ref: ${{ github.event.pull_request.head.sha }} - # Fetch all history so we can find the common ancestor (merge-base) - fetch-depth: 0 + # This is the original, secure checkout of the base branch. + ref: ${{ github.base_ref }} - name: Set up Python uses: actions/setup-python@v6 @@ -38,17 +36,21 @@ jobs: - name: Prepare JSON versions for comparison run: | - # Fetch the target branch to ensure we can compare against it - git fetch origin ${{ github.base_ref }} + # Fetch the PR's branch head and give it a local name 'pr' + git fetch origin pull/${{ github.event.pull_request.number }}/head:pr - # Find the exact commit where this branch split from the target branch - MERGE_BASE=$(git merge-base origin/${{ github.base_ref }} HEAD) - echo "Comparing HEAD against merge-base commit: $MERGE_BASE" + # The initial checkout may be shallow. To find a merge-base, + # we need more history. We can 'unshallow' the repository if needed. + git fetch --unshallow || true - # Copy the version of the file from the current PR branch (HEAD) - cp sherlock_project/resources/data.json data.json.head + # Find the merge-base commit between the target branch (master) and the PR branch (pr) + MERGE_BASE=$(git merge-base origin/${{ github.base_ref }} pr) + echo "Comparing PR head against merge-base commit: $MERGE_BASE" - # Extract the version of the file from the merge-base commit + # Safely extract the version of the file from the PR's head without checking it out + git show pr:sherlock_project/resources/data.json > data.json.head + + # Safely extract the version of the file from the merge-base commit git show $MERGE_BASE:sherlock_project/resources/data.json > data.json.base - name: Discover modified targets @@ -57,8 +59,16 @@ jobs: CHANGED=$( python - <<'EOF' import json - with open("data.json.base") as f: base = json.load(f) - with open("data.json.head") as f: head = json.load(f) + import sys + try: + with open("data.json.base") as f: base = json.load(f) + with open("data.json.head") as f: head = json.load(f) + except FileNotFoundError as e: + print(f"Error: Could not find {e.filename}", file=sys.stderr) + sys.exit(1) + except json.JSONDecodeError as e: + print(f"Error: Could not decode JSON from a file - {e}", file=sys.stderr) + sys.exit(1) changed = [] for k, v in head.items(): From 70e3c0ddd8fd162d162bdace19c296da96be861b Mon Sep 17 00:00:00 2001 From: shreyasNaik0101 Date: Sun, 5 Oct 2025 11:00:14 +0530 Subject: [PATCH 3/3] fix(ci): Address review feedback for correctness and efficiency --- .../workflows/validate_modified_targets.yml | 21 ++++++++++--------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/.github/workflows/validate_modified_targets.yml b/.github/workflows/validate_modified_targets.yml index 4738ae2b1..bb2445117 100644 --- a/.github/workflows/validate_modified_targets.yml +++ b/.github/workflows/validate_modified_targets.yml @@ -17,8 +17,9 @@ jobs: - name: Checkout repository uses: actions/checkout@v5 with: - # This is the original, secure checkout of the base branch. + # Checkout the base branch but fetch all history to avoid a second fetch call ref: ${{ github.base_ref }} + fetch-depth: 0 - name: Set up Python uses: actions/setup-python@v6 @@ -36,23 +37,21 @@ jobs: - name: Prepare JSON versions for comparison run: | - # Fetch the PR's branch head and give it a local name 'pr' + # Fetch only the PR's branch head (single network call in this step) git fetch origin pull/${{ github.event.pull_request.number }}/head:pr - # The initial checkout may be shallow. To find a merge-base, - # we need more history. We can 'unshallow' the repository if needed. - git fetch --unshallow || true - - # Find the merge-base commit between the target branch (master) and the PR branch (pr) + # Find the merge-base commit between the target branch and the PR branch MERGE_BASE=$(git merge-base origin/${{ github.base_ref }} pr) echo "Comparing PR head against merge-base commit: $MERGE_BASE" - # Safely extract the version of the file from the PR's head without checking it out + # Safely extract the file from the PR's head and the merge-base commit git show pr:sherlock_project/resources/data.json > data.json.head - - # Safely extract the version of the file from the merge-base commit git show $MERGE_BASE:sherlock_project/resources/data.json > data.json.base + # CRITICAL FIX: Overwrite the checked-out data.json with the one from the PR + # This ensures that pytest runs against the new, updated file. + cp data.json.head sherlock_project/resources/data.json + - name: Discover modified targets id: discover-modified run: | @@ -83,6 +82,8 @@ jobs: echo -e ">>> Changed targets: \n$(echo $CHANGED | tr ',' '\n')" echo "changed_targets=$CHANGED" >> "$GITHUB_OUTPUT" + # --- The rest of the steps below are unchanged --- + - name: Validate modified targets if: steps.discover-modified.outputs.changed_targets != '' continue-on-error: true