-
Notifications
You must be signed in to change notification settings - Fork 237
feat: Add e2e security test suite for context isolation validation #9095
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.
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
Draft
vkuprin
wants to merge
25
commits into
dev
Choose a base branch
from
security/e2e-test-setup
base: dev
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from all commits
Commits
Show all changes
25 commits
Select commit
Hold shift + click to select a range
1f1e0e7
feat: add comprehensive e2e security test suite
vkuprin 253140e
fix: correct npm audit command in security workflow
vkuprin b8fe3b7
fix: remove continue-on-error to properly fail CI on test failures
vkuprin bcdd1c9
fix: make regression tests compatible with headless mode
vkuprin f841691
fix: make validation tests compatible with headless mode
vkuprin 17a8fa6
fix: remove problematic security audit step from workflow
vkuprin 8df1ad6
fix: completely remove security audit job from workflow
vkuprin 2aa96bd
fix: make exposure tests compatible with headless mode
vkuprin d540b2d
fix: make all sandbox exposure tests compatible with headless mode
vkuprin badb65f
fix: adjust comprehensive exposure test thresholds for headless mode
vkuprin 70db383
fix: remove --headless flag in CI to work with Xvfb
vkuprin 4cae451
fix: remove --single-process flag to fix DevTools WebSocket connection
vkuprin 2dc719d
chore: ci fix
vkuprin 6904d50
chore: ci fix
vkuprin fd1a503
chore: ci fix
vkuprin 89e1451
chore: ci fix
vkuprin b38cf45
chore: try gtk init inline
vkuprin da6e109
chore: test ci new setup
vkuprin 092fdae
chore: test ci
vkuprin 950989a
chore: improve ci
vkuprin 79c5404
chore: try gtk init inline
vkuprin 675cb71
chore: cache playwright setup
vkuprin e7ee192
chore: latest cache version
vkuprin ad681b7
chore: testing ci/cd
vkuprin 27cafe5
chore: not needed flags
vkuprin File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,155 @@ | ||
name: Security E2E Tests | ||
|
||
on: | ||
push: | ||
branches: [main, staging, dev] | ||
pull_request: | ||
branches: [main, staging, dev] | ||
paths: | ||
- 'electron/src/**' | ||
- 'test/e2e-security/**' | ||
- '.github/workflows/security-tests.yml' | ||
|
||
jobs: | ||
security-tests: | ||
name: Security E2E Tests | ||
runs-on: ubuntu-latest | ||
timeout-minutes: 30 | ||
|
||
steps: | ||
- name: Cancel Previous Runs | ||
uses: styfle/cancel-workflow-action@85880fa0301c86cca9da44039ee3bb12d3bedbfa # v0.12.1 | ||
with: | ||
access_token: ${{github.token}} | ||
|
||
- name: Checkout repository | ||
uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7 | ||
with: | ||
fetch-depth: 2 | ||
|
||
- name: Set up Node.js | ||
uses: actions/setup-node@1e60f620b9541d16bece96c5465dc8ee9832be0b # v4.0.3 | ||
with: | ||
node-version: 18.x | ||
cache: 'yarn' | ||
cache-dependency-path: '**/yarn.lock' | ||
|
||
- name: Install dependencies | ||
run: yarn --immutable | ||
|
||
- name: Build Electron app | ||
run: yarn build:ts | ||
|
||
- name: Install security test dependencies | ||
run: | | ||
cd test/e2e-security | ||
yarn install | ||
|
||
- name: Cache Playwright browsers | ||
uses: actions/cache@v4 | ||
id: playwright-cache | ||
with: | ||
path: ~/.cache/ms-playwright | ||
key: ${{ runner.os }}-playwright-${{ hashFiles('test/e2e-security/yarn.lock') }} | ||
|
||
- name: Install Playwright browsers | ||
if: steps.playwright-cache.outputs.cache-hit != 'true' | ||
run: | | ||
cd test/e2e-security | ||
yarn playwright install --with-deps | ||
|
||
- name: Set up virtual display | ||
run: | | ||
sudo apt-get update | ||
sudo apt-get install -y xvfb x11-utils | ||
# Start Xvfb and keep it running | ||
export DISPLAY=:99 | ||
Xvfb :99 -ac -screen 0 1280x1024x24 > /dev/null 2>&1 & | ||
# Wait for Xvfb to be ready | ||
sleep 5 | ||
echo "DISPLAY=:99" >> $GITHUB_ENV | ||
# Verify Xvfb is working | ||
xdpyinfo -display :99 | head -10 || echo "Warning: Could not verify X server" | ||
|
||
- name: Run Security E2E Tests | ||
working-directory: test/e2e-security | ||
env: | ||
DISPLAY: ':99' | ||
ELECTRON_DISABLE_SECURITY_WARNINGS: true | ||
ELECTRON_DISABLE_GPU: true | ||
ELECTRON_NO_ATTACH_CONSOLE: true | ||
ELECTRON_ENABLE_LOGGING: true | ||
NODE_ENV: test | ||
WIRE_FORCE_EXTERNAL_AUTH: false | ||
HEADLESS: true | ||
run: | | ||
echo "DISPLAY is set to: $DISPLAY" | ||
echo "Verifying X server is running..." | ||
xdpyinfo -display :99 || echo "Warning: xdpyinfo failed, but continuing..." | ||
|
||
echo "Running Security Exposure Tests..." | ||
yarn test:security:exposure | ||
|
||
echo "Running Security Validation Tests..." | ||
yarn test:security:validation | ||
|
||
echo "Running Security Regression Tests..." | ||
yarn test:security:regression | ||
|
||
- name: Upload Security Test Report | ||
if: always() | ||
uses: actions/upload-artifact@50769540e7f4bd5e21e526ee35c689e35e0d6874 # v4.4.0 | ||
with: | ||
name: security-test-report | ||
path: test/e2e-security/security-test-report/ | ||
retention-days: 30 | ||
|
||
- name: Upload Test Results | ||
if: always() | ||
uses: actions/upload-artifact@50769540e7f4bd5e21e526ee35c689e35e0d6874 # v4.4.0 | ||
with: | ||
name: security-test-results | ||
path: test/e2e-security/test-results/ | ||
retention-days: 30 | ||
|
||
- name: Comment PR with Security Test Results | ||
if: github.event_name == 'pull_request' && always() | ||
uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # v7.0.1 | ||
with: | ||
script: | | ||
const fs = require('fs'); | ||
const path = require('path'); | ||
|
||
try { | ||
const reportPath = 'test/e2e-security/security-test-report/summary.json'; | ||
if (fs.existsSync(reportPath)) { | ||
const summary = JSON.parse(fs.readFileSync(reportPath, 'utf8')); | ||
|
||
const comment = `## 🛡️ Security Test Results | ||
|
||
**Context Isolation & Sandbox Validation** | ||
|
||
- **Total Tests**: ${summary.totalTests || 'N/A'} | ||
- **Passed**: ${summary.passed || 'N/A'} ✅ | ||
- **Failed**: ${summary.failed || 'N/A'} ${summary.failed > 0 ? '❌' : ''} | ||
- **Duration**: ${summary.duration || 'N/A'}ms | ||
|
||
**Security Validations** | ||
- Context Isolation: ${summary.contextIsolationVerified ? '✅ Verified' : '❌ Failed'} | ||
- Sandbox Enforcement: ${summary.sandboxVerified ? '✅ Verified' : '❌ Failed'} | ||
|
||
${summary.failed > 0 ? '⚠️ **Security tests failed!** Please review the test results and ensure all security measures are properly implemented.' : '🎉 **All security tests passed!** Context isolation and sandbox are working correctly.'} | ||
|
||
[View detailed report](https://github.com/${context.repo.owner}/${context.repo.repo}/actions/runs/${context.runId}) | ||
`; | ||
|
||
github.rest.issues.createComment({ | ||
issue_number: context.issue.number, | ||
owner: context.repo.owner, | ||
repo: context.repo.repo, | ||
body: comment | ||
}); | ||
} | ||
} catch (error) { | ||
console.log('Could not read test summary:', error); | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
# SonarCloud Configuration for Wire Desktop | ||
|
||
# Exclude e2e security test files from analysis entirely | ||
sonar.exclusions=test/e2e-security/**/* | ||
|
||
# Alternative: If we want to include them but ignore specific issues | ||
# sonar.issue.ignore.multicriteria=e1,e2,e3,e4,e5,e6,e7 | ||
|
||
# Node.js import convention issues | ||
# sonar.issue.ignore.multicriteria.e1.ruleKey=typescript:S4328 | ||
# sonar.issue.ignore.multicriteria.e1.resourceKey=test/e2e-security/**/* | ||
|
||
# GlobalThis preference issues | ||
# sonar.issue.ignore.multicriteria.e2.ruleKey=typescript:S2137 | ||
# sonar.issue.ignore.multicriteria.e2.resourceKey=test/e2e-security/**/* | ||
|
||
# Empty catch block issues | ||
# sonar.issue.ignore.multicriteria.e3.ruleKey=typescript:S2486 | ||
# sonar.issue.ignore.multicriteria.e3.resourceKey=test/e2e-security/**/* | ||
|
||
# Unused variable issues | ||
# sonar.issue.ignore.multicriteria.e4.ruleKey=typescript:S1481 | ||
# sonar.issue.ignore.multicriteria.e4.resourceKey=test/e2e-security/**/* | ||
|
||
# ESLint specific rules | ||
# sonar.issue.ignore.multicriteria.e5.ruleKey=eslint:prefer-node-protocol | ||
# sonar.issue.ignore.multicriteria.e5.resourceKey=test/e2e-security/**/* | ||
|
||
# sonar.issue.ignore.multicriteria.e6.ruleKey=eslint:no-restricted-globals | ||
# sonar.issue.ignore.multicriteria.e6.resourceKey=test/e2e-security/**/* | ||
|
||
# sonar.issue.ignore.multicriteria.e7.ruleKey=eslint:no-empty | ||
# sonar.issue.ignore.multicriteria.e7.resourceKey=test/e2e-security/**/* |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
# SonarQube Configuration for Wire Desktop | ||
|
||
# Project identification | ||
sonar.projectKey=wire-desktop | ||
sonar.projectName=Wire Desktop | ||
sonar.projectVersion=1.0 | ||
|
||
# Source code configuration | ||
sonar.sources=electron/src,test | ||
sonar.exclusions=**/node_modules/**,**/dist/**,**/build/**,**/*.min.js | ||
|
||
# Test configuration | ||
sonar.tests=test | ||
sonar.test.inclusions=**/*.spec.ts,**/*.test.ts | ||
|
||
# Language configuration | ||
sonar.typescript.node=node | ||
|
||
# E2E Security Tests - Special exclusions | ||
# These tests intentionally use patterns that violate general code quality rules | ||
# for security testing purposes (testing window object, intentional error catching, etc.) | ||
|
||
# Disable node: import convention for e2e security tests | ||
sonar.issue.ignore.multicriteria.e1.ruleKey=javascript:S4328 | ||
sonar.issue.ignore.multicriteria.e1.resourceKey=test/e2e-security/**/* | ||
|
||
# Disable globalThis preference for e2e security tests (need to test window object) | ||
sonar.issue.ignore.multicriteria.e2.ruleKey=javascript:S2137 | ||
sonar.issue.ignore.multicriteria.e2.resourceKey=test/e2e-security/**/* | ||
|
||
# Disable empty catch block warnings for e2e security tests (intentional) | ||
sonar.issue.ignore.multicriteria.e3.ruleKey=javascript:S2486 | ||
sonar.issue.ignore.multicriteria.e3.resourceKey=test/e2e-security/**/* | ||
|
||
# Disable unused variable warnings for malicious test scripts | ||
sonar.issue.ignore.multicriteria.e4.ruleKey=javascript:S1481 | ||
sonar.issue.ignore.multicriteria.e4.resourceKey=test/e2e-security/fixtures/malicious-scripts/**/* | ||
|
||
# Coverage exclusions for test fixtures and malicious scripts | ||
sonar.coverage.exclusions=test/e2e-security/fixtures/**/*,test/e2e-security/utils/injection-helpers.ts |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
{ | ||
"extends": ["../../.eslintrc.json"], | ||
"parserOptions": { | ||
"project": ["./tsconfig.json"] | ||
}, | ||
"rules": { | ||
"no-console": "off", | ||
"valid-jsdoc": "off", | ||
"@typescript-eslint/no-unused-vars": "off", | ||
"no-unsanitized/property": "off", | ||
"import/no-unresolved": "off" | ||
}, | ||
"settings": { | ||
"import/resolver": { | ||
"node": { | ||
"paths": ["node_modules", "../../node_modules"] | ||
} | ||
} | ||
} | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
# SonarQube exclusions for E2E Security Tests | ||
# These rules are disabled because security tests have specific requirements | ||
# that conflict with general code quality guidelines | ||
|
||
# Disable node: import convention for e2e tests (compatibility reasons) | ||
sonar.issue.ignore.multicriteria.e1.ruleKey=javascript:S4328 | ||
sonar.issue.ignore.multicriteria.e1.resourceKey=test/e2e-security/**/* | ||
|
||
# Disable globalThis preference for security tests (need to test window object specifically) | ||
sonar.issue.ignore.multicriteria.e2.ruleKey=javascript:S2137 | ||
sonar.issue.ignore.multicriteria.e2.resourceKey=test/e2e-security/**/* | ||
|
||
# Disable empty catch block warnings for security tests (intentional error suppression) | ||
sonar.issue.ignore.multicriteria.e3.ruleKey=javascript:S2486 | ||
sonar.issue.ignore.multicriteria.e3.resourceKey=test/e2e-security/**/* | ||
|
||
# Disable prefer globalThis over global for security tests | ||
sonar.issue.ignore.multicriteria.e4.ruleKey=javascript:S2137 | ||
sonar.issue.ignore.multicriteria.e4.resourceKey=test/e2e-security/**/* |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,67 @@ | ||
# SonarQube Exclusions for E2E Security Tests | ||
|
||
This document explains why certain SonarQube rules are disabled for the e2e-security test suite. | ||
|
||
## Excluded Rules and Rationale | ||
|
||
### 1. Node.js Import Convention (S4328) | ||
|
||
**Rule**: Prefer `node:fs` over `fs` **Why Excluded**: | ||
|
||
- E2E security tests need to maintain compatibility with various Node.js versions | ||
- Some test environments may not support the `node:` prefix | ||
- Legacy import style is more widely compatible for testing scenarios | ||
|
||
### 2. GlobalThis Preference (S2137) | ||
|
||
**Rule**: Prefer `globalThis` over `window`/`global` **Why Excluded**: | ||
|
||
- Security tests specifically need to test `window` object behavior in browser contexts | ||
- Tests verify that `window` object is properly isolated in Electron renderer processes | ||
- Using `globalThis` would defeat the purpose of testing browser-specific security boundaries | ||
|
||
### 3. Empty Catch Blocks (S2486) | ||
|
||
**Rule**: Handle exceptions or don't catch them at all **Why Excluded**: | ||
|
||
- Security tests intentionally catch and ignore errors to test isolation boundaries | ||
- Many tests verify that certain operations fail (throw errors) as expected | ||
- Empty catch blocks are used to test that APIs are properly blocked/sandboxed | ||
|
||
### 4. Unused Variables in Malicious Scripts | ||
|
||
**Rule**: Remove unused variables **Why Excluded**: | ||
|
||
- Malicious test scripts intentionally contain unused code to simulate real attack vectors | ||
- These scripts are designed to test security boundaries, not code quality | ||
- Removing "unused" code would make the security tests less realistic | ||
|
||
## Files Affected | ||
|
||
- `test/e2e-security/**/*` - All security test files | ||
- `test/e2e-security/fixtures/malicious-scripts/**/*` - Intentionally malicious test scripts | ||
- `test/e2e-security/utils/injection-helpers.ts` - Security testing utilities | ||
|
||
## Important Notes | ||
|
||
⚠️ **These exclusions should NOT be applied to production code** | ||
|
||
The exclusions are specifically scoped to the `test/e2e-security/` directory to ensure that: | ||
|
||
1. Production code still follows all quality guidelines | ||
2. Security tests can properly validate isolation and sandboxing | ||
3. Test fixtures can simulate realistic attack scenarios | ||
|
||
## Reviewing Security Test Code | ||
|
||
When reviewing security test code, focus on: | ||
|
||
- ✅ **Security effectiveness**: Does the test properly validate security boundaries? | ||
- ✅ **Test coverage**: Are all security scenarios covered? | ||
- ✅ **Documentation**: Are security test intentions clear? | ||
|
||
Rather than: | ||
|
||
- ❌ Code style preferences that conflict with security testing needs | ||
- ❌ Import conventions that may break test compatibility | ||
- ❌ Error handling patterns that are intentional for security validation |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be nice if we could use the concurrency feature directly, since this package is "kind of" deprecated by their own description: https://github.com/styfle/cancel-workflow-action