-
Notifications
You must be signed in to change notification settings - Fork 106
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
Drop all capablities and run with no-new-privileges #765
Conversation
llama-run attempts to write to $HOME. Since we are running as root inside of the container, the process tries to write to /root which is has permissions 500, which means even root procesess can not write their without CAP_DAC_OVERRIDE, since we don't want to give that CAP, we modify HOME to be /tmp/ Signed-off-by: Daniel J Walsh <[email protected]>
Reviewer's Guide by SourceryThis pull request updates the container initialization logic in the model setup to improve security by dropping all capabilities and enforcing no-new-privileges. It also sets the HOME environment variable to /tmp to avoid permission issues when running as root. No diagrams generated as the changes look simple and do not need a visual representation. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @rhatdan - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider grouping the added security options into a dedicated variable for improved clarity and maintainability.
- Verify that resetting HOME to /tmp doesn't affect any subprocesses or scripts expecting the usual path.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Looks fine... I'm not very well versed in this stuff, but this won't kill the GPU acceleration, like the interactions with /dev/dri and /dev/kfd (for ROCm)? |
I suspect what "podman run" does and what our generators do for quadlet, k8s, etc. Might have drifted also |
Should work fine for GPUs only an issue if the Capability would be required. The point about generators is true. |
llama-run attempts to write to $HOME. Since we are running as root inside of the container, the process tries to write to /root which is has permissions 500, which means even root procesess can not write their without CAP_DAC_OVERRIDE, since we don't want to give that CAP, we modify HOME to be /tmp/
Summary by Sourcery
Enhancements: