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 1/9] 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 2/9] 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 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 3/9] 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 4/9] 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 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 5/9] 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 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 6/9] 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 7/9] 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 8/9] 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 9/9] 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) } }