Compare commits

..
Author SHA1 Message Date
Alexandre Teixeira 0eb6b9eb1d ci: gate native suites with isolated browser fixtures 2026-10-29 17:35:45 +00:00
13 changed files with 468 additions and 163 deletions
+17
View File
@@ -0,0 +1,17 @@
"""Reject skipped/empty native CI runs without conflating driver child counts."""
import argparse
from pathlib import Path
import xml.etree.ElementTree as ET
parser = argparse.ArgumentParser(description=__doc__)
parser.add_argument('report', type=Path)
parser.add_argument('--minimum', type=int, required=True)
args = parser.parse_args()
root = ET.parse(args.report).getroot()
cases = list(root.iter('testcase'))
if len(cases) < args.minimum:
raise SystemExit(f'Incomplete execution: {len(cases)} cases, expected at least {args.minimum}')
for case in cases:
if any(case.find(tag) is not None for tag in ('skipped', 'failure', 'error')):
raise SystemExit(f'Case did not pass: {case.get("classname")}::{case.get("name")}')
print(f'Verified {len(cases)} executed cases; no skipped, failed or errored cases')
+46
View File
@@ -0,0 +1,46 @@
import fs from 'node:fs';
import path from 'node:path';
import { fileURLToPath } from 'node:url';
import { spawnSync } from 'node:child_process';
import { parse } from 'acorn';
const root = fileURLToPath(new URL('../../', import.meta.url));
const manifest = JSON.parse(fs.readFileSync(path.join(root, 'tests/native-ci-manifest.json'), 'utf8'));
function files(directory) {
return fs.readdirSync(directory, { withFileTypes: true }).flatMap(entry => {
const target = path.join(directory, entry.name);
return entry.isDirectory() ? files(target) : /\.(?:mjs|cjs|js)$/.test(entry.name) ? [target] : [];
});
}
function importsNativeTest(node) {
if (!node || typeof node !== 'object') return false;
if (node.type === 'ImportDeclaration' && node.source.value === 'node:test') return true;
if (node.type === 'CallExpression' && node.callee.name === 'require' && node.arguments[0]?.value === 'node:test') return true;
return Object.values(node).some(value => Array.isArray(value) ? value.some(importsNativeTest) : importsNativeTest(value));
}
const discovered = files(path.join(root, 'tests')).filter(file => {
const source = fs.readFileSync(file, 'utf8');
return source.includes('node:test') && importsNativeTest(parse(source, { ecmaVersion: 'latest', sourceType: 'module' }));
}).map(file => path.relative(root, file).split(path.sep).join('/')).sort();
const assigned = Object.values(manifest).flatMap(group => Array.isArray(group) ? group : Object.keys(group));
if (new Set(assigned).size !== assigned.length || JSON.stringify([...assigned].sort()) !== JSON.stringify(discovered)) {
throw new Error('Native test coverage must be exhaustive and disjoint; update tests/native-ci-manifest.json for added, removed or duplicated files');
}
for (const wrappers of Object.values(manifest['python-ci'])) {
for (const wrapper of wrappers) if (!fs.existsSync(path.join(root, wrapper))) throw new Error(`Missing pytest wrapper: ${wrapper}`);
}
if (process.argv.includes('--list')) {
console.log(JSON.stringify({ discovered, runners: manifest }, null, 2));
} else {
const reports = path.join(root, 'test-results/native-node');
fs.mkdirSync(reports, { recursive: true });
const result = spawnSync(process.execPath, ['--test', '--test-concurrency=1',
'--test-reporter=spec', '--test-reporter-destination=stdout',
'--test-reporter=junit', `--test-reporter-destination=${path.join(reports, 'junit.xml')}`,
...manifest['native-ci']], {
cwd: root, stdio: 'inherit',
env: { ...process.env, ENDPOINT_URL: 'http://127.0.0.1:1' },
});
if (result.error) throw result.error;
process.exitCode = result.status ?? 1;
}
+119
View File
@@ -0,0 +1,119 @@
name: Native test suites
on:
pull_request:
push:
branches: [main, dev]
workflow_dispatch:
permissions:
contents: read
concurrency:
group: native-tests-${{ github.ref }}
cancel-in-progress: true
jobs:
native-node:
name: Native Node regressions
runs-on: ubuntu-24.04
timeout-minutes: 15
steps:
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
with:
persist-credentials: false
- uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0
with:
python-version: '3.11'
- uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0
with:
node-version: '22.23.4'
cache: npm
- run: npm ci --ignore-scripts --no-audit --no-fund
- uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0
with:
path: ~/.cache/ms-playwright
key: ms-playwright-${{ runner.os }}-${{ hashFiles('package-lock.json') }}
- run: npx playwright install --with-deps chromium
- name: Validate exhaustive disjoint runner mapping and execute omitted suites
run: |
node .github/scripts/run_native_tests.mjs
python .github/scripts/check_test_execution.py test-results/native-node/junit.xml --minimum 43
- name: Save native test identities and results
if: always()
uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
with:
name: native-node-results
path: test-results/native-node/
retention-days: 7
if-no-files-found: warn
browser:
name: Chromium application regressions
runs-on: ubuntu-24.04
timeout-minutes: 30
env:
ODYSSEUS_E2E_DISPOSABLE_ARTIFACTS: '1'
ODYSSEUS_TEST_PYTHON: python
CHROMADB_HOST: 127.0.0.1
CHROMADB_PORT: '1'
steps:
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
with:
persist-credentials: false
- uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0
with:
python-version: '3.11'
- name: Install FFmpeg and establish system trust
run: |
sudo apt-get update
sudo apt-get install -y --no-install-recommends ffmpeg ca-certificates
python scripts/check_ca_trust.py --system
- uses: astral-sh/setup-uv@c18668ad3cf93ea998bef934396af7bb5c839dc7 # v10.2.0
with:
version: '0.13.5'
enable-cache: true
cache-dependency-glob: requirements.txt
prune-cache: false
- run: |
uv pip install --system --require-hashes --only-binary=:all: --no-deps -r requirements-bootstrap.txt
python scripts/check_ca_trust.py --python
uv pip install --system -r requirements.txt
- uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0
with:
node-version: '22.23.4'
cache: npm
- run: npm ci --ignore-scripts --no-audit --no-fund
- uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0
with:
path: ~/.cache/ms-playwright
key: ms-playwright-${{ runner.os }}-${{ hashFiles('package-lock.json') }}
- run: npx playwright install --with-deps chromium
- name: Run complete Chromium suite against a fresh authenticated app
env:
PLAYWRIGHT_JUNIT_OUTPUT_FILE: test-results/browser-junit.xml
run: |
npm run test:photo-editor -- --reporter=line,junit
python .github/scripts/check_test_execution.py test-results/browser-junit.xml --minimum 130
- name: Save per-test results and sanitized failure diagnostics
if: always()
uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
with:
name: chromium-regression-results
path: test-results/
retention-days: 7
if-no-files-found: warn
authoritative-native-suites:
name: Authoritative native suites
if: always()
needs: [native-node, browser]
runs-on: ubuntu-24.04
steps:
- name: Require both complete suites to succeed
env:
NODE_RESULT: ${{ needs.native-node.result }}
BROWSER_RESULT: ${{ needs.browser.result }}
run: |
test "$NODE_RESULT" = success
test "$BROWSER_RESULT" = success
+38
View File
@@ -0,0 +1,38 @@
# Native test merge gate
`Native test suites` runs on pull requests, dev/main pushes and manual dispatch.
`Authoritative native suites` fails unless both the native Node and complete
Chromium jobs succeed; it has no continue-on-error, retries or path bypass.
Repository administrators must require this check alongside the four pytest
checks in branch protection. The audited public dev branch had no protection or
ruleset, so merely adding workflow jobs cannot prevent a manual merge.
`tests/native-ci-manifest.json` exhaustively partitions all native Node files:
16 run in the native job,14 through existing pytest wrappers, one through the
existing frontend architecture CI step, and the formerly live email-reply file
through an explicitly identified Playwright driver. The manifest validator fails
for omitted/duplicate/unreachable entries. Node emits individual JUnit identities
and propagates its failure exit. No native file is newly run in each pytest shard.
The browser job uses the existing one-worker Chromium config and all original129
cases, plus one driver for the native email-reply contract. Its ephemeral account,
SQLite DB and server are created by the config/setup; reuseExistingServer remains
false. NPX startup is offline/cache-only in a private cache; Notes/editor behavior
and Python MCP services remain real. No developer app or external account is used.
Default CI trace suppression remains intact. Only this job's explicit opt-in plus
a newly generated authenticated synthetic fixture marker permits failure traces.
The existing structured sanitizer runs before artifacts are uploaded; invalid
ownership, failed sanitization and missing tooling purge sensitive artifacts.
Never upload the fixture directory, database or browser storage state.
The stroke budget now excludes measured synthetic event-delivery pacing and also
bounds actual Chromium main-thread work at the original5s. All240 samples and
pixel/thumbnail/undo/redo assertions remain. Per-case timing attachments expose
both clocks rather than treating driver frame cadence as application work.
Three Bombadil properties remain opt-in: they require a Bombadil/browser scenario
and bounded fuzz campaign; no authoritative execution is claimed. The two model
smoke scripts require supplied model endpoints/models and remain explicit live
operations. No credentials or live-service authorization are implied by CI setup.
Firefox/WebKit alternatives remain opt-in and are not certified by Chromium CI.
+19
View File
@@ -0,0 +1,19 @@
const fs = require('node:fs');
const os = require('node:os');
const path = require('node:path');
const marker = '.disposable-ci-fixture.json';
function disposableCIArtifacts() {
if (!(process.env.CI || process.env.GITHUB_ACTIONS) || process.env.ODYSSEUS_E2E_DISPOSABLE_ARTIFACTS !== '1') return false;
const directory = process.env.PHOTO_EDITOR_E2E_DATA_DIR;
if (!directory || process.env.PHOTO_EDITOR_E2E_AUTH === '0') return false;
const resolved = path.resolve(directory);
if (!path.basename(resolved).startsWith('odysseus-photo-editor-e2e-')) return false;
if (![os.tmpdir(), '/tmp', '/var/tmp'].some(base => resolved.startsWith(path.resolve(base) + path.sep))) return false;
if (process.env.PHOTO_EDITOR_E2E_DB_PATH !== path.join(resolved, 'app.db')) return false;
try {
const fixture = JSON.parse(fs.readFileSync(path.join(resolved, marker), 'utf8'));
return fixture.synthetic === true && fixture.authenticated === true && fixture.database === path.join(resolved, 'app.db');
} catch { return false; }
}
module.exports = { disposableCIArtifacts, marker };
+29 -2
View File
@@ -35,7 +35,7 @@ test('brush presets and shared stroke controls remain reusable', async ({ page }
await expect(page.locator('#ge-brush-preset option', { hasText: 'My Detail Brush' })).toHaveCount(0);
});
test('long sampled strokes stay responsive and undo atomically', async ({ page }) => {
test('long sampled strokes stay responsive and undo atomically', async ({ page, browserName }, testInfo) => {
await openBlankEditor(page);
await selectTool(page, 'brush');
await page.locator('.ge-size-slider').fill('700');
@@ -49,13 +49,40 @@ test('long sampled strokes stay responsive and undo atomically', async ({ page }
const before = await flattenedPixelDigest(page);
const start = await canvasPoint(page, 0.08, 0.5);
const end = await canvasPoint(page, 0.92, 0.5);
// Playwright delivers stepped moves at frame cadence: 240 events already
// cost ~4s on an inert page. Measure that transport cost with the same input,
// and separately bound real Chromium main-thread work at the original5s.
const control = await page.context().newPage();
let controlDuration;
try {
await control.setContent('<canvas style="position:fixed;inset:0;width:100vw;height:100vh"></canvas>');
const started = Date.now();
await control.mouse.move(start.x, start.y);
await control.mouse.down();
await control.mouse.move(end.x, end.y, { steps: 240 });
await control.mouse.up();
controlDuration = Date.now() - started;
} finally {
await control.close();
await page.bringToFront();
}
const metrics = browserName === 'chromium' ? await page.context().newCDPSession(page) : null;
if (metrics) await metrics.send('Performance.enable');
const taskTime = async () => (await metrics.send('Performance.getMetrics')).metrics.find(item => item.name === 'TaskDuration').value *1000;
const beforeTask = metrics ? await taskTime() : null;
const startedAt = Date.now();
await page.mouse.move(start.x, start.y);
await page.mouse.down();
await page.mouse.move(end.x, end.y, { steps: 240 });
await page.mouse.up();
const strokeDuration = Date.now() - startedAt;
expect(strokeDuration).toBeLessThan(5000);
const mainThreadDuration = metrics ? (await taskTime()) - beforeTask : null;
if (metrics) await metrics.detach();
await testInfo.attach('stroke-timing', { body: Buffer.from(JSON.stringify({
samples: 240, strokeDuration, controlDuration, mainThreadDuration,
})), contentType: 'application/json' });
expect(strokeDuration - controlDuration).toBeLessThan(5000);
if (mainThreadDuration !== null) expect(mainThreadDuration).toBeLessThan(5000);
const after = await flattenedPixelDigest(page);
expect(after).not.toEqual(before);
await expect(thumb).toHaveAttribute('width', '68');
@@ -0,0 +1,21 @@
const { test, expect } = require('@playwright/test');
const { spawnSync } = require('node:child_process');
const path = require('node:path');
// The previously opt-in node:test now uses only this config's owned app/account.
// Its native JUnit report retains its authored identity; this case is a driver.
test('native email reply regression on the disposable authenticated app', async ({ context, baseURL }, testInfo) => {
const origin = new URL(baseURL);
expect(origin.hostname).toBe('127.0.0.1');
const cookie = (await context.cookies(baseURL)).find(item => item.name === 'odysseus_session');
expect(cookie?.value).toBeTruthy();
const report = testInfo.outputPath('native-email-junit.xml');
const result = spawnSync(process.execPath, ['--test', '--test-reporter=spec', '--test-reporter-destination=stdout',
'--test-reporter=junit', `--test-reporter-destination=${report}`, 'tests/emailReplyBrowser.test.mjs'], {
cwd: path.resolve(__dirname, '../../..'), encoding: 'utf8', timeout: 80_000,
env: { ...process.env, ODYSSEUS_UI_TEST_URL: baseURL, ODYSSEUS_UI_TEST_COOKIE: cookie.value },
});
console.log(result.stdout);
expect(result.error, String(result.error)).toBeUndefined();
expect(result.status, result.stdout + result.stderr).toBe(0);
});
+17 -3
View File
@@ -2,6 +2,12 @@ const fs = require('node:fs');
const os = require('node:os');
const path = require('node:path');
const { defineConfig } = require('@playwright/test');
const { disposableCIArtifacts, marker } = require('./artifact-policy');
const isCI = Boolean(process.env.CI) || Boolean(process.env.GITHUB_ACTIONS);
const disposableOptIn = isCI && process.env.ODYSSEUS_E2E_DISPOSABLE_ARTIFACTS === '1';
if (disposableOptIn && (process.env.PHOTO_EDITOR_E2E_DATA_DIR || process.env.PHOTO_EDITOR_E2E_DB_PATH || process.env.PHOTO_EDITOR_E2E_AUTH === '0') && !disposableCIArtifacts()) {
throw new Error('CI artifact opt-in requires fresh authenticated fixture paths');
}
const port = Number(process.env.PHOTO_EDITOR_E2E_PORT || 7013);
const baseURL = `http://127.0.0.1:${port}`;
@@ -24,7 +30,9 @@ if (!['chromium', 'firefox', 'webkit'].includes(browserName)) {
throw new Error(`Unsupported PHOTO_EDITOR_E2E_BROWSER: ${browserName}`);
}
const isCI = Boolean(process.env.CI) || Boolean(process.env.GITHUB_ACTIONS);
if (disposableOptIn) {
fs.writeFileSync(path.join(dataDirectory, marker), JSON.stringify({ synthetic: true, authenticated: true, database: databasePath }));
}
const traceOptIn = !isCI && ['1', 'true'].includes(String(process.env.PLAYWRIGHT_ENABLE_TRACE || '').toLowerCase());
module.exports = defineConfig({
@@ -32,6 +40,8 @@ module.exports = defineConfig({
outputDir: path.join(__dirname, '..', '..', 'test-results', 'photo-editor'),
timeout: 90_000,
expect: { timeout: 10_000 },
forbidOnly: isCI,
retries: 0,
fullyParallel: false,
workers: 1,
reporter: isCI ? 'line' : 'line',
@@ -49,9 +59,9 @@ module.exports = defineConfig({
serviceWorkers: 'block',
...(authenticated ? { storageState: path.join(dataDirectory, 'browser-auth.json') } : {}),
screenshot: 'only-on-failure',
// Unconditional CI trace suppression: Playwright traces are NEVER recorded in CI.
// Default CI suppression remains; only the fresh synthetic fixture opts in.
// Local developer tracing requires explicit PLAYWRIGHT_ENABLE_TRACE=1.
trace: traceOptIn ? 'retain-on-failure' : 'off',
trace: (traceOptIn || disposableCIArtifacts()) ? 'retain-on-failure' : 'off',
},
webServer: {
command: `${JSON.stringify(python)} -m uvicorn app:app --host 127.0.0.1 --port ${port}`,
@@ -65,6 +75,10 @@ module.exports = defineConfig({
DATABASE_URL: `sqlite:///${databasePath}`,
ODYSSEUS_DATA_DIR: dataDirectory,
ODYSSEUS_STARTUP_WARMUPS: '0',
ODYSSEUS_TOOL_INDEX_PREWARM: '0',
ODYSSEUS_BROWSER_MCP_REQUIRE_CACHE: '1',
npm_config_offline: 'true',
npm_config_cache: path.join(dataDirectory, 'npm-cache'),
RESPONSE_COMPRESSION_ENABLED: 'false',
},
},
+7 -2
View File
@@ -2,6 +2,7 @@ const fs = require('node:fs');
const os = require('node:os');
const path = require('node:path');
const { execFileSync } = require('node:child_process');
const { disposableCIArtifacts } = require('./artifact-policy');
const repoRoot = path.resolve(__dirname, '..', '..');
const defaultResultsDir = path.join(repoRoot, 'test-results');
@@ -29,6 +30,9 @@ module.exports = async (options = {}) => {
const isPlaywrightConfig = options && (options.configFile || options.rootDir);
const testOpts = (!isPlaywrightConfig && typeof options === 'object') ? options : {};
// Validate fixture ownership before removing its temporary directory.
const disposableCI = disposableCIArtifacts();
// Confine cleanup of PHOTO_EDITOR_E2E_DATA_DIR:
// Strictly require it to be within an OS temporary directory and match the expected test prefix
const dataDirectory = process.env.PHOTO_EDITOR_E2E_DATA_DIR;
@@ -100,7 +104,7 @@ module.exports = async (options = {}) => {
const isCI = Boolean(process.env.CI) || Boolean(process.env.GITHUB_ACTIONS);
const traceOptIn = !isCI && ['1', 'true'].includes(String(process.env.PLAYWRIGHT_ENABLE_TRACE || '').toLowerCase());
if (isCI || !traceOptIn) {
if ((isCI && !disposableCI) || (!traceOptIn && !disposableCI)) {
// In CI or default local runs, purge all sensitive trace archives and artifacts
purgeSensitiveArtifacts(resultsDir);
} else {
@@ -124,7 +128,8 @@ module.exports = async (options = {}) => {
if (!sanitizerSucceeded) {
// Fail closed: purge all remaining zip or tmp archives
purgeZipAndTmp(resultsDir);
if (disposableCI) purgeSensitiveArtifacts(resultsDir);
else purgeZipAndTmp(resultsDir);
}
}
}
+71
View File
@@ -0,0 +1,71 @@
{
"native-ci": [
"tests/backgroundToolJobs.test.mjs",
"tests/chatEditorProgress.test.mjs",
"tests/chatImageDeletion.test.mjs",
"tests/chatPhotoEditorIntegrity.test.mjs",
"tests/chatProcessingHandoff.test.mjs",
"tests/copyChatHeader.test.mjs",
"tests/emailReplyStream.test.mjs",
"tests/galleryModelPickerSync.test.mjs",
"tests/generatedImageResult.test.mjs",
"tests/researchMobileButtons.test.mjs",
"tests/schemaThinkingProbe.test.mjs",
"tests/sidebarNewChat.test.mjs",
"tests/skillsApproval.test.mjs",
"tests/streamingRendererRaf.test.mjs",
"tests/toolFollowupOracle.test.mjs",
"tests/tool_followup_oracle.test.mjs"
],
"browser-ci": [
"tests/emailReplyBrowser.test.mjs"
],
"python-ci": {
"tests/historyResumeRendering.test.mjs": [
"tests/test_history_resume_rendering_js.py",
"tests/test_live_fallback_round_attribution.py"
],
"tests/live_thinking_scheduler.test.mjs": [
"tests/test_live_thinking_scheduler_js.py"
],
"tests/noteTestOracle.test.mjs": [
"tests/test_notes_reference_contracts_js.py"
],
"tests/notesApplication.test.mjs": [
"tests/test_notes_reference_contracts_js.py"
],
"tests/notesCharacterization.test.mjs": [
"tests/test_notes_reference_contracts_js.py"
],
"tests/notesDraftAutosave.test.mjs": [
"tests/test_notes_reference_contracts_js.py"
],
"tests/notesInfrastructure.test.mjs": [
"tests/test_notes_reference_contracts_js.py"
],
"tests/notesRecurrence.test.mjs": [
"tests/test_notes_reference_contracts_js.py"
],
"tests/notesReminders.test.mjs": [
"tests/test_notes_reference_contracts_js.py"
],
"tests/notesWorkflows.test.mjs": [
"tests/test_notes_reference_contracts_js.py"
],
"tests/selectedNotesContext.test.mjs": [
"tests/test_notes_reference_contracts_js.py"
],
"tests/streaming/invariant.test.mjs": [
"tests/test_streaming_segmenter_js.py"
],
"tests/streaming/segmenter.test.mjs": [
"tests/test_streaming_segmenter_js.py"
],
"tests/turnRendering.test.mjs": [
"tests/test_turn_rendering_js.py"
]
},
"architecture-ci": [
"tests/frontendArchitecture.test.mjs"
]
}
@@ -93,7 +93,7 @@ def test_custom_filter_reuses_convolve_kernel():
assert "convolve(readSource(snap), k, divisor, bias, 1)" in EFFECTS
def test_blur_and_custom_filter_aliases_registered():
def test_new_filter_aliases_registered():
assert "'box-blur': 'blur-box'" in EDITOR
assert "'surface-blur': 'blur-surface'" in EDITOR
assert "custom: 'other-custom'" in EDITOR
@@ -303,3 +303,52 @@ def test_teardown_rejects_unconfined_programmatic_paths():
res = _run_teardown(options={"resultsDir": "/etc"})
assert res.returncode == 1
assert "Security rejection" in res.stderr
def _disposable_ci_fixture(tmp_path):
data = tmp_path / "odysseus-photo-editor-e2e-fixture"
data.mkdir()
database = data / "app.db"
(data / ".disposable-ci-fixture.json").write_text(json.dumps({
"synthetic": True, "authenticated": True, "database": str(database),
}))
return {"CI": "true", "GITHUB_ACTIONS": "true",
"ODYSSEUS_E2E_DISPOSABLE_ARTIFACTS": "1",
"PHOTO_EDITOR_E2E_DATA_DIR": str(data), "PHOTO_EDITOR_E2E_DB_PATH": str(database),
"PHOTO_EDITOR_E2E_AUTH": "1", "ODYSSEUS_TEST_PYTHON": sys.executable}
def test_disposable_ci_retains_only_sanitized_failure_artifacts(tmp_path):
results = tmp_path / "results"
results.mkdir()
trace = results / "trace.zip"
with zipfile.ZipFile(trace, "w") as archive:
archive.writestr("0-trace.trace", json.dumps({"type": "action", "name": "fill", "params": {"value": "synthetic-secret"}}) + "\n")
screenshot = results / "failure.png"
screenshot.write_bytes(b"synthetic screenshot")
response = _run_teardown(_disposable_ci_fixture(tmp_path), {"resultsDir": str(results)})
assert response.returncode == 0, response.stderr
with zipfile.ZipFile(trace) as archive:
assert json.loads(archive.read("0-trace.trace"))["params"]["value"] == "[REDACTED]"
assert screenshot.exists()
@pytest.mark.parametrize("defect", ["missing-marker", "wrong-database", "unauthenticated", "sanitizer-failure"])
def test_disposable_ci_artifact_failures_purge_closed(tmp_path, defect):
results = tmp_path / "results"
results.mkdir()
(results / "trace.zip").write_bytes(b"invalid sensitive archive")
(results / "failure.png").write_bytes(b"sensitive screenshot")
environment = _disposable_ci_fixture(tmp_path)
if defect == "missing-marker":
(Path(environment["PHOTO_EDITOR_E2E_DATA_DIR"]) / ".disposable-ci-fixture.json").unlink()
elif defect == "wrong-database":
environment["PHOTO_EDITOR_E2E_DB_PATH"] = str(tmp_path / "other.db")
elif defect == "unauthenticated":
environment["PHOTO_EDITOR_E2E_AUTH"] = "0"
else:
environment["ODYSSEUS_TEST_PYTHON"] = "/missing-python"
response = _run_teardown(environment, {"resultsDir": str(results)})
assert response.returncode == 0, response.stderr
assert not (results / "trace.zip").exists()
assert not (results / "failure.png").exists()
+34 -155
View File
@@ -12,8 +12,6 @@ exercising every procfs-touching code path on both a Linux and a non-Linux
host. ``test_cookbook_stop_without_procfs.py`` covers the behaviour itself.
"""
import ast
import pytest
from pathlib import Path
REPO_ROOT = Path(__file__).resolve().parent.parent
@@ -77,112 +75,37 @@ def _scan_sites(tree: ast.AST) -> list[ast.Call]:
return sites
def _condition_requires_procfs(node: ast.AST, truth: bool) -> bool:
"""Whether this Boolean outcome proves a direct procfs check succeeded.
An ignored call, another function's check, or a check on only one possible
OR branch provides no proof. Keep unknown expressions conservative.
"""
if isinstance(node, ast.UnaryOp) and isinstance(node.op, ast.Not):
return _condition_requires_procfs(node.operand, not truth)
if isinstance(node, ast.BoolOp):
facts = [_condition_requires_procfs(value, truth) for value in node.values]
all_values_required = isinstance(node.op, ast.And) == truth
return any(facts) if all_values_required else all(facts)
if not isinstance(node, ast.Call):
return False
func = node.func
name = func.attr if isinstance(func, ast.Attribute) else getattr(func, "id", "")
subject_is_procfs = (
name == "has_procfs"
or (isinstance(func, ast.Attribute) and _is_procfs_root(func.value))
or any(_is_procfs_root(arg) for arg in node.args)
)
return truth and name in _GUARD_FUNCS and subject_is_procfs
def _guard_lines(tree: ast.AST) -> list[int]:
"""Line numbers of calls that test whether procfs is present."""
lines = []
for node in ast.walk(tree):
if not isinstance(node, ast.Call):
continue
func = node.func
name = func.attr if isinstance(func, ast.Attribute) else getattr(func, "id", "")
if name not in _GUARD_FUNCS:
continue
subject_is_procfs = (
name in {"has_procfs", "is_wsl"}
or (isinstance(func, ast.Attribute) and _is_procfs_root(func.value))
or any(_is_procfs_root(arg) for arg in node.args)
)
if subject_is_procfs:
lines.append(node.lineno)
return lines
def _guarded_scan_ids(tree: ast.AST) -> set[int]:
"""Follow conditional/early-exit paths instead of comparing line numbers.
Each function starts with no procfs assumption. Facts from a loop or try
body cannot escape to code which may run without that body completing.
This intentionally requires direct checks; arbitrary aliases/call graphs
are not a proof that procfs exists at the enumeration site.
"""
scans = {id(site) for site in _scan_sites(tree)}
guarded = set()
def expression(node, present):
if node is None:
return
if id(node) in scans and present:
guarded.add(id(node))
if isinstance(node, ast.BoolOp):
for value in node.values:
expression(value, present)
# Later AND operands run only after a true prefix; later OR
# operands run only after a false prefix.
present = present or _condition_requires_procfs(value, isinstance(node.op, ast.And))
elif isinstance(node, ast.IfExp):
expression(node.test, present)
expression(node.body, present or _condition_requires_procfs(node.test, True))
expression(node.orelse, present or _condition_requires_procfs(node.test, False))
elif isinstance(node, (ast.ListComp, ast.SetComp, ast.GeneratorExp, ast.DictComp)):
for generator in node.generators:
expression(generator.iter, present)
for condition in generator.ifs:
expression(condition, present)
present = present or _condition_requires_procfs(condition, True)
for attr in ("elt", "key", "value"):
expression(getattr(node, attr, None), present)
elif isinstance(node, ast.Lambda):
expression(node.body, False)
else:
for child in ast.iter_child_nodes(node):
expression(child, present)
def block(statements, present):
for statement in statements:
if isinstance(statement, (ast.FunctionDef, ast.AsyncFunctionDef)):
block(statement.body, False)
for value in statement.decorator_list + statement.args.defaults:
expression(value, present)
elif isinstance(statement, ast.ClassDef):
block(statement.body, present)
elif isinstance(statement, ast.If):
expression(statement.test, present)
yes = block(statement.body, present or _condition_requires_procfs(statement.test, True))
no = block(statement.orelse, present or _condition_requires_procfs(statement.test, False))
continuing = [fact for fact in (yes, no) if fact is not None]
if not continuing:
return None
present = all(continuing)
elif isinstance(statement, (ast.Return, ast.Raise, ast.Break, ast.Continue)):
for child in ast.iter_child_nodes(statement):
expression(child, present)
return None
elif isinstance(statement, (ast.For, ast.AsyncFor, ast.While)):
condition = statement.test if isinstance(statement, ast.While) else statement.iter
expression(condition, present)
body_present = present or (isinstance(statement, ast.While) and _condition_requires_procfs(condition, True))
block(statement.body, body_present)
block(statement.orelse, present)
elif isinstance(statement, (ast.Try, getattr(ast, "TryStar", ast.Try))):
block(statement.body, present)
block(statement.orelse, present)
block(statement.finalbody, present)
for handler in statement.handlers:
block(handler.body, present)
elif isinstance(statement, (ast.With, ast.AsyncWith)):
for item in statement.items:
expression(item.context_expr, present)
block(statement.body, present)
else:
expression(statement, present)
return present
block(tree.body, False)
return guarded
def _enclosing_scope(tree: ast.AST, node: ast.AST) -> ast.AST:
"""Smallest function/module scope containing ``node``."""
best = tree
for candidate in ast.walk(tree):
if not isinstance(candidate, (ast.FunctionDef, ast.AsyncFunctionDef)):
continue
end = getattr(candidate, "end_lineno", None) or candidate.lineno
if candidate.lineno <= node.lineno <= end:
if best is tree or candidate.lineno > best.lineno:
best = candidate
return best
def _app_python_files() -> list[Path]:
@@ -216,10 +139,12 @@ def _collect_sites() -> tuple[list[str], list[str]]:
sites = _scan_sites(tree)
if not sites:
continue
guards = _guarded_scan_ids(tree)
guards = _guard_lines(tree)
for site in sites:
scope = _enclosing_scope(tree, site)
start = getattr(scope, "lineno", 0)
label = f"{path.relative_to(REPO_ROOT)}:{site.lineno}"
if id(site) in guards:
if any(start <= g < site.lineno for g in guards):
guarded.append(label)
else:
unguarded.append(label)
@@ -229,7 +154,7 @@ def _collect_sites() -> tuple[list[str], list[str]]:
def test_every_procfs_scan_is_guarded_by_an_existence_check():
_guarded, unguarded = _collect_sites()
assert not unguarded, (
"procfs pid scans without an effective conditional existence guard — "
"procfs pid scans with no existence check in the enclosing function — "
"these raise FileNotFoundError on macOS and Windows: "
+ ", ".join(unguarded)
)
@@ -251,49 +176,3 @@ def test_the_guard_detector_still_sees_the_known_scans():
f"root was renamed. Found: {sorted(found)}"
)
assert len(found) >= 3, f"expected at least 3 procfs scans, found {sorted(found)}"
@pytest.mark.parametrize("body", [
'Path("/proc").exists(); os.listdir("/proc")',
'if Path("/proc").exists():\n pass\nos.listdir("/proc")',
'if not Path("/proc").exists():\n os.listdir("/proc")',
'if ready or Path("/proc").exists():\n os.listdir("/proc")',
'if ready:\n Path("/proc").exists()\nos.listdir("/proc")',
'os.listdir("/proc"); Path("/proc").exists()',
'if Path("/tmp").exists():\n os.listdir("/proc")',
'def other():\n Path("/proc").exists()\nos.listdir("/proc")',
'if not has_procfs():\n pass\nos.listdir("/proc")',
'for x in []:\n if not has_procfs():\n return\nos.listdir("/proc")',
'if has_procfs():\n return\nelse:\n os.listdir("/proc")',
'[pid for pid in os.listdir("/proc") if has_procfs()]',
], ids=["ignored", "expired-branch", "inverted", "unsafe-or", "sibling-branch",
"too-late", "wrong-path", "other-function", "no-early-exit", "loop-may-not-run",
"wrong-else", "late-comprehension-filter"])
def test_guard_detector_rejects_ineffective_checks(body):
tree = ast.parse("def scan():\n" + "\n".join(" " + line for line in body.splitlines()))
sites = _scan_sites(tree)
assert sites, "negative control must contain a recognized enumeration"
assert any(id(site) not in _guarded_scan_ids(tree) for site in sites)
@pytest.mark.parametrize("body", [
'if Path("/proc").exists():\n os.listdir("/proc")',
'if not Path("/proc").exists():\n return\nos.listdir("/proc")',
'if not has_procfs():\n raise RuntimeError()\nos.listdir(PROC_ROOT)',
'if has_procfs():\n pass\nelse:\n return\nPROC_ROOT.iterdir()',
'if ready and has_procfs():\n os.scandir("/proc")',
'if not has_procfs() or disabled:\n return\nPROC_ROOT.glob("*")',
'if sys.platform == "linux" and has_procfs():\n PROC_ROOT.iterdir()',
'for item in items:\n if not has_procfs():\n continue\n os.listdir("/proc")',
'if os.path.isdir("/proc"):\n os.listdir("/proc")',
'if has_procfs() and list(PROC_ROOT.iterdir()):\n pass',
'if not has_procfs() or list(PROC_ROOT.iterdir()):\n pass',
'[pid for item in items if has_procfs() for pid in os.listdir("/proc")]',
], ids=["positive-branch", "early-return", "early-raise", "else-exits", "and",
"negative-or", "platform-and-procfs", "loop-continue", "isdir",
"short-circuit-and", "short-circuit-or", "guarded-comprehension"])
def test_guard_detector_preserves_effective_checks(body):
tree = ast.parse("def scan():\n" + "\n".join(" " + line for line in body.splitlines()))
sites = _scan_sites(tree)
assert sites, "positive control must contain a recognized enumeration"
assert all(id(site) in _guarded_scan_ids(tree) for site in sites)