FoundKeyGang/FoundKey
FoundKeyGang/FoundKey
7
19
Fork 22
Code Issues 37 Pull requests 4 Releases Activity

server: block Create activites from blocked hosts #298

Closed
norm wants to merge 1 commit from block-creates into main
pull from: block-creates
merge into: FoundKeyGang:main
Conversation 1 Commits 1 Files changed 2 +11 -2
2 changed files with 11 additions and 2 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
packages/backend/src/remote/activitypub/kernel/create/index.ts
View file

@ -1,12 +1,15 @@
import { CacheableRemoteUser } from '@/models/entities/user.js';
import { toArray, concat, unique } from '@/prelude/array.js';
import { Resolver } from '@/remote/activitypub/resolver.js';
import { shouldBlockInstance } from '@/misc/should-block-instance.js';
import { extractDbHost } from '@/misc/convert-host.js';
import { ICreate, getApId, isPost, getApType } from '../../type.js';
import { apLogger } from '../../logger.js';
import createNote from './note.js';
export default async (actor: CacheableRemoteUser, activity: ICreate, resolver: Resolver): Promise<void> => {
const uri = getApId(activity);
if (await shouldBlockInstance(extractDbHost(uri))) return;
Johann150 commented 2022-12-24 09:59:48 +00:00
Review
Copy link

Shouldn't this already be covered by resolving the activity object further down? The object being created must be from the same host, otherwise it would have been caught by this check

Lines 21 to 23 in 8541932
if (extractDbHost(actor.uri) !== extractDbHost(note.id)) {
return 'skip: host in actor.uri !== note.id';
}

. I guess it was already embedded as an object so the usual checks aren't performed due to this short circuit

Lines 53 to 55 in 8541932
if (typeof value !== 'string') {
return value;
}

. Maybe a better idea to add the matching id/url and instance blocks in that case too.

Maybe also check this at a higher level, i.e. in performOneActivity.

Shouldn't this already be covered by resolving the activity object further down? The object being created must be from the same host, otherwise it would have been caught by this check https://akkoma.dev/FoundKeyGang/FoundKey/src/commit/85419326f868291036681cc9ddfbbc545d37f77d/packages/backend/src/remote/activitypub/kernel/create/note.ts#L21-L23. I guess it was already embedded as an object so the usual checks aren't performed due to this short circuit https://akkoma.dev/FoundKeyGang/FoundKey/src/commit/85419326f868291036681cc9ddfbbc545d37f77d/packages/backend/src/remote/activitypub/resolver.ts#L53-L55. Maybe a better idea to add the matching id/url and instance blocks in that case too. Maybe also check this at a higher level, i.e. in `performOneActivity`.
apLogger.info(`Create: ${uri}`);

10
packages/backend/src/remote/activitypub/kernel/create/note.ts
View file

@ -4,13 +4,19 @@ import { extractDbHost } from '@/misc/convert-host.js';
import { StatusError } from '@/misc/fetch.js';
import { Resolver } from '@/remote/activitypub/resolver.js';
import { createNote, fetchNote } from '@/remote/activitypub/models/note.js';
import { getApId, IObject, ICreate } from '@/remote/activitypub/type.js';
import { getApId, IObject } from '@/remote/activitypub/type.js';
import { shouldBlockInstance } from '@/misc/should-block-instance.js';
/**
* 投稿作成アクティビティを捌きます
* Handles post creation activity
*/
export default async function(resolver: Resolver, actor: CacheableRemoteUser, note: IObject, silent = false): Promise<string> {
const uri = getApId(note);
const host = extractDbHost(uri);
if (await shouldBlockInstance(host)) {
return 'skip: blocked host';
}
if (typeof note === 'object') {
if (actor.uri !== note.attributedTo) {