Skip to content

API Security integration#3685

Merged
hoolioh merged 28 commits intomasterfrom
julio/api-security
Nov 22, 2023
Merged

API Security integration#3685
hoolioh merged 28 commits intomasterfrom
julio/api-security

Conversation

@hoolioh
Copy link
Copy Markdown
Contributor

@hoolioh hoolioh commented Oct 5, 2023

What does this PR do?

This PR add support for schema extraction when calling the waf. It does so by passing a new key inside waf.context.processor address.

Since addresses' values are not coerced in the bindings the values are
passed as is except for status code which the waf requires to be a
string.
@pr-commenter
Copy link
Copy Markdown

pr-commenter Bot commented Oct 5, 2023

Benchmarks

Benchmark execution time: 2023-11-22 08:45:09

Comparing candidate commit 7e67975 in PR branch julio/api-security with baseline commit 7abbcc5 in branch master.

Found 1 performance improvements and 0 performance regressions! Performance is the same for 520 metrics, 11 unstable metrics.

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

  • 🟩 max_rss_usage [-133.126MB; -115.366MB] or [-13.813%; -11.970%]

The address is held in the context so there is no need to pass it twice.
Because of this there is no need to save the sample decision in the
context.
@hoolioh hoolioh changed the title Julio/api security API Security integration Oct 6, 2023
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Oct 6, 2023

Overall package size

Self size: 5.58 MB
Deduped: 61.57 MB
No deduping: 62.33 MB

Dependency sizes

name version self size total size
@datadog/native-iast-taint-tracking 1.6.4 16.43 MB 16.44 MB
@datadog/native-appsec 5.0.0 15.16 MB 15.17 MB
@datadog/pprof 4.0.1 9.32 MB 10.16 MB
protobufjs 7.2.4 2.74 MB 6.52 MB
@datadog/native-iast-rewriter 2.2.1 2.27 MB 2.36 MB
@opentelemetry/core 1.14.0 872.87 kB 1.47 MB
@datadog/native-metrics 2.0.0 898.77 kB 1.3 MB
@opentelemetry/api 1.4.1 780.32 kB 780.32 kB
import-in-the-middle 1.4.2 41.4 kB 704.79 kB
pprof-format 2.0.7 588.12 kB 588.12 kB
msgpack-lite 0.1.26 201.16 kB 281.59 kB
opentracing 0.14.7 194.81 kB 194.81 kB
semver 7.5.4 93.4 kB 123.8 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
ipaddr.js 2.1.0 60.23 kB 60.23 kB
ignore 5.2.4 51.22 kB 51.22 kB
int64-buffer 0.1.10 49.18 kB 49.18 kB
istanbul-lib-coverage 3.2.0 29.34 kB 29.34 kB
lodash.uniq 4.5.0 25.01 kB 25.01 kB
limiter 1.1.5 23.17 kB 23.17 kB
dc-polyfill 0.1.2 22.77 kB 22.77 kB
retry 0.13.1 18.85 kB 18.85 kB
lodash.kebabcase 4.1.1 17.75 kB 17.75 kB
node-abort-controller 3.1.1 16.89 kB 16.89 kB
lodash.pick 4.4.0 16.33 kB 16.33 kB
jest-docblock 29.7.0 8.99 kB 12.76 kB
crypto-randomuuid 1.0.0 11.18 kB 11.18 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
Copy link
Copy Markdown

codecov Bot commented Oct 6, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (7abbcc5) 85.25% compared to head (7e67975) 85.14%.
Report is 7 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3685      +/-   ##
==========================================
- Coverage   85.25%   85.14%   -0.11%     
==========================================
  Files         230      228       -2     
  Lines        9482     9345     -137     
  Branches       33       33              
==========================================
- Hits         8084     7957     -127     
+ Misses       1398     1388      -10     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Copy Markdown
Collaborator

@uurien uurien left a comment

Choose a reason for hiding this comment

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

Can we create an extra test in index.express.plugin.spec.js doing a real call?

