fix(secrets): harden Windows ACL fallback and strip BOM (#70662)

Fail closed when Windows ACL checks cannot be verified for file and exec secret providers unless the provider explicitly opts into allowInsecurePath. Strip UTF-8 BOMs from file-backed secrets and document the trusted-path override.\n\nThanks @zhanggpcsu.

CHANGELOG.md

@@ -10,6 +10,7 @@ Docs: https://docs.openclaw.ai
 
 - Android/security: stop `ASK_OPENCLAW` intents from auto-sending injected prompts, so external app actions only prefill the draft instead of dispatching it immediately. (#70714) Thanks @vincentkoc.
 - Control UI/chat: queue Stop-button aborts across Gateway reconnects so a disconnected active run is canceled on reconnect instead of only clearing local UI state. (#70673) Thanks @chinar-amrutkar.
+- Secrets/Windows: strip UTF-8 BOMs from file-backed secrets and keep unavailable ACL checks fail-closed unless trusted file or exec providers explicitly opt into `allowInsecurePath`. (#70662) Thanks @zhanggpcsu.
 - QQBot/security: require framework auth for `/bot-approve` so unauthorized QQ senders cannot change exec approval settings through the unauthenticated pre-dispatch slash-command path. (#70706) Thanks @vincentkoc.
 - MCP/tools: stop the ACPX OpenClaw tools bridge from listing or invoking owner-only tools such as `cron`, closing a privilege-escalation path for non-owner MCP callers. (#70698) Thanks @vincentkoc.
 - Feishu/onboarding: load Feishu setup surfaces through a setup-only barrel so first-run setup no longer imports Feishu's Lark SDK before bundled runtime deps are staged. (#70339) Thanks @andrejtr.

docs/cli/config.md

@@ -203,6 +203,7 @@ File provider (`--provider-source file`):
 - `--provider-path <path>` (required)
 - `--provider-mode <singleValue|json>`
 - `--provider-max-bytes <bytes>`
+- `--provider-allow-insecure-path`
 
 Exec provider (`--provider-source exec`):
 

docs/gateway/configuration-reference.md

@@ -3473,6 +3473,7 @@ Validation:
 Notes:
 
 - `file` provider supports `mode: "json"` and `mode: "singleValue"` (`id` must be `"value"` in singleValue mode).
+- File and exec provider paths fail closed when Windows ACL verification is unavailable. Set `allowInsecurePath: true` only for trusted paths that cannot be verified.
 - `exec` provider requires an absolute `command` path and uses protocol payloads on stdin/stdout.
 - By default, symlink command paths are rejected. Set `allowSymlinkCommand: true` to allow symlink paths while validating the resolved target path.
 - If `trustedDirs` is configured, the trusted-dir check applies to the resolved target path.

src/cli/config-cli.test.ts

@@ -885,6 +885,7 @@ describe("config cli", () => {
         "/tmp/vault.json",
         "--provider-mode",
         "json",
+        "--provider-allow-insecure-path",
       ]);
 
       expect(mockWriteConfigFile).toHaveBeenCalledTimes(1);
@@ -893,6 +894,7 @@ describe("config cli", () => {
         source: "file",
         path: "/tmp/vault.json",
         mode: "json",
+        allowInsecurePath: true,
       });
     });
 

src/cli/config-cli.ts

@@ -712,6 +712,7 @@ function buildProviderFromBuilder(opts: ConfigSetOptions): SecretProviderConfig
       ...(mode ? { mode } : {}),
       ...(timeoutMs !== undefined ? { timeoutMs } : {}),
       ...(maxBytes !== undefined ? { maxBytes } : {}),
+      ...(opts.providerAllowInsecurePath ? { allowInsecurePath: true } : {}),
     };
   } else {
     const command = opts.providerCommand?.trim();
@@ -1599,7 +1600,7 @@ export function registerConfigCli(program: Command) {
     )
     .option(
       "--provider-allow-insecure-path",
-      "Provider builder (exec): bypass strict path permission checks",
+      "Provider builder (file|exec): bypass strict path permission checks",
       false,
     )
     .option(

src/config/config.secrets-schema.test.ts

@@ -30,6 +30,7 @@ describe("config secret refs schema", () => {
             path: "~/.openclaw/secrets.json",
             mode: "json",
             timeoutMs: 10_000,
+            allowInsecurePath: true,
           },
           vault: {
             source: "exec",

src/config/schema.base.generated.ts

@@ -824,6 +824,9 @@ export const GENERATED_BASE_CONFIG_SCHEMA: BaseConfigSchemaResponse = {
                       exclusiveMinimum: 0,
                       maximum: 20971520,
                     },
+                    allowInsecurePath: {
+                      type: "boolean",
+                    },
                   },
                   required: ["source", "path"],
                   additionalProperties: false,

src/config/types.secrets.ts

@@ -233,6 +233,7 @@ export type FileSecretProviderConfig = {
   mode?: FileSecretProviderMode;
   timeoutMs?: number;
   maxBytes?: number;
+  allowInsecurePath?: boolean;
 };
 
 export type ExecSecretProviderConfig = {

src/config/zod-schema.core.ts

@@ -102,6 +102,7 @@ const SecretsFileProviderSchema = z
       .positive()
       .max(20 * 1024 * 1024)
       .optional(),
+    allowInsecurePath: z.boolean().optional(),
   })
   .strict();
 

src/secrets/configure.ts

@@ -446,6 +446,13 @@ async function promptFileProvider(
     initialValue: base?.maxBytes,
     max: 20 * 1024 * 1024,
   });
+  const allowInsecurePath = assertNoCancel(
+    await confirm({
+      message: "Allow insecure file path checks?",
+      initialValue: base?.allowInsecurePath ?? false,
+    }),
+    "Secrets configure cancelled.",
+  );
 
   return {
     source: "file",
@@ -453,6 +460,7 @@ async function promptFileProvider(
     mode,
     ...(timeoutMs ? { timeoutMs } : {}),
     ...(maxBytes ? { maxBytes } : {}),
+    ...(allowInsecurePath ? { allowInsecurePath: true } : {}),
   };
 }
 

src/secrets/resolve.test.ts

@@ -54,6 +54,7 @@ describe("secret ref resolver", () => {
     path: string;
     mode: "json" | "singleValue";
     timeoutMs?: number;
+    allowInsecurePath?: boolean;
   };
 
   function createExecProviderConfig(
@@ -454,4 +455,112 @@ describe("secret ref resolver", () => {
       ).rejects.toThrow(/Exec secret reference id must match|Secret reference id is empty/);
     }
   });
+
+  it("strips UTF-8 BOM from file provider payload before JSON parse", async () => {
+    const dir = await createCaseDir("bom-file");
+    const filePath = path.join(dir, "secrets-with-bom.json");
+    // Write JSON with UTF-8 BOM prefix (EF BB BF)
+    const bom = "\uFEFF";
+    await writeSecureFile(filePath, `${bom}{"apiKey":"sk-test-123"}`);
+
+    const value = await resolveSecretRefString(
+      { source: "file", provider: "filemain", id: "/apiKey" },
+      {
+        config: {
+          secrets: {
+            providers: {
+              filemain: createFileProviderConfig(filePath),
+            },
+          },
+        },
+      },
+    );
+    expect(value).toBe("sk-test-123");
+  });
+
+  it("strips UTF-8 BOM from file provider singleValue mode", async () => {
+    const dir = await createCaseDir("bom-single");
+    const filePath = path.join(dir, "secret-with-bom.txt");
+    const bom = "\uFEFF";
+    await writeSecureFile(filePath, `${bom}my-secret-value\n`);
+
+    const value = await resolveSecretRefString(
+      { source: "file", provider: "filemain", id: "value" },
+      {
+        config: {
+          secrets: {
+            providers: {
+              filemain: createFileProviderConfig(filePath, { mode: "singleValue" }),
+            },
+          },
+        },
+      },
+    );
+    expect(value).toBe("my-secret-value");
+  });
+
+  it("fails closed on Windows when file provider ACL source is unknown", async () => {
+    vi.spyOn(process, "platform", "get").mockReturnValue("win32" as unknown as NodeJS.Platform);
+
+    try {
+      const dir = await createCaseDir("win-acl");
+      const filePath = path.join(dir, "secrets.json");
+      await writeSecureFile(filePath, '{"token":"abc123"}');
+
+      await expect(
+        resolveSecretRefString(
+          { source: "file", provider: "filemain", id: "/token" },
+          {
+            config: {
+              secrets: {
+                providers: {
+                  filemain: createFileProviderConfig(filePath),
+                },
+              },
+            },
+          },
+        ),
+      ).rejects.toThrow(/ACL verification unavailable on Windows/);
+    } finally {
+      vi.restoreAllMocks();
+    }
+  });
+
+  it("allows trusted file provider opt-out when Windows ACL source is unknown", async () => {
+    vi.spyOn(process, "platform", "get").mockReturnValue("win32" as unknown as NodeJS.Platform);
+
+    try {
+      const dir = await createCaseDir("win-acl-opt-out");
+      const filePath = path.join(dir, "secrets.json");
+      await writeSecureFile(filePath, '{"token":"abc123"}');
+
+      const value = await resolveSecretRefString(
+        { source: "file", provider: "filemain", id: "/token" },
+        {
+          config: {
+            secrets: {
+              providers: {
+                filemain: createFileProviderConfig(filePath, { allowInsecurePath: true }),
+              },
+            },
+          },
+        },
+      );
+      expect(value).toBe("abc123");
+    } finally {
+      vi.restoreAllMocks();
+    }
+  });
+
+  it("fails closed on Windows when exec provider ACL source is unknown", async () => {
+    vi.spyOn(process, "platform", "get").mockReturnValue("win32" as unknown as NodeJS.Platform);
+
+    try {
+      await expect(resolveExecSecret(execProtocolV1ScriptPath)).rejects.toThrow(
+        /ACL verification unavailable on Windows/,
+      );
+    } finally {
+      vi.restoreAllMocks();
+    }
+  });
 });

src/secrets/resolve.ts

@@ -286,6 +286,7 @@ async function readFileProviderPayload(params: {
     const secureFilePath = await assertSecurePath({
       targetPath: filePath,
       label: `secrets.providers.${params.providerName}.path`,
+      allowInsecurePath: params.providerConfig.allowInsecurePath,
     });
     const timeoutMs = normalizePositiveInt(
       params.providerConfig.timeoutMs,
@@ -309,7 +310,7 @@ async function readFileProviderPayload(params: {
       if (payload.byteLength > maxBytes) {
         throw new Error(`File provider "${params.providerName}" exceeded maxBytes (${maxBytes}).`);
       }
-      const text = payload.toString("utf8");
+      const text = payload.toString("utf8").replace(/^\uFEFF/, "");
       if (params.providerConfig.mode === "singleValue") {
         return text.replace(/\r?\n$/, "");
       }

src/security/windows-acl.test.ts

@@ -440,7 +440,7 @@ Successfully processed 1 files`;
       expectInspectSuccess(result, 2);
       // /sid is passed so that account names are printed as SIDs, making the
       // audit locale-independent (fixes #35834).
-      expect(mockExec).toHaveBeenCalledWith("icacls", ["C:\\test\\file.txt", "/sid"]);
+      expect(mockExec).toHaveBeenCalledWith("icacls.exe", ["C:\\test\\file.txt", "/sid"]);
     });
 
     it("classifies *S-1-5-18 (SID form of SYSTEM from /sid) as trusted", async () => {
@@ -485,8 +485,8 @@ Successfully processed 1 files`;
       expectInspectSuccess(result, 2);
       expect(result.trusted).toHaveLength(2);
       expect(result.untrustedGroup).toHaveLength(0);
-      expect(mockExec).toHaveBeenNthCalledWith(1, "icacls", ["C:\\test\\file.txt", "/sid"]);
-      expect(mockExec).toHaveBeenNthCalledWith(2, "whoami", ["/user", "/fo", "csv", "/nh"]);
+      expect(mockExec).toHaveBeenNthCalledWith(1, "icacls.exe", ["C:\\test\\file.txt", "/sid"]);
+      expect(mockExec).toHaveBeenNthCalledWith(2, "whoami.exe", ["/user", "/fo", "csv", "/nh"]);
     });
 
     it("returns error state on exec failure", async () => {
@@ -534,6 +534,36 @@ Successfully processed 1 files`;
       expect(result.untrustedGroup).toHaveLength(1);
       expect(mockExec).toHaveBeenCalledTimes(2);
     });
+
+    it("uses SystemRoot for Windows system commands when available", async () => {
+      const mockExec = vi
+        .fn()
+        .mockResolvedValueOnce({
+          stdout: "C:\\test\\file.txt *S-1-5-21-111-222-333-1001:(F)",
+          stderr: "",
+        })
+        .mockResolvedValueOnce({
+          stdout: '"mock-host\\\\MockUser","S-1-5-21-111-222-333-1001"\r\n',
+          stderr: "",
+        });
+
+      const result = await inspectWindowsAcl("C:\\test\\file.txt", {
+        exec: mockExec,
+        env: { SystemRoot: "C:\\Windows" },
+      });
+
+      expectInspectSuccess(result, 1);
+      expect(mockExec).toHaveBeenNthCalledWith(1, "C:\\Windows\\System32\\icacls.exe", [
+        "C:\\test\\file.txt",
+        "/sid",
+      ]);
+      expect(mockExec).toHaveBeenNthCalledWith(2, "C:\\Windows\\System32\\whoami.exe", [
+        "/user",
+        "/fo",
+        "csv",
+        "/nh",
+      ]);
+    });
   });
 
   describe("formatWindowsAclSummary", () => {

src/security/windows-acl.ts

@@ -1,4 +1,5 @@
 import os from "node:os";
+import path from "node:path";
 import { runExec } from "../process/exec.js";
 import { normalizeLowercaseStringOrEmpty } from "../shared/string-coerce.js";
 
@@ -100,6 +101,15 @@ function buildTrustedPrincipals(env?: NodeJS.ProcessEnv): Set<string> {
   return trusted;
 }
 
+function resolveWindowsSystemCommand(command: string, env?: NodeJS.ProcessEnv): string {
+  const root =
+    env?.SystemRoot?.trim() ||
+    env?.SYSTEMROOT?.trim() ||
+    env?.windir?.trim() ||
+    env?.WINDIR?.trim();
+  return root ? path.win32.join(root, "System32", command) : command;
+}
+
 function classifyPrincipal(
   principal: string,
   trustedPrincipals: Set<string>,
@@ -270,9 +280,17 @@ export function summarizeWindowsAcl(
   return { trusted, untrustedWorld, untrustedGroup };
 }
 
-async function resolveCurrentUserSid(exec: ExecFn): Promise<string | null> {
+async function resolveCurrentUserSid(
+  exec: ExecFn,
+  env?: NodeJS.ProcessEnv,
+): Promise<string | null> {
   try {
-    const { stdout, stderr } = await exec("whoami", ["/user", "/fo", "csv", "/nh"]);
+    const { stdout, stderr } = await exec(resolveWindowsSystemCommand("whoami.exe", env), [
+      "/user",
+      "/fo",
+      "csv",
+      "/nh",
+    ]);
     const match = `${stdout}\n${stderr}`.match(/\*?S-\d+-\d+(?:-\d+)+/i);
     return match ? normalizeSid(match[0]) : null;
   } catch (err) {
@@ -297,7 +315,10 @@ export async function inspectWindowsAcl(
     // Windows (Russian, Chinese, etc.) where icacls prints Cyrillic / CJK
     // characters that may be garbled when Node reads them in the wrong code
     // page.  Fixes #35834.
-    const { stdout, stderr } = await exec("icacls", [targetPath, "/sid"]);
+    const { stdout, stderr } = await exec(resolveWindowsSystemCommand("icacls.exe", opts?.env), [
+      targetPath,
+      "/sid",
+    ]);
     const output = `${stdout}\n${stderr}`.trim();
     const entries = parseIcaclsOutput(output, targetPath);
     let effectiveEnv = opts?.env;
@@ -307,7 +328,7 @@ export async function inspectWindowsAcl(
       !effectiveEnv?.USERSID &&
       untrustedGroup.some((entry) => SID_RE.test(normalize(entry.principal)));
     if (needsUserSidResolution) {
-      const currentUserSid = await resolveCurrentUserSid(exec);
+      const currentUserSid = await resolveCurrentUserSid(exec, effectiveEnv);
       if (currentUserSid) {
         effectiveEnv = { ...effectiveEnv, USERSID: currentUserSid };
         ({ trusted, untrustedWorld, untrustedGroup } = summarizeWindowsAcl(entries, effectiveEnv));