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

Removed remark-html in favor of Rehype #119

Merged
merged 7 commits into from
Apr 5, 2024

Conversation

mgc1194
Copy link
Contributor

@mgc1194 mgc1194 commented Apr 4, 2024

Story

DependaBot marked a few dependencies in this repo as high or critical. In addition, we are updating with similar changes, the remark-plugins repo.

This PR does:

After the update only two high-severity dependencies are left:

# npm audit report

trim  <0.0.3
Severity: high
Regular Expression Denial of Service in trim - https://github.com/advisories/GHSA-w5p7-h5w8-2hfq
fix available via `npm audit fix --force`
Will install [email protected], which is a breaking change
node_modules/trim
  remark-parse  <=8.0.3
  Depends on vulnerable versions of trim
  node_modules/remark-parse

2 high severity vulnerabilities

However we don't use this library on our server side, therefore this vulnerability can't be exploited.

@mgc1194 mgc1194 requested a review from stephenliang April 4, 2024 02:09
Comment on lines +14 to +49
// create custom sanitization schema as per
// https://github.com/syntax-tree/hast-util-sanitize#schema
// to support our custom syntaxes
const schema = Object.assign({}, defaultSanitizationSchema);
schema.clobber = [];

// We use a _lot_ of image formatting stuff in our
// instructions, particularly in CSP
schema.attributes.img.push("height", "width");

// Add support for expandableImages
schema.tagNames.push("span");
schema.attributes.span = ["dataUrl", "className"];

// Add support for inline styles (gross)
// TODO replace all inline styles in our curriculum content with
// semantically-significant content
schema.attributes["*"].push("style", "className");

// ClickableText uses data-id on a bold tag.
schema.attributes["b"] = ["dataId"];

// Add support for Blockly XML
const blocklyTags = [
"block",
"functional_input",
"mutation",
"next",
"statement",
"title",
"field",
"value",
"xml",
];
schema.tagNames = schema.tagNames.concat(blocklyTags);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the same schema used in SafeMarkdown in our main repo.

Copy link
Member

Choose a reason for hiding this comment

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

Optional/nit: I wonder if we should make SafeMarkdown use this schema so that it's sourced in one location.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It would be great to have just one source for our custom schema. Here is a ticket to track that work: P20-858

"rehype-raw": "^5.1.0",
"rehype-sanitize": "^4.0.0",
"rehype-stringify": "^8.0.0",
"remark-parse": "^6.0.3",
Copy link
Contributor Author

@mgc1194 mgc1194 Apr 4, 2024

Choose a reason for hiding this comment

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

We can't use versions above 7 so I went with the most used version (6.0.4) immediately below. Tests start breaking after version 8, and version 9 of the remak/rehype environments has major changes that make upgrading way more difficult as explained in this comment.

Comment on lines +14 to +49
// create custom sanitization schema as per
// https://github.com/syntax-tree/hast-util-sanitize#schema
// to support our custom syntaxes
const schema = Object.assign({}, defaultSanitizationSchema);
schema.clobber = [];

// We use a _lot_ of image formatting stuff in our
// instructions, particularly in CSP
schema.attributes.img.push("height", "width");

// Add support for expandableImages
schema.tagNames.push("span");
schema.attributes.span = ["dataUrl", "className"];

// Add support for inline styles (gross)
// TODO replace all inline styles in our curriculum content with
// semantically-significant content
schema.attributes["*"].push("style", "className");

// ClickableText uses data-id on a bold tag.
schema.attributes["b"] = ["dataId"];

// Add support for Blockly XML
const blocklyTags = [
"block",
"functional_input",
"mutation",
"next",
"statement",
"title",
"field",
"value",
"xml",
];
schema.tagNames = schema.tagNames.concat(blocklyTags);

Copy link
Member

Choose a reason for hiding this comment

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

Optional/nit: I wonder if we should make SafeMarkdown use this schema so that it's sourced in one location.

@mgc1194 mgc1194 merged commit a63a122 into master Apr 5, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants