Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
96 changes: 96 additions & 0 deletions main/channels/telegram/telegram_bot.c
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,65 @@ static int64_t s_last_offset_save_us = 0;
static uint64_t s_seen_msg_keys[TG_DEDUP_CACHE_SIZE] = {0};
static size_t s_seen_msg_idx = 0;

/* Allowed user IDs for access control */
static int64_t s_allowed_users[MIMI_TG_MAX_ALLOWED_USERS] = {0};
static size_t s_allowed_users_count = 0;

static bool is_id_allowed(int64_t user_id, int64_t chat_id)
{
if (s_allowed_users_count == 0) return true;
for (size_t i = 0; i < s_allowed_users_count; i++) {
if (s_allowed_users[i] == user_id) return true;
if (s_allowed_users[i] == chat_id) return true;
}
return false;
}
Comment on lines +35 to +43
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Synchronize allowlist reads/writes to avoid transient auth bypass.

Line 47 sets s_allowed_users_count = 0, while Line 37 interprets 0 as allow-all. Without locking, an update can interleave with Line 428 checks and temporarily allow unauthorized messages.

Suggested fix (atomic snapshot + lock)
+static portMUX_TYPE s_allowed_users_mux = portMUX_INITIALIZER_UNLOCKED;
+
+static void set_allowed_users_snapshot(const int64_t *ids, size_t count)
+{
+    taskENTER_CRITICAL(&s_allowed_users_mux);
+    memset(s_allowed_users, 0, sizeof(s_allowed_users));
+    memcpy(s_allowed_users, ids, count * sizeof(int64_t));
+    s_allowed_users_count = count;
+    taskEXIT_CRITICAL(&s_allowed_users_mux);
+}
+
 static bool is_id_allowed(int64_t user_id, int64_t chat_id)
 {
-    if (s_allowed_users_count == 0) return true;
-    for (size_t i = 0; i < s_allowed_users_count; i++) {
-        if (s_allowed_users[i] == user_id) return true;
-        if (s_allowed_users[i] == chat_id) return true;
+    int64_t ids[MIMI_TG_MAX_ALLOWED_USERS] = {0};
+    size_t count = 0;
+    taskENTER_CRITICAL(&s_allowed_users_mux);
+    count = s_allowed_users_count;
+    memcpy(ids, s_allowed_users, sizeof(ids));
+    taskEXIT_CRITICAL(&s_allowed_users_mux);
+
+    if (count == 0) return true;
+    for (size_t i = 0; i < count; i++) {
+        if (ids[i] == user_id) return true;
+        if (ids[i] == chat_id) return true;
     }
     return false;
 }

Also applies to: 45-62, 415-433, 640-641

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@main/telegram/telegram_bot.c` around lines 35 - 43, The allowlist access is
racy: is_id_allowed reads s_allowed_users_count and s_allowed_users without
synchronization, allowing a transient auth bypass when another thread sets
s_allowed_users_count = 0 or updates the array. Fix by adding a mutex (e.g.,
s_allowed_users_mutex) and ensure all writers and readers of s_allowed_users and
s_allowed_users_count acquire it; specifically modify is_id_allowed to take the
lock, perform an atomic snapshot (copy s_allowed_users_count and the relevant
entries into local variables or a stack buffer), release the lock, then check
the snapshot; likewise protect any code that sets s_allowed_users_count or
updates s_allowed_users so updates and reads cannot interleave.


static void parse_allowed_users(const char *str)
{
s_allowed_users_count = 0;
if (!str || !str[0]) return;

char buf[256];
strncpy(buf, str, sizeof(buf) - 1);
buf[sizeof(buf) - 1] = '\0';

char *tok = strtok(buf, ",");
while (tok && s_allowed_users_count < MIMI_TG_MAX_ALLOWED_USERS) {
int64_t uid = strtoll(tok, NULL, 10);
if (uid != 0) {
s_allowed_users[s_allowed_users_count++] = uid;
}
Comment on lines +54 to +59
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# First, find and read the relevant file
find . -name "telegram_bot.c" -path "*/telegram/*"

Repository: memovai/mimiclaw

Length of output: 90


🏁 Script executed:

#!/bin/bash
# Read the file to examine the problematic code
wc -l main/telegram/telegram_bot.c

Repository: memovai/mimiclaw

Length of output: 92


🏁 Script executed:

#!/bin/bash
# Read lines 40-70 to see the parse_allowed_users function
sed -n '40,70p' main/telegram/telegram_bot.c

Repository: memovai/mimiclaw

Length of output: 854


🏁 Script executed:

#!/bin/bash
# Read lines 620-650 to see the telegram_set_allowed_users function and ESP_ERROR_CHECK usage
sed -n '620,650p' main/telegram/telegram_bot.c

Repository: memovai/mimiclaw

Length of output: 1017


Validate allowlist input and return errors instead of panicking in setter.

Lines 54-59 silently ignore malformed tokens (strtoll with NULL endpoint means "abc" returns 0, indistinguishable from valid input, and is skipped without error). The function has no way to report parsing failures (void return). Additionally, lines 632-639 use ESP_ERROR_CHECK in a public API returning esp_err_t, which panics instead of propagating errors. This should validate input, reject malformed entries, and return proper error codes from the public API.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@main/telegram/telegram_bot.c` around lines 54 - 59, Update the allowlist
parsing to validate each token using strtoll with an endptr and errno checks:
for each tok, call strtoll(tok, &endptr, 10), ensure endptr != tok (some digits
consumed), check errno for ERANGE and that the parsed uid fits expected range,
then only append to s_allowed_users and increment s_allowed_users_count up to
MIMI_TG_MAX_ALLOWED_USERS; on any malformed token return a suitable esp_err_t
(e.g., ESP_ERR_INVALID_ARG) instead of silently skipping. Change the setter’s
signature from void to esp_err_t (or the public API return type used elsewhere)
and replace ESP_ERROR_CHECK calls in the public API with propagating/returning
the esp_err_t so callers can handle failures; update callers accordingly to
handle the returned errors. Ensure all references to s_allowed_users,
s_allowed_users_count, MIMI_TG_MAX_ALLOWED_USERS, the strtoll call, and
ESP_ERROR_CHECK sites are adjusted to use the new validation and error-return
behavior.

tok = strtok(NULL, ",");
}
}
Comment on lines +45 to +62
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Silent truncation when exceeding max allowed users.

