Harden ClawHub ZIP extraction and Termux command validation

AndyClaw/src/main/java/org/ethereumphone/andyclaw/extensions/clawhub/ClawHubApi.kt

@@ -14,6 +14,7 @@ import okhttp3.Response
 import okhttp3.ResponseBody
 import java.io.File
 import java.io.IOException
+import java.io.OutputStream
 import java.util.concurrent.TimeUnit
 import java.util.logging.Logger
 import java.util.zip.ZipInputStream
@@ -188,33 +189,118 @@ class ClawHubApi(
 
     /**
      * Extract a ZIP response body into [targetDir], writing files relative
-     * to the target. Skips entries outside the target (zip-slip guard).
+     * to the target.
+     *
+     * Security controls:
+     * - path containment check using canonical paths + separator-safe matching
+     * - max entry count
+     * - max single-entry uncompressed bytes
+     * - max total uncompressed bytes (zip-bomb mitigation)
      */
     private fun extractZip(body: ResponseBody, targetDir: File) {
+        val canonicalTargetDir = targetDir.canonicalFile
+        val canonicalTargetPath = canonicalTargetDir.toPath()
+        var entryCount = 0
+        var totalUncompressedBytes = 0L
+
         ZipInputStream(body.byteStream()).use { zis ->
             var entry = zis.nextEntry
             while (entry != null) {
-                val outFile = File(targetDir, entry.name).canonicalFile
+                entryCount++
+                if (entryCount > MAX_ZIP_ENTRIES) {
+                    throw SecurityException("Zip exceeds entry limit ($MAX_ZIP_ENTRIES)")
+                }
+
+                if (entry.name.length > MAX_ZIP_ENTRY_NAME_CHARS) {
+                    throw SecurityException("Zip entry name too long: ${entry.name.take(80)}")
+                }
+
+                if (entry.size > MAX_ZIP_ENTRY_UNCOMPRESSED_BYTES) {
+                    throw SecurityException(
+                        "Zip entry too large by declared size: ${entry.name}"
+                    )
+                }
 
-                // Zip-slip guard: ensure extracted path stays within targetDir
-                if (!outFile.path.startsWith(targetDir.canonicalPath)) {
+                val outFile = File(canonicalTargetDir, entry.name).canonicalFile
+
+                // Zip-slip guard: ensure extracted path stays within target directory.
+                if (!outFile.toPath().startsWith(canonicalTargetPath)) {
                     throw SecurityException("Zip entry escapes target dir: ${entry.name}")
                 }
 
                 if (entry.isDirectory) {
-                    outFile.mkdirs()
+                    if (!outFile.exists() && !outFile.mkdirs()) {
+                        throw IOException("Failed to create directory for zip entry: ${entry.name}")
+                    }
                 } else {
-                    outFile.parentFile?.mkdirs()
-                    outFile.outputStream().use { out ->
-                        zis.copyTo(out)
+                    val parent = outFile.parentFile
+                    if (parent != null && !parent.exists() && !parent.mkdirs()) {
+                        throw IOException("Failed to create parent directory for ${entry.name}")
+                    }
+
+                    val bytesRemaining = MAX_ZIP_TOTAL_UNCOMPRESSED_BYTES - totalUncompressedBytes
+                    if (bytesRemaining <= 0) {
+                        throw SecurityException(
+                            "Zip exceeds total uncompressed size limit ($MAX_ZIP_TOTAL_UNCOMPRESSED_BYTES bytes)"
+                        )
+                    }
+
+                    val bytesWritten = outFile.outputStream().use { out ->
+                        copyZipEntryWithLimits(
+                            zis = zis,
+                            out = out,
+                            entryName = entry.name,
+                            entryLimitBytes = MAX_ZIP_ENTRY_UNCOMPRESSED_BYTES,
+                            totalRemainingBytes = bytesRemaining,
+                        )
+                    }
+                    totalUncompressedBytes += bytesWritten
+
+                    if (totalUncompressedBytes > MAX_ZIP_TOTAL_UNCOMPRESSED_BYTES) {
+                        throw SecurityException(
+                            "Zip exceeds total uncompressed size limit ($MAX_ZIP_TOTAL_UNCOMPRESSED_BYTES bytes)"
+                        )
                     }
                 }
+
                 zis.closeEntry()
                 entry = zis.nextEntry
             }
         }
     }
 
+    private fun copyZipEntryWithLimits(
+        zis: ZipInputStream,
+        out: OutputStream,
+        entryName: String,
+        entryLimitBytes: Long,
+        totalRemainingBytes: Long,
+    ): Long {
+        val buffer = ByteArray(ZIP_COPY_BUFFER_BYTES)
+        var entryBytes = 0L
+
+        while (true) {
+            val read = zis.read(buffer)
+            if (read <= 0) break
+
+            entryBytes += read
+            if (entryBytes > entryLimitBytes) {
+                throw SecurityException(
+                    "Zip entry exceeds uncompressed size limit ($entryLimitBytes bytes): $entryName"
+                )
+            }
+            if (entryBytes > totalRemainingBytes) {
+                throw SecurityException(
+                    "Zip exceeds total uncompressed size limit ($MAX_ZIP_TOTAL_UNCOMPRESSED_BYTES bytes)"
+                )
+            }
+
+            out.write(buffer, 0, read)
+        }
+
+        return entryBytes
+    }
+
     companion object {
         const val DEFAULT_REGISTRY = "https://clawhub.com"
 
@@ -224,6 +310,13 @@ class ClawHubApi(
         private const val V1_RESOLVE = "/api/v1/resolve"
         private const val V1_DOWNLOAD = "/api/v1/download"
 
+        // ZIP hardening caps to mitigate zip-bomb and path abuse.
+        private const val MAX_ZIP_ENTRIES = 2_000
+        private const val MAX_ZIP_ENTRY_NAME_CHARS = 512
+        private const val MAX_ZIP_ENTRY_UNCOMPRESSED_BYTES = 5L * 1024 * 1024
+        private const val MAX_ZIP_TOTAL_UNCOMPRESSED_BYTES = 25L * 1024 * 1024
+        private const val ZIP_COPY_BUFFER_BYTES = 8 * 1024
+
         private fun defaultClient(): OkHttpClient = OkHttpClient.Builder()
             .connectTimeout(15, TimeUnit.SECONDS)
             .readTimeout(30, TimeUnit.SECONDS)

app/build.gradle.kts

@@ -124,6 +124,7 @@ dependencies {
     // ContactsSDK for ethOS contacts with ETH address support
     implementation("com.github.EthereumPhone:ContactsSDK:0.1.0")
     testImplementation(libs.junit)
+    testImplementation("com.squareup.okhttp3:mockwebserver:4.12.0")
     androidTestImplementation(libs.androidx.junit)
     androidTestImplementation(libs.androidx.espresso.core)
     androidTestImplementation(platform(libs.androidx.compose.bom))

app/src/main/java/org/ethereumphone/andyclaw/skills/termux/ClawHubTermuxSkillAdapter.kt

@@ -10,7 +10,6 @@ import kotlinx.serialization.json.jsonPrimitive
 import kotlinx.serialization.json.put
 import org.ethereumphone.andyclaw.skills.AndyClawSkill
 import org.ethereumphone.andyclaw.skills.Skill
-import org.ethereumphone.andyclaw.skills.SkillEntry
 import org.ethereumphone.andyclaw.skills.SkillExecutionSpec
 import org.ethereumphone.andyclaw.skills.SkillFrontmatter
 import org.ethereumphone.andyclaw.skills.SkillLoader
@@ -81,6 +80,12 @@ class ClawHubTermuxSkillAdapter(
                 ?.filter { it.isDirectory && File(it, "SKILL.md").isFile }
                 ?.mapNotNull { dir ->
                     val slugName = dir.name
+                    val safeSlug = runCatching { TermuxShell.validateSlug(slugName) }
+                        .getOrElse {
+                            Log.w(TAG, "Skipping skill with invalid slug '$slugName': ${it.message}")
+                            return@mapNotNull null
+                        }
+
                     val parsedSkill = SkillLoader.parseSkillFile(
                         File(dir, "SKILL.md"), dir,
                     ) ?: run {
@@ -99,7 +104,7 @@ class ClawHubTermuxSkillAdapter(
                     if (exec != null && exec.type == "termux") {
                         ClawHubTermuxSkillAdapter(
                             skill = parsedSkill,
-                            slug = slugName,
+                            slug = safeSlug,
                             installedVersion = version,
                             executionSpec = exec,
                             metadata = meta,
@@ -154,8 +159,18 @@ class ClawHubTermuxSkillAdapter(
             ?: return SkillResult.Error("Unknown tool: $tool")
 
         // Build and execute the command
-        val command = buildCommand(toolSpec, params)
-        val skillHome = sync.skillHomePath(slug)
+        val command = try {
+            buildCommand(toolSpec, params)
+        } catch (e: IllegalArgumentException) {
+            return SkillResult.Error("Invalid Termux execution spec: ${e.message}")
+        }
+
+        val skillHome = try {
+            sync.skillHomePath(slug)
+        } catch (e: IllegalArgumentException) {
+            return SkillResult.Error("Invalid Termux skill path for '$slug': ${e.message}")
+        }
+
         val result = runner.run(command, workdir = skillHome, timeoutMs = 60_000)
 
         if (result.internalError != null) {
@@ -214,8 +229,10 @@ class ClawHubTermuxSkillAdapter(
     // ── Command building ────────────────────────────────────────────
 
     private fun buildCommand(toolSpec: SkillToolSpec, params: JsonObject): String {
-        val entrypoint = toolSpec.entrypoint ?: executionSpec.entrypoint
+        val rawEntrypoint = toolSpec.entrypoint ?: executionSpec.entrypoint
+        val entrypoint = TermuxShell.validateRelativePath(rawEntrypoint, "entrypoint")
         val scriptPath = "${sync.skillHomePath(slug)}/$entrypoint"
+        val quotedScriptPath = TermuxShell.quote(scriptPath)
 
         val allSimpleStrings = toolSpec.args.values.all { it.type == "string" }
         val argCount = toolSpec.args.size
@@ -225,20 +242,14 @@ class ClawHubTermuxSkillAdapter(
             val positional = toolSpec.args.keys.mapNotNull { key ->
                 params[key]?.jsonPrimitive?.contentOrNull
             }
-            val escaped = positional.joinToString(" ") { shellEscape(it) }
-            "'$scriptPath' $escaped".trim()
+            val escaped = positional.joinToString(" ") { TermuxShell.quote(it) }
+            if (escaped.isBlank()) quotedScriptPath else "$quotedScriptPath $escaped"
         } else {
             // JSON mode: entrypoint <tool> '<json>'
-            val json = params.toString().replace("'", "'\\''")
-            "'$scriptPath' '${toolSpec.name}' '$json'"
+            "$quotedScriptPath ${TermuxShell.quote(toolSpec.name)} ${TermuxShell.quote(params.toString())}"
         }
     }
 
-    private fun shellEscape(value: String): String {
-        // Wrap in single quotes, escaping embedded single quotes
-        return "'" + value.replace("'", "'\\''") + "'"
-    }
-
     // ── Manifest / tool definition builders ─────────────────────────
 
     private fun buildDescription(): String = buildString {

app/src/main/java/org/ethereumphone/andyclaw/skills/termux/TermuxShell.kt

@@ -0,0 +1,87 @@
+package org.ethereumphone.andyclaw.skills.termux
+
+/**
+ * Shared validation and escaping helpers for constructing shell commands
+ * executed via Termux.
+ */
+internal object TermuxShell {
+
+    private const val MAX_SLUG_CHARS = 128
+    private const val MAX_PATH_SEGMENT_CHARS = 255
+    private val BIN_REGEX = Regex("^[a-z0-9][a-z0-9+._-]{0,63}$")
+
+    /**
+     * Single-quote shell escaping for POSIX-compatible shells.
+     */
+    fun quote(value: String): String = "'" + value.replace("'", "'\\''") + "'"
+
+    /**
+     * Validate a skill slug used in Termux paths.
+     *
+     * The slug rules are intentionally permissive for backward compatibility:
+     * reject traversal/control characters and path separators, but allow
+     * mixed-case and spaces.
+     */
+    fun validateSlug(rawSlug: String): String {
+        val slug = rawSlug.trim()
+        require(slug.isNotEmpty()) { "Slug cannot be blank" }
+        require(slug.length <= MAX_SLUG_CHARS) {
+            "Slug is too long (max $MAX_SLUG_CHARS chars)"
+        }
+        require(slug != "." && slug != "..") {
+            "Slug cannot be '.' or '..'"
+        }
+        require('/' !in slug && '\\' !in slug) {
+            "Slug cannot contain path separators"
+        }
+        require(!containsControlChars(slug)) {
+            "Slug contains control characters"
+        }
+        return slug
+    }
+
+    /**
+     * Validate a relative path inside a synced skill directory.
+     *
+     * Path is normalised to forward slashes and traversal is rejected.
+     * Segment characters are permissive to avoid breaking existing skills,
+     * while still rejecting control chars and separators.
+     */
+    fun validateRelativePath(rawPath: String, label: String = "path"): String {
+        val normalised = rawPath.trim().replace('\\', '/')
+        require(normalised.isNotEmpty()) { "$label cannot be blank" }
+        require(!normalised.startsWith('/')) { "$label must be relative" }
+
+        val segments = normalised.split('/')
+        require(segments.none { it.isBlank() || it == "." || it == ".." }) {
+            "$label contains invalid traversal segments"
+        }
+        require(segments.all { it.length <= MAX_PATH_SEGMENT_CHARS }) {
+            "$label contains an overly long segment (max $MAX_PATH_SEGMENT_CHARS chars)"
+        }
+        require(segments.all { '/' !in it && '\\' !in it }) {
+            "$label contains invalid separators"
+        }
+        require(segments.all { !containsControlChars(it) }) {
+            "$label contains control characters"
+        }
+
+        return segments.joinToString("/")
+    }
+
+    /**
+     * Validate package/binary names declared in SKILL.md metadata.
+     */
+    fun validateBinName(rawBin: String): String {
+        val bin = rawBin.trim()
+        require(bin.isNotEmpty()) { "Binary name cannot be blank" }
+        require(BIN_REGEX.matches(bin)) {
+            "Invalid binary name '$rawBin'. Allowed pattern: ${BIN_REGEX.pattern}"
+        }
+        return bin
+    }
+
+    private fun containsControlChars(value: String): Boolean {
+        return value.any { ch -> ch.code < 32 || ch.code == 127 }
+    }
+}