Skip to content

Commit 0cd7aec

Browse files
sagzysagzy
committed
🐛 Fixed private mentions being treated as public
ref https://linear.app/ghost/issue/BER-2987 - we currently don't support private mentions or DMs, and they were wrongly been treated as public content - added an extra check when handling Create activity, to ignore non-public content
1 parent dd98177 commit 0cd7aec

File tree

5 files changed

+143
-51
lines changed

5 files changed

+143
-51
lines changed

‎features/handle-mention.feature‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,11 @@
11
Feature: Incoming mentions
22

3-
Scenario: We receive a Create(Note) with a mention
3+
Scenario: We receive a public mention from someone
44
Given an Actor "Person(Alice)"
5-
And a "Create(Note)" Activity "Note" by "Alice" with content "Hello @index@site.com" that mentions "Us"
6-
When "Alice" sends "Note" to the Inbox
7-
Then the request is accepted
5+
When "Alice" sends us a public mention
6+
And the mention is in our notifications
7+
8+
Scenario: We receive a private mention from someone
9+
Given an Actor "Person(Alice)"
10+
When "Alice" sends us a private mention
11+
And the mention is not in our notifications

‎features/step_definitions/activitypub_steps.js‎

Lines changed: 0 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -67,46 +67,6 @@ async function activityCreatedByWithContent(
6767
}
6868
}
6969

70-
async function activityCreatedByWithMention(
71-
activityDef,
72-
name,
73-
actorName,
74-
content,
75-
mentionedActorName,
76-
) {
77-
const { activity: activityType, object: objectName } =
78-
parseActivityString(activityDef);
79-
if (!activityType) {
80-
throw new Error(`could not match ${activityDef} to an activity`);
81-
}
82-
83-
const actor = this.actors[actorName];
84-
const mentionedActor = this.actors[mentionedActorName];
85-
const tags = [
86-
{
87-
type: 'Mention',
88-
name: `@${mentionedActor.username}@${mentionedActor.domain}`,
89-
href: mentionedActor.apId,
90-
},
91-
];
92-
const object =
93-
this.actors[objectName] ??
94-
this.activities[objectName] ??
95-
this.objects[objectName] ??
96-
(await createObject(objectName, actor, content, tags));
97-
98-
const activity = await createActivity(activityType, object, actor);
99-
100-
const parsed = parseActivityString(name);
101-
if (parsed.activity === null || parsed.object === null) {
102-
this.activities[name] = activity;
103-
this.objects[name] = object;
104-
} else {
105-
this.activities[parsed.activity] = activity;
106-
this.objects[parsed.object] = object;
107-
}
108-
}
109-
11070
Given('a {string} Activity {string} by {string}', activityCreatedBy);
11171

