fix(push): harden relay invalidation and url handling

This commit is contained in:
Nimrod Gutman
2026-03-11 22:26:17 +02:00
parent 4093adec71
commit 2fd046714e
10 changed files with 353 additions and 106 deletions

View File

@@ -47,7 +47,18 @@ struct PushBuildConfig {
guard let raw = bundle.object(forInfoDictionaryKey: key) as? String else { return nil }
let trimmed = raw.trimmingCharacters(in: .whitespacesAndNewlines)
guard !trimmed.isEmpty else { return nil }
return URL(string: trimmed)
guard let components = URLComponents(string: trimmed),
components.scheme?.lowercased() == "https",
let host = components.host,
!host.isEmpty,
components.user == nil,
components.password == nil,
components.query == nil,
components.fragment == nil
else {
return nil
}
return components.url
}
private static func readEnum<T: RawRepresentable>(

View File

@@ -109,7 +109,7 @@ actor PushRegistrationManager {
}
private static func isExpired(_ expiresAtMs: Int64?) -> Bool {
guard let expiresAtMs else { return false }
guard let expiresAtMs else { return true }
let nowMs = Int64(Date().timeIntervalSince1970 * 1000)
// Refresh shortly before expiry so reconnect-path republishes a live handle.
return expiresAtMs <= nowMs + 60_000

View File

@@ -10,7 +10,7 @@ const mocks = vi.hoisted(() => ({
ok: true,
params: rawParams,
})),
clearApnsRegistration: vi.fn(),
clearApnsRegistrationIfCurrent: vi.fn(),
loadApnsRegistration: vi.fn(),
resolveApnsAuthConfigFromEnv: vi.fn(),
resolveApnsRelayConfigFromEnv: vi.fn(),
@@ -33,7 +33,7 @@ vi.mock("../node-invoke-sanitize.js", () => ({
}));
vi.mock("../../infra/push-apns.js", () => ({
clearApnsRegistration: mocks.clearApnsRegistration,
clearApnsRegistrationIfCurrent: mocks.clearApnsRegistrationIfCurrent,
loadApnsRegistration: mocks.loadApnsRegistration,
resolveApnsAuthConfigFromEnv: mocks.resolveApnsAuthConfigFromEnv,
resolveApnsRelayConfigFromEnv: mocks.resolveApnsRelayConfigFromEnv,
@@ -197,7 +197,7 @@ describe("node.invoke APNs wake path", () => {
({ rawParams }: { rawParams: unknown }) => ({ ok: true, params: rawParams }),
);
mocks.loadApnsRegistration.mockClear();
mocks.clearApnsRegistration.mockClear();
mocks.clearApnsRegistrationIfCurrent.mockClear();
mocks.resolveApnsAuthConfigFromEnv.mockClear();
mocks.resolveApnsRelayConfigFromEnv.mockClear();
mocks.sendApnsBackgroundWake.mockClear();
@@ -311,7 +311,17 @@ describe("node.invoke APNs wake path", () => {
const call = respond.mock.calls[0] as RespondCall | undefined;
expect(call?.[0]).toBe(false);
expect(call?.[2]?.message).toBe("node not connected");
expect(mocks.clearApnsRegistration).toHaveBeenCalledWith("ios-node-stale");
expect(mocks.clearApnsRegistrationIfCurrent).toHaveBeenCalledWith({
nodeId: "ios-node-stale",
registration: {
nodeId: "ios-node-stale",
transport: "direct",
token: "abcd1234abcd1234abcd1234abcd1234",
topic: "ai.openclaw.ios",
environment: "sandbox",
updatedAtMs: 1,
},
});
});
it("does not clear relay registrations from wake failures", async () => {
@@ -380,7 +390,7 @@ describe("node.invoke APNs wake path", () => {
transport: "relay",
},
});
expect(mocks.clearApnsRegistration).not.toHaveBeenCalled();
expect(mocks.clearApnsRegistrationIfCurrent).not.toHaveBeenCalled();
});
it("forces one retry wake when the first wake still fails to reconnect", async () => {

View File

@@ -10,13 +10,13 @@ import {
verifyNodeToken,
} from "../../infra/node-pairing.js";
import {
clearApnsRegistration,
clearApnsRegistrationIfCurrent,
loadApnsRegistration,
resolveApnsAuthConfigFromEnv,
resolveApnsRelayConfigFromEnv,
sendApnsAlert,
sendApnsBackgroundWake,
shouldClearStoredApnsRegistration,
resolveApnsAuthConfigFromEnv,
resolveApnsRelayConfigFromEnv,
} from "../../infra/push-apns.js";
import {
buildCanvasScopedHostUrl,
@@ -95,22 +95,20 @@ type PendingNodeAction = {
const pendingNodeActionsById = new Map<string, PendingNodeAction[]>();
async function resolveNodePushConfig(
registration: NonNullable<Awaited<ReturnType<typeof loadApnsRegistration>>>,
) {
if (registration.transport === "relay") {
const relay = resolveApnsRelayConfigFromEnv(process.env);
return relay.ok
? { ok: true as const, relayConfig: relay.value }
: { ok: false as const, error: relay.error };
}
async function resolveDirectNodePushConfig() {
const auth = await resolveApnsAuthConfigFromEnv(process.env);
return auth.ok
? { ok: true as const, auth: auth.value }
: { ok: false as const, error: auth.error };
}
function resolveRelayNodePushConfig() {
const relay = resolveApnsRelayConfigFromEnv(process.env);
return relay.ok
? { ok: true as const, relayConfig: relay.value }
: { ok: false as const, error: relay.error };
}
async function clearStaleApnsRegistrationIfNeeded(
registration: NonNullable<Awaited<ReturnType<typeof loadApnsRegistration>>>,
nodeId: string,
@@ -124,7 +122,10 @@ async function clearStaleApnsRegistrationIfNeeded(
) {
return;
}
await clearApnsRegistration(nodeId);
await clearApnsRegistrationIfCurrent({
nodeId,
registration,
});
}
function isNodeEntry(entry: { role?: string; roles?: string[] }) {
@@ -273,24 +274,41 @@ export async function maybeWakeNodeWithApns(
return withDuration({ available: false, throttled: false, path: "no-registration" });
}
const resolved = await resolveNodePushConfig(registration);
if (!resolved.ok) {
state.lastWakeAtMs = Date.now();
let wakeResult;
if (registration.transport === "relay") {
const relay = resolveRelayNodePushConfig();
if (!relay.ok) {
return withDuration({
available: false,
throttled: false,
path: "no-auth",
apnsReason: resolved.error,
apnsReason: relay.error,
});
}
state.lastWakeAtMs = Date.now();
const wakeResult = await sendApnsBackgroundWake({
wakeResult = await sendApnsBackgroundWake({
registration,
nodeId,
wakeReason: opts?.wakeReason ?? "node.invoke",
auth: "auth" in resolved ? resolved.auth : undefined,
relayConfig: "relayConfig" in resolved ? resolved.relayConfig : undefined,
relayConfig: relay.relayConfig,
});
} else {
const auth = await resolveDirectNodePushConfig();
if (!auth.ok) {
return withDuration({
available: false,
throttled: false,
path: "no-auth",
apnsReason: auth.error,
});
}
wakeResult = await sendApnsBackgroundWake({
registration,
nodeId,
wakeReason: opts?.wakeReason ?? "node.invoke",
auth: auth.auth,
});
}
await clearStaleApnsRegistrationIfNeeded(registration, nodeId, wakeResult);
if (!wakeResult.ok) {
return withDuration({
@@ -353,25 +371,43 @@ export async function maybeSendNodeWakeNudge(nodeId: string): Promise<NodeWakeNu
if (!registration) {
return withDuration({ sent: false, throttled: false, reason: "no-registration" });
}
const resolved = await resolveNodePushConfig(registration);
if (!resolved.ok) {
try {
let result;
if (registration.transport === "relay") {
const relay = resolveRelayNodePushConfig();
if (!relay.ok) {
return withDuration({
sent: false,
throttled: false,
reason: "no-auth",
apnsReason: resolved.error,
apnsReason: relay.error,
});
}
try {
const result = await sendApnsAlert({
result = await sendApnsAlert({
registration,
nodeId,
title: "OpenClaw needs a quick reopen",
body: "Tap to reopen OpenClaw and restore the node connection.",
auth: "auth" in resolved ? resolved.auth : undefined,
relayConfig: "relayConfig" in resolved ? resolved.relayConfig : undefined,
relayConfig: relay.relayConfig,
});
} else {
const auth = await resolveDirectNodePushConfig();
if (!auth.ok) {
return withDuration({
sent: false,
throttled: false,
reason: "no-auth",
apnsReason: auth.error,
});
}
result = await sendApnsAlert({
registration,
nodeId,
title: "OpenClaw needs a quick reopen",
body: "Tap to reopen OpenClaw and restore the node connection.",
auth: auth.auth,
});
}
await clearStaleApnsRegistrationIfNeeded(registration, nodeId, result);
if (!result.ok) {
return withDuration({

View File

@@ -3,7 +3,7 @@ import { ErrorCodes } from "../protocol/index.js";
import { pushHandlers } from "./push.js";
vi.mock("../../infra/push-apns.js", () => ({
clearApnsRegistration: vi.fn(),
clearApnsRegistrationIfCurrent: vi.fn(),
loadApnsRegistration: vi.fn(),
normalizeApnsEnvironment: vi.fn(),
resolveApnsAuthConfigFromEnv: vi.fn(),
@@ -13,7 +13,7 @@ vi.mock("../../infra/push-apns.js", () => ({
}));
import {
clearApnsRegistration,
clearApnsRegistrationIfCurrent,
loadApnsRegistration,
normalizeApnsEnvironment,
resolveApnsAuthConfigFromEnv,
@@ -57,7 +57,7 @@ describe("push.test handler", () => {
vi.mocked(resolveApnsAuthConfigFromEnv).mockClear();
vi.mocked(resolveApnsRelayConfigFromEnv).mockClear();
vi.mocked(sendApnsAlert).mockClear();
vi.mocked(clearApnsRegistration).mockClear();
vi.mocked(clearApnsRegistrationIfCurrent).mockClear();
vi.mocked(shouldClearStoredApnsRegistration).mockReturnValue(false);
});
@@ -195,7 +195,17 @@ describe("push.test handler", () => {
});
await invoke();
expect(clearApnsRegistration).toHaveBeenCalledWith("ios-node-1");
expect(clearApnsRegistrationIfCurrent).toHaveBeenCalledWith({
nodeId: "ios-node-1",
registration: {
nodeId: "ios-node-1",
transport: "direct",
token: "abcd",
topic: "ai.openclaw.ios",
environment: "sandbox",
updatedAtMs: 1,
},
});
});
it("does not clear relay registrations after invalidation-shaped failures", async () => {
@@ -260,7 +270,7 @@ describe("push.test handler", () => {
},
overrideEnvironment: null,
});
expect(clearApnsRegistration).not.toHaveBeenCalled();
expect(clearApnsRegistrationIfCurrent).not.toHaveBeenCalled();
});
it("does not clear direct registrations when push.test overrides the environment", async () => {
@@ -320,6 +330,6 @@ describe("push.test handler", () => {
},
overrideEnvironment: "production",
});
expect(clearApnsRegistration).not.toHaveBeenCalled();
expect(clearApnsRegistrationIfCurrent).not.toHaveBeenCalled();
});
});

View File

@@ -1,5 +1,5 @@
import {
clearApnsRegistration,
clearApnsRegistrationIfCurrent,
loadApnsRegistration,
normalizeApnsEnvironment,
resolveApnsAuthConfigFromEnv,
@@ -97,7 +97,10 @@ export const pushHandlers: GatewayRequestHandlers = {
overrideEnvironment,
})
) {
await clearApnsRegistration(nodeId);
await clearApnsRegistrationIfCurrent({
nodeId,
registration,
});
}
respond(true, result, undefined);
});

View File

@@ -39,7 +39,7 @@ export async function writeTextAtomic(
await fs.mkdir(path.dirname(filePath), mkdirOptions);
const tmp = `${filePath}.${randomUUID()}.tmp`;
try {
await fs.writeFile(tmp, payload, "utf8");
await fs.writeFile(tmp, payload, { encoding: "utf8", mode });
try {
await fs.chmod(tmp, mode);
} catch {

View File

@@ -85,6 +85,9 @@ export function resolveApnsRelayConfigFromEnv(
if (parsed.protocol !== "https:" && parsed.protocol !== "http:") {
throw new Error("unsupported protocol");
}
if (!parsed.hostname) {
throw new Error("host required");
}
if (parsed.protocol === "http:" && !readAllowHttp(env.OPENCLAW_APNS_RELAY_ALLOW_HTTP)) {
throw new Error(
"http relay URLs require OPENCLAW_APNS_RELAY_ALLOW_HTTP=true (development only)",
@@ -93,6 +96,12 @@ export function resolveApnsRelayConfigFromEnv(
if (parsed.protocol === "http:" && !isLoopbackRelayHostname(parsed.hostname)) {
throw new Error("http relay URLs are limited to loopback hosts");
}
if (parsed.username || parsed.password) {
throw new Error("userinfo is not allowed");
}
if (parsed.search || parsed.hash) {
throw new Error("query and fragment are not allowed");
}
return {
ok: true,
value: {
@@ -119,6 +128,7 @@ async function sendApnsRelayRequest(params: {
}): Promise<ApnsRelayPushResponse> {
const response = await fetch(`${params.relayConfig.baseUrl}/v1/push/send`, {
method: "POST",
redirect: "manual",
headers: {
authorization: `Bearer ${params.relayConfig.authToken}`,
"content-type": "application/json",
@@ -131,6 +141,14 @@ async function sendApnsRelayRequest(params: {
}),
signal: AbortSignal.timeout(params.relayConfig.timeoutMs),
});
if (response.status >= 300 && response.status < 400) {
return {
ok: false,
status: response.status,
reason: "RelayRedirectNotAllowed",
environment: "production",
};
}
let json: unknown = null;
try {

View File

@@ -5,6 +5,7 @@ import path from "node:path";
import { afterEach, describe, expect, it, vi } from "vitest";
import {
clearApnsRegistration,
clearApnsRegistrationIfCurrent,
loadApnsRegistration,
normalizeApnsEnvironment,
registerApnsRegistration,
@@ -16,6 +17,7 @@ import {
shouldClearStoredApnsRegistration,
shouldInvalidateApnsRegistration,
} from "./push-apns.js";
import { sendApnsRelayPush } from "./push-apns.relay.js";
const tempDirs: string[] = [];
const testAuthPrivateKey = generateKeyPairSync("ec", { namedCurve: "prime256v1" })
@@ -29,6 +31,7 @@ async function makeTempDir(): Promise<string> {
}
afterEach(async () => {
vi.unstubAllGlobals();
while (tempDirs.length > 0) {
const dir = tempDirs.pop();
if (dir) {
@@ -170,6 +173,41 @@ describe("push APNs registration store", () => {
await expect(clearApnsRegistration("ios-node-1", baseDir)).resolves.toBe(true);
await expect(loadApnsRegistration("ios-node-1", baseDir)).resolves.toBeNull();
});
it("only clears a registration when the stored entry still matches", async () => {
vi.useFakeTimers();
try {
const baseDir = await makeTempDir();
vi.setSystemTime(new Date("2026-03-11T00:00:00Z"));
const stale = await registerApnsToken({
nodeId: "ios-node-1",
token: "ABCD1234ABCD1234ABCD1234ABCD1234",
topic: "ai.openclaw.ios",
environment: "sandbox",
baseDir,
});
vi.setSystemTime(new Date("2026-03-11T00:00:01Z"));
const fresh = await registerApnsToken({
nodeId: "ios-node-1",
token: "ABCD1234ABCD1234ABCD1234ABCD1234",
topic: "ai.openclaw.ios",
environment: "sandbox",
baseDir,
});
await expect(
clearApnsRegistrationIfCurrent({
nodeId: "ios-node-1",
registration: stale,
baseDir,
}),
).resolves.toBe(false);
await expect(loadApnsRegistration("ios-node-1", baseDir)).resolves.toEqual(fresh);
} finally {
vi.useRealTimers();
}
});
});
describe("push APNs env config", () => {
@@ -265,6 +303,26 @@ describe("push APNs env config", () => {
}
expect(resolved.error).toContain("loopback hosts");
});
it("rejects APNs relay URLs with query, fragment, or userinfo components", () => {
const withQuery = resolveApnsRelayConfigFromEnv({
OPENCLAW_APNS_RELAY_BASE_URL: "https://relay.example.com/path?debug=1",
OPENCLAW_APNS_RELAY_AUTH_TOKEN: "relay-secret",
} as NodeJS.ProcessEnv);
expect(withQuery.ok).toBe(false);
if (!withQuery.ok) {
expect(withQuery.error).toContain("query and fragment are not allowed");
}
const withUserinfo = resolveApnsRelayConfigFromEnv({
OPENCLAW_APNS_RELAY_BASE_URL: "https://user:pass@relay.example.com/path",
OPENCLAW_APNS_RELAY_AUTH_TOKEN: "relay-secret",
} as NodeJS.ProcessEnv);
expect(withUserinfo.ok).toBe(false);
if (!withUserinfo.ok) {
expect(withUserinfo.error).toContain("userinfo is not allowed");
}
});
});
describe("push APNs send semantics", () => {
@@ -451,6 +509,36 @@ describe("push APNs send semantics", () => {
});
});
it("does not follow relay redirects", async () => {
const fetchMock = vi.fn().mockResolvedValue({
ok: false,
status: 302,
json: vi.fn().mockRejectedValue(new Error("no body")),
});
vi.stubGlobal("fetch", fetchMock as unknown as typeof fetch);
const result = await sendApnsRelayPush({
relayConfig: {
baseUrl: "https://relay.example.com",
authToken: "relay-secret",
timeoutMs: 1000,
},
relayHandle: "relay-handle-123",
payload: { aps: { "content-available": 1 } },
pushType: "background",
priority: "5",
});
expect(fetchMock).toHaveBeenCalledTimes(1);
expect(fetchMock.mock.calls[0]?.[1]).toMatchObject({ redirect: "manual" });
expect(result).toMatchObject({
ok: false,
status: 302,
reason: "RelayRedirectNotAllowed",
environment: "production",
});
});
it("flags invalid device responses for registration invalidation", () => {
expect(shouldInvalidateApnsRegistration({ status: 400, reason: "BadDeviceToken" })).toBe(true);
expect(shouldInvalidateApnsRegistration({ status: 410, reason: "Unregistered" })).toBe(true);

View File

@@ -353,7 +353,11 @@ async function persistRegistrationsState(
baseDir?: string,
): Promise<void> {
const filePath = resolveApnsRegistrationPath(baseDir);
await writeJsonAtomic(filePath, state);
await writeJsonAtomic(filePath, state, {
mode: 0o600,
ensureDirMode: 0o700,
trailingNewline: true,
});
}
export function normalizeApnsEnvironment(value: unknown): ApnsEnvironment | null {
@@ -475,6 +479,51 @@ export async function clearApnsRegistration(nodeId: string, baseDir?: string): P
});
}
function isSameApnsRegistration(a: ApnsRegistration, b: ApnsRegistration): boolean {
if (
a.nodeId !== b.nodeId ||
a.transport !== b.transport ||
a.topic !== b.topic ||
a.environment !== b.environment ||
a.updatedAtMs !== b.updatedAtMs
) {
return false;
}
if (a.transport === "direct" && b.transport === "direct") {
return a.token === b.token;
}
if (a.transport === "relay" && b.transport === "relay") {
return (
a.relayHandle === b.relayHandle &&
a.installationId === b.installationId &&
a.distribution === b.distribution &&
a.tokenDebugSuffix === b.tokenDebugSuffix
);
}
return false;
}
export async function clearApnsRegistrationIfCurrent(params: {
nodeId: string;
registration: ApnsRegistration;
baseDir?: string;
}): Promise<boolean> {
const normalizedNodeId = normalizeNodeId(params.nodeId);
if (!normalizedNodeId) {
return false;
}
return await withLock(async () => {
const state = await loadRegistrationsState(params.baseDir);
const current = state.registrationsByNodeId[normalizedNodeId];
if (!current || !isSameApnsRegistration(current, params.registration)) {
return false;
}
delete state.registrationsByNodeId[normalizedNodeId];
await persistRegistrationsState(state, params.baseDir);
return true;
});
}
export function shouldInvalidateApnsRegistration(result: {
status: number;
reason?: string;
@@ -806,17 +855,54 @@ function createBackgroundPayload(params: { nodeId: string; wakeReason?: string }
};
}
export async function sendApnsAlert(params: {
auth?: ApnsAuthConfig;
relayConfig?: ApnsRelayConfig;
registration: ApnsRegistration;
type ApnsAlertCommonParams = {
nodeId: string;
title: string;
body: string;
timeoutMs?: number;
};
type DirectApnsAlertParams = ApnsAlertCommonParams & {
registration: DirectApnsRegistration;
auth: ApnsAuthConfig;
requestSender?: ApnsRequestSender;
relayConfig?: never;
relayRequestSender?: never;
};
type RelayApnsAlertParams = ApnsAlertCommonParams & {
registration: RelayApnsRegistration;
relayConfig: ApnsRelayConfig;
relayRequestSender?: ApnsRelayRequestSender;
}): Promise<ApnsPushAlertResult> {
auth?: never;
requestSender?: never;
};
type ApnsBackgroundWakeCommonParams = {
nodeId: string;
wakeReason?: string;
timeoutMs?: number;
};
type DirectApnsBackgroundWakeParams = ApnsBackgroundWakeCommonParams & {
registration: DirectApnsRegistration;
auth: ApnsAuthConfig;
requestSender?: ApnsRequestSender;
relayConfig?: never;
relayRequestSender?: never;
};
type RelayApnsBackgroundWakeParams = ApnsBackgroundWakeCommonParams & {
registration: RelayApnsRegistration;
relayConfig: ApnsRelayConfig;
relayRequestSender?: ApnsRelayRequestSender;
auth?: never;
requestSender?: never;
};
export async function sendApnsAlert(
params: DirectApnsAlertParams | RelayApnsAlertParams,
): Promise<ApnsPushAlertResult> {
const payload = createAlertPayload({
nodeId: params.nodeId,
title: params.title,
@@ -824,69 +910,54 @@ export async function sendApnsAlert(params: {
});
if (params.registration.transport === "relay") {
if (!params.relayConfig) {
throw new Error("APNs relay config required");
}
const relayParams = params as RelayApnsAlertParams;
return await sendRelayApnsPush({
relayConfig: params.relayConfig,
registration: params.registration,
relayConfig: relayParams.relayConfig,
registration: relayParams.registration,
payload,
pushType: "alert",
priority: "10",
requestSender: params.relayRequestSender,
requestSender: relayParams.relayRequestSender,
});
}
if (!params.auth) {
throw new Error("APNs auth required");
}
const directParams = params as DirectApnsAlertParams;
return await sendDirectApnsPush({
auth: params.auth,
registration: params.registration,
auth: directParams.auth,
registration: directParams.registration,
payload,
timeoutMs: params.timeoutMs,
requestSender: params.requestSender,
timeoutMs: directParams.timeoutMs,
requestSender: directParams.requestSender,
pushType: "alert",
priority: "10",
});
}
export async function sendApnsBackgroundWake(params: {
auth?: ApnsAuthConfig;
relayConfig?: ApnsRelayConfig;
registration: ApnsRegistration;
nodeId: string;
wakeReason?: string;
timeoutMs?: number;
requestSender?: ApnsRequestSender;
relayRequestSender?: ApnsRelayRequestSender;
}): Promise<ApnsPushWakeResult> {
export async function sendApnsBackgroundWake(
params: DirectApnsBackgroundWakeParams | RelayApnsBackgroundWakeParams,
): Promise<ApnsPushWakeResult> {
const payload = createBackgroundPayload({
nodeId: params.nodeId,
wakeReason: params.wakeReason,
});
if (params.registration.transport === "relay") {
if (!params.relayConfig) {
throw new Error("APNs relay config required");
}
const relayParams = params as RelayApnsBackgroundWakeParams;
return await sendRelayApnsPush({
relayConfig: params.relayConfig,
registration: params.registration,
relayConfig: relayParams.relayConfig,
registration: relayParams.registration,
payload,
pushType: "background",
priority: "5",
requestSender: params.relayRequestSender,
requestSender: relayParams.relayRequestSender,
});
}
if (!params.auth) {
throw new Error("APNs auth required");
}
const directParams = params as DirectApnsBackgroundWakeParams;
return await sendDirectApnsPush({
auth: params.auth,
registration: params.registration,
auth: directParams.auth,
registration: directParams.registration,
payload,
timeoutMs: params.timeoutMs,
requestSender: params.requestSender,
timeoutMs: directParams.timeoutMs,
requestSender: directParams.requestSender,
pushType: "background",
priority: "5",
});