[APPSEC-9341] Enable configuring blocking response via Remote Configuration #3099
Conversation
github-actions
Bot
added
appsec
40bee94 to
d1bf510
Compare
GustavoCaso
changed the title
3f803a9 to
0d6c696
Compare
GustavoCaso
changed the title
Codecov Report
@@ Coverage Diff @@
## master #3099 +/- ##
========================================
Coverage 98.16% 98.16%
========================================
Files 1323 1325 +2
Lines 75237 75457 +220
Branches 3430 3455 +25
========================================
+ Hits 73855 74072 +217
- Misses 1382 1385 +3
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
bddb865 to
d498af6
Compare
lloeki
left a comment
lloeki
left a comment
There was a problem hiding this comment.
LGTM, just a Q.
I think for those catch/throw protocols we should (but not in this PR) move to instances instead of hashes and arrays, it should make things easier to follow (and properly type-checked)
| end | ||
|
|
||
| throw(Datadog::AppSec::Ext::INTERRUPT, [nil, [:block, event]]) if block | ||
| throw(Datadog::AppSec::Ext::INTERRUPT, [nil, [[:block, event]]]) if block |
There was a problem hiding this comment.
Curious about the double array here: was it missing before or is that an intended API change?
There was a problem hiding this comment.
In was missing 😨
There was a problem hiding this comment.
While missing the functionality still worked because we did not use the values
[:block, {}].any? {|action, _event| action == :block }
# => trueWe now care about the returned value with the code changes on the request middleware. We use find rather than any?
blocked_event = request_response.find { |action, _options| action == :block }Old code
blocked_event = [:block, {}].find {|action, _event| action == :block }
#=> :blockNew Code
blocked_event = [[:block, {}]].find {|action, _event| action == :block }
# => [:block, {}]
Absolutely 😄 I mentioned in the PR description
|
3 participants
What does this PR do?
Update ASM to be able to configure the blocking response via Remote Configuration.
Motivation:
Allow customers to change the blocking response via the DD UI
The PR includes multiple changes:
AppSec::Processor::Actionsapart from storing in memory, we need to merge existing actions with the ones coming from the Remote Configuration and merging.The logic for merging replaces existing actions with new ones coming from remote configuration and keeps the ones not updated.
AppSec::Response.negotiatemethod to account for the WAF result actions and the configured informationAdditional Notes:
I've removed unused returned values from the methods
Reactive::<Class>.subscribe, andReactive::<Class>.publishThe returned values from the reactive engine are plain arrays ex:
[:block, event]or[:monitor, event]those could be refactored into their own classesAppSec::Processor::Actions::BlockandAppSec::Processor::Actions::Monitor. I left that exercise to a future PR.How to test the change?
For Datadog employees:
credentials of any kind, I've requested a review from
@DataDog/security-design-and-guidance.Unsure? Have a question? Request a review!