Greptile Summary

This PR redesigns several Android UI screens — ConnectTabScreen, SettingsSheet, VoiceTabScreen, ChatComposer, ChatMessageViews, and ChatSheetContent — to use grouped card sections, cleaner status layouts, and a more compact chat composer. No backend, permission-model, or gateway logic was changed.

Key findings from the review:

• ChatSheetContent.kt is missing windowInsetsPadding(WindowInsets.safeDrawing.only(WindowInsetsSides.Bottom)) on its outer Column, while both VoiceTabScreen and SettingsSheet include it. The imePadding() is also placed only on the inner composer Row rather than the outer container. On gesture-navigation devices this can cause the navigation bar to overlap the chat composer/Send button.
• SettingsSheet.kt has an unused Icons import and a dead settingsDangerButtonColors() helper function left over from the refactor.
• ChatMessageViews.kt contains a redundant !! non-null assertion inside an already null-checked branch in ChatBase64Image.
• All other files (ConnectTabScreen, VoiceTabScreen, ChatComposer) look clean — logic is unchanged and the UI restructuring is consistent.

Confidence Score: 3/5

• Safe to merge after addressing the missing bottom-inset padding in ChatSheetContent; all other issues are cosmetic.
• The redesign is purely visual and does not touch any backend, permissions, or data-flow logic. However, the missing windowInsetsPadding in ChatSheetContent is a real UX regression on gesture-navigation devices where the composer row can sit behind the navigation bar. The author also acknowledged no on-device UI walkthrough was performed, so this inset issue was not caught. Fixing it is straightforward, which keeps the score from going lower.
• apps/android/app/src/main/java/ai/openclaw/app/ui/chat/ChatSheetContent.kt (missing inset padding), apps/android/app/src/main/java/ai/openclaw/app/ui/SettingsSheet.kt (unused import + dead code)

This is a comment left during a code review.
Path: apps/android/app/src/main/java/ai/openclaw/app/ui/chat/ChatSheetContent.kt
Line: 94-100

Comment:
**Missing bottom-inset padding for navigation bar**

Both `VoiceTabScreen` and `SettingsSheet` apply `.windowInsetsPadding(WindowInsets.safeDrawing.only(WindowInsetsSides.Bottom))` to their outer containers. `ChatSheetContent` only applies `imePadding()` to the inner `Row` wrapping `ChatComposer`, which guards against the software keyboard but does nothing for the gesture navigation bar. On gesture-navigation devices this means the composer row can sit behind the navigation bar, making the Send button partially or fully inaccessible.

Consider applying inset handling at the outer `Column` level, consistent with the other screens:

```suggestion
  Column(
    modifier =
      Modifier
        .fillMaxSize()
        .padding(horizontal = 20.dp, vertical = 12.dp)
        .imePadding()
        .windowInsetsPadding(WindowInsets.safeDrawing.only(WindowInsetsSides.Bottom)),
    verticalArrangement = Arrangement.spacedBy(8.dp),
  ) {
```

(The `imePadding()` on the inner `Row` at line 121 can then be removed.)

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: apps/android/app/src/main/java/ai/openclaw/app/ui/SettingsSheet.kt
Line: 754-761

Comment:
**Dead code: `settingsDangerButtonColors` is never called**

`settingsDangerButtonColors()` is defined but has no call site anywhere in this file. If danger-styled buttons are not needed in the new layout, this function should be removed to keep the file clean.

```suggestion
```
(Delete lines 754–761.)

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: apps/android/app/src/main/java/ai/openclaw/app/ui/SettingsSheet.kt
Line: 37

Comment:
**Unused import**

`androidx.compose.material.icons.Icons` is imported but never referenced in the file — no `Icons.Default.*` or `Icon(...)` composable is used after the redesign. This will produce an IDE warning and should be removed.

```suggestion
```
(Delete line 37.)

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: apps/android/app/src/main/java/ai/openclaw/app/ui/chat/ChatMessageViews.kt
Line: 246

Comment:
**Redundant non-null assertion**

`image` is a local `val` that was already null-checked two lines above (`if (image != null)`). Kotlin's smart cast makes the `!!` operator unnecessary here — the compiler already knows `image` is non-null inside this branch. The assertion should be removed to avoid misleading readers into thinking a nullable access is possible.

```suggestion
            bitmap = image,
```

How can I resolve this? If you propose a fix, please make it concise.

_{Last reviewed commit: 2f62eba}

Landed on main.

• Landed source commit: 2f62eba
• Merge commit: 7638052

🔒 Aisle Security Analysis

We found 2 potential security issue(s) in this PR:

1. 🟡 Fragile connection gating via substring match on remotely-influenced status text (client-side DoS)

