Skip to content

ci: add supply-chain guard to block fork PRs that modify dependencies - #26511

Merged
shin-berri merged 1 commit into
litellm_internal_stagingfrom
cursor/guard-fork-dependencies-eec1
Apr 25, 2026
Merged

ci: add supply-chain guard to block fork PRs that modify dependencies#26511
shin-berri merged 1 commit into
litellm_internal_stagingfrom
cursor/guard-fork-dependencies-eec1

Conversation

@krrish-berri-2

Copy link
Copy Markdown
Contributor

Relevant issues

Supply-chain security hardening for dependency files.

Pre-Submission checklist

  • I have Added testing in the tests/test_litellm/ directory, Adding at least 1 test is a hard requirement - see details
  • My PR passes all unit tests on make test-unit
  • My PR's scope is as isolated as possible, it only solves 1 specific problem
  • I have requested a Greptile review by commenting @greptileai and received a Confidence Score of at least 4/5 before requesting a maintainer review

Type

🚄 Infrastructure

Changes

Adds a new GitHub Actions workflow (guard-fork-dependencies.yml) that blocks pull requests from forks when they:

  1. Modify uv.lock — any change to the lockfile from a fork is rejected outright
  2. Add new dependencies to any pyproject.toml — checks root, litellm-proxy-extras/, and enterprise/ pyproject.toml files using proper TOML parsing (stdlib tomllib) to detect newly introduced package names

Security properties

Property Detail
Event trigger pull_request (not pull_request_target) — no secrets exposed to fork code
Action pins All uses: refs pinned to full SHA hashes
Credentials persist-credentials: false on all checkouts
Permissions permissions: {} at workflow level (no GitHub token permissions)
No script injection No user-controlled input (title, body, branch, etc.) interpolated in run: blocks
Paths filter Only triggers when uv.lock or pyproject.toml files are actually modified

How it works

  • The job's if: condition skips it entirely for internal PRs (same-repo branches), so it never blocks maintainer work
  • For fork PRs that touch dependency files, it:
    • Checks out both the base branch and the PR head in separate directories
    • Compares uv.lock with diff
    • Extracts dependency names from all TOML sections ([project].dependencies, [project.optional-dependencies], [dependency-groups]) using tomllib and PEP 508 name parsing
    • Reports any newly added packages and fails the check

Validation

  • Python extraction script tested against all three pyproject.toml files in the repo
  • comm-based diff logic verified with both positive (new dep detected) and negative (identical deps) cases
  • YAML validated with actionlint (zero errors)

Note: This is a CI-only change (new workflow file). No code tests are affected — the workflow itself only runs on fork PRs that modify dependency files.

Slack Thread

Open in Web Open in Cursor 

Add a new CI workflow that rejects pull requests from forks when they:
- Modify uv.lock (any change at all)
- Add new dependencies to any pyproject.toml file (root, litellm-proxy-extras, enterprise)

Security properties:
- Uses pull_request (not pull_request_target) so no secrets are exposed
- All action refs pinned to full SHA hashes
- persist-credentials: false on all checkouts
- permissions: {} (no GitHub token permissions)
- No user-controlled input in run: blocks (no script injection)
- Proper TOML parsing via stdlib tomllib (not regex on raw text)
- Only triggers when dependency files are actually changed (paths filter)

Internal PRs (from branches in the canonical repo) skip the job entirely.

Co-authored-by: Krrish Dholakia <krrish-berri-2@users.noreply.github.com>
@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@codecov

codecov Bot commented Apr 25, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@yuneng-berri
yuneng-berri marked this pull request as ready for review April 25, 2026 19:49
@yuneng-berri
yuneng-berri requested a review from a team April 25, 2026 19:49
@veria-ai

veria-ai Bot commented Apr 25, 2026

Copy link
Copy Markdown
Contributor

Low: CI-only change with no security issues

This PR adds a GitHub Actions workflow that blocks fork PRs from modifying dependency files (uv.lock, pyproject.toml). It's a supply-chain hardening measure. The workflow uses pinned action SHAs, sets permissions: {} (least privilege), uses persist-credentials: false, and only runs on fork PRs. No runtime code is changed.


Status: 0 open
Risk: 1/10

Posted by Veria AI · 2026-04-25T19:50:46.405Z

@greptile-apps

greptile-apps Bot commented Apr 25, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds a new GitHub Actions workflow that blocks fork PRs from introducing new Python dependencies or modifying the uv.lock lockfile. The security design is sound — SHA-pinned actions, pull_request (not pull_request_target), no token permissions, and no user-controlled input in run: blocks — but three edge-case logic gaps could produce misleading failures or false positives in uncommon scenarios.

  • uv.lock diff (line 40): diff exits 2 on a missing file, so the step incorrectly reports "must not modify uv.lock" when the lockfile simply doesn't exist in one checkout.
  • Deleted pyproject.toml (lines 105-112): when the base has the file but the PR removes it, check_deps falls through to calling python3 on a non-existent path and dies without a ::error:: annotation.
  • Empty dep-list / comm false positive (line 117): echo "$empty_var" emits a blank line that comm treats as a real entry, potentially triggering a spurious "new dependency" failure when a TOML file has no tracked dependencies.

Confidence Score: 4/5

Safe to merge; all three issues are P2 edge cases that fail-safe (the check blocks rather than permits) and are unlikely to occur against litellm's real pyproject files.

Only P2 findings present — the workflow's security invariants are solid and it cannot be bypassed. The gaps cause confusing error messages or false-positive failures in rare edge cases, not security holes.

.github/workflows/guard-fork-dependencies.yml — the Reject uv.lock changes step and the check_deps shell function both have minor logic gaps worth fixing.

