Compare commits

..
Author SHA1 Message Date
ajnart acb58c60a3 style: auto-fix lint and format 2026-08-25 05:44:48 +00:00
ajnart 5f84ce590b fix(mcp): return valid string for void tool results
JSON.stringify(undefined) returns the value undefined, not a string,
so void-returning MCP tools produced content blocks with text: undefined
causing clients to reject the call with -32602 despite the action running.

Serialize to 'null' as a fallback so the text field is always a string.
2026-08-25 07:43:22 +02:00
Thomas a0149d5894 Merge pull request #6673 from homarr-labs/investigate-6672-immich-401 2026-08-22 19:44:03 +02:00
ajnart 8579fcbfc2 test(http,immich): address CodeRabbit review
- guarantee server teardown via try/finally in both wire-level tests
- add tuple-array HeadersInit coverage to the helper suite
2026-08-22 17:21:38 +02:00
ajnart b6f3aba23e test(http,immich): prove User-Agent fix at the wire level
Add two regression tests that fail on v1.76.0 and pass with the fix:
- wire-level test: a Headers instance (the shape @immich/sdk hands over)
  must reach the remote server with all headers intact
- real-SDK e2e: getServerStatistics through fetchWithTrustedCertificatesAsync
  against a live local server must deliver x-api-key
