diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1b4a0b3..d48ce44 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -548,9 +548,27 @@ jobs: - name: Lint run: npm run lint + - name: Run aiSettings tests + run: node src/utils/aiApi.test.mjs + - name: Build run: npm run build + website: + name: Website (script tests) + runs-on: ubuntu-latest + steps: + - name: Checkout repository + uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1 + + - name: Set up Node.js + uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4.4.0 + with: + node-version: "22" + + - name: Run website script tests + run: node website/test_toEmbedUrl.mjs + # ── Enforce branch flow: main may only receive PRs from dev ─────────────── # No-op on dev PRs (step skipped -> job succeeds). To allow hotfixes straight # to main, widen the condition below to also accept a 'hotfix/*' head branch. @@ -575,7 +593,7 @@ jobs: name: CI Summary runs-on: ubuntu-latest needs: - [lint, rule-validation, secret-scan, sast-bandit, sca-pip-audit, sbom, container-scan, backend-tests, frontend, enforce-source-branch] + [lint, rule-validation, secret-scan, sast-bandit, sca-pip-audit, sbom, container-scan, backend-tests, frontend, website, enforce-source-branch] if: always() steps: - name: Build summary @@ -589,6 +607,7 @@ jobs: CONTAINER_SCAN: ${{ needs.container-scan.result }} BACKEND_TESTS: ${{ needs.backend-tests.result }} FRONTEND: ${{ needs.frontend.result }} + WEBSITE: ${{ needs.website.result }} ENFORCE_SOURCE: ${{ needs.enforce-source-branch.result }} run: | python3 - <<'PYEOF' @@ -604,6 +623,7 @@ jobs: ("Container Scan (Trivy, INFRA 1 pending)", os.environ["CONTAINER_SCAN"]), ("Backend Tests (pytest + coverage)", os.environ["BACKEND_TESTS"]), ("Frontend (lint + build)", os.environ["FRONTEND"]), + ("Website (script tests)", os.environ["WEBSITE"]), ("Enforce dev to main source", os.environ["ENFORCE_SOURCE"]), ] @@ -654,5 +674,6 @@ jobs: needs.sbom.result != 'success' || needs.backend-tests.result != 'success' || needs.frontend.result != 'success' || + needs.website.result != 'success' || needs.enforce-source-branch.result != 'success' run: exit 1 diff --git a/api/app.py b/api/app.py index 738826a..34f5163 100644 --- a/api/app.py +++ b/api/app.py @@ -244,7 +244,8 @@ def ready(): @app.errorhandler(400) def bad_request(exc): - return jsonify({"error": "Bad request", "detail": str(exc), "request_id": get_request_id()}), 400 + logger.warning("Bad request: %s", exc) + return jsonify({"error": "Bad request", "request_id": get_request_id()}), 400 @app.errorhandler(401) def unauthorized(exc): diff --git a/api/routes/ai.py b/api/routes/ai.py index 5059ea4..776796a 100644 --- a/api/routes/ai.py +++ b/api/routes/ai.py @@ -158,6 +158,25 @@ def _read_request(): return body, None +_AI_ERROR_MESSAGES = { + 503: "AI knowledge base is not available", + 400: "Invalid AI request parameters", + 502: "AI provider request failed", +} + + +def _ai_error_response(exc: Exception, status: int, log_context: str): + """Return a safe, generic error response, logging the real exception server-side. + + Never reflect str(exc) back to the client: get_completion() wraps + third-party provider errors (e.g. "Anthropic request failed: {exc}"), + which could carry response bodies or connection details from the + underlying HTTP client that shouldn't reach the caller. + """ + logger.error("%s: %s", log_context, exc) + return jsonify({"error": _AI_ERROR_MESSAGES.get(status, "AI request failed")}), status + + @ai_bp.post("/api/ai/insights") @rate_limit(_AI_RATE_LIMIT) def insights(): @@ -223,7 +242,7 @@ def ai_summary(): try: context, sources = _context_for(findings_text) except VectorStoreNotBuilt as exc: - return jsonify({"error": str(exc)}), 503 + return _ai_error_response(exc, 503, "Vector store unavailable in ai_summary") prompt = ( "You are a cloud security advisor. Using ONLY the grounded knowledge " @@ -234,9 +253,9 @@ def ai_summary(): try: answer = get_completion(body["provider"], body["api_key"], prompt, model=body.get("model")) except ValueError as exc: - return jsonify({"error": str(exc)}), 400 + return _ai_error_response(exc, 400, "Invalid request in ai_summary") except RuntimeError as exc: - return jsonify({"error": str(exc)}), 502 + return _ai_error_response(exc, 502, "Provider failure in ai_summary") return jsonify( { @@ -262,7 +281,7 @@ def ai_prioritise(): try: context, sources = _context_for(findings_text) except VectorStoreNotBuilt as exc: - return jsonify({"error": str(exc)}), 503 + return _ai_error_response(exc, 503, "Vector store unavailable in ai_prioritise") prompt = ( "You are a cloud security advisor. Using ONLY the grounded knowledge " @@ -275,9 +294,9 @@ def ai_prioritise(): try: raw = get_completion(body["provider"], body["api_key"], prompt, model=body.get("model")) except ValueError as exc: - return jsonify({"error": str(exc)}), 400 + return _ai_error_response(exc, 400, "Invalid request in ai_prioritise") except RuntimeError as exc: - return jsonify({"error": str(exc)}), 502 + return _ai_error_response(exc, 502, "Provider failure in ai_prioritise") try: prioritised = json.loads(raw) @@ -307,7 +326,7 @@ def ai_ask(): try: context, sources = _context_for(question) except VectorStoreNotBuilt as exc: - return jsonify({"error": str(exc)}), 503 + return _ai_error_response(exc, 503, "Vector store unavailable in ai_ask") findings = body.get("findings", []) findings_text = _findings_to_text(findings) if findings else "Not provided." @@ -322,9 +341,9 @@ def ai_ask(): try: answer = get_completion(body["provider"], body["api_key"], prompt, model=body.get("model")) except ValueError as exc: - return jsonify({"error": str(exc)}), 400 + return _ai_error_response(exc, 400, "Invalid request in ai_ask") except RuntimeError as exc: - return jsonify({"error": str(exc)}), 502 + return _ai_error_response(exc, 502, "Provider failure in ai_ask") return jsonify( { @@ -352,15 +371,15 @@ def ai_threat_simulation(): try: context, sources = _context_for(findings_text) except VectorStoreNotBuilt as exc: - return jsonify({"error": str(exc)}), 503 + return _ai_error_response(exc, 503, "Vector store unavailable in ai_threat_simulation") prompt = _build_threat_simulation_prompt(findings_text, context) try: raw = get_completion(body["provider"], body["api_key"], prompt, model=body.get("model")) except ValueError as exc: - return jsonify({"error": str(exc)}), 400 + return _ai_error_response(exc, 400, "Invalid request in ai_threat_simulation") except RuntimeError as exc: - return jsonify({"error": str(exc)}), 502 + return _ai_error_response(exc, 502, "Provider failure in ai_threat_simulation") try: simulation = json.loads(raw) diff --git a/api/routes/compliance.py b/api/routes/compliance.py index 45dfef8..df65bdf 100644 --- a/api/routes/compliance.py +++ b/api/routes/compliance.py @@ -47,7 +47,8 @@ def get_compliance(framework: str): return jsonify(result) except FileNotFoundError as exc: - return jsonify({"error": f"Frameworks directory not found: {exc}"}), 500 + logger.error("Frameworks directory not found: %s", exc) + return jsonify({"error": "Compliance frameworks are not available"}), 500 except Exception as exc: logger.error("Failed to retrieve compliance score for %s: %s", framework, exc) - return jsonify({"error": "Compliance calculation failed", "detail": str(exc)}), 500 + return jsonify({"error": "Compliance calculation failed"}), 500 diff --git a/api/routes/drift.py b/api/routes/drift.py index 3ed61f4..427fa3d 100644 --- a/api/routes/drift.py +++ b/api/routes/drift.py @@ -147,4 +147,4 @@ def _rg(resource_id: str) -> str: except Exception as exc: logger.error("Failed to compute drift: %s", exc) - return jsonify({"error": "Failed to retrieve drift", "detail": str(exc)}), 500 + return jsonify({"error": "Failed to retrieve drift"}), 500 diff --git a/api/routes/findings.py b/api/routes/findings.py index 3a4939b..91f6afb 100644 --- a/api/routes/findings.py +++ b/api/routes/findings.py @@ -43,7 +43,7 @@ def list_findings(): return jsonify({"count": len(findings), "findings": findings}) except Exception as exc: logger.error("Failed to list findings: %s", exc) - return jsonify({"error": "Failed to retrieve findings", "detail": str(exc)}), 500 + return jsonify({"error": "Failed to retrieve findings"}), 500 @findings_bp.get("/api/findings/") @@ -57,7 +57,7 @@ def get_finding(finding_id: int): return jsonify(finding) except Exception as exc: logger.error("Failed to get finding %d: %s", finding_id, exc) - return jsonify({"error": "Database error", "detail": str(exc)}), 500 + return jsonify({"error": "Database error"}), 500 @findings_bp.get("/api/findings//playbook") @@ -126,4 +126,4 @@ def get_playbook(finding_id: int): except Exception as exc: logger.error("Failed to get playbook for finding %d: %s", finding_id, exc) - return jsonify({"error": "Failed to retrieve playbook", "detail": str(exc)}), 500 + return jsonify({"error": "Failed to retrieve playbook"}), 500 diff --git a/api/routes/prioritization.py b/api/routes/prioritization.py index 3353252..d19fe3d 100644 --- a/api/routes/prioritization.py +++ b/api/routes/prioritization.py @@ -180,4 +180,4 @@ def get_prioritization(): except Exception as exc: logger.error("Failed to build prioritization: %s", exc) - return jsonify({"error": "Failed to retrieve prioritization", "detail": str(exc)}), 500 + return jsonify({"error": "Failed to retrieve prioritization"}), 500 diff --git a/api/routes/resources.py b/api/routes/resources.py index ad0a4db..56a0309 100644 --- a/api/routes/resources.py +++ b/api/routes/resources.py @@ -121,4 +121,4 @@ def get_resources(): except Exception as exc: logger.error("Failed to build resources: %s", exc) - return jsonify({"error": "Failed to retrieve resources", "detail": str(exc)}), 500 + return jsonify({"error": "Failed to retrieve resources"}), 500 diff --git a/api/routes/scans.py b/api/routes/scans.py index c87914a..1d2f275 100644 --- a/api/routes/scans.py +++ b/api/routes/scans.py @@ -32,7 +32,7 @@ def list_scans(): return jsonify(result) except Exception as exc: logger.error("Failed to list scans: %s", exc) - return jsonify({"error": "Failed to retrieve scans", "detail": str(exc)}), 500 + return jsonify({"error": "Failed to retrieve scans"}), 500 @scans_bp.get("/api/scans/") @@ -46,7 +46,7 @@ def get_scan_status(scan_id): return jsonify(scan) except Exception as exc: logger.error("Failed to get scan status: %s", exc) - return jsonify({"error": "Database error", "detail": str(exc)}), 500 + return jsonify({"error": "Database error"}), 500 @scans_bp.post("/api/scans/trigger") @@ -73,7 +73,7 @@ def trigger_scan(): db.create_pending_scan(scan_id, subscription_id) except Exception as exc: logger.error("Failed to create pending scan: %s", exc, exc_info=True) - return jsonify({"error": "Database error", "detail": str(exc)}), 500 + return jsonify({"error": "Database error"}), 500 return jsonify( {"scan_id": scan_id, "status": "pending", "message": "Scan has been queued and will start shortly."} @@ -81,7 +81,7 @@ def trigger_scan(): except Exception as exc: logger.error("Critical error in trigger_scan route: %s", exc, exc_info=True) - return jsonify({"error": "Critical route failure", "detail": str(exc)}), 500 + return jsonify({"error": "Critical route failure"}), 500 def _run_enrichment_in_background(scan_id: str, findings: list, db_url: str) -> None: @@ -162,4 +162,4 @@ def enrich_scan(scan_id): except Exception as exc: logger.error("Failed to start enrichment for scan %s: %s", scan_id, exc) - return jsonify({"error": "Internal server error", "detail": str(exc)}), 500 + return jsonify({"error": "Internal server error"}), 500 diff --git a/api/routes/score.py b/api/routes/score.py index 9d0e125..22d157d 100644 --- a/api/routes/score.py +++ b/api/routes/score.py @@ -34,7 +34,7 @@ def get_score(): return jsonify(result) except Exception as exc: logger.error("Failed to calculate score: %s", exc) - return jsonify({"error": "Failed to calculate score", "detail": str(exc)}), 500 + return jsonify({"error": "Failed to calculate score"}), 500 @score_bp.get("/api/score/cve-summary") @@ -46,4 +46,4 @@ def get_cve_summary(): return jsonify(result) except Exception as exc: logger.error("Failed to fetch CVE summary: %s", exc) - return jsonify({"error": "Failed to fetch CVE summary", "detail": str(exc)}), 500 + return jsonify({"error": "Failed to fetch CVE summary"}), 500 diff --git a/frontend/src/utils/aiApi.js b/frontend/src/utils/aiApi.js index d393990..e7cf108 100644 --- a/frontend/src/utils/aiApi.js +++ b/frontend/src/utils/aiApi.js @@ -15,19 +15,27 @@ const API_BASE = import.meta.env.VITE_API_URL || (import.meta.env.DEV ? 'http://localhost:5000' : 'https://openshield-api.onrender.com'); const TIMEOUT = 30000; -// ── Provider settings stored in localStorage ─────────────────────────────── +// ── Provider settings ─────────────────────────────────────────────────────── +// The API key is the user's own bring-your-own AI provider credential. It is +// kept in memory only for the life of the page — never written to +// localStorage/sessionStorage, which any script running on the page can read +// (XSS-exfiltrable) and which persists indefinitely. The trade-off is that +// the key does not survive a page reload; provider/model (not secret) still +// persist in localStorage as before. +let apiKeyInMemory = ''; + export const aiSettings = { getProvider: () => localStorage.getItem('ai_provider') || 'anthropic', - getApiKey: () => localStorage.getItem('ai_api_key') || '', + getApiKey: () => apiKeyInMemory, getModel: () => localStorage.getItem('ai_model') || '', save: ({ provider, apiKey, model }) => { - if (provider) localStorage.setItem('ai_provider', provider); - if (apiKey !== undefined) /* key storage removed */; - if (model !== undefined) localStorage.setItem('ai_model', model || ''); + if (provider) localStorage.setItem('ai_provider', provider); + if (apiKey !== undefined) apiKeyInMemory = apiKey; + if (model !== undefined) localStorage.setItem('ai_model', model || ''); }, - isConfigured: () => !!localStorage.getItem('ai_api_key'), + isConfigured: () => !!apiKeyInMemory, clear: () => { - localStorage.removeItem('ai_api_key'); + apiKeyInMemory = ''; localStorage.removeItem('ai_provider'); localStorage.removeItem('ai_model'); }, diff --git a/frontend/src/utils/aiApi.test.mjs b/frontend/src/utils/aiApi.test.mjs new file mode 100644 index 0000000..2ec9e01 --- /dev/null +++ b/frontend/src/utils/aiApi.test.mjs @@ -0,0 +1,114 @@ +// Minimal, dependency-free test for aiSettings in aiApi.js (issue #180). +// +// frontend/ has no test runner configured (only eslint + vite). aiApi.js has +// no imports and only one Vite-only construct (import.meta.env), so this +// loads the real source (no duplication of the logic under test), replaces +// that one construct with a plain string, and evaluates it with a stubbed +// localStorage — no new devDependency required. +// +// Run with: node frontend/src/utils/aiApi.test.mjs + +import assert from 'node:assert/strict'; +import { readFileSync } from 'node:fs'; +import { fileURLToPath } from 'node:url'; +import path from 'node:path'; + +const __dirname = path.dirname(fileURLToPath(import.meta.url)); + +function loadAiApiModule() { + let source = readFileSync(path.join(__dirname, 'aiApi.js'), 'utf8'); + + // Neutralize the one Vite-only construct so this can run under plain Node. + source = source.replace( + /import\.meta\.env\.VITE_API_URL\s*\|\|\s*\(import\.meta\.env\.DEV \? '[^']*' : '[^']*'\)/, + "'http://localhost:5000'", + ); + assert.ok(!source.includes('import.meta'), 'failed to neutralize import.meta usage — test harness is stale'); + + // Turn `export const x = ...` into `const x = ...` and return the bindings + // via a wrapper function, so the real module body runs unmodified. + source = source.replace(/^export const /gm, 'const '); + source += '\nreturn { aiSettings, aiApi };'; + + const backingStore = new Map(); + const localStorageStub = { + getItem: (k) => (backingStore.has(k) ? backingStore.get(k) : null), + setItem: (k, v) => backingStore.set(k, String(v)), + removeItem: (k) => backingStore.delete(k), + }; + + const load = new Function('localStorage', 'fetch', 'AbortController', 'setTimeout', 'clearTimeout', source); + const noopFetch = () => Promise.reject(new Error('fetch should not be called in this test')); + const mod = load(localStorageStub, noopFetch, AbortController, setTimeout, clearTimeout); + return { ...mod, backingStore }; +} + +let failures = 0; +function check(description, fn) { + try { + fn(); + console.log(`PASS: ${description}`); + } catch (err) { + failures++; + console.error(`FAIL: ${description}\n ${err.message}`); + } +} + +// ── Fresh module per check: apiKeyInMemory is module-scoped state ────────── + +check('starts unconfigured with no key', () => { + const { aiSettings } = loadAiApiModule(); + assert.equal(aiSettings.isConfigured(), false); + assert.equal(aiSettings.getApiKey(), ''); +}); + +check('save() persists the key in memory and isConfigured() reflects it', () => { + const { aiSettings } = loadAiApiModule(); + aiSettings.save({ provider: 'anthropic', apiKey: 'sk-test-12345' }); + assert.equal(aiSettings.isConfigured(), true); + assert.equal(aiSettings.getApiKey(), 'sk-test-12345'); +}); + +check('the API key is never written to localStorage under any key', () => { + const { aiSettings, backingStore } = loadAiApiModule(); + aiSettings.save({ provider: 'groq', apiKey: 'super-secret-key-value', model: 'llama' }); + for (const storedValue of backingStore.values()) { + assert.ok( + !String(storedValue).includes('super-secret-key-value'), + `API key leaked into localStorage: ${JSON.stringify([...backingStore.entries()])}`, + ); + } +}); + +check('provider and model (non-secret) do persist in localStorage', () => { + const { aiSettings, backingStore } = loadAiApiModule(); + aiSettings.save({ provider: 'gemini', apiKey: 'irrelevant', model: 'gemini-pro' }); + assert.equal(backingStore.get('ai_provider'), 'gemini'); + assert.equal(backingStore.get('ai_model'), 'gemini-pro'); +}); + +check('clear() removes the in-memory key and localStorage entries', () => { + const { aiSettings, backingStore } = loadAiApiModule(); + aiSettings.save({ provider: 'anthropic', apiKey: 'to-be-cleared' }); + assert.equal(aiSettings.isConfigured(), true); + aiSettings.clear(); + assert.equal(aiSettings.isConfigured(), false); + assert.equal(aiSettings.getApiKey(), ''); + assert.equal(backingStore.has('ai_provider'), false); + assert.equal(backingStore.has('ai_model'), false); +}); + +check('a fresh module load never sees a key from a previous instance (no shared storage)', () => { + const first = loadAiApiModule(); + first.aiSettings.save({ provider: 'anthropic', apiKey: 'first-instance-key' }); + + const second = loadAiApiModule(); + assert.equal(second.aiSettings.isConfigured(), false); + assert.equal(second.aiSettings.getApiKey(), ''); +}); + +if (failures > 0) { + console.error(`\n${failures} test(s) failed`); + process.exit(1); +} +console.log('\nAll aiSettings tests passed'); diff --git a/tests/test_ai_error_exposure.py b/tests/test_ai_error_exposure.py new file mode 100644 index 0000000..781dd76 --- /dev/null +++ b/tests/test_ai_error_exposure.py @@ -0,0 +1,79 @@ +"""Tests that /api/ai/{summary,prioritise,ask,threat-simulation} never leak +exception internals to the client (issue #177, CodeQL: information exposure +through an exception). + +get_completion() in api/services/ai_provider.py wraps raw third-party +provider errors into RuntimeError messages (e.g. "Anthropic request failed: +{exc}"), which could carry response bodies or connection details from the +underlying HTTP client. These routes must never reflect that (or any other +exception's) message back to the caller. +""" + +import secrets +from unittest.mock import patch + +from ai.retriever import VectorStoreNotBuilt + +_LEAK_MARKER = "provider-internal-secret-token-abc123" + +_VALID_FINDINGS = [ + { + "rule_id": "AZ-NET-001", + "severity": "HIGH", + "title": "Example finding", + "description": "Example description.", + "remediation": "Example remediation.", + } +] + + +def _fake_api_key() -> str: + return secrets.token_urlsafe(24) + + +def _assert_no_leak(resp, expected_status): + assert resp.status_code == expected_status + body = resp.get_json() + assert _LEAK_MARKER not in str(body) + + +ENDPOINTS = { + "/api/ai/summary": {"provider": "anthropic", "api_key": None, "findings": _VALID_FINDINGS}, + "/api/ai/prioritise": {"provider": "anthropic", "api_key": None, "findings": _VALID_FINDINGS}, + "/api/ai/ask": {"provider": "anthropic", "api_key": None, "question": "What is my risk?"}, + "/api/ai/threat-simulation": {"provider": "anthropic", "api_key": None, "findings": _VALID_FINDINGS}, +} + + +def _payload(endpoint): + body = dict(ENDPOINTS[endpoint]) + body["api_key"] = _fake_api_key() + return body + + +@patch("api.routes.ai._context_for") +def test_vector_store_not_built_does_not_leak_exception(mock_context, client, auth_headers): + mock_context.side_effect = VectorStoreNotBuilt(_LEAK_MARKER) + for endpoint in ENDPOINTS: + resp = client.post(endpoint, json=_payload(endpoint), headers=auth_headers) + _assert_no_leak(resp, 503) + + +@patch("api.routes.ai.get_completion") +@patch("api.routes.ai._context_for") +def test_provider_value_error_does_not_leak_exception(mock_context, mock_gc, client, auth_headers): + mock_context.return_value = ("grounded context", []) + mock_gc.side_effect = ValueError(_LEAK_MARKER) + for endpoint in ENDPOINTS: + resp = client.post(endpoint, json=_payload(endpoint), headers=auth_headers) + _assert_no_leak(resp, 400) + + +@patch("api.routes.ai.get_completion") +@patch("api.routes.ai._context_for") +def test_provider_runtime_error_does_not_leak_exception(mock_context, mock_gc, client, auth_headers): + mock_context.return_value = ("grounded context", []) + mock_gc.side_effect = RuntimeError(f"Anthropic request failed: {_LEAK_MARKER}") + for endpoint in ENDPOINTS: + resp = client.post(endpoint, json=_payload(endpoint), headers=auth_headers) + _assert_no_leak(resp, 502) diff --git a/tests/test_error_exposure.py b/tests/test_error_exposure.py new file mode 100644 index 0000000..2322c78 --- /dev/null +++ b/tests/test_error_exposure.py @@ -0,0 +1,127 @@ +"""Tests that API error responses never leak exception internals (issue #177, +CodeQL: information exposure through an exception). + +Each route's _get_db() (or equivalent) is patched to raise, mirroring the +established per-module DB-mocking convention (see test_findings_playbook.py). +A secret-shaped marker string is used as the exception message so a leak +would be unambiguous rather than coincidentally matching real wording. +""" + +from unittest.mock import MagicMock, patch + +import api.routes.compliance as compliance_route +import api.routes.drift as drift_route +import api.routes.findings as findings_route +import api.routes.prioritization as prioritization_route +import api.routes.resources as resources_route +import api.routes.scans as scans_route +import api.routes.score as score_route + +_LEAK_MARKER = "connection to db-internal-host-secret-1234 refused" + + +def _raising_db(method_name): + db = MagicMock() + getattr(db, method_name).side_effect = RuntimeError(_LEAK_MARKER) + return db + + +def _assert_no_leak(resp, expected_status): + assert resp.status_code == expected_status + body = resp.get_json() + assert "detail" not in body + assert _LEAK_MARKER not in str(body) + + +def test_list_scans_error_does_not_leak_exception(client, auth_headers): + with patch.object(scans_route, "_get_db", return_value=_raising_db("get_scans")): + resp = client.get("/api/scans", headers=auth_headers) + _assert_no_leak(resp, 500) + + +def test_get_scan_status_error_does_not_leak_exception(client, auth_headers): + with patch.object(scans_route, "_get_db", return_value=_raising_db("get_scan")): + resp = client.get("/api/scans/some-id", headers=auth_headers) + _assert_no_leak(resp, 500) + + +def test_trigger_scan_db_error_does_not_leak_exception(client, auth_headers): + with patch.object(scans_route, "_get_db", return_value=_raising_db("create_pending_scan")): + resp = client.post( + "/api/scans/trigger", + json={"subscription_id": "00000000-0000-0000-0000-000000000001"}, + headers=auth_headers, + ) + _assert_no_leak(resp, 500) + + +def test_list_findings_error_does_not_leak_exception(client, auth_headers): + with patch.object(findings_route, "_get_db", return_value=_raising_db("get_findings")): + resp = client.get("/api/findings", headers=auth_headers) + _assert_no_leak(resp, 500) + + +def test_get_finding_error_does_not_leak_exception(client, auth_headers): + with patch.object(findings_route, "_get_db", return_value=_raising_db("get_finding_by_id")): + resp = client.get("/api/findings/1", headers=auth_headers) + _assert_no_leak(resp, 500) + + +def test_get_playbook_error_does_not_leak_exception(client, auth_headers): + with patch.object(findings_route, "_get_db", return_value=_raising_db("get_finding_by_id")): + resp = client.get("/api/findings/1/playbook", headers=auth_headers) + _assert_no_leak(resp, 500) + + +def test_score_error_does_not_leak_exception(client, auth_headers): + with patch.object(score_route, "_get_db", return_value=_raising_db("get_score")): + resp = client.get("/api/score", headers=auth_headers) + _assert_no_leak(resp, 500) + + +def test_cve_summary_error_does_not_leak_exception(client, auth_headers): + with patch.object(score_route, "_get_db", return_value=_raising_db("get_cve_summary")): + resp = client.get("/api/score/cve-summary", headers=auth_headers) + _assert_no_leak(resp, 500) + + +def test_resources_error_does_not_leak_exception(client, auth_headers): + with patch.object(resources_route, "_get_db", return_value=_raising_db("get_findings")): + resp = client.get("/api/resources", headers=auth_headers) + _assert_no_leak(resp, 500) + + +def test_prioritization_error_does_not_leak_exception(client, auth_headers): + with patch.object(prioritization_route, "_get_db", return_value=_raising_db("get_findings")): + resp = client.get("/api/prioritization", headers=auth_headers) + _assert_no_leak(resp, 500) + + +def test_drift_error_does_not_leak_exception(client, auth_headers): + with patch.object(drift_route, "_get_db", return_value=_raising_db("get_findings")): + resp = client.get("/api/drift", headers=auth_headers) + _assert_no_leak(resp, 500) + + +def test_compliance_generic_error_does_not_leak_exception(client, auth_headers): + with patch.object(compliance_route, "_get_db", return_value=_raising_db("get_compliance_score")): + resp = client.get("/api/compliance/cis", headers=auth_headers) + _assert_no_leak(resp, 500) + + +def test_compliance_missing_frameworks_dir_does_not_leak_exception(client, auth_headers): + db = MagicMock() + db.get_compliance_score.side_effect = FileNotFoundError(_LEAK_MARKER) + with patch.object(compliance_route, "_get_db", return_value=db): + resp = client.get("/api/compliance/cis", headers=auth_headers) + _assert_no_leak(resp, 500) + + +def test_bad_request_errorhandler_does_not_leak_exception(client, auth_headers): + resp = client.post( + "/api/scans/trigger", + data=b"{not valid json", + headers={**auth_headers, "Content-Type": "application/json"}, + ) + body = resp.get_json() + assert "detail" not in body diff --git a/website/script.js b/website/script.js index 24fecfb..3c08ba9 100644 --- a/website/script.js +++ b/website/script.js @@ -229,13 +229,27 @@ let selectedImageFile = null; // Base64 adds ~33% overhead, so the raw file must be under ~750 KB. const MAX_IMAGE_BYTES = 700 * 1024; +const EMBED_ALLOWED_HOSTS = new Set(['www.youtube.com', 'youtube.com', 'player.vimeo.com']); + function toEmbedUrl(raw) { if (!raw) return ''; const yt = raw.match(/(?:youtube\.com\/watch\?v=|youtu\.be\/)([a-zA-Z0-9_-]{11})/); if (yt) return `https://www.youtube.com/embed/${yt[1]}`; const vi = raw.match(/vimeo\.com\/(\d+)/); if (vi) return `https://player.vimeo.com/video/${vi[1]}`; - if (raw.includes('youtube.com/embed') || raw.includes('player.vimeo.com')) return raw; + + // Already-an-embed-URL fallback: verify the actual origin instead of a + // substring check, which a crafted string (e.g. containing + // "youtube.com/embed" anywhere but hosted elsewhere) can bypass and + // break out of the iframe's src="..." attribute when interpolated. + try { + const parsed = new URL(raw); + if (parsed.protocol === 'https:' && EMBED_ALLOWED_HOSTS.has(parsed.hostname)) { + return raw; + } + } catch { + // Not a valid absolute URL — fall through to reject below. + } return ''; } diff --git a/website/test_toEmbedUrl.mjs b/website/test_toEmbedUrl.mjs new file mode 100644 index 0000000..62274b5 --- /dev/null +++ b/website/test_toEmbedUrl.mjs @@ -0,0 +1,111 @@ +// Minimal, dependency-free test for toEmbedUrl() in script.js (issue #179). +// +// website/ is a plain static site with no build step and no existing test +// framework, so this loads the real script.js source via Node's built-in vm +// module (no duplication of the function under test) with just enough DOM +// stubbing for the file's top-level statements to execute without crashing. +// +// Run with: node website/test_toEmbedUrl.mjs + +import assert from 'node:assert/strict'; +import { readFileSync } from 'node:fs'; +import { fileURLToPath } from 'node:url'; +import path from 'node:path'; +import vm from 'node:vm'; + +const __dirname = path.dirname(fileURLToPath(import.meta.url)); +const source = readFileSync(path.join(__dirname, 'script.js'), 'utf8'); + +function stubElement() { + return { + addEventListener() {}, + classList: { contains: () => false, add() {}, remove() {}, toggle() {} }, + style: {}, + value: '', + }; +} + +const sandbox = { + window: { + addEventListener() {}, + requestAnimationFrame() {}, + location: { hash: '' }, + history: { pushState() {} }, + lucide: null, + }, + document: { + createElement: () => stubElement(), + getElementById: () => null, + querySelectorAll: () => [], + documentElement: { classList: { contains: () => false } }, + addEventListener() {}, + }, + localStorage: { + getItem: () => null, + setItem() {}, + removeItem() {}, + }, + siteContent: { blog: [], terminal: [] }, + marked: { parse: (s) => s }, + console, + URL, +}; +sandbox.window.document = sandbox.document; +vm.createContext(sandbox); +vm.runInContext(source, sandbox, { filename: 'script.js' }); + +const { toEmbedUrl } = sandbox; +assert.equal(typeof toEmbedUrl, 'function', 'toEmbedUrl must be defined at top level of script.js'); + +const cases = [ + // [input, expected output, description] + ['https://www.youtube.com/watch?v=dQw4w9WgXcQ', 'https://www.youtube.com/embed/dQw4w9WgXcQ', 'youtube watch URL'], + ['https://youtu.be/dQw4w9WgXcQ', 'https://www.youtube.com/embed/dQw4w9WgXcQ', 'youtu.be short URL'], + ['https://vimeo.com/12345678', 'https://player.vimeo.com/video/12345678', 'vimeo URL'], + ['https://www.youtube.com/embed/dQw4w9WgXcQ', 'https://www.youtube.com/embed/dQw4w9WgXcQ', 'already-embed youtube URL'], + ['https://player.vimeo.com/video/12345678', 'https://player.vimeo.com/video/12345678', 'already-embed vimeo URL'], + ['', '', 'empty input'], + [null, '', 'null input'], +]; + +const rejected = [ + // Inputs that must be rejected (return '') because they are not a genuine + // youtube.com/youtu.be/vimeo.com URL, even though some contain the + // substring "youtube.com/embed" or "player.vimeo.com" somewhere. + '">//youtube.com/embed', + 'https://evil.com/?x=youtube.com/embed', + 'https://youtube.com.evil.com/embed', + 'https://notyoutube.com/embed/xyz//player.vimeo.com', + 'javascript:alert(1)', + 'not a url at all but contains youtube.com/embed', +]; + +let failures = 0; + +for (const [input, expected, description] of cases) { + const actual = toEmbedUrl(input); + try { + assert.equal(actual, expected); + console.log(`PASS: ${description}`); + } catch { + failures++; + console.error(`FAIL: ${description} — input=${JSON.stringify(input)} got=${JSON.stringify(actual)} want=${JSON.stringify(expected)}`); + } +} + +for (const input of rejected) { + const actual = toEmbedUrl(input); + try { + assert.equal(actual, ''); + console.log(`PASS: rejects bypass attempt (${JSON.stringify(input.slice(0, 40))}...)`); + } catch { + failures++; + console.error(`FAIL: bypass NOT rejected — input=${JSON.stringify(input)} got=${JSON.stringify(actual)}`); + } +} + +if (failures > 0) { + console.error(`\n${failures} test(s) failed`); + process.exit(1); +} +console.log('\nAll toEmbedUrl tests passed');