From f5cd53500b96de614cb11ba8247aee777ca313fa Mon Sep 17 00:00:00 2001 From: kirich1409 Date: Tue, 19 May 2026 10:53:14 +0300 Subject: [PATCH 1/3] Add aggregator plugin and GeneratedFeaturedRegistry codegen Introduce dev.androidbroadcast.featured.application Gradle plugin that aggregates featured-manifest.json artifacts from project dependencies declared via featuredAggregation(project(...)) and generates a unified object GeneratedFeaturedRegistry { val all: List> } in build/generated/featured/commonMain/. The aggregator reuses the FEATURED_MANIFEST_USAGE / schemaMajorAttr contract published by dev.androidbroadcast.featured (PR A, schema v1) through a resolvable featuredAggregationClasspath configuration. generateFeaturedRegistry is @CacheableTask with stable (modulePath, key) sort order; duplicate keys across the aggregated graph fail the build naming both module paths. Co-Authored-By: Claude Opus 4.7 --- CHANGELOG.md | 1 + featured-gradle-plugin/build.gradle.kts | 4 + .../gradle/FeaturedApplicationPlugin.kt | 105 ++++++++ .../gradle/aggregation/AggregationContract.kt | 36 +++ .../GenerateFeaturedRegistryTask.kt | 102 +++++++ .../GeneratedFeaturedRegistryGenerator.kt | 147 ++++++++++ .../app/build.gradle.kts | 16 ++ .../app/src/main/AndroidManifest.xml | 2 + .../build.gradle.kts | 1 + .../feature-checkout/build.gradle.kts | 17 ++ .../src/main/AndroidManifest.xml | 2 + .../feature-profile/build.gradle.kts | 19 ++ .../src/main/AndroidManifest.xml | 2 + .../gradle.properties | 3 + .../settings.gradle.kts | 33 +++ .../FeaturedAggregationConfigurationTest.kt | 102 +++++++ .../FeaturedAggregationDuplicateKeyTest.kt | 84 ++++++ .../FeaturedAggregationIntegrationTest.kt | 198 ++++++++++++++ ...ateFeaturedRegistryTaskRegistrationTest.kt | 75 ++++++ .../GeneratedFeaturedRegistryGeneratorTest.kt | 252 ++++++++++++++++++ 20 files changed, 1201 insertions(+) create mode 100644 featured-gradle-plugin/src/main/kotlin/dev/androidbroadcast/featured/gradle/FeaturedApplicationPlugin.kt create mode 100644 featured-gradle-plugin/src/main/kotlin/dev/androidbroadcast/featured/gradle/aggregation/AggregationContract.kt create mode 100644 featured-gradle-plugin/src/main/kotlin/dev/androidbroadcast/featured/gradle/aggregation/GenerateFeaturedRegistryTask.kt create mode 100644 featured-gradle-plugin/src/main/kotlin/dev/androidbroadcast/featured/gradle/aggregation/GeneratedFeaturedRegistryGenerator.kt create mode 100644 featured-gradle-plugin/src/test/fixtures/aggregator-multi-module-project/app/build.gradle.kts create mode 100644 featured-gradle-plugin/src/test/fixtures/aggregator-multi-module-project/app/src/main/AndroidManifest.xml create mode 100644 featured-gradle-plugin/src/test/fixtures/aggregator-multi-module-project/build.gradle.kts create mode 100644 featured-gradle-plugin/src/test/fixtures/aggregator-multi-module-project/feature-checkout/build.gradle.kts create mode 100644 featured-gradle-plugin/src/test/fixtures/aggregator-multi-module-project/feature-checkout/src/main/AndroidManifest.xml create mode 100644 featured-gradle-plugin/src/test/fixtures/aggregator-multi-module-project/feature-profile/build.gradle.kts create mode 100644 featured-gradle-plugin/src/test/fixtures/aggregator-multi-module-project/feature-profile/src/main/AndroidManifest.xml create mode 100644 featured-gradle-plugin/src/test/fixtures/aggregator-multi-module-project/gradle.properties create mode 100644 featured-gradle-plugin/src/test/fixtures/aggregator-multi-module-project/settings.gradle.kts create mode 100644 featured-gradle-plugin/src/test/kotlin/dev/androidbroadcast/featured/gradle/aggregation/FeaturedAggregationConfigurationTest.kt create mode 100644 featured-gradle-plugin/src/test/kotlin/dev/androidbroadcast/featured/gradle/aggregation/FeaturedAggregationDuplicateKeyTest.kt create mode 100644 featured-gradle-plugin/src/test/kotlin/dev/androidbroadcast/featured/gradle/aggregation/FeaturedAggregationIntegrationTest.kt create mode 100644 featured-gradle-plugin/src/test/kotlin/dev/androidbroadcast/featured/gradle/aggregation/GenerateFeaturedRegistryTaskRegistrationTest.kt create mode 100644 featured-gradle-plugin/src/test/kotlin/dev/androidbroadcast/featured/gradle/aggregation/GeneratedFeaturedRegistryGeneratorTest.kt diff --git a/CHANGELOG.md b/CHANGELOG.md index cfcb5e7..5973379 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - Featured library plugin now publishes a per-module feature-flag manifest as a consumable Gradle artifact (`featuredManifest` configuration, schema v1). Existing flag-generation pipeline is unchanged. Consumer-side aggregation arrives in a follow-up release. +- New `dev.androidbroadcast.featured.application` Gradle plugin: aggregates `featured-manifest.json` artifacts from project dependencies declared via `featuredAggregation(project(...))` and generates `object GeneratedFeaturedRegistry { val all: List> }` in `build/generated/featured/commonMain/`. Apply alongside `dev.androidbroadcast.featured` in the application module; wire the output directory into your source set manually (e.g., `kotlin.sourceSets.commonMain.kotlin.srcDir(...)`). ## [1.0.0-Beta1] - 2026-05-17 diff --git a/featured-gradle-plugin/build.gradle.kts b/featured-gradle-plugin/build.gradle.kts index 36b432e..f4628d8 100644 --- a/featured-gradle-plugin/build.gradle.kts +++ b/featured-gradle-plugin/build.gradle.kts @@ -18,6 +18,10 @@ gradlePlugin { id = "dev.androidbroadcast.featured" implementationClass = "dev.androidbroadcast.featured.gradle.FeaturedPlugin" } + create("featuredApplication") { + id = "dev.androidbroadcast.featured.application" + implementationClass = "dev.androidbroadcast.featured.gradle.FeaturedApplicationPlugin" + } } } diff --git a/featured-gradle-plugin/src/main/kotlin/dev/androidbroadcast/featured/gradle/FeaturedApplicationPlugin.kt b/featured-gradle-plugin/src/main/kotlin/dev/androidbroadcast/featured/gradle/FeaturedApplicationPlugin.kt new file mode 100644 index 0000000..15c5e61 --- /dev/null +++ b/featured-gradle-plugin/src/main/kotlin/dev/androidbroadcast/featured/gradle/FeaturedApplicationPlugin.kt @@ -0,0 +1,105 @@ +package dev.androidbroadcast.featured.gradle + +import dev.androidbroadcast.featured.gradle.aggregation.FEATURED_AGGREGATION_CLASSPATH_CONFIGURATION_NAME +import dev.androidbroadcast.featured.gradle.aggregation.FEATURED_AGGREGATION_CONFIGURATION_NAME +import dev.androidbroadcast.featured.gradle.aggregation.FEATURED_REGISTRY_OBJECT +import dev.androidbroadcast.featured.gradle.aggregation.FEATURED_REGISTRY_PACKAGE +import dev.androidbroadcast.featured.gradle.aggregation.GENERATE_FEATURED_REGISTRY_TASK_NAME +import dev.androidbroadcast.featured.gradle.aggregation.GenerateFeaturedRegistryTask +import dev.androidbroadcast.featured.gradle.manifest.FEATURED_MANIFEST_USAGE +import dev.androidbroadcast.featured.gradle.manifest.SCHEMA_VERSION +import dev.androidbroadcast.featured.gradle.manifest.schemaMajorAttr +import org.gradle.api.Plugin +import org.gradle.api.Project +import org.gradle.api.attributes.Usage + +/** + * Gradle plugin ID: `dev.androidbroadcast.featured.application`. + * + * Aggregates `featured-manifest.json` artifacts from all project dependencies declared via + * `featuredAggregation(project(...))` and generates a unified + * `object GeneratedFeaturedRegistry { val all: List> }` Kotlin source file. + * + * Apply this plugin alongside `dev.androidbroadcast.featured` in the application or aggregator + * module: + * ```kotlin + * plugins { + * id("dev.androidbroadcast.featured") + * id("dev.androidbroadcast.featured.application") + * } + * + * dependencies { + * featuredAggregation(project(":feature:checkout")) + * featuredAggregation(project(":feature:profile")) + * } + * ``` + * + * The generated file is written to + * `build/generated/featured/commonMain/GeneratedFeaturedRegistry.kt`. + * Wire the output directory into your source set manually — the plugin does not auto-wire + * to avoid assumptions about whether the consuming module is KMP, AGP, or plain JVM: + * ```kotlin + * kotlin.sourceSets.getByName("commonMain").kotlin.srcDir( + * tasks.named("generateFeaturedRegistry").map { it.outputs.files.singleFile.parentFile } + * ) + * ``` + * + * Min Gradle version: 8.5+ (`configurations.dependencyScope()` / `.resolvable()` API). + */ +@Suppress("UnstableApiUsage") +public class FeaturedApplicationPlugin : Plugin { + override fun apply(target: Project) { + // Register the schemaMajorAttr in the project's attribute schema. This is idempotent — + // if FeaturedPlugin is also applied it calls the same registration first. + target.dependencies.attributesSchema.attribute(schemaMajorAttr) + + // User-facing declarable scope: consumers add project() dependencies here. + val declarable = + target.configurations.dependencyScope( + FEATURED_AGGREGATION_CONFIGURATION_NAME, + ) { cfg -> + cfg.description = + "Project dependencies whose featured-manifest.json should be aggregated into GeneratedFeaturedRegistry." + } + + // Internal resolvable classpath that carries the attribute contract used by Gradle's + // variant selection to match the `featuredManifest` consumable configuration published + // by each producer module applying `dev.androidbroadcast.featured`. + val classpath = + target.configurations.resolvable( + FEATURED_AGGREGATION_CLASSPATH_CONFIGURATION_NAME, + ) { cfg -> + cfg.description = + "Internal classpath resolving featured-manifest.json artifacts from featuredAggregation." + cfg.extendsFrom(declarable.get()) + cfg.attributes { attrs -> + attrs.attribute( + Usage.USAGE_ATTRIBUTE, + target.objects.named(Usage::class.java, FEATURED_MANIFEST_USAGE), + ) + // Mirror the schema-major attribute declared on the producer side so that Gradle's + // variant selection picks exactly the schema-v1 manifests. + attrs.attribute(schemaMajorAttr, SCHEMA_VERSION) + } + } + + target.tasks.register( + GENERATE_FEATURED_REGISTRY_TASK_NAME, + GenerateFeaturedRegistryTask::class.java, + ) { task -> + task.group = "featured" + task.description = + "Aggregates featured-manifest.json artifacts and generates GeneratedFeaturedRegistry.kt." + // Lazy artifact view — resolved at execution time, CC-compatible. + task.manifestFiles.from( + classpath.map { it.incoming.artifactView { view -> view.isLenient = false }.files }, + ) + task.outputPackage.set(FEATURED_REGISTRY_PACKAGE) + task.outputFile.convention( + target.layout.buildDirectory.file( + "generated/featured/commonMain/${FEATURED_REGISTRY_OBJECT}.kt", + ), + ) + } + } +} diff --git a/featured-gradle-plugin/src/main/kotlin/dev/androidbroadcast/featured/gradle/aggregation/AggregationContract.kt b/featured-gradle-plugin/src/main/kotlin/dev/androidbroadcast/featured/gradle/aggregation/AggregationContract.kt new file mode 100644 index 0000000..4207435 --- /dev/null +++ b/featured-gradle-plugin/src/main/kotlin/dev/androidbroadcast/featured/gradle/aggregation/AggregationContract.kt @@ -0,0 +1,36 @@ +package dev.androidbroadcast.featured.gradle.aggregation + +/** + * Name of the user-facing declarable Gradle configuration. + * Consumers add dependencies here via `featuredAggregation(project(...))`. + * Used by [FeaturedApplicationPlugin] to create the dependency scope. + */ +internal const val FEATURED_AGGREGATION_CONFIGURATION_NAME = "featuredAggregation" + +/** + * Name of the internal resolvable Gradle configuration. + * Extends [FEATURED_AGGREGATION_CONFIGURATION_NAME] and carries the attribute contract + * (`Usage = "featured-manifest"`, `schema-major = 1`) that Gradle uses to select the + * `featuredManifest` outgoing variant from each producer module. + */ +internal const val FEATURED_AGGREGATION_CLASSPATH_CONFIGURATION_NAME = "featuredAggregationClasspath" + +/** + * Task name registered by [FeaturedApplicationPlugin]. + * Running `./gradlew generateFeaturedRegistry` collects all manifests and writes the + * generated Kotlin source to the output file. + */ +internal const val GENERATE_FEATURED_REGISTRY_TASK_NAME = "generateFeaturedRegistry" + +/** + * Package name emitted at the top of the generated `GeneratedFeaturedRegistry.kt` file. + * Matches the package used by other Featured-generated sources in `commonMain`. + */ +internal const val FEATURED_REGISTRY_PACKAGE = "dev.androidbroadcast.featured.generated" + +/** + * Simple name of the generated Kotlin object and the output file (without `.kt` extension). + * Used both as the object identifier in the generated source and as the output filename by + * [GenerateFeaturedRegistryTask]. + */ +internal const val FEATURED_REGISTRY_OBJECT = "GeneratedFeaturedRegistry" diff --git a/featured-gradle-plugin/src/main/kotlin/dev/androidbroadcast/featured/gradle/aggregation/GenerateFeaturedRegistryTask.kt b/featured-gradle-plugin/src/main/kotlin/dev/androidbroadcast/featured/gradle/aggregation/GenerateFeaturedRegistryTask.kt new file mode 100644 index 0000000..a1033ab --- /dev/null +++ b/featured-gradle-plugin/src/main/kotlin/dev/androidbroadcast/featured/gradle/aggregation/GenerateFeaturedRegistryTask.kt @@ -0,0 +1,102 @@ +package dev.androidbroadcast.featured.gradle.aggregation + +import dev.androidbroadcast.featured.gradle.manifest.FeaturedManifest +import dev.androidbroadcast.featured.gradle.manifest.FeaturedManifestJson +import kotlinx.serialization.decodeFromString +import org.gradle.api.DefaultTask +import org.gradle.api.file.ConfigurableFileCollection +import org.gradle.api.file.RegularFileProperty +import org.gradle.api.provider.Property +import org.gradle.api.tasks.CacheableTask +import org.gradle.api.tasks.Input +import org.gradle.api.tasks.InputFiles +import org.gradle.api.tasks.OutputFile +import org.gradle.api.tasks.PathSensitive +import org.gradle.api.tasks.PathSensitivity +import org.gradle.api.tasks.TaskAction + +/** + * Aggregates `featured-manifest.json` files from all project dependencies declared via + * `featuredAggregation(...)` and generates `GeneratedFeaturedRegistry.kt`. + * + * Registered by [FeaturedApplicationPlugin] under the name `generateFeaturedRegistry`. + * + * Validation: duplicate flag keys across modules (including LOCAL + REMOTE of the same module) + * are rejected with an [IllegalStateException] naming both conflicting module paths. + */ +@CacheableTask +internal abstract class GenerateFeaturedRegistryTask : DefaultTask() { + /** + * The set of `featured-manifest.json` files resolved from `featuredAggregationClasspath`. + * + * [PathSensitivity.NONE] is used because only the file content matters for cache-key + * computation — the artifact path varies across machines and build cache entries. + */ + @get:InputFiles + @get:PathSensitive(PathSensitivity.NONE) + abstract val manifestFiles: ConfigurableFileCollection + + /** + * Package name written to the top of the generated source file. + * Defaults to [FEATURED_REGISTRY_PACKAGE]. + */ + @get:Input + abstract val outputPackage: Property + + /** + * Destination for the generated `GeneratedFeaturedRegistry.kt` source file. + * Convention: `build/generated/featured/commonMain/GeneratedFeaturedRegistry.kt`. + */ + @get:OutputFile + abstract val outputFile: RegularFileProperty + + @TaskAction + fun generate() { + val manifests = + manifestFiles.files + .map { file -> FeaturedManifestJson.decodeFromString(file.readText()) } + + validateUniqueKeys(manifests) + + val source = + GeneratedFeaturedRegistryGenerator.generate( + manifests = manifests, + packageName = outputPackage.get(), + ) + + val outFile = outputFile.get().asFile + outFile.parentFile.mkdirs() + outFile.writeText(source) + + val totalFlags = manifests.sumOf { it.flags.size } + logger.lifecycle( + "[featured] Generated registry with $totalFlags flag(s) from ${manifests.size} module(s) → ${outFile.path}", + ) + } +} + +/** + * Validates that no two [FlagDescriptor][dev.androidbroadcast.featured.gradle.manifest.FlagDescriptor] + * entries across all [manifests] share the same key. + * + * A flag declared in both `localFlags` and `remoteFlags` of the same module is treated as a + * duplicate because each key produces exactly one `ConfigParam` in the registry. + * + * @throws IllegalStateException when a duplicate key is detected, naming both module paths and the key. + */ +internal fun validateUniqueKeys(manifests: List) { + // Pair each descriptor with its module path for error reporting. + val pairs = manifests.flatMap { manifest -> manifest.flags.map { flag -> flag.key to manifest.modulePath } } + + // Group by key; any group with more than one entry is a duplicate. + pairs + .groupBy { (key, _) -> key } + .filter { (_, entries) -> entries.size > 1 } + .forEach { (key, entries) -> + val pathA = entries[0].second + val pathB = entries[1].second + throw IllegalStateException( + "Duplicate flag key '$key': declared in '$pathA' and '$pathB'", + ) + } +} diff --git a/featured-gradle-plugin/src/main/kotlin/dev/androidbroadcast/featured/gradle/aggregation/GeneratedFeaturedRegistryGenerator.kt b/featured-gradle-plugin/src/main/kotlin/dev/androidbroadcast/featured/gradle/aggregation/GeneratedFeaturedRegistryGenerator.kt new file mode 100644 index 0000000..7a2f2e9 --- /dev/null +++ b/featured-gradle-plugin/src/main/kotlin/dev/androidbroadcast/featured/gradle/aggregation/GeneratedFeaturedRegistryGenerator.kt @@ -0,0 +1,147 @@ +package dev.androidbroadcast.featured.gradle.aggregation + +import dev.androidbroadcast.featured.gradle.manifest.FeaturedManifest +import dev.androidbroadcast.featured.gradle.manifest.FlagDescriptor +import dev.androidbroadcast.featured.gradle.manifest.ValueType + +/** + * Generates `GeneratedFeaturedRegistry.kt` source from aggregated [FeaturedManifest] list. + * + * KMP-safe: imports only `dev.androidbroadcast.featured.ConfigParam` and enum FQNs + * (one import per distinct enum). All type arguments and default values for ENUM flags use + * the fully-qualified class name inline so the generated file compiles without requiring + * the enum types on the plugin's own classpath. + * + * Output determinism: descriptors are sorted by `(modulePath, key)` over the flattened + * list before generation, so the output is identical regardless of the order in which + * Gradle resolves the manifest artifacts. + */ +internal object GeneratedFeaturedRegistryGenerator { + private const val CONFIG_PARAM_IMPORT = "dev.androidbroadcast.featured.ConfigParam" + + /** + * Generates the full `GeneratedFeaturedRegistry.kt` source text. + * + * @param manifests Aggregated manifests from all producer modules. + * @param packageName Package declared at the top of the generated file. + * @return Complete Kotlin source as a single [String]. + */ + fun generate( + manifests: List, + packageName: String, + ): String { + // Flatten to (modulePath, descriptor) pairs, then sort for deterministic output. + // Sorting here (not in the caller) guarantees stability regardless of manifest order. + val sorted = + manifests + .flatMap { manifest -> manifest.flags.map { flag -> manifest.modulePath to flag } } + .sortedWith(compareBy({ it.first }, { it.second.key })) + + return buildString { + appendLine("// Auto-generated by Featured Gradle Plugin — do not edit manually.") + appendLine("package $packageName") + appendLine() + appendLine("import $CONFIG_PARAM_IMPORT") + appendLine() + appendLine("public object $FEATURED_REGISTRY_OBJECT {") + if (sorted.isEmpty()) { + appendLine(" public val all: List> = emptyList()") + } else { + appendLine(" public val all: List> = listOf(") + sorted.forEach { (_, descriptor) -> + val typeArg = descriptor.valueType.toKotlinTypeName(descriptor.enumTypeFqn) + val defaultLiteral = descriptor.toDefaultLiteral() + val args = + buildList { + add("key = \"${descriptor.key}\"") + add("defaultValue = $defaultLiteral") + if (descriptor.description != null) add("description = \"${descriptor.description}\"") + if (descriptor.category != null) add("category = \"${descriptor.category}\"") + } + // Kotlin accepts trailing commas in listOf() — always emit one for uniform diffs. + appendLine(" ConfigParam<$typeArg>(${args.joinToString(", ")}),") + } + appendLine(" )") + } + append("}") + } + } +} + +/** + * Maps this [ValueType] to the Kotlin type name used in the `ConfigParam` type argument. + * + * For [ValueType.ENUM], [enumTypeFqn] must be non-null; it is used as the full type reference. + */ +private fun ValueType.toKotlinTypeName(enumTypeFqn: String?): String = + when (this) { + ValueType.BOOLEAN -> { + "Boolean" + } + + ValueType.INT -> { + "Int" + } + + ValueType.LONG -> { + "Long" + } + + ValueType.FLOAT -> { + "Float" + } + + ValueType.DOUBLE -> { + "Double" + } + + ValueType.STRING -> { + "String" + } + + ValueType.ENUM -> { + requireNotNull(enumTypeFqn) { + "enumTypeFqn must be non-null for ValueType.ENUM" + } + } + } + +/** + * Produces the Kotlin literal for `defaultValue = ...` in the generated `ConfigParam` call. + * + * STRING: producer stores bare value (surrounding quotes already stripped); re-wrap and + * escape embedded backslashes and double-quotes. + * LONG: append `L` suffix. + * FLOAT: append `f` suffix. + * ENUM: rebuild as `enumTypeFqn.CONSTANT_NAME`. + * BOOLEAN, INT, DOUBLE: emit raw. + */ +private fun FlagDescriptor.toDefaultLiteral(): String = + when (valueType) { + ValueType.STRING -> { + // Escape in order: `\` first (avoids double-escaping), then `"`, then `$` + // (prevents Kotlin string template interpolation in the generated source). + val escaped = + defaultValue + .replace("\\", "\\\\") + .replace("\"", "\\\"") + .replace("$", "\${'\$'}") + "\"$escaped\"" + } + + ValueType.LONG -> { + "${defaultValue}L" + } + + ValueType.FLOAT -> { + "${defaultValue}f" + } + + ValueType.ENUM -> { + "$enumTypeFqn.$defaultValue" + } + + ValueType.BOOLEAN, ValueType.INT, ValueType.DOUBLE -> { + defaultValue + } + } diff --git a/featured-gradle-plugin/src/test/fixtures/aggregator-multi-module-project/app/build.gradle.kts b/featured-gradle-plugin/src/test/fixtures/aggregator-multi-module-project/app/build.gradle.kts new file mode 100644 index 0000000..33c9d94 --- /dev/null +++ b/featured-gradle-plugin/src/test/fixtures/aggregator-multi-module-project/app/build.gradle.kts @@ -0,0 +1,16 @@ +plugins { + id("com.android.application") version "9.1.0" + id("dev.androidbroadcast.featured") + id("dev.androidbroadcast.featured.application") +} + +android { + namespace = "com.example.testapp" + compileSdk = 36 + defaultConfig { minSdk = 24 } +} + +dependencies { + featuredAggregation(project(":feature-checkout")) + featuredAggregation(project(":feature-profile")) +} diff --git a/featured-gradle-plugin/src/test/fixtures/aggregator-multi-module-project/app/src/main/AndroidManifest.xml b/featured-gradle-plugin/src/test/fixtures/aggregator-multi-module-project/app/src/main/AndroidManifest.xml new file mode 100644 index 0000000..b2d3ea1 --- /dev/null +++ b/featured-gradle-plugin/src/test/fixtures/aggregator-multi-module-project/app/src/main/AndroidManifest.xml @@ -0,0 +1,2 @@ + + diff --git a/featured-gradle-plugin/src/test/fixtures/aggregator-multi-module-project/build.gradle.kts b/featured-gradle-plugin/src/test/fixtures/aggregator-multi-module-project/build.gradle.kts new file mode 100644 index 0000000..b1af0dc --- /dev/null +++ b/featured-gradle-plugin/src/test/fixtures/aggregator-multi-module-project/build.gradle.kts @@ -0,0 +1 @@ +// Root build file — no plugins applied at root level. diff --git a/featured-gradle-plugin/src/test/fixtures/aggregator-multi-module-project/feature-checkout/build.gradle.kts b/featured-gradle-plugin/src/test/fixtures/aggregator-multi-module-project/feature-checkout/build.gradle.kts new file mode 100644 index 0000000..27628c1 --- /dev/null +++ b/featured-gradle-plugin/src/test/fixtures/aggregator-multi-module-project/feature-checkout/build.gradle.kts @@ -0,0 +1,17 @@ +plugins { + id("com.android.library") version "9.1.0" + id("dev.androidbroadcast.featured") +} + +android { + namespace = "com.example.featurecheckout" + compileSdk = 36 + defaultConfig { minSdk = 24 } +} + +featured { + localFlags { + boolean("dark_mode", default = false) { category = "UI" } + enum("checkout_variant", typeFqn = "com.example.CheckoutVariant", default = "LEGACY") + } +} diff --git a/featured-gradle-plugin/src/test/fixtures/aggregator-multi-module-project/feature-checkout/src/main/AndroidManifest.xml b/featured-gradle-plugin/src/test/fixtures/aggregator-multi-module-project/feature-checkout/src/main/AndroidManifest.xml new file mode 100644 index 0000000..b2d3ea1 --- /dev/null +++ b/featured-gradle-plugin/src/test/fixtures/aggregator-multi-module-project/feature-checkout/src/main/AndroidManifest.xml @@ -0,0 +1,2 @@ + + diff --git a/featured-gradle-plugin/src/test/fixtures/aggregator-multi-module-project/feature-profile/build.gradle.kts b/featured-gradle-plugin/src/test/fixtures/aggregator-multi-module-project/feature-profile/build.gradle.kts new file mode 100644 index 0000000..326b95e --- /dev/null +++ b/featured-gradle-plugin/src/test/fixtures/aggregator-multi-module-project/feature-profile/build.gradle.kts @@ -0,0 +1,19 @@ +plugins { + id("com.android.library") version "9.1.0" + id("dev.androidbroadcast.featured") +} + +android { + namespace = "com.example.featureprofile" + compileSdk = 36 + defaultConfig { minSdk = 24 } +} + +featured { + localFlags { + string("avatar_placeholder", default = "default.png") + } + remoteFlags { + boolean("show_avatar", default = true) + } +} diff --git a/featured-gradle-plugin/src/test/fixtures/aggregator-multi-module-project/feature-profile/src/main/AndroidManifest.xml b/featured-gradle-plugin/src/test/fixtures/aggregator-multi-module-project/feature-profile/src/main/AndroidManifest.xml new file mode 100644 index 0000000..b2d3ea1 --- /dev/null +++ b/featured-gradle-plugin/src/test/fixtures/aggregator-multi-module-project/feature-profile/src/main/AndroidManifest.xml @@ -0,0 +1,2 @@ + + diff --git a/featured-gradle-plugin/src/test/fixtures/aggregator-multi-module-project/gradle.properties b/featured-gradle-plugin/src/test/fixtures/aggregator-multi-module-project/gradle.properties new file mode 100644 index 0000000..d621155 --- /dev/null +++ b/featured-gradle-plugin/src/test/fixtures/aggregator-multi-module-project/gradle.properties @@ -0,0 +1,3 @@ +android.useAndroidX=true +org.gradle.configuration-cache=true +org.gradle.unsafe.configuration-cache-problems=warn diff --git a/featured-gradle-plugin/src/test/fixtures/aggregator-multi-module-project/settings.gradle.kts b/featured-gradle-plugin/src/test/fixtures/aggregator-multi-module-project/settings.gradle.kts new file mode 100644 index 0000000..14a18d1 --- /dev/null +++ b/featured-gradle-plugin/src/test/fixtures/aggregator-multi-module-project/settings.gradle.kts @@ -0,0 +1,33 @@ +// AGP and the Featured plugin are injected via GradleRunner.withPluginClasspath(). +pluginManagement { + repositories { + google { + mavenContent { + includeGroupAndSubgroups("androidx") + includeGroupAndSubgroups("com.android") + includeGroupAndSubgroups("com.google") + } + } + mavenCentral() + gradlePluginPortal() + } +} + +dependencyResolutionManagement { + @Suppress("UnstableApiUsage") + repositories { + google { + mavenContent { + includeGroupAndSubgroups("androidx") + includeGroupAndSubgroups("com.android") + includeGroupAndSubgroups("com.google") + } + } + mavenCentral() + } +} + +rootProject.name = "aggregator-multi-module-project" +include(":feature-checkout") +include(":feature-profile") +include(":app") diff --git a/featured-gradle-plugin/src/test/kotlin/dev/androidbroadcast/featured/gradle/aggregation/FeaturedAggregationConfigurationTest.kt b/featured-gradle-plugin/src/test/kotlin/dev/androidbroadcast/featured/gradle/aggregation/FeaturedAggregationConfigurationTest.kt new file mode 100644 index 0000000..2624351 --- /dev/null +++ b/featured-gradle-plugin/src/test/kotlin/dev/androidbroadcast/featured/gradle/aggregation/FeaturedAggregationConfigurationTest.kt @@ -0,0 +1,102 @@ +package dev.androidbroadcast.featured.gradle.aggregation + +import dev.androidbroadcast.featured.gradle.manifest.SCHEMA_VERSION +import dev.androidbroadcast.featured.gradle.manifest.schemaMajorAttr +import org.gradle.api.attributes.Usage +import org.gradle.testfixtures.ProjectBuilder +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNotNull +import kotlin.test.assertTrue + +@Suppress("UnstableApiUsage") +class FeaturedAggregationConfigurationTest { + @Test + fun `featuredAggregation configuration is registered`() { + val project = ProjectBuilder.builder().build() + project.plugins.apply("dev.androidbroadcast.featured.application") + + val cfg = project.configurations.findByName(FEATURED_AGGREGATION_CONFIGURATION_NAME) + assertNotNull(cfg, "Expected '$FEATURED_AGGREGATION_CONFIGURATION_NAME' configuration to be registered") + } + + @Test + fun `featuredAggregation is declarable not consumable not resolvable`() { + val project = ProjectBuilder.builder().build() + project.plugins.apply("dev.androidbroadcast.featured.application") + + val cfg = project.configurations.findByName(FEATURED_AGGREGATION_CONFIGURATION_NAME) + assertNotNull(cfg) + assertTrue(cfg.isCanBeDeclared, "Expected isCanBeDeclared = true") + assertTrue(!cfg.isCanBeConsumed, "Expected isCanBeConsumed = false") + assertTrue(!cfg.isCanBeResolved, "Expected isCanBeResolved = false") + } + + @Test + fun `featuredAggregationClasspath configuration is registered`() { + val project = ProjectBuilder.builder().build() + project.plugins.apply("dev.androidbroadcast.featured.application") + + val cfg = project.configurations.findByName(FEATURED_AGGREGATION_CLASSPATH_CONFIGURATION_NAME) + assertNotNull(cfg, "Expected '$FEATURED_AGGREGATION_CLASSPATH_CONFIGURATION_NAME' configuration to be registered") + } + + @Test + fun `featuredAggregationClasspath is resolvable not consumable not declarable`() { + val project = ProjectBuilder.builder().build() + project.plugins.apply("dev.androidbroadcast.featured.application") + + val cfg = project.configurations.findByName(FEATURED_AGGREGATION_CLASSPATH_CONFIGURATION_NAME) + assertNotNull(cfg) + assertTrue(cfg.isCanBeResolved, "Expected isCanBeResolved = true") + assertTrue(!cfg.isCanBeConsumed, "Expected isCanBeConsumed = false") + assertTrue(!cfg.isCanBeDeclared, "Expected isCanBeDeclared = false") + } + + @Test + fun `featuredAggregationClasspath has Usage attribute featured-manifest`() { + val project = ProjectBuilder.builder().build() + project.plugins.apply("dev.androidbroadcast.featured.application") + + val cfg = project.configurations.findByName(FEATURED_AGGREGATION_CLASSPATH_CONFIGURATION_NAME) + assertNotNull(cfg) + val usageAttr = cfg.attributes.getAttribute(Usage.USAGE_ATTRIBUTE) + assertNotNull(usageAttr, "Expected Usage attribute to be set") + assertEquals( + "featured-manifest", + usageAttr.name, + "Expected Usage attribute name 'featured-manifest', got '${usageAttr.name}'", + ) + } + + @Test + fun `featuredAggregationClasspath has schema-major attribute equal to SCHEMA_VERSION`() { + val project = ProjectBuilder.builder().build() + project.plugins.apply("dev.androidbroadcast.featured.application") + + val cfg = project.configurations.findByName(FEATURED_AGGREGATION_CLASSPATH_CONFIGURATION_NAME) + assertNotNull(cfg) + val schemaAttr = cfg.attributes.getAttribute(schemaMajorAttr) + assertNotNull(schemaAttr, "Expected schema-major attribute to be set") + assertEquals( + SCHEMA_VERSION, + schemaAttr, + "Expected schema-major = $SCHEMA_VERSION, got $schemaAttr", + ) + } + + @Test + fun `featuredAggregationClasspath extends featuredAggregation`() { + val project = ProjectBuilder.builder().build() + project.plugins.apply("dev.androidbroadcast.featured.application") + + val declarable = project.configurations.findByName(FEATURED_AGGREGATION_CONFIGURATION_NAME) + val classpath = project.configurations.findByName(FEATURED_AGGREGATION_CLASSPATH_CONFIGURATION_NAME) + assertNotNull(declarable) + assertNotNull(classpath) + assertTrue( + classpath.extendsFrom.contains(declarable), + "Expected featuredAggregationClasspath to extend featuredAggregation", + ) + } +} diff --git a/featured-gradle-plugin/src/test/kotlin/dev/androidbroadcast/featured/gradle/aggregation/FeaturedAggregationDuplicateKeyTest.kt b/featured-gradle-plugin/src/test/kotlin/dev/androidbroadcast/featured/gradle/aggregation/FeaturedAggregationDuplicateKeyTest.kt new file mode 100644 index 0000000..f7420f3 --- /dev/null +++ b/featured-gradle-plugin/src/test/kotlin/dev/androidbroadcast/featured/gradle/aggregation/FeaturedAggregationDuplicateKeyTest.kt @@ -0,0 +1,84 @@ +package dev.androidbroadcast.featured.gradle.aggregation + +import dev.androidbroadcast.featured.gradle.manifest.FeaturedManifest +import dev.androidbroadcast.featured.gradle.manifest.FlagDescriptor +import dev.androidbroadcast.featured.gradle.manifest.FlagKind +import dev.androidbroadcast.featured.gradle.manifest.SCHEMA_VERSION +import dev.androidbroadcast.featured.gradle.manifest.ValueType +import kotlin.test.Test +import kotlin.test.assertContains +import kotlin.test.assertFailsWith + +class FeaturedAggregationDuplicateKeyTest { + private fun booleanFlag( + key: String, + kind: FlagKind = FlagKind.LOCAL, + ) = FlagDescriptor( + key = key, + propertyName = key, + kind = kind, + valueType = ValueType.BOOLEAN, + defaultValue = "false", + ) + + @Test + fun `no error for unique keys across modules`() { + val manifests = + listOf( + FeaturedManifest( + schemaVersion = SCHEMA_VERSION, + modulePath = ":feature-a", + flags = listOf(booleanFlag("dark_mode")), + ), + FeaturedManifest( + schemaVersion = SCHEMA_VERSION, + modulePath = ":feature-b", + flags = listOf(booleanFlag("show_banner")), + ), + ) + // Should not throw + validateUniqueKeys(manifests) + } + + @Test + fun `duplicate key across two modules throws with both module paths`() { + val manifests = + listOf( + FeaturedManifest( + schemaVersion = SCHEMA_VERSION, + modulePath = ":feature-a", + flags = listOf(booleanFlag("dark_mode")), + ), + FeaturedManifest( + schemaVersion = SCHEMA_VERSION, + modulePath = ":feature-b", + flags = listOf(booleanFlag("dark_mode")), + ), + ) + val ex = assertFailsWith { validateUniqueKeys(manifests) } + assertContains(ex.message ?: "", "dark_mode", message = "Message must contain the duplicate key") + assertContains(ex.message ?: "", ":feature-a", message = "Message must name first module path") + assertContains(ex.message ?: "", ":feature-b", message = "Message must name second module path") + } + + @Test + fun `same key in LOCAL and REMOTE of same module is a duplicate`() { + // A single module declaring the same key in both localFlags and remoteFlags. + val manifests = + listOf( + FeaturedManifest( + schemaVersion = SCHEMA_VERSION, + modulePath = ":feature-checkout", + flags = + listOf( + booleanFlag(key = "checkout_mode", kind = FlagKind.LOCAL), + booleanFlag(key = "checkout_mode", kind = FlagKind.REMOTE), + ), + ), + ) + val ex = assertFailsWith { validateUniqueKeys(manifests) } + assertContains(ex.message ?: "", "checkout_mode", message = "Message must contain the duplicate key") + // Both paths are the same module; message still names the module path twice + assertContains(ex.message ?: "", ":feature-checkout", message = "Message must name module path") + } +} diff --git a/featured-gradle-plugin/src/test/kotlin/dev/androidbroadcast/featured/gradle/aggregation/FeaturedAggregationIntegrationTest.kt b/featured-gradle-plugin/src/test/kotlin/dev/androidbroadcast/featured/gradle/aggregation/FeaturedAggregationIntegrationTest.kt new file mode 100644 index 0000000..84de5c3 --- /dev/null +++ b/featured-gradle-plugin/src/test/kotlin/dev/androidbroadcast/featured/gradle/aggregation/FeaturedAggregationIntegrationTest.kt @@ -0,0 +1,198 @@ +package dev.androidbroadcast.featured.gradle.aggregation + +import dev.androidbroadcast.featured.gradle.manifest.androidSdkDirOrNull +import dev.androidbroadcast.featured.gradle.manifest.copyManifestFixture +import org.gradle.testkit.runner.GradleRunner +import org.gradle.testkit.runner.TaskOutcome +import org.junit.Assume.assumeTrue +import org.junit.Before +import org.junit.Rule +import org.junit.Test +import org.junit.rules.TemporaryFolder +import java.io.File +import kotlin.test.assertEquals +import kotlin.test.assertTrue + +/** + * Integration tests for the multi-module registry aggregation using the + * `aggregator-multi-module-project` fixture (two Android library modules + aggregating app). + * + * Skipped when `ANDROID_HOME` / `ANDROID_SDK_ROOT` is not set. + */ +class FeaturedAggregationIntegrationTest { + @get:Rule + val tempFolder = TemporaryFolder() + + private lateinit var projectDir: File + + @Before + fun setUp() { + val sdkDir = androidSdkDirOrNull() + assumeTrue( + "ANDROID_HOME or ANDROID_SDK_ROOT must be set to run integration tests", + sdkDir != null, + ) + + projectDir = tempFolder.newFolder("aggregator-multi-module-project") + copyManifestFixture(fixtureName = "aggregator-multi-module-project", dest = projectDir) + + // Write local.properties with the real SDK path — use invariantSeparatorsPath so that + // a raw Windows SDK path would not corrupt local.properties. + projectDir.resolve("local.properties").writeText("sdk.dir=${sdkDir!!.invariantSeparatorsPath}\n") + } + + @Test + fun `generateFeaturedRegistry succeeds`() { + val result = + gradleRunner() + .withArguments(":app:$GENERATE_FEATURED_REGISTRY_TASK_NAME", "--stacktrace") + .build() + + val outcome = result.task(":app:$GENERATE_FEATURED_REGISTRY_TASK_NAME")?.outcome + assertEquals( + TaskOutcome.SUCCESS, + outcome, + "Expected :app:$GENERATE_FEATURED_REGISTRY_TASK_NAME to succeed, got $outcome\n${result.output}", + ) + } + + @Test + fun `generated file exists at expected path`() { + gradleRunner() + .withArguments(":app:$GENERATE_FEATURED_REGISTRY_TASK_NAME") + .build() + + val generatedFile = + projectDir.resolve( + "app/build/generated/featured/commonMain/${FEATURED_REGISTRY_OBJECT}.kt", + ) + assertTrue(generatedFile.exists(), "Expected generated file at ${generatedFile.path}") + } + + @Test + fun `generated source contains expected ConfigParam entries`() { + gradleRunner() + .withArguments(":app:$GENERATE_FEATURED_REGISTRY_TASK_NAME") + .build() + + val source = + projectDir + .resolve("app/build/generated/featured/commonMain/${FEATURED_REGISTRY_OBJECT}.kt") + .readText() + + assertTrue(source.contains("object $FEATURED_REGISTRY_OBJECT"), "Missing object declaration") + assertTrue(source.contains("listOf("), "Missing listOf() in generated source") + assertTrue( + source.contains("ConfigParam(key = \"dark_mode\""), + "Missing dark_mode (Boolean) entry", + ) + assertTrue( + source.contains("ConfigParam(key = \"checkout_variant\""), + "Missing checkout_variant (ENUM) entry", + ) + assertTrue( + source.contains("ConfigParam(key = \"show_avatar\""), + "Missing show_avatar (Boolean) entry", + ) + assertTrue( + source.contains("ConfigParam(key = \"avatar_placeholder\""), + "Missing avatar_placeholder (String) entry", + ) + } + + @Test + fun `second run without changes reports UP_TO_DATE`() { + gradleRunner() + .withArguments(":app:$GENERATE_FEATURED_REGISTRY_TASK_NAME") + .build() + + val result = + gradleRunner() + .withArguments(":app:$GENERATE_FEATURED_REGISTRY_TASK_NAME") + .build() + + val outcome = result.task(":app:$GENERATE_FEATURED_REGISTRY_TASK_NAME")?.outcome + assertTrue( + outcome == TaskOutcome.UP_TO_DATE || outcome == TaskOutcome.FROM_CACHE, + "Expected UP_TO_DATE or FROM_CACHE on second run, got $outcome", + ) + } + + @Test + fun `mutating a feature module invalidates the registry task`() { + gradleRunner() + .withArguments(":app:$GENERATE_FEATURED_REGISTRY_TASK_NAME") + .build() + + // Add a new flag to :feature-checkout to invalidate the manifest artifact. + val buildFile = projectDir.resolve("feature-checkout/build.gradle.kts") + buildFile.writeText( + buildFile.readText().replace( + "enum(\"checkout_variant\", typeFqn = \"com.example.CheckoutVariant\", default = \"LEGACY\")", + "enum(\"checkout_variant\", typeFqn = \"com.example.CheckoutVariant\", default = \"LEGACY\")\n" + + " int(\"max_retries\", default = 3)", + ), + ) + + val result = + gradleRunner() + .withArguments(":app:$GENERATE_FEATURED_REGISTRY_TASK_NAME") + .build() + + val outcome = result.task(":app:$GENERATE_FEATURED_REGISTRY_TASK_NAME")?.outcome + assertEquals( + TaskOutcome.SUCCESS, + outcome, + "Expected SUCCESS after input change, got $outcome", + ) + } + + @Test + fun `configuration cache stores on first run`() { + val result = + gradleRunner() + .withArguments( + ":app:$GENERATE_FEATURED_REGISTRY_TASK_NAME", + "--configuration-cache", + "--configuration-cache-problems=warn", + ).build() + + assertTrue( + result.output.contains("Configuration cache entry stored"), + "Expected 'Configuration cache entry stored' in output, got:\n${result.output}", + ) + } + + @Test + fun `configuration cache is reused on second run`() { + gradleRunner() + .withArguments( + ":app:$GENERATE_FEATURED_REGISTRY_TASK_NAME", + "--configuration-cache", + "--configuration-cache-problems=warn", + ).build() + + val secondRun = + gradleRunner() + .withArguments( + ":app:$GENERATE_FEATURED_REGISTRY_TASK_NAME", + "--configuration-cache", + "--configuration-cache-problems=warn", + ).build() + + assertTrue( + secondRun.output.contains("Configuration cache entry reused") || + secondRun.output.contains("Reusing configuration cache"), + "Expected CC reuse marker in second-run output, got:\n${secondRun.output}", + ) + } + + // ── Helpers ─────────────────────────────────────────────────────────────── + + private fun gradleRunner(): GradleRunner = + GradleRunner + .create() + .withProjectDir(projectDir) + .withPluginClasspath() + .forwardOutput() +} diff --git a/featured-gradle-plugin/src/test/kotlin/dev/androidbroadcast/featured/gradle/aggregation/GenerateFeaturedRegistryTaskRegistrationTest.kt b/featured-gradle-plugin/src/test/kotlin/dev/androidbroadcast/featured/gradle/aggregation/GenerateFeaturedRegistryTaskRegistrationTest.kt new file mode 100644 index 0000000..041f3e6 --- /dev/null +++ b/featured-gradle-plugin/src/test/kotlin/dev/androidbroadcast/featured/gradle/aggregation/GenerateFeaturedRegistryTaskRegistrationTest.kt @@ -0,0 +1,75 @@ +package dev.androidbroadcast.featured.gradle.aggregation + +import org.gradle.testfixtures.ProjectBuilder +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNotNull +import kotlin.test.assertTrue + +@Suppress("UnstableApiUsage") +class GenerateFeaturedRegistryTaskRegistrationTest { + @Test + fun `plugin registers generateFeaturedRegistry task`() { + val project = ProjectBuilder.builder().build() + project.plugins.apply("dev.androidbroadcast.featured.application") + + assertTrue( + project.tasks.names.contains(GENERATE_FEATURED_REGISTRY_TASK_NAME), + "Expected '$GENERATE_FEATURED_REGISTRY_TASK_NAME' task to be registered lazily by the plugin", + ) + } + + @Test + fun `generateFeaturedRegistry task is of correct type`() { + val project = ProjectBuilder.builder().build() + project.plugins.apply("dev.androidbroadcast.featured.application") + + val task = project.tasks.findByName(GENERATE_FEATURED_REGISTRY_TASK_NAME) + assertNotNull(task, "Expected '$GENERATE_FEATURED_REGISTRY_TASK_NAME' task to be registered") + assertTrue( + task is GenerateFeaturedRegistryTask, + "Expected task type GenerateFeaturedRegistryTask but was ${task::class.simpleName}", + ) + } + + @Test + fun `generateFeaturedRegistry task is in featured group`() { + val project = ProjectBuilder.builder().build() + project.plugins.apply("dev.androidbroadcast.featured.application") + + val task = project.tasks.findByName(GENERATE_FEATURED_REGISTRY_TASK_NAME) as? GenerateFeaturedRegistryTask + assertNotNull(task) + assertEquals("featured", task.group, "Expected task group 'featured' but was '${task.group}'") + } + + @Test + fun `generateFeaturedRegistry task outputPackage defaults to FEATURED_REGISTRY_PACKAGE`() { + val project = ProjectBuilder.builder().build() + project.plugins.apply("dev.androidbroadcast.featured.application") + + val task = project.tasks.findByName(GENERATE_FEATURED_REGISTRY_TASK_NAME) as? GenerateFeaturedRegistryTask + assertNotNull(task) + assertEquals( + FEATURED_REGISTRY_PACKAGE, + task.outputPackage.get(), + "Expected outputPackage == FEATURED_REGISTRY_PACKAGE", + ) + } + + @Test + fun `generateFeaturedRegistry task outputFile path follows convention`() { + val project = ProjectBuilder.builder().build() + project.plugins.apply("dev.androidbroadcast.featured.application") + + val task = project.tasks.findByName(GENERATE_FEATURED_REGISTRY_TASK_NAME) as? GenerateFeaturedRegistryTask + assertNotNull(task) + val outputPath = + task.outputFile + .get() + .asFile.path + assertTrue( + outputPath.endsWith("build/generated/featured/commonMain/${FEATURED_REGISTRY_OBJECT}.kt"), + "Expected outputFile to end with 'build/generated/featured/commonMain/${FEATURED_REGISTRY_OBJECT}.kt', got: $outputPath", + ) + } +} diff --git a/featured-gradle-plugin/src/test/kotlin/dev/androidbroadcast/featured/gradle/aggregation/GeneratedFeaturedRegistryGeneratorTest.kt b/featured-gradle-plugin/src/test/kotlin/dev/androidbroadcast/featured/gradle/aggregation/GeneratedFeaturedRegistryGeneratorTest.kt new file mode 100644 index 0000000..313f2b4 --- /dev/null +++ b/featured-gradle-plugin/src/test/kotlin/dev/androidbroadcast/featured/gradle/aggregation/GeneratedFeaturedRegistryGeneratorTest.kt @@ -0,0 +1,252 @@ +package dev.androidbroadcast.featured.gradle.aggregation + +import dev.androidbroadcast.featured.gradle.manifest.FeaturedManifest +import dev.androidbroadcast.featured.gradle.manifest.FlagDescriptor +import dev.androidbroadcast.featured.gradle.manifest.FlagKind +import dev.androidbroadcast.featured.gradle.manifest.SCHEMA_VERSION +import dev.androidbroadcast.featured.gradle.manifest.ValueType +import kotlin.test.Test +import kotlin.test.assertContains +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +class GeneratedFeaturedRegistryGeneratorTest { + private fun manifest( + modulePath: String, + vararg flags: FlagDescriptor, + ): FeaturedManifest = + FeaturedManifest( + schemaVersion = SCHEMA_VERSION, + modulePath = modulePath, + flags = flags.toList(), + ) + + private fun flag( + key: String, + valueType: ValueType, + defaultValue: String, + kind: FlagKind = FlagKind.LOCAL, + enumTypeFqn: String? = null, + description: String? = null, + category: String? = null, + ) = FlagDescriptor( + key = key, + propertyName = key, + kind = kind, + valueType = valueType, + defaultValue = defaultValue, + enumTypeFqn = enumTypeFqn, + description = description, + category = category, + ) + + @Test + fun `empty manifests list produces emptyList body`() { + val source = + GeneratedFeaturedRegistryGenerator.generate( + manifests = emptyList(), + packageName = FEATURED_REGISTRY_PACKAGE, + ) + assertContains(source, "emptyList()") + assertFalse(source.contains("listOf("), "Expected no listOf when empty") + } + + @Test + fun `single BOOLEAN local flag emits correct ConfigParam`() { + val source = + GeneratedFeaturedRegistryGenerator.generate( + manifests = listOf(manifest(":app", flag(key = "dark_mode", valueType = ValueType.BOOLEAN, defaultValue = "false"))), + packageName = FEATURED_REGISTRY_PACKAGE, + ) + assertContains(source, "ConfigParam(key = \"dark_mode\", defaultValue = false)") + } + + @Test + fun `LONG suffix is L`() { + val source = + GeneratedFeaturedRegistryGenerator.generate( + manifests = listOf(manifest(":app", flag(key = "timeout", valueType = ValueType.LONG, defaultValue = "123"))), + packageName = FEATURED_REGISTRY_PACKAGE, + ) + assertContains(source, "defaultValue = 123L") + } + + @Test + fun `FLOAT suffix is f`() { + val source = + GeneratedFeaturedRegistryGenerator.generate( + manifests = listOf(manifest(":app", flag(key = "ratio", valueType = ValueType.FLOAT, defaultValue = "1.5"))), + packageName = FEATURED_REGISTRY_PACKAGE, + ) + assertContains(source, "defaultValue = 1.5f") + } + + @Test + fun `DOUBLE emits raw value`() { + val source = + GeneratedFeaturedRegistryGenerator.generate( + manifests = listOf(manifest(":app", flag(key = "pi", valueType = ValueType.DOUBLE, defaultValue = "3.14"))), + packageName = FEATURED_REGISTRY_PACKAGE, + ) + assertContains(source, "defaultValue = 3.14") + assertFalse(source.contains("3.14f"), "DOUBLE must not have f suffix") + assertFalse(source.contains("3.14L"), "DOUBLE must not have L suffix") + } + + @Test + fun `INT emits raw value`() { + val source = + GeneratedFeaturedRegistryGenerator.generate( + manifests = listOf(manifest(":app", flag(key = "retries", valueType = ValueType.INT, defaultValue = "3"))), + packageName = FEATURED_REGISTRY_PACKAGE, + ) + assertContains(source, "defaultValue = 3") + assertFalse(source.contains("3L"), "INT must not have L suffix") + } + + @Test + fun `STRING re-wraps bare value in quotes`() { + // Producer stores bare value: "hello world" (no surrounding quotes) + val source = + GeneratedFeaturedRegistryGenerator.generate( + manifests = listOf(manifest(":app", flag(key = "label", valueType = ValueType.STRING, defaultValue = "hello world"))), + packageName = FEATURED_REGISTRY_PACKAGE, + ) + assertContains(source, "defaultValue = \"hello world\"") + } + + @Test + fun `STRING escapes embedded double quotes`() { + // Producer stores bare: say "hi" — generator must emit: "say \"hi\"" + val source = + GeneratedFeaturedRegistryGenerator.generate( + manifests = listOf(manifest(":app", flag(key = "greeting", valueType = ValueType.STRING, defaultValue = """say "hi""""))), + packageName = FEATURED_REGISTRY_PACKAGE, + ) + assertContains(source, """defaultValue = "say \"hi\"""") + } + + @Test + fun `ENUM emits enumTypeFqn dot constant as default and type arg`() { + val source = + GeneratedFeaturedRegistryGenerator.generate( + manifests = + listOf( + manifest( + ":feature", + flag( + key = "checkout_variant", + valueType = ValueType.ENUM, + defaultValue = "LEGACY", + enumTypeFqn = "com.example.CheckoutVariant", + ), + ), + ), + packageName = FEATURED_REGISTRY_PACKAGE, + ) + assertContains(source, "ConfigParam") + assertContains(source, "defaultValue = com.example.CheckoutVariant.LEGACY") + } + + @Test + fun `multi-module input lists all flags`() { + val moduleA = + manifest( + ":feature-a", + flag(key = "flag_a1", valueType = ValueType.BOOLEAN, defaultValue = "true"), + flag(key = "flag_a2", valueType = ValueType.INT, defaultValue = "1"), + ) + val moduleB = + manifest( + ":feature-b", + flag(key = "flag_b1", valueType = ValueType.STRING, defaultValue = "hello"), + flag(key = "flag_b2", valueType = ValueType.LONG, defaultValue = "99"), + ) + val source = + GeneratedFeaturedRegistryGenerator.generate( + manifests = listOf(moduleA, moduleB), + packageName = FEATURED_REGISTRY_PACKAGE, + ) + assertContains(source, "flag_a1") + assertContains(source, "flag_a2") + assertContains(source, "flag_b1") + assertContains(source, "flag_b2") + } + + @Test + fun `stable order manifests in B-A input produce flags sorted by modulePath then key`() { + // Manifests passed in [B, A] order — output must be A's flags first, then B's. + val moduleA = + manifest( + ":feature-a", + flag(key = "z_flag", valueType = ValueType.BOOLEAN, defaultValue = "false"), + flag(key = "a_flag", valueType = ValueType.BOOLEAN, defaultValue = "true"), + ) + val moduleB = + manifest( + ":feature-b", + flag(key = "m_flag", valueType = ValueType.INT, defaultValue = "5"), + ) + // Pass B before A intentionally + val source = + GeneratedFeaturedRegistryGenerator.generate( + manifests = listOf(moduleB, moduleA), + packageName = FEATURED_REGISTRY_PACKAGE, + ) + val aFlagPos = source.indexOf("a_flag") + val zFlagPos = source.indexOf("z_flag") + val mFlagPos = source.indexOf("m_flag") + + // :feature-a < :feature-b alphabetically; within :feature-a, a_flag < z_flag + assertTrue(aFlagPos < zFlagPos, "a_flag must appear before z_flag (within :feature-a)") + assertTrue(zFlagPos < mFlagPos, "z_flag (:feature-a) must appear before m_flag (:feature-b)") + } + + @Test + fun `optional description is emitted when non-null`() { + val source = + GeneratedFeaturedRegistryGenerator.generate( + manifests = + listOf( + manifest( + ":app", + flag( + key = "my_flag", + valueType = ValueType.BOOLEAN, + defaultValue = "true", + description = "Controls the widget", + ), + ), + ), + packageName = FEATURED_REGISTRY_PACKAGE, + ) + assertContains(source, "description = \"Controls the widget\"") + } + + @Test + fun `null description is omitted from ConfigParam args`() { + val source = + GeneratedFeaturedRegistryGenerator.generate( + manifests = + listOf( + manifest(":app", flag(key = "my_flag", valueType = ValueType.BOOLEAN, defaultValue = "false")), + ), + packageName = FEATURED_REGISTRY_PACKAGE, + ) + assertFalse(source.contains("description ="), "description must be absent when null") + } + + @Test + fun `since parameter is never emitted`() { + // Manifest schema v1 has no since field; ConfigParam accepts it but we never emit it + val source = + GeneratedFeaturedRegistryGenerator.generate( + manifests = + listOf( + manifest(":app", flag(key = "feature", valueType = ValueType.BOOLEAN, defaultValue = "true")), + ), + packageName = FEATURED_REGISTRY_PACKAGE, + ) + assertFalse(source.contains("since ="), "since must never be emitted") + } +} From 18a4158bfe230de774b01c241463ba7d2ab4c622 Mon Sep 17 00:00:00 2001 From: kirich1409 Date: Tue, 19 May 2026 11:42:34 +0300 Subject: [PATCH 2/3] Harden aggregator against malformed and hostile manifests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Apply finalize Round 1 findings across code-reviewer, /simplify, the pr-review-toolkit trio, and the architecture/build/security experts: - Escape newline / carriage-return / tab in escapeKotlinString so a description or STRING default with literal whitespace cannot break the generated source. - Validate enumTypeFqn against Kotlin FQN grammar and ENUM defaultValue against the Kotlin identifier grammar before codegen. A hostile project dependency can no longer inject Kotlin source through the featured-manifest.json payload that would execute on the next compile in the consumer module. - Sort manifests by modulePath inside validateUniqueKeys so the duplicate-key error lists origins in a deterministic order regardless of Gradle resolution order. - Wrap manifest parse and read errors with the offending file path so diagnostics name which artifact failed. - Thread the offending key + module path into the requireNotNull error raised when an ENUM descriptor is missing its enumTypeFqn. - Validate outputPackage against a Kotlin package-name regex at task execution time instead of waiting for the consumer compile to fail. - Narrow FeaturedApplicationPlugin to internal — Gradle still resolves the descriptor by FQN via reflection, and the consumer-facing surface is the plugin id alone. - Remove the redundant pre-sort and dead Origin class in the task; the generator does its own (modulePath, key) sort, and the validator now owns ordering for its own error path. - Drop a stale inline comment that paraphrased the generator KDoc. Tests: - Newline, carriage return, and tab escape tests for STRING defaults and descriptions. - Parse-error test asserting the wrapper carries the file path. - Descriptor-integrity tests covering enumTypeFqn / defaultValue injection attempts (semicolon, angle bracket, parenthesis, brace, whitespace, Unicode line separator, missing FQN). - Lazy-realization test mirroring the producer-side pattern. :featured-gradle-plugin:check — BUILD SUCCESSFUL, 266 tests, 0 failures. Co-Authored-By: Claude Opus 4.7 --- .../gradle/FeaturedApplicationPlugin.kt | 2 +- .../GenerateFeaturedRegistryTask.kt | 112 ++++++++++++--- .../GeneratedFeaturedRegistryGenerator.kt | 48 +++++-- ...turedAggregationDescriptorIntegrityTest.kt | 114 +++++++++++++++ .../FeaturedAggregationDuplicateKeyTest.kt | 55 +++++++- .../FeaturedAggregationParseErrorTest.kt | 46 +++++++ ...ateFeaturedRegistryTaskRegistrationTest.kt | 15 ++ .../GeneratedFeaturedRegistryGeneratorTest.kt | 130 ++++++++++++++++++ 8 files changed, 489 insertions(+), 33 deletions(-) create mode 100644 featured-gradle-plugin/src/test/kotlin/dev/androidbroadcast/featured/gradle/aggregation/FeaturedAggregationDescriptorIntegrityTest.kt create mode 100644 featured-gradle-plugin/src/test/kotlin/dev/androidbroadcast/featured/gradle/aggregation/FeaturedAggregationParseErrorTest.kt diff --git a/featured-gradle-plugin/src/main/kotlin/dev/androidbroadcast/featured/gradle/FeaturedApplicationPlugin.kt b/featured-gradle-plugin/src/main/kotlin/dev/androidbroadcast/featured/gradle/FeaturedApplicationPlugin.kt index 15c5e61..a70770f 100644 --- a/featured-gradle-plugin/src/main/kotlin/dev/androidbroadcast/featured/gradle/FeaturedApplicationPlugin.kt +++ b/featured-gradle-plugin/src/main/kotlin/dev/androidbroadcast/featured/gradle/FeaturedApplicationPlugin.kt @@ -47,7 +47,7 @@ import org.gradle.api.attributes.Usage * Min Gradle version: 8.5+ (`configurations.dependencyScope()` / `.resolvable()` API). */ @Suppress("UnstableApiUsage") -public class FeaturedApplicationPlugin : Plugin { +internal class FeaturedApplicationPlugin : Plugin { override fun apply(target: Project) { // Register the schemaMajorAttr in the project's attribute schema. This is idempotent — // if FeaturedPlugin is also applied it calls the same registration first. diff --git a/featured-gradle-plugin/src/main/kotlin/dev/androidbroadcast/featured/gradle/aggregation/GenerateFeaturedRegistryTask.kt b/featured-gradle-plugin/src/main/kotlin/dev/androidbroadcast/featured/gradle/aggregation/GenerateFeaturedRegistryTask.kt index a1033ab..3dceada 100644 --- a/featured-gradle-plugin/src/main/kotlin/dev/androidbroadcast/featured/gradle/aggregation/GenerateFeaturedRegistryTask.kt +++ b/featured-gradle-plugin/src/main/kotlin/dev/androidbroadcast/featured/gradle/aggregation/GenerateFeaturedRegistryTask.kt @@ -2,6 +2,7 @@ package dev.androidbroadcast.featured.gradle.aggregation import dev.androidbroadcast.featured.gradle.manifest.FeaturedManifest import dev.androidbroadcast.featured.gradle.manifest.FeaturedManifestJson +import dev.androidbroadcast.featured.gradle.manifest.ValueType import kotlinx.serialization.decodeFromString import org.gradle.api.DefaultTask import org.gradle.api.file.ConfigurableFileCollection @@ -15,6 +16,14 @@ import org.gradle.api.tasks.PathSensitive import org.gradle.api.tasks.PathSensitivity import org.gradle.api.tasks.TaskAction +private val PACKAGE_NAME_REGEX = Regex("[a-zA-Z][a-zA-Z0-9_]*(\\.[a-zA-Z][a-zA-Z0-9_]*)*") + +// Accepted grammar for ENUM descriptor fields interpolated verbatim into generated Kotlin source. +// Untrusted manifest content from a malicious project dependency can inject Kotlin source via +// ENUM FQN or constant name — we reject anything that does not match before calling the generator. +private val KOTLIN_FQN_REGEX = Regex("[A-Za-z_][A-Za-z0-9_]*(\\.[A-Za-z_][A-Za-z0-9_]*)*") +private val KOTLIN_IDENTIFIER_REGEX = Regex("[A-Za-z_][A-Za-z0-9_]*") + /** * Aggregates `featured-manifest.json` files from all project dependencies declared via * `featuredAggregation(...)` and generates `GeneratedFeaturedRegistry.kt`. @@ -52,16 +61,31 @@ internal abstract class GenerateFeaturedRegistryTask : DefaultTask() { @TaskAction fun generate() { + val pkg = outputPackage.get() + require(PACKAGE_NAME_REGEX.matches(pkg)) { + "outputPackage '$pkg' is not a valid Kotlin package name." + } + val manifests = manifestFiles.files - .map { file -> FeaturedManifestJson.decodeFromString(file.readText()) } + .map { file -> + try { + FeaturedManifestJson.decodeFromString(file.readText()) + } catch (e: Exception) { + throw IllegalStateException( + "Failed to read or parse Featured manifest at '${file.path}': ${e.message}", + e, + ) + } + } validateUniqueKeys(manifests) + validateFlagDescriptorIntegrity(manifests) val source = GeneratedFeaturedRegistryGenerator.generate( manifests = manifests, - packageName = outputPackage.get(), + packageName = pkg, ) val outFile = outputFile.get().asFile @@ -82,21 +106,77 @@ internal abstract class GenerateFeaturedRegistryTask : DefaultTask() { * A flag declared in both `localFlags` and `remoteFlags` of the same module is treated as a * duplicate because each key produces exactly one `ConfigParam` in the registry. * - * @throws IllegalStateException when a duplicate key is detected, naming both module paths and the key. + * All duplicate keys are collected and reported in a single [IllegalStateException] so that + * every conflict is visible without requiring repeated build invocations. Each origin includes + * both the module path and the [FlagKind] so same-module LOCAL/REMOTE collisions are + * distinguishable from cross-module collisions. + * + * Manifests are sorted by [FeaturedManifest.modulePath] internally before processing so that + * the duplicate error message lists origins in a deterministic order regardless of the order + * in which Gradle resolves manifest artifacts. + * + * @throws IllegalStateException listing every duplicate key and all conflicting origins. */ internal fun validateUniqueKeys(manifests: List) { - // Pair each descriptor with its module path for error reporting. - val pairs = manifests.flatMap { manifest -> manifest.flags.map { flag -> flag.key to manifest.modulePath } } - - // Group by key; any group with more than one entry is a duplicate. - pairs - .groupBy { (key, _) -> key } - .filter { (_, entries) -> entries.size > 1 } - .forEach { (key, entries) -> - val pathA = entries[0].second - val pathB = entries[1].second - throw IllegalStateException( - "Duplicate flag key '$key': declared in '$pathA' and '$pathB'", - ) + val triples = + manifests + .sortedBy { it.modulePath } + .flatMap { manifest -> + manifest.flags.map { flag -> Triple(flag.key, manifest.modulePath, flag.kind) } + } + + // Collect every key that appears more than once, together with all its origins. + val duplicates = + triples + .groupBy { (key, _, _) -> key } + .filter { (_, entries) -> entries.size > 1 } + + if (duplicates.isEmpty()) return + + val message = + buildString { + appendLine("Duplicate flag keys detected in aggregated Featured manifests:") + duplicates.forEach { (key, entries) -> + val origins = entries.joinToString(", ") { (_, path, kind) -> "'$path' ($kind)" } + appendLine(" - '$key': declared in $origins") + } } + throw IllegalStateException(message.trimEnd()) +} + +/** + * Validates the integrity of ENUM flag descriptors in [manifests] against Kotlin grammar before + * passing them to the code generator. + * + * Threat model: a malicious build-script author of a project dependency declared via + * `featuredAggregation(project(":evil"))` controls the contents of `featured-manifest.json` + * and can supply arbitrary strings for `enumTypeFqn` and `defaultValue`. Both fields are + * interpolated verbatim into the generated `.kt` file as Kotlin identifiers (not string + * literals), so injecting `;`, `{`, `(`, or similar characters produces syntactically valid + * Kotlin with arbitrary code that executes during the consuming project's `:compileKotlin`. + * + * We validate against Kotlin grammar here — single source of truth in the task — so the + * generator can never emit unintended syntax regardless of what arrives in the manifest. + * + * @throws IllegalArgumentException when any ENUM flag has an invalid [enumTypeFqn] or + * [defaultValue], naming the offending key and module in the message. + */ +internal fun validateFlagDescriptorIntegrity(manifests: List) { + manifests.forEach { manifest -> + manifest.flags + .filter { it.valueType == ValueType.ENUM } + .forEach { flag -> + requireNotNull(flag.enumTypeFqn) { + "enumTypeFqn must not be null for ENUM flag '${flag.key}' in module '${manifest.modulePath}'." + } + require(KOTLIN_FQN_REGEX.matches(flag.enumTypeFqn)) { + "Invalid enumTypeFqn '${flag.enumTypeFqn}' for flag '${flag.key}' in module '${manifest.modulePath}': " + + "must be a valid Kotlin fully-qualified name." + } + require(KOTLIN_IDENTIFIER_REGEX.matches(flag.defaultValue)) { + "Invalid ENUM defaultValue '${flag.defaultValue}' for flag '${flag.key}' in module '${manifest.modulePath}': " + + "must be a valid Kotlin identifier." + } + } + } } diff --git a/featured-gradle-plugin/src/main/kotlin/dev/androidbroadcast/featured/gradle/aggregation/GeneratedFeaturedRegistryGenerator.kt b/featured-gradle-plugin/src/main/kotlin/dev/androidbroadcast/featured/gradle/aggregation/GeneratedFeaturedRegistryGenerator.kt index 7a2f2e9..c9ba018 100644 --- a/featured-gradle-plugin/src/main/kotlin/dev/androidbroadcast/featured/gradle/aggregation/GeneratedFeaturedRegistryGenerator.kt +++ b/featured-gradle-plugin/src/main/kotlin/dev/androidbroadcast/featured/gradle/aggregation/GeneratedFeaturedRegistryGenerator.kt @@ -30,8 +30,6 @@ internal object GeneratedFeaturedRegistryGenerator { manifests: List, packageName: String, ): String { - // Flatten to (modulePath, descriptor) pairs, then sort for deterministic output. - // Sorting here (not in the caller) guarantees stability regardless of manifest order. val sorted = manifests .flatMap { manifest -> manifest.flags.map { flag -> manifest.modulePath to flag } } @@ -48,15 +46,20 @@ internal object GeneratedFeaturedRegistryGenerator { appendLine(" public val all: List> = emptyList()") } else { appendLine(" public val all: List> = listOf(") - sorted.forEach { (_, descriptor) -> + sorted.forEach { (modulePath, descriptor) -> + if (descriptor.valueType == ValueType.ENUM) { + requireNotNull(descriptor.enumTypeFqn) { + "enumTypeFqn must be non-null for ENUM flag '${descriptor.key}' in module '$modulePath'" + } + } val typeArg = descriptor.valueType.toKotlinTypeName(descriptor.enumTypeFqn) val defaultLiteral = descriptor.toDefaultLiteral() val args = buildList { - add("key = \"${descriptor.key}\"") + add("key = \"${escapeKotlinString(descriptor.key)}\"") add("defaultValue = $defaultLiteral") - if (descriptor.description != null) add("description = \"${descriptor.description}\"") - if (descriptor.category != null) add("category = \"${descriptor.category}\"") + if (descriptor.description != null) add("description = \"${escapeKotlinString(descriptor.description)}\"") + if (descriptor.category != null) add("category = \"${escapeKotlinString(descriptor.category)}\"") } // Kotlin accepts trailing commas in listOf() — always emit one for uniform diffs. appendLine(" ConfigParam<$typeArg>(${args.joinToString(", ")}),") @@ -106,11 +109,33 @@ private fun ValueType.toKotlinTypeName(enumTypeFqn: String?): String = } } +/** + * Escapes a bare string value so it is safe to embed inside a Kotlin double-quoted string literal. + * + * Escape order matters: `\` must be processed first to avoid double-escaping characters + * introduced by subsequent replacements. + * + * - `\` → `\\` (backslash) + * - `"` → `\"` (double-quote) + * - `$` → `${'$'}` (prevents Kotlin string-template interpolation in the generated source) + * - `\n` → `\\n` (newline) + * - `\r` → `\\r` (carriage return) + * - `\t` → `\\t` (tab) + */ +private fun escapeKotlinString(value: String): String = + value + .replace("\\", "\\\\") + .replace("\"", "\\\"") + .replace("$", "\${'\$'}") + .replace("\n", "\\n") + .replace("\r", "\\r") + .replace("\t", "\\t") + /** * Produces the Kotlin literal for `defaultValue = ...` in the generated `ConfigParam` call. * * STRING: producer stores bare value (surrounding quotes already stripped); re-wrap and - * escape embedded backslashes and double-quotes. + * escape via [escapeKotlinString]. * LONG: append `L` suffix. * FLOAT: append `f` suffix. * ENUM: rebuild as `enumTypeFqn.CONSTANT_NAME`. @@ -119,14 +144,7 @@ private fun ValueType.toKotlinTypeName(enumTypeFqn: String?): String = private fun FlagDescriptor.toDefaultLiteral(): String = when (valueType) { ValueType.STRING -> { - // Escape in order: `\` first (avoids double-escaping), then `"`, then `$` - // (prevents Kotlin string template interpolation in the generated source). - val escaped = - defaultValue - .replace("\\", "\\\\") - .replace("\"", "\\\"") - .replace("$", "\${'\$'}") - "\"$escaped\"" + "\"${escapeKotlinString(defaultValue)}\"" } ValueType.LONG -> { diff --git a/featured-gradle-plugin/src/test/kotlin/dev/androidbroadcast/featured/gradle/aggregation/FeaturedAggregationDescriptorIntegrityTest.kt b/featured-gradle-plugin/src/test/kotlin/dev/androidbroadcast/featured/gradle/aggregation/FeaturedAggregationDescriptorIntegrityTest.kt new file mode 100644 index 0000000..670b627 --- /dev/null +++ b/featured-gradle-plugin/src/test/kotlin/dev/androidbroadcast/featured/gradle/aggregation/FeaturedAggregationDescriptorIntegrityTest.kt @@ -0,0 +1,114 @@ +package dev.androidbroadcast.featured.gradle.aggregation + +import dev.androidbroadcast.featured.gradle.manifest.FeaturedManifest +import dev.androidbroadcast.featured.gradle.manifest.FlagDescriptor +import dev.androidbroadcast.featured.gradle.manifest.FlagKind +import dev.androidbroadcast.featured.gradle.manifest.SCHEMA_VERSION +import dev.androidbroadcast.featured.gradle.manifest.ValueType +import kotlin.test.Test +import kotlin.test.assertContains +import kotlin.test.assertFailsWith + +class FeaturedAggregationDescriptorIntegrityTest { + private fun enumFlag( + key: String = "checkout_variant", + enumTypeFqn: String? = "com.example.CheckoutVariant", + defaultValue: String = "LEGACY", + ) = FlagDescriptor( + key = key, + propertyName = key, + kind = FlagKind.LOCAL, + valueType = ValueType.ENUM, + defaultValue = defaultValue, + enumTypeFqn = enumTypeFqn, + ) + + private fun singleManifest(flag: FlagDescriptor) = + listOf( + FeaturedManifest( + schemaVersion = SCHEMA_VERSION, + modulePath = ":feature-a", + flags = listOf(flag), + ), + ) + + @Test + fun `valid ENUM flag with FQN and identifier constant does not throw`() { + // Sanity: a well-formed manifest passes without exception. + validateFlagDescriptorIntegrity(singleManifest(enumFlag())) + } + + @Test + fun `ENUM flag with semicolon in FQN throws IllegalArgumentException naming key and module`() { + // Simulates a malicious FQN that would inject a Kotlin init block into the generated source. + val maliciousFqn = "kotlin.Unit>(); init { injectCode() }; private val x: ConfigParam { + validateFlagDescriptorIntegrity(singleManifest(enumFlag(enumTypeFqn = maliciousFqn))) + } + val msg = ex.message ?: "" + assertContains(msg, "checkout_variant", message = "Message must name the flag key") + assertContains(msg, ":feature-a", message = "Message must name the module path") + } + + @Test + fun `ENUM flag with angle bracket in FQN throws`() { + assertFailsWith { + validateFlagDescriptorIntegrity(singleManifest(enumFlag(enumTypeFqn = "com.example.Foo"))) + } + } + + @Test + fun `ENUM flag with parenthesis in FQN throws`() { + assertFailsWith { + validateFlagDescriptorIntegrity(singleManifest(enumFlag(enumTypeFqn = "com.example().Foo"))) + } + } + + @Test + fun `ENUM flag with brace in FQN throws`() { + assertFailsWith { + validateFlagDescriptorIntegrity(singleManifest(enumFlag(enumTypeFqn = "com.example{}.Foo"))) + } + } + + @Test + fun `ENUM flag with space in FQN throws`() { + assertFailsWith { + validateFlagDescriptorIntegrity(singleManifest(enumFlag(enumTypeFqn = "com.example .Foo"))) + } + } + + @Test + fun `ENUM flag with Unicode line separator in FQN throws`() { + // U+2028 LINE SEPARATOR — not a valid Kotlin identifier character; must be rejected. + val fqnWithLineSeparator = "com.example
Foo" + assertFailsWith { + validateFlagDescriptorIntegrity(singleManifest(enumFlag(enumTypeFqn = fqnWithLineSeparator))) + } + } + + @Test + fun `ENUM flag with injection in defaultValue throws`() { + // Simulates a malicious constant name that would inject statements into the generated source. + val maliciousDefault = "INSTANCE; injectCode()" + val ex = + assertFailsWith { + validateFlagDescriptorIntegrity(singleManifest(enumFlag(defaultValue = maliciousDefault))) + } + val msg = ex.message ?: "" + assertContains(msg, "checkout_variant", message = "Message must name the flag key") + assertContains(msg, ":feature-a", message = "Message must name the module path") + } + + @Test + fun `ENUM flag with null enumTypeFqn throws IllegalArgumentException naming key and module`() { + val ex = + assertFailsWith { + validateFlagDescriptorIntegrity(singleManifest(enumFlag(enumTypeFqn = null))) + } + val msg = ex.message ?: "" + assertContains(msg, "checkout_variant", message = "Message must name the flag key") + assertContains(msg, ":feature-a", message = "Message must name the module path") + } +} diff --git a/featured-gradle-plugin/src/test/kotlin/dev/androidbroadcast/featured/gradle/aggregation/FeaturedAggregationDuplicateKeyTest.kt b/featured-gradle-plugin/src/test/kotlin/dev/androidbroadcast/featured/gradle/aggregation/FeaturedAggregationDuplicateKeyTest.kt index f7420f3..7e7d6f1 100644 --- a/featured-gradle-plugin/src/test/kotlin/dev/androidbroadcast/featured/gradle/aggregation/FeaturedAggregationDuplicateKeyTest.kt +++ b/featured-gradle-plugin/src/test/kotlin/dev/androidbroadcast/featured/gradle/aggregation/FeaturedAggregationDuplicateKeyTest.kt @@ -78,7 +78,60 @@ class FeaturedAggregationDuplicateKeyTest { ) val ex = assertFailsWith { validateUniqueKeys(manifests) } assertContains(ex.message ?: "", "checkout_mode", message = "Message must contain the duplicate key") - // Both paths are the same module; message still names the module path twice + // Same-module collision: both LOCAL and REMOTE markers must appear so the origin is distinguishable. + assertContains(ex.message ?: "", "LOCAL", message = "Message must name LOCAL kind") + assertContains(ex.message ?: "", "REMOTE", message = "Message must name REMOTE kind") assertContains(ex.message ?: "", ":feature-checkout", message = "Message must name module path") } + + @Test + fun `three modules colliding on same key all appear in error message`() { + val manifests = + listOf( + FeaturedManifest( + schemaVersion = SCHEMA_VERSION, + modulePath = ":feature-a", + flags = listOf(booleanFlag("shared_flag")), + ), + FeaturedManifest( + schemaVersion = SCHEMA_VERSION, + modulePath = ":feature-b", + flags = listOf(booleanFlag("shared_flag")), + ), + FeaturedManifest( + schemaVersion = SCHEMA_VERSION, + modulePath = ":feature-c", + flags = listOf(booleanFlag("shared_flag")), + ), + ) + val ex = assertFailsWith { validateUniqueKeys(manifests) } + val msg = ex.message ?: "" + assertContains(msg, "shared_flag", message = "Message must contain the duplicate key") + assertContains(msg, ":feature-a", message = "Message must name :feature-a") + assertContains(msg, ":feature-b", message = "Message must name :feature-b") + assertContains(msg, ":feature-c", message = "Message must name :feature-c") + } + + @Test + fun `same module LOCAL and REMOTE collision shows both LOCAL and REMOTE not just module path twice`() { + // Regression guard: before the fix the message read "':feature-checkout' and ':feature-checkout'" + // with no kind information — indistinguishable from a cross-module collision with identical names. + val manifests = + listOf( + FeaturedManifest( + schemaVersion = SCHEMA_VERSION, + modulePath = ":feature-checkout", + flags = + listOf( + booleanFlag(key = "show_avatar", kind = FlagKind.LOCAL), + booleanFlag(key = "show_avatar", kind = FlagKind.REMOTE), + ), + ), + ) + val ex = assertFailsWith { validateUniqueKeys(manifests) } + val msg = ex.message ?: "" + assertContains(msg, "show_avatar", message = "Message must contain the duplicate key") + assertContains(msg, "LOCAL", message = "Message must include LOCAL kind marker") + assertContains(msg, "REMOTE", message = "Message must include REMOTE kind marker") + } } diff --git a/featured-gradle-plugin/src/test/kotlin/dev/androidbroadcast/featured/gradle/aggregation/FeaturedAggregationParseErrorTest.kt b/featured-gradle-plugin/src/test/kotlin/dev/androidbroadcast/featured/gradle/aggregation/FeaturedAggregationParseErrorTest.kt new file mode 100644 index 0000000..bb4f85b --- /dev/null +++ b/featured-gradle-plugin/src/test/kotlin/dev/androidbroadcast/featured/gradle/aggregation/FeaturedAggregationParseErrorTest.kt @@ -0,0 +1,46 @@ +package dev.androidbroadcast.featured.gradle.aggregation + +import org.gradle.testfixtures.ProjectBuilder +import java.io.File +import java.nio.file.Files +import kotlin.test.Test +import kotlin.test.assertContains +import kotlin.test.assertFailsWith + +/** + * Verifies that a corrupt or malformed manifest file produces an [IllegalStateException] + * whose message includes the file path so the developer can locate the bad file immediately. + * + * Paired with Fix 3 in GenerateFeaturedRegistryTask. + */ +@Suppress("UnstableApiUsage") +class FeaturedAggregationParseErrorTest { + @Test + fun `malformed manifest json produces IllegalStateException containing file path`() { + val tempDir = Files.createTempDirectory("featured-parse-error-test").toFile() + try { + val badManifest = + File(tempDir, "featured-manifest.json").also { + it.writeText("""{ "broken": json""") + } + val outputFile = File(tempDir, "GeneratedFeaturedRegistry.kt") + + val project = ProjectBuilder.builder().build() + project.plugins.apply("dev.androidbroadcast.featured.application") + + val task = project.tasks.findByName(GENERATE_FEATURED_REGISTRY_TASK_NAME) as GenerateFeaturedRegistryTask + task.manifestFiles.from(badManifest) + task.outputPackage.set(FEATURED_REGISTRY_PACKAGE) + task.outputFile.set(outputFile) + + val ex = assertFailsWith { task.generate() } + assertContains( + ex.message ?: "", + badManifest.path, + message = "Exception message must include the path of the malformed manifest file", + ) + } finally { + tempDir.deleteRecursively() + } + } +} diff --git a/featured-gradle-plugin/src/test/kotlin/dev/androidbroadcast/featured/gradle/aggregation/GenerateFeaturedRegistryTaskRegistrationTest.kt b/featured-gradle-plugin/src/test/kotlin/dev/androidbroadcast/featured/gradle/aggregation/GenerateFeaturedRegistryTaskRegistrationTest.kt index 041f3e6..0cde118 100644 --- a/featured-gradle-plugin/src/test/kotlin/dev/androidbroadcast/featured/gradle/aggregation/GenerateFeaturedRegistryTaskRegistrationTest.kt +++ b/featured-gradle-plugin/src/test/kotlin/dev/androidbroadcast/featured/gradle/aggregation/GenerateFeaturedRegistryTaskRegistrationTest.kt @@ -72,4 +72,19 @@ class GenerateFeaturedRegistryTaskRegistrationTest { "Expected outputFile to end with 'build/generated/featured/commonMain/${FEATURED_REGISTRY_OBJECT}.kt', got: $outputPath", ) } + + @Test + fun `accessing featuredAggregationClasspath configuration does not eagerly realize generateFeaturedRegistry task`() { + val project = ProjectBuilder.builder().build() + project.plugins.apply("dev.androidbroadcast.featured.application") + + // Accessing the configuration by name must not trigger task realization. + project.configurations.getByName(FEATURED_AGGREGATION_CLASSPATH_CONFIGURATION_NAME) + + // The task must still be present in the task graph (registered lazily). + assertTrue( + project.tasks.names.contains(GENERATE_FEATURED_REGISTRY_TASK_NAME), + "Expected '$GENERATE_FEATURED_REGISTRY_TASK_NAME' to be in task names (lazy)", + ) + } } diff --git a/featured-gradle-plugin/src/test/kotlin/dev/androidbroadcast/featured/gradle/aggregation/GeneratedFeaturedRegistryGeneratorTest.kt b/featured-gradle-plugin/src/test/kotlin/dev/androidbroadcast/featured/gradle/aggregation/GeneratedFeaturedRegistryGeneratorTest.kt index 313f2b4..5ecaf60 100644 --- a/featured-gradle-plugin/src/test/kotlin/dev/androidbroadcast/featured/gradle/aggregation/GeneratedFeaturedRegistryGeneratorTest.kt +++ b/featured-gradle-plugin/src/test/kotlin/dev/androidbroadcast/featured/gradle/aggregation/GeneratedFeaturedRegistryGeneratorTest.kt @@ -249,4 +249,134 @@ class GeneratedFeaturedRegistryGeneratorTest { ) assertFalse(source.contains("since ="), "since must never be emitted") } + + // NIT 4 — escape paths for STRING default: backslash and dollar sign + + @Test + fun `STRING default with backslash is escaped`() { + // Producer stores bare: path\to\file — generator must emit: "path\\to\\file" + val source = + GeneratedFeaturedRegistryGenerator.generate( + manifests = + listOf(manifest(":app", flag(key = "path_flag", valueType = ValueType.STRING, defaultValue = """path\to\file"""))), + packageName = FEATURED_REGISTRY_PACKAGE, + ) + assertContains(source, """defaultValue = "path\\to\\file"""") + } + + @Test + fun `STRING default with dollar sign is escaped to prevent template interpolation`() { + // Producer stores bare: price $9.99 — generator must emit: "price ${'$'}9.99" + val source = + GeneratedFeaturedRegistryGenerator.generate( + manifests = + listOf(manifest(":app", flag(key = "price_flag", valueType = ValueType.STRING, defaultValue = "price \$9.99"))), + packageName = FEATURED_REGISTRY_PACKAGE, + ) + // The generated source must contain the Kotlin-safe form that prevents interpolation. + assertContains(source, "price \${'\$'}9.99") + } + + @Test + fun `key containing double quote is escaped in generated source`() { + val source = + GeneratedFeaturedRegistryGenerator.generate( + manifests = + listOf(manifest(":app", flag(key = """dark"mode""", valueType = ValueType.BOOLEAN, defaultValue = "false"))), + packageName = FEATURED_REGISTRY_PACKAGE, + ) + // Generated key must have the quote escaped: key = "dark\"mode" + assertContains(source, """key = "dark\"mode"""") + } + + @Test + fun `description containing dollar sign is escaped`() { + val source = + GeneratedFeaturedRegistryGenerator.generate( + manifests = + listOf( + manifest( + ":app", + flag( + key = "promo", + valueType = ValueType.BOOLEAN, + defaultValue = "true", + description = "Price: \$9.99", + ), + ), + ), + packageName = FEATURED_REGISTRY_PACKAGE, + ) + assertContains(source, "Price: \${'\$'}9.99") + } + + // Fix 1 — newline / tab escape in STRING default and description + + @Test + fun `STRING default with newline is escaped to backslash-n`() { + // Producer stores a value with a real newline character; generated source must not contain a raw newline. + val source = + GeneratedFeaturedRegistryGenerator.generate( + manifests = listOf(manifest(":app", flag(key = "multiline", valueType = ValueType.STRING, defaultValue = "line1\nline2"))), + packageName = FEATURED_REGISTRY_PACKAGE, + ) + assertContains(source, """defaultValue = "line1\nline2"""") + assertFalse(source.contains("line1\nline2"), "Raw newline must not appear in the generated source") + } + + @Test + fun `description with newline is escaped to backslash-n`() { + val source = + GeneratedFeaturedRegistryGenerator.generate( + manifests = + listOf( + manifest( + ":app", + flag( + key = "flag", + valueType = ValueType.BOOLEAN, + defaultValue = "false", + description = "first line\nsecond line", + ), + ), + ), + packageName = FEATURED_REGISTRY_PACKAGE, + ) + assertContains(source, """description = "first line\nsecond line"""") + assertFalse(source.contains("first line\nsecond line"), "Raw newline must not appear in description") + } + + @Test + fun `STRING default with tab is escaped to backslash-t`() { + val source = + GeneratedFeaturedRegistryGenerator.generate( + manifests = listOf(manifest(":app", flag(key = "tabbed", valueType = ValueType.STRING, defaultValue = "col1\tcol2"))), + packageName = FEATURED_REGISTRY_PACKAGE, + ) + assertContains(source, """defaultValue = "col1\tcol2"""") + assertFalse(source.contains("col1\tcol2"), "Raw tab must not appear in the generated source") + } + + // NIT 5 — category emit/omit + + @Test + fun `optional category is emitted when non-null`() { + val source = + GeneratedFeaturedRegistryGenerator.generate( + manifests = + listOf( + manifest( + ":app", + flag( + key = "dark_mode", + valueType = ValueType.BOOLEAN, + defaultValue = "false", + category = "UI", + ), + ), + ), + packageName = FEATURED_REGISTRY_PACKAGE, + ) + assertContains(source, "category = \"UI\"") + } } From 272a149793648079c4e31381c19b8b016a1916f0 Mon Sep 17 00:00:00 2001 From: kirich1409 Date: Tue, 19 May 2026 13:21:24 +0300 Subject: [PATCH 3/3] Address review: primitive default injection + enum classpath docs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit cubic #1 and Copilot #3: extend validateFlagDescriptorIntegrity from ENUM-only to an exhaustive when over all seven ValueTypes. Boolean, Int, Long, Float, and Double defaultValue strings are now validated against their respective Kotlin literal grammars before reaching the codegen. A malicious project dependency can no longer smuggle Kotlin through a primitive default like "0.also { ... }" for an Int flag or "false; init { ... }; private val x = true" for a Boolean. STRING is the only remaining unchecked branch — already neutralised by escapeKotlinString. Copilot #4: document that featuredAggregation(project(...)) resolves only the featured-manifest variant, not the producer's main runtime. Modules with enum flags additionally require implementation(project) in the consumer for the enum class to be on the compile classpath. Plugin KDoc and the CHANGELOG entry both call this out. Copilot #5: fix GeneratedFeaturedRegistryGenerator KDoc that claimed to import enum FQNs. The generator imports only ConfigParam and references enum types inline by FQN; KDoc now matches. Tests: 10 new primitive-literal cases — true/false pass, injection shapes throw, negative ints pass, Long max pass, scientific-notation double pass, brace/method-call shapes throw. :featured-gradle-plugin:check — BUILD SUCCESSFUL, 0 failures. Co-Authored-By: Claude Opus 4.7 --- CHANGELOG.md | 2 +- .../gradle/FeaturedApplicationPlugin.kt | 14 +++ .../GenerateFeaturedRegistryTask.kt | 89 +++++++++++---- .../GeneratedFeaturedRegistryGenerator.kt | 9 +- ...turedAggregationDescriptorIntegrityTest.kt | 103 ++++++++++++++++++ 5 files changed, 192 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5973379..7fe9f6c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - Featured library plugin now publishes a per-module feature-flag manifest as a consumable Gradle artifact (`featuredManifest` configuration, schema v1). Existing flag-generation pipeline is unchanged. Consumer-side aggregation arrives in a follow-up release. -- New `dev.androidbroadcast.featured.application` Gradle plugin: aggregates `featured-manifest.json` artifacts from project dependencies declared via `featuredAggregation(project(...))` and generates `object GeneratedFeaturedRegistry { val all: List> }` in `build/generated/featured/commonMain/`. Apply alongside `dev.androidbroadcast.featured` in the application module; wire the output directory into your source set manually (e.g., `kotlin.sourceSets.commonMain.kotlin.srcDir(...)`). +- New `dev.androidbroadcast.featured.application` Gradle plugin: aggregates `featured-manifest.json` artifacts from project dependencies declared via `featuredAggregation(project(...))` and generates `object GeneratedFeaturedRegistry { val all: List> }` in `build/generated/featured/commonMain/`. Apply alongside `dev.androidbroadcast.featured` in the application module; wire the output directory into your source set manually (e.g., `kotlin.sourceSets.commonMain.kotlin.srcDir(...)`). Modules declaring `enum` flags also require a regular `implementation(project(...))` dependency in the consumer so the enum class is on the compile classpath; primitive-only modules need only `featuredAggregation(...)`. ## [1.0.0-Beta1] - 2026-05-17 diff --git a/featured-gradle-plugin/src/main/kotlin/dev/androidbroadcast/featured/gradle/FeaturedApplicationPlugin.kt b/featured-gradle-plugin/src/main/kotlin/dev/androidbroadcast/featured/gradle/FeaturedApplicationPlugin.kt index a70770f..8b17388 100644 --- a/featured-gradle-plugin/src/main/kotlin/dev/androidbroadcast/featured/gradle/FeaturedApplicationPlugin.kt +++ b/featured-gradle-plugin/src/main/kotlin/dev/androidbroadcast/featured/gradle/FeaturedApplicationPlugin.kt @@ -44,6 +44,20 @@ import org.gradle.api.attributes.Usage * ) * ``` * + * **Enum flag classpath requirement.** A `featuredAggregation(project(":feature:foo"))` dependency + * resolves only the `featured-manifest` Gradle variant — it does NOT put the producer's enum types + * on the consumer's compile classpath. If `:feature:foo` declares an `enum` flag whose type lives + * in `:feature:foo`'s source set, the application module must add a regular runtime dependency on + * the same module so the enum class is visible at compile time: + * ```kotlin + * dependencies { + * featuredAggregation(project(":feature:foo")) + * implementation(project(":feature:foo")) // required for enum flag types + * } + * ``` + * For modules that declare only primitive flags (Boolean / Int / Long / Float / Double / String), + * the `featuredAggregation` line alone is sufficient. + * * Min Gradle version: 8.5+ (`configurations.dependencyScope()` / `.resolvable()` API). */ @Suppress("UnstableApiUsage") diff --git a/featured-gradle-plugin/src/main/kotlin/dev/androidbroadcast/featured/gradle/aggregation/GenerateFeaturedRegistryTask.kt b/featured-gradle-plugin/src/main/kotlin/dev/androidbroadcast/featured/gradle/aggregation/GenerateFeaturedRegistryTask.kt index 3dceada..87acb53 100644 --- a/featured-gradle-plugin/src/main/kotlin/dev/androidbroadcast/featured/gradle/aggregation/GenerateFeaturedRegistryTask.kt +++ b/featured-gradle-plugin/src/main/kotlin/dev/androidbroadcast/featured/gradle/aggregation/GenerateFeaturedRegistryTask.kt @@ -24,6 +24,16 @@ private val PACKAGE_NAME_REGEX = Regex("[a-zA-Z][a-zA-Z0-9_]*(\\.[a-zA-Z][a-zA-Z private val KOTLIN_FQN_REGEX = Regex("[A-Za-z_][A-Za-z0-9_]*(\\.[A-Za-z_][A-Za-z0-9_]*)*") private val KOTLIN_IDENTIFIER_REGEX = Regex("[A-Za-z_][A-Za-z0-9_]*") +// Accepted grammars for primitive defaultValue fields interpolated verbatim into the generated +// Kotlin source. Malicious manifests can embed arbitrary Kotlin by supplying e.g. an INT value +// that contains method-call suffixes or a BOOLEAN value with an extra statement appended. +// Each regex matches only the literal forms that Kotlin accepts for the respective numeric type. +private val BOOLEAN_LITERAL_REGEX = Regex("true|false") +private val INT_LITERAL_REGEX = Regex("-?\\d+") +private val LONG_LITERAL_REGEX = Regex("-?\\d+") +private val FLOAT_LITERAL_REGEX = Regex("-?\\d+(\\.\\d+)?([eE]-?\\d+)?") +private val DOUBLE_LITERAL_REGEX = Regex("-?\\d+(\\.\\d+)?([eE]-?\\d+)?") + /** * Aggregates `featured-manifest.json` files from all project dependencies declared via * `featuredAggregation(...)` and generates `GeneratedFeaturedRegistry.kt`. @@ -145,38 +155,79 @@ internal fun validateUniqueKeys(manifests: List) { } /** - * Validates the integrity of ENUM flag descriptors in [manifests] against Kotlin grammar before - * passing them to the code generator. + * Validates the integrity of all flag descriptors in [manifests] against Kotlin literal grammars + * before passing them to the code generator. * * Threat model: a malicious build-script author of a project dependency declared via * `featuredAggregation(project(":evil"))` controls the contents of `featured-manifest.json` - * and can supply arbitrary strings for `enumTypeFqn` and `defaultValue`. Both fields are - * interpolated verbatim into the generated `.kt` file as Kotlin identifiers (not string - * literals), so injecting `;`, `{`, `(`, or similar characters produces syntactically valid - * Kotlin with arbitrary code that executes during the consuming project's `:compileKotlin`. + * and can supply arbitrary strings for `enumTypeFqn` and `defaultValue`. These fields are + * interpolated verbatim into the generated `.kt` file, so injecting `;`, `{`, `(`, or similar + * characters produces syntactically valid Kotlin with arbitrary code that executes during the + * consuming project's `:compileKotlin`. * * We validate against Kotlin grammar here — single source of truth in the task — so the * generator can never emit unintended syntax regardless of what arrives in the manifest. * - * @throws IllegalArgumentException when any ENUM flag has an invalid [enumTypeFqn] or - * [defaultValue], naming the offending key and module in the message. + * @throws IllegalArgumentException when any flag has an invalid [defaultValue] (or, for ENUM, + * an invalid [enumTypeFqn]), naming the offending key and module in the message. */ internal fun validateFlagDescriptorIntegrity(manifests: List) { manifests.forEach { manifest -> - manifest.flags - .filter { it.valueType == ValueType.ENUM } - .forEach { flag -> - requireNotNull(flag.enumTypeFqn) { - "enumTypeFqn must not be null for ENUM flag '${flag.key}' in module '${manifest.modulePath}'." + manifest.flags.forEach { flag -> + when (flag.valueType) { + ValueType.BOOLEAN -> { + require(BOOLEAN_LITERAL_REGEX.matches(flag.defaultValue)) { + "Invalid Boolean defaultValue '${flag.defaultValue}' for flag '${flag.key}' " + + "in module '${manifest.modulePath}': must be 'true' or 'false'." + } + } + + ValueType.INT -> { + require(INT_LITERAL_REGEX.matches(flag.defaultValue)) { + "Invalid Int defaultValue '${flag.defaultValue}' for flag '${flag.key}' " + + "in module '${manifest.modulePath}': must be an integer literal (digits, optional leading minus)." + } + } + + ValueType.LONG -> { + require(LONG_LITERAL_REGEX.matches(flag.defaultValue)) { + "Invalid Long defaultValue '${flag.defaultValue}' for flag '${flag.key}' " + + "in module '${manifest.modulePath}': must be an integer literal (digits, optional leading minus)." + } + } + + ValueType.FLOAT -> { + require(FLOAT_LITERAL_REGEX.matches(flag.defaultValue)) { + "Invalid Float defaultValue '${flag.defaultValue}' for flag '${flag.key}' " + + "in module '${manifest.modulePath}': must be a numeric literal (digits, optional decimal and exponent)." + } } - require(KOTLIN_FQN_REGEX.matches(flag.enumTypeFqn)) { - "Invalid enumTypeFqn '${flag.enumTypeFqn}' for flag '${flag.key}' in module '${manifest.modulePath}': " + - "must be a valid Kotlin fully-qualified name." + + ValueType.DOUBLE -> { + require(DOUBLE_LITERAL_REGEX.matches(flag.defaultValue)) { + "Invalid Double defaultValue '${flag.defaultValue}' for flag '${flag.key}' " + + "in module '${manifest.modulePath}': must be a numeric literal (digits, optional decimal and exponent)." + } + } + + ValueType.STRING -> { + // STRING values are escaped via escapeKotlinString in the generator — no validation needed. } - require(KOTLIN_IDENTIFIER_REGEX.matches(flag.defaultValue)) { - "Invalid ENUM defaultValue '${flag.defaultValue}' for flag '${flag.key}' in module '${manifest.modulePath}': " + - "must be a valid Kotlin identifier." + + ValueType.ENUM -> { + requireNotNull(flag.enumTypeFqn) { + "enumTypeFqn must not be null for ENUM flag '${flag.key}' in module '${manifest.modulePath}'." + } + require(KOTLIN_FQN_REGEX.matches(flag.enumTypeFqn)) { + "Invalid enumTypeFqn '${flag.enumTypeFqn}' for flag '${flag.key}' in module '${manifest.modulePath}': " + + "must be a valid Kotlin fully-qualified name." + } + require(KOTLIN_IDENTIFIER_REGEX.matches(flag.defaultValue)) { + "Invalid ENUM defaultValue '${flag.defaultValue}' for flag '${flag.key}' in module '${manifest.modulePath}': " + + "must be a valid Kotlin identifier." + } } } + } } } diff --git a/featured-gradle-plugin/src/main/kotlin/dev/androidbroadcast/featured/gradle/aggregation/GeneratedFeaturedRegistryGenerator.kt b/featured-gradle-plugin/src/main/kotlin/dev/androidbroadcast/featured/gradle/aggregation/GeneratedFeaturedRegistryGenerator.kt index c9ba018..b23888f 100644 --- a/featured-gradle-plugin/src/main/kotlin/dev/androidbroadcast/featured/gradle/aggregation/GeneratedFeaturedRegistryGenerator.kt +++ b/featured-gradle-plugin/src/main/kotlin/dev/androidbroadcast/featured/gradle/aggregation/GeneratedFeaturedRegistryGenerator.kt @@ -5,12 +5,11 @@ import dev.androidbroadcast.featured.gradle.manifest.FlagDescriptor import dev.androidbroadcast.featured.gradle.manifest.ValueType /** - * Generates `GeneratedFeaturedRegistry.kt` source from aggregated [FeaturedManifest] list. + * Generates `GeneratedFeaturedRegistry.kt` source from the aggregated [FeaturedManifest] list. * - * KMP-safe: imports only `dev.androidbroadcast.featured.ConfigParam` and enum FQNs - * (one import per distinct enum). All type arguments and default values for ENUM flags use - * the fully-qualified class name inline so the generated file compiles without requiring - * the enum types on the plugin's own classpath. + * KMP-safe: imports only `dev.androidbroadcast.featured.ConfigParam`. Enum types are referenced + * inline by their fully-qualified name in both the `ConfigParam<...>` type argument and the + * `defaultValue = ...` expression — no separate enum imports are emitted. * * Output determinism: descriptors are sorted by `(modulePath, key)` over the flattened * list before generation, so the output is identical regardless of the order in which diff --git a/featured-gradle-plugin/src/test/kotlin/dev/androidbroadcast/featured/gradle/aggregation/FeaturedAggregationDescriptorIntegrityTest.kt b/featured-gradle-plugin/src/test/kotlin/dev/androidbroadcast/featured/gradle/aggregation/FeaturedAggregationDescriptorIntegrityTest.kt index 670b627..c43c0d0 100644 --- a/featured-gradle-plugin/src/test/kotlin/dev/androidbroadcast/featured/gradle/aggregation/FeaturedAggregationDescriptorIntegrityTest.kt +++ b/featured-gradle-plugin/src/test/kotlin/dev/androidbroadcast/featured/gradle/aggregation/FeaturedAggregationDescriptorIntegrityTest.kt @@ -23,6 +23,19 @@ class FeaturedAggregationDescriptorIntegrityTest { enumTypeFqn = enumTypeFqn, ) + private fun primitiveFlag( + key: String = "some_flag", + valueType: ValueType, + defaultValue: String, + ) = FlagDescriptor( + key = key, + propertyName = key, + kind = FlagKind.LOCAL, + valueType = valueType, + defaultValue = defaultValue, + enumTypeFqn = null, + ) + private fun singleManifest(flag: FlagDescriptor) = listOf( FeaturedManifest( @@ -111,4 +124,94 @@ class FeaturedAggregationDescriptorIntegrityTest { assertContains(msg, "checkout_variant", message = "Message must name the flag key") assertContains(msg, ":feature-a", message = "Message must name the module path") } + + // --- Primitive defaultValue validation tests --- + + @Test + fun `BOOLEAN defaultValue 'true' does not throw`() { + validateFlagDescriptorIntegrity(singleManifest(primitiveFlag(valueType = ValueType.BOOLEAN, defaultValue = "true"))) + } + + @Test + fun `BOOLEAN defaultValue 'false' does not throw`() { + validateFlagDescriptorIntegrity(singleManifest(primitiveFlag(valueType = ValueType.BOOLEAN, defaultValue = "false"))) + } + + @Test + fun `BOOLEAN defaultValue with appended statement throws naming key and module`() { + // Simulates injection of an extra statement appended to the boolean literal. + val ex = + assertFailsWith { + validateFlagDescriptorIntegrity( + singleManifest(primitiveFlag(key = "some_flag", valueType = ValueType.BOOLEAN, defaultValue = "true; init { evil() }")), + ) + } + val msg = ex.message ?: "" + assertContains(msg, "some_flag", message = "Message must name the flag key") + assertContains(msg, ":feature-a", message = "Message must name the module path") + } + + @Test + fun `INT defaultValue with method-call suffix throws`() { + // The exact attack vector from the security review: 0.also { ... } is a valid Kotlin expression + // but must not be emitted verbatim as a ConfigParam defaultValue literal. + val ex = + assertFailsWith { + validateFlagDescriptorIntegrity( + singleManifest(primitiveFlag(key = "some_flag", valueType = ValueType.INT, defaultValue = "0.also { injectCode() }")), + ) + } + val msg = ex.message ?: "" + assertContains(msg, "some_flag", message = "Message must name the flag key") + assertContains(msg, ":feature-a", message = "Message must name the module path") + } + + @Test + fun `INT defaultValue '-42' does not throw`() { + // Negative integers are valid and must be allowed. + validateFlagDescriptorIntegrity(singleManifest(primitiveFlag(valueType = ValueType.INT, defaultValue = "-42"))) + } + + @Test + fun `LONG defaultValue max signed 64-bit value does not throw`() { + validateFlagDescriptorIntegrity( + singleManifest(primitiveFlag(valueType = ValueType.LONG, defaultValue = "9223372036854775807")), + ) + } + + @Test + fun `FLOAT defaultValue '3_14' does not throw`() { + validateFlagDescriptorIntegrity(singleManifest(primitiveFlag(valueType = ValueType.FLOAT, defaultValue = "3.14"))) + } + + @Test + fun `FLOAT defaultValue with non-numeric prefix throws`() { + val ex = + assertFailsWith { + validateFlagDescriptorIntegrity( + singleManifest(primitiveFlag(key = "some_flag", valueType = ValueType.FLOAT, defaultValue = "NaN; injectCode()")), + ) + } + val msg = ex.message ?: "" + assertContains(msg, "some_flag", message = "Message must name the flag key") + assertContains(msg, ":feature-a", message = "Message must name the module path") + } + + @Test + fun `DOUBLE defaultValue scientific notation does not throw`() { + validateFlagDescriptorIntegrity(singleManifest(primitiveFlag(valueType = ValueType.DOUBLE, defaultValue = "1.5e10"))) + } + + @Test + fun `DOUBLE defaultValue with brace injection throws`() { + val ex = + assertFailsWith { + validateFlagDescriptorIntegrity( + singleManifest(primitiveFlag(key = "some_flag", valueType = ValueType.DOUBLE, defaultValue = "1.5} init { evil() }")), + ) + } + val msg = ex.message ?: "" + assertContains(msg, "some_flag", message = "Message must name the flag key") + assertContains(msg, ":feature-a", message = "Message must name the module path") + } }