From ca8425d712f8c49d934d0e43dc8135c3d83140d3 Mon Sep 17 00:00:00 2001 From: Matthew Keeler Date: Thu, 5 Sep 2024 15:53:35 -0400 Subject: [PATCH 1/3] fix: Add additional payload filter key validation Previously, we were allowing any non-empty string value to be provided as a payload filter key. However, customers can only create a filter key with a subset of characters. In an effort to warn them earlier but potentially invalid configurations, we are adding some basic key validation work as part of the start up sequence. --- contract-tests/service.rb | 1 + launchdarkly-server-sdk.gemspec | 1 + lib/ldclient-rb/config.rb | 2 +- lib/ldclient-rb/impl/util.rb | 15 ++++++++++ lib/ldclient-rb/util.rb | 5 ---- spec/impl/util_spec.rb | 52 +++++++++++++++++++++++++++++++++ 6 files changed, 70 insertions(+), 6 deletions(-) create mode 100644 spec/impl/util_spec.rb diff --git a/contract-tests/service.rb b/contract-tests/service.rb index 7db6e0f9..a1268c3c 100644 --- a/contract-tests/service.rb +++ b/contract-tests/service.rb @@ -31,6 +31,7 @@ 'all-flags-client-side-only', 'all-flags-details-only-for-tracked-flags', 'filtering', + 'filtering-strict', 'secure-mode-hash', 'tags', 'migrations', diff --git a/launchdarkly-server-sdk.gemspec b/launchdarkly-server-sdk.gemspec index 26c73c8d..44a1f49b 100644 --- a/launchdarkly-server-sdk.gemspec +++ b/launchdarkly-server-sdk.gemspec @@ -22,6 +22,7 @@ Gem::Specification.new do |spec| spec.required_ruby_version = ">= 3.0.0" spec.add_development_dependency "aws-sdk-dynamodb", "~> 1.57" + spec.add_development_dependency "libxml-ruby", "~> 5.0", ">= 5.0.3" spec.add_development_dependency "bundler", "2.2.33" spec.add_development_dependency "simplecov", "~> 0.21" spec.add_development_dependency "rspec", "~> 3.10" diff --git a/lib/ldclient-rb/config.rb b/lib/ldclient-rb/config.rb index 9fb0d8a4..91c929d4 100644 --- a/lib/ldclient-rb/config.rb +++ b/lib/ldclient-rb/config.rb @@ -77,7 +77,7 @@ def initialize(opts = {}) @socket_factory = opts[:socket_factory] @big_segments = opts[:big_segments] || BigSegmentsConfig.new(store: nil) @application = LaunchDarkly::Impl::Util.validate_application_info(opts[:application] || {}, @logger) - @payload_filter_key = opts[:payload_filter_key] + @payload_filter_key = LaunchDarkly::Impl::Util.validate_payload_filter_key(opts[:payload_filter_key] , @logger) @hooks = (opts[:hooks] || []).keep_if { |hook| hook.is_a? Interfaces::Hooks::Hook } @omit_anonymous_contexts = opts.has_key?(:omit_anonymous_contexts) && opts[:omit_anonymous_contexts] @data_source_update_sink = nil diff --git a/lib/ldclient-rb/impl/util.rb b/lib/ldclient-rb/impl/util.rb index 6c9801bb..2cca5c6c 100644 --- a/lib/ldclient-rb/impl/util.rb +++ b/lib/ldclient-rb/impl/util.rb @@ -75,6 +75,21 @@ def self.validate_application_info(app, logger) version: validate_application_value(app[:version], :version, logger), } end + + # + # @param value [String, nil] + # @param logger [Logger] + # @return [String, nil] + # + def self.validate_payload_filter_key(value, logger) + return nil if value.nil? + return value if value.is_a?(String) && /^[a-zA-Z0-9][a-zA-Z0-9._-]*$/.match?(value) + + logger.warn { + "Invalid payload filter configured, full environment will be fetched. Ensure the filter key is not empty and was copied correctly from LaunchDarkly settings." + } + nil + end end end end diff --git a/lib/ldclient-rb/util.rb b/lib/ldclient-rb/util.rb index a7d2cac0..5e8b40f2 100644 --- a/lib/ldclient-rb/util.rb +++ b/lib/ldclient-rb/util.rb @@ -77,11 +77,6 @@ module Util def self.add_payload_filter_key(uri, config) return uri if config.payload_filter_key.nil? - unless config.payload_filter_key.is_a?(String) && !config.payload_filter_key.empty? - config.logger.warn { "[LDClient] Filter key must be a non-empty string. No filtering will be applied." } - return uri - end - begin parsed = URI.parse(uri) new_query_params = URI.decode_www_form(String(parsed.query)) << ["filter", config.payload_filter_key] diff --git a/spec/impl/util_spec.rb b/spec/impl/util_spec.rb new file mode 100644 index 00000000..3d01d62a --- /dev/null +++ b/spec/impl/util_spec.rb @@ -0,0 +1,52 @@ +require "spec_helper" + +module LaunchDarkly + module Impl + describe "payload filter key validation" do + let(:logger) { double } + + it "silently discards nil" do + expect(logger).not_to receive(:warn) + expect(Util.validate_payload_filter_key(nil, logger)).to be_nil + end + + [true, 1, 1.0, [], {}].each do |value| + it "returns nil for invalid type #{value.class}" do + expect(logger).to receive(:warn) + expect(Util.validate_payload_filter_key(value, logger)).to be_nil + end + end + + [ + "", + "-cannot-start-with-dash", + "_cannot-start-with-underscore", + "-cannot-start-with-period", + "no spaces for you", + "org@special/characters", + ].each do |value| + it "returns nil for invalid value #{value}" do + expect(logger).to receive(:warn) + expect(Util.validate_payload_filter_key(value, logger)).to be_nil + end + end + + [ + "camelCase", + "snake_case", + "kebab-case", + "with.dots", + "with_underscores", + "with-hyphens", + "with1234numbers", + "with.many_1234-mixtures", + "1start-with-number", + ].each do |value| + it "passes for value #{value}" do + expect(logger).not_to receive(:warn) + expect(Util.validate_payload_filter_key(value, logger)).to eq(value) + end + end + end + end +end From 5a591c7b2aecea285ce1cd2fcd805b9ff3932a9f Mon Sep 17 00:00:00 2001 From: Matthew Keeler Date: Thu, 5 Sep 2024 21:10:03 -0400 Subject: [PATCH 2/3] consistent regex --- lib/ldclient-rb/impl/util.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ldclient-rb/impl/util.rb b/lib/ldclient-rb/impl/util.rb index 2cca5c6c..6768c601 100644 --- a/lib/ldclient-rb/impl/util.rb +++ b/lib/ldclient-rb/impl/util.rb @@ -83,7 +83,7 @@ def self.validate_application_info(app, logger) # def self.validate_payload_filter_key(value, logger) return nil if value.nil? - return value if value.is_a?(String) && /^[a-zA-Z0-9][a-zA-Z0-9._-]*$/.match?(value) + return value if value.is_a?(String) && /^[a-zA-Z0-9][._\-a-zA-Z0-9]*$/.match?(value) logger.warn { "Invalid payload filter configured, full environment will be fetched. Ensure the filter key is not empty and was copied correctly from LaunchDarkly settings." From 16b54fdbb6d43908f06a31937ea52734eaf88b75 Mon Sep 17 00:00:00 2001 From: Matthew Keeler Date: Fri, 6 Sep 2024 09:50:51 -0400 Subject: [PATCH 3/3] different xml library --- launchdarkly-server-sdk.gemspec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/launchdarkly-server-sdk.gemspec b/launchdarkly-server-sdk.gemspec index 44a1f49b..4f63c8b2 100644 --- a/launchdarkly-server-sdk.gemspec +++ b/launchdarkly-server-sdk.gemspec @@ -22,7 +22,7 @@ Gem::Specification.new do |spec| spec.required_ruby_version = ">= 3.0.0" spec.add_development_dependency "aws-sdk-dynamodb", "~> 1.57" - spec.add_development_dependency "libxml-ruby", "~> 5.0", ">= 5.0.3" + spec.add_development_dependency "rexml", "~> 3.3", ">= 3.3.7" spec.add_development_dependency "bundler", "2.2.33" spec.add_development_dependency "simplecov", "~> 0.21" spec.add_development_dependency "rspec", "~> 3.10"