Fix #6588: a file:// link in a card no longer makes the card impossible to open.

"A card whose description or a comment contains a `file://` URL cannot be opened.
Clicking the minicard plays the open animation but the card details panel never
mounts. There is no visible error by default because Blaze swallows the render
exception." Captured, it was:

  TypeError: this.__schemas__[t.toLowerCase()].validate is not a function
      testSchemaAt / matchAtStart / markdown-it (tokenize -> parse -> render)

WeKan registers eight custom URL schemes so links like `thunderlink:` and
`file://` are recognised, and it registered them the linkify-it 4/5 way:

  Markdown.linkify.add(scheme + ':', 'http:')      // "behave like http:"

linkify-it 6 removed string aliases and builds the definition by SPREADING it:

  const def = { normalize: ..., ...definition };

Spreading the string 'http:' produces {0:'h',1:'t',2:'t',3:'p',4:':'} - an entry
with no `validate` - and testSchemaAt then calls `.validate(...)` on it. So every
one of those eight schemes was a landmine in any card's text, and the card holding
one could not be opened again. Reproduced here against the installed linkify-it,
not from the report: the test builds that same registration and watches it throw.

Each scheme now carries a validate of its own, which is all the alias was standing
in for: the URL after the scheme - an optional `//`, then everything up to
whitespace or a bracket - with trailing sentence punctuation handed back, so "see
file://server/share/doc.xlsm." does not swallow the full stop.

WHAT THAT DOES NOT DO, because the list promises more than it delivers.
Registering a scheme makes markdown-it RECOGNISE it; two filters then decide
whether a reader gets a clickable link, and both refuse these. markdown-it's own
validateLink blocks file: (with javascript:, vbscript: and data:), and the
viewer's DOMPurify allows only http/https/ftp/ftps/mailto/tel/callto/cid/xmpp
hrefs. So none of the eight has ever produced a clickable link - its only
observable effect was the crash. Making them clickable means relaxing both
filters for schemes that launch local applications, which is the security decision
#3218 asks for and not something to slip in while fixing a crash. It goes to TODO
Later; the tests pin today's answer so the day it changes is a decision.

And the guarantee behind the fix, which is worth more than the fix: nothing a card
contains may make that card impossible to open. The markdown render is wrapped, so
a throw from a plugin, a pathological formula or the next markdown-it upgrade
shows the text as written - escaped, sanitised, unformatted, and readable - instead
of a panel that never mounts.

tests/markdownCustomUrlSchemes.test.cjs runs the real linkify-it and markdown-it
from node_modules: the old registration throwing, every scheme having a working
validate, the reported strings rendering, punctuation staying in the sentence, a
bare `file:` and junk input returning zero rather than throwing, and the viewer's
fallback returning sanitised text rather than nothing.

Verified by running the suite and the whole node suite (382 suites, 0 failed). Not
verified in a browser: this sandbox has none, so the crash and the fix are
exercised against the same dependency the browser would load.

Thanks to rmb82 and xet7 !

Fixes #6588.
This commit is contained in:
Lauri Ojansivu committed 2026-08-12 15:36:53 +03:00
1 parent 40ff8151ea
commit 75a23b76ae
2 files changed
+283 -4

No files matched your search

