Small cleanups #1285
Open
Small cleanups #1285
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Deployed to Cloudflare Pages
|
This was referenced Feb 21, 2024
csillag
force-pushed
the
csillag/small-cleanups
branch
from
6e46b66
to
2a22b11
Compare
lukaw3d
reviewed
Feb 21, 2024
| @@ -40,7 +41,7 @@ export const SnapshotCardExternalLink: FC<SnapshotCardExternalLinkProps> = ({ | |||
| > | |||
| {description} | |||
| </Typography> | |||
| {url && hasValidProtocol(url) && ( | |||
| {url && (hasValidProtocol(url) || url.startsWith('mailto:')) && ( | |||
There was a problem hiding this comment.
I don't think this is a good solution
(unimportant note: you are not using emailAccepted)
There was a problem hiding this comment.
(unimportant note: you are not using emailAccepted)
This part has been fixed.
There was a problem hiding this comment.
I don't think this is a good solution
What would you consider a good solution, instead of prop for the general purpose component?
Maybe a separate component used for displaying safe links?
There was a problem hiding this comment.
Code should make it obvious if it is safe. Right now you have to trace through the code to check if emailAccepted is only used with safe sources of URLs.
Better options:
- all our components should have a safe interface
- sanitize/block malicious mail links in our components that allow urls
- whitelist externalLinks and block all other mail links
- block all mail links in our components. Pass pontusx mail link directly to the unsafe Button component (
<Button href=externalLinks...makes it clear that it comes from a safe source) - block mail links and display pontusx email address instead
- all sources of links should be made safe. Sanitize malicious links in api.ts
Remove "mailto:" from the list of valid protocols, because it can be unsafe to click on mailto links. Supporting mailto: links can enable malicious URLs like javascript: and mailto:?attach=. We are trying to prevent that. Instead, provide a special "emailAccepted" when we know that the link is coming from a safe source, like our code.
csillag
force-pushed
the
csillag/small-cleanups
branch
from
bb88cc9
to
efc81f5
Compare
lukaw3d
reviewed
Feb 21, 2024
Comment on lines
+751
to
+752
| Amount: fromBaseUnits(event.body.Amount, paraTimesConfig[runtime].decimals), | ||
| Denomination: getTokensForScope({ network, layer: runtime })[0].ticker, |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Follow-ups from previous PRs.
For context, see: