Refactor status semantics and add structured scan logging

.github/workflows/build.yml

@@ -25,6 +25,9 @@ jobs:
     - name: Build Solution
       run: dotnet build ContextMenuProfiler.sln -c Release
 
+    - name: Run Quality Checks
+      run: dotnet run --project ContextMenuProfiler.QualityChecks\ContextMenuProfiler.QualityChecks.csproj -c Release --no-build
+
     - name: Upload Build Artifacts
       uses: actions/upload-artifact@v4
       with:

ContextMenuProfiler.Hook/src/ipc_server.cpp

@@ -5,14 +5,61 @@
 static const LONG kMaxConcurrentPipeClients = 4;
 static volatile LONG g_ActivePipeClients = 0;
 static const DWORD kWorkerTimeoutMs = 1800;
+static const DWORD kFrameHeaderBytes = 4;
+static const size_t kMaxRequestBytes = 16384;
+static const size_t kMaxResponseBytes = 65536;
 
 struct IpcWorkItem {
-    char request[2048];
+    std::string request;
     char response[65536];
     int maxLen;
     volatile LONG releaseByWorker;
 };
 
+static bool ReadExactFromPipe(HANDLE hPipe, void* buffer, DWORD bytesToRead) {
+    BYTE* cursor = reinterpret_cast<BYTE*>(buffer);
+    DWORD totalRead = 0;
+    while (totalRead < bytesToRead) {
+        DWORD chunkRead = 0;
+        BOOL ok = ReadFile(hPipe, cursor + totalRead, bytesToRead - totalRead, &chunkRead, NULL);
+        if (!ok || chunkRead == 0) {
+            return false;
+        }
+
+        totalRead += chunkRead;
+    }
+
+    return true;
+}
+
+static bool WriteFrameToPipe(HANDLE hPipe, const char* payload, DWORD payloadLength) {
+    if (payloadLength > static_cast<DWORD>(kMaxResponseBytes)) {
+        return false;
+    }
+
+    DWORD written = 0;
+    DWORD frameLength = payloadLength;
+    if (!WriteFile(hPipe, &frameLength, kFrameHeaderBytes, &written, NULL) || written != kFrameHeaderBytes) {
+        return false;
+    }
+
+    if (payloadLength == 0) {
+        FlushFileBuffers(hPipe);
+        return true;
+    }
+
+    if (!WriteFile(hPipe, payload, payloadLength, &written, NULL) || written != payloadLength) {
+        return false;
+    }
+
+    FlushFileBuffers(hPipe);
+    return true;
+}
+
+static void WriteJsonFrameToPipe(HANDLE hPipe, const char* json) {
+    WriteFrameToPipe(hPipe, json, static_cast<DWORD>(strlen(json)));
+}
+
 void DoIpcWorkInternal(const char* request, char* response, int maxLen) {
     std::string reqStr = request;
 
@@ -25,21 +72,33 @@ void DoIpcWorkInternal(const char* request, char* response, int maxLen) {
         return;
     }
 
-    std::string mode = "AUTO";
+    std::string mode;
     std::string clsidStr, pathStr, dllHintStr;
 
-    if (reqStr.substr(0, 4) == "COM|") {
-        mode = "COM";
-        reqStr = reqStr.substr(4);
-    } else if (reqStr.substr(0, 5) == "ECMD|") {
-        mode = "ECMD";
-        reqStr = reqStr.substr(5);
+    if (reqStr.rfind("CMP1|", 0) != 0) {
+        snprintf(response, maxLen, "{\"success\":false,\"code\":\"E_PROTOCOL\",\"error\":\"Unsupported Protocol\"}");
+        return;
+    }
+
+    reqStr = reqStr.substr(5);
+    size_t sep0 = reqStr.find('|');
+    if (sep0 == std::string::npos) {
+        snprintf(response, maxLen, "{\"success\":false,\"code\":\"E_FORMAT\",\"error\":\"Missing Mode\"}");
+        return;
+    }
+
+    mode = reqStr.substr(0, sep0);
+    reqStr = reqStr.substr(sep0 + 1);
+
+    if (mode != "AUTO" && mode != "COM" && mode != "ECMD") {
+        snprintf(response, maxLen, "{\"success\":false,\"code\":\"E_MODE\",\"error\":\"Unsupported Mode\"}");
+        return;
     }
 
     // Format: CLSID|Path[|DllHint]
     size_t sep1 = reqStr.find('|');
     if (sep1 == std::string::npos) {
-        snprintf(response, maxLen, "{\"success\":false,\"error\":\"Format Error\"}");
+        snprintf(response, maxLen, "{\"success\":false,\"code\":\"E_FORMAT\",\"error\":\"Format Error\"}");
         return;
     }
     clsidStr = reqStr.substr(0, sep1);
@@ -55,18 +114,27 @@ void DoIpcWorkInternal(const char* request, char* response, int maxLen) {
 
     CLSID clsid;
     wchar_t wClsid[64];
-    MultiByteToWideChar(CP_UTF8, 0, clsidStr.c_str(), -1, wClsid, 64);
+    if (MultiByteToWideChar(CP_UTF8, 0, clsidStr.c_str(), -1, wClsid, 64) <= 0) {
+        snprintf(response, maxLen, "{\"success\":false,\"code\":\"E_CLSID_UTF8\",\"error\":\"Bad CLSID Encoding\"}");
+        return;
+    }
     if (FAILED(CLSIDFromString(wClsid, &clsid))) {
-        snprintf(response, maxLen, "{\"success\":false,\"error\":\"Bad CLSID\"}");
+        snprintf(response, maxLen, "{\"success\":false,\"code\":\"E_CLSID\",\"error\":\"Bad CLSID\"}");
         return;
     }
 
     wchar_t wPath[MAX_PATH];
-    MultiByteToWideChar(CP_UTF8, 0, pathStr.c_str(), -1, wPath, MAX_PATH);
+    if (MultiByteToWideChar(CP_UTF8, 0, pathStr.c_str(), -1, wPath, MAX_PATH) <= 0) {
+        snprintf(response, maxLen, "{\"success\":false,\"code\":\"E_PATH_UTF8\",\"error\":\"Bad Path Encoding\"}");
+        return;
+    }
 
     wchar_t wDllHint[MAX_PATH] = { 0 };
     if (!dllHintStr.empty()) {
-        MultiByteToWideChar(CP_UTF8, 0, dllHintStr.c_str(), -1, wDllHint, MAX_PATH);
+        if (MultiByteToWideChar(CP_UTF8, 0, dllHintStr.c_str(), -1, wDllHint, MAX_PATH) <= 0) {
+            snprintf(response, maxLen, "{\"success\":false,\"code\":\"E_DLLHINT_UTF8\",\"error\":\"Bad DllHint Encoding\"}");
+            return;
+        }
     }
 
     // We'll pass the dllHint to the handlers
@@ -99,7 +167,7 @@ void DoIpcWork(const char* request, char* response, int maxLen) {
 DWORD WINAPI DoIpcWorkThread(LPVOID param) {
     IpcWorkItem* item = (IpcWorkItem*)param;
     item->response[0] = '\0';
-    DoIpcWork(item->request, item->response, item->maxLen);
+    DoIpcWork(item->request.c_str(), item->response, item->maxLen);
     if (InterlockedCompareExchange(&item->releaseByWorker, 0, 0) == 1) {
         delete item;
     }
@@ -110,41 +178,45 @@ DWORD WINAPI HandlePipeClientThread(LPVOID param) {
     HANDLE hPipe = (HANDLE)param;
     CoInitializeEx(NULL, COINIT_APARTMENTTHREADED);
 
-    char req[2048];
-    DWORD read = 0;
-    DWORD written = 0;
-
-    if (ReadFile(hPipe, req, 2047, &read, NULL) && read > 0) {
-        req[read] = '\0';
+    DWORD requestLength = 0;
+    if (ReadExactFromPipe(hPipe, &requestLength, kFrameHeaderBytes)) {
+        if (requestLength == 0 || requestLength > static_cast<DWORD>(kMaxRequestBytes)) {
+            WriteJsonFrameToPipe(hPipe, "{\"success\":false,\"code\":\"E_REQ_TOO_LARGE\",\"error\":\"Request Too Large\"}");
+        } else {
+            std::string req(requestLength, '\0');
+            if (!ReadExactFromPipe(hPipe, &req[0], requestLength)) {
+                WriteJsonFrameToPipe(hPipe, "{\"success\":false,\"code\":\"E_REQ_READ\",\"error\":\"Request Read Failed\"}");
+            } else {
         IpcWorkItem* workItem = new IpcWorkItem();
-        strncpy_s(workItem->request, sizeof(workItem->request), req, _TRUNCATE);
+                workItem->request = std::move(req);
         workItem->response[0] = '\0';
-        workItem->maxLen = 65535;
+        workItem->maxLen = static_cast<int>(kMaxResponseBytes) - 1;
         workItem->releaseByWorker = 0;
 
         HANDLE hWorkThread = CreateThread(NULL, 0, DoIpcWorkThread, workItem, 0, NULL);
         if (!hWorkThread) {
-            const char* errRes = "{\"success\":false,\"error\":\"Hook Worker Launch Failed\"}";
-            WriteFile(hPipe, errRes, (DWORD)strlen(errRes), &written, NULL);
-            FlushFileBuffers(hPipe);
+                    WriteJsonFrameToPipe(hPipe, "{\"success\":false,\"error\":\"Hook Worker Launch Failed\"}");
             delete workItem;
         } else {
             DWORD waitRc = WaitForSingleObject(hWorkThread, kWorkerTimeoutMs);
             if (waitRc == WAIT_OBJECT_0) {
-                int resLen = (int)strlen(workItem->response);
+                DWORD resLen = static_cast<DWORD>(strnlen_s(workItem->response, workItem->maxLen));
                 if (resLen > 0) {
-                    WriteFile(hPipe, workItem->response, (DWORD)resLen, &written, NULL);
-                    FlushFileBuffers(hPipe);
+                            WriteFrameToPipe(hPipe, workItem->response, resLen);
+                        } else {
+                            WriteJsonFrameToPipe(hPipe, "{\"success\":false,\"error\":\"Empty Hook Response\"}");
                 }
                 delete workItem;
             } else {
-                const char* timeoutRes = "{\"success\":false,\"error\":\"Hook Worker Timeout\"}";
-                WriteFile(hPipe, timeoutRes, (DWORD)strlen(timeoutRes), &written, NULL);
-                FlushFileBuffers(hPipe);
+                        WriteJsonFrameToPipe(hPipe, "{\"success\":false,\"error\":\"Hook Worker Timeout\"}");
                 InterlockedExchange(&workItem->releaseByWorker, 1);
             }
             CloseHandle(hWorkThread);
         }
+            }
+        }
+    } else {
+        WriteJsonFrameToPipe(hPipe, "{\"success\":false,\"code\":\"E_REQ_HEADER\",\"error\":\"Request Header Read Failed\"}");
     }
 
     DisconnectNamedPipe(hPipe);
@@ -160,17 +232,12 @@ DWORD WINAPI PipeThread(LPVOID) {
     Gdiplus::GdiplusStartupInput gsi;
     Gdiplus::GdiplusStartup(&g_GdiplusToken, &gsi, NULL);
 
-    PSECURITY_DESCRIPTOR pSD = (PSECURITY_DESCRIPTOR)LocalAlloc(LPTR, SECURITY_DESCRIPTOR_MIN_LENGTH);
-    InitializeSecurityDescriptor(pSD, SECURITY_DESCRIPTOR_REVISION);
-    SetSecurityDescriptorDacl(pSD, TRUE, NULL, FALSE);
-    SECURITY_ATTRIBUTES sa = { sizeof(sa), pSD, FALSE };
-
     while (!g_ShouldExit) {
         HANDLE hPipe = CreateNamedPipeA(
             "\\\\.\\pipe\\ContextMenuProfilerHook",
             PIPE_ACCESS_DUPLEX,
-            PIPE_TYPE_BYTE | PIPE_READMODE_BYTE | PIPE_WAIT,
-            PIPE_UNLIMITED_INSTANCES, 65536, 65536, 0, &sa);
+            PIPE_TYPE_MESSAGE | PIPE_READMODE_MESSAGE | PIPE_WAIT | PIPE_REJECT_REMOTE_CLIENTS,
+            PIPE_UNLIMITED_INSTANCES, 65536, 65536, 0, NULL);
         if (hPipe == INVALID_HANDLE_VALUE) { Sleep(100); continue; }
 
         if (ConnectNamedPipe(hPipe, NULL) || GetLastError() == ERROR_PIPE_CONNECTED) {
@@ -182,9 +249,7 @@ DWORD WINAPI PipeThread(LPVOID) {
             if (active > kMaxConcurrentPipeClients) {
                 InterlockedDecrement(&g_ActivePipeClients);
                 const char* busyRes = "{\"success\":false,\"error\":\"Hook Busy\"}";
-                DWORD written = 0;
-                WriteFile(hPipe, busyRes, (DWORD)strlen(busyRes), &written, NULL);
-                FlushFileBuffers(hPipe);
+                WriteJsonFrameToPipe(hPipe, busyRes);
                 DisconnectNamedPipe(hPipe);
                 CloseHandle(hPipe);
                 continue;
@@ -203,7 +268,6 @@ DWORD WINAPI PipeThread(LPVOID) {
 
     Gdiplus::GdiplusShutdown(g_GdiplusToken);
     MH_Uninitialize();
-    LocalFree(pSD);
     CoUninitialize();
     LogToFile(L"--- Pipe Thread Exited Cleanly ---\n");
     return 0;

ContextMenuProfiler.QualityChecks/ContextMenuProfiler.QualityChecks.csproj

@@ -0,0 +1,12 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <OutputType>Exe</OutputType>
+    <TargetFramework>net8.0-windows10.0.19041.0</TargetFramework>
+    <Nullable>enable</Nullable>
+    <ImplicitUsings>enable</ImplicitUsings>
+  </PropertyGroup>
+
+  <ItemGroup>
+    <ProjectReference Include="..\ContextMenuProfiler.UI\ContextMenuProfiler.UI.csproj" />
+  </ItemGroup>
+</Project>