+86 -4
View File
@@ -63,9 +63,68 @@ var urlschemes = [
// #6588: a card whose description or comment contains a `file://` link could not
// be OPENED at all - the details panel never mounted, and the swallowed exception
// was
//
// TypeError: this.__schemas__[...].validate is not a function
//
// thrown out of markdown-it's linkify pass, so the whole card render died on one
// link.
//
// The cause is the line that used to be here: `linkify.add(scheme + ':', 'http:')`.
// A STRING second argument was linkify-it 4/5's way of saying "this scheme behaves
// like that one". linkify-it 6 removed aliases and builds the definition by
// spreading it:
//
// const def = { normalize: ..., ...definition };
//
// Spreading the string 'http:' gives `{0:'h',1:'t',2:'t',3:'p',4:':'}` - an entry
// with NO validate - and `testSchemaAt` then calls `.validate(...)` on it. So
// every one of the schemes below was a landmine: any text containing `file:`,
// `onenote:`, `thunderlink:` and the rest crashed the viewer, and the card holding
// it could not be opened again.
//
// Each scheme now carries a validate of its own, which is also what the alias was
// only ever standing in for. It matches the URL after the scheme - an optional
// `//`, then everything up to whitespace or a bracket - and hands back trailing
// sentence punctuation, so "see file://server/share/doc.xlsm." does not swallow
// the full stop.
const SCHEME_TAIL_RE = /^\/\/[^\s<>"'`{}|\\^\[\]]+|^[^\s<>"'`{}|\\^\[\]]+/;
const TRAILING_PUNCTUATION_RE = /[.,;:!?)\]}>'"]+$/;
function validateSchemeTail(text, pos) {
const tail = String(text || '').slice(pos);
const match = SCHEME_TAIL_RE.exec(tail);
if (!match) return 0;
// A scheme with nothing after it ("file:" on its own) is not a link.
const link = match[0].replace(TRAILING_PUNCTUATION_RE, '');
return link.length;
}
// WHAT THIS DOES AND DOES NOT ACHIEVE, since the list above promises more than it
// delivers: registering a scheme here makes markdown-it RECOGNISE it as a link,
// and two later filters then decide whether the reader gets a clickable one.
// markdown-it's own validateLink refuses `file:` (along with javascript:,
// vbscript: and data:), and the viewer's DOMPurify allows only
// http/https/ftp/ftps/mailto/tel/callto/cid/xmpp hrefs (secureDOMPurify.js), so
// EVERY scheme in this list currently has its href removed before it reaches the
// page. They render as text, which is what they did before this fix as well - on
// the cards that could still be opened.
//
// Making them genuinely clickable means relaxing both filters, and each of these
// schemes launches a local application, so that is a security decision (the ask in
// #3218) and not something to slip in while fixing a crash. What is fixed here is
// that a card containing one can be OPENED.
//
// Exposed for tests/markdownCustomUrlSchemes.test.cjs, which renders real
// markdown through this configuration rather than reading it.
Markdown.validateSchemeTail = validateSchemeTail;
Markdown.customUrlSchemes = urlschemes;
// put all url schemes into the linkify configuration to automatically make it clickable
for(var i=0; i<urlschemes.length;i++){
Markdown.linkify.add(urlschemes[i]+":",'http:');
for (var i = 0; i < urlschemes.length; i++) {
Markdown.linkify.add(urlschemes[i] + ':', { validate: validateSchemeTail });
}
const emojiPlugin = markdownItEmoji.full || markdownItEmoji.default || markdownItEmoji;
@@ -281,8 +340,31 @@ Blaze.Template.registerHelper('markdown', new Template('markdown', function () {
// Prevent hiding info: https://wekan.github.io/hall-of-fame/invisiblebleed/
// If text does not have hidden markdown link, render all markdown.
// Also show html comments.
const renderedMarkdown = Markdown.render(text).replace('<!--', '<font color="red" title="Warning! Hidden HTML comment!" aria-label="Warning! Hidden HTML comment!">&lt;!--</font>').replace('-->', '<font color="red" title="Warning! Hidden HTML comment!" aria-label="Warning! Hidden HTML comment!">--&gt;</font>');
const sanitized = DOMPurify.sanitize(renderedMarkdown, getSecureDOMPurifyConfig());
//
// #6588: NOTHING a card contains may make that card impossible to open. A
// renderer that throws here throws inside a Blaze view, Blaze swallows the
// exception, and the details panel simply never mounts - the reporter saw a
// card that played the open animation and then showed nothing, with no error
// anywhere until they overrode Meteor._debug. One malformed link brought down
// the whole view.
//
// The linkify crash that caused it is fixed above, but the guarantee is worth
// more than that one fix: a plugin, a pathological formula or the next
// markdown-it upgrade can all throw. The content is then shown as escaped
// plain text, which is exactly what the "show code as plain text" branch above
// renders - unformatted, but readable, and the card opens.
let sanitized;
try {
const renderedMarkdown = Markdown.render(text).replace('<!--', '<font color="red" title="Warning! Hidden HTML comment!" aria-label="Warning! Hidden HTML comment!">&lt;!--</font>').replace('-->', '<font color="red" title="Warning! Hidden HTML comment!" aria-label="Warning! Hidden HTML comment!">--&gt;</font>');
sanitized = DOMPurify.sanitize(renderedMarkdown, getSecureDOMPurifyConfig());
} catch (error) {
const message = (error && error.message) ? error.message : String(error);
// eslint-disable-next-line no-console
console.error('[markdown] rendering failed, showing the text as-is:', message);
const title = 'This text could not be formatted, so it is shown as it was written';
sanitized = '<pre title="' + title + '" aria-label="' + title + '">'
+ DOMPurify.sanitize(escapeHtmlSource(text), getSecureDOMPurifyConfig()) + '</pre>';
}
return HTML.Raw(sanitized);
}
}));
+197
View File
@@ -0,0 +1,197 @@
'use strict';
// wekan/wekan#6588: "Card cannot be opened when a comment/description contains a
// `file://` link". Clicking the minicard played the open animation and the details
// panel never mounted; Blaze swallowed the exception, so there was no visible
// error at all until the reporter overrode Meteor._debug and caught:
//
// TypeError: this.__schemas__[t.toLowerCase()].validate is not a function
// testSchemaAt / matchAtStart / markdown-it (tokenize -> parse -> render)
//
// Run: node tests/markdownCustomUrlSchemes.test.cjs
//
// WeKan registers eight custom URL schemes so links like `thunderlink:` and
// `file://` become clickable (packages/markdown/src/template-integration.js). It
// registered them the linkify-it 4/5 way:
//
// Markdown.linkify.add(scheme + ':', 'http:') // "behave like http:"
//
// linkify-it 6 removed string aliases and builds the definition by SPREADING it:
// `{ normalize: ..., ...'http:' }` is `{0:'h',1:'t',2:'t',3:'p',4:':'}`, an entry
// with no `validate` at all - and testSchemaAt calls `.validate(...)` on it. Every
// one of those eight schemes was therefore a landmine in any card's text.
//
// This suite runs the REAL linkify-it and markdown-it from node_modules, with the
// same registration the package performs, so it reproduces the crash against the
// shipped dependency rather than against a description of it.
const assert = require('assert');
const fs = require('fs');
const path = require('path');
const vm = require('vm');
const repoRoot = path.resolve(__dirname, '..');
const source = fs.readFileSync(
path.join(repoRoot, 'packages/markdown/src/template-integration.js'), 'utf8');
let passed = 0;
const queued = [];
function test(name, fn) { queued.push([name, fn]); }
console.log('markdownCustomUrlSchemes:');
// The scheme list and the validator, taken out of the package source (which
// imports Meteor and cannot be required here) and run for real.
function extract(name) {
const start = source.indexOf(`function ${name}(`);
assert.notStrictEqual(start, -1, `${name} is gone from the markdown package`);
const open = source.indexOf('{', start);
let depth = 0;
let end = -1;
for (let i = open; i < source.length; i += 1) {
if (source[i] === '{') depth += 1;
else if (source[i] === '}') { depth -= 1; if (depth === 0) { end = i + 1; break; } }
}
const preamble = source.slice(source.indexOf('const SCHEME_TAIL_RE'), start);
const ctx = { module: { exports: {} } };
vm.createContext(ctx);
vm.runInContext(`${preamble}\n${source.slice(start, end)}\nmodule.exports = ${name};`, ctx);
return ctx.module.exports;
}
const validateSchemeTail = extract('validateSchemeTail');
function schemesFromSource() {
const start = source.indexOf('var urlschemes = [');
const end = source.indexOf('];', start);
assert.ok(start !== -1 && end !== -1, 'the scheme list is gone');
return source.slice(start, end)
.match(/"[a-z]+"/g)
.map(s => s.replace(/"/g, ''));
}
const SCHEMES = schemesFromSource();
async function linkifyLike(register) {
const { LinkifyIt } = await import('linkify-it');
const linkify = new LinkifyIt();
register(linkify);
return linkify;
}
test('the shipped linkify-it really does crash on the old alias registration', async () => {
// The bug, reproduced against the dependency that is installed: if this ever
// stops throwing, linkify-it has changed again and the fix below can be
// revisited - but until then, the alias form is not an option.
const linkify = await linkifyLike(l => l.add('file:', 'http:'));
assert.deepStrictEqual(Object.keys(linkify.__schemas__['file:']).sort(),
['0', '1', '2', '3', '4', 'normalize'],
'the string is spread into indexed properties, so there is no validate');
assert.throws(() => linkify.test('see file://server/share/doc.xlsm'),
/validate is not a function/,
'and one such link in a card is what made the card impossible to open');
});
test('every scheme WeKan registers has a working validate now', async () => {
const linkify = await linkifyLike(l => {
for (const scheme of SCHEMES) l.add(scheme + ':', { validate: validateSchemeTail });
});
for (const scheme of SCHEMES) {
const entry = linkify.__schemas__[scheme + ':'];
assert.strictEqual(typeof entry.validate, 'function', `${scheme}: has no validate`);
}
assert.strictEqual(SCHEMES.length, 8, 'the eight schemes are still registered');
assert.ok(SCHEMES.includes('file'), 'including the one from the report');
});
test('the reported text renders instead of throwing', async () => {
const linkify = await linkifyLike(l => {
for (const scheme of SCHEMES) l.add(scheme + ':', { validate: validateSchemeTail });
});
for (const text of [
'see file://server/share/doc.xlsm',
'<file://server/share/doc.xlsm>',
'thunderlink:message-id-123 and onenote:https://example/x',
'file:', // a scheme with nothing after it
'mailspring:compose?to=someone@example.org',
]) {
assert.doesNotThrow(() => linkify.test(text), `threw on: ${text}`);
assert.doesNotThrow(() => linkify.match(text), `threw matching: ${text}`);
}
const matches = linkify.match('see file://server/share/doc.xlsm');
assert.ok(matches && matches.length === 1, 'and the link is still recognised as a link');
assert.strictEqual(matches[0].url, 'file://server/share/doc.xlsm');
});
test('markdown-it renders a card containing such a link, end to end', async () => {
const { default: MarkdownIt } = await import('markdown-it');
const md = new MarkdownIt({ html: true, linkify: true, typographer: true, breaks: true });
for (const scheme of SCHEMES) md.linkify.add(scheme + ':', { validate: validateSchemeTail });
const rendered = md.render('Network path: file://server/share/doc.xlsm\n\nAnd a normal https://wekan.fi link.');
assert.match(rendered, /file:\/\/server\/share\/doc\.xlsm/,
'the text is still there - which is the whole point: the card renders');
assert.match(rendered, /href="https:\/\/wekan\.fi"/, 'and ordinary links still work');
});
test('a file: link renders as TEXT, and that is the honest outcome (negative)', async () => {
// Registering the scheme makes markdown-it RECOGNISE it. Two filters then decide
// whether a reader gets a clickable link, and both refuse: markdown-it's own
// validateLink blocks file: (with javascript:, vbscript: and data:), and the
// viewer's DOMPurify allows only http/https/ftp/ftps/mailto/tel/callto/cid/xmpp
// hrefs. So this list has never produced a clickable link for any of its
// schemes - its only observable effect was the crash. Making them clickable
// means relaxing both, for schemes that launch local applications, which is a
// security decision (#3218) rather than part of a crash fix.
const { default: MarkdownIt } = await import('markdown-it');
const md = new MarkdownIt({ linkify: true });
for (const scheme of SCHEMES) md.linkify.add(scheme + ':', { validate: validateSchemeTail });
const rendered = md.render('file://server/share/doc.xlsm');
assert.ok(!/href="file:/.test(rendered),
'markdown-it refuses a file: href, and this test is here so that the day it '
+ 'changes is a decision rather than a surprise');
const uriAllowlist = fs.readFileSync(
path.join(repoRoot, 'packages/markdown/src/secureDOMPurify.js'), 'utf8');
assert.match(uriAllowlist, /ALLOWED_URI_REGEXP/,
'and the viewer keeps its own allowlist, which is the second refusal');
});
test('trailing punctuation stays in the sentence, not in the link', () => {
// linkify calls validate with the position AFTER the scheme, so pos is 5 for
// "file:" - the length it returns is the length of the link that follows.
const after = 'file:'.length;
assert.strictEqual(validateSchemeTail('file://a/b.xlsm.', after), '//a/b.xlsm'.length,
'a full stop after a link is the end of the sentence');
assert.strictEqual(validateSchemeTail('file://a/b)', after), '//a/b'.length);
assert.strictEqual(validateSchemeTail('file://a/b, and more', after), '//a/b'.length);
});
test('a scheme with nothing usable after it is not a link (negative)', () => {
assert.strictEqual(validateSchemeTail('', 0), 0);
assert.strictEqual(validateSchemeTail('file:', 5), 0, 'nothing after the colon');
assert.strictEqual(validateSchemeTail('file: some text', 5), 0, 'a space right after it');
assert.strictEqual(validateSchemeTail(null, 0), 0, 'junk in, zero out - never a throw');
assert.strictEqual(validateSchemeTail(undefined, 0), 0);
});
test('a link with no // still counts, which is what thunderlink: looks like', () => {
assert.strictEqual(validateSchemeTail('thunderlink:msgid@example', 12), 'msgid@example'.length);
assert.ok(validateSchemeTail('conisio://open/doc', 8) > 0);
});
test('the viewer never lets a render failure close the card (negative)', () => {
// The fix above removes the known crash; this keeps the guarantee when the next
// one arrives from a plugin, a formula or an upgrade.
const helper = source.slice(source.indexOf("registerHelper('markdown'"));
assert.ok(/try \{/.test(helper) && /Markdown\.render\(text\)/.test(helper),
'the render has to be inside a try');
const catchBlock = helper.slice(helper.indexOf('} catch (error)'));
assert.ok(/escapeHtmlSource\(text\)/.test(catchBlock),
'and the fallback shows the text as written, rather than an empty card');
assert.ok(/DOMPurify\.sanitize/.test(catchBlock),
'still sanitized: a render that failed is not a reason to inject raw text');
assert.ok(!/return;\s*}/.test(catchBlock.slice(0, 200)),
'and it must RETURN something - returning nothing is the blank panel again');
});
(async () => {
for (const [name, fn] of queued) { await fn(); passed += 1; console.log(' ok -', name); }
console.log(`\nmarkdownCustomUrlSchemes: ${passed} tests passed`);
})().catch(e => { console.error(e); process.exit(1); });