Skip to content

security: enforce renderer CSP and externalize theme bootstrap #172

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
Mehdi-Bl merged 3 commits into from
Feb 19, 2026
Merged

security: enforce renderer CSP and externalize theme bootstrap #172

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
471c2c7
security: enforce renderer CSP and externalize theme bootstrap
Mehdi-Bl Feb 19, 2026
b87630a
test: make QA screenshot CSP-safe and cover theme bootstrap
Mehdi-Bl Feb 19, 2026
af67960
test: include renderer public scripts in coverage collection
Mehdi-Bl Feb 19, 2026
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
13 changes: 13 additions & 0 deletions docs/DEVELOPMENT.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,19 @@ make mend-scan
- `RENOVATE_TOKEN_FILE`
- `gh auth token` (GitHub CLI fallback)

## Renderer CSP Policy

- The renderer HTML (`src/renderer/public/index.html`) defines a strict CSP via
`<meta http-equiv="Content-Security-Policy" ...>`.
- Policy baseline:
- `script-src 'self'`
- `style-src 'self'`
- `object-src 'none'`
- `base-uri 'none'`
- No `'unsafe-inline'` or `'unsafe-eval'` exceptions are allowed.
- Dark mode bootstrap is intentionally externalized to `src/renderer/public/theme-bootstrap.js`
so startup theme logic works without inline scripts.

## Manual Setup (Without Make)

