From 70b16db3805d1fcd16c6f5ded9ca3e1fc8172ac5 Mon Sep 17 00:00:00 2001 From: Matteo Mekhail <67237370+matteoiscrying@users.noreply.github.com> Date: Fri, 14 Aug 2026 20:41:29 +1000 Subject: [PATCH 01/16] Align plugin settings with capability parameters --- .../org/wip/complete/CompleteExamplePlugin.kt | 4 +- .../plugin/ui/PluginSettingsContent.kt | 5 ++- docs/PluginDevelopment.md | 5 ++- .../wip/plugintoolkit/api/ManifestModels.kt | 3 +- .../api/annotations/Annotations.kt | 8 +++- .../plugintoolkit/api/ManifestModelsTest.kt | 20 +++++++++ .../api/processor/ManifestJsonGenerator.kt | 8 ++++ .../processor/generators/ManifestGenerator.kt | 43 ++++++++++++++++++- 8 files changed, 88 insertions(+), 8 deletions(-) diff --git a/completeExample/src/main/kotlin/org/wip/complete/CompleteExamplePlugin.kt b/completeExample/src/main/kotlin/org/wip/complete/CompleteExamplePlugin.kt index 3bb804ac..fcb26363 100644 --- a/completeExample/src/main/kotlin/org/wip/complete/CompleteExamplePlugin.kt +++ b/completeExample/src/main/kotlin/org/wip/complete/CompleteExamplePlugin.kt @@ -39,7 +39,9 @@ import java.io.File data class CompleteExampleSettings( @PluginSetting( description = "Public configuration value example", - defaultValue = "default_api_key" + defaultValue = "default_api_key", + minLength = 8, + semanticTypes = ["text/plain"] ) val apiKey: String? = "default_api_key", @PluginSetting( diff --git a/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/ui/PluginSettingsContent.kt b/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/ui/PluginSettingsContent.kt index a05b5883..334451c2 100644 --- a/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/ui/PluginSettingsContent.kt +++ b/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/ui/PluginSettingsContent.kt @@ -483,8 +483,11 @@ fun PluginSettingsContent( description = meta.description, type = meta.type, defaultValue = meta.defaultValue, + constraints = meta.constraints, required = meta.required, - secret = meta.secret + secret = meta.secret, + semanticTypes = meta.semanticTypes, + autogeneratedPattern = meta.autogeneratedPattern ), value = SettingsUtils.jsonToString(value, meta.type), onValueChange = { diff --git a/docs/PluginDevelopment.md b/docs/PluginDevelopment.md index d69b5025..d08fb3ed 100644 --- a/docs/PluginDevelopment.md +++ b/docs/PluginDevelopment.md @@ -37,7 +37,8 @@ The `@PluginSetting` annotation supports identical validation constraints to tho data class MyAdvancedSettings( @PluginSetting( description = "Service Endpoint", - regex = "^https?://.*" + regex = "^https?://.*", + semanticTypes = ["text/uri"] ) val endpoint: String, @PluginSetting( @@ -48,6 +49,8 @@ data class MyAdvancedSettings( ) ``` +Settings also accept `semanticTypes` and `pathTemplate`, matching capability parameters. Semantic types select specialized controls such as color or file inputs; a path template derives a value from other configured fields. + ### 2. Capabilities A plugin provides one or more **Capabilities**. These are functions annotated with `@Capability`. Each capability becomes a task that the host application can execute. diff --git a/plugin-api/src/commonMain/kotlin/org/wip/plugintoolkit/api/ManifestModels.kt b/plugin-api/src/commonMain/kotlin/org/wip/plugintoolkit/api/ManifestModels.kt index d82c0c3f..0da84ce9 100644 --- a/plugin-api/src/commonMain/kotlin/org/wip/plugintoolkit/api/ManifestModels.kt +++ b/plugin-api/src/commonMain/kotlin/org/wip/plugintoolkit/api/ManifestModels.kt @@ -246,6 +246,8 @@ data class SettingMetadata( val required: Boolean = false, val secret: Boolean = false, val constraints: ParameterConstraints? = null, + val semanticTypes: List = emptyList(), + val autogeneratedPattern: String? = null, /** * List of capability names that require this setting. * This allows UI to show which capabilities are locked behind this setting @@ -632,4 +634,3 @@ data class PluginAction( val functionName: String, val parameters: Map? = null ) - diff --git a/plugin-api/src/commonMain/kotlin/org/wip/plugintoolkit/api/annotations/Annotations.kt b/plugin-api/src/commonMain/kotlin/org/wip/plugintoolkit/api/annotations/Annotations.kt index b82f97a5..77edcbdf 100644 --- a/plugin-api/src/commonMain/kotlin/org/wip/plugintoolkit/api/annotations/Annotations.kt +++ b/plugin-api/src/commonMain/kotlin/org/wip/plugintoolkit/api/annotations/Annotations.kt @@ -169,6 +169,8 @@ annotation class CapabilityOutput( * @property defaultValue The default value for the setting (as a string). * @property required Whether the setting is mandatory for the plugin to function. * @property secret Whether the setting contains sensitive information (e.g., API keys). + * @property semanticTypes Semantic hints used to select a specialized editor (for example `color/rgb`). + * @property pathTemplate Optional template used to derive this setting from other values. */ @Target(AnnotationTarget.PROPERTY) @Retention(AnnotationRetention.SOURCE) @@ -184,7 +186,9 @@ annotation class PluginSetting( val regex: String = "", val multiSelect: Boolean = false, val minChoices: Int = -1, - val maxChoices: Int = -1 + val maxChoices: Int = -1, + val semanticTypes: Array = [], + val pathTemplate: String = "" ) /** @@ -271,4 +275,4 @@ annotation class ComplexObject( val id: String = "", val description: String = "", val version: Int = 1 -) \ No newline at end of file +) diff --git a/plugin-api/src/commonTest/kotlin/org/wip/plugintoolkit/api/ManifestModelsTest.kt b/plugin-api/src/commonTest/kotlin/org/wip/plugintoolkit/api/ManifestModelsTest.kt index a7e8ba13..f3930871 100644 --- a/plugin-api/src/commonTest/kotlin/org/wip/plugintoolkit/api/ManifestModelsTest.kt +++ b/plugin-api/src/commonTest/kotlin/org/wip/plugintoolkit/api/ManifestModelsTest.kt @@ -1,10 +1,30 @@ package org.wip.plugintoolkit.api import kotlinx.serialization.json.Json +import kotlinx.serialization.json.JsonPrimitive import kotlin.test.Test import kotlin.test.assertEquals class ManifestModelsTest { + @Test + fun `setting metadata preserves capability-style input hints`() { + val metadata = SettingMetadata( + defaultValue = JsonPrimitive("#112233"), + description = "Brand color", + type = DataType.Primitive(PrimitiveType.STRING), + constraints = ParameterConstraints(regex = "#[0-9A-Fa-f]{6}"), + semanticTypes = parseSemanticTypes("color/rgb"), + autogeneratedPattern = "{theme}/brand.hex" + ) + val json = Json { encodeDefaults = true } + + val decoded = json.decodeFromString(json.encodeToString(metadata)) + + assertEquals(metadata.constraints, decoded.constraints) + assertEquals(metadata.semanticTypes, decoded.semanticTypes) + assertEquals(metadata.autogeneratedPattern, decoded.autogeneratedPattern) + } + @Test fun testCapabilityDeserialization() { val jsonString = """{ diff --git a/plugin-api/src/jvmMain/kotlin/org/wip/plugintoolkit/api/processor/ManifestJsonGenerator.kt b/plugin-api/src/jvmMain/kotlin/org/wip/plugintoolkit/api/processor/ManifestJsonGenerator.kt index 06505316..7334907d 100644 --- a/plugin-api/src/jvmMain/kotlin/org/wip/plugintoolkit/api/processor/ManifestJsonGenerator.kt +++ b/plugin-api/src/jvmMain/kotlin/org/wip/plugintoolkit/api/processor/ManifestJsonGenerator.kt @@ -279,6 +279,12 @@ object ManifestJsonGenerator { val multiSelect = ann.arguments.find { it.name?.asString() == "multiSelect" }?.value as? Boolean ?: false val minChoices = ann.arguments.find { it.name?.asString() == "minChoices" }?.value as? Int ?: -1 val maxChoices = ann.arguments.find { it.name?.asString() == "maxChoices" }?.value as? Int ?: -1 + val semanticTypes = + (ann.arguments.find { it.name?.asString() == "semanticTypes" }?.value as? List<*>) + ?.filterIsInstance() + ?.flatMap { parseSemanticTypes(it) } + ?: emptyList() + val pathTemplate = ann.arguments.find { it.name?.asString() == "pathTemplate" }?.value as? String ?: "" val hasConstraints = !minValue.isNaN() || !maxValue.isNaN() || minLength != -1 || maxLength != -1 || regex.isNotEmpty() || multiSelect || minChoices != -1 || maxChoices != -1 @@ -303,6 +309,8 @@ object ManifestJsonGenerator { required = required, secret = secret, constraints = constraints, + semanticTypes = semanticTypes, + autogeneratedPattern = pathTemplate.ifBlank { null }, requiredByCapabilities = requiredBy ) } diff --git a/plugin-api/src/jvmMain/kotlin/org/wip/plugintoolkit/api/processor/generators/ManifestGenerator.kt b/plugin-api/src/jvmMain/kotlin/org/wip/plugintoolkit/api/processor/generators/ManifestGenerator.kt index ff0682ea..6668ef16 100644 --- a/plugin-api/src/jvmMain/kotlin/org/wip/plugintoolkit/api/processor/generators/ManifestGenerator.kt +++ b/plugin-api/src/jvmMain/kotlin/org/wip/plugintoolkit/api/processor/generators/ManifestGenerator.kt @@ -355,6 +355,19 @@ object ManifestGenerator { val secret = ann.arguments.find { it.name?.asString() == "secret" }?.value as? Boolean ?: false val propName = prop.simpleName.asString() val propType = prop.type.resolve().toTypeName() + val minValue = ann.arguments.find { it.name?.asString() == "minValue" }?.value as? Double ?: Double.NaN + val maxValue = ann.arguments.find { it.name?.asString() == "maxValue" }?.value as? Double ?: Double.NaN + val minLength = ann.arguments.find { it.name?.asString() == "minLength" }?.value as? Int ?: -1 + val maxLength = ann.arguments.find { it.name?.asString() == "maxLength" }?.value as? Int ?: -1 + val regex = ann.arguments.find { it.name?.asString() == "regex" }?.value as? String ?: "" + val multiSelect = ann.arguments.find { it.name?.asString() == "multiSelect" }?.value as? Boolean ?: false + val minChoices = ann.arguments.find { it.name?.asString() == "minChoices" }?.value as? Int ?: -1 + val maxChoices = ann.arguments.find { it.name?.asString() == "maxChoices" }?.value as? Int ?: -1 + val semanticTypeValues = + (ann.arguments.find { it.name?.asString() == "semanticTypes" }?.value as? List<*>) + ?.filterIsInstance() + ?: emptyList() + val pathTemplate = ann.arguments.find { it.name?.asString() == "pathTemplate" }?.value as? String ?: "" val defaultValueCode = if (defaultVal.isNotEmpty()) { try { kotlinx.serialization.json.Json.parseToJsonElement(defaultVal) @@ -366,17 +379,43 @@ object ManifestGenerator { CodeBlock.of("null") } + val hasConstraints = + !minValue.isNaN() || !maxValue.isNaN() || minLength != -1 || maxLength != -1 || + regex.isNotEmpty() || multiSelect || minChoices != -1 || maxChoices != -1 + val constraintsCode = if (hasConstraints) { + CodeBlock.of( + "%T(minValue = %L, maxValue = %L, minLength = %L, maxLength = %L, regex = %L, multiSelect = %L, minChoices = %L, maxChoices = %L)", + CN_PARAMETER_CONSTRAINTS, + if (!minValue.isNaN()) minValue else "null", + if (!maxValue.isNaN()) maxValue else "null", + if (minLength != -1) minLength else "null", + if (maxLength != -1) maxLength else "null", + if (regex.isNotEmpty()) CodeBlock.of("%S", regex) else CodeBlock.of("null"), + if (multiSelect) "true" else "null", + if (minChoices != -1) minChoices else "null", + if (maxChoices != -1) maxChoices else "null" + ) + } else CodeBlock.of("null") + val semanticTypesCode = generateSemanticTypesCode( + semanticTypeValues.flatMap { org.wip.plugintoolkit.api.parseSemanticTypes(it) } + ) + val autogeneratedPatternCode = + if (pathTemplate.isBlank()) CodeBlock.of("null") else CodeBlock.of("%S", pathTemplate) + settingsCode.add( - "%S to %T(defaultValue = %L, description = %S, type = %M<%T>(), required = %L, secret = %L)", + "%S to %T(defaultValue = %L, description = %S, type = %M<%T>(), constraints = %L, required = %L, secret = %L, semanticTypes = %L, autogeneratedPattern = %L)", propName, CN_SETTING_METADATA, defaultValueCode, desc, MN_GET_DATA_TYPE, propType, + constraintsCode, required, - secret + secret, + semanticTypesCode, + autogeneratedPatternCode ) if (index < settingsProperties.size - 1) settingsCode.add(",\n") else settingsCode.add("\n") } From 485a9f3bb715b56987281180ae1857dcb2a0f196 Mon Sep 17 00:00:00 2001 From: Matteo Mekhail <67237370+matteoiscrying@users.noreply.github.com> Date: Fri, 14 Aug 2026 20:48:15 +1000 Subject: [PATCH 02/16] Add safe directory operations to scoped filesystems --- .../logic/DefaultExecutionFileSystem.kt | 23 +++++++++++++ .../plugin/logic/DefaultPluginFileSystem.kt | 33 +++++++++++++++++++ .../logic/DefaultExecutionFileSystemTest.kt | 18 ++++++++++ docs/PluginDevelopment.md | 1 + .../org/wip/plugintoolkit/api/Interfaces.kt | 4 +++ 5 files changed, 79 insertions(+) diff --git a/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/DefaultExecutionFileSystem.kt b/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/DefaultExecutionFileSystem.kt index fea6db80..3d842b4d 100644 --- a/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/DefaultExecutionFileSystem.kt +++ b/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/DefaultExecutionFileSystem.kt @@ -97,5 +97,28 @@ class DefaultExecutionFileSystem( } } + override suspend fun createDirectory(relativePath: RelativePath): Result = runCatching { + SystemFileSystem.createDirectories(resolvePath(relativePath)) + } + + override suspend fun deleteDirectory(relativePath: RelativePath, recursive: Boolean): Result = runCatching { + require(relativePath != RelativePath.ROOT) { "The execution sandbox root cannot be deleted" } + deleteDirectory(resolvePath(relativePath), recursive) + } + + private fun deleteDirectory(path: Path, recursive: Boolean) { + if (!SystemFileSystem.exists(path)) return + require(SystemFileSystem.metadataOrNull(path)?.isDirectory == true) { "Path is not a directory: $path" } + val children = SystemFileSystem.list(path) + require(recursive || children.isEmpty()) { "Directory is not empty: $path" } + if (recursive) { + children.forEach { child -> + if (SystemFileSystem.metadataOrNull(child)?.isDirectory == true) deleteDirectory(child, true) + else SystemFileSystem.delete(child) + } + } + SystemFileSystem.delete(path) + } + override fun getBasePath(): String = sandboxPath } diff --git a/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/DefaultPluginFileSystem.kt b/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/DefaultPluginFileSystem.kt index e32f5a16..8f9a3997 100644 --- a/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/DefaultPluginFileSystem.kt +++ b/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/DefaultPluginFileSystem.kt @@ -93,6 +93,15 @@ class DefaultPluginFileSystem( } } + override suspend fun createDirectory(relativePath: RelativePath): Result = runCatching { + SystemFileSystem.createDirectories(resolvePath(relativePath)) + } + + override suspend fun deleteDirectory(relativePath: RelativePath, recursive: Boolean): Result = runCatching { + require(relativePath != RelativePath.ROOT) { "The plugin files root cannot be deleted" } + deleteDirectory(resolvePath(relativePath), recursive) + } + override suspend fun extractResource(resourcePath: String, targetRelativePath: RelativePath): Result { if (resourcePath.contains("..") || resourcePath.startsWith("/") || resourcePath.startsWith("\\") || resourcePath.contains("\u0000")) { return Result.failure(SecurityException("Invalid resource path: $resourcePath")) @@ -138,11 +147,35 @@ class DefaultPluginFileSystem( override suspend fun deleteFile(relativePath: RelativePath): Result = fs.deleteFromCache(relativePath) + + override suspend fun createDirectory(relativePath: RelativePath): Result = runCatching { + SystemFileSystem.createDirectories(fs.resolveCachePath(relativePath)) + } + + override suspend fun deleteDirectory(relativePath: RelativePath, recursive: Boolean): Result = + runCatching { + require(relativePath != RelativePath.ROOT) { "The plugin cache root cannot be deleted" } + fs.deleteDirectory(fs.resolveCachePath(relativePath), recursive) + } } } } } + private fun deleteDirectory(path: Path, recursive: Boolean) { + if (!SystemFileSystem.exists(path)) return + require(SystemFileSystem.metadataOrNull(path)?.isDirectory == true) { "Path is not a directory: $path" } + val children = SystemFileSystem.list(path) + require(recursive || children.isEmpty()) { "Directory is not empty: $path" } + if (recursive) { + children.forEach { child -> + if (SystemFileSystem.metadataOrNull(child)?.isDirectory == true) deleteDirectory(child, true) + else SystemFileSystem.delete(child) + } + } + SystemFileSystem.delete(path) + } + private fun resolveCachePath(relativePath: RelativePath): Path { val resolved = Path(cachePath, relativePath.value) val normalized = resolved.toString().replace('\\', '/') diff --git a/composeApp/src/commonTest/kotlin/org/wip/plugintoolkit/features/plugin/logic/DefaultExecutionFileSystemTest.kt b/composeApp/src/commonTest/kotlin/org/wip/plugintoolkit/features/plugin/logic/DefaultExecutionFileSystemTest.kt index 51da2951..406a0b05 100644 --- a/composeApp/src/commonTest/kotlin/org/wip/plugintoolkit/features/plugin/logic/DefaultExecutionFileSystemTest.kt +++ b/composeApp/src/commonTest/kotlin/org/wip/plugintoolkit/features/plugin/logic/DefaultExecutionFileSystemTest.kt @@ -84,6 +84,24 @@ class DefaultExecutionFileSystemTest { assertTrue(files.contains("file2.txt")) } + @Test + fun testCreateAndDeleteDirectory() = runTest { + val directory = RelativePath.from("models/nested").getOrThrow() + assertTrue(fileSystem.createDirectory(directory).isSuccess) + assertTrue(fileSystem.exists(directory)) + + fileSystem.writeTextFile(RelativePath.from("models/nested/model.txt").getOrThrow(), "model") + assertTrue(fileSystem.deleteDirectory(RelativePath.from("models").getOrThrow()).isFailure) + assertTrue(fileSystem.deleteDirectory(RelativePath.from("models").getOrThrow(), recursive = true).isSuccess) + assertFalse(fileSystem.exists(RelativePath.from("models").getOrThrow())) + } + + @Test + fun testCannotDeleteSandboxRoot() = runTest { + assertTrue(fileSystem.deleteDirectory(RelativePath.ROOT, recursive = true).isFailure) + assertTrue(SystemFileSystem.exists(Path(sandboxPath))) + } + @Test fun testPathTraversalPrevention() = runTest { // Attempt to create a path outside the sandbox using ../ diff --git a/docs/PluginDevelopment.md b/docs/PluginDevelopment.md index d69b5025..bfa1153a 100644 --- a/docs/PluginDevelopment.md +++ b/docs/PluginDevelopment.md @@ -317,6 +317,7 @@ While you can set a plugin to "not support cancellation" the host app can force- The `PluginContext` (and focused interfaces like `PluginLogger`, `PluginFileSystem`, `ExecutionFileSystem`, `HostFileSystem`) provide access to host services: - **Logger**: `PluginLogger` (e.g. `logger.info("Message")`) - **Plugin File System**: `PluginFileSystem` (Persistent, isolated storage for the plugin. Preserved across executions. e.g. `fileSystem.getBasePath()`) +- **Directory operations**: scoped file systems support `createDirectory` and guarded `deleteDirectory`; recursive deletion must be requested explicitly and the sandbox root can never be deleted. - **Execution File System**: `ExecutionFileSystem` (Temporary, isolated sandbox storage for the current execution. Cleared automatically after the flow finishes.) - **Host File System**: `HostFileSystem` (External file access. Restricted to paths explicitly granted by the user via file input/output parameters: `@CapabilityInput` and `@CapabilityOutput`.) - **Plugin Storage**: `PluginStorage` (`context.storage`) provides a persistent, internal key-value store (`get`, `put`, `getAll`, `remove`) for saving plugin-internal state without polluting user settings. diff --git a/plugin-api/src/commonMain/kotlin/org/wip/plugintoolkit/api/Interfaces.kt b/plugin-api/src/commonMain/kotlin/org/wip/plugintoolkit/api/Interfaces.kt index 5b4702b9..3b86f7c0 100644 --- a/plugin-api/src/commonMain/kotlin/org/wip/plugintoolkit/api/Interfaces.kt +++ b/plugin-api/src/commonMain/kotlin/org/wip/plugintoolkit/api/Interfaces.kt @@ -102,6 +102,10 @@ interface ScopedFileSystem { suspend fun exists(relativePath: RelativePath): Boolean suspend fun listFiles(relativePath: RelativePath = RelativePath.ROOT): List suspend fun deleteFile(relativePath: RelativePath): Result + suspend fun createDirectory(relativePath: RelativePath): Result = + Result.failure(UnsupportedOperationException("Directory creation is not supported by this host")) + suspend fun deleteDirectory(relativePath: RelativePath, recursive: Boolean = false): Result = + Result.failure(UnsupportedOperationException("Directory deletion is not supported by this host")) /** * Get the absolute base path of the managed file area. From e3535b4b13278e888e4a8ad2eb6deaed8315545b Mon Sep 17 00:00:00 2001 From: Matteo Mekhail <67237370+matteoiscrying@users.noreply.github.com> Date: Fri, 14 Aug 2026 20:52:11 +1000 Subject: [PATCH 03/16] Resolve custom plugin defaults consistently --- .../plugin/logic/PluginLifecycleManager.kt | 11 +--- .../plugin/model/PluginSettingsStore.kt | 20 +++++++ .../features/plugin/ui/PluginContent.kt | 11 +--- .../plugin/ui/PluginSettingsContent.kt | 12 ++--- .../plugin/model/PluginSettingDefaultsTest.kt | 53 +++++++++++++++++++ 5 files changed, 80 insertions(+), 27 deletions(-) create mode 100644 composeApp/src/commonTest/kotlin/org/wip/plugintoolkit/features/plugin/model/PluginSettingDefaultsTest.kt diff --git a/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/PluginLifecycleManager.kt b/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/PluginLifecycleManager.kt index 6119f11e..ca55a892 100644 --- a/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/PluginLifecycleManager.kt +++ b/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/PluginLifecycleManager.kt @@ -21,6 +21,7 @@ import org.wip.plugintoolkit.core.utils.FileSystem import org.wip.plugintoolkit.features.job.logic.JobManager import org.wip.plugintoolkit.features.job.model.JobStatus import org.wip.plugintoolkit.features.plugin.model.PluginSettingsStore +import org.wip.plugintoolkit.features.plugin.model.resolveCustomSettings import org.wip.plugintoolkit.features.settings.logic.SettingsRepository import org.wip.plugintoolkit.features.settings.model.PluginUnplugBehavior import org.wip.plugintoolkit.features.plugin.utils.PluginCompatibilityUtils @@ -332,15 +333,7 @@ class PluginLifecycleManager( val storedSettings = overriddenSettings ?: loadPluginSettings(pkg) val actualManifest = manifest ?: getManifest(pkg) - val mergedSettings = mutableMapOf() - - // 1. Manifest defaults - actualManifest?.settings?.forEach { (key, meta) -> - meta.defaultValue?.let { mergedSettings[key] = it } - } - - // 2. User overrides - mergedSettings.putAll(storedSettings.settings) + val mergedSettings = storedSettings.resolveCustomSettings(actualManifest) val pluginLogger = jobManager.getPluginLogger(pkg, jobId) val progressReporter = object : ProgressReporter { diff --git a/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/model/PluginSettingsStore.kt b/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/model/PluginSettingsStore.kt index 472b0a24..a2418e3e 100644 --- a/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/model/PluginSettingsStore.kt +++ b/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/model/PluginSettingsStore.kt @@ -2,6 +2,10 @@ package org.wip.plugintoolkit.features.plugin.model import kotlinx.serialization.Serializable import kotlinx.serialization.json.JsonElement +import kotlinx.serialization.json.JsonPrimitive +import org.wip.plugintoolkit.api.DataType +import org.wip.plugintoolkit.api.PluginManifest +import org.wip.plugintoolkit.api.PrimitiveType @Serializable data class PluginSettingsStore( @@ -9,3 +13,19 @@ data class PluginSettingsStore( val globalParams: Map = emptyMap(), val capabilityParams: Map> = emptyMap() ) + +fun PluginManifest.defaultCustomSettings(): Map = settings.orEmpty().mapNotNull { (key, metadata) -> + val type = metadata.type + val value = metadata.defaultValue ?: if ( + type is DataType.Primitive && type.primitiveType == PrimitiveType.BOOLEAN + ) JsonPrimitive(false) else null + value?.let { key to it } +}.toMap() + +/** Manifest defaults with persisted user values taking precedence. */ +fun PluginSettingsStore.resolveCustomSettings(manifest: PluginManifest?): Map = + (manifest?.defaultCustomSettings() ?: emptyMap()) + settings + +/** Values available to generated inputs and lock evaluation in the settings UI. */ +fun PluginSettingsStore.resolveProvidedValues(manifest: PluginManifest?): Map = + resolveCustomSettings(manifest) + globalParams diff --git a/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/ui/PluginContent.kt b/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/ui/PluginContent.kt index 62632836..a68aa328 100644 --- a/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/ui/PluginContent.kt +++ b/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/ui/PluginContent.kt @@ -40,15 +40,13 @@ import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.text.font.FontWeight import androidx.compose.ui.unit.dp -import kotlinx.serialization.json.JsonPrimitive -import org.wip.plugintoolkit.api.DataType -import org.wip.plugintoolkit.api.PrimitiveType import org.jetbrains.compose.resources.stringResource import org.wip.plugintoolkit.api.Capability import org.wip.plugintoolkit.api.ParameterRole import org.wip.plugintoolkit.api.PluginManifest import org.wip.plugintoolkit.features.job.model.BackgroundJob import org.wip.plugintoolkit.features.job.model.JobStatus +import org.wip.plugintoolkit.features.plugin.model.resolveProvidedValues import org.wip.plugintoolkit.features.plugin.viewmodel.PluginViewModel import org.wip.plugintoolkit.shared.components.plugin.JobResultCard import plugintoolkit.composeapp.generated.resources.Res @@ -121,12 +119,7 @@ fun PluginContent( val providedSettings = remember(pluginId, pluginSettingsState) { val store = if (pluginId != null) pluginSettingsState[pluginId] ?: pluginManager.loadPluginSettings(pluginId) else null val manifest = viewModel.selectedPlugin?.getManifest()?.getOrNull() - val manifestDefaults = (manifest?.settings?.mapValues { (_, meta) -> - meta.defaultValue ?: if (meta.type is DataType.Primitive && (meta.type as DataType.Primitive).primitiveType == PrimitiveType.BOOLEAN) { - JsonPrimitive(false) - } else null - }?.filterValues { it != null } ?: emptyMap()) as Map - manifestDefaults + (store?.settings ?: emptyMap()) + (store?.globalParams ?: emptyMap()) + store?.resolveProvidedValues(manifest) ?: emptyMap() } if (selectedCapability == null) { diff --git a/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/ui/PluginSettingsContent.kt b/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/ui/PluginSettingsContent.kt index a05b5883..d2b98758 100644 --- a/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/ui/PluginSettingsContent.kt +++ b/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/ui/PluginSettingsContent.kt @@ -74,6 +74,7 @@ import org.wip.plugintoolkit.api.PluginAction import org.wip.plugintoolkit.api.PrimitiveType import org.wip.plugintoolkit.core.model.localized import org.wip.plugintoolkit.core.theme.ToolkitTheme +import org.wip.plugintoolkit.features.plugin.model.resolveProvidedValues import org.wip.plugintoolkit.features.plugin.utils.SettingsUtils import org.wip.plugintoolkit.features.plugin.viewmodel.PluginSettingsViewModel import org.wip.plugintoolkit.shared.components.ToolkitChip @@ -413,15 +414,8 @@ fun PluginSettingsContent( ) } } else { - val manifestDefaults = remember(manifest) { - (manifest.settings?.mapValues { (_, meta) -> - meta.defaultValue ?: if (meta.type is DataType.Primitive && (meta.type as DataType.Primitive).primitiveType == PrimitiveType.BOOLEAN) { - JsonPrimitive(false) - } else null - }?.filterValues { it != null } ?: emptyMap()) as Map - } - val providedSettings = remember(manifestDefaults, store.settings) { - manifestDefaults + store.settings + val providedSettings = remember(manifest, store.settings, store.globalParams) { + store.resolveProvidedValues(manifest) } LazyColumn( diff --git a/composeApp/src/commonTest/kotlin/org/wip/plugintoolkit/features/plugin/model/PluginSettingDefaultsTest.kt b/composeApp/src/commonTest/kotlin/org/wip/plugintoolkit/features/plugin/model/PluginSettingDefaultsTest.kt new file mode 100644 index 00000000..5d5cecef --- /dev/null +++ b/composeApp/src/commonTest/kotlin/org/wip/plugintoolkit/features/plugin/model/PluginSettingDefaultsTest.kt @@ -0,0 +1,53 @@ +package org.wip.plugintoolkit.features.plugin.model + +import kotlinx.serialization.json.JsonPrimitive +import org.wip.plugintoolkit.api.DataType +import org.wip.plugintoolkit.api.PluginInfo +import org.wip.plugintoolkit.api.PluginManifest +import org.wip.plugintoolkit.api.PrimitiveType +import org.wip.plugintoolkit.api.Requirements +import org.wip.plugintoolkit.api.SettingMetadata +import kotlin.test.Test +import kotlin.test.assertEquals + +class PluginSettingDefaultsTest { + private val manifest = PluginManifest( + manifestVersion = "1", + plugin = PluginInfo("example", "Example", "1.0", "Example plugin"), + requirements = Requirements(128, 10), + settings = mapOf( + "endpoint" to SettingMetadata( + defaultValue = JsonPrimitive("https://example.test"), + description = "Endpoint", + type = DataType.Primitive(PrimitiveType.STRING) + ), + "enabled" to SettingMetadata( + description = "Enabled", + type = DataType.Primitive(PrimitiveType.BOOLEAN) + ) + ) + ) + + @Test + fun `manifest defaults are available before a user saves settings`() { + val resolved = PluginSettingsStore().resolveCustomSettings(manifest) + + assertEquals(JsonPrimitive("https://example.test"), resolved["endpoint"]) + assertEquals(JsonPrimitive(false), resolved["enabled"]) + } + + @Test + fun `user values override defaults and global values are exposed separately`() { + val store = PluginSettingsStore( + settings = mapOf("endpoint" to JsonPrimitive("https://custom.test")), + globalParams = mapOf("region" to JsonPrimitive("eu")) + ) + + val custom = store.resolveCustomSettings(manifest) + val provided = store.resolveProvidedValues(manifest) + + assertEquals(JsonPrimitive("https://custom.test"), custom["endpoint"]) + assertEquals(JsonPrimitive(false), custom["enabled"]) + assertEquals(JsonPrimitive("eu"), provided["region"]) + } +} From dea9ce8bfb8927a61fd73757a421e8987e58a1a1 Mon Sep 17 00:00:00 2001 From: Matteo Mekhail <67237370+matteoiscrying@users.noreply.github.com> Date: Fri, 14 Aug 2026 20:54:35 +1000 Subject: [PATCH 04/16] Divide required and optional plugin settings --- .../composeResources/values-it/strings.xml | 2 + .../composeResources/values/strings.xml | 4 +- .../plugin/ui/PluginSettingsContent.kt | 163 ++++++++++-------- 3 files changed, 100 insertions(+), 69 deletions(-) diff --git a/composeApp/src/commonMain/composeResources/values-it/strings.xml b/composeApp/src/commonMain/composeResources/values-it/strings.xml index 6c6bccf3..06c715fe 100644 --- a/composeApp/src/commonMain/composeResources/values-it/strings.xml +++ b/composeApp/src/commonMain/composeResources/values-it/strings.xml @@ -1,4 +1,6 @@ + Obbligatorie + Facoltative PluginToolkit Runner Dashboard diff --git a/composeApp/src/commonMain/composeResources/values/strings.xml b/composeApp/src/commonMain/composeResources/values/strings.xml index 48aa407f..69076655 100644 --- a/composeApp/src/commonMain/composeResources/values/strings.xml +++ b/composeApp/src/commonMain/composeResources/values/strings.xml @@ -191,6 +191,8 @@ Actions Custom Settings Global Parameter Defaults + Required + Optional Capability: %1$s Configure required settings to unlock options Locked capability: %1$s @@ -427,4 +429,4 @@ Warning: In-Place Settings Opening settings in-place may cause some components to not update their unlocked states until reloaded. Are you sure you want to enable this mode? By Section - \ No newline at end of file + diff --git a/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/ui/PluginSettingsContent.kt b/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/ui/PluginSettingsContent.kt index a05b5883..398c8017 100644 --- a/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/ui/PluginSettingsContent.kt +++ b/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/ui/PluginSettingsContent.kt @@ -95,6 +95,8 @@ import plugintoolkit.composeapp.generated.resources.plugin_settings_by_section import plugintoolkit.composeapp.generated.resources.plugin_settings_capability import plugintoolkit.composeapp.generated.resources.plugin_settings_custom import plugintoolkit.composeapp.generated.resources.plugin_settings_global_defaults +import plugintoolkit.composeapp.generated.resources.plugin_settings_optional +import plugintoolkit.composeapp.generated.resources.plugin_settings_required import plugintoolkit.composeapp.generated.resources.settings import plugintoolkit.composeapp.generated.resources.settings_locked_capability import plugintoolkit.composeapp.generated.resources.settings_no_results @@ -125,6 +127,8 @@ fun PluginSettingsContent( val actionsTitle = stringResource(Res.string.plugin_settings_actions) val customTitle = stringResource(Res.string.plugin_settings_custom) val globalTitle = stringResource(Res.string.plugin_settings_global_defaults) + val requiredTitle = stringResource(Res.string.plugin_settings_required) + val optionalTitle = stringResource(Res.string.plugin_settings_optional) val capabilityTitles = manifest.capabilities.associate { it.name to stringResource(Res.string.plugin_settings_capability, it.name) @@ -474,75 +478,83 @@ fun PluginSettingsContent( modifier = Modifier.fillMaxWidth().padding(top = ToolkitTheme.spacing.small), verticalArrangement = Arrangement.spacedBy(ToolkitTheme.spacing.mediumSmall) ) { - customSettings.forEach { (key, meta) -> - Column(modifier = Modifier.fillMaxWidth()) { - val value = store.settings[key] ?: meta.defaultValue - DynamicParameterInput( - name = key, - metadata = ParameterMetadata( - description = meta.description, - type = meta.type, - defaultValue = meta.defaultValue, - required = meta.required, - secret = meta.secret - ), - value = SettingsUtils.jsonToString(value, meta.type), - onValueChange = { - viewModel.updateSetting( - key, - SettingsUtils.stringToJson(it, meta.type) - ) - }, - enabled = !isBusy, - providedSettings = providedSettings, - providedLocks = locks - ) - - val lockedOptionsForSetting = lockedEnumOptions[key]?.distinct() ?: emptyList() - - if (meta.requiredByCapabilities.isNotEmpty() || lockedOptionsForSetting.isNotEmpty()) { - Row( - modifier = Modifier - .fillMaxWidth() - .padding( - start = ToolkitTheme.spacing.medium, - bottom = ToolkitTheme.spacing.mediumSmall, - end = ToolkitTheme.spacing.medium - ) - .horizontalScroll(rememberScrollState()), - horizontalArrangement = Arrangement.spacedBy(ToolkitTheme.spacing.small) - ) { - meta.requiredByCapabilities.forEach { capName -> - ToolkitChip( - text = stringResource( - Res.string.settings_locked_capability, - capName - ), - icon = { - Icon( - Icons.Default.Lock, - contentDescription = null, - modifier = Modifier.size(ToolkitTheme.dimensions.iconExtraSmall) - ) - }, - style = ToolkitChipStyle.Tinted - ) - } - if (lockedOptionsForSetting.isNotEmpty()) { - ToolkitChip( - text = "Unlocks Enum Options", - modifier = Modifier.tooltip( - text = "Unlocks values:\n" + lockedOptionsForSetting.joinToString("\n"), - ), - icon = { - Icon( - Icons.Default.Lock, - contentDescription = null, - modifier = Modifier.size(ToolkitTheme.dimensions.iconExtraSmall) - ) - }, - style = ToolkitChipStyle.Outlined + listOf( + requiredTitle to customSettings.filterValues { it.required }, + optionalTitle to customSettings.filterValues { !it.required } + ).forEach { (groupTitle, groupSettings) -> + if (groupSettings.isNotEmpty()) { + PluginSettingGroupHeader(groupTitle, groupSettings.size) + } + groupSettings.forEach { (key, meta) -> + Column(modifier = Modifier.fillMaxWidth()) { + val value = store.settings[key] ?: meta.defaultValue + DynamicParameterInput( + name = key, + metadata = ParameterMetadata( + description = meta.description, + type = meta.type, + defaultValue = meta.defaultValue, + required = meta.required, + secret = meta.secret + ), + value = SettingsUtils.jsonToString(value, meta.type), + onValueChange = { + viewModel.updateSetting( + key, + SettingsUtils.stringToJson(it, meta.type) ) + }, + enabled = !isBusy, + providedSettings = providedSettings, + providedLocks = locks + ) + + val lockedOptionsForSetting = lockedEnumOptions[key]?.distinct() ?: emptyList() + + if (meta.requiredByCapabilities.isNotEmpty() || lockedOptionsForSetting.isNotEmpty()) { + Row( + modifier = Modifier + .fillMaxWidth() + .padding( + start = ToolkitTheme.spacing.medium, + bottom = ToolkitTheme.spacing.mediumSmall, + end = ToolkitTheme.spacing.medium + ) + .horizontalScroll(rememberScrollState()), + horizontalArrangement = Arrangement.spacedBy(ToolkitTheme.spacing.small) + ) { + meta.requiredByCapabilities.forEach { capName -> + ToolkitChip( + text = stringResource( + Res.string.settings_locked_capability, + capName + ), + icon = { + Icon( + Icons.Default.Lock, + contentDescription = null, + modifier = Modifier.size(ToolkitTheme.dimensions.iconExtraSmall) + ) + }, + style = ToolkitChipStyle.Tinted + ) + } + if (lockedOptionsForSetting.isNotEmpty()) { + ToolkitChip( + text = "Unlocks Enum Options", + modifier = Modifier.tooltip( + text = "Unlocks values:\n" + lockedOptionsForSetting.joinToString("\n"), + ), + icon = { + Icon( + Icons.Default.Lock, + contentDescription = null, + modifier = Modifier.size(ToolkitTheme.dimensions.iconExtraSmall) + ) + }, + style = ToolkitChipStyle.Outlined + ) + } } } } @@ -674,6 +686,21 @@ private fun PluginSectionHeader(title: String) { ) } +@Composable +private fun PluginSettingGroupHeader(title: String, count: Int) { + Text( + text = "$title ($count)", + style = MaterialTheme.typography.labelLarge, + fontWeight = FontWeight.SemiBold, + color = MaterialTheme.colorScheme.onSurfaceVariant, + modifier = Modifier.padding( + start = ToolkitTheme.spacing.medium, + top = ToolkitTheme.spacing.small, + bottom = ToolkitTheme.spacing.extraSmall + ) + ) +} + @Composable private fun ActionParametersDialog( action: PluginAction, From d343ba973e6d61b4e9abc3c756c901a7d458a070 Mon Sep 17 00:00:00 2001 From: Matteo Mekhail <67237370+matteoiscrying@users.noreply.github.com> Date: Sat, 15 Aug 2026 05:38:39 +1000 Subject: [PATCH 05/16] fix: complete setting metadata integration --- .../plugin/ui/PluginSettingsContent.kt | 3 +- .../viewmodel/PluginSettingsViewModel.kt | 57 +++++++++++++++++-- .../PluginSettingsAutogenerationTest.kt | 40 +++++++++++++ .../wip/plugintoolkit/api/ManifestModels.kt | 37 ++++++++++-- .../processor/generators/ManifestGenerator.kt | 24 +++++++- .../SettingMetadataBinaryCompatibilityTest.kt | 27 +++++++++ 6 files changed, 176 insertions(+), 12 deletions(-) create mode 100644 composeApp/src/commonTest/kotlin/org/wip/plugintoolkit/features/plugin/viewmodel/PluginSettingsAutogenerationTest.kt create mode 100644 plugin-api/src/jvmTest/kotlin/org/wip/plugintoolkit/api/SettingMetadataBinaryCompatibilityTest.kt diff --git a/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/ui/PluginSettingsContent.kt b/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/ui/PluginSettingsContent.kt index 334451c2..d818e734 100644 --- a/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/ui/PluginSettingsContent.kt +++ b/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/ui/PluginSettingsContent.kt @@ -496,7 +496,8 @@ fun PluginSettingsContent( SettingsUtils.stringToJson(it, meta.type) ) }, - enabled = !isBusy, + enabled = !isBusy && meta.autogeneratedPattern == null, + isAutoGenerated = meta.autogeneratedPattern != null, providedSettings = providedSettings, providedLocks = locks ) diff --git a/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/viewmodel/PluginSettingsViewModel.kt b/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/viewmodel/PluginSettingsViewModel.kt index 6094902a..6be18748 100644 --- a/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/viewmodel/PluginSettingsViewModel.kt +++ b/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/viewmodel/PluginSettingsViewModel.kt @@ -7,25 +7,72 @@ import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.update import kotlinx.coroutines.launch import kotlinx.serialization.json.JsonElement +import org.wip.plugintoolkit.api.SettingMetadata +import org.wip.plugintoolkit.features.flows.logic.PathPatternResolver import org.wip.plugintoolkit.features.job.logic.JobManager import org.wip.plugintoolkit.features.job.model.JobStatus import org.wip.plugintoolkit.features.plugin.logic.PluginManager +import org.wip.plugintoolkit.features.plugin.model.PluginSettingsStore +import org.wip.plugintoolkit.features.plugin.utils.SettingsUtils + +internal fun resolveAutogeneratedSettings( + metadata: Map, + settings: Map, + additionalValues: Map = emptyMap() +): Map { + val resolvedSettings = settings.toMutableMap() + val defaults = metadata.mapNotNull { (key, value) -> value.defaultValue?.let { key to it } }.toMap() + + repeat(metadata.size.coerceAtLeast(1)) { + var changed = false + metadata.forEach { (key, settingMetadata) -> + val pattern = settingMetadata.autogeneratedPattern?.takeIf { it.isNotBlank() } ?: return@forEach + val availableValues = defaults + additionalValues + resolvedSettings + val stringValues = availableValues.mapValues { (valueKey, value) -> + val valueType = metadata[valueKey]?.type + if (valueType != null) SettingsUtils.jsonToString(value, valueType) else value.toString().trim('"') + } + val generated = runCatching { PathPatternResolver.tryResolve(pattern, stringValues) } + .getOrNull() + .orEmpty() + val generatedValue = SettingsUtils.stringToJson(generated, settingMetadata.type) + if (resolvedSettings[key] != generatedValue) { + resolvedSettings[key] = generatedValue + changed = true + } + } + if (!changed) return resolvedSettings + } + + return resolvedSettings +} class PluginSettingsViewModel( val pkg: String, private val pluginManager: PluginManager, private val jobManager: JobManager, ) : ViewModel() { - private val _store = MutableStateFlow(pluginManager.loadPluginSettings(pkg)) + val manifest = pluginManager.getManifest(pkg) + private val initialStore = pluginManager.loadPluginSettings(pkg) + private val _store = MutableStateFlow(initialStore.withAutogeneratedSettings()) val store = _store.asStateFlow() private val _isBusy = MutableStateFlow(false) val isBusy = _isBusy.asStateFlow() - val manifest = pluginManager.getManifest(pkg) - val locks = MutableStateFlow>(emptyMap()) + private fun PluginSettingsStore.withAutogeneratedSettings(): PluginSettingsStore { + val settingMetadata = manifest?.settings ?: return this + return copy( + settings = resolveAutogeneratedSettings( + metadata = settingMetadata, + settings = settings, + additionalValues = globalParams + ) + ) + } + init { viewModelScope.launch { pluginManager.refreshLocks(pkg) @@ -45,7 +92,7 @@ class PluginSettingsViewModel( fun updateSetting(key: String, value: JsonElement) { _store.update { current -> - val updated = current.copy(settings = current.settings + (key to value)) + val updated = current.copy(settings = current.settings + (key to value)).withAutogeneratedSettings() viewModelScope.launch { val newLocks = pluginManager.refreshLocks(pkg, updated) locks.value = newLocks @@ -56,7 +103,7 @@ class PluginSettingsViewModel( fun updateGlobalParam(key: String, value: JsonElement) { _store.update { current -> - val updated = current.copy(globalParams = current.globalParams + (key to value)) + val updated = current.copy(globalParams = current.globalParams + (key to value)).withAutogeneratedSettings() viewModelScope.launch { val newLocks = pluginManager.refreshLocks(pkg, updated) locks.value = newLocks diff --git a/composeApp/src/commonTest/kotlin/org/wip/plugintoolkit/features/plugin/viewmodel/PluginSettingsAutogenerationTest.kt b/composeApp/src/commonTest/kotlin/org/wip/plugintoolkit/features/plugin/viewmodel/PluginSettingsAutogenerationTest.kt new file mode 100644 index 00000000..37a6191b --- /dev/null +++ b/composeApp/src/commonTest/kotlin/org/wip/plugintoolkit/features/plugin/viewmodel/PluginSettingsAutogenerationTest.kt @@ -0,0 +1,40 @@ +package org.wip.plugintoolkit.features.plugin.viewmodel + +import kotlinx.serialization.json.JsonPrimitive +import org.wip.plugintoolkit.api.DataType +import org.wip.plugintoolkit.api.PrimitiveType +import org.wip.plugintoolkit.api.SettingMetadata +import kotlin.test.Test +import kotlin.test.assertEquals + +class PluginSettingsAutogenerationTest { + private val stringType = DataType.Primitive(PrimitiveType.STRING) + private val metadata = mapOf( + "input" to SettingMetadata(description = "Input", type = stringType), + "output" to SettingMetadata( + description = "Output", + type = stringType, + autogeneratedPattern = "{input.dir}/{input.nameWithoutExtension}.result" + ) + ) + + @Test + fun `derived setting follows its dependency`() { + val result = resolveAutogeneratedSettings( + metadata = metadata, + settings = mapOf("input" to JsonPrimitive("work/photo.png")) + ) + + assertEquals(JsonPrimitive("work/photo.result"), result["output"]) + } + + @Test + fun `derived setting is cleared when dependency is missing`() { + val result = resolveAutogeneratedSettings( + metadata = metadata, + settings = mapOf("output" to JsonPrimitive("stale.result")) + ) + + assertEquals(JsonPrimitive(""), result["output"]) + } +} diff --git a/plugin-api/src/commonMain/kotlin/org/wip/plugintoolkit/api/ManifestModels.kt b/plugin-api/src/commonMain/kotlin/org/wip/plugintoolkit/api/ManifestModels.kt index 0da84ce9..52509212 100644 --- a/plugin-api/src/commonMain/kotlin/org/wip/plugintoolkit/api/ManifestModels.kt +++ b/plugin-api/src/commonMain/kotlin/org/wip/plugintoolkit/api/ManifestModels.kt @@ -246,15 +246,44 @@ data class SettingMetadata( val required: Boolean = false, val secret: Boolean = false, val constraints: ParameterConstraints? = null, - val semanticTypes: List = emptyList(), - val autogeneratedPattern: String? = null, /** * List of capability names that require this setting. * This allows UI to show which capabilities are locked behind this setting * without making the setting globally required for the plugin to load. */ - val requiredByCapabilities: List = emptyList() -) + val requiredByCapabilities: List = emptyList(), + // Keep new fields after the original constructor fields so component7 retains + // its pre-existing requiredByCapabilities meaning for old compiled callers. + val semanticTypes: List = emptyList(), + val autogeneratedPattern: String? = null +) { + /** + * Retains the JVM constructor used by plugins compiled before semantic hints and + * autogenerated patterns were added. The host classloader deliberately shares + * plugin-api classes, so removing this descriptor would break installed plugins + * before their compatibility metadata can be inspected. + */ + @Deprecated("Binary compatibility constructor", level = DeprecationLevel.HIDDEN) + constructor( + defaultValue: JsonElement? = null, + description: String, + type: DataType, + required: Boolean = false, + secret: Boolean = false, + constraints: ParameterConstraints? = null, + requiredByCapabilities: List = emptyList() + ) : this( + defaultValue = defaultValue, + description = description, + type = type, + required = required, + secret = secret, + constraints = constraints, + requiredByCapabilities = requiredByCapabilities, + semanticTypes = emptyList(), + autogeneratedPattern = null + ) +} /** * The complete manifest of a plugin, describing its capabilities and requirements. diff --git a/plugin-api/src/jvmMain/kotlin/org/wip/plugintoolkit/api/processor/generators/ManifestGenerator.kt b/plugin-api/src/jvmMain/kotlin/org/wip/plugintoolkit/api/processor/generators/ManifestGenerator.kt index 6668ef16..2c0f770c 100644 --- a/plugin-api/src/jvmMain/kotlin/org/wip/plugintoolkit/api/processor/generators/ManifestGenerator.kt +++ b/plugin-api/src/jvmMain/kotlin/org/wip/plugintoolkit/api/processor/generators/ManifestGenerator.kt @@ -368,6 +368,17 @@ object ManifestGenerator { ?.filterIsInstance() ?: emptyList() val pathTemplate = ann.arguments.find { it.name?.asString() == "pathTemplate" }?.value as? String ?: "" + val requiredByCapabilities = functions.mapNotNull { function -> + val capabilityAnnotation = function.annotations.find { + it.hasQualifiedName(CAPABILITY_ANNOTATION) + } ?: return@mapNotNull null + val requiredSettings = + (capabilityAnnotation.arguments.find { it.name?.asString() == "requiresSettings" }?.value as? List<*>) + ?.filterIsInstance() + ?: emptyList() + if (propName !in requiredSettings) return@mapNotNull null + capabilityAnnotation.arguments.find { it.name?.asString() == "name" }?.value as? String + } val defaultValueCode = if (defaultVal.isNotEmpty()) { try { kotlinx.serialization.json.Json.parseToJsonElement(defaultVal) @@ -401,10 +412,18 @@ object ManifestGenerator { ) val autogeneratedPatternCode = if (pathTemplate.isBlank()) CodeBlock.of("null") else CodeBlock.of("%S", pathTemplate) + val requiredByCapabilitiesCode = if (requiredByCapabilities.isEmpty()) { + CodeBlock.of("emptyList()") + } else { + CodeBlock.of( + "listOf(%L)", + requiredByCapabilities.joinToString { "\"$it\"" } + ) + } settingsCode.add( - "%S to %T(defaultValue = %L, description = %S, type = %M<%T>(), constraints = %L, required = %L, secret = %L, semanticTypes = %L, autogeneratedPattern = %L)", + "%S to %T(defaultValue = %L, description = %S, type = %M<%T>(), constraints = %L, required = %L, secret = %L, semanticTypes = %L, autogeneratedPattern = %L, requiredByCapabilities = %L)", propName, CN_SETTING_METADATA, defaultValueCode, @@ -415,7 +434,8 @@ object ManifestGenerator { required, secret, semanticTypesCode, - autogeneratedPatternCode + autogeneratedPatternCode, + requiredByCapabilitiesCode ) if (index < settingsProperties.size - 1) settingsCode.add(",\n") else settingsCode.add("\n") } diff --git a/plugin-api/src/jvmTest/kotlin/org/wip/plugintoolkit/api/SettingMetadataBinaryCompatibilityTest.kt b/plugin-api/src/jvmTest/kotlin/org/wip/plugintoolkit/api/SettingMetadataBinaryCompatibilityTest.kt new file mode 100644 index 00000000..e8ba24ab --- /dev/null +++ b/plugin-api/src/jvmTest/kotlin/org/wip/plugintoolkit/api/SettingMetadataBinaryCompatibilityTest.kt @@ -0,0 +1,27 @@ +package org.wip.plugintoolkit.api + +import kotlinx.serialization.json.JsonElement +import kotlin.test.Test +import kotlin.test.assertTrue + +class SettingMetadataBinaryCompatibilityTest { + @Test + fun `retains pre-hints JVM constructor`() { + val oldParameterTypes = listOf( + JsonElement::class.java, + String::class.java, + DataType::class.java, + Boolean::class.javaPrimitiveType, + Boolean::class.javaPrimitiveType, + ParameterConstraints::class.java, + List::class.java + ) + + assertTrue( + SettingMetadata::class.java.declaredConstructors.any { constructor -> + constructor.parameterTypes.toList() == oldParameterTypes + }, + "SettingMetadata must keep the constructor used by plugins compiled against the previous API" + ) + } +} From 848eb8ddb7274d73adc909c4184da60854dfbfad Mon Sep 17 00:00:00 2001 From: Matteo Mekhail <67237370+matteoiscrying@users.noreply.github.com> Date: Sat, 15 Aug 2026 05:49:37 +1000 Subject: [PATCH 06/16] fix: harden sandboxed file deletion --- .../logic/DefaultExecutionFileSystem.kt | 39 +------ .../plugin/logic/DefaultPluginFileSystem.kt | 43 ++------ .../plugin/logic/SandboxFileOperations.kt | 62 +++++++++++ .../logic/SandboxFileSystemSecurityTest.kt | 104 ++++++++++++++++++ .../org/wip/plugintoolkit/api/RelativePath.kt | 15 +-- .../wip/plugintoolkit/api/RelativePathTest.kt | 4 + 6 files changed, 190 insertions(+), 77 deletions(-) create mode 100644 composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/SandboxFileOperations.kt create mode 100644 composeApp/src/jvmTest/kotlin/org/wip/plugintoolkit/features/plugin/logic/SandboxFileSystemSecurityTest.kt diff --git a/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/DefaultExecutionFileSystem.kt b/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/DefaultExecutionFileSystem.kt index 3d842b4d..818461ff 100644 --- a/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/DefaultExecutionFileSystem.kt +++ b/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/DefaultExecutionFileSystem.kt @@ -17,25 +17,10 @@ class DefaultExecutionFileSystem( SystemFileSystem.createDirectories(Path(sandboxPath)) } - private fun resolvePath(relativePath: RelativePath): Path { - val resolved = Path(sandboxPath, relativePath.value) - val file = java.io.File(resolved.toString()) - val normalized = try { - file.canonicalPath - } catch (e: Exception) { - throw SecurityException("Failed to resolve canonical path for '${relativePath.value}': ${e.message}") - } - val baseFile = java.io.File(sandboxPath) - val baseCanonical = try { - baseFile.canonicalPath - } catch (e: Exception) { - throw SecurityException("Failed to resolve base canonical path for '$sandboxPath': ${e.message}") - } + private val sandboxOperations = SandboxFileOperations(sandboxPath) - if (normalized != baseCanonical && !normalized.startsWith(baseCanonical + java.io.File.separator)) { - throw SecurityException("Access to path '${relativePath.value}' is denied. It is outside the sandbox.") - } - return resolved + private fun resolvePath(relativePath: RelativePath): Path { + return sandboxOperations.resolve(relativePath) } override suspend fun readFile(relativePath: RelativePath): ByteArray? { @@ -102,22 +87,8 @@ class DefaultExecutionFileSystem( } override suspend fun deleteDirectory(relativePath: RelativePath, recursive: Boolean): Result = runCatching { - require(relativePath != RelativePath.ROOT) { "The execution sandbox root cannot be deleted" } - deleteDirectory(resolvePath(relativePath), recursive) - } - - private fun deleteDirectory(path: Path, recursive: Boolean) { - if (!SystemFileSystem.exists(path)) return - require(SystemFileSystem.metadataOrNull(path)?.isDirectory == true) { "Path is not a directory: $path" } - val children = SystemFileSystem.list(path) - require(recursive || children.isEmpty()) { "Directory is not empty: $path" } - if (recursive) { - children.forEach { child -> - if (SystemFileSystem.metadataOrNull(child)?.isDirectory == true) deleteDirectory(child, true) - else SystemFileSystem.delete(child) - } - } - SystemFileSystem.delete(path) + require(relativePath.value.isNotEmpty()) { "The execution sandbox root cannot be deleted" } + sandboxOperations.deleteDirectory(resolvePath(relativePath), recursive) } override fun getBasePath(): String = sandboxPath diff --git a/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/DefaultPluginFileSystem.kt b/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/DefaultPluginFileSystem.kt index 8f9a3997..100ddb77 100644 --- a/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/DefaultPluginFileSystem.kt +++ b/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/DefaultPluginFileSystem.kt @@ -23,15 +23,11 @@ class DefaultPluginFileSystem( SystemFileSystem.createDirectories(Path(cachePath)) } - private fun resolvePath(relativePath: RelativePath): Path { - val resolved = Path(basePath, relativePath.value) - val normalized = resolved.toString().replace('\\', '/') - val baseCanonical = Path(basePath).toString().replace('\\', '/') + private val filesOperations = SandboxFileOperations(basePath) + private val cacheOperations = SandboxFileOperations(cachePath) - if (normalized != baseCanonical && !normalized.startsWith(if (baseCanonical.endsWith("/")) baseCanonical else "$baseCanonical/")) { - throw SecurityException("Access to path '${relativePath.value}' is denied. It is outside the plugin files directory.") - } - return resolved + private fun resolvePath(relativePath: RelativePath): Path { + return filesOperations.resolve(relativePath) } override suspend fun readFile(relativePath: RelativePath): ByteArray? { @@ -98,8 +94,8 @@ class DefaultPluginFileSystem( } override suspend fun deleteDirectory(relativePath: RelativePath, recursive: Boolean): Result = runCatching { - require(relativePath != RelativePath.ROOT) { "The plugin files root cannot be deleted" } - deleteDirectory(resolvePath(relativePath), recursive) + require(relativePath.value.isNotEmpty()) { "The plugin files root cannot be deleted" } + filesOperations.deleteDirectory(resolvePath(relativePath), recursive) } override suspend fun extractResource(resourcePath: String, targetRelativePath: RelativePath): Result { @@ -154,37 +150,16 @@ class DefaultPluginFileSystem( override suspend fun deleteDirectory(relativePath: RelativePath, recursive: Boolean): Result = runCatching { - require(relativePath != RelativePath.ROOT) { "The plugin cache root cannot be deleted" } - fs.deleteDirectory(fs.resolveCachePath(relativePath), recursive) + require(relativePath.value.isNotEmpty()) { "The plugin cache root cannot be deleted" } + fs.cacheOperations.deleteDirectory(fs.resolveCachePath(relativePath), recursive) } } } } } - private fun deleteDirectory(path: Path, recursive: Boolean) { - if (!SystemFileSystem.exists(path)) return - require(SystemFileSystem.metadataOrNull(path)?.isDirectory == true) { "Path is not a directory: $path" } - val children = SystemFileSystem.list(path) - require(recursive || children.isEmpty()) { "Directory is not empty: $path" } - if (recursive) { - children.forEach { child -> - if (SystemFileSystem.metadataOrNull(child)?.isDirectory == true) deleteDirectory(child, true) - else SystemFileSystem.delete(child) - } - } - SystemFileSystem.delete(path) - } - private fun resolveCachePath(relativePath: RelativePath): Path { - val resolved = Path(cachePath, relativePath.value) - val normalized = resolved.toString().replace('\\', '/') - val baseCanonical = Path(cachePath).toString().replace('\\', '/') - - if (normalized != baseCanonical && !normalized.startsWith(if (baseCanonical.endsWith("/")) baseCanonical else "$baseCanonical/")) { - throw SecurityException("Access to path '${relativePath.value}' is denied. It is outside the plugin cache directory.") - } - return resolved + return cacheOperations.resolve(relativePath) } private suspend fun readFromCache(relativePath: RelativePath): ByteArray? { diff --git a/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/SandboxFileOperations.kt b/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/SandboxFileOperations.kt new file mode 100644 index 00000000..7503baa1 --- /dev/null +++ b/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/SandboxFileOperations.kt @@ -0,0 +1,62 @@ +package org.wip.plugintoolkit.features.plugin.logic + +import kotlinx.io.files.Path +import org.wip.plugintoolkit.api.RelativePath +import java.nio.file.FileVisitResult +import java.nio.file.Files +import java.nio.file.LinkOption +import java.nio.file.Path as NioPath +import java.nio.file.Paths +import java.nio.file.SimpleFileVisitor +import java.nio.file.attribute.BasicFileAttributes + +internal class SandboxFileOperations(root: String) { + private val base = Paths.get(root).toAbsolutePath().normalize() + private val realBase = base.toRealPath() + + fun resolve(relativePath: RelativePath): Path { + if (base.toRealPath() != realBase) { + throw SecurityException("The sandbox root changed after it was initialized") + } + val candidate = base.resolve(relativePath.value).normalize() + if (!candidate.startsWith(base)) { + throw SecurityException("Access to path '${relativePath.value}' is outside the sandbox") + } + + var current = base + base.relativize(candidate).forEach { segment -> + current = current.resolve(segment) + if (Files.exists(current, LinkOption.NOFOLLOW_LINKS)) { + val realCurrent = current.toRealPath() + if (!realCurrent.startsWith(realBase)) { + throw SecurityException("Access to path '${relativePath.value}' escapes the sandbox through a symbolic link") + } + } + } + return Path(candidate.toString()) + } + + fun deleteDirectory(path: Path, recursive: Boolean) { + val nioPath = Paths.get(path.toString()) + if (!Files.exists(nioPath, LinkOption.NOFOLLOW_LINKS)) return + require(Files.isDirectory(nioPath, LinkOption.NOFOLLOW_LINKS)) { "Path is not a directory: $path" } + + if (!recursive) { + Files.delete(nioPath) + return + } + + Files.walkFileTree(nioPath, object : SimpleFileVisitor() { + override fun visitFile(file: NioPath, attrs: BasicFileAttributes): FileVisitResult { + Files.delete(file) + return FileVisitResult.CONTINUE + } + + override fun postVisitDirectory(dir: NioPath, error: java.io.IOException?): FileVisitResult { + if (error != null) throw error + Files.delete(dir) + return FileVisitResult.CONTINUE + } + }) + } +} diff --git a/composeApp/src/jvmTest/kotlin/org/wip/plugintoolkit/features/plugin/logic/SandboxFileSystemSecurityTest.kt b/composeApp/src/jvmTest/kotlin/org/wip/plugintoolkit/features/plugin/logic/SandboxFileSystemSecurityTest.kt new file mode 100644 index 00000000..5a88da3e --- /dev/null +++ b/composeApp/src/jvmTest/kotlin/org/wip/plugintoolkit/features/plugin/logic/SandboxFileSystemSecurityTest.kt @@ -0,0 +1,104 @@ +package org.wip.plugintoolkit.features.plugin.logic + +import kotlinx.coroutines.test.runTest +import org.wip.plugintoolkit.api.RelativePath +import org.wip.plugintoolkit.api.ScopedFileSystem +import java.nio.file.FileVisitResult +import java.nio.file.Files +import java.nio.file.Path +import java.nio.file.SimpleFileVisitor +import java.nio.file.attribute.BasicFileAttributes +import kotlin.test.AfterTest +import kotlin.test.BeforeTest +import kotlin.test.Test +import kotlin.test.assertFailsWith +import kotlin.test.assertTrue + +class SandboxFileSystemSecurityTest { + private lateinit var testRoot: Path + + @BeforeTest + fun setUp() { + testRoot = Files.createTempDirectory("plugin-toolkit-sandbox-") + } + + @AfterTest + fun tearDown() { + if (!Files.exists(testRoot)) return + Files.walkFileTree(testRoot, object : SimpleFileVisitor() { + override fun visitFile(file: Path, attrs: BasicFileAttributes): FileVisitResult { + Files.deleteIfExists(file) + return FileVisitResult.CONTINUE + } + + override fun postVisitDirectory(dir: Path, error: java.io.IOException?): FileVisitResult { + if (error != null) throw error + Files.deleteIfExists(dir) + return FileVisitResult.CONTINUE + } + }) + } + + @Test + fun rootAliasesCannotDeleteAnySandboxRoot() = runTest { + val alias = RelativePath.from("././").getOrThrow() + val execution = DefaultExecutionFileSystem(testRoot.resolve("execution").toString()) + val pluginInstall = testRoot.resolve("plugin").toString() + val plugin = DefaultPluginFileSystem(pluginInstall) + val cache = DefaultPluginFileSystem.createCacheOnly(pluginInstall) + + assertTrue(execution.deleteDirectory(alias, recursive = true).isFailure) + assertTrue(plugin.deleteDirectory(alias, recursive = true).isFailure) + assertTrue(cache.deleteDirectory(alias, recursive = true).isFailure) + assertTrue(Files.isDirectory(testRoot.resolve("execution"))) + assertTrue(Files.isDirectory(testRoot.resolve("plugin/files"))) + assertTrue(Files.isDirectory(testRoot.resolve("plugin/cache"))) + } + + @Test + fun executionSandboxCannotReadThroughSymlinkAndDoesNotFollowItOnDelete() = runTest { + val sandbox = testRoot.resolve("execution") + val outside = createOutsideSecret() + val fileSystem = DefaultExecutionFileSystem(sandbox.toString()) + + verifySymlinkIsContained(fileSystem, sandbox, outside) + } + + @Test + fun pluginFilesCannotReadThroughSymlinkAndDoNotFollowItOnDelete() = runTest { + val install = testRoot.resolve("plugin") + val outside = createOutsideSecret() + val fileSystem = DefaultPluginFileSystem(install.toString()) + + verifySymlinkIsContained(fileSystem, install.resolve("files"), outside) + } + + @Test + fun pluginCacheCannotReadThroughSymlinkAndDoesNotFollowItOnDelete() = runTest { + val install = testRoot.resolve("plugin") + val outside = createOutsideSecret() + val fileSystem = DefaultPluginFileSystem.createCacheOnly(install.toString()) + + verifySymlinkIsContained(fileSystem, install.resolve("cache"), outside) + } + + private fun createOutsideSecret(): Path { + val outside = Files.createDirectories(testRoot.resolve("outside")) + Files.writeString(outside.resolve("secret.txt"), "must survive") + return outside + } + + private suspend fun verifySymlinkIsContained( + fileSystem: ScopedFileSystem, + sandbox: Path, + outside: Path + ) { + val nested = Files.createDirectories(sandbox.resolve("nested")) + Files.createSymbolicLink(nested.resolve("escape"), outside) + val escapedFile = RelativePath.from("nested/escape/secret.txt").getOrThrow() + + assertFailsWith { fileSystem.readTextFile(escapedFile) } + assertTrue(fileSystem.deleteDirectory(RelativePath.from("nested").getOrThrow(), recursive = true).isSuccess) + assertTrue(Files.readString(outside.resolve("secret.txt")) == "must survive") + } +} diff --git a/plugin-api/src/commonMain/kotlin/org/wip/plugintoolkit/api/RelativePath.kt b/plugin-api/src/commonMain/kotlin/org/wip/plugintoolkit/api/RelativePath.kt index 7940117a..650ae950 100644 --- a/plugin-api/src/commonMain/kotlin/org/wip/plugintoolkit/api/RelativePath.kt +++ b/plugin-api/src/commonMain/kotlin/org/wip/plugintoolkit/api/RelativePath.kt @@ -13,7 +13,6 @@ value class RelativePath private constructor(val value: String) { companion object { val ROOT = RelativePath("") private val NULL_BYTE_REGEX = Regex("\u0000") - private val TRAVERSAL_REGEX = Regex("""(?:^|/|\\|\.|\u2024|\uFF0E|\u3002)(?:\.\.|\u2024\u2024|\uFF0E\uFF0E|%2e%2e|%2E%2E|%252e%252e)(?:/|\\|${'$'}|\.)""", RegexOption.IGNORE_CASE) private val ENCODED_SLASH_REGEX = Regex("""%2f|%5c""", RegexOption.IGNORE_CASE) /** @@ -38,25 +37,23 @@ value class RelativePath private constructor(val value: String) { return Result.failure(SecurityException("Path must be relative, but contains drive letter: $normalized")) } - // Check for encoded slashes or traversal sequences - if (normalized.contains(ENCODED_SLASH_REGEX) || normalized.contains(TRAVERSAL_REGEX)) { + if (normalized.contains(ENCODED_SLASH_REGEX)) { return Result.failure(SecurityException("Path traversal attempt detected: $normalized")) } - // Normalized path checks: convert unicode dot variants to regular dot for safety check val sanitized = normalized .replace("\u2024", ".") .replace("\uFF0E", ".") .replace("\u3002", ".") .replace("%2e", ".", ignoreCase = true) - .replace("%2f", "/", ignoreCase = true) - .replace("%5c", "\\", ignoreCase = true) + .replace('\\', '/') - if (sanitized.contains(TRAVERSAL_REGEX) || sanitized.contains("../") || sanitized.contains("..\\")) { + val segments = sanitized.split('/').filter { it.isNotEmpty() && it != "." } + if (segments.any { it == ".." || it.contains(Regex("%25(?:2e|2f|5c)", RegexOption.IGNORE_CASE)) }) { return Result.failure(SecurityException("Path traversal attempt detected: $normalized")) } - - return Result.success(RelativePath(normalized)) + + return Result.success(RelativePath(segments.joinToString("/"))) } } } diff --git a/plugin-api/src/commonTest/kotlin/org/wip/plugintoolkit/api/RelativePathTest.kt b/plugin-api/src/commonTest/kotlin/org/wip/plugintoolkit/api/RelativePathTest.kt index 96ae01bd..400f17d5 100644 --- a/plugin-api/src/commonTest/kotlin/org/wip/plugintoolkit/api/RelativePathTest.kt +++ b/plugin-api/src/commonTest/kotlin/org/wip/plugintoolkit/api/RelativePathTest.kt @@ -52,5 +52,9 @@ class RelativePathTest { val path = " foo/bar.txt ".toRelativePath() assertTrue(path.isSuccess) assertEquals("foo/bar.txt", path.getOrNull()?.value, "Should trim whitespace") + assertEquals(RelativePath.ROOT, ".".toRelativePath().getOrThrow()) + assertEquals(RelativePath.ROOT, "././".toRelativePath().getOrThrow()) + assertEquals("foo/bar", "foo/./bar".toRelativePath().getOrThrow().value) + assertEquals("foo/bar", "foo\\bar".toRelativePath().getOrThrow().value) } } From 2eff25943c54354c0a244de2650d76c83fbaa011 Mon Sep 17 00:00:00 2001 From: Matteo Mekhail <67237370+matteoiscrying@users.noreply.github.com> Date: Sat, 15 Aug 2026 05:51:06 +1000 Subject: [PATCH 07/16] fix: preserve unset plugin settings --- .../plugin/model/PluginSettingsStore.kt | 9 +-- .../features/plugin/ui/PluginContent.kt | 3 +- .../plugin/ui/PluginSettingsContent.kt | 6 +- .../logic/PluginLifecycleManagerTest.kt | 59 +++++++++++++++++++ .../plugin/model/PluginSettingDefaultsTest.kt | 16 ++++- 5 files changed, 79 insertions(+), 14 deletions(-) diff --git a/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/model/PluginSettingsStore.kt b/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/model/PluginSettingsStore.kt index a2418e3e..8ed2f01a 100644 --- a/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/model/PluginSettingsStore.kt +++ b/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/model/PluginSettingsStore.kt @@ -2,10 +2,7 @@ package org.wip.plugintoolkit.features.plugin.model import kotlinx.serialization.Serializable import kotlinx.serialization.json.JsonElement -import kotlinx.serialization.json.JsonPrimitive -import org.wip.plugintoolkit.api.DataType import org.wip.plugintoolkit.api.PluginManifest -import org.wip.plugintoolkit.api.PrimitiveType @Serializable data class PluginSettingsStore( @@ -15,11 +12,7 @@ data class PluginSettingsStore( ) fun PluginManifest.defaultCustomSettings(): Map = settings.orEmpty().mapNotNull { (key, metadata) -> - val type = metadata.type - val value = metadata.defaultValue ?: if ( - type is DataType.Primitive && type.primitiveType == PrimitiveType.BOOLEAN - ) JsonPrimitive(false) else null - value?.let { key to it } + metadata.defaultValue?.let { key to it } }.toMap() /** Manifest defaults with persisted user values taking precedence. */ diff --git a/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/ui/PluginContent.kt b/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/ui/PluginContent.kt index a68aa328..897b845d 100644 --- a/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/ui/PluginContent.kt +++ b/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/ui/PluginContent.kt @@ -119,7 +119,8 @@ fun PluginContent( val providedSettings = remember(pluginId, pluginSettingsState) { val store = if (pluginId != null) pluginSettingsState[pluginId] ?: pluginManager.loadPluginSettings(pluginId) else null val manifest = viewModel.selectedPlugin?.getManifest()?.getOrNull() - store?.resolveProvidedValues(manifest) ?: emptyMap() + (store ?: org.wip.plugintoolkit.features.plugin.model.PluginSettingsStore()) + .resolveProvidedValues(manifest) } if (selectedCapability == null) { diff --git a/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/ui/PluginSettingsContent.kt b/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/ui/PluginSettingsContent.kt index d2b98758..153830b8 100644 --- a/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/ui/PluginSettingsContent.kt +++ b/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/ui/PluginSettingsContent.kt @@ -74,7 +74,7 @@ import org.wip.plugintoolkit.api.PluginAction import org.wip.plugintoolkit.api.PrimitiveType import org.wip.plugintoolkit.core.model.localized import org.wip.plugintoolkit.core.theme.ToolkitTheme -import org.wip.plugintoolkit.features.plugin.model.resolveProvidedValues +import org.wip.plugintoolkit.features.plugin.model.resolveCustomSettings import org.wip.plugintoolkit.features.plugin.utils.SettingsUtils import org.wip.plugintoolkit.features.plugin.viewmodel.PluginSettingsViewModel import org.wip.plugintoolkit.shared.components.ToolkitChip @@ -414,8 +414,8 @@ fun PluginSettingsContent( ) } } else { - val providedSettings = remember(manifest, store.settings, store.globalParams) { - store.resolveProvidedValues(manifest) + val providedSettings = remember(manifest, store.settings) { + store.resolveCustomSettings(manifest) } LazyColumn( diff --git a/composeApp/src/commonTest/kotlin/org/wip/plugintoolkit/features/plugin/logic/PluginLifecycleManagerTest.kt b/composeApp/src/commonTest/kotlin/org/wip/plugintoolkit/features/plugin/logic/PluginLifecycleManagerTest.kt index acf03ddb..eced3f26 100644 --- a/composeApp/src/commonTest/kotlin/org/wip/plugintoolkit/features/plugin/logic/PluginLifecycleManagerTest.kt +++ b/composeApp/src/commonTest/kotlin/org/wip/plugintoolkit/features/plugin/logic/PluginLifecycleManagerTest.kt @@ -7,9 +7,16 @@ import org.wip.plugintoolkit.core.utils.FileSystem import org.wip.plugintoolkit.features.job.logic.JobManager import org.wip.plugintoolkit.features.plugin.model.InstalledPlugin import org.wip.plugintoolkit.features.plugin.model.PluginSettingsStore +import org.wip.plugintoolkit.features.plugin.model.resolveCustomSettings import org.wip.plugintoolkit.features.settings.logic.SettingsPersistence import org.wip.plugintoolkit.features.settings.logic.SettingsRepository import org.wip.plugintoolkit.features.settings.model.AppSettings +import org.wip.plugintoolkit.api.DataType +import org.wip.plugintoolkit.api.PluginInfo +import org.wip.plugintoolkit.api.PluginManifest +import org.wip.plugintoolkit.api.PrimitiveType +import org.wip.plugintoolkit.api.Requirements +import org.wip.plugintoolkit.api.SettingMetadata import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertNotSame @@ -48,6 +55,58 @@ class PluginLifecycleManagerTest { override fun openLatestLog() {} } + @Test + fun testPluginContextReceivesExactlyResolvedCustomSettings() = runTest { + val fileSystem = FakeFileSystem() + val settingsRepo = SettingsRepository(FakeSettingsPersistence(), backgroundScope) + val registry = PluginRegistry( + settingsRepo, + backgroundScope, + loomDispatcher, + io.mockk.mockk(relaxed = true) + ) + val lifecycleManager = PluginLifecycleManager( + registry, + JobManager(backgroundScope, settingsRepo), + settingsRepo, + fileSystem + ) + val pkg = "test.context.defaults" + registry.addOrUpdatePlugin( + InstalledPlugin(pkg, "Test", "1.0.0", "/tmp/test.context.defaults") + ) + val manifest = PluginManifest( + manifestVersion = "1", + plugin = PluginInfo(pkg, "Test", "1.0.0", "Test plugin"), + requirements = Requirements(128, 10), + settings = mapOf( + "endpoint" to SettingMetadata( + defaultValue = JsonPrimitive("https://default.test"), + description = "Endpoint", + type = DataType.Primitive(PrimitiveType.STRING) + ), + "optionalFlag" to SettingMetadata( + description = "Optional flag", + type = DataType.Primitive(PrimitiveType.BOOLEAN) + ) + ) + ) + val store = PluginSettingsStore( + settings = mapOf("endpoint" to JsonPrimitive("https://custom.test")), + globalParams = mapOf("region" to JsonPrimitive("eu")) + ) + + val context = lifecycleManager.createPluginContext( + pkg = pkg, + manifest = manifest, + overriddenSettings = store + ) + + assertEquals(store.resolveCustomSettings(manifest), context.settings) + kotlin.test.assertFalse(context.settings.containsKey("optionalFlag")) + kotlin.test.assertFalse(context.settings.containsKey("region")) + } + @Test fun testSettingsCaching() = runTest { val fileSystem = FakeFileSystem() diff --git a/composeApp/src/commonTest/kotlin/org/wip/plugintoolkit/features/plugin/model/PluginSettingDefaultsTest.kt b/composeApp/src/commonTest/kotlin/org/wip/plugintoolkit/features/plugin/model/PluginSettingDefaultsTest.kt index 5d5cecef..6b645980 100644 --- a/composeApp/src/commonTest/kotlin/org/wip/plugintoolkit/features/plugin/model/PluginSettingDefaultsTest.kt +++ b/composeApp/src/commonTest/kotlin/org/wip/plugintoolkit/features/plugin/model/PluginSettingDefaultsTest.kt @@ -9,6 +9,7 @@ import org.wip.plugintoolkit.api.Requirements import org.wip.plugintoolkit.api.SettingMetadata import kotlin.test.Test import kotlin.test.assertEquals +import kotlin.test.assertFalse class PluginSettingDefaultsTest { private val manifest = PluginManifest( @@ -33,7 +34,7 @@ class PluginSettingDefaultsTest { val resolved = PluginSettingsStore().resolveCustomSettings(manifest) assertEquals(JsonPrimitive("https://example.test"), resolved["endpoint"]) - assertEquals(JsonPrimitive(false), resolved["enabled"]) + assertFalse(resolved.containsKey("enabled")) } @Test @@ -47,7 +48,18 @@ class PluginSettingDefaultsTest { val provided = store.resolveProvidedValues(manifest) assertEquals(JsonPrimitive("https://custom.test"), custom["endpoint"]) - assertEquals(JsonPrimitive(false), custom["enabled"]) + assertFalse(custom.containsKey("enabled")) assertEquals(JsonPrimitive("eu"), provided["region"]) } + + @Test + fun `global parameters cannot shadow custom settings in custom setting resolution`() { + val store = PluginSettingsStore( + settings = mapOf("endpoint" to JsonPrimitive("https://custom.test")), + globalParams = mapOf("endpoint" to JsonPrimitive("global-collision")) + ) + + assertEquals(JsonPrimitive("https://custom.test"), store.resolveCustomSettings(manifest)["endpoint"]) + assertEquals(JsonPrimitive("global-collision"), store.resolveProvidedValues(manifest)["endpoint"]) + } } From badf10c3b6b813fe8372f7271c936703428eff19 Mon Sep 17 00:00:00 2001 From: Matteo Mekhail <67237370+matteoiscrying@users.noreply.github.com> Date: Sat, 15 Aug 2026 05:52:53 +1000 Subject: [PATCH 08/16] fix: test setting groups and target deep links --- .../plugin/ui/PluginSettingsContent.kt | 30 ++++++++++++-- .../plugin/ui/PluginSettingPartitionTest.kt | 40 +++++++++++++++++++ 2 files changed, 66 insertions(+), 4 deletions(-) create mode 100644 composeApp/src/commonTest/kotlin/org/wip/plugintoolkit/features/plugin/ui/PluginSettingPartitionTest.kt diff --git a/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/ui/PluginSettingsContent.kt b/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/ui/PluginSettingsContent.kt index 398c8017..e9372f81 100644 --- a/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/ui/PluginSettingsContent.kt +++ b/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/ui/PluginSettingsContent.kt @@ -21,6 +21,8 @@ import androidx.compose.foundation.lazy.LazyColumn import androidx.compose.foundation.lazy.items import androidx.compose.foundation.lazy.rememberLazyListState import androidx.compose.foundation.rememberScrollState +import androidx.compose.foundation.relocation.BringIntoViewRequester +import androidx.compose.foundation.relocation.bringIntoViewRequester import androidx.compose.foundation.shape.RoundedCornerShape import androidx.compose.material.icons.Icons import androidx.compose.material.icons.filled.Bolt @@ -51,6 +53,7 @@ import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember import androidx.compose.runtime.rememberCoroutineScope import androidx.compose.runtime.setValue +import androidx.compose.runtime.withFrameNanos import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.draw.clip @@ -72,6 +75,7 @@ import org.wip.plugintoolkit.api.DataType import org.wip.plugintoolkit.api.ParameterMetadata import org.wip.plugintoolkit.api.PluginAction import org.wip.plugintoolkit.api.PrimitiveType +import org.wip.plugintoolkit.api.SettingMetadata import org.wip.plugintoolkit.core.model.localized import org.wip.plugintoolkit.core.theme.ToolkitTheme import org.wip.plugintoolkit.features.plugin.utils.SettingsUtils @@ -104,6 +108,11 @@ import plugintoolkit.composeapp.generated.resources.settings_search_placeholder import org.wip.plugintoolkit.shared.components.verticalFadingEdges +internal fun partitionSettings( + settings: Map +): Pair, Map> = + settings.filterValues { it.required } to settings.filterValues { !it.required } + @Composable fun PluginSettingsContent( pkg: String, @@ -177,6 +186,10 @@ fun PluginSettingsContent( val hasGlobalParams = globalParams.isNotEmpty() val hasCapabilities = capabilities.isNotEmpty() val hasAnyResults = hasActions || hasCustomSettings || hasGlobalParams || hasCapabilities + val (requiredSettings, optionalSettings) = remember(customSettings) { partitionSettings(customSettings) } + val customSettingRequesters = remember(customSettings.keys) { + customSettings.keys.associateWith { BringIntoViewRequester() } + } val lockedEnumOptions = remember(manifest) { val result = mutableMapOf>() @@ -250,7 +263,8 @@ fun PluginSettingsContent( // Auto-scroll to requested setting or section LaunchedEffect(scrollToSetting, sectionIndices, customSettings) { if (scrollToSetting != null) { - val targetKey = if (customSettings.containsKey(scrollToSetting)) { + val isCustomSetting = customSettings.containsKey(scrollToSetting) + val targetKey = if (isCustomSetting) { "section_custom" } else if (capabilities.any { it.parameters?.containsKey(scrollToSetting) == true }) { val cap = capabilities.first { it.parameters?.containsKey(scrollToSetting) == true } @@ -264,6 +278,10 @@ fun PluginSettingsContent( val targetIndex = targetKey?.let { sectionIndices[it] } if (targetIndex != null) { lazyListState.animateScrollToItem(targetIndex) + if (isCustomSetting) { + withFrameNanos { } + customSettingRequesters[scrollToSetting]?.bringIntoView() + } } } } @@ -479,14 +497,18 @@ fun PluginSettingsContent( verticalArrangement = Arrangement.spacedBy(ToolkitTheme.spacing.mediumSmall) ) { listOf( - requiredTitle to customSettings.filterValues { it.required }, - optionalTitle to customSettings.filterValues { !it.required } + requiredTitle to requiredSettings, + optionalTitle to optionalSettings ).forEach { (groupTitle, groupSettings) -> if (groupSettings.isNotEmpty()) { PluginSettingGroupHeader(groupTitle, groupSettings.size) } groupSettings.forEach { (key, meta) -> - Column(modifier = Modifier.fillMaxWidth()) { + Column( + modifier = Modifier + .fillMaxWidth() + .bringIntoViewRequester(customSettingRequesters.getValue(key)) + ) { val value = store.settings[key] ?: meta.defaultValue DynamicParameterInput( name = key, diff --git a/composeApp/src/commonTest/kotlin/org/wip/plugintoolkit/features/plugin/ui/PluginSettingPartitionTest.kt b/composeApp/src/commonTest/kotlin/org/wip/plugintoolkit/features/plugin/ui/PluginSettingPartitionTest.kt new file mode 100644 index 00000000..93d238d9 --- /dev/null +++ b/composeApp/src/commonTest/kotlin/org/wip/plugintoolkit/features/plugin/ui/PluginSettingPartitionTest.kt @@ -0,0 +1,40 @@ +package org.wip.plugintoolkit.features.plugin.ui + +import org.wip.plugintoolkit.api.DataType +import org.wip.plugintoolkit.api.PrimitiveType +import org.wip.plugintoolkit.api.SettingMetadata +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertTrue + +class PluginSettingPartitionTest { + @Test + fun `partitions required and optional settings while preserving order`() { + val settings = linkedMapOf( + "optionalFirst" to setting(required = false), + "requiredFirst" to setting(required = true), + "requiredSecond" to setting(required = true), + "optionalSecond" to setting(required = false) + ) + + val (required, optional) = partitionSettings(settings) + + assertEquals(listOf("requiredFirst", "requiredSecond"), required.keys.toList()) + assertEquals(listOf("optionalFirst", "optionalSecond"), optional.keys.toList()) + assertEquals(settings.keys, (required.keys + optional.keys).toSet()) + } + + @Test + fun `empty settings produce two empty groups`() { + val (required, optional) = partitionSettings(emptyMap()) + + assertTrue(required.isEmpty()) + assertTrue(optional.isEmpty()) + } + + private fun setting(required: Boolean) = SettingMetadata( + description = "Setting", + type = DataType.Primitive(PrimitiveType.STRING), + required = required + ) +} From 9d7e9d54cfed5cace6cdc21c3a98ac10a2c6c9db Mon Sep 17 00:00:00 2001 From: Matteo Mekhail <67237370+matteoiscrying@users.noreply.github.com> Date: Sat, 15 Aug 2026 07:05:00 +1000 Subject: [PATCH 09/16] fix: resolve generated settings safely at runtime --- .../plugin/logic/PluginLifecycleManager.kt | 11 ++-- .../plugin/logic/PluginSettingsResolver.kt | 62 +++++++++++++++++++ .../viewmodel/PluginSettingsViewModel.kt | 44 +------------ .../PluginSettingsAutogenerationTest.kt | 25 +++++++- 4 files changed, 94 insertions(+), 48 deletions(-) create mode 100644 composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/PluginSettingsResolver.kt diff --git a/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/PluginLifecycleManager.kt b/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/PluginLifecycleManager.kt index 6119f11e..b93ce0c1 100644 --- a/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/PluginLifecycleManager.kt +++ b/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/PluginLifecycleManager.kt @@ -287,6 +287,7 @@ class PluginLifecycleManager( } val decryptedStore = store.copy(settings = decryptedSettings) + .withResolvedAutogeneratedSettings(manifest?.settings.orEmpty()) _pluginSettingsState.update { it + (pkg to decryptedStore) } return decryptedStore } @@ -296,7 +297,8 @@ class PluginLifecycleManager( val settingsFile = "${plugin.installPath}/settings.json" val manifest = getManifest(pkg) - val encryptedSettings = store.settings.mapValues { (key, value) -> + val resolvedStore = store.withResolvedAutogeneratedSettings(manifest?.settings.orEmpty()) + val encryptedSettings = resolvedStore.settings.mapValues { (key, value) -> val isSecret = manifest?.settings?.get(key)?.secret == true if (isSecret && value is kotlinx.serialization.json.JsonPrimitive && value.isString) { val encrypted = org.wip.plugintoolkit.core.utils.SecureStorage.encrypt(value.content) @@ -305,12 +307,12 @@ class PluginLifecycleManager( value } } - val storeToSave = store.copy(settings = encryptedSettings) + val storeToSave = resolvedStore.copy(settings = encryptedSettings) try { fileSystem.writeFile(settingsFile, json.encodeToString(storeToSave)) // Update cache with the decrypted store - _pluginSettingsState.update { it + (pkg to store) } + _pluginSettingsState.update { it + (pkg to resolvedStore) } } catch (t: Throwable) { Logger.e(t) { "Failed to save settings for $pkg" } } @@ -330,8 +332,9 @@ class PluginLifecycleManager( val installPath = plugin?.installPath ?: "" val jarFullPath = plugin?.let { "${it.installPath}/${it.jarFileName}" } - val storedSettings = overriddenSettings ?: loadPluginSettings(pkg) val actualManifest = manifest ?: getManifest(pkg) + val storedSettings = (overriddenSettings ?: loadPluginSettings(pkg)) + .withResolvedAutogeneratedSettings(actualManifest?.settings.orEmpty()) val mergedSettings = mutableMapOf() // 1. Manifest defaults diff --git a/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/PluginSettingsResolver.kt b/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/PluginSettingsResolver.kt new file mode 100644 index 00000000..4caebe43 --- /dev/null +++ b/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/PluginSettingsResolver.kt @@ -0,0 +1,62 @@ +package org.wip.plugintoolkit.features.plugin.logic + +import kotlinx.serialization.json.JsonElement +import kotlinx.serialization.json.JsonPrimitive +import org.wip.plugintoolkit.api.DataType +import org.wip.plugintoolkit.api.PrimitiveType +import org.wip.plugintoolkit.api.SettingMetadata +import org.wip.plugintoolkit.features.flows.logic.PathPatternResolver +import org.wip.plugintoolkit.features.plugin.model.PluginSettingsStore +import org.wip.plugintoolkit.features.plugin.utils.SettingsUtils + +internal fun resolveAutogeneratedSettings( + metadata: Map, + settings: Map, + additionalValues: Map = emptyMap() +): Map { + val resolvedSettings = settings.toMutableMap() + val defaults = metadata.mapNotNull { (key, value) -> value.defaultValue?.let { key to it } }.toMap() + + repeat(metadata.size.coerceAtLeast(1)) { + var changed = false + metadata.forEach { (key, settingMetadata) -> + val pattern = settingMetadata.autogeneratedPattern?.takeIf { it.isNotBlank() } ?: return@forEach + val availableValues = defaults + additionalValues + resolvedSettings + val stringValues = availableValues.mapValues { (valueKey, value) -> + val valueType = metadata[valueKey]?.type + if (valueType != null) SettingsUtils.jsonToString(value, valueType) else value.toString().trim('"') + } + val generated = runCatching { PathPatternResolver.tryResolve(pattern, stringValues) }.getOrNull() + ?: return@forEach + val generatedValue = generatedSettingValue(generated, settingMetadata.type) ?: return@forEach + if (resolvedSettings[key] != generatedValue) { + resolvedSettings[key] = generatedValue + changed = true + } + } + if (!changed) return resolvedSettings + } + + return resolvedSettings +} + +internal fun PluginSettingsStore.withResolvedAutogeneratedSettings( + metadata: Map +): PluginSettingsStore = copy( + settings = resolveAutogeneratedSettings(metadata, settings, globalParams) +) + +private fun generatedSettingValue(value: String, type: DataType): JsonElement? = when (type) { + is DataType.Primitive -> when (type.primitiveType) { + PrimitiveType.STRING, PrimitiveType.ANY, PrimitiveType.UNKNOWN -> JsonPrimitive(value) + PrimitiveType.BOOLEAN -> value.toBooleanStrictOrNull()?.let(::JsonPrimitive) + PrimitiveType.INT -> value.toIntOrNull()?.let(::JsonPrimitive) + PrimitiveType.LONG -> value.toLongOrNull()?.let(::JsonPrimitive) + PrimitiveType.SHORT -> value.toShortOrNull()?.let { JsonPrimitive(it.toInt()) } + PrimitiveType.BYTE -> value.toByteOrNull()?.let { JsonPrimitive(it.toInt()) } + PrimitiveType.DOUBLE -> value.toDoubleOrNull()?.let(::JsonPrimitive) + PrimitiveType.FLOAT -> value.toFloatOrNull()?.let { JsonPrimitive(it.toDouble()) } + PrimitiveType.UNIT -> null + } + else -> value.takeIf { it.isNotBlank() }?.let { runCatching { SettingsUtils.stringToJson(it, type) }.getOrNull() } +} diff --git a/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/viewmodel/PluginSettingsViewModel.kt b/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/viewmodel/PluginSettingsViewModel.kt index 6be18748..3b614b0f 100644 --- a/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/viewmodel/PluginSettingsViewModel.kt +++ b/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/viewmodel/PluginSettingsViewModel.kt @@ -7,45 +7,11 @@ import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.update import kotlinx.coroutines.launch import kotlinx.serialization.json.JsonElement -import org.wip.plugintoolkit.api.SettingMetadata -import org.wip.plugintoolkit.features.flows.logic.PathPatternResolver import org.wip.plugintoolkit.features.job.logic.JobManager import org.wip.plugintoolkit.features.job.model.JobStatus import org.wip.plugintoolkit.features.plugin.logic.PluginManager +import org.wip.plugintoolkit.features.plugin.logic.withResolvedAutogeneratedSettings import org.wip.plugintoolkit.features.plugin.model.PluginSettingsStore -import org.wip.plugintoolkit.features.plugin.utils.SettingsUtils - -internal fun resolveAutogeneratedSettings( - metadata: Map, - settings: Map, - additionalValues: Map = emptyMap() -): Map { - val resolvedSettings = settings.toMutableMap() - val defaults = metadata.mapNotNull { (key, value) -> value.defaultValue?.let { key to it } }.toMap() - - repeat(metadata.size.coerceAtLeast(1)) { - var changed = false - metadata.forEach { (key, settingMetadata) -> - val pattern = settingMetadata.autogeneratedPattern?.takeIf { it.isNotBlank() } ?: return@forEach - val availableValues = defaults + additionalValues + resolvedSettings - val stringValues = availableValues.mapValues { (valueKey, value) -> - val valueType = metadata[valueKey]?.type - if (valueType != null) SettingsUtils.jsonToString(value, valueType) else value.toString().trim('"') - } - val generated = runCatching { PathPatternResolver.tryResolve(pattern, stringValues) } - .getOrNull() - .orEmpty() - val generatedValue = SettingsUtils.stringToJson(generated, settingMetadata.type) - if (resolvedSettings[key] != generatedValue) { - resolvedSettings[key] = generatedValue - changed = true - } - } - if (!changed) return resolvedSettings - } - - return resolvedSettings -} class PluginSettingsViewModel( val pkg: String, @@ -64,13 +30,7 @@ class PluginSettingsViewModel( private fun PluginSettingsStore.withAutogeneratedSettings(): PluginSettingsStore { val settingMetadata = manifest?.settings ?: return this - return copy( - settings = resolveAutogeneratedSettings( - metadata = settingMetadata, - settings = settings, - additionalValues = globalParams - ) - ) + return withResolvedAutogeneratedSettings(settingMetadata) } init { diff --git a/composeApp/src/commonTest/kotlin/org/wip/plugintoolkit/features/plugin/viewmodel/PluginSettingsAutogenerationTest.kt b/composeApp/src/commonTest/kotlin/org/wip/plugintoolkit/features/plugin/viewmodel/PluginSettingsAutogenerationTest.kt index 37a6191b..2bdeab26 100644 --- a/composeApp/src/commonTest/kotlin/org/wip/plugintoolkit/features/plugin/viewmodel/PluginSettingsAutogenerationTest.kt +++ b/composeApp/src/commonTest/kotlin/org/wip/plugintoolkit/features/plugin/viewmodel/PluginSettingsAutogenerationTest.kt @@ -4,6 +4,7 @@ import kotlinx.serialization.json.JsonPrimitive import org.wip.plugintoolkit.api.DataType import org.wip.plugintoolkit.api.PrimitiveType import org.wip.plugintoolkit.api.SettingMetadata +import org.wip.plugintoolkit.features.plugin.logic.resolveAutogeneratedSettings import kotlin.test.Test import kotlin.test.assertEquals @@ -29,12 +30,32 @@ class PluginSettingsAutogenerationTest { } @Test - fun `derived setting is cleared when dependency is missing`() { + fun `derived setting preserves the last explicit value when dependency is missing`() { val result = resolveAutogeneratedSettings( metadata = metadata, settings = mapOf("output" to JsonPrimitive("stale.result")) ) - assertEquals(JsonPrimitive(""), result["output"]) + assertEquals(JsonPrimitive("stale.result"), result["output"]) + } + + @Test + fun `invalid generated value does not replace a valid persisted value`() { + val numberType = DataType.Primitive(PrimitiveType.INT) + val result = resolveAutogeneratedSettings( + metadata = metadata + ( + "count" to SettingMetadata( + description = "Count", + type = numberType, + autogeneratedPattern = "{input.nameWithoutExtension}" + ) + ), + settings = mapOf( + "input" to JsonPrimitive("photo.png"), + "count" to JsonPrimitive(42) + ) + ) + + assertEquals(JsonPrimitive(42), result["count"]) } } From 144d594989a7b26c6444f737300bc59dffb4d8e4 Mon Sep 17 00:00:00 2001 From: Matteo Mekhail <67237370+matteoiscrying@users.noreply.github.com> Date: Sat, 15 Aug 2026 07:08:04 +1000 Subject: [PATCH 10/16] fix: preserve safe paths and isolate cache operations --- .../plugin/logic/DefaultPluginFileSystem.kt | 34 +++++++++++++++++-- .../plugin/logic/PluginLifecycleManager.kt | 2 +- .../logic/SandboxFileSystemSecurityTest.kt | 22 ++++++++++++ .../org/wip/plugintoolkit/api/RelativePath.kt | 22 +++++++----- .../wip/plugintoolkit/api/RelativePathTest.kt | 2 ++ 5 files changed, 70 insertions(+), 12 deletions(-) diff --git a/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/DefaultPluginFileSystem.kt b/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/DefaultPluginFileSystem.kt index 100ddb77..a68b6753 100644 --- a/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/DefaultPluginFileSystem.kt +++ b/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/DefaultPluginFileSystem.kt @@ -121,8 +121,8 @@ class DefaultPluginFileSystem( override fun getBasePath(): String = basePath companion object { - fun createCacheOnly(pluginInstallPath: String): PluginFileSystem { - return DefaultPluginFileSystem(pluginInstallPath).let { fs -> + fun createCacheOnly(pluginInstallPath: String, jarPath: String? = null): PluginFileSystem { + return DefaultPluginFileSystem(pluginInstallPath, jarPath).let { fs -> // Create a variant that uses cachePath as basePath object : PluginFileSystem by fs { override fun getBasePath(): String = fs.cachePath @@ -141,6 +141,13 @@ class DefaultPluginFileSystem( override suspend fun exists(relativePath: RelativePath): Boolean = SystemFileSystem.exists(fs.resolveCachePath(relativePath)) + override suspend fun listFiles(relativePath: RelativePath): List { + val path = fs.resolveCachePath(relativePath) + if (!SystemFileSystem.exists(path)) return emptyList() + if (SystemFileSystem.metadataOrNull(path)?.isDirectory != true) return emptyList() + return SystemFileSystem.list(path).map { it.name } + } + override suspend fun deleteFile(relativePath: RelativePath): Result = fs.deleteFromCache(relativePath) @@ -153,6 +160,11 @@ class DefaultPluginFileSystem( require(relativePath.value.isNotEmpty()) { "The plugin cache root cannot be deleted" } fs.cacheOperations.deleteDirectory(fs.resolveCachePath(relativePath), recursive) } + + override suspend fun extractResource( + resourcePath: String, + targetRelativePath: RelativePath + ): Result = fs.extractResourceToCache(resourcePath, targetRelativePath) } } } @@ -207,4 +219,22 @@ class DefaultPluginFileSystem( Result.failure(e) } } + + private suspend fun extractResourceToCache( + resourcePath: String, + targetRelativePath: RelativePath + ): Result { + if (resourcePath.contains("..") || resourcePath.startsWith("/") || + resourcePath.startsWith("\\") || resourcePath.contains("\u0000")) { + return Result.failure(SecurityException("Invalid resource path: $resourcePath")) + } + return runCatching { + withContext(loomDispatcher) { + val jar = jarPath ?: error("No JAR path configured for resource extraction") + val data = org.wip.plugintoolkit.core.utils.PlatformUtils.readBytesFromZip(jar, resourcePath) + ?: error("Resource not found in JAR: $resourcePath") + writeToCache(targetRelativePath, data).getOrThrow() + } + } + } } diff --git a/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/PluginLifecycleManager.kt b/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/PluginLifecycleManager.kt index 6119f11e..bd8d897d 100644 --- a/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/PluginLifecycleManager.kt +++ b/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/PluginLifecycleManager.kt @@ -359,7 +359,7 @@ class PluginLifecycleManager( logger = pluginLogger, progress = progressReporter, fileSystem = DefaultPluginFileSystem(installPath, jarFullPath), - cacheFileSystem = DefaultPluginFileSystem.createCacheOnly(installPath), + cacheFileSystem = DefaultPluginFileSystem.createCacheOnly(installPath, jarFullPath), executionFileSystem = executionFileSystem ?: DefaultExecutionFileSystem("${installPath}/temp_execution"), hostFileSystem = HostFileSystemImpl(allowedPaths, isDestructiveAllowed), settings = mergedSettings, diff --git a/composeApp/src/jvmTest/kotlin/org/wip/plugintoolkit/features/plugin/logic/SandboxFileSystemSecurityTest.kt b/composeApp/src/jvmTest/kotlin/org/wip/plugintoolkit/features/plugin/logic/SandboxFileSystemSecurityTest.kt index 5a88da3e..55b1f401 100644 --- a/composeApp/src/jvmTest/kotlin/org/wip/plugintoolkit/features/plugin/logic/SandboxFileSystemSecurityTest.kt +++ b/composeApp/src/jvmTest/kotlin/org/wip/plugintoolkit/features/plugin/logic/SandboxFileSystemSecurityTest.kt @@ -8,10 +8,13 @@ import java.nio.file.Files import java.nio.file.Path import java.nio.file.SimpleFileVisitor import java.nio.file.attribute.BasicFileAttributes +import java.util.jar.JarEntry +import java.util.jar.JarOutputStream import kotlin.test.AfterTest import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.test.assertFailsWith +import kotlin.test.assertEquals import kotlin.test.assertTrue class SandboxFileSystemSecurityTest { @@ -82,6 +85,25 @@ class SandboxFileSystemSecurityTest { verifySymlinkIsContained(fileSystem, install.resolve("cache"), outside) } + @Test + fun cacheOnlyVariantListsAndExtractsResourcesInsideCache() = runTest { + val install = testRoot.resolve("plugin") + Files.createDirectories(install) + val jar = install.resolve("plugin.jar") + JarOutputStream(Files.newOutputStream(jar)).use { output -> + output.putNextEntry(JarEntry("assets/example.txt")) + output.write("resource".encodeToByteArray()) + output.closeEntry() + } + val fileSystem = DefaultPluginFileSystem.createCacheOnly(install.toString(), jar.toString()) + val target = RelativePath.from("nested/example.txt").getOrThrow() + + assertTrue(fileSystem.extractResource("assets/example.txt", target).isSuccess) + assertEquals("resource", fileSystem.readTextFile(target)) + assertEquals(listOf("example.txt"), fileSystem.listFiles(RelativePath.from("nested").getOrThrow())) + assertTrue(Files.notExists(install.resolve("files/nested/example.txt"))) + } + private fun createOutsideSecret(): Path { val outside = Files.createDirectories(testRoot.resolve("outside")) Files.writeString(outside.resolve("secret.txt"), "must survive") diff --git a/plugin-api/src/commonMain/kotlin/org/wip/plugintoolkit/api/RelativePath.kt b/plugin-api/src/commonMain/kotlin/org/wip/plugintoolkit/api/RelativePath.kt index 650ae950..a9b1104c 100644 --- a/plugin-api/src/commonMain/kotlin/org/wip/plugintoolkit/api/RelativePath.kt +++ b/plugin-api/src/commonMain/kotlin/org/wip/plugintoolkit/api/RelativePath.kt @@ -41,18 +41,22 @@ value class RelativePath private constructor(val value: String) { return Result.failure(SecurityException("Path traversal attempt detected: $normalized")) } - val sanitized = normalized - .replace("\u2024", ".") - .replace("\uFF0E", ".") - .replace("\u3002", ".") - .replace("%2e", ".", ignoreCase = true) - .replace('\\', '/') - - val segments = sanitized.split('/').filter { it.isNotEmpty() && it != "." } - if (segments.any { it == ".." || it.contains(Regex("%25(?:2e|2f|5c)", RegexOption.IGNORE_CASE)) }) { + val segments = normalized.replace('\\', '/').split('/').filter { it.isNotEmpty() && it != "." } + val validationSegments = segments.map { segment -> + segment + .replace("\u2024", ".") + .replace("\uFF0E", ".") + .replace("\u3002", ".") + .replace("%2e", ".", ignoreCase = true) + } + if (validationSegments.any { + it == ".." || it.contains(Regex("%25(?:2e|2f|5c)", RegexOption.IGNORE_CASE)) + } + ) { return Result.failure(SecurityException("Path traversal attempt detected: $normalized")) } + // Validation uses a security-normalized view, but the filename itself is not decoded or rewritten. return Result.success(RelativePath(segments.joinToString("/"))) } } diff --git a/plugin-api/src/commonTest/kotlin/org/wip/plugintoolkit/api/RelativePathTest.kt b/plugin-api/src/commonTest/kotlin/org/wip/plugintoolkit/api/RelativePathTest.kt index 400f17d5..b328f843 100644 --- a/plugin-api/src/commonTest/kotlin/org/wip/plugintoolkit/api/RelativePathTest.kt +++ b/plugin-api/src/commonTest/kotlin/org/wip/plugintoolkit/api/RelativePathTest.kt @@ -56,5 +56,7 @@ class RelativePathTest { assertEquals(RelativePath.ROOT, "././".toRelativePath().getOrThrow()) assertEquals("foo/bar", "foo/./bar".toRelativePath().getOrThrow().value) assertEquals("foo/bar", "foo\\bar".toRelativePath().getOrThrow().value) + assertEquals("file%2ename.txt", "file%2ename.txt".toRelativePath().getOrThrow().value) + assertEquals("file\u2024txt", "file\u2024txt".toRelativePath().getOrThrow().value) } } From 153c666de6ab313329402f8dfb4e828310a63ed7 Mon Sep 17 00:00:00 2001 From: Matteo Mekhail <67237370+matteoiscrying@users.noreply.github.com> Date: Sat, 15 Aug 2026 07:12:22 +1000 Subject: [PATCH 11/16] fix: preserve unchanged settings cache identity --- .../features/plugin/logic/PluginSettingsResolver.kt | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/PluginSettingsResolver.kt b/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/PluginSettingsResolver.kt index 4caebe43..4c5a413b 100644 --- a/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/PluginSettingsResolver.kt +++ b/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/PluginSettingsResolver.kt @@ -42,9 +42,10 @@ internal fun resolveAutogeneratedSettings( internal fun PluginSettingsStore.withResolvedAutogeneratedSettings( metadata: Map -): PluginSettingsStore = copy( - settings = resolveAutogeneratedSettings(metadata, settings, globalParams) -) +): PluginSettingsStore { + val resolved = resolveAutogeneratedSettings(metadata, settings, globalParams) + return if (resolved == settings) this else copy(settings = resolved) +} private fun generatedSettingValue(value: String, type: DataType): JsonElement? = when (type) { is DataType.Primitive -> when (type.primitiveType) { From a9f0142042a83e1cd3d063cfa3ca0d85a1ff96cf Mon Sep 17 00:00:00 2001 From: Matteo Mekhail <67237370+matteoiscrying@users.noreply.github.com> Date: Sat, 15 Aug 2026 07:44:01 +1000 Subject: [PATCH 12/16] fix: handle unavailable sandbox roots safely --- .../plugin/logic/DefaultExecutionFileSystem.kt | 9 +++++---- .../plugin/logic/DefaultPluginFileSystem.kt | 17 +++++++++-------- .../plugin/logic/SandboxFileOperations.kt | 7 +++++++ .../logic/SandboxFileSystemSecurityTest.kt | 16 ++++++++++++++++ 4 files changed, 37 insertions(+), 12 deletions(-) diff --git a/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/DefaultExecutionFileSystem.kt b/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/DefaultExecutionFileSystem.kt index 818461ff..22b82374 100644 --- a/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/DefaultExecutionFileSystem.kt +++ b/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/DefaultExecutionFileSystem.kt @@ -24,13 +24,13 @@ class DefaultExecutionFileSystem( } override suspend fun readFile(relativePath: RelativePath): ByteArray? { - val path = resolvePath(relativePath) + val path = sandboxOperations.resolveIfRootExists(relativePath) ?: return null if (!SystemFileSystem.exists(path)) return null return SystemFileSystem.source(path).buffered().use { it.readByteArray() } } override suspend fun readTextFile(relativePath: RelativePath): String? { - val path = resolvePath(relativePath) + val path = sandboxOperations.resolveIfRootExists(relativePath) ?: return null if (!SystemFileSystem.exists(path)) return null return SystemFileSystem.source(path).buffered().use { it.readString() } } @@ -58,11 +58,12 @@ class DefaultExecutionFileSystem( } override suspend fun exists(relativePath: RelativePath): Boolean { - return SystemFileSystem.exists(resolvePath(relativePath)) + val path = sandboxOperations.resolveIfRootExists(relativePath) ?: return false + return SystemFileSystem.exists(path) } override suspend fun listFiles(relativePath: RelativePath): List { - val path = resolvePath(relativePath) + val path = sandboxOperations.resolveIfRootExists(relativePath) ?: return emptyList() if (!SystemFileSystem.exists(path)) return emptyList() val metadata = SystemFileSystem.metadataOrNull(path) if (metadata?.isDirectory != true) return emptyList() diff --git a/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/DefaultPluginFileSystem.kt b/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/DefaultPluginFileSystem.kt index a68b6753..f0241242 100644 --- a/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/DefaultPluginFileSystem.kt +++ b/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/DefaultPluginFileSystem.kt @@ -31,13 +31,13 @@ class DefaultPluginFileSystem( } override suspend fun readFile(relativePath: RelativePath): ByteArray? { - val path = resolvePath(relativePath) + val path = filesOperations.resolveIfRootExists(relativePath) ?: return null if (!SystemFileSystem.exists(path)) return null return SystemFileSystem.source(path).buffered().use { it.readByteArray() } } override suspend fun readTextFile(relativePath: RelativePath): String? { - val path = resolvePath(relativePath) + val path = filesOperations.resolveIfRootExists(relativePath) ?: return null if (!SystemFileSystem.exists(path)) return null return SystemFileSystem.source(path).buffered().use { it.readString() } } @@ -65,11 +65,12 @@ class DefaultPluginFileSystem( } override suspend fun exists(relativePath: RelativePath): Boolean { - return SystemFileSystem.exists(resolvePath(relativePath)) + val path = filesOperations.resolveIfRootExists(relativePath) ?: return false + return SystemFileSystem.exists(path) } override suspend fun listFiles(relativePath: RelativePath): List { - val path = resolvePath(relativePath) + val path = filesOperations.resolveIfRootExists(relativePath) ?: return emptyList() if (!SystemFileSystem.exists(path)) return emptyList() val metadata = SystemFileSystem.metadataOrNull(path) if (metadata?.isDirectory != true) return emptyList() @@ -139,10 +140,10 @@ class DefaultPluginFileSystem( fs.writeTextToCache(relativePath, text) override suspend fun exists(relativePath: RelativePath): Boolean = - SystemFileSystem.exists(fs.resolveCachePath(relativePath)) + fs.cacheOperations.resolveIfRootExists(relativePath)?.let(SystemFileSystem::exists) ?: false override suspend fun listFiles(relativePath: RelativePath): List { - val path = fs.resolveCachePath(relativePath) + val path = fs.cacheOperations.resolveIfRootExists(relativePath) ?: return emptyList() if (!SystemFileSystem.exists(path)) return emptyList() if (SystemFileSystem.metadataOrNull(path)?.isDirectory != true) return emptyList() return SystemFileSystem.list(path).map { it.name } @@ -175,13 +176,13 @@ class DefaultPluginFileSystem( } private suspend fun readFromCache(relativePath: RelativePath): ByteArray? { - val path = resolveCachePath(relativePath) + val path = cacheOperations.resolveIfRootExists(relativePath) ?: return null if (!SystemFileSystem.exists(path)) return null return SystemFileSystem.source(path).buffered().use { it.readByteArray() } } private suspend fun readTextFromCache(relativePath: RelativePath): String? { - val path = resolveCachePath(relativePath) + val path = cacheOperations.resolveIfRootExists(relativePath) ?: return null if (!SystemFileSystem.exists(path)) return null return SystemFileSystem.source(path).buffered().use { it.readString() } } diff --git a/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/SandboxFileOperations.kt b/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/SandboxFileOperations.kt index 7503baa1..e4e2d902 100644 --- a/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/SandboxFileOperations.kt +++ b/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/SandboxFileOperations.kt @@ -5,6 +5,7 @@ import org.wip.plugintoolkit.api.RelativePath import java.nio.file.FileVisitResult import java.nio.file.Files import java.nio.file.LinkOption +import java.nio.file.NoSuchFileException import java.nio.file.Path as NioPath import java.nio.file.Paths import java.nio.file.SimpleFileVisitor @@ -36,6 +37,12 @@ internal class SandboxFileOperations(root: String) { return Path(candidate.toString()) } + fun resolveIfRootExists(relativePath: RelativePath): Path? = try { + resolve(relativePath) + } catch (_: NoSuchFileException) { + null + } + fun deleteDirectory(path: Path, recursive: Boolean) { val nioPath = Paths.get(path.toString()) if (!Files.exists(nioPath, LinkOption.NOFOLLOW_LINKS)) return diff --git a/composeApp/src/jvmTest/kotlin/org/wip/plugintoolkit/features/plugin/logic/SandboxFileSystemSecurityTest.kt b/composeApp/src/jvmTest/kotlin/org/wip/plugintoolkit/features/plugin/logic/SandboxFileSystemSecurityTest.kt index 55b1f401..2034d3c0 100644 --- a/composeApp/src/jvmTest/kotlin/org/wip/plugintoolkit/features/plugin/logic/SandboxFileSystemSecurityTest.kt +++ b/composeApp/src/jvmTest/kotlin/org/wip/plugintoolkit/features/plugin/logic/SandboxFileSystemSecurityTest.kt @@ -15,6 +15,8 @@ import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.test.assertFailsWith import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertNull import kotlin.test.assertTrue class SandboxFileSystemSecurityTest { @@ -104,6 +106,20 @@ class SandboxFileSystemSecurityTest { assertTrue(Files.notExists(install.resolve("files/nested/example.txt"))) } + @Test + fun missingSandboxRootUsesReadSemanticsAndRejectsWrites() = runTest { + val install = testRoot.resolve("plugin") + val fileSystem = DefaultPluginFileSystem(install.toString()) + val file = RelativePath.from("missing.txt").getOrThrow() + Files.delete(install.resolve("files")) + + assertNull(fileSystem.readFile(file)) + assertNull(fileSystem.readTextFile(file)) + assertFalse(fileSystem.exists(file)) + assertEquals(emptyList(), fileSystem.listFiles()) + assertTrue(fileSystem.writeTextFile(file, "data").isFailure) + } + private fun createOutsideSecret(): Path { val outside = Files.createDirectories(testRoot.resolve("outside")) Files.writeString(outside.resolve("secret.txt"), "must survive") From 2f15ae110076fbb0641e37215258aeaa1b184a70 Mon Sep 17 00:00:00 2001 From: Matteo Mekhail <67237370+matteoiscrying@users.noreply.github.com> Date: Sat, 15 Aug 2026 09:49:03 +1000 Subject: [PATCH 13/16] fix: apply manifest defaults to capability gates --- .../features/flows/ui/PaletteSidebar.kt | 6 ++++-- .../plugin/ui/DirectExecutionSidebar.kt | 3 ++- .../plugin/model/PluginSettingDefaultsTest.kt | 21 +++++++++++++++++++ 3 files changed, 27 insertions(+), 3 deletions(-) diff --git a/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/flows/ui/PaletteSidebar.kt b/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/flows/ui/PaletteSidebar.kt index 6ae40b5a..b050683e 100644 --- a/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/flows/ui/PaletteSidebar.kt +++ b/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/flows/ui/PaletteSidebar.kt @@ -53,6 +53,7 @@ import org.wip.plugintoolkit.api.PluginEntry import org.wip.plugintoolkit.core.theme.ToolkitTheme import org.wip.plugintoolkit.features.flows.model.Flow import org.wip.plugintoolkit.features.plugin.logic.PluginManager +import org.wip.plugintoolkit.features.plugin.model.resolveProvidedValues import org.wip.plugintoolkit.features.plugin.ui.lockedClickInterceptor import org.wip.plugintoolkit.shared.components.ToolkitTextField import plugintoolkit.composeapp.generated.resources.Res @@ -248,8 +249,9 @@ private fun CapabilitiesPalette( ) ) caps.forEach { cap -> - val isReady = remember(cap, settingsStore.settings, manifest?.settings) { - cap.isReady(settingsStore.settings, manifest?.settings) + val providedSettings = settingsStore.resolveProvidedValues(manifest) + val isReady = remember(cap, providedSettings, manifest?.settings) { + cap.isReady(providedSettings, manifest?.settings) } val targetSettingKey = cap.requiredLocks.firstOrNull() diff --git a/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/ui/DirectExecutionSidebar.kt b/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/ui/DirectExecutionSidebar.kt index 04b1be57..adcf844e 100644 --- a/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/ui/DirectExecutionSidebar.kt +++ b/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/ui/DirectExecutionSidebar.kt @@ -50,6 +50,7 @@ import org.wip.plugintoolkit.api.Capability import org.wip.plugintoolkit.api.PluginEntry import org.wip.plugintoolkit.core.model.localized import org.wip.plugintoolkit.core.theme.ToolkitTheme +import org.wip.plugintoolkit.features.plugin.model.resolveProvidedValues import org.wip.plugintoolkit.shared.components.ToolkitTextField import org.wip.plugintoolkit.shared.components.sidebar.NavigationSidebar import org.wip.plugintoolkit.shared.components.sidebar.SidebarElement @@ -167,7 +168,7 @@ fun DirectExecutionSidebar( val manifest = plugin.getManifest().getOrThrow() val pluginManager: org.wip.plugintoolkit.features.plugin.logic.PluginManager = org.koin.compose.koinInject() val settingsStore = pluginManager.loadPluginSettings(pluginId) - val settings = settingsStore.settings + settingsStore.globalParams + val settings = settingsStore.resolveProvidedValues(manifest) val pluginLocksState by pluginManager.pluginLocksState.collectAsState() val locks = pluginLocksState[pluginId] ?: pluginLocksState.values.fold(emptyMap()) { acc, map -> acc + map } diff --git a/composeApp/src/commonTest/kotlin/org/wip/plugintoolkit/features/plugin/model/PluginSettingDefaultsTest.kt b/composeApp/src/commonTest/kotlin/org/wip/plugintoolkit/features/plugin/model/PluginSettingDefaultsTest.kt index 6b645980..7b1765df 100644 --- a/composeApp/src/commonTest/kotlin/org/wip/plugintoolkit/features/plugin/model/PluginSettingDefaultsTest.kt +++ b/composeApp/src/commonTest/kotlin/org/wip/plugintoolkit/features/plugin/model/PluginSettingDefaultsTest.kt @@ -1,15 +1,19 @@ package org.wip.plugintoolkit.features.plugin.model import kotlinx.serialization.json.JsonPrimitive +import org.wip.plugintoolkit.api.Capability import org.wip.plugintoolkit.api.DataType import org.wip.plugintoolkit.api.PluginInfo import org.wip.plugintoolkit.api.PluginManifest import org.wip.plugintoolkit.api.PrimitiveType import org.wip.plugintoolkit.api.Requirements import org.wip.plugintoolkit.api.SettingMetadata +import org.wip.plugintoolkit.features.plugin.utils.CapabilityLockStatus +import org.wip.plugintoolkit.features.plugin.utils.CapabilityLockUtils import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFalse +import kotlin.test.assertTrue class PluginSettingDefaultsTest { private val manifest = PluginManifest( @@ -62,4 +66,21 @@ class PluginSettingDefaultsTest { assertEquals(JsonPrimitive("https://custom.test"), store.resolveCustomSettings(manifest)["endpoint"]) assertEquals(JsonPrimitive("global-collision"), store.resolveProvidedValues(manifest)["endpoint"]) } + + @Test + fun `manifest defaults unlock capability gates before settings are persisted`() { + val capability = Capability( + name = "call", + description = "Call the configured endpoint", + returnType = DataType.Primitive(PrimitiveType.STRING), + requiresSettings = listOf("endpoint") + ) + val provided = PluginSettingsStore().resolveProvidedValues(manifest) + + assertTrue(capability.isReady(provided, manifest.settings)) + assertTrue( + CapabilityLockUtils.checkCapabilityLockStatus(capability, emptyMap(), provided) is + CapabilityLockStatus.Unlocked + ) + } } From e203a5c0d805d9ce725ef3f2c2e02d0b1c296e72 Mon Sep 17 00:00:00 2001 From: Matteo Mekhail <67237370+matteoiscrying@users.noreply.github.com> Date: Sat, 15 Aug 2026 10:07:11 +1000 Subject: [PATCH 14/16] fix: keep cache filesystem operations isolated --- .../plugin/logic/DefaultPluginFileSystem.kt | 37 ++++++++++++++++++- .../logic/SandboxFileSystemSecurityTest.kt | 35 ++++++++++++++++++ 2 files changed, 71 insertions(+), 1 deletion(-) diff --git a/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/DefaultPluginFileSystem.kt b/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/DefaultPluginFileSystem.kt index f0241242..3e32e1db 100644 --- a/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/DefaultPluginFileSystem.kt +++ b/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/DefaultPluginFileSystem.kt @@ -1,6 +1,8 @@ package org.wip.plugintoolkit.features.plugin.logic import kotlinx.coroutines.withContext +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.flow import kotlinx.io.buffered import kotlinx.io.files.Path import kotlinx.io.files.SystemFileSystem @@ -125,7 +127,7 @@ class DefaultPluginFileSystem( fun createCacheOnly(pluginInstallPath: String, jarPath: String? = null): PluginFileSystem { return DefaultPluginFileSystem(pluginInstallPath, jarPath).let { fs -> // Create a variant that uses cachePath as basePath - object : PluginFileSystem by fs { + object : PluginFileSystem { override fun getBasePath(): String = fs.cachePath override suspend fun readFile(relativePath: RelativePath): ByteArray? = fs.readFromCache(relativePath) @@ -139,6 +141,39 @@ class DefaultPluginFileSystem( override suspend fun writeTextFile(relativePath: RelativePath, text: String): Result = fs.writeTextToCache(relativePath, text) + // Keep every compound/stream operation explicitly cache-routed. In + // particular, do not use Kotlin interface delegation here: generated + // forwards would bypass these overrides and touch persistent files. + override suspend fun readStream(relativePath: RelativePath): Flow = flow { + fs.readFromCache(relativePath)?.let { emit(it) } + } + + override suspend fun writeStream( + relativePath: RelativePath, + stream: Flow + ): Result = runCatching { + val bytes = mutableListOf() + stream.collect { chunk -> chunk.forEach { byte -> bytes.add(byte) } } + fs.writeToCache(relativePath, bytes.toByteArray()).getOrThrow() + } + + override suspend fun copyFile( + source: RelativePath, + destination: RelativePath + ): Result = runCatching { + val content = fs.readFromCache(source) + ?: throw IllegalArgumentException("Source file does not exist") + fs.writeToCache(destination, content).getOrThrow() + } + + override suspend fun moveFile( + source: RelativePath, + destination: RelativePath + ): Result = runCatching { + copyFile(source, destination).getOrThrow() + fs.deleteFromCache(source).getOrThrow() + } + override suspend fun exists(relativePath: RelativePath): Boolean = fs.cacheOperations.resolveIfRootExists(relativePath)?.let(SystemFileSystem::exists) ?: false diff --git a/composeApp/src/jvmTest/kotlin/org/wip/plugintoolkit/features/plugin/logic/SandboxFileSystemSecurityTest.kt b/composeApp/src/jvmTest/kotlin/org/wip/plugintoolkit/features/plugin/logic/SandboxFileSystemSecurityTest.kt index 2034d3c0..63059d18 100644 --- a/composeApp/src/jvmTest/kotlin/org/wip/plugintoolkit/features/plugin/logic/SandboxFileSystemSecurityTest.kt +++ b/composeApp/src/jvmTest/kotlin/org/wip/plugintoolkit/features/plugin/logic/SandboxFileSystemSecurityTest.kt @@ -1,5 +1,6 @@ package org.wip.plugintoolkit.features.plugin.logic +import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.test.runTest import org.wip.plugintoolkit.api.RelativePath import org.wip.plugintoolkit.api.ScopedFileSystem @@ -106,6 +107,40 @@ class SandboxFileSystemSecurityTest { assertTrue(Files.notExists(install.resolve("files/nested/example.txt"))) } + @Test + fun cacheOnlyCompoundAndStreamOperationsNeverTouchPersistentFiles() = runTest { + val install = testRoot.resolve("plugin") + val fileSystem = DefaultPluginFileSystem.createCacheOnly(install.toString()) + val source = RelativePath.from("source.bin").getOrThrow() + val copied = RelativePath.from("copied.bin").getOrThrow() + val moved = RelativePath.from("moved.bin").getOrThrow() + val streamed = RelativePath.from("streamed.bin").getOrThrow() + + Files.writeString(install.resolve("files/source.bin"), "persistent") + assertTrue(fileSystem.writeFile(source, "cache".encodeToByteArray()).isSuccess) + + assertTrue(fileSystem.copyFile(source, copied).isSuccess) + assertEquals("cache", Files.readString(install.resolve("cache/copied.bin"))) + assertTrue(Files.notExists(install.resolve("files/copied.bin"))) + + assertTrue(fileSystem.moveFile(source, moved).isSuccess) + assertTrue(Files.notExists(install.resolve("cache/source.bin"))) + assertEquals("cache", Files.readString(install.resolve("cache/moved.bin"))) + assertEquals("persistent", Files.readString(install.resolve("files/source.bin"))) + assertTrue(Files.notExists(install.resolve("files/moved.bin"))) + + assertTrue( + fileSystem.writeStream( + streamed, + flowOf("stream-".encodeToByteArray(), "cache".encodeToByteArray()) + ).isSuccess + ) + val chunks = mutableListOf() + fileSystem.readStream(streamed).collect { chunks.add(it) } + assertEquals("stream-cache", chunks.flatMap { it.asIterable() }.toByteArray().decodeToString()) + assertTrue(Files.notExists(install.resolve("files/streamed.bin"))) + } + @Test fun missingSandboxRootUsesReadSemanticsAndRejectsWrites() = runTest { val install = testRoot.resolve("plugin") From 3927e3749725c5d08f5dd1ca4a84bbe155acd1ca Mon Sep 17 00:00:00 2001 From: Matteo Mekhail <67237370+matteoiscrying@users.noreply.github.com> Date: Sat, 15 Aug 2026 10:08:22 +1000 Subject: [PATCH 15/16] Revert "Merge branch 'codex/issue-10-plugin-filesystem' into codex/issue-11-custom-setting-defaults" This reverts commit ce2f1a83e22acd9d69a76d040a342ac8f25d2394, reversing changes made to 2f15ae110076fbb0641e37215258aeaa1b184a70. --- .../logic/DefaultExecutionFileSystem.kt | 39 ++-- .../plugin/logic/DefaultPluginFileSystem.kt | 126 +++---------- .../plugin/logic/PluginLifecycleManager.kt | 2 +- .../plugin/logic/SandboxFileOperations.kt | 69 ------- .../logic/DefaultExecutionFileSystemTest.kt | 18 -- .../logic/SandboxFileSystemSecurityTest.kt | 177 ------------------ docs/PluginDevelopment.md | 1 - .../org/wip/plugintoolkit/api/Interfaces.kt | 4 - .../org/wip/plugintoolkit/api/RelativePath.kt | 31 ++- .../wip/plugintoolkit/api/RelativePathTest.kt | 6 - 10 files changed, 64 insertions(+), 409 deletions(-) delete mode 100644 composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/SandboxFileOperations.kt delete mode 100644 composeApp/src/jvmTest/kotlin/org/wip/plugintoolkit/features/plugin/logic/SandboxFileSystemSecurityTest.kt diff --git a/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/DefaultExecutionFileSystem.kt b/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/DefaultExecutionFileSystem.kt index 22b82374..fea6db80 100644 --- a/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/DefaultExecutionFileSystem.kt +++ b/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/DefaultExecutionFileSystem.kt @@ -17,20 +17,35 @@ class DefaultExecutionFileSystem( SystemFileSystem.createDirectories(Path(sandboxPath)) } - private val sandboxOperations = SandboxFileOperations(sandboxPath) - private fun resolvePath(relativePath: RelativePath): Path { - return sandboxOperations.resolve(relativePath) + val resolved = Path(sandboxPath, relativePath.value) + val file = java.io.File(resolved.toString()) + val normalized = try { + file.canonicalPath + } catch (e: Exception) { + throw SecurityException("Failed to resolve canonical path for '${relativePath.value}': ${e.message}") + } + val baseFile = java.io.File(sandboxPath) + val baseCanonical = try { + baseFile.canonicalPath + } catch (e: Exception) { + throw SecurityException("Failed to resolve base canonical path for '$sandboxPath': ${e.message}") + } + + if (normalized != baseCanonical && !normalized.startsWith(baseCanonical + java.io.File.separator)) { + throw SecurityException("Access to path '${relativePath.value}' is denied. It is outside the sandbox.") + } + return resolved } override suspend fun readFile(relativePath: RelativePath): ByteArray? { - val path = sandboxOperations.resolveIfRootExists(relativePath) ?: return null + val path = resolvePath(relativePath) if (!SystemFileSystem.exists(path)) return null return SystemFileSystem.source(path).buffered().use { it.readByteArray() } } override suspend fun readTextFile(relativePath: RelativePath): String? { - val path = sandboxOperations.resolveIfRootExists(relativePath) ?: return null + val path = resolvePath(relativePath) if (!SystemFileSystem.exists(path)) return null return SystemFileSystem.source(path).buffered().use { it.readString() } } @@ -58,12 +73,11 @@ class DefaultExecutionFileSystem( } override suspend fun exists(relativePath: RelativePath): Boolean { - val path = sandboxOperations.resolveIfRootExists(relativePath) ?: return false - return SystemFileSystem.exists(path) + return SystemFileSystem.exists(resolvePath(relativePath)) } override suspend fun listFiles(relativePath: RelativePath): List { - val path = sandboxOperations.resolveIfRootExists(relativePath) ?: return emptyList() + val path = resolvePath(relativePath) if (!SystemFileSystem.exists(path)) return emptyList() val metadata = SystemFileSystem.metadataOrNull(path) if (metadata?.isDirectory != true) return emptyList() @@ -83,14 +97,5 @@ class DefaultExecutionFileSystem( } } - override suspend fun createDirectory(relativePath: RelativePath): Result = runCatching { - SystemFileSystem.createDirectories(resolvePath(relativePath)) - } - - override suspend fun deleteDirectory(relativePath: RelativePath, recursive: Boolean): Result = runCatching { - require(relativePath.value.isNotEmpty()) { "The execution sandbox root cannot be deleted" } - sandboxOperations.deleteDirectory(resolvePath(relativePath), recursive) - } - override fun getBasePath(): String = sandboxPath } diff --git a/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/DefaultPluginFileSystem.kt b/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/DefaultPluginFileSystem.kt index 3e32e1db..e32f5a16 100644 --- a/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/DefaultPluginFileSystem.kt +++ b/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/DefaultPluginFileSystem.kt @@ -1,8 +1,6 @@ package org.wip.plugintoolkit.features.plugin.logic import kotlinx.coroutines.withContext -import kotlinx.coroutines.flow.Flow -import kotlinx.coroutines.flow.flow import kotlinx.io.buffered import kotlinx.io.files.Path import kotlinx.io.files.SystemFileSystem @@ -25,21 +23,25 @@ class DefaultPluginFileSystem( SystemFileSystem.createDirectories(Path(cachePath)) } - private val filesOperations = SandboxFileOperations(basePath) - private val cacheOperations = SandboxFileOperations(cachePath) - private fun resolvePath(relativePath: RelativePath): Path { - return filesOperations.resolve(relativePath) + val resolved = Path(basePath, relativePath.value) + val normalized = resolved.toString().replace('\\', '/') + val baseCanonical = Path(basePath).toString().replace('\\', '/') + + if (normalized != baseCanonical && !normalized.startsWith(if (baseCanonical.endsWith("/")) baseCanonical else "$baseCanonical/")) { + throw SecurityException("Access to path '${relativePath.value}' is denied. It is outside the plugin files directory.") + } + return resolved } override suspend fun readFile(relativePath: RelativePath): ByteArray? { - val path = filesOperations.resolveIfRootExists(relativePath) ?: return null + val path = resolvePath(relativePath) if (!SystemFileSystem.exists(path)) return null return SystemFileSystem.source(path).buffered().use { it.readByteArray() } } override suspend fun readTextFile(relativePath: RelativePath): String? { - val path = filesOperations.resolveIfRootExists(relativePath) ?: return null + val path = resolvePath(relativePath) if (!SystemFileSystem.exists(path)) return null return SystemFileSystem.source(path).buffered().use { it.readString() } } @@ -67,12 +69,11 @@ class DefaultPluginFileSystem( } override suspend fun exists(relativePath: RelativePath): Boolean { - val path = filesOperations.resolveIfRootExists(relativePath) ?: return false - return SystemFileSystem.exists(path) + return SystemFileSystem.exists(resolvePath(relativePath)) } override suspend fun listFiles(relativePath: RelativePath): List { - val path = filesOperations.resolveIfRootExists(relativePath) ?: return emptyList() + val path = resolvePath(relativePath) if (!SystemFileSystem.exists(path)) return emptyList() val metadata = SystemFileSystem.metadataOrNull(path) if (metadata?.isDirectory != true) return emptyList() @@ -92,15 +93,6 @@ class DefaultPluginFileSystem( } } - override suspend fun createDirectory(relativePath: RelativePath): Result = runCatching { - SystemFileSystem.createDirectories(resolvePath(relativePath)) - } - - override suspend fun deleteDirectory(relativePath: RelativePath, recursive: Boolean): Result = runCatching { - require(relativePath.value.isNotEmpty()) { "The plugin files root cannot be deleted" } - filesOperations.deleteDirectory(resolvePath(relativePath), recursive) - } - override suspend fun extractResource(resourcePath: String, targetRelativePath: RelativePath): Result { if (resourcePath.contains("..") || resourcePath.startsWith("/") || resourcePath.startsWith("\\") || resourcePath.contains("\u0000")) { return Result.failure(SecurityException("Invalid resource path: $resourcePath")) @@ -124,10 +116,10 @@ class DefaultPluginFileSystem( override fun getBasePath(): String = basePath companion object { - fun createCacheOnly(pluginInstallPath: String, jarPath: String? = null): PluginFileSystem { - return DefaultPluginFileSystem(pluginInstallPath, jarPath).let { fs -> + fun createCacheOnly(pluginInstallPath: String): PluginFileSystem { + return DefaultPluginFileSystem(pluginInstallPath).let { fs -> // Create a variant that uses cachePath as basePath - object : PluginFileSystem { + object : PluginFileSystem by fs { override fun getBasePath(): String = fs.cachePath override suspend fun readFile(relativePath: RelativePath): ByteArray? = fs.readFromCache(relativePath) @@ -141,83 +133,35 @@ class DefaultPluginFileSystem( override suspend fun writeTextFile(relativePath: RelativePath, text: String): Result = fs.writeTextToCache(relativePath, text) - // Keep every compound/stream operation explicitly cache-routed. In - // particular, do not use Kotlin interface delegation here: generated - // forwards would bypass these overrides and touch persistent files. - override suspend fun readStream(relativePath: RelativePath): Flow = flow { - fs.readFromCache(relativePath)?.let { emit(it) } - } - - override suspend fun writeStream( - relativePath: RelativePath, - stream: Flow - ): Result = runCatching { - val bytes = mutableListOf() - stream.collect { chunk -> chunk.forEach { byte -> bytes.add(byte) } } - fs.writeToCache(relativePath, bytes.toByteArray()).getOrThrow() - } - - override suspend fun copyFile( - source: RelativePath, - destination: RelativePath - ): Result = runCatching { - val content = fs.readFromCache(source) - ?: throw IllegalArgumentException("Source file does not exist") - fs.writeToCache(destination, content).getOrThrow() - } - - override suspend fun moveFile( - source: RelativePath, - destination: RelativePath - ): Result = runCatching { - copyFile(source, destination).getOrThrow() - fs.deleteFromCache(source).getOrThrow() - } - override suspend fun exists(relativePath: RelativePath): Boolean = - fs.cacheOperations.resolveIfRootExists(relativePath)?.let(SystemFileSystem::exists) ?: false - - override suspend fun listFiles(relativePath: RelativePath): List { - val path = fs.cacheOperations.resolveIfRootExists(relativePath) ?: return emptyList() - if (!SystemFileSystem.exists(path)) return emptyList() - if (SystemFileSystem.metadataOrNull(path)?.isDirectory != true) return emptyList() - return SystemFileSystem.list(path).map { it.name } - } + SystemFileSystem.exists(fs.resolveCachePath(relativePath)) override suspend fun deleteFile(relativePath: RelativePath): Result = fs.deleteFromCache(relativePath) - - override suspend fun createDirectory(relativePath: RelativePath): Result = runCatching { - SystemFileSystem.createDirectories(fs.resolveCachePath(relativePath)) - } - - override suspend fun deleteDirectory(relativePath: RelativePath, recursive: Boolean): Result = - runCatching { - require(relativePath.value.isNotEmpty()) { "The plugin cache root cannot be deleted" } - fs.cacheOperations.deleteDirectory(fs.resolveCachePath(relativePath), recursive) - } - - override suspend fun extractResource( - resourcePath: String, - targetRelativePath: RelativePath - ): Result = fs.extractResourceToCache(resourcePath, targetRelativePath) } } } } private fun resolveCachePath(relativePath: RelativePath): Path { - return cacheOperations.resolve(relativePath) + val resolved = Path(cachePath, relativePath.value) + val normalized = resolved.toString().replace('\\', '/') + val baseCanonical = Path(cachePath).toString().replace('\\', '/') + + if (normalized != baseCanonical && !normalized.startsWith(if (baseCanonical.endsWith("/")) baseCanonical else "$baseCanonical/")) { + throw SecurityException("Access to path '${relativePath.value}' is denied. It is outside the plugin cache directory.") + } + return resolved } private suspend fun readFromCache(relativePath: RelativePath): ByteArray? { - val path = cacheOperations.resolveIfRootExists(relativePath) ?: return null + val path = resolveCachePath(relativePath) if (!SystemFileSystem.exists(path)) return null return SystemFileSystem.source(path).buffered().use { it.readByteArray() } } private suspend fun readTextFromCache(relativePath: RelativePath): String? { - val path = cacheOperations.resolveIfRootExists(relativePath) ?: return null + val path = resolveCachePath(relativePath) if (!SystemFileSystem.exists(path)) return null return SystemFileSystem.source(path).buffered().use { it.readString() } } @@ -255,22 +199,4 @@ class DefaultPluginFileSystem( Result.failure(e) } } - - private suspend fun extractResourceToCache( - resourcePath: String, - targetRelativePath: RelativePath - ): Result { - if (resourcePath.contains("..") || resourcePath.startsWith("/") || - resourcePath.startsWith("\\") || resourcePath.contains("\u0000")) { - return Result.failure(SecurityException("Invalid resource path: $resourcePath")) - } - return runCatching { - withContext(loomDispatcher) { - val jar = jarPath ?: error("No JAR path configured for resource extraction") - val data = org.wip.plugintoolkit.core.utils.PlatformUtils.readBytesFromZip(jar, resourcePath) - ?: error("Resource not found in JAR: $resourcePath") - writeToCache(targetRelativePath, data).getOrThrow() - } - } - } } diff --git a/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/PluginLifecycleManager.kt b/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/PluginLifecycleManager.kt index 23ec9f93..ca55a892 100644 --- a/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/PluginLifecycleManager.kt +++ b/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/PluginLifecycleManager.kt @@ -352,7 +352,7 @@ class PluginLifecycleManager( logger = pluginLogger, progress = progressReporter, fileSystem = DefaultPluginFileSystem(installPath, jarFullPath), - cacheFileSystem = DefaultPluginFileSystem.createCacheOnly(installPath, jarFullPath), + cacheFileSystem = DefaultPluginFileSystem.createCacheOnly(installPath), executionFileSystem = executionFileSystem ?: DefaultExecutionFileSystem("${installPath}/temp_execution"), hostFileSystem = HostFileSystemImpl(allowedPaths, isDestructiveAllowed), settings = mergedSettings, diff --git a/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/SandboxFileOperations.kt b/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/SandboxFileOperations.kt deleted file mode 100644 index e4e2d902..00000000 --- a/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/SandboxFileOperations.kt +++ /dev/null @@ -1,69 +0,0 @@ -package org.wip.plugintoolkit.features.plugin.logic - -import kotlinx.io.files.Path -import org.wip.plugintoolkit.api.RelativePath -import java.nio.file.FileVisitResult -import java.nio.file.Files -import java.nio.file.LinkOption -import java.nio.file.NoSuchFileException -import java.nio.file.Path as NioPath -import java.nio.file.Paths -import java.nio.file.SimpleFileVisitor -import java.nio.file.attribute.BasicFileAttributes - -internal class SandboxFileOperations(root: String) { - private val base = Paths.get(root).toAbsolutePath().normalize() - private val realBase = base.toRealPath() - - fun resolve(relativePath: RelativePath): Path { - if (base.toRealPath() != realBase) { - throw SecurityException("The sandbox root changed after it was initialized") - } - val candidate = base.resolve(relativePath.value).normalize() - if (!candidate.startsWith(base)) { - throw SecurityException("Access to path '${relativePath.value}' is outside the sandbox") - } - - var current = base - base.relativize(candidate).forEach { segment -> - current = current.resolve(segment) - if (Files.exists(current, LinkOption.NOFOLLOW_LINKS)) { - val realCurrent = current.toRealPath() - if (!realCurrent.startsWith(realBase)) { - throw SecurityException("Access to path '${relativePath.value}' escapes the sandbox through a symbolic link") - } - } - } - return Path(candidate.toString()) - } - - fun resolveIfRootExists(relativePath: RelativePath): Path? = try { - resolve(relativePath) - } catch (_: NoSuchFileException) { - null - } - - fun deleteDirectory(path: Path, recursive: Boolean) { - val nioPath = Paths.get(path.toString()) - if (!Files.exists(nioPath, LinkOption.NOFOLLOW_LINKS)) return - require(Files.isDirectory(nioPath, LinkOption.NOFOLLOW_LINKS)) { "Path is not a directory: $path" } - - if (!recursive) { - Files.delete(nioPath) - return - } - - Files.walkFileTree(nioPath, object : SimpleFileVisitor() { - override fun visitFile(file: NioPath, attrs: BasicFileAttributes): FileVisitResult { - Files.delete(file) - return FileVisitResult.CONTINUE - } - - override fun postVisitDirectory(dir: NioPath, error: java.io.IOException?): FileVisitResult { - if (error != null) throw error - Files.delete(dir) - return FileVisitResult.CONTINUE - } - }) - } -} diff --git a/composeApp/src/commonTest/kotlin/org/wip/plugintoolkit/features/plugin/logic/DefaultExecutionFileSystemTest.kt b/composeApp/src/commonTest/kotlin/org/wip/plugintoolkit/features/plugin/logic/DefaultExecutionFileSystemTest.kt index 406a0b05..51da2951 100644 --- a/composeApp/src/commonTest/kotlin/org/wip/plugintoolkit/features/plugin/logic/DefaultExecutionFileSystemTest.kt +++ b/composeApp/src/commonTest/kotlin/org/wip/plugintoolkit/features/plugin/logic/DefaultExecutionFileSystemTest.kt @@ -84,24 +84,6 @@ class DefaultExecutionFileSystemTest { assertTrue(files.contains("file2.txt")) } - @Test - fun testCreateAndDeleteDirectory() = runTest { - val directory = RelativePath.from("models/nested").getOrThrow() - assertTrue(fileSystem.createDirectory(directory).isSuccess) - assertTrue(fileSystem.exists(directory)) - - fileSystem.writeTextFile(RelativePath.from("models/nested/model.txt").getOrThrow(), "model") - assertTrue(fileSystem.deleteDirectory(RelativePath.from("models").getOrThrow()).isFailure) - assertTrue(fileSystem.deleteDirectory(RelativePath.from("models").getOrThrow(), recursive = true).isSuccess) - assertFalse(fileSystem.exists(RelativePath.from("models").getOrThrow())) - } - - @Test - fun testCannotDeleteSandboxRoot() = runTest { - assertTrue(fileSystem.deleteDirectory(RelativePath.ROOT, recursive = true).isFailure) - assertTrue(SystemFileSystem.exists(Path(sandboxPath))) - } - @Test fun testPathTraversalPrevention() = runTest { // Attempt to create a path outside the sandbox using ../ diff --git a/composeApp/src/jvmTest/kotlin/org/wip/plugintoolkit/features/plugin/logic/SandboxFileSystemSecurityTest.kt b/composeApp/src/jvmTest/kotlin/org/wip/plugintoolkit/features/plugin/logic/SandboxFileSystemSecurityTest.kt deleted file mode 100644 index 63059d18..00000000 --- a/composeApp/src/jvmTest/kotlin/org/wip/plugintoolkit/features/plugin/logic/SandboxFileSystemSecurityTest.kt +++ /dev/null @@ -1,177 +0,0 @@ -package org.wip.plugintoolkit.features.plugin.logic - -import kotlinx.coroutines.flow.flowOf -import kotlinx.coroutines.test.runTest -import org.wip.plugintoolkit.api.RelativePath -import org.wip.plugintoolkit.api.ScopedFileSystem -import java.nio.file.FileVisitResult -import java.nio.file.Files -import java.nio.file.Path -import java.nio.file.SimpleFileVisitor -import java.nio.file.attribute.BasicFileAttributes -import java.util.jar.JarEntry -import java.util.jar.JarOutputStream -import kotlin.test.AfterTest -import kotlin.test.BeforeTest -import kotlin.test.Test -import kotlin.test.assertFailsWith -import kotlin.test.assertEquals -import kotlin.test.assertFalse -import kotlin.test.assertNull -import kotlin.test.assertTrue - -class SandboxFileSystemSecurityTest { - private lateinit var testRoot: Path - - @BeforeTest - fun setUp() { - testRoot = Files.createTempDirectory("plugin-toolkit-sandbox-") - } - - @AfterTest - fun tearDown() { - if (!Files.exists(testRoot)) return - Files.walkFileTree(testRoot, object : SimpleFileVisitor() { - override fun visitFile(file: Path, attrs: BasicFileAttributes): FileVisitResult { - Files.deleteIfExists(file) - return FileVisitResult.CONTINUE - } - - override fun postVisitDirectory(dir: Path, error: java.io.IOException?): FileVisitResult { - if (error != null) throw error - Files.deleteIfExists(dir) - return FileVisitResult.CONTINUE - } - }) - } - - @Test - fun rootAliasesCannotDeleteAnySandboxRoot() = runTest { - val alias = RelativePath.from("././").getOrThrow() - val execution = DefaultExecutionFileSystem(testRoot.resolve("execution").toString()) - val pluginInstall = testRoot.resolve("plugin").toString() - val plugin = DefaultPluginFileSystem(pluginInstall) - val cache = DefaultPluginFileSystem.createCacheOnly(pluginInstall) - - assertTrue(execution.deleteDirectory(alias, recursive = true).isFailure) - assertTrue(plugin.deleteDirectory(alias, recursive = true).isFailure) - assertTrue(cache.deleteDirectory(alias, recursive = true).isFailure) - assertTrue(Files.isDirectory(testRoot.resolve("execution"))) - assertTrue(Files.isDirectory(testRoot.resolve("plugin/files"))) - assertTrue(Files.isDirectory(testRoot.resolve("plugin/cache"))) - } - - @Test - fun executionSandboxCannotReadThroughSymlinkAndDoesNotFollowItOnDelete() = runTest { - val sandbox = testRoot.resolve("execution") - val outside = createOutsideSecret() - val fileSystem = DefaultExecutionFileSystem(sandbox.toString()) - - verifySymlinkIsContained(fileSystem, sandbox, outside) - } - - @Test - fun pluginFilesCannotReadThroughSymlinkAndDoNotFollowItOnDelete() = runTest { - val install = testRoot.resolve("plugin") - val outside = createOutsideSecret() - val fileSystem = DefaultPluginFileSystem(install.toString()) - - verifySymlinkIsContained(fileSystem, install.resolve("files"), outside) - } - - @Test - fun pluginCacheCannotReadThroughSymlinkAndDoesNotFollowItOnDelete() = runTest { - val install = testRoot.resolve("plugin") - val outside = createOutsideSecret() - val fileSystem = DefaultPluginFileSystem.createCacheOnly(install.toString()) - - verifySymlinkIsContained(fileSystem, install.resolve("cache"), outside) - } - - @Test - fun cacheOnlyVariantListsAndExtractsResourcesInsideCache() = runTest { - val install = testRoot.resolve("plugin") - Files.createDirectories(install) - val jar = install.resolve("plugin.jar") - JarOutputStream(Files.newOutputStream(jar)).use { output -> - output.putNextEntry(JarEntry("assets/example.txt")) - output.write("resource".encodeToByteArray()) - output.closeEntry() - } - val fileSystem = DefaultPluginFileSystem.createCacheOnly(install.toString(), jar.toString()) - val target = RelativePath.from("nested/example.txt").getOrThrow() - - assertTrue(fileSystem.extractResource("assets/example.txt", target).isSuccess) - assertEquals("resource", fileSystem.readTextFile(target)) - assertEquals(listOf("example.txt"), fileSystem.listFiles(RelativePath.from("nested").getOrThrow())) - assertTrue(Files.notExists(install.resolve("files/nested/example.txt"))) - } - - @Test - fun cacheOnlyCompoundAndStreamOperationsNeverTouchPersistentFiles() = runTest { - val install = testRoot.resolve("plugin") - val fileSystem = DefaultPluginFileSystem.createCacheOnly(install.toString()) - val source = RelativePath.from("source.bin").getOrThrow() - val copied = RelativePath.from("copied.bin").getOrThrow() - val moved = RelativePath.from("moved.bin").getOrThrow() - val streamed = RelativePath.from("streamed.bin").getOrThrow() - - Files.writeString(install.resolve("files/source.bin"), "persistent") - assertTrue(fileSystem.writeFile(source, "cache".encodeToByteArray()).isSuccess) - - assertTrue(fileSystem.copyFile(source, copied).isSuccess) - assertEquals("cache", Files.readString(install.resolve("cache/copied.bin"))) - assertTrue(Files.notExists(install.resolve("files/copied.bin"))) - - assertTrue(fileSystem.moveFile(source, moved).isSuccess) - assertTrue(Files.notExists(install.resolve("cache/source.bin"))) - assertEquals("cache", Files.readString(install.resolve("cache/moved.bin"))) - assertEquals("persistent", Files.readString(install.resolve("files/source.bin"))) - assertTrue(Files.notExists(install.resolve("files/moved.bin"))) - - assertTrue( - fileSystem.writeStream( - streamed, - flowOf("stream-".encodeToByteArray(), "cache".encodeToByteArray()) - ).isSuccess - ) - val chunks = mutableListOf() - fileSystem.readStream(streamed).collect { chunks.add(it) } - assertEquals("stream-cache", chunks.flatMap { it.asIterable() }.toByteArray().decodeToString()) - assertTrue(Files.notExists(install.resolve("files/streamed.bin"))) - } - - @Test - fun missingSandboxRootUsesReadSemanticsAndRejectsWrites() = runTest { - val install = testRoot.resolve("plugin") - val fileSystem = DefaultPluginFileSystem(install.toString()) - val file = RelativePath.from("missing.txt").getOrThrow() - Files.delete(install.resolve("files")) - - assertNull(fileSystem.readFile(file)) - assertNull(fileSystem.readTextFile(file)) - assertFalse(fileSystem.exists(file)) - assertEquals(emptyList(), fileSystem.listFiles()) - assertTrue(fileSystem.writeTextFile(file, "data").isFailure) - } - - private fun createOutsideSecret(): Path { - val outside = Files.createDirectories(testRoot.resolve("outside")) - Files.writeString(outside.resolve("secret.txt"), "must survive") - return outside - } - - private suspend fun verifySymlinkIsContained( - fileSystem: ScopedFileSystem, - sandbox: Path, - outside: Path - ) { - val nested = Files.createDirectories(sandbox.resolve("nested")) - Files.createSymbolicLink(nested.resolve("escape"), outside) - val escapedFile = RelativePath.from("nested/escape/secret.txt").getOrThrow() - - assertFailsWith { fileSystem.readTextFile(escapedFile) } - assertTrue(fileSystem.deleteDirectory(RelativePath.from("nested").getOrThrow(), recursive = true).isSuccess) - assertTrue(Files.readString(outside.resolve("secret.txt")) == "must survive") - } -} diff --git a/docs/PluginDevelopment.md b/docs/PluginDevelopment.md index bfa1153a..d69b5025 100644 --- a/docs/PluginDevelopment.md +++ b/docs/PluginDevelopment.md @@ -317,7 +317,6 @@ While you can set a plugin to "not support cancellation" the host app can force- The `PluginContext` (and focused interfaces like `PluginLogger`, `PluginFileSystem`, `ExecutionFileSystem`, `HostFileSystem`) provide access to host services: - **Logger**: `PluginLogger` (e.g. `logger.info("Message")`) - **Plugin File System**: `PluginFileSystem` (Persistent, isolated storage for the plugin. Preserved across executions. e.g. `fileSystem.getBasePath()`) -- **Directory operations**: scoped file systems support `createDirectory` and guarded `deleteDirectory`; recursive deletion must be requested explicitly and the sandbox root can never be deleted. - **Execution File System**: `ExecutionFileSystem` (Temporary, isolated sandbox storage for the current execution. Cleared automatically after the flow finishes.) - **Host File System**: `HostFileSystem` (External file access. Restricted to paths explicitly granted by the user via file input/output parameters: `@CapabilityInput` and `@CapabilityOutput`.) - **Plugin Storage**: `PluginStorage` (`context.storage`) provides a persistent, internal key-value store (`get`, `put`, `getAll`, `remove`) for saving plugin-internal state without polluting user settings. diff --git a/plugin-api/src/commonMain/kotlin/org/wip/plugintoolkit/api/Interfaces.kt b/plugin-api/src/commonMain/kotlin/org/wip/plugintoolkit/api/Interfaces.kt index 3b86f7c0..5b4702b9 100644 --- a/plugin-api/src/commonMain/kotlin/org/wip/plugintoolkit/api/Interfaces.kt +++ b/plugin-api/src/commonMain/kotlin/org/wip/plugintoolkit/api/Interfaces.kt @@ -102,10 +102,6 @@ interface ScopedFileSystem { suspend fun exists(relativePath: RelativePath): Boolean suspend fun listFiles(relativePath: RelativePath = RelativePath.ROOT): List suspend fun deleteFile(relativePath: RelativePath): Result - suspend fun createDirectory(relativePath: RelativePath): Result = - Result.failure(UnsupportedOperationException("Directory creation is not supported by this host")) - suspend fun deleteDirectory(relativePath: RelativePath, recursive: Boolean = false): Result = - Result.failure(UnsupportedOperationException("Directory deletion is not supported by this host")) /** * Get the absolute base path of the managed file area. diff --git a/plugin-api/src/commonMain/kotlin/org/wip/plugintoolkit/api/RelativePath.kt b/plugin-api/src/commonMain/kotlin/org/wip/plugintoolkit/api/RelativePath.kt index a9b1104c..7940117a 100644 --- a/plugin-api/src/commonMain/kotlin/org/wip/plugintoolkit/api/RelativePath.kt +++ b/plugin-api/src/commonMain/kotlin/org/wip/plugintoolkit/api/RelativePath.kt @@ -13,6 +13,7 @@ value class RelativePath private constructor(val value: String) { companion object { val ROOT = RelativePath("") private val NULL_BYTE_REGEX = Regex("\u0000") + private val TRAVERSAL_REGEX = Regex("""(?:^|/|\\|\.|\u2024|\uFF0E|\u3002)(?:\.\.|\u2024\u2024|\uFF0E\uFF0E|%2e%2e|%2E%2E|%252e%252e)(?:/|\\|${'$'}|\.)""", RegexOption.IGNORE_CASE) private val ENCODED_SLASH_REGEX = Regex("""%2f|%5c""", RegexOption.IGNORE_CASE) /** @@ -37,27 +38,25 @@ value class RelativePath private constructor(val value: String) { return Result.failure(SecurityException("Path must be relative, but contains drive letter: $normalized")) } - if (normalized.contains(ENCODED_SLASH_REGEX)) { + // Check for encoded slashes or traversal sequences + if (normalized.contains(ENCODED_SLASH_REGEX) || normalized.contains(TRAVERSAL_REGEX)) { return Result.failure(SecurityException("Path traversal attempt detected: $normalized")) } - val segments = normalized.replace('\\', '/').split('/').filter { it.isNotEmpty() && it != "." } - val validationSegments = segments.map { segment -> - segment - .replace("\u2024", ".") - .replace("\uFF0E", ".") - .replace("\u3002", ".") - .replace("%2e", ".", ignoreCase = true) - } - if (validationSegments.any { - it == ".." || it.contains(Regex("%25(?:2e|2f|5c)", RegexOption.IGNORE_CASE)) - } - ) { + // Normalized path checks: convert unicode dot variants to regular dot for safety check + val sanitized = normalized + .replace("\u2024", ".") + .replace("\uFF0E", ".") + .replace("\u3002", ".") + .replace("%2e", ".", ignoreCase = true) + .replace("%2f", "/", ignoreCase = true) + .replace("%5c", "\\", ignoreCase = true) + + if (sanitized.contains(TRAVERSAL_REGEX) || sanitized.contains("../") || sanitized.contains("..\\")) { return Result.failure(SecurityException("Path traversal attempt detected: $normalized")) } - - // Validation uses a security-normalized view, but the filename itself is not decoded or rewritten. - return Result.success(RelativePath(segments.joinToString("/"))) + + return Result.success(RelativePath(normalized)) } } } diff --git a/plugin-api/src/commonTest/kotlin/org/wip/plugintoolkit/api/RelativePathTest.kt b/plugin-api/src/commonTest/kotlin/org/wip/plugintoolkit/api/RelativePathTest.kt index b328f843..96ae01bd 100644 --- a/plugin-api/src/commonTest/kotlin/org/wip/plugintoolkit/api/RelativePathTest.kt +++ b/plugin-api/src/commonTest/kotlin/org/wip/plugintoolkit/api/RelativePathTest.kt @@ -52,11 +52,5 @@ class RelativePathTest { val path = " foo/bar.txt ".toRelativePath() assertTrue(path.isSuccess) assertEquals("foo/bar.txt", path.getOrNull()?.value, "Should trim whitespace") - assertEquals(RelativePath.ROOT, ".".toRelativePath().getOrThrow()) - assertEquals(RelativePath.ROOT, "././".toRelativePath().getOrThrow()) - assertEquals("foo/bar", "foo/./bar".toRelativePath().getOrThrow().value) - assertEquals("foo/bar", "foo\\bar".toRelativePath().getOrThrow().value) - assertEquals("file%2ename.txt", "file%2ename.txt".toRelativePath().getOrThrow().value) - assertEquals("file\u2024txt", "file\u2024txt".toRelativePath().getOrThrow().value) } } From cc6c9d72eb46e248aac4eea6f9baac5629cc4fe5 Mon Sep 17 00:00:00 2001 From: Matteo Mekhail <67237370+matteoiscrying@users.noreply.github.com> Date: Sat, 15 Aug 2026 10:08:22 +1000 Subject: [PATCH 16/16] Revert "Merge branch 'codex/issue-11-custom-setting-defaults' into codex/issue-12-divide-plugin-settings" This reverts commit db3dfd211f87ed4cd3e564e3d5b96dcf9fa820c1, reversing changes made to c6cb1eb8c901274b8c76d7ea5769ba749d1e96d9. --- .../logic/DefaultExecutionFileSystem.kt | 39 ++-- .../plugin/logic/DefaultPluginFileSystem.kt | 126 +++---------- .../plugin/logic/PluginLifecycleManager.kt | 2 +- .../plugin/logic/SandboxFileOperations.kt | 69 ------- .../logic/DefaultExecutionFileSystemTest.kt | 18 -- .../logic/SandboxFileSystemSecurityTest.kt | 177 ------------------ docs/PluginDevelopment.md | 1 - .../org/wip/plugintoolkit/api/Interfaces.kt | 4 - .../org/wip/plugintoolkit/api/RelativePath.kt | 31 ++- .../wip/plugintoolkit/api/RelativePathTest.kt | 6 - 10 files changed, 64 insertions(+), 409 deletions(-) delete mode 100644 composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/SandboxFileOperations.kt delete mode 100644 composeApp/src/jvmTest/kotlin/org/wip/plugintoolkit/features/plugin/logic/SandboxFileSystemSecurityTest.kt diff --git a/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/DefaultExecutionFileSystem.kt b/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/DefaultExecutionFileSystem.kt index 22b82374..fea6db80 100644 --- a/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/DefaultExecutionFileSystem.kt +++ b/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/DefaultExecutionFileSystem.kt @@ -17,20 +17,35 @@ class DefaultExecutionFileSystem( SystemFileSystem.createDirectories(Path(sandboxPath)) } - private val sandboxOperations = SandboxFileOperations(sandboxPath) - private fun resolvePath(relativePath: RelativePath): Path { - return sandboxOperations.resolve(relativePath) + val resolved = Path(sandboxPath, relativePath.value) + val file = java.io.File(resolved.toString()) + val normalized = try { + file.canonicalPath + } catch (e: Exception) { + throw SecurityException("Failed to resolve canonical path for '${relativePath.value}': ${e.message}") + } + val baseFile = java.io.File(sandboxPath) + val baseCanonical = try { + baseFile.canonicalPath + } catch (e: Exception) { + throw SecurityException("Failed to resolve base canonical path for '$sandboxPath': ${e.message}") + } + + if (normalized != baseCanonical && !normalized.startsWith(baseCanonical + java.io.File.separator)) { + throw SecurityException("Access to path '${relativePath.value}' is denied. It is outside the sandbox.") + } + return resolved } override suspend fun readFile(relativePath: RelativePath): ByteArray? { - val path = sandboxOperations.resolveIfRootExists(relativePath) ?: return null + val path = resolvePath(relativePath) if (!SystemFileSystem.exists(path)) return null return SystemFileSystem.source(path).buffered().use { it.readByteArray() } } override suspend fun readTextFile(relativePath: RelativePath): String? { - val path = sandboxOperations.resolveIfRootExists(relativePath) ?: return null + val path = resolvePath(relativePath) if (!SystemFileSystem.exists(path)) return null return SystemFileSystem.source(path).buffered().use { it.readString() } } @@ -58,12 +73,11 @@ class DefaultExecutionFileSystem( } override suspend fun exists(relativePath: RelativePath): Boolean { - val path = sandboxOperations.resolveIfRootExists(relativePath) ?: return false - return SystemFileSystem.exists(path) + return SystemFileSystem.exists(resolvePath(relativePath)) } override suspend fun listFiles(relativePath: RelativePath): List { - val path = sandboxOperations.resolveIfRootExists(relativePath) ?: return emptyList() + val path = resolvePath(relativePath) if (!SystemFileSystem.exists(path)) return emptyList() val metadata = SystemFileSystem.metadataOrNull(path) if (metadata?.isDirectory != true) return emptyList() @@ -83,14 +97,5 @@ class DefaultExecutionFileSystem( } } - override suspend fun createDirectory(relativePath: RelativePath): Result = runCatching { - SystemFileSystem.createDirectories(resolvePath(relativePath)) - } - - override suspend fun deleteDirectory(relativePath: RelativePath, recursive: Boolean): Result = runCatching { - require(relativePath.value.isNotEmpty()) { "The execution sandbox root cannot be deleted" } - sandboxOperations.deleteDirectory(resolvePath(relativePath), recursive) - } - override fun getBasePath(): String = sandboxPath } diff --git a/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/DefaultPluginFileSystem.kt b/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/DefaultPluginFileSystem.kt index 3e32e1db..e32f5a16 100644 --- a/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/DefaultPluginFileSystem.kt +++ b/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/DefaultPluginFileSystem.kt @@ -1,8 +1,6 @@ package org.wip.plugintoolkit.features.plugin.logic import kotlinx.coroutines.withContext -import kotlinx.coroutines.flow.Flow -import kotlinx.coroutines.flow.flow import kotlinx.io.buffered import kotlinx.io.files.Path import kotlinx.io.files.SystemFileSystem @@ -25,21 +23,25 @@ class DefaultPluginFileSystem( SystemFileSystem.createDirectories(Path(cachePath)) } - private val filesOperations = SandboxFileOperations(basePath) - private val cacheOperations = SandboxFileOperations(cachePath) - private fun resolvePath(relativePath: RelativePath): Path { - return filesOperations.resolve(relativePath) + val resolved = Path(basePath, relativePath.value) + val normalized = resolved.toString().replace('\\', '/') + val baseCanonical = Path(basePath).toString().replace('\\', '/') + + if (normalized != baseCanonical && !normalized.startsWith(if (baseCanonical.endsWith("/")) baseCanonical else "$baseCanonical/")) { + throw SecurityException("Access to path '${relativePath.value}' is denied. It is outside the plugin files directory.") + } + return resolved } override suspend fun readFile(relativePath: RelativePath): ByteArray? { - val path = filesOperations.resolveIfRootExists(relativePath) ?: return null + val path = resolvePath(relativePath) if (!SystemFileSystem.exists(path)) return null return SystemFileSystem.source(path).buffered().use { it.readByteArray() } } override suspend fun readTextFile(relativePath: RelativePath): String? { - val path = filesOperations.resolveIfRootExists(relativePath) ?: return null + val path = resolvePath(relativePath) if (!SystemFileSystem.exists(path)) return null return SystemFileSystem.source(path).buffered().use { it.readString() } } @@ -67,12 +69,11 @@ class DefaultPluginFileSystem( } override suspend fun exists(relativePath: RelativePath): Boolean { - val path = filesOperations.resolveIfRootExists(relativePath) ?: return false - return SystemFileSystem.exists(path) + return SystemFileSystem.exists(resolvePath(relativePath)) } override suspend fun listFiles(relativePath: RelativePath): List { - val path = filesOperations.resolveIfRootExists(relativePath) ?: return emptyList() + val path = resolvePath(relativePath) if (!SystemFileSystem.exists(path)) return emptyList() val metadata = SystemFileSystem.metadataOrNull(path) if (metadata?.isDirectory != true) return emptyList() @@ -92,15 +93,6 @@ class DefaultPluginFileSystem( } } - override suspend fun createDirectory(relativePath: RelativePath): Result = runCatching { - SystemFileSystem.createDirectories(resolvePath(relativePath)) - } - - override suspend fun deleteDirectory(relativePath: RelativePath, recursive: Boolean): Result = runCatching { - require(relativePath.value.isNotEmpty()) { "The plugin files root cannot be deleted" } - filesOperations.deleteDirectory(resolvePath(relativePath), recursive) - } - override suspend fun extractResource(resourcePath: String, targetRelativePath: RelativePath): Result { if (resourcePath.contains("..") || resourcePath.startsWith("/") || resourcePath.startsWith("\\") || resourcePath.contains("\u0000")) { return Result.failure(SecurityException("Invalid resource path: $resourcePath")) @@ -124,10 +116,10 @@ class DefaultPluginFileSystem( override fun getBasePath(): String = basePath companion object { - fun createCacheOnly(pluginInstallPath: String, jarPath: String? = null): PluginFileSystem { - return DefaultPluginFileSystem(pluginInstallPath, jarPath).let { fs -> + fun createCacheOnly(pluginInstallPath: String): PluginFileSystem { + return DefaultPluginFileSystem(pluginInstallPath).let { fs -> // Create a variant that uses cachePath as basePath - object : PluginFileSystem { + object : PluginFileSystem by fs { override fun getBasePath(): String = fs.cachePath override suspend fun readFile(relativePath: RelativePath): ByteArray? = fs.readFromCache(relativePath) @@ -141,83 +133,35 @@ class DefaultPluginFileSystem( override suspend fun writeTextFile(relativePath: RelativePath, text: String): Result = fs.writeTextToCache(relativePath, text) - // Keep every compound/stream operation explicitly cache-routed. In - // particular, do not use Kotlin interface delegation here: generated - // forwards would bypass these overrides and touch persistent files. - override suspend fun readStream(relativePath: RelativePath): Flow = flow { - fs.readFromCache(relativePath)?.let { emit(it) } - } - - override suspend fun writeStream( - relativePath: RelativePath, - stream: Flow - ): Result = runCatching { - val bytes = mutableListOf() - stream.collect { chunk -> chunk.forEach { byte -> bytes.add(byte) } } - fs.writeToCache(relativePath, bytes.toByteArray()).getOrThrow() - } - - override suspend fun copyFile( - source: RelativePath, - destination: RelativePath - ): Result = runCatching { - val content = fs.readFromCache(source) - ?: throw IllegalArgumentException("Source file does not exist") - fs.writeToCache(destination, content).getOrThrow() - } - - override suspend fun moveFile( - source: RelativePath, - destination: RelativePath - ): Result = runCatching { - copyFile(source, destination).getOrThrow() - fs.deleteFromCache(source).getOrThrow() - } - override suspend fun exists(relativePath: RelativePath): Boolean = - fs.cacheOperations.resolveIfRootExists(relativePath)?.let(SystemFileSystem::exists) ?: false - - override suspend fun listFiles(relativePath: RelativePath): List { - val path = fs.cacheOperations.resolveIfRootExists(relativePath) ?: return emptyList() - if (!SystemFileSystem.exists(path)) return emptyList() - if (SystemFileSystem.metadataOrNull(path)?.isDirectory != true) return emptyList() - return SystemFileSystem.list(path).map { it.name } - } + SystemFileSystem.exists(fs.resolveCachePath(relativePath)) override suspend fun deleteFile(relativePath: RelativePath): Result = fs.deleteFromCache(relativePath) - - override suspend fun createDirectory(relativePath: RelativePath): Result = runCatching { - SystemFileSystem.createDirectories(fs.resolveCachePath(relativePath)) - } - - override suspend fun deleteDirectory(relativePath: RelativePath, recursive: Boolean): Result = - runCatching { - require(relativePath.value.isNotEmpty()) { "The plugin cache root cannot be deleted" } - fs.cacheOperations.deleteDirectory(fs.resolveCachePath(relativePath), recursive) - } - - override suspend fun extractResource( - resourcePath: String, - targetRelativePath: RelativePath - ): Result = fs.extractResourceToCache(resourcePath, targetRelativePath) } } } } private fun resolveCachePath(relativePath: RelativePath): Path { - return cacheOperations.resolve(relativePath) + val resolved = Path(cachePath, relativePath.value) + val normalized = resolved.toString().replace('\\', '/') + val baseCanonical = Path(cachePath).toString().replace('\\', '/') + + if (normalized != baseCanonical && !normalized.startsWith(if (baseCanonical.endsWith("/")) baseCanonical else "$baseCanonical/")) { + throw SecurityException("Access to path '${relativePath.value}' is denied. It is outside the plugin cache directory.") + } + return resolved } private suspend fun readFromCache(relativePath: RelativePath): ByteArray? { - val path = cacheOperations.resolveIfRootExists(relativePath) ?: return null + val path = resolveCachePath(relativePath) if (!SystemFileSystem.exists(path)) return null return SystemFileSystem.source(path).buffered().use { it.readByteArray() } } private suspend fun readTextFromCache(relativePath: RelativePath): String? { - val path = cacheOperations.resolveIfRootExists(relativePath) ?: return null + val path = resolveCachePath(relativePath) if (!SystemFileSystem.exists(path)) return null return SystemFileSystem.source(path).buffered().use { it.readString() } } @@ -255,22 +199,4 @@ class DefaultPluginFileSystem( Result.failure(e) } } - - private suspend fun extractResourceToCache( - resourcePath: String, - targetRelativePath: RelativePath - ): Result { - if (resourcePath.contains("..") || resourcePath.startsWith("/") || - resourcePath.startsWith("\\") || resourcePath.contains("\u0000")) { - return Result.failure(SecurityException("Invalid resource path: $resourcePath")) - } - return runCatching { - withContext(loomDispatcher) { - val jar = jarPath ?: error("No JAR path configured for resource extraction") - val data = org.wip.plugintoolkit.core.utils.PlatformUtils.readBytesFromZip(jar, resourcePath) - ?: error("Resource not found in JAR: $resourcePath") - writeToCache(targetRelativePath, data).getOrThrow() - } - } - } } diff --git a/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/PluginLifecycleManager.kt b/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/PluginLifecycleManager.kt index b889509d..4a76a74e 100644 --- a/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/PluginLifecycleManager.kt +++ b/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/PluginLifecycleManager.kt @@ -355,7 +355,7 @@ class PluginLifecycleManager( logger = pluginLogger, progress = progressReporter, fileSystem = DefaultPluginFileSystem(installPath, jarFullPath), - cacheFileSystem = DefaultPluginFileSystem.createCacheOnly(installPath, jarFullPath), + cacheFileSystem = DefaultPluginFileSystem.createCacheOnly(installPath), executionFileSystem = executionFileSystem ?: DefaultExecutionFileSystem("${installPath}/temp_execution"), hostFileSystem = HostFileSystemImpl(allowedPaths, isDestructiveAllowed), settings = mergedSettings, diff --git a/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/SandboxFileOperations.kt b/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/SandboxFileOperations.kt deleted file mode 100644 index e4e2d902..00000000 --- a/composeApp/src/commonMain/kotlin/org/wip/plugintoolkit/features/plugin/logic/SandboxFileOperations.kt +++ /dev/null @@ -1,69 +0,0 @@ -package org.wip.plugintoolkit.features.plugin.logic - -import kotlinx.io.files.Path -import org.wip.plugintoolkit.api.RelativePath -import java.nio.file.FileVisitResult -import java.nio.file.Files -import java.nio.file.LinkOption -import java.nio.file.NoSuchFileException -import java.nio.file.Path as NioPath -import java.nio.file.Paths -import java.nio.file.SimpleFileVisitor -import java.nio.file.attribute.BasicFileAttributes - -internal class SandboxFileOperations(root: String) { - private val base = Paths.get(root).toAbsolutePath().normalize() - private val realBase = base.toRealPath() - - fun resolve(relativePath: RelativePath): Path { - if (base.toRealPath() != realBase) { - throw SecurityException("The sandbox root changed after it was initialized") - } - val candidate = base.resolve(relativePath.value).normalize() - if (!candidate.startsWith(base)) { - throw SecurityException("Access to path '${relativePath.value}' is outside the sandbox") - } - - var current = base - base.relativize(candidate).forEach { segment -> - current = current.resolve(segment) - if (Files.exists(current, LinkOption.NOFOLLOW_LINKS)) { - val realCurrent = current.toRealPath() - if (!realCurrent.startsWith(realBase)) { - throw SecurityException("Access to path '${relativePath.value}' escapes the sandbox through a symbolic link") - } - } - } - return Path(candidate.toString()) - } - - fun resolveIfRootExists(relativePath: RelativePath): Path? = try { - resolve(relativePath) - } catch (_: NoSuchFileException) { - null - } - - fun deleteDirectory(path: Path, recursive: Boolean) { - val nioPath = Paths.get(path.toString()) - if (!Files.exists(nioPath, LinkOption.NOFOLLOW_LINKS)) return - require(Files.isDirectory(nioPath, LinkOption.NOFOLLOW_LINKS)) { "Path is not a directory: $path" } - - if (!recursive) { - Files.delete(nioPath) - return - } - - Files.walkFileTree(nioPath, object : SimpleFileVisitor() { - override fun visitFile(file: NioPath, attrs: BasicFileAttributes): FileVisitResult { - Files.delete(file) - return FileVisitResult.CONTINUE - } - - override fun postVisitDirectory(dir: NioPath, error: java.io.IOException?): FileVisitResult { - if (error != null) throw error - Files.delete(dir) - return FileVisitResult.CONTINUE - } - }) - } -} diff --git a/composeApp/src/commonTest/kotlin/org/wip/plugintoolkit/features/plugin/logic/DefaultExecutionFileSystemTest.kt b/composeApp/src/commonTest/kotlin/org/wip/plugintoolkit/features/plugin/logic/DefaultExecutionFileSystemTest.kt index 406a0b05..51da2951 100644 --- a/composeApp/src/commonTest/kotlin/org/wip/plugintoolkit/features/plugin/logic/DefaultExecutionFileSystemTest.kt +++ b/composeApp/src/commonTest/kotlin/org/wip/plugintoolkit/features/plugin/logic/DefaultExecutionFileSystemTest.kt @@ -84,24 +84,6 @@ class DefaultExecutionFileSystemTest { assertTrue(files.contains("file2.txt")) } - @Test - fun testCreateAndDeleteDirectory() = runTest { - val directory = RelativePath.from("models/nested").getOrThrow() - assertTrue(fileSystem.createDirectory(directory).isSuccess) - assertTrue(fileSystem.exists(directory)) - - fileSystem.writeTextFile(RelativePath.from("models/nested/model.txt").getOrThrow(), "model") - assertTrue(fileSystem.deleteDirectory(RelativePath.from("models").getOrThrow()).isFailure) - assertTrue(fileSystem.deleteDirectory(RelativePath.from("models").getOrThrow(), recursive = true).isSuccess) - assertFalse(fileSystem.exists(RelativePath.from("models").getOrThrow())) - } - - @Test - fun testCannotDeleteSandboxRoot() = runTest { - assertTrue(fileSystem.deleteDirectory(RelativePath.ROOT, recursive = true).isFailure) - assertTrue(SystemFileSystem.exists(Path(sandboxPath))) - } - @Test fun testPathTraversalPrevention() = runTest { // Attempt to create a path outside the sandbox using ../ diff --git a/composeApp/src/jvmTest/kotlin/org/wip/plugintoolkit/features/plugin/logic/SandboxFileSystemSecurityTest.kt b/composeApp/src/jvmTest/kotlin/org/wip/plugintoolkit/features/plugin/logic/SandboxFileSystemSecurityTest.kt deleted file mode 100644 index 63059d18..00000000 --- a/composeApp/src/jvmTest/kotlin/org/wip/plugintoolkit/features/plugin/logic/SandboxFileSystemSecurityTest.kt +++ /dev/null @@ -1,177 +0,0 @@ -package org.wip.plugintoolkit.features.plugin.logic - -import kotlinx.coroutines.flow.flowOf -import kotlinx.coroutines.test.runTest -import org.wip.plugintoolkit.api.RelativePath -import org.wip.plugintoolkit.api.ScopedFileSystem -import java.nio.file.FileVisitResult -import java.nio.file.Files -import java.nio.file.Path -import java.nio.file.SimpleFileVisitor -import java.nio.file.attribute.BasicFileAttributes -import java.util.jar.JarEntry -import java.util.jar.JarOutputStream -import kotlin.test.AfterTest -import kotlin.test.BeforeTest -import kotlin.test.Test -import kotlin.test.assertFailsWith -import kotlin.test.assertEquals -import kotlin.test.assertFalse -import kotlin.test.assertNull -import kotlin.test.assertTrue - -class SandboxFileSystemSecurityTest { - private lateinit var testRoot: Path - - @BeforeTest - fun setUp() { - testRoot = Files.createTempDirectory("plugin-toolkit-sandbox-") - } - - @AfterTest - fun tearDown() { - if (!Files.exists(testRoot)) return - Files.walkFileTree(testRoot, object : SimpleFileVisitor() { - override fun visitFile(file: Path, attrs: BasicFileAttributes): FileVisitResult { - Files.deleteIfExists(file) - return FileVisitResult.CONTINUE - } - - override fun postVisitDirectory(dir: Path, error: java.io.IOException?): FileVisitResult { - if (error != null) throw error - Files.deleteIfExists(dir) - return FileVisitResult.CONTINUE - } - }) - } - - @Test - fun rootAliasesCannotDeleteAnySandboxRoot() = runTest { - val alias = RelativePath.from("././").getOrThrow() - val execution = DefaultExecutionFileSystem(testRoot.resolve("execution").toString()) - val pluginInstall = testRoot.resolve("plugin").toString() - val plugin = DefaultPluginFileSystem(pluginInstall) - val cache = DefaultPluginFileSystem.createCacheOnly(pluginInstall) - - assertTrue(execution.deleteDirectory(alias, recursive = true).isFailure) - assertTrue(plugin.deleteDirectory(alias, recursive = true).isFailure) - assertTrue(cache.deleteDirectory(alias, recursive = true).isFailure) - assertTrue(Files.isDirectory(testRoot.resolve("execution"))) - assertTrue(Files.isDirectory(testRoot.resolve("plugin/files"))) - assertTrue(Files.isDirectory(testRoot.resolve("plugin/cache"))) - } - - @Test - fun executionSandboxCannotReadThroughSymlinkAndDoesNotFollowItOnDelete() = runTest { - val sandbox = testRoot.resolve("execution") - val outside = createOutsideSecret() - val fileSystem = DefaultExecutionFileSystem(sandbox.toString()) - - verifySymlinkIsContained(fileSystem, sandbox, outside) - } - - @Test - fun pluginFilesCannotReadThroughSymlinkAndDoNotFollowItOnDelete() = runTest { - val install = testRoot.resolve("plugin") - val outside = createOutsideSecret() - val fileSystem = DefaultPluginFileSystem(install.toString()) - - verifySymlinkIsContained(fileSystem, install.resolve("files"), outside) - } - - @Test - fun pluginCacheCannotReadThroughSymlinkAndDoesNotFollowItOnDelete() = runTest { - val install = testRoot.resolve("plugin") - val outside = createOutsideSecret() - val fileSystem = DefaultPluginFileSystem.createCacheOnly(install.toString()) - - verifySymlinkIsContained(fileSystem, install.resolve("cache"), outside) - } - - @Test - fun cacheOnlyVariantListsAndExtractsResourcesInsideCache() = runTest { - val install = testRoot.resolve("plugin") - Files.createDirectories(install) - val jar = install.resolve("plugin.jar") - JarOutputStream(Files.newOutputStream(jar)).use { output -> - output.putNextEntry(JarEntry("assets/example.txt")) - output.write("resource".encodeToByteArray()) - output.closeEntry() - } - val fileSystem = DefaultPluginFileSystem.createCacheOnly(install.toString(), jar.toString()) - val target = RelativePath.from("nested/example.txt").getOrThrow() - - assertTrue(fileSystem.extractResource("assets/example.txt", target).isSuccess) - assertEquals("resource", fileSystem.readTextFile(target)) - assertEquals(listOf("example.txt"), fileSystem.listFiles(RelativePath.from("nested").getOrThrow())) - assertTrue(Files.notExists(install.resolve("files/nested/example.txt"))) - } - - @Test - fun cacheOnlyCompoundAndStreamOperationsNeverTouchPersistentFiles() = runTest { - val install = testRoot.resolve("plugin") - val fileSystem = DefaultPluginFileSystem.createCacheOnly(install.toString()) - val source = RelativePath.from("source.bin").getOrThrow() - val copied = RelativePath.from("copied.bin").getOrThrow() - val moved = RelativePath.from("moved.bin").getOrThrow() - val streamed = RelativePath.from("streamed.bin").getOrThrow() - - Files.writeString(install.resolve("files/source.bin"), "persistent") - assertTrue(fileSystem.writeFile(source, "cache".encodeToByteArray()).isSuccess) - - assertTrue(fileSystem.copyFile(source, copied).isSuccess) - assertEquals("cache", Files.readString(install.resolve("cache/copied.bin"))) - assertTrue(Files.notExists(install.resolve("files/copied.bin"))) - - assertTrue(fileSystem.moveFile(source, moved).isSuccess) - assertTrue(Files.notExists(install.resolve("cache/source.bin"))) - assertEquals("cache", Files.readString(install.resolve("cache/moved.bin"))) - assertEquals("persistent", Files.readString(install.resolve("files/source.bin"))) - assertTrue(Files.notExists(install.resolve("files/moved.bin"))) - - assertTrue( - fileSystem.writeStream( - streamed, - flowOf("stream-".encodeToByteArray(), "cache".encodeToByteArray()) - ).isSuccess - ) - val chunks = mutableListOf() - fileSystem.readStream(streamed).collect { chunks.add(it) } - assertEquals("stream-cache", chunks.flatMap { it.asIterable() }.toByteArray().decodeToString()) - assertTrue(Files.notExists(install.resolve("files/streamed.bin"))) - } - - @Test - fun missingSandboxRootUsesReadSemanticsAndRejectsWrites() = runTest { - val install = testRoot.resolve("plugin") - val fileSystem = DefaultPluginFileSystem(install.toString()) - val file = RelativePath.from("missing.txt").getOrThrow() - Files.delete(install.resolve("files")) - - assertNull(fileSystem.readFile(file)) - assertNull(fileSystem.readTextFile(file)) - assertFalse(fileSystem.exists(file)) - assertEquals(emptyList(), fileSystem.listFiles()) - assertTrue(fileSystem.writeTextFile(file, "data").isFailure) - } - - private fun createOutsideSecret(): Path { - val outside = Files.createDirectories(testRoot.resolve("outside")) - Files.writeString(outside.resolve("secret.txt"), "must survive") - return outside - } - - private suspend fun verifySymlinkIsContained( - fileSystem: ScopedFileSystem, - sandbox: Path, - outside: Path - ) { - val nested = Files.createDirectories(sandbox.resolve("nested")) - Files.createSymbolicLink(nested.resolve("escape"), outside) - val escapedFile = RelativePath.from("nested/escape/secret.txt").getOrThrow() - - assertFailsWith { fileSystem.readTextFile(escapedFile) } - assertTrue(fileSystem.deleteDirectory(RelativePath.from("nested").getOrThrow(), recursive = true).isSuccess) - assertTrue(Files.readString(outside.resolve("secret.txt")) == "must survive") - } -} diff --git a/docs/PluginDevelopment.md b/docs/PluginDevelopment.md index f7e68b33..d08fb3ed 100644 --- a/docs/PluginDevelopment.md +++ b/docs/PluginDevelopment.md @@ -320,7 +320,6 @@ While you can set a plugin to "not support cancellation" the host app can force- The `PluginContext` (and focused interfaces like `PluginLogger`, `PluginFileSystem`, `ExecutionFileSystem`, `HostFileSystem`) provide access to host services: - **Logger**: `PluginLogger` (e.g. `logger.info("Message")`) - **Plugin File System**: `PluginFileSystem` (Persistent, isolated storage for the plugin. Preserved across executions. e.g. `fileSystem.getBasePath()`) -- **Directory operations**: scoped file systems support `createDirectory` and guarded `deleteDirectory`; recursive deletion must be requested explicitly and the sandbox root can never be deleted. - **Execution File System**: `ExecutionFileSystem` (Temporary, isolated sandbox storage for the current execution. Cleared automatically after the flow finishes.) - **Host File System**: `HostFileSystem` (External file access. Restricted to paths explicitly granted by the user via file input/output parameters: `@CapabilityInput` and `@CapabilityOutput`.) - **Plugin Storage**: `PluginStorage` (`context.storage`) provides a persistent, internal key-value store (`get`, `put`, `getAll`, `remove`) for saving plugin-internal state without polluting user settings. diff --git a/plugin-api/src/commonMain/kotlin/org/wip/plugintoolkit/api/Interfaces.kt b/plugin-api/src/commonMain/kotlin/org/wip/plugintoolkit/api/Interfaces.kt index 3b86f7c0..5b4702b9 100644 --- a/plugin-api/src/commonMain/kotlin/org/wip/plugintoolkit/api/Interfaces.kt +++ b/plugin-api/src/commonMain/kotlin/org/wip/plugintoolkit/api/Interfaces.kt @@ -102,10 +102,6 @@ interface ScopedFileSystem { suspend fun exists(relativePath: RelativePath): Boolean suspend fun listFiles(relativePath: RelativePath = RelativePath.ROOT): List suspend fun deleteFile(relativePath: RelativePath): Result - suspend fun createDirectory(relativePath: RelativePath): Result = - Result.failure(UnsupportedOperationException("Directory creation is not supported by this host")) - suspend fun deleteDirectory(relativePath: RelativePath, recursive: Boolean = false): Result = - Result.failure(UnsupportedOperationException("Directory deletion is not supported by this host")) /** * Get the absolute base path of the managed file area. diff --git a/plugin-api/src/commonMain/kotlin/org/wip/plugintoolkit/api/RelativePath.kt b/plugin-api/src/commonMain/kotlin/org/wip/plugintoolkit/api/RelativePath.kt index a9b1104c..7940117a 100644 --- a/plugin-api/src/commonMain/kotlin/org/wip/plugintoolkit/api/RelativePath.kt +++ b/plugin-api/src/commonMain/kotlin/org/wip/plugintoolkit/api/RelativePath.kt @@ -13,6 +13,7 @@ value class RelativePath private constructor(val value: String) { companion object { val ROOT = RelativePath("") private val NULL_BYTE_REGEX = Regex("\u0000") + private val TRAVERSAL_REGEX = Regex("""(?:^|/|\\|\.|\u2024|\uFF0E|\u3002)(?:\.\.|\u2024\u2024|\uFF0E\uFF0E|%2e%2e|%2E%2E|%252e%252e)(?:/|\\|${'$'}|\.)""", RegexOption.IGNORE_CASE) private val ENCODED_SLASH_REGEX = Regex("""%2f|%5c""", RegexOption.IGNORE_CASE) /** @@ -37,27 +38,25 @@ value class RelativePath private constructor(val value: String) { return Result.failure(SecurityException("Path must be relative, but contains drive letter: $normalized")) } - if (normalized.contains(ENCODED_SLASH_REGEX)) { + // Check for encoded slashes or traversal sequences + if (normalized.contains(ENCODED_SLASH_REGEX) || normalized.contains(TRAVERSAL_REGEX)) { return Result.failure(SecurityException("Path traversal attempt detected: $normalized")) } - val segments = normalized.replace('\\', '/').split('/').filter { it.isNotEmpty() && it != "." } - val validationSegments = segments.map { segment -> - segment - .replace("\u2024", ".") - .replace("\uFF0E", ".") - .replace("\u3002", ".") - .replace("%2e", ".", ignoreCase = true) - } - if (validationSegments.any { - it == ".." || it.contains(Regex("%25(?:2e|2f|5c)", RegexOption.IGNORE_CASE)) - } - ) { + // Normalized path checks: convert unicode dot variants to regular dot for safety check + val sanitized = normalized + .replace("\u2024", ".") + .replace("\uFF0E", ".") + .replace("\u3002", ".") + .replace("%2e", ".", ignoreCase = true) + .replace("%2f", "/", ignoreCase = true) + .replace("%5c", "\\", ignoreCase = true) + + if (sanitized.contains(TRAVERSAL_REGEX) || sanitized.contains("../") || sanitized.contains("..\\")) { return Result.failure(SecurityException("Path traversal attempt detected: $normalized")) } - - // Validation uses a security-normalized view, but the filename itself is not decoded or rewritten. - return Result.success(RelativePath(segments.joinToString("/"))) + + return Result.success(RelativePath(normalized)) } } } diff --git a/plugin-api/src/commonTest/kotlin/org/wip/plugintoolkit/api/RelativePathTest.kt b/plugin-api/src/commonTest/kotlin/org/wip/plugintoolkit/api/RelativePathTest.kt index b328f843..96ae01bd 100644 --- a/plugin-api/src/commonTest/kotlin/org/wip/plugintoolkit/api/RelativePathTest.kt +++ b/plugin-api/src/commonTest/kotlin/org/wip/plugintoolkit/api/RelativePathTest.kt @@ -52,11 +52,5 @@ class RelativePathTest { val path = " foo/bar.txt ".toRelativePath() assertTrue(path.isSuccess) assertEquals("foo/bar.txt", path.getOrNull()?.value, "Should trim whitespace") - assertEquals(RelativePath.ROOT, ".".toRelativePath().getOrThrow()) - assertEquals(RelativePath.ROOT, "././".toRelativePath().getOrThrow()) - assertEquals("foo/bar", "foo/./bar".toRelativePath().getOrThrow().value) - assertEquals("foo/bar", "foo\\bar".toRelativePath().getOrThrow().value) - assertEquals("file%2ename.txt", "file%2ename.txt".toRelativePath().getOrThrow().value) - assertEquals("file\u2024txt", "file\u2024txt".toRelativePath().getOrThrow().value) } }