Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,12 @@ Inspired by [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).

Note: this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [0.20.0] - Unreleased
### Added
- Added regular expressions capabilities to targeting. See [README.md](README.md#Regexp-Targeting) for usage examples.
### Fixed
- Use `Pysch.safe_load_file` over `Pysch.load_file` with `Regexp` as only permitted class.

## [0.19.0] - 2021-07-09
### Fixed
- Fixed a bug where accessing a setting that is a `Hash` defined across multiple file entries
Expand Down
10 changes: 5 additions & 5 deletions Gemfile.lock
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
PATH
remote: .
specs:
process_settings (0.19.0)
process_settings (0.20.0.pre.2)
activesupport (>= 4.2, < 7)
json
listen (~> 3.0)
Expand All @@ -26,11 +26,11 @@ GEM
concurrent-ruby (1.1.7)
diff-lcs (1.3)
docile (1.4.0)
ffi (1.15.3)
ffi (1.15.5)
i18n (1.8.5)
concurrent-ruby (~> 1.0)
json (2.1.0)
listen (3.5.1)
json (2.6.1)
listen (3.7.1)
rb-fsevent (~> 0.10, >= 0.10.3)
rb-inotify (~> 0.9, >= 0.9.10)
method_source (0.9.0)
Expand All @@ -45,7 +45,7 @@ GEM
psych (3.3.2)
rainbow (3.0.0)
rake (12.3.3)
rb-fsevent (0.11.0)
rb-fsevent (0.11.1)
rb-inotify (0.10.1)
ffi (~> 1.0)
rspec (3.8.0)
Expand Down
8 changes: 8 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,14 @@ target:
```
This will be applied in any process that has (`service_name == "frontend"` OR `service_name == "auth"`) AND `datacenter == "AWS-US-EAST-1"`.

### Regexp Targeting
To provide a regular expression for targeting, use the ruby regex keyword `!ruby/regexp` followed by your regular expression. For example:
```
target:
service_name: !ruby/regexp /frontend-\d/

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoa! This is really cool to not preclude slashes around strings. In the past we've been nervous about using this YAML ruby class support because it has a much wider surface area for security vulnerabilities (since this Ruby class will be invoked). That doesn't seem like a big deal here, but we might want to get an opinion from someone with a security background.

@ttstarck ttstarck Mar 25, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ttstarck ttstarck Mar 25, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ColinDKelley @jebentier It looks like we technically are already open to this potential vulnerability! I can switch how we are parsing yaml to use Pysch.safe_load_file and specify allowing only Regexp as a permitted class besides the default permitted classes

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See

json_doc = Psych.load_file(file_path)

@ColinDKelley ColinDKelley Mar 25, 2022

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point--we are already vulnerable! And because the process_settings are set by developers, I think we'll be fine? Worth mentioning that to in the request for a security sign-off. (It's the cases where raw YAML is parsed from the outside world that have caused serious security problems. And that's why safe_load_file was invented.)

And if it doesn't add much scope, I really like your suggestion of switching over to Pysch.safe_load_file here with only Regexp permitted. But if that takes more than a couple minutes, feel free to create a ticket for Octothorpe instead.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added: c6d10fe

```
This will be applied in any process that has `service_name` that has `frontend-` followed with a number. As an example this will match `"frontend-1"`

### Precedence
The settings YAML files are always combined in alphabetical order by file path. Later settings take precedence over the earlier ones.

Expand Down
12 changes: 12 additions & 0 deletions lib/process_settings/target.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,12 @@ def with_static_context(target_value, static_context_hash)
with_static_context_hash(target_value, static_context_hash)
when true, false
!target_value == !static_context_hash
when Regexp
if static_context_hash.is_a?(String)
static_context_hash.match?(target_value)
else
static_context_hash == target_value
end
else
target_value == static_context_hash
end
Expand Down Expand Up @@ -90,6 +96,12 @@ def target_key_matches?(target_value, context_hash)
end
when true, false
target_value
when Regexp
if context_hash.is_a?(String)
context_hash.match?(target_value)
else
context_hash == target_value
end
else
target_value == context_hash
end
Expand Down
2 changes: 1 addition & 1 deletion lib/process_settings/targeted_settings.rb
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ def from_array(settings_array, only_meta: false)
end

def from_file(file_path, only_meta: false)
json_doc = Psych.load_file(file_path)
json_doc = Psych.safe_load_file(file_path, permitted_classes: [Regexp])
from_array(json_doc, only_meta: only_meta)
end
end
Expand Down
2 changes: 1 addition & 1 deletion lib/process_settings/version.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# frozen_string_literal: true

module ProcessSettings
VERSION = '0.19.0'
VERSION = '0.20.0.pre.2'
end
5 changes: 5 additions & 0 deletions spec/bin/combine_process_settings_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,11 @@
max_recording_seconds: 600
answer_odds: 100
status_change_min_days:
- filename: regex_target.yml
target:
service: !ruby/regexp /frontend/
settings:
test_setting: 100
- filename: stop_incoming_requests.yml
target:
region: east
Expand Down
47 changes: 47 additions & 0 deletions spec/fixtures/production/combined_process_settings-20.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
---
#
# Don't edit this file directly! It was generated by combine_process_settings from the files in staging/settings/.
#
- filename: honeypot.yml
settings:
honeypot:
max_recording_seconds: 600
answer_odds: 100
status_change_min_days:
- filename: regex_target.yml
target:
service: !ruby/regexp /frontend/
settings:
test_setting: 100
- filename: telecom/log_level.yml
target:
app: telecom
settings:
logging:
level: debug
- filename: telecom/stop_incoming_requests.yml
target:
app: telecom
region: west
settings:
incoming_requests: 0
- filename: telecom/debug_sip_private_caller_id.yml
target:
app: telecom
region: west
caller_id:
- "+18053334444"
- "+12755554321"
- "+18052223344"
settings:
log_stream:
sip: caller_id_privacy
- filename: cca/tech-1234_call_counts_drift_investigation.yml
target:
app: ccn
settings:
call_counts:
complete_sync_seconds: 60
- meta:
version: 20
END: true
5 changes: 5 additions & 0 deletions spec/fixtures/production/settings/regex_target.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
target:
service: !ruby/regexp /frontend/
settings:
test_setting: 100
150 changes: 150 additions & 0 deletions spec/lib/process_settings/target_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,81 @@
expect(process_target.target_key_matches?(context_hash)).to be_truthy
expect(process_target.target_key_matches?({})).to be_falsey
end

describe "for regex matching" do
subject { process_target.target_key_matches?(context_hash) }
let(:process_target) { described_class.new(target_hash) }
let(:context_hash) do
{
'service' => service,
'region' => 'east',
'cdr' => { 'caller' => '+18056807000' }
}
end

context "with target value is a regular expression" do
let(:target_hash) { { 'service' => /telecom/ } }

context "when context hash value matches" do
let(:service) { 'telecom-1' }

it { is_expected.to be_truthy }
end

context "with an empty context hash" do
let(:context_hash) { {} }
it { is_expected.to be_falsey }
end

context "with a non-matching context value" do
let(:service) { 'tele' }
it { is_expected.to be_falsey }
end
end

context "when target value is a regex as a string" do
let(:target_hash) { { 'service' => '/telecom/' } }

context "when context hash value has the string but not the slash" do
let(:service) { 'telecom' }

it { is_expected.to be_falsey }
end

context "when context hash value exactly matches the string target value" do
let(:service) { '/telecom/' }
it { is_expected.to be_truthy }
end
end

context "when target value is valid and also has embedded slashes" do
let(:target_hash) { { 'service' => /tmp\/dir\/log/ } }

context "when context hash value matches the target" do
let(:service) { 'tmp/dir/log/service-log.txt' }
it { is_expected.to be_truthy }
end

context "when context hash only has whats between the embedded slashes" do
let(:service) { 'dir' }
it { is_expected.to be_falsey }
end
end

context "when context hash and target hash don't match keys" do
let(:context_hash) do
{
'service' => { 'name' => service },
'region' => 'east',
'cdr' => { 'caller' => '+18056807000' }
}
end
let(:target_hash) { { 'service' => /telecom/ } }
let(:service) { 'telecom-1' }

it { is_expected.to be_falsey }
end
end
end

describe "#with_static_context" do
Expand Down Expand Up @@ -333,6 +408,81 @@
it { should eq(target_hash) }
end
end

describe "for regex matching" do
subject { process_target.with_static_context(context_hash).json_doc }
let(:process_target) { described_class.new(target_hash) }
let(:context_hash) do
{
'service' => service,
'region' => 'east',
'cdr' => { 'caller' => '+18056807000' }
}
end

context "with target value is a regular expression" do
let(:target_hash) { { 'service' => /telecom/ } }

context "when context hash value matches" do
let(:service) { 'telecom-1' }

it { is_expected.to be_truthy }
end

context "with an empty context hash" do
let(:context_hash) { {} }
it { is_expected.to eq(target_hash) }
end

context "with a non-matching context value" do
let(:service) { 'tele' }
it { is_expected.to be_falsey }
end
end

context "when target value is a regex as a string" do
let(:target_hash) { { 'service' => '/telecom/' } }

context "when context hash value has the string but not the slash" do
let(:service) { 'telecom' }

it { is_expected.to be_falsey }
end

context "when context hash value exactly matches the string target value" do
let(:service) { '/telecom/' }
it { is_expected.to be_truthy }
end
end

context "when target value is valid and also has embedded slashes" do
let(:target_hash) { { 'service' => /tmp\/dir\/log/ } }

context "when context hash value matches the target" do
let(:service) { 'tmp/dir/log/service-log.txt' }
it { is_expected.to be_truthy }
end

context "when context hash only has whats between the embedded slashes" do
let(:service) { 'dir' }
it { is_expected.to be_falsey }
end
end

context "when context hash and target hash don't match keys" do
let(:context_hash) do
{
'service' => { 'name' => service },
'region' => 'east',
'cdr' => { 'caller' => '+18056807000' }
}
end
let(:target_hash) { { 'service' => /telecom/ } }
let(:service) { 'telecom-1' }

it { is_expected.to be_falsey }
end
end
end

describe "class methods" do
Expand Down
15 changes: 15 additions & 0 deletions spec/lib/process_settings/targeted_settings_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,21 @@
end
end

context "with ruby's special regular expression keyword" do
let(:file_path) { File.expand_path("../../fixtures/production/combined_process_settings-20.yml", __dir__) }

describe ".from_file" do
it "reads from yaml file and properly converts the special keyword and expression in the file into a ruby Regexp object" do
targeted_settings = described_class.from_file(file_path)

regex_target_setting = targeted_settings.targeted_settings_array[1]
expect(regex_target_setting.filename).to eq("regex_target.yml")
expect(regex_target_setting.target.json_doc).to eq({ "service" => /frontend/ })
expect(regex_target_setting.settings.json_doc).to eq({ "test_setting" => 100 })
end
end
end

describe ".from_array" do
it "delegates" do
target_and_settings = described_class.from_array(TARGETED_SETTINGS)
Expand Down