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
6 changes: 4 additions & 2 deletions packages/desktop/src-tauri/capabilities/default.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,10 @@
{
"identifier": "http:default",
"allow": [
{ "url": "http://*" },
{ "url": "http://*:*" },
{ "url": "http://localhost" },
{ "url": "http://localhost:*" },
{ "url": "http://127.0.0.1" },
{ "url": "http://127.0.0.1:*" },
{ "url": "https://*" },
{ "url": "https://*:*" }
]
Expand Down
2 changes: 1 addition & 1 deletion packages/desktop/src-tauri/tauri.conf.json
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
}
],
"security": {
"csp": null
"csp": "default-src 'self'; script-src 'self'; style-src 'self' 'unsafe-inline'; img-src 'self' data: blob:; font-src 'self' data:; connect-src 'self' http://localhost:* http://127.0.0.1:* https:; object-src 'none'; base-uri 'none'; frame-ancestors 'none'"
}
},
"plugins": {
Expand Down
13 changes: 13 additions & 0 deletions packages/web/src/components/settings/SettingsForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
isCloudInstance,
} from "@/lib/config";
import { COLOR } from "@/lib/constants";
import { tokenTransportError } from "@/lib/security";

export type ConnectionPreset = "cloud" | "self-hosted";

Expand Down Expand Up @@ -81,6 +82,13 @@ export function SettingsForm({
async function handleTest() {
setChecking(true);
setHealth({ status: "checking", message: "Connecting..." });
const transportError = token.trim() ? tokenTransportError(baseUrl) : null;
if (transportError) {
setErrors({ baseUrl: transportError });
setHealth({ status: "unreachable", message: transportError });
setChecking(false);
return;
}
const result = await checkConnection(baseUrl, token || undefined);
setHealth(result);
setChecking(false);
Expand Down Expand Up @@ -112,6 +120,11 @@ export function SettingsForm({
setErrors({ token: "API key is required for Honcho Cloud" });
return;
}
const transportError = token.trim() ? tokenTransportError(result.data.baseUrl) : null;
if (transportError) {
setErrors({ baseUrl: transportError });
return;
}
setErrors({});

let id: string;
Expand Down
17 changes: 15 additions & 2 deletions packages/web/src/components/workspaces/WebhookManager.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,12 @@ import { Input } from "@/components/ui/input";
import { Body, Muted, PageTitle, SectionHeading } from "@/components/ui/typography";
import { useDemo } from "@/hooks/useDemo";
import { COLOR } from "@/lib/constants";
import { isHttpOrHttpsUrl, isSafeExternalUrl } from "@/lib/security";

const urlSchema = z.string().url({ message: "Must be a valid URL" });
const urlSchema = z
.string()
.url({ message: "Must be a valid URL" })
.refine(isHttpOrHttpsUrl, { message: "Webhook URL must use http or https" });

interface Props {
workspaceId: string;
Expand Down Expand Up @@ -50,6 +54,15 @@ export function WebhookManager({ workspaceId }: Props) {
setTimeout(() => setTestResult(null), 5000);
};

const handleOpenWebhook = async (webhookUrl: string) => {
if (!isSafeExternalUrl(webhookUrl)) {
setTestResult(`Blocked unsafe webhook URL: ${webhookUrl}`);
setTimeout(() => setTestResult(null), 5000);
return;
}
await open(webhookUrl);
};

const list = Array.isArray(webhooks) ? webhooks : [];

return (
Expand Down Expand Up @@ -161,7 +174,7 @@ export function WebhookManager({ workspaceId }: Props) {
</span>
<button
type="button"
onClick={() => void open((wh as { url: string }).url)}
onClick={() => void handleOpenWebhook((wh as { url: string }).url)}
className="flex-shrink-0"
style={{
color: "var(--text-4)",
Expand Down
2 changes: 1 addition & 1 deletion packages/web/src/hooks/useHealthStatus.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ const POLL_INTERVAL_MS = 30_000;
export function useHealthStatus() {
const { active } = useInstances();
return useQuery({
queryKey: ["health", active?.id, active?.baseUrl, active?.token],
queryKey: ["health", active?.id, active?.baseUrl, Boolean(active?.token)],
queryFn: async () => {
if (!active) throw new Error("No active instance");
return checkConnection(active.baseUrl, active.token || undefined);
Expand Down
38 changes: 38 additions & 0 deletions packages/web/src/lib/security.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
const LOOPBACK_HOSTS = new Set(["localhost", "127.0.0.1", "::1", "[::1]", "0.0.0.0"]);

function parseUrl(value: string): URL | null {
try {
return new URL(value);
} catch {
return null;
}
}

export function isLoopbackUrl(value: string): boolean {
const parsed = parseUrl(value);
if (!parsed) return false;
return LOOPBACK_HOSTS.has(parsed.hostname.toLowerCase());
}

export function isHttpOrHttpsUrl(value: string): boolean {
const parsed = parseUrl(value);
return parsed?.protocol === "https:" || parsed?.protocol === "http:";
}

export function isSafeExternalUrl(value: string): boolean {
const parsed = parseUrl(value);
return parsed?.protocol === "https:" || parsed?.protocol === "http:";
}

export function isSecureTokenTransport(baseUrl: string): boolean {
const parsed = parseUrl(baseUrl);
if (!parsed) return false;
if (parsed.protocol === "https:") return true;
if (parsed.protocol === "http:" && LOOPBACK_HOSTS.has(parsed.hostname.toLowerCase())) return true;
return false;
}

export function tokenTransportError(baseUrl: string): string | null {
if (isSecureTokenTransport(baseUrl)) return null;
return "API tokens require HTTPS unless connecting to localhost.";
}
8 changes: 4 additions & 4 deletions packages/web/src/test/app.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,10 @@ describe("Sidebar/useDemo availability across routes", () => {
const { demo } = useDemo();
return <span data-testid="demo-flag">{String(demo)}</span>;
}
// After the fix, DemoProvider wraps the app at the root (main.tsx /
// __root.tsx) so consumers anywhere in the tree resolve. This test
// renders a consumer as a sibling of the router under the same provider
// the production wiring uses.
// After the fix, DemoProvider and MetadataProvider wrap the app at
// the root (main.tsx) so consumers anywhere in the tree resolve.
// This test renders a consumer as a sibling of the router under the
// same providers the production wiring uses.
localStorage.clear();
expect(() => {
const router = createRouter({
Expand Down
19 changes: 19 additions & 0 deletions packages/web/src/test/health-status.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,23 @@ describe("useHealthStatus", () => {
const { result } = renderHook(() => useHealthStatus(), { wrapper: wrap(qc) });
await waitFor(() => expect(result.current.data?.status).toBe("ok"));
});

it("does not include raw tokens in query cache keys", async () => {
saveStore({
instances: [
{ id: "i1", name: "Local", baseUrl: "http://localhost:8000", token: "super-secret-token" },
],
activeId: "i1",
});
httpFetch.mockResolvedValue(new Response("{}", { status: 200 }));
const qc = new QueryClient({ defaultOptions: { queries: { retry: false } } });
renderHook(() => useHealthStatus(), { wrapper: wrap(qc) });
await waitFor(() => expect(httpFetch).toHaveBeenCalled());

const cacheKeys = qc
.getQueryCache()
.getAll()
.map((query) => JSON.stringify(query.queryKey));
expect(cacheKeys.join("\n")).not.toContain("super-secret-token");
});
});
37 changes: 37 additions & 0 deletions packages/web/src/test/security.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import { describe, expect, it } from "vitest";
import {
isHttpOrHttpsUrl,
isSafeExternalUrl,
isSecureTokenTransport,
tokenTransportError,
} from "@/lib/security";

describe("security URL helpers", () => {
it("only allows http and https URLs for external OS opens", () => {
expect(isSafeExternalUrl("https://example.com/webhook")).toBe(true);
expect(isSafeExternalUrl("http://localhost:3000/webhook")).toBe(true);
expect(isSafeExternalUrl("file:///etc/passwd")).toBe(false);
expect(isSafeExternalUrl("javascript:alert(1)")).toBe(false);
expect(isSafeExternalUrl("openconcho://settings")).toBe(false);
});

it("only accepts http and https webhook endpoints", () => {
expect(isHttpOrHttpsUrl("https://hooks.example.com/a")).toBe(true);
expect(isHttpOrHttpsUrl("http://hooks.example.com/a")).toBe(true);
expect(isHttpOrHttpsUrl("ftp://hooks.example.com/a")).toBe(false);
expect(isHttpOrHttpsUrl("notaurl")).toBe(false);
});

it("requires HTTPS before sending tokens to non-loopback hosts", () => {
expect(isSecureTokenTransport("https://honcho.example.com")).toBe(true);
expect(isSecureTokenTransport("http://localhost:8000")).toBe(true);
expect(isSecureTokenTransport("http://127.0.0.1:8000")).toBe(true);
expect(isSecureTokenTransport("http://192.168.1.50:8000")).toBe(false);
expect(isSecureTokenTransport("http://100.67.206.76:8000")).toBe(false);
});

it("returns a user-facing error for insecure token transport", () => {
expect(tokenTransportError("http://100.67.206.76:8000")).toMatch(/HTTPS/);
expect(tokenTransportError("https://honcho.example.com")).toBeNull();
});
});
18 changes: 18 additions & 0 deletions packages/web/src/test/settings-form.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,24 @@ describe("SettingsForm — self-hosted preset", () => {
const store = loadStore();
expect(store.instances).toHaveLength(1);
});

it("blocks saving a token for a non-localhost HTTP endpoint", async () => {
const user = userEvent.setup();
renderForm(<SettingsForm instance={null} preset="self-hosted" />);
const baseUrl = screen.getByPlaceholderText("http://localhost:8000");
await user.clear(baseUrl);
await user.type(baseUrl, "http://100.67.206.76:8000");
await user.type(
screen.getByPlaceholderText(/required only if your instance has auth enabled/i),
"secret-token",
);
await user.click(screen.getByRole("button", { name: /Add Instance/i }));

expect(
screen.getByText(/API tokens require HTTPS unless connecting to localhost/i),
).toBeInTheDocument();
expect(loadStore().instances).toHaveLength(0);
});
});

describe("SettingsForm — edit mode auto-detects cloud", () => {
Expand Down
6 changes: 2 additions & 4 deletions packages/web/src/test/setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,8 @@ import "@testing-library/jest-dom/vitest";
import { cleanup } from "@testing-library/react";
import { afterEach, vi } from "vitest";

// jsdom doesn't implement matchMedia; theme code reads it on mount.
if (!window.scrollTo) {
window.scrollTo = vi.fn() as unknown as typeof window.scrollTo;
}
// jsdom defines scrollTo but leaves it unimplemented; router scroll restoration calls it.
window.scrollTo = vi.fn() as unknown as typeof window.scrollTo;

if (!window.matchMedia) {
window.matchMedia = vi.fn().mockImplementation((query: string) => ({
Expand Down
Loading