If more than MIMI_TG_MAX_ALLOWED_USERS (8) IDs are provided, entries beyond the limit are silently discarded. Operators may not realize their full whitelist wasn't applied.

💡 Suggested improvement
     char *tok = strtok(buf, ",");
     while (tok && s_allowed_users_count < MIMI_TG_MAX_ALLOWED_USERS) {
         int64_t uid = strtoll(tok, NULL, 10);
         if (uid != 0) {
             s_allowed_users[s_allowed_users_count++] = uid;
         }
         tok = strtok(NULL, ",");
     }
+    if (tok) {
+        ESP_LOGW(TAG, "Allowed users list truncated at %d entries", MIMI_TG_MAX_ALLOWED_USERS);
+    }
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@main/telegram/telegram_bot.c` around lines 45 - 62, The parse_allowed_users
function silently drops IDs beyond MIMI_TG_MAX_ALLOWED_USERS; update
parse_allowed_users to detect when more tokens are present after filling
s_allowed_users up to MIMI_TG_MAX_ALLOWED_USERS and emit a clear warning (e.g.
via the program's logging facility or stderr) that N entries were ignored, and
include which limit was hit; keep using s_allowed_users_count and
s_allowed_users for storage and ensure the function still bounds copies into buf
safely.


static void load_allowed_users(void)
{
/* NVS takes priority */
nvs_handle_t nvs;
if (nvs_open(MIMI_NVS_TG, NVS_READONLY, &nvs) == ESP_OK) {
char buf[256] = {0};
size_t len = sizeof(buf);
if (nvs_get_str(nvs, MIMI_NVS_KEY_ALLOWED_USERS, buf, &len) == ESP_OK) {
parse_allowed_users(buf);
nvs_close(nvs);
ESP_LOGI(TAG, "Loaded %d allowed user(s) from NVS", (int)s_allowed_users_count);
return;
}
nvs_close(nvs);
}

/* Fall back to build-time secret */
if (MIMI_SECRET_ALLOWED_USERS[0]) {
parse_allowed_users(MIMI_SECRET_ALLOWED_USERS);
ESP_LOGI(TAG, "Loaded %d allowed user(s) from build config", (int)s_allowed_users_count);
return;
}

ESP_LOGI(TAG, "No user whitelist configured, allowing all users");
}

/* HTTP response accumulator */
typedef struct {
char *buf;
Expand Down Expand Up @@ -353,6 +412,25 @@ static void process_updates(const char *json_str)
seen_msg_insert(msg_key);
}

/* Check if sender or chat is allowed */
int64_t sender_id = 0;
cJSON *from = cJSON_GetObjectItem(message, "from");
if (from) {
cJSON *from_id = cJSON_GetObjectItem(from, "id");
if (cJSON_IsNumber(from_id)) {
sender_id = (int64_t)from_id->valuedouble;
}
}
int64_t chat_id_num = 0;
if (cJSON_IsNumber(chat_id)) {
chat_id_num = (int64_t)chat_id->valuedouble;
}
if (!is_id_allowed(sender_id, chat_id_num)) {
ESP_LOGW(TAG, "Ignoring message from unauthorized user %" PRId64 " in chat %" PRId64,
sender_id, chat_id_num);
continue;
}

ESP_LOGI(TAG, "Message update_id=%" PRId64 " message_id=%d from chat %s: %.40s...",
uid, msg_id_val, chat_id_str, text->valuestring);

Expand Down Expand Up @@ -423,6 +501,8 @@ esp_err_t telegram_bot_init(void)

/* s_bot_token is already initialized from MIMI_SECRET_TG_TOKEN as fallback */

load_allowed_users();

if (s_bot_token[0]) {
ESP_LOGI(TAG, "Telegram bot token loaded (len=%d)", (int)strlen(s_bot_token));
} else {
Expand Down Expand Up @@ -549,6 +629,22 @@ esp_err_t telegram_send_message(const char *chat_id, const char *text)
return all_ok ? ESP_OK : ESP_FAIL;
}

esp_err_t telegram_set_allowed_users(const char *user_ids)
{
nvs_handle_t nvs;
esp_err_t err = nvs_open(MIMI_NVS_TG, NVS_READWRITE, &nvs);
if (err != ESP_OK) return err;
err = nvs_set_str(nvs, MIMI_NVS_KEY_ALLOWED_USERS, user_ids);
if (err != ESP_OK) { nvs_close(nvs); return err; }
err = nvs_commit(nvs);
nvs_close(nvs);
if (err != ESP_OK) return err;

parse_allowed_users(user_ids);
ESP_LOGI(TAG, "Allowed users updated: %d user(s)", (int)s_allowed_users_count);
return ESP_OK;
}

esp_err_t telegram_set_token(const char *token)
{
nvs_handle_t nvs;
Expand Down
6 changes: 6 additions & 0 deletions main/channels/telegram/telegram_bot.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,3 +25,9 @@ esp_err_t telegram_send_message(const char *chat_id, const char *text);
*/
esp_err_t telegram_set_token(const char *token);

/**
* Set allowed Telegram user IDs (comma-separated string).
* Positive IDs filter by sender, negative IDs allow entire group chats.
* Saves to NVS and applies immediately. Empty string = allow all.
*/
esp_err_t telegram_set_allowed_users(const char *user_ids);
42 changes: 38 additions & 4 deletions main/cli/serial_cli.c
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,28 @@ static int cmd_set_tavily_key(int argc, char **argv)
return 0;
}

/* --- set_allowed_users command --- */
static struct {
struct arg_str *user_ids;
struct arg_end *end;
} allowed_users_args;

static int cmd_set_allowed_users(int argc, char **argv)
{
int nerrors = arg_parse(argc, argv, (void **)&allowed_users_args);
if (nerrors != 0) {
arg_print_errors(stderr, allowed_users_args.end, argv[0]);
return 1;
}
esp_err_t err = telegram_set_allowed_users(allowed_users_args.user_ids->sval[0]);
if (err != ESP_OK) {
printf("Failed to set allowed users: %s\n", esp_err_to_name(err));
return 1;
}
printf("Allowed users set.\n");
return 0;
}

/* --- wifi_scan command --- */
static int cmd_wifi_scan(int argc, char **argv)
{
Expand Down Expand Up @@ -567,6 +589,7 @@ static int cmd_config_show(int argc, char **argv)
print_config_u16("Proxy Port", MIMI_NVS_PROXY, MIMI_NVS_KEY_PROXY_PORT, MIMI_SECRET_PROXY_PORT);
print_config("Search Key", MIMI_NVS_SEARCH, MIMI_NVS_KEY_API_KEY, MIMI_SECRET_SEARCH_KEY, true);
print_config("Tavily Key", MIMI_NVS_SEARCH, MIMI_NVS_KEY_TAVILY_KEY, MIMI_SECRET_TAVILY_KEY, true);
print_config("Allowed IDs", MIMI_NVS_TG, MIMI_NVS_KEY_ALLOWED_USERS, MIMI_SECRET_ALLOWED_USERS, false);
printf("=============================\n");
return 0;
}
Expand Down Expand Up @@ -653,9 +676,9 @@ typedef struct {

static void web_search_task(void *arg)
{
web_search_task_ctx_t *task_ctx = (web_search_task_ctx_t *)arg;
task_ctx->err = tool_web_search_execute(task_ctx->input_json, task_ctx->output, task_ctx->output_size);
xSemaphoreGive(task_ctx->done);
web_search_task_ctx_t *ctx = (web_search_task_ctx_t *)arg;
ctx->err = tool_web_search_execute(ctx->input_json, ctx->output, ctx->output_size);
xSemaphoreGive(ctx->done);
vTaskDelete(NULL);
}

Expand Down Expand Up @@ -995,6 +1018,17 @@ esp_err_t serial_cli_init(void)
};
esp_console_cmd_register(&tavily_key_cmd);

/* set_allowed_users */
allowed_users_args.user_ids = arg_str1(NULL, NULL, "<ids>", "Comma-separated user IDs");
allowed_users_args.end = arg_end(1);
esp_console_cmd_t allowed_users_cmd = {
.command = "set_allowed_users",
.help = "Set allowed Telegram user IDs (e.g. set_allowed_users 123456789,-1001234567890)",
.func = &cmd_set_allowed_users,
.argtable = &allowed_users_args,
};
esp_console_cmd_register(&allowed_users_cmd);

/* set_proxy */
proxy_args.host = arg_str1(NULL, NULL, "<host>", "Proxy host/IP");
proxy_args.port = arg_int1(NULL, NULL, "<port>", "Proxy port");
Expand Down Expand Up @@ -1080,4 +1114,4 @@ esp_err_t serial_cli_init(void)
ESP_LOGI(TAG, "Serial CLI started");

return ESP_OK;
}
}
5 changes: 5 additions & 0 deletions main/mimi_config.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@
#ifndef MIMI_SECRET_TAVILY_KEY
#define MIMI_SECRET_TAVILY_KEY ""
#endif
#ifndef MIMI_SECRET_ALLOWED_USERS
#define MIMI_SECRET_ALLOWED_USERS ""
#endif

/* WiFi */
#define MIMI_WIFI_MAX_RETRY 10
Expand All @@ -60,6 +63,7 @@
#define MIMI_TG_POLL_CORE 0
#define MIMI_TG_CARD_SHOW_MS 3000
#define MIMI_TG_CARD_BODY_SCALE 3
#define MIMI_TG_MAX_ALLOWED_USERS 8

/* Feishu Bot */
#define MIMI_FEISHU_MAX_MSG_LEN 4096
Expand Down Expand Up @@ -153,6 +157,7 @@
#define MIMI_NVS_KEY_PROXY_HOST "host"
#define MIMI_NVS_KEY_PROXY_PORT "port"
#define MIMI_NVS_KEY_PROXY_TYPE "proxy_type"
#define MIMI_NVS_KEY_ALLOWED_USERS "allowed_users"

/* WiFi Onboarding (Captive Portal) */
#define MIMI_ONBOARD_AP_PREFIX "MimiClaw-"
Expand Down
7 changes: 5 additions & 2 deletions main/mimi_secrets.h.example
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,10 @@
#define MIMI_SECRET_PROXY_PORT ""
#define MIMI_SECRET_PROXY_TYPE "" /* "http" or "socks5" */

/* Brave Search API */
/* Web Search API */
#define MIMI_SECRET_SEARCH_KEY ""
/* Tavily Search API */
#define MIMI_SECRET_TAVILY_KEY ""

/* Telegram allowed user IDs (comma-separated, e.g. "123456789,987654321"). Empty = allow all. */
/* Negative IDs (e.g. "-1001234567890") allow all users in that group chat. */
#define MIMI_SECRET_ALLOWED_USERS ""