Harden Termux command validation and shell escaping

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 }
+    }
+}

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

@@ -55,21 +55,33 @@ class TermuxSkillSync(
         .getSharedPreferences(PREFS_NAME, Context.MODE_PRIVATE)
 
     /** Returns the absolute Termux-side path for a synced skill. */
-    fun skillHomePath(slug: String): String =
-        "${TermuxCommandRunner.TERMUX_HOME}/$SKILLS_BASE/$slug"
+    fun skillHomePath(slug: String): String {
+        val safeSlug = TermuxShell.validateSlug(slug)
+        return "${TermuxCommandRunner.TERMUX_HOME}/$SKILLS_BASE/$safeSlug"
+    }
 
     // ── Sync tracking ───────────────────────────────────────────────
 
     fun getSyncedSlugs(): Set<String> =
         prefs.getStringSet(KEY_SYNCED_SLUGS, emptySet()) ?: emptySet()
 
     private fun markSynced(slug: String) {
-        val slugs = getSyncedSlugs().toMutableSet().apply { add(slug) }
+        val safeSlug = runCatching { TermuxShell.validateSlug(slug) }
+            .getOrElse {
+                Log.w(TAG, "Refusing to mark invalid synced slug '$slug': ${it.message}")
+                return
+            }
+        val slugs = getSyncedSlugs().toMutableSet().apply { add(safeSlug) }
         prefs.edit().putStringSet(KEY_SYNCED_SLUGS, slugs).apply()
     }
 
     private fun markRemoved(slug: String) {
-        val slugs = getSyncedSlugs().toMutableSet().apply { remove(slug) }
+        val safeSlug = runCatching { TermuxShell.validateSlug(slug) }
+            .getOrElse {
+                Log.w(TAG, "Refusing to remove invalid synced slug '$slug': ${it.message}")
+                return
+            }
+        val slugs = getSyncedSlugs().toMutableSet().apply { remove(safeSlug) }
         prefs.edit().putStringSet(KEY_SYNCED_SLUGS, slugs).apply()
     }
 
@@ -86,7 +98,13 @@ class TermuxSkillSync(
             return SyncResult(false, "Termux is not installed")
         }
 
-        val skillHome = skillHomePath(slug)
+        val safeSlug = try {
+            TermuxShell.validateSlug(slug)
+        } catch (e: IllegalArgumentException) {
+            return SyncResult(false, "Invalid slug '$slug': ${e.message}")
+        }
+
+        val skillHome = skillHomePath(safeSlug)
 
         // Collect files, respecting size limits
         val files = sourceDir.walkTopDown()
@@ -99,7 +117,7 @@ class TermuxSkillSync(
 
         // Wipe old version and create base directory
         val mkdirResult = runner.run(
-            "rm -rf '$skillHome' && mkdir -p '$skillHome'",
+            "rm -rf ${TermuxShell.quote(skillHome)} && mkdir -p ${TermuxShell.quote(skillHome)}",
             timeoutMs = SYNC_TIMEOUT_MS,
         )
         if (!mkdirResult.isSuccess) {
@@ -108,39 +126,69 @@ class TermuxSkillSync(
 
         // Write each file via base64
         for (file in files) {
-            val relativePath = file.relativeTo(sourceDir).path
+            val rawRelativePath = file.relativeTo(sourceDir).path.replace(File.separatorChar, '/')
+            val relativePath = try {
+                TermuxShell.validateRelativePath(rawRelativePath, "skill file path")
+            } catch (e: IllegalArgumentException) {
+                return SyncResult(false, "Invalid file path '$rawRelativePath': ${e.message}")
+            }
+
             val targetPath = "$skillHome/$relativePath"
-            val targetDir = targetPath.substringBeforeLast('/')
+            val targetDir = targetPath.substringBeforeLast('/', skillHome)
             val base64 = Base64.getEncoder().encodeToString(file.readBytes())
 
             val writeResult = runner.run(
-                "mkdir -p '$targetDir' && printf '%s' '$base64' | base64 -d > '$targetPath'",
+                "mkdir -p ${TermuxShell.quote(targetDir)} && " +
+                    "printf '%s' ${TermuxShell.quote(base64)} | " +
+                    "base64 -d > ${TermuxShell.quote(targetPath)}",
                 timeoutMs = 15_000,
             )
             if (!writeResult.isSuccess) {
-                Log.w(TAG, "Failed to write $relativePath for $slug: ${writeResult.stderr}")
+                Log.w(TAG, "Failed to write $relativePath for $safeSlug: ${writeResult.stderr}")
                 return SyncResult(false, "Failed to write $relativePath: ${writeResult.stderr}")
             }
         }
 
         // Mark all scripts executable
         runner.run(
-            "find '$skillHome' -type f \\( -name '*.sh' -o -path '*/scripts/*' \\) -exec chmod +x {} +",
+            "find ${TermuxShell.quote(skillHome)} -type f \\( -name '*.sh' -o -path '*/scripts/*' \\) -exec chmod +x {} +",
             timeoutMs = 10_000,
         )
 
-        markSynced(slug)
-        Log.i(TAG, "Synced $slug (${files.size} files, ${totalBytes / 1024} KB)")
+        markSynced(safeSlug)
+        Log.i(TAG, "Synced $safeSlug (${files.size} files, ${totalBytes / 1024} KB)")
         return SyncResult(true, fileCount = files.size)
     }
 
     /**
      * Run the skill's declared setup script (if any) after sync.
      */
     suspend fun runSetup(slug: String, setupPath: String): TermuxCommandResult {
-        val skillHome = skillHomePath(slug)
+        val safeSlug = try {
+            TermuxShell.validateSlug(slug)
+        } catch (e: IllegalArgumentException) {
+            return TermuxCommandResult(
+                exitCode = -1,
+                stdout = "",
+                stderr = "",
+                internalError = "Invalid slug '$slug': ${e.message}",
+            )
+        }
+        val safeSetupPath = try {
+            TermuxShell.validateRelativePath(setupPath, "setup path")
+        } catch (e: IllegalArgumentException) {
+            return TermuxCommandResult(
+                exitCode = -1,
+                stdout = "",
+                stderr = "",
+                internalError = "Invalid setup path '$setupPath': ${e.message}",
+            )
+        }
+
+        val skillHome = skillHomePath(safeSlug)
+        val quotedSetupPath = TermuxShell.quote(safeSetupPath)
         return runner.run(
-            "cd '$skillHome' && chmod +x '$setupPath' && bash '$setupPath'",
+            "cd ${TermuxShell.quote(skillHome)} && chmod +x $quotedSetupPath && bash $quotedSetupPath",
             timeoutMs = 120_000,
         )
     }
@@ -211,10 +259,21 @@ class TermuxSkillSync(
             return TermuxCommandResult(exitCode = 0, stdout = "", stderr = "")
         }
 
+        val safeBins = try {
+            bins.map { TermuxShell.validateBinName(it) }.distinct()
+        } catch (e: IllegalArgumentException) {
+            return TermuxCommandResult(
+                exitCode = -1,
+                stdout = "",
+                stderr = "",
+                internalError = "Invalid required binary name: ${e.message}",
+            )
+        }
+
         ensureNonInteractiveDefaults()
 
-        val checkScript = bins.joinToString("; ") { bin ->
-            "command -v '$bin' >/dev/null 2>&1 || MISSING=\"\$MISSING $bin\""
+        val checkScript = safeBins.joinToString("; ") { bin ->
+            "command -v ${TermuxShell.quote(bin)} >/dev/null 2>&1 || MISSING=\"\$MISSING $bin\""
         }
         val script = "MISSING=''; $checkScript; " +
             "if [ -n \"\$MISSING\" ]; then " +
@@ -232,9 +291,15 @@ class TermuxSkillSync(
      * Remove a synced skill from Termux home.
      */
     suspend fun removeSkill(slug: String): Boolean {
-        val skillHome = skillHomePath(slug)
-        val result = runner.run("rm -rf '$skillHome'", timeoutMs = 10_000)
-        markRemoved(slug)
+        val safeSlug = runCatching { TermuxShell.validateSlug(slug) }
+            .getOrElse {
+                Log.w(TAG, "Cannot remove invalid slug '$slug': ${it.message}")
+                return false
+            }
+
+        val skillHome = skillHomePath(safeSlug)
+        val result = runner.run("rm -rf ${TermuxShell.quote(skillHome)}", timeoutMs = 10_000)
+        markRemoved(safeSlug)
         return result.isSuccess
     }