diff --git a/src/sidebar/git-panel.js b/src/sidebar/git-panel.js index a5cc063..aa9d1d2 100644 --- a/src/sidebar/git-panel.js +++ b/src/sidebar/git-panel.js @@ -1,3 +1,15 @@ +// Escape repo-derived strings (branch/file names, commit messages, git stderr) before +// interpolating them into innerHTML. Quotes are included so attribute contexts +// (data-file, data-branch) cannot be broken out of. +function escapeHtml(str) { + return String(str) + .replace(/&/g, '&') + .replace(//g, '>') + .replace(/"/g, '"') + .replace(/'/g, '''); +} + function renderGitPanel( container, { gitStatus, gitDiff, gitStage, gitCommit, gitLog, gitBranches, gitCheckout, gitPush, gitPull } @@ -47,7 +59,7 @@ function renderGitPanel( if (!status || !changesEl) return; if (status.error) { - changesEl.innerHTML = `

${status.error}

`; + changesEl.innerHTML = `

${escapeHtml(status.error)}

`; return; } @@ -65,11 +77,11 @@ function renderGitPanel( changesEl.innerHTML = files .map( (f) => ` -
+
${f.status} - ${f.file} - - + ${escapeHtml(f.file)} + +
` ) @@ -101,8 +113,8 @@ function renderGitPanel( .map( (entry) => `
-
${entry.message}
-
${entry.date?.substring(0, 10) || ''} · ${entry.author_name || ''}
+
${escapeHtml(entry.message)}
+
${entry.date?.substring(0, 10) || ''} · ${escapeHtml(entry.author_name || '')}
` ) @@ -127,7 +139,7 @@ function renderGitPanel( if (!result) return; if (result.error) { - branchesEl.innerHTML = `

${result.error}

`; + branchesEl.innerHTML = `

${escapeHtml(result.error)}

