fix(security): reject unverifiable X-Cloud-Sig in cloud-sync HMAC check (#13679 PR A) (#13804)

Merged in the 2026-09-16 sweep of the maintainer's own open PRs, at the owner's explicit instruction. No push was made to the PR branch: the merge took the head as the owning session left it (verified OPEN, non-draft and MERGEABLE against the release tip immediately before merging).

changelog.d/fixes/13679-cloudsync-hmac-fail-open.md

@@ -0,0 +1 @@
+- **fix(auth):** `verifyCloudSignature()` no longer accepts an unverifiable `X-Cloud-Sig` when `OMNIROUTE_CLOUD_SYNC_SECRET` is unset — a forged/garbage signature is rejected outright, and the new opt-in `OMNIROUTE_CLOUD_SYNC_ENFORCE_SIGNATURE=true` flag rejects unsigned Cloud-sync payloads too (default stays legacy pass-through for v3.8.x; the default flips in v3.9) ([#13679](https://github.com/diegosouzapw/OmniRoute/issues/13679))

src/lib/cloudSync.ts

@@ -13,6 +13,14 @@ const CLOUD_SYNC_SECRET = process.env.OMNIROUTE_CLOUD_SYNC_SECRET || "";
 // hostile CLOUD_URL cannot silently swap user OAuth tokens.
 const CLOUD_SYNC_SECRETS_ENABLED = process.env.OMNIROUTE_CLOUD_SYNC_SECRETS === "true";
 
+// #13679 PR A — opt-in early enforcement of the "no secret configured" branch
+// below. Bringing the v3.9 enforce-by-default switch forward as an explicit
+// opt-out-safe flag: default OFF preserves v3.8.x back-compat for peers that
+// haven't rotated in a shared secret yet (an unsigned payload still passes).
+// Set to "true" to reject even an unsigned payload when no local secret is
+// configured — the default flips to enforced in v3.9.
+const CLOUD_SYNC_ENFORCE_SIGNATURE = process.env.OMNIROUTE_CLOUD_SYNC_ENFORCE_SIGNATURE === "true";
+
 type JsonRecord = Record<string, unknown>;
 
 function asRecord(value: unknown): JsonRecord {
@@ -40,18 +48,38 @@ function toDateMs(value: unknown): number {
 //   2. We verify the signature with `crypto.timingSafeEqual` before parsing the
 //      JSON, so a MITM on the CLOUD_URL channel — or a misconfigured CLOUD_URL
 //      pointing at an attacker — cannot inject providers/tokens.
-// If `OMNIROUTE_CLOUD_SYNC_SECRET` is unset, signature validation is logged but
-// not enforced (back-compat for users on v3.8.x who haven't issued a shared
-// secret yet). The enforce-by-default switch will flip in v3.9.
+// If `OMNIROUTE_CLOUD_SYNC_SECRET` is unset, a PRESENT signature is always
+// rejected (#13679 PR A — we have no key to check it against, so a signature
+// we cannot verify is treated as invalid rather than blindly trusted) and an
+// ABSENT signature falls through in legacy unverified mode by default
+// (back-compat for users on v3.8.x who haven't issued a shared secret yet;
+// opt in early via `OMNIROUTE_CLOUD_SYNC_ENFORCE_SIGNATURE=true`). The
+// enforce-by-default switch for the absent-signature case will flip in v3.9.
 export function verifyCloudSignature(rawBody: string, sigHeader: string | null): boolean {
   if (!CLOUD_SYNC_SECRET) {
     if (sigHeader) {
-      // We can't verify, but the server is at least trying. Pass through.
-      return true;
+      // We have no secret to verify against, so a signature we can't check is
+      // treated as invalid rather than passed through (#13679 PR A item (b) —
+      // closes the "forge any X-Cloud-Sig and it's accepted" fail-open case).
+      console.warn(
+        "[cloudSync] OMNIROUTE_CLOUD_SYNC_SECRET is not set but the Cloud response carries an " +
+          "X-Cloud-Sig header — rejecting an unverifiable signature. Set the secret to enable " +
+          "verification."
+      );
+      return false;
+    }
+    if (CLOUD_SYNC_ENFORCE_SIGNATURE) {
+      console.warn(
+        "[cloudSync] OMNIROUTE_CLOUD_SYNC_SECRET is not set and the Cloud response carries no " +
+          "X-Cloud-Sig, and OMNIROUTE_CLOUD_SYNC_ENFORCE_SIGNATURE=true — rejecting unsigned payload."
+      );
+      return false;
     }
     console.warn(
       "[cloudSync] OMNIROUTE_CLOUD_SYNC_SECRET is not set and the Cloud response carries no X-Cloud-Sig. " +
-        "Token sync runs in legacy unverified mode — set the secret to enforce HMAC verification."
+        "Token sync runs in legacy unverified mode — set the secret (or " +
+        "OMNIROUTE_CLOUD_SYNC_ENFORCE_SIGNATURE=true) to enforce HMAC verification. This legacy " +
+        "pass-through default will flip to enforced in v3.9."
     );
     return true;
   }
@@ -207,4 +235,9 @@ async function updateLocalTokens(cloudProviders: unknown) {
   }
 }
 
-export { CLOUD_URL, CLOUD_SYNC_TIMEOUT_MS, CLOUD_SYNC_SECRETS_ENABLED };
+export {
+  CLOUD_URL,
+  CLOUD_SYNC_TIMEOUT_MS,
+  CLOUD_SYNC_SECRETS_ENABLED,
+  CLOUD_SYNC_ENFORCE_SIGNATURE,
+};

tests/unit/security/cloudsync-signature-fail-open-13679.test.ts

@@ -0,0 +1,91 @@
+/**
+ * Regression for #13679 PR A: verifyCloudSignature() must not fail open when a
+ * present X-Cloud-Sig cannot be verified (no local OMNIROUTE_CLOUD_SYNC_SECRET).
+ *
+ * Before this fix: a garbage/forged X-Cloud-Sig header was ALWAYS accepted when
+ * the local secret was unset ("we can't verify, but the server is at least
+ * trying — pass through"). That let a MITM on the CLOUD_URL channel, or a
+ * misconfigured/compromised CLOUD_URL, forge any signature value and have it
+ * accepted — defeating the point of the signature check for any install that
+ * hasn't issued a shared secret yet.
+ *
+ * Fix (owner decision 2026-09-15, PR A):
+ *  (b) unconditional: a PRESENT-but-unverifiable signature is now rejected,
+ *      regardless of the opt-in enforce flag below.
+ *  (a) opt-in only (OMNIROUTE_CLOUD_SYNC_ENFORCE_SIGNATURE=true, default OFF):
+ *      also rejects a payload carrying NO signature at all. Default stays
+ *      legacy pass-through for v3.8.x peers that haven't rotated in a shared
+ *      secret yet — the default flips to enforced in v3.9.
+ */
+import { test } from "node:test";
+import assert from "node:assert/strict";
+
+const ORIGINAL_SECRET = process.env.OMNIROUTE_CLOUD_SYNC_SECRET;
+const ORIGINAL_ENFORCE = process.env.OMNIROUTE_CLOUD_SYNC_ENFORCE_SIGNATURE;
+
+function restoreEnv() {
+  if (ORIGINAL_SECRET === undefined) delete process.env.OMNIROUTE_CLOUD_SYNC_SECRET;
+  else process.env.OMNIROUTE_CLOUD_SYNC_SECRET = ORIGINAL_SECRET;
+  if (ORIGINAL_ENFORCE === undefined) delete process.env.OMNIROUTE_CLOUD_SYNC_ENFORCE_SIGNATURE;
+  else process.env.OMNIROUTE_CLOUD_SYNC_ENFORCE_SIGNATURE = ORIGINAL_ENFORCE;
+}
+
+test.after(restoreEnv);
+
+test("issue #13679: a present-but-unverifiable X-Cloud-Sig is rejected even without a local secret", async () => {
+  delete process.env.OMNIROUTE_CLOUD_SYNC_SECRET;
+  delete process.env.OMNIROUTE_CLOUD_SYNC_ENFORCE_SIGNATURE;
+  try {
+    const { verifyCloudSignature } = await import(
+      `../../../src/lib/cloudSync.ts?case=13679-forged-${Date.now()}-${Math.random()}`
+    );
+    const rawBody = JSON.stringify({ providers: [{ id: "evil", accessToken: "stolen" }] });
+    const forgedSig = "0".repeat(64);
+
+    assert.equal(
+      verifyCloudSignature(rawBody, forgedSig),
+      false,
+      "a garbage X-Cloud-Sig must be REJECTED even when the local secret is unset (fail-open closed)"
+    );
+  } finally {
+    restoreEnv();
+  }
+});
+
+test("issue #13679: legacy peers with NO X-Cloud-Sig header still pass by default (v3.8.x back-compat)", async () => {
+  delete process.env.OMNIROUTE_CLOUD_SYNC_SECRET;
+  delete process.env.OMNIROUTE_CLOUD_SYNC_ENFORCE_SIGNATURE;
+  try {
+    const { verifyCloudSignature } = await import(
+      `../../../src/lib/cloudSync.ts?case=13679-legacy-${Date.now()}-${Math.random()}`
+    );
+    const rawBody = JSON.stringify({ providers: [] });
+
+    assert.equal(
+      verifyCloudSignature(rawBody, null),
+      true,
+      "an unsigned payload from a legacy peer must still pass through by default in v3.8.x"
+    );
+  } finally {
+    restoreEnv();
+  }
+});
+
+test("issue #13679: OMNIROUTE_CLOUD_SYNC_ENFORCE_SIGNATURE=true rejects an unsigned payload too", async () => {
+  delete process.env.OMNIROUTE_CLOUD_SYNC_SECRET;
+  process.env.OMNIROUTE_CLOUD_SYNC_ENFORCE_SIGNATURE = "true";
+  try {
+    const { verifyCloudSignature } = await import(
+      `../../../src/lib/cloudSync.ts?case=13679-enforced-${Date.now()}-${Math.random()}`
+    );
+    const rawBody = JSON.stringify({ providers: [] });
+
+    assert.equal(
+      verifyCloudSignature(rawBody, null),
+      false,
+      "with the opt-in enforce flag set, an unsigned payload must be rejected"
+    );
+  } finally {
+    restoreEnv();
+  }
+});