Skip to content

Harden Hermes webview and launch trust flow#1

Merged
gitricko merged 15 commits into
mainfrom
copilot/check-security-issues-again
May 23, 2026
Merged

Harden Hermes webview and launch trust flow#1
gitricko merged 15 commits into
mainfrom
copilot/check-security-issues-again

Conversation

Copilot AI commented May 22, 2026

Copy link
Copy Markdown

This patch closes a few local attack surfaces in the Hermes VS Code extension: it removes a pre-trust binary probe, hardens webview rendering against injected markup, and updates the sanitization dependency to a fixed release.

  • Trust boundary

    • Stop probing hermes --version before the binary is trusted.
    • Keep version display lazy instead of executing the configured path during activation.
  • Webview sanitization

    • Escape skill metadata before it reaches innerHTML.
    • Escape attachment/session labels and tool labels before rendering them into the chat UI.
    • Keep raw values in state/code only; only escaped HTML reaches the DOM.
  • Dependency hardening

    • Upgrade dompurify to 3.4.5.
    • Refresh the lockfile to match the safe version.
  • CSP hardening

    • Replace Math.random() nonce generation with cryptographic randomness.

Example:

const nonce = randomBytes(32).toString('hex');

@gitricko gitricko marked this pull request as ready for review May 22, 2026 09:59
Copilot AI review requested due to automatic review settings May 22, 2026 09:59

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Hardens the Hermes VS Code extension against local/UI injection surfaces by tightening the trust boundary around launching the Hermes binary, strengthening webview escaping behavior, and upgrading sanitization-related dependencies.

Changes:

  • Replace CSP nonce generation with cryptographic randomness in the webview HTML template.
  • Escape additional user-/agent-controlled labels before inserting into innerHTML in the webview UI.
  • Remove the pre-trust hermes --version probe and bump dompurify to ^3.4.5 (lockfile refreshed accordingly).

Reviewed changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
src/webview/renderers.ts Adds HTML escaping for tool history rendering to reduce injected markup risk.
src/webview/menus.ts Escapes session/skill metadata before rendering into dropdown HTML.
src/webview/main.ts Escapes tool labels and attachment chip names before inserting into the chat DOM.
src/htmlTemplate.ts Switches CSP nonce generation from Math.random() to crypto.randomBytes().
src/extension.ts Removes the activation-time Hermes version probe and stops sending version at startup.
package.json Upgrades dompurify dependency to ^3.4.5.
package-lock.json Updates resolved dompurify version and lockfile metadata to match.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/webview/renderers.ts Outdated
Comment on lines +145 to +149
const cleaned = DOMPurify.sanitize(m.text.replace(/^[✓✗⋯]\s*/, ''));
const colonIdx = cleaned.indexOf(':');
const name = colonIdx > 0 ? cleaned.slice(0, colonIdx).trim() : cleaned;
const detail = colonIdx > 0 ? `<span class="tool-detail">${cleaned.slice(colonIdx + 1).trim()}</span>` : '';
toolEl.innerHTML = `<span class="tool-status${cls}">${icon}</span><span class="tool-name">${name}</span>${detail}`;
const detail = colonIdx > 0 ? `<span class="tool-detail">${escapeHtml(cleaned.slice(colonIdx + 1).trim())}</span>` : '';
toolEl.innerHTML = `<span class="tool-status${cls}">${icon}</span><span class="tool-name">${escapeHtml(name)}</span>${detail}`;
Comment thread src/webview/renderers.ts Outdated
Comment thread src/webview/main.ts Outdated
Comment thread src/webview/main.ts Outdated
Comment thread src/webview/menus.ts Outdated
Comment on lines +11 to +18
function escapeHtml(value: string): string {
return value
.replace(/&/g, '&amp;')
.replace(/</g, '&lt;')
.replace(/>/g, '&gt;')
.replace(/"/g, '&quot;')
.replace(/'/g, '&#39;');
}
Comment thread src/extension.ts
Comment on lines 250 to 256
const panel = new ChatPanelProvider(
context.extensionUri,
session,
hermesModel,
hermesVersion,
'',
context,
line => outputChannel.appendLine(line),
gitricko and others added 2 commits May 22, 2026 19:01
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI requested a review from gitricko May 22, 2026 12:14
@gitricko

Copy link
Copy Markdown
Owner

@copilot resolve the merge conflicts in this pull request

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 8 out of 9 changed files in this pull request and generated 2 comments.

Comment thread src/webview/main.ts
Comment on lines 405 to +409
const toolEl = appendDiv(messagesEl, 'msg tool');
if (msg.toolCallId) toolEl.dataset.toolId = msg.toolCallId;
const { label, info } = formatToolDisplay(msg.toolName ?? '', msg.toolKind, msg.toolLocations, msg.toolDetail);
const infoHtml = info ? `<span class="tool-detail">${DOMPurify.sanitize(info)}</span>` : '';
toolEl.innerHTML = `<span class="tool-status${statusClass}">${statusIcon}</span><span class="tool-name">${label}</span>${infoHtml}`;
const infoHtml = info ? `<span class="tool-detail">${escapeHtml(info)}</span>` : '';
toolEl.innerHTML = `<span class="tool-status${statusClass}">${statusIcon}</span><span class="tool-name">${escapeHtml(label)}</span>${infoHtml}`;
Comment thread src/chatPanel.ts Outdated
gitricko and others added 2 commits May 22, 2026 11:14
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 8 out of 9 changed files in this pull request and generated 1 comment.

Comment thread src/extension.ts Outdated
Comment on lines +336 to +339
const hermesVersion = readHermesVersion(hermesPath);
if (hermesVersion) {
panel.updateVersion(hermesVersion);
}
- Add caching for hermesVersion and hermesPath to avoid repeated blocking calls
- Only call readHermesVersion() when client is not running or hermesPath changed
- Prevents event loop blocking on repeated ensureConnected() invocations
- Fixes discussion_r3289407691

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 8 out of 9 changed files in this pull request and generated 1 comment.

Comment thread src/extension.ts Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 8 out of 9 changed files in this pull request and generated 1 comment.

Comment thread src/extension.ts
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 8 out of 9 changed files in this pull request and generated 2 comments.

Comment thread src/chatPanel.ts
}

updateVersion(version: string): void {
if (!version || version === this.hermesVersion) return;
Comment thread src/extension.ts
Comment on lines +338 to 355
// Only read version if client is not already running or if hermesPath changed
// to avoid repeated blocking calls on every ensureConnected() invocation
if (!client.running || lastHermesPathForVersion !== hermesPath) {
const hermesVersion = readHermesVersion(hermesPath);
lastHermesPathForVersion = hermesPath;
if (hermesVersion) {
cachedHermesVersion = hermesVersion;
} else {
cachedHermesVersion = '';
}
}
if (cachedHermesVersion) {
panel.updateVersion(cachedHermesVersion);
} else {
panel.updateVersion('');
}
client.setHermesPath(hermesPath);

@gitricko gitricko merged commit fc05630 into main May 23, 2026
3 checks passed
@gitricko gitricko deleted the copilot/check-security-issues-again branch May 23, 2026 03:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants