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

SOC2: Updates nitro-cli to v1.3.1 #472

Merged
merged 2 commits into from
Jul 17, 2024
Merged

Conversation

taylorjdawson
Copy link
Contributor

@taylorjdawson taylorjdawson commented Jul 8, 2024

Summary & Motivation (Problem vs. Solution)

This PR addresses security vulnerabilities by updating key dependencies to their latest, patched versions, ensuring SOC2 compliance. Specifically this PR updates nitro-cli to v1.3.1.

Vulnerable Package Updates

Package Module Old Version New Version Vulnerability Severity
shlex qos_enclave 1.1.0 1.3.0 GHSA-r7qv-8r2h-pg27 HIGH
vmm-sys-util qos_enclave 0.11.1 0.12.1 CVE-2023-50711 MEDIUM
rustix qos_enclave 0.37.15 0.38.34 GHSA-c827-hfw6-qwvm MEDIUM

Required Package Updates

Package Module Old Version New Version Reason
nitro-cli qos_enclave 1.2.2 1.3.1 Update due to vulnerable dependencies: shlex, vmm-sys-util, rustix

How I Tested These Changes

  • Running make in root

Pre merge check list

  • Update CHANGELOG.MD

@taylorjdawson taylorjdawson requested a review from a team as a code owner July 8, 2024 19:53
@james-callahan
Copy link
Contributor

Is there a way we can split up these cargo.lock updates?
There's too much to review in one single go.

@r-n-o
Copy link
Contributor

r-n-o commented Jul 10, 2024

@james-callahan unfortunately I don't think that's an option. There's a lot of updates in cargo.lock because nitro-cli is a crate with a lot of dependencies itself: https://github.com/aws/aws-nitro-enclaves-cli/blob/837e1146ea76e1e9e4f2d1ad3e35a60811152473/Cargo.toml#L9-L30

If we want to update this crate we have to update the transitive dependencies in our cargo.lock, no way around it I'm afraid. And I bet some of nitro-cli's dependencies themselves have many dependencies and so on and so forth. How deep down the rabbit hole should we go here? My proposal is to review https://github.com/aws/aws-nitro-enclaves-cli/compare/v1.2.2..v1.3.1 (just did it, looks reasonable to me, nothing malicious), and we can wait until we have better tooling to review all of the transitive dependency updates. It's impractical to do it manually right now.

@taylorjdawson taylorjdawson requested a review from r-n-o July 11, 2024 20:43
@cr-tk cr-tk mentioned this pull request Jul 15, 2024
1 task
@taylorjdawson taylorjdawson changed the title Updates nitro-cli to v1.3.1 SOC2: Updates nitro-cli to v1.3.1 Jul 15, 2024
Copy link
Collaborator

@cr-tk cr-tk left a comment

Choose a reason for hiding this comment

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

Using experimental review diff analysis tooling, already-trusted-in-rust-core-projects comparisons for versions, diff review and other steps, @lrvick and I were able to substantially reduce the amount of necessary review and manually go through all remaining dependency code changes.

I can therefore approve this PR and propose someone from @tkhq/qos-operators approves it to unblock the merge 👍

@cr-tk
Copy link
Collaborator

cr-tk commented Jul 16, 2024

CC @r-n-o @jack-kearney as status update.

@cr-tk
Copy link
Collaborator

cr-tk commented Jul 17, 2024

@jack-kearney @lrvick can one of you formally approve this? Thanks!

@jack-kearney jack-kearney merged commit 406af02 into main Jul 17, 2024
5 checks passed
@jack-kearney jack-kearney deleted the taylor/update-nitro-cli-1.3.1 branch July 17, 2024 18:31
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