ubiquity-os / ubiquity-os-kernel

1 stars 13 forks source link

feat: plugins can now be spawned as Workers #42

Closed gentlementlegen closed 4 months ago

gentlementlegen commented 4 months ago

Resolves #39

gentlementlegen commented 4 months ago

Tests seem to be failing due to empty environment variables during the tests https://github.com/ubiquity/ubiquibot-kernel/actions/runs/9149319076/job/25152816695?pr=42#step:5:7 which I cannot fix myself.

0x4007 commented 4 months ago

@rndquu youre admin please facilitate

rndquu commented 4 months ago

@gentlementlegen

Check this workflow

How it works right now:

  1. PR is opened
  2. The development branch is checked out here (not the PR branch because the workflow is running on pull_request_target)
  3. Tests are run against the development branch

If here we change the code to checkout the PR branch using pull_request_target then any fork will have access to these env variables.

So right now this workflow is literally 1 line away from leaking secrets.

There are viable 2 options:

  1. Refactor tests and mock everything in a way that we wouldn't need to provide any env vars
  2. Keep using this workflow with pull_request_target, checkout the PR branch here, hardcode env variables and use some dummy values that are used only for testing
gentlementlegen commented 4 months ago

@rndquu will be mocking as we cannot have dummy values since it tries to reach a real endpoint.

gentlementlegen commented 4 months ago

Changes in this PR

Test still expected to fail for pull_request_target, successful pull_request test here

whilefoo commented 4 months ago

we can merge this, right?

gentlementlegen commented 4 months ago

@whilefoo was waiting for second validation but if you're good with it for sure