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
20 changes: 20 additions & 0 deletions backend/__tests__/unit/routes/installables.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,26 @@ describe('installable connector routes', () => {
expect(installationService.install).not.toHaveBeenCalled();
});

it('rejects a departed CREATOR before any install row is claimed', async () => {
// TASK-161: `leavePod` filters `members` and never clears `createdBy`, so
// this pod has no members and still names the caller as its creator. The
// install route used to admit them through the creator clause; installing
// seeds the pod's gate ON, so it must take the pod's write path instead.
Pod.findById.mockResolvedValue({
_id: podId,
createdBy: { toString: () => '64b64c48c4f37a6b2f34c111' },
members: [],
});

const res = await request(app)
.post('/api/installables/telegram/install')
.set(auth)
.send({ podId });

expect(res.status).toBe(403);
expect(installationService.install).not.toHaveBeenCalled();
});

it('rejects an invalid podId without querying a pod or claiming an install', async () => {
const response = await request(app)
.post('/api/installables/telegram/install')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ jest.mock('../../../middleware/integrationRateLimit', () => ({
listIntegrationsRateLimit: (_req, _res, next) => next(),
}));
jest.mock('../../../services/dmService', () => ({ canViewPod: jest.fn(() => true) }));
jest.mock('../../../utils/isPodMember', () => jest.fn(() => true));
jest.mock('jsonwebtoken', () => ({ sign: jest.fn(() => 't'), verify: jest.fn(), decode: jest.fn() }));
// The create path initializes and connects the provider; only the row matters.
jest.mock('../../../services/discordService', () => jest.fn().mockImplementation(() => ({
Expand Down
39 changes: 36 additions & 3 deletions backend/__tests__/unit/routes/integrations.linkedUserId.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ describe('PATCH /api/integrations/:id — user-scoped connector gates', () => {

it('checks every requested gate before writing any of them', async () => {
Pod.findById
.mockResolvedValueOnce({ _id: allowedPodId, createdBy: 'user-1', members: [] })
.mockResolvedValueOnce({ _id: allowedPodId, createdBy: 'someone-else', members: ['user-1'] })
.mockResolvedValueOnce({ _id: forbiddenPodId, createdBy: 'someone-else', members: [] });

const res = await request(app)
Expand All @@ -239,7 +239,9 @@ describe('PATCH /api/integrations/:id — user-scoped connector gates', () => {
});

it('allows the linked owner to write gates for pods they belong to', async () => {
Pod.findById.mockResolvedValue({ _id: allowedPodId, createdBy: 'user-1', members: [] });
// Listed in `members`, which is what "belong to" means — the fixture used to
// leave `members` empty and lean on `createdBy` (TASK-161).
Pod.findById.mockResolvedValue({ _id: allowedPodId, createdBy: 'someone-else', members: ['user-1'] });

const res = await request(app)
.patch(`/api/integrations/${userScopedIntegrationId}`)
Expand All @@ -251,7 +253,7 @@ describe('PATCH /api/integrations/:id — user-scoped connector gates', () => {
});

it('allows the linked owner to select a member pod as the active inbound destination', async () => {
Pod.findById.mockResolvedValue({ _id: allowedPodId, createdBy: 'user-1', members: [] });
Pod.findById.mockResolvedValue({ _id: allowedPodId, createdBy: 'someone-else', members: ['user-1'] });

const res = await request(app)
.patch(`/api/integrations/${userScopedIntegrationId}`)
Expand All @@ -262,6 +264,37 @@ describe('PATCH /api/integrations/:id — user-scoped connector gates', () => {
expect(update.podId).toBe(allowedPodId);
});

it.each([
['created and then left', { _id: 'pod', createdBy: 'user-1', members: [] }],
['never a member', { _id: 'pod', createdBy: 'someone-else', members: [] }],
])('refuses a gate for a pod the linked owner %s', async (_label, podDoc) => {
// TASK-161: `leavePod` filters `members` and never clears `createdBy`, so
// the first cell is a creator who left. Both must refuse, or a connector
// could aim inbound messages at a pod its owner cannot post in.
Pod.findById.mockResolvedValue({ ...podDoc, _id: allowedPodId });

const res = await request(app)
.patch(`/api/integrations/${userScopedIntegrationId}`)
.send({ config: { gates: { [allowedPodId]: { enabled: true } } } });

expect(res.status).toBe(403);
expect(Integration.findByIdAndUpdate).not.toHaveBeenCalled();
});

it.each([
['created and then left', { _id: 'pod', createdBy: 'user-1', members: [] }],
['never a member', { _id: 'pod', createdBy: 'someone-else', members: [] }],
])('refuses selecting a pod the linked owner %s as the active destination', async (_label, podDoc) => {
Pod.findById.mockResolvedValue({ ...podDoc, _id: allowedPodId });

const res = await request(app)
.patch(`/api/integrations/${userScopedIntegrationId}`)
.send({ podId: allowedPodId });

expect(res.status).toBe(403);
expect(Integration.findByIdAndUpdate).not.toHaveBeenCalled();
});

it('refuses selecting an active pod the linked owner is no longer a member of', async () => {
Pod.findById.mockResolvedValue({ _id: forbiddenPodId, createdBy: 'someone-else', members: [] });

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ jest.mock('../../../middleware/integrationRateLimit', () => ({
listIntegrationsRateLimit: (_req, _res, next) => next(),
}));
jest.mock('../../../services/dmService', () => ({ canViewPod: jest.fn() }));
jest.mock('../../../utils/isPodMember', () => jest.fn(() => true));
jest.mock('jsonwebtoken', () => ({ sign: jest.fn(() => 't'), verify: jest.fn(), decode: jest.fn() }));

const { MongoMemoryServer } = require('mongodb-memory-server');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ jest.mock('../../../middleware/integrationRateLimit', () => ({
listIntegrationsRateLimit: (_req, _res, next) => next(),
}));
jest.mock('../../../services/dmService', () => ({ canViewPod: jest.fn() }));
jest.mock('../../../utils/isPodMember', () => jest.fn(() => true));
jest.mock('jsonwebtoken', () => ({ sign: jest.fn(() => 't'), verify: jest.fn(), decode: jest.fn() }));

const { MongoMemoryServer } = require('mongodb-memory-server');
Expand Down
34 changes: 33 additions & 1 deletion backend/__tests__/unit/routes/integrations.validation.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,13 @@ describe('integration manifest validation', () => {

beforeEach(() => {
jest.clearAllMocks();
Pod.findById.mockResolvedValue({ _id: 'pod-1', createdBy: { toString: () => 'user-1' } });
// The pod lists the caller: a real pod lists its creator, and the
// connector sites read `pod.members` alone (TASK-161).
Pod.findById.mockResolvedValue({
_id: 'pod-1',
createdBy: { toString: () => 'user-1' },
members: [{ toString: () => 'user-1' }],
});
User.findById.mockResolvedValue({ _id: 'user-1' });
consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(() => {});
Integration.findOne.mockResolvedValue(null);
Expand Down Expand Up @@ -209,4 +215,30 @@ describe('integration manifest validation', () => {
expect(res.status).toBe(400);
expect(res.body.missing).toEqual(expect.arrayContaining(['channelId']));
});

it('refuses to create a connection for a pod the caller created and then left', async () => {
// TASK-161, from Vera 74671: `:393` is the write gate, and the population it
// exists to refuse is exactly the departed creator — the one person the
// permissive predicate admitted. Every other create arm here lists the
// caller in `members`, so before this arm the site had no witness at all:
// `createdBy` merely identifies the owner and must not stand in for it.
Pod.findById.mockResolvedValue({
_id: 'pod-1',
createdBy: { toString: () => 'user-1' },
members: [],
});
const before = Integration.__getLastInstance();

const res = await request(app)
.post('/api/integrations')
.send({
podId: 'pod-1',
type: 'groupme',
config: { webhookListenerEnabled: true },
});

expect(res.status).toBe(403);
expect(res.body.message).toBe('Access denied');
expect(Integration.__getLastInstance()).toBe(before);
});
});
19 changes: 19 additions & 0 deletions backend/__tests__/unit/routes/slackOAuth.installables.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,25 @@ describe('Slack installable OAuth routes', () => {
expect(SlackApi.mock.results[0].value.postMessage).toHaveBeenCalledWith('D1', '[<Evil|pod>] connected');
});

test('refuses to confirm a bind whose pod the caller created and left', async () => {
// TASK-161. `leavePod` filters `members` and never clears `createdBy`; the
// permissive predicate counted that as membership, so the confirm step —
// which stores the chat a connector will speak into — admitted a person the
// pod's own write path refuses. `slack_pod_access_denied` had no arm at all.
const pending = {
teamId: 'T1', slackUserId: 'U1', chatId: 'D1', botTokenRef: 'secret-ref',
expiresAt: new Date(Date.now() + 60_000),
};
Integration.findOne.mockResolvedValue({ ...integration, config: { pendingBind: pending } });
Pod.findById.mockResolvedValueOnce({ createdBy: String(ownerId), members: [] });

const response = await request(app).post('/api/installables/slack/confirm');

expect(response.status).toBe(403);
expect(response.body.code).toBe('slack_pod_access_denied');
expect(Integration.findOneAndUpdate).not.toHaveBeenCalled();
});

test('a reconnect clears the reason an earlier flip left on the row (wren 73838)', async () => {
const pending = {
teamId: 'T1', slackUserId: 'U1', chatId: 'D1', botTokenRef: 'secret-ref',
Expand Down
17 changes: 15 additions & 2 deletions backend/__tests__/unit/services/connectorRelayPolicy.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,21 @@ describe('connectorRelayPolicy — the routed-target conjunction', () => {
expect(isRoutedPodTarget(target({ pod: undefined }))).toBe(false);
});

it('admits a pod whose creator is not listed in members', () => {
expect(isRoutedPodTarget(target({ pod: pod({ createdBy: 'user-1' }) }))).toBe(true);
it('refuses a pod whose creator is not listed in members', () => {
// TASK-161. `leavePod` filters `members` and never clears `createdBy`, so
// this cell IS "a creator who has left" — and the pod's own write path
// (`createMessage`) 401s them. The permissive `isPodMember` counted them as
// a member so that a pod whose `members` forgot its creator still
// authorises them; a connector must reach the same verdict as the write.
expect(isRoutedPodTarget(target({ pod: pod({ createdBy: 'user-1' }) }))).toBe(false);
});

it('still admits a creator who is also listed — the clause is gone, not the creator', () => {
// The complement, so the arm above cannot pass by refusing every pod that
// names a creator at all.
expect(isRoutedPodTarget(target({
pod: pod({ createdBy: 'user-1', members: ['user-1'] }),
}))).toBe(true);
});

it('refuses a missing user id rather than stringifying it into a match', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,53 @@ describe('decision card closure fan-out', () => {
expect((await Integration.findById(missingLink._id)).config.cards[0].closedAt).toEqual(expect.any(Date));
});

test('sends no closing line to a creator who left, while a listed sibling still receives one', async () => {
// TASK-161, from Vera 74671: `canSendClosingLine` had no witness for the
// population the permissive predicate admitted. `createdBy` identifies the
// pod's creator; it is not membership, and this arm is the one that says so.
// The listed sibling is the control — without it a `not.toHaveBeenCalled`
// would be indistinguishable from an arm that never ran.
const thirdId = new mongoose.Types.ObjectId();
Pod.findById.mockImplementation(() => chain({ createdBy: memberId, members: [ownerId, thirdId] }));
const origin = await Integration.create({
podId, scope: 'user', type: 'telegram', createdBy: ownerId, isActive: true, status: 'connected',
config: {
liveRelay: true, linkedUserId: String(ownerId), chatType: 'private', chatId: 'origin',
gates: { [String(podId)]: { enabled: true, since: new Date() } },
cards: [{ podMessageId: cardId, tgMessageId: '11', sentAt: new Date() }],
},
});
const leftCreator = await Integration.create({
podId, scope: 'user', type: 'telegram', createdBy: memberId, isActive: true, status: 'connected',
config: {
liveRelay: true, linkedUserId: String(memberId), chatType: 'private', chatId: 'left-creator',
gates: { [String(podId)]: { enabled: true, since: new Date() } },
cards: [{ podMessageId: cardId, tgMessageId: '12', sentAt: new Date() }],
},
});
const listed = await Integration.create({
podId, scope: 'user', type: 'telegram', createdBy: thirdId, isActive: true, status: 'connected',
config: {
liveRelay: true, linkedUserId: String(thirdId), chatType: 'private', chatId: 'listed',
gates: { [String(podId)]: { enabled: true, since: new Date() } },
cards: [{ podMessageId: cardId, tgMessageId: '13', sentAt: new Date() }],
},
});

await fanoutDecisionClosure(
{ _id: new mongoose.Types.ObjectId(), podId, messageId: cardId, ruling: { value: 'Now', byUsername: 'Sam' } },
{ via: 'workspace', integrationId: origin._id },
);

expect(telegramSend.sendMessage).toHaveBeenCalledTimes(1);
expect(telegramSend.sendMessage).toHaveBeenCalledWith(
'telegram-token', 'listed', '✓ Ruled by Sam: Now', { replyToMessageId: '13', plainText: true },
);
// Closure is unconditional (it is a receipt, not a delivery), so the
// departure shows up in the send and not in `closedAt`.
expect((await Integration.findById(leftCreator._id)).config.cards[0].closedAt).toEqual(expect.any(Date));
});

test('returns after durable closure while a sibling provider send is still pending', async () => {
let releaseSend;
const pendingSend = new Promise((resolve) => { releaseSend = resolve; });
Expand Down
14 changes: 14 additions & 0 deletions backend/__tests__/unit/services/decisionCardReply.bridges.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,20 @@ describe.each(['telegram', 'slack'])('%s decision reply', (provider) => {
expect(confirmation()).not.toContain('secret');
});

test('refuses a card reply from a pod creator who left, recording nothing', async () => {
// TASK-161, from Vera 74671: wren's ruling named this as witness (v) and the
// arm did not exist. The arm above departs through `members`; the population
// the permissive predicate admitted is the CREATOR who left — present in
// `createdBy`, absent from `members`. `assertOpen` is the other half of the
// claim: the refusal has to leave the standing ruling and the ledger alone.
Pod.findById.mockImplementation(() => chain({ name: 'Launch', members: [], createdBy: ownerId }));
await receive();
expect(choose).not.toHaveBeenCalled();
expect(messages).toHaveLength(0);
await assertOpen();
expect(confirmation()).toContain("You're no longer in Launch");
});

test('409-ruled: loser text goes under ask, not winner; standing ruling and ledger stay unchanged', async () => {
await receive();
const original = await ledger();
Expand Down
38 changes: 26 additions & 12 deletions backend/__tests__/unit/services/installableEventHandlers.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -455,24 +455,36 @@ describe('installable event dispatcher', () => {
// reddens as well.
describe('the outbound selector agrees with connectorRelayPolicy', () => {
// The matrix the row names: member / non-member × gate on / off / absent.
//
// TASK-161 adds the CREATOR cell — a subject who is the pod's `createdBy` and
// is not listed in `members`, i.e. a creator who has left. It is the cell
// where the two sides used to agree on "yes": the selector unioned
// `pod.createdBy` into memberIds exactly as the permissive `isPodMember`
// did, so green meant the wrong answer rather than an untested one. Both
// sides now exclude it, and this asserts the exclusion rather than the
// agreement alone.
const CELLS = [
{ label: 'member, gate on', member: true, gate: true },
{ label: 'member, gate off', member: true, gate: false },
{ label: 'member, gate absent', member: true, gate: null },
{ label: 'non-member, gate on', member: false, gate: true },
{ label: 'non-member, gate off', member: false, gate: false },
{ label: 'non-member, gate absent', member: false, gate: null },
{ label: 'member, gate on', membership: 'listed', gate: true },
{ label: 'member, gate off', membership: 'listed', gate: false },
{ label: 'member, gate absent', membership: 'listed', gate: null },
{ label: 'non-member, gate on', membership: 'absent', gate: true },
{ label: 'non-member, gate off', membership: 'absent', gate: false },
{ label: 'non-member, gate absent', membership: 'absent', gate: null },
{ label: 'creator not listed, gate on', membership: 'creator', gate: true },
{ label: 'creator not listed, gate off', membership: 'creator', gate: false },
{ label: 'creator not listed, gate absent', membership: 'creator', gate: null },
];

it.each(CELLS)('$label: the selector and the predicate reach the same verdict', async ({
member, gate,
membership, gate,
}) => {
const podId = freshId();
const creatorId = freshId();
const ownerId = freshId();
// Installed while the owner IS a member, then removed for the non-member
// cells — the same shape the membership arm above uses, and the only order
// that leaves the row itself unchanged between cells.
// and creator cells — the only order that leaves the row itself unchanged
// between cells. `creatorId === ownerId` is the whole difference between
// the creator cell and the plain non-member one.
const creatorId = membership === 'creator' ? ownerId : freshId();
await createPod(podId, creatorId, [ownerId]);
const installed = await install({ installableId: 'telegram', installedBy: ownerId, podId });
// `install` seeds the installed pod's gate ON, so the absent cell has to
Expand All @@ -489,7 +501,9 @@ describe('installable event dispatcher', () => {
{ $set: { [`config.gates.${podId}.enabled`]: gate } },
);
}
if (!member) await Pod.updateOne({ _id: podId }, { $pull: { members: ownerId } });
if (membership !== 'listed') {
await Pod.updateOne({ _id: podId }, { $pull: { members: ownerId } });
}

const row = await Integration.findById(installed.integration._id).lean();
const pod = await Pod.findById(podId).lean();
Expand All @@ -507,7 +521,7 @@ describe('installable event dispatcher', () => {

// Both halves, then their conjunction, then the verdict all three must
// reach — so agreement-by-both-saying-no cannot pass as agreement.
const expected = member && gate === true;
const expected = membership === 'listed' && gate === true;
expect(isGatedPodTarget(row, podId)).toBe(gate === true);
expect(selectorSays).toBe(expected);
expect(predicateSays).toBe(expected);
Expand Down
Loading
Loading