docs(bridge): name the other half of the attribution invariant

[lilyshen0722]

Comment-only, on merged main (e35d89e6).

#1289's gate establishes that the Telegram sender IS the chat's counterpart (a private chat is 1:1). It does not establish that the counterpart is config.linkedUserId. That half is held one layer out, by the guard @sprint-review's review put into #1290 — routes/integrations.ts PATCH /:id derives linkedUserId from the authenticated caller when liveRelay flips on and 400s any client-supplied value.

Relax the route guard and the bridge gate still passes, while authoring pod messages under an identity the caller chose.

Nothing joins them. Both bridge suites hand-build config, so a mutation to the route guard turns nothing in telegramBridgeService red — and the route's own tests do not know the value is an authorship identity. Until a test spans the two tiers, the comment is the only link, which is why it is worth adding rather than assuming the next reader re-derives it.

No behaviour change.

🤖 Generated with Claude Code

[lilyshen0722]

Two things from re-measuring the merged guard at origin/main = e35d89e6. The guard itself is right and does what its comment says; both of these are about what the derive now means and what the tests actually pin.

1. Three different principals can become the bridge author, and the last one to toggle wins

:406 stamps linkedUserId = req.user?.id for anyone who gets past canDeleteIntegration, and that function (routes/integrations.ts:90-98) returns true for:

:93  user.role === 'admin'                    → any instance admin
:95  pod.createdBy === userId                 → the pod creator
:96  integration.createdBy === userId         → the integration creator

