Skip to content

Conversation

@seaerchin
Copy link
Collaborator

Problem

closes https://linear.app/ogp/issue/ISOM-2009/vapt-low-internal-and-private-ip-addresses-as-long-urls

Solution

  • validate that the domain section of the url is not an IP address
  • this validator is used across the dashboard, api and bulk upload
  • also validated that this prevents users from changing existing links to have an IP address as the long url
    • i'm ok w/ the case where users cannot change existing long urls that are IP addresses but i haven't validated teh scale of the problem

@linear
Copy link

linear bot commented Dec 4, 2025

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds validation to prevent IP addresses (both internal and private) from being used as long URLs in the link shortening service. This addresses a security vulnerability where users could potentially use IP addresses to access internal network resources. The fix is implemented in the shared validation utility and will be enforced across the dashboard, API, and bulk upload features.

Key Changes:

  • Added IP address validation to isValidUrl() function using validator.isIP() to reject URLs with IP-based hostnames
  • Updated package-lock.json with automatic dependency updates, including moving date-fns from dev to production dependencies (correctly reflects its usage in production code)

Reviewed changes

Copilot reviewed 1 out of 2 changed files in this pull request and generated 3 comments.

File Description
src/shared/util/validation.ts Added IP address check after URL validation to reject URLs with IP-based hostnames
package-lock.json Automatic lockfile updates including dependency classification fixes (date-fns moved to production)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Contributor

@dcshzj dcshzj left a comment

Choose a reason for hiding this comment

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

Approving first, but I think Copilot got valid points regarding the test coverage, would be good to add.

@seaerchin seaerchin merged commit a152286 into develop Dec 9, 2025
18 checks passed
@seaerchin seaerchin deleted the fix/long-urls branch December 9, 2025 05:05
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.

2 participants