Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Suppress CWE-400 for node-sass:4.13.1 #9517

Merged
merged 1 commit into from Mar 16, 2020

Conversation

ccaominh
Copy link
Contributor

Description

The vulnerability is fixed in 4.13.1: sass/node-sass#2816 (comment)

But the dependency check plugin thinks its still broken as the affected/fixed versions has not been updated yet on Sonatype OSS Index: https://ossindex.sonatype.org/vuln/c97f4ae7-be1f-4f71-b238-7c095b126e74


This PR has:

  • been self-reviewed.
  • added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.

The vulnerability is fixed in 4.13.1:
sass/node-sass#2816 (comment)

But the dependency check plugin thinks its still broken as the
affected/fixed versions has not been updated yet on Sonatype OSS Index:
https://ossindex.sonatype.org/vuln/c97f4ae7-be1f-4f71-b238-7c095b126e74
@codecov-io
Copy link

Codecov Report

Merging #9517 into master will increase coverage by 0.92%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #9517      +/-   ##
============================================
+ Coverage     63.58%   64.51%   +0.92%     
  Complexity    23569    23569              
============================================
  Files          3054     2936     -118     
  Lines        126200   119976    -6224     
  Branches      17453    16070    -1383     
============================================
- Hits          80249    77404    -2845     
+ Misses        38754    35592    -3162     
+ Partials       7197     6980     -217
Impacted Files Coverage Δ Complexity Δ
...tions/spatial/split/LinearGutmanSplitStrategy.java 91.89% <0%> (-8.11%) 9% <0%> (-1%)
.../apache/druid/client/BatchServerInventoryView.java 80.76% <0%> (-5.77%) 10% <0%> (-1%)
...main/java/org/apache/druid/client/DruidServer.java 74.68% <0%> (-2.54%) 33% <0%> (-1%)
...e/druid/concurrent/ConcurrentAwaitableCounter.java 67.5% <0%> (-2.5%) 11% <0%> (-1%)
...druid/server/coordinator/CuratorLoadQueuePeon.java 76.71% <0%> (-2.06%) 21% <0%> (-1%)
...uid/curator/inventory/CuratorInventoryManager.java 65.68% <0%> (-1.97%) 9% <0%> (ø)
.../druid/indexing/kinesis/KinesisRecordSupplier.java 54.43% <0%> (-1.59%) 33% <0%> (ø)
...egment/loading/SegmentLoaderLocalCacheManager.java 85.92% <0%> (-1.49%) 32% <0%> (-1%)
...uid/client/AbstractCuratorServerInventoryView.java 55.55% <0%> (-1.02%) 16% <0%> (ø)
.../druid/java/util/emitter/core/HttpPostEmitter.java 76.76% <0%> (-0.76%) 45% <0%> (ø)
... and 124 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6afd55c...c1abaa3. Read the comment docs.

@ccaominh
Copy link
Contributor Author

I'm not sure why codecov is commenting on PRs since that should be disabled: https://github.com/apache/druid/blob/master/.codecov.yml#L24

Clearly the coverage report is inaccurate since this PR did not change any source code.

Copy link
Member

@clintropolis clintropolis left a comment

Choose a reason for hiding this comment

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

thanks for fixing; I think we need to come up with a process to periodically review these suppressions, though I have no idea what that looks like

@ccaominh ccaominh merged commit 100d587 into apache:master Mar 16, 2020
@ccaominh ccaominh deleted the suppress-cwe-400 branch March 16, 2020 17:12
@jihoonson jihoonson added this to the 0.18.0 milestone Mar 26, 2020
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.

None yet

4 participants