Skip to content

Commit ac20ab6

Browse files
offendingcommitoffendingcommit
authored
Merge pull request #22 from offendingcommit/fix/web-security-hardening
fix: harden token and URL transport security
2 parents 62a5085 + 5cfbae6 commit ac20ab6

11 files changed

Lines changed: 152 additions & 14 deletions

File tree

‎packages/desktop/src-tauri/capabilities/default.json‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,10 @@
88
{
99
"identifier": "http:default",
1010
"allow": [
11-
{ "url": "http://*" },
12-
{ "url": "http://*:*" },
11+
{ "url": "http://localhost" },
12+
{ "url": "http://localhost:*" },
13+
{ "url": "http://127.0.0.1" },
14+
{ "url": "http://127.0.0.1:*" },
1315
{ "url": "https://*" },
1416
{ "url": "https://*:*" }
1517
]

‎packages/desktop/src-tauri/tauri.conf.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@
3030
}
3131
],
3232
"security": {
33-
"csp": null
33+
"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'"
3434
}
3535
},
3636
"plugins": {

‎packages/web/src/components/settings/SettingsForm.tsx‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import {
2424
isCloudInstance,
2525
} from "@/lib/config";
2626
import { COLOR } from "@/lib/constants";
27+
import { tokenTransportError } from "@/lib/security";
2728

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

@@ -81,6 +82,13 @@ export function SettingsForm({
8182
async function handleTest() {
8283
setChecking(true);
8384
setHealth({ status: "checking", message: "Connecting..." });
85+
const transportError = token.trim() ? tokenTransportError(baseUrl) : null;
86+
if (transportError) {
87+
setErrors({ baseUrl: transportError });
88+
setHealth({ status: "unreachable", message: transportError });
89+
setChecking(false);
90+
return;
91+
}
8492
const result = await checkConnection(baseUrl, token || undefined);
8593
setHealth(result);
8694
setChecking(false);
@@ -112,6 +120,11 @@ export function SettingsForm({
112120
setErrors({ token: "API key is required for Honcho Cloud" });
113121
return;
114122
}
123+
const transportError = token.trim() ? tokenTransportError(result.data.baseUrl) : null;
124+
if (transportError) {
125+
setErrors({ baseUrl: transportError });
126+
return;
127+
}
115128
setErrors({});
116129

117130
let id: string;

‎packages/web/src/components/workspaces/WebhookManager.tsx‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,12 @@ import { Input } from "@/components/ui/input";
1313
import { Body, Muted, PageTitle, SectionHeading } from "@/components/ui/typography";
1414
import { useDemo } from "@/hooks/useDemo";
1515
import { COLOR } from "@/lib/constants";
16+
import { isHttpOrHttpsUrl, isSafeExternalUrl } from "@/lib/security";
1617

17-
const urlSchema = z.string().url({ message: "Must be a valid URL" });
18+
const urlSchema = z
19+
.string()
20+
.url({ message: "Must be a valid URL" })
21+
.refine(isHttpOrHttpsUrl, { message: "Webhook URL must use http or https" });
1822

1923
interface Props {
2024
workspaceId: string;
@@ -50,6 +54,15 @@ export function WebhookManager({ workspaceId }: Props) {
5054
setTimeout(() => setTestResult(null), 5000);
5155
};
5256

57+
const handleOpenWebhook = async (webhookUrl: string) => {
58+
if (!isSafeExternalUrl(webhookUrl)) {
59+
setTestResult(`Blocked unsafe webhook URL: ${webhookUrl}`);
60+
setTimeout(() => setTestResult(null), 5000);
61+
return;
62+
}
63+
await open(webhookUrl);
64+
};
65+
5366
const list = Array.isArray(webhooks) ? webhooks : [];
5467

5568
return (
@@ -161,7 +174,7 @@ export function WebhookManager({ workspaceId }: Props) {
161174
</span>
162175
<button
163176
type="button"
164-
onClick={() => void open((wh as { url: string }).url)}
177+
onClick={() => void handleOpenWebhook((wh as { url: string }).url)}
165178
className="flex-shrink-0"
166179
style={{
167180
color: "var(--text-4)",

‎packages/web/src/hooks/useHealthStatus.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ const POLL_INTERVAL_MS = 30_000;
77
export function useHealthStatus() {
88
const { active } = useInstances();
99
return useQuery({
10-
queryKey: ["health", active?.id, active?.baseUrl, active?.token],
10+
queryKey: ["health", active?.id, active?.baseUrl, Boolean(active?.token)],
1111
queryFn: async () => {
1212
if (!active) throw new Error("No active instance");
1313
return checkConnection(active.baseUrl, active.token || undefined);

‎packages/web/src/lib/security.ts‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
const LOOPBACK_HOSTS = new Set(["localhost", "127.0.0.1", "::1", "[::1]", "0.0.0.0"]);
2+
3+
function parseUrl(value: string): URL | null {
4+
try {
5+
return new URL(value);
6+
} catch {
7+
return null;
8+
}
9+
}
10+
11+
export function isLoopbackUrl(value: string): boolean {
12+
const parsed = parseUrl(value);
13+
if (!parsed) return false;
14+
return LOOPBACK_HOSTS.has(parsed.hostname.toLowerCase());
15+
}
16+
17+
export function isHttpOrHttpsUrl(value: string): boolean {
18+
const parsed = parseUrl(value);
19+
return parsed?.protocol === "https:" || parsed?.protocol === "http:";
20+
}
21+
22+
export function isSafeExternalUrl(value: string): boolean {
23+
const parsed = parseUrl(value);
24+
return parsed?.protocol === "https:" || parsed?.protocol === "http:";
25+
}
26+
27+
export function isSecureTokenTransport(baseUrl: string): boolean {
28+
const parsed = parseUrl(baseUrl);
29+
if (!parsed) return false;
30+
if (parsed.protocol === "https:") return true;
31+
if (parsed.protocol === "http:" && LOOPBACK_HOSTS.has(parsed.hostname.toLowerCase())) return true;
32+
return false;
33+
}
34+
35+
export function tokenTransportError(baseUrl: string): string | null {
36+
if (isSecureTokenTransport(baseUrl)) return null;
37+
return "API tokens require HTTPS unless connecting to localhost.";
38+
}

‎packages/web/src/test/app.test.tsx‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,10 +43,10 @@ describe("Sidebar/useDemo availability across routes", () => {
4343
const { demo } = useDemo();
4444
return <span data-testid="demo-flag">{String(demo)}</span>;
4545
}
46-
// After the fix, DemoProvider wraps the app at the root (main.tsx /
47-
// __root.tsx) so consumers anywhere in the tree resolve. This test
48-
// renders a consumer as a sibling of the router under the same provider
49-
// the production wiring uses.
46+
// After the fix, DemoProvider and MetadataProvider wrap the app at
47+
// the root (main.tsx) so consumers anywhere in the tree resolve.
48+
// This test renders a consumer as a sibling of the router under the
49+
// same providers the production wiring uses.
5050
localStorage.clear();
5151
expect(() => {
5252
const router = createRouter({

‎packages/web/src/test/health-status.test.tsx‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,4 +39,23 @@ describe("useHealthStatus", () => {
3939
const { result } = renderHook(() => useHealthStatus(), { wrapper: wrap(qc) });
4040
await waitFor(() => expect(result.current.data?.status).toBe("ok"));
4141
});
42+
43+
it("does not include raw tokens in query cache keys", async () => {
44+
saveStore({
45+
instances: [
46+
{ id: "i1", name: "Local", baseUrl: "http://localhost:8000", token: "super-secret-token" },
47+
],
48+
activeId: "i1",
49+
});
50+
httpFetch.mockResolvedValue(new Response("{}", { status: 200 }));
51+
const qc = new QueryClient({ defaultOptions: { queries: { retry: false } } });
52+
renderHook(() => useHealthStatus(), { wrapper: wrap(qc) });
53+
await waitFor(() => expect(httpFetch).toHaveBeenCalled());
54+
55+
const cacheKeys = qc
56+
.getQueryCache()
57+
.getAll()
58+
.map((query) => JSON.stringify(query.queryKey));
59+
expect(cacheKeys.join("\n")).not.toContain("super-secret-token");
60+
});
4261
});

‎packages/web/src/test/security.test.ts‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
import { describe, expect, it } from "vitest";
2+
import {
3+
isHttpOrHttpsUrl,
4+
isSafeExternalUrl,
5+
isSecureTokenTransport,
6+
tokenTransportError,
7+
} from "@/lib/security";
8+
9+
describe("security URL helpers", () => {
10+
it("only allows http and https URLs for external OS opens", () => {
11+
expect(isSafeExternalUrl("https://example.com/webhook")).toBe(true);
12+
expect(isSafeExternalUrl("http://localhost:3000/webhook")).toBe(true);
13+
expect(isSafeExternalUrl("file:///etc/passwd")).toBe(false);
14+
expect(isSafeExternalUrl("javascript:alert(1)")).toBe(false);
15+
expect(isSafeExternalUrl("openconcho://settings")).toBe(false);
16+
});
17+
18+
it("only accepts http and https webhook endpoints", () => {
19+
expect(isHttpOrHttpsUrl("https://hooks.example.com/a")).toBe(true);
20+
expect(isHttpOrHttpsUrl("http://hooks.example.com/a")).toBe(true);
21+
expect(isHttpOrHttpsUrl("ftp://hooks.example.com/a")).toBe(false);
22+
expect(isHttpOrHttpsUrl("notaurl")).toBe(false);
23+
});
24+
25+
it("requires HTTPS before sending tokens to non-loopback hosts", () => {
26+
expect(isSecureTokenTransport("https://honcho.example.com")).toBe(true);
27+
expect(isSecureTokenTransport("http://localhost:8000")).toBe(true);
28+
expect(isSecureTokenTransport("http://127.0.0.1:8000")).toBe(true);
29+
expect(isSecureTokenTransport("http://192.168.1.50:8000")).toBe(false);
30+
expect(isSecureTokenTransport("http://100.67.206.76:8000")).toBe(false);
31+
});
32+
33+
it("returns a user-facing error for insecure token transport", () => {
34+
expect(tokenTransportError("http://100.67.206.76:8000")).toMatch(/HTTPS/);
35+
expect(tokenTransportError("https://honcho.example.com")).toBeNull();
36+
});
37+
});

‎packages/web/src/test/settings-form.test.tsx‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,24 @@ describe("SettingsForm — self-hosted preset", () => {
5252
const store = loadStore();
5353
expect(store.instances).toHaveLength(1);
5454
});
55+
56+
it("blocks saving a token for a non-localhost HTTP endpoint", async () => {
57+
const user = userEvent.setup();
58+
renderForm(<SettingsForm instance={null} preset="self-hosted" />);
59+
const baseUrl = screen.getByPlaceholderText("http://localhost:8000");
60+
await user.clear(baseUrl);
61+
await user.type(baseUrl, "http://100.67.206.76:8000");
62+
await user.type(
63+
screen.getByPlaceholderText(/required only if your instance has auth enabled/i),
64+
"secret-token",
65+
);
66+
await user.click(screen.getByRole("button", { name: /Add Instance/i }));
67+
68+
expect(
69+
screen.getByText(/API tokens require HTTPS unless connecting to localhost/i),
70+
).toBeInTheDocument();
71+
expect(loadStore().instances).toHaveLength(0);
72+
});
5573
});
5674

5775
describe("SettingsForm — edit mode auto-detects cloud", () => {

0 commit comments

Comments
 (0)