Comment thread packages/dd-trace/src/config.js Outdated
Comment thread packages/dd-trace/src/config.js Outdated
Comment thread packages/dd-trace/src/appsec/index.js Outdated
Comment thread packages/dd-trace/src/appsec/waf/waf_context_wrapper.js
Comment thread packages/dd-trace/src/appsec/index.js
Comment thread packages/dd-trace/src/appsec/index.js
derivatives.forEach((address) => {
for (const [key, value] of Object.entries(address)) {
if (key.includes('_dd.appsec.s.req')) {
const gzippedValue = zlib.gzipSync(JSON.stringify(value))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is there a way for us to not do this synchronously ? zlib will use a different thread and here the whole process will wait for that operation :/

Comment thread packages/dd-trace/src/appsec/index.js Outdated
Comment thread index.d.ts Outdated
Comment thread package.json Outdated
Copy link
Copy Markdown
Member

@simon-id simon-id left a comment

Choose a reason for hiding this comment

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

Maybe also add a statistical test that make sure the sampling is working as intended ?

Comment thread index.d.ts
Comment thread packages/dd-trace/src/config.js Outdated
Comment thread packages/dd-trace/src/config.js Outdated
Comment thread packages/dd-trace/src/config.js
Comment thread packages/dd-trace/src/appsec/waf/waf_context_wrapper.js
Comment thread packages/dd-trace/test/config.spec.js
Comment thread packages/dd-trace/test/appsec/reporter.spec.js Outdated
Comment thread packages/dd-trace/src/config.js Outdated
Comment thread packages/dd-trace/test/appsec/index.spec.js Outdated
Comment thread packages/dd-trace/test/appsec/index.spec.js Outdated
Comment thread packages/dd-trace/test/appsec/index.express.plugin.spec.js Outdated
@hoolioh hoolioh force-pushed the julio/api-security branch 4 times, most recently from 0a33761 to b0a755f Compare November 17, 2023 16:51
@hoolioh hoolioh merged commit 0a1f575 into master Nov 22, 2023
khanayan123 pushed a commit that referenced this pull request Dec 12, 2023
* Config variables.

* Add support for schema reporting.

* Add support for schema extraction on request addresses.

* Pass response's status code as string.
---------

Co-authored-by: Ugaitz Urien <ugaitz.urien@datadoghq.com>
Co-authored-by: simon-id <simon.id@datadoghq.com>
khanayan123 pushed a commit that referenced this pull request Dec 13, 2023
* Config variables.

* Add support for schema reporting.

* Add support for schema extraction on request addresses.

* Pass response's status code as string.
---------

Co-authored-by: Ugaitz Urien <ugaitz.urien@datadoghq.com>
Co-authored-by: simon-id <simon.id@datadoghq.com>
This was referenced Dec 13, 2023
khanayan123 pushed a commit that referenced this pull request Dec 14, 2023
* Config variables.

* Add support for schema reporting.

* Add support for schema extraction on request addresses.

* Pass response's status code as string.
---------

Co-authored-by: Ugaitz Urien <ugaitz.urien@datadoghq.com>
Co-authored-by: simon-id <simon.id@datadoghq.com>
khanayan123 pushed a commit that referenced this pull request Dec 14, 2023
* Config variables.

* Add support for schema reporting.

* Add support for schema extraction on request addresses.

* Pass response's status code as string.
---------

Co-authored-by: Ugaitz Urien <ugaitz.urien@datadoghq.com>
Co-authored-by: simon-id <simon.id@datadoghq.com>
khanayan123 pushed a commit that referenced this pull request Jan 2, 2024
* Config variables.

* Add support for schema reporting.

* Add support for schema extraction on request addresses.

* Pass response's status code as string.
---------

Co-authored-by: Ugaitz Urien <ugaitz.urien@datadoghq.com>
Co-authored-by: simon-id <simon.id@datadoghq.com>
khanayan123 pushed a commit that referenced this pull request Jan 2, 2024
* Config variables.

* Add support for schema reporting.

* Add support for schema extraction on request addresses.

* Pass response's status code as string.
---------

Co-authored-by: Ugaitz Urien <ugaitz.urien@datadoghq.com>
Co-authored-by: simon-id <simon.id@datadoghq.com>
khanayan123 pushed a commit that referenced this pull request Jan 2, 2024
* Config variables.

* Add support for schema reporting.

* Add support for schema extraction on request addresses.

* Pass response's status code as string.
---------

Co-authored-by: Ugaitz Urien <ugaitz.urien@datadoghq.com>
Co-authored-by: simon-id <simon.id@datadoghq.com>
khanayan123 pushed a commit that referenced this pull request Jan 2, 2024
* Config variables.

* Add support for schema reporting.

* Add support for schema extraction on request addresses.

* Pass response's status code as string.
---------

Co-authored-by: Ugaitz Urien <ugaitz.urien@datadoghq.com>
Co-authored-by: simon-id <simon.id@datadoghq.com>
@tlhunter tlhunter deleted the julio/api-security branch January 19, 2024 22:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants