diff --git a/docs/changelog.md b/docs/changelog.md index 0ab81c811..6654862f0 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -7,6 +7,48 @@ +## 🔓 fix(csp): the Manager's update check was blocked by our own CSP, in every production deployment (2026-08-12) + +**Repo:** EDDI (`fix/csp-allow-github-release-check`) + +The Manager ships an opt-in *"is a newer EDDI released?"* check that reads +`api.github.com/repos/labsai/EDDI/releases/latest` straight from the browser. Under the +`csp-default` filter's `connect-src 'self'` the browser refuses that request **before it leaves the +page** — so the feature worked against a dev server, which sends no CSP, and was dead everywhere it +actually shipped. + +It failed misleadingly, too. A CSP-blocked `fetch` rejects with the same `TypeError` as an +unreachable host, so the Manager reported *"could not reach api.github.com — check your network or +any outbound proxy"*, pointing operators at a network path and a proxy that were never involved. +(The Manager now tells the two apart by listening for `securitypolicyviolation` and names CSP as the +cause — labsai/EDDI-Manager#138.) + +`connect-src` in `csp-default` now carries `https://api.github.com`. The exception is narrow by +construction: read-only, one public endpoint, no `Authorization` header, `credentials: "omit"`, and +`referrerPolicy: "no-referrer"` — so not even this deployment's hostname, which for a self-hosted +instance *is* deployment data, reaches GitHub. Nothing is requested until an operator presses +*Check now* or opts into the per-reload check. **The Swagger UI policy is untouched**: it never +calls GitHub, and widening it would be pure surface. + +Both halves are now pinned, in two places for one reason. `InfrastructureIT` asserts them over HTTP +— the real proof — and previously checked only `script-src`, so it would have stayed green if the +source were dropped again or pasted into the Swagger policy. But its Swagger case is guarded by an +`Assumptions.assumeFalse` and skips whenever the profile does not serve Swagger UI, which as of +2026-08-12 is every integration run (11 run, 1 skipped) — so the half that says *do not widen this +one* was +asserted nowhere that executes. `CspPolicyTest` therefore reads the two configured headers straight +from `application.properties`, with no container and no assumption: the application policy must +carry the source, the Swagger policy must not, and neither may reach it through `default-src` or +`script-src`. Mutation-checked — removing the source turns it red. + +Verified in a browser, serving the Manager's production bundle behind this exact header: with +`connect-src 'self'` the check is blocked and reports CSP; with the source added it completes and +returns the latest release and its notes. + +--- + + + ## 🔢 docs(mcp): the MCP tool catalogue was eight tools short, and a count sweep of both READMEs (2026-08-11) **Repo:** EDDI (`docs/group-collaboration-refresh`) diff --git a/src/main/resources/application.properties b/src/main/resources/application.properties index 1c1a643a3..f81b7bc4a 100644 --- a/src/main/resources/application.properties +++ b/src/main/resources/application.properties @@ -313,11 +313,17 @@ quarkus.http.header.Permissions-Policy.value=camera=(), microphone=(), geolocati # Negative lookahead excludes /q/swagger-ui — its own relaxed CSP filter handles it. # Without this exclusion both CSP headers are sent and the browser enforces the # most-restrictive intersection, blocking Swagger UI's inline scripts. +# api.github.com in connect-src: the Manager's update check ("is a newer EDDI +# released?") reads api.github.com/repos/labsai/EDDI/releases/latest straight +# from the browser, without credentials and with no referrer. Under 'self' +# alone the browser refuses it, so the feature could only ever work in local +# dev. Read-only, one public endpoint, and the request carries nothing about +# this deployment; the check itself stays opt-in in the Manager. quarkus.http.filter.csp-default.matches=/(?!q/swagger-ui(/|$)).* quarkus.http.filter.csp-default.order=10 quarkus.http.filter.csp-default.header."Content-Security-Policy"=default-src 'self'; \ script-src 'self'; style-src 'self' 'unsafe-inline'; img-src 'self' data:; \ - connect-src 'self' ${eddi.keycloak.public.url:}; \ + connect-src 'self' https://api.github.com ${eddi.keycloak.public.url:}; \ font-src 'self'; frame-ancestors 'none'; # Swagger UI: relaxed CSP (order=20, higher priority — overrides default) # Swagger UI requires 'unsafe-inline' + 'unsafe-eval' for inline scripts and JSON schema rendering. diff --git a/src/test/java/ai/labs/eddi/configs/CspPolicyTest.java b/src/test/java/ai/labs/eddi/configs/CspPolicyTest.java new file mode 100644 index 000000000..11e3b2958 --- /dev/null +++ b/src/test/java/ai/labs/eddi/configs/CspPolicyTest.java @@ -0,0 +1,133 @@ +/* + * Copyright EDDI contributors + * SPDX-License-Identifier: Apache-2.0 + */ +package ai.labs.eddi.configs; + +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +import java.io.Reader; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Properties; + +import static org.junit.jupiter.api.Assertions.*; + +/** + * The CSP headers are two hand-edited strings in one properties block, and the + * difference between them is the whole policy: the application may reach + * api.github.com (the Manager's release check runs in the browser), Swagger UI + * may not, because it never calls GitHub. + *
+ * {@code InfrastructureIT} asserts this over HTTP, which is the real proof — + * but its Swagger case is guarded by an {@code Assumptions.assumeFalse} and is + * skipped whenever the profile does not serve Swagger UI, which is every + * integration run today. So the half of the boundary that says "do not widen + * this one" was asserted nowhere that executes. + *
+ * Reading the configuration directly costs no container and no assumption, so + * both halves hold on every build. + * + * @since 6.3.0 + */ +@DisplayName("CSP policy") +class CspPolicyTest { + + private static final String DEFAULT_HEADER = "quarkus.http.filter.csp-default.header.\"Content-Security-Policy\""; + private static final String SWAGGER_HEADER = "quarkus.http.filter.csp-swagger.header.\"Content-Security-Policy\""; + + private static final String GITHUB_API = "https://api.github.com"; + + /** + * Read from the source tree rather than the classpath: {@code + * src/test/resources/application.properties} shadows the main file for tests + * and defines no CSP at all, so a classpath lookup finds nothing. The file + * below is the artefact these assertions exist to protect. + */ + private static Properties applicationProperties() throws Exception { + var path = Path.of(System.getProperty("basedir", ".")) + .resolve("src/main/resources/application.properties"); + assertTrue(Files.isRegularFile(path), "Expected the application config at " + path); + + var properties = new Properties(); + // Properties.load joins the trailing-backslash continuations the CSP + // headers are written across. + try (Reader in = Files.newBufferedReader(path)) { + properties.load(in); + } + return properties; + } + + /** Isolates one directive, so a source in a neighbouring one cannot match. */ + private static String directive(String csp, String name) { + for (var part : csp.split(";")) { + var trimmed = part.trim(); + var tokens = trimmed.split("\\s+", 2); + if (tokens.length > 0 && tokens[0].equals(name)) { + return trimmed; + } + } + return ""; + } + + /** + * Whether a directive lists exactly this source. + *
+ * Sources are whitespace-delimited, and equality is the only safe test on the + * permissive side: {@code contains} would also accept + * {@code https://api.github.com.evil}, a different host entirely that permits + * nothing we want. The prohibitive assertions below stay substring checks on + * purpose — there, matching more broadly is the stricter reading. + */ + private static boolean allows(String directive, String source) { + for (var token : directive.trim().split("\\s+")) { + if (token.equals(source)) { + return true; + } + } + return false; + } + + @Test + @DisplayName("the application policy may reach the GitHub API") + void applicationConnectSrcAllowsGitHubApi() throws Exception { + var csp = applicationProperties().getProperty(DEFAULT_HEADER); + assertNotNull(csp, DEFAULT_HEADER + " must be configured"); + + var connectSrc = directive(csp, "connect-src"); + assertTrue(allows(connectSrc, GITHUB_API), + "The Manager's update check reads api.github.com from the browser; without this " + + "source the browser refuses it before it leaves the page: " + connectSrc); + } + + @Test + @DisplayName("the Swagger UI policy does not") + void swaggerConnectSrcDoesNotAllowGitHubApi() throws Exception { + var csp = applicationProperties().getProperty(SWAGGER_HEADER); + assertNotNull(csp, SWAGGER_HEADER + " must be configured"); + + var connectSrc = directive(csp, "connect-src"); + assertFalse(connectSrc.contains("api.github.com"), + "Swagger UI never calls GitHub, so widening its connect-src is pure surface: " + + connectSrc); + } + + @Test + @DisplayName("neither policy relaxes anything else to reach it") + void theExceptionIsScopedToConnectSrc() throws Exception { + var properties = applicationProperties(); + + for (var key : new String[]{DEFAULT_HEADER, SWAGGER_HEADER}) { + var csp = properties.getProperty(key); + assertNotNull(csp, key + " must be configured"); + // A source pasted into default-src would grant it to every fetch + // directive that falls back, which is the opposite of a narrow + // exception. + assertFalse(directive(csp, "default-src").contains("api.github.com"), + key + " must keep the GitHub source out of default-src: " + csp); + assertFalse(directive(csp, "script-src").contains("api.github.com"), + key + " must not allow scripts from GitHub: " + csp); + } + } +} diff --git a/src/test/java/ai/labs/eddi/integration/InfrastructureIT.java b/src/test/java/ai/labs/eddi/integration/InfrastructureIT.java index aa92aab72..e797eb6e6 100644 --- a/src/test/java/ai/labs/eddi/integration/InfrastructureIT.java +++ b/src/test/java/ai/labs/eddi/integration/InfrastructureIT.java @@ -112,6 +112,12 @@ void swaggerUiCspHeader() { "Swagger UI CSP must allow 'unsafe-inline' for inline scripts: " + csp); Assertions.assertTrue(csp.contains("'unsafe-eval'"), "Swagger UI CSP must allow 'unsafe-eval' for JSON schema rendering: " + csp); + // The GitHub exception belongs to the application policy alone. Swagger UI + // never calls GitHub, and the two headers sit in one properties block edited + // by hand — so a copy-paste that widens this one has to fail here. + var swaggerConnectSrc = extractDirective(csp, "connect-src"); + Assertions.assertFalse(swaggerConnectSrc.contains("api.github.com"), + "Swagger UI connect-src must NOT carry the GitHub exception: " + swaggerConnectSrc); } @Test @@ -130,6 +136,13 @@ void apiPathCspHeader() { var scriptSrc = extractDirective(csp, "script-src"); Assertions.assertFalse(scriptSrc.contains("'unsafe-inline'"), "Non-Swagger script-src must NOT allow 'unsafe-inline': " + scriptSrc); + // The Manager's update check reads api.github.com from the browser. Without + // this source the browser refuses the request before it leaves the page, and + // the rejection is indistinguishable from an unreachable host — so tightening + // this back kills the feature quietly rather than loudly. + var connectSrc = extractDirective(csp, "connect-src"); + Assertions.assertTrue(allowsSource(connectSrc, "https://api.github.com"), + "Non-Swagger connect-src must allow the Manager's release check: " + connectSrc); } /** @@ -147,6 +160,22 @@ private static String extractDirective(String csp, String directive) { return ""; } + /** + * Whether a CSP directive lists exactly this source. Sources are + * whitespace-delimited, and equality is the only safe test on the permissive + * side: a substring match would also accept https://api.github.com.evil, which + * is a different host permitting nothing we want. The prohibitive assertions + * stay substring checks, where matching more broadly is stricter. + */ + private static boolean allowsSource(String directive, String source) { + for (var token : directive.trim().split("\\s+")) { + if (token.equals(source)) { + return true; + } + } + return false; + } + // ==================== Coordinator Admin ==================== @Test