fix(secrets): reject empty secret values and classify oversized input as validation (#121947)

docs/cli/secrets.md

@@ -53,7 +53,7 @@ openclaw secrets store rm <NAME>...
 openclaw secrets store import [--from <file>]
 ```
 
-Names must match `^[A-Z][A-Z0-9_]{0,127}$`. Values are limited to 64 KiB (65,536 UTF-8 bytes). `--kind secret|env` overrides automatic kind detection; otherwise names ending in common credential suffixes such as `_API_KEY`, `_TOKEN`, `_PASSWORD`, `_PRIVATE_KEY`, or `_SECRET` become `secret`, and other names become `env`.
+Names must match `^[A-Z][A-Z0-9_]{0,127}$`. Values are limited to 64 KiB (65,536 UTF-8 bytes); an oversized value is rejected with exit code 2 whether it arrives from stdin, `--value`, or `--value-file`. A `secret` entry may not be empty, because an empty credential cannot be diagnosed later (`get` refuses secret kinds and listings mask them); `env` entries may be empty. `--kind secret|env` overrides automatic kind detection; otherwise names ending in common credential suffixes such as `_API_KEY`, `_TOKEN`, `_PASSWORD`, `_PRIVATE_KEY`, or `_SECRET` become `secret`, and other names become `env`.
 
 ### Set values safely
 

docs/gateway/secrets.md

@@ -282,7 +282,7 @@ Entries have a `secret` or `env` kind. The kind controls CLI disclosure, not Sec
 
 `secret` entries are never injected into subprocess environments. They remain available only through `store` SecretRefs because plaintext env injection would bypass the store disclosure boundary; safe secret injection requires a future egress-substitution mechanism.
 
-Names use the same uppercase grammar as env SecretRefs, and each UTF-8 value is limited to 64 KiB (65,536 bytes). This supports PEM keys and service-account JSON without inheriting the smaller limits of ordinary environment variables.
+Names use the same uppercase grammar as env SecretRefs, and each UTF-8 value is limited to 64 KiB (65,536 bytes). A `secret` entry must carry a value; empty secrets are rejected because they would surface only as a confusing downstream auth failure. `env` entries may be empty. This supports PEM keys and service-account JSON without inheriting the smaller limits of ordinary environment variables.
 
 Reference an entry from `openclaw.json` with the `store` source:
 

src/cli/secrets-store-cli.test.ts

@@ -20,7 +20,12 @@ const mocks = await vi.hoisted(async () => {
 });
 
 vi.mock("../runtime.js", () => ({ defaultRuntime: mocks.defaultRuntime }));
-vi.mock("../secrets/store/secret-store.js", () => ({
+vi.mock("../secrets/store/secret-store.js", async (importOriginal) => ({
+  // Import the real validation error: the CLI classifies size/empty failures by it,
+  // and a stub class would silently change the mapped exit code.
+  SecretStoreValidationError: (
+    await importOriginal<typeof import("../secrets/store/secret-store.js")>()
+  ).SecretStoreValidationError,
   SECRET_STORE_VALUE_MAX_BYTES: 64 * 1024,
   listSecretStoreEntries: (params: unknown) => mocks.list(params),
   readSecretStoreValue: (params: unknown) => mocks.read(params),
@@ -79,6 +84,25 @@ describe("secrets store CLI", () => {
     expect(mocks.write).not.toHaveBeenCalled();
   });
 
+  it("reports an oversized --value-file as validation (exit 2), matching the stdin path", async () => {
+    const dir = await fs.mkdtemp(path.join(os.tmpdir(), "secret-store-cli-oversize-"));
+    const file = path.join(dir, "too-big.txt");
+    await fs.writeFile(file, "a".repeat(64 * 1024 + 1), "utf8");
+    try {
+      // Same violation as an oversized stdin value, so it must share exit code 2
+      // rather than falling through to the generic runtime-failure code.
+      await expect(
+        createProgram().parseAsync(
+          ["secrets", "store", "set", "BIG_ENV_VALUE", "--kind", "env", "--value-file", file],
+          { from: "user" },
+        ),
+      ).rejects.toThrow("__exit__:2");
+      expect(mocks.write).not.toHaveBeenCalled();
+    } finally {
+      await fs.rm(dir, { recursive: true, force: true });
+    }
+  });
+
   it("refuses get for secret entries without reading their values", async () => {
     mocks.list.mockReturnValue([{ name: "SERVICE_API_KEY", kind: "secret" }]);
     await expect(

src/cli/secrets-store-cli.ts

@@ -73,7 +73,8 @@ function mapStoreError(error: unknown): SecretStoreCliFailure {
   if (
     validation?.name === "SecretStoreValidationError" &&
     (validation.code === "SECRET_STORE_INVALID_NAME" ||
-      validation.code === "SECRET_STORE_VALUE_TOO_LARGE")
+      validation.code === "SECRET_STORE_VALUE_TOO_LARGE" ||
+      validation.code === "SECRET_STORE_VALUE_EMPTY")
   ) {
     return new SecretStoreCliFailure(2, validation.message ?? "Invalid secret store input.");
   }

src/cli/secrets-store-input.ts

@@ -3,7 +3,10 @@ import { password } from "@clack/prompts";
 import { readByteStreamWithLimit } from "@openclaw/media-core/read-byte-stream-with-limit";
 import { readFileDescriptorBounded } from "../infra/boundary-file-read.js";
 import { parseSecretStoreDotEnvText } from "../secrets/store/dotenv.js";
-import { SECRET_STORE_VALUE_MAX_BYTES } from "../secrets/store/secret-store.js";
+import {
+  SECRET_STORE_VALUE_MAX_BYTES,
+  SecretStoreValidationError,
+} from "../secrets/store/secret-store.js";
 
 const SECRET_STORE_IMPORT_MAX_BYTES = 16 * 1024 * 1024;
 
@@ -14,7 +17,13 @@ function stripOneTerminalNewline(value: string): string {
 async function readBoundedStdin(maxBytes: number): Promise<string> {
   const bytes = await readByteStreamWithLimit(process.stdin, {
     maxBytes,
-    onOverflow: ({ maxBytes: limit }) => new Error(`Stdin input exceeds ${limit} bytes.`),
+    // Oversized input is the same validation failure as an oversized stored value,
+    // so it must carry the typed code the CLI maps to exit 2 on every input path.
+    onOverflow: ({ maxBytes: limit }) =>
+      new SecretStoreValidationError(
+        "SECRET_STORE_VALUE_TOO_LARGE",
+        `Stdin input exceeds ${limit} bytes.`,
+      ),
   });
   return bytes.toString("utf8");
 }
@@ -27,7 +36,10 @@ async function readBoundedFile(pathname: string, maxBytes: number): Promise<stri
       throw new Error(`Input path is not a regular file: ${pathname}`);
     }
     if (stat.size > maxBytes) {
-      throw new Error(`Input file exceeds ${maxBytes} bytes: ${pathname}`);
+      throw new SecretStoreValidationError(
+        "SECRET_STORE_VALUE_TOO_LARGE",
+        `Input file exceeds ${maxBytes} bytes: ${pathname}`,
+      );
     }
     return (await readFileDescriptorBounded(file.fd, maxBytes)).toString("utf8");
   } finally {

src/secrets/store/secret-store.test.ts

@@ -137,6 +137,33 @@ describe("secret store", () => {
     ).toThrow(expect.objectContaining({ code: "SECRET_STORE_VALUE_TOO_LARGE" }));
   });
 
+  it("rejects an empty secret value but keeps empty env values legal", () => {
+    const database = createDatabaseOptions();
+    // A silently-empty secret (a failed `op read |` pipe) is undiagnosable later:
+    // get refuses secret kinds and listings mask them, so reject it at the writer.
+    expect(() =>
+      writeSecretStoreEntry({
+        scope: team,
+        name: "EMPTY_SECRET",
+        value: "",
+        kind: "secret",
+        updatedBy: null,
+        database,
+      }),
+    ).toThrow(expect.objectContaining({ code: "SECRET_STORE_VALUE_EMPTY" }));
+
+    writeSecretStoreEntry({
+      scope: team,
+      name: "EMPTY_ENV",
+      value: "",
+      kind: "env",
+      updatedBy: null,
+      database,
+    });
+    const stored = readSecretStoreValue({ scope: team, name: "EMPTY_ENV", database });
+    expect(stored.ok && stored.value).toBe("");
+  });
+
   it("treats a missing lazy table as empty and preserves schema version 6 on ensure", () => {
     const database = createDatabaseOptions();
     openOpenClawStateDatabase(database);

src/secrets/store/secret-store.ts

@@ -38,7 +38,10 @@ type SecretStoreReadError =
   | { code: "SECRET_STORE_INVALID_NAME"; message: string }
   | { code: "SECRET_STORE_UNAVAILABLE"; message: string; cause: unknown };
 
-type SecretStoreValidationCode = "SECRET_STORE_INVALID_NAME" | "SECRET_STORE_VALUE_TOO_LARGE";
+type SecretStoreValidationCode =
+  | "SECRET_STORE_INVALID_NAME"
+  | "SECRET_STORE_VALUE_TOO_LARGE"
+  | "SECRET_STORE_VALUE_EMPTY";
 
 export class SecretStoreValidationError extends Error {
   constructor(
@@ -66,7 +69,7 @@ function assertSecretStoreName(name: string): void {
   }
 }
 
-function assertSecretStoreValue(value: string): void {
+function assertSecretStoreValue(value: string, kind: SecretStoreKind): void {
   const bytes = Buffer.byteLength(value, "utf8");
   if (bytes > SECRET_STORE_VALUE_MAX_BYTES) {
     throw new SecretStoreValidationError(
@@ -74,6 +77,16 @@ function assertSecretStoreValue(value: string): void {
       `Secret store value exceeds ${SECRET_STORE_VALUE_MAX_BYTES} UTF-8 bytes.`,
     );
   }
+  // An empty credential is never meaningful and cannot be diagnosed later: `get`
+  // refuses secret kinds and listings mask them, so a silently-empty secret (a
+  // failed `op read |` pipe, for example) would surface only as a confusing 401.
+  // Env entries may legitimately be empty.
+  if (kind === "secret" && value.length === 0) {
+    throw new SecretStoreValidationError(
+      "SECRET_STORE_VALUE_EMPTY",
+      "Secret store value is empty. Secret entries require a value; check the command that produced it.",
+    );
+  }
 }
 
 function isMissingSecretStoreTableError(error: unknown): boolean {
@@ -188,7 +201,7 @@ export function writeSecretStoreEntry(params: {
   database?: OpenClawStateDatabaseOptions;
 }): void {
   assertSecretStoreName(params.name);
-  assertSecretStoreValue(params.value);
+  assertSecretStoreValue(params.value, params.kind);
   const { scopeKind, scopeId } = normalizeScope(params.scope);
   const now = Date.now();
   runOpenClawStateWriteTransaction(