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

fix: support running in FIPS-enabled environments #3571

Merged
merged 2 commits into from
Jan 25, 2025

Conversation

marcodlk
Copy link
Contributor

📝 Summary

Problem

Currently, marimo will fail on FIPS-enabled environments where md5 hashing is disabled for security purposes. This means that js_hash cannot be calculated in marimo/_plugins/ui/_impl/from_anywidget.py.

Solution

Since md5 is being used for file integrity checks, this particular use of md5 can be considered FIPS compliant. Since python 3.9, the usedforsecurity flag has been introduced in hashlib functions, which allows for bypassing the FIPS enforcement check. As such, using usedforsecurity=False when invoking hashlib.md5 solves the issue.

🔍 Description of Changes

Use usedforsecurity=False when invoking hashlib.md5 in marimo._plugins.ui._impl.from_anywidget.anywidget.__init__.

📋 Checklist

  • I have read the contributor guidelines.
  • For large changes, or changes that affect the public API: this change was discussed or approved through an issue, on Discord, or the community discussions (Please provide a link if applicable).
  • I have added tests for the changes made.
  • I have run the code and verified that it works as expected.

I did not add tests because I'm not sure how to go about mocking a FIPS enabled environment, and the change is a noop for non-FIPS environments.

Working to setup an EC2 instance with FIPS enabled to verify the changes work off of my fork, but have not done so yet.

📜 Reviewers

@mscolnick (guessing you as the reviewer, as you've been engaging w previous python questions I've had)

Copy link

vercel bot commented Jan 25, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
marimo-storybook ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 25, 2025 1:18am

Copy link

vercel bot commented Jan 25, 2025

@marcodlk is attempting to deploy a commit to the marimo Team on Vercel.

A member of the Team first needs to authorize it.

Copy link

github-actions bot commented Jan 25, 2025

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@marcodlk
Copy link
Contributor Author

I have read the CLA Document and I hereby sign the CLA

@marcodlk
Copy link
Contributor Author

marcodlk commented Jan 25, 2025

I've verified in an ec2 instance with FIPS enabled that the change fixes the issue. Is that sufficient for this? Feels not worth the effort to put together a FIPS-enabled docker image to replicate the issue and setup a test for, given the noop nature of the change for all existing supported cases.

@marcodlk marcodlk marked this pull request as ready for review January 25, 2025 05:47
@mscolnick
Copy link
Contributor

Thanks for this! This is perfect

@mscolnick mscolnick merged commit 420e110 into marimo-team:main Jan 25, 2025
29 of 30 checks passed
Copy link

🚀 Development release published. You may be able to view the changes at https://marimo.app?v=0.10.18-dev3

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