From 9d20fba35912d0ab8d7b9f9b33fe815c11a94a2b Mon Sep 17 00:00:00 2001 From: dakkar Date: Thu, 2 May 2024 17:05:44 +0100 Subject: [PATCH 1/6] lint our MRs we still can't run the automated tests, but at least we can typecheck and lint! --- .gitlab-ci.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index be00aa7097..0dd0a64d61 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -19,6 +19,8 @@ testCommit: - pnpm install --frozen-lockfile - pnpm run build - pnpm run migrate + - pnpm run --filter=backend lint + - pnpm run --filter=frontend eslint cache: key: test policy: pull-push From aa7271469e26959775ae492975af2a2d5ddc59af Mon Sep 17 00:00:00 2001 From: dakkar Date: Sat, 18 May 2024 15:56:21 +0100 Subject: [PATCH 2/6] fix types in `config.ts` my recent changes to allow overrides from the environment had bad types --- packages/backend/src/config.ts | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/packages/backend/src/config.ts b/packages/backend/src/config.ts index f6ce9b3cdf..111e8aaa9b 100644 --- a/packages/backend/src/config.ts +++ b/packages/backend/src/config.ts @@ -327,11 +327,11 @@ function applyEnvOverrides(config: Source) { // these inner functions recurse through the config structure, using // the given steps, building the env variable name - function _apply_top(steps: (string | number)[]) { + function _apply_top(steps: (string | string[] | number | number[])[]) { _walk('', [], steps); } - function _walk(name: string, path: (string | number)[], steps: (string | number)[]) { + function _walk(name: string, path: (string | number)[], steps: (string | string[] | number | number[])[]) { // are there more steps after this one? recurse if (steps.length > 1) { const thisStep = steps.shift(); @@ -368,7 +368,7 @@ function applyEnvOverrides(config: Source) { } // this recurses down, bailing out if there's no config to override - function _descend(name: string, path: (string | number)[], thisStep: string | number, steps: (string | number)[]) { + function _descend(name: string, path: (string | number)[], thisStep: string | number, steps: (string | string[] | number | number[])[]) { name = `${name}${_step2name(thisStep)}_`; path = [ ...path, thisStep ]; _walk(name, path, steps); @@ -390,10 +390,10 @@ function applyEnvOverrides(config: Source) { } } - const alwaysStrings = { 'chmodSocket': 1 }; + const alwaysStrings = { 'chmodSocket': 1 } as any; function _assign(path: (string | number)[], lastStep: string | number, value: string) { - let thisConfig = config; + let thisConfig = config as any; for (const step of path) { if (!thisConfig[step]) { thisConfig[step] = {}; @@ -403,9 +403,11 @@ function applyEnvOverrides(config: Source) { if (!alwaysStrings[lastStep]) { if (value.match(/^[0-9]+$/)) { - value = parseInt(value); + thisConfig[lastStep] = parseInt(value); + return; } else if (value.match(/^(true|false)$/i)) { - value = !!value.match(/^true$/i); + thisConfig[lastStep] = !!value.match(/^true$/i); + return; } } @@ -416,7 +418,7 @@ function applyEnvOverrides(config: Source) { _apply_top([['url', 'port', 'socket', 'chmodSocket', 'disableHsts']]); _apply_top(['db', ['host', 'port', 'db', 'user', 'pass']]); - _apply_top(['dbSlaves', config.dbSlaves?.keys(), ['host', 'port', 'db', 'user', 'pass']]); + _apply_top(['dbSlaves', Array.from((config.dbSlaves ?? []).keys()), ['host', 'port', 'db', 'user', 'pass']]); _apply_top([ ['redis', 'redisForPubsub', 'redisForJobQueue', 'redisForTimelines'], ['host','port','username','pass','db','prefix'], From c05cc63e24c654e5e5d2d098e00a2aa669b61adf Mon Sep 17 00:00:00 2001 From: dakkar Date: Sat, 18 May 2024 16:36:06 +0100 Subject: [PATCH 3/6] look inside `url` when checking activity origin - #512 The previous assertion that: > if it's a complicated thing and the `activity.id` doesn't match, I > think we're fine rejecting the activity was wrong: at least peertube sends activities that have `url` as an array of objects. Notice that this does *not*, in fact, fix #512: the peertube activity does not contain its short URL (`https://example.com/w/someid`), so there's no way to confirm that it is the activity we requested. --- .../activitypub/misc/check-against-url.ts | 22 +++++--- .../test/unit/misc/check-against-url.ts | 51 +++++++++++++++++++ 2 files changed, 66 insertions(+), 7 deletions(-) create mode 100644 packages/backend/test/unit/misc/check-against-url.ts diff --git a/packages/backend/src/core/activitypub/misc/check-against-url.ts b/packages/backend/src/core/activitypub/misc/check-against-url.ts index 78ba891a2e..34e4907267 100644 --- a/packages/backend/src/core/activitypub/misc/check-against-url.ts +++ b/packages/backend/src/core/activitypub/misc/check-against-url.ts @@ -4,16 +4,24 @@ */ import type { IObject } from '../type.js'; +function getHrefFrom(one: IObject|string): string | undefined { + if (typeof(one) === 'string') return one; + return one.href; +} + export function assertActivityMatchesUrls(activity: IObject, urls: string[]) { const idOk = activity.id !== undefined && urls.includes(activity.id); + if (idOk) return; - // technically `activity.url` could be an `ApObject = IObject | - // string | (IObject | string)[]`, but if it's a complicated thing - // and the `activity.id` doesn't match, I think we're fine - // rejecting the activity - const urlOk = typeof(activity.url) === 'string' && urls.includes(activity.url); + const url = activity.url; + if (url) { + // `activity.url` can be an `ApObject = IObject | string | (IObject + // | string)[]`, we have to look inside it + const activityUrls = Array.isArray(url) ? url.map(getHrefFrom) : [getHrefFrom(url)]; + const goodUrl = activityUrls.find(u => u && urls.includes(u)); - if (!idOk && !urlOk) { - throw new Error(`bad Activity: neither id(${activity?.id}) nor url(${activity?.url}) match location(${urls})`); + if (goodUrl) return; } + + throw new Error(`bad Activity: neither id(${activity?.id}) nor url(${JSON.stringify(activity?.url)}) match location(${urls})`); } diff --git a/packages/backend/test/unit/misc/check-against-url.ts b/packages/backend/test/unit/misc/check-against-url.ts new file mode 100644 index 0000000000..1cc12cbea2 --- /dev/null +++ b/packages/backend/test/unit/misc/check-against-url.ts @@ -0,0 +1,51 @@ +/* + * SPDX-FileCopyrightText: dakkar and sharkey-project + * SPDX-License-Identifier: AGPL-3.0-only + */ + +import type { IObject } from '@/core/activitypub/type.js'; +import { describe, expect, test } from '@jest/globals'; +import { assertActivityMatchesUrls } from '@/core/activitypub/misc/check-against-url.js'; + +function assertOne(activity: IObject) { + // return a function so we can use `.toThrow` + return () => assertActivityMatchesUrls(activity, ['good']); +} + +describe('assertActivityMatchesUrls', () => { + test('id', () => { + expect(assertOne({ id: 'bad' })).toThrow(/bad Activity/); + expect(assertOne({ id: 'good' })).not.toThrow(); + }); + + test('simple url', () => { + expect(assertOne({ url: 'bad' })).toThrow(/bad Activity/); + expect(assertOne({ url: 'good' })).not.toThrow(); + }); + + test('array of urls', () => { + expect(assertOne({ url: ['bad'] })).toThrow(/bad Activity/); + expect(assertOne({ url: ['bad', 'other'] })).toThrow(/bad Activity/); + expect(assertOne({ url: ['good'] })).not.toThrow(); + expect(assertOne({ url: ['bad', 'good'] })).not.toThrow(); + }); + + test('array of objects', () => { + expect(assertOne({ url: [{ href: 'bad' }] })).toThrow(/bad Activity/); + expect(assertOne({ url: [{ href: 'bad' }, { href: 'other' }] })).toThrow(/bad Activity/); + expect(assertOne({ url: [{ href: 'good' }] })).not.toThrow(); + expect(assertOne({ url: [{ href: 'bad' }, { href: 'good' }] })).not.toThrow(); + }); + + test('mixed array', () => { + expect(assertOne({ url: [{ href: 'bad' }, 'other'] })).toThrow(/bad Activity/); + expect(assertOne({ url: [{ href: 'bad' }, 'good'] })).not.toThrow(); + expect(assertOne({ url: ['bad', { href: 'good' }] })).not.toThrow(); + }); + + test('id and url', () => { + expect(assertOne({ id: 'other', url: 'bad' })).toThrow(/bad Activity/); + expect(assertOne({ id: 'bad', url: 'good' })).not.toThrow(); + expect(assertOne({ id: 'good', url: 'bad' })).not.toThrow(); + }); +}); From 453a023cb95008761cfe2e8471f12328800d0398 Mon Sep 17 00:00:00 2001 From: dakkar Date: Sat, 18 May 2024 17:19:17 +0100 Subject: [PATCH 4/6] correcty load/save `withBots` - fixes #528 there is no `tlWithBots` setting, it's inside `tl.filter` and we need more complicated code to set it --- packages/frontend/src/pages/settings/general.vue | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/packages/frontend/src/pages/settings/general.vue b/packages/frontend/src/pages/settings/general.vue index 1e4e815d5d..f404e3265f 100644 --- a/packages/frontend/src/pages/settings/general.vue +++ b/packages/frontend/src/pages/settings/general.vue @@ -293,6 +293,7 @@ import { definePageMetadata } from '@/scripts/page-metadata.js'; import { miLocalStorage } from '@/local-storage.js'; import { globalEvents } from '@/events.js'; import { claimAchievement } from '@/scripts/achievements.js'; +import { deepMerge } from '@/scripts/merge.js'; const lang = ref(miLocalStorage.getItem('lang')); const fontSize = ref(miLocalStorage.getItem('fontSize')); @@ -319,7 +320,14 @@ const reactionsDisplaySize = computed(defaultStore.makeGetterSetter('reactionsDi const limitWidthOfReaction = computed(defaultStore.makeGetterSetter('limitWidthOfReaction')); const collapseRenotes = computed(defaultStore.makeGetterSetter('collapseRenotes')); const clickToOpen = computed(defaultStore.makeGetterSetter('clickToOpen')); -const showBots = computed(defaultStore.makeGetterSetter('tlWithBots')); +// copied from src/pages/timeline.vue +const showBots = computed({ + get: () => defaultStore.reactiveState.tl.value.filter.withBots, + set: (newValue) => { + const out = deepMerge({ filter: { withBots: newValue } }, defaultStore.state.tl); + defaultStore.set('tl', out); + }, +}); const collapseFiles = computed(defaultStore.makeGetterSetter('collapseFiles')); const autoloadConversation = computed(defaultStore.makeGetterSetter('autoloadConversation')); const reduceAnimation = computed(defaultStore.makeGetterSetter('animation', v => !v, v => !v)); From 38d98e504882d9482f5adeec8df6156e42606c02 Mon Sep 17 00:00:00 2001 From: dakkar Date: Thu, 30 May 2024 11:29:31 +0100 Subject: [PATCH 5/6] nicer type for `alwaysStrings`, thanks Marie --- packages/backend/src/config.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/backend/src/config.ts b/packages/backend/src/config.ts index 111e8aaa9b..7eb5b86b18 100644 --- a/packages/backend/src/config.ts +++ b/packages/backend/src/config.ts @@ -390,7 +390,7 @@ function applyEnvOverrides(config: Source) { } } - const alwaysStrings = { 'chmodSocket': 1 } as any; + const alwaysStrings = { 'chmodSocket': true } as { [key: string]: boolean }; function _assign(path: (string | number)[], lastStep: string | number, value: string) { let thisConfig = config as any; From 9f885ca56780d5647ca137cede23143bd0342d03 Mon Sep 17 00:00:00 2001 From: Max Walters Date: Fri, 31 May 2024 00:38:02 +0000 Subject: [PATCH 6/6] fix typo in readme --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index c002acbff9..6840503243 100644 --- a/README.md +++ b/README.md @@ -34,7 +34,7 @@ - **ActivityPub support**\ Not on Sharkey? No problem! Not only can Sharkey instances talk to each other, but you can make friends with people on other networks like Mastodon and Pixelfed! - **Federated Backgrounds and Music status**\ -You can add a background to your profile as well as a music status via ListenBrainz, show everyone what music you are currently listening too +You can add a background to your profile as well as a music status via ListenBrainz, show everyone what music you are currently listening to - **Mastodon API**\ Sharkey implements the Mastodon API unlike normal Misskey - **UI/UX Improvements**\