Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ci: protect against surreptitious lockfile changes #236

Merged
merged 16 commits into from Apr 28, 2023
Merged

ci: protect against surreptitious lockfile changes #236

merged 16 commits into from Apr 28, 2023

Conversation

maxrake
Copy link
Contributor

@maxrake maxrake commented Apr 25, 2023

This PR is meant to be merged...but only after some changes are made to the lockfile. It is being used to demonstrate an attack where the lockfile is updated surreptitiously and the changes are attempted to be approved through a PR, due to the way GitHub's linguist library hides/collapses known lockfiles.

The real changes made here include:

  • Add a Discord shield to the README
  • Add lockfile refresh steps to workflows
    • A poetry lock --no-update command is added to all the workflows where poetry is used to install an environment from the lockfile
    • Doing so "refreshes" the lockfile and will remove any entries in the lockfile that are not actually dependencies of packages defined in the pyproject.toml file
    • It does not produce an error or non-zero return code when changes are made, but at least the lockfile will be in a better state before it gets used
  • Add test to ensure build system has not changed without review
  • Update lockfile dependencies to the latest versions

A `poetry lock --no-update` command is added to all the workflows where `poetry` is used to install an environment from the lockfile. Doing so "refreshes" the lockfile and will remove any entries in the lockfile that are not actually dependencies of packages defined in the `pyproject.toml` file. It does not produce an error or non-zero return code when changes are made, but at least the lockfile will be in a better state before it gets used.
Arbitrary code execution can occur when building/installing packages from source distributions. This test guards against changes to the established/vetted build system. There may be legitimate times to change the build system requirements and/or backend, but those changes will be more apparent in code reviews since this test will also have to change. Changes to the values in the `pyproject.toml` file may be subtle and go unnoticed. In the worst case, it is possible for the values to be changed to malicious entries that seek to cause harm in CI systems.
@maxrake maxrake self-assigned this Apr 25, 2023
@maxrake maxrake requested a review from a team as a code owner April 25, 2023 01:31
@maxrake maxrake requested review from andreaphylum and kylewillmon and removed request for andreaphylum April 25, 2023 01:31
@github-actions
Copy link

Phylum OSS Supply Chain Risk Analysis - INCOMPLETE

The analysis contains 1 package(s) Phylum has not yet processed,
preventing a complete risk analysis. Phylum is processing these
packages currently and should complete soon.
Please wait for up to 30 minutes, then re-run the analysis.

View this project in the Phylum UI

@github-actions
Copy link

Phylum OSS Supply Chain Risk Analysis - SUCCESS

The Phylum risk analysis is complete and has passed the active policy.

View this project in the Phylum UI

@github-actions
Copy link

Phylum OSS Supply Chain Risk Analysis - INCOMPLETE

The analysis contains 1 package(s) Phylum has not yet processed,
preventing a complete risk analysis. Phylum is processing these
packages currently and should complete soon.
Please wait for up to 30 minutes, then re-run the analysis.

View this project in the Phylum UI

@github-actions
Copy link

Phylum OSS Supply Chain Risk Analysis - SUCCESS

The Phylum risk analysis is complete and has passed the active policy.

View this project in the Phylum UI

README.md Show resolved Hide resolved
tests/unit/test_package_metadata.py Show resolved Hide resolved
@maxrake maxrake requested a review from cd-work April 27, 2023 16:34
@maxrake maxrake merged commit c064260 into main Apr 28, 2023
10 checks passed
@maxrake maxrake deleted the blog_demo branch April 28, 2023 20:05
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.

None yet

2 participants