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
5 changes: 5 additions & 0 deletions .changeset/fix-pmp-proxy.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
default: patch
---

Fixed per-message profile proxies not unwrapping and generally just not working.
45 changes: 37 additions & 8 deletions src/app/features/room/RoomInput.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ import {
replaceWithElement,
BlockType,
} from '$components/editor';
import { plainToEditorInput } from '$components/editor/input';
import { EmojiBoard, EmojiBoardTab } from '$components/emoji-board';
import { UseStateProvider } from '$components/UseStateProvider';
import type { TUploadContent } from '$utils/matrix';
Expand Down Expand Up @@ -777,7 +778,7 @@ export const RoomInput = forwardRef<HTMLDivElement, RoomInputProps>(

// check if its a pk command
if (pkCompatEnable && PKitCommandMessageHandler.isPKCommand(plainText)) {
pluralkitCmdMessageHandler.handleMessage(plainText);
await pluralkitCmdMessageHandler.handleMessage(plainText);
resetEditor(editor); // clear the editor
return; // don't do anything besides handling the command
}
Expand Down Expand Up @@ -813,6 +814,37 @@ export const RoomInput = forwardRef<HTMLDivElement, RoomInputProps>(

if (plainText === '') return;

// PluralKit-style proxy wrappers (per-message profile proxies) must be stripped
// *before* building `content`, otherwise we end up sending the wrapper verbatim.
let proxiedPerMessageProfile:
| Awaited<ReturnType<(typeof pluralkitProxyMessageHandler)['getPmpBasedOnMessage']>>
| undefined;
if (pmpProxyingEnable) {
proxiedPerMessageProfile =
await pluralkitProxyMessageHandler.getPmpBasedOnMessage(plainText);
if (proxiedPerMessageProfile) {
const stripped = pluralkitProxyMessageHandler.stripProxyFromMessage(plainText);
if (stripped !== undefined) {
// Re-run the normal outgoing pipeline on the stripped content so the message
// goes through the same transforms/parsers as any other message.
serializedChildren = plainToEditorInput(stripped);

outgoingMessageTransforms.forEach((transform) => {
if (!transform.shouldApply(serializedChildren, outgoingTransformContext)) return;
serializedChildren = transform.apply(serializedChildren, outgoingTransformContext);
});

plainText = toPlainText(serializedChildren, true, nicknameReplacement).trim();
customHtml = trimCustomHtml(
toMatrixCustomHTML(serializedChildren, {
stripNickname: true,
nickNameReplacement: nicknameReplacement,
})
);
}
}
}

const body = plainText;
const formattedBody = customHtml;
const mentionData = getMentions(mx, roomId, editor);
Expand Down Expand Up @@ -848,13 +880,10 @@ export const RoomInput = forwardRef<HTMLDivElement, RoomInputProps>(
* This allows the server to apply the correct profile-based transformations (e.g. font size adjustments) when processing the message,
* and also allows clients to display an accurate preview of how the message will look with the profile applied while it's being composed.
*/
const perMessageProfile =
pmpProxyingEnable && pluralkitProxyMessageHandler.isAProxiedMessage(plainText)
? await pluralkitProxyMessageHandler.getPmpBasedOnMessage(plainText)
: await getCurrentlyUsedPerMessageProfileForRoom(mx, roomId);

if (pmpProxyingEnable && pluralkitProxyMessageHandler.isAProxiedMessage(plainText))
plainText = pluralkitProxyMessageHandler.stripProxyFromMessage(plainText) ?? plainText;
let perMessageProfile = await getCurrentlyUsedPerMessageProfileForRoom(mx, roomId);
if (pmpProxyingEnable) {
if (proxiedPerMessageProfile) perMessageProfile = proxiedPerMessageProfile;
}
if (perMessageProfile) {
content[prefix.MATRIX_UNSTABLE_PER_MESSAGE_PROFILE_PROPERTY_NAME] =
convertPerMessageProfileToBeeperFormat(
Expand Down
85 changes: 85 additions & 0 deletions src/app/features/room/pmpProxyOutgoingPipeline.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
import { describe, expect, it } from 'vitest';

import {
plainToEditorInput,
toMatrixCustomHTML,
toPlainText,
trimCustomHtml,
} from '$components/editor';
import { outgoingMessageTransforms } from './outgoingMessageTransforms';
import { buildSettingsLink, getSettingsLinkLabel } from '$features/settings/settingsLink';

function runOutgoingPipeline(input: string, settingsLinkBaseUrl = 'https://app.example') {
let children = plainToEditorInput(input);
const context = { settingsLinkBaseUrl };

outgoingMessageTransforms.forEach((t) => {
if (!t.shouldApply(children, context)) return;
children = t.apply(children, context);
});

const plain = toPlainText(children, true).trim();
const html = trimCustomHtml(
toMatrixCustomHTML(children, {
stripNickname: true,
nickNameReplacement: new Map(),
})
);

return { children, plain, html };
}

describe('PMP proxy outgoing pipeline parity', () => {
it('renders markdown like normal messages (bold)', () => {
const { plain, html } = runOutgoingPipeline('**bold**');
expect(plain).toBe('**bold**');
// Our markdown pipeline injects `data-md` markers for round-tripping.
expect(html).toMatch(/<strong[^>]*>bold<\/strong>/);
});

it('preserves markdown links in plaintext and renders anchors in html', () => {
const { plain, html } = runOutgoingPipeline('[Sable](https://example.com)');
expect(plain).toBe('[Sable](https://example.com)');
expect(html).toContain('<a href="https://example.com"');
expect(html).toContain('>Sable</a>');
});

it('escapes raw html so it is not treated as markup', () => {
const { plain, html } = runOutgoingPipeline('<b>nope</b>');
expect(plain).toBe('<b>nope</b>');
// markdownToHtml sanitizes/strips raw tags; ensure it does not render as actual <b>.
expect(html).toContain('nope');
expect(html).not.toContain('<b>nope</b>');
});

it('applies outgoing transforms (settings link rewrite) like normal messages', () => {
const base = 'https://app.example';
const url = buildSettingsLink(base, 'appearance', 'message-link-preview');
const label = getSettingsLinkLabel('appearance', 'message-link-preview');

const { plain, html } = runOutgoingPipeline(`see ${url}`, base);

expect(plain).toContain(`[${label}](${url})`);
// HTML encodes & as &amp; in attributes
const encodedUrl = url.replaceAll('&', '&amp;');
expect(html).toContain(`<a href="${encodedUrl}"`);
const encodedLabel = label.replaceAll('>', '&gt;');
expect(html).toContain(`>${encodedLabel}</a>`);
});

it('supports multi-paragraph messages (keeps line breaks)', () => {
const { plain, html } = runOutgoingPipeline('first line\n\nsecond line');
// Plaintext keeps the blank line separation
expect(plain).toBe('first line\n\nsecond line');
// HTML should contain two paragraphs
expect(html).toMatch(/<p>first line<\/p>[\s\S]*<p>second line<\/p>/);
});

it('supports fenced code blocks and renders them as code', () => {
const md = ['```ts', 'const x = 1', '```'].join('\n');
const { plain, html } = runOutgoingPipeline(md);
expect(plain).toBe(md);
// Allow flexibility in the exact HTML produced by the markdown pipeline
expect(html).toMatch(/<pre[\s\S]*><code[\s\S]*>[\s\S]*const x = 1[\s\S]*<\/code><\/pre>/);
});
});
2 changes: 1 addition & 1 deletion src/app/hooks/useCommands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -632,7 +632,7 @@ export const useCommands = (mx: MatrixClient, room: Room): CommandRecord => {
exe: async (payload) => {
const pid: string = splitWithSpace(payload)[0] ?? '';
const proxy: string = splitWithSpace(payload)[1] ?? '';
pkitcmdHandler.handleMessage(`pk;member "${pid}" proxy ${proxy}`, true);
await pkitcmdHandler.handleMessage(`pk;member "${pid}" proxy ${proxy}`, true);
},
},
[Command.MyRoomAvatar]: {
Expand Down
30 changes: 30 additions & 0 deletions src/app/hooks/usePerMessageProfile.proxy.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import { describe, expect, it } from 'vitest';

import { parsePerMessageProfileProxyAssociation } from './usePerMessageProfile';

describe('parsePerMessageProfileProxyAssociation', () => {
it('parses a regex string with flags (RegExp#toString form)', () => {
const assoc = {
profileId: 'p1',
regexString: '/^\\[text\\] (.+)$/i',
setAt: 123,
};

const parsed = parsePerMessageProfileProxyAssociation(assoc);
expect(parsed.profileId).toBe('p1');
expect(parsed.setAt).toBe(123);
expect(parsed.regex.test('[text] Hello')).toBe(true);
expect(parsed.regex.test('[TEXT] hello')).toBe(true); // i flag
});

it('parses a regex string without flags', () => {
const assoc = {
profileId: 'p1',
regexString: '/^\\[(.+)\\]$/',
};

const parsed = parsePerMessageProfileProxyAssociation(assoc);
expect(parsed.regex.test('[ok]')).toBe(true);
expect(parsed.regex.test('[no] trailing')).toBe(false);
});
});
8 changes: 5 additions & 3 deletions src/app/hooks/usePerMessageProfile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -158,10 +158,12 @@ export type InternalPerMessageProfileProxyAssociation = {
export function parsePerMessageProfileProxyAssociation(
assoc: PerMessageProfileProxyAssociation
): InternalPerMessageProfileProxyAssociation {
const m = assoc.regexString.match(/^\/([\s\S]*)\/([gimsuy]*)$/);
const source = m?.[1] ?? assoc.regexString;
const flags = m?.[2] ?? '';
return {
profileId: assoc.profileId,
// we need to remove artifacts from the toString conversion
regex: new RegExp(assoc.regexString.slice(1, -1)),
regex: new RegExp(source, flags),
setAt: assoc.setAt,
} satisfies InternalPerMessageProfileProxyAssociation;
}
Expand Down Expand Up @@ -431,7 +433,7 @@ export async function getProfileAssociatedWithProxy(
mx: MatrixClient,
proxy: string
): Promise<PerMessageProfile | undefined> {
const profileId = getAssociationsMap(
const profileId = getProxyAssociationMap(
mx
.getAccountData(
`${ACCOUNT_DATA_PREFIX}.proxyassociation` as Parameters<typeof mx.getAccountData>[0]
Expand Down
11 changes: 7 additions & 4 deletions src/app/plugins/pluralkit-handler/PKitCommandMessageHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,10 @@ export class PKitCommandMessageHandler {
this.room,
this.mx.getSafeUserId()
);
addOrUpdatePerMessageProfile(this.mx, { id: generatedID, name: memberName });
await addOrUpdatePerMessageProfile(this.mx, {
id: generatedID,
name: memberName,
});
sendFeedback(
`added new member has been created with id: ${generatedID} and name ${memberName}`,
this.room,
Expand Down Expand Up @@ -169,7 +172,7 @@ export class PKitCommandMessageHandler {
this.room,
this.mx.getSafeUserId()
);
addOrUpdatePerMessageProfile(this.mx, pmp);
await addOrUpdatePerMessageProfile(this.mx, pmp);
} else if (pkMemberRemoveProxy.test(this.message)) {
const cmdParts = pkMemberRemoveProxy.exec(this.message);
if (!cmdParts) return;
Expand All @@ -195,7 +198,7 @@ export class PKitCommandMessageHandler {
);
return;
}
dropProxyAssociationForPMP(this.mx, matchAgainst);
await dropProxyAssociationForPMP(this.mx, matchAgainst);

sendFeedback(
`Persona with ${this.useIdInsteadOfNameWherePossible ? 'id' : 'name'} "${name}" (${pmpId}) is now no longer associated with ${matchAgainst}`,
Expand Down Expand Up @@ -227,7 +230,7 @@ export class PKitCommandMessageHandler {
return;
}
const matchAgainstRegExp = buildRegex(matchAgainst);
associateProxyWithProfile(this.mx, pmpId, matchAgainst, matchAgainstRegExp, false);
await associateProxyWithProfile(this.mx, pmpId, matchAgainst, matchAgainstRegExp, false);
sendFeedback(
`Persona with ${this.useIdInsteadOfNameWherePossible ? 'id' : 'name'} "${name}" (${pmpId}) is now associated with ${matchAgainst}`,
this.room,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import type { Mock } from 'vitest';
import { describe, expect, it, vi, beforeEach } from 'vitest';

import { PKitProxyMessageHandler } from './PKitProxyMessageHandler';
import type { MatrixClient } from '$types/matrix-sdk';

// Mock the hook module that provides proxy associations + profile lookup
vi.mock('$hooks/usePerMessageProfile', () => ({
getAllPerMessageProfileProxies: vi.fn<() => Promise<unknown[]>>(),
getPerMessageProfileById: vi.fn<() => Promise<unknown>>(),
parsePerMessageProfileProxyAssociation: vi.fn<() => unknown>(),
}));

const mocked = await import('$hooks/usePerMessageProfile');

describe('PKitProxyMessageHandler', () => {
beforeEach(() => {
vi.resetAllMocks();
});

it('returns false for isAProxiedMessage before init', () => {
const handler = new PKitProxyMessageHandler({} as unknown as MatrixClient);
expect(handler.isAProxiedMessage('[test] hi')).toBe(false);
});

it('matches a proxied message, returns pmp, and strips content', async () => {
const proxyRegex = /^\[(.+)\]$/;

(mocked.getAllPerMessageProfileProxies as unknown as Mock).mockResolvedValueOnce([
{ profileId: 'p1', regexString: proxyRegex.toString() },
]);
(mocked.parsePerMessageProfileProxyAssociation as unknown as Mock).mockReturnValueOnce({
profileId: 'p1',
regex: proxyRegex,
});
(mocked.getPerMessageProfileById as unknown as Mock).mockResolvedValueOnce({
id: 'p1',
name: 'Test',
});

const handler = new PKitProxyMessageHandler({} as unknown as MatrixClient);

const pmp = await handler.getPmpBasedOnMessage('[hello]');
expect(pmp).toEqual({ id: 'p1', name: 'Test' });

// getPmpBasedOnMessage refreshes/init() so we should be inited now
expect(handler.isAProxiedMessage('[hello]')).toBe(true);
expect(handler.stripProxyFromMessage('[hello]')).toBe('hello');
});
});
11 changes: 7 additions & 4 deletions src/app/plugins/pluralkit-handler/PKitProxyMessageHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,9 @@ export class PKitProxyMessageHandler {
this.succInit = true;
} catch (err) {
this.succInit = false;
throw new Error(`failed to init pmp proxy handler: ${String(err)}`, { cause: err });
throw new Error(`failed to init pmp proxy handler: ${String(err)}`, {
cause: err,
});
}
}

Expand All @@ -63,7 +65,7 @@ export class PKitProxyMessageHandler {
* @param message the message to check
*/
public isAProxiedMessage(message: string): boolean {
if (!this.succInit) throw new Error('PK proxy message handler is not initialized');
if (!this.succInit) return false;
return this.proxiesAssocs.some((assoc) => assoc.regex.test(message));
}

Expand All @@ -73,7 +75,8 @@ export class PKitProxyMessageHandler {
* @returns the matching Per-Message-Profile, if any
*/
public async getPmpBasedOnMessage(message: string): Promise<PerMessageProfile | undefined> {
if (!this.succInit) await this.init();
// Always refresh so newly-added proxies apply immediately.
await this.init();
// check if the message matches our formats
// maybe a bit unsafe, as we are evaluating regex that aren't necessarily by us, could be _maybe_ manipulated
const profileId = this.proxiesAssocs.find((assoc) => assoc.regex.test(message))?.profileId;
Expand All @@ -89,7 +92,7 @@ export class PKitProxyMessageHandler {
* @memberof PKitProxyMessageHandler
*/
public stripProxyFromMessage(message: string): string | undefined {
if (!this.succInit) throw new Error('PK proxy message handler is not initialized');
if (!this.succInit) return undefined;
let m;
this.proxiesAssocs.forEach((assoc) => {
const match = assoc.regex.exec(message);
Expand Down
Loading