Page MenuHomePhabricator

Fix an issue with incorrect authorization handling in Working Copy build steps

Authored by epriestley on Oct 30 2015, 3:01 PM.



Fixes T9669. Two issues:

  • We were using repositoryPHIDs instead of blueprintPHIDs for the list of allowed blueprints. Use the correct value.
  • We weren't enforcing allowedBlueprintPHIDs fully correctly. We did require an authorization, so the net effect was correct in nearly all cases, but we could have selected from too large a pool in the case where the application itself was doing the authorization (e.g., from the command line).
Test Plan

Ran a build through Drydock/Harbormaster locally.

Diff Detail

rP Phabricator
Lint Not Applicable
Tests Not Applicable

Event Timeline

epriestley retitled this revision from to Fix an issue with incorrect authorization handling in Working Copy build steps.
epriestley updated this object.
epriestley edited the test plan for this revision. (Show Details)
epriestley added a reviewer: chad.

This line is the important one we were missing -- we didn't actually do anything with $blueprint_phids before.


...although it didn't really matter much, because we almost always enforced an effectively identical check here anyway.


...but could have gone through this branch without enforcing the check, causing a CLI allocation to end up on a surprising blueprint.

Does this also add the buildable repository asdefault repository cloned ?

Does this also add the buildable repository as default repository cloned ?

The repository for the buildable is already always cloned.


Here's where we explicitly configure the lease to clone the buildable's repository, no matter what Also Clone is set to.

chad edited edge metadata.
This revision is now accepted and ready to land.Oct 30 2015, 3:53 PM

That was what I though. I wasn't sure about it though.
Thanks for confirming.

This revision was automatically updated to reflect the committed changes.