-
-
Notifications
You must be signed in to change notification settings - Fork 10.8k
Migrated lexical node renderers from Koenig #23548
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
Conversation
…417) refs TryGhost/Product#2225 Node definitions were previously duplicated in the editor and in the renderer. We wanted to add URL transformation details to nodes which would have meant a further duplication without a way to have core node logic (payload getters/setters, import, render, etc) separated in a Node friendly way for use in other server-side packages. - added `kg-default-nodes` package as a central location for all base node logic, the Lexical equivalent to our Mobiledoc `kg-default-cards` package - added `KoenigDecoratorNode` class and associated `$isKoenigCard` utility function - all of our card nodes should extend from `KoenigDecoratorNode` - `$isKoenigCard` allows the HTML renderer to defer to the node's `exportDOM()` method as we know that will be implemented how we expect for our own cards - extracted all generic image card logic into the new package - split out the DOM import handling to a separate file (`ImageParser`) to make unit testing easier (this is due to be expanded upon later so keeping it separate will help prevent the node file growing too big and keeping it focused on payload data) - extracted `ImageNode` rendering from `kg-lexical-html-renderer` to `kg-default-nodes` - `kg-lexical-html-renderer` shouldn't care about card-specific rendering, it should be focused on the core rendering logic and overrides of built-in Lexical node output - Lexical nodes have an `exportDOM` method for generating HTML from the node that we can make use of in our cards because we're in control of their output - moved image element renderer logic to `ImageRenderer` file used by `ImageNode.exportDOM` and updated it so it uses the `.getX()` methods for all data so that node usage is consistent everywhere - moved image rendering utils to the `kg-default-nodes` package - added `title` to the `ImageNode` dataset/payload as it's used in the renderer - updated `ImageNode` definition in the `koenig-lexical` package to extend from the `ImageNode` class in the `kg-default-nodes` package - kept editor-specific data attributes in the editor node class - overrode the editor-specific rendering/decoration methods - added `urlTransformMap` to `ImageNode` - used by `@tryghost/url-utils` - maps serialized data properties to the URL transform type that matches the data type - migrated image card render tests from `kg-lexical-html-renderer` to `kg-default-cards` - card render output logic is located with the card so the tests should be located there too - added `html` test util to prettify expected output and get syntax highlighting in tests - added `should.prettifyTo` assertion that prettifies the expected string to compare against a prettified string so we can be looser with html whitespace in tests - added initial tests for image node parsing and json import/export - added sourcemap generation to the demo build config to make debugging easier
no issue - removes large block of duplicated code at the top of every renderer file
no issue Cleanup of functions re-used across multiple node parsers. - `buildCleanBasicHtmlForElement(elem)` is a factory for getting a `cleanBasicHtml()` method, using the element's `ownerDocument` to set up the required `createDocument` option - `readCaptionFromElement(elem)` extracts html content from all `figcaption` elements and runs it through `cleanBasicHtml()` - `readImageAttributesFromElement()` extracts an image attributes object from the passed in element
closes TryGhost/Product#2894, closes TryGhost/Product#2916 -parser is empty, placeholder to prevent ref issues -twitter embed may need some refinement -needs tests
no refs -import was causing issues for running other tests
refs TryGhost/Team#2637
closes TryGhost/Product#2917, closes TryGhost/Product#2918 -need to complete unit tests for the parser
refs TryGhost/Product#3341 - The renderer for the bookmark, embed nodes was setting the textContent on the caption element, which was causing the raw HTML to be rendered - Changed it to set the innerHTML instead, so the HTML is rendered properly - Embed and bookmark cards wrapped the entire card in `.not-kg-prose` which was overriding the styles in the caption editor - Moved `.not-kg-prose` to just the content of the card, excluding the caption editor
no refs -copy was failing because we were passing a bad empty return -lexical was expecting an element and we returned an empty text node -now return empty span elements instead of text node
refs TryGhost/Product#3365 - in kg-default-nodes: - added a generator function to generate the boilerplate code inside kg-default-nodes. This method should generate all required base methods from on a set of data properties, but still allow methods to be overwritten by individual nodes - updated getter & setters to use ES6 syntax (getter: node.content, setter: node.content = 'newValue' - added missing tests for urlTransformMap, clone, getType methods for each node - removed INSERT_X_COMMAND - transformed parsers to functional components - used abstracted renderEmptyContainer() utility function in relevant renderers - in koenig-lexical: - created INSERT_X_COMMAND directly - removed duplicated createDOM, updateDOM - updated getters and setters with ES6 syntax - splited functional and class component in old cards, using the MyNode.jsx / MyNodeComponent.jsx existing pattern
* 🐛 Fixed callout card render format closes #17215 - Fixes an issue that caused some templates to render the card incorrectly. - Refactored the renderCalloutNode function to parse the calloutText into a DOM tree using jsdom - Created a cleanDOM helper function to traverse the DOM tree and remove unwanted HTML tags - Retained only 'A', 'STRONG', and 'EM' tags in the HTML string to bring it in line with how mobiledoc rendered HTML.
refs TryGhost/Koenig#665 - updated use of `assert/strict` - updated file names to match file name conventions - had to disable `ghost/filenames/match-exported-class` because it does not currently pick up named exports, we do not want default exports in our node files as the exports get re-exported in the main library export map
closes TryGhost/Product#3707 - brought lexical renderer in line with md
no issue - the missing slashes resulted in minor differences between mobiledoc and lexical rendered output, mostly just fixing this to reduce the noise when comparing output
no issues - product card images have explicit width/height attributes, and when the width value is large (e.g. 2560), it causes an overflow on Gmail on Android - the reason why there's no issue on other email clients is that Gmail on Android has autofit feature which makes the email width fit inside the screen automatically, and it made the email width very narrow when the product card image width isn't responsive - this fix makes the product card image width responsive by explicitly setting it 100%
closes TryGhost/Product#3858 - swapped css classes for author and publisher - while 'incorrect', this would be a breaking change for themes and should remain swapped; see comments in code
- stripped code elements from the rendered email output when using a placeholder - retained code elements within exportJSON so that formatting looks appropriate across editor save/loads - moved to parsing html as props further complicate the regexes - added tests to cover these cases
closes TryGhost/Product#3984 - placeholder strings like {variable} will now be formatted as code in the editor so users can know they are supported - code formatting is stripped out in the renderer - created new `ReplacementStringsPlugin` for use in the nested editors
…+ copy/paste (#993) refs TryGhost/Product#4008 Copy/paste of callout cards in the old editor could result in the `backgroundColor` property containing a non-class value like `rgba(124, 139, 154, 0.13)` which would then error when we attempted to use that as a class name during rendering. - added regex to validate the `backgroundColor` value when rendering, if it's not present or doesn't match a class-like string then we fall back to using `'white'` - added a similar fix to `mobiledocToLexical()` so we can fix bad values when converting so there's less likelihood of known bad values ending up in converted content
no issue - brings output in line with original editor - allows rendered output to be parsed back to original input
no issues - product card outputs the original width/height of the image in emails which results in overflown images in Outlook - this uses the same technique used for image card to resize the image dimensions - the image card resizes the image width to 600px while it's 560px for the product card because of the padding of 20px on the sides
closes TryGhost/Product#3897 - accent style could be inappropriately applied to images
closes TryGhost/Product#3802 - removed text color style when background is transparent - this allows styles from parent class to be applied to help readability in most cases
refs #17753 - added lazy loading attribute to header card renderer (v2+)
closes #17753 - added background image width and height attributes to `HeaderNode` - updated header node components to capture width and height when an image is uploaded - updated header node renderer to output `srcset` attribute on the `<picture>` element
#1076) refs TryGhost/Product#4133 - we already create a JSDOM instance inside the `LexicalHtmlRenderer` that's assigned to `this.dom` and re-used for it's lifecycle so we don't need to pass in an external dom instance and potentially create performance issues if a new instance is created for every card render - modified `shouldRender` test helper to pass in a global JSDOM instance to speed tests up by not creating unnecessary JSDOM instances - updated `kg-default-nodes` to use `options.dom` in place of `options.createDocument` when available - modified tests to represent that usage and speed them up by not creating unnecessary JSDOM instances
* Removed tweet embed container Refs TryGhost/Product#4167 (comment) - Wrapping the tweet embed in a container div was causing styling issues * Fixed test
* Added toggle to Call to Action card to show/hide dividers Ref https://linear.app/ghost/issue/PROD-1628/add-setting-to-showhide-dividers - This supports the use case where the card is used as inline with text, and the dividers are not needed. * Updated Call to Action Card to support no-dividers styles Ref https://linear.app/ghost/issue/PROD-1628/add-setting-to-showhide-dividers * Updated tests to include showDividers * Changed class name to kg-cta-no-dividers Ref https://linear.app/ghost/issue/PROD-1628/add-setting-to-showhide-dividers - Changed classname from kg-cta-has-dividers to kg-cta-no-dividers to ensure backwards compatibility
ref https://linear.app/ghost/issue/PROD-1688 - updated product card renderer to output different border-radius values dependent on the `design.buttonCorners` option value that will be passed through from Ghost when the `emailCustomizationAlpha` flag is enabled - switches between a `rounded`, `squared`, and `pill` radius - this required changes in the card renderer because it currently uses inline styles
no issue - manual DOM manipulation to build render output is difficult to read and update - switched to using a string template to better match other cards and improve the development experience
ref https://linear.app/ghost/issue/PROD-1747 - duplicated email template under the emailCustomizationAlpha flag - replaced inline styles with classes - email template in Ghost has been updated to add the previous styles in it's CSS class definitions
ref TryGhost/Koenig@457ba22 - when switching to a string template we accidentally introduced a doubly-nested paragraph as one was included in the string template and in the `document.createElement('p')` that string template was injected into - added a `oneline` tagged template function so the output matches the previous manual DOM build - keeps snapshot changes for email content minimal to make real changes more noticeable - avoids unwanted newlines being added to the plaintext email output - uses `.mjs` so we can import the file directly in a test file, otherwise we have to expose this utility as part of the package's public interface
no issue - maps `html` name to our `oneline` tagged template function - when `html` is used for a literal string, editors will automatically apply HTML syntax highlighting which makes template strings nicer to work with
…1516) ref https://linear.app/ghost/issue/PROD-1688 - All buttons in cards now use the same markup and styling; this should make it easier to maintain and update the email templates and ensures consistent styling. - All three cards now use more robust email-safe HTML. - All these changes are behind the `emailCustomizationAlpha` feature flag.
ref https://linear.app/ghost/issue/PROD-1688/implement-button-corners-setting - The button markup and styling are now consistent across the email template - Added emailCustomizationAlpha feature flag to enable new CTA card button rendering
#1518) Ref https://linear.app/ghost/issue/PROD-1688/implement-button-corners-setting - The button in the CTA card wouldn't center, Outlook needs the align attribute to be set - The header and subheader in the header card wouldn't center, Outlook needs the align attribute to be set
no issue - exporting the fns allows for tests to easily use them without jumping through lots of commonjs/esmodule hoops - adapted `oneline` fn to accept a plain string too so we can normalize strings in tests for comparison
ref https://linear.app/ghost/issue/PROD-1717 - updated header and call-to-action cards to support an outline button style option - these cards needed specific support because they override default button styles to allow for custom button colors
ref https://linear.app/ghost/issue/PROD-1691 ref https://linear.app/ghost/issue/PROD-1747 - switched conditionals where appropriate from `emailCustomizationAlpha` to `emailCustomizationBeta` - for the larger template conditionals uses `emailCustomization || emailCustomizationAlpha` to avoid duplicating code unnecessarily - button style code within those same template functions is still behind the alpha flag
ref https://linear.app/ghost/issue/PROD-1684 - This markup is more email safe and ensures spacing is applied in Outlook - Inline styles have been removed in favour of custom css in `styles.hbs` --------- Co-authored-by: Kevin Ansfield <[email protected]>
ref https://linear.app/ghost/issue/PROD-1717 - extracted central part of button node email renderer as the rendered HTML is duplicated across multiple cards - added tests to check alpha and beta output is the same - for the alpha version, the wrapping element was changed to `<div>` and the return type changed to `'inner'` because tables inside paragraphs is not valid HTML and results in an empty paragraph being added after the table in the final HTML output - added `clsx` dependency to make working with class lists nicer
ref https://linear.app/ghost/issue/PROD-1720 - we're moving the current implementation of button style behind the beta flag for wider internal testing - updated conditionals and tests to match new logic - removed duplicated test
no issue - the updated renderer output for the toggle node wasn't being output for the beta flag which meant our CSS changes that depended on the class names weren't working
WalkthroughThis change introduces a comprehensive set of custom node renderer modules for the Koenig editor's lexical nodes, each handling a specific card or node type such as audio, bookmark, button, callout, call-to-action, codeblock, email-cta, email, embed, file, gallery, header (v1 and v2), horizontal rule, html, image, markdown, paywall, product, signup, toggle, and video. Each renderer module exports a function that takes a node and rendering options, and outputs either a DOM element or an HTML string suitable for web or email targets, with logic tailored to each context. Supporting utility functions for document creation, HTML escaping, image resizing, sanitization, slugification, visibility handling, and responsive image attributes are added. The core node renderers are exported from a centralized index module mapping node types to their renderers. Extensive unit and integration tests are introduced to verify correct rendering output for all node types and contexts, including feature flag variations and email customization modes. Possibly related PRs
✨ Finishing Touches
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 (
|
4eee400
to
d18eb67
Compare
ref https://linear.app/ghost/issue/PROD-1784 - converted all files from es6 to cjs - fixed require paths - added all nodes to our custom node renderers object in `services/koenig/node-renderers/index.js` - modified node renderer exports to use a single default export of the main renderer function for easy use in the index file - added tests to ensure all custom node renderers are present and called when rendering email golden post - added explicit dependencies that were previously sub-dependencies of `@tryghost/kg-default-nodes` - `@tryghost/kg-clean-basic-html` - `@tryghost/kg-markdown-html-renderer`
d18eb67
to
fb2d2e7
Compare
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.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
🛑 Comments failed to post (59)
ghost/core/core/server/services/koenig/render-partials/email-button.js (1)
9-9:
⚠️ Potential issueRemove unused parameter from JSDoc.
The
textColor
parameter is documented in the JSDoc but not used in the function implementation.* @param {string} [options.text=''] - * @param {string} [options.textColor] * @param {string} [options.url='']
📝 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.* @param {string} [options.text=''] * @param {string} [options.url='']
🤖 Prompt for AI Agents
In ghost/core/core/server/services/koenig/render-partials/email-button.js at line 9, the JSDoc includes a parameter textColor that is not used in the function implementation. Remove the textColor parameter from the JSDoc comment to keep the documentation accurate and avoid confusion.
ghost/core/core/server/services/koenig/render-utils/is-unsplash-image.js (1)
1-3: 🛠️ Refactor suggestion
Add input validation and consider edge cases.
The function lacks input validation and could fail on non-string inputs. Consider these improvements:
- Type checking: The function should validate that
url
is a string- Edge case handling: Handle null/undefined inputs gracefully
- Regex specificity: The current regex could match partial strings that aren't valid URLs
const isUnsplashImage = function (url) { + if (!url || typeof url !== 'string') { + return false; + } - return /images\.unsplash\.com/.test(url); + return /(?:https?:\/\/)?images\.unsplash\.com/.test(url); };📝 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 isUnsplashImage = function (url) { if (!url || typeof url !== 'string') { return false; } return /(?:https?:\/\/)?images\.unsplash\.com/.test(url); };
🤖 Prompt for AI Agents
In ghost/core/core/server/services/koenig/render-utils/is-unsplash-image.js at lines 1 to 3, the function lacks input validation and may fail on non-string or null/undefined inputs. Add a type check to ensure the input is a string before running the regex test. Also, improve the regex to more specifically match valid Unsplash image URLs, avoiding partial or invalid matches. Handle null or undefined inputs gracefully by returning false or an appropriate fallback.
ghost/core/package.json (1)
89-89: 💡 Verification agent
🧩 Analysis chain
Verify dependency versions and security status.
Please ensure the newly added dependencies are using the latest stable versions and are free from known security vulnerabilities.
Also applies to: 96-96, 227-227, 246-246
🏁 Script executed:
#!/bin/bash # Description: Check for latest versions and security advisories for new dependencies echo "Checking latest versions..." for package in "@tryghost/kg-clean-basic-html" "@tryghost/kg-markdown-html-renderer" "@prettier/sync" "html-minifier"; do echo "=== $package ===" curl -s "https://registry.npmjs.org/$package" | jq '.["dist-tags"].latest' done echo -e "\nChecking security advisories..." gh api graphql -f query=' { securityVulnerabilities(first: 10, ecosystem: NPM, package: "@tryghost/kg-clean-basic-html") { nodes { advisory { summary severity publishedAt } vulnerableVersionRange firstPatchedVersion { identifier } } } }' gh api graphql -f query=' { securityVulnerabilities(first: 10, ecosystem: NPM, package: "html-minifier") { nodes { advisory { summary severity publishedAt } vulnerableVersionRange firstPatchedVersion { identifier } } } }'Length of output: 2114
Security risk in html-minifier dependency
The other three packages are at their latest versions with no known advisories, but [email protected] has a HIGH-severity ReDoS vulnerability (affects <=4.0.0, no patched release).
Please address this by either:
- Switching to a maintained fork (e.g. html-minifier-terser)
- Waiting for and upgrading to a patched version once available
- Removing or sandboxing its use to mitigate ReDoS risks
Affected location:
- ghost/core/package.json: line 246 ([devDependency] [email protected])
🤖 Prompt for AI Agents
In ghost/core/package.json at line 246, the html-minifier dependency version 4.0.0 has a known high-severity ReDoS vulnerability with no patched release available. To fix this, replace [email protected] with a maintained and secure alternative such as html-minifier-terser, or remove/sandbox its usage to mitigate the risk. Ensure to update the package.json accordingly and verify compatibility with the rest of the codebase.
ghost/core/core/server/services/koenig/render-utils/slugify.js (1)
1-15: 🛠️ Refactor suggestion
Improve input validation and handle edge cases.
The function has several areas for improvement:
- Missing input validation: No type checking for the input parameter
- Edge case handling: Doesn't handle empty strings or multiple consecutive dashes
- Leading/trailing dashes: Could result in strings starting or ending with dashes
function slugify(str) { + if (!str || typeof str !== 'string') { + return ''; + } + // Remove HTML tags str = str.replace(/<[^>]*>?/gm, ''); // Remove any non-word character with whitespace str = str.replace(/[^\w\s]/gi, ''); // Replace any whitespace character with a dash str = str.replace(/\s+/g, '-'); // Convert to lowercase str = str.toLowerCase(); + // Remove leading and trailing dashes + str = str.replace(/^-+|-+$/g, ''); return str; }📝 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.function slugify(str) { if (!str || typeof str !== 'string') { return ''; } // Remove HTML tags str = str.replace(/<[^>]*>?/gm, ''); // Remove any non-word character with whitespace str = str.replace(/[^\w\s]/gi, ''); // Replace any whitespace character with a dash str = str.replace(/\s+/g, '-'); // Convert to lowercase str = str.toLowerCase(); // Remove leading and trailing dashes str = str.replace(/^-+|-+$/g, ''); return str; }
🤖 Prompt for AI Agents
In ghost/core/core/server/services/koenig/render-utils/slugify.js lines 1 to 15, the slugify function lacks input validation and does not handle edge cases such as empty strings, multiple consecutive dashes, or leading/trailing dashes. Add a type check to ensure the input is a string and return an empty string or appropriate fallback if not. Modify the logic to collapse multiple consecutive dashes into a single dash and trim any leading or trailing dashes before returning the final slug.
ghost/core/core/server/services/koenig/render-utils/escape-html.js (1)
6-13: 🛠️ Refactor suggestion
Add input validation to prevent runtime errors.
The function should validate that the input is a string to prevent errors when called with null, undefined, or non-string values.
function escapeHtml(unsafe) { + if (typeof unsafe !== 'string') { + return ''; + } return unsafe .replace(/&/g, '&') .replace(/</g, '<') .replace(/>/g, '>') .replace(/"/g, '"') .replace(/'/g, '''); }🤖 Prompt for AI Agents
In ghost/core/core/server/services/koenig/render-utils/escape-html.js around lines 6 to 13, the escapeHtml function currently assumes the input is always a string, which can cause runtime errors if called with null, undefined, or non-string values. Add input validation at the start of the function to check if the input is a string; if not, convert it to an empty string or a safe default before performing the replacements. This will prevent errors and ensure consistent behavior.
ghost/core/test/unit/server/services/koenig/node-renderers/html-renderer.test.js (2)
26-31: 🛠️ Refactor suggestion
Address the TODO comment and fix the template literal formatting.
The TODO comment indicates a known issue with exact matching, and the template literal includes leading/trailing newlines that may cause assertion failures or make tests brittle.
- // TODO: fix this, needs exact match because comments get lost in assertPrettifiesTo - assert.equal(result.html, ` -<!--kg-card-begin: html--> -<p>Paragraph with:</p><ul><li>list</li><li>items</li></ul> -<!--kg-card-end: html--> -`); + const expected = '<!--kg-card-begin: html-->\n<p>Paragraph with:</p><ul><li>list</li><li>items</li></ul>\n<!--kg-card-end: html-->'; + assert.equal(result.html.trim(), expected);📝 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 expected = '<!--kg-card-begin: html-->\n<p>Paragraph with:</p><ul><li>list</li><li>items</li></ul>\n<!--kg-card-end: html-->'; assert.equal(result.html.trim(), expected);
🤖 Prompt for AI Agents
In ghost/core/test/unit/server/services/koenig/node-renderers/html-renderer.test.js around lines 26 to 31, the test assertion uses a template literal with leading and trailing newlines causing brittle exact match failures. Remove the leading and trailing newlines from the template literal so the expected HTML string matches exactly without extra whitespace, ensuring the assertion is stable and the TODO comment about fixing exact match is resolved.
48-53: 🛠️ Refactor suggestion
Same formatting and TODO issues exist in the email test block.
The email test has the same template literal formatting issue and TODO comment as the web test.
Apply the same fix as suggested for the web test:
- // TODO: fix this, needs exact match because comments get lost in assertPrettifiesTo - assert.equal(result.html, ` -<!--kg-card-begin: html--> -<p>Paragraph with:</p><ul><li>list</li><li>items</li></ul> -<!--kg-card-end: html--> -`); + const expected = '<!--kg-card-begin: html-->\n<p>Paragraph with:</p><ul><li>list</li><li>items</li></ul>\n<!--kg-card-end: html-->'; + assert.equal(result.html.trim(), expected);📝 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 expected = '<!--kg-card-begin: html-->\n<p>Paragraph with:</p><ul><li>list</li><li>items</li></ul>\n<!--kg-card-end: html-->'; assert.equal(result.html.trim(), expected);
🤖 Prompt for AI Agents
In ghost/core/test/unit/server/services/koenig/node-renderers/html-renderer.test.js around lines 48 to 53, the email test block has the same formatting and TODO issues as the web test block. Fix the email test by ensuring the template literal for the expected HTML matches exactly, preserving all whitespace and comments, and remove the TODO comment. Apply the same exact formatting fix used in the web test to the email test to prevent comment loss in assertPrettifiesTo.
ghost/core/core/server/services/koenig/render-utils/build-clean-basic-html-for-element.js (1)
3-15: 🛠️ Refactor suggestion
Add input validation and error handling for robustness.
The function lacks defensive programming practices that could lead to runtime errors:
- No validation that
domNode
is actually a DOM node- No null/undefined checks for
domNode.ownerDocument
- Direct
innerHTML
assignment without validation of the HTML parameterConsider adding input validation:
function buildCleanBasicHtmlForElement(domNode) { + if (!domNode || typeof domNode !== 'object' || !domNode.ownerDocument) { + throw new Error('Invalid domNode: must be a DOM node with ownerDocument'); + } + return function _cleanBasicHtml(html, additionalOptions = {}) { + if (typeof html !== 'string') { + throw new Error('HTML parameter must be a string'); + } + const cleanedHtml = cleanBasicHtml(html, { createDocument: (_html) => { const newDoc = domNode.ownerDocument.implementation.createHTMLDocument(); newDoc.body.innerHTML = _html; return newDoc; }, ...additionalOptions }); return cleanedHtml; }; }📝 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.function buildCleanBasicHtmlForElement(domNode) { if (!domNode || typeof domNode !== 'object' || !domNode.ownerDocument) { throw new Error('Invalid domNode: must be a DOM node with ownerDocument'); } return function _cleanBasicHtml(html, additionalOptions = {}) { if (typeof html !== 'string') { throw new Error('HTML parameter must be a string'); } const cleanedHtml = cleanBasicHtml(html, { createDocument: (_html) => { const newDoc = domNode.ownerDocument.implementation.createHTMLDocument(); newDoc.body.innerHTML = _html; return newDoc; }, ...additionalOptions }); return cleanedHtml; }; }
🤖 Prompt for AI Agents
In ghost/core/core/server/services/koenig/render-utils/build-clean-basic-html-for-element.js between lines 3 and 15, add input validation to ensure domNode is a valid DOM node and that domNode.ownerDocument exists before using it. Also, validate that the html parameter is a string before assigning it to innerHTML to prevent runtime errors. Implement checks that throw descriptive errors or handle invalid inputs gracefully to improve robustness.
ghost/core/test/unit/server/services/koenig/node-renderers/file-renderer.test.js (1)
37-38: 💡 Verification agent
🧩 Analysis chain
Investigate empty file size in web rendering.
The web rendering test shows an empty
kg-file-card-filesize
div, while the email rendering test (lines 108-110) displays "121 KB". This inconsistency suggests either:
- The web renderer doesn't format file sizes
- There's a missing implementation in the web renderer
Please verify if this behavior is intentional by examining the actual file renderer implementation:
🏁 Script executed:
#!/bin/bash # Check the file renderer implementation to understand filesize handling ast-grep --pattern $'class="kg-file-card-filesize"'Length of output: 52
🏁 Script executed:
#!/bin/bash # Locate the file-renderer implementation fd file-renderer.js # Inspect the first 200 lines of the implementation to see how filesize is handled sed -n '1,200p' ghost/core/server/services/koenig/node-renderers/file-renderer.jsLength of output: 283
🏁 Script executed:
#!/bin/bash # Dynamically locate the file-renderer implementation and inspect its top section file=$(fd file-renderer.js) echo "Inspecting file: $file" sed -n '1,200p' "$file" # Search for any mentions of filesize formatting within the file rg -C3 "size" "$file" || echo "No explicit filesize handling found"Length of output: 7705
Web renderer uses
formattedFileSize
, so filesize is empty by defaultThe
cardTemplate
pulls fromnode.formattedFileSize
—which isn’t set in your test fixture—so the<div class="kg-file-card-filesize">
stays empty. To surface the actual file size (like in email rendering), convertnode.fileSize
viabytesToSize
.Locations to update:
- File:
ghost/core/server/services/koenig/node-renderers/file-renderer.js
- Function:
cardTemplate
Suggested diff:
--- a/ghost/core/server/services/koenig/node-renderers/file-renderer.js +++ b/ghost/core/server/services/koenig/node-renderers/file-renderer.js @@ function cardTemplate(node, document) { - const filesize = document.createElement('div'); - filesize.setAttribute('class', 'kg-file-card-filesize'); - filesize.textContent = node.formattedFileSize || ''; + const filesize = document.createElement('div'); + filesize.setAttribute('class', 'kg-file-card-filesize'); + // derive human-readable size if formattedFileSize is missing + filesize.textContent = bytesToSize(node.fileSize) || '';This ensures web cards display a file size without requiring
formattedFileSize
onnode
.📝 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.function cardTemplate(node, document) { // … other elements … const filesize = document.createElement('div'); filesize.setAttribute('class', 'kg-file-card-filesize'); // derive human-readable size if formattedFileSize is missing filesize.textContent = bytesToSize(node.fileSize) || ''; // … append filesize into the card, etc. … }
🤖 Prompt for AI Agents
In ghost/core/test/unit/server/services/koenig/node-renderers/file-renderer.test.js at lines 37-38, the web rendering test shows an empty filesize div because the test node lacks the formattedFileSize property used by the web renderer. To fix this, update the test fixture to include a formattedFileSize value by converting the raw fileSize using the bytesToSize utility, or modify the cardTemplate in ghost/core/server/services/koenig/node-renderers/file-renderer.js to compute formattedFileSize from fileSize if not already set. This will ensure the filesize div displays the correct size in web rendering tests, matching the email rendering behavior.
ghost/core/core/server/services/koenig/render-utils/get-resized-image-dimensions.js (1)
1-22:
⚠️ Potential issueFix missing return statement for edge case.
The function lacks a return statement when neither
desiredWidth
nordesiredHeight
is provided, which will returnundefined
and could cause runtime errors in consuming code.Add a default return to handle this edge case:
} + + // Return original dimensions if no desired dimensions provided + return { + width, + height + }; };Alternatively, consider throwing an error to make the invalid usage explicit:
} + + throw new Error('Either desiredWidth or desiredHeight must be provided'); };🤖 Prompt for AI Agents
In ghost/core/core/server/services/koenig/render-utils/get-resized-image-dimensions.js lines 1 to 22, the function getResizedImageDimensions does not return a value if neither desiredWidth nor desiredHeight is provided, leading to undefined returns and potential runtime errors. Add a default return statement at the end of the function to handle this edge case, such as returning the original image dimensions or null, or alternatively throw an error to explicitly indicate invalid usage.
ghost/core/test/unit/server/services/koenig/node-renderers/codeblock-renderer.test.js (1)
51-56:
⚠️ Potential issueFix incorrect function call in email test.
The email test section is calling
renderForWeb
instead ofrenderForEmail
, which means it's not actually testing the email rendering path.it('renders without caption', function () { - const result = renderForWeb(getTestData({caption: ''})); + const result = renderForEmail(getTestData({caption: ''})); assertPrettifiesTo(result.html, html` <pre><code class="language-javascript"><script></script></code></pre> `); });📝 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.it('renders without caption', function () { const result = renderForEmail(getTestData({caption: ''})); assertPrettifiesTo(result.html, html` <pre><code class="language-javascript"><script></script></code></pre> `); });
🤖 Prompt for AI Agents
In ghost/core/test/unit/server/services/koenig/node-renderers/codeblock-renderer.test.js around lines 51 to 56, the test for email rendering incorrectly calls renderForWeb instead of renderForEmail. Update the function call in this test to use renderForEmail to ensure the email rendering path is properly tested.
ghost/core/test/unit/server/services/koenig/node-renderers/header-v2-renderer.test.js (1)
108-108:
⚠️ Potential issueFix malformed CSS style attribute.
The style attribute contains invalid CSS syntax with an extra
#ffffff
value that's not associated with any property.Apply this diff to fix the CSS syntax:
- style="color: #000000; background-color: #ffffff; #ffffff">The button</a> + style="color: #000000; background-color: #ffffff">The button</a>📝 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.style="color: #000000; background-color: #ffffff">The button</a>
🤖 Prompt for AI Agents
In ghost/core/test/unit/server/services/koenig/node-renderers/header-v2-renderer.test.js at line 108, the style attribute in the HTML contains an invalid CSS syntax with an extra `#ffffff` value not linked to any CSS property. Remove the stray `#ffffff` from the style attribute so that only valid CSS declarations remain, ensuring the style attribute is properly formatted.
ghost/core/core/server/services/koenig/render-utils/size-byte-converter.js (1)
16-26: 🛠️ Refactor suggestion
Fix redundant logic and improve consistency.
The function has several issues that should be addressed:
- Redundant zero checks (lines 17-18 and 21-23)
- Inconsistent return values for zero cases
- Unnecessary
parseInt
usage- Missing bounds checking
Apply this refactor:
function bytesToSize(bytes) { - if (!bytes) { - return '0 Byte'; - } const sizes = ['Bytes', 'KB', 'MB', 'GB', 'TB']; - if (bytes === 0) { - return '0 Byte'; + if (!bytes || bytes === 0) { + return '0 Bytes'; } - const i = parseInt(Math.floor(Math.log(bytes) / Math.log(1024))); + const i = Math.floor(Math.log(bytes) / Math.log(1024)); + if (i >= sizes.length) { + return Math.round((bytes / Math.pow(1024, sizes.length - 1))) + ' ' + sizes[sizes.length - 1]; + } return Math.round((bytes / Math.pow(1024, i))) + ' ' + sizes[i]; }📝 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.function bytesToSize(bytes) { const sizes = ['Bytes', 'KB', 'MB', 'GB', 'TB']; if (!bytes || bytes === 0) { return '0 Bytes'; } const i = Math.floor(Math.log(bytes) / Math.log(1024)); if (i >= sizes.length) { return Math.round((bytes / Math.pow(1024, sizes.length - 1))) + ' ' + sizes[sizes.length - 1]; } return Math.round((bytes / Math.pow(1024, i))) + ' ' + sizes[i]; }
🤖 Prompt for AI Agents
In ghost/core/core/server/services/koenig/render-utils/size-byte-converter.js lines 16 to 26, remove the redundant zero checks by consolidating the condition that handles zero bytes into a single check at the start. Ensure the return value for zero bytes is consistent, using "0 Bytes" instead of "0 Byte". Replace the unnecessary parseInt call with just Math.floor for the index calculation. Add bounds checking to ensure the index does not exceed the sizes array length to avoid errors for very large byte values.
ghost/core/test/unit/server/services/koenig/node-renderers/video-renderer.test.js (1)
125-125:
⚠️ Potential issueFix missing space in caption text.
The caption text is missing a space between "a" and the bold tag, resulting in "This is acaption" instead of "This is a caption".
Apply this diff to fix the caption spacing:
- <figcaption>This is a<b>caption</b></figcaption> + <figcaption>This is a <b>caption</b></figcaption>📝 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.<figcaption>This is a <b>caption</b></figcaption>
🤖 Prompt for AI Agents
In ghost/core/test/unit/server/services/koenig/node-renderers/video-renderer.test.js at line 125, the caption text lacks a space between "a" and the <b> tag, causing the words to run together. Add a space after "a" so the caption reads "This is a <b>caption</b>" to correct the spacing.
ghost/core/core/server/services/koenig/node-renderers/button-renderer.js (1)
28-30: 💡 Verification agent
🧩 Analysis chain
Verify HTML escaping for user content to prevent XSS.
The
buttonUrl
,buttonText
, andnode.alignment
values are being inserted directly into HTML templates without explicit escaping. While thehtml
tagged template function may handle escaping, this should be verified to prevent XSS vulnerabilities.Additionally, consider adding explicit validation for
node.alignment
to ensure it only accepts expected values (left
,center
,right
).Also applies to: 45-48, 63-64, 88-88
🏁 Script executed:
#!/bin/bash # Verify that the html tagged template function properly escapes user content echo "Checking if html tagged template function handles XSS prevention..." rg -A 10 -B 5 "const html.*=" --type js rg -A 15 "function.*html\(" --type jsLength of output: 77151
Sanitize and escape interpolated values in the email branch of the button renderer
Our inspection shows that the
html
tagged‐template (inservices/koenig/render‐utils/tagged‐template‐fns.js
) is just a synonym foroneline
and does not perform any HTML‐escaping—only whitespace stripping. In the email branch ofbutton‐renderer.js
, you interpolate unescaped user input:
- In
ghost/core/core/server/services/koenig/node-renderers/button-renderer.js
(email branch around lines 45–48):return { element: html` <div class="kg-card kg-button-card kg-align-${node.alignment}"> <a href="${node.buttonUrl}" class="kg-btn kg-btn-accent"> ${node.buttonText || 'Button Title'} </a> </div>` };node.buttonUrl
,node.buttonText
andnode.alignment
go straight into attributes and content without escaping.Recommendations:
- Import and use an HTML‐escaping helper (e.g.
escapeHtml
) on all interpolations in that template:const {escapeHtml} = require('../…/utils/escape-html'); … html` <div class="kg-card kg-button-card kg-align-${escapeHtml(node.alignment)}"> <a href="${escapeHtml(node.buttonUrl)}" class="kg-btn kg-btn-accent"> ${escapeHtml(node.buttonText || 'Button Title')} </a> </div>`- Whitelist
node.alignment
(allow only'left'|'center'|'right'
) so arbitrary class names can’t be injected.- Keep using
setAttribute
/textContent
in the frontend branch (lines 28–30) as they’re safe, but apply similar URL‐validation (e.g. rejectjavascript:
URIs) if needed.Please update the email template branch accordingly to close this XSS vector.
🤖 Prompt for AI Agents
In ghost/core/core/server/services/koenig/node-renderers/button-renderer.js around lines 28 to 30, the frontend branch safely uses setAttribute and textContent, so no change is needed there. However, in the email template branch around lines 45 to 48, user inputs node.buttonUrl, node.buttonText, and node.alignment are interpolated directly into HTML without escaping, causing XSS risk. To fix this, import an HTML-escaping helper like escapeHtml and apply it to all these interpolated values in the template. Additionally, whitelist node.alignment to only allow 'left', 'center', or 'right' to prevent arbitrary class injection. This will ensure all user content is properly sanitized and safe in the email output.
ghost/core/core/server/services/koenig/node-renderers/toggle-renderer.js (2)
9-9:
⚠️ Potential issuePotential XSS vulnerability with unescaped user content.
The
node.heading
andnode.content
values are being inserted directly into HTML templates without explicit escaping. This could lead to XSS attacks if users provide malicious content.Consider adding explicit HTML escaping or verify that the content is sanitized upstream:
+const {escapeHtml} = require('../render-utils/escape-html'); function cardTemplate({node}) { return ( ` <div class="kg-card kg-toggle-card" data-kg-toggle-state="close"> <div class="kg-toggle-heading"> - <h4 class="kg-toggle-heading-text">${node.heading}</h4> + <h4 class="kg-toggle-heading-text">${escapeHtml(node.heading)}</h4> <button class="kg-toggle-card-icon" aria-label="Expand toggle to read content"> <svg id="Regular" xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24"> <path class="cls-1" d="M23.25,7.311,12.53,18.03a.749.749,0,0,1-1.06,0L.75,7.311"></path> </svg> </button> </div> - <div class="kg-toggle-content">${node.content}</div> + <div class="kg-toggle-content">${escapeHtml(node.content)}</div> </div> ` ); }Also applies to: 16-16, 30-30, 35-35, 48-49
🤖 Prompt for AI Agents
In ghost/core/core/server/services/koenig/node-renderers/toggle-renderer.js at lines 9, 16, 30, 35, and 48-49, the code inserts node.heading and node.content directly into HTML without escaping, risking XSS attacks. To fix this, apply proper HTML escaping to these values before inserting them into the template or ensure they are sanitized upstream. Use a reliable escaping utility to convert special characters to safe HTML entities to prevent script injection.
55-69: 🛠️ Refactor suggestion
Add validation for required node properties.
The function doesn't validate that
node.heading
andnode.content
exist before using them, which could lead to rendering "undefined" in the output.function renderToggleNode(node, options = {}) { addCreateDocumentOption(options); + + // Validate required properties + if (!node.heading || !node.content) { + const document = options.createDocument(); + return renderEmptyContainer(document); + } const document = options.createDocument();You'll also need to import
renderEmptyContainer
:const {addCreateDocumentOption} = require('../render-utils/add-create-document-option'); +const {renderEmptyContainer} = require('../render-utils/render-empty-container'); const {html} = require('../render-utils/tagged-template-fns.js');
📝 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.// At the top of the file, alongside the other requires: const {addCreateDocumentOption} = require('../render-utils/add-create-document-option'); const {renderEmptyContainer} = require('../render-utils/render-empty-container'); const {html} = require('../render-utils/tagged-template-fns.js'); function renderToggleNode(node, options = {}) { addCreateDocumentOption(options); // Validate required properties if (!node.heading || !node.content) { const document = options.createDocument(); return renderEmptyContainer(document); } const document = options.createDocument(); const htmlString = options.target === 'email' ? emailCardTemplate({node}, options) : cardTemplate({node}); const container = document.createElement('div'); container.innerHTML = htmlString.trim(); const element = container.firstElementChild; return {element}; }
🤖 Prompt for AI Agents
In ghost/core/core/server/services/koenig/node-renderers/toggle-renderer.js around lines 55 to 69, add validation to check that node.heading and node.content exist before rendering to prevent "undefined" appearing in the output. If either property is missing, return the result of renderEmptyContainer instead. Also, import renderEmptyContainer at the top of the file to use it for this validation fallback.
ghost/core/core/server/services/koenig/node-renderers/embed/types/twitter.js (5)
36-41: 🛠️ Refactor suggestion
Apply optional chaining for nested property access.
Static analysis correctly identifies more opportunities for optional chaining.
- const hasImageOrVideo = tweetData.attachments && tweetData.attachments && tweetData.attachments.media_keys; + const hasImageOrVideo = tweetData.attachments?.media_keys; - const hasPoll = tweetData.attachments && tweetData.attachments && tweetData.attachments.poll_ids; + const hasPoll = tweetData.attachments?.poll_ids;📝 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 hasImageOrVideo = tweetData.attachments?.media_keys; if (hasImageOrVideo) { tweetImageUrl = tweetData.includes.media[0].preview_image_url || tweetData.includes.media[0].url; } const hasPoll = tweetData.attachments?.poll_ids;
🧰 Tools
🪛 Biome (1.9.4)
[error] 36-36: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 40-40: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🤖 Prompt for AI Agents
In ghost/core/core/server/services/koenig/node-renderers/embed/types/twitter.js between lines 36 and 41, the code redundantly checks nested properties without using optional chaining, which can lead to errors if intermediate properties are undefined. Refactor the conditions to use optional chaining (?.) for accessing nested properties like tweetData.attachments?.media_keys and tweetData.attachments?.poll_ids to safely handle cases where attachments or nested properties might be missing.
29-33: 🛠️ Refactor suggestion
Apply static analysis suggestions for optional chaining.
The static analysis tool correctly identifies opportunities to use optional chaining for safer property access.
- const mentions = tweetData.entities && tweetData.entities.mentions || []; - const urls = tweetData.entities && tweetData.entities.urls || []; - const hashtags = tweetData.entities && tweetData.entities.hashtags || []; + const mentions = tweetData.entities?.mentions || []; + const urls = tweetData.entities?.urls || []; + const hashtags = tweetData.entities?.hashtags || [];📝 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 mentions = tweetData.entities?.mentions || []; const urls = tweetData.entities?.urls || []; const hashtags = tweetData.entities?.hashtags || []; const entities = mentions.concat(urls).concat(hashtags).sort((a, b) => a.start - b.start); let tweetContent = tweetData.text;
🧰 Tools
🪛 Biome (1.9.4)
[error] 29-29: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 30-30: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 31-31: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🤖 Prompt for AI Agents
In ghost/core/core/server/services/koenig/node-renderers/embed/types/twitter.js around lines 29 to 33, replace the current property access on tweetData.entities with optional chaining to safely access mentions, urls, and hashtags. Change expressions like tweetData.entities && tweetData.entities.mentions to tweetData.entities?.mentions to improve code safety and readability.
95-156: 💡 Verification agent
🧩 Analysis chain
Complex email template with potential security concerns.
The email template generation is extensive but properly escapes user content. However, verify that all interpolated values are safely escaped, especially the tweet content which may contain user-generated HTML.
🏁 Script executed:
#!/bin/bash # Description: Check for HTML escaping patterns in similar renderers # Look for HTML escaping in other email templates rg -A 3 -B 3 "escapeHtml|innerHTML.*trim" --type js # Check if tweetContent needs additional escaping rg -A 5 "innerHTML.*content|content.*innerHTML" --type jsLength of output: 74123
Escape user-generated tweet content to prevent XSS
The Twitter card renderer currently interpolates
tweetContent
(and other fields) directly into the email HTML without escaping. To match the approach used elsewhere (e.g.escape-html
in truncate.js), wrap all dynamic values inescapeHtml
.• File:
ghost/core/core/server/services/koenig/node-renderers/embed/types/twitter.js
- Add at the top:
const {escapeHtml} = require('../../../render-utils/escape-html');- Around line 100 (where
tweetContent
is injected), update:- ${tweetContent} + ${escapeHtml(tweetContent)}• While you’re here, consider escaping other interpolated values as well (e.g.
authorUser.name
,authorUser.username
,tweetImageUrl
,tweetTime
,tweetDate
).🤖 Prompt for AI Agents
In ghost/core/core/server/services/koenig/node-renderers/embed/types/twitter.js around lines 95 to 156, the code directly injects user-generated content like tweetContent, authorUser.name, authorUser.username, tweetImageUrl, tweetTime, and tweetDate into the HTML without escaping, which risks XSS vulnerabilities. To fix this, import the escapeHtml function from '../../../render-utils/escape-html' at the top of the file, then wrap all these dynamic values with escapeHtml before injecting them into the HTML template to ensure proper escaping and prevent XSS attacks.
15-28: 💡 Verification agent
🧩 Analysis chain
Verify date formatting and number formatting behavior.
The date and number formatting setup looks correct, but ensure that the locale-specific formatting aligns with Ghost's internationalization requirements.
🏁 Script executed:
#!/bin/bash # Description: Check if Ghost has specific locale requirements for date/number formatting # Search for existing date formatting patterns in Ghost codebase rg -A 3 -B 3 "DateTime\.from.*toLocaleString" --type js # Search for number formatting patterns rg -A 3 -B 3 "Intl\.NumberFormat" --type jsLength of output: 10251
Ensure email embeds respect user locale and timezone
The Twitter‐embed renderer is hard-coding “en-US” and omitting timezone/locale settings, but other email renderers (e.g. EmailRenderer.js) use the site’s locale/timezone. Update the code in
ghost/core/core/server/services/koenig/node-renderers/embed/types/twitter.js
(around lines 15–28) to pull inlocale
andtimezone
from settings and apply them consistently:– Replace
const numberFormatter = new Intl.NumberFormat('en-US', { style: 'decimal', notation: 'compact', unitDisplay: 'narrow', maximumFractionDigits: 1 });with something like
const locale = this.#settingsCache.get('locale', DEFAULT_LOCALE); const timezone = this.#settingsCache.get('timezone'); const numberFormatter = new Intl.NumberFormat(locale, { style: 'decimal', notation: 'compact', unitDisplay: 'narrow', maximumFractionDigits: 1 });– Replace
const tweetTime = DateTime.fromISO(tweetData.created_at).toLocaleString(DateTime.TIME_SIMPLE); const tweetDate = DateTime.fromISO(tweetData.created_at).toLocaleString(DateTime.DATE_MED);with
const dt = DateTime.fromISO(tweetData.created_at) .setZone(timezone) .setLocale(locale); const tweetTime = dt.toLocaleString(DateTime.TIME_SIMPLE); const tweetDate = dt.toLocaleString(DateTime.DATE_MED);This aligns with the patterns in
EmailRenderer.js
and ensures embeds honor Ghost’s internationalization settings.🧰 Tools
🪛 Biome (1.9.4)
[error] 25-25: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🤖 Prompt for AI Agents
In ghost/core/core/server/services/koenig/node-renderers/embed/types/twitter.js around lines 15 to 28, the code currently hardcodes the locale as 'en-US' and does not apply the site's timezone or locale settings when formatting numbers and dates. To fix this, retrieve the locale and timezone from the settings cache (e.g., this.#settingsCache.get('locale', DEFAULT_LOCALE) and this.#settingsCache.get('timezone')), then use these values to create the Intl.NumberFormat instance and to set the zone and locale on the DateTime object before formatting tweetTime and tweetDate. This will ensure the formatting respects the user's locale and timezone as done in other email renderers.
42-93: 💡 Verification agent
🧩 Analysis chain
Complex entity parsing logic needs validation.
The entity parsing and content replacement logic is complex. Ensure that the entity boundaries and text replacement work correctly for all Twitter entity types.
🏁 Script executed:
#!/bin/bash # Description: Look for existing Twitter entity parsing logic in Ghost codebase for comparison # Search for Twitter-related entity processing rg -A 5 -B 5 "entities.*mentions|entities.*urls|entities.*hashtags" --type js # Search for existing Twitter parsing patterns rg -A 10 "twitter.*entities|tweet.*entities" --type jsLength of output: 7464
Handle all Twitter entity types in the renderer
The current parsing logic createsimg_url
parts but never renders them, causing image URLs to be dropped.• File:
ghost/core/core/server/services/koenig/node-renderers/embed/types/twitter.js
Location: inside theparts.reduce
callbackSuggested fix (add before the final
return partContent;
):if (part.type === 'url') { return partContent + `<span style="color: #1DA1F2; word-break: break-all;">${part.data}</span>`; } + if (part.type === 'img_url') { + // Render embedded images for Twitter media URLs + return partContent + `<img src="${part.data}" alt="" style="max-width:100%;"/>`; + } return partContent;📝 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.tweetContent = parts.reduce((partContent, part) => { if (part.type === 'text') { return partContent + part.data; } if (part.type === 'mention') { return partContent + `<span style="color: #1DA1F2;">${part.data}</span>`; } if (part.type === 'hashtag') { return partContent + `<span style="color: #1DA1F2;">${part.data}</span>`; } if (part.type === 'url') { return partContent + `<span style="color: #1DA1F2; word-break: break-all;">${part.data}</span>`; } + if (part.type === 'img_url') { + // Render embedded images for Twitter media URLs + return partContent + `<img src="${part.data}" alt="" style="max-width:100%;"/>`; + } return partContent; }, '');
🤖 Prompt for AI Agents
In ghost/core/core/server/services/koenig/node-renderers/embed/types/twitter.js between lines 42 and 93, the code creates parts with type 'img_url' for image URLs but does not handle rendering them in the parts.reduce callback, causing these image URLs to be dropped. To fix this, add a condition in the reduce function to detect parts with type 'img_url' and append the appropriate HTML to render the image (e.g., an <img> tag with the URL as the source) before the final return statement in the callback.
ghost/core/core/server/services/koenig/node-renderers/codeblock-renderer.js (1)
15-17: 🛠️ Refactor suggestion
Validate language input for XSS protection.
The
node.language
value is directly interpolated into a CSS class name without validation. This could potentially lead to XSS if the language contains malicious content.Consider validating the language input:
if (node.language) { - code.setAttribute('class', `language-${node.language}`); + // Only allow alphanumeric characters and common language identifiers + const sanitizedLanguage = node.language.replace(/[^a-zA-Z0-9\-_]/g, ''); + if (sanitizedLanguage) { + code.setAttribute('class', `language-${sanitizedLanguage}`); + } }📝 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.if (node.language) { // Only allow alphanumeric characters and common language identifiers const sanitizedLanguage = node.language.replace(/[^a-zA-Z0-9\-_]/g, ''); if (sanitizedLanguage) { code.setAttribute('class', `language-${sanitizedLanguage}`); } }
🤖 Prompt for AI Agents
In ghost/core/core/server/services/koenig/node-renderers/codeblock-renderer.js around lines 15 to 17, the node.language value is directly used in a CSS class without validation, risking XSS attacks. Fix this by validating node.language against a whitelist of allowed language identifiers or by sanitizing the input to ensure it contains only safe characters (e.g., alphanumeric and hyphens). Only set the class attribute if the language passes validation to prevent injection of malicious content.
ghost/core/core/server/services/koenig/render-utils/truncate.js (1)
23-30: 🛠️ Refactor suggestion
Remove redundant condition check.
The condition on line 23
text.length > maxLengthMobile
is redundant since it was already checked on line 19. Also, the nested condition on line 26 checks the same thing again.Simplify the logic:
-if (text && text.length > maxLengthMobile) { +if (text) { let ellipsis = ''; - if (text.length > maxLengthMobile && text.length <= maxLength) { + if (text.length <= maxLength) { ellipsis = '<span class="hide-desktop">…</span>'; } else if (text.length > maxLength) { ellipsis = '…'; }Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In ghost/core/core/server/services/koenig/render-utils/truncate.js between lines 23 and 30, the condition text.length > maxLengthMobile is redundantly checked multiple times. Remove the initial if check on line 23 since line 19 already ensures text length exceeds maxLengthMobile. Then simplify the nested conditions by only checking if text length is less than or equal to maxLength or greater than maxLength to set the ellipsis accordingly.
ghost/core/core/server/services/koenig/node-renderers/callout-renderer.js (1)
12-14: 🛠️ Refactor suggestion
Avoid mutating input node object.
The function directly mutates the input
node.backgroundColor
property, which could be unexpected behavior for callers who don't expect their input to be modified.Consider using a local variable instead:
// backgroundColor can end up with `rgba(0, 0, 0, 0)` from old mobiledoc copy/paste // that is invalid when used in a class name so fall back to `white` when we don't have // something that looks like a valid class -if (!node.backgroundColor || !node.backgroundColor.match(/^[a-zA-Z\d-]+$/)) { - node.backgroundColor = 'white'; -} +const backgroundColor = (!node.backgroundColor || !node.backgroundColor.match(/^[a-zA-Z\d-]+$/)) + ? 'white' + : node.backgroundColor; -element.classList.add('kg-card', 'kg-callout-card', `kg-callout-card-${node.backgroundColor}`); +element.classList.add('kg-card', 'kg-callout-card', `kg-callout-card-${backgroundColor}`);📝 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.// backgroundColor can end up with `rgba(0, 0, 0, 0)` from old mobiledoc copy/paste // that is invalid when used in a class name so fall back to `white` when we don't have // something that looks like a valid class const backgroundColor = (!node.backgroundColor || !node.backgroundColor.match(/^[a-zA-Z\d-]+$/)) ? 'white' : node.backgroundColor; // … other rendering logic … element.classList.add( 'kg-card', 'kg-callout-card', `kg-callout-card-${backgroundColor}` );
🤖 Prompt for AI Agents
In ghost/core/core/server/services/koenig/node-renderers/callout-renderer.js around lines 12 to 14, the code directly mutates the input node object's backgroundColor property, which can cause unexpected side effects. To fix this, avoid modifying node.backgroundColor directly; instead, assign the validated or default background color to a new local variable and use that variable in subsequent logic without altering the original node object.
ghost/core/core/server/services/koenig/node-renderers/header-v1-renderer.js (2)
26-27: 💡 Verification agent
❓ Verification inconclusive
Direct style injection should be validated.
The background image style is directly injected without validation. Ensure that
backgroundImageSrc
is properly validated and sanitized to prevent CSS injection attacks.Also applies to: 32-33
🏁 Script executed:
#!/bin/bash # Search for validation of backgroundImageSrc in the codebase rg -A 3 -B 3 "backgroundImageSrc.*validat|validat.*backgroundImageSrc"Length of output: 72
Validate and sanitize
backgroundImageSrc
before injecting into CSSThe
backgroundImageSrc
value is being interpolated directly into a CSSurl()
declaration, which can open the door to CSS injection if an attacker controls or manipulates that string. We need to ensure that only safe, allowed URLs or paths are used here.Please address in ghost/core/core/server/services/koenig/node-renderers/header-v1-renderer.js:
- Lines 26–27:
backgroundImageStyle: node.style === 'image' ? `background-image: url(${node.backgroundImageSrc})` : '', backgroundImageSrc: node.backgroundImageSrc- Similarly on lines 32–33 (secondary renderer call)
Recommendations:
- Whitelist only
https://
(or your trusted) protocols.- URL-encode or escape characters that break out of
url()
.- Leverage an existing sanitization utility (e.g., Ghost’s XSS sanitizers or a CSS-safe URL encoder) before interpolation.
Fixing this will prevent malicious payloads from injecting arbitrary CSS rules.
🤖 Prompt for AI Agents
In ghost/core/core/server/services/koenig/node-renderers/header-v1-renderer.js around lines 26-27 and 32-33, the backgroundImageSrc is directly interpolated into a CSS url() without validation, risking CSS injection. To fix this, validate that backgroundImageSrc starts with a trusted protocol like "https://", then sanitize or URL-encode the value to escape characters that could break out of the url() context. Use an existing sanitization utility from Ghost or a CSS-safe URL encoder before injecting the value into the style string.
39-40: 💡 Verification agent
🧩 Analysis chain
Consider XSS risk with innerHTML usage.
Using
innerHTML
for header and subheader content could pose XSS risks if the content is not properly sanitized upstream. Verify that content sanitization occurs before reaching this renderer.Also applies to: 47-48
🏁 Script executed:
#!/bin/bash # Search for sanitization or validation of header/subheader content in the codebase rg -A 5 -B 5 "header.*sanitiz|sanitiz.*header|header.*escape|escape.*header"Length of output: 10965
Ensure header and subheader are safely rendered
UsinginnerHTML
fortemplateData.header
(and similarly fortemplateData.subheader
) can introduce XSS if that content isn’t already sanitized. I didn’t find any upstream sanitization or escaping for these fields, so please update the renderer to either:
- Use
textContent
when you only need plain text, or- Sanitize the HTML (e.g. via
sanitize-html
or Ghost’s DOMPurify integration) before assigning toinnerHTML
.Locations to update:
- ghost/core/core/server/services/koenig/node-renderers/header-v1-renderer.js
- Lines 39–40:
- headerElement.innerHTML = templateData.header;
- // if no HTML needed:
- headerElement.textContent = templateData.header;
// OR, if HTML is required:
// headerElement.innerHTML = sanitize(templateData.header);
- Lines 47–48 (subheader): apply the same change.
🤖 Prompt for AI Agents
In ghost/core/core/server/services/koenig/node-renderers/header-v1-renderer.js at lines 39-40 and 47-48, the code assigns templateData.header and templateData.subheader directly to innerHTML, which risks XSS since no upstream sanitization is confirmed. To fix this, either replace innerHTML with textContent if only plain text is needed, or integrate a sanitization step (using a library like sanitize-html or Ghost’s DOMPurify) to clean the HTML content before assigning it to innerHTML, ensuring safe rendering of header and subheader.
ghost/core/core/server/services/koenig/node-renderers/gallery-renderer.js (2)
128-133:
⚠️ Potential issueURL constructor could throw for invalid URLs.
The
new URL(image.src)
constructor will throw an error ifimage.src
is not a valid URL. This should be wrapped in a try-catch block.if (isUnsplashImage(image.src)) { - const unsplashUrl = new URL(image.src); - unsplashUrl.searchParams.set('w', '1200'); - img.setAttribute('src', unsplashUrl.href); + try { + const unsplashUrl = new URL(image.src); + unsplashUrl.searchParams.set('w', '1200'); + img.setAttribute('src', unsplashUrl.href); + } catch (error) { + // Invalid URL, skip modification + } }📝 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.if (isUnsplashImage(image.src)) { try { const unsplashUrl = new URL(image.src); unsplashUrl.searchParams.set('w', '1200'); img.setAttribute('src', unsplashUrl.href); } catch (error) { // Invalid URL, skip modification } }
🤖 Prompt for AI Agents
In ghost/core/core/server/services/koenig/node-renderers/gallery-renderer.js around lines 128 to 133, the code uses the URL constructor with image.src which can throw an error if the URL is invalid. Wrap the creation of the URL object and subsequent operations inside a try-catch block to safely handle any exceptions and prevent the renderer from crashing. In the catch block, you can either skip modifying the src or log the error as appropriate.
123-125:
⚠️ Potential issueURL regex parsing could fail for malformed URLs.
The regex match
image.src.match(/(.*\/content\/images)\/(.*)/)
assumes a specific URL structure. If the URL doesn't match this pattern, the destructuring assignment will throw an error.- const [, imagesPath, filename] = image.src.match(/(.*\/content\/images)\/(.*)/); - img.setAttribute('src', `${imagesPath}/size/w${srcWidth}/${filename}`); + const match = image.src.match(/(.*\/content\/images)\/(.*)/); + if (match) { + const [, imagesPath, filename] = match; + img.setAttribute('src', `${imagesPath}/size/w${srcWidth}/${filename}`); + }📝 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 match = image.src.match(/(.*\/content\/images)\/(.*)/); if (match) { const [, imagesPath, filename] = match; img.setAttribute('src', `${imagesPath}/size/w${srcWidth}/${filename}`); }
🤖 Prompt for AI Agents
In ghost/core/core/server/services/koenig/node-renderers/gallery-renderer.js around lines 123 to 125, the regex match on image.src assumes the URL always matches the pattern, which can cause an error if it doesn't. To fix this, first check if the match result is not null before destructuring. If the match fails, handle the case gracefully, such as skipping the modification or using a fallback, to avoid runtime errors.
ghost/core/core/server/services/koenig/node-renderers/embed-renderer.js (1)
67-67:
⚠️ Potential issueSecurity concern: Potential XSS vulnerability.
The direct assignment of
caption
toinnerHTML
could introduce XSS vulnerabilities if the caption contains untrusted HTML content.Consider using a safer method to set caption content:
- figcaption.innerHTML = caption; + figcaption.textContent = caption;Or if HTML formatting in captions is intentional, ensure the caption is properly sanitized before rendering.
📝 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.figcaption.textContent = caption;
🤖 Prompt for AI Agents
In ghost/core/core/server/services/koenig/node-renderers/embed-renderer.js at line 67, assigning the caption directly to figcaption.innerHTML poses an XSS risk. To fix this, replace the innerHTML assignment with a safer method such as setting textContent to the caption if HTML is not needed, or sanitize the caption thoroughly before assigning it to innerHTML if HTML formatting is required.
ghost/core/core/server/services/koenig/node-renderers/image-renderer.js (3)
124-124:
⚠️ Potential issueSecurity concern: Potential XSS vulnerability in caption.
Similar to the embed renderer, directly setting
innerHTML
with caption content poses an XSS risk if the caption contains untrusted HTML.Consider using a safer approach:
- caption.innerHTML = node.caption; + caption.textContent = node.caption;Or if HTML formatting in captions is required, ensure proper sanitization is applied.
📝 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.caption.textContent = node.caption;
🤖 Prompt for AI Agents
In ghost/core/core/server/services/koenig/node-renderers/image-renderer.js at line 124, directly assigning node.caption to caption.innerHTML creates a potential XSS vulnerability. To fix this, avoid using innerHTML for untrusted content; instead, set the caption text using textContent or apply proper sanitization to the caption before assigning it to innerHTML if HTML formatting is necessary.
32-32: 🛠️ Refactor suggestion
Add null safety for alt attribute.
The code directly sets the alt attribute without checking if
node.alt
exists, which could result in setting "undefined" or "null" as the alt text.- img.setAttribute('alt', node.alt); + img.setAttribute('alt', node.alt || '');📝 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.img.setAttribute('alt', node.alt || '');
🤖 Prompt for AI Agents
In ghost/core/core/server/services/koenig/node-renderers/image-renderer.js at line 32, the alt attribute is set directly from node.alt without checking if it exists, which can lead to "undefined" or "null" being assigned. Add a null safety check to ensure that the alt attribute is only set if node.alt is a valid non-null, non-undefined value, or provide a default empty string to avoid invalid alt text.
107-109: 🛠️ Refactor suggestion
Add error handling for URL regex matching.
The regex match could fail if the URL doesn't match the expected pattern, potentially causing a runtime error when destructuring the match result.
- const [, imagesPath, filename] = node.src.match(/(.*\/content\/images)\/(.*)/); - img.setAttribute('src', `${imagesPath}/size/w${srcWidth}/${filename}`); + const match = node.src.match(/(.*\/content\/images)\/(.*)/); + if (match) { + const [, imagesPath, filename] = match; + img.setAttribute('src', `${imagesPath}/size/w${srcWidth}/${filename}`); + }📝 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 match = node.src.match(/(.*\/content\/images)\/(.*)/); if (match) { const [, imagesPath, filename] = match; img.setAttribute('src', `${imagesPath}/size/w${srcWidth}/${filename}`); }
🤖 Prompt for AI Agents
In ghost/core/core/server/services/koenig/node-renderers/image-renderer.js around lines 107 to 109, the code destructures the result of a regex match without checking if the match succeeded, which can cause runtime errors if the URL doesn't match the pattern. To fix this, add a check to verify that the match result is not null before destructuring and using it. If the match fails, handle the error gracefully, for example by skipping the attribute setting or logging a warning.
ghost/core/test/unit/server/services/koenig/node-renderers/bookmark-renderer.test.js (2)
100-101:
⚠️ Potential issueFix invalid HTML attributes in email rendering.
The
src
attribute is invalid on<span>
elements. The expected output shows:<span class="kg-bookmark-author" src="Ghost - The Professional Publishing Platform"> <span class="kg-bookmark-publisher" src="ghost">The
src
attribute should only be used on elements like<img>
,<script>
,<iframe>
, etc. This will cause HTML validation errors.🤖 Prompt for AI Agents
In ghost/core/test/unit/server/services/koenig/node-renderers/bookmark-renderer.test.js around lines 100 to 101, the <span> elements incorrectly use the src attribute, which is invalid HTML. Remove the src attributes from these <span> tags to fix the HTML validation errors, ensuring only valid attributes remain on the elements.
43-44: 💡 Verification agent
🧩 Analysis chain
Verify author and publisher field mapping.
The expected output appears to have swapped the author and publisher values. In the test data:
author: 'ghost'
publisher: 'Ghost - The Professional Publishing Platform'
But in the expected HTML:
kg-bookmark-author
class contains "Ghost - The Professional Publishing Platform"kg-bookmark-publisher
class contains "ghost"This suggests either the test data or expected output is incorrect.
🏁 Script executed:
#!/bin/bash # Search for how author/publisher fields are handled in the bookmark renderer ast-grep --pattern 'kg-bookmark-author' ast-grep --pattern 'kg-bookmark-publisher'Length of output: 1489
To verify the field mapping in the renderer, let’s pull in the actual implementation and inspect how
author
andpublisher
are assigned:
🏁 Script executed:
#!/bin/bash # Locate and display the bookmark-renderer implementation to confirm field mappings file=$(fd -I -t f -p bookmark-renderer.js) echo "Bookmark renderer file found at: $file" sed -n '1,200p' "$file"Length of output: 9225
Remove invalid
src
attributes on<span>
in emailTemplateThe swapped mapping of
publisher → .kg-bookmark-author
andauthor → .kg-bookmark-publisher
is intentional (for theme backwards-compatibility), so no change is needed there. However, HTML<span>
elements do not support asrc
attribute—these should be removed to produce valid markup and match test expectations.• File:
ghost/core/core/server/services/koenig/node-renderers/bookmark-renderer.js
InemailTemplate
, update the two<span>
lines:- ${publisher ? `<span class="kg-bookmark-author" src="${publisher}">${publisher}</span>` : ''} - ${author ? `<span class="kg-bookmark-publisher" src="${author}">${author}</span>` : ''} + ${publisher ? `<span class="kg-bookmark-author">${publisher}</span>` : ''} + ${author ? `<span class="kg-bookmark-publisher">${author}</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.${publisher ? `<span class="kg-bookmark-author">${publisher}</span>` : ''} ${author ? `<span class="kg-bookmark-publisher">${author}</span>` : ''}
🤖 Prompt for AI Agents
In ghost/core/test/unit/server/services/koenig/node-renderers/bookmark-renderer.test.js around lines 43 to 44, the test's expected HTML output includes invalid src attributes on <span> elements within the emailTemplate. Since <span> elements do not support src attributes, remove these src attributes from the <span> tags for kg-bookmark-author and kg-bookmark-publisher to ensure valid HTML and alignment with test expectations, while keeping the swapped author and publisher text mapping unchanged for backwards compatibility.
ghost/core/test/unit/server/services/koenig/node-renderers/button-renderer.test.js (1)
101-103: 🛠️ Refactor suggestion
Fix invalid HTML output for email customization.
The TODO comment indicates that the HTML output is invalid and doesn't work with prettier. Invalid HTML can cause rendering issues in email clients and should be resolved.
Consider replacing this with the alpha version mentioned in the comment, or fix the HTML structure to be valid.
🤖 Prompt for AI Agents
In ghost/core/test/unit/server/services/koenig/node-renderers/button-renderer.test.js around lines 101 to 103, the HTML output in the test assertion is invalid and incompatible with prettier, causing potential rendering issues. Update the test to use the corrected alpha version of the HTML output as indicated in the TODO comment, or modify the HTML structure to ensure it is valid and well-formed, so the test reflects proper email-compatible HTML.
ghost/core/test/unit/server/services/koenig/node-renderers/product-renderer.test.js (1)
159-159: 💡 Verification agent
🧩 Analysis chain
Verify whitespace handling in HTML content.
The expected output shows missing spaces before HTML tags:
This is a<b>title</b>
(should beThis is a <b>title</b>
)This is a<b>description</b>
(should beThis is a <b>description</b>
)This pattern appears consistently across different email customization modes. Verify that the renderer preserves whitespace correctly when processing HTML content, as missing spaces can affect readability.
Also applies to: 171-171, 230-230, 242-242
🏁 Script executed:
#!/bin/bash # Check how the product renderer handles HTML content and whitespace ast-grep --pattern 'productTitle' ast-grep --pattern 'productDescription'Length of output: 526
🏁 Script executed:
#!/bin/bash set -e # Locate the product-renderer implementation file=$(fd -t f product-renderer.js | head -n1) echo "Found renderer: $file" # Show the lines where title and description HTML are generated rg -n 'kg-product-title' "$file" -C 5 rg -n 'kg-product-description' "$file" -C 5Length of output: 1889
Renderer preserves whitespace – update test expectations
The
product-renderer
(ghost/core/core/server/services/koenig/node-renderers/product-renderer.js) rendersdata.productTitle
anddata.productDescription
verbatim (lines 108 and 119). It does not insert or remove spaces around HTML tags. The missing space in your test output is coming from the test data, not the renderer.Action items:
- In
ghost/core/test/unit/server/services/koenig/node-renderers/product-renderer.test.js
, update the HTML in your test fixtures to include a space before<b>
in both the title and description:
- Lines 159, 171, 230, 242: change
This is a<b>…
toThis is a <b>…
- Update any affected snapshots or inline expected strings accordingly.
No changes are required to the renderer itself.
🤖 Prompt for AI Agents
In ghost/core/test/unit/server/services/koenig/node-renderers/product-renderer.test.js at lines 159, 171, 230, and 242, the test fixtures have HTML strings missing a space before the <b> tag (e.g., "This is a<b>title</b>"). Update these test fixtures to include a space before the <b> tag (e.g., "This is a <b>title</b>") to match the actual renderer behavior, which preserves whitespace verbatim. Also update any related snapshots or expected output strings to reflect this corrected spacing. No changes are needed in the renderer code itself.
ghost/core/core/server/services/koenig/node-renderers/email-cta-renderer.js (2)
50-51:
⚠️ Potential issueConsistent XSS concern with button template.
Similar to the HTML content, the button template is cleaned but the base template itself may contain unescaped content that could be manipulated.
The button template variables are properly escaped, but ensure the cleaning functions are robust against all attack vectors. Consider using a dedicated HTML sanitization library for email content.
🤖 Prompt for AI Agents
In ghost/core/core/server/services/koenig/node-renderers/email-cta-renderer.js around lines 50 to 51, the button template is cleaned but may still contain unescaped or unsafe HTML that could lead to XSS vulnerabilities. To fix this, replace or augment the current cleaning functions with a robust, dedicated HTML sanitization library designed for email content to ensure all injected content is properly escaped and safe before setting innerHTML.
31-32:
⚠️ Potential issuePotential XSS vulnerability: HTML content is not properly escaped.
The
html
content from the node is cleaned but not escaped before being added toinnerHTML
. This could allow XSS attacks if malicious HTML is present in the node data.Consider escaping the HTML content or using a more secure method:
-const cleanedHtml = wrapReplacementStrings(removeCodeWrappersFromHelpers(removeSpaces(html),document)); -element.innerHTML = element.innerHTML + cleanedHtml; +const cleanedHtml = wrapReplacementStrings(removeCodeWrappersFromHelpers(removeSpaces(html),document)); +// Use textContent for safer insertion or implement proper HTML sanitization +element.innerHTML = element.innerHTML + escapeHtml(cleanedHtml);Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In ghost/core/core/server/services/koenig/node-renderers/email-cta-renderer.js around lines 31 to 32, the html content is cleaned but directly appended to element.innerHTML without escaping, which risks XSS vulnerabilities. To fix this, escape the html content properly before appending or use a safer method such as creating DOM elements and setting textContent or using a library function that sanitizes and escapes HTML to prevent injection of malicious scripts.
ghost/core/core/server/services/koenig/node-renderers/product-renderer.js (2)
94-139:
⚠️ Potential issueXSS vulnerability: Unescaped product data in email templates.
Similar XSS issues exist in both email template variations where product data is interpolated without escaping.
Apply HTML escaping consistently across all email template content, particularly for:
data.productImageSrc
data.productTitle
data.productDescription
data.productUrl
data.productButton
The same escaping pattern should be applied to both email template branches.
Also applies to: 142-180
🤖 Prompt for AI Agents
In ghost/core/core/server/services/koenig/node-renderers/product-renderer.js around lines 94 to 139, the product data interpolated into the email template is not HTML-escaped, causing an XSS vulnerability. To fix this, apply consistent HTML escaping to all interpolated product data fields including productImageSrc, productTitle, productDescription, productUrl, and productButton before inserting them into the template strings. Ensure the same escaping approach is applied to the similar code block between lines 142 and 180 to fully mitigate the issue.
54-70:
⚠️ Potential issueXSS vulnerability: Unescaped product data in template.
Multiple product data fields are directly interpolated into the HTML template without escaping, creating potential XSS vulnerabilities.
Apply HTML escaping to all dynamic content:
-<img src="${data.productImageSrc}" ${data.productImageWidth ? `width="${data.productImageWidth}"` : ''} ${data.productImageHeight ? `height="${data.productImageHeight}"` : ''} class="kg-product-card-image" loading="lazy" /> +<img src="${escapeHtml(data.productImageSrc)}" ${data.productImageWidth ? `width="${escapeHtml(data.productImageWidth)}"` : ''} ${data.productImageHeight ? `height="${escapeHtml(data.productImageHeight)}"` : ''} class="kg-product-card-image" loading="lazy" /> -<h4 class="kg-product-card-title">${data.productTitle}</h4> +<h4 class="kg-product-card-title">${escapeHtml(data.productTitle)}</h4> -<div class="kg-product-card-description">${data.productDescription}</div> +<div class="kg-product-card-description">${escapeHtml(data.productDescription)}</div> -<a href="${data.productUrl}" class="kg-product-card-button kg-product-card-btn-accent" target="_blank" rel="noopener noreferrer"><span>${data.productButton}</span></a> +<a href="${escapeHtml(data.productUrl)}" class="kg-product-card-button kg-product-card-btn-accent" target="_blank" rel="noopener noreferrer"><span>${escapeHtml(data.productButton)}</span></a>Add the escapeHtml import at the top of the file.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In ghost/core/core/server/services/koenig/node-renderers/product-renderer.js around lines 54 to 70, multiple product data fields are directly interpolated into the HTML template without escaping, causing potential XSS vulnerabilities. To fix this, import the escapeHtml function at the top of the file and apply it to all dynamic content variables such as productImageSrc, productImageWidth, productImageHeight, productTitle, star1 to star5, starIcon, productDescription, productUrl, and productButton before inserting them into the template string.
ghost/core/core/server/services/koenig/render-utils/srcset-attribute.js (2)
19-36:
⚠️ Potential issuePotential security issue: Regex extraction without validation.
The regex match on line 20 could fail if the URL doesn't match the expected pattern, leading to destructuring undefined values.
Add proper validation for the regex match:
if (isLocalContentImage(src, options.siteUrl)) { - const [, imagesPath, filename] = src.match(/(.*\/content\/images)\/(.*)/); + const match = src.match(/(.*\/content\/images)\/(.*)/); + if (!match) { + return; // Skip if URL doesn't match expected pattern + } + const [, imagesPath, filename] = match; const srcs = [];📝 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.if (isLocalContentImage(src, options.siteUrl)) { const match = src.match(/(.*\/content\/images)\/(.*)/); if (!match) { return; // Skip if URL doesn't match expected pattern } const [, imagesPath, filename] = match; const srcs = []; srcsetWidths.forEach((srcsetWidth) => { if (srcsetWidth === width) { // use original image path if width matches exactly (avoids 302s from size->original) srcs.push(`${src} ${srcsetWidth}w`); } else if (srcsetWidth <= width) { // avoid creating srcset sizes larger than intrinsic image width srcs.push(`${imagesPath}/size/w${srcsetWidth}/${filename} ${srcsetWidth}w`); } }); if (srcs.length) { return srcs.join(', '); } }
🤖 Prompt for AI Agents
In ghost/core/core/server/services/koenig/render-utils/srcset-attribute.js around lines 19 to 36, the regex match on line 20 is used without checking if it succeeded, which can cause errors if the URL doesn't match the expected pattern. Fix this by storing the match result in a variable, verifying it is not null before destructuring, and handling the case where the match fails gracefully to avoid runtime exceptions.
39-49: 💡 Verification agent
🧩 Analysis chain
Verify URL constructor error handling.
The
new URL(src)
constructor can throw an error for malformed URLs, but there's no error handling.Check if the calling code handles URL construction errors or if we need to add try-catch here:
🏁 Script executed:
#!/bin/bash # Search for usage of getSrcsetAttribute to see error handling patterns rg -A 10 -B 5 "getSrcsetAttribute|setSrcsetAttribute" --type jsLength of output: 14912
🏁 Script executed:
#!/bin/bash # Locate the isUnsplashImage implementation to verify its behavior rg -n "isUnsplashImage" -n ghost/core/core/server/services/koenig/render-utils/is-unsplash-image.jsLength of output: 164
🏁 Script executed:
#!/bin/bash # Display the implementation of isUnsplashImage to check error handling sed -n '1,200p' ghost/core/core/server/services/koenig/render-utils/is-unsplash-image.jsLength of output: 221
Add error handling around URL construction in srcset-attribute.js
The
new URL(src)
call can throw for malformedsrc
values, which would crash the renderer. We should wrap it in a try-catch and bail out gracefully.• File:
ghost/core/core/server/services/koenig/render-utils/srcset-attribute.js
Lines: ~39–49Proposed diff:
if (isUnsplashImage(src)) { - const unsplashUrl = new URL(src); + let unsplashUrl; + try { + unsplashUrl = new URL(src); + } catch (error) { + return; + } const srcs = []; srcsetWidths.forEach((srcsetWidth) => {📝 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.if (isUnsplashImage(src)) { let unsplashUrl; try { unsplashUrl = new URL(src); } catch (error) { return; } const srcs = []; srcsetWidths.forEach((srcsetWidth) => { unsplashUrl.searchParams.set('w', srcsetWidth); srcs.push(`${unsplashUrl.href} ${srcsetWidth}w`); }); return srcs.join(', '); }
🤖 Prompt for AI Agents
In ghost/core/core/server/services/koenig/render-utils/srcset-attribute.js around lines 39 to 49, the code calls new URL(src) without error handling, which can throw if src is malformed. Wrap the new URL(src) call in a try-catch block to catch any exceptions. If an error occurs, handle it gracefully by returning an empty string or a safe fallback value to prevent the renderer from crashing.
ghost/core/test/unit/server/services/koenig/node-renderers/call-to-action-renderer.test.js (1)
35-71: 🛠️ Refactor suggestion
Expand test coverage beyond happy path scenarios.
The current tests only verify the default test data scenario. Consider adding tests for:
- Missing or empty required fields (e.g.,
textValue
,buttonText
,imageUrl
)- Invalid URLs in
buttonUrl
andimageUrl
- HTML injection attempts in text fields
- Edge cases for color values and layout options
- Visibility settings variations
This will help ensure the renderer handles edge cases gracefully and maintains security.
🤖 Prompt for AI Agents
In ghost/core/test/unit/server/services/koenig/node-renderers/call-to-action-renderer.test.js between lines 35 and 71, the test coverage only includes the default happy path scenario. To improve robustness, add new test cases that cover missing or empty required fields like textValue, buttonText, and imageUrl; invalid URLs for buttonUrl and imageUrl; attempts at HTML injection in text fields; edge cases for color values and layout options; and variations in visibility settings. Each test should verify that the renderer handles these edge cases correctly and securely without breaking or producing unsafe output.
ghost/core/core/server/services/koenig/node-renderers/video-renderer.js (3)
35-35:
⚠️ Potential issueFix missing quotes in HTML attributes.
The
data-kg-thumbnail
anddata-kg-custom-thumbnail
attributes are missing quotes around their values, which could cause HTML parsing issues.- <figure class="${cardClasses}" data-kg-thumbnail=${node.thumbnailSrc} data-kg-custom-thumbnail=${node.customThumbnailSrc}> + <figure class="${cardClasses}" data-kg-thumbnail="${node.thumbnailSrc}" data-kg-custom-thumbnail="${node.customThumbnailSrc}">📝 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.<figure class="${cardClasses}" data-kg-thumbnail="${node.thumbnailSrc}" data-kg-custom-thumbnail="${node.customThumbnailSrc}">
🤖 Prompt for AI Agents
In ghost/core/core/server/services/koenig/node-renderers/video-renderer.js at line 35, the HTML attributes data-kg-thumbnail and data-kg-custom-thumbnail are missing quotes around their values. Fix this by adding double quotes around the attribute values to ensure proper HTML syntax and prevent parsing issues.
28-28: 🛠️ Refactor suggestion
Replace external dependency with local solution.
The hard-coded
spacergif.org
URL creates an external dependency that could fail or be compromised. Consider using a local transparent image or CSS-based solution instead.- const posterSpacerSrc = `https://img.spacergif.org/v1/${width}x${height}/0a/spacer.png`; + const posterSpacerSrc = 'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mNkYPhfDwAChwGA60e6kgAAAABJRU5ErkJggg=='; // 1x1 transparent PNG🤖 Prompt for AI Agents
In ghost/core/core/server/services/koenig/node-renderers/video-renderer.js at line 28, the code uses a hard-coded URL from spacergif.org for a transparent spacer image, creating an external dependency. Replace this URL with a local transparent image asset or implement a CSS-based transparent spacer to eliminate reliance on an external service and improve reliability.
38-45:
⚠️ Potential issueSanitize user-controlled values to prevent XSS.
Direct interpolation of
node.src
andthumbnailSrc
into HTML attributes without sanitization creates XSS vulnerabilities. These values should be properly escaped.Consider implementing HTML attribute escaping:
+const escapeHtml = (str) => str.replace(/[&<>"']/g, (match) => { + const escapeMap = { '&': '&', '<': '<', '>': '>', '"': '"', "'": ''' }; + return escapeMap[match]; +}); - src="${node.src}" + src="${escapeHtml(node.src)}" - style="background: transparent url('${thumbnailSrc}') 50% 50% / cover no-repeat;" + style="background: transparent url('${escapeHtml(thumbnailSrc)}') 50% 50% / cover no-repeat;"🤖 Prompt for AI Agents
In ghost/core/core/server/services/koenig/node-renderers/video-renderer.js around lines 38 to 45, the direct interpolation of user-controlled values node.src and thumbnailSrc into HTML attributes can lead to XSS vulnerabilities. To fix this, sanitize or escape these values before inserting them into the HTML attributes to prevent injection of malicious code. Use a proper HTML attribute escaping function or library to ensure these values are safe for rendering.
ghost/core/core/server/services/koenig/node-renderers/signup-renderer.js (3)
20-20:
⚠️ Potential issueCritical XSS vulnerability in label interpolation.
Direct interpolation of label values into hidden inputs without sanitization creates a critical XSS vulnerability. An attacker could inject malicious scripts through the labels array.
- ${nodeData.labels.map(label => `<input data-members-label type="hidden" value="${label}" />`).join('\n')} + ${nodeData.labels.map(label => `<input data-members-label type="hidden" value="${escapeHtml(label)}" />`).join('\n')}Add HTML escaping function:
const escapeHtml = (str) => str.replace(/[&<>"']/g, (match) => { const escapeMap = { '&': '&', '<': '<', '>': '>', '"': '"', "'": ''' }; return escapeMap[match]; });🤖 Prompt for AI Agents
In ghost/core/core/server/services/koenig/node-renderers/signup-renderer.js at line 20, the label values are directly interpolated into hidden input elements without escaping, causing a critical XSS vulnerability. To fix this, define an HTML escaping function that replaces &, <, >, ", and ' characters with their corresponding HTML entities, then apply this function to each label value before interpolation to ensure safe rendering.
41-44:
⚠️ Potential issueMultiple XSS vulnerabilities in content interpolation.
Direct interpolation of user-controlled content (
header
,subheader
,disclaimer
) into HTML without sanitization creates XSS vulnerabilities.- <h2 class="kg-signup-card-heading" ${nodeData.textColor ? `style="color: ${nodeData.textColor};"` : ''}>${nodeData.header}</h2> - <p class="kg-signup-card-subheading" ${nodeData.textColor ? `style="color: ${nodeData.textColor};"` : ''}>${nodeData.subheader}</p> + <h2 class="kg-signup-card-heading" ${nodeData.textColor ? `style="color: ${escapeHtml(nodeData.textColor)};"` : ''}>${escapeHtml(nodeData.header)}</h2> + <p class="kg-signup-card-subheading" ${nodeData.textColor ? `style="color: ${escapeHtml(nodeData.textColor)};"` : ''}>${escapeHtml(nodeData.subheader)}</p> - <p class="kg-signup-card-disclaimer" ${nodeData.textColor ? `style="color: ${nodeData.textColor};"` : ''}>${nodeData.disclaimer}</p> + <p class="kg-signup-card-disclaimer" ${nodeData.textColor ? `style="color: ${escapeHtml(nodeData.textColor)};"` : ''}>${escapeHtml(nodeData.disclaimer)}</p>Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In ghost/core/core/server/services/koenig/node-renderers/signup-renderer.js around lines 41 to 44, user-controlled content (header, subheader, disclaimer) is directly interpolated into HTML, causing XSS vulnerabilities. To fix this, sanitize or escape these values before inserting them into the HTML template to prevent malicious scripts from executing. Use a proper HTML escaping utility or library to encode special characters in these strings.
23-23:
⚠️ Potential issueXSS vulnerability in style attribute.
Direct interpolation of
buttonTextColor
into the style attribute without sanitization creates an XSS vulnerability. An attacker could inject malicious CSS or break out of the style context.- <button class="kg-signup-card-button ${buttonAccent}" style="${buttonStyle}color: ${nodeData.buttonTextColor};" type="submit"> + <button class="kg-signup-card-button ${buttonAccent}" style="${buttonStyle}color: ${escapeHtml(nodeData.buttonTextColor)};" type="submit">📝 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.<button class="kg-signup-card-button ${buttonAccent}" style="${buttonStyle}color: ${escapeHtml(nodeData.buttonTextColor)};" type="submit">
🤖 Prompt for AI Agents
In ghost/core/core/server/services/koenig/node-renderers/signup-renderer.js at line 23, the direct interpolation of buttonTextColor into the style attribute creates an XSS vulnerability. To fix this, sanitize or validate the buttonTextColor value to ensure it contains only safe CSS color values before including it in the style attribute, preventing injection of malicious code.
ghost/core/core/server/services/koenig/node-renderers/audio-renderer.js (4)
26-26:
⚠️ Potential issueValidate and sanitize thumbnail URL to prevent XSS.
Direct assignment of
node.thumbnailSrc
toimg.src
without validation could allow XSS throughjavascript:
URLs or other malicious schemes.+const sanitizeUrl = (url) => { + if (!url || typeof url !== 'string') return ''; + const trimmed = url.trim(); + if (trimmed.startsWith('javascript:') || trimmed.startsWith('data:text/html')) { + return ''; + } + return trimmed; +}; - img.src = node.thumbnailSrc; + img.src = sanitizeUrl(node.thumbnailSrc);📝 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.// Add this helper (e.g. near the top of the file) const sanitizeUrl = (url) => { if (!url || typeof url !== 'string') return ''; const trimmed = url.trim(); if (trimmed.startsWith('javascript:') || trimmed.startsWith('data:text/html')) { return ''; } return trimmed; }; // …later, replace the insecure assignment: - img.src = node.thumbnailSrc; + img.src = sanitizeUrl(node.thumbnailSrc);
🤖 Prompt for AI Agents
In ghost/core/core/server/services/koenig/node-renderers/audio-renderer.js at line 26, the assignment of node.thumbnailSrc directly to img.src can lead to XSS vulnerabilities if the URL contains malicious schemes like javascript:. To fix this, validate and sanitize node.thumbnailSrc before assigning it to img.src by ensuring it is a safe URL, such as checking it starts with http:// or https://, or using a URL sanitizer utility to strip or reject unsafe URLs.
242-248: 🛠️ Refactor suggestion
Improve duration formatting with error handling.
The
getFormattedDuration
function should handle invalid duration values more robustly and the default value of 200 seems arbitrary.-function getFormattedDuration(duration = 200) { +function getFormattedDuration(duration = 0) { + if (typeof duration !== 'number' || duration < 0 || !isFinite(duration)) { + return '0:00'; + } const minutes = Math.floor(duration / 60); const seconds = Math.floor(duration - (minutes * 60)); const paddedSeconds = String(seconds).padStart(2, '0'); const formattedDuration = `${minutes}:${paddedSeconds}`; return formattedDuration; }📝 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.function getFormattedDuration(duration = 0) { if (typeof duration !== 'number' || duration < 0 || !isFinite(duration)) { return '0:00'; } const minutes = Math.floor(duration / 60); const seconds = Math.floor(duration - (minutes * 60)); const paddedSeconds = String(seconds).padStart(2, '0'); const formattedDuration = `${minutes}:${paddedSeconds}`; return formattedDuration; }
🤖 Prompt for AI Agents
In ghost/core/core/server/services/koenig/node-renderers/audio-renderer.js around lines 242 to 248, improve the getFormattedDuration function by adding validation to handle invalid duration inputs such as negative numbers or non-numeric values. Remove the arbitrary default value of 200 and instead require a valid duration parameter or return a sensible default like "0:00" when the input is invalid. This will make the function more robust and prevent incorrect formatting.
114-114:
⚠️ Potential issueFix typo in variable name.
There's a typo in the variable name
audioDUrationNode
- it should beaudioDurationNode
.- const audioDUrationNode = document.createElement('span'); - audioDUrationNode.setAttribute('class', 'kg-audio-duration'); - audioDUrationNode.textContent = node.duration; - audioDurationTotal.appendChild(audioDUrationNode); + const audioDurationNode = document.createElement('span'); + audioDurationNode.setAttribute('class', 'kg-audio-duration'); + audioDurationNode.textContent = node.duration; + audioDurationTotal.appendChild(audioDurationNode);📝 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 audioDurationNode = document.createElement('span'); audioDurationNode.setAttribute('class', 'kg-audio-duration'); audioDurationNode.textContent = node.duration; audioDurationTotal.appendChild(audioDurationNode);
🤖 Prompt for AI Agents
In ghost/core/core/server/services/koenig/node-renderers/audio-renderer.js at line 114, fix the typo in the variable name from `audioDUrationNode` to `audioDurationNode` by correcting the capitalization to ensure consistent and correct naming.
177-190:
⚠️ Potential issueSanitize URLs and content in email template to prevent XSS.
Direct interpolation of
postUrl
,thumbnailSrc
, andtitle
into HTML without sanitization creates XSS vulnerabilities in the email template.+const escapeHtml = (str) => str.replace(/[&<>"']/g, (match) => { + const escapeMap = { '&': '&', '<': '<', '>': '>', '"': '"', "'": ''' }; + return escapeMap[match]; +}); - <a href="${options.postUrl}" style="display: block; width: 60px; height: 60px; padding-top: 4px; padding-right: 16px; padding-bottom: 4px; padding-left: 4px; border-radius: 2px;"> + <a href="${escapeHtml(options.postUrl)}" style="display: block; width: 60px; height: 60px; padding-top: 4px; padding-right: 16px; padding-bottom: 4px; padding-left: 4px; border-radius: 2px;"> - <img src="${node.thumbnailSrc}" class="${thumbnailCls}" style="width: 60px; height: 60px; object-fit: cover; border: 0; border-radius: 2px;"> + <img src="${escapeHtml(node.thumbnailSrc)}" class="${thumbnailCls}" style="width: 60px; height: 60px; object-fit: cover; border: 0; border-radius: 2px;"> - <a href="${options.postUrl}" class="kg-audio-title">${node.title}</a> + <a href="${escapeHtml(options.postUrl)}" class="kg-audio-title">${escapeHtml(node.title)}</a>Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In ghost/core/core/server/services/koenig/node-renderers/audio-renderer.js around lines 177 to 190, the direct interpolation of user-controlled values like postUrl, thumbnailSrc, and title into the email template HTML can lead to XSS vulnerabilities. To fix this, sanitize these values before inserting them into the HTML by escaping special characters or using a trusted sanitization library to ensure no malicious scripts can be injected. Replace the direct template string insertions with sanitized versions of these variables.
ghost/core/core/server/services/koenig/node-renderers/header-v2-renderer.js (2)
44-49: 🛠️ Refactor suggestion
Enhance URL validation for security.
While the code checks for empty URLs, it should also validate the URL format to prevent potential XSS attacks through javascript: URLs or other malicious schemes.
Add URL validation:
const button = () => { - if (nodeData.buttonEnabled && nodeData.buttonUrl && nodeData.buttonUrl.trim() !== '') { + if (nodeData.buttonEnabled && nodeData.buttonUrl && nodeData.buttonUrl.trim() !== '' && isValidUrl(nodeData.buttonUrl)) { return `<a href="${nodeData.buttonUrl}" class="kg-header-card-button ${buttonAccent}" style="${buttonStyle}color: ${nodeData.buttonTextColor};" data-button-color="${nodeData.buttonColor}" data-button-text-color="${nodeData.buttonTextColor}">${nodeData.buttonText}</a>`; } return ''; };Consider adding a URL validation utility function:
function isValidUrl(url) { try { const parsed = new URL(url); return ['http:', 'https:', 'mailto:'].includes(parsed.protocol); } catch { return false; } }🤖 Prompt for AI Agents
In ghost/core/core/server/services/koenig/node-renderers/header-v2-renderer.js around lines 44 to 49, the button URL is only checked for emptiness but not validated for safe URL schemes, which can lead to XSS vulnerabilities. Implement a URL validation function that parses the URL and ensures its protocol is one of 'http:', 'https:', or 'mailto:'. Then update the button rendering logic to use this validation function and only render the button if the URL passes this check.
30-42:
⚠️ Potential issuePotential duplicate IDs and missing URL validation.
The use of
slugify
for generating IDs from user content could lead to duplicate IDs if headers/subheaders have the same text. Additionally, thebuttonUrl
is inserted directly into the href attribute without validation.Consider these improvements:
- Add a unique identifier to prevent duplicate IDs
- Validate and sanitize the button URL
const header = () => { if (nodeData.header) { - return `<h2 id="${slugify(nodeData.header)}" class="kg-header-card-heading" style="color: ${nodeData.textColor};" data-text-color="${nodeData.textColor}">${nodeData.header}</h2>`; + // Consider adding a unique identifier or omitting the ID if not essential + return `<h2 class="kg-header-card-heading" style="color: ${nodeData.textColor};" data-text-color="${nodeData.textColor}">${nodeData.header}</h2>`; } return ''; }; const subheader = () => { if (nodeData.subheader) { - return `<p id="${slugify(nodeData.subheader)}" class="kg-header-card-subheading" style="color: ${nodeData.textColor};" data-text-color="${nodeData.textColor}">${nodeData.subheader}</p>`; + return `<p class="kg-header-card-subheading" style="color: ${nodeData.textColor};" data-text-color="${nodeData.textColor}">${nodeData.subheader}</p>`; } return ''; };📝 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 header = () => { if (nodeData.header) { // Consider adding a unique identifier or omitting the ID if not essential return `<h2 class="kg-header-card-heading" style="color: ${nodeData.textColor};" data-text-color="${nodeData.textColor}">${nodeData.header}</h2>`; } return ''; }; const subheader = () => { if (nodeData.subheader) { return `<p class="kg-header-card-subheading" style="color: ${nodeData.textColor};" data-text-color="${nodeData.textColor}">${nodeData.subheader}</p>`; } return ''; };
🤖 Prompt for AI Agents
In ghost/core/core/server/services/koenig/node-renderers/header-v2-renderer.js around lines 30 to 42, the IDs generated by slugify for headers and subheaders may cause duplicates if the text is the same, and the buttonUrl is used without validation. To fix this, append a unique identifier (like a counter or a UUID) to the slugified IDs to ensure uniqueness, and validate and sanitize the buttonUrl before inserting it into the href attribute to prevent unsafe URLs.
ghost/core/core/server/services/koenig/node-renderers/call-to-action-renderer.js (3)
61-336: 🛠️ Refactor suggestion
Critical: Refactor to eliminate massive code duplication.
The
emailCTATemplate
function is 275 lines long with significant duplication between feature flag branches. This violates DRY principles and makes maintenance error-prone.Extract common template generation logic:
function emailCTATemplate(dataset, options = {}) { const styles = calculateButtonStyles(dataset, options); const imageDimensions = calculateImageDimensions(dataset); const useCustomization = options?.feature?.emailCustomization || options?.feature?.emailCustomizationAlpha; const contentRenderer = dataset.layout === 'minimal' ? renderMinimalContent : renderDefaultContent; return ` <table class="${getTableClasses(dataset)}" border="0" cellpadding="0" cellspacing="0" width="100%"> ${renderSponsorLabel(dataset)} ${contentRenderer(dataset, styles, imageDimensions, useCustomization)} </table> `; } function calculateButtonStyles(dataset, options) { // Extract button style calculation logic } function renderMinimalContent(dataset, styles, imageDimensions, useCustomization) { // Extract minimal layout rendering } function renderDefaultContent(dataset, styles, imageDimensions, useCustomization) { // Extract default layout rendering }🤖 Prompt for AI Agents
In ghost/core/core/server/services/koenig/node-renderers/call-to-action-renderer.js around lines 61 to 336, the emailCTATemplate function contains large duplicated blocks for feature flag branches and layout types, violating DRY principles. Refactor by extracting common logic into helper functions: create a calculateButtonStyles function to handle button style computation, a calculateImageDimensions function for image sizing, and separate renderMinimalContent and renderDefaultContent functions for the two layout types. Then, unify the main function to call these helpers and assemble the final template, reducing duplication and improving maintainability.
9-15:
⚠️ Potential issueAdd URL validation in wrapWithLink function.
The function directly inserts user-provided URLs into href attributes without validation, which could lead to XSS vulnerabilities.
Add URL validation:
const wrapWithLink = (dataset, content) => { if (!showButton(dataset)) { return content; } + + // Validate URL to prevent XSS + try { + const url = new URL(dataset.buttonUrl); + if (!['http:', 'https:', 'mailto:'].includes(url.protocol)) { + return content; + } + } catch { + return content; + } return `<a href="${dataset.buttonUrl}">${content}</a>`; };📝 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 wrapWithLink = (dataset, content) => { if (!showButton(dataset)) { return content; } // Validate URL to prevent XSS try { const url = new URL(dataset.buttonUrl); if (!['http:', 'https:', 'mailto:'].includes(url.protocol)) { return content; } } catch { return content; } return `<a href="${dataset.buttonUrl}">${content}</a>`; };
🤖 Prompt for AI Agents
In ghost/core/core/server/services/koenig/node-renderers/call-to-action-renderer.js between lines 9 and 15, the wrapWithLink function inserts dataset.buttonUrl directly into the href attribute without validation, risking XSS vulnerabilities. To fix this, add URL validation before using dataset.buttonUrl by checking if it is a valid and safe URL (e.g., using a URL parsing method or a whitelist of allowed protocols). Only return the anchor tag with the href if the URL passes validation; otherwise, return the content without the link.
102-107: 🛠️ Refactor suggestion
Add safety checks for image URL parsing.
The regex assumes a specific URL structure and doesn't handle potential match failures, which could cause runtime errors.
Add null checks and error handling:
if (isLocalContentImage(dataset.imageUrl, options.siteUrl) && options.canTransformImage?.(dataset.imageUrl)) { - const [, imagesPath, filename] = dataset.imageUrl.match(/(.*\/content\/images)\/(.*)/); + const match = dataset.imageUrl.match(/(.*\/content\/images)\/(.*)/); + if (!match) { + console.warn('Unable to parse image URL for optimization:', dataset.imageUrl); + return; + } + const [, imagesPath, filename] = match; const iconSize = options?.imageOptimization?.internalImageSizes?.['email-cta-minimal-image'] || {width: 256, height: 256}; dataset.imageUrl = `${imagesPath}/size/w${iconSize.width}h${iconSize.height}/${filename}`; }📝 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.if (isLocalContentImage(dataset.imageUrl, options.siteUrl) && options.canTransformImage?.(dataset.imageUrl)) { const match = dataset.imageUrl.match(/(.*\/content\/images)\/(.*)/); if (!match) { console.warn('Unable to parse image URL for optimization:', dataset.imageUrl); return; } const [, imagesPath, filename] = match; const iconSize = options?.imageOptimization?.internalImageSizes?.['email-cta-minimal-image'] || {width: 256, height: 256}; // default to 256 since we know the image is a square dataset.imageUrl = `${imagesPath}/size/w${iconSize.width}h${iconSize.height}/${filename}`; }
🤖 Prompt for AI Agents
In ghost/core/core/server/services/koenig/node-renderers/call-to-action-renderer.js around lines 102 to 107, the code destructures the result of a regex match on dataset.imageUrl without checking if the match succeeded, which can cause runtime errors if the URL doesn't match the expected pattern. To fix this, add a null check after the match call to ensure it returned a valid array before destructuring. If the match is null, handle the case gracefully, such as by skipping the URL transformation or logging a warning, to prevent errors.
ref https://linear.app/ghost/issue/PROD-1784 - added basic output regression tests for all node renderers - added test utilities to facilitate testing node renderers without a full Lexical environment - added e2e coverage exclude for koenig service folder as everything is integration/unit tested and covering all branches via e2e tests too doesn't add any value
fb2d2e7
to
003fed4
Compare
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: 4
🧹 Nitpick comments (2)
ghost/core/test/unit/server/services/koenig/node-renderers/button-renderer.test.js (1)
71-72
: Address the TODO comment about the email renderer bug.The comment indicates a bug where the email renderer doesn't show default button text when
buttonText
is missing. This should be tracked and fixed.Would you like me to create an issue to track this bug? The expected behavior should match the web renderer's fallback to "Button Title" when
buttonText
is empty.ghost/core/test/unit/server/services/koenig/node-renderers/product-renderer.test.js (1)
105-105
: Consider testing dynamic star rating valuesThe tests use a hardcoded star rating image URL with "undefined" in the filename (
star-rating-undefined.png
), which suggests the rating value might not be properly set in the test data. Consider adding tests with specific rating values to ensure the renderer handles different ratings correctly.Consider adding test cases with different rating values:
it('renders with specific star rating', function () { const data = getTestData({productRating: 4}); const result = renderForEmail(data); // Assert the correct star rating image is used });Also applies to: 165-165, 236-236
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
📒 Files selected for processing (29)
ghost/core/.c8rc.e2e.json
(1 hunks)ghost/core/package.json
(3 hunks)ghost/core/test/unit/server/services/koenig/node-renderers/audio-renderer.test.js
(1 hunks)ghost/core/test/unit/server/services/koenig/node-renderers/bookmark-renderer.test.js
(1 hunks)ghost/core/test/unit/server/services/koenig/node-renderers/button-renderer.test.js
(1 hunks)ghost/core/test/unit/server/services/koenig/node-renderers/call-to-action-renderer.test.js
(1 hunks)ghost/core/test/unit/server/services/koenig/node-renderers/callout-renderer.test.js
(1 hunks)ghost/core/test/unit/server/services/koenig/node-renderers/codeblock-renderer.test.js
(1 hunks)ghost/core/test/unit/server/services/koenig/node-renderers/email-cta-renderer.test.js
(1 hunks)ghost/core/test/unit/server/services/koenig/node-renderers/email-renderer.test.js
(1 hunks)ghost/core/test/unit/server/services/koenig/node-renderers/embed-renderer.test.js
(1 hunks)ghost/core/test/unit/server/services/koenig/node-renderers/file-renderer.test.js
(1 hunks)ghost/core/test/unit/server/services/koenig/node-renderers/gallery-renderer.test.js
(1 hunks)ghost/core/test/unit/server/services/koenig/node-renderers/header-v1-renderer.test.js
(1 hunks)ghost/core/test/unit/server/services/koenig/node-renderers/header-v2-renderer.test.js
(1 hunks)ghost/core/test/unit/server/services/koenig/node-renderers/horizontalrule-renderer.test.js
(1 hunks)ghost/core/test/unit/server/services/koenig/node-renderers/html-renderer.test.js
(1 hunks)ghost/core/test/unit/server/services/koenig/node-renderers/image-renderer.test.js
(1 hunks)ghost/core/test/unit/server/services/koenig/node-renderers/markdown-renderer.test.js
(1 hunks)ghost/core/test/unit/server/services/koenig/node-renderers/paywall-renderer.test.js
(1 hunks)ghost/core/test/unit/server/services/koenig/node-renderers/product-renderer.test.js
(1 hunks)ghost/core/test/unit/server/services/koenig/node-renderers/signup-renderer.test.js
(1 hunks)ghost/core/test/unit/server/services/koenig/node-renderers/toggle-renderer.test.js
(1 hunks)ghost/core/test/unit/server/services/koenig/node-renderers/video-renderer.test.js
(1 hunks)ghost/core/test/unit/server/services/koenig/test-utils/assert-prettifies-to.js
(1 hunks)ghost/core/test/unit/server/services/koenig/test-utils/build-call-renderer.js
(1 hunks)ghost/core/test/unit/server/services/koenig/test-utils/html.js
(1 hunks)ghost/core/test/unit/server/services/koenig/test-utils/index.js
(1 hunks)ghost/core/test/unit/server/services/koenig/test-utils/visibility.js
(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- ghost/core/test/unit/server/services/koenig/node-renderers/horizontalrule-renderer.test.js
- ghost/core/.c8rc.e2e.json
🚧 Files skipped from review as they are similar to previous changes (24)
- ghost/core/test/unit/server/services/koenig/test-utils/html.js
- ghost/core/test/unit/server/services/koenig/test-utils/assert-prettifies-to.js
- ghost/core/test/unit/server/services/koenig/node-renderers/paywall-renderer.test.js
- ghost/core/package.json
- ghost/core/test/unit/server/services/koenig/node-renderers/markdown-renderer.test.js
- ghost/core/test/unit/server/services/koenig/node-renderers/html-renderer.test.js
- ghost/core/test/unit/server/services/koenig/node-renderers/video-renderer.test.js
- ghost/core/test/unit/server/services/koenig/node-renderers/header-v2-renderer.test.js
- ghost/core/test/unit/server/services/koenig/test-utils/visibility.js
- ghost/core/test/unit/server/services/koenig/node-renderers/file-renderer.test.js
- ghost/core/test/unit/server/services/koenig/node-renderers/email-renderer.test.js
- ghost/core/test/unit/server/services/koenig/test-utils/index.js
- ghost/core/test/unit/server/services/koenig/node-renderers/email-cta-renderer.test.js
- ghost/core/test/unit/server/services/koenig/node-renderers/codeblock-renderer.test.js
- ghost/core/test/unit/server/services/koenig/node-renderers/header-v1-renderer.test.js
- ghost/core/test/unit/server/services/koenig/node-renderers/callout-renderer.test.js
- ghost/core/test/unit/server/services/koenig/test-utils/build-call-renderer.js
- ghost/core/test/unit/server/services/koenig/node-renderers/gallery-renderer.test.js
- ghost/core/test/unit/server/services/koenig/node-renderers/toggle-renderer.test.js
- ghost/core/test/unit/server/services/koenig/node-renderers/call-to-action-renderer.test.js
- ghost/core/test/unit/server/services/koenig/node-renderers/image-renderer.test.js
- ghost/core/test/unit/server/services/koenig/node-renderers/audio-renderer.test.js
- ghost/core/test/unit/server/services/koenig/node-renderers/embed-renderer.test.js
- ghost/core/test/unit/server/services/koenig/node-renderers/signup-renderer.test.js
🧰 Additional context used
🧬 Code Graph Analysis (1)
ghost/core/test/unit/server/services/koenig/node-renderers/product-renderer.test.js (14)
ghost/core/test/unit/server/services/koenig/node-renderers/audio-renderer.test.js (2)
assert
(1-1)require
(2-2)ghost/core/test/unit/server/services/koenig/node-renderers/call-to-action-renderer.test.js (2)
assert
(1-1)require
(2-2)ghost/core/test/unit/server/services/koenig/node-renderers/button-renderer.test.js (2)
assert
(1-1)require
(2-2)ghost/core/test/unit/server/services/koenig/node-renderers/callout-renderer.test.js (2)
assert
(1-1)require
(2-2)ghost/core/test/unit/server/services/koenig/node-renderers/email-cta-renderer.test.js (2)
assert
(1-1)require
(2-2)ghost/core/test/unit/server/services/koenig/node-renderers/embed-renderer.test.js (2)
assert
(1-1)require
(2-2)ghost/core/test/unit/server/services/koenig/node-renderers/email-renderer.test.js (2)
assert
(1-1)require
(2-2)ghost/core/test/unit/server/services/koenig/node-renderers/file-renderer.test.js (2)
assert
(1-1)require
(2-2)ghost/core/test/unit/server/services/koenig/node-renderers/horizontalrule-renderer.test.js (2)
assert
(1-1)require
(2-2)ghost/core/test/unit/server/services/koenig/node-renderers/header-v1-renderer.test.js (2)
assert
(1-1)require
(2-2)ghost/core/test/unit/server/services/koenig/node-renderers/header-v2-renderer.test.js (2)
assert
(1-1)require
(2-2)ghost/core/test/unit/server/services/koenig/node-renderers/gallery-renderer.test.js (2)
assert
(1-1)require
(2-2)ghost/core/test/unit/server/services/koenig/node-renderers/image-renderer.test.js (2)
assert
(1-1)require
(2-2)ghost/core/test/unit/server/services/koenig/node-renderers/codeblock-renderer.test.js (1)
require
(1-1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Setup
🔇 Additional comments (10)
ghost/core/test/unit/server/services/koenig/node-renderers/button-renderer.test.js (2)
1-12
: Well-structured test setup.The helper functions for generating test data and calling renderers provide a clean and maintainable test structure.
22-48
: Comprehensive web rendering test coverage.The web rendering tests properly cover the main scenarios including default rendering, missing button text (with fallback), and missing button URL (no rendering).
ghost/core/test/unit/server/services/koenig/node-renderers/product-renderer.test.js (3)
5-18
: LGTM - Good test data helper functionThe test data helper function is well-structured and follows good practices by allowing overrides for flexible test scenarios.
20-26
: LGTM - Clean helper functions for different render targetsThe render helper functions provide good abstraction for testing different targets (web vs email).
135-204
:❌ Incorrect review comment
Investigate duplicate test scenarios for email customization features
The
emailCustomization
andemailCustomizationAlpha
test scenarios produce identical HTML output, which raises questions about whether:
- These are truly different features that should produce different outputs
- One feature is deprecated/redundant
- The test coverage is insufficient to demonstrate the differences
This duplication could indicate a bug in the implementation or tests.
Please verify if these feature flags should produce different outputs by running:
Also applies to: 206-275
🏁 Script executed:
#!/bin/bash # Check if emailCustomization and emailCustomizationAlpha are used differently in the codebase echo "Searching for emailCustomization usage patterns..." rg -A 3 -B 3 "emailCustomization" echo -e "\n\nSearching for emailCustomizationAlpha usage patterns..." rg -A 3 -B 3 "emailCustomizationAlpha"Length of output: 66028
Duplicate output for both
emailCustomization
andemailCustomizationAlpha
is intentionalThe codebase deliberately treats these two flags as equivalent (all renderers and the email service use
if (…emailCustomization || emailCustomizationAlpha)
), so the identical snapshots under each flag are expected. No changes required.Likely an incorrect or invalid review comment.
ghost/core/test/unit/server/services/koenig/node-renderers/bookmark-renderer.test.js (5)
1-3
: LGTM! Good use of strict assertions and test utilities.The imports are appropriate and using
assert/strict
is a best practice for more robust test assertions.
7-27
: Well-designed helper functions with good separation of concerns.The
getTestData
function with override capability and the separaterenderForWeb
/renderForEmail
functions provide clean abstractions for the tests.
29-82
: Comprehensive web rendering tests with good security coverage.The tests effectively cover normal rendering, edge cases, and XSS protection. The verification that caption HTML remains unescaped while other fields are properly escaped demonstrates good security awareness.
4-187
: Excellent test organization and comprehensive coverage.The test suite demonstrates best practices with:
- Clear separation between web and email rendering tests
- Comprehensive coverage including normal cases, edge cases, and security scenarios
- Consistent testing patterns and good use of snapshot testing
- Proper XSS protection verification while allowing rich content in captions
The test structure is maintainable and provides confidence in the bookmark renderer's functionality.
165-185
:✅ Verification successful
Verify the different quote escaping behavior between web and email.
The email test expects double-escaped quotes (
&quot;
) while the web test expects single-escaped quotes ("kewl"
). This difference in escaping behavior should be verified to ensure it's intentional.Line 179:
doing &quot;kewl&quot; stuff
(email)
vs Web line 75:doing "kewl" stuff
(web)Check if this escaping difference is intentional by examining the bookmark renderer implementation:
🏁 Script executed:
#!/bin/bash # Description: Check the bookmark renderer implementation for escaping logic # Expected: Understand why email and web have different quote escaping echo "Examining bookmark renderer implementation..." fd bookmark-renderer.js -t f --exec cat {} \; echo -e "\nSearching for HTML escaping utilities..." rg -A 5 -B 5 "escape|htmlEscape" ghost/core/core/server/services/koenig/Length of output: 23883
🏁 Script executed:
#!/bin/bash # Find the renderForEmail utility used in tests echo "Searching for renderForEmail in the repository..." rg -n "renderForEmail" -n .Length of output: 11060
Email double-escaping of quotes is intentional
Verified that in the email renderer we call escapeHtml on the raw text and then again inside truncateHtml, which converts
"
→"
and then"
→&quot;
. The frontend renderer uses textContent, so quotes remain literal. No change needed.
describe('email (emailCustomization)', function () { | ||
it('matches snapshot for default test data', function () { | ||
const result = renderForEmail(getTestData(), {feature: {emailCustomization: true}}); | ||
|
||
assert.ok(result.html); | ||
|
||
// TODO: The HTML output is invalid so doesn't work with prettier. Replace with alpha version | ||
assert.equal(result.html, '<p><table border="0" cellpadding="0" cellspacing="0"><tbody><tr><td><table class="btn btn-accent" border="0" cellspacing="0" cellpadding="0" align="center"><tbody><tr><td align="center"><a href="http://blog.com/post1">click me</a></td></tr></tbody></table></td></tr></tbody></table></p>'); | ||
}); | ||
}); | ||
|
||
describe('email (emailCustomizationAlpha)', function () { | ||
it('matches snapshot for default test data', function () { | ||
const result = renderForEmail(getTestData(), {feature: {emailCustomizationAlpha: true}}); | ||
|
||
assert.ok(result.html); | ||
|
||
assertPrettifiesTo(result.html, html` | ||
<table border="0" cellpadding="0" cellspacing="0"> | ||
<tbody> | ||
<tr> | ||
<td> | ||
<table class="btn btn-accent" border="0" cellspacing="0" cellpadding="0" align="center"> | ||
<tbody> | ||
<tr> | ||
<td align="center"> | ||
<a href="http://blog.com/post1">click me</a> | ||
</td> | ||
</tr> | ||
</tbody> | ||
</table> | ||
</td> | ||
</tr> | ||
</tbody> | ||
</table> | ||
`); | ||
}); | ||
}); |
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.
💡 Verification agent
🧩 Analysis chain
Verify the differences between emailCustomization feature flags.
The emailCustomization
and emailCustomizationAlpha
flags produce different HTML structures. Ensure this is intentional and document the differences.
Run this script to understand the renderer implementations for these feature flags:
🏁 Script executed:
#!/bin/bash
# Description: Examine the button renderer implementation to understand feature flag differences
# Search for emailCustomization feature flag usage in button renderer
rg -A 10 -B 5 "emailCustomization" ghost/core/core/server/services/koenig/node-renderers/
# Look for the button renderer implementation
cat ghost/core/core/server/services/koenig/node-renderers/button-renderer.js
Length of output: 19441
Document intentional differences between emailCustomization and emailCustomizationAlpha
Both branches under emailTemplate
deliberately produce different wrapper elements and markup. Please add a brief comment in ghost/core/core/server/services/koenig/node-renderers/button-renderer.js
above the if (options.feature?.emailCustomization)…else if (options.feature?.emailCustomizationAlpha)
block explaining:
emailCustomization
• wraps the generated<table>
in a<p>
element
• uses hand-rolled table markup (no explicit<tbody>
in source) for legacy snapshot testsemailCustomizationAlpha
• wraps its<table>
in a<div>
and returns{type: 'inner'}
• delegates to therenderEmailButton
partial (which emits its own<tbody>
)
This will clarify why the two feature flags yield different HTML shapes.
🤖 Prompt for AI Agents
In ghost/core/core/server/services/koenig/node-renderers/button-renderer.js
around the block handling options.feature.emailCustomization and
options.feature.emailCustomizationAlpha, add a comment explaining the
intentional differences: emailCustomization wraps the generated table in a p
element and uses hand-rolled table markup without explicit tbody for legacy
snapshot tests, while emailCustomizationAlpha wraps its table in a div, returns
{type: 'inner'}, and delegates to renderEmailButton partial which emits its own
tbody. This clarifies why the two feature flags produce different HTML
structures.
// TODO: The HTML output is invalid so doesn't work with prettier. Replace with alpha version | ||
assert.equal(result.html, '<p><table border="0" cellpadding="0" cellspacing="0"><tbody><tr><td><table class="btn btn-accent" border="0" cellspacing="0" cellpadding="0" align="center"><tbody><tr><td align="center"><a href="http://blog.com/post1">click me</a></td></tr></tbody></table></td></tr></tbody></table></p>'); | ||
}); |
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.
🛠️ Refactor suggestion
Replace brittle string comparison with prettified assertion.
The direct string comparison is fragile and harder to maintain. The TODO comment mentions replacing this with the alpha version approach.
Consider using the same assertPrettifiesTo
approach as the alpha version, or if the HTML is indeed invalid, fix the renderer to generate valid HTML first:
-// TODO: The HTML output is invalid so doesn't work with prettier. Replace with alpha version
-assert.equal(result.html, '<p><table border="0" cellpadding="0" cellspacing="0"><tbody><tr><td><table class="btn btn-accent" border="0" cellspacing="0" cellpadding="0" align="center"><tbody><tr><td align="center"><a href="http://blog.com/post1">click me</a></td></tr></tbody></table></td></tr></tbody></table></p>');
+assertPrettifiesTo(result.html, html`
+ <p>
+ <table border="0" cellpadding="0" cellspacing="0">
+ <tbody>
+ <tr>
+ <td>
+ <table class="btn btn-accent" border="0" cellspacing="0" cellpadding="0" align="center">
+ <tbody>
+ <tr>
+ <td align="center">
+ <a href="http://blog.com/post1">click me</a>
+ </td>
+ </tr>
+ </tbody>
+ </table>
+ </td>
+ </tr>
+ </tbody>
+ </table>
+ </p>
+`);
📝 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.
// TODO: The HTML output is invalid so doesn't work with prettier. Replace with alpha version | |
assert.equal(result.html, '<p><table border="0" cellpadding="0" cellspacing="0"><tbody><tr><td><table class="btn btn-accent" border="0" cellspacing="0" cellpadding="0" align="center"><tbody><tr><td align="center"><a href="http://blog.com/post1">click me</a></td></tr></tbody></table></td></tr></tbody></table></p>'); | |
}); | |
assertPrettifiesTo(result.html, html` | |
<p> | |
<table border="0" cellpadding="0" cellspacing="0"> | |
<tbody> | |
<tr> | |
<td> | |
<table class="btn btn-accent" border="0" cellspacing="0" cellpadding="0" align="center"> | |
<tbody> | |
<tr> | |
<td align="center"> | |
<a href="http://blog.com/post1">click me</a> | |
</td> | |
</tr> | |
</tbody> | |
</table> | |
</td> | |
</tr> | |
</tbody> | |
</table> | |
</p> | |
`); | |
}); |
🤖 Prompt for AI Agents
In
ghost/core/test/unit/server/services/koenig/node-renderers/button-renderer.test.js
around lines 101 to 103, the test uses a brittle direct string comparison for
HTML output which is fragile and hard to maintain. Replace the assert.equal call
with an assertion that uses a prettified HTML comparison method like
assertPrettifiesTo, matching the approach used in the alpha version tests. If
the HTML output is invalid, first fix the renderer to produce valid HTML before
updating the test to use the prettified assertion.
</tr> | ||
<tr> | ||
<td valign="top"> | ||
<h4 class="kg-product-title">This is a<b>title</b></h4> |
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.
Fix missing spaces in HTML content
The expected HTML output has missing spaces between text content and HTML tags, which could indicate an issue with the renderer or test data.
Lines 159, 171, 230, and 242 show:
This is a<b>title</b>
should beThis is a <b>title</b>
This is a<b>description</b>
should beThis is a <b>description</b>
- <h4 class="kg-product-title">This is a<b>title</b></h4>
+ <h4 class="kg-product-title">This is a <b>title</b></h4>
- This is a<b>description</b>
+ This is a <b>description</b>
Also applies to: 171-171, 230-230, 242-242
🤖 Prompt for AI Agents
In
ghost/core/test/unit/server/services/koenig/node-renderers/product-renderer.test.js
at lines 159, 171, 230, and 242, the expected HTML strings are missing spaces
between text and inline tags like <b>. Update these lines to include a space
before the <b> tag, changing instances like "This is a<b>title</b>" to "This is
a <b>title</b>" to correctly reflect the intended spacing in the rendered
output.
assertPrettifiesTo(result.html, html` | ||
<div> | ||
<!--[if !mso !vml]--> | ||
<figure class="kg-card kg-bookmark-card kg-card-hascaption"> | ||
<a class="kg-bookmark-container" href="https://www.ghost.org/"> | ||
<div class="kg-bookmark-content"> | ||
<div class="kg-bookmark-title">Ghost: The Creator Economy Platform</div> | ||
<div class="kg-bookmark-description">doing kewl stuff</div> | ||
<div class="kg-bookmark-metadata"> | ||
<img class="kg-bookmark-icon" src="https://www.ghost.org/favicon.ico" alt=""> | ||
<span class="kg-bookmark-author" src="Ghost - The Professional Publishing Platform">Ghost - The Professional Publishing Platform</span> | ||
<span class="kg-bookmark-publisher" src="ghost">ghost</span> | ||
</div> | ||
</div> | ||
<div class="kg-bookmark-thumbnail" style="background-image: url('https://ghost.org/images/meta/ghost.png')"> | ||
<img src="https://ghost.org/images/meta/ghost.png" alt="" onerror="this.style.display='none'"> | ||
</div> | ||
</a> | ||
<figcaption>caption here</figcaption> | ||
</figure> | ||
<!--[endif]--><!--[if vml]> | ||
<table class="kg-card kg-bookmark-card--outlook" style="margin: 0; padding: 0; width: 100%; border: 1px solid #e5eff5; background: #ffffff; font-family: -apple-system, BlinkMacSystemFont, 'Segoe UI', Roboto, Oxygen, Ubuntu, Cantarell, 'Open Sans', 'Helvetica Neue', sans-serif; border-collapse: collapse; border-spacing: 0;" width="100%"> | ||
<tr> | ||
<td width="100%" style="padding: 20px"> | ||
<table style="margin: 0; padding: 0; border-collapse: collapse; border-spacing: 0;"> | ||
<tr> | ||
<td class="kg-bookmark-title--outlook"> | ||
<a href="https://www.ghost.org/" style="text-decoration: none; color: #15212a; font-size: 15px; line-height: 1.5em; font-weight: 600;"> | ||
Ghost: The Creator Economy Platform | ||
</a> | ||
</td> | ||
</tr> | ||
<tr> | ||
<td> | ||
<div class="kg-bookmark-description--outlook"> | ||
<a href="https://www.ghost.org/" style="text-decoration: none; margin-top: 12px; color: #738a94; font-size: 13px; line-height: 1.5em; font-weight: 400;"> | ||
doing kewl stuff | ||
</a> | ||
</div> | ||
</td> | ||
</tr> | ||
<tr> | ||
<td class="kg-bookmark-metadata--outlook" style="padding-top: 14px; color: #15212a; font-size: 13px; font-weight: 400; line-height: 1.5em;"> | ||
<table style="margin: 0; padding: 0; border-collapse: collapse; border-spacing: 0;"> | ||
<tr> | ||
<td valign="middle" class="kg-bookmark-icon--outlook" style="padding-right: 8px; font-size: 0; line-height: 1.5em;"> | ||
<a href="https://www.ghost.org/" style="text-decoration: none; color: #15212a"> | ||
<img src="https://www.ghost.org/favicon.ico" width="22" height="22" alt=" "> | ||
</a> | ||
</td> | ||
|
||
<td valign="middle" class="kg-bookmark-byline--outlook"> | ||
<a href="https://www.ghost.org/" style="text-decoration: none; color: #15212a"> | ||
Ghost - The Professional Publishing Platform • ghost | ||
</a> | ||
</td> | ||
</tr> | ||
</table> | ||
</td> | ||
</tr> | ||
</table> | ||
</td> | ||
</tr> | ||
</table> | ||
<div class="kg-bookmark-spacer--outlook" style="height: 1.5em"> </div> | ||
<![endif]--> | ||
</div> | ||
`); |
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.
💡 Verification agent
🧩 Analysis chain
Verify the unusual src
attributes on span elements.
The email snapshot contains src
attributes on span elements (lines 100-101):
<span class="kg-bookmark-author" src="Ghost - The Professional Publishing Platform">
<span class="kg-bookmark-publisher" src="ghost">
The src
attribute is typically used for elements like <img>
, not <span>
. This could be unintentional or might be a workaround for specific email clients.
Run this script to check if this pattern is used elsewhere in the codebase:
🏁 Script executed:
#!/bin/bash
# Description: Check for span elements with src attributes in the codebase
# Expected: Determine if this is a pattern or an anomaly
echo "Searching for span elements with src attributes..."
rg -A 2 -B 2 '<span[^>]*src=' --type html --type js
echo -e "\nSearching in bookmark renderer specifically..."
fd bookmark-renderer -t f --exec rg -A 5 -B 5 'src=' {} \;
Length of output: 8015
Remove invalid src
attributes from <span>
in bookmark-renderer (code and tests)
The src
attribute is not valid on <span>
elements and should be removed. Please update both the renderer and its snapshot tests.
• In ghost/core/core/server/services/koenig/node-renderers/bookmark-renderer.js
- Remove
src="${publisher}"
andsrc="${author}"
from the author/publisher<span>
tags.
• Inghost/core/test/unit/server/services/koenig/node-renderers/bookmark-renderer.test.js
- Update the expected HTML to drop the
src
attributes on the two<span>
elements.
Suggested diff:
--- a/core/server/services/koenig/node-renderers/bookmark-renderer.js
+++ b/core/server/services/koenig/node-renderers/bookmark-renderer.js
@@ -47,7 +47,7 @@ module.exports = ({url, title, description, icon, publisher, author, thumbna
- ${publisher ? `<span class="kg-bookmark-author" src="${publisher}">${publisher}</span>` : ''}
+ ${publisher ? `<span class="kg-bookmark-author">${publisher}</span>` : ''}
- ${author ? `<span class="kg-bookmark-publisher" src="${author}">${author}</span>` : ''}
+ ${author ? `<span class="kg-bookmark-publisher">${author}</span>` : ''}
--- a/test/unit/server/services/koenig/node-renderers/bookmark-renderer.test.js
+++ b/test/unit/server/services/koenig/node-renderers/bookmark-renderer.test.js
@@ Lines 100-101:
- <span class="kg-bookmark-author" src="Ghost - The Professional Publishing Platform">Ghost - The Professional Publishing Platform</span>
- <span class="kg-bookmark-publisher" src="ghost">ghost</span>
+ <span class="kg-bookmark-author">Ghost - The Professional Publishing Platform</span>
+ <span class="kg-bookmark-publisher">ghost</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.
assertPrettifiesTo(result.html, html` | |
<div> | |
<!--[if !mso !vml]--> | |
<figure class="kg-card kg-bookmark-card kg-card-hascaption"> | |
<a class="kg-bookmark-container" href="https://www.ghost.org/"> | |
<div class="kg-bookmark-content"> | |
<div class="kg-bookmark-title">Ghost: The Creator Economy Platform</div> | |
<div class="kg-bookmark-description">doing kewl stuff</div> | |
<div class="kg-bookmark-metadata"> | |
<img class="kg-bookmark-icon" src="https://www.ghost.org/favicon.ico" alt=""> | |
<span class="kg-bookmark-author" src="Ghost - The Professional Publishing Platform">Ghost - The Professional Publishing Platform</span> | |
<span class="kg-bookmark-publisher" src="ghost">ghost</span> | |
</div> | |
</div> | |
<div class="kg-bookmark-thumbnail" style="background-image: url('https://ghost.org/images/meta/ghost.png')"> | |
<img src="https://ghost.org/images/meta/ghost.png" alt="" onerror="this.style.display='none'"> | |
</div> | |
</a> | |
<figcaption>caption here</figcaption> | |
</figure> | |
<!--[endif]--><!--[if vml]> | |
<table class="kg-card kg-bookmark-card--outlook" style="margin: 0; padding: 0; width: 100%; border: 1px solid #e5eff5; background: #ffffff; font-family: -apple-system, BlinkMacSystemFont, 'Segoe UI', Roboto, Oxygen, Ubuntu, Cantarell, 'Open Sans', 'Helvetica Neue', sans-serif; border-collapse: collapse; border-spacing: 0;" width="100%"> | |
<tr> | |
<td width="100%" style="padding: 20px"> | |
<table style="margin: 0; padding: 0; border-collapse: collapse; border-spacing: 0;"> | |
<tr> | |
<td class="kg-bookmark-title--outlook"> | |
<a href="https://www.ghost.org/" style="text-decoration: none; color: #15212a; font-size: 15px; line-height: 1.5em; font-weight: 600;"> | |
Ghost: The Creator Economy Platform | |
</a> | |
</td> | |
</tr> | |
<tr> | |
<td> | |
<div class="kg-bookmark-description--outlook"> | |
<a href="https://www.ghost.org/" style="text-decoration: none; margin-top: 12px; color: #738a94; font-size: 13px; line-height: 1.5em; font-weight: 400;"> | |
doing kewl stuff | |
</a> | |
</div> | |
</td> | |
</tr> | |
<tr> | |
<td class="kg-bookmark-metadata--outlook" style="padding-top: 14px; color: #15212a; font-size: 13px; font-weight: 400; line-height: 1.5em;"> | |
<table style="margin: 0; padding: 0; border-collapse: collapse; border-spacing: 0;"> | |
<tr> | |
<td valign="middle" class="kg-bookmark-icon--outlook" style="padding-right: 8px; font-size: 0; line-height: 1.5em;"> | |
<a href="https://www.ghost.org/" style="text-decoration: none; color: #15212a"> | |
<img src="https://www.ghost.org/favicon.ico" width="22" height="22" alt=" "> | |
</a> | |
</td> | |
<td valign="middle" class="kg-bookmark-byline--outlook"> | |
<a href="https://www.ghost.org/" style="text-decoration: none; color: #15212a"> | |
Ghost - The Professional Publishing Platform • ghost | |
</a> | |
</td> | |
</tr> | |
</table> | |
</td> | |
</tr> | |
</table> | |
</td> | |
</tr> | |
</table> | |
<div class="kg-bookmark-spacer--outlook" style="height: 1.5em"> </div> | |
<![endif]--> | |
</div> | |
`); | |
assertPrettifiesTo(result.html, html` | |
<div> | |
<!--[if !mso !vml]--> | |
<figure class="kg-card kg-bookmark-card kg-card-hascaption"> | |
<a class="kg-bookmark-container" href="https://www.ghost.org/"> | |
<div class="kg-bookmark-content"> | |
<div class="kg-bookmark-title">Ghost: The Creator Economy Platform</div> | |
<div class="kg-bookmark-description">doing kewl stuff</div> | |
<div class="kg-bookmark-metadata"> | |
<img class="kg-bookmark-icon" src="https://www.ghost.org/favicon.ico" alt=""> | |
<span class="kg-bookmark-author">Ghost - The Professional Publishing Platform</span> | |
<span class="kg-bookmark-publisher">ghost</span> | |
</div> | |
</div> | |
<div class="kg-bookmark-thumbnail" style="background-image: url('https://ghost.org/images/meta/ghost.png')"> | |
<img src="https://ghost.org/images/meta/ghost.png" alt="" onerror="this.style.display='none'"> | |
</div> | |
</a> | |
<figcaption>caption here</figcaption> | |
</figure> | |
<!--[endif]--><!--[if vml]> | |
<table class="kg-card kg-bookmark-card--outlook" style="margin: 0; padding: 0; width: 100%; border: 1px solid #e5eff5; background: #ffffff; font-family: -apple-system, BlinkMacSystemFont, 'Segoe UI', Roboto, Oxygen, Ubuntu, Cantarell, 'Open Sans', 'Helvetica Neue', sans-serif; border-collapse: collapse; border-spacing: 0;" width="100%"> | |
<tr> | |
<td width="100%" style="padding: 20px"> | |
<table style="margin: 0; padding: 0; border-collapse: collapse; border-spacing: 0;"> | |
<tr> | |
<td class="kg-bookmark-title--outlook"> | |
<a href="https://www.ghost.org/" style="text-decoration: none; color: #15212a; font-size: 15px; line-height: 1.5em; font-weight: 600;"> | |
Ghost: The Creator Economy Platform | |
</a> | |
</td> | |
</tr> | |
<tr> | |
<td> | |
<div class="kg-bookmark-description--outlook"> | |
<a href="https://www.ghost.org/" style="text-decoration: none; margin-top: 12px; color: #738a94; font-size: 13px; line-height: 1.5em; font-weight: 400;"> | |
doing kewl stuff | |
</a> | |
</div> | |
</td> | |
</tr> | |
<tr> | |
<td class="kg-bookmark-metadata--outlook" style="padding-top: 14px; color: #15212a; font-size: 13px; font-weight: 400; line-height: 1.5em;"> | |
<!-- … rest of snapshot unchanged … --> |
🤖 Prompt for AI Agents
In
ghost/core/test/unit/server/services/koenig/node-renderers/bookmark-renderer.test.js
lines 90 to 157, the test HTML includes invalid src attributes on span elements
for author and publisher. Remove the src attributes from these span tags in the
expected HTML to match valid HTML standards. Also, ensure the corresponding
renderer file
ghost/core/core/server/services/koenig/node-renderers/bookmark-renderer.js is
updated to not output src attributes on span elements for author and publisher.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #23548 +/- ##
==========================================
- Coverage 72.73% 72.73% -0.01%
==========================================
Files 1530 1529 -1
Lines 111010 110998 -12
Branches 13682 13681 -1
==========================================
- Hits 80740 80729 -11
+ Misses 29246 29245 -1
Partials 1024 1024
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
closes https://linear.app/ghost/issue/PROD-1784
services/koenig/node-renderers/index.js
@tryghost/kg-default-nodes