diff --git a/AGENTS.md b/AGENTS.md index fd26fd9..e2f17b3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -8,7 +8,7 @@ This file contains your core, non-negotiable root rules. For detailed framework The project is structured into distinct, decoupled framework modules. **Do not violate module boundaries or introduce circular dependencies.** -- **Dependencies Flow:** `core` (standalone) <- `cache`, `exposed` & `validator` <- `ktor` ; `koin` (standalone dependency carrier) <- `koin-exposed` ; `ktor-test` (standalone dependency carrier) ; `plugin` (Gradle convention plugins) +- **Dependencies Flow:** `core` (standalone) <- `cache`, `exposed` & `validator` <- `ktor` <- `graphql` ; `koin` (standalone dependency carrier) <- `koin-exposed` ; `ktor-test` (standalone dependency carrier) ; `plugin` (Gradle convention plugins) - **`core`**: Base logging (`LoggerFactory`), utilities. MUST remain zero-dependency relative to other submodules. - **`cache`**: Two-level caching engine (`Cache`), L1 LRU memory cache (`L1Cache`), L2 Valkey client (`ValkeyWrapper`), CBOR binary serialization (`BinaryCodec`), bucket versioning, and single-flight loader deduplication (`SingleFlight`). - **`exposed`**: Database connection management (`DatabaseWrapper`), Exposed ORM integration, Liquibase migrations, `@Transactional` annotations & proxying (`TransactionalProxy`), and repository base classes (`UpsertRepository`, `EntityRepository`). @@ -17,6 +17,7 @@ The project is structured into distinct, decoupled framework modules. **Do not v - **`koin`**: Dependency carrier publishing the Koin runtime APIs (`koin-bom`, `koin-core`, `koin-annotations`) via `api(...)`; contains no source code. - **`koin-exposed`**: Koin bridge for transactions — `applyTransactionalProxies()` post-processor replacing eligible singleton definitions with `TransactionalProxy`-wrapping factories; depends on `exposed` and the `koin` carrier. - **`ktor-test`**: Dependency carrier publishing the Ktor server test host (`ktor-server-test-host`) and the Ktor client mock engine (`ktor-client-mock`) via `api(...)`; contains no source code. +- **`graphql`**: GraphQL engine wiring (`graphql-kotlin` on `graphql-java`), the `/graphql` route, the security stack guarding it (body limit, compression, `DoubleReceive`, rate limit charged by query cost), the static cost estimator and the document bounds, plus the orchestration of consumer-provided DataLoaders. It MUST NOT depend on `exposed`: the framework has no repository, so DataLoaders are consumer-owned. - **`plugin`**: Gradle convention plugins for downstream projects (e.g. `fr.shikkanime.framework.ktor`, `fr.shikkanime.framework.koin`). For detailed architectural principles, read the [Architecture Guide](guidelines/ARCHITECTURE.md). @@ -55,6 +56,7 @@ Each framework module has its own dedicated `AGENTS.md` specifying module-specif - [`ktor-test/AGENTS.md`](ktor-test/AGENTS.md): Ktor server test host dependency carrier (no source code). - [`koin/AGENTS.md`](koin/AGENTS.md): Koin dependency carrier (no source code). - [`koin-exposed/AGENTS.md`](koin-exposed/AGENTS.md): Koin bridge for `@Transactional` auto-proxying. +- [`graphql/AGENTS.md`](graphql/AGENTS.md): GraphQL engine, endpoint security stack, cost model, and the DataLoader boundary. - [`plugin/AGENTS.md`](plugin/AGENTS.md): Gradle convention plugins for downstream projects. ## 5. Before Submitting Changes diff --git a/buildSrc/src/main/kotlin/kotlin-jvm.gradle.kts b/buildSrc/src/main/kotlin/kotlin-jvm.gradle.kts index 80472a8..05fa534 100644 --- a/buildSrc/src/main/kotlin/kotlin-jvm.gradle.kts +++ b/buildSrc/src/main/kotlin/kotlin-jvm.gradle.kts @@ -55,6 +55,18 @@ tasks.withType().configureEach { // Configure all test Gradle tasks to use JUnitPlatform. useJUnitPlatform() + // The build host has 3.7 GiB of RAM and runs the Gradle daemon, the Kotlin daemon and this + // fork at the same time. Without a cap on the fork, the three JVMs together exhaust memory and + // the kernel kills the build. 640 MiB holds a test JVM for this project comfortably; the fork + // also runs with a serial collector, which costs a little throughput and saves a lot of heap. + maxHeapSize = "640m" + jvmArgs("-XX:MaxMetaspaceSize=256m", "-XX:+UseSerialGC") + + // One fork at a time: parallel forks multiply the memory cost by the fork count, and the + // suite is small enough that the wall-clock saving is not worth an OOM kill. + maxParallelForks = 1 + forkEvery = 0 + // Log information about all test results, not only the failed ones. testLogging { events( diff --git a/exposed/src/main/kotlin/repositories/EntityRepository.kt b/exposed/src/main/kotlin/repositories/EntityRepository.kt index 3dfd5ca..d4a2978 100644 --- a/exposed/src/main/kotlin/repositories/EntityRepository.kt +++ b/exposed/src/main/kotlin/repositories/EntityRepository.kt @@ -15,6 +15,14 @@ import org.jetbrains.exposed.v1.jdbc.SizedIterable abstract class EntityRepository>( entityClass: EntityClass ) : UpsertRepository(entityClass) { + /** + * Finds every entity of the table. + * + * @return a lazily evaluated Exposed entity sequence containing every entity. + */ + fun findAll(): SizedIterable = + entityClass.all() + /** * Finds every entity whose identifier belongs to [ids]. * diff --git a/exposed/src/main/kotlin/repositories/UpsertRepository.kt b/exposed/src/main/kotlin/repositories/UpsertRepository.kt index a7db287..3794c01 100644 --- a/exposed/src/main/kotlin/repositories/UpsertRepository.kt +++ b/exposed/src/main/kotlin/repositories/UpsertRepository.kt @@ -57,3 +57,4 @@ abstract class UpsertRepository>( protected fun ENTITY.applyFlush(): ENTITY = this.apply { flush() } } + diff --git a/gradle.properties b/gradle.properties index 4e52a88..acb01f4 100644 --- a/gradle.properties +++ b/gradle.properties @@ -7,5 +7,5 @@ org.gradle.caching=true org.gradle.configuration-cache=true # Framework version -version=0.0.21-SNAPSHOT +version=0.0.22-SNAPSHOT diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 6a39c08..bb4884b 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -6,6 +6,7 @@ liquibase = "5.0.4" h2 = "2.5.252" bouncyCastle = "1.86" ktor = "3.6.0" +graphqlKotlin = "10.2.2" swaggerCodeGenerators = "1.0.62" valkeyGlide = "2.5.3" kotlinxSerialization = "1.11.0" @@ -39,11 +40,18 @@ ktorSerializationCbor = { module = "io.ktor:ktor-serialization-kotlinx-cbor", ve ktorServerSwagger = { module = "io.ktor:ktor-server-swagger", version.ref = "ktor" } swaggerCodeGenerators = { module = "io.swagger.codegen.v3:swagger-codegen-generators", version.ref = "swaggerCodeGenerators" } ktorServerCors = { module = "io.ktor:ktor-server-cors", version.ref = "ktor" } +ktorServerRateLimit = { module = "io.ktor:ktor-server-rate-limit", version.ref = "ktor" } +ktorServerBodyLimit = { module = "io.ktor:ktor-server-body-limit", version.ref = "ktor" } +ktorServerCompression = { module = "io.ktor:ktor-server-compression", version.ref = "ktor" } +ktorServerDoubleReceive = { module = "io.ktor:ktor-server-double-receive", version.ref = "ktor" } ktorServerTestHost = { module = "io.ktor:ktor-server-test-host", version.ref = "ktor" } ktorClientCore = { module = "io.ktor:ktor-client-core", version.ref = "ktor" } ktorClientOkHttp = { module = "io.ktor:ktor-client-okhttp", version.ref = "ktor" } ktorClientContentNegociation = { module = "io.ktor:ktor-client-content-negotiation", version.ref = "ktor" } ktorClientMock = { module = "io.ktor:ktor-client-mock", version.ref = "ktor" } +graphqlKotlinServer = { module = "com.expediagroup:graphql-kotlin-server", version.ref = "graphqlKotlin" } +graphqlKotlinKtorServer = { module = "com.expediagroup:graphql-kotlin-ktor-server", version.ref = "graphqlKotlin" } +graphqlKotlinDataLoader = { module = "com.expediagroup:graphql-kotlin-dataloader", version.ref = "graphqlKotlin" } kotlinxSerializationCore = { module = "org.jetbrains.kotlinx:kotlinx-serialization-core", version.ref = "kotlinxSerialization" } kotlinxSerializationJson = { module = "org.jetbrains.kotlinx:kotlinx-serialization-json", version.ref = "kotlinxSerialization" } kotlinxSerializationProtobuf = { module = "org.jetbrains.kotlinx:kotlinx-serialization-protobuf", version.ref = "kotlinxSerialization" } diff --git a/graphql/AGENTS.md b/graphql/AGENTS.md new file mode 100644 index 0000000..02bd3da --- /dev/null +++ b/graphql/AGENTS.md @@ -0,0 +1,134 @@ +# Module Rules: GraphQL (`graphql`) + +This file contains specific rules for the `graphql` submodule. All agents working within this +submodule must strictly adhere to these guidelines in addition to the root [`AGENTS.md`](../AGENTS.md). + +## Module Purpose & Scope + +The `graphql` module serves the same use cases as REST over a second transport, using +`graphql-kotlin` (ExpediaGroup) on top of `graphql-java`. It owns the GraphQL engine wiring, the +schema configuration, the `/graphql` route, the security stack that guards it, and the +orchestration of consumer-provided DataLoaders. + +It does **not** own business logic, repositories, or resolvers. Those belong to the consumer. + +## 1. Module Hierarchy & Dependencies + +- `graphql` depends on `core`, `ktor`, and the graphql-kotlin libraries. +- `graphql` **MUST NOT** depend on `exposed`. The framework has no repository and cannot know what + to batch; a DataLoader that did would be a framework coupled to a domain that changes. +- Consumers that need DataLoaders declare a `KotlinDataLoaderRegistryFactory` instance in Koin and + pass it to `configureGraphQL`. Nothing else. + +## 2. Security Stack — non-negotiable + +The endpoint must carry all four guards. Each one covers a vector the others do not: + +| Guard | Covers | +|---|---| +| `RequestBodyLimit` | Oversized payload, measured on the **decoded** stream so compression cannot inflate past it | +| `Compression` | Response size; safe alongside the body limit precisely because that limit is measured after decoding | +| `DoubleReceive` | Makes the body re-readable, which `requestWeight` needs; without it every request breaks | +| `RateLimit` + `requestWeight` | Sustained abuse, charged by query cost rather than by request count | + +Plus the document bounds, enforced by an interception on the route: + +- `SecurityLimits.maxQueryDepth` +- `SecurityLimits.maxQueryComplexity` +- introspection exemption, when `introspectionEnabled` + +**The refusal guard must stay an interception.** Registering it as a sibling `post { }` handler +lets the plugin's own handler win by registration order, which silently executes the hostile +document instead of refusing it. This was a real bug, caught by the route tests. + +**`requestWeight` never charges zero, and never charges the whole bucket for a refusal.** A batch +array, invalid syntax, a query too deep for the parser, or one that breaks the bounds is charged +`RateLimitConfig.unpriceableRequestWeight` — a fixed, moderate penalty. A zero-weight request is +unlimited, which is worse than no rate limit at all; and charging the whole bucket would hand an +attacker a free denial of service, since a few hundred bytes of malformed input would lock out +every client sharing the same address for a whole refill period. + +**`RateLimitConfig.limit` must exceed `maxQueryComplexity`.** Otherwise the bucket is smaller than +the complexity the bounds accept, and a query the bounds explicitly allow can never be executed: the +limiter refuses it for as long as the refill period lasts. + +**`configureGraphQL`'s `rateLimitKey` defaults to the caller's address.** The module owns no +authentication, so that is the only thing it can know — and behind a shared address every client +then spends one budget. Consumers that can identify their callers should pass their own key. It is +a parameter of `configureGraphQL`, not a field of `RateLimitConfig`: a function held in a data class +becomes part of `equals` and `hashCode`, and two configurations built with equivalent lambdas would +compare unequal. + +## 3. Cost Model Rules + +`estimateCost` is a pure function of the document: no schema lookup, no database access. The cost +must be knowable *before* execution. + +- Every field costs one unit, plus the cost of its own selections. +- A **fragment spread pays every time it appears**. A shared "already seen" set would price the + fragment bomb at the cost of a single expansion — the opposite of what is needed. +- **The price must stay affordable to compute.** Paying for every spread is what makes the price + attack-shaped, but a ladder of fragments where each level spreads the next one twice costs 2^k, + which reaches billions of nodes in under a kilobyte of payload. Both walks therefore stop at a + budget derived from the configured bounds — `SecurityLimits.costCeiling`, + `costVisitBudget`, `depthVisitBudget`. A walk that has to be *exact* is a walk an attacker can + make unbounded. +- The ceiling is **derived from `maxQueryComplexity`, never a hard-coded constant**. A fixed + ceiling below a consumer's bound would saturate under it, and the hostile document would then + pass the comparison. +- Arithmetic is **`Long`**. An unbounded `Int` sum wraps, and a wrapped zero both passes the bound + and weighs one unit in the limiter. +- Cost is checked **before** depth, so a document refused on cost never pays for the more explosive + of the two walks. +- The depth walk resolves spreads, so a document three levels deep cannot hide a fifty-level + expansion. +- Cycles are tracked **per branch**, so two sibling spreads of one fragment each pay full price + while a self-referencing fragment still terminates. +- Introspection is exempt only when the document asks for **nothing else**. A document mixing + `__schema` and business fields is priced normally, or the exemption becomes a smuggling route. + +## 4. Introspection + +Public by default, matching the already-public Swagger UI. The exemption from the depth and +complexity bounds is explicit and lives in `isIntrospectionOnly`, because the introspection query +is deep and expensive by nature: lowering the threshold instead would break GraphiQL and every +codegen client. + +Three rules hold that exemption, and each closes a measured bypass: + +- **Only `__schema` and `__type` grant it** (`META_FIELDS`). `__typename` resolves against the + query type, not the schema, so it carries no introspection work. +- **The document must read the schema somewhere** (`documentReadsSchema`). Otherwise + `{ __typename }` qualifies, and so does a document aliasing it thousands of times — the cheapest + possible bomb, free of charge. `__typename` may still *accompany* a real schema read + (`COMPANION_META_FIELDS`), which is what GraphiQL, Apollo and Relay send. +- **A subtree may not compound through spreads.** Fields under `__schema` are bounded by the + schema's own shape, so a fixed selection count is a fixed cost; a spread that reaches another + spread is not. `{ __schema { types { ...F0 } } }` with a ladder of them qualified as + introspection on name alone and skipped both bounds entirely. A spread reaching only fields is + allowed — that is what a codegen client's `...FullType` looks like. + +## 5. Error Contract + +A refused document answers **HTTP 400** with a GraphQL `errors` array. GraphQL reports execution +errors with HTTP 200, but a refused document never reached the engine — it is a rejected request, +not a failed execution, and a caller branching on the status code must tell the two apart. + +Never surface an internal exception message to a client. + +## 6. Identifiers — `UUID` scalar + +graphql-kotlin has no built-in scalar for `kotlin.uuid.Uuid`. `uuidScalar` (with `UuidCoercing`) +exposes it as a `UUID` scalar and `UuidSchemaGeneratorHooks` registers it for every `Uuid` the schema +meets, so a DTO shared with REST carries its identifiers as `Uuid` and never as a string duplicate. +The scalar speaks the canonical hyphenated hexadecimal form in both directions. + +## 7. Out of Scope for This Module + +- Websocket subscriptions: the artifact ships `getSubscriptionServer()`, but the route does not + advertise subscriptions, and `schemaSubscriptions` should stay empty unless that changes. +- Batch requests: **refused outright** with a 400, not priced. graphql-java would execute every + element of the array as a separate operation, so a batch would answer N operations having been + analysed as one document. The refusal is in the interception, ahead of the handler. +- Rate limiting the REST surface: the plugin is installed, but wiring it to every existing route is + a separate change. diff --git a/graphql/build.gradle.kts b/graphql/build.gradle.kts new file mode 100644 index 0000000..30bcf68 --- /dev/null +++ b/graphql/build.gradle.kts @@ -0,0 +1,18 @@ +plugins { + id("buildsrc.convention.kotlin-jvm") +} + +dependencies { + implementation(project(":core")) + api(project(":ktor")) + api(libs.graphqlKotlinServer) + api(libs.graphqlKotlinKtorServer) + api(libs.graphqlKotlinDataLoader) + implementation(libs.ktorServerRateLimit) + implementation(libs.ktorServerBodyLimit) + implementation(libs.ktorServerCompression) + implementation(libs.ktorServerDoubleReceive) + testImplementation(kotlin("test")) + testImplementation(libs.bundles.testEcosystem) + testImplementation(project(":ktor-test")) +} diff --git a/graphql/src/main/kotlin/CostEstimator.kt b/graphql/src/main/kotlin/CostEstimator.kt new file mode 100644 index 0000000..165484f --- /dev/null +++ b/graphql/src/main/kotlin/CostEstimator.kt @@ -0,0 +1,231 @@ +package fr.shikkanime.graphql + +import graphql.language.Document +import graphql.language.Field +import graphql.language.FragmentDefinition +import graphql.language.FragmentSpread +import graphql.language.InlineFragment +import graphql.language.OperationDefinition +import graphql.language.Selection +import graphql.language.SelectionSet + +/** + * Estimates the static execution cost of a parsed GraphQL document. + * + * The estimate is a pure function of the document: no schema lookup, no database access. That is + * deliberate, because the cost has to be known *before* execution to feed the rate limiter. + * + * Every selected field costs one unit and contributes the cost of its own selections, so nesting + * and breadth both add up. Aliased fields are visited once per alias, which is what makes an + * aliasing bomb expensive: a few hundred bytes of payload resolve hundreds of times over. + * + * A fragment spread costs its whole selection set **every time it appears**. Counting a repeated + * spread once would be the opposite of what is needed: the fragment bomb is precisely a small + * fragment spread dozens of times, so a shared "already seen" set would price the attack at the + * cost of a single expansion. + * + * **Bounded on purpose.** Paying for every spread is what makes the price attack-shaped, but the + * price itself must stay affordable to compute: a ladder of fragments where each level spreads the + * next one twice costs 2^k, which reaches billions of nodes in under a kilobyte of payload. The + * walk therefore stops after [SecurityLimits.costVisitBudget] nodes, and a truncated walk reports + * [SecurityLimits.costCeiling] — one past the complexity bound, so the document is refused exactly + * as it would have been by its true cost. A walk that ran to completion reports the real total + * instead: the number is charged to the rate limiter, and clamping a legitimately expensive + * document down to the ceiling would misprice it. + * + * The arithmetic is [Long], and the recursion is depth-bounded, both so the function is total. An + * unbounded `Int` sum wraps, and a wrapped zero would pass the bound *and* weigh one unit in the + * limiter; an unbounded recursion on a document nested thousands of levels deep raises a + * [StackOverflowError], which is an `Error` and takes the thread with it. + * + * Known limitation: the estimate is static. It cannot tell that `episodes(limit: 1000)` costs more + * than `episodes(limit: 10)`, because that depends on the arguments and on the data. Nothing bounds + * that today: the engine carries no complexity instrumentation, so a data-dependent expansion is + * bounded by the request timeout alone. + * + * @param document Parsed document to price. + * @param limits Bounds the price saturates at. + * @return The estimated cost, never below one so a request is never free. + */ +fun estimateCost(document: Document, limits: SecurityLimits = SecurityLimits.DEFAULT): Long { + val fragments = document.definitions + .filterIsInstance() + .associateBy { fragment -> fragment.name } + + val ceiling = limits.costCeiling + val budget = CostBudget(limits.costVisitBudget) + + val total = document.definitions + .filterIsInstance() + .fold(0L) { carried, operation -> + carried + selectionsCost(operation.selectionSet, fragments, HashSet(), budget, 0) + } + + // Only a truncated walk reports the ceiling. A walk that finished priced the whole document, so + // its total is exact and must not be clamped: the same number is charged to the rate limiter, + // and clamping a legitimately expensive document down to the ceiling would misprice it. + return when { + budget.exhausted() || budget.truncated() -> ceiling + else -> total.coerceAtLeast(1L) + } +} + +private fun selectionsCost( + selectionSet: SelectionSet, + fragments: Map, + expandedFragments: Set, + budget: CostBudget, + depth: Int +): Long = + selectionSet.selections.sumOf { selection -> + selectionCost(selection, fragments, expandedFragments, budget, depth) + } + +/** + * Cost of one selection. + * + * Two stops, both needed. The node budget bounds a walk whose cost is exponential in the fragment + * count while the document stays small. [MAX_PRICED_DEPTH] bounds the *call stack*: a document + * nested thousands of levels deep costs only a few thousand visits, so the budget alone would let + * it recurse until the JVM raised a [StackOverflowError] — which is an `Error`, not an exception, + * and takes the thread down instead of being caught. The parser rejects such a document on the + * request path, but this function is public and must hold on its own. + */ +private fun selectionCost( + selection: Selection<*>, + fragments: Map, + expandedFragments: Set, + budget: CostBudget, + depth: Int +): Long { + // Both stops mean "the rest of this document was never priced", so both mark the walk + // truncated. A depth cut that only returned zero would leave the budget untouched and the + // caller would report the partial total as if it were the real one. + if (depth >= MAX_PRICED_DEPTH) { + budget.markTruncated() + + return 0L + } + + if (budget.exhausted()) { + return 0L + } + + budget.spend() + + return when (selection) { + is Field -> 1L + (selection.selectionSet + ?.let { childSet -> selectionsCost(childSet, fragments, expandedFragments, budget, depth + 1) } + .orZero()) + + is InlineFragment -> + selectionsCost(selection.selectionSet, fragments, expandedFragments, budget, depth + 1) + + is FragmentSpread -> fragmentCost(selection, fragments, expandedFragments, budget, depth + 1) + + else -> 0L + } +} + +/** + * Cost of one spread: the selections it brings in, or one unit when the fragment is unknown. + * + * The expanded set is copied down the branch rather than mutated, so two sibling spreads of the + * same fragment each pay full price while a fragment that spreads itself still terminates. + */ +private fun fragmentCost( + spread: FragmentSpread, + fragments: Map, + expandedFragments: Set, + budget: CostBudget, + depth: Int +): Long { + if (spread.name in expandedFragments) { + return 0L + } + + val definition = fragments[spread.name] ?: return 1L + + return selectionsCost(definition.selectionSet, fragments, expandedFragments + spread.name, budget, depth) +} + +/** + * How many selections the pricing walk may visit before it reports the ceiling. + * + * Spending the budget stops the descent; the caller then reports + * [SecurityLimits.costCeiling] rather than the partial total, so a truncated walk can never look + * cheaper than the document really is. + */ +private class CostBudget(private var visitsLeft: Long) { + private var wasTruncated = false + + fun exhausted(): Boolean = + visitsLeft <= 0L + + /** Records that the walk stopped before reaching the whole document. */ + fun markTruncated() { + wasTruncated = true + } + + fun truncated(): Boolean = + wasTruncated + + fun spend() { + visitsLeft-- + } + + fun visitsLeft(): Long = + visitsLeft +} + +private fun Long?.orZero(): Long = this ?: 0L + +/** Deepest nesting the pricing walk follows, whatever the node budget still allows. */ +private const val MAX_PRICED_DEPTH = 64 + +/** + * Number of nodes the pricing walk visits before its budget runs out. + * + * Exposed so the bound is testable: a wall-clock assertion cannot reliably tell a linear walk from + * an exponential one, but a node count is a deterministic function of the document. + */ +internal fun countCostNodes(document: Document, limits: SecurityLimits): Long { + val fragments = document.definitions + .filterIsInstance() + .associateBy { fragment -> fragment.name } + + val budget = CostBudget(limits.costVisitBudget) + + document.definitions + .filterIsInstance() + .forEach { operation -> + selectionsCost(operation.selectionSet, fragments, HashSet(), budget, 0) + } + + return limits.costVisitBudget - budget.visitsLeft() +} + +/** + * Converts a cost into the weight charged to the rate limit bucket. + * + * Clamped on both ends for two reasons. The floor is the module's hard rule: a zero-weight request + * is unlimited, which is worse than having no rate limit at all. The cap is what keeps one document + * from draining the whole bucket, which would turn a single heavy query into an outage for every + * other request arriving in the same window. + * + * @receiver Cost reported by [estimateCost]. + * @param minimum Smallest weight a request may be charged. + * @param maximum Largest weight a request may be charged. + * @return A weight within `[minimum, maximum]`. + */ +fun Long.toRateLimitWeight(minimum: Int, maximum: Int): Int = + coerceIn(minimum.toLong(), maximum.toLong()).toInt() + +/** + * Documents this module is not expected to receive. + * + * Exposed so the cost model stays verifiable: a batch request is a JSON array of operations, and + * graphql-java would otherwise process each element as a separate, unrated document. + */ +internal fun isBatchRequest(body: String): Boolean = + body.trimStart().startsWith("[") \ No newline at end of file diff --git a/graphql/src/main/kotlin/DataLoaders.kt b/graphql/src/main/kotlin/DataLoaders.kt new file mode 100644 index 0000000..fa37d5b --- /dev/null +++ b/graphql/src/main/kotlin/DataLoaders.kt @@ -0,0 +1,36 @@ +package fr.shikkanime.graphql + +import com.expediagroup.graphql.dataloader.KotlinDataLoaderRegistryFactory +import graphql.GraphQLContext +import org.dataloader.DataLoaderRegistry + +/** + * Builds the factory used when a consumer supplies no DataLoader. + * + * `KotlinDataLoaderRegistryFactory` is a final class, so the empty case is expressed by + * instantiating it with no loader rather than by subclassing it. The no-arg constructor is the + * library's own "no loaders" case, which keeps the module working out of the box for a project + * with no N+1 problem. + * + * @return A factory generating an empty registry. + */ +fun noOpDataLoaderRegistryFactory(): KotlinDataLoaderRegistryFactory = + KotlinDataLoaderRegistryFactory() + +/** + * Whether this factory carries at least one DataLoader. + * + * Lets the caller tell "the consumer configured batching" from "the consumer configured nothing", + * which decides whether the N+1 mitigation is actually in place. + */ +fun KotlinDataLoaderRegistryFactory.hasLoaders(): Boolean = + generate(GraphQLContext.newContext().build(), null).keys.isNotEmpty() + +/** + * Generates the DataLoader registry for one request. + * + * @param graphQLContext Context of the running operation, passed to each loader. + * @return The registry holding every loader the consumer declared. + */ +internal fun KotlinDataLoaderRegistryFactory.generateRegistry(): DataLoaderRegistry = + generate(GraphQLContext.newContext().build(), null) diff --git a/graphql/src/main/kotlin/GraphQLConfig.kt b/graphql/src/main/kotlin/GraphQLConfig.kt new file mode 100644 index 0000000..c3e9f34 --- /dev/null +++ b/graphql/src/main/kotlin/GraphQLConfig.kt @@ -0,0 +1,87 @@ +package fr.shikkanime.graphql + +import io.ktor.server.application.ApplicationCall +import kotlin.time.Duration +import kotlin.time.Duration.Companion.minutes + +/** + * Rate limiting applied to the GraphQL endpoint. + * + * The bucket is token-based: each request consumes as many tokens as the estimated cost of its + * query, so a heavy query drains the bucket faster than a light one. A flat per-request count + * would let a single expensive document through as cheaply as a trivial one. + * + * The bucket has to be able to hold at least one document priced at the complexity bound, otherwise + * a query the bounds explicitly accept can never be executed: it would be refused by the limiter + * for as long as the refill period lasts. [limit] therefore defaults to several times + * `maxQueryComplexity` rather than to it, so a heavy query is possible but cannot drain the bucket + * on its own. + * + * @property enabled Whether the endpoint is rate limited at all. + * @property limit Bucket capacity, expressed in cost units rather than requests. + * @property refillPeriod How often the bucket is refilled to full. + * @property minimumRequestWeight Weight charged when the query cost cannot be determined, so a + * request is never free. Zero would let a caller bypass the limit entirely. + * @property unpriceableRequestWeight Weight charged for a request that could not be priced at all — + * a batch array, invalid syntax, or a document the bounds refuse. Deliberately far below [limit]: + * charging the whole bucket would let a few hundred bytes of malformed input lock every client + * sharing the same address out for a whole refill period, which is a denial of service handed to + * the attacker for free. + */ +data class RateLimitConfig( + val enabled: Boolean = true, + val limit: Int = 1000, + val refillPeriod: Duration = 1.minutes, + val minimumRequestWeight: Int = 1, + val unpriceableRequestWeight: Int = 10 +) + +/** + * Identifies the bucket a request is charged to. + * + * Kept out of [RateLimitConfig] on purpose: a function held in a data class becomes part of + * `equals`, `hashCode` and `copy`, so two configurations built with equivalent lambdas would + * compare unequal — a trap that shows up as a mysteriously failing test rather than as a type + * error. Consumers behind a shared address pass their own key to + * [fr.shikkanime.graphql.configureGraphQL] to stop one client from spending everyone's budget. + * + * @receiver The call being charged. + * @return A stable value identifying the caller's bucket. + */ +val defaultRateLimitKey: (ApplicationCall) -> Any = + { call -> call.request.local.remoteHost } + +/** + * Configuration of the GraphQL module. + * + * Every value here is a security decision rather than a cosmetic one: raising + * [maxQueryComplexity] or [requestBodyLimit] widens the DoS surface, so a consumer that overrides + * them should know what it is trading away. The defaults are conservative but generous enough for + * a normal admin query; they are starting points to measure, not constants to trust. + * + * @property path Route the GraphQL endpoint is bound to. A missing leading slash is added. + * @property maxQueryDepth Maximum selection nesting accepted before execution is refused. + * @property maxQueryComplexity Maximum estimated cost accepted before execution is refused. + * @property introspectionEnabled Whether the schema can be queried through introspection. Public + * by default, matching the already-public Swagger UI. + * @property rateLimit Rate limiting applied to the endpoint. + * @property requestBodyLimit Maximum accepted request body size, measured on the decoded stream so + * a compressed payload cannot exceed it after inflation. + */ +data class GraphQLConfig( + val path: String = DEFAULT_PATH, + val maxQueryDepth: Int = 8, + val maxQueryComplexity: Int = 200, + val introspectionEnabled: Boolean = true, + val rateLimit: RateLimitConfig = RateLimitConfig(), + val requestBodyLimit: Long = DEFAULT_REQUEST_BODY_LIMIT +) { + /** [path] guaranteed to start with a slash. */ + val normalizedPath: String = + if (path.startsWith("/")) path else "/$path" + + private companion object { + private const val DEFAULT_PATH = "/graphql" + private const val DEFAULT_REQUEST_BODY_LIMIT = 64L * 1024 + } +} diff --git a/graphql/src/main/kotlin/GraphQLModules.kt b/graphql/src/main/kotlin/GraphQLModules.kt new file mode 100644 index 0000000..4748e0a --- /dev/null +++ b/graphql/src/main/kotlin/GraphQLModules.kt @@ -0,0 +1,435 @@ +package fr.shikkanime.graphql + +import com.expediagroup.graphql.dataloader.KotlinDataLoaderRegistryFactory +import com.expediagroup.graphql.server.ktor.DefaultKtorGraphQLContextFactory +import com.expediagroup.graphql.server.ktor.GraphQL +import com.expediagroup.graphql.server.ktor.graphQLPostRoute +import com.expediagroup.graphql.server.operations.Mutation +import com.expediagroup.graphql.server.operations.Query +import com.expediagroup.graphql.server.operations.Subscription +import fr.shikkanime.core.LoggerFactory +import kotlinx.coroutines.CancellationException +import java.nio.charset.StandardCharsets +import java.util.logging.Level +import java.util.logging.Logger +import graphql.language.Document +import graphql.parser.InvalidSyntaxException +import graphql.parser.Parser +import graphql.parser.exceptions.ParseCancelledTooDeepException +import io.ktor.http.ContentType +import io.ktor.http.HttpStatusCode +import io.ktor.server.application.Application +import io.ktor.server.application.ApplicationCallPipeline +import io.ktor.server.application.PipelineCall +import io.ktor.server.application.ApplicationCall +import io.ktor.server.application.install +import io.ktor.server.plugins.PayloadTooLargeException +import io.ktor.server.plugins.bodylimit.RequestBodyLimit +import io.ktor.server.plugins.compression.Compression +import io.ktor.server.plugins.compression.gzip +import io.ktor.server.plugins.doublereceive.DoubleReceive +import io.ktor.server.plugins.ratelimit.RateLimit +import io.ktor.server.plugins.ratelimit.RateLimitName +import io.ktor.server.plugins.ratelimit.rateLimit +import io.ktor.server.request.path +import io.ktor.server.request.receiveText +import io.ktor.server.response.respondText +import io.ktor.server.routing.Route +import io.ktor.server.routing.intercept +import io.ktor.util.pipeline.PipelineContext +import io.ktor.server.routing.route +import io.ktor.server.routing.routing +import kotlinx.serialization.json.Json +import kotlinx.serialization.json.JsonPrimitive +import kotlinx.serialization.json.jsonObject +import kotlinx.serialization.json.jsonPrimitive + +/** Name of the rate limiter guarding the GraphQL endpoint. */ +internal const val GRAPHQL_RATE_LIMIT_NAME = "graphql" + +private const val BATCH_REFUSED = "Batch requests are not supported by this endpoint" +private const val UNREADABLE_REFUSED = "Request refused: the GraphQL document could not be read" +private const val INTROSPECTION_REFUSED = "Query refused: schema introspection is disabled" + +private val REQUEST_JSON = Json { ignoreUnknownKeys = true } + +/** Logger for the module's own diagnostics, outside the application lifecycle. */ +private val MODULE_LOGGER = LoggerFactory.getLogger() + +/** + * Installs the GraphQL module: engine, schema, route, and the security stack guarding it. + * + * The order of the guards is deliberate. The body limit runs first, so an oversized payload never + * reaches the parser. The document is then priced and charged to the rate limit bucket, so a heavy + * query drains it faster than a light one. Only after that is the query executed. + * + * What the rate limit is **not**: a guard standing in front of the analysis. The weight comes from + * a document that has already been parsed, so pricing happens after parsing, not before. It limits + * sustained abuse, while the complexity and depth bounds refuse the single hostile query. Both are + * needed — bounds alone let an attacker trickle expensive queries, and rate limiting alone never + * notices that one query is a bomb. + * + * @property config Bounds and endpoint configuration. + * @property dataLoaderRegistryFactory DataLoaders the consumer provides. The framework only wires + * them: it has no repository and cannot know what to batch. + * @property schemaQueries Queries exposed by the schema. + * @property schemaMutations Mutations exposed by the schema. + * @property schemaSubscriptions Subscriptions exposed by the schema. Websocket transport is not + * enabled by this module, so declaring one would advertise an endpoint that cannot serve it. + * @property schemaPackages Packages scanned for annotated types. The generator refuses to build + * a schema without one, so it is required even though listing the queries explicitly is enough. + * @property rateLimitKey Identifies the bucket each request is charged to. Defaults to the caller's + * address, the only identity this module can know since it owns no authentication. A consumer + * behind a shared address should pass its own key, or one client spends everyone's budget. + */ +fun Application.configureGraphQL( + config: GraphQLConfig = GraphQLConfig(), + dataLoaderRegistryFactory: KotlinDataLoaderRegistryFactory = noOpDataLoaderRegistryFactory(), + schemaPackages: List, + schemaQueries: List = emptyList(), + schemaMutations: List = emptyList(), + schemaSubscriptions: List = emptyList(), + rateLimitKey: (ApplicationCall) -> Any = defaultRateLimitKey +) { + val logger = LoggerFactory.getLogger() + val limits = config.toSecurityLimits() + + // Measured on the decoded stream, so a compressed payload cannot inflate past the limit. + install(RequestBodyLimit) { + bodyLimit { config.requestBodyLimit } + } + + install(Compression) { + gzip() + } + + install(GraphQL) { + schema { + packages = schemaPackages + queries = schemaQueries + mutations = schemaMutations + subscriptions = schemaSubscriptions + hooks = UuidSchemaGeneratorHooks() + } + + engine { + this.dataLoaderRegistryFactory = dataLoaderRegistryFactory + } + + server { + this.contextFactory = DefaultKtorGraphQLContextFactory() + } + } + + if (config.rateLimit.enabled) { + install(RateLimit) { + register(RateLimitName(GRAPHQL_RATE_LIMIT_NAME)) { + rateLimiter(limit = config.rateLimit.limit, refillPeriod = config.rateLimit.refillPeriod) + requestKey(rateLimitKey) + requestWeight { call, _ -> call.graphqlWeight(config, limits, logger) } + } + } + } + + routing { + // `rateLimit` is applied only when the plugin is installed: Ktor throws at startup when a + // route references a limiter that was never registered, so calling it unconditionally + // would make `rateLimit.enabled = false` a boot-time crash rather than an opt-out. + if (config.rateLimit.enabled) { + rateLimit(RateLimitName(GRAPHQL_RATE_LIMIT_NAME)) { + bindGraphQLRoute(config, limits, logger) + } + } else { + bindGraphQLRoute(config, limits, logger) + } + } +} + +/** + * Registers the guarded GraphQL route on the current receiver. + * + * Kept out of [configureGraphQL] so the same body serves both the rate-limited and the opt-out + * branch, instead of being duplicated across the two paths. + */ +private fun Route.bindGraphQLRoute( + config: GraphQLConfig, + limits: SecurityLimits, + logger: Logger +) { +// The plugin registers its own POST handler, so a sibling `post { }` would be tried + // first only by luck of registration order. The guard is therefore an interception on + // the route, which runs before any handler: a document that breaks the bounds is + // answered here and the engine never sees it. + intercept(ApplicationCallPipeline.Call) { + val call = context +// Both spellings of the path have to be considered. Ktor's router decodes the path + // before matching it, but `path()` hands back the encoded form, so comparing it alone + // let `POST /%67raphql` skip every guard and still reach the handler — measured: a + // 300-alias document refused on `/graphql` answered 200 with 4 kB on `/%67raphql`. + val requestPath = call.request.path() + val decodedPath = requestPath.decodeIfPercentEncoded() + + if (!isGraphQLPath(requestPath, config.normalizedPath) && + !isGraphQLPath(decodedPath, config.normalizedPath) + ) { + proceed() + return@intercept + } + + val carried = call.carriedQuery() + + if (carried.isBatch) { + respondRefusal(BATCH_REFUSED) + finish() + return@intercept + } + + val document = parseOrNull(carried, logger) + + when { + // An unparseable document must not reach the engine: it is exactly the shape an + // attacker uses to slip past a check that only understands valid syntax. + document == null -> { + respondRefusal(UNREADABLE_REFUSED) + finish() + } + + exceedsLimits(document, limits) -> { + respondRefusal(refusalMessage(document, limits)) + finish() + } + + else -> proceed() + } + } + + route(config.normalizedPath) { + // requestWeight reads the body to price the query, which would otherwise consume + // the one-shot request stream and leave the GraphQL handler with an empty body. + // The in-memory cache suits a GraphQL body: small and short-lived, so a file cache + // would be pure overhead. + install(DoubleReceive) { + maxSize(config.requestBodyLimit) + useFileForCache { false } + } + } + + graphQLPostRoute(config.normalizedPath) +} + +/** + * Whether the request targets the GraphQL endpoint. + * + * Compared on the exact path rather than a prefix, so a sibling route such as `/graphqli` is not + * dragged through the GraphQL guards. + */ +private fun isGraphQLPath(requestPath: String, graphqlPath: String): Boolean = + requestPath == graphqlPath || requestPath == "$graphqlPath/" + +/** + * The percent-decoded form of a path, or the path itself when it holds nothing to decode. + * + * Ktor's router decodes a path segment before matching it, so `/%67raphql` reaches the GraphQL + * handler while `request.path()` still reports `/%67raphql`. A guard that compares only the raw + * form therefore lets an encoded path past every check. Decoding here mirrors what the router + * does, so both spellings of the endpoint are guarded. + * + * @receiver Path as reported by the request. + * @return The decoded path, or the original when it cannot be decoded. + */ +private fun String.decodeIfPercentEncoded(): String = + if (contains('%')) { + runCatching { java.net.URLDecoder.decode(this, StandardCharsets.UTF_8) }.getOrDefault(this) + } else { + this + } + +/** Bounds derived from the endpoint configuration. */ +internal fun GraphQLConfig.toSecurityLimits(): SecurityLimits = + SecurityLimits( + maxQueryDepth = maxQueryDepth, + maxQueryComplexity = maxQueryComplexity, + introspectionEnabled = introspectionEnabled + ) + +/** + * Answers a refused document. + * + * The body is a GraphQL `errors` array so a GraphQL client parses it like any other GraphQL + * response. The status is 400 rather than the 200 GraphQL uses for execution errors: this document + * never reached the engine, so it is a rejected request, not a failed execution — and a caller + * branching on the status code must be able to tell the two apart. + */ +private suspend fun PipelineContext.respondRefusal(message: String) { + context.respondText( + text = buildString { + append("""{"errors":[{"message":""") + // `JsonPrimitive.toString()` is already a JSON string literal, quotes included, so + // encoding *that* produced a doubly-quoted value. Encoding the raw message is enough. + append(REQUEST_JSON.encodeToString(message)) + append("}]}") + }, + contentType = ContentType.Application.Json, + status = HttpStatusCode.BadRequest + ) +} + +/** + * Cost charged to the rate limit bucket for one request. + * + * A document that cannot be priced — a batch array, invalid syntax, a query too deep for the + * parser, or one that breaks the bounds — is charged [RateLimitConfig.unpriceableRequestWeight], a + * moderate fixed penalty rather than the whole bucket. The floor matters most: the weight is never + * zero, because a zero-weight request is unlimited and worse than having no rate limit at all. The + * ceiling matters just as much: charging a refusal the whole bucket would hand an attacker a free + * denial of service, since a few hundred bytes of malformed input would lock out every client + * behind the same address for a whole refill period. + * + * A priced document pays its real cost, clamped to what the bucket can absorb. + */ +private suspend fun ApplicationCall.graphqlWeight( + config: GraphQLConfig, + limits: SecurityLimits, + logger: Logger +): Int { + val carried = carriedQuery() + val document = parseOrNull(carried, logger) + + if (carried.isBatch || document == null || exceedsLimits(document, limits)) { + return config.rateLimit.unpriceableRequestWeight + .coerceAtLeast(config.rateLimit.minimumRequestWeight) + .coerceAtMost(config.rateLimit.limit) + } + + val cost = estimateCost(document, limits) + + return cost.toRateLimitWeight( + minimum = config.rateLimit.minimumRequestWeight, + maximum = config.rateLimit.limit + ) +} + +/** + * Reads the cached body, which DoubleReceive made re-readable. + * + * An oversized body is not an unreadable one: [io.ktor.server.plugins.PayloadTooLargeException] + * means the request broke the body limit, and answering "could not be read" would send an operator + * hunting for a parsing bug that does not exist. It is re-thrown so the platform answers 413. + */ +private suspend fun ApplicationCall.readCachedText(): String? = + try { + receiveText() + } catch (exception: PayloadTooLargeException) { + throw exception + } catch (exception: CancellationException) { + // A client that walked away cancels the call. Swallowing that breaks structured + // concurrency: the coroutine keeps running on a request nobody is waiting for, and the + // cancellation becomes a 400 that blames the payload instead of the disconnect. + throw exception + } catch (exception: Exception) { + MODULE_LOGGER.log(Level.FINE, "Could not read the request body", exception) + + null + } + +/** + * Parses the query carried by a GraphQL request body. + * + * Returns null for a batch request, an unparseable body, or a body carrying no query. Each of + * those is charged the maximum weight upstream, so this function only has to be honest about + * failure, not about the consequence. + */ +private fun parseOrNull(carried: CarriedQuery, logger: Logger): Document? { + if (carried.isBatch) { + logger.warning("Refusing a graphql batch request") + return null + } + + val query = (carried as? CarriedQuery.Single)?.query ?: return null + + return try { + Parser().parseDocument(query) + } catch (exception: InvalidSyntaxException) { + logger.warning("Refusing a malformed graphql document") + null + } catch (exception: ParseCancelledTooDeepException) { + logger.warning("Refusing a graphql document too deep to parse") + null + } +} + +/** + * The query a request carries, and whether it is a batch. + * + * A batch is not a document this module can price: graphql-java would execute every element as a + * separate operation, so a batch would slip past a check that only ever looks at one parsed + * document. It is therefore never allowed through — see [isBatch]. + */ +internal sealed interface CarriedQuery { + /** One query, ready to be parsed. */ + data class Single(val query: String) : CarriedQuery + + /** A batch array, which this module refuses. */ + data object Batch : CarriedQuery + + /** A request carrying no query at all. */ + data object None : CarriedQuery + + /** Whether this request is a batch array. */ + val isBatch: Boolean + get() = this is Batch +} + +/** + * Reads the query a call carries, from its body. + * + * Only the body, and deliberately: this module registers the POST route alone, so there is no + * handler a query string could reach. Note that a GET still crosses the route interception first — + * Ktor answers 405 only once the interception has let the call through — so a bodiless verb is + * refused here as an unreadable document rather than by the router. + */ +internal suspend fun ApplicationCall.carriedQuery(): CarriedQuery { + val body = readCachedText() + + if (body.isNullOrBlank()) { + return CarriedQuery.None + } + + if (isBatchRequest(body)) { + return CarriedQuery.Batch + } + + val query = body.extractQuery() ?: return CarriedQuery.None + + return CarriedQuery.Single(query) +} + +/** + * Message describing why a document was refused, safe to return to a client. + * + * Reports the bound that actually broke rather than the first one checked, so an operator reading + * the response is not sent to the wrong limit. + */ +private fun refusalMessage(document: Document, limits: SecurityLimits): String { + return when { + isIntrospectionOnly(document) -> INTROSPECTION_REFUSED + + estimateCost(document, limits) > limits.maxQueryComplexity -> + "Query refused: its estimated cost exceeds the maximum of ${limits.maxQueryComplexity}" + + else -> "Query refused: its nesting depth exceeds the maximum of ${limits.maxQueryDepth}" + } +} + +/** + * Reads the `query` member of a GraphQL request body. + * + * @return The query text, or null when the body is not a JSON object carrying one. + */ +internal fun String.extractQuery(): String? { + val queryField = runCatching { + REQUEST_JSON.parseToJsonElement(this).jsonObject["query"] + }.getOrNull() ?: return null + + return runCatching { queryField.jsonPrimitive.content }.getOrNull() +} diff --git a/graphql/src/main/kotlin/SecurityInstrumentations.kt b/graphql/src/main/kotlin/SecurityInstrumentations.kt new file mode 100644 index 0000000..68b81e3 --- /dev/null +++ b/graphql/src/main/kotlin/SecurityInstrumentations.kt @@ -0,0 +1,524 @@ +package fr.shikkanime.graphql + +import graphql.language.Document +import graphql.language.Field +import graphql.language.FragmentDefinition +import graphql.language.FragmentSpread +import graphql.language.InlineFragment +import graphql.language.OperationDefinition +import graphql.language.Selection +import graphql.language.SelectionSet + +/** + * Bounds applied to a GraphQL document before it is executed. + * + * @property maxQueryDepth Maximum selection nesting accepted. + * @property maxQueryComplexity Maximum estimated cost accepted. + * @property introspectionEnabled Whether introspection is served. When false, an introspection + * query is refused even if it stays within the other bounds, because the schema is the thing + * being protected. + */ +data class SecurityLimits( + val maxQueryDepth: Int, + val maxQueryComplexity: Int, + val introspectionEnabled: Boolean = true +) { + /** + * Cost reported by a document expensive enough that pricing it precisely is pointless. + * + * Derived from [maxQueryComplexity] rather than fixed: a hard-coded ceiling would saturate + * *below* the bound of a consumer configured with a larger one, and the hostile document would + * then pass `cost > maxQueryComplexity`. One past the bound is the smallest value that keeps + * every saturated document refused, whatever the configuration. + */ + val costCeiling: Long = + maxQueryComplexity.toLong() + 1L + + /** + * Largest number of selections the cost walk may visit before it gives up. + * + * A document cannot ask for more selections than it has bytes, so the budget scales with the + * body limit the caller configured. It exists to stop an expansion that is exponential in the + * number of fragments rather than linear in the number of selections. + */ + val costVisitBudget: Long = + COST_VISITS_PER_BYTE * (maxQueryComplexity.toLong() + FRAGMENT_VISIT_FACTOR) + + /** + * Largest number of branches the depth walk may visit before it gives up. + * + * Derived from [maxQueryComplexity], not [maxQueryDepth]: an explosive ladder is shallow, so a + * budget tied to the depth bound would be tiny and would truncate documents that have nothing + * to do with depth. Complexity is the bound that actually prices the expansion. + */ + val depthVisitBudget: Long = + DEPTH_VISITS_PER_BYTE * (maxQueryComplexity.toLong() + FRAGMENT_VISIT_FACTOR) + + companion object { + /** + * Bounds used when a caller prices a document outside a configured endpoint. + * + * Exposed so [estimateCost] keeps a total function for its own tests and for any consumer + * pricing a document outside a request. The endpoint always passes the limits it was + * configured with, so the ceiling follows the deployment rather than these defaults. + */ + val DEFAULT: SecurityLimits = + SecurityLimits(maxQueryDepth = 8, maxQueryComplexity = 200) + } +} + +/** + * Whether the document only asks for schema metadata. + * + * Introspection is deep and expensive by nature, so it is exempt from the depth and complexity + * bounds when public. Refusing it would break GraphiQL and every codegen client, which is why the + * exemption has to be explicit rather than a lowered threshold. + * + * A document that mixes introspection with business fields is **not** exempt: it would otherwise + * be a way to smuggle a heavy query past the limits. + */ +fun isIntrospectionOnly(document: Document): Boolean { + val operations = document.definitions.filterIsInstance() + + if (operations.isEmpty()) { + return false + } + + // A companion meta field may ride along but never carries the exemption by itself: the + // document has to read the schema somewhere. Without this, `{ __typename }` — and a document + // aliasing it thousands of times — would qualify while doing no introspection work at all. + if (!documentReadsSchema(document)) { + return false + } + + return operations.all { operation -> + operation.selectionSet.selections.all { + isIntrospectionField(it, documentFragments(document), emptySet()) + } + } +} + +/** + * Whether the document selects a field that actually reads the schema. + * + * `__schema` and `__type` do; `__typename` does not — it resolves against the query type and + * reveals one name. It is the only meta field a document can flood, so it cannot be the thing + * that grants the exemption. + */ +private fun documentReadsSchema(document: Document): Boolean = + document.definitions + .filterIsInstance() + .any { operation -> + operation.selectionSet.selections.any { + it.startsSchemaRead( + fragments = documentFragments(document), + depth = 0, + visited = emptySet(), + budget = SchemaReadBudget(SCHEMA_READ_VISIT_CAP) + ) + } + } + +/** + * Node allowance for the schema-read probe. + * + * The probe answers a yes/no question and runs *before* the cost estimator, which is the walk that + * actually carries a budget. Without an allowance of its own it inherits the hazard it exists to + * classify: a ladder of spreads expanded until an answer came back, measured at 329 s on a + * 24-level document. A safety net, not the primary defence — the exponential shape is cut by + * [MAX_TRACKED_DEPTH] and the per-branch visited set. + */ +private const val SCHEMA_READ_VISIT_CAP = 256 + +/** Remaining visits for one schema-read probe. */ +private class SchemaReadBudget(private var left: Int) { + fun exhausted(): Boolean = + left <= 0 + + fun spend() { + left-- + } +} + +/** + * Whether this selection reaches a field that reads the schema. + * + * @param fragments Fragment definitions of the document. + * @param depth Nesting level, which bounds a legitimately deep document. + * @param visited Fragment names already walked on this branch, so a cycle terminates. + * @param budget Node allowance, since this runs ahead of the budgeted cost walk. + */ +private fun Selection<*>.startsSchemaRead( + fragments: Map, + depth: Int, + visited: Set, + budget: SchemaReadBudget +): Boolean = + when { + depth > MAX_TRACKED_DEPTH || budget.exhausted() -> false + + this is Field -> name in META_FIELDS + + this is FragmentSpread -> { + val name = this.name + + when { + name in visited -> false + + else -> { + budget.spend() + + fragments[name]?.selectionSet?.selections + ?.any { it.startsSchemaRead(fragments, depth + 1, visited + name, budget) } + ?: false + } + } + } + + this is InlineFragment -> selectionSet.selections.any { + it.startsSchemaRead(fragments, depth + 1, visited, budget) + } + + else -> false + } + +private fun isIntrospectionField( + selection: Selection<*>, + fragments: Map, + expanding: Set +): Boolean = + when (selection) { + // Only a meta field *starts* an introspection request. Everything below it is schema + // metadata — `types`, `fields`, `name`, `ofType` qualify by being selected on `__schema`, + // not by name — so sub-fields are not re-checked against META_FIELDS. + // + // What is checked is whether the subtree can multiply its work through spreads. Fields + // under `__schema` are bounded by the schema's own shape, so a fixed selection count costs + // a fixed amount; a spread that expands other spreads does not, and + // `{ __schema { types { ...F0 } } }` with a ladder of them qualified as introspection on + // name alone and skipped both bounds entirely. + is Field -> when { + selection.name in META_FIELDS -> + !carriesExpandingSpread(selection.selectionSet, fragments, expanding) + + // A companion may ride along with a schema read the document already performs. + // `isIntrospectionOnly` requires that read, so a document of nothing but + // `__typename` is priced and bounded like any other. + selection.name in COMPANION_META_FIELDS -> true + + else -> false + } + + // An inline fragment wrapping meta fields is still introspection, since it selects + // nothing else. + is InlineFragment -> selection.selectionSet.selections.all { + isIntrospectionField(it, fragments, expanding) + } + + // A spread at the root of an operation is introspection only if the fragment it points at + // is, which keeps a codegen client's `...SchemaFragment` working without letting an + // arbitrary fragment claim the exemption. + // A spread is introspection only if the fragment it names is, *and* does not multiply its + // own work. Without the second half the ladder hid behind a named fragment: checking only + // that its body selects meta fields let `{ __schema { ...F0 } }` through while `F0`'s body + // spread another fragment twice per level. + is FragmentSpread -> { + val name = selection.name + + when { + name in expanding -> true + + else -> { + val body = fragments[name]?.selectionSet + + body != null && + !spreadMultiplies(name, fragments, expanding) && + body.selections.all { isIntrospectionField(it, fragments, expanding + name) } + } + } + } + + else -> false + } + +/** + * Whether a selection set's fragment spreads can compound into an unbounded amount of work. + * + * A codegen client's introspection query spreads a handful of named fragments, each *reached once* + * and each selecting fixed fields — `...FullType` reaches `...InputValue` through + * `fields { args { ... } }`, exactly once. That is a fixed amount of work. + * + * The bomb reaches the same fragment **more than once**, so each occurrence adds its work again and + * the total grows multiplicatively: measured, a ladder of spreads doubling per level reached a cost + * of 201 in 422 bytes and skipped the bounds entirely. + * + * Repetition, not mere presence, is the whole distinction. Asking only "is there a spread" refuses + * GraphiQL; following every spread descends a `TypeRef` chain forever, since those nest + * `ofType { ofType { ... } }` by design. + * + * @param selectionSet Selections to inspect, `null` for a leaf field such as `__typename`. + * @param fragments Fragment definitions of the document. + * @param visited Fragment names already walked on this branch, so a cycle terminates. + */ +private fun carriesExpandingSpread( + selectionSet: SelectionSet?, + fragments: Map, + visited: Set +): Boolean { + val selections = selectionSet?.selections ?: return false + + return selections.any { selection -> + when (selection) { + is FragmentSpread -> spreadMultiplies(selection.name, fragments, visited) + is Field -> carriesExpandingSpread(selection.selectionSet, fragments, visited) + is InlineFragment -> carriesExpandingSpread(selection.selectionSet, fragments, visited) + else -> false + } + } +} + +/** + * Whether expanding [name] repeats work, i.e. its body reaches the same spread more than once. + * + * @param name Fragment being expanded. + * @param fragments Fragment definitions of the document. + * @param visited Names already walked on this branch, so a cycle terminates. + */ +private fun spreadMultiplies( + name: String, + fragments: Map, + visited: Set +): Boolean { + if (name in visited) { + return false + } + + val body = fragments[name]?.selectionSet ?: return false + + return body.spreadNamesBelow(visited + name) + .groupingBy { it } + .eachCount() + .values + .any { occurrences -> occurrences > 1 } +} + +/** + * Every fragment spread reachable below this selection set, fields included, counted with + * multiplicity. + * + * Depth matters: `fragment F0 { a: types { ...F1 } b: types { ...F1 } }` repeats `...F1` twice, but + * not twice at its *root* — each sits under its own field, so counting only the root sees none. + * Fragments already on this branch are skipped, so a cycle contributes nothing. + * + * @param visited Names already on this branch, so a cycle terminates. + * @param depth Nesting level, bounding a legitimately deep schema walk. + */ +private fun SelectionSet.spreadNamesBelow( + visited: Set, + depth: Int = 0 +): List { + if (depth > MAX_TRACKED_DEPTH) { + return emptyList() + } + + return selections.flatMap { selection -> + when (selection) { + is FragmentSpread -> if (selection.name in visited) emptyList() else listOf(selection.name) + + is Field -> selection.selectionSet?.spreadNamesBelow(visited, depth + 1) ?: emptyList() + is InlineFragment -> selection.selectionSet.spreadNamesBelow(visited, depth + 1) + else -> emptyList() + } + } +} + + + +/** + * Whether the document breaks one of the [limits]. + * + * @param document Parsed document to check. + * @param limits Bounds to enforce. + * @return `true` when the document must not be executed. + */ +fun exceedsLimits(document: Document, limits: SecurityLimits): Boolean { + if (isIntrospectionOnly(document)) { + return !limits.introspectionEnabled + } + + // Cost is checked first, and each walk stops at its own ceiling. The order matters: pricing is + // the cheaper of the two analyses, so a document refused on cost never pays for the depth walk, + // which resolves spreads and is the more explosive of the pair. + val cost = estimateCost(document, limits) + + if (cost > limits.maxQueryComplexity) { + return true + } + + return operationsMaxDepth(document, limits) > limits.maxQueryDepth +} + +/** + * Deepest selection nesting of the document, counting from 1 for a root field. + * + * The walk stops at [MAX_TRACKED_DEPTH] rather than recursing further: a document deep enough to + * hit the bound is already refused, so counting the exact depth past that point would cost time on + * exactly the inputs an attacker sends. + */ +private fun operationsMaxDepth(document: Document, limits: SecurityLimits): Int = + document.definitions + .filterIsInstance() + .maxOfOrNull { operation -> + depthOf( + selectionSet = operation.selectionSet, + current = 0, + fragments = documentFragments(document), + limits = limits, + budget = Budget(limits.depthVisitBudget) + ) + } + ?: 0 + +/** + * Deepest nesting, counting fragment spreads as the depth of the fragment they expand to. + * + * Resolving spreads matters for the limits to mean anything: the fragment bomb nests through + * fragments, so a document whose visible fields look three deep can expand to fifty. Cycle + * tracking is per branch, so a fragment that spreads itself stops instead of looping. + */ +private fun depthOf( + selectionSet: SelectionSet, + current: Int, + fragments: Map = emptyMap(), + expanding: Set = emptySet(), + limits: SecurityLimits, + budget: Budget +): Int { + // An exhausted budget means the walk gave up before finishing, not that the document is + // shallow. Reporting `current` there accepted a document whose expansion was never walked, so + // a truncated walk reports past every possible bound instead. + if (budget.exhausted()) { + return Int.MAX_VALUE + } + + // Stopping at the configured bound, capped, keeps the walk bounded by the same number the + // caller will compare it against. + if (current >= minOf(MAX_TRACKED_DEPTH, limits.maxQueryDepth + 1)) { + return current + } + + budget.spend() + + return selectionSet.selections.maxOfOrNull { selection -> + val childSelections = when (selection) { + is Field -> selection.selectionSet + is InlineFragment -> selection.selectionSet + is FragmentSpread -> { + val definition = fragments[selection.name] + + if (definition == null || selection.name in expanding) { + null + } else { + return@maxOfOrNull depthOf( + selectionSet = definition.selectionSet, + current = current + 1, + fragments = fragments, + expanding = expanding + selection.name, + limits = limits, + budget = budget + ) + } + } + + else -> null + } + + if (childSelections == null) { + current + 1 + } else { + depthOf( + selectionSet = childSelections, + current = current + 1, + fragments = fragments, + expanding = expanding, + limits = limits, + budget = budget + ) + } + } ?: current + 1 +} + +/** + * Shared allowance for one analysis walk. + * + * The walks resolve fragment spreads, so their cost is exponential in the number of fragments while + * the document stays comfortably inside the body limit. The budget is what stops that: once spent, + * the walk stops descending and reports what it has. It is mutable and passed by reference so the + * whole walk shares one allowance rather than restarting it per branch. + */ +private class Budget(private var remaining: Long) { + fun exhausted(): Boolean = + remaining <= 0L + + fun spend() { + remaining-- + } + + fun remainingVisits(): Long = + remaining +} + +/** + * Number of nodes the depth walk visits before its budget runs out. + * + * Exposed so the bound is testable: a wall-clock assertion cannot see the difference between a + * linear walk and an exponential one reliably, but a node count can. + */ +internal fun countDepthNodes(document: Document, limits: SecurityLimits): Long { + val fragments = documentFragments(document) + val budget = Budget(limits.depthVisitBudget) + + document.definitions + .filterIsInstance() + .forEach { operation -> + depthOf(operation.selectionSet, 0, fragments, emptySet(), limits, budget) + } + + return limits.depthVisitBudget - budget.remainingVisits() +} + +private fun documentFragments(document: Document): Map = + document.definitions + .filterIsInstance() + .associateBy { fragment -> fragment.name } + +private const val MAX_TRACKED_DEPTH = 64 + +/** Visit budget multiplier applied to the cost bound. */ +private const val COST_VISITS_PER_BYTE = 64L + +/** Visit budget multiplier applied to the depth bound. */ +private const val DEPTH_VISITS_PER_BYTE = 64L + +/** Head-room the visit budgets keep for fragment expansion beyond the bound itself. */ +private const val FRAGMENT_VISIT_FACTOR = 256L + +/** The root fields that make a document an introspection request. */ +/** + * The root fields that read the schema, and so can *start* an introspection request. + * + * `__typename` is deliberately absent: it resolves against the query type rather than the schema, + * so it carries no introspection work. Listing it here would exempt any document whose selections + * are all `__typename` — including one that aliases it thousands of times — which hands the + * exemption to the cheapest possible bomb. It is accepted as an addition instead, in + * [COMPANION_META_FIELDS], because GraphiQL, Apollo and Relay all send it. + */ +private val META_FIELDS = setOf("__schema", "__type") + +/** + * Meta fields that may accompany an introspection request without granting the exemption. + * + * `__typename` costs one unit and reveals the query type's name. Refusing a client that sends it + * alongside `__schema` would break the codegen tools the exemption exists for. + */ +private val COMPANION_META_FIELDS = setOf("__typename") diff --git a/graphql/src/main/kotlin/UuidScalar.kt b/graphql/src/main/kotlin/UuidScalar.kt new file mode 100644 index 0000000..a74e731 --- /dev/null +++ b/graphql/src/main/kotlin/UuidScalar.kt @@ -0,0 +1,121 @@ +package fr.shikkanime.graphql + +import com.expediagroup.graphql.generator.hooks.FlowSubscriptionSchemaGeneratorHooks +import com.expediagroup.graphql.generator.hooks.SchemaGeneratorHooks +import graphql.GraphQLContext +import graphql.execution.CoercedVariables +import graphql.language.StringValue +import graphql.language.Value +import graphql.schema.Coercing +import graphql.schema.CoercingParseLiteralException +import graphql.schema.CoercingParseValueException +import graphql.schema.CoercingSerializeException +import graphql.schema.GraphQLScalarType +import graphql.schema.GraphQLType +import java.util.Locale +import kotlin.reflect.KType +import kotlin.uuid.Uuid + +/** + * GraphQL scalar carrying a [Uuid] across the schema. + * + * graphql-kotlin maps only a handful of Kotlin types to built-in scalars, so a shared DTO holding + * a [Uuid] identifier would otherwise fail schema generation and force a duplicated DTO typed on + * strings. The scalar speaks the canonical hyphenated hexadecimal form in both directions. + */ +val uuidScalar: GraphQLScalarType = + GraphQLScalarType.newScalar() + .name("UUID") + .description("kotlin.uuid.Uuid rendered as its hyphenated hexadecimal string") + .coercing(UuidCoercing()) + .build() + +/** + * Reads and writes [Uuid] values for [uuidScalar]. + * + * Every direction funnels through `Uuid.parseOrNull` and `Uuid.toString`, so the schema, the + * variables and the serialized response all agree on the same textual form. + */ +class UuidCoercing : Coercing { + /** + * Renders a resolver result as its textual identifier. + * + * @param dataFetcherResult value produced by a resolver, expected to be a [Uuid]. + * @param graphQLContext context of the running operation. + * @param locale locale of the running operation. + * @return the hyphenated hexadecimal form of [dataFetcherResult]. + * @throws CoercingSerializeException when the result is not a [Uuid]. + */ + override fun serialize( + dataFetcherResult: Any, + graphQLContext: GraphQLContext, + locale: Locale + ): String = + (dataFetcherResult as? Uuid)?.toString() + ?: throw CoercingSerializeException("Cannot serialize $dataFetcherResult as a UUID") + + /** + * Parses a variable value into a [Uuid]. + * + * @param input raw variable value, expected to be a textual identifier. + * @param graphQLContext context of the running operation. + * @param locale locale of the running operation. + * @return the parsed identifier. + * @throws CoercingParseValueException when [input] is not a valid identifier. + */ + override fun parseValue(input: Any, graphQLContext: GraphQLContext, locale: Locale): Uuid = + (input as? String)?.let(Uuid::parseOrNull) + ?: throw CoercingParseValueException("Cannot parse $input as a UUID") + + /** + * Parses an inline literal into a [Uuid]. + * + * @param input literal node from the query document. + * @param variables coerced variables of the running operation. + * @param graphQLContext context of the running operation. + * @param locale locale of the running operation. + * @return the parsed identifier. + * @throws CoercingParseLiteralException when [input] is not a valid identifier. + */ + override fun parseLiteral( + input: Value<*>, + variables: CoercedVariables, + graphQLContext: GraphQLContext, + locale: Locale + ): Uuid = + (input as? StringValue)?.value?.let(Uuid::parseOrNull) + ?: throw CoercingParseLiteralException("Cannot parse $input as a UUID") + + /** + * Renders a [Uuid] back into a literal node. + * + * @param input identifier to render. + * @param graphQLContext context of the running operation. + * @param locale locale of the running operation. + * @return the identifier as a string literal node. + */ + override fun valueToLiteral(input: Any, graphQLContext: GraphQLContext, locale: Locale): Value<*> = + StringValue.of((input as Uuid).toString()) +} + +/** + * Schema hooks exposing [uuidScalar] for every [Uuid] the schema meets. + * + * Every other hook keeps the framework default ([FlowSubscriptionSchemaGeneratorHooks]): the + * scalar is a widening of the generator, not a replacement of its behaviour. + */ +class UuidSchemaGeneratorHooks( + private val delegate: SchemaGeneratorHooks = FlowSubscriptionSchemaGeneratorHooks() +) : SchemaGeneratorHooks by delegate { + /** + * Resolves the GraphQL type of a Kotlin type. + * + * @param type Kotlin type the generator is building a schema type for. + * @return [uuidScalar] when [type] is a [Uuid], the delegate answer otherwise. + */ + override fun willGenerateGraphQLType(type: KType): GraphQLType? = + if (type.classifier == Uuid::class) + uuidScalar + else + delegate.willGenerateGraphQLType(type) +} diff --git a/graphql/src/test/kotlin/CostEstimatorHardeningTest.kt b/graphql/src/test/kotlin/CostEstimatorHardeningTest.kt new file mode 100644 index 0000000..5918b04 --- /dev/null +++ b/graphql/src/test/kotlin/CostEstimatorHardeningTest.kt @@ -0,0 +1,259 @@ +package fr.shikkanime.graphql + +import graphql.language.Document +import graphql.parser.Parser +import org.junit.jupiter.api.DisplayName +import org.junit.jupiter.api.Nested +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.Timeout +import java.util.concurrent.TimeUnit +import kotlin.test.assertEquals +import kotlin.test.assertTrue + +/** + * Cost and depth analysis under adversarial input. + * + * Both walks run on untrusted documents, so their own worst case matters as much as their verdict: a + * guard that refuses the right document after exponential work is still a denial of service. The + * ceiling is therefore part of the contract, and these tests pin it. + */ +class CostEstimatorHardeningTest { + private fun parse(query: String): Document = + Parser().parseDocument(query) + + /** + * A ladder of fragments where each level spreads the next one twice, so the cost doubles at + * every level. Under 1.3 KB it reaches 2^31, and both walks are exponential unless bounded. + */ + private fun fragmentLadder(levels: Int): String { + val document = StringBuilder("{ ...F0 }") + + for (level in 0 until levels) { + val next = if (level == levels - 1) "episodes { ping }" else "...F${level + 1}" + + document.append(" fragment F$level on Query { $next $next }") + } + + return document.toString() + } + + private val limits = SecurityLimits(maxQueryDepth = 8, maxQueryComplexity = 200) + + @Nested + @DisplayName("Given an exponential fragment ladder") + inner class GivenExponentialFragmentLadder { + @Test + @Timeout(value = 10, unit = TimeUnit.SECONDS) + fun `should saturate at the ceiling of the configured bound`() { + // The ceiling is derived from the bound rather than hard-coded: a fixed ceiling lower + // than a consumer's `maxQueryComplexity` would saturate *below* the bound, and the + // hostile document would then pass `cost > maxQueryComplexity`. + // Given + val document = parse(fragmentLadder(levels = 28)) + + // When + val cost = estimateCost(document, limits) + + // Then + assertEquals(limits.costCeiling, cost) + } + + @Test + @Timeout(value = 10, unit = TimeUnit.SECONDS) + fun `should follow the ceiling of a raised bound`() { + // The regression this pins: with a hard-coded ceiling of 200 and a consumer bound of + // 20 000, a saturated document reported 200 and passed the check. + // Given + val generous = SecurityLimits(maxQueryDepth = 8, maxQueryComplexity = 20_000) + val document = parse(fragmentLadder(levels = 28)) + + // When + val cost = estimateCost(document, generous) + + // Then + assertEquals(generous.costCeiling, cost) + assertTrue(cost > generous.maxQueryComplexity, "saturation must break the bound it belongs to") + } + + @Test + @Timeout(value = 10, unit = TimeUnit.SECONDS) + fun `should saturate rather than wrap around to a small cost`() { + // The overflow is what turns a slow guard into an open one: 2^32 as an Int is 0, and a + // zero cost both passes the bound and weighs 1 in the rate limiter — the one outcome + // the module forbids outright. + // Given + val document = parse(fragmentLadder(levels = 26)) + + // When + val cost = estimateCost(document, limits) + + // Then + assertTrue(cost >= limits.costCeiling, "cost must never wrap below the ceiling, got $cost") + } + + @Test + @Timeout(value = 10, unit = TimeUnit.SECONDS) + fun `should keep pricing ordinary documents exactly`() { + // The ceiling must not leak into legitimate pricing: the same number is charged to the + // rate limit bucket, so it has to stay exact below the ceiling. + // Given + val document = parse("{ episodes { anime { title } } }") + + // When + val cost = estimateCost(document, limits) + + // Then + assertEquals(3, cost) + } + + @Test + @Timeout(value = 10, unit = TimeUnit.SECONDS) + fun `should be refused on cost once it saturates`() { + // Given + val document = parse(fragmentLadder(levels = 28)) + + // When & Then + assertTrue(exceedsLimits(document, limits), "an explosive document must break the complexity bound") + } + } + + @Nested + @DisplayName("Given an explosive document priced for the depth walk") + inner class GivenExplosiveDocumentDepthWalk { + @Test + @Timeout(value = 10, unit = TimeUnit.SECONDS) + fun `should refuse it without walking every branch`() { + // Bounding `estimateCost` alone is not enough: `exceedsLimits` also runs the depth walk, + // which expands the same ladder. Capping the cost first means the depth walk is never + // reached for a document that is already refused on cost. + // Given + val document = parse(fragmentLadder(levels = 24)) + + // When & Then — the timeout is the assertion; the walk would otherwise take minutes. + assertTrue(exceedsLimits(document, limits)) + } + + @Test + @Timeout(value = 10, unit = TimeUnit.SECONDS) + fun `should measure depth within the node budget`() { + // Depth is bounded per branch, but the total number of branches is not: a ladder of + // 20 fragments is 20 levels deep, far under `MAX_TRACKED_DEPTH`, yet expands into a + // million branches. The counter is what makes that observable without a clock. + // Given + val document = parse(fragmentLadder(levels = 20)) + + // When + val visited = countDepthNodes(document, limits) + + // Then + assertTrue( + visited <= limits.depthVisitBudget, + "depth walk visited $visited nodes, over the budget of ${limits.depthVisitBudget}" + ) + } + } + + @Nested + @DisplayName("Given a document nested deeper than the parser allows") + inner class GivenUnboundedSelectionNesting { + @Test + @Timeout(value = 10, unit = TimeUnit.SECONDS) + fun `should price it instead of overflowing the stack`() { + // The parser refuses such a document on the request path, but `estimateCost` is public + // and has to hold on its own. An unbounded recursion raises a `StackOverflowError`, and + // that is an `Error` rather than an exception: it cannot be caught, so it takes the + // thread down instead of failing the request. + // Given — built through the API, because the parser caps nesting at 500 levels. + var selection: graphql.language.Selection<*> = graphql.language.Field.newField("ping").build() + + repeat(3_000) { + selection = graphql.language.Field.newField("nested") + .selectionSet( + graphql.language.SelectionSet.newSelectionSet() + .selections(listOf(selection)) + .build() + ) + .build() + } + + val document = graphql.language.Document.newDocument() + .definition( + graphql.language.OperationDefinition.newOperationDefinition() + .name("Deep") + .selectionSet( + graphql.language.SelectionSet.newSelectionSet() + .selections(listOf(selection)) + .build() + ) + .build() + ) + .build() + + // When + val cost = estimateCost(document, limits) + + // Then + assertTrue(cost >= 1L, "a deeply nested document still has a positive cost, got $cost") + } + } + + @Nested + @DisplayName("Given the weight charged for a saturated document") + inner class GivenWeightFromSaturatedDocument { + @Test + fun `should never charge a free request`() { + // The module rules state it outright: a zero-weight request is unlimited, which is + // worse than no rate limit at all. Saturation must land on the ceiling and the + // conversion must keep the floor. + // Given + val saturated = estimateCost(parse(fragmentLadder(levels = 26)), limits) + + // When + val weight = saturated.toRateLimitWeight(minimum = 1, maximum = 100) + + // Then + assertTrue(weight >= 1, "weight must be at least one, got $weight") + assertTrue(weight > 1, "a saturated document must cost more than a trivial one, got $weight") + } + + @Test + fun `should never charge more than the bucket it drains`() { + // A weight above the bucket capacity makes the bucket unreachable for the whole refill + // period, which turns one heavy document into a self-inflicted outage. + // Given + val saturated = estimateCost(parse(fragmentLadder(levels = 26)), limits) + + // When + val weight = saturated.toRateLimitWeight(minimum = 1, maximum = 100) + + // Then + assertTrue(weight in 1..100, "weight must fit the bucket, got $weight") + } + + @Test + fun `should charge a trivial document exactly one`() { + // Given + val trivial = estimateCost(parse("{ ping }"), limits) + + // When + val weight = trivial.toRateLimitWeight(minimum = 1, maximum = 100) + + // Then + assertEquals(1, weight) + } + + @Test + fun `should charge an affordable document its real cost`() { + // Between the floor and the cap the cost is exact: that is the whole point of charging + // by cost rather than by request. + // Given + val affordable = estimateCost(parse("{ " + (1..40).joinToString(" ") { "a$it: ping" } + " }"), limits) + + // When + val weight = affordable.toRateLimitWeight(minimum = 1, maximum = 100) + + // Then + assertEquals(40, weight) + } + } +} \ No newline at end of file diff --git a/graphql/src/test/kotlin/CostEstimatorPerformanceTest.kt b/graphql/src/test/kotlin/CostEstimatorPerformanceTest.kt new file mode 100644 index 0000000..1aa1a8e --- /dev/null +++ b/graphql/src/test/kotlin/CostEstimatorPerformanceTest.kt @@ -0,0 +1,238 @@ +package fr.shikkanime.graphql + +import graphql.parser.Parser +import org.junit.jupiter.api.DisplayName +import org.junit.jupiter.api.Nested +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.Timeout +import java.util.concurrent.TimeUnit +import kotlin.system.measureNanoTime +import kotlin.test.assertEquals +import kotlin.test.assertEquals +import kotlin.test.assertTrue + +/** + * Performance guards for the cost and depth walks. + * + * A wall-clock assertion is flaky in CI, so it is deliberately absent: the runners throttle CPU and + * pause for GC, and a threshold loose enough to survive that gives false confidence about a + * regression to exponential expansion. What is asserted instead is the *shape* of the work — the + * number of nodes each walk visits — which is a deterministic function of the document and holds + * identically on every machine. + * + * `@Timeout` covers the remaining half: only it can notice a hang, and it fails the build instead of + * stalling the suite. It is set wide enough that a loaded runner does not trip it, and narrow enough + * that the exponential regression fails in seconds rather than minutes. + */ +class CostEstimatorPerformanceTest { + private val limits = SecurityLimits(maxQueryDepth = 8, maxQueryComplexity = 200) + + private companion object { + /** + * How much slower a four-times-larger document may price. + * + * A linear walk lands near 4, quadratic near 16. Eight leaves room for the measurement + * noise of a shared runner while still failing on any super-linear work. + */ + const val LINEAR_SCALING_TOLERANCE = 8.0 + + /** + * How many branches a 22-level ladder would expand to with no depth bound at all. + * + * 2^22 is past `Int`, so the walk cannot reach it — that is the point. The measured visit + * count has to stay far below it for the depth bound to be what stopped the walk. + */ + const val EXPLOSIVE_LADDER_UNBOUNDED = 1_000_000L + } + + private fun fragmentLadder(levels: Int): String { + val document = StringBuilder("{ ...F0 }") + + for (level in 0 until levels) { + val next = if (level == levels - 1) "episodes { ping }" else "...F${level + 1}" + + document.append(" fragment F$level on Query { $next $next }") + } + + return document.toString() + } + + @Nested + @DisplayName("Given the nodes the cost walk visits") + inner class GivenCostWalkNodes { + @Test + @Timeout(value = 10, unit = TimeUnit.SECONDS) + fun `should stay within the budget on a wide document`() { + // A 64 KiB body of aliases is the largest shape worth pricing. Linear in the fields, + // so the node count is the field count — anything super-linear means the walk stopped + // visiting each selection once. + // Given + val aliases = (1..4_000).joinToString(" ") { "a$it: ping" } + val document = Parser().parseDocument("{ $aliases }") + + // When + val visited = countCostNodes(document, limits) + + // Then — strictly under, not `<=`: the counter is derived from the budget itself, so + // `<=` holds by construction and would pass even if the walk looped forever. `<` is what + // says the document finished inside its allowance. + assertTrue( + visited < limits.costVisitBudget, + "cost walk used the whole budget ($visited of ${limits.costVisitBudget}); " + + "the walk must finish inside its allowance, not be cut off" + ) + } + + @Test + @Timeout(value = 10, unit = TimeUnit.SECONDS) + fun `should scale its time with the field count, not its square`() { + // A wall-clock assertion on a single document proves nothing here: measured, pricing a + // 4000-field document takes ~0.013 ms, so a 250 ms threshold would still pass after a + // 3800x regression — exactly the false confidence a loose bound gives. What matters is + // the *shape* of the cost, so this compares two documents and asserts a ratio: pricing + // four times the fields must stay roughly four times the work, not sixteen. A regression + // to quadratic or exponential work fails at any absolute threshold. + // Given + val small = Parser().parseDocument("{ " + (1..1_000).joinToString(" ") { "a$it: ping" } + " }") + val large = Parser().parseDocument("{ " + (1..4_000).joinToString(" ") { "a$it: ping" } + " }") + + // Warm up so JIT compilation is not charged to the first measurement. + repeat(20) { estimateCost(small, limits); estimateCost(large, limits) } + + // When + val smallCost = measureNanoTime { repeat(40) { estimateCost(small, limits) } } + val largeCost = measureNanoTime { repeat(40) { estimateCost(large, limits) } } + + // Then — generous on both sides so a loaded runner cannot trip it, tight enough that + // super-linear work fails. + val ratio = largeCost.toDouble() / smallCost.coerceAtLeast(1L) + assertTrue( + ratio < LINEAR_SCALING_TOLERANCE, + "4x the fields cost ${"%.1f".format(ratio)}x the time, over the " + + "${LINEAR_SCALING_TOLERANCE}x a linear walk allows" + ) + } + + @Test + @Timeout(value = 10, unit = TimeUnit.SECONDS) + fun `should visit nodes linearly on a flat document`() { + // Ten fields must not cost more node visits than a hundred by a factor of ten squared. + // This is the invariant that a memoisation bug or a double-counting would break. + // Given + val ten = Parser().parseDocument("{ " + (1..10).joinToString(" ") { "a$it: ping" } + " }") + val hundred = Parser().parseDocument("{ " + (1..100).joinToString(" ") { "a$it: ping" } + " }") + + // When + val tenVisits = countCostNodes(ten, limits) + val hundredVisits = countCostNodes(hundred, limits) + + // Then + assertTrue( + hundredVisits <= tenVisits * 12, + "visits grew from $tenVisits to $hundredVisits for a 10x wider document" + ) + } + } + + @Nested + @DisplayName("Given the nodes the depth walk visits") + inner class GivenDepthWalkNodes { + @Test + @Timeout(value = 10, unit = TimeUnit.SECONDS) + fun `should stay within the budget on an explosive ladder`() { + // The ladder is 20 levels deep, comfortably under `MAX_TRACKED_DEPTH`, but expands into + // a million branches. Depth is bounded per branch and unbounded in total, which is the + // gap this budget closes. + // Given + val document = Parser().parseDocument(fragmentLadder(levels = 20)) + + // When + val visited = countDepthNodes(document, limits) + + // Then + assertTrue( + visited <= limits.depthVisitBudget, + "depth walk visited $visited nodes, over the budget of ${limits.depthVisitBudget}" + ) + } + + @Test + @Timeout(value = 10, unit = TimeUnit.SECONDS) + fun `should not grow with the level count`() { + // Two ladders of very different explosive size must cost the same bounded amount of + // work: the walk stops at the configured depth either way, so the fragment count does + // not reach it. Before that bound existed, the deeper one took thousands of times + // longer. + // Given + val small = Parser().parseDocument(fragmentLadder(levels = 12)) + val large = Parser().parseDocument(fragmentLadder(levels = 22)) + + // When + val smallVisits = countDepthNodes(small, limits) + val largeVisits = countDepthNodes(large, limits) + + // Then — the walk stops as soon as it is past the configured depth bound, which is + // long before the node budget could run out. Asserting `== depthVisitBudget` here + // would pin the weaker of the two stops; this one is the one that keeps the walk short. + assertTrue( + largeVisits <= limits.depthVisitBudget, + "the larger ladder exceeded the budget: $largeVisits" + ) + assertTrue( + largeVisits < EXPLOSIVE_LADDER_UNBOUNDED, + "an explosive ladder must stop well before the unbounded count, got $largeVisits" + ) + // Both ladders stop at the depth bound, so they cost the *same* bounded number of + // visits. That equality is the property: a walk bounded by the configured depth does + // not grow with the fragment count, which is what the exponential regression broke. + assertEquals( + smallVisits, + largeVisits, + "a deeper ladder must not cost more branches once the depth bound applies" + ) + } + } + + @Nested + @DisplayName("Given a cyclic document") + inner class GivenCyclicDocument { + @Test + @Timeout(value = 10, unit = TimeUnit.SECONDS) + fun `should terminate on mutually recursive fragments`() { + // Termination is a performance property as much as a correctness one: a walk that does + // not stop on a cycle is a hang, not a wrong number. + // Given + val document = Parser().parseDocument( + "{ episodes { ...A } } fragment A on Episode { ...B } fragment B on Episode { ...A }" + ) + + // When + val cost = estimateCost(document, limits) + val refused = exceedsLimits(document, limits) + + // Then + assertTrue(cost >= 1, "a cyclic document still has a positive cost") + assertTrue(!refused, "a two-fragment cycle expands to almost nothing") + } + } + + @Nested + @DisplayName("Given a document priced to the ceiling") + inner class GivenDocumentAtTheCeiling { + @Test + @Timeout(value = 10, unit = TimeUnit.SECONDS) + fun `should be flat past the ceiling`() { + // Saturation flattens the curve: levels that differ by thousands of units of true cost + // all report the same number. That is the property a guard relies on to refuse fast. + // Given + val documents = listOf(24, 28, 30).map { Parser().parseDocument(fragmentLadder(levels = it)) } + + // When + val costs = documents.map { estimateCost(it, limits) } + + // Then + assertEquals(1, costs.distinct().size, "every saturated document must report the same cost: $costs") + assertEquals(limits.costCeiling, costs.first()) + } + } +} \ No newline at end of file diff --git a/graphql/src/test/kotlin/CostEstimatorTest.kt b/graphql/src/test/kotlin/CostEstimatorTest.kt new file mode 100644 index 0000000..8a70b75 --- /dev/null +++ b/graphql/src/test/kotlin/CostEstimatorTest.kt @@ -0,0 +1,220 @@ +package fr.shikkanime.graphql + +import graphql.parser.Parser +import graphql.language.Document +import org.junit.jupiter.api.DisplayName +import org.junit.jupiter.api.Nested +import org.junit.jupiter.api.Test +import org.junit.jupiter.params.ParameterizedTest +import org.junit.jupiter.params.provider.ValueSource +import kotlin.test.assertEquals +import kotlin.test.assertTrue + +class CostEstimatorTest { + private fun parse(query: String): Document = + Parser().parseDocument(query) + + @Nested + @DisplayName("Given a trivial query") + inner class GivenTrivialQuery { + @Test + fun `should cost exactly one`() { + // Given + val document = parse("{ ping }") + + // When + val cost = estimateCost(document) + + // Then + assertEquals(1, cost) + } + } + + @Nested + @DisplayName("Given aliased fields") + inner class GivenAliasedFields { + @Test + fun `should count one unit per alias`() { + // Given — the aliasing bomb: tiny payload, huge execution cost. + val document = parse("{ a: ping b: ping c: ping d: ping e: ping }") + + // When + val cost = estimateCost(document) + + // Then + assertEquals(5, cost) + } + + @Test + fun `should count the same field once when it is not aliased`() { + // Given + val document = parse("{ ping }") + + // When + val cost = estimateCost(document) + + // Then + assertEquals(1, cost) + } + + @Test + fun `should grow linearly with the alias count`() { + // Given + val five = parse("{ a1: ping a2: ping a3: ping a4: ping a5: ping }") + val ten = parse( + "{ a1: ping a2: ping a3: ping a4: ping a5: ping " + + "a6: ping a7: ping a8: ping a9: ping a10: ping }" + ) + + // When + val costFive = estimateCost(five) + val costTen = estimateCost(ten) + + // Then + assertEquals(5, costFive) + assertEquals(10, costTen) + } + } + + @Nested + @DisplayName("Given nested selections") + inner class GivenNestedSelections { + @Test + fun `should add the children of each field`() { + // Given — `episodes` plus its `title`, versus `episodes`, `anime` and `title`. + val shallow = parse("{ episodes { title } }") + val deeper = parse("{ episodes { anime { title } } }") + + // When + val costShallow = estimateCost(shallow) + val costDeeper = estimateCost(deeper) + + // Then + assertEquals(2, costShallow) + assertEquals(3, costDeeper) + } + + @Test + fun `should count two for two sibling fields`() { + // Given + val flat = parse("{ a: ping b: ping }") + + // When + val cost = estimateCost(flat) + + // Then + assertEquals(2, cost) + } + + @Test + fun `should ignore an inline fragment`() { + // An inline fragment is a filter, not extra work: it selects the same fields. + // Given + val document = parse("{ episodes { ... on Episode { title } } }") + + // When + val cost = estimateCost(document) + + // Then + assertEquals(2, cost) + } + } + + @Nested + @DisplayName("Given a fragment") + inner class GivenFragment { + @Test + fun `should count the spread selections`() { + // Given + val document = parse( + """ + query Q { episodes { ...F } } + fragment F on Episode { title number } + """.trimIndent() + ) + + // When + val cost = estimateCost(document) + + // Then + assertEquals(3, cost) + } + + @Test + fun `should not hang on a cyclic spread`() { + // A self-referencing fragment is the recursion bomb; the walk must stop instead of + // overflowing the stack. + // Given + val document = parse( + """ + query Q { episodes { ...F } } + fragment F on Episode { title ...F } + """.trimIndent() + ) + + // When + val cost = estimateCost(document) + + // Then + assertEquals(2, cost) + } + } + + @Nested + @DisplayName("Given a large but shallow document") + inner class GivenLargeButShallowDocument { + @Test + fun `should still report a high cost`() { + // Depth limits do not catch this shape: a thousand flat fields is one level deep but + // a thousand times the work. + // Given + val document = parse("{ " + (1..1000).joinToString(" ") { "a$it: ping" } + " }") + + // When + val cost = estimateCost(document) + + // Then + assertEquals(1000, cost) + } + } + + @Nested + @DisplayName("Given an introspection query") + inner class GivenIntrospectionQuery { + @ParameterizedTest + @ValueSource( + strings = [ + "{ __schema { types { name } } }", + "{ __type(name: \"Episode\") { name fields { name } } }" + ] + ) + fun `should report a positive cost`(query: String) { + // Given + val document = parse(query) + + // When + val cost = estimateCost(document) + + // Then + assertTrue(cost > 0) + } + } + + @Nested + @DisplayName("Given an empty document") + inner class GivenEmptyDocument { + @Test + fun `should never report a zero cost`() { + // A zero weight would make the request free in the rate limiter, which is worse than + // having no rate limit at all. + // Given + val document = parse("query Empty { __typename }") + + // When + val cost = estimateCost(document) + + // Then + assertTrue(cost >= 1, "cost must be at least one, got $cost") + } + } +} diff --git a/graphql/src/test/kotlin/DataLoadersTest.kt b/graphql/src/test/kotlin/DataLoadersTest.kt new file mode 100644 index 0000000..071b5d9 --- /dev/null +++ b/graphql/src/test/kotlin/DataLoadersTest.kt @@ -0,0 +1,100 @@ +package fr.shikkanime.graphql + +import com.expediagroup.graphql.dataloader.KotlinDataLoader +import com.expediagroup.graphql.dataloader.KotlinDataLoaderRegistryFactory +import graphql.GraphQLContext +import org.dataloader.DataLoader +import org.dataloader.DataLoaderFactory +import org.dataloader.DataLoaderOptions +import org.dataloader.DataLoaderRegistry +import org.junit.jupiter.api.DisplayName +import org.junit.jupiter.api.Nested +import org.junit.jupiter.api.Test +import java.util.concurrent.CompletableFuture +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +class DataLoadersTest { + // Fixtures first: everything the tests below rely on. + private val pingLoader = object : KotlinDataLoader { + override val dataLoaderName = "PingDataLoader" + + override fun getDataLoader(graphQLContext: GraphQLContext): DataLoader = + DataLoaderFactory.newDataLoader( + { keys: List -> CompletableFuture.completedFuture(keys) }, + DataLoaderOptions.newOptions().build() + ) + } + + private fun registryOf(factory: KotlinDataLoaderRegistryFactory): DataLoaderRegistry = + factory.generate(GraphQLContext.newContext().build(), null) + + @Nested + @DisplayName("Given no DataLoader registered") + inner class GivenNoDataLoader { + @Test + fun `should generate an empty registry without throwing`() { + // Given + val factory = noOpDataLoaderRegistryFactory() + + // When + val registry = registryOf(factory) + + // Then + assertTrue(registry.keys.isEmpty()) + } + + @Test + fun `should report that it is not usable`() { + // Given + val factory = noOpDataLoaderRegistryFactory() + + // Then + assertFalse(factory.hasLoaders()) + } + } + + @Nested + @DisplayName("Given a consumer registry factory") + inner class GivenConsumerRegistryFactory { + @Test + fun `should expose the consumer loader`() { + // Given + val factory = KotlinDataLoaderRegistryFactory(pingLoader) + + // When + val registry = registryOf(factory) + + // Then + assertTrue(registry.keys.contains("PingDataLoader")) + } + + @Test + fun `should report that it carries loaders`() { + // Given + val factory = KotlinDataLoaderRegistryFactory(pingLoader) + + // Then + assertTrue(factory.hasLoaders()) + } + } + + @Nested + @DisplayName("Given the default factory") + inner class GivenDefaultFactory { + @Test + fun `should register no key at all`() { + // The no-op factory must never register a key, or a resolver looking up a loader by + // name would find nothing while the registry claims to be populated. + // Given + val factory = noOpDataLoaderRegistryFactory() + + // When + val registry = registryOf(factory) + + // Then + assertEquals(0, registry.keys.size) + } + } +} diff --git a/graphql/src/test/kotlin/GraphQLConfigTest.kt b/graphql/src/test/kotlin/GraphQLConfigTest.kt new file mode 100644 index 0000000..4c834f8 --- /dev/null +++ b/graphql/src/test/kotlin/GraphQLConfigTest.kt @@ -0,0 +1,95 @@ +package fr.shikkanime.graphql + +import org.junit.jupiter.api.DisplayName +import org.junit.jupiter.api.Nested +import org.junit.jupiter.api.Test +import kotlin.test.assertEquals +import kotlin.test.assertTrue + +class GraphQLConfigTest { + @Nested + @DisplayName("Given the default configuration") + inner class GivenDefaultConfiguration { + private val config = GraphQLConfig() + + @Test + fun `should expose a usable endpoint path`() { + // Then + assertTrue(config.path.isNotBlank()) + } + + @Test + fun `should keep the security limits positive`() { + // Then + assertTrue(config.maxQueryDepth > 0) + assertTrue(config.maxQueryComplexity > 0) + assertTrue(config.requestBodyLimit > 0) + } + + @Test + fun `should leave introspection public`() { + // Then + assertTrue(config.introspectionEnabled) + } + + @Test + fun `should apply a rate limit by default`() { + // Then + assertTrue(config.rateLimit.enabled) + assertTrue(config.rateLimit.limit > 0) + } + } + + @Nested + @DisplayName("Given a custom configuration") + inner class GivenCustomConfiguration { + @Test + fun `should override every value`() { + // Given + val config = GraphQLConfig( + path = "/internal/graphql", + maxQueryDepth = 3, + maxQueryComplexity = 42, + introspectionEnabled = false, + requestBodyLimit = 1024, + rateLimit = RateLimitConfig(limit = 5, enabled = false) + ) + + // Then + assertEquals("/internal/graphql", config.path) + assertEquals(3, config.maxQueryDepth) + assertEquals(42, config.maxQueryComplexity) + assertEquals(false, config.introspectionEnabled) + assertEquals(1024L, config.requestBodyLimit) + assertEquals(false, config.rateLimit.enabled) + assertEquals(5, config.rateLimit.limit) + } + } + + @Nested + @DisplayName("Given a path without a leading slash") + inner class GivenPathWithoutLeadingSlash { + @Test + fun `should normalize it`() { + // Given + val config = GraphQLConfig(path = "graphql") + + // Then + assertEquals("/graphql", config.normalizedPath) + assertEquals("graphql", config.path, "the raw value must stay untouched") + } + } + + @Nested + @DisplayName("Given a path with a leading slash") + inner class GivenPathWithLeadingSlash { + @Test + fun `should keep it as-is`() { + // Given + val config = GraphQLConfig(path = "/internal/graphql") + + // Then + assertEquals("/internal/graphql", config.normalizedPath) + } + } +} diff --git a/graphql/src/test/kotlin/GraphQLModulesTest.kt b/graphql/src/test/kotlin/GraphQLModulesTest.kt new file mode 100644 index 0000000..1598ed2 --- /dev/null +++ b/graphql/src/test/kotlin/GraphQLModulesTest.kt @@ -0,0 +1,332 @@ +package fr.shikkanime.graphql + +import com.expediagroup.graphql.server.operations.Query +import com.expediagroup.graphql.server.ktor.DefaultKtorGraphQLContextFactory +import io.ktor.client.request.header +import io.ktor.client.request.post +import io.ktor.client.request.setBody +import io.ktor.client.statement.HttpResponse +import io.ktor.client.statement.bodyAsChannel +import io.ktor.client.statement.bodyAsText +import io.ktor.utils.io.jvm.javaio.toInputStream +import io.ktor.http.ContentType +import io.ktor.http.HttpHeaders +import io.ktor.http.HttpStatusCode +import io.ktor.http.contentType +import io.ktor.server.testing.ApplicationTestBuilder +import io.ktor.server.testing.testApplication +import org.junit.jupiter.api.DisplayName +import org.junit.jupiter.api.Nested +import org.junit.jupiter.api.Test +import kotlin.test.assertEquals +import kotlin.test.assertTrue +import java.util.zip.GZIPInputStream +import kotlin.time.Duration.Companion.minutes + +class GraphQLModulesTest { + // Fixtures first: everything the tests below rely on. + // graphql-java rejects a non-public query type, so the fixtures must stay public. + private val config = GraphQLConfig( + maxQueryDepth = 6, + maxQueryComplexity = 100, + requestBodyLimit = 4 * 1024, + rateLimit = RateLimitConfig(limit = 5, refillPeriod = 5.minutes) + ) + + class PingQuery : Query { + fun ping(): String = "pong" + } + + class NestedQuery : Query { + fun episodes(): EpisodeQuery = EpisodeQuery() + } + + class EpisodeQuery { + fun ping(): String = "pong" + } + + private fun ApplicationTestBuilder.graphqlApplication( + config: GraphQLConfig = this@GraphQLModulesTest.config + ) { + application { + configureGraphQL( + config = config, + schemaPackages = listOf("fr.shikkanime.graphql"), + schemaQueries = listOf(PingQuery(), NestedQuery()) + ) + } + } + + private suspend fun ApplicationTestBuilder.post(body: String): HttpResponse = + client.post(config.normalizedPath) { + contentType(ContentType.Application.Json) + setBody(body) + } + + private suspend fun ApplicationTestBuilder.post(body: String, acceptEncoding: String): HttpResponse = + client.post(config.normalizedPath) { + contentType(ContentType.Application.Json) + header(HttpHeaders.AcceptEncoding, acceptEncoding) + setBody(body) + } + + private fun query(vararg fields: String): String { + val selection = if (fields.isEmpty()) "ping" else fields.joinToString(" ") + return """{"query":"{ $selection }"}""" + } + + private fun aliasFields(count: Int): String = + (1..count).joinToString(" ") { index -> "a$index: ping" } + + private suspend fun ApplicationTestBuilder.bodyOf(response: HttpResponse): String = + response.bodyAsText() + + @Nested + @DisplayName("Given a normal query") + inner class GivenNormalQuery { + @Test + fun `should be answered`() = testApplication { + // Given + graphqlApplication() + + // When + val response = post(query()) + + // Then + assertEquals(HttpStatusCode.OK, response.status) + assertTrue(bodyOf(response).contains("pong")) + } + + @Test + fun `should serve a gzip request`() = testApplication { + // Compression stays on: the body limit measures the decoded stream, so a compressed + // payload cannot slip past it. Ktor compresses the response, so the client has to + // decompress it — which also proves the plugin decompressed the *request* correctly + // rather than handing the engine raw bytes. + // Given + graphqlApplication() + + // When + val response = client.post(config.normalizedPath) { + contentType(ContentType.Application.Json) + header(HttpHeaders.AcceptEncoding, "gzip") + setBody(query()) + } + + // Then + assertEquals(HttpStatusCode.OK, response.status) + assertEquals("gzip", response.headers[HttpHeaders.ContentEncoding], "response must be compressed") + + val decompressed = GZIPInputStream(response.bodyAsChannel().toInputStream()) + .readBytes() + .decodeToString() + + assertTrue(decompressed.contains("pong"), "got $decompressed") + } + } + + @Nested + @DisplayName("Given introspection") + inner class GivenIntrospection { + @Test + fun `should be public`() = testApplication { + // The Swagger UI is already public, so the GraphQL schema is too. + // Given + graphqlApplication() + + // When + val response = post("""{"query":"{ __schema { types { name } } }"}""") + + // Then + assertEquals(HttpStatusCode.OK, response.status) + assertTrue(bodyOf(response).contains("__schema")) + } + } + + @Nested + @DisplayName("Given an oversized body") + inner class GivenOversizedBody { + @Test + fun `should be refused`() = testApplication { + // Given — larger than the configured 4 KiB limit. + graphqlApplication() + + // When + val padding = "x".repeat(8 * 1024) + val response = post("""{"query":"{ ping }","variables":{"pad":"$padding"}}""") + + // Then + assertEquals(HttpStatusCode.PayloadTooLarge, response.status) + } + } + + @Nested + @DisplayName("Given a hostile document") + inner class GivenHostileDocument { + @Test + fun `should refuse an alias bomb`() = testApplication { + // Given + graphqlApplication() + + // When + val response = post(query(*aliasFields(200).split(" ").toTypedArray())) + + // Then + assertTrue( + bodyOf(response).contains("errors"), + "expected a refusal, got ${response.status} ${bodyOf(response)}" + ) + } + + @Test + fun `should refuse a deep document`() = testApplication { + // Given + graphqlApplication() + + // When + val deep = "episodes { ".repeat(12) + "ping" + " }".repeat(12) + val response = post("""{"query":"{ $deep }"}""") + + // Then + assertTrue( + bodyOf(response).contains("errors"), + "expected a refusal, got ${response.status} ${bodyOf(response)}" + ) + } + } + + @Nested + @DisplayName("Given a batch request") + inner class GivenBatchRequest { + @Test + fun `should be refused`() = testApplication { + // A batch is not one document: graphql-java would execute every element separately, so + // a check that parses a single document would never see the others. + // Given + graphqlApplication() + + // When + val batch = "[" + (1..10).joinToString(",") { """{"query":"{ ping }"}""" } + "]" + val response = post(batch) + + // Then + assertEquals(HttpStatusCode.BadRequest, response.status) + assertTrue(bodyOf(response).contains("Batch"), "got ${bodyOf(response)}") + } + } + + @Nested + @DisplayName("Given an unreadable document") + inner class GivenUnreadableDocument { + @Test + fun `should be refused instead of reaching the engine`() = testApplication { + // Malformed syntax is the shape an attacker uses to slip past a check that only + // understands valid documents. + // Given + graphqlApplication() + + // When + val response = post("""{"query":"{ ping "}""") + + // Then + assertEquals(HttpStatusCode.BadRequest, response.status) + assertTrue(bodyOf(response).contains("could not be read"), "got ${bodyOf(response)}") + } + + @Test + fun `should refuse a body carrying no query`() = testApplication { + // Given + graphqlApplication() + + // When + val response = post("""{"variables":{}}""") + + // Then + assertEquals(HttpStatusCode.BadRequest, response.status) + } + } + + @Nested + @DisplayName("Given introspection disabled") + inner class GivenIntrospectionDisabled { + @Test + fun `should refuse an introspection query`() = testApplication { + // Given + graphqlApplication(GraphQLConfig( + maxQueryDepth = 6, + maxQueryComplexity = 100, + requestBodyLimit = 4 * 1024, + introspectionEnabled = false, + rateLimit = RateLimitConfig(limit = 5, refillPeriod = 5.minutes) + )) + + // When + val response = post("""{"query":"{ __schema { types { name } } }"}""") + + // Then + assertEquals(HttpStatusCode.BadRequest, response.status) + assertTrue(bodyOf(response).contains("introspection"), "got ${bodyOf(response)}") + } + + @Test + fun `should still answer a normal query`() = testApplication { + // Given + graphqlApplication(GraphQLConfig( + maxQueryDepth = 6, + maxQueryComplexity = 100, + requestBodyLimit = 4 * 1024, + introspectionEnabled = false, + rateLimit = RateLimitConfig(limit = 5, refillPeriod = 5.minutes) + )) + + // When + val response = post(query()) + + // Then + assertEquals(HttpStatusCode.OK, response.status) + assertTrue(bodyOf(response).contains("pong"), "got ${bodyOf(response)}") + } + } + + @Nested + @DisplayName("Given the rate limiter") + inner class GivenRateLimiter { + @Test + fun `should answer 429 once the bucket is empty`() = testApplication { + // The bucket holds `limit` cost units and refills on a 5 minute period, so a ping + // costs one and the sixth request inside the period has nothing left. + // Given + graphqlApplication() + + // When + val statuses = (1..config.rateLimit.limit + 2).map { post(query()).status } + + // Then + assertEquals(HttpStatusCode.TooManyRequests, statuses.last()) + } + + @Test + fun `should charge a heavy document more than a light one`() = testApplication { + // This is the point of requestWeight: a bucket of 5 units fits five pings but only + // two nested documents, since each costs two. + // Given + graphqlApplication() + + // When + val lightStatuses = (1..config.rateLimit.limit).map { post(query()).status } + val heavyStatuses = (1..config.rateLimit.limit).map { + post(query("episodes { ping }")).status + } + + // Then + assertTrue( + lightStatuses.none { it == HttpStatusCode.TooManyRequests }, + "light queries must fit the bucket, got $lightStatuses" + ) + assertTrue( + heavyStatuses.any { it == HttpStatusCode.TooManyRequests }, + "heavy queries must exhaust it faster, got $heavyStatuses" + ) + } + } +} diff --git a/graphql/src/test/kotlin/GraphQLRateLimitBudgetTest.kt b/graphql/src/test/kotlin/GraphQLRateLimitBudgetTest.kt new file mode 100644 index 0000000..039ea4b --- /dev/null +++ b/graphql/src/test/kotlin/GraphQLRateLimitBudgetTest.kt @@ -0,0 +1,180 @@ +package fr.shikkanime.graphql + +import com.expediagroup.graphql.server.operations.Query +import io.ktor.client.request.post +import io.ktor.client.request.setBody +import io.ktor.client.statement.HttpResponse +import io.ktor.client.statement.bodyAsText +import io.ktor.http.ContentType +import io.ktor.http.HttpStatusCode +import io.ktor.http.contentType +import io.ktor.server.testing.ApplicationTestBuilder +import io.ktor.server.testing.testApplication +import org.junit.jupiter.api.DisplayName +import org.junit.jupiter.api.Nested +import org.junit.jupiter.api.Test +import kotlin.test.assertEquals +import kotlin.test.assertTrue + +/** + * Regressions on how the endpoint behaves with the shipped defaults. + * + * Each test pins a behaviour that the security stack must not break on the way to refusing hostile + * documents: introspection is public and must stay usable, and a legitimate query priced above the + * bucket must still be answerable. + */ +class GraphQLRateLimitBudgetTest { + // Fixtures. + private val defaults = GraphQLConfig() + + class PingQuery : Query { + fun ping(): String = "pong" + } + + private fun ApplicationTestBuilder.graphql(config: GraphQLConfig) { + application { + configureGraphQL( + config = config, + schemaPackages = listOf("fr.shikkanime.graphql"), + schemaQueries = listOf(PingQuery()) + ) + } + } + + private suspend fun ApplicationTestBuilder.post(body: String): HttpResponse = + client.post(defaults.normalizedPath) { + contentType(ContentType.Application.Json) + setBody(body) + } + + private fun aliases(count: Int): String = + "{ " + (1..count).joinToString(" ") { "a$it: ping" } + " }" + + @Nested + @DisplayName("Given a schema introspection query") + inner class GivenIntrospectionQuery { + @Test + fun `should be served with the default configuration`() = testApplication { + // Introspection is exempt from the document bounds because it is deep and expensive by + // nature, but the exemption must not leave it priced above the rate limit bucket: the + // weight would then exceed the bucket and every introspection request would be + // answered with 429, which breaks GraphiQL and every codegen client. + // Given + graphql(defaults) + + // When + val response = post("""{"query":"{ __schema { types { name } } }"}""") + + // Then + assertEquals(HttpStatusCode.OK, response.status) + assertTrue(response.bodyAsText().contains("__schema")) + } + + @Test + fun `should stay served when repeated`() = testApplication { + // A single request could fit by luck. The bucket must absorb the realistic number of + // introspection calls a codegen client makes. + // Given + graphql(defaults) + + // When + val statuses = (1..10).map { + post("""{"query":"{ __schema { types { name } } }"}""").status + } + + // Then + assertTrue( + statuses.none { it == HttpStatusCode.TooManyRequests }, + "introspection must not exhaust the bucket, got $statuses" + ) + } + } + + @Nested + @DisplayName("Given a legitimate query priced above the bucket") + inner class GivenAffordableQuery { + @Test + fun `should not be permanently rejected`() = testApplication { + // The bounds promise up to `maxQueryComplexity`; the bucket must therefore hold at least + // one document at that price. Otherwise the advertised range is unusable and the client + // sees 429 for a query the bounds accept. + // Given + graphql(defaults) + + // When — cost 150 sits between the old default bucket and the default complexity bound, + // repeated to prove the limiter does not lock the client out for good. + val body = """{"query":"${aliases(150)}"}""" + val statuses = (1..5).map { post(body).status } + + // Then + assertTrue( + statuses.all { it == HttpStatusCode.OK }, + "a query within the complexity bound must stay answerable, got $statuses" + ) + } + + @Test + fun `should not lock out a cheap query that follows`() = testApplication { + // One heavy request must not spend the whole bucket: a client that sends one big query + // then a trivial one has to get both answered. + // Given + graphql(defaults) + + // When + // `aliases` already closes its selection set, so the body must not close it twice — + // an extra brace made this a parse failure, and the test then measured the + // unreadable-body penalty instead of the cost of a heavy document. + val heavy = post("""{"query":"${aliases(150)}"}""") + + assertEquals(HttpStatusCode.OK, heavy.status, "a query within the bound must be answerable") + + val cheap = post("""{"query":"{ ping }"}""").status + + // Then + assertEquals(HttpStatusCode.OK, cheap) + } + } + + @Nested + @DisplayName("Given a document the endpoint refuses") + inner class GivenRefusedDocument { + @Test + fun `should not spend the whole bucket`() = testApplication { + // Charging a refusal the full bucket lets a few hundred bytes of malformed input lock + // every client behind the same address for a whole refill period. + // Given + graphql(defaults) + + // When + // 400 aliases is past the complexity bound, so this one *is* refused by the bounds — + // with a single closing brace, not by the parser. + val refused = post("""{"query":"${aliases(400)}"}""") + + assertEquals( + HttpStatusCode.BadRequest, + refused.status, + "the document must be refused on its own cost" + ) + + val cheap = post("""{"query":"{ ping }"}""").status + + // Then + assertEquals(HttpStatusCode.OK, cheap) + } + + @Test + fun `should not spend the whole bucket on an unreadable body`() = testApplication { + // An unpriceable body is the cheapest possible input for an attacker, so it must be + // the least expensive thing the limiter can be asked to absorb. + // Given + graphql(defaults) + + // When + post("""{"variables":{"a":1}}""") + val cheap = post("""{"query":"{ ping }"}""").status + + // Then + assertEquals(HttpStatusCode.OK, cheap) + } + } +} \ No newline at end of file diff --git a/graphql/src/test/kotlin/SecurityInstrumentationsTest.kt b/graphql/src/test/kotlin/SecurityInstrumentationsTest.kt new file mode 100644 index 0000000..d8f8882 --- /dev/null +++ b/graphql/src/test/kotlin/SecurityInstrumentationsTest.kt @@ -0,0 +1,257 @@ +package fr.shikkanime.graphql + +import graphql.parser.Parser +import graphql.parser.exceptions.ParseCancelledTooDeepException +import org.junit.jupiter.api.DisplayName +import org.junit.jupiter.api.Nested +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.assertThrows +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +class SecurityInstrumentationsTest { + // Fixtures first. + private val limits = SecurityLimits( + maxQueryDepth = 6, + maxQueryComplexity = 100, + introspectionEnabled = true + ) + + private fun isRefused(query: String, securityLimits: SecurityLimits = limits): Boolean = + exceedsLimits( + document = Parser().parseDocument(query), + limits = securityLimits + ) + + private fun aliasFields(count: Int): String = + (1..count).joinToString(" ") { index -> "a$index: ping" } + + private fun aliasBomb(count: Int): String = + "{ ${aliasFields(count)} }" + + private fun deepQuery(depth: Int): String { + var query = "" + repeat(depth) { query += "{ nested" } + query += " { ping }" + repeat(depth) { query += " }" } + + return query + } + + @Nested + @DisplayName("Given a hostile document") + inner class GivenHostileDocument { + @Test + fun `should refuse a deeply nested query`() { + // Given + val query = deepQuery(depth = 20) + + // Then + assertTrue(isRefused(query), "a depth-20 query must be refused") + } + + @Test + fun `should refuse an alias bomb`() { + // Given — 200 aliases of the same field in a few kilobytes. + val query = aliasBomb(count = 200) + + // Then + assertTrue(isRefused(query), "an alias bomb must be refused") + } + + @Test + fun `should refuse a fragment bomb`() { + // Given — nested fragment spreads, the classic graphql-java recursion attack. + val query = buildString { + append("query Q { ") + repeat(60) { append("f$it: episodes { ...F$it } ") } + append("}") + repeat(60) { append("fragment F$it on Episode { ...F${it + 1} } ") } + append("fragment F60 on Episode { title }") + } + + // Then + assertTrue(isRefused(query), "a fragment bomb must be refused") + } + } + + @Nested + @DisplayName("Given a legitimate document") + inner class GivenLegitimateDocument { + @Test + fun `should accept a normal query`() { + // Given + val query = "{ episodes { number title anime { title } } }" + + // Then + assertFalse(isRefused(query)) + } + + @Test + fun `should accept it even with tight limits`() { + // Given — the limits must not make GraphQL unusable. + val tight = SecurityLimits(maxQueryDepth = 3, maxQueryComplexity = 50) + + // Then + assertFalse(isRefused("{ episodes { title } }", tight)) + } + } + + @Nested + @DisplayName("Given introspection") + inner class GivenIntrospection { + @Test + fun `should survive the depth and complexity limits`() { + // The introspection query is deep and complex by nature; refusing it breaks GraphiQL + // and every codegen client. + // Given + val query = "{ __schema { types { name fields { name } } } }" + + // Then + assertFalse(isRefused(query)) + } + + @Test + fun `should be detected as introspection`() { + // Given + val schemaQuery = Parser().parseDocument("{ __schema { types { name } } }") + val typeQuery = Parser().parseDocument("{ __type(name: \"X\") { name } }") + val business = Parser().parseDocument("{ episodes { title } }") + + // Then + assertTrue(isIntrospectionOnly(schemaQuery)) + assertTrue(isIntrospectionOnly(typeQuery)) + assertFalse(isIntrospectionOnly(business)) + } + + @Test + fun `should be refused when disabled`() { + // Given + val closed = SecurityLimits( + maxQueryDepth = 6, + maxQueryComplexity = 100, + introspectionEnabled = false + ) + + // Then + assertTrue(isRefused("{ __schema { types { name } } }", closed)) + } + + @Test + fun `should not exempt a query that mixes introspection and business fields`() { + // The smuggling case: claiming introspection to skip the limits while actually asking + // for data. An OR would be a hole; the check must be strict. + // Given + val mixed = "{ __schema { types { name } } episodes { title } }" + + // Then + assertFalse(isIntrospectionOnly(Parser().parseDocument(mixed))) + } + + @Test + fun `should refuse the mixed query on its own merits`() { + // Given — deep enough to break the limit, disguised as introspection. + val deep = "episodes { ".repeat(12) + "title" + " }".repeat(12) + val mixed = "{ __schema { types { name } } $deep }" + + // Then + assertTrue(isRefused(mixed)) + } + } + + @Nested + @DisplayName("Given multiple operations in one document") + inner class GivenMultipleOperations { + @Test + fun `should refuse when any operation breaks the limits`() { + // A client sends one document holding several operations and picks one by name. The + // bounds must apply to the whole document, not just the selected operation, or an + // attacker attaches a bomb as a second operation and calls it by `operationName`. + // Given — the cheap operation first, the bomb second: passing `operationName` must + // not let the bomb through. + val bomb = aliasFields(count = 200) + val document = "query Cheap { ping } query Expensive { $bomb }" + + // Then + assertTrue(isRefused(document)) + } + + @Test + fun `should accept when every operation is within the limits`() { + // Given + val document = "query A { ping } query B { episodes { title } }" + + // Then + assertFalse(isRefused(document)) + } + } + + @Nested + @DisplayName("Given a mutation") + inner class GivenMutation { + @Test + fun `should be bounded like a query`() { + // A mutation writes, so it must never be cheaper to abuse than a read. Same document + // shape as the alias-bomb query, declared as a mutation. + // Given + val document = "mutation M { ${aliasFields(count = 200)} }" + + // Then + assertTrue(isRefused(document)) + } + + @Test + fun `should accept a normal mutation`() { + // Given + val document = "mutation M { saveEpisode(id: 1) { id } }" + + // Then + assertFalse(isRefused(document)) + } + } + + @Nested + @DisplayName("Given a conditional field") + inner class GivenConditionalField { + @Test + fun `should be priced even though a directive may skip it`() { + // `@include(if: false)` does not run the resolver, but the caller controls the + // variable, so the field may well run. Pricing the static document is the safe choice: + // it over-charges a query that may be cheap rather than under-charges one that is not. + // Given — 200 conditionally included aliases, skippable at will. + val fields = (1..200).joinToString(" ") { index -> "a$index: ping @include(if: \$show)" } + val document = "query Q(\$show: Boolean!) { $fields }" + + // Then + assertTrue(isRefused(document)) + } + } + + @Nested + @DisplayName("Given an unbounded nesting") + inner class GivenUnboundedNesting { + @Test + fun `should be refused by the parser before the limits are reached`() { + // graphql-java caps nesting at 500 grammar levels and cancels parsing, so a document + // that deep never reaches our checks. The defence holds, just one layer earlier. + // Given + val query = deepQuery(depth = 5_000) + + // Then + assertThrows { + Parser().parseDocument(query) + } + } + + @Test + fun `should refuse a nesting that parses but breaks the limit`() { + // Just under the parser cap, so the document is valid and the depth check is what + // rejects it. + // Given + val query = deepQuery(depth = 20) + + // Then + assertTrue(isRefused(query)) + } + } +} diff --git a/guidelines/API_CONVENTIONS.md b/guidelines/API_CONVENTIONS.md index 2bdc43f..d12281b 100644 --- a/guidelines/API_CONVENTIONS.md +++ b/guidelines/API_CONVENTIONS.md @@ -24,6 +24,11 @@ class UserController { - Use `@PathParam("name")` for route path segments. - Use `@RequestBody` for JSON payload deserialization. - Add `@Valid` on parameters to automatically validate the deserialized object via `Validator.validate(...)`. +- Type a parameter with its real domain type, never with `String` to re-parse it: + `@PathParam("id") id: Uuid`. +- A non-nullable parameter whose value does not match its type is refused with an HTTP 400 carrying + a `MessageDto` error (`InvalidParameterException`). A nullable parameter is optional by nature: an + absent or unconvertible value resolves to `null` and never refuses the request. ## 3. Standardized Response Payloads diff --git a/guidelines/ARCHITECTURE.md b/guidelines/ARCHITECTURE.md index 89f3296..6e22a7d 100644 --- a/guidelines/ARCHITECTURE.md +++ b/guidelines/ARCHITECTURE.md @@ -8,6 +8,7 @@ core (base logging & core utils) ├── exposed (database, HikariCP, Exposed ORM, Liquibase, EntityRepository, Transactional) └── validator (reflection-based validation, custom annotations) └── ktor (Ktor web framework integration, RestController, routing, OpenAPI, ResponseEntity) +└── graphql (GraphQL engine, /graphql route, endpoint security stack, cost model, DataLoader orchestration) standalone dependency carriers (no source code): koin (standalone dependency carrier: Koin runtime APIs) └── koin-exposed (Koin bridge for transactions: applyTransactionalProxies post-processor wrapping @Transactional services in TransactionalProxy) @@ -21,6 +22,7 @@ ktor-test (standalone dependency carrier: Ktor server test host) - **`exposed`**: Coordinates database connections via `DatabaseWrapper` (HikariCP + Exposed + Liquibase), provides `TransactionalProxy` dynamic proxies for `@Transactional` methods, and supplies `EntityRepository` (entity reads) and `UpsertRepository` with an upsert DSL (`ifExists`, `newIfNotExists`, `applyFlush`). - **`validator`**: Provides a runtime reflection-based validation engine (`Validator`) and validation annotations (`@RequireAtLeastOneValid`, `@NotNull`, `@NotBlank`, `@NotEmpty`). - **`ktor`**: Bridges Ktor (server **and client**) with framework conventions. Discovers `@RestController` endpoints, binds routes (`@GetMapping`, `@PostMapping`, `@PatchMapping`), resolves parameter arguments (`@QueryParam`, `@PathParam`, `@RequestBody`), executes automatic validation via `@Valid`, packages responses with `ResponseEntity`, generates OpenAPI metadata (`@Operation`, `@ApiResponses`), handles error formatting via `MessageDto`, and exposes a preconfigured HTTP client (`createHttpClient`). +- **`graphql`**: Serves the same use cases as REST over a GraphQL transport (`graphql-kotlin` on `graphql-java`). Owns the engine wiring, the `/graphql` route, the security stack guarding it, the static cost estimator, the document bounds, and the orchestration of consumer-provided DataLoaders. Does not depend on `exposed`. - **`koin`**: Dependency carrier publishing the Koin runtime APIs (`koin-bom`, `koin-core`, `koin-annotations`) via `api(...)`; contains no source code. - **`koin-exposed`**: Bridges Koin with the `exposed` transaction model. Provides `applyTransactionalProxies()`, a runtime post-processor replacing eligible Koin singleton definitions with `TransactionalProxy`-wrapping factories so consumers never declare proxy bindings manually. Relies on `@KoinInternalApi` registry access; revisit on Koin upgrades. - **`ktor-test`**: Dependency carrier publishing Ktor's server test host (`ktor-server-test-host`) via `api(...)` so consumers write `testApplication`-based endpoint tests without referencing Ktor directly; contains no source code. @@ -29,6 +31,22 @@ ktor-test (standalone dependency carrier: Ktor server test host) The framework extends itself to consuming services: **module dependencies are exposed with `api(...)`** so that projects building on the framework can use them to their full potential (e.g. `api(libs.bundles.ktorServerEcosystem)`, `api(libs.bundles.ktorClientEcosystem)`). Use `implementation` only for dependencies that are genuinely internal to a module. +## Running Both Transports Over One DTO + +A DTO used by REST **and** GraphQL does not need a transport-specific shape. The rule that makes it +work is what the field is declared as, because graphql-kotlin resolves a field differently depending +on it: + +- a **property** is read on every request, whatever the client asked for; +- a **function** is called only when the client selects it. + +So a plain property is always loaded, and a relation that is expensive to load belongs in a function +that resolves it lazily. That is what keeps the same type cheap on both transports: REST serialises +everything, GraphQL serialises the selection, and neither pays for what was not asked. + +The type itself stays transport-agnostic: no `GraphQL` annotation is needed on a DTO that REST also +serves, and none should be added, or REST starts depending on the GraphQL library. + ## Technical Stack This project is built with **Kotlin JVM**, **Gradle (buildSrc)**, **Ktor**, **Exposed ORM**, **HikariCP**, **Liquibase**, **AWS Glide (Valkey/Redis)**, **Kotlin Reflect**, **Kotlinx Serialization**, **JUnit 6**, and **MockK**. diff --git a/guidelines/DATABASE.md b/guidelines/DATABASE.md index 964e718..ee545ab 100644 --- a/guidelines/DATABASE.md +++ b/guidelines/DATABASE.md @@ -22,8 +22,8 @@ Operations modifying state or executing Exposed queries must run within a transa ## 3. EntityRepository & Upsert DSL All Exposed repositories should inherit from `EntityRepository` (entity reads). -It extends `UpsertRepository` (upsert DSL) and provides standard `findById`, `findAllByIds`, and a -fluent upsert DSL: +It extends `UpsertRepository` (upsert DSL) and provides standard `findAll`, `findById`, +`findAllByIds`, and a fluent upsert DSL: ```kotlin override fun persist(id: Int?, data: String): MyEntity { diff --git a/guidelines/KOTLIN_CONVENTIONS.md b/guidelines/KOTLIN_CONVENTIONS.md index a2d4860..67ada13 100644 --- a/guidelines/KOTLIN_CONVENTIONS.md +++ b/guidelines/KOTLIN_CONVENTIONS.md @@ -5,6 +5,10 @@ - **Prefer immutability**: use `val`, read-only collections (`List`, `Map`), and `data class`. - **Add KDoc comments** to all public framework classes, interfaces, annotations, and functions to document behavior and rules. - **Leverage Kotlin features safely**: use null safety features (`?.`, `?:`, `let`), inline functions where appropriate, and reflection extension functions cleanly. +- **Name a property in camelCase, a type in PascalCase**: a top-level value reads `uuidScalar`, never + `UuidScalar`, which would name a type. +- **Refuse first, then assign**: a guard `if (invalid) throw ...` at the top of the block followed by + the plain assignment, never the refusal nested inside a conditional expression. ```kotlin fun getLogger(clazz: Class<*>): Logger = diff --git a/ktor/src/main/kotlin/ControllerArgumentResolver.kt b/ktor/src/main/kotlin/ControllerArgumentResolver.kt index 65ae474..9d06202 100644 --- a/ktor/src/main/kotlin/ControllerArgumentResolver.kt +++ b/ktor/src/main/kotlin/ControllerArgumentResolver.kt @@ -14,6 +14,32 @@ import kotlin.uuid.Uuid * Resolves controller function arguments from a Ktor application call. */ internal object ControllerArgumentResolver { + /** + * Converts a textual parameter to the type of [kParameter], refusing a required parameter whose + * value does not match its type. + * + * A nullable parameter is optional by nature: an unconvertible value resolves to `null`, exactly + * like an absent value, so an optional parameter never refuses the request. A non-nullable + * parameter is required: an unconvertible value is a client error. + * + * @param kParameter controller function parameter being resolved. + * @param name HTTP name of the parameter, used in the refusal message. + * @param stringValue raw parameter value. + * @return the converted value, or `null` for a nullable parameter with no convertible value. + * @throws InvalidParameterException when a non-nullable parameter value fails its conversion. + */ + private fun parseParameter(kParameter: KParameter, name: String, stringValue: String): Any? { + val kClass = kParameter.type.classifier as? KClass<*> + val parsed = parseValue(kClass, stringValue) + + if (!kParameter.type.isMarkedNullable && parsed == null) + throw InvalidParameterException( + "Parameter '$name' must be a valid ${kClass?.simpleName ?: "value"}" + ) + + return parsed + } + /** * Converts a textual HTTP parameter to the type expected by a controller function. * @@ -69,8 +95,11 @@ internal object ControllerArgumentResolver { val stringValue = call.request.queryParameters[queryParamAnnotation.name] if (stringValue != null) { - val kClass = kParameter.type.classifier as? KClass<*> - args[kParameter] = parseValue(kClass, stringValue) + args[kParameter] = parseParameter( + kParameter, + queryParamAnnotation.name, + stringValue + ) } else if (!queryParamAnnotation.required && kParameter.type.isMarkedNullable) { args[kParameter] = null } @@ -81,8 +110,11 @@ internal object ControllerArgumentResolver { val stringValue = call.parameters[pathParamAnnotation.name] if (stringValue != null) { - val kClass = kParameter.type.classifier as? KClass<*> - args[kParameter] = parseValue(kClass, stringValue) + args[kParameter] = parseParameter( + kParameter, + pathParamAnnotation.name, + stringValue + ) } } diff --git a/ktor/src/main/kotlin/ControllerRequestHandler.kt b/ktor/src/main/kotlin/ControllerRequestHandler.kt index 9e72e46..6a52a68 100644 --- a/ktor/src/main/kotlin/ControllerRequestHandler.kt +++ b/ktor/src/main/kotlin/ControllerRequestHandler.kt @@ -65,6 +65,9 @@ internal object ControllerRequestHandler { is ObjectNotValidException -> call.respond(HttpStatusCode.BadRequest, MessageDto.error(cause.message)) + is InvalidParameterException -> + call.respond(HttpStatusCode.BadRequest, MessageDto.error(cause.message)) + else -> call.respond(HttpStatusCode.InternalServerError, MessageDto.error("An unexpected error occurred")) } diff --git a/ktor/src/main/kotlin/InvalidParameterException.kt b/ktor/src/main/kotlin/InvalidParameterException.kt new file mode 100644 index 0000000..f78fe43 --- /dev/null +++ b/ktor/src/main/kotlin/InvalidParameterException.kt @@ -0,0 +1,11 @@ +package fr.shikkanime.ktor + +/** + * Indicates that a request parameter could not be converted to the type the controller expects. + * + * A malformed path, query or body parameter is a client error, not a server failure: the request + * handler translates it to an HTTP 400 carrying a [fr.shikkanime.ktor.dtos.MessageDto] error. + * + * @property message human-readable description naming the parameter and the expected type. + */ +class InvalidParameterException(override val message: String) : IllegalArgumentException(message) diff --git a/ktor/src/test/kotlin/ControllerArgumentResolverTest.kt b/ktor/src/test/kotlin/ControllerArgumentResolverTest.kt index 784348a..9d7c54d 100644 --- a/ktor/src/test/kotlin/ControllerArgumentResolverTest.kt +++ b/ktor/src/test/kotlin/ControllerArgumentResolverTest.kt @@ -30,6 +30,11 @@ class ControllerArgumentResolverTest { @QueryParam("optional", required = false) optional: String? ): ResponseEntity = ResponseEntity.ok("ok") + + fun optionalEndpoint( + @QueryParam("id", required = false) id: Uuid? + ): ResponseEntity = + ResponseEntity.ok(id?.toString() ?: "none") } @Nested @@ -84,7 +89,7 @@ class ControllerArgumentResolverTest { } @Test - suspend fun `should return null when numeric or boolean conversions fail`() { + suspend fun `should refuse the request when a numeric or boolean conversion fails`() { // Given val call = mockk(relaxed = true) every { call.request.queryParameters["longVal"] } returns "invalid_number" @@ -93,6 +98,24 @@ class ControllerArgumentResolverTest { val controller = SampleController() val function = SampleController::typeEndpoint + // When + val exception = org.junit.jupiter.api.assertThrows { + ControllerArgumentResolver.resolve(call, function, controller) + } + + // Then + assertTrue(exception.message.contains("must be a valid")) + } + + @Test + suspend fun `should resolve an optional parameter to null when its value does not match its type`() { + // Given + val call = mockk(relaxed = true) + every { call.request.queryParameters["id"] } returns "not-a-uuid" + + val controller = SampleController() + val function = SampleController::optionalEndpoint + // When val args = ControllerArgumentResolver.resolve(call, function, controller) diff --git a/settings.gradle.kts b/settings.gradle.kts index fb0466d..1252417 100644 --- a/settings.gradle.kts +++ b/settings.gradle.kts @@ -18,6 +18,7 @@ include( ":validator", ":ktor", ":ktor-test", + ":graphql", ":cache", ":koin", ":koin-exposed",