Important Files Changed

Filename Overview
.github/workflows/guard-fork-dependencies.yml New supply-chain guard workflow — well-structured overall (SHA-pinned actions, no secrets, pull_request trigger) but has three edge-case logic gaps: misleading error when uv.lock is absent, unhandled pr_file-deleted scenario in check_deps, and a false-positive comm comparison when the PR pyproject.toml has zero dependencies.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[pull_request event] --> B{Is fork PR?\nhead.repo != github.repository}
    B -- No --> C[Skip job entirely\ninternal PR unaffected]
    B -- Yes --> D{paths filter:\nuv.lock or pyproject.toml changed?}
    D -- No --> E[Workflow not triggered]
    D -- Yes --> F[Checkout base branch → ./base]
    F --> G[Checkout PR head → ./pr]
    G --> H{diff base/uv.lock\nvs pr/uv.lock}
    H -- Same --> I[uv.lock ✅]
    H -- Different or missing --> J[❌ exit 1 Lockfile changed]
    I --> K[extract_deps.py: parse base pyproject.toml files]
    K --> L[extract_deps.py: parse PR pyproject.toml files]
    L --> M{comm -13\nnew names in PR?}
    M -- None --> N[✅ All checks pass]
    M -- New names found --> O[❌ exit 1 New dependencies blocked]
Loading

Reviews (1): Last reviewed commit: "ci: add supply-chain guard to block fork..." | Re-trigger Greptile

Comment on lines +105 to +112
if [ ! -f "$base_file" ] && [ ! -f "$pr_file" ]; then
return 0
fi

if [ ! -f "$base_file" ] && [ -f "$pr_file" ]; then
echo "::error::Fork PR introduces a new $label that does not exist on the base branch."
return 1
fi

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Missing guard for deleted pr_file

When a fork PR deletes a pyproject.toml that exists on the base branch, base_file is present but pr_file is absent. Neither early-return guard covers this case (! -f base && ! -f pr or ! -f base && -f pr), so execution falls through to python3 /tmp/extract_deps.py "$pr_file", which raises FileNotFoundError. With set -euo pipefail active the shell exits immediately without printing a ::error:: annotation, producing a confusing job failure instead of a clear message.

Suggested change
if [ ! -f "$base_file" ] && [ ! -f "$pr_file" ]; then
return 0
fi
if [ ! -f "$base_file" ] && [ -f "$pr_file" ]; then
echo "::error::Fork PR introduces a new $label that does not exist on the base branch."
return 1
fi
if [ ! -f "$base_file" ] && [ ! -f "$pr_file" ]; then
return 0
fi
if [ ! -f "$base_file" ] && [ -f "$pr_file" ]; then
echo "::error::Fork PR introduces a new $label that does not exist on the base branch."
return 1
fi
if [ -f "$base_file" ] && [ ! -f "$pr_file" ]; then
echo "$label: file removed in PR — no new dependencies possible."
return 0
fi

Comment on lines +39 to +44
run: |
if ! diff -q base/uv.lock pr/uv.lock >/dev/null 2>&1; then
echo "::error::Fork PRs must not modify uv.lock. Dependency lockfile changes must come from a branch in the canonical repository."
exit 1
fi
echo "uv.lock is unchanged."

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 diff exit code 2 triggers misleading "must not modify uv.lock" error

diff exits with status 2 (not 1) when one of the compared paths does not exist. Because the step uses ! diff ... 2>&1, any missing file (e.g. base/uv.lock absent on a branch that predates lockfile tracking) causes the condition to be true and emits "Fork PRs must not modify uv.lock" — a misleading message that hides the real cause.

Suggested change
run: |
if ! diff -q base/uv.lock pr/uv.lock >/dev/null 2>&1; then
echo "::error::Fork PRs must not modify uv.lock. Dependency lockfile changes must come from a branch in the canonical repository."
exit 1
fi
echo "uv.lock is unchanged."
if [ ! -f base/uv.lock ] && [ ! -f pr/uv.lock ]; then
echo "uv.lock does not exist in either tree — skipping check."
elif ! diff -q base/uv.lock pr/uv.lock >/dev/null 2>&1; then
echo "::error::Fork PRs must not modify uv.lock. Dependency lockfile changes must come from a branch in the canonical repository."
exit 1
else
echo "uv.lock is unchanged."
fi

base_deps=$(python3 /tmp/extract_deps.py "$base_file")
pr_deps=$(python3 /tmp/extract_deps.py "$pr_file")

new_deps=$(comm -13 <(echo "$base_deps") <(echo "$pr_deps"))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 echo "$empty_var" injects a blank line into comm

When pr_deps is empty (a pyproject.toml with no tracked dependencies), echo "$pr_deps" emits a single newline that comm treats as an empty-string entry. Since "" sorts before any real package name, comm -13 sees it as a line present only in the PR stream and sets new_deps to a non-empty value (just whitespace), which causes [ -n "$new_deps" ] to fire — a false-positive "new dependency" failure.

Suggested change
new_deps=$(comm -13 <(echo "$base_deps") <(echo "$pr_deps"))
new_deps=$(comm -13 <(printf '%s\n' $base_deps | sort) <(printf '%s\n' $pr_deps | sort))

@shin-berri
shin-berri merged commit 0a16615 into litellm_internal_staging Apr 25, 2026
43 checks passed
@shin-berri
shin-berri deleted the cursor/guard-fork-dependencies-eec1 branch April 25, 2026 19:54
fzowl pushed a commit to fzowl/litellm that referenced this pull request Jun 24, 2026
…dencies-eec1

ci: add supply-chain guard to block fork PRs that modify dependencies
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.

5 participants