Description

The Connect button behavior is gated by a substring match on statusText:

• statusText is a UI-facing string (MainViewModel.statusText -> NodeRuntime.statusText) that is not a stable state indicator.
• NodeRuntime can set the underlying operatorStatusText / nodeStatusText from GatewaySession disconnect callbacks.
• GatewaySession composes disconnect messages using server-provided WebSocket close reasons (and exception messages), which can be influenced by a malicious/compromised gateway.
• If the resulting statusText contains the magic substring "operator offline", the UI refuses to perform a normal connect with the user-provided configuration and instead calls refreshGatewayConnection(). This can trap users in a reconnect loop to a previously-cached endpoint and prevent switching to a new endpoint/credentials without killing the app.

Vulnerable code:

if (statusText.contains("operator offline", ignoreCase = true)) {
  validationText = null
  viewModel.refreshGatewayConnection()
  return@​Button
}

Provenance (remote influence):

• ConnectTabScreen.statusText comes from NodeRuntime.statusText.
• NodeRuntime.operatorStatusText is set from GatewaySession.onDisconnected(message).
• GatewaySession passes disconnect messages like "Gateway closed: $reason" where $reason is the WebSocket close reason sent by the server.

This creates a practical client-side denial-of-service / lock-in vector when connecting to an attacker-controlled gateway (or when a gateway is compromised): the gateway can repeatedly close with a reason containing operator offline, causing the app to keep taking the refresh path and preventing a clean connect flow with new settings.

Recommendation

Do not use user-facing status strings for control flow.

• Introduce an explicit connection state model (sealed class / enum + structured error codes) in the ViewModel/runtime, e.g. GatewayConnectionState with fields like operatorConnected, nodeConnected, lastDisconnectCode.
• Gate the refresh/reconnect path on that structured state rather than statusText.contains(...).

Example (sketch):

enum class GatewayDisconnectCode { OPERATOR_OFFLINE, AUTH_FAILED, NETWORK_ERROR, UNKNOWN }

data class GatewayConnectionState(
  val operatorConnected: Boolean,
  val nodeConnected: Boolean,
  val lastDisconnectCode: GatewayDisconnectCode? = null,
)

​// UI
val state by viewModel.gatewayConnectionState.collectAsState()
Button(onClick = {
  if (!state.operatorConnected && state.nodeConnected && state.lastDisconnectCode == GatewayDisconnectCode.OPERATOR_OFFLINE) {
    viewModel.refreshGatewayConnection()
  } else {
    viewModel.connectManual()
  }
}) { /* ... */ }

Additionally, ensure disconnect reasons from the network are mapped to fixed codes (and not used directly as logic inputs).

2. 🔵 Chat message provenance spoofing: unknown roles rendered as “OpenClaw” (assistant)

Description

The chat UI derives the displayed sender label from ChatMessage.role and maps any role other than user and system to “OpenClaw”.

• ChatMessage.role is populated from gateway-controlled JSON (chat.history) without a whitelist/enum (see ChatController.parseHistory: role is taken directly from obj["role"]).
• In the UI, ChatMessageBubble() normalizes the role string and calls roleLabel(role).
• roleLabel() now falls back to “OpenClaw” for all other roles, so messages with roles like tool, function, developer, or any unexpected value will be misattributed as the assistant/brand.

Impact (security UX / anti-spoofing):

• A compromised/malicious gateway (or unexpected upstream role) can cause tool outputs or non-assistant messages to appear as if they were authored by “OpenClaw”, enabling phishing/social engineering and hiding provenance.

Vulnerable code:

private fun roleLabel(role: String): String {
  return when (role) {
    "user" -> "You"
    "system" -> "System"
    else -> "OpenClaw"
  }
}

Recommendation

Use an explicit allowlist of roles and do not treat unknown roles as the assistant.

• Add explicit handling for roles you expect (at minimum: assistant, tool), and a safe fallback such as "Unknown" (optionally include the raw role for debugging).
• Consider using a sealed class/enum for roles at the model/parsing layer to prevent arbitrary strings reaching the renderer.
• Give tool/function roles distinct styling (label + color) so users can distinguish tool output from assistant text.

Example fix:

private fun roleLabel(role: String): String = when (role) {
  "user" -> "You"
  "assistant" -> "OpenClaw"
  "system" -> "System"
  "tool" -> "Tool"
  "developer" -> "Developer"
  else -> "Unknown" // or "Unknown (${role.take(24)})"
}

And update bubbleStyle() similarly to style tool separately from assistant.

Analyzed PR: #44894 at commit 2f62eba

_{Last updated on: 2026-03-13T09:19:05Z}