CI: Reject `ubiquibot-config.yml` changes from unauthorized contributors

[0x4007]

Only admins and billing_managers should have the authority to change the bot config due to price sensitivity. Specifically anything related to finances.

I quickly parsed out the finance related properties from the default config:

{
  evmNetworkId: 100,
  priceMultiplier: 1,
  issueCreatorMultiplier: 2,
  paymentPermitMaxPrice: 9007199254740991,
  assistivePricing: false,
  commentIncentives: false,
  incentives: {
    comment: {
      elements: {},
      totals: {
        word: 0,
      },
    },
  },
  enableAccessControl: {
    label: false,
    organization: true,
  },
}

[BeanieMen]

How would it reject it?

[BeanieMen]

/start

[ubiquibot[bot]]

Deadline	Mon, 09 Oct 2023 10:20:42 UTC
Registered Wallet	0x219d695ff93b443fc3E943BD1052805AF83C6612

Tips:

• Use /wallet 0x0000...0000 if you want to update your registered payment wallet address @user.
• Be sure to open a draft pull request as soon as possible to communicate updates on your progress.
• Be sure to provide timely updates to us when requested, or you will be automatically unassigned from the bounty.

[BeanieMen]

/stop

[ubiquibot[bot]]

You have been unassigned from the bounty @me505

[bojan07]

/apply

[ubiquibot[bot]]

Available commands

- /start: Assign the origin sender to the issue automatically.
- /stop: Unassign the origin sender from the issue automatically.
- /help: List all available commands.
- /autopay: Toggle automatic payment for the completion of the current issue.
- /query: Comments the users multiplier and address
- /multiplier: Set the bounty payout multiplier for a specific contributor, and provide the reason for why. 
  example usage: "/wallet @user 0.5 'Multiplier reason'"
- /allow: Set access control. (Admin Only)
- /wallet: <WALLET_ADDRESS | ENS_NAME>: Register the hunter's wallet address. 
  ex1: /wallet 0x0000000000000000000000000000000000000000
  ex2: /wallet vitalik.eth

@bojan07

[bojan07]

/help

[ubiquibot[bot]]

Available commands

- /start: Assign the origin sender to the issue automatically.
- /stop: Unassign the origin sender from the issue automatically.
- /help: List all available commands.
- /autopay: Toggle automatic payment for the completion of the current issue.
- /query: Comments the users multiplier and address
- /multiplier: Set the bounty payout multiplier for a specific contributor, and provide the reason for why. 
  example usage: "/wallet @user 0.5 'Multiplier reason'"
- /allow: Set access control. (Admin Only)
- /wallet: <WALLET_ADDRESS | ENS_NAME>: Register the hunter's wallet address. 
  ex1: /wallet 0x0000000000000000000000000000000000000000
  ex2: /wallet vitalik.eth

[bojan07]

/multiplier

[ubiquibot[bot]]

Insufficient permissions to update the payout multiplier. You are not an admin or billing_manager

[bojan07]

/start

[ubiquibot[bot]]

Deadline	Wed, 11 Oct 2023 19:50:49 UTC
Registered Wallet	0xc6fa133f3290e14Ad91C7449f8D8101A6f894E25

Tips:

• Use /wallet 0x0000...0000 if you want to update your registered payment wallet address @user.
• Be sure to open a draft pull request as soon as possible to communicate updates on your progress.
• Be sure to provide timely updates to us when requested, or you will be automatically unassigned from the bounty.

[ubiquibot[bot]]

Do you have any updates @bojan07? If you would like to release the bounty back to the DevPool, please comment /stop Last activity time: Wed Oct 11 2023 17:50:44 GMT+0000 (Coordinated Universal Time)

[ubiquibot[bot]]

@bojan07 - Releasing the bounty back to dev pool because the allocated duration already ended! Last activity time: Wed Oct 11 2023 17:50:44 GMT+0000 (Coordinated Universal Time)

[Keyrxng]

I'll draft this tomorrow today

