fix: address review — env var VAPID precedence and bootstrap race guard

- 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
This commit is contained in:
eduardocruz
2026-03-13 00:40:57 -03:00
committed by Val Alexander
parent 15437b1153
commit 8858c3cf01
2 changed files with 50 additions and 15 deletions

View File

@@ -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", () => {

View File

@@ -89,25 +89,40 @@ async function persistState(state: WebPushRegistrationState, baseDir?: string):
// --- VAPID keys ---
export async function resolveVapidKeys(baseDir?: string): Promise<VapidKeyPair> {
const filePath = resolveVapidKeysPath(baseDir);
const existing = await readJsonFile<VapidKeyPair>(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<VapidKeyPair>(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 {