RFC improved gating capabilities, whitelists

[bradrydzewski]

Initial support for gating was implemented per #1935. The initial implementation blocks pull requests from non-project members attempting to change the yaml, pending approval. This effectively eliminates the need to sign the yaml file.

There was also discussion in issue thread #1935 to implement gating based on user membership and whitelists, as opposed to checking for yaml changes. This issue will discuss pros / cons / alternate implementations.

consideration 1

There will be teams that want gated deployments. For example, an employee wants to drone deploy <repo> <number> <environment>. There will be teams that want this gated, and approved by a supervisor or team leader. Gating is not limited to pull request events.

consideration 2

Maintaining a whitelist and approving every pull request, especially for larger open source projects like drone, could become a burden. The benefit to eliminating the signature was simplifying the workflow. We should keep usability and simplicity in mind here.

consideration 3, whitelists

The challenge with whitelists are that we would end up blocking most pull request issued to the drone/drone repository. We get a ton of casual commits. If we were to whitelist every casual committer, they would have the ability to expose my pull request secrets (gitter and slack only) and the trust factor simply may not be present yet. If we don't whitelist casual committers, I end up having to manually approve every pull request AND every subsequent synchronize event.

This is one benefit to the yaml comparison. While it doesn't protect against a zero-day docker exploit or someone trying to consume my server resources, it does prevent them from leaking secrets and only requires I get involved when the yaml changes, which is infrequent.

[bradrydzewski]

I'll get the ball rolling here to spark some ideas ...

Idea 1, Yaml Configuration

It would be interesting if the approval process could be defined in the yaml and take advantage of our plugin capabilities. This could be in the same or separate yaml. Open to thoughts on either option.

Example configuration in new validate section:

validate:
  maintainer:
    image: plugins/maintainers
    when:
      event: pull_request

pipeline:
  build:
    image: golang
    commands: [ go build, go test ]

Pros to this approach:

• the approval process is pluggable
• the approval process is versioned in the yaml
• could allow pull requests to (more safely) use privileged mode

Cons to this approach:

We cannot trust the yaml supplied by the pull request. This means we would need to source the approvals configuration from the target branch. Sourcing the pipeline and approvals from separate versions of the same yaml file might cause confusion.

Plugins will also have limited data available at runtime and will probably need to make external api requests. This could make the approval process setup a bit annoying.

Technical Challenges to this approach:

This would result in the build executing AND all matrix jobs if applicable, and then failing when the validation section fails. This feels very suboptimal.

[bradrydzewski]

Idea 2, Maintainers file, Override behavior with Plugins

The second approach I've considered is using a maintainers file (similar to lgtm) to designate a set of individuals whose pull requests are automatically executed without required approval. This could also be described as a white-list.

The maintainers file could look like this:

bradrydzewski
gtaylor
donny-dont

If the username is not found in the maintainers file, the build is blocked pending manual intervention. A project member (with push or admin access) would have the ability to manually approve the pull request, allowing the build to continue, or decline.

One problem with maintainers files is that people will ask for all sort of funky logic. They will want custom formats. They will want to source the maintainers file from separate repositories, they will ask for inheritance, etc. As I type this paragraph I cringe thinking about the level of flexibility people will expect of Drone.

Override with Plugins

The builtin logic could be overriden with a plugin. The plugin would be in the form of a REST endpoint that gets invoked at runtime. The endpoint would return true / false indicating approval is required. How that plugin reaches that conclusion would be wholly up to the plugin, meaning there would be a lot of flexibility here to implement custom approval logic and workflows.

Disabled by Default

This could be disabled by default, and enabled with a toggle in the repository settings. Disabled by default also could mean insecure by default. If you expose a secret to your pull request without some sort of approval workflow, a bad actor could author a malicious pull request and expose the secret.

We could probably show a warning in the user interface.

[bradrydzewski]

Idea 3, Check for Yaml changes, use plugins for everything else

The third option is to keep the current implementation (from #1935) and block the build if the yaml changes. The pipeline could include additional steps for verification and whitelists.

Example plugin verifies committer is member of whitelist

pipeline:
+ check_membership:
+   image: plugins/membership
+   when:
+     event: pull_request

  build:
    image: golang
    commands:
      - go build

Now imagine a bad actor submits a pull request to change this behavior ... drone will detect the change to the yaml and block the pull request, pending manual review.

pipeline:
- check_membership:
-   image: plugins/membership
-   when:
-     event: pull_request

  build:
    image: golang
    commands:
      - go build

[tboerger]

I like the idea of pluggable checks within the drone.yml, but per default I would prefer the maintainers file.

[tboerger]

Ah, the maintainers file needs to be fetched from the target branch, otherwise somebody can put himself on the maintainers file.

[gtaylor]

Some un-organized initial thoughts:

• It would feel weird to manage secrets via the CLI and/or an external API call, but gate access via the yaml. I have a slight preference to keep auth and secret management out of the YAML.
• As far as bare minimum MVP, I could see a simple image-based approach to making this work.

Purely plugin/image-based approach

• For any event that Drone would act on, we fire up the configured build/deploy/event guardian (or whatever we call it) plugin image and pass in the full event payload like we normally do.
• The plugin could make calls to external services like the Github or Gitlab API, a corporate LDAP instance, whatever. It could even just look at the event body and make a decision based on some simple heuristic.
• We could probably pass secrets into the plugin via the existing secrets system?
• Be it through some combination of exit codes and stdout/stderr or something else, the plugin's only job is to look at the event payload and make a decision on whether the build can proceed past "pending". Responses could be "Allow", "Hold-Pending verification", or "Stop".
  • Allow means we continue immediately.
  • Hold means we put a hold on the build. It will be held until you invoke a drone CLI incantation or make a specific API call to Drone's API. This allows people to write the comment or email-based approval systems, along with any other means. Doesn't really matter.
  • Stop means the build is rejected. It could be manually started by someone with Drone access, but it won't be flagged as pending approval (hold).

Specifying build guardian plugin

Keep this as simple as humanly possible for MVP:

• Allow defining a global default build guardian plugin.
• Also allow defining a per-repo build guardian.
• For consistency with secrets, these should probably both be done through the API/CLI instead of env vars.

Ideas for future expansion

• Org-wide default build guardians plugins. This would fit nicely into your hosted offering, since they could call out to their auth mechanisms of choice without having to go on-prem.

Other things to consider

• I'm not sure whether the build guardian plugin should do anything other than decide the fate of the build. Should it also be where the user would fire off any webhooks they might want, or should Drone core do that?

[bradrydzewski]

changeset merged to provide a custom backend for gating builds and approval.

DRONE_GATEKEEPER_ENDPOINT="http://my.custom.gatekeeper"

the backend must implement the following endpoints:

POST /sender/{owner}/{name}/{username}    // verify user is authorized sender. Request includes build details. Response is true / false
GET /sender/{owner}/{name}                // get permanently authorized senders
GET /sender/{owner}/{name}/{username}     // get permanently authorized sender by username
POST /sender/{owner}/{name}               // create permanently authorized (or blocked) sender
PATCH /sender/{owner}/{name}/{hostname}   // update permanently authorized (or blocked) sender
DELETE /sender/{owner}/{name}/{hostname}  // delete permanently authorized (or blocked) sender

the backend is responsible for securing itself. The recommended approach is to use private networking or a basic auth.

DRONE_GATEKEEPER_ENDPOINT="http://{username}:{password}@my.custom.gatekeeper"

the full details will be documented prior to the official release. I don't expect the plugin interface to be flexible enough for all use cases, but hopefully people will write plugins and help us improve.