Skip to content
Merged
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
2 changes: 1 addition & 1 deletion .sobelow-conf
Original file line number Diff line number Diff line change
@@ -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]
28 changes: 19 additions & 9 deletions assets/hooks/toast.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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;
Expand All @@ -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) {
Expand Down Expand Up @@ -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);
}
Expand All @@ -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);
}
Expand All @@ -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);
}
Expand All @@ -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);
}
Expand Down
94 changes: 85 additions & 9 deletions assets/test/hooks/toast.test.ts
Original file line number Diff line number Diff line change
@@ -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" }],
})
Expand All @@ -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" }],
Expand All @@ -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" }],
Expand All @@ -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);
});
});

Expand Down Expand Up @@ -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);
});
});
2 changes: 2 additions & 0 deletions config/config.exs
Original file line number Diff line number Diff line change
@@ -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",
Expand Down
21 changes: 21 additions & 0 deletions guides/MCP.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
18 changes: 17 additions & 1 deletion installer/lib/corex_new/cli.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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

_ ->
Expand Down Expand Up @@ -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
1 change: 1 addition & 0 deletions installer/lib/corex_new/generate.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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"])

Expand Down
3 changes: 2 additions & 1 deletion installer/lib/corex_new/patches.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
27 changes: 26 additions & 1 deletion installer/lib/corex_new/post_generate.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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])
Expand Down
3 changes: 3 additions & 0 deletions installer/lib/mix/tasks/corex.new.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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!()
Expand Down
2 changes: 1 addition & 1 deletion installer/templates/corex/assets/js/app.js.eex
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import {LiveSocket} from "phoenix_live_view"
<%= if @corex_js_import == "corex" do %>
import corex from "corex"
<% else %>
import corex from "<%= @corex_js_import %>"
import corex from <%= inspect(@corex_js_import) %>
<% end %>
import {hooks as colocatedHooks} from "phoenix-colocated/<%= @otp_app %>"
import topbar from "../vendor/topbar"
Expand Down
12 changes: 12 additions & 0 deletions installer/test/core_new_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,18 @@ defmodule Mix.Tasks.Corex.NewTest do
Mix.Tasks.Corex.New.run(["valid", "--module", "String"])
end
end)

in_tmp("web module invalid", fn ->
assert_raise Mix.Error, ~r"Module name must be a valid Elixir alias", fn ->
Mix.Tasks.Corex.New.run(["valid", "--web-module", "not.valid", "--no-install"])
end
end)

in_tmp("web module taken", fn ->
assert_raise Mix.Error, ~r"Module name \w+ is already taken", fn ->
Mix.Tasks.Corex.New.run(["valid", "--web-module", "String", "--no-install"])
end
end)
end

test "invalid options" do
Expand Down
Loading
Loading