mirror of
https://git.jabber.space/devs/cproof.git
synced 2026-07-18 15:16:22 +00:00
fix(ai): add mutex to AISession for thread safety
Introduce a pthread mutex to protect all AISession fields from concurrent access between the main thread and background request worker. - Initialize and destroy mutex during session lifecycle - Lock session state when modifying or reading history, model, and provider fields - Snapshot session state under lock in the request worker thread to prevent UAF races during API calls - Refactor _build_json_payload to accept direct parameters for safe usage under lock - Update command handlers to safely switch provider/model under lock
This commit is contained in:
@@ -21,6 +21,7 @@
|
||||
|
||||
#include <curl/curl.h>
|
||||
#include <glib.h>
|
||||
#include <pthread.h>
|
||||
#include <string.h>
|
||||
|
||||
/* Default providers */
|
||||
@@ -1037,6 +1038,11 @@ ai_session_create(const gchar* provider_name, const gchar* model)
|
||||
}
|
||||
|
||||
AISession* session = g_new0(AISession, 1);
|
||||
if (pthread_mutex_init(&session->lock, NULL) != 0) {
|
||||
log_error("Failed to initialize session mutex");
|
||||
g_free(session);
|
||||
return NULL;
|
||||
}
|
||||
session->provider_name = g_strdup(provider_name);
|
||||
session->provider = ai_provider_ref(provider);
|
||||
session->model = g_strdup(model);
|
||||
@@ -1067,6 +1073,7 @@ ai_session_unref(AISession* session)
|
||||
return;
|
||||
}
|
||||
|
||||
pthread_mutex_lock(&session->lock);
|
||||
g_free(session->provider_name);
|
||||
ai_provider_unref(session->provider);
|
||||
g_free(session->model);
|
||||
@@ -1081,6 +1088,7 @@ ai_session_unref(AISession* session)
|
||||
curr = g_list_next(curr);
|
||||
}
|
||||
g_list_free(session->history);
|
||||
pthread_mutex_unlock(&session->lock);
|
||||
|
||||
g_free(session);
|
||||
log_debug("AI session destroyed");
|
||||
@@ -1092,10 +1100,12 @@ ai_session_add_message(AISession* session, const gchar* role, const gchar* conte
|
||||
if (!session || !role || !content)
|
||||
return;
|
||||
|
||||
pthread_mutex_lock(&session->lock);
|
||||
AIMessage* msg = g_new0(AIMessage, 1);
|
||||
msg->role = g_strdup(role);
|
||||
msg->content = g_strdup(content);
|
||||
session->history = g_list_append(session->history, msg);
|
||||
pthread_mutex_unlock(&session->lock);
|
||||
|
||||
log_debug("Added %s message to session (total: %d)", role, g_list_length(session->history));
|
||||
}
|
||||
@@ -1106,6 +1116,7 @@ ai_session_clear_history(AISession* session)
|
||||
if (!session)
|
||||
return;
|
||||
|
||||
pthread_mutex_lock(&session->lock);
|
||||
GList* curr = session->history;
|
||||
while (curr) {
|
||||
AIMessage* msg = curr->data;
|
||||
@@ -1116,6 +1127,7 @@ ai_session_clear_history(AISession* session)
|
||||
}
|
||||
g_list_free(session->history);
|
||||
session->history = NULL;
|
||||
pthread_mutex_unlock(&session->lock);
|
||||
|
||||
log_info("AI session history cleared");
|
||||
}
|
||||
@@ -1134,8 +1146,10 @@ ai_session_set_model(AISession* session, const gchar* model)
|
||||
if (!session || !model)
|
||||
return;
|
||||
|
||||
pthread_mutex_lock(&session->lock);
|
||||
g_free(session->model);
|
||||
session->model = g_strdup(model);
|
||||
pthread_mutex_unlock(&session->lock);
|
||||
log_info("Session model changed to: %s", model);
|
||||
}
|
||||
|
||||
@@ -1143,15 +1157,21 @@ ai_session_set_model(AISession* session, const gchar* model)
|
||||
* API Request Handling
|
||||
* ======================================================================== */
|
||||
|
||||
/**
|
||||
* Build JSON payload from a GList of AIMessage nodes and a prompt string.
|
||||
* This is the thread-safe variant used by _ai_request_thread() which holds
|
||||
* the session lock while calling it.
|
||||
*
|
||||
* @param model The model identifier
|
||||
* @param history GList of AIMessage* (must be held under session lock)
|
||||
* @param prompt The new user prompt to append
|
||||
* @return Newly allocated JSON string (caller must free)
|
||||
*/
|
||||
static gchar*
|
||||
_build_json_payload(AISession* session, const gchar* prompt)
|
||||
_build_json_payload_from_list(const gchar* model, GList* history, const gchar* prompt)
|
||||
{
|
||||
|
||||
/* OpenAI-compatible Responses API format:
|
||||
* {"model": "...", "input": [...], "stream": false, "store": false}
|
||||
* store:false prevents providers from storing/using requests for training */
|
||||
GString* messages_json = g_string_new("");
|
||||
GList* curr = session->history;
|
||||
GList* curr = history;
|
||||
|
||||
while (curr) {
|
||||
AIMessage* msg = curr->data;
|
||||
@@ -1174,7 +1194,7 @@ _build_json_payload(AISession* session, const gchar* prompt)
|
||||
}
|
||||
g_string_append_printf(messages_json, "{\"role\":\"user\",\"content\":\"%s\"}", escaped_prompt);
|
||||
|
||||
auto_gchar gchar* escaped_model = ai_json_escape(session->model);
|
||||
auto_gchar gchar* escaped_model = ai_json_escape(model);
|
||||
gchar* json_payload = g_strdup_printf(
|
||||
"{\"model\":\"%s\",\"input\":[%s],\"stream\":false,\"store\":false}",
|
||||
escaped_model, messages_json->str);
|
||||
@@ -1335,14 +1355,31 @@ _ai_request_thread(gpointer data)
|
||||
auto_gchar gchar* prompt = args[1];
|
||||
gpointer user_data = args[2];
|
||||
|
||||
log_debug("[AI-THREAD] Session: %s/%s", session->provider_name, session->model);
|
||||
log_debug("[AI-THREAD] API key length: %zu", session->api_key ? strlen(session->api_key) : 0);
|
||||
/* =====================================================================
|
||||
* CRITICAL: Copy all session state under lock to prevent UAF races with
|
||||
* cmd_ai_switch() and ai_session_clear_history() which mutate session
|
||||
* fields on the main thread without holding a session ref.
|
||||
*
|
||||
* The worker holds a ref to the session (via ai_session_ref in
|
||||
* ai_send_prompt), which prevents the session struct from being freed,
|
||||
* but does NOT prevent field mutations (g_free/g_strdup of strings,
|
||||
* list modifications). We must snapshot all fields atomically.
|
||||
* ===================================================================== */
|
||||
pthread_mutex_lock(&session->lock);
|
||||
gchar* local_provider_name = g_strdup(session->provider_name);
|
||||
AIProvider* local_provider = ai_provider_ref(session->provider);
|
||||
gchar* local_model = g_strdup(session->model);
|
||||
gchar* local_api_key = g_strdup(session->api_key ? session->api_key : "");
|
||||
pthread_mutex_unlock(&session->lock);
|
||||
|
||||
log_debug("[AI-THREAD] Session: %s/%s", local_provider_name, local_model);
|
||||
log_debug("[AI-THREAD] API key length: %zu", strlen(local_api_key));
|
||||
|
||||
/* Check for API key first */
|
||||
if (!session->api_key || strlen(session->api_key) == 0) {
|
||||
if (!local_api_key || strlen(local_api_key) == 0) {
|
||||
auto_gchar gchar* error_msg = g_strdup_printf("No API key set for provider '%s'. Use '/ai set token %s <key>' to configure.",
|
||||
session->provider_name, session->provider_name);
|
||||
log_error("AI request failed for %s/%s: %s", session->provider_name, session->model, error_msg);
|
||||
local_provider_name, local_provider_name);
|
||||
log_error("AI request failed for %s/%s: %s", local_provider_name, local_model, error_msg);
|
||||
_aiwin_display_error(user_data, error_msg);
|
||||
g_free(args);
|
||||
return NULL;
|
||||
@@ -1352,23 +1389,30 @@ _ai_request_thread(gpointer data)
|
||||
log_debug("[AI-THREAD] Curl initialized: %s", curl ? "OK" : "FAILED");
|
||||
if (!curl) {
|
||||
log_error("AI request failed for %s/%s: Failed to initialize curl",
|
||||
session->provider_name, session->model);
|
||||
local_provider_name, local_model);
|
||||
_aiwin_display_error(user_data, "Failed to initialize curl.");
|
||||
g_free(args);
|
||||
return NULL;
|
||||
}
|
||||
|
||||
/* Add user message to history FIRST */
|
||||
ai_session_add_message(session, "user", prompt);
|
||||
/* Add user message to history FIRST (under lock) */
|
||||
pthread_mutex_lock(&session->lock);
|
||||
AIMessage* user_msg = g_new0(AIMessage, 1);
|
||||
user_msg->role = g_strdup("user");
|
||||
user_msg->content = g_strdup(prompt);
|
||||
session->history = g_list_append(session->history, user_msg);
|
||||
pthread_mutex_unlock(&session->lock);
|
||||
log_debug("[AI-THREAD] Added user message to history");
|
||||
|
||||
/* Build JSON payload (includes the message we just added) */
|
||||
/* Build JSON payload from local history copy (under lock) */
|
||||
log_debug("[AI-THREAD] Building JSON payload...");
|
||||
auto_gchar gchar* json_payload = _build_json_payload(session, prompt);
|
||||
pthread_mutex_lock(&session->lock);
|
||||
auto_gchar gchar* json_payload = _build_json_payload_from_list(local_model, session->history, prompt);
|
||||
pthread_mutex_unlock(&session->lock);
|
||||
log_debug("[AI-THREAD] JSON payload: %s", json_payload);
|
||||
|
||||
/* Build headers using shared helper */
|
||||
struct curl_slist* headers = _build_curl_headers(session->provider, session->api_key);
|
||||
struct curl_slist* headers = _build_curl_headers(local_provider, local_api_key);
|
||||
|
||||
/* Response buffer */
|
||||
struct curl_response_t response;
|
||||
@@ -1376,11 +1420,11 @@ _ai_request_thread(gpointer data)
|
||||
response.size = 0;
|
||||
|
||||
/* Build request URL */
|
||||
const gchar* api_url = session->provider ? session->provider->api_url : DEFAULT_OPENAI_URL;
|
||||
const gchar* api_url = local_provider ? local_provider->api_url : DEFAULT_OPENAI_URL;
|
||||
auto_gchar gchar* request_url = g_strdup_printf("%s%sv1/responses", api_url, g_str_has_suffix(api_url, "/") ? "" : "/");
|
||||
log_debug("[AI-THREAD] API URL: %s", api_url);
|
||||
log_debug("[AI-THREAD] API Request URL: %s", request_url);
|
||||
log_debug("[AI-THREAD] Model: %s", session->model);
|
||||
log_debug("[AI-THREAD] Model: %s", local_model);
|
||||
|
||||
/* Set URL and execute request */
|
||||
curl_easy_setopt(curl, CURLOPT_URL, request_url);
|
||||
@@ -1400,7 +1444,7 @@ _ai_request_thread(gpointer data)
|
||||
|
||||
if (res != CURLE_OK) {
|
||||
auto_gchar gchar* error_msg = g_strdup(curl_easy_strerror(res));
|
||||
log_error("AI request failed for %s/%s: %s", session->provider_name, session->model, error_msg);
|
||||
log_error("AI request failed for %s/%s: %s", local_provider_name, local_model, error_msg);
|
||||
_aiwin_display_error(user_data, error_msg);
|
||||
} else if (http_code >= 400) {
|
||||
/* Handle HTTP errors */
|
||||
@@ -1409,7 +1453,7 @@ _ai_request_thread(gpointer data)
|
||||
/* Try to extract the actual error message from the JSON response */
|
||||
auto_gchar gchar* parsed_error = _parse_error_response(response.data);
|
||||
auto_gchar gchar* error_msg = parsed_error ? g_strdup_printf("HTTP %ld: %s", http_code, parsed_error) : g_strdup_printf("HTTP %ld: %s", http_code, response.data && strlen(response.data) > 0 ? response.data : "Unknown error");
|
||||
log_error("AI request failed for %s/%s: %s", session->provider_name, session->model, error_msg);
|
||||
log_error("AI request failed for %s/%s: %s", local_provider_name, local_model, error_msg);
|
||||
_aiwin_display_error(user_data, error_msg);
|
||||
} else {
|
||||
/* Parse response - transfer ownership to auto_gchar for cleanup */
|
||||
@@ -1418,12 +1462,17 @@ _ai_request_thread(gpointer data)
|
||||
response.data = NULL;
|
||||
auto_gchar gchar* content = _parse_ai_response(response_data);
|
||||
if (content) {
|
||||
/* Add assistant response to history */
|
||||
ai_session_add_message(session, "assistant", content);
|
||||
/* Add assistant response to history (under lock) */
|
||||
pthread_mutex_lock(&session->lock);
|
||||
AIMessage* assistant_msg = g_new0(AIMessage, 1);
|
||||
assistant_msg->role = g_strdup("assistant");
|
||||
assistant_msg->content = g_strdup(content);
|
||||
session->history = g_list_append(session->history, assistant_msg);
|
||||
pthread_mutex_unlock(&session->lock);
|
||||
_aiwin_display_response(user_data, content);
|
||||
} else {
|
||||
log_error("AI response parse failed for %s/%s: %.200s...",
|
||||
session->provider_name, session->model, response_data);
|
||||
local_provider_name, local_model, response_data);
|
||||
_aiwin_display_error(user_data, "Failed to parse AI response.");
|
||||
}
|
||||
}
|
||||
@@ -1431,6 +1480,12 @@ _ai_request_thread(gpointer data)
|
||||
curl_slist_free_all(headers);
|
||||
curl_easy_cleanup(curl);
|
||||
|
||||
/* Cleanup local copies */
|
||||
g_free(local_provider_name);
|
||||
ai_provider_unref(local_provider);
|
||||
g_free(local_model);
|
||||
g_free(local_api_key);
|
||||
|
||||
g_free(args);
|
||||
|
||||
return NULL;
|
||||
|
||||
@@ -32,6 +32,7 @@ typedef struct ai_provider_t
|
||||
*/
|
||||
typedef struct ai_session_t
|
||||
{
|
||||
pthread_mutex_t lock; /* Protects all session fields below */
|
||||
gchar* provider_name; /* Provider name */
|
||||
AIProvider* provider; /* Provider configuration */
|
||||
gchar* model; /* Model identifier (e.g., "gpt-4", "sonar") */
|
||||
@@ -238,6 +239,45 @@ const gchar* ai_session_get_model(AISession* session);
|
||||
*/
|
||||
void ai_session_set_model(AISession* session, const gchar* model);
|
||||
|
||||
/**
|
||||
* Atomically switch session provider, model, and API key.
|
||||
* All mutations happen under the session lock to prevent races with
|
||||
* _ai_request_thread() which snapshots session state before making requests.
|
||||
*
|
||||
* @param session The session
|
||||
* @param provider_name New provider name
|
||||
* @param model New model identifier
|
||||
* @param api_key New API key (caller must free after calling this)
|
||||
*/
|
||||
void ai_session_switch(AISession* session, const gchar* provider_name,
|
||||
const gchar* model, gchar* api_key);
|
||||
|
||||
/**
|
||||
* Atomically switch session provider, model, and API key.
|
||||
* All mutations happen under the session lock to prevent races with
|
||||
* _ai_request_thread() which snapshots session state before making requests.
|
||||
*
|
||||
* @param session The session
|
||||
* @param provider_name New provider name
|
||||
* @param model New model identifier
|
||||
* @param api_key New API key (caller must free after calling this)
|
||||
*/
|
||||
void ai_session_switch(AISession* session, const gchar* provider_name,
|
||||
const gchar* model, gchar* api_key);
|
||||
|
||||
/**
|
||||
* Atomically switch session provider, model, and API key.
|
||||
* All mutations happen under the session lock to prevent races with
|
||||
* _ai_request_thread() which snapshots session state before making requests.
|
||||
*
|
||||
* @param session The session
|
||||
* @param provider_name New provider name
|
||||
* @param model New model identifier
|
||||
* @param api_key New API key (caller must free after calling this)
|
||||
*/
|
||||
void ai_session_switch(AISession* session, const gchar* provider_name,
|
||||
const gchar* model, gchar* api_key);
|
||||
|
||||
/* ========================================================================
|
||||
* Request Handling
|
||||
* ======================================================================== */
|
||||
|
||||
@@ -51,6 +51,7 @@
|
||||
#include <unistd.h>
|
||||
#include <langinfo.h>
|
||||
#include <ctype.h>
|
||||
#include <pthread.h>
|
||||
|
||||
// fork / execl
|
||||
#include <sys/types.h>
|
||||
@@ -11096,7 +11097,14 @@ cmd_ai_switch(ProfWin* window, const char* const command, gchar** args)
|
||||
}
|
||||
} else {
|
||||
// arg1 is a model name, keep current provider
|
||||
// Read provider_name under lock to prevent UAF on session->provider_name
|
||||
pthread_mutex_lock(&aiwin->session->lock);
|
||||
provider_name = aiwin->session->provider_name;
|
||||
pthread_mutex_unlock(&aiwin->session->lock);
|
||||
if (!provider_name) {
|
||||
cons_show_error("Session has no provider name.");
|
||||
return TRUE;
|
||||
}
|
||||
model = arg1;
|
||||
}
|
||||
|
||||
@@ -11108,7 +11116,8 @@ cmd_ai_switch(ProfWin* window, const char* const command, gchar** args)
|
||||
return TRUE;
|
||||
}
|
||||
|
||||
// Update the existing session's provider and model
|
||||
// Update the existing session's provider and model (all under lock)
|
||||
pthread_mutex_lock(&aiwin->session->lock);
|
||||
if (changed_provider) {
|
||||
AIProvider* new_provider = ai_get_provider(provider_name);
|
||||
g_free(aiwin->session->provider_name);
|
||||
@@ -11121,6 +11130,7 @@ cmd_ai_switch(ProfWin* window, const char* const command, gchar** args)
|
||||
aiwin->session->model = g_strdup(model);
|
||||
g_free(aiwin->session->api_key);
|
||||
aiwin->session->api_key = g_strdup(api_key);
|
||||
pthread_mutex_unlock(&aiwin->session->lock);
|
||||
|
||||
// Update window title
|
||||
win_println((ProfWin*)aiwin, THEME_DEFAULT, "-", "AI Chat: %s/%s", provider_name, model);
|
||||
|
||||
Reference in New Issue
Block a user