Skip to content

Filter colliding keys before Object.assign in ActivityContext#596

Merged
corinagum merged 1 commit into
mainfrom
cg/property-overwrite
May 27, 2026
Merged

Filter colliding keys before Object.assign in ActivityContext#596
corinagum merged 1 commit into
mainfrom
cg/property-overwrite

Conversation

@corinagum
Copy link
Copy Markdown
Collaborator

@corinagum corinagum commented May 26, 2026

Summary

The ActivityContext constructor calls Object.assign(this, rest) to copy the public options onto the instance. Because ActivityContext declares [key: string]: any to support plugin-supplied extra context, callers can pass arbitrary properties through. If any of those property names happened to match a method defined on the prototype (send, reply, quote, signin, signout, next, toInterface), the spread would shadow the method with whatever value the caller supplied, leaving the context with a broken or removed behavior for that name.

This change filters rest against a derived set of method names derived from the prototype chain at module load and drops any colliding key before the assign. Legitimate extra context properties still flow through unchanged.

Test plan

  • New unit tests in activity.test.ts cover: drop of shadowing properties, preservation of non-colliding extra properties, and that ctx.send() still routes to the prototype implementation when an extra property of the same name is supplied
  • Full activity.test.ts suite passes (28/28)

🤖 Generated with Claude Code

@corinagum corinagum marked this pull request as ready for review May 26, 2026 22:58
Copilot AI review requested due to automatic review settings May 26, 2026 22:58
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens ActivityContext construction by preventing user/plugin-supplied extra context properties from overwriting (shadowing) ActivityContext prototype methods when options are copied onto the instance.

Changes:

  • Filters constructor rest options to drop keys that collide with ActivityContext prototype methods/getters before Object.assign(this, rest).
  • Adds a module-load computed PROTECTED_METHOD_NAMES set derived from the ActivityContext prototype chain to keep protection automatically in sync with future method additions.
  • Adds unit tests to ensure colliding keys are dropped, non-colliding keys still pass through, and ctx.send() continues to call the real implementation.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
packages/apps/src/contexts/activity.ts Drops colliding keys before assigning constructor options; computes protected prototype method names once at module load.
packages/apps/src/contexts/activity.test.ts Adds regression tests covering prototype method shadowing prevention and preservation of extra context fields.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread packages/apps/src/contexts/activity.ts
@corinagum corinagum merged commit ac3a249 into main May 27, 2026
8 checks passed
@corinagum corinagum deleted the cg/property-overwrite branch May 27, 2026 18:12
corinagum added a commit that referenced this pull request May 27, 2026
## Summary

Brings `origin/main` into `release` for the **2.0.12** release. **No
carve-outs this time** — Quoted Replies is included.

`version.json`: `2.0.12-preview.{height}` → `2.0.12`.

Aligns with teams.py 2.0.12 (PR #442 already on PyPI / pending publish).

## What's in this release (delta from 2.0.11)

**Features**
- Quoted Replies + new quotes features — previously held back via the
2.0.11 carve-out (#576)
- SuggestedActionSubmitActivity for `suggestedAction/submit` invoke
(#591)
- Default targeted replies for targeted inbound messages (#592)
- Filter colliding keys before `Object.assign` in `ActivityContext`
(#596)
- Prompt Preview support (#536)
- A2A sample (#584)
- AI/MCPClient (#572)
- Allow passing custom HTTP client via `AppOptions`-equivalent
- Reactions API marked GA (#575), Rename `ReactionClient.remove()` →
`delete()` (#567)

**Security & fixes**
- Security hardening: tighten cross-origin policies (#595)
- Lock JsonWebToken trust-boundary contract (#586)
- `fix(deps)`: npm audit fix — clears all high-severity advisories
(#599)
- `fix(apps)`: log inbound activities at info, warn on missing
Authorization (#568)
- `fix`: App user-agent merging (#573)
- `fix`: Add null checks in `local-memory.ts` to prevent role crash
(#438)
- `fix`: `@microsoft/teams.client` import error in webpack 5 (#566)
- Switch to named imports without subpaths (#561)

**Deprecations / package changes**
- Deprecate `DevtoolsPlugin` in favor of Microsoft 365 Agents Playground
(#593)
- Remove in-repo Teams CLI package (#580) — moved to
`teams-sdk/packages/cli`
- Deprecate AI Libraries (#588)
- Correct imports + return types in misc. packages (#589)

**Dependency bumps**
- turbo 2.8.11 → 2.9.14 (#587)
- qs 6.15.0 → 6.15.2 (#594)
- hono 4.12.14 → 4.12.16 (#562)
- npm audit fix bundle (#599): axios 1.13.5 → 1.16.1, fast-uri 3.1.0 →
3.1.2, ws 8.19.0 → 8.21.0, @azure/msal-node hoisted 5.2.2, uuid 11.1.0 →
11.1.1, @azure/identity 4.13.0 → 4.13.1, express-rate-limit, ip-address,
brace-expansion, etc.

## Quoted Replies inclusion notes

- Teams client rendering is in-sync with the wire format as of
2026-05-06.
- APX QR rollout completed: Public 2026-04-10, GCCH/DoD/Gallatin
2026-04-14.
- No SDK-side carve-outs needed; this matches the teams.py 2.0.12 (PR
#442) decision.

## Branch structure note

`prep-release/2.0.12` is reset to `origin/main` + a one-line
`version.json` bump. Diff against `release` (this PR) shows the full
delta from 2.0.11. We deliberately did not use a 2-parent merge commit —
the auto-merge tried to interleave non-overlapping hunks from main's QR
additions with release's QR carve-outs in `activity.ts`, producing a
semantically broken file. Reset-to-main + version bump is the clean way
to express "release should equal main."

## Test plan

- [x] `npm install` clean (post-audit-fix lockfile)
- [x] `npm run build` — 33/33 targets clean
- [x] `npm test` — 14/14 task targets, 279+ tests pass
- [x] E2E echo bot smoke test against `cg-test-bot-py` via canary ABS —
done on `cg/audit-deps-ts` (now merged); covers axios + msal-node code
paths
- [ ] Pipeline Build + Test stages green on `release` after merge
- [ ] Publish pipeline run with **Public** → ESRP → npm
`@microsoft/teams.*@2.0.12`
- [ ] Smoke install: `npm view @microsoft/teams.apps@2.0.12` after
publish
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants