Add security defense tests + eliminate system() command injection

TDD: 31 security tests covering shell injection prevention, SQLite
authorizer (ATTACH/DETACH blocked), SQL injection via Cypher, path
containment, and shell-free subprocess execution.

- Add cbm_exec_no_shell() in compat_fs: fork+execvp (POSIX), _spawnvp
  (Windows) — executes commands without shell interpretation
- Replace system() with cbm_exec_no_shell() for unzip extraction and
  version verification in update command — eliminates CodeQL
  command-line-injection alerts
- CodeQL: switch to build-mode manual for 100% source file coverage
- CodeQL gate: fix race condition between scan completion and alert API
  propagation (60s settle + double-check polling)
- Dismiss TOCTOU in pass_envscan.c (benign read-only directory walk)

Author: DeusData

.github/workflows/codeql.yml

@@ -14,11 +14,17 @@ jobs:
     steps:
       - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
 
+      - name: Install build dependencies
+        run: sudo apt-get update && sudo apt-get install -y zlib1g-dev
+
       - name: Initialize CodeQL
         uses: github/codeql-action/init@38697555549f1db7851b81482ff19f1fa5c4fedc # v4
         with:
           languages: c-cpp
-          build-mode: none
+          build-mode: manual
+
+      - name: Build for CodeQL analysis
+        run: scripts/build.sh
 
       - name: Perform CodeQL Analysis
         uses: github/codeql-action/analyze@38697555549f1db7851b81482ff19f1fa5c4fedc # v4

.github/workflows/dry-run.yml

@@ -120,11 +120,30 @@ jobs:
         env:
           GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
         run: |
-          ALERTS=$(gh api repos/${{ github.repository }}/code-scanning/alerts?state=open --jq 'length' 2>/dev/null || echo "0")
-          echo "Open code scanning alerts: $ALERTS"
+          # Wait for GitHub to finish processing alert state changes.
+          # There is a race between CodeQL marking the workflow as "completed"
+          # and the alerts API reflecting new/closed alerts from that scan.
+          echo "Waiting 60s for alert API to settle after CodeQL completion..."
+          sleep 60
+
+          # Poll alerts twice with a gap to confirm the count is stable
+          ALERTS1=$(gh api 'repos/${{ github.repository }}/code-scanning/alerts?state=open' --jq 'length' 2>/dev/null || echo "0")
+          echo "Open alerts (check 1): $ALERTS1"
+          sleep 15
+          ALERTS2=$(gh api 'repos/${{ github.repository }}/code-scanning/alerts?state=open' --jq 'length' 2>/dev/null || echo "0")
+          echo "Open alerts (check 2): $ALERTS2"
+
+          # Use the higher count (conservative — if either check sees alerts, block)
+          ALERTS=$ALERTS2
+          if [ "$ALERTS1" -gt "$ALERTS2" ]; then
+            ALERTS=$ALERTS1
+          fi
+
           if [ "$ALERTS" -gt 0 ]; then
             echo "BLOCKED: $ALERTS open code scanning alert(s) found."
-            echo "Fix them before releasing: https://github.com/${{ github.repository }}/security/code-scanning"
+            gh api 'repos/${{ github.repository }}/code-scanning/alerts?state=open' \
+              --jq '.[] | "  #\(.number) [\(.rule.security_severity_level // .rule.severity)] \(.rule.id) — \(.most_recent_instance.location.path):\(.most_recent_instance.location.start_line)"' 2>/dev/null || true
+            echo "Fix them: https://github.com/${{ github.repository }}/security/code-scanning"
             exit 1
           fi
           echo "=== CodeQL gate passed (0 open alerts) ==="

.github/workflows/release.yml

@@ -122,11 +122,30 @@ jobs:
         env:
           GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
         run: |
-          ALERTS=$(gh api repos/${{ github.repository }}/code-scanning/alerts?state=open --jq 'length' 2>/dev/null || echo "0")
-          echo "Open code scanning alerts: $ALERTS"
+          # Wait for GitHub to finish processing alert state changes.
+          # There is a race between CodeQL marking the workflow as "completed"
+          # and the alerts API reflecting new/closed alerts from that scan.
+          echo "Waiting 60s for alert API to settle after CodeQL completion..."
+          sleep 60
+
+          # Poll alerts twice with a gap to confirm the count is stable
+          ALERTS1=$(gh api 'repos/${{ github.repository }}/code-scanning/alerts?state=open' --jq 'length' 2>/dev/null || echo "0")
+          echo "Open alerts (check 1): $ALERTS1"
+          sleep 15
+          ALERTS2=$(gh api 'repos/${{ github.repository }}/code-scanning/alerts?state=open' --jq 'length' 2>/dev/null || echo "0")
+          echo "Open alerts (check 2): $ALERTS2"
+
+          # Use the higher count (conservative — if either check sees alerts, block)
+          ALERTS=$ALERTS2
+          if [ "$ALERTS1" -gt "$ALERTS2" ]; then
+            ALERTS=$ALERTS1
+          fi
+
           if [ "$ALERTS" -gt 0 ]; then
             echo "BLOCKED: $ALERTS open code scanning alert(s) found."