```bash
Expand Down
1 change: 1 addition & 0 deletions jest.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ module.exports = {
coverageDirectory: '<rootDir>/coverage',
collectCoverageFrom: [
'<rootDir>/src/renderer/components/**/*.{js,jsx,ts,tsx}',
'<rootDir>/src/renderer/public/**/*.js',
'<rootDir>/src/main/index.ts',
'<rootDir>/src/main/preload.ts',
'<rootDir>/src/main/security/navigation-guard.ts',
Expand Down
53 changes: 43 additions & 10 deletions scripts/capture-ui-screenshot.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,15 @@ const DEFAULT_SCREENSHOT_NAME = `ui-${process.platform}-${process.arch}.png`;
const DEFAULT_LOCALE = 'en';
const SUPPORTED_LOCALES = ['en', 'es', 'fr', 'de'];
const FIXED_MTIME = 1700000000000;
const QA_DISABLE_ANIMATIONS_STYLESHEET_PATH = '/__qa__/disable-animations.css';
const QA_DISABLE_ANIMATIONS_STYLESHEET_CONTENT = `
*, *::before, *::after {
transition-duration: 0s !important;
animation-duration: 0s !important;
animation-delay: 0s !important;
scroll-behavior: auto !important;
}
`;

function loadSecretScannerHelpers() {
const compiledSecretScannerPath = path.join(
Expand Down Expand Up @@ -133,6 +142,22 @@ function resolveFilePath(requestUrl) {

function createStaticServer() {
return http.createServer((request, response) => {
let requestPath;

try {
requestPath = decodeURIComponent((request.url || '/').split('?')[0]);
} catch {
response.writeHead(400, { 'Content-Type': 'text/plain; charset=UTF-8' });
response.end('Bad Request');
return;
}

if (requestPath === QA_DISABLE_ANIMATIONS_STYLESHEET_PATH) {
response.writeHead(200, { 'Content-Type': 'text/css; charset=UTF-8' });
response.end(QA_DISABLE_ANIMATIONS_STYLESHEET_CONTENT);
return;
}

const requestedPath = resolveFilePath(request.url || '/');

if (!requestedPath) {
Expand Down Expand Up @@ -575,16 +600,24 @@ async function captureLocaleScreenshots(page) {

async function captureAppStateScreenshots(page) {
await runStep('Disable animations for stable screenshots', async () => {
await page.addStyleTag({
content: `
*, *::before, *::after {
transition-duration: 0s !important;
animation-duration: 0s !important;
animation-delay: 0s !important;
scroll-behavior: auto !important;
}
`,
});
await page.evaluate(async (stylesheetHref) => {
const existingStylesheet = document.querySelector('link[data-qa-disable-animations="true"]');
if (existingStylesheet instanceof HTMLLinkElement) {
return;
}

await new Promise((resolve, reject) => {
const stylesheetLink = document.createElement('link');
stylesheetLink.rel = 'stylesheet';
stylesheetLink.href = stylesheetHref;
stylesheetLink.setAttribute('data-qa-disable-animations', 'true');
stylesheetLink.onload = resolve;
stylesheetLink.onerror = () => {
reject(new Error('Failed to load QA disable-animations stylesheet'));
};
document.head.appendChild(stylesheetLink);
});
}, QA_DISABLE_ANIMATIONS_STYLESHEET_PATH);
});

await runStep('Wait for app root', async () => {
Expand Down
20 changes: 5 additions & 15 deletions src/renderer/public/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,23 +3,13 @@
<head>
<meta charset="UTF-8" />
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
<meta
http-equiv="Content-Security-Policy"
content="default-src 'self'; script-src 'self'; style-src 'self'; img-src 'self' data: assets:; font-src 'self' data:; connect-src 'self' https: http:; object-src 'none'; base-uri 'none'; frame-ancestors 'none'; form-action 'none'"

@gemini-code-assist gemini-code-assist Bot Feb 19, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

security-medium medium

The connect-src directive in the Content Security Policy is overly permissive, allowing connections over https: and, more critically, http:. This constitutes a medium-severity vulnerability (Insecure Communication) as allowing http: undermines HTTPS security, making the application vulnerable to man-in-the-middle attacks and data exfiltration. It's strongly recommended to restrict this directive to 'self' and only specific, trusted domains, prioritizing https: for all external connections.

Suggested change
content="default-src 'self'; script-src 'self'; style-src 'self'; img-src 'self' data: assets:; font-src 'self' data:; connect-src 'self' https: http:; object-src 'none'; base-uri 'none'; frame-ancestors 'none'; form-action 'none'"
content="default-src 'self'; script-src 'self'; style-src 'self'; img-src 'self' data: assets:; font-src 'self' data:; connect-src 'self'; object-src 'none'; base-uri 'none'; frame-ancestors 'none'; form-action 'none'"

@gemini-code-assist gemini-code-assist Bot Feb 19, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The img-src directive includes assets:, which is not a standard CSP source keyword. If this is intended to allow images from a local assets directory, the 'self' keyword should already cover this. If assets: refers to a custom protocol scheme (e.g., in Electron), it would be beneficial to add a comment for clarity. If it's a typo or not needed, it should be removed to keep the policy clean.

Suggested change
content="default-src 'self'; script-src 'self'; style-src 'self'; img-src 'self' data: assets:; font-src 'self' data:; connect-src 'self' https: http:; object-src 'none'; base-uri 'none'; frame-ancestors 'none'; form-action 'none'"
content="default-src 'self'; script-src 'self'; style-src 'self'; img-src 'self' data:; font-src 'self' data:; connect-src 'self' https: http:; object-src 'none'; base-uri 'none'; frame-ancestors 'none'; form-action 'none'"

/>
Comment on lines +6 to +9

@qodo-free-for-open-source-projects qodo-free-for-open-source-projects Bot Feb 19, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

2. Frame-ancestors ignored in meta csp 🐞 Bug ⛨ Security

The CSP specification explicitly prohibits frame-ancestors inside a `<meta
http-equiv='Content-Security-Policy'>` tag — browsers and Electron's Chromium engine are required to
silently ignore it. Since Electron loads the renderer via loadFile() (file:// protocol) with no
HTTP response headers and no onHeadersReceived hook, the intended clickjacking protection is
completely non-operational.
Agent Prompt 
## Issue description
The `frame-ancestors 'none'` CSP directive is placed inside a `<meta http-equiv='Content-Security-Policy'>` tag. Per the CSP specification, this directive MUST be ignored when delivered via a meta tag — it is only effective via HTTP response headers. Electron loads the renderer using `loadFile()` (file:// protocol) with no HTTP headers, so the protection is completely non-functional.

## Issue Context
The directive appears in the meta CSP tag in `src/renderer/public/index.html`. The main process in `src/main/index.ts` uses `mainWindow.loadFile()` and has no `session.webRequest.onHeadersReceived` hook to inject HTTP-level CSP headers.

## Fix Focus Areas
- `src/renderer/public/index.html[8-8]` — Remove `frame-ancestors 'none'` from the `content` attribute of the meta CSP tag to avoid false security confidence
- `src/main/index.ts[264-310]` — Optionally add a `session.defaultSession.webRequest.onHeadersReceived` handler in `bootstrapApp()` to inject `Content-Security-Policy: frame-ancestors 'none'` as a real HTTP response header if framing protection is a genuine requirement

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

<title>AI Code Fusion</title>
<link rel="stylesheet" href="../../../dist/renderer/output.css" />
<script>
// Apply dark mode immediately to prevent flash of light theme
const appWindow = globalThis;
const savedMode = localStorage.getItem('darkMode');
const prefersDark =
typeof appWindow.matchMedia === 'function' &&
appWindow.matchMedia('(prefers-color-scheme: dark)').matches;
const shouldEnableDarkMode = savedMode === 'true' || (savedMode === null && prefersDark);

if (shouldEnableDarkMode) {
document.documentElement.classList.add('dark');
} else {
document.documentElement.classList.remove('dark');
}
</script>
<script src="./theme-bootstrap.js"></script>

@qodo-free-for-open-source-projects qodo-free-for-open-source-projects Bot Feb 19, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

1. qa:screenshot not run for ui change 📘 Rule violation ✓ Correctness

The PR modifies startup UI rendering behavior by replacing the inline dark-mode bootstrap script
with an external theme-bootstrap.js file, which qualifies as a UI behavior change. The PR
description's validation section omits npm run qa:screenshot, violating the requirement to run
screenshot QA whenever UI behavior or layout changes are introduced.
Agent Prompt 
## Issue description
The PR modifies startup UI rendering behavior — the inline dark-mode bootstrap script has been externalized to `theme-bootstrap.js` — but `npm run qa:screenshot` was not executed or reported in the PR validation steps.

## Issue Context
Compliance rule ID 4 requires the screenshot QA gate (`npm run qa:screenshot`) to pass whenever UI behavior or layout changes are introduced. The dark-mode bootstrap change affects the initial visual state of the application (prevents flash of light theme), which qualifies as a UI behavior change. The catalog at `tests/catalog.md` line 18 also lists `npm run qa:screenshot` as the designated UI screenshot gate command.

## Fix Focus Areas
- Run `npm run qa:screenshot` locally and confirm zero failures
- Add `npm run qa:screenshot` to the PR description validation section
- src/renderer/public/index.html[12-12]
- src/renderer/public/theme-bootstrap.js[1-22]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

</head>
<body
class="h-screen overflow-hidden bg-gray-100 dark:bg-gray-900 transition-colors duration-200"
Expand Down
22 changes: 22 additions & 0 deletions src/renderer/public/theme-bootstrap.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
(function applyInitialTheme() {
try {
// Apply dark mode before React mounts to avoid a light-theme flash.
const appWindow = globalThis;
const savedMode = localStorage.getItem('darkMode');
const prefersDark =
typeof appWindow.matchMedia === 'function' &&
appWindow.matchMedia('(prefers-color-scheme: dark)').matches;
const shouldEnableDarkMode = savedMode === 'true' || (savedMode === null && prefersDark);

if (shouldEnableDarkMode) {
document.documentElement.classList.add('dark');
return;
}

document.documentElement.classList.remove('dark');
} catch (error) {
// Fall back to light mode if storage/media APIs are unavailable.
console.warn('Theme bootstrap failed', error);
document.documentElement.classList.remove('dark');
}
})();
2 changes: 2 additions & 0 deletions tests/catalog.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ Purpose: quick map of what is covered, why it exists, and which command to run.
| `tests/unit/components/file-tree.test.tsx` | `src/renderer/components/FileTree.tsx` | Tree render, folder expand/collapse, select all, empty-state behavior |
| `tests/unit/components/language-selector.test.tsx` | `src/renderer/components/LanguageSelector.tsx` | Locale selector rendering, language switching, and localStorage persistence |
| `tests/unit/components/source-tab.test.tsx` | `src/renderer/components/SourceTab.tsx` | Token-count loading state, stale async guard behavior, and metadata-driven cache recount validation |
| `tests/unit/renderer/theme-bootstrap.test.ts` | `src/renderer/public/theme-bootstrap.js` | Early theme bootstrap behavior across persisted mode, system preference fallback, and storage failure handling |
| `tests/unit/i18n/locales-parity.test.ts` | `src/renderer/i18n/locales/*/common.json` | Locale key parity across EN/ES/FR/DE resources |
| `tests/unit/file-analyzer.test.ts` | `src/utils/file-analyzer.ts` | Include/exclude rules, gitignore behavior, binary handling, error cases |
| `tests/unit/gitignore-parser.test.ts` | `src/utils/gitignore-parser.ts` | Pattern parsing, negation behavior, caching, nested path handling |
Expand Down Expand Up @@ -57,6 +58,7 @@ Purpose: quick map of what is covered, why it exists, and which command to run.
| `tests/unit/main/updater.test.ts` | `src/main/updater.ts` | Alpha/stable channel selection, platform gating, update-check result handling |
| `tests/unit/main/updater-smoke.test.ts` | `src/main/updater.ts` | Manual updater-check flow, stable-vs-alpha prerelease assertions, Linux-disabled guard, and structured updater check observability events |
| `tests/unit/main/feature-flags.test.ts` | `src/main/feature-flags.ts` | OpenFeature normalization, env/remote merge rules, secure remote fetch behavior |
| `tests/unit/main/csp-policy.test.ts` | `src/renderer/public/index.html` | CSP policy contract and no-inline-script enforcement for renderer bootstrap |
| `tests/unit/main/navigation-guard.test.ts` | `src/main/security/navigation-guard.ts` | External URL allowlist checks and in-app navigation allow/deny behavior |
| `tests/unit/main/path-security.test.ts` | `src/main/security/path-guard.ts` | Root-path authorization, temp-root boundaries, symlink-aware realpath resolution |
| `tests/unit/main/preload.test.ts` | `src/main/preload.ts` | Preload bridge external URL protocol guard for `shell.openExternal` |
Expand Down
35 changes: 35 additions & 0 deletions tests/unit/main/csp-policy.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import path from 'node:path';

const realFs = jest.requireActual('node:fs') as typeof import('node:fs');
const rendererIndexPath = path.resolve(__dirname, '../../../src/renderer/public/index.html');

const readRendererIndex = () => realFs.readFileSync(rendererIndexPath, 'utf8');

describe('renderer CSP policy', () => {
it('defines a strict CSP policy without unsafe script/style exceptions', () => {
const indexHtml = readRendererIndex();
const cspMatch = indexHtml.match(
/<meta\s+http-equiv="Content-Security-Policy"\s+content="([^"]+)"/
);

expect(cspMatch).not.toBeNull();
const cspValue = cspMatch?.[1] ?? '';

expect(cspValue).toContain("default-src 'self'");
expect(cspValue).toContain("script-src 'self'");
expect(cspValue).toContain("style-src 'self'");
expect(cspValue).toContain("object-src 'none'");
expect(cspValue).toContain("base-uri 'none'");
expect(cspValue).not.toContain("'unsafe-inline'");
expect(cspValue).not.toContain("'unsafe-eval'");
});
Comment on lines +9 to +25

@gemini-code-assist gemini-code-assist Bot Feb 19, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

This test is a great start for ensuring the CSP is enforced. To make it more robust and prevent accidental weakening of the policy in the future, consider using a snapshot test for the CSP value. This will ensure the entire policy is tracked and any changes must be explicitly reviewed and approved.

Suggested change
it('defines a strict CSP policy without unsafe script/style exceptions', () => {
const indexHtml = readRendererIndex();
const cspMatch = indexHtml.match(
/<meta\s+http-equiv="Content-Security-Policy"\s+content="([^"]+)"/
);
expect(cspMatch).not.toBeNull();
const cspValue = cspMatch?.[1] ?? '';
expect(cspValue).toContain("default-src 'self'");
expect(cspValue).toContain("script-src 'self'");
expect(cspValue).toContain("style-src 'self'");
expect(cspValue).toContain("object-src 'none'");
expect(cspValue).toContain("base-uri 'none'");
expect(cspValue).not.toContain("'unsafe-inline'");
expect(cspValue).not.toContain("'unsafe-eval'");
});
it('defines a strict CSP policy', () => {
const indexHtml = readRendererIndex();
const cspMatch = indexHtml.match(
/<meta\s+http-equiv="Content-Security-Policy"\s+content="([^"_]+)"/
);
expect(cspMatch).not.toBeNull();
const cspValue = cspMatch?.[1] ?? '';
// A snapshot test ensures the entire policy is tracked.
// Any change to the CSP will require the snapshot to be updated,
// making regressions easier to spot during code review.
expect(cspValue).toMatchSnapshot();
// It's still a good idea to keep explicit checks for critical items.
expect(cspValue).not.toContain("'unsafe-inline'");
expect(cspValue).not.toContain("'unsafe-eval'");
});


it('loads theme bootstrap from an external script instead of inline script tags', () => {
const indexHtml = readRendererIndex();

expect(indexHtml).toContain('<script src="./theme-bootstrap.js"></script>');

const inlineScriptTags = [...indexHtml.matchAll(/<script(?![^>]*\bsrc=)[^>]*>/g)];
expect(inlineScriptTags).toHaveLength(0);
});
});
69 changes: 69 additions & 0 deletions tests/unit/renderer/theme-bootstrap.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
const loadThemeBootstrapScript = () => {
jest.isolateModules(() => {
require('../../../src/renderer/public/theme-bootstrap.js');
});
};

const createMatchMediaMock = (matches) =>
jest.fn().mockImplementation(() => ({
matches,
media: '(prefers-color-scheme: dark)',
onchange: null,
addEventListener: jest.fn(),
removeEventListener: jest.fn(),
addListener: jest.fn(),
removeListener: jest.fn(),
dispatchEvent: jest.fn(),
}));

describe('theme-bootstrap script', () => {
let addSpy;
let removeSpy;
let matchMediaSpy;

beforeEach(() => {
jest.resetModules();
addSpy = jest.spyOn(document.documentElement.classList, 'add');
removeSpy = jest.spyOn(document.documentElement.classList, 'remove');
matchMediaSpy = jest.spyOn(window, 'matchMedia').mockImplementation(createMatchMediaMock(false));
window.localStorage.removeItem('darkMode');
});

afterEach(() => {
addSpy.mockRestore();
removeSpy.mockRestore();
matchMediaSpy.mockRestore();
jest.restoreAllMocks();
document.documentElement.classList.remove('dark');
});

it('enables dark class when persisted setting is true', () => {
window.localStorage.setItem('darkMode', 'true');

loadThemeBootstrapScript();

expect(addSpy).toHaveBeenCalledWith('dark');
expect(removeSpy).not.toHaveBeenCalled();
});

it('falls back to system preference when persisted value is absent', () => {
matchMediaSpy.mockImplementation(createMatchMediaMock(false));

loadThemeBootstrapScript();

expect(addSpy).not.toHaveBeenCalled();
expect(removeSpy).toHaveBeenCalledWith('dark');
});

it('keeps light mode when storage access throws', () => {
const warnSpy = jest.spyOn(console, 'warn');
jest.spyOn(Storage.prototype, 'getItem').mockImplementation(() => {
throw new Error('storage unavailable');
});

loadThemeBootstrapScript();

expect(removeSpy).toHaveBeenCalledWith('dark');
expect(warnSpy).toHaveBeenCalled();
});
});