Skip to content

Fix bugs in handling server names that includes colons#999

Closed
Twi1ightSparkle wants to merge 5 commits intomatrix-org:mainfrom
Twi1ightSparkle:hooksWithColonInServerName
Closed

Fix bugs in handling server names that includes colons#999
Twi1ightSparkle wants to merge 5 commits intomatrix-org:mainfrom
Twi1ightSparkle:hooksWithColonInServerName

Conversation

@Twi1ightSparkle
Copy link
Copy Markdown
Contributor

@Twi1ightSparkle Twi1ightSparkle commented Dec 11, 2024

I ran into this issue when testing something with generic hooks on Hookshot locally where the Synapse server name is localhost:8448

INFO 12:16:27:794 [GenericHookConnection] onGenericHook !QrmxBsBKPajykCohPz:localhost:8448 7e789bf2-7b87-4b10-9438-a22a6f1090d2
ERROR 12:16:27:800 [Appservice] Error registering user: User ID is in use
ERROR 12:16:27:804 [MatrixHttpClient] (REQ-15) {
  errcode: 'M_FORBIDDEN',
  error: 'Application service cannot masquerade as this user (@_webhooks_hookname:localhost).'
}
WARN 12:16:27:804 [Bridge] Failed to handle generic webhook MatrixError: M_FORBIDDEN: Application service cannot masquerade as this user (@_webhooks_hookname:localhost).
    at Object.defaultErrorHandler [as errorHandler] (/usr/bin/matrix-hookshot/node_modules/matrix-bot-sdk/lib/http.js:10:9)
    at doHttpRequest (/usr/bin/matrix-hookshot/node_modules/matrix-bot-sdk/lib/http.js:98:31)
    at process.processTicksAndRejections (node:internal/process/task_queues:105:5)
    at async descriptor.value (/usr/bin/matrix-hookshot/node_modules/matrix-bot-sdk/lib/metrics/decorators.js:19:32)
    at async descriptor.value (/usr/bin/matrix-hookshot/node_modules/matrix-bot-sdk/lib/metrics/decorators.js:19:32)
    at async GenericHookConnection.ensureDisplayname (/usr/bin/matrix-hookshot/Connections/GenericHook.js:271:13)
    at async GenericHookConnection.onGenericHook (/usr/bin/matrix-hookshot/Connections/GenericHook.js:478:13)
    at async /usr/bin/matrix-hookshot/Bridge.js:435:25
    at async Promise.all (index 0)
    at async LocalMQ.<anonymous> (/usr/bin/matrix-hookshot/Bridge.js:410:13) {
  body: {
    errcode: 'M_FORBIDDEN',
    error: 'Application service cannot masquerade as this user (@_webhooks_hookname:localhost).'
  },
  statusCode: 403,
  errcode: 'M_FORBIDDEN',
  error: 'Application service cannot masquerade as this user (@_webhooks_hookname:localhost).',
  retryAfterMs: undefined
}

Signed-off-by: Twilight Sparkle 19155609+Twi1ightSparkle@users.noreply.github.com

@Twi1ightSparkle Twi1ightSparkle requested a review from a team as a code owner December 11, 2024 12:54
@Twi1ightSparkle
Copy link
Copy Markdown
Contributor Author

This also solves for if the server name is an IPv6 address

> user = '@foo:1:2:3::4:8448'
'@foo:1:2:3::4:8448'
> user.split(':')
[
  '@foo', '1',
  '2',    '3',
  '',     '4',
  '8448'
]
> user.substring(user.indexOf(':')+1)
'1:2:3::4:8448'

@Twi1ightSparkle Twi1ightSparkle changed the title Fix bug where generic hooks doesn't work when the server name has the port in it Fix bug where generic hooks doesn't work when the server name includes a colon Dec 11, 2024
@Twi1ightSparkle
Copy link
Copy Markdown
Contributor Author

Searched through the code base for any more instances of .split(':'). Found two in the permissions code, fixed above. AfaIct there are no more instances of this bug

@Twi1ightSparkle Twi1ightSparkle changed the title Fix bug where generic hooks doesn't work when the server name includes a colon Fix bugs in handling server names that includes colons Dec 12, 2024
@Twi1ightSparkle
Copy link
Copy Markdown
Contributor Author

Any updates on getting this merged?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant