test(server): make security mutation analysis reliable (#1544)

Author: FelixTJDietrich

.changeset/safer-oauth-boundaries.md

@@ -0,0 +1,5 @@
+---
+"hephaestus": patch
+---
+
+OAuth return targets now reject excessive percent-encoding, and OAuth intent cookies enforce expiration and future timestamps at millisecond precision.

.github/workflows/security-mutation.yml

@@ -0,0 +1,64 @@
+name: Security mutation testing
+
+on:
+  workflow_dispatch:
+  pull_request:
+    paths:
+      - ".github/workflows/security-mutation.yml"
+      - ".github/actions/setup-pnpm-node/**"
+      - ".node-version"
+      - "package.json"
+      - "pnpm-lock.yaml"
+      - "scripts/run-security-mutations.ts"
+      - "scripts/run-security-mutations.test.ts"
+      - "server/application/pom.xml"
+      - "server/generated-clients/**"
+      - "server/.mvn/**"
+      - "server/mvnw"
+      - "server/pom.xml"
+      - "server/application/src/**/java/de/tum/cit/aet/hephaestus/core/auth/oauth/**"
+      - "server/application/src/**/java/de/tum/cit/aet/hephaestus/core/security/**"
+      - "server/application/src/**/java/de/tum/cit/aet/hephaestus/integration/core/connection/**"
+      - "server/application/src/**/java/de/tum/cit/aet/hephaestus/integration/core/oauth/state/**"
+      - "server/application/src/**/java/de/tum/cit/aet/hephaestus/integration/scm/gitlab/webhook/**"
+
+permissions:
+  contents: read
+
+concurrency:
+  group: security-mutation-${{ github.ref }}
+  cancel-in-progress: true
+
+jobs:
+  pitest:
+    name: Security mutation suite
+    runs-on: ubuntu-latest
+    timeout-minutes: 10
+    steps:
+      - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
+        with:
+          persist-credentials: false
+
+      - uses: actions/setup-java@dd06d9cba3e5552c54d9f8ea23572deb30010f7c # v6.0.0
+        with:
+          distribution: temurin
+          java-version: "21"
+          cache: maven
+          cache-dependency-path: "server/**/pom.xml"
+
+      - uses: ./.github/actions/setup-pnpm-node
+
+      - name: Install repository tooling
+        run: pnpm install --frozen-lockfile
+
+      - name: Run security mutation suite
+        run: timeout --kill-after=30s 8m pnpm run test:server:mutation
+
+      - name: Upload mutation report
+        if: always()
+        uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
+        with:
+          name: security-mutation-report-${{ github.run_id }}-${{ github.run_attempt }}
+          path: server/application/target/pit-reports
+          if-no-files-found: error
+          retention-days: 30

docs/auth-architecture.md

@@ -112,7 +112,7 @@ Login `ClientRegistration`s are built from the `login_provider` store by
 `LoginProviderClientRegistrationRepository` in `core.auth.provider` (it depends only on the store +
 Spring Security — integration may not reach into `core.auth.provider`). The integration side keeps
 `RegistrationToGitProviderResolver` (maps a registration to its provider record via the
-`GitProviderRegistry` SPI) and the reusable `IssuerDiscoveryProbe`; `ImpersonationGuard` lives in
+`GitProviderRegistry` SPI); `ImpersonationGuard` lives in
 `core.security`.
 
 Boundaries: `workspace` / `integration.scm` / `notification` depend on `core.auth` (read model +

docs/contributor/security-mutation-testing.md

@@ -0,0 +1,48 @@
+---
+sidebar_position: 9
+title: Security mutation testing
+---
+
+# Security mutation testing
+
+PIT mutates a curated set of security-boundary classes and runs their focused unit tests. The
+advisory result measures whether those tests reject injected changes; it is not a security guarantee
+or a mutation-score gate.
+
+## Run the suite
+
+The **Security mutation testing** workflow runs when a pull request changes an affected security
+area or the suite's build inputs. It also supports manual dispatch. Run the same analysis locally
+with JDK 21:
+
+```bash
+pnpm run test:server:mutation
+```
+
+The command prepares reactor dependencies, compiles the target tests, and runs PIT. It fails if any
+Maven phase fails, the report is missing or invalid, or PIT leaves a mutation in a technical or
+incomplete state. The job summary reports timing and outcomes. PIT's HTML/XML reports and a Markdown
+summary are written below the application module's `target/pit-reports` directory and uploaded by
+the workflow even on failure.
+
+The command evaluates the full target set without incremental analysis and is limited to eight
+minutes, leaving two minutes for artifact upload.
+
+## Triage
+
+The pull-request author reviews reported survivors and uncovered mutations in affected code. The
+affected server CODEOWNERS review any accepted classification.
+
+- Add a public-behavior test when a mutant exposes an unverified contract.
+- Explain an accepted equivalent or unproductive mutant in the pull-request description instead of
+  asserting private call order or implementation details.
+- Remove unwired code instead of building mutation tests around it.
+- Treat technical and incomplete statuses as an invalid run, never as killed mutations.
+
+[Issue #1498](https://github.com/ls1intum/Hephaestus/issues/1498) records the suite's evaluation and
+the decision to keep it non-required and advisory.
+
+## Further reading
+
+- [PIT Maven guide](https://pitest.org/quickstart/maven/)
+- [Mutation testing at Google](https://testing.googleblog.com/2021/04/mutation-testing.html)

package.json

@@ -28,6 +28,7 @@
 		"test:server:architecture": "cd server && ./mvnw -pl application -am test -Parchitecture-tests --batch-mode",
 		"test:server:verification": "cd server && ./mvnw -pl application -am test -Parchitecture-tests -Dsurefire.includedGroups=unit,architecture -DskipCoverage=false --batch-mode",
 		"test:server:integration": "cd server && ./mvnw -pl application -am test -Dsurefire.includedGroups=integration -Dparallel=none --batch-mode",
+		"test:server:mutation": "node --import tsx scripts/run-security-mutations.ts",
 		"summarize:test-results": "node scripts/summarize-test-results.ts",
 		"build:webapp": "pnpm --filter webapp run build",
 		"nats:extract-examples": "node --import tsx scripts/nats-extract-examples.ts",
@@ -42,7 +43,7 @@
 		"format:config:check": "oxfmt --check *.ts *.json .oxfmtrc.json .oxlintrc.json .changeset/*.cjs .changeset/*.json .vscode/*.json scripts/tsconfig.json project.code-workspace",
 		"lint:agents": "oxlint server docker scripts docs .changeset .github.qkg1.topmitlint.config.ts",
 		"lint:agents:fix": "oxlint --fix server docker scripts docs .changeset .github.qkg1.topmitlint.config.ts",
-		"check:agents": "pnpm run format:agents:check && pnpm run format:docs:check && pnpm run format:config:check && pnpm run lint:agents && pnpm run typecheck:agents && pnpm run typecheck:scripts",
+		"check:agents": "pnpm run format:agents:check && pnpm run format:docs:check && pnpm run format:config:check && pnpm run lint:agents && pnpm run typecheck:agents && pnpm run typecheck:scripts && node --test scripts/run-security-mutations.test.ts",
 		"check:agents:fix": "pnpm run format:agents && pnpm run format:docs && pnpm run format:config && oxlint --fix server docker scripts docs .changeset .github.qkg1.topmitlint.config.ts",
 		"ci:agents": "pnpm run format:agents:check && pnpm run format:docs:check && pnpm run format:config:check && oxlint -f github server docker scripts docs .changeset .github.qkg1.topmitlint.config.ts",
 		"format:server": "pnpm run format:java",
@@ -104,6 +105,7 @@
 		"ajv-formats": "3.0.1",
 		"commander": "14.0.3",
 		"fast-xml-parser": "^5.11.0",
+		"fast-xml-validator": "1.4.2",
 		"graphql": "16.14.0",
 		"husky": "9.1.7",
 		"jsdom": "29.1.1",

pnpm-lock.yaml

@@ -1,6 +1,6 @@
 #!/usr/bin/env node
 import { execFile } from "node:child_process";
-import { readFile } from "node:fs/promises";
+import { access, readFile } from "node:fs/promises";
 import { resolve } from "node:path";
 import { promisify } from "node:util";
 
@@ -33,7 +33,19 @@ export async function discoverJavaSourcePaths(
 			},
 		},
 	);
-	const paths = stdout.split("\0").filter(isHandwrittenJavaSource);
+	const candidates = stdout.split("\0").filter(isHandwrittenJavaSource);
+	const present = await Promise.all(
+		candidates.map(async (path) => {
+			try {
+				await access(resolve(root, path));
+				return path;
+			} catch (error) {
+				if (isNodeError(error) && error.code === "ENOENT") return undefined;
+				throw error;
+			}
+		}),
+	);
+	const paths = present.filter((path): path is string => path !== undefined);
 	if (paths.length === 0) {
 		throw new Error(
 			"No handwritten Java sources found under server/application/src/{main,test}/java; refusing to pass without checking anything.",
@@ -42,6 +54,10 @@ export async function discoverJavaSourcePaths(
 	return paths;
 }
 
+function isNodeError(error: unknown): error is NodeJS.ErrnoException {
+	return error instanceof Error;
+}
+
 function unicodeEscapes(source: string): string {
 	return source.replace(/\\u+([0-9a-fA-F]{4})/g, (_, hex: string) =>
 		String.fromCharCode(Number.parseInt(hex, 16)),