Skip to content

feat: config service as param#628

Open
zach88 wants to merge 1 commit intobh2smith:mainfrom
zach88:feat/config-service-param
Open

feat: config service as param#628
zach88 wants to merge 1 commit intobh2smith:mainfrom
zach88:feat/config-service-param

Conversation

@zach88
Copy link

@zach88 zach88 commented Oct 31, 2024

Resolves #627

What this PR changes:

  • Looks for the configUrl parameter in the query string to use it as a custom ConfigService URL.
  • Uses the hardcoded ConfigService URL as a fallback if the parameter is not present.

- Looks for the configUrl parameter in the query string to use it as a custom ConfigService URL.
- Uses the hardcoded ConfigService URL as a fallback if the parameter is not present.
Copy link
Owner

@bh2smith bh2smith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me. We should wait on @schmanu before merge. Thanks for your contribution here @zach88! 👻🎃🦇

@schmanu
Copy link
Collaborator

schmanu commented Nov 1, 2024

Hey @zach88,

I am not sure if this could lead to potential attack vectors by providing a malicious config service.
Maybe for now we could extract the config service URL into an .env variable and you could re-deploy the Safe app on IPFS with a different env variable set?

In the meantime I will create an issue to return the transactionService URL in the safe-apps-sdk's getChainInfo function. That would be the securest and easiest way to retrieve the chain config of the Safe's network.

@zach88
Copy link
Author

zach88 commented Nov 1, 2024

Actually, the url needs to be configured at the config service level, so any safe app that doesn't match the URL will be flagged as not trusted, just like it does for custom safe-apps. however, exposing the config service through the safe-app-sdk would be a nice workaround. thanks @schmanu

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.

Config service url as param

3 participants