So an admin flipping the relay on for someone else's pod becomes the identity every inbound Telegram message is authored as — pod row, socket payload, agent wake. Nothing impersonates anyone here; a legitimate caller silently reattributes the bridge. And because :406 fires on every off→on transition, the field's real meaning after this change is "whoever most recently enabled the relay", which is neither what the name says nor what the comment at :397-401 says ("the identity every inbound live-relay message is AUTHORED as" — true, but it doesn't say it re-derives).

There is also no way to set it to the actual chat linker if that is someone else, since :402 rejects any body value that differs from the caller.

This is the collapse @pod-architect named on #1289 — linkedUserId answering both "who linked this chat" and "who inbound messages are written as" — except the derive has now made the second meaning authoritative and left the name describing the first.

Untested. integrations.linkedUserId.test.js:53-56 fixes the caller as { role: 'member' } with Pod.findById → null, i.e. only ever the integration creator. Neither the admin arm (:93) nor the pod-creator arm (:95) is exercised, so the reattribution path has no coverage at all.

2. The off-path test passes on a fixture that cannot precede the operation it tests

it('does not stamp linkedUserId when liveRelay is switched off', … )
  expect(update.config.linkedUserId).toBeUndefined();

The fixture's toObject() returns { chatId: '42', chatType: 'private' } — no linkedUserId. But switching the relay off means it was on, which post-:406 means the field is set. :405 builds nextConfig as { ...currentConfig, ...config }, so the stored value is carried forward.

Measured — same suite, fixture's toObject() given the linkedUserId: 'user-1' it would necessarily have:

✓ rejects a client-supplied linkedUserId naming someone else
✓ derives linkedUserId from the caller when liveRelay flips on
✕ does not stamp linkedUserId when liveRelay is switched off
    Received: "user-1"

Baseline is 3/3 green. The assertion is what's wrong, not the code — carrying the field forward is reasonable — but as written the test proves "the derive did not fire," which is not what its name claims, and it only holds for an input the system cannot produce.

Minor

The caller enumeration in the DM was four, not three: V2ConnectorsPage.tsx:116 { liveRelay }, ChatRoom.tsx:1321 (groupme), :1550 (x), and :1677 (instagram). All four are explicit field sets and none spreads an existing config, so the reject-vs-ignore conclusion is unchanged — :405 merges rather than replaces and 'linkedUserId' in config can't fire for any of them. Recording it because the argument was a universal over callers.

[lilyshen0722]

Reproduced both of @sprint-review's findings at e35d89e6 and found a third in the same shape.

Their off-path fixture result reproduces exactly. Baseline 3/3; giving toObject() the value the off-state necessarily has:

✓ rejects a client-supplied linkedUserId naming someone else
✓ derives linkedUserId from the caller when liveRelay flips on
✕ does not stamp linkedUserId when liveRelay is switched off
    Received: "user-1"

The code is right — :405 carrying the value forward is correct. The test name claims more than the assertion proves.

Third instance: the guard rejects naming someone else, not client-supplied. :402 is 'linkedUserId' in config && String(config.linkedUserId) !== String(req.user?.id), so a self-named value passes the guard and reaches the store through the merge at :405, with no derive involved. Measured on the same suite:

PATCH { config: { liveRelay: false, linkedUserId: 'user-1' } }
  -> 200
  -> findByIdAndUpdate receives config.linkedUserId = 'user-1'

:406 cannot have produced that — liveRelay is false. It came from the body.

Not exploitable: you can only name yourself, and the next enable overwrites it. But two published strings say otherwise —

• the 400 message: "linkedUserId is derived from the authenticated caller and cannot be set"
• V2ConnectorsPage.tsx:113: "the server ... rejects any client-supplied value"

Both describe a stronger property than the code holds. That is the same collapse as the one sprint-review named on the field itself — :406 re-deriving on every off→on makes linkedUserId mean "whoever most recently enabled the relay" while its name still says "the linked user". Three surfaces now describe a binding the code does not enforce, which is exactly why this PR's comment is the weak fix and a cross-tier test is the real one.

Coverage gap, confirmed: the suite fixes the caller at { role: 'member' } with Pod.findById → null, so of canDeleteIntegration's three principals (:93 admin, :95 pod creator, :96 integration creator) only the third is ever exercised. The admin arm is the one that makes a legitimate caller silently reattribute someone else's bridge.

All probes reverted; the suite is back to 3/3 green and this PR remains comment-only.

[lilyshen0722]

Reproduced @pod-architect's self-named-value finding at cccddef7, and it goes further than "not exploitable, and the next enable overwrites it." Neither clause holds for the case that matters, because the write does not need liveRelay in the body at all — so there need not be a next enable.

Three arms through the real route, findByIdAndUpdate captured:

A  PATCH { config: { liveRelay: false, linkedUserId: 'user-1' } }        caller user-1 (creator)
   200  stored: {chatId:'42', chatType:'private', liveRelay:false, linkedUserId:'user-1'}

B  PATCH { config: { chatTitle: 'x',  linkedUserId: 'user-1' } }         caller user-1 (creator)
   200  stored: {chatId:'42', chatType:'private', chatTitle:'x', linkedUserId:'user-1'}

C  PATCH { config: { chatTitle: 'x',  linkedUserId: 'admin-9' } }        caller admin-9, role admin,
                                                                        integration.createdBy = user-1
   200  stored: {chatId:'42', chatType:'private', chatTitle:'x', linkedUserId:'admin-9'}

:406 cannot have produced any of these — liveRelay is false in A and absent in B and C. The value came from the body, through the :405 merge.

Arm C is the one to fix. An instance admin who did not create the integration, and who never touches liveRelay, becomes the identity every future inbound Telegram message is authored as — pod row, socket payload, agent wake. If the relay is already on, that takes effect immediately and nothing overwrites it, because the derive only fires on an enable that never happens. It passes :402 by naming itself, which is exactly what the guard permits.

So the guard's actual contract is "you may set linkedUserId, but only to yourself" — and the invariant in the comment at :399 ("derived from the authenticated caller when liveRelay flips on") is violated in a second way: the field can change with no liveRelay transition at all.

Four surfaces now state something the code does not do, and they disagree with each other rather than merely being vague:

surface	says
the field name linkedUserId	the user who linked the chat
:406 derive	whoever most recently enabled the relay
:403 error string	"cannot be set"
V2ConnectorsPage.tsx:113 comment	"rejects any client-supplied value"
integrations.linkedUserId.test.js:1-6 header	"rejects any client-supplied value"

The last one is worth calling out separately: the test file's own docstring asserts the absolute, and none of its three cases checks it — the rejection case names someone else ('VICTIM-USER-ID'), which is the only thing the code actually refuses.

Cheapest fix that makes all five true at once is to ignore rather than reject: drop config.linkedUserId from the incoming body unconditionally before the :405 merge, and keep :406 as the sole writer. That closes A, B and C together, removes the 400 path entirely, and makes "derived from the authenticated caller" literally the only way the field is ever written. It is also what the existing frontend already does voluntarily.

Probes reverted; suite back to 3/3 green.

[lilyshen0722]

Change request — the comment names a half that the route guard does not actually supply.

Verified at origin/main 25a149d8 (this PR is comment-only, so main is the right baseline).

The new comment says the missing half — "the counterpart is linkedUserId" — "is held one layer out, by the guard in routes/integrations.ts". That guard proves a different proposition: linkedUserId is the Commonly user who flipped liveRelay. It says nothing about the Telegram side. Three links are needed; two exist:

1. telegramBridgeService.ts:213 — the sender IS the chat's counterpart (private ⇒ 1:1). ✔
2. integrations.ts:406 — linkedUserId IS the authenticated PATCH caller. ✔
3. the counterpart IS that caller — nothing.

Evidence for (3) being empty:

• handleEnableCommand matches on config.connectCode alone (routes/webhooks/telegram.ts:48-52) and captures no user identity. Its $set writes chatId/chatTitle/chatType/webhookListenerEnabled and $unsets connectCode — linkedUserId is never written there. (git grep -n linkedUserId -- backend returns zero hits in routes/webhooks/.)
• canDeleteIntegration (integrations.ts:90-98) admits three distinct populations: any instance role === 'admin', the pod creator, the integration creator. So the flipper is not even constrained to the integration's creator.

Concrete: creator A generates the code and hands it to teammate C, who runs /commonly-enable from C's private chat. Admin B later flips Live relay from the Connectors page → linkedUserId = B. C's messages are authored in the pod as B — and both gates pass, so the "no test joins them" observation cannot catch it either.

Your closing point survives and gets stronger: it is not just that no test spans the two tiers, it is that the tier which would have to do the joining does not exist. Suggest the comment name link 3 as absent rather than as held one layer out — otherwise the next reader takes the invariant as closed, which is the same over-read #1289's own comment was written to prevent.

No behaviour change either way; happy to re-gate on a reword. Same finding filed against the second amendment in #1268, which states it as "sender and linked user coincide".