diff --git a/docs/install/migrating-matrix.md b/docs/install/migrating-matrix.md index 03c154c4367..27e1dc98c67 100644 --- a/docs/install/migrating-matrix.md +++ b/docs/install/migrating-matrix.md @@ -304,7 +304,7 @@ new backup key can load correctly after restart. - Meaning: the provided key could not be parsed or did not match the expected format. - What to do: retry with the exact recovery key from your Matrix client or recovery-key file. -`Matrix recovery key was applied, but this device still lacks full Matrix identity trust. ...` +`Matrix recovery key was applied, but this device still lacks full Matrix identity trust.` - Meaning: OpenClaw could apply the recovery key, but Matrix still has not established full cross-signing identity trust for this device. Check the diff --git a/extensions/matrix/src/cli.test.ts b/extensions/matrix/src/cli.test.ts index ac99b0b5914..5c79f519d3b 100644 --- a/extensions/matrix/src/cli.test.ts +++ b/extensions/matrix/src/cli.test.ts @@ -415,6 +415,40 @@ describe("matrix CLI verification commands", () => { expect(consoleLogMock).toHaveBeenCalledWith("Initiated by OpenClaw: no"); }); + it("sanitizes remote Matrix verification metadata before printing it", async () => { + listMatrixVerificationsMock.mockResolvedValue([ + mockMatrixVerificationSummary({ + id: "self-\u001B[31m1", + transactionId: "txn-\n1", + otherUserId: "@bot\u001B[2J:example.org", + otherDeviceId: "PHONE\r123", + phaseName: "started\u001B[0m", + methods: ["m.sas.v1\nspoof"], + chosenMethod: "m.sas.v1\u001B[1m", + sas: { + emoji: [ + ["🐶", "Dog\u001B[31m"], + ["🐱", "Cat\nspoof"], + ], + }, + error: "Remote\u001B[31m cancelled\nforged", + }), + ]); + const program = buildProgram(); + + await program.parseAsync(["matrix", "verify", "list"], { from: "user" }); + + expect(consoleLogMock).toHaveBeenCalledWith("Verification id: self-1"); + expect(consoleLogMock).toHaveBeenCalledWith("Transaction id: txn-1"); + expect(consoleLogMock).toHaveBeenCalledWith("Other user: @bot:example.org"); + expect(consoleLogMock).toHaveBeenCalledWith("Other device: PHONE123"); + expect(consoleLogMock).toHaveBeenCalledWith("Phase: started"); + expect(consoleLogMock).toHaveBeenCalledWith("Methods: m.sas.v1spoof"); + expect(consoleLogMock).toHaveBeenCalledWith("Chosen method: m.sas.v1"); + expect(consoleLogMock).toHaveBeenCalledWith("SAS emoji: 🐶 Dog | 🐱 Catspoof"); + expect(consoleLogMock).toHaveBeenCalledWith("Verification error: Remote cancelledforged"); + }); + it("shows Matrix SAS diagnostics and confirm/mismatch guidance", async () => { getMatrixVerificationSasMock.mockResolvedValue({ decimal: [1234, 5678, 9012], diff --git a/extensions/matrix/src/cli.ts b/extensions/matrix/src/cli.ts index 980bf8e1ccd..31ac8371636 100644 --- a/extensions/matrix/src/cli.ts +++ b/extensions/matrix/src/cli.ts @@ -645,31 +645,91 @@ function printVerificationTrustDiagnostics(status: { console.log(`Signed by owner: ${status.signedByOwner ? "yes" : "no"}`); } -function printMatrixVerificationSummary(summary: MatrixCliVerificationSummary): void { - console.log(`Verification id: ${summary.id}`); - if (summary.transactionId) { - console.log(`Transaction id: ${summary.transactionId}`); +function sanitizeMatrixCliText(value: string): string { + let withoutAnsi = ""; + for (let index = 0; index < value.length; index++) { + const code = value.charCodeAt(index); + if (code !== 0x1b) { + withoutAnsi += value[index]; + continue; + } + + const marker = value[index + 1]; + if (marker === "[") { + index += 2; + while (index < value.length && !isAnsiFinalByte(value.charCodeAt(index))) { + index++; + } + continue; + } + if (marker === "]") { + index += 2; + while (index < value.length) { + const current = value.charCodeAt(index); + if (current === 0x07) { + break; + } + if (current === 0x1b && value[index + 1] === "\\") { + index++; + break; + } + index++; + } + continue; + } + index++; } - console.log(`Other user: ${summary.otherUserId}`); - console.log(`Other device: ${summary.otherDeviceId ?? "unknown"}`); + + let sanitized = ""; + for (const character of withoutAnsi) { + const code = character.charCodeAt(0); + if ((code >= 0x20 && code !== 0x7f) || code > 0x7f) { + sanitized += character; + } + } + return sanitized; +} + +function isAnsiFinalByte(code: number): boolean { + return code >= 0x40 && code <= 0x7e; +} + +function formatMatrixCliSasEmoji(emoji: NonNullable): string { + return emoji + .map( + ([emojiValue, label]) => + `${sanitizeMatrixCliText(emojiValue)} ${sanitizeMatrixCliText(label)}`, + ) + .join(" | "); +} + +function printMatrixVerificationSummary(summary: MatrixCliVerificationSummary): void { + console.log(`Verification id: ${sanitizeMatrixCliText(summary.id)}`); + if (summary.transactionId) { + console.log(`Transaction id: ${sanitizeMatrixCliText(summary.transactionId)}`); + } + console.log(`Other user: ${sanitizeMatrixCliText(summary.otherUserId)}`); + console.log(`Other device: ${sanitizeMatrixCliText(summary.otherDeviceId ?? "unknown")}`); console.log(`Self-verification: ${summary.isSelfVerification ? "yes" : "no"}`); console.log(`Initiated by OpenClaw: ${summary.initiatedByMe ? "yes" : "no"}`); - console.log(`Phase: ${summary.phaseName}`); + console.log(`Phase: ${sanitizeMatrixCliText(summary.phaseName)}`); console.log(`Pending: ${summary.pending ? "yes" : "no"}`); console.log(`Completed: ${summary.completed ? "yes" : "no"}`); - console.log(`Methods: ${summary.methods.length ? summary.methods.join(", ") : "none"}`); + console.log( + `Methods: ${ + summary.methods.length ? summary.methods.map(sanitizeMatrixCliText).join(", ") : "none" + }`, + ); if (summary.chosenMethod) { - console.log(`Chosen method: ${summary.chosenMethod}`); + console.log(`Chosen method: ${sanitizeMatrixCliText(summary.chosenMethod)}`); } if (summary.hasSas && summary.sas?.emoji?.length) { - console.log( - `SAS emoji: ${summary.sas.emoji.map(([emoji, label]) => `${emoji} ${label}`).join(" | ")}`, - ); + console.log(`SAS emoji: ${formatMatrixCliSasEmoji(summary.sas.emoji)}`); } else if (summary.hasSas && summary.sas?.decimal) { console.log(`SAS decimals: ${summary.sas.decimal.join(" ")}`); } if (summary.error) { - console.log(`Verification error: ${summary.error}`); + console.log(`Verification error: ${sanitizeMatrixCliText(summary.error)}`); } } @@ -688,7 +748,7 @@ function printMatrixVerificationSummaries(summaries: MatrixCliVerificationSummar function printMatrixVerificationSas(sas: MatrixCliVerificationSas): void { if (sas.emoji?.length) { - console.log(`SAS emoji: ${sas.emoji.map(([emoji, label]) => `${emoji} ${label}`).join(" | ")}`); + console.log(`SAS emoji: ${formatMatrixCliSasEmoji(sas.emoji)}`); } else if (sas.decimal) { console.log(`SAS decimals: ${sas.decimal.join(" ")}`); } else { diff --git a/extensions/matrix/src/matrix/actions/verification.test.ts b/extensions/matrix/src/matrix/actions/verification.test.ts index 2e975b4356a..a3d6ba1f597 100644 --- a/extensions/matrix/src/matrix/actions/verification.test.ts +++ b/extensions/matrix/src/matrix/actions/verification.test.ts @@ -368,6 +368,58 @@ describe("matrix verification actions", () => { expect(getOwnDeviceVerificationStatus).toHaveBeenCalledTimes(2); }); + it("waits for SAS data without restarting an already-started self-verification", async () => { + const requested = { + completed: false, + hasSas: false, + id: "verification-1", + phaseName: "requested", + transactionId: "tx-self", + }; + const started = { + ...requested, + phaseName: "started", + }; + const sas = { + ...started, + hasSas: true, + sas: { + decimal: [1, 2, 3], + }, + }; + const completed = { + ...sas, + completed: true, + phaseName: "done", + }; + const crypto = { + confirmVerificationSas: vi.fn(async () => completed), + listVerifications: vi.fn().mockResolvedValueOnce([started]).mockResolvedValueOnce([sas]), + requestVerification: vi.fn(async () => requested), + startVerification: vi.fn(), + }; + const bootstrapOwnDeviceVerification = vi.fn(async () => ({ + success: true, + verification: mockVerifiedOwnerStatus(), + })); + withStartedActionClientMock.mockImplementation(async (_opts, run) => { + return await run({ + bootstrapOwnDeviceVerification, + crypto, + getOwnDeviceVerificationStatus: vi.fn(async () => mockVerifiedOwnerStatus()), + }); + }); + + await expect( + runMatrixSelfVerification({ confirmSas: vi.fn(async () => true), timeoutMs: 500 }), + ).resolves.toMatchObject({ + completed: true, + deviceOwnerVerified: true, + }); + + expect(crypto.startVerification).not.toHaveBeenCalled(); + }); + it("fails self-verification if SAS completes but full identity trust cannot be established", async () => { const requested = { completed: false, diff --git a/extensions/matrix/src/matrix/actions/verification.ts b/extensions/matrix/src/matrix/actions/verification.ts index eaff46c67d5..7b05d9a13dc 100644 --- a/extensions/matrix/src/matrix/actions/verification.ts +++ b/extensions/matrix/src/matrix/actions/verification.ts @@ -61,6 +61,10 @@ function isMatrixVerificationReadyForSas(summary: MatrixVerificationSummary): bo ); } +function shouldStartMatrixSasVerification(summary: MatrixVerificationSummary): boolean { + return !summary.hasSas && summary.phaseName !== "started" && !summary.completed; +} + async function waitForMatrixVerificationSummary(params: { crypto: MatrixCryptoActionFacade; label: string; @@ -195,7 +199,9 @@ export async function runMatrixSelfVerification( } await params.onReady?.(ready); - const started = ready.hasSas ? ready : await crypto.startVerification(ready.id, "sas"); + const started = shouldStartMatrixSasVerification(ready) + ? await crypto.startVerification(ready.id, "sas") + : ready; let sasSummary = started; if (!sasSummary.hasSas) { sasSummary = await waitForMatrixVerificationSummary({ diff --git a/extensions/matrix/src/matrix/client/logging.test.ts b/extensions/matrix/src/matrix/client/logging.test.ts index 7066c2f9cfc..8ebe1950f5d 100644 --- a/extensions/matrix/src/matrix/client/logging.test.ts +++ b/extensions/matrix/src/matrix/client/logging.test.ts @@ -13,6 +13,8 @@ describe("Matrix SDK logging", () => { setMatrixSdkLogMode("default"); setMatrixSdkConsoleLogging(false); vi.restoreAllMocks(); + delete process.env.MATRIX_SDK_DEBUG; + delete process.env.OPENCLAW_MATRIX_SDK_DEBUG; }); it("suppresses Matrix SDK client logs in quiet mode", () => { @@ -27,4 +29,16 @@ describe("Matrix SDK logging", () => { expect(info).not.toHaveBeenCalled(); }); + + it("does not force Matrix JS SDK debug logs by default", () => { + const loglevelLogger = matrixJsSdkLogger as unknown as { + levels: { INFO: number }; + setLevel: (level: number | string, persist?: boolean) => void; + }; + const setLevel = vi.spyOn(loglevelLogger, "setLevel").mockImplementation(() => undefined); + + ensureMatrixSdkLoggingConfigured(); + + expect(setLevel).toHaveBeenLastCalledWith(loglevelLogger.levels.INFO, false); + }); }); diff --git a/extensions/matrix/src/matrix/client/logging.ts b/extensions/matrix/src/matrix/client/logging.ts index 9e03989312c..7970c1e46b7 100644 --- a/extensions/matrix/src/matrix/client/logging.ts +++ b/extensions/matrix/src/matrix/client/logging.ts @@ -17,7 +17,7 @@ type MatrixJsSdkLogger = { }; type MatrixJsSdkLoglevelLogger = MatrixJsSdkLogger & { - levels?: { DEBUG?: number }; + levels?: { DEBUG?: number; ERROR?: number; INFO?: number }; methodFactory?: ( methodName: string, logLevel: number, @@ -129,10 +129,20 @@ function applyMatrixJsSdkLogger(): void { writeMatrixSdkLog(method, module, messageOrObject); }; }; - logger.setLevel?.(logger.levels?.DEBUG ?? "debug", false); + logger.setLevel?.(resolveMatrixJsSdkLogLevel(logger), false); logger.rebuild?.(); } +function resolveMatrixJsSdkLogLevel(logger: MatrixJsSdkLoglevelLogger): number | string { + if (matrixSdkLogMode === "quiet") { + return logger.levels?.ERROR ?? "error"; + } + if (process.env.OPENCLAW_MATRIX_SDK_DEBUG === "1" || process.env.MATRIX_SDK_DEBUG === "1") { + return logger.levels?.DEBUG ?? "debug"; + } + return logger.levels?.INFO ?? "info"; +} + function createMatrixJsSdkLoggerInstance(prefix: string): MatrixJsSdkLogger { const log = (method: MatrixLogMethod, ...messageOrObject: unknown[]): void => { if (matrixSdkLogMode === "quiet") { diff --git a/src/security/skill-scanner.test.ts b/src/security/skill-scanner.test.ts index a215b1d5fff..524f6783fb2 100644 --- a/src/security/skill-scanner.test.ts +++ b/src/security/skill-scanner.test.ts @@ -241,17 +241,32 @@ const options: ExecOptions = { timeout: 5000 }; expect(findings.some((f) => f.ruleId === "dangerous-exec")).toBe(false); }); - it("does not flag argv-only self-reexec as shell command execution", () => { + it("does not flag the qa-matrix argv-only self-reexec as shell command execution", () => { const source = ` import { spawn } from "node:child_process"; -const child = spawn(process.execPath, [distEntryPath, ...args], { +const child = spawn(process.execPath, [distEntryPath, ...params.args], { stdio: ["pipe", "pipe", "pipe"], }); `; - const findings = scanSource(source, "plugin.ts"); + const findings = scanSource( + source, + path.resolve( + process.cwd(), + "extensions/qa-matrix/src/runners/contract/scenario-runtime-cli.ts", + ), + ); expect(findings.some((f) => f.ruleId === "dangerous-exec")).toBe(false); }); + it("still flags plugin self-reexec as shell command execution", () => { + const source = ` +import { spawn } from "node:child_process"; +const child = spawn(process.execPath, userControlledArgs, { shell: true }); +`; + const findings = scanSource(source, "plugin.ts"); + expect(findings.some((f) => f.ruleId === "dangerous-exec")).toBe(true); + }); + it("returns empty array for clean plugin code", () => { const source = ` export function greet(name: string): string { diff --git a/src/security/skill-scanner.ts b/src/security/skill-scanner.ts index 8ea6adf69bb..ccb95a1bfa9 100644 --- a/src/security/skill-scanner.ts +++ b/src/security/skill-scanner.ts @@ -216,10 +216,26 @@ function truncateEvidence(evidence: string, maxLen = 120): string { return `${evidence.slice(0, maxLen)}…`; } -function isAllowedNodeSelfReexec(line: string): boolean { +function isAllowedNodeSelfReexec(filePath: string, line: string): boolean { + if ( + path.resolve(filePath) !== + path.resolve( + process.cwd(), + "extensions", + "qa-matrix", + "src", + "runners", + "contract", + "scenario-runtime-cli.ts", + ) + ) { + return false; + } // Spawning the current Node executable with an argv array is not shell // execution. Keep direct shell/process launches blocked below. - return /\bspawn\s*\(\s*process\.execPath\s*,/.test(line); + return /\bspawn\s*\(\s*process\.execPath\s*,\s*\[\s*distEntryPath\s*,\s*\.{3}params\.args\s*\]/.test( + line, + ); } export function scanSource(source: string, filePath: string): SkillScanFinding[] { @@ -244,7 +260,7 @@ export function scanSource(source: string, filePath: string): SkillScanFinding[] if (!match) { continue; } - if (rule.ruleId === "dangerous-exec" && isAllowedNodeSelfReexec(line)) { + if (rule.ruleId === "dangerous-exec" && isAllowedNodeSelfReexec(filePath, line)) { continue; }