Gangams/fix geneva configmap injection issues - #1755
Closed
Ganga Mahesh Siddem (ganga1980) wants to merge 9 commits into
Closed
Gangams/fix geneva configmap injection issues#1755Ganga Mahesh Siddem (ganga1980) wants to merge 9 commits into
Ganga Mahesh Siddem (ganga1980) wants to merge 9 commits into
Conversation
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
Contributor
|
This PR is stale because it has been open 7 days with no activity. Remove stale label or comment or this will be closed in 5 days. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces significant security and robustness improvements to the way configuration values are handled and written to environment variable files in the installer scripts. The main changes include the introduction of a new
ConfigValueSanitizermodule for safe quoting and validation of config values, stricter validation of attacker-controlled inputs, and simplification of shell script environment variable sourcing.Security and Input Validation Improvements:
ConfigValueSanitizermodule (ConfigValueSanitizer.rb) that provides methods for safely quoting shell values, sanitizing single-line values for Windows, and validating values against allowed patterns.ConfigValueSanitizer.valid?for all attacker-controllable values and provide clear error messages when invalid values are detected. [1] [2]Safe Emission of Environment Variables:
tomlparser.rbandtomlparser-geneva-config.rbto useConfigValueSanitizer.shell_quotefor shell files andConfigValueSanitizer.single_linefor Windows files, preventing injection of shell metacharacters or newlines. [1] [2] [3] [4]Shell Script Simplification:
~/.bashrcinmain.shby replacingwhile readloops with directcatredirection, improving performance and reliability. [1] [2] [3] [4] [5]Installer Packaging:
ConfigValueSanitizer.rbto the installer datafiles for deployment.These changes collectively harden the agent installation process against configmap injection attacks and ensure that only valid, safe values are emitted to shell environments.
Unit test runs
test results ...
sshadmin@DESKTOP-U2I87MQ:~/Docker-Provider$ for f in test/unit-tests/ruby/*_test.rb; do ruby "$f" -v; done
Run options: -v --seed 17144
Running:
ConfigValueSanitizerTests#test_sourcing_quoted_values_does_not_execute_injected_commands = 0.01 s = .
ConfigValueSanitizerTests#test_shell_quote_handles_empty_and_nil_values = 0.00 s = .
ConfigValueSanitizerTests#test_shell_quote_wraps_value_in_single_quotes = 0.00 s = .
ConfigValueSanitizerTests#test_shell_quote_escapes_embedded_single_quote = 0.00 s = .
ConfigValueSanitizerTests#test_single_line_replaces_line_breaks = 0.00 s = .
ConfigValueSanitizerTests#test_valid_accepts_only_full_matches_of_non_empty_strings = 0.00 s = .
Finished in 0.105053s, 57.1139 runs/s, 199.8986 assertions/s.
6 runs, 21 assertions, 0 failures, 0 errors, 0 skips
Run options: -v --seed 37370
Running:
TomlParserGenevaConfigTests#test_geneva_environment_rules = 1.27 s = .
TomlParserGenevaConfigTests#test_auth_id_supports_managed_identity_resource_id = 0.13 s = .
TomlParserGenevaConfigTests#test_valid_geneva_config_is_written_quoted = 0.18 s = .
TomlParserGenevaConfigTests#test_invalid_namespaces_are_filtered_out = 0.15 s = .
TomlParserGenevaConfigTests#test_geneva_account_name_rules = 1.11 s = .
TomlParserGenevaConfigTests#test_environment_with_shell_metacharacters_is_rejected = 0.16 s = .
TomlParserGenevaConfigTests#test_geneva_region_rules = 0.81 s = .
TomlParserGenevaConfigTests#test_invalid_windows_namespace_rejects_geneva_config = 0.14 s = .
TomlParserGenevaConfigTests#test_command_substitution_in_geneva_values_is_rejected = 0.18 s = .
TomlParserGenevaConfigTests#test_geneva_namespace_rules = 1.08 s = .
TomlParserGenevaConfigTests#test_namespace_prefix_wildcard_is_still_supported = 0.12 s = .
Finished in 5.436478s, 2.0234 runs/s, 18.3943 assertions/s.
11 runs, 100 assertions, 0 failures, 0 errors, 0 skips
Run options: -v --seed 28425
Running:
TomlParserLogCollectionTests#test_container_log_settings_with_shell_metacharacters_do_not_execute = 0.16 s = .
TomlParserLogCollectionTests#test_excluded_namespace_with_shell_metacharacters_does_not_execute = 0.13 s = .
TomlParserLogCollectionTests#test_excluded_namespaces_are_written_quoted = 0.14 s = .
Finished in 0.542492s, 5.5300 runs/s, 18.4334 assertions/s.
3 runs, 10 assertions, 0 failures, 0 errors, 0 skips