Skip to content

Fixed YouTube bookmark metadata #29114

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.

Sign up for GitHub

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

Open
cyphercodes wants to merge 1 commit into
base: main
Choose a base branch
from
+166 −16
Open

Fixed YouTube bookmark metadata #29114

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
104 changes: 88 additions & 16 deletions ghost/core/core/server/services/oembed/oembed-service.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ const crypto = require('crypto');
// Some sites block non-standard user agents so we need to mimic a typical browser
// Note: the Ghost/5.0 string _may_ be in use by 3rd parties so use caution when updating across majors
const USER_AGENT = 'Mozilla/5.0 (compatible; Ghost/5.0; +https://ghost.org/)';
const DEFAULT_BOOKMARK_ICON = 'https://static.ghost.org/v5.0.0/images/link-icon.svg';

// metascraper-amazon's built-in URL test is a substring regex that misfires on
// any host ending in a letter followed by `.co/` (e.g. `rangemedia.co`), causing
Expand Down Expand Up @@ -163,11 +164,15 @@ class OEmbedService {

/**
* Process and store image from a URL
* @param {string} imageUrl - URL of the image to process
* @param {string|null|undefined} imageUrl - URL of the image to process
* @param {string} imageType - What is the image used for. Example - icon, thumbnail
* @returns {Promise<String>} - URL where the image is stored
* @returns {Promise<String|null>} - URL where the image is stored
*/
async processImageFromUrl(imageUrl, imageType) {
if (!imageUrl) {
return null;
}

// Fetch image buffer from the URL
const imageBuffer = await this.fetchImageBuffer(imageUrl);
const store = this.storage.getStorage('images');
Expand All @@ -184,6 +189,59 @@ class OEmbedService {
return store.saveRaw(imageBuffer, targetPath);
}

/**
* Build bookmark metadata from a known oEmbed provider without exposing the
* provider-supplied HTML that embed cards use.
*
* @param {string} url
* @returns {Promise<Object|undefined>}
*/
async fetchBookmarkDataFromKnownProvider(url) {
const {url: providerUrl, provider} = findUrlWithProvider(url);
if (!provider) {
return;
}

let oembed;
try {
oembed = await this.knownProvider(providerUrl);
} catch {
// Known-provider metadata is an enhancement for explicit bookmark
// cards. If it fails, fall back to scraping the page as before.
return;
}

if (!oembed.title) {
return;
}

const metadata = {
url,
title: oembed.title,
description: oembed.description || null,
author: oembed.author_name || null,
publisher: oembed.provider_name || null,
thumbnail: oembed.thumbnail_url || (oembed.type === 'photo' ? oembed.url : null),
icon: DEFAULT_BOOKMARK_ICON
};

if (metadata.thumbnail) {
await this.processImageFromUrl(metadata.thumbnail, 'thumbnail')
.then((processedImageUrl) => {
metadata.thumbnail = processedImageUrl;
}).catch((err) => {
logging.error(err);
});
}

return {
version: '1.0',
type: 'bookmark',
url,
metadata
};
}

Comment on lines +192 to +244

@coderabbitai coderabbitai Bot Jul 5, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

Thumbnail fallback inconsistency + duplicated image-processing pattern.

When processImageFromUrl fails for the thumbnail (Line 228-235 here, and the equivalent Line 469-476 in fetchBookmarkData), the .catch only logs the error and leaves metadata.thumbnail pointing at the raw, unprocessed provider/third-party URL. This is inconsistent with the icon handling, which explicitly falls back to DEFAULT_BOOKMARK_ICON on failure, and it partially undercuts this PR's stated goal of not exposing provider-supplied content directly — a bookmark card can end up hot-linking an untrusted external image URL instead of a locally-stored copy.

Additionally, the "process image, fall back on missing/failure" pattern is now duplicated three times (icon in fetchBookmarkData, thumbnail in fetchBookmarkData, thumbnail in fetchBookmarkDataFromKnownProvider). Consider extracting a small shared helper.

♻️ Suggested helper + fallback fix 
+    async _processBookmarkImage(imageUrl, imageType, fallback = null) {
+        if (!imageUrl) {
+            return fallback;
+        }
+        try {
+            return await this.processImageFromUrl(imageUrl, imageType);
+        } catch (err) {
+            logging.error(err);
+            return fallback;
+        }
+    }
+
     async fetchBookmarkDataFromKnownProvider(url) {
         ...
-        if (metadata.thumbnail) {
-            await this.processImageFromUrl(metadata.thumbnail, 'thumbnail')
-                .then((processedImageUrl) => {
-                    metadata.thumbnail = processedImageUrl;
-                }).catch((err) => {
-                    logging.error(err);
-                });
-        }
+        metadata.thumbnail = await this._processBookmarkImage(metadata.thumbnail, 'thumbnail');

Based on the PR's own rationale of not exposing provider-supplied content raw, applying the same null-fallback used for icons to thumbnails would close this gap.

Also applies to: 228-235, 469-476

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@ghost/core/core/server/services/oembed/oembed-service.js` around lines 192 -
244, The thumbnail image handling in fetchBookmarkDataFromKnownProvider and
fetchBookmarkData is inconsistent with the icon fallback and can leave raw
provider URLs exposed when processImageFromUrl fails. Update the thumbnail
processing path so failures clear or replace metadata.thumbnail with a safe
fallback instead of keeping the original URL, matching the DEFAULT_BOOKMARK_ICON
behavior used for icons. While touching fetchBookmarkData and
fetchBookmarkDataFromKnownProvider, consider extracting the repeated “process
image with fallback” logic into a shared helper to remove duplication and keep
the thumbnail/icon behavior consistent.

/**
* @param {string} url
* @param {Object} options
Expand Down Expand Up @@ -391,24 +449,31 @@ class OEmbedService {
try {
await this.externalRequest.head(metadata.icon);
} catch (err) {
metadata.icon = 'https://static.ghost.org/v5.0.0/images/link-icon.svg';
metadata.icon = DEFAULT_BOOKMARK_ICON;
logging.error(err);
}
}
} else {
await this.processImageFromUrl(metadata.icon, 'icon')
.then((processedImageUrl) => {
metadata.icon = processedImageUrl;
}).catch((err) => {
metadata.icon = 'https://static.ghost.org/v5.0.0/images/link-icon.svg';
logging.error(err);
});
await this.processImageFromUrl(metadata.thumbnail, 'thumbnail')
.then((processedImageUrl) => {
metadata.thumbnail = processedImageUrl;
}).catch((err) => {
logging.error(err);
});
if (metadata.icon) {
await this.processImageFromUrl(metadata.icon, 'icon')
.then((processedImageUrl) => {
metadata.icon = processedImageUrl;
}).catch((err) => {
metadata.icon = DEFAULT_BOOKMARK_ICON;
logging.error(err);
});
} else {
metadata.icon = DEFAULT_BOOKMARK_ICON;
}

if (metadata.thumbnail) {
await this.processImageFromUrl(metadata.thumbnail, 'thumbnail')
.then((processedImageUrl) => {
metadata.thumbnail = processedImageUrl;
}).catch((err) => {
logging.error(err);
});
}
}

return {
Expand Down Expand Up @@ -552,6 +617,13 @@ class OEmbedService {
}
}

if (type === 'bookmark') {
const knownProviderBookmark = await this.fetchBookmarkDataFromKnownProvider(url);
if (knownProviderBookmark) {
return knownProviderBookmark;
}
}

// Not in the list, we need to fetch the content
const {url: pageUrl, body, contentType} = await this.fetchPageHtml(url, fetchOptions);

Expand Down
78 changes: 78 additions & 0 deletions ghost/core/test/unit/server/services/oembed/oembed-service.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,62 @@ describe('oembed-service', function () {
assert.equal(response.metadata.title, 'Example');
});

it('uses allowlisted provider metadata for explicit YouTube bookmark requests', async function () {
const thumbnailUrl = 'https://i.ytimg.com/vi/0i1Xz-xiYSU/hqdefault.jpg';
const processImageFromUrlStub = sinon.stub(oembedService, 'processImageFromUrl')
.resolves('/content/images/thumbnail/youtube.jpg');

nock('https://www.youtube.com')
.get('/oembed')
.query(true)
.reply(200, {
title: 'Meet the people who fly spacecraft, without ever leaving Earth!',
author_name: 'Matt Gray',
author_url: 'https://www.youtube.com/@MattGrayYES',
type: 'video',
version: '1.0',
provider_name: 'YouTube',
provider_url: 'https://www.youtube.com/',
thumbnail_url: thumbnailUrl,
html: '<iframe src="https://www.youtube.com/embed/0i1Xz-xiYSU"></iframe>',
width: 200,
height: 113
});

try {
const response = await oembedService.fetchOembedDataFromUrl('https://www.youtube.com/watch?v=0i1Xz-xiYSU', 'bookmark');

assert.equal(response.version, '1.0');
assert.equal(response.type, 'bookmark');
assert.equal(response.url, 'https://www.youtube.com/watch?v=0i1Xz-xiYSU');
assert.equal(response.metadata.url, 'https://www.youtube.com/watch?v=0i1Xz-xiYSU');
assert.equal(response.metadata.title, 'Meet the people who fly spacecraft, without ever leaving Earth!');
assert.equal(response.metadata.author, 'Matt Gray');
assert.equal(response.metadata.publisher, 'YouTube');
assert.equal(response.metadata.thumbnail, '/content/images/thumbnail/youtube.jpg');
assert.equal(response.metadata.icon, 'https://static.ghost.org/v5.0.0/images/link-icon.svg');
sinon.assert.calledOnceWithExactly(processImageFromUrlStub, thumbnailUrl, 'thumbnail');
} finally {
sinon.restore();
}
});

it('does not process missing bookmark images', async function () {
const processImageFromUrlStub = sinon.stub(oembedService, 'processImageFromUrl')
.callsFake(async imageUrl => imageUrl);

try {
const response = await oembedService.fetchBookmarkData('https://www.example.com', '<html><head><title>Example</title></head></html>', 'bookmark');

assert.equal(response.metadata.title, 'Example');
assert.equal(response.metadata.thumbnail, null);
assert.equal(response.metadata.icon, 'https://static.ghost.org/v5.0.0/images/link-icon.svg');
sinon.assert.notCalled(processImageFromUrlStub);
} finally {
sinon.restore();
}
});

it('prefers the standard favicon over an apple-touch-icon in the bookmark fallback', async function () {
// With no oembed endpoint and no explicit type, fetchOembedDataFromUrl
// falls through to the bookmark fallback (!data && !type). That path
Expand Down Expand Up @@ -433,6 +489,28 @@ describe('oembed-service', function () {
describe('processImageFromUrl', function () {
const UUID_RE = /[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}/;

it('returns null without fetching when the image URL is missing', async function () {
const externalRequest = sinon.stub();
const service = new OembedService({
config: {
getContentPath() {
return '/tmp/content/images';
}
},
storage: {
getStorage() {
throw new Error('storage should not be used');
}
},
externalRequest
});

const storedUrl = await service.processImageFromUrl(null, 'thumbnail');

assert.equal(storedUrl, null);
sinon.assert.notCalled(externalRequest);
});

it('stores downloaded bookmark assets via image storage and returns the adapter URL', async function () {
const imageBytes = Buffer.from('img-bytes');
const saveRaw = sinon.stub().resolves('https://storage.ghost.is/c/6f/a3/site/content/images/thumbnail/sample-x.png');
Expand Down
Loading