Page MenuHomePhabricator

Would like people who can't edit Harbormaster plans to be able to run them
Closed, ResolvedPublic

Description

Currently, a user who doesn't have Harbormaster privileges to edit a build plan can't push the button to run that build plan on a buildable.

I have a new team member who doesn't need to edit Harbormaster build plans (yet), but I'd like him to be able to run some of our integration tests that are represented as Harbormaster build plans. In order to run them, we had to give him permission to edit them as well.

We'd like to separate invocation permission from editing permission.

@yelirekim said:

if they agree it's a change that should be made
i'll fix it

Event Timeline

I have mixed feelings about relaxing this. I think there are basically two parts to this:

  • running the plan at all; and
  • running the plan on particular things.

Users who can view a plan can already run it via Herald, which currently only requires view permission in order to write a "Run build plans:" rule. However, this limits what they can run it on to things they have permission to edit, generally (e.g., even with an "Always" condition, they still need to create a revision or push a commit to trigger Herald). This is at least some sort of limit to power.

If we relaxed this, any user who could see a plan could run it on any object. The particular attack this enables is:

  • A build plan exists like "Deploy Code to Orbital Satellites".
  • An attacker runs the plan on an old version of the code which contains a known security vulnerability.
  • This code passes build checks and is beamed up to the satellites.
  • The attacker exploits the known vulnerability and compromises the satellites.

I'm not sure what the best way to defuse this attack is, but relaxing this permission lowers the barrier to performing it. I'd like to have a better plan in place for dealing with this threat, even if we don't implement it today, before making it more accessible.

Some ideas offhand:

  • Separate "can run builds" permission?
    • Probably just clutter 99% of the time.
    • Not "safe by default" unless administrators are very careful.
    • But maybe actually want we want so we can quorum it (T9515) and advise installs to put M-of-N release engineer manual approval on "Deploy Code to Orbital Satellites" plans. I think this got a mention in T9530.
  • Let plans act differently during test runs vs real runs?
    • Not safe by default, really hard/not-obvious to make safe.
    • Greatly reduces the value of having test runs.
    • Spooky magic.
  • Blacklist known bad versions as unbuildable?
    • Kind of conflates unbuildable/undeployable.
    • Not safe by default.

I don't really love any of these, although I maybe lean toward a separate "run" permission.

Do you have any particular concerns about granting the new team member (who, hypothetically, we'll assume is a state-sponsored actor from a sophisticated foreign nation who has deftly evaded countermeasures in the hiring process so far) edit access? In the scenario above, "run" access is pretty much just as dangerous as "edit" access. Is there a different kind of attack they could execute given "edit" access that I'm not thinking about? Or are you just concerned that they may make mistakes (or, perhaps, intentionally sabotage operations by appearing to make mistakes, given that they are an agent of a foreign state)?

It's also probably bad/inconsistent that you can run builds via Herald which you could not run via the web UI.

(You can't write "Run Build Plans: ..." personal rules, but you can write them in object rules, which isn't too much different.)

Or are you just concerned that they may make mistakes

This. In the interim we've given him edit permission and a word of caution.

eadler added a project: Restricted Project.Aug 5 2016, 4:45 PM

I am also interested in this; it's not uncommon that an engineer is testing a regression somewhere and wants to trigger a full CI build of an old revision. Today, their options are (a) have admin access to the Harbormaster Build Plan, which requires a large amount of trust or (b) bother someone who does have admin access to press buttons.

In our environment, there's very limited danger in letting someone run particular builds (since these plans are used for automated unit/integration testing, not for deployment).

I would be fine with just a checkbox in the Build Plan that is labeled something like "Viewers can Run Builds Manually".

After D20229, the rules are:

  • By default, with "Runnable" set to "If Editable", you must have permission to edit a build plan in order to manually run it or restart, pause, resume, or abort associated builds.
  • With "Runnable" set to "If Viewable", you may take these actions as long as you have permission to view the build plan.