mirror of
https://github.com/moltbot/moltbot.git
synced 2026-03-07 22:44:16 +00:00
Doctor/Security: fix telegram numeric ID + symlink config permission warnings (#19844)
Merged via /review-pr -> /prepare-pr -> /merge-pr.
Prepared head SHA: e42bf1e48d
Co-authored-by: joshp123 <1497361+joshp123@users.noreply.github.com>
Co-authored-by: joshp123 <1497361+joshp123@users.noreply.github.com>
Reviewed-by: @joshp123
This commit is contained in:
@@ -394,6 +394,7 @@ public struct SendParams: Codable, Sendable {
|
||||
public let gifplayback: Bool?
|
||||
public let channel: String?
|
||||
public let accountid: String?
|
||||
public let threadid: String?
|
||||
public let sessionkey: String?
|
||||
public let idempotencykey: String
|
||||
|
||||
@@ -405,6 +406,7 @@ public struct SendParams: Codable, Sendable {
|
||||
gifplayback: Bool?,
|
||||
channel: String?,
|
||||
accountid: String?,
|
||||
threadid: String?,
|
||||
sessionkey: String?,
|
||||
idempotencykey: String
|
||||
) {
|
||||
@@ -415,6 +417,7 @@ public struct SendParams: Codable, Sendable {
|
||||
self.gifplayback = gifplayback
|
||||
self.channel = channel
|
||||
self.accountid = accountid
|
||||
self.threadid = threadid
|
||||
self.sessionkey = sessionkey
|
||||
self.idempotencykey = idempotencykey
|
||||
}
|
||||
@@ -426,6 +429,7 @@ public struct SendParams: Codable, Sendable {
|
||||
case gifplayback = "gifPlayback"
|
||||
case channel
|
||||
case accountid = "accountId"
|
||||
case threadid = "threadId"
|
||||
case sessionkey = "sessionKey"
|
||||
case idempotencykey = "idempotencyKey"
|
||||
}
|
||||
|
||||
@@ -394,6 +394,7 @@ public struct SendParams: Codable, Sendable {
|
||||
public let gifplayback: Bool?
|
||||
public let channel: String?
|
||||
public let accountid: String?
|
||||
public let threadid: String?
|
||||
public let sessionkey: String?
|
||||
public let idempotencykey: String
|
||||
|
||||
@@ -405,6 +406,7 @@ public struct SendParams: Codable, Sendable {
|
||||
gifplayback: Bool?,
|
||||
channel: String?,
|
||||
accountid: String?,
|
||||
threadid: String?,
|
||||
sessionkey: String?,
|
||||
idempotencykey: String
|
||||
) {
|
||||
@@ -415,6 +417,7 @@ public struct SendParams: Codable, Sendable {
|
||||
self.gifplayback = gifplayback
|
||||
self.channel = channel
|
||||
self.accountid = accountid
|
||||
self.threadid = threadid
|
||||
self.sessionkey = sessionkey
|
||||
self.idempotencykey = idempotencykey
|
||||
}
|
||||
@@ -426,6 +429,7 @@ public struct SendParams: Codable, Sendable {
|
||||
case gifplayback = "gifPlayback"
|
||||
case channel
|
||||
case accountid = "accountId"
|
||||
case threadid = "threadId"
|
||||
case sessionkey = "sessionKey"
|
||||
case idempotencykey = "idempotencyKey"
|
||||
}
|
||||
|
||||
16
src/channels/telegram/allow-from.test.ts
Normal file
16
src/channels/telegram/allow-from.test.ts
Normal file
@@ -0,0 +1,16 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { isNumericTelegramUserId, normalizeTelegramAllowFromEntry } from "./allow-from.js";
|
||||
|
||||
describe("telegram allow-from helpers", () => {
|
||||
it("normalizes tg/telegram prefixes", () => {
|
||||
expect(normalizeTelegramAllowFromEntry(" TG:123 ")).toBe("123");
|
||||
expect(normalizeTelegramAllowFromEntry("telegram:@someone")).toBe("@someone");
|
||||
});
|
||||
|
||||
it("accepts signed numeric IDs", () => {
|
||||
expect(isNumericTelegramUserId("123456789")).toBe(true);
|
||||
expect(isNumericTelegramUserId("-1001234567890")).toBe(true);
|
||||
expect(isNumericTelegramUserId("@someone")).toBe(false);
|
||||
expect(isNumericTelegramUserId("12 34")).toBe(false);
|
||||
});
|
||||
});
|
||||
@@ -7,5 +7,5 @@ export function normalizeTelegramAllowFromEntry(raw: unknown): string {
|
||||
}
|
||||
|
||||
export function isNumericTelegramUserId(raw: string): boolean {
|
||||
return /^\d+$/.test(raw);
|
||||
return /^-?\d+$/.test(raw);
|
||||
}
|
||||
|
||||
@@ -223,8 +223,10 @@ export async function noteStateIntegrity(
|
||||
|
||||
if (configPath && existsFile(configPath) && process.platform !== "win32") {
|
||||
try {
|
||||
const linkStat = fs.lstatSync(configPath);
|
||||
const stat = fs.statSync(configPath);
|
||||
if ((stat.mode & 0o077) !== 0) {
|
||||
const isSymlink = linkStat.isSymbolicLink();
|
||||
if (!isSymlink && (stat.mode & 0o077) !== 0) {
|
||||
warnings.push(
|
||||
`- Config file is group/world readable (${displayConfigPath ?? configPath}). Recommend chmod 600.`,
|
||||
);
|
||||
|
||||
@@ -80,7 +80,19 @@ export async function inspectPathPermissions(
|
||||
};
|
||||
}
|
||||
|
||||
const bits = modeBits(st.mode);
|
||||
let effectiveMode = st.mode;
|
||||
let effectiveIsDir = st.isDir;
|
||||
if (st.isSymlink) {
|
||||
try {
|
||||
const target = await fs.stat(targetPath);
|
||||
effectiveMode = typeof target.mode === "number" ? target.mode : st.mode;
|
||||
effectiveIsDir = target.isDirectory();
|
||||
} catch {
|
||||
// Keep lstat-derived metadata when target lookup fails.
|
||||
}
|
||||
}
|
||||
|
||||
const bits = modeBits(effectiveMode);
|
||||
const platform = opts?.platform ?? process.platform;
|
||||
|
||||
if (platform === "win32") {
|
||||
@@ -89,8 +101,8 @@ export async function inspectPathPermissions(
|
||||
return {
|
||||
ok: true,
|
||||
isSymlink: st.isSymlink,
|
||||
isDir: st.isDir,
|
||||
mode: st.mode,
|
||||
isDir: effectiveIsDir,
|
||||
mode: effectiveMode,
|
||||
bits,
|
||||
source: "unknown",
|
||||
worldWritable: false,
|
||||
@@ -103,8 +115,8 @@ export async function inspectPathPermissions(
|
||||
return {
|
||||
ok: true,
|
||||
isSymlink: st.isSymlink,
|
||||
isDir: st.isDir,
|
||||
mode: st.mode,
|
||||
isDir: effectiveIsDir,
|
||||
mode: effectiveMode,
|
||||
bits,
|
||||
source: "windows-acl",
|
||||
worldWritable: acl.untrustedWorld.some((entry) => entry.canWrite),
|
||||
@@ -118,8 +130,8 @@ export async function inspectPathPermissions(
|
||||
return {
|
||||
ok: true,
|
||||
isSymlink: st.isSymlink,
|
||||
isDir: st.isDir,
|
||||
mode: st.mode,
|
||||
isDir: effectiveIsDir,
|
||||
mode: effectiveMode,
|
||||
bits,
|
||||
source: "posix",
|
||||
worldWritable: isWorldWritable(bits),
|
||||
|
||||
@@ -386,6 +386,38 @@ describe("security audit", () => {
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("uses symlink target permissions for config checks", async () => {
|
||||
if (isWindows) {
|
||||
return;
|
||||
}
|
||||
|
||||
const tmp = await makeTmpDir("config-symlink");
|
||||
const stateDir = path.join(tmp, "state");
|
||||
await fs.mkdir(stateDir, { recursive: true, mode: 0o700 });
|
||||
|
||||
const targetConfigPath = path.join(tmp, "managed-openclaw.json");
|
||||
await fs.writeFile(targetConfigPath, "{}\n", "utf-8");
|
||||
await fs.chmod(targetConfigPath, 0o444);
|
||||
|
||||
const configPath = path.join(stateDir, "openclaw.json");
|
||||
await fs.symlink(targetConfigPath, configPath);
|
||||
|
||||
const res = await runSecurityAudit({
|
||||
config: {},
|
||||
includeFilesystem: true,
|
||||
includeChannelSecurity: false,
|
||||
stateDir,
|
||||
configPath,
|
||||
});
|
||||
|
||||
expect(res.findings).toEqual(
|
||||
expect.arrayContaining([expect.objectContaining({ checkId: "fs.config.symlink" })]),
|
||||
);
|
||||
expect(res.findings.some((f) => f.checkId === "fs.config.perms_writable")).toBe(false);
|
||||
expect(res.findings.some((f) => f.checkId === "fs.config.perms_world_readable")).toBe(false);
|
||||
expect(res.findings.some((f) => f.checkId === "fs.config.perms_group_readable")).toBe(false);
|
||||
});
|
||||
|
||||
it("warns when small models are paired with web/browser tools", async () => {
|
||||
const cfg: OpenClawConfig = {
|
||||
agents: { defaults: { model: { primary: "ollama/mistral-8b" } } },
|
||||
|
||||
@@ -186,6 +186,7 @@ async function collectFilesystemFindings(params: {
|
||||
exec: params.execIcacls,
|
||||
});
|
||||
if (configPerms.ok) {
|
||||
const skipReadablePermWarnings = configPerms.isSymlink;
|
||||
if (configPerms.isSymlink) {
|
||||
findings.push({
|
||||
checkId: "fs.config.symlink",
|
||||
@@ -208,7 +209,7 @@ async function collectFilesystemFindings(params: {
|
||||
env: params.env,
|
||||
}),
|
||||
});
|
||||
} else if (configPerms.worldReadable) {
|
||||
} else if (!skipReadablePermWarnings && configPerms.worldReadable) {
|
||||
findings.push({
|
||||
checkId: "fs.config.perms_world_readable",
|
||||
severity: "critical",
|
||||
@@ -222,7 +223,7 @@ async function collectFilesystemFindings(params: {
|
||||
env: params.env,
|
||||
}),
|
||||
});
|
||||
} else if (configPerms.groupReadable) {
|
||||
} else if (!skipReadablePermWarnings && configPerms.groupReadable) {
|
||||
findings.push({
|
||||
checkId: "fs.config.perms_group_readable",
|
||||
severity: "warn",
|
||||
|
||||
Reference in New Issue
Block a user