`; return; } @@ -142,9 +154,9 @@ function renderGitPanel( branchesEl.innerHTML = names .map( (name) => ` -
- ${name === current ? '● ' : ''}${name} - ${name === current ? '' : ``} +
+ ${name === current ? '● ' : ''}${escapeHtml(name)} + ${name === current ? '' : ``}
` ) diff --git a/tests/git-panel.test.js b/tests/git-panel.test.js new file mode 100644 index 0000000..f62aeb9 --- /dev/null +++ b/tests/git-panel.test.js @@ -0,0 +1,146 @@ +/** + * Tests for the Git sidebar panel XSS hardening. + * renderGitPanel takes injected git operations (no electron mock needed); these tests + * verify that repository-derived strings (branch names, file names, commit messages, + * author names, error text) are HTML-escaped in both element and attribute contexts. + */ + +const { renderGitPanel } = require('../src/sidebar/git-panel'); + +const flush = () => new Promise((resolve) => setTimeout(resolve, 20)); + +const EMPTY_STATUS = { modified: [], not_added: [], created: [], deleted: [], staged: [] }; +const EMPTY_BRANCHES = () => ({ all: [], current: '' }); + +function makeOps(overrides = {}) { + return { + gitStatus: jest.fn().mockResolvedValue(EMPTY_STATUS), + gitDiff: jest.fn().mockResolvedValue(''), + gitStage: jest.fn().mockResolvedValue({}), + gitCommit: jest.fn().mockResolvedValue({}), + gitLog: jest.fn().mockResolvedValue({ all: [] }), + gitBranches: jest.fn().mockResolvedValue(EMPTY_BRANCHES()), + gitCheckout: jest.fn().mockResolvedValue({}), + gitPush: jest.fn().mockResolvedValue({}), + gitPull: jest.fn().mockResolvedValue({}), + ...overrides, + }; +} + +function mountPanel(ops) { + document.body.innerHTML = ''; + const container = document.createElement('div'); + document.body.appendChild(container); + renderGitPanel(container, ops); + return container; +} + +describe('Git panel XSS escaping', () => { + it('renders a hostile branch name as literal text with no img element in the DOM', async () => { + const payload = ''; + mountPanel( + makeOps({ + gitBranches: jest.fn().mockResolvedValue({ all: [payload, 'master'], current: 'master' }), + }) + ); + await flush(); + + const branchesEl = document.getElementById('git-branches'); + expect(branchesEl.querySelectorAll('img')).toHaveLength(0); + expect(branchesEl.textContent).toContain(payload); + const item = branchesEl.querySelector('[data-branch]'); + expect(item).not.toBeNull(); + expect(item.dataset.branch).toBe(payload); + }); + + it('keeps a hostile branch name inert in attribute and text contexts', async () => { + const payload = '" onclick="alert(1)" data-x="'; + mountPanel( + makeOps({ + gitBranches: jest.fn().mockResolvedValue({ all: [payload], current: '' }), + }) + ); + await flush(); + + const item = document.querySelector('.git-branch-item'); + expect(item.dataset.branch).toBe(payload); + expect(item.getAttribute('onclick')).toBeNull(); + expect(item.textContent).toContain(payload); + }); + + it('renders a commit message with an event-handler payload as inert text', async () => { + const ops = makeOps({ + gitLog: jest.fn().mockResolvedValue({ + all: [ + { + message: ' fix build', + author_name: 'Attacker ', + date: '2024-05-01T10:00:00', + }, + ], + }), + }); + mountPanel(ops); + await flush(); + + const logEl = document.getElementById('git-log'); + expect(logEl.querySelectorAll('img, script')).toHaveLength(0); + const msg = logEl.querySelector('.git-log-msg'); + expect(msg.textContent).toBe(' fix build'); + const meta = logEl.querySelector('.git-log-meta'); + expect(meta.textContent).toContain('Attacker '); + }); + + it('does not let a quoted file name break out of the data-file attribute', async () => { + const evilFile = 'notes" onmouseover="alert(1)" data-evil="x.md'; + mountPanel( + makeOps({ gitStatus: jest.fn().mockResolvedValue({ ...EMPTY_STATUS, modified: [evilFile] }) }) + ); + await flush(); + + const fileRow = document.querySelector('.git-file'); + expect(fileRow.getAttribute('data-file')).toBe(evilFile); + expect(fileRow.getAttribute('onmouseover')).toBeNull(); + expect(fileRow.getAttribute('data-evil')).toBeNull(); + expect(fileRow.textContent).toContain(evilFile); + const stageBtn = fileRow.querySelector('.git-stage-btn'); + expect(stageBtn.dataset.file).toBe(evilFile); + }); + + it('escapes HTML in a git status error message', async () => { + const error = 'fatal: not a git repository '; + mountPanel(makeOps({ gitStatus: jest.fn().mockResolvedValue({ error }) })); + await flush(); + + const changesEl = document.getElementById('git-changes'); + expect(changesEl.querySelectorAll('img')).toHaveLength(0); + expect(changesEl.querySelector('.git-info').textContent).toBe(error); + }); + + it('escapes HTML in a git branch listing error message', async () => { + const error = 'refs/heads/ is invalid'; + mountPanel(makeOps({ gitBranches: jest.fn().mockResolvedValue({ error }) })); + await flush(); + + const branchesEl = document.getElementById('git-branches'); + expect(branchesEl.querySelectorAll('script')).toHaveLength(0); + expect(branchesEl.querySelector('.git-info').textContent).toBe(error); + }); + + it('still renders benign branch names, files, and commits normally', async () => { + mountPanel( + makeOps({ + gitStatus: jest.fn().mockResolvedValue({ ...EMPTY_STATUS, modified: ['README.md'] }), + gitLog: jest.fn().mockResolvedValue({ + all: [{ message: 'initial commit', author_name: 'Dev', date: '2024-05-01T10:00:00' }], + }), + gitBranches: jest.fn().mockResolvedValue({ all: ['master'], current: 'master' }), + }) + ); + await flush(); + + expect(document.querySelector('.git-file-name').textContent).toBe('README.md'); + expect(document.querySelector('.git-log-msg').textContent).toBe('initial commit'); + expect(document.querySelector('.git-branch-name').textContent).toContain('master'); + }); +});