fix(security): harden trace teardown and gallery error responses

- CodeQL #1202 (tests/e2e/teardown.js): replace execSync shell execution with
  execFileSync, passing python executable and sanitizer arguments separately with
  shell: false. Preserve ODYSSEUS_TEST_PYTHON support, timeout handling, and
  fail-closed deletion.
- CodeQL #1203 (routes/gallery/gallery_routes.py): prevent exception information
  exposure in /api/image/edit-progress by returning a generic rejection response
  without leaking internal hostnames, IPs, resolver errors, or credentials.
- Add regression tests covering paths with spaces, metacharacters, synthetic command
  substitutions, process failure modes, fail-closed artifact purge, and SSRF
  rejection confidentiality.

tests/e2e/teardown.js

@@ -1,6 +1,6 @@
 const fs = require('node:fs');
 const path = require('node:path');
-const { execSync } = require('node:child_process');
+const { execFileSync } = require('node:child_process');
 
 module.exports = async () => {
   const dataDirectory = process.env.PHOTO_EDITOR_E2E_DATA_DIR;
@@ -13,8 +13,8 @@ module.exports = async () => {
   // Secure default tracing: never publish trace archives in CI or without explicit opt-in.
   // When traces are not opted-in, purge all archives unconditionally.
   // When local opt-in tracing is active, run the strict fail-closed structured sanitizer.
-  const resultsDir = path.join(__dirname, '..', '..', 'test-results');
-  const reportDir = path.join(__dirname, '..', '..', 'playwright-report');
+  const resultsDir = process.env.ODYSSEUS_TEST_RESULTS_DIR || path.join(__dirname, '..', '..', 'test-results');
+  const reportDir = process.env.ODYSSEUS_TEST_REPORT_DIR || path.join(__dirname, '..', '..', 'playwright-report');
   if (fs.existsSync(reportDir)) {
     try {
       fs.rmSync(reportDir, { recursive: true, force: true });
@@ -65,12 +65,14 @@ module.exports = async () => {
       purgeSensitiveArtifacts(resultsDir);
     } else {
       const python = process.env.ODYSSEUS_TEST_PYTHON || 'python';
-      const sanitizerScript = path.join(__dirname, '..', '..', 'scripts', 'sanitize_trace_artifacts.py');
+      const sanitizerScript = process.env.ODYSSEUS_TEST_SANITIZER_SCRIPT || path.join(__dirname, '..', '..', 'scripts', 'sanitize_trace_artifacts.py');
+      const timeout = Number(process.env.ODYSSEUS_TEST_SANITIZER_TIMEOUT_MS) || 30_000;
       let sanitizerSucceeded = false;
       try {
-        execSync(`${JSON.stringify(python)} ${JSON.stringify(sanitizerScript)} ${JSON.stringify(resultsDir)}`, {
+        execFileSync(python, [sanitizerScript, resultsDir], {
           stdio: 'ignore',
-          timeout: 30_000,
+          timeout,
+          shell: false,
         });
         sanitizerSucceeded = true;
       } catch (_) {

tests/test_gallery_endpoint_enforcement.py

@@ -220,7 +220,7 @@ def test_edit_progress_blocks_private_ip_when_policy_enabled(mock_provider_liste
 
         assert isinstance(resp, dict)
         assert resp["available"] is False
-        assert "rejected" in str(resp.get("reason"))
+        assert resp.get("reason") == "endpoint URL rejected"
         # Zero outbound polling because private IP was blocked by policy!
         assert len(TrackingPollHandler.requests_received) == 0
     finally:
@@ -229,6 +229,116 @@ def test_edit_progress_blocks_private_ip_when_policy_enabled(mock_provider_liste
         db.close()
 
 
+def test_edit_progress_rejection_does_not_expose_exception_details_or_secrets(mock_provider_listener, caplog):
+    """CodeQL #1203: Outbound URL rejection must never leak exception strings or internal destinations."""
+    db = SessionLocal()
+    ep_id = "ep-test-sensitive-exception"
+    synthetic_secret = "synthetic-resolver-secret-marker-42"
+    internal_host = "cluster-internal-worker.prod.corp"
+    internal_ip = "10.128.0.42"
+    try:
+        ep = ModelEndpoint(
+            id=ep_id,
+            name="Internal Worker Endpoint",
+            base_url=f"http://user:{synthetic_secret}@{internal_host}:8000",
+            model_type="image",
+            is_enabled=True,
+            owner="alice",
+            cached_models=json.dumps(["sd-v1"]),
+        )
+        db.merge(ep)
+        db.commit()
+
+        router = setup_gallery_routes()
+        route = next(r for r in router.routes if r.path == "/api/image/edit-progress")
+
+        req = create_gallery_request(
+            "GET",
+            "/api/image/edit-progress",
+            query_params={"request_id": "req-safe-test", "endpoint_id": ep_id},
+            user="alice",
+        )
+
+        synthetic_rejection_reason = (
+            f"host does not resolve: [Errno -2] Name or service not known: "
+            f"http://user:{synthetic_secret}@{internal_host} -> {internal_ip}"
+        )
+
+        with patch("routes.gallery.gallery_routes.require_privilege", return_value="alice"):
+            with patch("src.url_safety.check_outbound_url", return_value=(False, synthetic_rejection_reason)):
+                TrackingPollHandler.requests_received = []
+                resp = asyncio.run(route.endpoint(req))
+
+        # 1. Operation is rejected with a stable generic reason
+        assert isinstance(resp, dict)
+        assert resp["available"] is False
+        assert resp.get("reason") == "endpoint URL rejected"
+
+        # 2. No synthetic secrets or internal host details exposed to API client
+        resp_str = json.dumps(resp)
+        assert synthetic_secret not in resp_str
+        assert internal_host not in resp_str
+        assert internal_ip not in resp_str
+        assert "Errno" not in resp_str
+        assert "resolve" not in resp_str
+
+        # 3. No outbound HTTP request occurred
+        assert len(TrackingPollHandler.requests_received) == 0
+
+        # 4. Server-side log recorded rejection diagnostic without raw credentials
+        assert any("edit-progress endpoint rejected" in rec.message for rec in caplog.records)
+        for rec in caplog.records:
+            assert synthetic_secret not in rec.message
+    finally:
+        db.query(ModelEndpoint).filter(ModelEndpoint.id == ep_id).delete()
+        db.commit()
+        db.close()
+
+
+def test_edit_progress_valid_endpoint_succeeds(mock_provider_listener, monkeypatch):
+    """Valid authorized endpoint allows progress polling and returns provider payload."""
+    monkeypatch.setenv("IMAGE_BLOCK_PRIVATE_IPS", "false")
+    db = SessionLocal()
+    ep_id = "ep-test-valid-progress"
+    try:
+        ep = ModelEndpoint(
+            id=ep_id,
+            name="Valid SD Provider",
+            base_url=mock_provider_listener,
+            model_type="image",
+            is_enabled=True,
+            owner="alice",
+            cached_models=json.dumps(["sd-v1"]),
+        )
+        db.merge(ep)
+        db.commit()
+
+        router = setup_gallery_routes()
+        route = next(r for r in router.routes if r.path == "/api/image/edit-progress")
+
+        req = create_gallery_request(
+            "GET",
+            "/api/image/edit-progress",
+            query_params={"request_id": "valid-progress-req", "endpoint_id": ep_id},
+            user="alice",
+        )
+
+        with patch("routes.gallery.gallery_routes.require_privilege", return_value="alice"):
+            TrackingPollHandler.requests_received = []
+            resp = asyncio.run(route.endpoint(req))
+
+        assert isinstance(resp, dict)
+        assert resp["available"] is True
+        assert resp.get("progress") == 50
+        # Exactly one outbound request to the provider progress route
+        assert len(TrackingPollHandler.requests_received) == 1
+        assert TrackingPollHandler.requests_received[0].endswith("/images/progress/valid-progress-req")
+    finally:
+        db.query(ModelEndpoint).filter(ModelEndpoint.id == ep_id).delete()
+        db.commit()
+        db.close()
+
+
 def test_harmonize_rejects_chat_only_endpoint(mock_provider_listener):
     """POST /api/image/harmonize rejects a chat-only endpoint that has no image capability."""
     db = SessionLocal()

tests/test_playwright_teardown_security.py

@@ -0,0 +1,256 @@
+"""Regression tests for Playwright global teardown security hardening (CodeQL #1202).
+
+Validates that:
+1. Environment variables and paths passed to teardown cannot trigger shell command execution.
+2. Synthetic command substitutions and shell metacharacters fail safely without invoking a shell.
+3. Paths containing spaces and non-standard characters are preserved without word-splitting.
+4. Process errors (missing executable, nonzero exit, timeout) fail closed by purging artifacts.
+5. Secure default tracing (unconditional CI suppression and local default purging) remains enforced.
+6. Opt-in local runs execute the sanitizer and preserve sanitized archives.
+"""
+
+import json
+import os
+import subprocess
+import sys
+import tempfile
+import zipfile
+from pathlib import Path
+
+import pytest
+
+REPO_ROOT = Path(__file__).resolve().parents[1]
+TEARDOWN_PATH = REPO_ROOT / "tests" / "e2e" / "teardown.js"
+
+
+def _run_teardown(env: dict) -> subprocess.CompletedProcess:
+    """Run teardown.js in an isolated Node process with custom environment."""
+    full_env = os.environ.copy()
+    full_env.update(env)
+    script = (
+        "const teardown = require('./tests/e2e/teardown.js');\n"
+        "teardown().then(() => process.exit(0)).catch((err) => {\n"
+        "  console.error(err);\n"
+        "  process.exit(1);\n"
+        "});\n"
+    )
+    return subprocess.run(
+        ["node", "-e", script],
+        cwd=REPO_ROOT,
+        env=full_env,
+        capture_output=True,
+        text=True,
+    )
+
+
+def test_teardown_unconditional_ci_suppression(tmp_path):
+    """In CI environments, all sensitive trace artifacts are purged unconditionally."""
+    results_dir = tmp_path / "test-results"
+    results_dir.mkdir()
+    (results_dir / "trace.zip").write_bytes(b"PK0304 sensitive trace archive")
+    (results_dir / "session.har").write_text('{"log": {"entries": []}}')
+    (results_dir / "failure.png").write_bytes(b"\x89PNG sensitive screenshot")
+    (results_dir / "temp.tmp").write_text("in-progress trace data")
+
+    res = _run_teardown({
+        "CI": "true",
+        "GITHUB_ACTIONS": "true",
+        "PLAYWRIGHT_ENABLE_TRACE": "1",  # Even with opt-in, CI must purge unconditionally
+        "ODYSSEUS_TEST_RESULTS_DIR": str(results_dir),
+    })
+    assert res.returncode == 0
+    # All sensitive artifacts must be purged
+    assert not (results_dir / "trace.zip").exists()
+    assert not (results_dir / "session.har").exists()
+    assert not (results_dir / "failure.png").exists()
+    assert not (results_dir / "temp.tmp").exists()
+
+
+def test_teardown_local_default_purges_without_opt_in(tmp_path):
+    """Local runs without explicit PLAYWRIGHT_ENABLE_TRACE=1 purge artifacts."""
+    results_dir = tmp_path / "test-results"
+    results_dir.mkdir()
+    (results_dir / "trace.zip").write_bytes(b"PK0304 trace")
+    (results_dir / "session.har").write_text('{"log": {}}')
+    (results_dir / "failure.png").write_bytes(b"\x89PNG")
+    (results_dir / "temp.tmp").write_text("temp")
+
+    res = _run_teardown({
+        "CI": "",
+        "GITHUB_ACTIONS": "",
+        "PLAYWRIGHT_ENABLE_TRACE": "0",  # No opt-in
+        "ODYSSEUS_TEST_RESULTS_DIR": str(results_dir),
+    })
+    assert res.returncode == 0
+    assert not (results_dir / "trace.zip").exists()
+    assert not (results_dir / "session.har").exists()
+    assert not (results_dir / "failure.png").exists()
+    assert not (results_dir / "temp.tmp").exists()
+
+
+def test_teardown_sanitizer_success_with_opt_in(tmp_path):
+    """Opt-in local runs execute the Python sanitizer and retain sanitized archives."""
+    results_dir = tmp_path / "test-results"
+    results_dir.mkdir()
+    trace_file = results_dir / "trace.zip"
+    with zipfile.ZipFile(trace_file, "w") as z:
+        action = {"type": "action", "name": "fill", "params": {"value": "secret_password"}}
+        z.writestr("0-trace.trace", json.dumps(action).encode() + b"\n")
+
+    res = _run_teardown({
+        "CI": "",
+        "GITHUB_ACTIONS": "",
+        "PLAYWRIGHT_ENABLE_TRACE": "1",
+        "ODYSSEUS_TEST_PYTHON": sys.executable,
+        "ODYSSEUS_TEST_RESULTS_DIR": str(results_dir),
+    })
+    assert res.returncode == 0
+    # Archive is retained because sanitizer succeeded
+    assert trace_file.exists()
+    with zipfile.ZipFile(trace_file, "r") as z:
+        content = json.loads(z.read("0-trace.trace").decode())
+        assert content["params"]["value"] == "[REDACTED]"
+
+
+def test_teardown_missing_python_executable_fails_closed(tmp_path):
+    """If the python executable is missing, teardown fails closed and purges archives."""
+    results_dir = tmp_path / "test-results"
+    results_dir.mkdir()
+    trace_file = results_dir / "trace.zip"
+    trace_file.write_bytes(b"PK0304 trace")
+    tmp_file = results_dir / "trace.tmp"
+    tmp_file.write_text("trace tmp")
+
+    res = _run_teardown({
+        "CI": "",
+        "GITHUB_ACTIONS": "",
+        "PLAYWRIGHT_ENABLE_TRACE": "1",
+        "ODYSSEUS_TEST_PYTHON": "/usr/bin/nonexistent_python_binary_xyz_404",
+        "ODYSSEUS_TEST_RESULTS_DIR": str(results_dir),
+    })
+    assert res.returncode == 0
+    # Fail closed: archives must be purged
+    assert not trace_file.exists()
+    assert not tmp_file.exists()
+
+
+def test_teardown_nonzero_sanitizer_exit_fails_closed(tmp_path):
+    """If the sanitizer exits nonzero, teardown fails closed and purges archives."""
+    failing_script = tmp_path / "fail_sanitizer.py"
+    failing_script.write_text("import sys; sys.exit(2)\n")
+
+    results_dir = tmp_path / "test-results"
+    results_dir.mkdir()
+    trace_file = results_dir / "trace.zip"
+    trace_file.write_bytes(b"PK0304 trace")
+    tmp_file = results_dir / "trace.tmp"
+    tmp_file.write_text("trace tmp")
+
+    res = _run_teardown({
+        "CI": "",
+        "GITHUB_ACTIONS": "",
+        "PLAYWRIGHT_ENABLE_TRACE": "1",
+        "ODYSSEUS_TEST_PYTHON": sys.executable,
+        "ODYSSEUS_TEST_SANITIZER_SCRIPT": str(failing_script),
+        "ODYSSEUS_TEST_RESULTS_DIR": str(results_dir),
+    })
+    assert res.returncode == 0
+    assert not trace_file.exists()
+    assert not tmp_file.exists()
+
+
+def test_teardown_sanitizer_timeout_fails_closed(tmp_path):
+    """If the sanitizer hangs or exceeds timeout, teardown fails closed and purges archives."""
+    hanging_script = tmp_path / "hang_sanitizer.py"
+    hanging_script.write_text("import time; time.sleep(5)\n")
+
+    results_dir = tmp_path / "test-results"
+    results_dir.mkdir()
+    trace_file = results_dir / "trace.zip"
+    trace_file.write_bytes(b"PK0304 trace")
+    tmp_file = results_dir / "trace.tmp"
+    tmp_file.write_text("trace tmp")
+
+    res = _run_teardown({
+        "CI": "",
+        "GITHUB_ACTIONS": "",
+        "PLAYWRIGHT_ENABLE_TRACE": "1",
+        "ODYSSEUS_TEST_PYTHON": sys.executable,
+        "ODYSSEUS_TEST_SANITIZER_SCRIPT": str(hanging_script),
+        "ODYSSEUS_TEST_SANITIZER_TIMEOUT_MS": "300",  # 300ms timeout
+        "ODYSSEUS_TEST_RESULTS_DIR": str(results_dir),
+    })
+    assert res.returncode == 0
+    # Fail closed: timeout killed the process and purged archives
+    assert not trace_file.exists()
+    assert not tmp_file.exists()
+
+
+def test_teardown_handles_paths_containing_spaces(tmp_path):
+    """Paths with spaces in results directory and sanitizer script execute cleanly."""
+    spaced_root = tmp_path / "space dir test" / "sub dir results"
+    spaced_root.mkdir(parents=True)
+    trace_file = spaced_root / "trace.zip"
+    with zipfile.ZipFile(trace_file, "w") as z:
+        action = {"type": "action", "params": {"value": "secret123"}}
+        z.writestr("0.trace", json.dumps(action).encode() + b"\n")
+
+    spaced_script = tmp_path / "sanitizer tools" / "run sanitizer.py"
+    spaced_script.parent.mkdir(parents=True)
+    spaced_script.write_text(
+        "import sys, json, zipfile, os\n"
+        "target = sys.argv[1]\n"
+        "assert os.path.exists(target)\n"
+        "marker = os.path.join(target, 'executed.marker')\n"
+        "with open(marker, 'w') as f: f.write('ok')\n"
+    )
+
+    res = _run_teardown({
+        "CI": "",
+        "GITHUB_ACTIONS": "",
+        "PLAYWRIGHT_ENABLE_TRACE": "1",
+        "ODYSSEUS_TEST_PYTHON": sys.executable,
+        "ODYSSEUS_TEST_SANITIZER_SCRIPT": str(spaced_script),
+        "ODYSSEUS_TEST_RESULTS_DIR": str(spaced_root),
+    })
+    assert res.returncode == 0
+    assert (spaced_root / "executed.marker").exists()
+
+
+@pytest.mark.parametrize("payload", [
+    '$(touch "{marker}")',
+    '`touch "{marker}"`',
+    '; touch "{marker}"',
+    '&& touch "{marker}"',
+    '|| touch "{marker}"',
+    '| touch "{marker}"',
+    '& touch "{marker}"',
+    '> "{marker}"',
+])
+def test_teardown_synthetic_command_substitution_and_metacharacters(tmp_path, payload):
+    """Untrusted environment strings in ODYSSEUS_TEST_PYTHON cannot execute shell commands.
+
+    With execFileSync and shell: false, any shell metacharacters and command substitutions
+    are treated strictly as literal filename characters for execve, resulting in ENOENT
+    without invoking any subshell or creating the marker file.
+    """
+    marker = tmp_path / "shell_pwned_marker.txt"
+    results_dir = tmp_path / "results"
+    results_dir.mkdir()
+    trace_file = results_dir / "trace.zip"
+    trace_file.write_bytes(b"PK0304 trace")
+
+    formatted_payload = f"python {payload.format(marker=str(marker))}"
+
+    res = _run_teardown({
+        "CI": "",
+        "GITHUB_ACTIONS": "",
+        "PLAYWRIGHT_ENABLE_TRACE": "1",
+        "ODYSSEUS_TEST_PYTHON": formatted_payload,
+        "ODYSSEUS_TEST_RESULTS_DIR": str(results_dir),
+    })
+    assert res.returncode == 0
+    # The shell marker MUST NOT have been created!
+    assert not marker.exists(), f"Security failure: shell command executed for payload {payload}!"
+    # Fail closed: trace must be purged because python execution failed
+    assert not trace_file.exists()