mirror of
https://github.com/openclaw/openclaw.git
synced 2026-07-21 10:16:44 +00:00
fix(push-web): ignore blank VAPID environment overrides (#109609)
* fix(push-web): ignore blank VAPID environment values * fix: normalize migrated web push VAPID inputs Co-authored-by: mushuiyu886 <yang.haoyu@xydigit.com> --------- Co-authored-by: Peter Steinberger <steipete@gmail.com>
This commit is contained in:
co-authored by
Peter Steinberger
parent
ced50f88e9
commit
7af9358841
@@ -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";
|
||||
|
||||
@@ -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<VapidKeyPair>
|
||||
}
|
||||
|
||||
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 ---
|
||||
|
||||
@@ -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<VapidKeyPair>).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 () => {
|
||||
|
||||
@@ -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<string, WebPushSubscription>
|
||||
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<ParsedLegacyState> {
|
||||
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> | 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) {
|
||||
|
||||
Reference in New Issue
Block a user