Skip to content

Adding a temporary fix to stop non-maintainers from RCE #19

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

Closed
wants to merge 4 commits into from

Conversation

unlox775-code-dot-org
Copy link

@unlox775-code-dot-org unlox775-code-dot-org commented Oct 26, 2023

Summary

This PR introduces a change to our CI/CD process concerning the AWS CodeBuild project. Specifically, we’re implementing a filter to the primary GitHub source, which checks against a regex pattern of user IDs. These IDs currently represent our existing maintainers and repository owners. While this is a quick, interim solution to a larger security issue, I acknowledge its unsustainability in the long run.

Security Issue Background

A vulnerability was identified where an external entity could push a change to the build spec in our public repositories. When this happens, GitHub sends a webhook push, and CodeBuild, in turn, reads and executes the modified build spec. This behavior effectively grants remote code execution capabilities in our environment. While we can limit access through specific IAM roles, the potential risk remains significant.

Long-Term Solution Ideas

1.	Private Repository for CI: Migrate our CI infrastructure code to a private repository. This action would ensure the main code remains accessible for public contribution while safeguarding our CI setup.
2.	GitHub Actions with Quality Gate: Use GitHub Actions and introduce a quality gate that halts PRs not originating from maintainers, giving us the chance to review and approve changes before execution.

The current change is a temporary measure. It’ll necessitate manual updates as team dynamics evolve. In the meantime, we’re actively exploring more sustainable solutions to ensure a secure CI/CD process.

See Child PR's:

Copy link
Contributor

@cat5inthecradle cat5inthecradle left a comment

Choose a reason for hiding this comment

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

This will need to be deployed manually.

@unlox775-code-dot-org
Copy link
Author

As it was a list of lists, I read somewhere that the outer list was AND, with the inner list was OR. I will look that up

@cat5inthecradle
Copy link
Contributor

deployed manually

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.

3 participants