Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

chore(dgeni): update to latest dgeni and dgeni-packages #17141

Closed
wants to merge 2 commits into from

Conversation

Splaktar
Copy link
Member

@Splaktar Splaktar commented Jun 5, 2021

AngularJS is in LTS mode

We are no longer accepting changes that are not critical bug fixes into this project.
See https://blog.angular.io/stable-angularjs-and-long-term-support-7e077635ee9c for more detail.

Does this PR fix a regression since 1.7.0, a security flaw, or a problem caused by a new browser version?
Yes

What is the current behavior? (You can also link to an open issue here)

  • from: 389 vulnerabilities found - Severity: 259 Low | 28 Moderate | 98 High | 4 Critical

What is the new behavior (if this is a feature change)?

  • to: 300 vulnerabilities found - Severity: 189 Low | 28 Moderate | 79 High | 4 Critical

Does this PR introduce a breaking change?
No.

  • See the broken markdown rendering screen shots at the end
  • errorOnUnmatchedLinks used to pass but it no longer does, this is likely because of the
    failure of the markdown renderer to function

Please check if the PR fulfills these requirements

  • The commit message follows our guidelines
  • Fix/Feature: Docs have been added/updated
  • Fix/Feature: Tests have been added; existing tests pass

Other information:

Before

Screen Shot 2021-06-05 at 16 59 17

After

Screen Shot 2021-06-05 at 16 50 52

This same problem was observed in AngularJS Material in PR angular/material#11881.

Fixed rendering in a 2nd commit

chore(dgeni): fix rendering of docs markdown

  • this adds 1 High severity advisory back in by pinning at marked@0.3.6
  • 301 vulnerabilities found - Severity: 189 Low | 28 Moderate | 80 High | 4 Critical
  • in reality, there are many serious security issues with this old version of marked
  • but our docs don't render with the newer versions due to
    Regression: since 0.28.0, marked filter does not render markdown in HTML blocks dgeni-packages#310
  • another downside is that this forces firebase-tools to use an old, vulnerable
    version of marked for parsing console output

- from: 389 vulnerabilities found - Severity: 259 Low | 28 Moderate | 98 High | 4 Critical
- to: 300 vulnerabilities found - Severity: 189 Low | 28 Moderate | 79 High | 4 Critical
- `errorOnUnmatchedLinks` used to pass but it no longer does, this is likely because of the
  failure of the markdown renderer to function
@google-cla google-cla bot added the cla: yes label Jun 5, 2021
@Splaktar
Copy link
Member Author

Splaktar commented Jun 5, 2021

This appears to be caused only by the update to dgeni-packages@0.28.0, which has breaking changes related to marked, as keeping dgeni@0.4.9 still reproduces the issue.

@Splaktar
Copy link
Member Author

Splaktar commented Jun 5, 2021

I wrote up the issue that I was able to isolate a bit better via this PR in angular/dgeni-packages#310.

@Splaktar
Copy link
Member Author

Splaktar commented Jun 6, 2021

I've opened markedjs/marked#2085 for this as well.

- this adds 1 High severity advisory back in by pinning at `marked@0.3.6`
- 301 vulnerabilities found - Severity: 189 Low | 28 Moderate | 80 High | 4 Critical
- in reality, there are many serious security issues with this old version of `marked`
- but our docs don't render with the newer versions due to
  angular/dgeni-packages#310
- another downside is that this forces `firebase-tools` to use an old, vulnerable
  version of marked for parsing console output
@Splaktar
Copy link
Member Author

Splaktar commented Jun 6, 2021

Even with the concerns above, this now fixes 88 High and Low severity vulnerabilities without breaking rendering of markdown in the docs.

@Splaktar Splaktar marked this pull request as ready for review June 6, 2021 04:35
@@ -106,7 +105,8 @@
"//2": "(E.g. see https://github.com/gulpjs/gulp/issues/2162 and https://github.com/nodejs/node/issues/25132.)",
"natives": "1.1.6",
"//3": "`graceful-fs` needs to be pinned to support gulp 3, on Node v12+",
"graceful-fs": "^4.2.3"
"graceful-fs": "^4.2.3",
"marked": "0.3.6"
Copy link
Member

Choose a reason for hiding this comment

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

Rather than pinning marked we should just fix the docs to be CommonMark compliant.

Copy link
Member Author

Choose a reason for hiding this comment

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

OK, I wasn't sure that was something that we were open to doing. I'll look into it.

@Splaktar Splaktar marked this pull request as draft June 7, 2021 15:18
@Splaktar
Copy link
Member Author

Closing in favor of #17163 at Pete's suggestion.

@Splaktar Splaktar closed this Nov 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants