Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Sep 19, 2025

Adds an onConnection callback option to Uppy Companion for authenticating WebSocket connections, addressing security concerns raised in #5922.

Problem

The WebSocket endpoint for upload progress (/api/<token>) had no authentication mechanism, creating a potential security vulnerability. Security auditors flagged this as a concern since anyone could potentially connect to these WebSocket endpoints without authorization.

Solution

This PR introduces an optional onConnection callback that allows developers to implement custom authentication logic for WebSocket connections:

const options = {
  onConnection: async (ws, req) => {
    // Custom authentication logic
    const token = req.headers.authorization || 
                  new URL(req.url, 'http://localhost').searchParams.get('token')
    
    if (!isValidToken(token)) {
      throw new Error('Unauthorized')
    }
    // Connection allowed if no exception thrown
  }
}

const { app, setupSocket } = companion.app(options)
const server = app.listen(port)
setupSocket(server)

Key Features

  • Early Authentication: The callback is invoked immediately upon WebSocket connection, before any processing
  • Full Request Access: Receives both WebSocket instance and HTTP request object for flexible authentication
  • Proper Error Handling: Failed authentication closes connections with 1008 (Policy Violation) status code
  • Async Support: Supports async functions for database lookups or external authentication services
  • Backward Compatible: Existing usage patterns continue to work unchanged

Implementation Details

  • Added onConnection: null to default companion options
  • Modified setupSocket to accept options parameter and call authentication callback
  • Updated companion app to return setupSocket function with options bound
  • Added comprehensive test coverage (4 new tests) while maintaining all existing functionality
  • Updated documentation with usage examples

Testing

All existing tests continue to pass (114/114), plus new tests covering:

  • Successful authentication scenarios
  • Authentication failure handling with correct close codes
  • Normal operation without authentication callback
  • Proper parameter passing to callback function

The implementation is minimal and surgical, adding only the requested security feature without any breaking changes to existing APIs.

Fixes #5922


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

@changeset-bot
Copy link

changeset-bot bot commented Sep 19, 2025

⚠️ No Changeset found

Latest commit: 7be0e39

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copilot AI changed the title [WIP] make an option onConnection to uppy/companion as described in @transloadit/uppy/issues/5922 Add onConnection option for WebSocket authentication in Companion Sep 19, 2025
Copilot AI requested a review from mifi September 19, 2025 09:12
Copilot finished work on behalf of mifi September 19, 2025 09:12
@socket-security
Copy link

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Addedtailwindcss@​4.1.131001008498100

View full report

@socket-security
Copy link

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Addedtailwindcss@​4.1.131001008498100

View full report

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.

Websocket for provider based upload progress is not secured

2 participants