Skip to content

feat(security): enforce strict markdown sanitization in song rendering#145

Draft
JaniL wants to merge 1 commit into
mainfrom
security/strict-markdown-sanitization
Draft

feat(security): enforce strict markdown sanitization in song rendering#145
JaniL wants to merge 1 commit into
mainfrom
security/strict-markdown-sanitization

Conversation

@JaniL

@JaniL JaniL commented Mar 1, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • move song markdown rendering into a dedicated helper to centralize security-sensitive HTML generation
  • enforce an explicit strict sanitize schema so only a narrow, markdown-focused set of tags and attributes is rendered
  • restrict rendered link protocols to http and https to prevent unsafe URL schemes before HTML is injected

Validation

  • run npm run lint (passes; existing Next.js config warning remains unrelated to this change)

Move markdown rendering into a dedicated helper and apply an explicit strict sanitize schema instead of relying on implicit plugin defaults. Only a narrow set of markdown tags is allowed and links are constrained to http/https before HTML is injected into the page.

@ConcernedHobbit ConcernedHobbit left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Approved with a suggestion to drop http link support

Comment thread lib/markdown.ts
a: ["href", "title"],
},
protocols: {
href: ["http", "https"],

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Screw it, let's lock it down to https here. I don't see a reason to support outgoing http links.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
href: ["http", "https"],
href: ["https"],

@ConcernedHobbit

Copy link
Copy Markdown
Collaborator

Thanks! :)

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