fix(push): harden relay validation and invalidation

This commit is contained in:
Nimrod Gutman
2026-03-11 20:34:35 +02:00
committed by Nimrod Gutman
parent 7783805f24
commit a46047ab00
9 changed files with 418 additions and 26 deletions

View File

@@ -72,6 +72,7 @@ Release behavior:
Required env for beta builds:
- `OPENCLAW_PUSH_RELAY_BASE_URL=https://relay.example.com`
This must be a plain `https://host[:port][/path]` base URL without whitespace, query params, fragments, or xcconfig metacharacters.
Archive without upload:

View File

@@ -51,6 +51,25 @@ write_generated_file() {
mv -f "${tmp_file}" "${output_path}"
}
validate_push_relay_base_url() {
local value="$1"
if [[ "${value}" =~ [[:space:]] ]]; then
echo "Invalid OPENCLAW_PUSH_RELAY_BASE_URL: whitespace is not allowed." >&2
exit 1
fi
if [[ "${value}" == *'$'* || "${value}" == *'('* || "${value}" == *')'* || "${value}" == *'='* ]]; then
echo "Invalid OPENCLAW_PUSH_RELAY_BASE_URL: contains forbidden xcconfig characters." >&2
exit 1
fi
if [[ ! "${value}" =~ ^https://[A-Za-z0-9.-]+(:[0-9]{1,5})?(/[A-Za-z0-9._~!&*+,;:@%/-]*)?$ ]]; then
echo "Invalid OPENCLAW_PUSH_RELAY_BASE_URL: expected https://host[:port][/path]." >&2
exit 1
fi
}
while [[ $# -gt 0 ]]; do
case "$1" in
--)
@@ -96,6 +115,8 @@ if [[ -z "${PUSH_RELAY_BASE_URL}" ]]; then
exit 1
fi
validate_push_relay_base_url "${PUSH_RELAY_BASE_URL}"
# `.xcconfig` treats `//` as a comment opener. Break the URL with a helper setting
# so Xcode still resolves it back to `https://...` at build time.
PUSH_RELAY_BASE_URL_XCCONFIG="$(

View File

@@ -16,7 +16,7 @@ const mocks = vi.hoisted(() => ({
resolveApnsRelayConfigFromEnv: vi.fn(),
sendApnsBackgroundWake: vi.fn(),
sendApnsAlert: vi.fn(),
shouldInvalidateApnsRegistration: vi.fn(() => false),
shouldClearStoredApnsRegistration: vi.fn(() => false),
}));
vi.mock("../../config/config.js", () => ({
@@ -39,7 +39,7 @@ vi.mock("../../infra/push-apns.js", () => ({
resolveApnsRelayConfigFromEnv: mocks.resolveApnsRelayConfigFromEnv,
sendApnsBackgroundWake: mocks.sendApnsBackgroundWake,
sendApnsAlert: mocks.sendApnsAlert,
shouldInvalidateApnsRegistration: mocks.shouldInvalidateApnsRegistration,
shouldClearStoredApnsRegistration: mocks.shouldClearStoredApnsRegistration,
}));
type RespondCall = [
@@ -202,7 +202,7 @@ describe("node.invoke APNs wake path", () => {
mocks.resolveApnsRelayConfigFromEnv.mockClear();
mocks.sendApnsBackgroundWake.mockClear();
mocks.sendApnsAlert.mockClear();
mocks.shouldInvalidateApnsRegistration.mockReturnValue(false);
mocks.shouldClearStoredApnsRegistration.mockReturnValue(false);
});
afterEach(() => {
@@ -296,7 +296,7 @@ describe("node.invoke APNs wake path", () => {
environment: "sandbox",
transport: "direct",
});
mocks.shouldInvalidateApnsRegistration.mockReturnValue(true);
mocks.shouldClearStoredApnsRegistration.mockReturnValue(true);
const nodeRegistry = {
get: vi.fn(() => undefined),
@@ -314,6 +314,75 @@ describe("node.invoke APNs wake path", () => {
expect(mocks.clearApnsRegistration).toHaveBeenCalledWith("ios-node-stale");
});
it("does not clear relay registrations from wake failures", async () => {
mocks.loadApnsRegistration.mockResolvedValue({
nodeId: "ios-node-relay",
transport: "relay",
relayHandle: "relay-handle-123",
installationId: "install-123",
topic: "ai.openclaw.ios",
environment: "production",
distribution: "official",
updatedAtMs: 1,
tokenDebugSuffix: "abcd1234",
});
mocks.resolveApnsRelayConfigFromEnv.mockReturnValue({
ok: true,
value: {
baseUrl: "https://relay.example.com",
authToken: "relay-secret",
timeoutMs: 1000,
},
});
mocks.sendApnsBackgroundWake.mockResolvedValue({
ok: false,
status: 410,
reason: "Unregistered",
tokenSuffix: "abcd1234",
topic: "ai.openclaw.ios",
environment: "production",
transport: "relay",
});
mocks.shouldClearStoredApnsRegistration.mockReturnValue(false);
const nodeRegistry = {
get: vi.fn(() => undefined),
invoke: vi.fn().mockResolvedValue({ ok: true }),
};
const respond = await invokeNode({
nodeRegistry,
requestParams: { nodeId: "ios-node-relay", idempotencyKey: "idem-relay" },
});
const call = respond.mock.calls[0] as RespondCall | undefined;
expect(call?.[0]).toBe(false);
expect(call?.[2]?.message).toBe("node not connected");
expect(mocks.shouldClearStoredApnsRegistration).toHaveBeenCalledWith({
registration: {
nodeId: "ios-node-relay",
transport: "relay",
relayHandle: "relay-handle-123",
installationId: "install-123",
topic: "ai.openclaw.ios",
environment: "production",
distribution: "official",
updatedAtMs: 1,
tokenDebugSuffix: "abcd1234",
},
result: {
ok: false,
status: 410,
reason: "Unregistered",
tokenSuffix: "abcd1234",
topic: "ai.openclaw.ios",
environment: "production",
transport: "relay",
},
});
expect(mocks.clearApnsRegistration).not.toHaveBeenCalled();
});
it("forces one retry wake when the first wake still fails to reconnect", async () => {
vi.useFakeTimers();
mockSuccessfulWakeConfig("ios-node-throttle");

View File

@@ -16,7 +16,7 @@ import {
resolveApnsRelayConfigFromEnv,
sendApnsAlert,
sendApnsBackgroundWake,
shouldInvalidateApnsRegistration,
shouldClearStoredApnsRegistration,
} from "../../infra/push-apns.js";
import {
buildCanvasScopedHostUrl,
@@ -112,10 +112,16 @@ async function resolveNodePushConfig(
}
async function clearStaleApnsRegistrationIfNeeded(
registration: NonNullable<Awaited<ReturnType<typeof loadApnsRegistration>>>,
nodeId: string,
params: { status: number; reason?: string },
) {
if (!shouldInvalidateApnsRegistration(params)) {
if (
!shouldClearStoredApnsRegistration({
registration,
result: params,
})
) {
return;
}
await clearApnsRegistration(nodeId);
@@ -285,7 +291,7 @@ export async function maybeWakeNodeWithApns(
auth: "auth" in resolved ? resolved.auth : undefined,
relayConfig: "relayConfig" in resolved ? resolved.relayConfig : undefined,
});
await clearStaleApnsRegistrationIfNeeded(nodeId, wakeResult);
await clearStaleApnsRegistrationIfNeeded(registration, nodeId, wakeResult);
if (!wakeResult.ok) {
return withDuration({
available: true,
@@ -366,7 +372,7 @@ export async function maybeSendNodeWakeNudge(nodeId: string): Promise<NodeWakeNu
auth: "auth" in resolved ? resolved.auth : undefined,
relayConfig: "relayConfig" in resolved ? resolved.relayConfig : undefined,
});
await clearStaleApnsRegistrationIfNeeded(nodeId, result);
await clearStaleApnsRegistrationIfNeeded(registration, nodeId, result);
if (!result.ok) {
return withDuration({
sent: false,

View File

@@ -9,7 +9,7 @@ vi.mock("../../infra/push-apns.js", () => ({
resolveApnsAuthConfigFromEnv: vi.fn(),
resolveApnsRelayConfigFromEnv: vi.fn(),
sendApnsAlert: vi.fn(),
shouldInvalidateApnsRegistration: vi.fn(),
shouldClearStoredApnsRegistration: vi.fn(),
}));
import {
@@ -19,7 +19,7 @@ import {
resolveApnsAuthConfigFromEnv,
resolveApnsRelayConfigFromEnv,
sendApnsAlert,
shouldInvalidateApnsRegistration,
shouldClearStoredApnsRegistration,
} from "../../infra/push-apns.js";
type RespondCall = [boolean, unknown?, { code: number; message: string }?];
@@ -58,7 +58,7 @@ describe("push.test handler", () => {
vi.mocked(resolveApnsRelayConfigFromEnv).mockClear();
vi.mocked(sendApnsAlert).mockClear();
vi.mocked(clearApnsRegistration).mockClear();
vi.mocked(shouldInvalidateApnsRegistration).mockReturnValue(false);
vi.mocked(shouldClearStoredApnsRegistration).mockReturnValue(false);
});
it("rejects invalid params", async () => {
@@ -186,7 +186,7 @@ describe("push.test handler", () => {
environment: "sandbox",
transport: "direct",
});
vi.mocked(shouldInvalidateApnsRegistration).mockReturnValue(true);
vi.mocked(shouldClearStoredApnsRegistration).mockReturnValue(true);
const { invoke } = createInvokeParams({
nodeId: "ios-node-1",
@@ -197,4 +197,129 @@ describe("push.test handler", () => {
expect(clearApnsRegistration).toHaveBeenCalledWith("ios-node-1");
});
it("does not clear relay registrations after invalidation-shaped failures", async () => {
vi.mocked(loadApnsRegistration).mockResolvedValue({
nodeId: "ios-node-1",
transport: "relay",
relayHandle: "relay-handle-123",
installationId: "install-123",
topic: "ai.openclaw.ios",
environment: "production",
distribution: "official",
updatedAtMs: 1,
tokenDebugSuffix: "abcd1234",
});
vi.mocked(resolveApnsRelayConfigFromEnv).mockReturnValue({
ok: true,
value: {
baseUrl: "https://relay.example.com",
authToken: "relay-secret",
timeoutMs: 1000,
},
});
vi.mocked(normalizeApnsEnvironment).mockReturnValue(null);
vi.mocked(sendApnsAlert).mockResolvedValue({
ok: false,
status: 410,
reason: "Unregistered",
tokenSuffix: "abcd1234",
topic: "ai.openclaw.ios",
environment: "production",
transport: "relay",
});
vi.mocked(shouldClearStoredApnsRegistration).mockReturnValue(false);
const { invoke } = createInvokeParams({
nodeId: "ios-node-1",
title: "Wake",
body: "Ping",
});
await invoke();
expect(shouldClearStoredApnsRegistration).toHaveBeenCalledWith({
registration: {
nodeId: "ios-node-1",
transport: "relay",
relayHandle: "relay-handle-123",
installationId: "install-123",
topic: "ai.openclaw.ios",
environment: "production",
distribution: "official",
updatedAtMs: 1,
tokenDebugSuffix: "abcd1234",
},
result: {
ok: false,
status: 410,
reason: "Unregistered",
tokenSuffix: "abcd1234",
topic: "ai.openclaw.ios",
environment: "production",
transport: "relay",
},
overrideEnvironment: null,
});
expect(clearApnsRegistration).not.toHaveBeenCalled();
});
it("does not clear direct registrations when push.test overrides the environment", async () => {
vi.mocked(loadApnsRegistration).mockResolvedValue({
nodeId: "ios-node-1",
transport: "direct",
token: "abcd",
topic: "ai.openclaw.ios",
environment: "sandbox",
updatedAtMs: 1,
});
vi.mocked(resolveApnsAuthConfigFromEnv).mockResolvedValue({
ok: true,
value: {
teamId: "TEAM123",
keyId: "KEY123",
privateKey: "-----BEGIN PRIVATE KEY-----\nabc\n-----END PRIVATE KEY-----", // pragma: allowlist secret
},
});
vi.mocked(normalizeApnsEnvironment).mockReturnValue("production");
vi.mocked(sendApnsAlert).mockResolvedValue({
ok: false,
status: 400,
reason: "BadDeviceToken",
tokenSuffix: "1234abcd",
topic: "ai.openclaw.ios",
environment: "production",
transport: "direct",
});
vi.mocked(shouldClearStoredApnsRegistration).mockReturnValue(false);
const { invoke } = createInvokeParams({
nodeId: "ios-node-1",
title: "Wake",
body: "Ping",
environment: "production",
});
await invoke();
expect(shouldClearStoredApnsRegistration).toHaveBeenCalledWith({
registration: {
nodeId: "ios-node-1",
transport: "direct",
token: "abcd",
topic: "ai.openclaw.ios",
environment: "sandbox",
updatedAtMs: 1,
},
result: {
ok: false,
status: 400,
reason: "BadDeviceToken",
tokenSuffix: "1234abcd",
topic: "ai.openclaw.ios",
environment: "production",
transport: "direct",
},
overrideEnvironment: "production",
});
expect(clearApnsRegistration).not.toHaveBeenCalled();
});
});

View File

@@ -5,7 +5,7 @@ import {
resolveApnsAuthConfigFromEnv,
resolveApnsRelayConfigFromEnv,
sendApnsAlert,
shouldInvalidateApnsRegistration,
shouldClearStoredApnsRegistration,
} from "../../infra/push-apns.js";
import { ErrorCodes, errorShape, validatePushTestParams } from "../protocol/index.js";
import { respondInvalidParams, respondUnavailableOnThrow } from "./nodes.helpers.js";
@@ -90,7 +90,13 @@ export const pushHandlers: GatewayRequestHandlers = {
if (!result) {
return;
}
if (shouldInvalidateApnsRegistration(result)) {
if (
shouldClearStoredApnsRegistration({
registration,
result,
overrideEnvironment,
})
) {
await clearApnsRegistration(nodeId);
}
respond(true, result, undefined);

View File

@@ -53,6 +53,16 @@ function readAllowHttp(value: string | undefined): boolean {
return normalized === "1" || normalized === "true" || normalized === "yes";
}
function isLoopbackRelayHostname(hostname: string): boolean {
const normalized = hostname.trim().toLowerCase();
return (
normalized === "localhost" ||
normalized === "::1" ||
normalized === "[::1]" ||
/^127(?:\.\d{1,3}){3}$/.test(normalized)
);
}
function parseReason(value: unknown): string | undefined {
return typeof value === "string" && value.trim().length > 0 ? value.trim() : undefined;
}
@@ -80,6 +90,9 @@ export function resolveApnsRelayConfigFromEnv(
"http relay URLs require OPENCLAW_APNS_RELAY_ALLOW_HTTP=true (development only)",
);
}
if (parsed.protocol === "http:" && !isLoopbackRelayHostname(parsed.hostname)) {
throw new Error("http relay URLs are limited to loopback hosts");
}
return {
ok: true,
value: {

View File

@@ -13,6 +13,7 @@ import {
resolveApnsRelayConfigFromEnv,
sendApnsAlert,
sendApnsBackgroundWake,
shouldClearStoredApnsRegistration,
shouldInvalidateApnsRegistration,
} from "./push-apns.js";
@@ -100,6 +101,63 @@ describe("push APNs registration store", () => {
).rejects.toThrow("invalid APNs token");
});
it("rejects relay registrations that do not use production/official values", async () => {
const baseDir = await makeTempDir();
await expect(
registerApnsRegistration({
nodeId: "ios-node-relay",
transport: "relay",
relayHandle: "relay-handle-123",
installationId: "install-123",
topic: "ai.openclaw.ios",
environment: "staging",
distribution: "official",
baseDir,
}),
).rejects.toThrow("relay registrations must use production environment");
await expect(
registerApnsRegistration({
nodeId: "ios-node-relay",
transport: "relay",
relayHandle: "relay-handle-123",
installationId: "install-123",
topic: "ai.openclaw.ios",
environment: "production",
distribution: "beta",
baseDir,
}),
).rejects.toThrow("relay registrations must use official distribution");
});
it("rejects oversized relay registration identifiers", async () => {
const baseDir = await makeTempDir();
const oversized = "x".repeat(257);
await expect(
registerApnsRegistration({
nodeId: "ios-node-relay",
transport: "relay",
relayHandle: oversized,
installationId: "install-123",
topic: "ai.openclaw.ios",
environment: "production",
distribution: "official",
baseDir,
}),
).rejects.toThrow("relayHandle too long");
await expect(
registerApnsRegistration({
nodeId: "ios-node-relay",
transport: "relay",
relayHandle: "relay-handle-123",
installationId: oversized,
topic: "ai.openclaw.ios",
environment: "production",
distribution: "official",
baseDir,
}),
).rejects.toThrow("installationId too long");
});
it("clears registrations", async () => {
const baseDir = await makeTempDir();
await registerApnsToken({
@@ -179,19 +237,34 @@ describe("push APNs env config", () => {
it("allows APNs relay http URLs only when explicitly enabled", () => {
const resolved = resolveApnsRelayConfigFromEnv({
OPENCLAW_APNS_RELAY_BASE_URL: "http://relay.example.com",
OPENCLAW_APNS_RELAY_BASE_URL: "http://127.0.0.1:8787",
OPENCLAW_APNS_RELAY_AUTH_TOKEN: "relay-secret",
OPENCLAW_APNS_RELAY_ALLOW_HTTP: "true",
} as NodeJS.ProcessEnv);
expect(resolved).toMatchObject({
ok: true,
value: {
baseUrl: "http://relay.example.com",
baseUrl: "http://127.0.0.1:8787",
authToken: "relay-secret",
timeoutMs: 10_000,
},
});
});
it("rejects http relay URLs for non-loopback hosts even when explicitly enabled", () => {
const resolved = resolveApnsRelayConfigFromEnv({
OPENCLAW_APNS_RELAY_BASE_URL: "http://relay.example.com",
OPENCLAW_APNS_RELAY_AUTH_TOKEN: "relay-secret",
OPENCLAW_APNS_RELAY_ALLOW_HTTP: "true",
} as NodeJS.ProcessEnv);
expect(resolved).toMatchObject({
ok: false,
});
if (resolved.ok) {
return;
}
expect(resolved.error).toContain("loopback hosts");
});
});
describe("push APNs send semantics", () => {
@@ -385,4 +458,51 @@ describe("push APNs send semantics", () => {
false,
);
});
it("only clears stored registrations for direct APNs failures without an override mismatch", () => {
expect(
shouldClearStoredApnsRegistration({
registration: {
nodeId: "ios-node-direct",
transport: "direct",
token: "ABCD1234ABCD1234ABCD1234ABCD1234",
topic: "ai.openclaw.ios",
environment: "sandbox",
updatedAtMs: 1,
},
result: { status: 400, reason: "BadDeviceToken" },
}),
).toBe(true);
expect(
shouldClearStoredApnsRegistration({
registration: {
nodeId: "ios-node-relay",
transport: "relay",
relayHandle: "relay-handle-123",
installationId: "install-123",
topic: "ai.openclaw.ios",
environment: "production",
distribution: "official",
updatedAtMs: 1,
},
result: { status: 410, reason: "Unregistered" },
}),
).toBe(false);
expect(
shouldClearStoredApnsRegistration({
registration: {
nodeId: "ios-node-direct",
transport: "direct",
token: "ABCD1234ABCD1234ABCD1234ABCD1234",
topic: "ai.openclaw.ios",
environment: "sandbox",
updatedAtMs: 1,
},
result: { status: 400, reason: "BadDeviceToken" },
overrideEnvironment: "production",
}),
).toBe(false);
});
});

View File

@@ -110,6 +110,7 @@ type RegisterApnsParams = RegisterDirectApnsParams | RegisterRelayApnsParams;
const APNS_STATE_FILENAME = "push/apns-registrations.json";
const APNS_JWT_TTL_MS = 50 * 60 * 1000;
const DEFAULT_APNS_TIMEOUT_MS = 10_000;
const MAX_RELAY_IDENTIFIER_LENGTH = 256;
const withLock = createAsyncLock();
let cachedJwt: { cacheKey: string; token: string; expiresAtMs: number } | null = null;
@@ -138,6 +139,19 @@ function normalizeInstallationId(value: string): string {
return value.trim();
}
function validateRelayIdentifier(value: string, fieldName: string): string {
if (!value) {
throw new Error(`${fieldName} required`);
}
if (value.length > MAX_RELAY_IDENTIFIER_LENGTH) {
throw new Error(`${fieldName} too long`);
}
if (/[^\x21-\x7e]/.test(value)) {
throw new Error(`${fieldName} invalid`);
}
return value;
}
function normalizeTopic(value: string): string {
return value.trim();
}
@@ -371,16 +385,16 @@ export async function registerApnsRegistration(
let next: ApnsRegistration;
if (params.transport === "relay") {
const relayHandle = normalizeRelayHandle(params.relayHandle);
const installationId = normalizeInstallationId(params.installationId);
const environment = normalizeApnsEnvironment(params.environment) ?? "production";
const distribution = normalizeDistribution(params.distribution) ?? "official";
if (!relayHandle) {
throw new Error("relayHandle required");
}
if (!installationId) {
throw new Error("installationId required");
}
const relayHandle = validateRelayIdentifier(
normalizeRelayHandle(params.relayHandle),
"relayHandle",
);
const installationId = validateRelayIdentifier(
normalizeInstallationId(params.installationId),
"installationId",
);
const environment = normalizeApnsEnvironment(params.environment);
const distribution = normalizeDistribution(params.distribution);
if (environment !== "production") {
throw new Error("relay registrations must use production environment");
}
@@ -471,6 +485,23 @@ export function shouldInvalidateApnsRegistration(result: {
return result.status === 400 && result.reason?.trim() === "BadDeviceToken";
}
export function shouldClearStoredApnsRegistration(params: {
registration: ApnsRegistration;
result: { status: number; reason?: string };
overrideEnvironment?: ApnsEnvironment | null;
}): boolean {
if (params.registration.transport !== "direct") {
return false;
}
if (
params.overrideEnvironment &&
params.overrideEnvironment !== params.registration.environment
) {
return false;
}
return shouldInvalidateApnsRegistration(params.result);
}
export async function resolveApnsAuthConfigFromEnv(
env: NodeJS.ProcessEnv = process.env,
): Promise<ApnsAuthConfigResolution> {