Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 79 additions & 0 deletions packages/apps/src/contexts/activity.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -641,4 +641,83 @@ describe('ActivityContext', () => {
});
});
});

describe('constructor — prototype method shadowing', () => {
it('drops context properties that would shadow prototype methods', () => {
const activity = buildIncomingMessageActivity('Hello world');
const malicious = {
send: jest.fn(),
reply: jest.fn(),
quote: jest.fn(),
signin: jest.fn(),
signout: jest.fn(),
};

const ctx = new ActivityContext({
appId: 'test-app',
activity,
ref: mockRef,
log: mockLogger,
api: mockApiClient,
appGraph: {} as GraphClient,
userGraph: {} as GraphClient,
storage: mockStorage,
connectionName: 'test-connection',
next: jest.fn(),
activitySender: mockSender,
...malicious,
} as any);

for (const name of ['send', 'reply', 'quote', 'signin', 'signout'] as const) {
expect(Object.prototype.hasOwnProperty.call(ctx, name)).toBe(false);
expect(ctx[name]).toBe(ActivityContext.prototype[name]);
}
});

it('still allows new properties from extra context', () => {
const activity = buildIncomingMessageActivity('Hello world');
const ctx = new ActivityContext({
appId: 'test-app',
activity,
ref: mockRef,
log: mockLogger,
api: mockApiClient,
appGraph: {} as GraphClient,
userGraph: {} as GraphClient,
storage: mockStorage,
connectionName: 'test-connection',
next: jest.fn(),
activitySender: mockSender,
customField: 'still here',
} as any);

expect((ctx as any).customField).toBe('still here');
});

it('routes ctx.send() to the prototype method even when a colliding key is supplied', async () => {
const activity = buildIncomingMessageActivity('Hello world');
const maliciousSend = jest.fn();

const ctx = new ActivityContext({
appId: 'test-app',
activity,
ref: mockRef,
log: mockLogger,
api: mockApiClient,
appGraph: {} as GraphClient,
userGraph: {} as GraphClient,
storage: mockStorage,
connectionName: 'test-connection',
next: jest.fn(),
activitySender: mockSender,
// Simulates a plugin's onActivity context attempting to inject its own send.
send: maliciousSend,
} as any);

await ctx.send({ type: 'message', text: 'real send' });

expect(maliciousSend).not.toHaveBeenCalled();
expect(mockSender.send).toHaveBeenCalledTimes(1);
});
});
});
31 changes: 31 additions & 0 deletions packages/apps/src/contexts/activity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,16 @@ export class ActivityContext<T extends Activity = Activity, TExtraCtx extends {}
rest.activity = TypingActivity.from(rest.activity).toInterface();
}

// SECURITY: drop any keys in `rest` that would shadow prototype methods.
// Plugin-supplied context can add new properties via the [key: string]: any
// index signature, but must not overwrite methods that callers rely on for
// trust (send, reply, quote, signin, signout).
for (const key of PROTECTED_METHOD_NAMES) {
if (key in rest) {
delete (rest as Record<string, unknown>)[key];
}
}

Object.assign(this, rest);
this.activitySender = activitySender;
this.next = next;
Expand Down Expand Up @@ -485,3 +495,24 @@ export class ActivityContext<T extends Activity = Activity, TExtraCtx extends {}
}

}

// Names of prototype methods (and getters) on ActivityContext that must not be
// shadowed by instance properties from external context. Computed once from the
// prototype chain at module load, so any method or accessor added to the class
// in the future is protected automatically with no maintenance.
const PROTECTED_METHOD_NAMES: ReadonlySet<string> = (() => {
const names = new Set<string>();
let proto: object | null = ActivityContext.prototype;
while (proto && proto !== Object.prototype) {
for (const name of Object.getOwnPropertyNames(proto)) {
if (name === 'constructor') continue;
const descriptor = Object.getOwnPropertyDescriptor(proto, name);
if (!descriptor) continue;
if (typeof descriptor.value === 'function' || typeof descriptor.get === 'function') {
names.add(name);
}
Comment thread
corinagum marked this conversation as resolved.
}
proto = Object.getPrototypeOf(proto);
}
return names;
})();
Loading