[0x4007]

We did pause merges on this repo until the refactor is merged in though. But at least it'll allow you to get a head start on the research. You might have merge conflicts though.

[Keyrxng]

We did pause merges on this repo until the refactor is merged in though.

I noticed that but I need to work so try and stop me :))

You might have merge conflicts though.

On this one I think I'll get away with but the others I have dormant, definitely will have a host of them but I appreciate the heads-up

[Keyrxng]

@pavlovcik or @rndquu, the roles come from the org right? Not the db?

[rndquu]

@pavlovcik or @rndquu, the roles come from the org right? Not the db?

yes

[Keyrxng]

yes

figured and a few mins after I asked found where to inv myself 😂 cheers mate

[Keyrxng]

Tougher than I expected but nearly there

I'm using a dummy account to QA and it seems that billing_manager is overridden by any other role as opposed to listing all applicable as the role prop is an enum. So I'm trying to think of edge cases but I've little behind the scenes info. What I mean by that is, are billing_manager's active contributors or are they more 'back office'? It'll need covered but just trying to think of workarounds for the below.

What I've tested so far, for instance:

• In a private repo the billing manager would need invited and therefor would have a role assigned as part of the invite for that repo in particular

• At an org level, they cannot be a member otherwise it overrides but this in turn stops the api needed to actually fetch the role info.

I'll get there but if you've any insight that would help I'd appreciate it

[Keyrxng]

Q & A:

• Rest API: As far as I can see, the only way to return billing_manager role is through the teams api, so are they assigned a team typically?

• Rest API: It seems like you can pull it via the invitation endpoint but pending invites expire and accepted ones aren't kept so that's a bust, not a really a question either

• GraphQL Doc link: "Organization billing plan does not support access to auditLog information" So we can't use auditLog, unless I'm mistaken and UBQ would have access to that?

• Are billing_managers often an admin as well, I'd imagine so but reading the github docs throw me off as they abstract the billing manager away from being directly involved.

---

Basically then it seems like checking for billing_manager status is out of the question unless it's a paid Enterprise, unless they are added to the org as an admin as well I think I'll only be able to check for admin status on config updates.

Please correct me if I'm wrong

[Keyrxng]

The config, I assume for a particular reason, doesn't use double quotes which is making it hard to parse in order to do a granular review of changed properties.

• Is this a necessity that it be so granular?
• Or is it sufficient enough to just do: "isAdmin ? pass : fail"

The point being here, whenever someone without authority changes the config we want to be warned the config has changed. However, in cases where the change is required the CI will need to be confirmed by the reviewers ANYWAY, so in all scenarios it fails and would continue to fail unless committed by an admin or overridden as an acceptable change. Considering this, "isAdmin ? pass : fail" makes sense to me.

[Keyrxng]

Forgot about the other action that updates the config. Got it working against my private org repo, draft it maybe tonight probably tomorrow

[0x4007]

I've got mixed feelings on the yml implementation. Installation to partner repos will be more complicated if they need to add yml.

Instead ideally the bot should handle it in its core code so that when they add the bot to their repo, it can handle this use case.

I haven't come to a conclusion on this because I asked @wannacfuture to research delegating long running processes to GitHub actions (permit generation and AI related tasks) which might require us to figure some flow that allows yml installation when the bot is added to the repo etc.

[Keyrxng]

At first I did start building it into the Bot yknow and spoke myself out of it with incorrect thinking that it would be easier as an action but it would have been simpler probably, that's why I ask the below, which itself doesn't makes sense but my train of thought was all wrong at first lmao

pavlovcik or rndquu, the roles come from the org right? Not the db?

So for now just sit on things until the research comes in, not a problem

[0x4007]

So for now just sit on things until the research comes in, not a problem

@wannacfuture any remarks to add?

[ubiquibot[bot]]

These linked pull requests are closed: #880

[0x4007]

Remind me after the refactor and we'll get reviews going again etc.

[BeanieMen]

Yay