From 201420a7ee919abe1f712cfddcba8ae7aecb2162 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Tue, 10 Mar 2026 23:40:10 +0000 Subject: [PATCH] fix: harden secret-file readers --- CHANGELOG.md | 1 + docs/channels/line.md | 2 + docs/channels/nextcloud-talk.md | 2 +- docs/channels/telegram.md | 4 +- docs/channels/zalo.md | 4 +- docs/gateway/configuration-reference.md | 2 +- docs/gateway/security/index.md | 2 +- docs/start/setup.md | 2 +- extensions/irc/src/accounts.test.ts | 30 +++- extensions/irc/src/accounts.ts | 23 ++- .../nextcloud-talk/src/accounts.test.ts | 30 ++++ extensions/nextcloud-talk/src/accounts.ts | 16 +-- extensions/zalo/src/token.test.ts | 19 +++ extensions/zalo/src/token.ts | 13 +- src/acp/secret-file.test.ts | 52 +------ src/acp/secret-file.ts | 45 +----- src/config/types.telegram.ts | 2 +- src/infra/secret-file.test.ts | 70 +++++++++ src/infra/secret-file.ts | 133 ++++++++++++++++++ src/line/accounts.test.ts | 30 ++++ src/line/accounts.ts | 11 +- src/plugin-sdk/core.ts | 7 + src/telegram/account-inspect.test.ts | 28 ++++ src/telegram/account-inspect.ts | 31 ++-- src/telegram/token.test.ts | 15 ++ src/telegram/token.ts | 47 +++---- 26 files changed, 433 insertions(+), 188 deletions(-) create mode 100644 extensions/nextcloud-talk/src/accounts.test.ts create mode 100644 src/infra/secret-file.test.ts create mode 100644 src/infra/secret-file.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index e285807e999..2254328bd5c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,7 @@ Docs: https://docs.openclaw.ai - ACP/ACPX plugin: bump the bundled `acpx` pin to `0.1.16` so plugin-local installs and strict version checks match the latest published CLI. (#41975) Thanks @dutifulbob. - macOS/LaunchAgent install: tighten LaunchAgent directory and plist permissions during install so launchd bootstrap does not fail when the target home path or generated plist inherited group/world-writable modes. - Gateway/Control UI: keep dashboard auth tokens in session-scoped browser storage so same-tab refreshes preserve remote token auth without restoring long-lived localStorage token persistence, while scoping tokens to the selected gateway URL and fragment-only bootstrap flow. (#40892) thanks @velvet-shark. +- Secret files: harden CLI and channel credential file reads against path-swap races by requiring direct regular files for `*File` secret inputs and rejecting symlink-backed secret files. - Models/Kimi Coding: send `anthropic-messages` tools in native Anthropic format again so `kimi-coding` stops degrading tool calls into XML/plain-text pseudo invocations instead of real `tool_use` blocks. (#38669, #39907, #40552) Thanks @opriz. - Context engine/tests: add bundled-registry regression coverage for cross-chunk resolution, plugin-sdk re-exports, and concurrent chunk registration. (#40460) thanks @dsantoreis. - Agents/embedded runner: bound compaction retry waiting and drain embedded runs during SIGUSR1 restart so session lanes recover instead of staying blocked behind compaction. (#40324) thanks @cgdusek. diff --git a/docs/channels/line.md b/docs/channels/line.md index 50972d93d21..a965dc6e991 100644 --- a/docs/channels/line.md +++ b/docs/channels/line.md @@ -87,6 +87,8 @@ Token/secret files: } ``` +`tokenFile` and `secretFile` must point to regular files. Symlinks are rejected. + Multiple accounts: ```json5 diff --git a/docs/channels/nextcloud-talk.md b/docs/channels/nextcloud-talk.md index d4ab9e2c397..7797b1276ff 100644 --- a/docs/channels/nextcloud-talk.md +++ b/docs/channels/nextcloud-talk.md @@ -115,7 +115,7 @@ Provider options: - `channels.nextcloud-talk.enabled`: enable/disable channel startup. - `channels.nextcloud-talk.baseUrl`: Nextcloud instance URL. - `channels.nextcloud-talk.botSecret`: bot shared secret. -- `channels.nextcloud-talk.botSecretFile`: secret file path. +- `channels.nextcloud-talk.botSecretFile`: regular-file secret path. Symlinks are rejected. - `channels.nextcloud-talk.apiUser`: API user for room lookups (DM detection). - `channels.nextcloud-talk.apiPassword`: API/app password for room lookups. - `channels.nextcloud-talk.apiPasswordFile`: API password file path. diff --git a/docs/channels/telegram.md b/docs/channels/telegram.md index 7c32c29ab19..f2467d12b0a 100644 --- a/docs/channels/telegram.md +++ b/docs/channels/telegram.md @@ -892,7 +892,7 @@ Primary reference: - `channels.telegram.enabled`: enable/disable channel startup. - `channels.telegram.botToken`: bot token (BotFather). -- `channels.telegram.tokenFile`: read token from file path. +- `channels.telegram.tokenFile`: read token from a regular file path. Symlinks are rejected. - `channels.telegram.dmPolicy`: `pairing | allowlist | open | disabled` (default: pairing). - `channels.telegram.allowFrom`: DM allowlist (numeric Telegram user IDs). `allowlist` requires at least one sender ID. `open` requires `"*"`. `openclaw doctor --fix` can resolve legacy `@username` entries to IDs and can recover allowlist entries from pairing-store files in allowlist migration flows. - `channels.telegram.actions.poll`: enable or disable Telegram poll creation (default: enabled; still requires `sendMessage`). @@ -953,7 +953,7 @@ Primary reference: Telegram-specific high-signal fields: -- startup/auth: `enabled`, `botToken`, `tokenFile`, `accounts.*` +- startup/auth: `enabled`, `botToken`, `tokenFile`, `accounts.*` (`tokenFile` must point to a regular file; symlinks are rejected) - access control: `dmPolicy`, `allowFrom`, `groupPolicy`, `groupAllowFrom`, `groups`, `groups.*.topics.*`, top-level `bindings[]` (`type: "acp"`) - exec approvals: `execApprovals`, `accounts.*.execApprovals` - command/menu: `commands.native`, `commands.nativeSkills`, `customCommands` diff --git a/docs/channels/zalo.md b/docs/channels/zalo.md index 8e5d8ab0382..77b288b0ab7 100644 --- a/docs/channels/zalo.md +++ b/docs/channels/zalo.md @@ -179,7 +179,7 @@ Provider options: - `channels.zalo.enabled`: enable/disable channel startup. - `channels.zalo.botToken`: bot token from Zalo Bot Platform. -- `channels.zalo.tokenFile`: read token from file path. +- `channels.zalo.tokenFile`: read token from a regular file path. Symlinks are rejected. - `channels.zalo.dmPolicy`: `pairing | allowlist | open | disabled` (default: pairing). - `channels.zalo.allowFrom`: DM allowlist (user IDs). `open` requires `"*"`. The wizard will ask for numeric IDs. - `channels.zalo.groupPolicy`: `open | allowlist | disabled` (default: allowlist). @@ -193,7 +193,7 @@ Provider options: Multi-account options: - `channels.zalo.accounts..botToken`: per-account token. -- `channels.zalo.accounts..tokenFile`: per-account token file. +- `channels.zalo.accounts..tokenFile`: per-account regular token file. Symlinks are rejected. - `channels.zalo.accounts..name`: display name. - `channels.zalo.accounts..enabled`: enable/disable account. - `channels.zalo.accounts..dmPolicy`: per-account DM policy. diff --git a/docs/gateway/configuration-reference.md b/docs/gateway/configuration-reference.md index 5cad5acea9d..2a27470fd36 100644 --- a/docs/gateway/configuration-reference.md +++ b/docs/gateway/configuration-reference.md @@ -203,7 +203,7 @@ WhatsApp runs through the gateway's web channel (Baileys Web). It starts automat } ``` -- Bot token: `channels.telegram.botToken` or `channels.telegram.tokenFile`, with `TELEGRAM_BOT_TOKEN` as fallback for the default account. +- Bot token: `channels.telegram.botToken` or `channels.telegram.tokenFile` (regular file only; symlinks rejected), with `TELEGRAM_BOT_TOKEN` as fallback for the default account. - Optional `channels.telegram.defaultAccount` overrides default account selection when it matches a configured account id. - In multi-account setups (2+ account ids), set an explicit default (`channels.telegram.defaultAccount` or `channels.telegram.accounts.default`) to avoid fallback routing; `openclaw doctor` warns when this is missing or invalid. - `configWrites: false` blocks Telegram-initiated config writes (supergroup ID migrations, `/config set|unset`). diff --git a/docs/gateway/security/index.md b/docs/gateway/security/index.md index 571f91cf405..8b790f4ded6 100644 --- a/docs/gateway/security/index.md +++ b/docs/gateway/security/index.md @@ -199,7 +199,7 @@ If you run `--deep`, OpenClaw also attempts a best-effort live Gateway probe. Use this when auditing access or deciding what to back up: - **WhatsApp**: `~/.openclaw/credentials/whatsapp//creds.json` -- **Telegram bot token**: config/env or `channels.telegram.tokenFile` +- **Telegram bot token**: config/env or `channels.telegram.tokenFile` (regular file only; symlinks rejected) - **Discord bot token**: config/env or SecretRef (env/file/exec providers) - **Slack tokens**: config/env (`channels.slack.*`) - **Pairing allowlists**: diff --git a/docs/start/setup.md b/docs/start/setup.md index 4b6113743f8..205f14d20a5 100644 --- a/docs/start/setup.md +++ b/docs/start/setup.md @@ -127,7 +127,7 @@ openclaw health Use this when debugging auth or deciding what to back up: - **WhatsApp**: `~/.openclaw/credentials/whatsapp//creds.json` -- **Telegram bot token**: config/env or `channels.telegram.tokenFile` +- **Telegram bot token**: config/env or `channels.telegram.tokenFile` (regular file only; symlinks rejected) - **Discord bot token**: config/env or SecretRef (env/file/exec providers) - **Slack tokens**: config/env (`channels.slack.*`) - **Pairing allowlists**: diff --git a/extensions/irc/src/accounts.test.ts b/extensions/irc/src/accounts.test.ts index 59a72d7cbcb..afd1b597b81 100644 --- a/extensions/irc/src/accounts.test.ts +++ b/extensions/irc/src/accounts.test.ts @@ -1,5 +1,8 @@ +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; import { describe, expect, it } from "vitest"; -import { listIrcAccountIds, resolveDefaultIrcAccountId } from "./accounts.js"; +import { listIrcAccountIds, resolveDefaultIrcAccountId, resolveIrcAccount } from "./accounts.js"; import type { CoreConfig } from "./types.js"; function asConfig(value: unknown): CoreConfig { @@ -76,3 +79,28 @@ describe("resolveDefaultIrcAccountId", () => { expect(resolveDefaultIrcAccountId(cfg)).toBe("aaa"); }); }); + +describe("resolveIrcAccount", () => { + it.runIf(process.platform !== "win32")("rejects symlinked password files", () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-irc-account-")); + const passwordFile = path.join(dir, "password.txt"); + const passwordLink = path.join(dir, "password-link.txt"); + fs.writeFileSync(passwordFile, "secret-pass\n", "utf8"); + fs.symlinkSync(passwordFile, passwordLink); + + const cfg = asConfig({ + channels: { + irc: { + host: "irc.example.com", + nick: "claw", + passwordFile: passwordLink, + }, + }, + }); + + const account = resolveIrcAccount({ cfg }); + expect(account.password).toBe(""); + expect(account.passwordSource).toBe("none"); + fs.rmSync(dir, { recursive: true, force: true }); + }); +}); diff --git a/extensions/irc/src/accounts.ts b/extensions/irc/src/accounts.ts index d61499c4d39..13d48fffdb7 100644 --- a/extensions/irc/src/accounts.ts +++ b/extensions/irc/src/accounts.ts @@ -1,5 +1,5 @@ -import { readFileSync } from "node:fs"; import { DEFAULT_ACCOUNT_ID, normalizeAccountId } from "openclaw/plugin-sdk/account-id"; +import { tryReadSecretFileSync } from "openclaw/plugin-sdk/core"; import { createAccountListHelpers, normalizeResolvedSecretInputString, @@ -100,13 +100,11 @@ function resolvePassword(accountId: string, merged: IrcAccountConfig) { } if (merged.passwordFile?.trim()) { - try { - const filePassword = readFileSync(merged.passwordFile.trim(), "utf-8").trim(); - if (filePassword) { - return { password: filePassword, source: "passwordFile" as const }; - } - } catch { - // Ignore unreadable files here; status will still surface missing configuration. + const filePassword = tryReadSecretFileSync(merged.passwordFile, "IRC password file", { + rejectSymlink: true, + }); + if (filePassword) { + return { password: filePassword, source: "passwordFile" as const }; } } @@ -137,11 +135,10 @@ function resolveNickServConfig(accountId: string, nickserv?: IrcNickServConfig): envPassword || ""; if (!resolvedPassword && passwordFile) { - try { - resolvedPassword = readFileSync(passwordFile, "utf-8").trim(); - } catch { - // Ignore unreadable files; monitor/probe status will surface failures. - } + resolvedPassword = + tryReadSecretFileSync(passwordFile, "IRC NickServ password file", { + rejectSymlink: true, + }) ?? ""; } const merged: IrcNickServConfig = { diff --git a/extensions/nextcloud-talk/src/accounts.test.ts b/extensions/nextcloud-talk/src/accounts.test.ts new file mode 100644 index 00000000000..dbc43690a3b --- /dev/null +++ b/extensions/nextcloud-talk/src/accounts.test.ts @@ -0,0 +1,30 @@ +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { describe, expect, it } from "vitest"; +import { resolveNextcloudTalkAccount } from "./accounts.js"; +import type { CoreConfig } from "./types.js"; + +describe("resolveNextcloudTalkAccount", () => { + it.runIf(process.platform !== "win32")("rejects symlinked botSecretFile paths", () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-nextcloud-talk-")); + const secretFile = path.join(dir, "secret.txt"); + const secretLink = path.join(dir, "secret-link.txt"); + fs.writeFileSync(secretFile, "bot-secret\n", "utf8"); + fs.symlinkSync(secretFile, secretLink); + + const cfg = { + channels: { + "nextcloud-talk": { + baseUrl: "https://cloud.example.com", + botSecretFile: secretLink, + }, + }, + } as CoreConfig; + + const account = resolveNextcloudTalkAccount({ cfg }); + expect(account.secret).toBe(""); + expect(account.secretSource).toBe("none"); + fs.rmSync(dir, { recursive: true, force: true }); + }); +}); diff --git a/extensions/nextcloud-talk/src/accounts.ts b/extensions/nextcloud-talk/src/accounts.ts index 74bb45cfd8b..2cfba6fea44 100644 --- a/extensions/nextcloud-talk/src/accounts.ts +++ b/extensions/nextcloud-talk/src/accounts.ts @@ -1,4 +1,4 @@ -import { readFileSync } from "node:fs"; +import { tryReadSecretFileSync } from "openclaw/plugin-sdk/core"; import { createAccountListHelpers, DEFAULT_ACCOUNT_ID, @@ -88,13 +88,13 @@ function resolveNextcloudTalkSecret( } if (merged.botSecretFile) { - try { - const fileSecret = readFileSync(merged.botSecretFile, "utf-8").trim(); - if (fileSecret) { - return { secret: fileSecret, source: "secretFile" }; - } - } catch { - // File not found or unreadable, fall through. + const fileSecret = tryReadSecretFileSync( + merged.botSecretFile, + "Nextcloud Talk bot secret file", + { rejectSymlink: true }, + ); + if (fileSecret) { + return { secret: fileSecret, source: "secretFile" }; } } diff --git a/extensions/zalo/src/token.test.ts b/extensions/zalo/src/token.test.ts index d6b02f30483..ff3e84ce293 100644 --- a/extensions/zalo/src/token.test.ts +++ b/extensions/zalo/src/token.test.ts @@ -1,3 +1,6 @@ +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; import { describe, expect, it } from "vitest"; import { resolveZaloToken } from "./token.js"; import type { ZaloConfig } from "./types.js"; @@ -55,4 +58,20 @@ describe("resolveZaloToken", () => { expect(res.token).toBe("work-token"); expect(res.source).toBe("config"); }); + + it.runIf(process.platform !== "win32")("rejects symlinked token files", () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-zalo-token-")); + const tokenFile = path.join(dir, "token.txt"); + const tokenLink = path.join(dir, "token-link.txt"); + fs.writeFileSync(tokenFile, "file-token\n", "utf8"); + fs.symlinkSync(tokenFile, tokenLink); + + const cfg = { + tokenFile: tokenLink, + } as ZaloConfig; + const res = resolveZaloToken(cfg); + expect(res.token).toBe(""); + expect(res.source).toBe("none"); + fs.rmSync(dir, { recursive: true, force: true }); + }); }); diff --git a/extensions/zalo/src/token.ts b/extensions/zalo/src/token.ts index 00ed1d720f7..10a4aca6cd1 100644 --- a/extensions/zalo/src/token.ts +++ b/extensions/zalo/src/token.ts @@ -1,5 +1,5 @@ -import { readFileSync } from "node:fs"; import { DEFAULT_ACCOUNT_ID, normalizeAccountId } from "openclaw/plugin-sdk/account-id"; +import { tryReadSecretFileSync } from "openclaw/plugin-sdk/core"; import type { BaseTokenResolution } from "openclaw/plugin-sdk/zalo"; import { normalizeResolvedSecretInputString, normalizeSecretInputString } from "./secret-input.js"; import type { ZaloConfig } from "./types.js"; @@ -9,16 +9,7 @@ export type ZaloTokenResolution = BaseTokenResolution & { }; function readTokenFromFile(tokenFile: string | undefined): string { - const trimmedPath = tokenFile?.trim(); - if (!trimmedPath) { - return ""; - } - try { - return readFileSync(trimmedPath, "utf8").trim(); - } catch { - // ignore read failures - return ""; - } + return tryReadSecretFileSync(tokenFile, "Zalo token file", { rejectSymlink: true }) ?? ""; } export function resolveZaloToken( diff --git a/src/acp/secret-file.test.ts b/src/acp/secret-file.test.ts index 4db2d265d7f..bef3cf3ed02 100644 --- a/src/acp/secret-file.test.ts +++ b/src/acp/secret-file.test.ts @@ -1,54 +1,12 @@ -import { mkdir, symlink, writeFile } from "node:fs/promises"; -import path from "node:path"; -import { afterEach, describe, expect, it } from "vitest"; -import { createTrackedTempDirs } from "../test-utils/tracked-temp-dirs.js"; +import { describe, expect, it } from "vitest"; import { MAX_SECRET_FILE_BYTES, readSecretFromFile } from "./secret-file.js"; -const tempDirs = createTrackedTempDirs(); -const createTempDir = () => tempDirs.make("openclaw-secret-file-test-"); - -afterEach(async () => { - await tempDirs.cleanup(); -}); - describe("readSecretFromFile", () => { - it("reads and trims a regular secret file", async () => { - const dir = await createTempDir(); - const file = path.join(dir, "secret.txt"); - await writeFile(file, " top-secret \n", "utf8"); - - expect(readSecretFromFile(file, "Gateway password")).toBe("top-secret"); + it("keeps the shared secret-file limit", () => { + expect(MAX_SECRET_FILE_BYTES).toBe(16 * 1024); }); - it("rejects files larger than the secret-file limit", async () => { - const dir = await createTempDir(); - const file = path.join(dir, "secret.txt"); - await writeFile(file, "x".repeat(MAX_SECRET_FILE_BYTES + 1), "utf8"); - - expect(() => readSecretFromFile(file, "Gateway password")).toThrow( - `Gateway password file at ${file} exceeds ${MAX_SECRET_FILE_BYTES} bytes.`, - ); - }); - - it("rejects non-regular files", async () => { - const dir = await createTempDir(); - const nestedDir = path.join(dir, "secret-dir"); - await mkdir(nestedDir); - - expect(() => readSecretFromFile(nestedDir, "Gateway password")).toThrow( - `Gateway password file at ${nestedDir} must be a regular file.`, - ); - }); - - it("rejects symlinks", async () => { - const dir = await createTempDir(); - const target = path.join(dir, "target.txt"); - const link = path.join(dir, "secret-link.txt"); - await writeFile(target, "top-secret\n", "utf8"); - await symlink(target, link); - - expect(() => readSecretFromFile(link, "Gateway password")).toThrow( - `Gateway password file at ${link} must not be a symlink.`, - ); + it("exposes the hardened secret reader", () => { + expect(typeof readSecretFromFile).toBe("function"); }); }); diff --git a/src/acp/secret-file.ts b/src/acp/secret-file.ts index 45ec36d28cb..902e0fc0627 100644 --- a/src/acp/secret-file.ts +++ b/src/acp/secret-file.ts @@ -1,43 +1,10 @@ -import fs from "node:fs"; -import { resolveUserPath } from "../utils.js"; +import { DEFAULT_SECRET_FILE_MAX_BYTES, readSecretFileSync } from "../infra/secret-file.js"; -export const MAX_SECRET_FILE_BYTES = 16 * 1024; +export const MAX_SECRET_FILE_BYTES = DEFAULT_SECRET_FILE_MAX_BYTES; export function readSecretFromFile(filePath: string, label: string): string { - const resolvedPath = resolveUserPath(filePath.trim()); - if (!resolvedPath) { - throw new Error(`${label} file path is empty.`); - } - - let stat: fs.Stats; - try { - stat = fs.lstatSync(resolvedPath); - } catch (err) { - throw new Error(`Failed to inspect ${label} file at ${resolvedPath}: ${String(err)}`, { - cause: err, - }); - } - if (stat.isSymbolicLink()) { - throw new Error(`${label} file at ${resolvedPath} must not be a symlink.`); - } - if (!stat.isFile()) { - throw new Error(`${label} file at ${resolvedPath} must be a regular file.`); - } - if (stat.size > MAX_SECRET_FILE_BYTES) { - throw new Error(`${label} file at ${resolvedPath} exceeds ${MAX_SECRET_FILE_BYTES} bytes.`); - } - - let raw = ""; - try { - raw = fs.readFileSync(resolvedPath, "utf8"); - } catch (err) { - throw new Error(`Failed to read ${label} file at ${resolvedPath}: ${String(err)}`, { - cause: err, - }); - } - const secret = raw.trim(); - if (!secret) { - throw new Error(`${label} file at ${resolvedPath} is empty.`); - } - return secret; + return readSecretFileSync(filePath, label, { + maxBytes: MAX_SECRET_FILE_BYTES, + rejectSymlink: true, + }); } diff --git a/src/config/types.telegram.ts b/src/config/types.telegram.ts index 41c047e860c..45eac2fb310 100644 --- a/src/config/types.telegram.ts +++ b/src/config/types.telegram.ts @@ -93,7 +93,7 @@ export type TelegramAccountConfig = { /** If false, do not start this Telegram account. Default: true. */ enabled?: boolean; botToken?: string; - /** Path to file containing bot token (for secret managers like agenix). */ + /** Path to a regular file containing the bot token; symlinks are rejected. */ tokenFile?: string; /** Control reply threading when reply tags are present (off|first|all). */ replyToMode?: ReplyToMode; diff --git a/src/infra/secret-file.test.ts b/src/infra/secret-file.test.ts new file mode 100644 index 00000000000..788b4c75e23 --- /dev/null +++ b/src/infra/secret-file.test.ts @@ -0,0 +1,70 @@ +import { mkdir, symlink, writeFile } from "node:fs/promises"; +import path from "node:path"; +import { afterEach, describe, expect, it } from "vitest"; +import { createTrackedTempDirs } from "../test-utils/tracked-temp-dirs.js"; +import { + DEFAULT_SECRET_FILE_MAX_BYTES, + readSecretFileSync, + tryReadSecretFileSync, +} from "./secret-file.js"; + +const tempDirs = createTrackedTempDirs(); +const createTempDir = () => tempDirs.make("openclaw-secret-file-test-"); + +afterEach(async () => { + await tempDirs.cleanup(); +}); + +describe("readSecretFileSync", () => { + it("reads and trims a regular secret file", async () => { + const dir = await createTempDir(); + const file = path.join(dir, "secret.txt"); + await writeFile(file, " top-secret \n", "utf8"); + + expect(readSecretFileSync(file, "Gateway password")).toBe("top-secret"); + }); + + it("rejects files larger than the secret-file limit", async () => { + const dir = await createTempDir(); + const file = path.join(dir, "secret.txt"); + await writeFile(file, "x".repeat(DEFAULT_SECRET_FILE_MAX_BYTES + 1), "utf8"); + + expect(() => readSecretFileSync(file, "Gateway password")).toThrow( + `Gateway password file at ${file} exceeds ${DEFAULT_SECRET_FILE_MAX_BYTES} bytes.`, + ); + }); + + it("rejects non-regular files", async () => { + const dir = await createTempDir(); + const nestedDir = path.join(dir, "secret-dir"); + await mkdir(nestedDir); + + expect(() => readSecretFileSync(nestedDir, "Gateway password")).toThrow( + `Gateway password file at ${nestedDir} must be a regular file.`, + ); + }); + + it("rejects symlinks when configured", async () => { + const dir = await createTempDir(); + const target = path.join(dir, "target.txt"); + const link = path.join(dir, "secret-link.txt"); + await writeFile(target, "top-secret\n", "utf8"); + await symlink(target, link); + + expect(() => readSecretFileSync(link, "Gateway password", { rejectSymlink: true })).toThrow( + `Gateway password file at ${link} must not be a symlink.`, + ); + }); + + it("returns undefined from the non-throwing helper for rejected files", async () => { + const dir = await createTempDir(); + const target = path.join(dir, "target.txt"); + const link = path.join(dir, "secret-link.txt"); + await writeFile(target, "top-secret\n", "utf8"); + await symlink(target, link); + + expect(tryReadSecretFileSync(link, "Telegram bot token", { rejectSymlink: true })).toBe( + undefined, + ); + }); +}); diff --git a/src/infra/secret-file.ts b/src/infra/secret-file.ts new file mode 100644 index 00000000000..d62fb326d6b --- /dev/null +++ b/src/infra/secret-file.ts @@ -0,0 +1,133 @@ +import fs from "node:fs"; +import { resolveUserPath } from "../utils.js"; +import { openVerifiedFileSync } from "./safe-open-sync.js"; + +export const DEFAULT_SECRET_FILE_MAX_BYTES = 16 * 1024; + +export type SecretFileReadOptions = { + maxBytes?: number; + rejectSymlink?: boolean; +}; + +export type SecretFileReadResult = + | { + ok: true; + secret: string; + resolvedPath: string; + } + | { + ok: false; + message: string; + resolvedPath?: string; + error?: unknown; + }; + +export function loadSecretFileSync( + filePath: string, + label: string, + options: SecretFileReadOptions = {}, +): SecretFileReadResult { + const trimmedPath = filePath.trim(); + const resolvedPath = resolveUserPath(trimmedPath); + if (!resolvedPath) { + return { ok: false, message: `${label} file path is empty.` }; + } + + const maxBytes = options.maxBytes ?? DEFAULT_SECRET_FILE_MAX_BYTES; + + let previewStat: fs.Stats; + try { + previewStat = fs.lstatSync(resolvedPath); + } catch (error) { + return { + ok: false, + resolvedPath, + error, + message: `Failed to inspect ${label} file at ${resolvedPath}: ${String(error)}`, + }; + } + + if (options.rejectSymlink && previewStat.isSymbolicLink()) { + return { + ok: false, + resolvedPath, + message: `${label} file at ${resolvedPath} must not be a symlink.`, + }; + } + if (!previewStat.isFile()) { + return { + ok: false, + resolvedPath, + message: `${label} file at ${resolvedPath} must be a regular file.`, + }; + } + if (previewStat.size > maxBytes) { + return { + ok: false, + resolvedPath, + message: `${label} file at ${resolvedPath} exceeds ${maxBytes} bytes.`, + }; + } + + const opened = openVerifiedFileSync({ + filePath: resolvedPath, + rejectPathSymlink: options.rejectSymlink, + maxBytes, + }); + if (!opened.ok) { + const error = + opened.reason === "validation" ? new Error("security validation failed") : opened.error; + return { + ok: false, + resolvedPath, + error, + message: `Failed to read ${label} file at ${resolvedPath}: ${String(error)}`, + }; + } + + try { + const raw = fs.readFileSync(opened.fd, "utf8"); + const secret = raw.trim(); + if (!secret) { + return { + ok: false, + resolvedPath, + message: `${label} file at ${resolvedPath} is empty.`, + }; + } + return { ok: true, secret, resolvedPath }; + } catch (error) { + return { + ok: false, + resolvedPath, + error, + message: `Failed to read ${label} file at ${resolvedPath}: ${String(error)}`, + }; + } finally { + fs.closeSync(opened.fd); + } +} + +export function readSecretFileSync( + filePath: string, + label: string, + options: SecretFileReadOptions = {}, +): string { + const result = loadSecretFileSync(filePath, label, options); + if (result.ok) { + return result.secret; + } + throw new Error(result.message, result.error ? { cause: result.error } : undefined); +} + +export function tryReadSecretFileSync( + filePath: string | undefined, + label: string, + options: SecretFileReadOptions = {}, +): string | undefined { + if (!filePath?.trim()) { + return undefined; + } + const result = loadSecretFileSync(filePath, label, options); + return result.ok ? result.secret : undefined; +} diff --git a/src/line/accounts.test.ts b/src/line/accounts.test.ts index 6a4770c379a..06433f6f8e7 100644 --- a/src/line/accounts.test.ts +++ b/src/line/accounts.test.ts @@ -1,3 +1,6 @@ +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; import { describe, it, expect, beforeEach, afterEach } from "vitest"; import type { OpenClawConfig } from "../config/config.js"; import { @@ -97,6 +100,33 @@ describe("LINE accounts", () => { expect(account.channelSecret).toBe(""); expect(account.tokenSource).toBe("none"); }); + + it.runIf(process.platform !== "win32")("rejects symlinked token and secret files", () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-line-account-")); + const tokenFile = path.join(dir, "token.txt"); + const tokenLink = path.join(dir, "token-link.txt"); + const secretFile = path.join(dir, "secret.txt"); + const secretLink = path.join(dir, "secret-link.txt"); + fs.writeFileSync(tokenFile, "file-token\n", "utf8"); + fs.writeFileSync(secretFile, "file-secret\n", "utf8"); + fs.symlinkSync(tokenFile, tokenLink); + fs.symlinkSync(secretFile, secretLink); + + const cfg: OpenClawConfig = { + channels: { + line: { + tokenFile: tokenLink, + secretFile: secretLink, + }, + }, + }; + + const account = resolveLineAccount({ cfg }); + expect(account.channelAccessToken).toBe(""); + expect(account.channelSecret).toBe(""); + expect(account.tokenSource).toBe("none"); + fs.rmSync(dir, { recursive: true, force: true }); + }); }); describe("resolveDefaultLineAccountId", () => { diff --git a/src/line/accounts.ts b/src/line/accounts.ts index 6e93cf40fea..9047ceccaa3 100644 --- a/src/line/accounts.ts +++ b/src/line/accounts.ts @@ -1,5 +1,5 @@ -import fs from "node:fs"; import type { OpenClawConfig } from "../config/config.js"; +import { tryReadSecretFileSync } from "../infra/secret-file.js"; import { DEFAULT_ACCOUNT_ID, normalizeAccountId as normalizeSharedAccountId, @@ -16,14 +16,7 @@ import type { export { DEFAULT_ACCOUNT_ID } from "../routing/account-id.js"; function readFileIfExists(filePath: string | undefined): string | undefined { - if (!filePath) { - return undefined; - } - try { - return fs.readFileSync(filePath, "utf-8").trim(); - } catch { - return undefined; - } + return tryReadSecretFileSync(filePath, "LINE credential file", { rejectSymlink: true }); } function resolveToken(params: { diff --git a/src/plugin-sdk/core.ts b/src/plugin-sdk/core.ts index d70ea17738f..5a74c6e089c 100644 --- a/src/plugin-sdk/core.ts +++ b/src/plugin-sdk/core.ts @@ -18,6 +18,13 @@ export { listDevicePairing, rejectDevicePairing, } from "../infra/device-pairing.js"; +export { + DEFAULT_SECRET_FILE_MAX_BYTES, + loadSecretFileSync, + readSecretFileSync, + tryReadSecretFileSync, +} from "../infra/secret-file.js"; +export type { SecretFileReadOptions, SecretFileReadResult } from "../infra/secret-file.js"; export { runPluginCommandWithTimeout, diff --git a/src/telegram/account-inspect.test.ts b/src/telegram/account-inspect.test.ts index 83ad113202b..b25bd223667 100644 --- a/src/telegram/account-inspect.test.ts +++ b/src/telegram/account-inspect.test.ts @@ -1,3 +1,6 @@ +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; import { describe, expect, it } from "vitest"; import type { OpenClawConfig } from "../config/config.js"; import { withEnv } from "../test-utils/env.js"; @@ -76,4 +79,29 @@ describe("inspectTelegramAccount SecretRef resolution", () => { expect(account.token).toBe(""); }); }); + + it.runIf(process.platform !== "win32")( + "treats symlinked token files as configured_unavailable", + () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-telegram-inspect-")); + const tokenFile = path.join(dir, "token.txt"); + const tokenLink = path.join(dir, "token-link.txt"); + fs.writeFileSync(tokenFile, "123:token\n", "utf8"); + fs.symlinkSync(tokenFile, tokenLink); + + const cfg: OpenClawConfig = { + channels: { + telegram: { + tokenFile: tokenLink, + }, + }, + }; + + const account = inspectTelegramAccount({ cfg, accountId: "default" }); + expect(account.tokenSource).toBe("tokenFile"); + expect(account.tokenStatus).toBe("configured_unavailable"); + expect(account.token).toBe(""); + fs.rmSync(dir, { recursive: true, force: true }); + }, + ); }); diff --git a/src/telegram/account-inspect.ts b/src/telegram/account-inspect.ts index 0ffbe0281ff..2db9db06e3e 100644 --- a/src/telegram/account-inspect.ts +++ b/src/telegram/account-inspect.ts @@ -1,4 +1,3 @@ -import fs from "node:fs"; import type { OpenClawConfig } from "../config/config.js"; import { coerceSecretRef, @@ -6,6 +5,7 @@ import { normalizeSecretInputString, } from "../config/types.secrets.js"; import type { TelegramAccountConfig } from "../config/types.telegram.js"; +import { tryReadSecretFileSync } from "../infra/secret-file.js"; import { resolveAccountWithDefaultFallback } from "../plugin-sdk/account-resolution.js"; import { DEFAULT_ACCOUNT_ID, normalizeAccountId } from "../routing/session-key.js"; import { resolveDefaultSecretProviderAlias } from "../secrets/ref-contract.js"; @@ -37,27 +37,14 @@ function inspectTokenFile(pathValue: unknown): { if (!tokenFile) { return null; } - if (!fs.existsSync(tokenFile)) { - return { - token: "", - tokenSource: "tokenFile", - tokenStatus: "configured_unavailable", - }; - } - try { - const token = fs.readFileSync(tokenFile, "utf-8").trim(); - return { - token, - tokenSource: "tokenFile", - tokenStatus: token ? "available" : "configured_unavailable", - }; - } catch { - return { - token: "", - tokenSource: "tokenFile", - tokenStatus: "configured_unavailable", - }; - } + const token = tryReadSecretFileSync(tokenFile, "Telegram bot token", { + rejectSymlink: true, + }); + return { + token: token ?? "", + tokenSource: "tokenFile", + tokenStatus: token ? "available" : "configured_unavailable", + }; } function canResolveEnvSecretRefInReadOnlyPath(params: { diff --git a/src/telegram/token.test.ts b/src/telegram/token.test.ts index fa1dc037b0c..f888ddbfc36 100644 --- a/src/telegram/token.test.ts +++ b/src/telegram/token.test.ts @@ -48,6 +48,21 @@ describe("resolveTelegramToken", () => { fs.rmSync(dir, { recursive: true, force: true }); }); + it.runIf(process.platform !== "win32")("rejects symlinked tokenFile paths", () => { + vi.stubEnv("TELEGRAM_BOT_TOKEN", ""); + const dir = withTempDir(); + const tokenFile = path.join(dir, "token.txt"); + const tokenLink = path.join(dir, "token-link.txt"); + fs.writeFileSync(tokenFile, "file-token\n", "utf-8"); + fs.symlinkSync(tokenFile, tokenLink); + + const cfg = { channels: { telegram: { tokenFile: tokenLink } } } as OpenClawConfig; + const res = resolveTelegramToken(cfg); + expect(res.token).toBe(""); + expect(res.source).toBe("none"); + fs.rmSync(dir, { recursive: true, force: true }); + }); + it("falls back to config token when no env or tokenFile", () => { vi.stubEnv("TELEGRAM_BOT_TOKEN", ""); const cfg = { diff --git a/src/telegram/token.ts b/src/telegram/token.ts index 81b0ac49d70..3615c703582 100644 --- a/src/telegram/token.ts +++ b/src/telegram/token.ts @@ -1,8 +1,8 @@ -import fs from "node:fs"; import type { BaseTokenResolution } from "../channels/plugins/types.js"; import type { OpenClawConfig } from "../config/config.js"; import { normalizeResolvedSecretInputString } from "../config/types.secrets.js"; import type { TelegramAccountConfig } from "../config/types.telegram.js"; +import { tryReadSecretFileSync } from "../infra/secret-file.js"; import { DEFAULT_ACCOUNT_ID, normalizeAccountId } from "../routing/session-key.js"; export type TelegramTokenSource = "env" | "tokenFile" | "config" | "none"; @@ -46,23 +46,17 @@ export function resolveTelegramToken( ); const accountTokenFile = accountCfg?.tokenFile?.trim(); if (accountTokenFile) { - if (!fs.existsSync(accountTokenFile)) { - opts.logMissingFile?.( - `channels.telegram.accounts.${accountId}.tokenFile not found: ${accountTokenFile}`, - ); - return { token: "", source: "none" }; - } - try { - const token = fs.readFileSync(accountTokenFile, "utf-8").trim(); - if (token) { - return { token, source: "tokenFile" }; - } - } catch (err) { - opts.logMissingFile?.( - `channels.telegram.accounts.${accountId}.tokenFile read failed: ${String(err)}`, - ); - return { token: "", source: "none" }; + const token = tryReadSecretFileSync( + accountTokenFile, + `channels.telegram.accounts.${accountId}.tokenFile`, + { rejectSymlink: true }, + ); + if (token) { + return { token, source: "tokenFile" }; } + opts.logMissingFile?.( + `channels.telegram.accounts.${accountId}.tokenFile not found or unreadable: ${accountTokenFile}`, + ); return { token: "", source: "none" }; } @@ -77,19 +71,14 @@ export function resolveTelegramToken( const allowEnv = accountId === DEFAULT_ACCOUNT_ID; const tokenFile = telegramCfg?.tokenFile?.trim(); if (tokenFile) { - if (!fs.existsSync(tokenFile)) { - opts.logMissingFile?.(`channels.telegram.tokenFile not found: ${tokenFile}`); - return { token: "", source: "none" }; - } - try { - const token = fs.readFileSync(tokenFile, "utf-8").trim(); - if (token) { - return { token, source: "tokenFile" }; - } - } catch (err) { - opts.logMissingFile?.(`channels.telegram.tokenFile read failed: ${String(err)}`); - return { token: "", source: "none" }; + const token = tryReadSecretFileSync(tokenFile, "channels.telegram.tokenFile", { + rejectSymlink: true, + }); + if (token) { + return { token, source: "tokenFile" }; } + opts.logMissingFile?.(`channels.telegram.tokenFile not found or unreadable: ${tokenFile}`); + return { token: "", source: "none" }; } const configToken = normalizeResolvedSecretInputString({