mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 09:50:42 +00:00
fix: address Matrix check triage
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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],
|
||||
|
||||
@@ -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<MatrixCliVerificationSas["emoji"]>): 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 {
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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({
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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") {
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user