Skip to content

Conversation

sergiubreban
Copy link
Contributor

@sergiubreban sergiubreban commented Aug 28, 2025

✅ Checklist

  • npx changeset was attached.
  • Covered by automatic tests. We couldn't find the original requirement for this part of the code, and I scanned the Earn project for all uses. I found out that only internal ledger domain URLs should trigger this functionality. Everything looks fine from my tests. We should try all the use-cases for this modal interaction to ensure no regression.
  • Impact of the changes: - there should be no impact for the regular user. The attackers will be blocked if trying to use a malicious url in the query params (anything other than ledger domains | protocol)
    • ...

📝 Description

Validate and whitelist incoming deeplink inputs to prevent misuse while preserving legitimate functionality. details and example of malicious request can be found in the ticket

❓ Context


🧐 Checklist for the PR Reviewers

  • The code aligns with the requirements described in the linked JIRA or GitHub issue.
  • The PR description clearly documents the changes made and explains any technical trade-offs or design decisions.
  • There are no undocumented trade-offs, technical debt, or maintainability issues.
  • The PR has been tested thoroughly, and any potential edge cases have been considered and handled.
  • Any new dependencies have been justified and documented.
  • Performance considerations have been taken into account. (changes have been profiled or benchmarked if necessary)

@sergiubreban sergiubreban requested a review from a team as a code owner August 28, 2025 13:02
Copy link

vercel bot commented Aug 28, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

4 Skipped Deployments
Project Deployment Preview Comments Updated (UTC)
ledger-live-github-bot Ignored Ignored Preview Sep 1, 2025 2:30pm
native-ui-storybook Ignored Ignored Preview Sep 1, 2025 2:30pm
react-ui-storybook Ignored Ignored Preview Sep 1, 2025 2:30pm
web-tools Ignored Ignored Preview Sep 1, 2025 2:30pm

@live-github-bot live-github-bot bot added the mobile Has changes in LLM label Aug 28, 2025
@sergiubreban sergiubreban force-pushed the LIVE-20345-security-sanitise-earns-deeplink-query-string-and-enforce-allow-list-in-deeplinks-provider-tsx branch from 0233e00 to 1e1ebb2 Compare August 28, 2025 13:26
gre-ledger
gre-ledger previously approved these changes Sep 1, 2025
Copy link
Contributor

@beths-ledger beths-ledger left a comment

Choose a reason for hiding this comment

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

Looks generally good but I have some concern about the max length requirements - is max length required for the tooltips?

Copy link
Contributor

@beths-ledger beths-ledger left a comment

Choose a reason for hiding this comment

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

nice

@sergiubreban sergiubreban merged commit 6ec7305 into develop Sep 1, 2025
55 checks passed
@sergiubreban sergiubreban deleted the LIVE-20345-security-sanitise-earns-deeplink-query-string-and-enforce-allow-list-in-deeplinks-provider-tsx branch September 1, 2025 15:31
Copy link

sentry-io bot commented Sep 2, 2025

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

Did you find this useful? React with a 👍 or 👎

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mobile Has changes in LLM
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants