Skip to content

Commit f5ca647

Browse files
author
epriestley
committed
Add bin/repository edit for CLI repository editing
Summary: Ref T4039. This is mostly to deal with that, to prevent the security issues associated with mutable local paths. The next diff will lock them in the web UI. I also added a confirmation prompt to `bin/repository delete`, which was a little scary without one. See one comment inline about the `--as` flag. I don't love this, but when I started adding all the stuff we'd need to let this transaction show up as "Administrator" it quickly got pretty big. Test Plan: Ran `bin/repository edit ...`, saw an edit with a transaction show up on the web UI. Reviewers: btrahan Reviewed By: btrahan CC: aran Maniphest Tasks: T4039 Differential Revision: https://secure.phabricator.com/D7579
1 parent 626b3da commit f5ca647

File tree

5 files changed

+128
-1
lines changed

5 files changed

+128
-1
lines changed

scripts/repository/manage_repositories.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
$workflows = array(
1919
new PhabricatorRepositoryManagementPullWorkflow(),
2020
new PhabricatorRepositoryManagementDiscoverWorkflow(),
21+
new PhabricatorRepositoryManagementEditWorkflow(),
2122
new PhabricatorRepositoryManagementListWorkflow(),
2223
new PhabricatorRepositoryManagementDeleteWorkflow(),
2324
new PhabricatorRepositoryManagementMarkImportedWorkflow(),

src/__phutil_library_map__.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1731,6 +1731,7 @@
17311731
'PhabricatorRepositoryListController' => 'applications/repository/controller/PhabricatorRepositoryListController.php',
17321732
'PhabricatorRepositoryManagementDeleteWorkflow' => 'applications/repository/management/PhabricatorRepositoryManagementDeleteWorkflow.php',
17331733
'PhabricatorRepositoryManagementDiscoverWorkflow' => 'applications/repository/management/PhabricatorRepositoryManagementDiscoverWorkflow.php',
1734+
'PhabricatorRepositoryManagementEditWorkflow' => 'applications/repository/management/PhabricatorRepositoryManagementEditWorkflow.php',
17341735
'PhabricatorRepositoryManagementImportingWorkflow' => 'applications/repository/management/PhabricatorRepositoryManagementImportingWorkflow.php',
17351736
'PhabricatorRepositoryManagementListWorkflow' => 'applications/repository/management/PhabricatorRepositoryManagementListWorkflow.php',
17361737
'PhabricatorRepositoryManagementMarkImportedWorkflow' => 'applications/repository/management/PhabricatorRepositoryManagementMarkImportedWorkflow.php',
@@ -4174,6 +4175,7 @@
41744175
'PhabricatorRepositoryListController' => 'PhabricatorRepositoryController',
41754176
'PhabricatorRepositoryManagementDeleteWorkflow' => 'PhabricatorRepositoryManagementWorkflow',
41764177
'PhabricatorRepositoryManagementDiscoverWorkflow' => 'PhabricatorRepositoryManagementWorkflow',
4178+
'PhabricatorRepositoryManagementEditWorkflow' => 'PhabricatorRepositoryManagementWorkflow',
41774179
'PhabricatorRepositoryManagementImportingWorkflow' => 'PhabricatorRepositoryManagementWorkflow',
41784180
'PhabricatorRepositoryManagementListWorkflow' => 'PhabricatorRepositoryManagementWorkflow',
41794181
'PhabricatorRepositoryManagementMarkImportedWorkflow' => 'PhabricatorRepositoryManagementWorkflow',

src/applications/metamta/contentsource/PhabricatorContentSource.php

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ final class PhabricatorContentSource {
99
const SOURCE_MOBILE = 'mobile';
1010
const SOURCE_TABLET = 'tablet';
1111
const SOURCE_FAX = 'fax';
12+
const SOURCE_CONSOLE = 'console';
1213
const SOURCE_LEGACY = 'legacy';
1314

1415
private $source;
@@ -39,6 +40,12 @@ public static function newFromSerialized($serialized) {
3940
return $obj;
4041
}
4142

43+
public static function newConsoleSource() {
44+
return self::newForSource(
45+
PhabricatorContentSource::SOURCE_CONSOLE,
46+
array());
47+
}
48+
4249
public static function newFromRequest(AphrontRequest $request) {
4350
return self::newForSource(
4451
PhabricatorContentSource::SOURCE_WEB,
@@ -61,6 +68,7 @@ public static function getSourceNameMap() {
6168
self::SOURCE_MOBILE => pht('Mobile'),
6269
self::SOURCE_TABLET => pht('Tablet'),
6370
self::SOURCE_FAX => pht('Fax'),
71+
self::SOURCE_CONSOLE => pht('Console'),
6472
self::SOURCE_LEGACY => pht('Legacy'),
6573
self::SOURCE_UNKNOWN => pht('Other'),
6674
);

src/applications/repository/management/PhabricatorRepositoryManagementDeleteWorkflow.php

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,10 @@ public function didConstruct() {
1414
'name' => 'verbose',
1515
'help' => 'Show additional debugging information.',
1616
),
17+
array(
18+
'name' => 'force',
19+
'help' => 'Do not prompt for confirmation.',
20+
),
1721
array(
1822
'name' => 'repos',
1923
'wildcard' => true,
@@ -30,9 +34,25 @@ public function execute(PhutilArgumentParser $args) {
3034
}
3135

3236
$console = PhutilConsole::getConsole();
37+
38+
if (!$args->getArg('force')) {
39+
$console->writeOut("%s\n\n", pht('These repositories will be deleted:'));
40+
41+
foreach ($repos as $repo) {
42+
$console->writeOut(
43+
" %s %s\n",
44+
'r'.$repo->getCallsign(),
45+
$repo->getName());
46+
}
47+
48+
$prompt = pht('Permanently delete these repositories?');
49+
if (!$console->confirm($prompt)) {
50+
return 1;
51+
}
52+
}
53+
3354
foreach ($repos as $repo) {
3455
$console->writeOut("Deleting '%s'...\n", $repo->getCallsign());
35-
3656
$repo->delete();
3757
}
3858

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
<?php
2+
3+
final class PhabricatorRepositoryManagementEditWorkflow
4+
extends PhabricatorRepositoryManagementWorkflow {
5+
6+
public function didConstruct() {
7+
$this
8+
->setName('edit')
9+
->setExamples('**edit** --as __username__ __repository__ ...')
10+
->setSynopsis('Edit __repository__, named by callsign.')
11+
->setArguments(
12+
array(
13+
array(
14+
'name' => 'repos',
15+
'wildcard' => true,
16+
),
17+
array(
18+
'name' => 'as',
19+
'param' => 'user',
20+
'help' => 'Edit as user.',
21+
),
22+
array(
23+
'name' => 'local-path',
24+
'param' => 'path',
25+
'help' => 'Edit the local path.',
26+
),
27+
));
28+
}
29+
30+
public function execute(PhutilArgumentParser $args) {
31+
$repos = $this->loadRepositories($args, 'repos');
32+
33+
if (!$repos) {
34+
throw new PhutilArgumentUsageException(
35+
"Specify one or more repositories to edit, by callsign.");
36+
}
37+
38+
$console = PhutilConsole::getConsole();
39+
40+
// TODO: It would be nice to just take this action as "Administrator" or
41+
// similar, since that would make it easier to use this script, harder to
42+
// impersonate users, and more clear to viewers what happened. However,
43+
// the omnipotent user doesn't have a PHID right now, can't be loaded,
44+
// doesn't have a handle, etc. Adding all of that is fairly involved, and
45+
// I want to wait for stronger use cases first.
46+
47+
$username = $args->getArg('as');
48+
if (!$username) {
49+
throw new PhutilArgumentUsageException(
50+
pht("Specify a user to edit as with --as <username>."));
51+
}
52+
53+
$actor = id(new PhabricatorPeopleQuery())
54+
->setViewer(PhabricatorUser::getOmnipotentUser())
55+
->withUsernames(array($username))
56+
->executeOne();
57+
58+
if (!$actor) {
59+
throw new PhutilArgumentUsageException(
60+
pht("No such user '%s'!", $username));
61+
}
62+
63+
foreach ($repos as $repo) {
64+
$console->writeOut("Editing '%s'...\n", $repo->getCallsign());
65+
66+
$xactions = array();
67+
68+
$type_local_path = PhabricatorRepositoryTransaction::TYPE_LOCAL_PATH;
69+
70+
if ($args->getArg('local-path')) {
71+
$xactions[] = id(new PhabricatorRepositoryTransaction())
72+
->setTransactionType($type_local_path)
73+
->setNewValue($args->getArg('local-path'));
74+
}
75+
76+
if (!$xactions) {
77+
throw new PhutilArgumentUsageException(
78+
pht("Specify one or more fields to edit!"));
79+
}
80+
81+
$content_source = PhabricatorContentSource::newConsoleSource();
82+
83+
$editor = id(new PhabricatorRepositoryEditor())
84+
->setActor($actor)
85+
->setContentSource($content_source)
86+
->setContinueOnNoEffect(true)
87+
->setContinueOnMissingFields(true)
88+
->applyTransactions($repo, $xactions);
89+
}
90+
91+
$console->writeOut("Done.\n");
92+
93+
return 0;
94+
}
95+
96+
}

0 commit comments

Comments
 (0)