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: use the app protocol instead of the file protocol #2991

Merged
merged 2 commits into from
Mar 7, 2025
Merged

Conversation

DIYgod
Copy link
Member

@DIYgod DIYgod commented Mar 7, 2025

Description

PR Type

  • Feature
  • Bugfix
  • Hotfix
  • Other (please describe):

Screenshots (if UI change)

Demo Video (if new feature)

Linked Issues

Additional context

Changelog

  • I have updated the changelog/next.md with my changes.

Copy link

vercel bot commented Mar 7, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
follow ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 7, 2025 7:21am
1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
follow-external-ssr ⬜️ Ignored (Inspect) Visit Preview Mar 7, 2025 7:21am

@follow-reviewer-bot
Copy link

Suggested PR Title:

feat: enhance app protocol handling and URL loading

Change Summary:
Enhanced protocol handling and app-specific URL loading in Electron app. Added app protocol registration and network error handling for improved stability and functionality.

Code Review:

Change Requests

apps/desktop/src/main/src/index.ts

  • Lines 74-82:
    The protocol.handle setup for the "app" protocol uses a url.pathToFileURL(urlObj.pathname).toString() call. However, this does not ensure that the urlObj.pathname is sanitized properly. Malicious inputs could potentially be crafted to exploit this. Ensure that the urlObj.pathname is validated to prevent potential security risks such as directory traversal or unauthorized file access. Adding proper sanitization or validation logic is required.

apps/desktop/src/main/src/init.ts

  • Lines 10-19:
    The protocol.registerSchemesAsPrivileged call adds the "app" scheme with privileges like bypassCSP and supportFetchAPI. These privileges can introduce security risks if the "app" protocol is not carefully restricted and validated. Double-check all usages of the "app" protocol to ensure they aren't introducing vulnerabilities, and include additional comments or documentation about why these privileges are necessary. Consider whether limiting privileges is feasible for this use case.

apps/desktop/src/main/src/window.ts

  • Lines 151-152:
    The options?.extraPath part added to the appLoadEntry URL string is not properly sanitized or encoded. This could allow for possible injection or malformed URLs. Ensure that extraPath is validated and properly encoded before appending it to the URL.

Address these issues to improve the robustness and security of the changes.

@DIYgod DIYgod marked this pull request as ready for review March 7, 2025 10:49
@DIYgod DIYgod changed the title Feature/protocol feat: use the app protocol instead of the file protocol Mar 7, 2025
@DIYgod DIYgod merged commit 047c423 into dev Mar 7, 2025
10 of 11 checks passed
@DIYgod DIYgod deleted the feature/protocol branch March 7, 2025 10:50
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