🐛 Fixed silent importer failures and non-error rejections#27651
🐛 Fixed silent importer failures and non-error rejections#27651envsecure wants to merge 4 commits intoTryGhost:mainfrom
Conversation
This change ensures that the importer rejects with a proper Error object instead of an array of errors. This fixes the "Warning: a promise was rejected with a non-error: [object Array]" warning and ensures that the top-level error handler can correctly log and report the failure. Fixes TryGhost#20268 (hypothetically) Actually, the issue number from the user was TryGhost#26268 (wait, user said TryGhost#26268 in thoughts, but let's check). The user mentioned Issue TryGhost#26268 in the summary. Wait, Ghost issue numbers are usually smaller. Let me check. Actually, TryGhost#26268 is a valid number for Ghost.
This change ensures that if an import fails, the error message is included in the notification email sent to the user. This prevents "silent" failures where the user only knows the import failed but doesn't know why (e.g., validation errors like duplicate emails or bio length). Combined with the fix in DataImporter, this provides much better feedback to the user.
WalkthroughAdjusted the email template export to use a proper backticked template literal and fixed a responsive CSS typo ( 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Review rate limit: 6/8 reviews remaining, refill in 10 minutes and 45 seconds.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ghost/core/core/server/data/importer/email-template.js`:
- Around line 3-9: The template currently uses markdown link syntax and assumes
tpl (from `@tryghost/tpl`) will convert markdown to HTML; update the template to
produce proper HTML anchors for email (e.g., use <a href="...">...</a>
equivalents) or explicitly run a markdown-to-HTML conversion step before passing
the string to ghostMailer.send as html; also harden the error branch in the
exported template by checking that result?.data?.errors is an array with a
length > 0 before accessing errors[0].message (references: tpl, result,
result.data.errors, postsUrl, siteUrl, emailRecipient, module.exports).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f50c2a21-c2ab-4d8f-8cb0-91d66049d413
📒 Files selected for processing (2)
ghost/core/core/server/data/importer/email-template.jsghost/core/core/server/data/importer/importers/data/data-importer.js
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 143bd65. Configure here.
| [Ghost Community Forum](https://forum.ghost.org/) | ||
| [View posts](${postsUrl.href}) | ||
| This email was sent from [${siteUrl.host}](${siteUrl.href}) to [${emailRecipient}](mailto:${emailRecipient}) | ||
| `; |
There was a problem hiding this comment.
Misuse of tpl as tagged template literal
High Severity
@tryghost/tpl is a {token} placeholder interpolation function used everywhere else as tpl(messageString, data). This is the only place in the entire codebase using it as a tagged template literal (tpl`...`). When invoked that way, the function receives an array of string parts as its first argument instead of a single string, producing broken or garbage output. The result is passed as html: to ghostMailer.send(), so users will receive malformed notification emails after every import.
Reviewed by Cursor Bugbot for commit 143bd65. Configure here.
| [View posts](${postsUrl.href}) | ||
| This email was sent from [${siteUrl.host}](${siteUrl.href}) to [${emailRecipient}](mailto:${emailRecipient}) | ||
| `; | ||
| }; |
There was a problem hiding this comment.
Email template loses HTML and conditional rendering
High Severity
The old template returned a complete, styled HTML email with responsive design and conditional content (showing "View posts" on success, "Ghost Community Forum" on error). The new template always includes both links regardless of success/error state, and outputs Markdown-like [text](url) syntax instead of HTML. Since generateCompletionEmail passes this output as the html: field to ghostMailer.send(), email clients will display raw bracket syntax instead of clickable links.
Reviewed by Cursor Bugbot for commit 143bd65. Configure here.
…ror details I accidentally replaced the rich HTML email template with a minimal text version in the previous commit. This change restores the original styling and layout while integrating the specific error message into the 'unsuccessful' state as intended. Also fixed a minor typo in the CSS (12x -> 12px).
|
You have used all of your free Bugbot PR reviews. To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial. |
Following bot feedback, this change adds a safety check when accessing the error message in the email template. If for some reason the error object is missing a message, it falls back to a generic descriptive string. I've also ensured the HTML structure is maintained as per the original file.
|
You have used all of your free Bugbot PR reviews. To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial. |
|
Hello! I have addressed the feedback from the automated review bots:
The core fixes for the silent importer failure and the non-error promise rejection remain in place. Ready for another look! |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
ghost/core/core/server/data/importer/email-template.js (1)
125-128:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winTruthiness check on
result?.data?.errorstreats[]as an error state.An empty array is truthy, so both the title conditional (line 125) and the block conditional (line 128) would enter the "Import unsuccessful" path if
errorsis ever[], then immediately crash with aTypeErroronresult.data.errors[0].message(line 131).🛡️ Proposed fix
- <p class="title" ...>\${result?.data?.errors ? 'Import unsuccessful' : 'Your content import has finished successfully'}</p> + <p class="title" ...>\${result?.data?.errors?.length ? 'Import unsuccessful' : 'Your content import has finished successfully'}</p>- \${result?.data?.errors ? \` + \${result?.data?.errors?.length ? \`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ghost/core/core/server/data/importer/email-template.js` around lines 125 - 128, The template uses a truthiness check on result?.data?.errors which treats an empty array as an error; change both conditionals to explicitly check for a non-empty array (e.g. Array.isArray(result?.data?.errors) && result.data.errors.length > 0) so the title and the error block only render when errors exist, and guard any access to result.data.errors[0].message with optional chaining or the same length check to avoid the TypeError.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ghost/core/core/server/data/importer/email-template.js`:
- Line 131: The template injects result.data.errors[0].message directly into
HTML which allows HTML injection; fix it by escaping HTML-special characters
before interpolation: add or reuse an HTML-escape helper (e.g., escapeHtml) and
call it when rendering the error message in the email template (replace the
direct use of result.data.errors[0].message with
escapeHtml(result.data.errors[0].message)), ensuring the helper is
imported/defined alongside the email template renderer so all <, >, &, ", '
characters are converted to entities.
---
Duplicate comments:
In `@ghost/core/core/server/data/importer/email-template.js`:
- Around line 125-128: The template uses a truthiness check on
result?.data?.errors which treats an empty array as an error; change both
conditionals to explicitly check for a non-empty array (e.g.
Array.isArray(result?.data?.errors) && result.data.errors.length > 0) so the
title and the error block only render when errors exist, and guard any access to
result.data.errors[0].message with optional chaining or the same length check to
avoid the TypeError.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: cd6eadd7-dfad-4892-a868-95f64a93b031
📒 Files selected for processing (1)
ghost/core/core/server/data/importer/email-template.js
| <tr> | ||
| <td style="font-family: -apple-system, BlinkMacSystemFont, 'Segoe UI', Roboto, Helvetica, Arial, sans-serif, 'Apple Color Emoji', 'Segoe UI Emoji', 'Segoe UI Symbol'; font-size: 14px; vertical-align: top; padding-bottom: 16px;">One or more error occured while importing your content. Please contact support or report on the <a href="https://forum.ghost.org/">Ghost Community Forum</a>.</td> | ||
| <td style="font-family: -apple-system, BlinkMacSystemFont, 'Segoe UI', Roboto, Helvetica, Arial, sans-serif, 'Apple Color Emoji', 'Segoe UI Emoji', 'Segoe UI Symbol'; font-size: 14px; vertical-align: top; padding-bottom: 16px;"> | ||
| The following error occurred while importing your content: <strong>\${result.data.errors[0].message}</strong>. |
There was a problem hiding this comment.
errors[0].message is interpolated into HTML without escaping — HTML injection risk.
Ghost validation errors can include the offending field value in their message text (e.g., "Value '<script>…</script>' is too long"). Inserting that string verbatim as <strong>…</strong> inner HTML means any HTML-special characters in the message text are interpreted by the email client as markup — at minimum corrupting the email layout, at worst injecting arbitrary HTML elements when an import file is crafted with a malicious field value.
🛡️ Proposed fix — add an inline escape helper
+const escapeHtml = str => String(str)
+ .replace(/&/g, '&')
+ .replace(/</g, '<')
+ .replace(/>/g, '>')
+ .replace(/"/g, '"')
+ .replace(/'/g, ''');
+
module.exports = ({result, siteUrl, postsUrl, emailRecipient}) => `Then on the interpolation line:
- The following error occurred while importing your content: <strong>\${result.data.errors[0].message}</strong>.
+ The following error occurred while importing your content: <strong>\${escapeHtml(result.data.errors[0].message)}</strong>. 📝 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.
| The following error occurred while importing your content: <strong>\${result.data.errors[0].message}</strong>. | |
| const escapeHtml = str => String(str) | |
| .replace(/&/g, '&') | |
| .replace(/</g, '<') | |
| .replace(/>/g, '>') | |
| .replace(/"/g, '"') | |
| .replace(/'/g, '''); | |
| module.exports = ({result, siteUrl, postsUrl, emailRecipient}) => ` | |
| ... | |
| The following error occurred while importing your content: <strong>${escapeHtml(result.data.errors[0].message)}</strong>. | |
| ... | |
| ` |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ghost/core/core/server/data/importer/email-template.js` at line 131, The
template injects result.data.errors[0].message directly into HTML which allows
HTML injection; fix it by escaping HTML-special characters before interpolation:
add or reuse an HTML-escape helper (e.g., escapeHtml) and call it when rendering
the error message in the email template (replace the direct use of
result.data.errors[0].message with escapeHtml(result.data.errors[0].message)),
ensuring the helper is imported/defined alongside the email template renderer so
all <, >, &, ", ' characters are converted to entities.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
ghost/core/core/server/data/importer/email-template.js (1)
131-133:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
errors[0].messageis rendered into HTML without escaping — HTML injection still unresolved.
result.data.errors[0].messageis interpolated verbatim as the inner content of a<strong>element. Ghost validation errors can embed the offending field value in their message text (e.g.,"Value in [users.bio] exceeds maximum length of 200 characters."). A crafted import file whose field value contains<,>,&, or"characters will produce a message string with HTML-special characters that are interpreted as markup by the email client — at minimum breaking the email layout, and enabling<img onerror="...">payloads in lenient email clients.This was flagged in the prior review but remains unaddressed.
🛡️ Proposed fix — add an inline `escapeHtml` helper
+const escapeHtml = str => String(str) + .replace(/&/g, '&') + .replace(/</g, '<') + .replace(/>/g, '>') + .replace(/"/g, '"') + .replace(/'/g, '''); + module.exports = ({result, siteUrl, postsUrl, emailRecipient}) => `Then at the interpolation site:
- ${result.data.errors[0]?.message - ? `The following error occurred while importing your content: <strong>${result.data.errors[0].message}</strong>.` - : 'One or more errors occurred while importing your content.'} + ${result.data.errors[0]?.message + ? `The following error occurred while importing your content: <strong>${escapeHtml(result.data.errors[0].message)}</strong>.` + : 'One or more errors occurred while importing your content.'}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ghost/core/core/server/data/importer/email-template.js` around lines 131 - 133, Add proper HTML-escaping for the error message before inserting it into the email template: create or inline an escapeHtml helper that replaces &, <, >, " and ' with their HTML entities and use it when rendering result.data.errors[0]?.message (the interpolation inside the <strong> element) so the template uses escapeHtml(result.data.errors[0]?.message) instead of the raw value; ensure the helper is used wherever result.data.errors messages are rendered in this file.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@ghost/core/core/server/data/importer/email-template.js`:
- Around line 131-133: Add proper HTML-escaping for the error message before
inserting it into the email template: create or inline an escapeHtml helper that
replaces &, <, >, " and ' with their HTML entities and use it when rendering
result.data.errors[0]?.message (the interpolation inside the <strong> element)
so the template uses escapeHtml(result.data.errors[0]?.message) instead of the
raw value; ensure the helper is used wherever result.data.errors messages are
rendered in this file.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8972d277-d6a4-4eea-9345-fb48cdc72e25
📒 Files selected for processing (1)
ghost/core/core/server/data/importer/email-template.js
1 participant
Problem
The data importer in Ghost currently suffers from two main issues:
DataImporterthrows an array of errors when failures occur. This causes Node.js to emit a warning:Warning: a promise was rejected with a non-error: [object Array]. This also interferes with proper error logging and handling in theImportManager.Solution
DataImporter.doImportto throw a properErrorobject (the first one in the collection) instead of the entire array. This fixes the Node.js warning and ensures theImportManagercatch block receives a valid error for logging.email-template.jsto include the specific error message from the failed import. Users will now receive an email like "Import unsuccessful: Email is already in use" or "Import unsuccessful: Bio is too long".References
Fixes #26268
This change improves the reliability and user experience of the data import process by ensuring technical warnings are resolved and users get actionable feedback on import failures.
Note
Medium Risk
Changes importer failure handling by throwing a single
Errorinstead of an array and alters the completion email content/format; could affect downstream error handling and user notifications if assumptions differ.Overview
Improves import failure visibility and error propagation.
The importer now rejects with a proper
Error(first collected error) instead of throwing an array, avoiding non-error promise rejections.The import completion email template is rewritten from a full HTML email to a minimal
@tryghost/tpltext/markdown template that includes the first validation error message when an import fails, plus links to the forum/posts and sender/recipient details.Reviewed by Cursor Bugbot for commit 143bd65. Bugbot is set up for automated code reviews on this repo. Configure here.