-            echo "Fix them before releasing: https://github.com/${{ github.repository }}/security/code-scanning"
+            gh api 'repos/${{ github.repository }}/code-scanning/alerts?state=open' \
+              --jq '.[] | "  #\(.number) [\(.rule.security_severity_level // .rule.severity)] \(.rule.id) — \(.most_recent_instance.location.path):\(.most_recent_instance.location.start_line)"' 2>/dev/null || true
+            echo "Fix them: https://github.com/${{ github.repository }}/security/code-scanning"
             exit 1
           fi
           echo "=== CodeQL gate passed (0 open alerts on current commit) ==="

Makefile.cbm

@@ -288,7 +288,9 @@ TEST_MEM_SRCS = tests/test_mem.c
 
 TEST_UI_SRCS = tests/test_ui.c
 
-ALL_TEST_SRCS = $(TEST_FOUNDATION_SRCS) $(TEST_EXTRACTION_SRCS) $(TEST_STORE_SRCS) $(TEST_CYPHER_SRCS) $(TEST_MCP_SRCS) $(TEST_DISCOVER_SRCS) $(TEST_GRAPH_BUFFER_SRCS) $(TEST_PIPELINE_SRCS) $(TEST_WATCHER_SRCS) $(TEST_LZ4_SRCS) $(TEST_SQLITE_WRITER_SRCS) $(TEST_GO_LSP_SRCS) $(TEST_C_LSP_SRCS) $(TEST_TRACES_SRCS) $(TEST_HTTPLINK_SRCS) $(TEST_CLI_SRCS) $(TEST_MEM_SRCS) $(TEST_UI_SRCS) $(TEST_INTEGRATION_SRCS)
+TEST_SECURITY_SRCS = tests/test_security.c
+
+ALL_TEST_SRCS = $(TEST_FOUNDATION_SRCS) $(TEST_EXTRACTION_SRCS) $(TEST_STORE_SRCS) $(TEST_CYPHER_SRCS) $(TEST_MCP_SRCS) $(TEST_DISCOVER_SRCS) $(TEST_GRAPH_BUFFER_SRCS) $(TEST_PIPELINE_SRCS) $(TEST_WATCHER_SRCS) $(TEST_LZ4_SRCS) $(TEST_SQLITE_WRITER_SRCS) $(TEST_GO_LSP_SRCS) $(TEST_C_LSP_SRCS) $(TEST_TRACES_SRCS) $(TEST_HTTPLINK_SRCS) $(TEST_CLI_SRCS) $(TEST_MEM_SRCS) $(TEST_UI_SRCS) $(TEST_SECURITY_SRCS) $(TEST_INTEGRATION_SRCS)
 
 # ── Build directories ────────────────────────────────────────────
 

scripts/security-allowlist.txt

@@ -4,14 +4,14 @@
 # Any call to a listed function in a .c file under src/ that is NOT on this
 # list causes the security audit (scripts/security-audit.sh) to fail.
 
-# ── Foundation: platform abstraction (defines cbm_popen wrapper) ───────────
+# ── Foundation: platform abstraction (defines cbm_popen wrapper + shell-free exec) ──
 src/foundation/compat_fs.c:popen:cbm_popen wrapper definition (POSIX)
 src/foundation/compat_fs.c:cbm_popen:cbm_popen function definition
+src/foundation/compat_fs.c:fork:cbm_exec_no_shell — fork+execvp for shell-free subprocess execution
+src/foundation/compat_fs.c:execvp:cbm_exec_no_shell — direct exec without shell interpretation
 
 # ── CLI: update command (user-initiated, interactive) ──────────────────────
 src/cli/cli.c:system:curl download of release binary (update cmd)
-src/cli/cli.c:system:unzip extraction on Windows (update cmd)
-src/cli/cli.c:system:version verification after update (update cmd)
 src/cli/cli.c:cbm_popen:sha256 checksum verification (update cmd)
 src/cli/cli.c:popen:sha256 checksum computation via shasum
 

src/cli/cli.c

