Pull request overview

Updates the worker “join” contract to include a signature and wires it through the worker client and core API join handling.

Changes:

• Add optional signature to worker join DTOs (core API + generated clients).
• Pass WORKER_SIGNATURE from the worker when calling /workers/join.
• Enforce worker signature checking in WorkersService.join.

Reviewed changes

Copilot reviewed 6 out of 8 changed files in this pull request and generated 2 comments.

        const worker: any = await coreApi.workersControllerJoin({
          apiKey: process.env.API_KEY! || process.env.OASM_CLOUD_APIKEY!,
          signature: process.env.WORKER_SIGNATURE,
        });
        Tool.workerId = worker.id;
        Tool.token = worker.token;
        logger.success(
          `CONNECTED ✅ WorkerId: ${Tool.workerId?.split("-")[0]}`,
        );

        // Wait until Tool.workerId is set (by SSE handler)
        await this.waitUntil(() => !!Tool.workerId, 1000);
        return; // success, exit the method
      } catch (e: any) {
        // If we get a 401 error, don't retry - just throw an API key invalid error
        if (e?.response?.status === 401) {
          logger.error("API key is invalid. Cannot connect to core.");
        }

        attempt++;
        logger.error(
          `Cannot connect to core ${process.env.API} (attempt ${attempt}):`,
        );
        await this.sleep(1000 * attempt); // exponential backoff delay
      }

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

Code Review

The pull request introduces a new signature field for worker joining, enhancing security by allowing workers to authenticate with a shared secret. The changes span across type definitions, DTOs, the worker controller, and the worker service to implement this new authentication mechanism. There are a couple of high-severity issues related to type consistency and handling of the optional signature field that need to be addressed.

Hey @mizhm, thanks for working on this security enhancement!

The PR looks good overall and adds an important security layer by implementing worker signature validation. The approach of requiring a signature from workers and validating it against a configured WORKER_SIGNATURE environment variable is solid.

However, I noticed a potential security concern that needs to be addressed:

1. The signature field is marked as optional (signature?: string) in some DTOs but the validation logic treats it as required
2. The code uses || '' which could allow empty signatures to pass validation if the WORKER_SIGNATURE environment variable is not set
3. The example.env files don't include the new WORKER_SIGNATURE variable that workers will need

Could you please address these points:

1. Make the signature field consistently required in the DTO validation when it's expected
2. Ensure proper handling when WORKER_SIGNATURE is not configured (perhaps fail securely rather than allowing empty signatures)
3. Update the example.env files to include the WORKER_SIGNATURE variable with appropriate documentation

This security feature is important, so let's make sure it's implemented robustly. Please make these changes and we can merge this.