-
Notifications
You must be signed in to change notification settings - Fork 2
Add macro for inline badges #109
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
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for docs-extensions-and-macros ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughThe changes introduce a new Sequence Diagram(s)sequenceDiagram
participant User
participant MacroRegistry
participant BadgeMacro
User->>MacroRegistry: Register badge macro
MacroRegistry->>BadgeMacro: Call register()
BadgeMacro->>MacroRegistry: Add 'badge' inline macro
User->>MacroRegistry: Use badge::[label=Beta, size=large, tooltip="..."]
MacroRegistry->>BadgeMacro: Process macro with attributes
BadgeMacro->>MacroRegistry: Return styled HTML span
MacroRegistry->>User: Rendered badge HTML in documentation
Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Suggested reviewers
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
macros/badge.js (2)
1-1
: Remove redundant 'use strict' directive.The 'use strict' directive is unnecessary in ES modules as they are automatically in strict mode.
-'use strict';
5-5
: Remove unnecessary 'this' aliasing.Arrow functions inherit 'this' from their enclosing scope, making the alias unnecessary.
- const self = this; - self.named('badge'); - self.process((parent, target, attrs) => { + this.named('badge'); + this.process((parent, target, attrs) => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.json
is excluded by!**/package-lock.json
📒 Files selected for processing (4)
extensions/generate-rp-connect-info.js
(1 hunks)macros/badge.js
(1 hunks)macros/enterprise-label.js
(0 hunks)package.json
(2 hunks)
💤 Files with no reviewable changes (1)
- macros/enterprise-label.js
🧰 Additional context used
🪛 Biome (1.9.4)
macros/badge.js
[error] 1-1: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.
(lint/suspicious/noRedundantUseStrict)
[error] 5-5: This aliasing of this is unnecessary.
Arrow functions inherits this
from their enclosing scope.
Safe fix: Use this instead of an alias.
(lint/complexity/noUselessThisAlias)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Redirect rules - docs-extensions-and-macros
- GitHub Check: Header rules - docs-extensions-and-macros
- GitHub Check: Pages changed - docs-extensions-and-macros
🔇 Additional comments (8)
package.json (2)
3-3
: LGTM! Version increment is appropriate.The patch version increment is suitable for adding new functionality (badge macro) while maintaining backward compatibility.
61-61
: LGTM! Export entry follows established pattern.The new badge macro export follows the existing convention and properly exposes the functionality.
extensions/generate-rp-connect-info.js (6)
118-124
: LGTM! Helper function improves code modularity.The
isConnectorDocPath
function effectively encapsulates the directory checking logic, making the code more readable and maintainable.
126-131
: LGTM! Clean helper function for cloud documentation detection.The
isCloudDoc
function properly encapsulates the cloud documentation path checking logic with clear parameter naming and logic.
135-136
: LGTM! Destructuring improves code readability.The destructuring assignment makes variable usage clearer and reduces repetitive property access.
141-141
: LGTM! Improved function call enhances readability.Using the helper function
isConnectorDocPath
makes the condition more self-documenting than the previous inline string check.
149-149
: LGTM! Enhanced conditional logic with helper function.The use of
isCloudDoc
helper function with the cloud support check makes the logic clearer and more maintainable.
158-169
: LGTM! Improved warning granularity enhances debugging.The separation of warnings into distinct cases (missing self-managed docs vs missing cloud docs) provides better insight into documentation gaps and will aid in maintenance.
const label = attrs.label || 'label'; | ||
const className = `badge--${label.toLowerCase().replace(/\s+/g, '-')}`; | ||
const isLarge = attrs.size === 'large'; | ||
const sizeClass = isLarge ? 'badge--large' : ''; | ||
const tooltip = attrs.tooltip; | ||
const tooltipAttr = tooltip ? ` data-tooltip="${tooltip}"` : ''; | ||
|
||
// Add brackets if not large | ||
const renderedLabel = isLarge ? label : `(${label})`; | ||
|
||
return `<span class="badge ${className} ${sizeClass}"${tooltipAttr}>${renderedLabel}</span>`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Address potential XSS vulnerability in HTML generation.
The macro directly interpolates user-provided attributes into HTML without proper escaping, which could lead to XSS attacks if malicious content is provided in the tooltip
attribute.
Apply this diff to escape HTML attributes:
const label = attrs.label || 'label';
const className = `badge--${label.toLowerCase().replace(/\s+/g, '-')}`;
const isLarge = attrs.size === 'large';
const sizeClass = isLarge ? 'badge--large' : '';
const tooltip = attrs.tooltip;
- const tooltipAttr = tooltip ? ` data-tooltip="${tooltip}"` : '';
+ const tooltipAttr = tooltip ? ` data-tooltip="${tooltip.replace(/"/g, '"').replace(/</g, '<').replace(/>/g, '>')}"` : '';
// Add brackets if not large
- const renderedLabel = isLarge ? label : `(${label})`;
+ // Escape HTML entities in label
+ const escapedLabel = label.replace(/</g, '<').replace(/>/g, '>').replace(/"/g, '"').replace(/'/g, ''');
+ const renderedLabel = isLarge ? escapedLabel : `(${escapedLabel})`;
- return `<span class="badge ${className} ${sizeClass}"${tooltipAttr}>${renderedLabel}</span>`;
+ return `<span class="badge ${className} ${sizeClass}"${tooltipAttr}>${renderedLabel}</span>`;
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const label = attrs.label || 'label'; | |
const className = `badge--${label.toLowerCase().replace(/\s+/g, '-')}`; | |
const isLarge = attrs.size === 'large'; | |
const sizeClass = isLarge ? 'badge--large' : ''; | |
const tooltip = attrs.tooltip; | |
const tooltipAttr = tooltip ? ` data-tooltip="${tooltip}"` : ''; | |
// Add brackets if not large | |
const renderedLabel = isLarge ? label : `(${label})`; | |
return `<span class="badge ${className} ${sizeClass}"${tooltipAttr}>${renderedLabel}</span>`; | |
const label = attrs.label || 'label'; | |
const className = `badge--${label.toLowerCase().replace(/\s+/g, '-')}`; | |
const isLarge = attrs.size === 'large'; | |
const sizeClass = isLarge ? 'badge--large' : ''; | |
const tooltip = attrs.tooltip; | |
const tooltipAttr = tooltip | |
? ` data-tooltip="${tooltip | |
.replace(/"/g, '"') | |
.replace(/</g, '<') | |
.replace(/>/g, '>')}"` | |
: ''; | |
// Escape HTML entities in label | |
const escapedLabel = label | |
.replace(/</g, '<') | |
.replace(/>/g, '>') | |
.replace(/"/g, '"') | |
.replace(/'/g, '''); | |
const renderedLabel = isLarge | |
? escapedLabel | |
: `(${escapedLabel})`; | |
return `<span class="badge ${className} ${sizeClass}"${tooltipAttr}>${renderedLabel}</span>`; |
🤖 Prompt for AI Agents
In macros/badge.js between lines 8 and 18, user-provided attributes like tooltip
are directly inserted into HTML without escaping, risking XSS attacks. To fix
this, implement a function to escape special HTML characters in attribute values
and apply it to all interpolated user inputs such as tooltip and label before
including them in the returned HTML string.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is run as part of a static site generator called Antora.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@JakeSCahill Thank you for that important clarification! You're absolutely right - the context of Antora as a static site generator significantly changes the security implications of my previous concern.
Since this macro executes at build time as part of the static site generation process rather than processing untrusted user input at runtime, the XSS risk is much lower. The input to this macro would typically come from documentation authors writing AsciiDoc content, which is a controlled and trusted environment.
While HTML escaping is still a good defensive programming practice (especially if the macro might be reused in other contexts), the urgency is much lower in this static site generator scenario. The immediate security concern I raised doesn't apply in the same way to build-time processing of trusted documentation content.
The macro implementation is appropriate for its intended use case in the Antora documentation build process.
✏️ Learnings added
Learnt from: JakeSCahill
PR: redpanda-data/docs-extensions-and-macros#109
File: macros/badge.js:8-18
Timestamp: 2025-06-24T11:03:59.146Z
Learning: Badge macro in macros/badge.js runs as part of Antora static site generator at build time, processing trusted documentation author content rather than untrusted user input, which significantly reduces XSS security concerns.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
This pull request introduces a new
badge
macro, refactoring of the logic for handling Redpanda Connect and Cloud URLs, and the removal of theenterprise-label
macro.Macro Updates:
badge
macro: A new inline macro was introduced to create badge elements with customizable labels, sizes, and tooltips. This enhances the ability to generate dynamic badges in documentation. (macros/badge.js
, macros/badge.jsR1-R21)enterprise-label
macro: Theenterprise-label
macro was removed, simplifying the codebase by eliminating unused functionality. (macros/enterprise-label.js
, macros/enterprise-label.jsL1-L11)Refactoring and Enhancements:
isConnectorDocPath
andisCloudDoc
to streamline and encapsulate logic for identifying documentation paths. Improved logging for missing documentation warnings. (extensions/generate-rp-connect-info.js
, extensions/generate-rp-connect-info.jsR118-R169)Package Metadata:
4.6.2
to4.6.3
to reflect the new changes. (package.json
, package.jsonL3-R3)badge
macro to exports: Registered the newbadge
macro in the package exports for inclusion in the build. (package.json
, package.jsonL60-R61)