diff --git a/.github/workflows/quality-gate.yml b/.github/workflows/quality-gate.yml index f9f2f71..8af0e0c 100644 --- a/.github/workflows/quality-gate.yml +++ b/.github/workflows/quality-gate.yml @@ -240,6 +240,12 @@ jobs: working-directory: csv-table-hub-main run: npm ci + # #112: the frontend's own unit tests (vitest). No job ran them before, so a change to the one place the app + # builds HTML by hand (the print window in src/lib/export.ts) could lose its escaping without anything failing. + - name: Frontend unit tests (vitest) + working-directory: csv-table-hub-main + run: npm test + - name: Install Chromium for Playwright run: python -m playwright install --with-deps chromium diff --git a/Makefile b/Makefile index b0faa6a..5eb3bca 100644 --- a/Makefile +++ b/Makefile @@ -1,5 +1,5 @@ .PHONY: test-free test-gate test-evals test-e2e test-all \ - test-unit test-system test-egress-blocked check-bundle test-sit test-performance test-accessibility test-uat \ + test-unit test-system test-egress-blocked check-bundle security-evidence test-sit test-performance test-accessibility test-uat \ lint lint-diff health \ eval-list eval-compare eval-summary select-tests \ csv-load csv-list csv-demo @@ -60,6 +60,16 @@ test-egress-blocked: check-bundle: python3 scripts/check_bundle_egress.py csv-table-hub-main/.output +# The tests behind docs/SECURITY_REVIEW_PACK.md (#112): authentication, tenant isolation, roles, upload handling and the +# audit trail. Needs PostgreSQL. A passing run is evidence for the reviewer, not the review. +security-evidence: + python3 -m pytest tests/system/test_accounts.py tests/system/test_password_reset.py tests/system/test_oidc.py \ + tests/system/test_oidc_https.py tests/system/test_check_oidc.py tests/system/test_tenant_isolation.py \ + tests/system/test_tenant_te_migrations.py tests/system/test_roles.py tests/system/test_runtime_role.py \ + tests/system/test_audit_append_only.py tests/system/test_preserved_uploads.py tests/system/test_staged_import.py \ + tests/system/test_versioned_datasets.py tests/system/test_reports_and_policy.py \ + tests/unit/test_security.py tests/unit/test_api_hardening.py tests/unit/test_sovereignty.py -v + test-sit: $(call run_category,tests/system_integration) diff --git a/api/oidc.py b/api/oidc.py index 8de4c39..bfc6c8a 100644 --- a/api/oidc.py +++ b/api/oidc.py @@ -18,6 +18,9 @@ from api.config import settings ALGORITHMS = ["RS256", "ES256"] +# Seconds to wait for the IdP's key set. PyJWT's default is 30, which would hold a request thread that long for +# every sign-in attempt while the IdP is down or unreachable. +JWKS_TIMEOUT_SECONDS = 5 def enabled() -> bool: @@ -36,12 +39,15 @@ def check_config() -> None: @lru_cache(maxsize=4) def _jwks(url: str): - """Key source for ``url``: https:// is fetched (and re-fetched on an unknown - key id, for IdP key rotation); file:// is a JWKS file published inside an + """Key source for ``url``: https:// is fetched; file:// is a JWKS file published inside an enclave. Plain http:// is refused, since keys fetched over it could be swapped. + + An https:// key set is cached for PyJWT's lifespan (5 minutes), and an unknown key id triggers a + re-fetch, for IdP key rotation, but at most once per PyJWT's cooldown (30 seconds): so random key ids + can't be used to hammer the IdP, and a freshly rotated key can be refused for up to 30 seconds. """ if url.startswith("https://"): - return jwt.PyJWKClient(url, cache_keys=True) + return jwt.PyJWKClient(url, cache_keys=True, timeout=JWKS_TIMEOUT_SECONDS) if url.startswith("file://"): return jwt.PyJWKSet.from_json(Path(url2pathname(urlparse(url).path)).read_text(encoding="utf-8")) raise ValueError(f"OIDC_JWKS_URL must be https:// or file://, got {url!r}") @@ -58,12 +64,20 @@ def _signing_key(id_token: str): raise jwt.InvalidTokenError(f"unknown key id {kid!r}") from None +def verify(id_token: str) -> dict: + """The token's claims once its signature, issuer, audience and expiry check out; raises ``jwt.PyJWTError`` if not. + + ``username_from`` hides the reason; this keeps it, for scripts/check_oidc.py. + """ + key = _signing_key(id_token) + return jwt.decode(id_token, key, algorithms=ALGORITHMS, audience=settings.OIDC_AUDIENCE, + issuer=settings.OIDC_ISSUER, options={"require": ["exp", "iat", "iss", "aud"]}) + + def username_from(id_token: str) -> str | None: """The verified token's username claim, or None if anything about it is wrong.""" try: - key = _signing_key(id_token) - claims = jwt.decode(id_token, key, algorithms=ALGORITHMS, audience=settings.OIDC_AUDIENCE, - issuer=settings.OIDC_ISSUER, options={"require": ["exp", "iat", "iss", "aud"]}) + claims = verify(id_token) except jwt.PyJWTError: return None username = claims.get(settings.OIDC_USERNAME_CLAIM) diff --git a/csv-table-hub-main/src/__tests__/export.test.ts b/csv-table-hub-main/src/__tests__/export.test.ts index 2f93ad5..73be0ea 100644 --- a/csv-table-hub-main/src/__tests__/export.test.ts +++ b/csv-table-hub-main/src/__tests__/export.test.ts @@ -305,3 +305,67 @@ describe("downloadAuditReport", () => { expect(text).toContain("Imported"); }); }); + +// ─── print window (PDF): the one place HTML is built by hand ────────────────── +// +// The print window is written with document.write(), not React, so nothing escapes for us, and +// it shares the app's origin (and so its sessionStorage, which holds the sign-in token). File +// names, cell values and reasons all come from uploaded CSVs. Security review pack, #112. + +describe("print window HTML (PDF) escapes everything user-supplied", () => { + const HOSTILE = [ + ``, + `">`, + `'-alert(1)-'`, + `<already> & `, + ]; + + let written: string; + + beforeEach(() => { + written = ""; + vi.useFakeTimers(); + vi.spyOn(window, "open").mockImplementation(() => ({ + document: { write: (html: string) => { written += html; }, close: vi.fn() }, + focus: vi.fn(), + print: vi.fn(), + close: vi.fn(), + }) as unknown as Window); + }); + afterEach(() => { + vi.useRealTimers(); + vi.restoreAllMocks(); + }); + + function assertNothingExecutable() { + expect(written).not.toMatch(/