11272
Given(
@@ -119,11 +79,6 @@ Given(
11979
activityCreatedBy,
12080
);
12181

122-
Given(
123-
'a {string} Activity {string} by {string} with content {string} that mentions {string}',
124-
activityCreatedByWithMention,
125-
);
126-
12782
Given('an Actor {string}', async function (actorDef) {
12883
const { type, name } = parseActorString(actorDef);
12984

‎features/step_definitions/mention_steps.js‎

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
import { Then, When } from '@cucumber/cucumber';
2+
3+
import assert from 'node:assert';
4+
5+
import { createActivity, createObject } from '../support/fixtures.js';
6+
import { waitForItemInNotifications } from '../support/notifications.js';
7+
import { fetchActivityPub } from '../support/request.js';
8+
9+
When('{string} sends us a public mention', async function (actorName) {
10+
const actor = this.actors[actorName];
11+
if (!actor) {
12+
throw new Error(
13+
`Actor ${actorName} not found - did you forget a step?`,
14+
);
15+
}
16+
17+
// Use the existing local actor (Us) from the test context
18+
const localActor = this.actors.Us;
19+
if (!localActor) {
20+
throw new Error('Local actor (Us) not found in test context');
21+
}
22+
23+
const mention = `@${localActor.preferredUsername}@self.test`;
24+
const tags = [
25+
{
26+
type: 'Mention',
27+
name: mention,
28+
href: localActor.id,
29+
},
30+
];
31+
32+
// Create a public Note with the mention (uses defaults: to='as:Public')
33+
const object = await createObject('Note', actor, `Hello ${mention}`, tags);
34+
const activity = await createActivity('Create', object, actor);
35+
36+
await fetchActivityPub('https://self.test/.ghost/activitypub/inbox/index', {
37+
method: 'POST',
38+
headers: {
39+
'Content-Type': 'application/ld+json',
40+
},
41+
body: JSON.stringify(activity),
42+
});
43+
44+
this.mentionId = object.id;
45+
this.activities['Create(Note)'] = activity;
46+
this.objects.Note = object;
47+
});
48+
49+
When('{string} sends us a private mention', async function (actorName) {
50+
const actor = this.actors[actorName];
51+
if (!actor) {
52+
throw new Error(
53+
`Actor ${actorName} not found - did you forget a step?`,
54+
);
55+
}
56+
57+
// Use the existing local actor (Us) from the test context
58+
const localActor = this.actors.Us;
59+
if (!localActor) {
60+
throw new Error('Local actor (Us) not found in test context');
61+
}
62+
63+
const mention = `@${localActor.preferredUsername}@self.test`;
64+
const tags = [
65+
{
66+
type: 'Mention',
67+
name: mention,
68+
href: localActor.id,
69+
},
70+
];
71+
72+
// Create a Note with the mention
73+
const object = await createObject('Note', actor, `Hello ${mention}`, tags);
74+
75+
// Make it private by addressing it only to the mentioned user
76+
object.to = localActor.id;
77+
object.cc = [];
78+
79+
const activity = await createActivity('Create', object, actor);
80+
activity.to = localActor.id;
81+
activity.cc = [];
82+
83+
await fetchActivityPub('https://self.test/.ghost/activitypub/inbox/index', {
84+
method: 'POST',
85+
headers: {
86+
'Content-Type': 'application/ld+json',
87+
},
88+
body: JSON.stringify(activity),
89+
});
90+
91+
this.mentionId = object.id;
92+
this.activities['Create(Note)'] = activity;
93+
this.objects.Note = object;
94+
});
95+
96+
Then('the mention is in our notifications', async function () {
97+
if (!this.mentionId) {
98+
throw new Error(
99+
'You need to call a step which creates a mention before this',
100+
);
101+
}
102+
103+
const found = await waitForItemInNotifications(this.mentionId);
104+
assert(found, `Expected mention ${this.mentionId} to be in notifications`);
105+
});
106+
107+
Then('the mention is not in our notifications', async function () {
108+
if (!this.mentionId) {
109+
throw new Error(
110+
'You need to call a step which creates a mention before this',
111+
);
112+
}
113+
114+
try {
115+
await waitForItemInNotifications(this.mentionId);
116+
assert.fail(`Expected mention ${this.mentionId} to not be in notifications`);
117+
} catch (error) {
118+
assert.equal(
119+
error.message,
120+
`Max retries reached when waiting on item in notifications`,
121+
);
122+
}
123+
});

‎features/support/notifications.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ export async function waitForItemInNotifications(
3939

4040
if (options.retryCount === MAX_RETRIES) {
4141
throw new Error(
42-
`Max retries reached (${MAX_RETRIES}) when waiting on item in notifications`,
42+
`Max retries reached when waiting on item in notifications`,
4343
);
4444
}
4545

‎src/activity-handlers/create.handler.ts‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import type { Context, Create } from '@fedify/fedify';
1+
import { type Context, type Create, PUBLIC_COLLECTION } from '@fedify/fedify';
22

33
import type { ContextData } from '@/app';
44
import { exhaustiveCheck, getError, isError } from '@/core/result';
@@ -21,6 +21,16 @@ export class CreateHandler {
2121
return;
2222
}
2323

24+
const recipients = [...create.toIds, ...create.ccIds].map(
25+
(id) => id.href,
26+
);
27+
const isPublic = recipients.includes(PUBLIC_COLLECTION.href);
28+
29+
if (!isPublic) {
30+
ctx.data.logger.info('Create activity is not public - exit');
31+
return;
32+
}
33+
2434
// This handles storing the posts in the posts table
2535
const postResult = await this.postService.getByApId(create.objectId);
2636

0 commit comments

Comments
 (0)