Skip to content

Commit 58c01ca

Browse files
committed
Store task definitions in a prototype-safe Map
`FederationBuilderImpl.taskDefinitions` was a plain object, so the duplicate check `name in this.taskDefinitions` and the lookups `this.taskDefinitions[taskName]` consulted the prototype chain. Task names are arbitrary user-supplied strings, so a name such as "constructor", "toString", or "__proto__" was wrongly reported as already defined and resolved to an inherited method on lookup. Switch the registry to a `Map`, which is immune to prototype keys by construction and avoids the clone footgun where a later spread or `Object.assign` would silently reintroduce the prototype. Sibling registries stay plain objects since they are keyed by controlled values (type-id URLs). Add a regression test covering names that collide with `Object.prototype`. #803 (comment) Assisted-by: Claude Code:claude-opus-4-8
1 parent f7c9a34 commit 58c01ca

3 files changed

Lines changed: 38 additions & 11 deletions

File tree

packages/fedify/src/federation/builder.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,7 @@ export class FederationBuilderImpl<TContextData>
188188
TContextData
189189
>
190190
>;
191-
taskDefinitions: Record<string, TaskDefinitionInternal<TContextData>>;
191+
taskDefinitions: Map<string, TaskDefinitionInternal<TContextData>>;
192192

193193
/**
194194
* Symbol registry for unique identification of unnamed symbols.
@@ -201,7 +201,7 @@ export class FederationBuilderImpl<TContextData>
201201
this.objectTypeIds = {};
202202
this.collectionCallbacks = {};
203203
this.collectionTypeIds = {};
204-
this.taskDefinitions = {};
204+
this.taskDefinitions = new Map();
205205
}
206206

207207
/**
@@ -267,7 +267,7 @@ export class FederationBuilderImpl<TContextData>
267267
f.unverifiedActivityHandler = this.unverifiedActivityHandler;
268268
f.outboxPermanentFailureHandler = this.outboxPermanentFailureHandler;
269269
f.idempotencyStrategy = this.idempotencyStrategy;
270-
f.taskDefinitions = { ...this.taskDefinitions };
270+
f.taskDefinitions = new Map(this.taskDefinitions);
271271
return f;
272272
}
273273

@@ -607,18 +607,18 @@ export class FederationBuilderImpl<TContextData>
607607
name: string,
608608
options: TaskDefinitionOptions<TContextData, TSchema>,
609609
): TaskDefinition<TContextData, StandardSchemaV1.InferOutput<TSchema>> {
610-
if (name in this.taskDefinitions) {
610+
if (this.taskDefinitions.has(name)) {
611611
throw new TypeError(`Task ${JSON.stringify(name)} is already defined.`);
612612
}
613-
this.taskDefinitions[name] = {
613+
this.taskDefinitions.set(name, {
614614
name,
615615
schema: options.schema,
616616
handler: options.handler as TaskHandler<TContextData, unknown>,
617617
onError: options
618618
.onError as TaskDefinitionInternal<TContextData>["onError"],
619619
retryPolicy: options.retryPolicy,
620620
queue: options.queue,
621-
};
621+
});
622622
return { name, schema: options.schema };
623623
}
624624

packages/fedify/src/federation/middleware.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -957,7 +957,7 @@ export class FederationImpl<TContextData>
957957
}
958958

959959
resolveTaskQueue(taskName: string): MessageQueue | undefined {
960-
const def = this.taskDefinitions[taskName];
960+
const def = this.taskDefinitions.get(taskName);
961961
const resolved = def?.queue ?? this.taskQueue;
962962
if (resolved != null) return resolved;
963963
return this.taskQueueResolution === "strict" ? undefined : this.outboxQueue;
@@ -2119,7 +2119,7 @@ export class FederationImpl<TContextData>
21192119
message: TaskMessage,
21202120
): Promise<void> {
21212121
const logger = getLogger(["fedify", "federation", "task"]);
2122-
const def = this.taskDefinitions[message.taskName];
2122+
const def = this.taskDefinitions.get(message.taskName);
21232123
if (def == null) {
21242124
// Unknown task: a handler won't appear by retrying. Drop and log.
21252125
logger.warn(

packages/fedify/src/federation/tasks/tasks.test.ts

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,33 @@ test("defineTask()", async (t) => {
169169
);
170170
});
171171

172+
await t.step("accepts names that collide with Object.prototype", () => {
173+
const federation = createFederation<void>({
174+
...baseOptions,
175+
queue: { task: new MockQueue() },
176+
}) as FederationImpl<void>;
177+
// These names exist on Object.prototype; a plain-object registry would
178+
// mistake them for already-defined tasks (`name in {}`) and would return
179+
// an inherited method on lookup.
180+
for (const name of ["constructor", "toString", "hasOwnProperty"]) {
181+
const task = federation.defineTask(name, {
182+
schema: stringSchema,
183+
handler: () => {},
184+
});
185+
strictEqual(task.name, name);
186+
strictEqual(federation.taskDefinitions.get(name)?.name, name);
187+
}
188+
// A genuine duplicate still throws.
189+
throws(
190+
() =>
191+
federation.defineTask("toString", {
192+
schema: stringSchema,
193+
handler: () => {},
194+
}),
195+
{ name: "TypeError", message: /already defined/ },
196+
);
197+
});
198+
172199
await t.step("build() clones the task registry", async () => {
173200
const builder = createFederationBuilder<void>();
174201
builder.defineTask("first", {
@@ -187,11 +214,11 @@ test("defineTask()", async (t) => {
187214
...baseOptions,
188215
queue: { task: new MockQueue() },
189216
}) as FederationImpl<void>;
190-
deepStrictEqual(Object.keys(f1.taskDefinitions), ["first"]);
191-
deepStrictEqual(Object.keys(f2.taskDefinitions), ["first", "second"]);
217+
deepStrictEqual([...f1.taskDefinitions.keys()], ["first"]);
218+
deepStrictEqual([...f2.taskDefinitions.keys()], ["first", "second"]);
192219
// Defining on a built federation does not leak back into the builder:
193220
f1.defineTask("third", { schema: stringSchema, handler: () => {} });
194-
deepStrictEqual(Object.keys(f2.taskDefinitions), ["first", "second"]);
221+
deepStrictEqual([...f2.taskDefinitions.keys()], ["first", "second"]);
195222
});
196223
});
197224

0 commit comments

Comments
 (0)