diff --git a/src/infra/push-web.test.ts b/src/infra/push-web.test.ts index 6edc4bf2ac5d..456b8168a828 100644 --- a/src/infra/push-web.test.ts +++ b/src/infra/push-web.test.ts @@ -112,9 +112,9 @@ describe("resolveVapidKeys", () => { "OPENCLAW_VAPID_PRIVATE_KEY", "OPENCLAW_VAPID_SUBJECT", ]); - setTestEnvValue("OPENCLAW_VAPID_PUBLIC_KEY", environmentKeys.publicKey); - setTestEnvValue("OPENCLAW_VAPID_PRIVATE_KEY", environmentKeys.privateKey); - setTestEnvValue("OPENCLAW_VAPID_SUBJECT", environmentKeys.subject); + setTestEnvValue("OPENCLAW_VAPID_PUBLIC_KEY", ` ${environmentKeys.publicKey} `); + setTestEnvValue("OPENCLAW_VAPID_PRIVATE_KEY", ` ${environmentKeys.privateKey} `); + setTestEnvValue("OPENCLAW_VAPID_SUBJECT", ` ${environmentKeys.subject} `); try { await expect(resolveVapidKeys(tmpDir)).resolves.toEqual(environmentKeys); expect(readPersistedVapidKeyPair(tmpDir)).toBeNull(); @@ -124,6 +124,31 @@ describe("resolveVapidKeys", () => { } }); + it("treats blank environment values as unset", async () => { + const envSnapshot = captureEnv([ + "OPENCLAW_VAPID_PUBLIC_KEY", + "OPENCLAW_VAPID_PRIVATE_KEY", + "OPENCLAW_VAPID_SUBJECT", + ]); + setTestEnvValue("OPENCLAW_VAPID_PUBLIC_KEY", " "); + setTestEnvValue("OPENCLAW_VAPID_PRIVATE_KEY", " "); + setTestEnvValue("OPENCLAW_VAPID_SUBJECT", " "); + try { + const keys = await resolveVapidKeys(tmpDir); + expect(keys).toEqual( + createWebPushVapidKeyPair( + "test-public-key-base64url", + "test-private-key-base64url", + "https://openclaw.ai", + ), + ); + expect(readPersistedVapidKeyPair(tmpDir)).toEqual(keys); + expect(vi.mocked(webPush.generateVAPIDKeys)).toHaveBeenCalledTimes(1); + } finally { + envSnapshot.restore(); + } + }); + it("applies the current subject to a persisted identity", async () => { const initial = await resolveVapidKeys(tmpDir); process.env.OPENCLAW_VAPID_SUBJECT = "mailto:changed@test.com"; diff --git a/src/infra/push-web.ts b/src/infra/push-web.ts index 62c7f66dfa7e..17dd1de2576b 100644 --- a/src/infra/push-web.ts +++ b/src/infra/push-web.ts @@ -2,7 +2,7 @@ import { randomUUID } from "node:crypto"; import fs from "node:fs"; import path from "node:path"; -import { expectDefined } from "@openclaw/normalization-core"; +import { expectDefined, normalizeOptionalString } from "@openclaw/normalization-core"; import { resolveStateDir } from "../config/paths.js"; import { createLazyRuntimeModule } from "../shared/lazy-runtime.js"; import { @@ -109,15 +109,17 @@ export async function resolveVapidKeys(baseDir?: string): Promise } function resolveVapidSubjectFromEnv(): string { - return process.env.OPENCLAW_VAPID_SUBJECT || DEFAULT_WEB_PUSH_VAPID_SUBJECT; + return ( + normalizeOptionalString(process.env.OPENCLAW_VAPID_SUBJECT) ?? DEFAULT_WEB_PUSH_VAPID_SUBJECT + ); } function resolveVapidPublicKeyFromEnv(): string | undefined { - return process.env.OPENCLAW_VAPID_PUBLIC_KEY || undefined; + return normalizeOptionalString(process.env.OPENCLAW_VAPID_PUBLIC_KEY); } function resolveVapidPrivateKeyFromEnv(): string | undefined { - return process.env.OPENCLAW_VAPID_PRIVATE_KEY || undefined; + return normalizeOptionalString(process.env.OPENCLAW_VAPID_PRIVATE_KEY); } // --- Subscription CRUD --- diff --git a/src/infra/state-migrations.web-push.test.ts b/src/infra/state-migrations.web-push.test.ts index b2bfc6a7f376..c1c311a27cc0 100644 --- a/src/infra/state-migrations.web-push.test.ts +++ b/src/infra/state-migrations.web-push.test.ts @@ -38,7 +38,7 @@ describe("legacy Web Push Doctor migration", () => { function useStateDir(): string { const stateDir = tempDirs.make("openclaw-web-push-migration-"); - envSnapshot ??= captureEnv(["OPENCLAW_STATE_DIR"]); + envSnapshot ??= captureEnv(["OPENCLAW_STATE_DIR", "OPENCLAW_VAPID_SUBJECT"]); setTestEnvValue("OPENCLAW_STATE_DIR", stateDir); return stateDir; } @@ -198,23 +198,56 @@ describe("legacy Web Push Doctor migration", () => { }); it.each([ - ["missing", undefined], - ["empty", ""], - ])("normalizes a %s legacy VAPID subject", async (_label, subject) => { + ["missing legacy subject", undefined, undefined, DEFAULT_WEB_PUSH_VAPID_SUBJECT], + ["empty legacy subject", "", undefined, DEFAULT_WEB_PUSH_VAPID_SUBJECT], + ["blank injected subject", undefined, " ", DEFAULT_WEB_PUSH_VAPID_SUBJECT], + [ + "padded injected subject", + undefined, + " mailto:injected@example.com ", + "mailto:injected@example.com", + ], + [ + "padded legacy subject", + " mailto:legacy@example.com ", + "mailto:injected@example.com", + "mailto:legacy@example.com", + ], + ])("normalizes a %s", async (_label, legacySubject, injectedSubject, expectedSubject) => { const stateDir = useStateDir(); - const legacyKeys = vapidKeys({ subject: subject ?? "" }); - if (subject === undefined) { + setTestEnvValue("OPENCLAW_VAPID_SUBJECT", "mailto:ambient@example.com"); + const legacyKeys = vapidKeys({ subject: legacySubject ?? "" }); + if (legacySubject === undefined) { delete (legacyKeys as Partial).subject; } await writeLegacyState({ stateDir, vapid: legacyKeys }); const result = await migrateLegacyWebPush({ detected: detectLegacyWebPush({ stateDir, doctorOnlyStateMigrations: true }), + env: { ...process.env, OPENCLAW_VAPID_SUBJECT: injectedSubject }, stateDir, }); expect(result.warnings).toEqual([]); - expect(readPersistedVapidKeyPair(stateDir)?.subject).toBe(DEFAULT_WEB_PUSH_VAPID_SUBJECT); + expect(readPersistedVapidKeyPair(stateDir)?.subject).toBe(expectedSubject); + }); + + it("rejects a present non-string legacy VAPID subject", async () => { + const stateDir = useStateDir(); + const paths = await writeLegacyState({ + stateDir, + vapid: { ...vapidKeys(), subject: 42 }, + }); + + const result = await migrateLegacyWebPush({ + detected: detectLegacyWebPush({ stateDir, doctorOnlyStateMigrations: true }), + env: { ...process.env, OPENCLAW_VAPID_SUBJECT: "mailto:fallback@example.com" }, + stateDir, + }); + + expect(result.warnings[0]).toContain("VAPID keys are invalid"); + expect(readPersistedVapidKeyPair(stateDir)).toBeNull(); + expect(fs.existsSync(paths.vapidKeysPath!)).toBe(true); }); it("removes an empty valid store only after opening SQLite", async () => { diff --git a/src/infra/state-migrations.web-push.ts b/src/infra/state-migrations.web-push.ts index c7f8741f641a..c6214afcf1d9 100644 --- a/src/infra/state-migrations.web-push.ts +++ b/src/infra/state-migrations.web-push.ts @@ -5,6 +5,7 @@ import path from "node:path"; import type { DatabaseSync } from "node:sqlite"; import { root, type Root } from "@openclaw/fs-safe"; import { isRecord } from "@openclaw/normalization-core/record-coerce"; +import { normalizeOptionalString } from "@openclaw/normalization-core/string-coerce"; import { runOpenClawStateWriteTransaction } from "../state/openclaw-state-db.js"; import { formatErrorMessage } from "./errors.js"; import { acquireGatewayLock, GatewayLockError } from "./gateway-lock.js"; @@ -247,20 +248,22 @@ function parseLegacySubscriptions(raw: string): Map return subscriptions; } -function parseLegacyVapidKeys(raw: string): VapidKeyPair { +function parseLegacyVapidKeys(raw: string, env: NodeJS.ProcessEnv): VapidKeyPair { const parsed = JSON.parse(raw) as unknown; if (!isRecord(parsed)) { throw new Error("legacy Web Push VAPID keys must be an object"); } assertOnlyKeys(parsed, VAPID_KEYS, "legacy Web Push VAPID keys"); + if (parsed.subject !== undefined && typeof parsed.subject !== "string") { + throw new Error("legacy Web Push VAPID keys are invalid"); + } const subject = - parsed.subject === undefined || parsed.subject === "" - ? process.env.OPENCLAW_VAPID_SUBJECT || DEFAULT_WEB_PUSH_VAPID_SUBJECT - : parsed.subject; + normalizeOptionalString(parsed.subject) ?? + normalizeOptionalString(env.OPENCLAW_VAPID_SUBJECT) ?? + DEFAULT_WEB_PUSH_VAPID_SUBJECT; if ( !isValidWebPushKey(parsed.publicKey) || !isValidWebPushKey(parsed.privateKey) || - typeof subject !== "string" || subject.length > 512 ) { throw new Error("legacy Web Push VAPID keys are invalid"); @@ -272,6 +275,7 @@ async function readLegacyState( stateRoot: Root, stateDir: string, detected: LegacyStateDetection["webPush"], + env: NodeJS.ProcessEnv, ): Promise { await recoverInterruptedClaim( stateRoot, @@ -305,7 +309,7 @@ async function readLegacyState( detected.vapidKeysPath, LEGACY_VAPID_KEYS_MAX_BYTES, ); - vapidKeys = parseLegacyVapidKeys(snapshot.raw); + vapidKeys = parseLegacyVapidKeys(snapshot.raw, env); snapshots.push(snapshot); } return { subscriptions, vapidKeys, snapshots }; @@ -582,6 +586,7 @@ async function migrateLegacyWebPushWithExclusiveStateOwnership(params: { stateRoot: Root; detected: LegacyStateDetection["webPush"]; stateDir: string; + env: NodeJS.ProcessEnv; beforeClaim?: () => void; beforeVerify?: () => void; removeSource?: (sourcePath: string) => Promise | void; @@ -595,7 +600,12 @@ async function migrateLegacyWebPushWithExclusiveStateOwnership(params: { let legacy: ParsedLegacyState; try { - legacy = await readLegacyState(params.stateRoot, params.stateDir, params.detected); + legacy = await readLegacyState( + params.stateRoot, + params.stateDir, + params.detected, + params.env, + ); } catch (error) { warnings.push(`Failed reading legacy Web Push state: ${String(error)}`); return { changes, warnings }; @@ -720,6 +730,7 @@ export async function migrateLegacyWebPush(params: { }); result = await migrateLegacyWebPushWithExclusiveStateOwnership({ ...params, + env, stateRoot, }); } catch (error) {