Details

Reviewers

Group Reviewers

Restricted Project

Commits

rSTAGING2696cfecd29b: [land-bot] Only operate on trusted patches
rABC2696cfecd29b: [land-bot] Only operate on trusted patches

Summary

Even though the land bot will be running in a jail, so the results of the
build do not matter even if they are horribly clobbered, it's possible for the build
to leak sensitive information such as the SSH private key that will be used to push
changes upstream. To ensure this is not possible, all revisions going through land
bot will at least need to be already greened by folks in the Bitcoin ABC reviewer group.

Test Plan

CONDUIT_TOKEN=<your-token> ./land-patch.sh --dry-run -r Dxxxx

Where xxxx point to revisions with different statuses:

Accepted by Bitcoin ABC reviewers (patch is applied, smoke tests run, etc.)
Rejected by Bitcoin ABC reviewers (fails as expected)
Not reviewed by Bitcoin ABC reviewers (fails as expected)

Diff Detail

Repository

rABC Bitcoin ABC

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

jasonbcox created this revision.May 8 2020, 23:13

Herald added a reviewer: Restricted Project. · View Herald TranscriptMay 8 2020, 23:13

jasonbcox requested review of this revision.May 8 2020, 23:13

Build Bitcoin-ABC / Diffs / Diff Testing (build-diff) started.

Fix lint issue

Build Bitcoin-ABC / Diffs / Diff Testing (build-diff) started.

The build failed due to an unexpected infrastructure outage. The administrators have been notified to investigate. Sorry for the inconvenience.

Harbormaster completed remote builds in B10667: Diff 19854.May 8 2020, 23:19

The build failed due to an unexpected infrastructure outage. The administrators have been notified to investigate. Sorry for the inconvenience.

Harbormaster completed remote builds in B10668: Diff 19855.May 8 2020, 23:21

Build Bitcoin-ABC / Diffs / Diff Testing (build-diff) started.

Build Bitcoin-ABC / Diffs / Diff Testing (build-diff) passed.

I don't understand why you need to do the check in this script. Isn't it already checked by arc land ?

You are reimplementing rules that already exist and are checked by arc land. Just do what arc land does.

This revision now requires changes to proceed.May 11 2020, 13:50

In D6017#147418, @Fabien wrote:

I don't understand why you need to do the check in this script. Isn't it already checked by arc land ?

It's necessary to check before doing arc land because arc land behavior can be changed by the patch by modifying .arcconfig or any of the libraries loaded by .arcconfig.