Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: config service as param #628

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

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