2026-08-22 17:16:31 +02:00
ajnart 31e218295d fix(http): preserve SDK headers when injecting User-Agent
The v1.76.0 User-Agent injection object-spread `fetchOptions.headers`, which
silently drops all headers when the value is a Headers instance. The Immich
SDK passes a Headers object with the merged x-api-key header, so requests
lost their API key and returned 401 (issue #6672). Build a real Headers
instance instead, keeping case-insensitive user-agent handling.
2026-08-22 17:12:43 +02:00
8 changed files with 228 additions and 47 deletions

No files matched your search

@@ -178,37 +178,35 @@ export const BoardSettingsForm = ({ board, permissions, hasFullAccess, hideVisib
const isPending = savePartialSettings.isPending || saveLayouts.isPending;
return (
<Stack gap="xl">
<form onSubmit={form.onSubmit((values) => void handleSubmitAsync(values))}>
<Stack gap="xl">
<GeneralSettingsContent board={board} form={form} />
<LayoutSettingsContent form={form} />
<BackgroundSettingsContent form={form} />
<ColorSettingsContent form={form} />
<CustomCssSettingsContent form={form} />
<BehaviorSettingsContent form={form} />
<form onSubmit={form.onSubmit((values) => void handleSubmitAsync(values))}>
<Stack gap="xl">
<GeneralSettingsContent board={board} form={form} />
<LayoutSettingsContent form={form} />
<BackgroundSettingsContent form={form} />
<ColorSettingsContent form={form} />
<CustomCssSettingsContent form={form} />
<BehaviorSettingsContent form={form} />
{form.isDirty() && (
<UnsavedChangesBar>
<Button type="button" disabled={isPending} variant="default" onClick={handleDiscard}>
{t("common.action.discard")}
</Button>
<Button loading={isPending} type="submit" disabled={!form.isValid()}>
{t("common.action.saveChanges")}
</Button>
</UnsavedChangesBar>
)}
</Stack>
</form>
{hasFullAccess && (
<SectionCard title={tSection("access.title")}>
<BoardAccessSettings board={board} initialPermissions={permissions} />
</SectionCard>
)}
{hasFullAccess && (
<SectionCard title={tSection("access.title")}>
<BoardAccessSettings board={board} initialPermissions={permissions} />
</SectionCard>
)}
{hasFullAccess && <DangerZoneSettingsContent hideVisibility={hideVisibility} />}
{hasFullAccess && <DangerZoneSettingsContent hideVisibility={hideVisibility} />}
</Stack>
{form.isDirty() && (
<UnsavedChangesBar>
<Button type="button" disabled={isPending} variant="default" onClick={handleDiscard}>
{t("common.action.discard")}
</Button>
<Button loading={isPending} type="submit" disabled={!form.isValid()}>
{t("common.action.saveChanges")}
</Button>
</UnsavedChangesBar>
)}
</Stack>
</form>
);
};
@@ -14,6 +14,7 @@ import { db } from "@homarr/db";
import { getPackageVersion } from "~/versions/package-reader";
import { extractMcpTools } from "../_extract-tools";
import { serializeMcpToolResult } from "../_serialize-result";
const logger = createLogger({ module: "mcpRoute" });
@@ -203,7 +204,7 @@ const mcpHandler = createMcpHandler(
const input = args && Object.keys(args).length > 0 ? args : undefined;
const result = await procedure(input);
return {
content: [{ type: "text" as const, text: JSON.stringify(result) }],
content: [{ type: "text" as const, text: serializeMcpToolResult(result) }],
};
} catch (error) {
const message = sanitizeErrorMessage(error);
@@ -0,0 +1,23 @@
import { describe, expect, it } from "vitest";
import { serializeMcpToolResult } from "./_serialize-result";
describe("serializeMcpToolResult", () => {
it("serializes objects as JSON strings", () => {
expect(serializeMcpToolResult({ boardId: "1" })).toBe('{"boardId":"1"}');
});
it("serializes primitive values", () => {
expect(serializeMcpToolResult("hello")).toBe('"hello"');
expect(serializeMcpToolResult(42)).toBe("42");
expect(serializeMcpToolResult(true)).toBe("true");
});
it("serializes null as the string null", () => {
expect(serializeMcpToolResult(null)).toBe("null");
});
it("serializes void results as the string null so MCP content text is always a string", () => {
expect(serializeMcpToolResult(undefined)).toBe("null");
});
});
@@ -0,0 +1,3 @@
export function serializeMcpToolResult(result: unknown): string {
return JSON.stringify(result) ?? "null";
}
@@ -5,4 +5,5 @@ export {
createCustomCheckServerIdentity,
createHttpsAgentAsync,
fetchWithTrustedCertificatesAsync,
mergeHeadersWithUserAgent,
} from "./request";
@@ -18,6 +18,14 @@ import packageJson from "../../../../../package.json";
export const getDefaultUserAgent = () => `Homarr/${packageJson.version} (+https://homarr.dev)`;
export const mergeHeadersWithUserAgent = (headers: unknown): Headers => {
const mergedHeaders = new Headers(headers as HeadersInit);
if (!mergedHeaders.has("user-agent")) {
mergedHeaders.set("user-agent", getDefaultUserAgent());
}
return mergedHeaders;
};
export const createCustomCheckServerIdentity = (
trustedHostnames: TrustedCertificateHostname[],
): typeof checkServerIdentity => {
@@ -77,30 +85,18 @@ export const fetchWithTrustedCertificatesAsync = async (
undefined,
options?.bodyTimeout !== undefined ? { bodyTimeout: options.bodyTimeout } : undefined,
));
const buildFetchOptions = (fetchOptions: Omit<RequestInit, "dispatcher" | "bodyTimeout" | "timeout">) => ({
...fetchOptions,
headers: mergeHeadersWithUserAgent(fetchOptions.headers) as unknown as RequestInit["headers"],
});
if (options?.timeout) {
const { bodyTimeout: _bodyTimeout, dispatcher: _dispatcher, ...fetchOptions } = options;
return await withTimeoutAsync(
async (signal) =>
fetch(url, {
...fetchOptions,
headers: {
"User-Agent": getDefaultUserAgent(),
...fetchOptions.headers,
},
signal,
dispatcher: agent,
}),
async (signal) => fetch(url, { ...buildFetchOptions(fetchOptions), signal, dispatcher: agent }),
options.timeout,
);
}
const { bodyTimeout: _bodyTimeout, dispatcher: _dispatcher, ...fetchOptions } = options ?? {};
return fetch(url, {
...fetchOptions,
headers: {
"User-Agent": getDefaultUserAgent(),
...fetchOptions.headers,
},
dispatcher: agent,
});
return fetch(url, { ...buildFetchOptions(fetchOptions), dispatcher: agent });
};
@@ -0,0 +1,120 @@
// @vitest-environment node
import { createServer } from "node:http";
import type { Server } from "node:http";
import type { AddressInfo } from "node:net";
import type { RequestInit } from "undici";
import { describe, expect, test, vi } from "vitest";
import { fetchWithTrustedCertificatesAsync, mergeHeadersWithUserAgent } from "@homarr/core/infrastructure/http";
vi.mock("@homarr/core/infrastructure/certificates", () => ({
getAllTrustedCertificatesAsync: vi.fn(async () => []),
getTrustedCertificateHostnamesAsync: vi.fn(async () => []),
}));
describe("mergeHeadersWithUserAgent", () => {
test("should keep existing headers when given a Headers instance", () => {
// Arrange
const headers = new Headers({ "x-api-key": "some-api-key", Accept: "application/json" });
// Act
const merged = mergeHeadersWithUserAgent(headers);
// Assert
expect(merged.get("x-api-key")).toBe("some-api-key");
expect(merged.get("accept")).toBe("application/json");
expect(merged.get("user-agent")).toContain("Homarr/");
});
test("should keep existing headers when given a record", () => {
// Arrange
const headers = { "x-api-key": "some-api-key", Accept: "application/json" };
// Act
const merged = mergeHeadersWithUserAgent(headers);
// Assert
expect(merged.get("x-api-key")).toBe("some-api-key");
expect(merged.get("accept")).toBe("application/json");
expect(merged.get("user-agent")).toContain("Homarr/");
});
test("should keep existing headers when given a tuple array", () => {
// Arrange
const headers: [string, string][] = [
["x-api-key", "some-api-key"],
["Accept", "application/json"],
];
// Act
const merged = mergeHeadersWithUserAgent(headers);
// Assert
expect(merged.get("x-api-key")).toBe("some-api-key");
expect(merged.get("accept")).toBe("application/json");
expect(merged.get("user-agent")).toContain("Homarr/");
});
test("should not override an existing user-agent in any casing", () => {
// Arrange
const headers = new Headers({ "USER-AGENT": "custom-agent" });
// Act
const merged = mergeHeadersWithUserAgent(headers);
// Assert
expect(merged.get("user-agent")).toBe("custom-agent");
});
test("should fall back to the default user agent when no headers are provided", () => {
// Act
const merged = mergeHeadersWithUserAgent(undefined);
// Assert
expect(merged.get("user-agent")).toContain("Homarr/");
});
});
const startCaptureServerAsync = async (): Promise<{
server: Server;
getReceivedHeaders: () => Record<string, string | string[] | undefined>;
port: number;
}> => {
let receivedHeaders: Record<string, string | string[] | undefined> = {};
const server = createServer((req, res) => {
receivedHeaders = req.headers;
res.writeHead(200, { "Content-Type": "application/json" });
res.end("{}");
});
await new Promise<void>((resolve) => server.listen(0, resolve));
const port = (server.address() as AddressInfo).port;
return {
server,
port,
getReceivedHeaders: () => receivedHeaders,
};
};
describe("fetchWithTrustedCertificatesAsync", () => {
test("should deliver a Headers instance with all headers to the remote server", async () => {
// Arrange - the @immich/sdk hands over a Headers instance after merging its defaults
const { server, port, getReceivedHeaders } = await startCaptureServerAsync();
const headers = new Headers({ "x-api-key": "immich-api-key", Accept: "application/json" });
// Act
try {
const response = await fetchWithTrustedCertificatesAsync(`http://127.0.0.1:${port}/api/server/statistics`, {
headers: headers as unknown as RequestInit["headers"],
});
// Assert
expect(response.status).toBe(200);
expect(getReceivedHeaders()["x-api-key"]).toBe("immich-api-key");
expect(getReceivedHeaders()["user-agent"]).toContain("Homarr/");
} finally {
server.close();
}
});
});
@@ -0,0 +1,39 @@
// @vitest-environment node
import { createServer } from "node:http";
import type { AddressInfo } from "node:net";
import { getServerStatistics, init } from "@immich/sdk";
import { describe, expect, test, vi } from "vitest";
import { fetchWithTrustedCertificatesAsync } from "@homarr/core/infrastructure/http";
vi.mock("@homarr/core/infrastructure/certificates", () => ({
getAllTrustedCertificatesAsync: vi.fn(async () => []),
getTrustedCertificateHostnamesAsync: vi.fn(async () => []),
}));
describe("real @immich/sdk end-to-end through fetchWithTrustedCertificatesAsync", () => {
test("x-api-key set via init() reaches the server", async () => {
let receivedXApiKey: string | string[] | undefined;
const server = createServer((req, res) => {
receivedXApiKey = req.headers["x-api-key"];
res.writeHead(200, { "Content-Type": "application/json" });
res.end(JSON.stringify({ usage: 1000, photos: 5, videos: 2 }));
});
await new Promise<void>((resolve) => server.listen(0, resolve));
const port = (server.address() as AddressInfo).port;
init({ baseUrl: `http://127.0.0.1:${port}/api`, apiKey: "sdk-configured-key" });
try {
const stats = await getServerStatistics({
fetch: fetchWithTrustedCertificatesAsync as unknown as typeof fetch,
});
expect(receivedXApiKey).toBe("sdk-configured-key");
expect(stats).toMatchObject({ usage: 1000, photos: 5, videos: 2 });
} finally {
server.close();
}
});
});