Fix(chore): security hardening (#55)

* fix(toast): ignore labelHtml and execJs from untrusted DOM events

* fix(mix): contain corex.code output paths within project root

* fix(mix): validate generator flags and migration_dir

* fix(mix): stop EEx re-eval in inject_eex_before_final_end

* fix(installer): harden --dev path and COREX_NEW_CACHE_DIR copy

* fix(installer): validate web_module like root module

* fix(color-picker): reject CSS breakout in inline style values

* fix(components): harden file upload cancel, navigate, and pagination URLs

* chore: re-enable sobelow checks and add hex.audit to release.check

* fix(mcp): redact tool errors by default and warn on remote access

Author: karim-semmoud

.sobelow-conf

@@ -1 +1 @@
-[exit: false, format: "txt", ignore_files: [], ignore: ["Traversal.FileModule", "RCE.EEx", "Config"], out: nil, private: false, router: nil, skip: false, threshold: :low, verbose: false, version: false]
+[exit: false, format: "txt", ignore_files: ["lib/mix/corex.ex", "lib/mix/corex/gen/context.ex", "lib/corex/doc_parity.ex"], ignore: ["Config"], out: nil, private: false, router: nil, skip: false, threshold: :low, verbose: false, version: false]

assets/hooks/toast.ts

@@ -36,7 +36,7 @@ export function parseSingleExecJsEffect(raw: unknown): string | null {
   return encoded;
 }
 
-export function parseActionSpec(raw: unknown): ToastActionSpec | null {
+export function parseServerActionSpec(raw: unknown): ToastActionSpec | null {
   const o = asRecord(raw);
   const label = o.label;
   if (typeof label !== "string" || label.length === 0) return null;
@@ -55,6 +55,12 @@ export function parseActionSpec(raw: unknown): ToastActionSpec | null {
   return spec;
 }
 
+export function parseDomActionSpec(_raw: unknown): ToastActionSpec | null {
+  return null;
+}
+
+export const parseActionSpec = parseServerActionSpec;
+
 function buildZagAction(
   spec: ToastActionSpec,
   rt: ToastHookRuntime
@@ -217,8 +223,10 @@ const ToastHook: Hook<object & ToastHookState, HTMLElement> = {
 
     const rt = buildRuntime(this);
 
-    const buildCreateOptions = (payload: ToastCreatePayload): Options => {
-      const spec = parseActionSpec(payload.action);
+    const buildCreateOptions = (payload: ToastCreatePayload, trusted: boolean): Options => {
+      const spec = trusted
+        ? parseServerActionSpec(payload.action)
+        : parseDomActionSpec(payload.action);
       const base: Options = {
         title: payload.title ?? "",
         description: payload.description,
@@ -235,7 +243,7 @@ const ToastHook: Hook<object & ToastHookState, HTMLElement> = {
       return base;
     };
 
-    const buildUpdatePatch = (payload: ToastUpdatePayload): Partial<Options> => {
+    const buildUpdatePatch = (payload: ToastUpdatePayload, trusted: boolean): Partial<Options> => {
       const patch: Partial<Options> = {};
       if (payload.title !== undefined) patch.title = payload.title;
       if (payload.description !== undefined) patch.description = payload.description;
@@ -246,7 +254,9 @@ const ToastHook: Hook<object & ToastHookState, HTMLElement> = {
       } else if (payload.loading === false || payload.loading === "false") {
         patch.meta = { loading: false };
       }
-      const spec = parseActionSpec(payload.action);
+      const spec = trusted
+        ? parseServerActionSpec(payload.action)
+        : parseDomActionSpec(payload.action);
       if (spec) {
         patch.action = buildZagAction(spec, rt);
       } else if (payload.action === null) {
@@ -284,7 +294,7 @@ const ToastHook: Hook<object & ToastHookState, HTMLElement> = {
         const st = getToastStore(payload.groupId || this.groupId);
         if (!st) return;
         try {
-          st.create(buildCreateOptions(payload));
+          st.create(buildCreateOptions(payload, true));
         } catch (error) {
           console.error("Failed to create toast:", error);
         }
@@ -296,7 +306,7 @@ const ToastHook: Hook<object & ToastHookState, HTMLElement> = {
         const st = getToastStore(payload.groupId || this.groupId);
         if (!st || !payload.id) return;
         try {
-          st.update(payload.id, buildUpdatePatch(payload));
+          st.update(payload.id, buildUpdatePatch(payload, true));
         } catch (error) {
           console.error("Failed to update toast:", error);
         }
@@ -312,7 +322,7 @@ const ToastHook: Hook<object & ToastHookState, HTMLElement> = {
       const st = getToastStore(detail.groupId || this.groupId);
       if (!st) return;
       try {
-        st.create(buildCreateOptions(detail));
+        st.create(buildCreateOptions(detail, false));
       } catch (error) {
         console.error("Failed to create toast:", error);
       }
@@ -323,7 +333,7 @@ const ToastHook: Hook<object & ToastHookState, HTMLElement> = {
       const st = getToastStore(detail.groupId || this.groupId);
       if (!st || !detail.id) return;
       try {
-        st.update(detail.id, buildUpdatePatch(detail));
+        st.update(detail.id, buildUpdatePatch(detail, false));
       } catch (error) {
         console.error("Failed to update toast:", error);
       }

assets/test/hooks/toast.test.ts

@@ -1,13 +1,18 @@
 import { describe, expect, it, afterEach } from "vitest";
 import type { CallbackRef } from "phoenix_live_view/assets/js/types/view_hook";
-import { parseActionSpec, Toast } from "../../hooks/toast";
+import {
+  parseActionSpec,
+  parseDomActionSpec,
+  parseServerActionSpec,
+  Toast,
+} from "../../hooks/toast";
 import { getToastStore } from "../../components/toast";
 import { callHookDestroyed, callHookMounted, mockHookContext } from "../helpers/mock-hook";
 
-describe("parseActionSpec", () => {
+describe("parseServerActionSpec", () => {
   it("parses valid exec_js action", () => {
     expect(
-      parseActionSpec({
+      parseServerActionSpec({
         label: "Undo",
         effects: [{ kind: "exec_js", encoded: "abc" }],
       })
@@ -16,7 +21,7 @@ describe("parseActionSpec", () => {
 
   it("includes className when present", () => {
     expect(
-      parseActionSpec({
+      parseServerActionSpec({
         label: "Go",
         class: " button--sm ",
         effects: [{ kind: "exec_js", encoded: "x" }],
@@ -26,7 +31,7 @@ describe("parseActionSpec", () => {
 
   it("includes labelHtml when present", () => {
     expect(
-      parseActionSpec({
+      parseServerActionSpec({
         label: "<span>Open</span>",
         labelHtml: true,
         effects: [{ kind: "exec_js", encoded: "x" }],
@@ -35,10 +40,37 @@ describe("parseActionSpec", () => {
   });
 
   it("returns null for invalid shape", () => {
-    expect(parseActionSpec(null)).toBeNull();
-    expect(parseActionSpec({ label: "" })).toBeNull();
-    expect(parseActionSpec({ label: "X", effects: [] })).toBeNull();
-    expect(parseActionSpec({ label: "X", effects: [{ kind: "other", encoded: "x" }] })).toBeNull();
+    expect(parseServerActionSpec(null)).toBeNull();
+    expect(parseServerActionSpec({ label: "" })).toBeNull();
+    expect(parseServerActionSpec({ label: "X", effects: [] })).toBeNull();
+    expect(
+      parseServerActionSpec({ label: "X", effects: [{ kind: "other", encoded: "x" }] })
+    ).toBeNull();
+  });
+});
+
+describe("parseDomActionSpec", () => {
+  it("rejects all action payloads including labelHtml and exec_js", () => {
+    expect(parseDomActionSpec(null)).toBeNull();
+    expect(
+      parseDomActionSpec({
+        label: "<img src=x onerror=alert(1)>",
+        labelHtml: true,
+        effects: [{ kind: "exec_js", encoded: "evil" }],
+      })
+    ).toBeNull();
+    expect(
+      parseDomActionSpec({
+        label: "Undo",
+        effects: [{ kind: "exec_js", encoded: "abc" }],
+      })
+    ).toBeNull();
+  });
+});
+
+describe("parseActionSpec", () => {
+  it("aliases parseServerActionSpec", () => {
+    expect(parseActionSpec).toBe(parseServerActionSpec);
   });
 });
 
@@ -71,4 +103,48 @@ describe("Toast hook lifecycle", () => {
     expect(getToastStore(groupId)).toBeUndefined();
     expect(el.dataset.toastGroup).toBeUndefined();
   });
+
+  it("DOM toast:create ignores untrusted action payloads", () => {
+    const el = document.createElement("div");
+    el.id = groupId;
+    document.body.appendChild(el);
+
+    const execJs = () => {
+      throw new Error("execJs should not run");
+    };
+
+    const { hook } = mockHookContext(el, {
+      connected: false,
+      overrides: {
+        groupId: "",
+        handlers: [] as CallbackRef[],
+        domListeners: [] as Array<{ el: HTMLElement; name: string; fn: EventListener }>,
+        js: () => ({ exec: execJs }),
+      },
+    });
+
+    callHookMounted(Toast, hook);
+
+    el.dispatchEvent(
+      new CustomEvent("toast:create", {
+        detail: {
+          id: "dom-toast",
+          title: "Hello",
+          action: {
+            label: '<img src=x onerror="window.__toastDomXss=1">',
+            labelHtml: true,
+            effects: [{ kind: "exec_js", encoded: "evil" }],
+          },
+        },
+      })
+    );
+
+    const action = el.querySelector<HTMLElement>(
+      '[data-scope="toast"][data-part="action-trigger"]'
+    );
+    expect(action).toBeNull();
+
+    document.body.removeChild(el);
+    callHookDestroyed(Toast, hook);
+  });
 });

config/config.exs

@@ -1,5 +1,7 @@
 import Config
 
+config :corex, mcp_verbose_errors: false
+
 config :logger, :console,
   colors: [enabled: false],
   format: "\n$time $metadata[$level] $message\n",

guides/MCP.md

@@ -114,6 +114,27 @@ Verbose MCP logging:
 config :corex, debug: true
 ```
 
+## Security
+
+| Setting | Default | Purpose |
+| ------- | ------- | ------- |
+| Loopback-only access | on | Non-loopback clients receive 403 unless you opt in |
+| `allow_remote_access: true` | off | Allows non-loopback IPs; logs a warning at plug init. Restrict with a firewall or VPN. Never use in production. |
+| `config :corex, mcp_verbose_errors: false` | off | Tool failures return a generic message to MCP clients; full exceptions stay in server logs |
+| `config :corex, debug: true` | off | Enables verbose MCP JSON-RPC debug logging on `Corex.MCP.Server` |
+
+Example for local debugging only:
+
+```elixir
+config :corex, debug: true, mcp_verbose_errors: true
+```
+
+Remote access (discouraged outside trusted networks):
+
+```elixir
+plug Corex.MCP, allow_remote_access: true
+```
+
 ## Related
 
 - [Installation](installation.html) — `mix corex.new` enables MCP in dev by default

installer/lib/corex_new/cli.ex

@@ -32,8 +32,12 @@ defmodule Corex.New.Cli do
         :ok
 
       v when is_binary(v) ->
-        if String.trim(v) == "" do
+        trimmed = String.trim(v)
+
+        if trimmed == "" do
           Mix.raise("--dev requires a non-empty path (for example: --dev ../corex)")
+        else
+          validate_dev_path!(trimmed)
         end
 
       _ ->
@@ -111,4 +115,16 @@ defmodule Corex.New.Cli do
       Mix.raise("Please select another directory for installation.")
     end
   end
+
+  def validate_dev_path!(path) when is_binary(path) do
+    if String.match?(path, ~r/["\r\n\x00]/) do
+      Mix.raise("""
+      --dev path contains invalid characters.
+
+      Provide a filesystem path without quotes or newlines.
+      """)
+    end
+
+    :ok
+  end
 end

installer/lib/corex_new/generate.ex

@@ -330,6 +330,7 @@ defmodule Corex.New.Generate do
         trimmed = String.trim(path)
 
         if trimmed != "" do
+          Corex.New.Cli.validate_dev_path!(trimmed)
           corex_root = Path.expand(trimmed, install_dir)
           mjs = Path.join([corex_root, "priv", "static", "corex.mjs"])
 

installer/lib/corex_new/patches.ex

@@ -436,7 +436,8 @@ defmodule Corex.New.Patches do
         trimmed = String.trim(path)
 
         if trimmed != "" do
-          ~s([path: "#{trimmed}", override: true])
+          Corex.New.Cli.validate_dev_path!(trimmed)
+          "[path: #{inspect(trimmed)}, override: true]"
         else
           corex_dep_constraint()
         end

installer/lib/corex_new/post_generate.ex

@@ -6,16 +6,41 @@ defmodule Corex.New.PostGenerate do
   def copy_cached_build(project_path) do
     case System.fetch_env("COREX_NEW_CACHE_DIR") do
       {:ok, cache_dir} ->
+        cache_dir = validate_cache_dir!(cache_dir)
+
         if File.exists?(cache_dir) do
           Mix.shell().info("Copying cached build from #{cache_dir}")
-          System.cmd("cp", ["-Rp", Path.join(cache_dir, "."), project_path])
+          copy_tree!(cache_dir, project_path)
         end
 
       :error ->
         :ok
     end
   end
 
+  defp validate_cache_dir!(cache_dir) do
+    expanded = Path.expand(cache_dir)
+
+    cond do
+      String.match?(cache_dir, ~r/["\r\n\x00]/) ->
+        Mix.raise("COREX_NEW_CACHE_DIR contains invalid characters")
+
+      not File.dir?(expanded) ->
+        Mix.raise("COREX_NEW_CACHE_DIR is not a directory: #{inspect(cache_dir)}")
+
+      true ->
+        expanded
+    end
+  end
+
+  defp copy_tree!(source_dir, dest_dir) do
+    File.mkdir_p!(dest_dir)
+
+    for entry <- File.ls!(source_dir) do
+      File.cp_r!(Path.join(source_dir, entry), Path.join(dest_dir, entry))
+    end
+  end
+
   def init_git(project_path) do
     if git_available?() and not inside_git_repo?(project_path) do
       Mix.shell().info([:green, "* initializing git repository", :reset])

installer/lib/mix/tasks/corex.new.ex

@@ -183,6 +183,9 @@ defmodule Mix.Tasks.Corex.New do
         true -> Module.concat([Macro.camelize(app) <> "Web"])
       end
 
+    check_module_name_validity!(web_module)
+    check_module_name_availability!(web_module)
+
     install_dir = PhxWrapper.web_project_path(phx_root, opts)
 
     PhxWrapper.ensure_phx_new!()