Disable appsec if missing or malformed user custom rules file #2969

simon-id · 2023-04-04T14:33:47Z

What does this PR do?

Restores the previous behavior of disabling appsec if user custom rules could not be loaded because a missing file or malformed json.
Remove esbuild support for node v14

Motivation

If user custom rules couldn't be loaded appsec will enabled with the default recommended rules.

Plugin Checklist

Additional Notes

github-actions · 2023-04-04T14:34:41Z

Overall package size

Self size: 4.02 MB
Deduped: 59.61 MB
No deduping: 59.65 MB

Dependency sizes

name	version	self size	total size
@datadog/native-iast-taint-tracking	1.3.1	13.47 MB	13.47 MB
@datadog/pprof	2.1.0	12.31 MB	13.2 MB
@datadog/native-appsec	2.0.0	12.33 MB	12.34 MB
@datadog/native-metrics	1.5.0	7.1 MB	7.11 MB
protobufjs	7.1.2	2.76 MB	6.55 MB
@datadog/native-iast-rewriter	2.0.1	2.09 MB	2.1 MB
opentracing	0.14.7	194.81 kB	194.81 kB
@datadog/sketches-js	2.1.0	109.9 kB	109.9 kB
lodash.sortby	4.7.0	75.76 kB	75.76 kB
lru-cache	7.14.0	74.95 kB	74.95 kB
semver	5.7.1	61.58 kB	61.58 kB
ipaddr.js	2.0.1	59.52 kB	59.52 kB
ignore	5.2.0	48.87 kB	48.87 kB
import-in-the-middle	1.3.4	32.7 kB	37.17 kB
istanbul-lib-coverage	3.2.0	29.34 kB	29.34 kB
retry	0.10.1	27.44 kB	27.44 kB
lodash.uniq	4.5.0	25.01 kB	25.01 kB
limiter	1.1.5	23.17 kB	23.17 kB
lodash.kebabcase	4.1.1	17.75 kB	17.75 kB
lodash.pick	4.4.0	16.33 kB	16.33 kB
node-abort-controller	3.0.1	14.33 kB	14.33 kB
crypto-randomuuid	1.0.0	11.18 kB	11.18 kB
diagnostics_channel	1.1.0	7.07 kB	7.07 kB
path-to-regexp	0.1.7	6.78 kB	6.78 kB
koalas	1.0.2	6.47 kB	6.47 kB
methods	1.1.2	5.29 kB	5.29 kB
module-details-from-path	1.0.3	4.47 kB	4.47 kB

_{🤖 This report was automatically generated by heaviest-objects-in-the-universe}

codecov · 2023-04-04T14:39:22Z

Codecov Report

Merging #2969 (13f6f78) into master (79fc8ec) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #2969      +/-   ##
==========================================
- Coverage   87.64%   87.63%   -0.01%     
==========================================
  Files         329      329              
  Lines       11747    11746       -1     
  Branches       33       33              
==========================================
- Hits        10296    10294       -2     
- Misses       1451     1452       +1

Impacted Files	Coverage Δ
packages/dd-trace/src/config.js	`98.81% <ø> (ø)`
packages/dd-trace/src/appsec/index.js	`100.00% <100.00%> (ø)`

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

pr-commenter · 2023-04-04T14:49:41Z

Benchmarks

Comparing candidate commit 1b735ae in PR branch simon-id-patch-10 with baseline commit 79fc8ec in branch master.

Found 2 performance improvements and 8 performance regressions! Performance is the same for 656 metrics, 42 unstable metrics.

scenario:appsec-appsec-enabled-with-attacks-14

🟥 cpu_usage_percentage [+8.106%; +9.229%]
🟩 execution_time [-41.236ms; -36.030ms] or [-9.598%; -8.386%]

scenario:spans-finish-immediately-16

🟥 max_rss_usage [+4.100KB; +4.178KB] or [+8.517%; +8.680%]

scenario:plugin-graphql-with-depth-off-16

🟥 max_rss_usage [+111.400KB; +123.632KB] or [+13.149%; +14.593%]

scenario:plugin-q-with-tracer-16

🟥 cpu_user_time [+51.606ms; +65.602ms] or [+6.415%; +8.155%]
🟥 execution_time [+58.386ms; +66.973ms] or [+6.642%; +7.619%]

scenario:plugin-dns-with-tracer-16

🟥 max_rss_usage [+3.880KB; +4.195KB] or [+7.565%; +8.178%]

scenario:appsec-appsec-enabled-with-attacks-16

🟥 cpu_usage_percentage [+8.484%; +9.349%]
🟩 execution_time [-44.357ms; -40.223ms] or [-10.311%; -9.350%]

scenario:plugin-dns-with-tracer-18

🟥 max_rss_usage [+3.554KB; +3.904KB] or [+6.086%; +6.685%]

…rse user rules file

