Skip to content

Conversation

@TeslaSSTec
Copy link

Removed filtering of the hash section from subscriptionUrl.

Resolve remnawave/panel#165

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Greptile Overview

Summary

This PR fixes a bug in subscription URL handling where hash fragments containing usernames were being stripped out during URL construction. The main fix removes the `url.hash = ''` line from the `constructSubscriptionUrl` utility function, allowing hash-based user identification to be preserved in subscription URLs.

The PR also includes several related improvements: enhanced client IP forwarding throughout the backend services (switching from @supercharge/request-ip to @kastov/request-ip library), addition of Cloudflare Zero Trust authentication support with optional client credentials, and comprehensive SEO protection via robots meta tags and HTTP headers. The backend now properly passes real client IPs to the Remnawave panel API instead of the subscription page server's IP, which is crucial for rate limiting, geolocation, and audit logging.

Additionally, the PR includes dependency updates (notably NestJS core packages and the @remnawave/backend-contract from 0.7.1 to 2.1.42), version bumps to 6.0.8 for both frontend and backend, Docker improvements with Node.js version pinning, and build automation enhancements with a new tag-release Makefile target. A minor Russian translation typo was also corrected.

Important Files Changed

Changed Files
Filename Score Overview
frontend/src/shared/utils/construct-subscription-url/construct-subscription-url.tsx 4/5 Removed hash filtering to preserve username information in subscription URLs
backend/src/common/axios/axios.service.ts 4/5 Added client IP forwarding and Cloudflare Zero Trust authentication support
backend/src/modules/root/root.service.ts 5/5 Updated to pass client IP to all axios service methods
backend/src/modules/root/root.controller.ts 4/5 Added @ClientIp() decorator to extract and forward real client IP
backend/src/common/middlewares/get-real-ip.ts 4/5 Switched to @kastov/request-ip library and improved IP detection reliability
backend/src/common/decorators/get-ip/get-ip.ts 4/5 Simplified IP extraction to use standardized request.clientIp property
backend/src/common/middlewares/no-robots.middleware.ts 5/5 Added new middleware to prevent search engine indexing with comprehensive robot exclusion
backend/src/main.ts 4/5 Reorganized middleware order and added SEO protection and security hardening
backend/src/common/config/app-config/config.schema.ts 5/5 Added optional Cloudflare Zero Trust client credentials to environment schema
backend/.env.sample 5/5 Added Cloudflare Zero Trust configuration variables to sample environment file
Dockerfile 4/5 Pinned Node.js version, added PM2 configuration, and simplified container startup
frontend/index.html 5/5 Added robots meta tag to complement server-side SEO protection
backend/package.json 4/5 Version bump to 6.0.8 and dependency updates including IP detection library
frontend/package.json 5/5 Version bump to 6.0.8 for the bug fix release
Makefile 5/5 Added automated release tagging functionality
frontend/public/assets/app-config.json 5/5 Fixed typo in Russian translation for Clash Verge app instructions
backend/src/common/middlewares/index.ts 5/5 Added export for no-robots middleware
backend/src/common/utils/validate-env-config.ts 5/5 Enhanced error handling for environment configuration validation

Confidence score: 4/5

  • This PR is generally safe to merge with some areas requiring attention due to dependency changes and middleware restructuring
  • Score reflects solid implementation of the core fix but complexity from multiple simultaneous changes including major dependency updates
  • Pay close attention to the axios service changes and middleware reordering in main.ts

Sequence Diagram

sequenceDiagram
    participant User
    participant Frontend
    participant ConstructSubscriptionUrl
    participant UFO as UFO Library

    User->>Frontend: "Access subscription page"
    Frontend->>ConstructSubscriptionUrl: "constructSubscriptionUrl(currentUrl, shortUuid)"
    ConstructSubscriptionUrl->>UFO: "parseURL(currentUrl)"
    UFO-->>ConstructSubscriptionUrl: "parsed URL object"
    
    Note over ConstructSubscriptionUrl: Clear search params and auth
    ConstructSubscriptionUrl->>ConstructSubscriptionUrl: "url.search = ''"
    ConstructSubscriptionUrl->>ConstructSubscriptionUrl: "url.auth = ''"
    
    Note over ConstructSubscriptionUrl: Process pathname segments
    ConstructSubscriptionUrl->>ConstructSubscriptionUrl: "split pathname by '/'"
    ConstructSubscriptionUrl->>ConstructSubscriptionUrl: "filter empty segments"
    ConstructSubscriptionUrl->>ConstructSubscriptionUrl: "get last segment"
    
    alt "lastSegment !== shortUuid"
        ConstructSubscriptionUrl->>ConstructSubscriptionUrl: "pop last segment"
        ConstructSubscriptionUrl->>ConstructSubscriptionUrl: "push shortUuid"
        ConstructSubscriptionUrl->>UFO: "joinURL('/', ...segments)"
        UFO-->>ConstructSubscriptionUrl: "new pathname"
        ConstructSubscriptionUrl->>ConstructSubscriptionUrl: "url.pathname = newPath"
    end
    
    ConstructSubscriptionUrl->>UFO: "stringifyParsedURL(url)"
    UFO-->>ConstructSubscriptionUrl: "constructed URL string"
    ConstructSubscriptionUrl-->>Frontend: "return constructed URL"
    Frontend-->>User: "Display subscription URL"
Loading

Additional Comments (1)

  1. Dockerfile, line 30 (link)

    style: The docker-entrypoint.sh file is still being copied but no longer used in CMD. Consider removing this line to clean up the image.

18 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

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.

1 participant