fix: add allow-list for global config keys from Docker labels#487
fix: add allow-list for global config keys from Docker labels#487
Conversation
Prevent Docker labels on service containers from overriding security-sensitive global config fields like allow-host-jobs-from-labels, web authentication settings, pprof, and default-user. Only explicitly allow-listed keys (notifications, logging, scheduling) can be set via labels. Blocked keys emit a security warning. Closes #486
Add tests for: - Blocked security-sensitive keys (host jobs, web auth, pprof, default-user) - Allowed safe keys (notifications, logging, scheduling) - Full attack scenario: label cannot enable host job execution
Summary of ChangesHello @CybotTM, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the security posture by introducing an allow-list mechanism for global configuration keys derived from Docker labels. It prevents malicious or misconfigured containers from overriding critical security settings, thereby closing a privilege escalation vector and ensuring that sensitive configurations can only be managed through the main configuration file. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
✅ Mutation Testing ResultsMutation Score: 100.00% (threshold: 60%)
What is mutation testing?Mutation testing measures test quality by introducing small changes (mutations) to the code and checking if tests detect them. A higher score means better test effectiveness.
|
There was a problem hiding this comment.
Pull request overview
This PR implements a security fix to prevent privilege escalation via Docker labels by introducing an allow-list for global configuration keys that can be set by service containers. The fix addresses issue #486 where malicious containers with ofelia.service=true could override security-critical settings like allow-host-jobs-from-labels to enable arbitrary command execution on the host.
Changes:
- Adds
globalLabelAllowListmap incli/docker-labels.godefining which global config keys may be set via Docker labels - Implements filtering logic that blocks non-allowed keys and logs security warnings
- Adds comprehensive test coverage for blocked/allowed keys and end-to-end attack prevention
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| cli/docker-labels.go | Adds globalLabelAllowList map with 23 allowed keys and filtering logic to block security-sensitive keys from being set via Docker labels |
| cli/docker_labels_test.go | Adds three test cases validating blocked keys, allowed keys, and end-to-end host job escalation prevention |
Address findings from 3 independent code reviews (Codex, Gemini Security, Gemini Full): - Remove save-folder, allow-remote-presets, trusted-preset-sources, and preset-cache-dir from the allow-list (arbitrary file write and remote config injection vectors) - Add bounds check for parts[1] to prevent panic on malformed labels with no dots (pre-existing DoS vector) - Include container name in security warning log for incident attribution - Update tests to cover newly blocked keys
Merging this PR will degrade performance by 40.19%
Performance Changes
Comparing 
|
Address Copilot review feedback: - Remove webhook-allowed-hosts from allow-list (SSRF risk) - Add comment explaining why webhook security keys are blocked - Expand blocked-keys test to cover all 16 security-sensitive keys (web-address, web-username, web-token-expiry, web-max-login-attempts, pprof-address, webhook-allowed-hosts)
|
🚀 Released in v0.21.0 Thank you for your contribution! 🙏 This is now available in the latest release. Please test and verify everything works as expected in your environment. If you encounter any issues, please open a new issue. |
Summary
globalLabelAllowListincli/docker-labels.gothat defines which global config keys may be set via Docker labels on service containersallow-host-jobs-from-labels, web auth settings, pprof,default-user) from being overridden via labels — they can only be set via config fileSECURITY: Blocked global config key ...warning logSecurity Context
Any container with
ofelia.service=truecould override any global config field via Docker labels, includingallow-host-jobs-from-labels(enabling host code execution) and web authentication settings. This PR closes that privilege escalation vector with a secure-by-default allow-list.See detailed analysis in #486.
Test Plan
TestGlobalLabelAllowListBlocksSecurityKeys— verifies 7 security-sensitive keys are blockedTestGlobalLabelAllowListPermitsSafeKeys— verifies 5 safe keys pass throughTestGlobalLabelAllowListPreventsHostJobEscalation— full attack scenario end-to-endgo test ./...)Closes #486