tlhunter · 2023-04-04T21:47:55Z

This looks good to me and I'm fine with merging it to master. The three failing tests should be unrelated.

tlhunter · 2023-04-04T23:14:35Z

There's a bunch going on in this PR by now but the goal is to unblock a release:

Qard · 2023-04-05T00:19:43Z

README.md

@@ -202,7 +202,7 @@ For these reasons it's necessary to have custom-built bundler plugins. Such plug

 ### Esbuild Support

-This library provides experimental esbuild support in the form of an esbuild plugin, and requires at least Node.js v14.17. To use the plugin, make sure you have `dd-trace@3+` installed, and then require the `dd-trace/esbuild` module when building your bundle.
+This library provides experimental esbuild support in the form of an esbuild plugin, and currently requires at least Node.js v16.17 or v18.7. To use the plugin, make sure you have `dd-trace@3+` installed, and then require the `dd-trace/esbuild` module when building your bundle.


The support range for this changed somehow? Just a note to ensure we update the corp docs too.

simon-id · 2023-04-05T08:00:14Z

Please update the title and description of the PR

simon-id

LGTM

* Update config.js * Add test to ensure appsec rules are undefined if could not load or parse user rules file * trying to allow non v14 integration tests to run * print node version when esbuild test fails * esbuild refactor broke support for certain node.js versions * display git status when cherry pick fails --------- Co-authored-by: Igor Unanua <igor.unanua@datadoghq.com> Co-authored-by: Thomas Hunter II <tlhunter@datadog.com>

iunanua added the semver-patch label Apr 4, 2023

Update config.js

3b2c9fa

iunanua force-pushed the simon-id-patch-10 branch from 73cb5b2 to 3b2c9fa Compare April 4, 2023 16:30

iunanua and others added 4 commits April 4, 2023 20:01

Add test to ensure appsec rules are undefined if could not load or pa…

94f5474

…rse user rules file

trying to allow non v14 integration tests to run

307e105

print node version when esbuild test fails

15d5782

esbuild refactor broke support for certain node.js versions

1b735ae

tlhunter force-pushed the simon-id-patch-10 branch from da9a311 to 1b735ae Compare April 4, 2023 19:25

display git status when cherry pick fails

13f6f78

tlhunter approved these changes Apr 4, 2023

View reviewed changes

tlhunter marked this pull request as ready for review April 4, 2023 23:07

tlhunter requested review from a team as code owners April 4, 2023 23:07

Qard approved these changes Apr 5, 2023

View reviewed changes

iunanua approved these changes Apr 5, 2023

View reviewed changes

simon-id commented Apr 5, 2023

View reviewed changes

iunanua changed the title ~~Update config.js~~ Disable appsec if missing or malformed user custom rules file Apr 5, 2023

iunanua merged commit c2df06e into master Apr 5, 2023

tlhunter deleted the simon-id-patch-10 branch December 19, 2023 16:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Disable appsec if missing or malformed user custom rules file #2969

Disable appsec if missing or malformed user custom rules file #2969

simon-id commented Apr 4, 2023 •

edited by iunanua

Loading

github-actions bot commented Apr 4, 2023 •

edited

Loading

codecov bot commented Apr 4, 2023 •

edited

Loading

pr-commenter bot commented Apr 4, 2023 •

edited

Loading

tlhunter commented Apr 4, 2023

tlhunter commented Apr 4, 2023

Qard Apr 5, 2023

simon-id commented Apr 5, 2023

simon-id left a comment

Disable appsec if missing or malformed user custom rules file #2969

Disable appsec if missing or malformed user custom rules file #2969

Conversation

simon-id commented Apr 4, 2023 • edited by iunanua Loading

What does this PR do?

Motivation

Plugin Checklist

Additional Notes

github-actions bot commented Apr 4, 2023 • edited Loading

Overall package size

Dependency sizes

codecov bot commented Apr 4, 2023 • edited Loading

Codecov Report

pr-commenter bot commented Apr 4, 2023 • edited Loading

Benchmarks

scenario:appsec-appsec-enabled-with-attacks-14

scenario:spans-finish-immediately-16

scenario:plugin-graphql-with-depth-off-16

scenario:plugin-q-with-tracer-16

scenario:plugin-dns-with-tracer-16

scenario:appsec-appsec-enabled-with-attacks-16

scenario:plugin-dns-with-tracer-18

tlhunter commented Apr 4, 2023

tlhunter commented Apr 4, 2023

Qard Apr 5, 2023

Choose a reason for hiding this comment

simon-id commented Apr 5, 2023

simon-id left a comment

Choose a reason for hiding this comment

simon-id commented Apr 4, 2023 •

edited by iunanua

Loading

github-actions bot commented Apr 4, 2023 •

edited

Loading

codecov bot commented Apr 4, 2023 •

edited

Loading

pr-commenter bot commented Apr 4, 2023 •

edited

Loading