Skip to content

Conversation

@effigies
Copy link
Member

@effigies effigies commented Oct 28, 2025

In #3556, @tsalo discovered he couldn't (effectively) run pixi lock from a mac, which I believe is due to prefix-dev/pixi#3081.

This PR runs pixi lock on PRs and commits to the incoming branch if they are out-of-date.

#3559 demonstrates (see, e.g., 35151f7).

@effigies effigies requested a review from Copilot October 28, 2025 22:07
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the pixi lockfile checking mechanism by moving it from the Docker workflow to a dedicated workflow that automatically updates lockfiles on pull requests. The key changes include:

  • Creating a new automated workflow that checks and updates pixi lockfiles on pull requests
  • Removing the manual pixi check step from the Docker build workflow
  • Implementing automatic lockfile updates with push-back to the PR branch

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
.github/workflows/pixi-lock.yml New workflow that automatically checks and updates pixi lockfiles for pull requests using pull_request_target trigger
.github/workflows/docker.yml Removed the check-pixi job that previously validated lockfile consistency during Docker builds

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

@@ -0,0 +1,55 @@
on:
pull_request_target
Copy link

Copilot AI Oct 28, 2025

Choose a reason for hiding this comment

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

Using pull_request_target with contents: write permission and pushing to the source repository creates a security risk. This workflow has write access and executes in the context of the base repository, but it pushes to the fork repository URL. For forks from untrusted sources, this could be exploited. Consider adding authentication for the git push operation or using a GitHub App token with limited scope instead of pushing directly to $REMOTE.

Copilot uses AI. Check for mistakes.
Copy link
Member Author

Choose a reason for hiding this comment

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

What is the exploitation path? This PR runs known code on an unknown branch and pushes to that branch. An attacker does not choose what code runs and the result is not pushed directly to the upstream.

bash -c '(pixi lock --check && git checkout .) || true'
- name: Push updated lockfile, if needed
run: |
git push $REMOTE HEAD:$BRANCH
Copy link

Copilot AI Oct 28, 2025

Choose a reason for hiding this comment

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

The git push command will fail for fork PRs because it attempts to push to the fork repository URL without authentication. The GITHUB_TOKEN used for checkout only has permissions for the base repository, not the fork. This will cause authentication failures when pushing to forks.

Copilot uses AI. Check for mistakes.
Copy link
Member Author

Choose a reason for hiding this comment

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

If the user does not allow pushes from maintainers and a commit is generated, then this workflow should fail.

Comment on lines +1 to +2
on:
pull_request_target
Copy link

Copilot AI Oct 28, 2025

Choose a reason for hiding this comment

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

The workflow trigger is missing a name field at the top of the file. All GitHub Actions workflows should include a descriptive name for better identification in the Actions UI. Consider adding name: Pixi lockfile maintenance or similar at the beginning of the file.

Copilot uses AI. Check for mistakes.
@codecov
Copy link

codecov bot commented Oct 28, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.93%. Comparing base (a1c53ad) to head (d7e5307).
⚠️ Report is 3 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #3560   +/-   ##
=======================================
  Coverage   72.93%   72.93%           
=======================================
  Files          60       60           
  Lines        4818     4818           
  Branches      625      625           
=======================================
  Hits         3514     3514           
  Misses       1162     1162           
  Partials      142      142           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@effigies effigies merged commit de1344d into master Oct 28, 2025
18 of 22 checks passed
@effigies effigies deleted the pixi-lock branch October 28, 2025 22:33
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