Skip to content

Escape git-installed plugin metadata in the Plugin Manager installed list #31

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

Merged
cchacs merged 1 commit into from
Oct 6, 2026
Merged

Escape git-installed plugin metadata in the Plugin Manager installed list #31

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
79 changes: 58 additions & 21 deletions src/renderer/plugin-manager/screen.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,55 @@
setTimeout(() => installMsg.classList.add('hidden'), 5000);
}

// A git-installed plugin's metadata comes from the plugin.json of whatever
// repository the user cloned, so the name and description are attacker-chosen
// text. They go in as textContent and the buttons carry their target in
// `dataset`, so nothing here ever becomes markup — this renderer runs with
// webSecurity:false, so a stray `<` would execute rather than being inert.
function installedRow(plugin) {
const div = document.createElement('div');
div.className = 'flex items-center gap-3 p-3 rounded bg-slate-800/50 border border-slate-700';

const meta = document.createElement('div');
meta.className = 'flex-1';

const name = document.createElement('div');
name.className = 'text-sm font-medium text-slate-200';
name.textContent = plugin.manifest?.name || plugin.name;

const desc = document.createElement('div');
desc.className = 'text-xs text-slate-400';
desc.textContent = plugin.manifest?.description || '';

const version = document.createElement('div');
version.className = 'text-xs text-slate-500 mt-0.5';
version.textContent = 'v' + (plugin.version || 'unknown');

meta.appendChild(name);
meta.appendChild(desc);
meta.appendChild(version);
div.appendChild(meta);

const actions = document.createElement('div');
actions.className = 'flex gap-2';
if (plugin.hasGit) {
actions.appendChild(installedAction('pm-update text-xs px-2 py-1 rounded bg-blue-600 hover:bg-blue-500', plugin.name, 'Update'));
}
actions.appendChild(installedAction('pm-remove text-xs px-2 py-1 rounded bg-red-600/50 hover:bg-red-500', plugin.name, 'Remove'));
div.appendChild(actions);

return div;
}

function installedAction(className, name, label) {
const button = document.createElement('button');
button.type = 'button';
button.className = className;
button.textContent = label;
button.dataset.name = name;
return button;
}

async function refreshList() {
listContainer.innerHTML = '<div class="text-sm text-slate-500">Loading...</div>';

Expand All @@ -42,25 +91,7 @@

listContainer.innerHTML = '';
for (const plugin of installed) {
const div = document.createElement('div');
div.className = 'flex items-center gap-3 p-3 rounded bg-slate-800/50 border border-slate-700';

const name = plugin.manifest?.name || plugin.name;
const desc = plugin.manifest?.description || '';
const version = plugin.version || 'unknown';

div.innerHTML = `
<div class="flex-1">
<div class="text-sm font-medium text-slate-200">${name}</div>
<div class="text-xs text-slate-400">${desc}</div>
<div class="text-xs text-slate-500 mt-0.5">v${version}</div>
</div>
<div class="flex gap-2">
${plugin.hasGit ? `<button class="pm-update text-xs px-2 py-1 rounded bg-blue-600 hover:bg-blue-500" data-name="${plugin.name}">Update</button>` : ''}
<button class="pm-remove text-xs px-2 py-1 rounded bg-red-600/50 hover:bg-red-500" data-name="${plugin.name}">Remove</button>
</div>
`;
listContainer.appendChild(div);
listContainer.appendChild(installedRow(plugin));
}

// Bind update buttons
Expand All @@ -85,7 +116,13 @@
});
});
} catch (e) {
listContainer.innerHTML = `<div class="text-sm text-red-400">Error loading plugins: ${e.message}</div>`;
const error = document.createElement('div');
error.className = 'text-sm text-red-400';
// The message comes back across the bridge from main, so it is text
// rather than markup here too.
error.textContent = 'Error loading plugins: ' + (e?.message || e);
listContainer.innerHTML = '';
listContainer.appendChild(error);
}
}

Expand Down Expand Up @@ -237,7 +274,7 @@
updateInstallLabel();
await updateCheckBadge();
} catch (e) {
catalogBox.innerHTML = `<div class="text-sm text-red-400">Error loading the plugin catalog: ${esc(e.message)}</div>`;
catalogBox.innerHTML = `<div class="text-sm text-red-400">Error loading the plugin catalog: ${esc(e?.message || e)}</div>`;

Check warning on line 277 in src/renderer/plugin-manager/screen.js

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

src/renderer/plugin-manager/screen.js#L277 

Unsafe assignment to innerHTML

Check failure on line 277 in src/renderer/plugin-manager/screen.js

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

src/renderer/plugin-manager/screen.js#L277 

User controlled data in a `catalogBox.innerHTML` is an anti-pattern that can lead to XSS vulnerabilities

Check failure on line 277 in src/renderer/plugin-manager/screen.js

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

src/renderer/plugin-manager/screen.js#L277 

User controlled data in methods like `innerHTML`, `outerHTML` or `document.write` is an anti-pattern that can lead to XSS vulnerabilities
}
}

Expand Down
12 changes: 9 additions & 3 deletions src/renderer/screen.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1456,11 +1456,14 @@ window.__feedBackDesktopAudioHooks = window.__feedBackDesktopAudioHooks || {};
const color = typeColors[slot.type] || 'slate';
const div = document.createElement('div');
div.className = `flex items-center gap-3 p-3 rounded bg-slate-800/50 border border-${color}-500/30`;
// The slot name is the user's VST/NAM/IR file name, so escape it
// before it becomes markup. The slot id comes from the addon's own
// counter rather than off disk, so the inline handlers keep it bare.
div.innerHTML = `
<span class="text-xs font-medium px-2 py-0.5 rounded bg-${color}-500/20 text-${color}-400">
${typeNames[slot.type] || '?'}
</span>
<span class="flex-1 text-sm ${slot.bypassed ? 'line-through text-slate-500' : 'text-slate-200'}">${slot.name}</span>
<span class="flex-1 text-sm ${slot.bypassed ? 'line-through text-slate-500' : 'text-slate-200'}">${escHtml(slot.name)}</span>
${slot.hasEditor ? `<button class="text-xs px-2 py-1 rounded bg-blue-600/50 hover:bg-blue-500"
onclick="_aeOpenEditor(${slot.id})">Edit</button>` : ''}
<button class="text-xs px-2 py-1 rounded ${slot.bypassed ? 'bg-yellow-600' : 'bg-slate-600'} hover:opacity-80"
Expand Down Expand Up @@ -1520,10 +1523,13 @@ window.__feedBackDesktopAudioHooks = window.__feedBackDesktopAudioHooks || {};
for (const plugin of filtered) {
const div = document.createElement('div');
div.className = 'flex items-center gap-3 p-2 rounded hover:bg-slate-700/50 cursor-pointer';
// Name, manufacturer and format come from the scanned bundle on disk,
// so escape them: this window runs with webSecurity:false, which leaves
// no CSP to make a stray < inert.
div.innerHTML = `
<div class="flex-1">
<div class="text-sm text-slate-200">${plugin.name}</div>
<div class="text-xs text-slate-400">${plugin.manufacturer} · ${plugin.format}</div>
<div class="text-sm text-slate-200">${escHtml(plugin.name)}</div>
<div class="text-xs text-slate-400">${escHtml(plugin.manufacturer)} · ${escHtml(plugin.format)}</div>
</div>
`;
div.addEventListener('click', async () => {
Expand Down
Loading
Loading