mirror of
https://github.com/openclaw/openclaw.git
synced 2026-03-12 07:20:45 +00:00
Matrix: harden media and credential handling
This commit is contained in:
@@ -363,6 +363,7 @@ export async function resolveMatrixAuth(params?: {
|
||||
credentialsMatchConfig(cached, {
|
||||
homeserver: resolved.homeserver,
|
||||
userId: resolved.userId || "",
|
||||
accessToken: resolved.accessToken,
|
||||
})
|
||||
? cached
|
||||
: null;
|
||||
|
||||
@@ -4,6 +4,7 @@ import path from "node:path";
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import { setMatrixRuntime } from "../runtime.js";
|
||||
import {
|
||||
credentialsMatchConfig,
|
||||
loadMatrixCredentials,
|
||||
clearMatrixCredentials,
|
||||
resolveMatrixCredentialsPath,
|
||||
@@ -116,4 +117,38 @@ describe("matrix credentials storage", () => {
|
||||
expect(fs.existsSync(currentPath)).toBe(false);
|
||||
expect(fs.existsSync(legacyPath)).toBe(false);
|
||||
});
|
||||
|
||||
it("requires a token match when userId is absent", () => {
|
||||
expect(
|
||||
credentialsMatchConfig(
|
||||
{
|
||||
homeserver: "https://matrix.example.org",
|
||||
userId: "@old:example.org",
|
||||
accessToken: "tok-old",
|
||||
createdAt: "2026-01-01T00:00:00.000Z",
|
||||
},
|
||||
{
|
||||
homeserver: "https://matrix.example.org",
|
||||
userId: "",
|
||||
accessToken: "tok-new",
|
||||
},
|
||||
),
|
||||
).toBe(false);
|
||||
|
||||
expect(
|
||||
credentialsMatchConfig(
|
||||
{
|
||||
homeserver: "https://matrix.example.org",
|
||||
userId: "@bot:example.org",
|
||||
accessToken: "tok-123",
|
||||
createdAt: "2026-01-01T00:00:00.000Z",
|
||||
},
|
||||
{
|
||||
homeserver: "https://matrix.example.org",
|
||||
userId: "",
|
||||
accessToken: "tok-123",
|
||||
},
|
||||
),
|
||||
).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -157,11 +157,13 @@ export function clearMatrixCredentials(
|
||||
|
||||
export function credentialsMatchConfig(
|
||||
stored: MatrixStoredCredentials,
|
||||
config: { homeserver: string; userId: string },
|
||||
config: { homeserver: string; userId: string; accessToken?: string },
|
||||
): boolean {
|
||||
// If userId is empty (token-based auth), only match homeserver
|
||||
if (!config.userId) {
|
||||
return stored.homeserver === config.homeserver;
|
||||
if (!config.accessToken) {
|
||||
return false;
|
||||
}
|
||||
return stored.homeserver === config.homeserver && stored.accessToken === config.accessToken;
|
||||
}
|
||||
return stored.homeserver === config.homeserver && stored.userId === config.userId;
|
||||
}
|
||||
|
||||
@@ -52,8 +52,10 @@ describe("downloadMatrixMedia", () => {
|
||||
file,
|
||||
});
|
||||
|
||||
// decryptMedia should be called with just the file object (it handles download internally)
|
||||
expect(decryptMedia).toHaveBeenCalledWith(file);
|
||||
expect(decryptMedia).toHaveBeenCalledWith(file, {
|
||||
maxBytes: 1024,
|
||||
readIdleTimeoutMs: 30_000,
|
||||
});
|
||||
expect(saveMediaBuffer).toHaveBeenCalledWith(
|
||||
Buffer.from("decrypted"),
|
||||
"image/png",
|
||||
@@ -99,4 +101,24 @@ describe("downloadMatrixMedia", () => {
|
||||
expect(decryptMedia).not.toHaveBeenCalled();
|
||||
expect(saveMediaBuffer).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("passes byte limits through plain media downloads", async () => {
|
||||
const downloadContent = vi.fn().mockResolvedValue(Buffer.from("plain"));
|
||||
|
||||
const client = {
|
||||
downloadContent,
|
||||
} as unknown as import("../sdk.js").MatrixClient;
|
||||
|
||||
await downloadMatrixMedia({
|
||||
client,
|
||||
mxcUrl: "mxc://example/file",
|
||||
contentType: "image/png",
|
||||
maxBytes: 4096,
|
||||
});
|
||||
|
||||
expect(downloadContent).toHaveBeenCalledWith("mxc://example/file", {
|
||||
maxBytes: 4096,
|
||||
readIdleTimeoutMs: 30_000,
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -16,25 +16,18 @@ type EncryptedFile = {
|
||||
v: string;
|
||||
};
|
||||
|
||||
const MATRIX_MEDIA_DOWNLOAD_IDLE_TIMEOUT_MS = 30_000;
|
||||
|
||||
async function fetchMatrixMediaBuffer(params: {
|
||||
client: MatrixClient;
|
||||
mxcUrl: string;
|
||||
maxBytes: number;
|
||||
}): Promise<{ buffer: Buffer } | null> {
|
||||
// The client wrapper exposes mxcToHttp for Matrix media URIs.
|
||||
const url = params.client.mxcToHttp(params.mxcUrl);
|
||||
if (!url) {
|
||||
return null;
|
||||
}
|
||||
|
||||
// Use the client's download method which handles auth
|
||||
try {
|
||||
const raw = await params.client.downloadContent(params.mxcUrl);
|
||||
const buffer = Buffer.isBuffer(raw) ? raw : Buffer.from(raw);
|
||||
|
||||
if (buffer.byteLength > params.maxBytes) {
|
||||
throw new Error("Matrix media exceeds configured size limit");
|
||||
}
|
||||
const buffer = await params.client.downloadContent(params.mxcUrl, {
|
||||
maxBytes: params.maxBytes,
|
||||
readIdleTimeoutMs: MATRIX_MEDIA_DOWNLOAD_IDLE_TIMEOUT_MS,
|
||||
});
|
||||
return { buffer };
|
||||
} catch (err) {
|
||||
throw new Error(`Matrix media download failed: ${String(err)}`, { cause: err });
|
||||
@@ -54,9 +47,12 @@ async function fetchEncryptedMediaBuffer(params: {
|
||||
throw new Error("Cannot decrypt media: crypto not enabled");
|
||||
}
|
||||
|
||||
// decryptMedia handles downloading and decrypting the encrypted content internally
|
||||
const decrypted = await params.client.crypto.decryptMedia(
|
||||
params.file as Parameters<typeof params.client.crypto.decryptMedia>[0],
|
||||
{
|
||||
maxBytes: params.maxBytes,
|
||||
readIdleTimeoutMs: MATRIX_MEDIA_DOWNLOAD_IDLE_TIMEOUT_MS,
|
||||
},
|
||||
);
|
||||
|
||||
if (decrypted.byteLength > params.maxBytes) {
|
||||
|
||||
@@ -570,7 +570,14 @@ export class MatrixClient {
|
||||
return this.client.mxcUrlToHttp(mxcUrl, undefined, undefined, undefined, true, false, true);
|
||||
}
|
||||
|
||||
async downloadContent(mxcUrl: string, allowRemote = true): Promise<Buffer> {
|
||||
async downloadContent(
|
||||
mxcUrl: string,
|
||||
opts: {
|
||||
allowRemote?: boolean;
|
||||
maxBytes?: number;
|
||||
readIdleTimeoutMs?: number;
|
||||
} = {},
|
||||
): Promise<Buffer> {
|
||||
const parsed = parseMxc(mxcUrl);
|
||||
if (!parsed) {
|
||||
throw new Error(`Invalid Matrix content URI: ${mxcUrl}`);
|
||||
@@ -579,8 +586,10 @@ export class MatrixClient {
|
||||
const response = await this.httpClient.requestRaw({
|
||||
method: "GET",
|
||||
endpoint,
|
||||
qs: { allow_remote: allowRemote },
|
||||
qs: { allow_remote: opts.allowRemote ?? true },
|
||||
timeoutMs: this.localTimeoutMs,
|
||||
maxBytes: opts.maxBytes,
|
||||
readIdleTimeoutMs: opts.readIdleTimeoutMs,
|
||||
});
|
||||
return response;
|
||||
}
|
||||
|
||||
@@ -25,7 +25,10 @@ export type MatrixCryptoFacade = {
|
||||
isRoomEncrypted: (roomId: string) => Promise<boolean>;
|
||||
requestOwnUserVerification: () => Promise<unknown | null>;
|
||||
encryptMedia: (buffer: Buffer) => Promise<{ buffer: Buffer; file: Omit<EncryptedFile, "url"> }>;
|
||||
decryptMedia: (file: EncryptedFile) => Promise<Buffer>;
|
||||
decryptMedia: (
|
||||
file: EncryptedFile,
|
||||
opts?: { maxBytes?: number; readIdleTimeoutMs?: number },
|
||||
) => Promise<Buffer>;
|
||||
getRecoveryKey: () => Promise<{
|
||||
encodedPrivateKey?: string;
|
||||
keyId?: string | null;
|
||||
@@ -66,7 +69,10 @@ export function createMatrixCryptoFacade(deps: {
|
||||
eventType: string,
|
||||
stateKey?: string,
|
||||
) => Promise<Record<string, unknown>>;
|
||||
downloadContent: (mxcUrl: string) => Promise<Buffer>;
|
||||
downloadContent: (
|
||||
mxcUrl: string,
|
||||
opts?: { maxBytes?: number; readIdleTimeoutMs?: number },
|
||||
) => Promise<Buffer>;
|
||||
}): MatrixCryptoFacade {
|
||||
return {
|
||||
prepare: async (_joinedRooms: string[]) => {
|
||||
@@ -116,8 +122,11 @@ export function createMatrixCryptoFacade(deps: {
|
||||
},
|
||||
};
|
||||
},
|
||||
decryptMedia: async (file: EncryptedFile): Promise<Buffer> => {
|
||||
const encrypted = await deps.downloadContent(file.url);
|
||||
decryptMedia: async (
|
||||
file: EncryptedFile,
|
||||
opts?: { maxBytes?: number; readIdleTimeoutMs?: number },
|
||||
): Promise<Buffer> => {
|
||||
const encrypted = await deps.downloadContent(file.url, opts);
|
||||
const metadata: EncryptedFile = {
|
||||
url: file.url,
|
||||
key: file.key,
|
||||
|
||||
@@ -43,6 +43,8 @@ export class MatrixAuthedHttpClient {
|
||||
endpoint: string;
|
||||
qs?: QueryParams;
|
||||
timeoutMs: number;
|
||||
maxBytes?: number;
|
||||
readIdleTimeoutMs?: number;
|
||||
allowAbsoluteEndpoint?: boolean;
|
||||
}): Promise<Buffer> {
|
||||
const { response, buffer } = await performMatrixRequest({
|
||||
@@ -53,6 +55,8 @@ export class MatrixAuthedHttpClient {
|
||||
qs: params.qs,
|
||||
timeoutMs: params.timeoutMs,
|
||||
raw: true,
|
||||
maxBytes: params.maxBytes,
|
||||
readIdleTimeoutMs: params.readIdleTimeoutMs,
|
||||
allowAbsoluteEndpoint: params.allowAbsoluteEndpoint,
|
||||
});
|
||||
if (!response.ok) {
|
||||
|
||||
95
extensions/matrix/src/matrix/sdk/read-response-with-limit.ts
Normal file
95
extensions/matrix/src/matrix/sdk/read-response-with-limit.ts
Normal file
@@ -0,0 +1,95 @@
|
||||
async function readChunkWithIdleTimeout(
|
||||
reader: ReadableStreamDefaultReader<Uint8Array>,
|
||||
chunkTimeoutMs: number,
|
||||
): Promise<Awaited<ReturnType<typeof reader.read>>> {
|
||||
let timeoutId: ReturnType<typeof setTimeout> | undefined;
|
||||
let timedOut = false;
|
||||
|
||||
return await new Promise((resolve, reject) => {
|
||||
const clear = () => {
|
||||
if (timeoutId !== undefined) {
|
||||
clearTimeout(timeoutId);
|
||||
timeoutId = undefined;
|
||||
}
|
||||
};
|
||||
|
||||
timeoutId = setTimeout(() => {
|
||||
timedOut = true;
|
||||
clear();
|
||||
void reader.cancel().catch(() => undefined);
|
||||
reject(new Error(`Matrix media download stalled: no data received for ${chunkTimeoutMs}ms`));
|
||||
}, chunkTimeoutMs);
|
||||
|
||||
void reader.read().then(
|
||||
(result) => {
|
||||
clear();
|
||||
if (!timedOut) {
|
||||
resolve(result);
|
||||
}
|
||||
},
|
||||
(err) => {
|
||||
clear();
|
||||
if (!timedOut) {
|
||||
reject(err);
|
||||
}
|
||||
},
|
||||
);
|
||||
});
|
||||
}
|
||||
|
||||
export async function readResponseWithLimit(
|
||||
res: Response,
|
||||
maxBytes: number,
|
||||
opts?: {
|
||||
onOverflow?: (params: { size: number; maxBytes: number; res: Response }) => Error;
|
||||
chunkTimeoutMs?: number;
|
||||
},
|
||||
): Promise<Buffer> {
|
||||
const onOverflow =
|
||||
opts?.onOverflow ??
|
||||
((params: { size: number; maxBytes: number }) =>
|
||||
new Error(`Content too large: ${params.size} bytes (limit: ${params.maxBytes} bytes)`));
|
||||
const chunkTimeoutMs = opts?.chunkTimeoutMs;
|
||||
|
||||
const body = res.body;
|
||||
if (!body || typeof body.getReader !== "function") {
|
||||
const fallback = Buffer.from(await res.arrayBuffer());
|
||||
if (fallback.length > maxBytes) {
|
||||
throw onOverflow({ size: fallback.length, maxBytes, res });
|
||||
}
|
||||
return fallback;
|
||||
}
|
||||
|
||||
const reader = body.getReader();
|
||||
const chunks: Uint8Array[] = [];
|
||||
let total = 0;
|
||||
try {
|
||||
while (true) {
|
||||
const { done, value } = chunkTimeoutMs
|
||||
? await readChunkWithIdleTimeout(reader, chunkTimeoutMs)
|
||||
: await reader.read();
|
||||
if (done) {
|
||||
break;
|
||||
}
|
||||
if (value?.length) {
|
||||
total += value.length;
|
||||
if (total > maxBytes) {
|
||||
try {
|
||||
await reader.cancel();
|
||||
} catch {}
|
||||
throw onOverflow({ size: total, maxBytes, res });
|
||||
}
|
||||
chunks.push(value);
|
||||
}
|
||||
}
|
||||
} finally {
|
||||
try {
|
||||
reader.releaseLock();
|
||||
} catch {}
|
||||
}
|
||||
|
||||
return Buffer.concat(
|
||||
chunks.map((chunk) => Buffer.from(chunk)),
|
||||
total,
|
||||
);
|
||||
}
|
||||
67
extensions/matrix/src/matrix/sdk/transport.test.ts
Normal file
67
extensions/matrix/src/matrix/sdk/transport.test.ts
Normal file
@@ -0,0 +1,67 @@
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { performMatrixRequest } from "./transport.js";
|
||||
|
||||
describe("performMatrixRequest", () => {
|
||||
beforeEach(() => {
|
||||
vi.unstubAllGlobals();
|
||||
});
|
||||
|
||||
it("rejects oversized raw responses before buffering the whole body", async () => {
|
||||
vi.stubGlobal(
|
||||
"fetch",
|
||||
vi.fn(
|
||||
async () =>
|
||||
new Response("too-big", {
|
||||
status: 200,
|
||||
headers: {
|
||||
"content-length": "8192",
|
||||
},
|
||||
}),
|
||||
),
|
||||
);
|
||||
|
||||
await expect(
|
||||
performMatrixRequest({
|
||||
homeserver: "https://matrix.example.org",
|
||||
accessToken: "token",
|
||||
method: "GET",
|
||||
endpoint: "/_matrix/media/v3/download/example/id",
|
||||
timeoutMs: 5000,
|
||||
raw: true,
|
||||
maxBytes: 1024,
|
||||
}),
|
||||
).rejects.toThrow("Matrix media exceeds configured size limit");
|
||||
});
|
||||
|
||||
it("applies streaming byte limits when raw responses omit content-length", async () => {
|
||||
const chunk = new Uint8Array(768);
|
||||
const stream = new ReadableStream<Uint8Array>({
|
||||
start(controller) {
|
||||
controller.enqueue(chunk);
|
||||
controller.enqueue(chunk);
|
||||
controller.close();
|
||||
},
|
||||
});
|
||||
vi.stubGlobal(
|
||||
"fetch",
|
||||
vi.fn(
|
||||
async () =>
|
||||
new Response(stream, {
|
||||
status: 200,
|
||||
}),
|
||||
),
|
||||
);
|
||||
|
||||
await expect(
|
||||
performMatrixRequest({
|
||||
homeserver: "https://matrix.example.org",
|
||||
accessToken: "token",
|
||||
method: "GET",
|
||||
endpoint: "/_matrix/media/v3/download/example/id",
|
||||
timeoutMs: 5000,
|
||||
raw: true,
|
||||
maxBytes: 1024,
|
||||
}),
|
||||
).rejects.toThrow("Matrix media exceeds configured size limit");
|
||||
});
|
||||
});
|
||||
@@ -1,3 +1,5 @@
|
||||
import { readResponseWithLimit } from "./read-response-with-limit.js";
|
||||
|
||||
export type HttpMethod = "GET" | "POST" | "PUT" | "DELETE";
|
||||
|
||||
type QueryValue =
|
||||
@@ -107,6 +109,8 @@ export async function performMatrixRequest(params: {
|
||||
body?: unknown;
|
||||
timeoutMs: number;
|
||||
raw?: boolean;
|
||||
maxBytes?: number;
|
||||
readIdleTimeoutMs?: number;
|
||||
allowAbsoluteEndpoint?: boolean;
|
||||
}): Promise<{ response: Response; text: string; buffer: Buffer }> {
|
||||
const isAbsoluteEndpoint =
|
||||
@@ -152,7 +156,24 @@ export async function performMatrixRequest(params: {
|
||||
signal: controller.signal,
|
||||
});
|
||||
if (params.raw) {
|
||||
const bytes = Buffer.from(await response.arrayBuffer());
|
||||
const contentLength = response.headers.get("content-length");
|
||||
if (params.maxBytes && contentLength) {
|
||||
const length = Number(contentLength);
|
||||
if (Number.isFinite(length) && length > params.maxBytes) {
|
||||
throw new Error(
|
||||
`Matrix media exceeds configured size limit (${length} bytes > ${params.maxBytes} bytes)`,
|
||||
);
|
||||
}
|
||||
}
|
||||
const bytes = params.maxBytes
|
||||
? await readResponseWithLimit(response, params.maxBytes, {
|
||||
onOverflow: ({ maxBytes, size }) =>
|
||||
new Error(
|
||||
`Matrix media exceeds configured size limit (${size} bytes > ${maxBytes} bytes)`,
|
||||
),
|
||||
chunkTimeoutMs: params.readIdleTimeoutMs,
|
||||
})
|
||||
: Buffer.from(await response.arrayBuffer());
|
||||
return {
|
||||
response,
|
||||
text: bytes.toString("utf8"),
|
||||
|
||||
Reference in New Issue
Block a user