From 4ea22472f860a47f8f667ec63212bc01607e721a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 13 Mar 2026 00:02:00 +0000 Subject: [PATCH 1/3] Initial plan From f463b207c6c9a60c13711eba63243ce33abaf99b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 13 Mar 2026 00:15:37 +0000 Subject: [PATCH 2/3] Add input sanitization for semicolon in text evaluation - Validate primary key value is not empty/nil after semicolon splitting - Strip whitespace from primary key value after splitting - Skip empty parts from trailing/consecutive semicolons - Add test fixtures and test cases for edge cases Co-authored-by: northrup <11050+northrup@users.noreply.github.com> --- .../data/groups/calculated/text.rb | 9 +++-- .../data/groups/calculated/text_spec.rb | 33 +++++++++++++++++++ .../ldap-config/text/leading-semicolon.txt | 2 ++ .../ldap-config/text/only-semicolon.txt | 2 ++ .../ldap-config/text/trailing-semicolon.txt | 2 ++ 5 files changed, 46 insertions(+), 2 deletions(-) create mode 100644 spec/unit/fixtures/ldap-config/text/leading-semicolon.txt create mode 100644 spec/unit/fixtures/ldap-config/text/only-semicolon.txt create mode 100644 spec/unit/fixtures/ldap-config/text/trailing-semicolon.txt diff --git a/lib/entitlements/data/groups/calculated/text.rb b/lib/entitlements/data/groups/calculated/text.rb index b78828a..dab65d0 100644 --- a/lib/entitlements/data/groups/calculated/text.rb +++ b/lib/entitlements/data/groups/calculated/text.rb @@ -316,8 +316,13 @@ def parsed_predicate(val) return { key: v } unless v.include?(";") parts = v.split(/\s*;\s*/) - op_hash = { key: parts.shift } - parts.each do |part| + primary_value = parts.shift + if primary_value.nil? || primary_value.strip.empty? + raise ArgumentError, "Rule Error: Empty value with semicolon predicate in #{filename}!" + end + + op_hash = { key: primary_value.strip } + parts.reject { |part| part.strip.empty? }.each do |part| if part =~ /\A(\w+)\s*=\s*(\S+)\s*\z/ predicate_keyword, predicate_value = Regexp.last_match(1), Regexp.last_match(2) unless SEMICOLON_PREDICATES.include?(predicate_keyword) diff --git a/spec/unit/entitlements/data/groups/calculated/text_spec.rb b/spec/unit/entitlements/data/groups/calculated/text_spec.rb index 538fd09..a349931 100644 --- a/spec/unit/entitlements/data/groups/calculated/text_spec.rb +++ b/spec/unit/entitlements/data/groups/calculated/text_spec.rb @@ -589,5 +589,38 @@ expect(result).to eq(answer) end end + + context "with a leading semicolon in the value" do + let(:filename) { fixture("ldap-config/text/leading-semicolon.txt") } + + it "raises an error about empty value" do + expect do + subject.send(:parsed_data) + end.to raise_error(ArgumentError, /Rule Error: Empty value with semicolon predicate in .+leading-semicolon.txt!/) + end + end + + context "with only a semicolon as the value" do + let(:filename) { fixture("ldap-config/text/only-semicolon.txt") } + + it "raises an error about empty value" do + expect do + subject.send(:parsed_data) + end.to raise_error(ArgumentError, /Rule Error: Empty value with semicolon predicate in .+only-semicolon.txt!/) + end + end + + context "with a trailing semicolon in the value" do + let(:filename) { fixture("ldap-config/text/trailing-semicolon.txt") } + + it "parses correctly ignoring trailing semicolon" do + result = subject.send(:parsed_data) + answer = { + "description" => {"=" => [{ key: "Trailing semicolon test" }], "!=" => [], "&=" => []}, + "username" => {"=" => [{ key: "blackmanx" }], "!=" => [], "&=" => []} + } + expect(result).to eq(answer) + end + end end end diff --git a/spec/unit/fixtures/ldap-config/text/leading-semicolon.txt b/spec/unit/fixtures/ldap-config/text/leading-semicolon.txt new file mode 100644 index 0000000..1da4752 --- /dev/null +++ b/spec/unit/fixtures/ldap-config/text/leading-semicolon.txt @@ -0,0 +1,2 @@ +description = Leading semicolon test +username = ;expiration = 2025-01-01 diff --git a/spec/unit/fixtures/ldap-config/text/only-semicolon.txt b/spec/unit/fixtures/ldap-config/text/only-semicolon.txt new file mode 100644 index 0000000..5e0b9e0 --- /dev/null +++ b/spec/unit/fixtures/ldap-config/text/only-semicolon.txt @@ -0,0 +1,2 @@ +description = Only semicolon test +username = ; diff --git a/spec/unit/fixtures/ldap-config/text/trailing-semicolon.txt b/spec/unit/fixtures/ldap-config/text/trailing-semicolon.txt new file mode 100644 index 0000000..f5a476b --- /dev/null +++ b/spec/unit/fixtures/ldap-config/text/trailing-semicolon.txt @@ -0,0 +1,2 @@ +description = Trailing semicolon test +username = blackmanx; From 0d11eab9c8659759784ee882988e2237be960648 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 13 Mar 2026 11:27:31 +0000 Subject: [PATCH 3/3] Refocus on description field semicolon handling per review feedback Revert parsed_predicate changes and instead add tests ensuring semicolons in the description free-form text area are preserved correctly and not treated as predicate separators. Co-authored-by: northrup <11050+northrup@users.noreply.github.com> --- .../data/groups/calculated/text.rb | 9 +--- .../data/groups/calculated/text_spec.rb | 51 +++++++------------ ...cription-with-predicate-like-semicolon.txt | 2 + .../description-with-semicolon-narrative.txt | 2 + .../ldap-config/text/leading-semicolon.txt | 2 - .../ldap-config/text/only-semicolon.txt | 2 - .../ldap-config/text/trailing-semicolon.txt | 2 - 7 files changed, 24 insertions(+), 46 deletions(-) create mode 100644 spec/unit/fixtures/ldap-config/text/description-with-predicate-like-semicolon.txt create mode 100644 spec/unit/fixtures/ldap-config/text/description-with-semicolon-narrative.txt delete mode 100644 spec/unit/fixtures/ldap-config/text/leading-semicolon.txt delete mode 100644 spec/unit/fixtures/ldap-config/text/only-semicolon.txt delete mode 100644 spec/unit/fixtures/ldap-config/text/trailing-semicolon.txt diff --git a/lib/entitlements/data/groups/calculated/text.rb b/lib/entitlements/data/groups/calculated/text.rb index dab65d0..b78828a 100644 --- a/lib/entitlements/data/groups/calculated/text.rb +++ b/lib/entitlements/data/groups/calculated/text.rb @@ -316,13 +316,8 @@ def parsed_predicate(val) return { key: v } unless v.include?(";") parts = v.split(/\s*;\s*/) - primary_value = parts.shift - if primary_value.nil? || primary_value.strip.empty? - raise ArgumentError, "Rule Error: Empty value with semicolon predicate in #{filename}!" - end - - op_hash = { key: primary_value.strip } - parts.reject { |part| part.strip.empty? }.each do |part| + op_hash = { key: parts.shift } + parts.each do |part| if part =~ /\A(\w+)\s*=\s*(\S+)\s*\z/ predicate_keyword, predicate_value = Regexp.last_match(1), Regexp.last_match(2) unless SEMICOLON_PREDICATES.include?(predicate_keyword) diff --git a/spec/unit/entitlements/data/groups/calculated/text_spec.rb b/spec/unit/entitlements/data/groups/calculated/text_spec.rb index a349931..d1f5913 100644 --- a/spec/unit/entitlements/data/groups/calculated/text_spec.rb +++ b/spec/unit/entitlements/data/groups/calculated/text_spec.rb @@ -60,6 +60,24 @@ subject.description end.to raise_error(RuntimeError, /description cannot use '!=' operator in .+not-equals-description.txt!/) end + + it "preserves semicolons in a free-form narrative description" do + filename = fixture("ldap-config/text/description-with-semicolon-narrative.txt") + subject = described_class.new(filename: filename) + expect(subject.description).to eq("This group manages auth; it also handles access control for the team") + end + + it "does not parse semicolons in description as predicates" do + filename = fixture("ldap-config/text/semicolons-in-description.txt") + subject = described_class.new(filename: filename) + expect(subject.description).to eq("the; description; can; have; semicolons") + end + + it "does not treat predicate-like text after semicolons in description as predicates" do + filename = fixture("ldap-config/text/description-with-predicate-like-semicolon.txt") + subject = described_class.new(filename: filename) + expect(subject.description).to eq("This group provides access to resources; expiration = 2099-12-31 is not a predicate here") + end end describe "#initialize_filters" do @@ -589,38 +607,5 @@ expect(result).to eq(answer) end end - - context "with a leading semicolon in the value" do - let(:filename) { fixture("ldap-config/text/leading-semicolon.txt") } - - it "raises an error about empty value" do - expect do - subject.send(:parsed_data) - end.to raise_error(ArgumentError, /Rule Error: Empty value with semicolon predicate in .+leading-semicolon.txt!/) - end - end - - context "with only a semicolon as the value" do - let(:filename) { fixture("ldap-config/text/only-semicolon.txt") } - - it "raises an error about empty value" do - expect do - subject.send(:parsed_data) - end.to raise_error(ArgumentError, /Rule Error: Empty value with semicolon predicate in .+only-semicolon.txt!/) - end - end - - context "with a trailing semicolon in the value" do - let(:filename) { fixture("ldap-config/text/trailing-semicolon.txt") } - - it "parses correctly ignoring trailing semicolon" do - result = subject.send(:parsed_data) - answer = { - "description" => {"=" => [{ key: "Trailing semicolon test" }], "!=" => [], "&=" => []}, - "username" => {"=" => [{ key: "blackmanx" }], "!=" => [], "&=" => []} - } - expect(result).to eq(answer) - end - end end end diff --git a/spec/unit/fixtures/ldap-config/text/description-with-predicate-like-semicolon.txt b/spec/unit/fixtures/ldap-config/text/description-with-predicate-like-semicolon.txt new file mode 100644 index 0000000..1267771 --- /dev/null +++ b/spec/unit/fixtures/ldap-config/text/description-with-predicate-like-semicolon.txt @@ -0,0 +1,2 @@ +description = This group provides access to resources; expiration = 2099-12-31 is not a predicate here +username = mainecoon diff --git a/spec/unit/fixtures/ldap-config/text/description-with-semicolon-narrative.txt b/spec/unit/fixtures/ldap-config/text/description-with-semicolon-narrative.txt new file mode 100644 index 0000000..31944c5 --- /dev/null +++ b/spec/unit/fixtures/ldap-config/text/description-with-semicolon-narrative.txt @@ -0,0 +1,2 @@ +description = This group manages auth; it also handles access control for the team +username = mainecoon diff --git a/spec/unit/fixtures/ldap-config/text/leading-semicolon.txt b/spec/unit/fixtures/ldap-config/text/leading-semicolon.txt deleted file mode 100644 index 1da4752..0000000 --- a/spec/unit/fixtures/ldap-config/text/leading-semicolon.txt +++ /dev/null @@ -1,2 +0,0 @@ -description = Leading semicolon test -username = ;expiration = 2025-01-01 diff --git a/spec/unit/fixtures/ldap-config/text/only-semicolon.txt b/spec/unit/fixtures/ldap-config/text/only-semicolon.txt deleted file mode 100644 index 5e0b9e0..0000000 --- a/spec/unit/fixtures/ldap-config/text/only-semicolon.txt +++ /dev/null @@ -1,2 +0,0 @@ -description = Only semicolon test -username = ; diff --git a/spec/unit/fixtures/ldap-config/text/trailing-semicolon.txt b/spec/unit/fixtures/ldap-config/text/trailing-semicolon.txt deleted file mode 100644 index f5a476b..0000000 --- a/spec/unit/fixtures/ldap-config/text/trailing-semicolon.txt +++ /dev/null @@ -1,2 +0,0 @@ -description = Trailing semicolon test -username = blackmanx;