ubiquity / devpool-directory-tasks

Open bounties for the devpool-directory repository
1 stars 3 forks source link

Delete unauthorized issue right after it was opened #35

Closed rndquu closed 1 month ago

rndquu commented 1 month ago

Right now there is a window for phishing since the sync workflow runs once an hour so an issue created by an adversary will be visible for a whole hour at work.ubq.fi in the worst case.

We could run delete-unauthorized-issues.ts in a separate workflow on issues.opened event to delete such issues right after they're created.

Ideally we must prevent such issues from opening at all but (as far as I know) there's only 1 way to restrict opening github issues and if we restrict interactions only for contributors then collaborators wouldn't be able to open PRs against https://github.com/ubiquity/devpool-directory.

rndquu commented 1 month ago

@Keyrxng Pls check the description. 2 hours time estimate should work, right?

Keyrxng commented 1 month ago

after the 1hr period we may have multiple new issues opened

Why is that an issue?

Not an issue exactly. Should delete-unauthorized-issues.ts remain as-is and loop all issues or be refactored to assert that the payload issue is authorized?

As it is, say three issues open and one is unauthed. We'll have 3 workflow runs and the first one to start will effectively delete any unauthed issues so the following two runs would have no effect. It's not a problem it's just not very "clean" as we may have useless runs spawning.

I think it could stay as-is, it's not a huge problem but thought I'd highlight it.

2hr as-is or 4hr with refactoring

rndquu commented 1 month ago

after the 1hr period we may have multiple new issues opened

Why is that an issue?

Not an issue exactly. Should delete-unauthorized-issues.ts remain as-is and loop all issues or be refactored to assert that the payload issue is authorized?

As it is, say three issues open and one is unauthed. We'll have 3 workflow runs and the first one to start will effectively delete any unauthed issues so the following two runs would have no effect. It's not a problem it's just not very "clean" as we may have useless runs spawning.

I think it could stay as-is, it's not a huge problem but thought I'd highlight it.

2hr as-is or 4hr with refactoring

Let's keep it simple:

  1. Keep current logic (with deleting unauthorized issues once an hour)
  2. Simply add a frontend check https://github.com/ubiquity/work.ubq.fi/issues/84
  3. Close current issue as "not planned"

What do you think?

Keyrxng commented 1 month ago

Let's keep it simple:

This is the simpler option yeah. The frontend check in order to be more robust should have the same logic as here re: checking app installs but means the UI needs APP_ID and APP_PRIVATE_KEY. Failing that I guess just env vars or hardcoding the values for our orgs would suffice.

rndquu commented 1 month ago

Let's keep it simple:

This is the simpler option yeah. The frontend check in order to be more robust should have the same logic as here re: checking app installs but means the UI needs APP_ID and APP_PRIVATE_KEY. Failing that I guess just env vars or hardcoding the values for our orgs would suffice.

Hardcoding authorized issue creators is enough (on the frontend)

ubiquity-os[bot] commented 1 month ago
# Issue was not closed as completed. Skipping.