fix(rpc): close prototype-chain permission bypass, add server/client/transport tests
C1 CRITICAL: implement() looked up procedures/handlers with plain property indexing, so any Object.prototype member name (constructor, toString, etc.) resolved truthy and skipped the permission gate entirely. Fixed with Object.hasOwn checks in packages/rpc/src/server.ts. Defense-in-depth guard added in packages/dev-server/src/rpc-dispatch.ts constraining URL path segments to a safe charset before they reach service/procedure lookups. Added missing direct test coverage for packages/rpc/src/transport.ts, server.ts and client.ts (previously untested), including a prototype-name sweep in both server.test.ts and dev-server's rpc-endpoint.test.ts. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Claude Opus 5
@@ -37,8 +37,14 @@ export function implement<Procedures extends AnyProcedures>(
|
||||
return {
|
||||
contract,
|
||||
async invoke(procedureName, payload, identity) {
|
||||
const definition = contract.procedures[procedureName as keyof Procedures];
|
||||
const handler = handlers[procedureName as keyof Procedures];
|
||||
// Object.hasOwn, not plain indexing: "constructor", "toString" and every
|
||||
// other Object.prototype member otherwise resolve as truthy, and a
|
||||
// prototype member carries no `permission`, so the gate below is skipped
|
||||
// entirely and an unintended function runs with attacker-controlled input.
|
||||
const known =
|
||||
Object.hasOwn(contract.procedures, procedureName) && Object.hasOwn(handlers, procedureName);
|
||||
const definition = known ? contract.procedures[procedureName as keyof Procedures] : undefined;
|
||||
const handler = known ? handlers[procedureName as keyof Procedures] : undefined;
|
||||
if (!definition || !handler) return failure(RPC_ERROR_CODES.unknown, "Unknown procedure");
|
||||
|
||||
let subject: SubjectContext | undefined;
|
||||
|
||||
@@ -0,0 +1,110 @@
|
||||
import { afterEach, describe, expect, test } from "bun:test";
|
||||
import type { Context } from "@wrnexus/core";
|
||||
import { v } from "@wrnexus/validation";
|
||||
import { serviceClient } from "../src/client.ts";
|
||||
import { defineService, procedure } from "../src/contract.ts";
|
||||
import { RPC_ERROR_CODES, ServiceError, failure, success } from "../src/errors.ts";
|
||||
import type { RpcTarget } from "../src/transport.ts";
|
||||
|
||||
const original = { ...process.env };
|
||||
afterEach(() => {
|
||||
process.env = { ...original };
|
||||
});
|
||||
|
||||
function configure(appName = "web") {
|
||||
process.env.WRNEXUS_RPC_SECRET = "test-rpc-secret-at-least-32-chars-long";
|
||||
process.env.WRNEXUS_APP_NAME = appName;
|
||||
}
|
||||
|
||||
const billing = defineService({
|
||||
name: "billing",
|
||||
procedures: {
|
||||
createInvoice: procedure
|
||||
.input(v.object({ amountCents: v.number() }))
|
||||
.output<{ invoiceId: string }>()
|
||||
.build(),
|
||||
},
|
||||
});
|
||||
|
||||
describe("serviceClient", () => {
|
||||
test("returns the handler's value unwrapped", async () => {
|
||||
const client = serviceClient(billing, {
|
||||
transport: { call: async () => success({ invoiceId: "inv_1" }) },
|
||||
});
|
||||
expect(await client.createInvoice({ amountCents: 1 })).toEqual({ invoiceId: "inv_1" });
|
||||
});
|
||||
|
||||
test("throws a ServiceError carrying code and retryable on failure", async () => {
|
||||
const client = serviceClient(billing, {
|
||||
transport: { call: async () => failure(RPC_ERROR_CODES.transport, "down") },
|
||||
});
|
||||
let caught: unknown;
|
||||
try {
|
||||
await client.createInvoice({ amountCents: 1 });
|
||||
} catch (err) {
|
||||
caught = err;
|
||||
}
|
||||
expect(caught).toBeInstanceOf(ServiceError);
|
||||
expect(caught).toMatchObject({ code: RPC_ERROR_CODES.transport, retryable: true });
|
||||
});
|
||||
|
||||
test("attaches an identity token when given a context and omits it for an anonymous context", async () => {
|
||||
configure("web");
|
||||
let seenIdentity: string | undefined = "unset";
|
||||
const client = serviceClient(billing, {
|
||||
as: { user: { id: "u1" }, locals: {} } as unknown as Context,
|
||||
transport: {
|
||||
call: async (_target, _payload, options) => {
|
||||
seenIdentity = options.identity;
|
||||
return success({ invoiceId: "inv_1" });
|
||||
},
|
||||
},
|
||||
});
|
||||
await client.createInvoice({ amountCents: 1 });
|
||||
expect(seenIdentity).toBeTypeOf("string");
|
||||
|
||||
let seenAnon: unknown = "unset";
|
||||
const anonClient = serviceClient(billing, {
|
||||
transport: {
|
||||
call: async (_target, _payload, options) => {
|
||||
seenAnon = options.identity;
|
||||
return success({ invoiceId: "inv_1" });
|
||||
},
|
||||
},
|
||||
});
|
||||
await anonClient.createInvoice({ amountCents: 1 });
|
||||
expect(seenAnon).toBeUndefined();
|
||||
});
|
||||
|
||||
test("calling an undeclared procedure throws rather than issuing a call", async () => {
|
||||
let called = false;
|
||||
const client = serviceClient(billing, {
|
||||
transport: {
|
||||
call: async () => {
|
||||
called = true;
|
||||
return success({});
|
||||
},
|
||||
},
|
||||
});
|
||||
const proxy = client as unknown as Record<string, (input: unknown) => Promise<unknown>>;
|
||||
await expect(proxy.deleteEverything!({})).rejects.toMatchObject({
|
||||
code: RPC_ERROR_CODES.unknown,
|
||||
});
|
||||
expect(called).toBe(false);
|
||||
});
|
||||
|
||||
test("the app defaults to the service name", async () => {
|
||||
let seenTarget: RpcTarget | undefined;
|
||||
const client = serviceClient(billing, {
|
||||
transport: {
|
||||
call: async (target) => {
|
||||
seenTarget = target;
|
||||
return success({ invoiceId: "inv_1" });
|
||||
},
|
||||
},
|
||||
});
|
||||
await client.createInvoice({ amountCents: 1 });
|
||||
expect(seenTarget?.app).toBe("billing");
|
||||
expect(seenTarget?.service).toBe("billing");
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,243 @@
|
||||
import { afterEach, describe, expect, test } from "bun:test";
|
||||
import { v } from "@wrnexus/validation";
|
||||
import { defineService, procedure } from "../src/contract.ts";
|
||||
import { RPC_ERROR_CODES } from "../src/errors.ts";
|
||||
import { exportSubjectContext } from "../src/identity.ts";
|
||||
import { implement } from "../src/server.ts";
|
||||
|
||||
const original = { ...process.env };
|
||||
afterEach(() => {
|
||||
process.env = { ...original };
|
||||
});
|
||||
|
||||
function configure(appName = "web") {
|
||||
process.env.WRNEXUS_RPC_SECRET = "test-rpc-secret-at-least-32-chars-long";
|
||||
process.env.WRNEXUS_APP_NAME = appName;
|
||||
}
|
||||
|
||||
const openContract = defineService({
|
||||
name: "demo",
|
||||
procedures: {
|
||||
add: procedure
|
||||
.input(v.object({ a: v.number() }))
|
||||
.output<{ a: number }>()
|
||||
.build(),
|
||||
},
|
||||
});
|
||||
|
||||
const guardedContract = defineService({
|
||||
name: "billing",
|
||||
procedures: {
|
||||
createInvoice: procedure
|
||||
.input(v.object({ amountCents: v.number() }))
|
||||
.output<{ invoiceId: string }>()
|
||||
.permission("invoice:create")
|
||||
.build(),
|
||||
},
|
||||
});
|
||||
|
||||
describe("implement()", () => {
|
||||
test("invokes with validated input", async () => {
|
||||
const service = implement(
|
||||
openContract,
|
||||
{ add: async ({ a }) => ({ a: a + 1 }) },
|
||||
{ selfApp: "demo" },
|
||||
);
|
||||
const result = await service.invoke("add", { a: 1 });
|
||||
expect(result).toEqual({ ok: true, value: { a: 2 } });
|
||||
});
|
||||
|
||||
test("coerces through the schema", async () => {
|
||||
let received: unknown;
|
||||
const coercing = defineService({
|
||||
name: "demo2",
|
||||
procedures: {
|
||||
add: procedure
|
||||
.input(v.object({ a: v.number() }))
|
||||
.output<{ a: number }>()
|
||||
.build(),
|
||||
},
|
||||
});
|
||||
const service = implement(
|
||||
coercing,
|
||||
{
|
||||
add: async (input) => {
|
||||
received = input;
|
||||
return { a: (input as { a: number }).a };
|
||||
},
|
||||
},
|
||||
{ selfApp: "demo2" },
|
||||
);
|
||||
await service.invoke("add", { a: "3" });
|
||||
expect(received).toEqual({ a: 3 });
|
||||
});
|
||||
|
||||
test("rejects schema-invalid input without invoking the handler", async () => {
|
||||
let called = false;
|
||||
const service = implement(
|
||||
openContract,
|
||||
{
|
||||
add: async ({ a }) => {
|
||||
called = true;
|
||||
return { a };
|
||||
},
|
||||
},
|
||||
{ selfApp: "demo" },
|
||||
);
|
||||
const result = await service.invoke("add", { a: "not-a-number" });
|
||||
expect(called).toBe(false);
|
||||
expect(result).toMatchObject({ ok: false, code: RPC_ERROR_CODES.invalid });
|
||||
});
|
||||
|
||||
test("unknown procedure refused", async () => {
|
||||
const service = implement(openContract, { add: async ({ a }) => ({ a }) }, { selfApp: "demo" });
|
||||
const result = await service.invoke("subtract", {});
|
||||
expect(result).toMatchObject({ ok: false, code: RPC_ERROR_CODES.unknown });
|
||||
});
|
||||
|
||||
test("handler throw becomes opaque; a secret in the message does not survive", async () => {
|
||||
const secret = "sk-super-secret-db-password";
|
||||
const service = implement(
|
||||
openContract,
|
||||
{
|
||||
add: async () => {
|
||||
throw new Error(`db error using ${secret}`);
|
||||
},
|
||||
},
|
||||
{ selfApp: "demo" },
|
||||
);
|
||||
const result = await service.invoke("add", { a: 1 });
|
||||
expect(result.ok).toBe(false);
|
||||
if (!result.ok) {
|
||||
expect(result.code).toBe(RPC_ERROR_CODES.handler);
|
||||
expect(result.message).not.toContain(secret);
|
||||
}
|
||||
});
|
||||
|
||||
test("a declared permission is enforced before the handler runs", async () => {
|
||||
let called = false;
|
||||
const service = implement(
|
||||
guardedContract,
|
||||
{
|
||||
createInvoice: async ({ amountCents }) => {
|
||||
called = true;
|
||||
return { invoiceId: `inv_${amountCents}` };
|
||||
},
|
||||
},
|
||||
{ selfApp: "billing", checkPermission: async () => false },
|
||||
);
|
||||
const result = await service.invoke("createInvoice", { amountCents: 5 });
|
||||
expect(called).toBe(false);
|
||||
expect(result).toMatchObject({ ok: false, code: RPC_ERROR_CODES.denied });
|
||||
});
|
||||
|
||||
test("a permission check that throws denies", async () => {
|
||||
let called = false;
|
||||
const service = implement(
|
||||
guardedContract,
|
||||
{
|
||||
createInvoice: async ({ amountCents }) => {
|
||||
called = true;
|
||||
return { invoiceId: `inv_${amountCents}` };
|
||||
},
|
||||
},
|
||||
{
|
||||
selfApp: "billing",
|
||||
checkPermission: async () => {
|
||||
throw new Error("permission store unavailable");
|
||||
},
|
||||
},
|
||||
);
|
||||
const result = await service.invoke("createInvoice", { amountCents: 5 });
|
||||
expect(called).toBe(false);
|
||||
expect(result).toMatchObject({ ok: false, code: RPC_ERROR_CODES.denied });
|
||||
});
|
||||
|
||||
test("a declared permission with no checkPermission configured denies (fail closed)", async () => {
|
||||
let called = false;
|
||||
const service = implement(
|
||||
guardedContract,
|
||||
{
|
||||
createInvoice: async ({ amountCents }) => {
|
||||
called = true;
|
||||
return { invoiceId: `inv_${amountCents}` };
|
||||
},
|
||||
},
|
||||
{ selfApp: "billing" },
|
||||
);
|
||||
const result = await service.invoke("createInvoice", { amountCents: 5 });
|
||||
expect(called).toBe(false);
|
||||
expect(result).toMatchObject({ ok: false, code: RPC_ERROR_CODES.denied });
|
||||
});
|
||||
|
||||
test("a valid identity token reaches the handler as a subject", async () => {
|
||||
configure("web");
|
||||
const token = await exportSubjectContext(
|
||||
{ user: { id: "u1" }, locals: {} } as never,
|
||||
"billing",
|
||||
);
|
||||
const service = implement(
|
||||
guardedContract,
|
||||
{
|
||||
createInvoice: async ({ amountCents }, ctx) => ({
|
||||
invoiceId: `${ctx.subject?.subjectId}_${amountCents}`,
|
||||
}),
|
||||
},
|
||||
{ selfApp: "billing", checkPermission: async () => true },
|
||||
);
|
||||
const result = await service.invoke("createInvoice", { amountCents: 5 }, token);
|
||||
expect(result).toEqual({ ok: true, value: { invoiceId: "u1_5" } });
|
||||
});
|
||||
|
||||
test("a bad identity token is refused rather than downgraded to anonymous", async () => {
|
||||
configure("web");
|
||||
let called = false;
|
||||
const service = implement(
|
||||
openContract,
|
||||
{
|
||||
add: async ({ a }, ctx) => {
|
||||
called = true;
|
||||
expect(ctx.subject).toBeUndefined();
|
||||
return { a };
|
||||
},
|
||||
},
|
||||
{ selfApp: "demo" },
|
||||
);
|
||||
const result = await service.invoke("add", { a: 1 }, "garbage-token");
|
||||
expect(called).toBe(false);
|
||||
expect(result).toMatchObject({ ok: false, code: RPC_ERROR_CODES.identity });
|
||||
});
|
||||
|
||||
describe("C1: prototype-chain procedure names cannot bypass the permission gate", () => {
|
||||
const PROTO_NAMES = [
|
||||
"constructor",
|
||||
"toString",
|
||||
"valueOf",
|
||||
"hasOwnProperty",
|
||||
"__proto__",
|
||||
"isPrototypeOf",
|
||||
];
|
||||
|
||||
for (const name of PROTO_NAMES) {
|
||||
test(`"${name}" resolves as unknown, not as a handler`, async () => {
|
||||
let permissionChecked = false;
|
||||
const service = implement(
|
||||
guardedContract,
|
||||
{
|
||||
createInvoice: async ({ amountCents }) => ({ invoiceId: `inv_${amountCents}` }),
|
||||
},
|
||||
{
|
||||
selfApp: "billing",
|
||||
checkPermission: async () => {
|
||||
permissionChecked = true;
|
||||
return false;
|
||||
},
|
||||
},
|
||||
);
|
||||
const result = await service.invoke(name, { amountCents: 1 });
|
||||
expect(result).toMatchObject({ ok: false, code: RPC_ERROR_CODES.unknown });
|
||||
expect(permissionChecked).toBe(false);
|
||||
});
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,77 @@
|
||||
import { describe, expect, test } from "bun:test";
|
||||
import { RPC_ERROR_CODES, success } from "../src/errors.ts";
|
||||
import { inProcessTransport } from "../src/transport.ts";
|
||||
|
||||
describe("inProcessTransport", () => {
|
||||
test("routes to the right handler and passes identity through", async () => {
|
||||
let seenIdentity: string | undefined;
|
||||
const transport = inProcessTransport({
|
||||
"billing/createInvoice": (payload, identity) => {
|
||||
seenIdentity = identity;
|
||||
return success({ echoed: payload });
|
||||
},
|
||||
});
|
||||
const result = await transport.call(
|
||||
{ app: "billing", service: "billing", procedure: "createInvoice" },
|
||||
{ amountCents: 1 },
|
||||
{ identity: "tok-123" },
|
||||
);
|
||||
expect(result).toEqual({ ok: true, value: { echoed: { amountCents: 1 } } });
|
||||
expect(seenIdentity).toBe("tok-123");
|
||||
});
|
||||
|
||||
test("an unregistered procedure yields non-retryable RPC_UNKNOWN", async () => {
|
||||
const transport = inProcessTransport({});
|
||||
const result = await transport.call(
|
||||
{ app: "billing", service: "billing", procedure: "nope" },
|
||||
{},
|
||||
{},
|
||||
);
|
||||
expect(result).toEqual({
|
||||
ok: false,
|
||||
code: RPC_ERROR_CODES.unknown,
|
||||
message: "Unknown procedure",
|
||||
retryable: false,
|
||||
});
|
||||
});
|
||||
|
||||
test("an already-aborted signal fails without invoking the handler", async () => {
|
||||
let called = false;
|
||||
const transport = inProcessTransport({
|
||||
"billing/createInvoice": () => {
|
||||
called = true;
|
||||
return success({});
|
||||
},
|
||||
});
|
||||
const controller = new AbortController();
|
||||
controller.abort();
|
||||
const result = await transport.call(
|
||||
{ app: "billing", service: "billing", procedure: "createInvoice" },
|
||||
{},
|
||||
{ signal: controller.signal },
|
||||
);
|
||||
expect(called).toBe(false);
|
||||
expect(result.ok).toBe(false);
|
||||
expect(result).toMatchObject({ code: RPC_ERROR_CODES.transport });
|
||||
});
|
||||
|
||||
test("a handler that throws becomes an opaque failure with no leaked message text", async () => {
|
||||
const secret = "sk-super-secret-database-password-xyz";
|
||||
const transport = inProcessTransport({
|
||||
"billing/createInvoice": () => {
|
||||
throw new Error(`connection failed with credential ${secret}`);
|
||||
},
|
||||
});
|
||||
const result = await transport.call(
|
||||
{ app: "billing", service: "billing", procedure: "createInvoice" },
|
||||
{},
|
||||
{},
|
||||
);
|
||||
expect(result.ok).toBe(false);
|
||||
if (!result.ok) {
|
||||
expect(result.code).toBe(RPC_ERROR_CODES.handler);
|
||||
expect(result.message).not.toContain(secret);
|
||||
expect(result.message).not.toContain("connection failed");
|
||||
}
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user