-
Notifications
You must be signed in to change notification settings - Fork 54
CHEF-29762 add chef-ice package_manger support to generated scripts #417
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
Signed-off-by: Corey Hemminger <hemminger@hotmail.com>
Signed-off-by: Corey Hemminger <hemminger@hotmail.com>
3b0e2c3 to
1969e4e
Compare
Signed-off-by: Corey Hemminger <hemminger@hotmail.com>
Signed-off-by: Corey Hemminger <hemminger@hotmail.com>
…will be source of truth Signed-off-by: Corey Hemminger <hemminger@hotmail.com>
Signed-off-by: Corey Hemminger <hemminger@hotmail.com>
…ssing license_id option Signed-off-by: Corey Hemminger <hemminger@hotmail.com>
Signed-off-by: Corey Hemminger <hemminger@hotmail.com>
| # @return [String] the omnibus URL (commercial/trial or standard omnitruck) | ||
| # @api private | ||
| def omnibus_url_for_license | ||
| return omnibus_url if license_id.nil? || license_id.to_s.empty? || !omnibus_url.include?("omnitruck.chef.io") |
Check failure
Code scanning / CodeQL
Incomplete URL substring sanitization High
omnitruck.chef.io
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 33 minutes ago
In general, the problem should be fixed by parsing the URL and validating the host component explicitly, rather than using a substring check on the entire URL string. That means using URI.parse (from Ruby’s standard library) to get uri.host, and then comparing that to an allowlist ("omnitruck.chef.io" and any officially supported aliases) or a strict pattern, instead of using .include?.
In this specific file, on line 227 we should replace !omnibus_url.include?("omnitruck.chef.io") with a small helper that safely checks the host of omnibus_url. To keep behavior consistent and localized:
- Add a private helper method in
ScriptGeneratorthat:- Uses
URI.parse(omnibus_url.to_s)to parse the URL. - Returns
trueonly ifuri.hostmatches"omnitruck.chef.io"(or, if appropriate for this project, matches a small set of official hosts). - Rescues
URI::InvalidURIErrorand returnsfalsein that case.
- Uses
- Update
omnibus_url_for_licenseto call this helper (e.g.,!omnitruck_host?(omnibus_url)instead of!omnibus_url.include?("omnitruck.chef.io")).
We only need to modify lib/mixlib/install/script_generator.rb, adding the helper method near the other private helpers (e.g., after metadata_endpoint_from_project) and updating the conditional in omnibus_url_for_license. URI is a standard Ruby library; since the file doesn’t currently require it explicitly, we should add require "uri" near the top alongside the other requires.
-
Copy modified line R24 -
Copy modified lines R224-R239 -
Copy modified line R244
| @@ -21,6 +21,7 @@ | ||
| require_relative "generator/powershell" | ||
| require_relative "dist" | ||
| require "cgi" | ||
| require "uri" | ||
|
|
||
| module Mixlib | ||
| class Install | ||
| @@ -220,11 +221,27 @@ | ||
| end | ||
| end | ||
|
|
||
| # Returns true if the given URL points to the official Omnitruck host. | ||
| # @param url [String] the URL to check | ||
| # @return [Boolean] whether the URL's host matches the Omnitruck host | ||
| # @api private | ||
| def omnitruck_host?(url) | ||
| return false if url.nil? || url.to_s.empty? | ||
|
|
||
| begin | ||
| uri = URI.parse(url.to_s) | ||
| host = uri.host | ||
| host == "omnitruck.chef.io" | ||
| rescue URI::InvalidURIError | ||
| false | ||
| end | ||
| end | ||
|
|
||
| # Returns the appropriate omnibus URL based on whether license_id is provided | ||
| # @return [String] the omnibus URL (commercial/trial or standard omnitruck) | ||
| # @api private | ||
| def omnibus_url_for_license | ||
| return omnibus_url if license_id.nil? || license_id.to_s.empty? || !omnibus_url.include?("omnitruck.chef.io") | ||
| return omnibus_url if license_id.nil? || license_id.to_s.empty? || !omnitruck_host?(omnibus_url) | ||
|
|
||
| # Determine if this is a trial or commercial license | ||
| base_url = if license_id.start_with?("free-", "trial-") |
|



Description
add chef-ice package_manger support to generated scripts
Types of changes
Checklist:
Gemfile.lockhas changed, I have used--conservativeto do it and included the full output in the Description above.