@@ -2810,15 +2810,9 @@ int cbm_cmd_update(int argc, char **argv) {
         fclose(out);
         free(bin_data);
     } else {
-        /* Windows: unzip — validate paths before shell interpolation */
-        if (!cbm_validate_shell_arg(bin_dir) || !cbm_validate_shell_arg(tmp_archive)) {
-            fprintf(stderr, "error: path contains unsafe characters\n");
-            cbm_unlink(tmp_archive);
-            return 1;
-        }
-        snprintf(cmd, sizeof(cmd), "unzip -o -d '%s' '%s' 2>/dev/null", bin_dir, tmp_archive);
-        // NOLINTNEXTLINE(cert-env33-c) — intentional CLI subprocess for extraction
-        rc = system(cmd);
+        /* Zip extraction: exec unzip directly without shell interpretation */
+        const char *unzip_argv[] = {"unzip", "-o", "-d", bin_dir, tmp_archive, NULL};
+        rc = cbm_exec_no_shell(unzip_argv);
         cbm_unlink(tmp_archive);
         if (rc != 0) {
             fprintf(stderr, "error: extraction failed\n");
@@ -2839,12 +2833,11 @@ int cbm_cmd_update(int argc, char **argv) {
     int skill_count = cbm_install_skills(skills_dir, true, false);
     printf("Updated %d skill(s).\n", skill_count);
 
-    /* Step 7: Verify new version */
+    /* Step 7: Verify new version (exec directly, no shell interpretation) */
     printf("\nUpdate complete. Verifying:\n");
-    if (cbm_validate_shell_arg(bin_dest)) {
-        snprintf(cmd, sizeof(cmd), "'%s' --version", bin_dest);
-        // NOLINTNEXTLINE(cert-env33-c,clang-analyzer-optin.taint.GenericTaint)
-        (void)system(cmd);
+    {
+        const char *ver_argv[] = {bin_dest, "--version", NULL};
+        (void)cbm_exec_no_shell(ver_argv);
     }
 
     printf("\nAll project indexes were cleared. They will be rebuilt\n");

src/foundation/compat_fs.c

@@ -144,13 +144,21 @@ int cbm_rmdir(const char *path) {
     return _rmdir(path);
 }
 
+int cbm_exec_no_shell(const char *const *argv) {
+    if (!argv || !argv[0]) {
+        return -1;
+    }
+    return (int)_spawnvp(_P_WAIT, argv[0], argv);
+}
+
 #else /* POSIX */
 
 /* ── POSIX implementation ─────────────────────────────────────── */
 
 #include <dirent.h>
 #include <errno.h>
 #include <sys/stat.h>
+#include <sys/wait.h>
 #include <unistd.h>
 
 struct cbm_dir {
@@ -248,4 +256,30 @@ int cbm_rmdir(const char *path) {
     return rmdir(path);
 }
 
+int cbm_exec_no_shell(const char *const *argv) {
+    if (!argv || !argv[0]) {
+        return -1;
+    }
+    pid_t pid = fork();
+    if (pid < 0) {
+        return -1;
+    }
+    if (pid == 0) {
+        /* Child: exec directly — no shell interpretation */
+        /* 127 = standard "command not found" exit code (POSIX convention) */
+        enum { EXEC_NOT_FOUND = 127 };
+        execvp(argv[0], (char *const *)argv);
+        _exit(EXEC_NOT_FOUND);
+    }
+    /* Parent: wait for child */
+    int status = 0;
+    if (waitpid(pid, &status, 0) < 0) {
+        return -1;
+    }
+    if (WIFEXITED(status)) {
+        return WEXITSTATUS(status);
+    }
+    return -1; /* killed by signal */
+}
+
 #endif /* _WIN32 */

src/foundation/compat_fs.h

@@ -50,4 +50,10 @@ int cbm_unlink(const char *path);
 /* Delete an empty directory. Returns 0 on success. */
 int cbm_rmdir(const char *path);
 
+/* Execute a command without shell interpretation.
+ * argv is a NULL-terminated array: {"cmd", "arg1", "arg2", NULL}.
+ * Returns the process exit code, or -1 on fork/exec failure.
+ * POSIX: fork() + execvp(). Windows: _spawnvp(). */
+int cbm_exec_no_shell(const char *const *argv);
+
 #endif /* CBM_COMPAT_FS_H */

tests/test_main.c

@@ -48,6 +48,7 @@ extern void suite_worker_pool(void);
 extern void suite_parallel(void);
 extern void suite_mem(void);
 extern void suite_ui(void);
+extern void suite_security(void);
 extern void suite_integration(void);
 
 int main(void) {
@@ -132,6 +133,9 @@ int main(void) {
     /* UI (config, embedded assets, layout) */
     RUN_SUITE(ui);
 
+    /* Security defenses */
+    RUN_SUITE(security);
+
     /* Integration (end-to-end) */
     RUN_SUITE(integration);