From 8858c3cf0119afef2fb374abd7d4d0b06118233b Mon Sep 17 00:00:00 2001 From: eduardocruz Date: Fri, 13 Mar 2026 00:40:57 -0300 Subject: [PATCH] =?UTF-8?q?fix:=20address=20review=20=E2=80=94=20env=20var?= =?UTF-8?q?=20VAPID=20precedence=20and=20bootstrap=20race=20guard?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Check OPENCLAW_VAPID_PUBLIC_KEY/PRIVATE_KEY env vars before falling back to persisted keys, so operators can share a stable VAPID identity across multiple gateway instances - Wrap VAPID key bootstrap in async lock to prevent concurrent first-run callers from generating different keypairs - Add test for env var precedence --- src/infra/push-web.test.ts | 20 +++++++++++++++++ src/infra/push-web.ts | 45 +++++++++++++++++++++++++------------- 2 files changed, 50 insertions(+), 15 deletions(-) diff --git a/src/infra/push-web.test.ts b/src/infra/push-web.test.ts index d2621a0de88..31fd692d950 100644 --- a/src/infra/push-web.test.ts +++ b/src/infra/push-web.test.ts @@ -49,6 +49,26 @@ describe("resolveVapidKeys", () => { expect(keys2.publicKey).toBe(keys.publicKey); expect(keys2.privateKey).toBe(keys.privateKey); }); + + it("prefers env vars over persisted keys", async () => { + // Persist keys first. + await resolveVapidKeys(tmpDir); + + // Set env overrides. + process.env.OPENCLAW_VAPID_PUBLIC_KEY = "env-public"; + process.env.OPENCLAW_VAPID_PRIVATE_KEY = "env-private"; + process.env.OPENCLAW_VAPID_SUBJECT = "mailto:env@test.com"; + try { + const keys = await resolveVapidKeys(tmpDir); + expect(keys.publicKey).toBe("env-public"); + expect(keys.privateKey).toBe("env-private"); + expect(keys.subject).toBe("mailto:env@test.com"); + } finally { + delete process.env.OPENCLAW_VAPID_PUBLIC_KEY; + delete process.env.OPENCLAW_VAPID_PRIVATE_KEY; + delete process.env.OPENCLAW_VAPID_SUBJECT; + } + }); }); describe("subscription CRUD", () => { diff --git a/src/infra/push-web.ts b/src/infra/push-web.ts index d81f4f52229..be4000faa06 100644 --- a/src/infra/push-web.ts +++ b/src/infra/push-web.ts @@ -89,25 +89,40 @@ async function persistState(state: WebPushRegistrationState, baseDir?: string): // --- VAPID keys --- export async function resolveVapidKeys(baseDir?: string): Promise { - const filePath = resolveVapidKeysPath(baseDir); - const existing = await readJsonFile(filePath); - if (existing?.publicKey && existing?.privateKey) { + // Env vars take precedence — allows operators to share a stable VAPID + // identity across multiple gateway instances. + const envPublic = resolveVapidPublicKeyFromEnv(); + const envPrivate = resolveVapidPrivateKeyFromEnv(); + if (envPublic && envPrivate) { return { - publicKey: existing.publicKey, - privateKey: existing.privateKey, - subject: existing.subject || resolveVapidSubjectFromEnv(), + publicKey: envPublic, + privateKey: envPrivate, + subject: resolveVapidSubjectFromEnv(), }; } - // Auto-generate and persist. - const keys = webPush.generateVAPIDKeys(); - const pair: VapidKeyPair = { - publicKey: keys.publicKey, - privateKey: keys.privateKey, - subject: resolveVapidSubjectFromEnv(), - }; - await writeJsonAtomic(filePath, pair, { trailingNewline: true }); - return pair; + // Fall back to persisted keys, generating on first use under a lock to + // prevent concurrent bootstraps from writing different keypairs. + return await withLock(async () => { + const filePath = resolveVapidKeysPath(baseDir); + const existing = await readJsonFile(filePath); + if (existing?.publicKey && existing?.privateKey) { + return { + publicKey: existing.publicKey, + privateKey: existing.privateKey, + subject: existing.subject || resolveVapidSubjectFromEnv(), + }; + } + + const keys = webPush.generateVAPIDKeys(); + const pair: VapidKeyPair = { + publicKey: keys.publicKey, + privateKey: keys.privateKey, + subject: resolveVapidSubjectFromEnv(), + }; + await writeJsonAtomic(filePath, pair, { trailingNewline: true }); + return pair; + }); } function resolveVapidSubjectFromEnv(): string {