-
Notifications
You must be signed in to change notification settings - Fork 4k
Secure ROCm apt repo setup in build_cmake workflow #4531
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
base: main
Are you sure you want to change the base?
Conversation
…ion) - Replaced deprecated apt-key with signed-by keyring - Enforced HTTPS for ROCm repo - Removed insecure apt-get flags (--allow-insecure-repositories, --allow-unauthenticated) - Added SHA256 verification for ROCm GPG key - Scoped trust to ROCm repo only This prevents potential MITM/supply chain attacks in CI builds.
Hi @sanjay20m! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at [email protected]. Thanks! |
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
Hi, can I know what further changes I will have to make, like any tweaks required ?? |
Our friends at AMD are best equipped to review this one @ItsPitt (when you have time) |
After a long time, just to ask if any changes are required, please tell me :) |
I think this looks good from our side! |
@sanjay20m looks there is a build failure, could you help take a look? Thanks! After everything is green, I will import the change |
Done ! @junjieqi |
@sanjay20m looks like there is another failure on arm64 SVE |
Hi @junjieqi Thanks for reviewing! I wanted to clarify that the change in this PR is strictly scoped to hardening the ROCm apt repository setup (HTTPS, signed-by keyring, checksum verification, removal of insecure flags). The current build failure is happening in the Conda environment step, where the solver is revising libblas/libcblas/liblapack and exiting with code 1: The following packages will be REVISED: This looks like a Conda dependency resolution issue unrelated to the apt repo setup. If needed, I can help pin the BLAS/LAPACK versions in the workflow or enable strict channel priority to resolve it, but I think it makes sense to review/merge the security fix separately since the two are orthogonal. |
Hi @sanjay20m , thanks for the reply. I'm wondering if we need to pin the BLAS/LAPACK first before this one. Once we pin them, and we could rebase current pr with master to resolve the build failure |
Fix ROCm setup: remove duplicate cleanup and target.lst entries - Removed redundant cleanup commands that were outside the ROCm patch. - Ensure MI200-class accelerator fake entry (`gfx90a`) is only added once. - Streamlined BLAS/LAPACK pinning and ROCm package installation steps.
Hi! @junjieqi, now I think it may satisfy you ... |
Thank you for your prompt response! Looks the linuxx86_64 still failed and it is related with the latest change. Thank you for your patience again! |
Summary
This PR hardens the ROCm apt repository setup in the
build_cmake
GitHub Actions workflow to prevent potential supply chain attacks.Changes Made
--allow-insecure-repositories
--allow-unauthenticated
apt-key
usage withsigned-by
keyring methodSecurity Impact
These changes:
Rationale
GitHub-hosted workflows that install packages from third-party repos must ensure:
This change addresses a potential supply chain vulnerability in the GitHub Actions workflow by enforcing secure package verification.