1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-23 23:32:40 +01:00
Commit graph

529 commits

Author SHA1 Message Date
Andre Klapper
a76444a8e2 Update mimemailparser from May 2011 version to 8.0.4
Summary:
Bump to version 8.0.4 from 2024-09-11 per https://github.com/php-mime-mail-parser/php-mime-mail-parser/releases before this ancient code copy falls apart.
`scripts/mail/mail_handler.php` (used for incoming (!) mail) is the only consumer.

Closes T15940

Test Plan: Feed `mail_handler.php` with various test emails (formats: plain text, HTML, multipart; encodings: UTF-8, ASCII, ISO-8859-something) by manually replacing `php://stdin` with corresponding text files and adding some `phlog`s for output checking as I don't have mail server glue handy. Get only expected errors for broken emails.

Reviewers: O1 Blessed Committers, 20after4

Reviewed By: O1 Blessed Committers, 20after4

Subscribers: 20after4, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15940

Differential Revision: https://we.phorge.it/D25829
2024-11-01 08:59:05 +01:00
Andre Klapper
644f179dd2 Make incoming mail handling more robust / correct
Summary:
* Properly handle when no mail headers at all can be parsed
* Properly handle when mail headers can be parsed but no subject line can be found

```
EXCEPTION: (RuntimeException) Undefined index: subject
```

Closes T15769

Test Plan: See T15769

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: 20after4, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15769

Differential Revision: https://we.phorge.it/D25565
2024-04-04 22:19:54 +02:00
Waldir Pimenta
a741f5d65c Change some instances of "phabricator" to "phorge"
Summary:
Just a small set of replacements in locations that seem innocuous (user-facing messages, documentation, etc.)

Ref T15006

Test Plan:
Nothing should change in terms of behavior. The places where these changes were made should now say "phorge".

Example tests:

- Manage a single User and click on Delete User and see the popup
- Run a test email and check the output
  ./bin/mail send-test --to username
- Visit /maniphest/, shift+click on at least 1 Task, click on Bulk Edit Selected,
  Continue, see the popup
- See the mentioned documentation with your big eyes. Eyes do not explode \o/

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15006

Differential Revision: https://we.phorge.it/D25473
2023-11-18 22:14:15 +00:00
Aviv Eyal
cb938d869c Teach ./bin/celerity about sprite maps
Summary:
Add `./bin/celerity sprites`, to replace script `./scripts/celerity/generate_sprites.php`.
Also make new workflow run `./bin/celerity map` at the same time.

Fixes T15437.

Test Plan: Changes a file that goes in the sprites, run new command

Reviewers: O1 Blessed Committers, speck

Reviewed By: O1 Blessed Committers, speck

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15437

Differential Revision: https://we.phorge.it/D25274
2023-06-03 09:57:05 -07:00
Valerio Bozzolan
a5c93dea56 Fix InvalidArgumentException on commit hook
Summary:
Fix a regression introduced here:

96ae4ba13a

I reproduced this exception executing "svn commit" on a hosted repository.

That crash happened because the PHP getenv() function can return false.
But, that is a very terrible value that blasts the non-string-empty check.

So, now the default getenv() value is skipped, without causing problems.

Closes T15253
Ref T15190

Test Plan: - I've run `svn commit` and I have not encountered any issue now

Reviewers: O1 Blessed Committers, avivey

Reviewed By: O1 Blessed Committers, avivey

Subscribers: speck, tobiaswiese, Matthew, Cigaryno

Maniphest Tasks: T15253, T15190

Differential Revision: https://we.phorge.it/D25122
2023-04-14 22:09:23 +02:00
Valerio Bozzolan
96ae4ba13a PHP 8.1: fixes for strlen() not accepting NULL anymore, part 2
Summary:
This is a fix for PHP 8.1 deprecation of strlen(NULL), for these Phorge components:

- scripts
- aphront
- project

The strlen() was used in Phabricator to check if a generic value was a non-empty string.
For this reason, Phorge adopts phutil_nonempty_string() that checks that.

Note: this may highlight other absurd input values that might be worth correcting
instead of just ignoring. If your phutil_nonempty_string() throws an exception, just
report it to Phorge to evaluate and fix together that specific corner case.

Closes T15223
Ref T15190
Ref T15064

Test Plan: - check with your big eyes that there are no obvious typos

Reviewers: O1 Blessed Committers, avivey

Reviewed By: O1 Blessed Committers, avivey

Subscribers: speck, tobiaswiese, Matthew, Cigaryno

Maniphest Tasks: T15223, T15190, T15064

Differential Revision: https://we.phorge.it/D25105
2023-04-01 15:20:13 +02:00
Aviv Eyal
c3f0c0b0f7 Merge Phacility/master into phorge 2022-07-25 11:50:26 -07:00
epriestley
039cbec155 Remove product literal strings in "pht()", part 11
Summary: Ref T13658.

Test Plan: Static checks only, none of these looked easily reachable.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13658

Differential Revision: https://secure.phabricator.com/D21777
2022-04-25 16:46:24 -07:00
epriestley
3e82ab5adb Remove product literal strings in "pht()", part 1
Summary: Ref T13658. I used the linter in D21763 to identify these and `split` them into arbitrary groups of 10 files.

Test Plan:
This test plan is non-exhaustive, because some of these strings are difficult to reach.

  - Looked at "Create Service" in Almanac.
  - Used "bin/auth" to go through a one-time auth workflow (not all related strings can be hit on a single workflow).
  - Started the "Generate Keypair" worfklow in "SSH Public Keys".

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13658

Differential Revision: https://secure.phabricator.com/D21765
2022-04-25 12:22:25 -07:00
epriestley
6136964093 Fix a PHP 8.1 strlen() issue with "mysql.pass" configuration
Summary:
Ref T13588. This configuration value may not be set.

Also fix an issue in `bin/storage` and whatever else I hit between now and this diff actually uploading.

Also fix a MySQLi report mode difference, beginning in PHP 8.1.

Also update a bunch of "static" property usage in Lisk.

Test Plan: Ran `bin/files ...` locally under PHP 8.1.

Maniphest Tasks: T13588

Differential Revision: https://secure.phabricator.com/D21744
2021-12-16 15:24:21 -08:00
epriestley
8103d36177 Remove "bin/celerity" dependency on database setup
Summary: Ref PHI2157. Like other low-level tools, "bin/celerity" does not need databases configured in order to execute.

Test Plan: Ran `bin/celerity map` with and without the database available.

Differential Revision: https://secure.phabricator.com/D21730
2021-11-19 10:11:06 -08:00
Leon Eckardt
fd1922c763 Update Install and Update Scripts to the new Paths and Repository links
Summary:
Set the Reposity links to actually install Phorge, Update Documentation links to the new Website.

Notes:
 - Github Mirrors are not set up. Use one anyway? Or just use the Repos at we.phorge.it?
 - Documentation Links still contain "phabricator". The Docs are changed, but the Diviner Books are not rebuilt. (Add a Cronjob or do it after every Commit?)
 - mysql-server is not included anymore in Debian environments. Should mariadb-server be used?
 - Should we print somewhere that Phorge is a Fork of Phabricator?

Test Plan:
Run the install script.
It should install Phorge and display Links to we.phore.it

Reviewers: O1 Blessed Committers, speck

Reviewed By: O1 Blessed Committers, speck

Subscribers: speck, tobiaswiese

Differential Revision: https://we.phorge.it/D25019
2021-09-04 15:48:49 -04:00
epriestley
af6cc0c934 Use "resolve()", not "execute()", for PhutilExecPassthru callsites in Phabricator
Summary: Ref T13660. Clean up callsites to "PhutilExecPassthru->execute()" to prepare to deprecate it.

Test Plan:
  - Grepped for "PhutilExecPassthru" and looked for callsites.
  - Ran `GIT_SSH=.../ssh-connect git ls-remote origin` to execute the "ssh-connect" code.
  - The two passthru future methods have no callers and could possibly be removed, but I'm just letting sleeping dogs lie for now.

Reviewers: cspeckmim

Reviewed By: cspeckmim

Maniphest Tasks: T13660

Differential Revision: https://secure.phabricator.com/D21703
2021-07-21 10:21:06 -07:00
epriestley
5521f76fe4 Remove some "Phacility" and "epriestley" references
Summary:
Ref T13658. This just scrubs some of the simple references from the codebase.

Most of what's left is in documentation which won't be relevant for a fork and/or which I need to separately revise (or more-or-less delete) at some point anyway.

I removed the "install RHEL" and "install Ubuntu" scripts outright since I don't have any reasonable way to test them and don't plan to maintain them.

Test Plan: Grepped for "phacility", "epriestley"; ran unit tests.

Reviewers: cspeckmim

Reviewed By: cspeckmim

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13658

Differential Revision: https://secure.phabricator.com/D21678
2021-07-08 10:46:17 -07:00
epriestley
12341e4bc8 Forbid disabled devices from authenticating via SSH or HTTP
Summary:
Ref T13641. Phabricator sometimes makes intracluster requests that authenticate as a device.

Forbid these requests from authenticating as a disabled device.

Test Plan:
  - Ran `bin/ssh-exec --phabricator-ssh-device ...` as an enabled/disabled device (worked; sensible error).
  - Made Conduit calls as an enable/disabled device (worked; sensible error).

Maniphest Tasks: T13641

Differential Revision: https://secure.phabricator.com/D21635
2021-03-16 15:51:51 -07:00
epriestley
42c0c0e3d2 Remove or correct various "phabricator/" references to "libphutil"
Summary:
Ref T13395. "libphutil/" was stripped for parts, but some documentation still references it. This is mostly minor corrections, but:

  - Removes "Javelin at Facebook", long obsolete.
  - Removes "php FPM warmup", which was always a prototype and is obsoleted by PHP preloading in recent PHP.

Test Plan: `grep` / reading

Maniphest Tasks: T13395

Differential Revision: https://secure.phabricator.com/D21624
2021-03-16 10:28:07 -07:00
epriestley
10162ad43b Support an SSH error log
Summary: Ref T13624. Depends on D21578. In "sshd" subprocess contexts, use "PhutilErrorLog" to direct errors to both stderr and, if configured, a logfile on disk.

Test Plan:
  - Confiugured an error log.
  - Forced `ssh-auth` to fatal.
  - Saw errors on stderr and in log.

Maniphest Tasks: T13624

Differential Revision: https://secure.phabricator.com/D21579
2021-02-26 14:54:54 -08:00
epriestley
3e38579fee Update install and upgrade documentation for libphutil
Summary:
Ref T13395. Libphutil has merged into Arcanist and no longer needs to be installed or upgraded. Additionally:

  - The minimum PHP version is now PHP 5.5.
  - Although older versions of PHP should still install APC, modern versions come with Opcache and do not need APC. Setup issues guide administrators thorugh the correct install procedure now.

Test Plan: Read documentation.

Maniphest Tasks: T13395

Differential Revision: https://secure.phabricator.com/D21550
2021-02-08 10:20:00 -08:00
epriestley
9ce1271805 Improve the quality of SSH error messages
Summary: See PHI1784. Currently, users who pass an invalid SSH command to Phabricator's SSH handler get an unhelpful error message. Make it more helpful.

Test Plan: Ran `./bin/ssh-exec` with no arguments (old, helpful error), invalid arguments (before: unhelpful error; after: helpful error), and valid arguments (old, helpful behavior).

Differential Revision: https://secure.phabricator.com/D21362
2020-06-16 08:59:36 -07:00
epriestley
356d9e8e19 Update a Phabricator -> Arcanist include path for scripts in Phabricator
Summary: Ref T13395. Since there's very little code which really makes sense in "scripts/", I've moved most of it to other places.

Test Plan: Ran `bin/phd`.

Maniphest Tasks: T13395

Differential Revision: https://secure.phabricator.com/D20994
2020-02-14 08:32:26 -08:00
epriestley
35a18146a2 Merge a small amount of remaining "libphutil/" code with Phabricator, break libphutil dependency
Summary: Ref T13395. Moves a small amount of remaining "libphutil/" code into "phabricator/" and stops us from loading "libphutil/".

Test Plan: Browsed around; there are likely remaining issues.

Maniphest Tasks: T13395

Differential Revision: https://secure.phabricator.com/D20981
2020-02-12 15:17:36 -08:00
epriestley
4a53fc339e Don't use "phutil_hashes_are_identical()" to compare public keys
Summary:
Ref T13436. There's no real security value to doing this comparison, it just wards off evil "security researchers" who get upset if you ever compare two strings with a non-constant-time algorithm.

In practice, SSH public keys are pretty long, pretty public, and have pretty similar lengths. This leads to a relatively large amount of work to do constant-time comparisons on them (we frequently can't abort early after identifying differing string length).

Test Plan: Ran `bin/ssh-auth --sshd-key ...` on `secure` with ~1K keys, saw runtime drop by ~50% (~400ms to ~200ms) with `===`.

Maniphest Tasks: T13436

Differential Revision: https://secure.phabricator.com/D20875
2019-10-28 18:34:30 -07:00
epriestley
24f771c1bc Add an optional "--sshd-key" argument to "bin/ssh-auth" for reading "%k" from modern sshd
Summary: Depends on D20873. Ref T13436. Allow callers to configure "bin/ssh-auth --sshd-key %k" as an "AuthorizedKeysCommand"; if they do, and we recognize the key, emit just that key in the output.

Test Plan:
  - Used `git pull` locally, still worked fine.
  - Instrumented things, saw the public key lookup actually work and emit a single key.
  - Ran without "--sshd-key", got a full key list as before.

Maniphest Tasks: T13436

Differential Revision: https://secure.phabricator.com/D20874
2019-10-28 17:57:03 -07:00
epriestley
02f85f03bd Remove the "ssh-auth-key" script
Summary:
Ref T13436. Historically, this script could be used with a forked copy of "sshd" to do lower-cost per-key auth.

Relatively modern "sshd" supports "%f" to "AuthorizedKeysCommand", which effectively moots this.

Users have never been instructed to use this script for anything, and we moved away from this specific patch to "sshd" some time ago.

Test Plan: Grepped for "ssh-auth-key", no hits.

Maniphest Tasks: T13436

Differential Revision: https://secure.phabricator.com/D20873
2019-10-28 17:52:37 -07:00
epriestley
64b399d9be Remove "bin/accountadmin" and "scripts/user/add_user.php"
Summary:
Fixes T13382. Depends on D20724. These ancient scripts are no longer necessary since we've had a smooth web-based onboarding process for a long time.

I retained `bin/user empower` and `bin/user enable` for recovering from situations where you accidentally delete or disable all administrators. This is normally difficult, but some users are industrious.

Test Plan: Grepped for `accountadmin` and `add_user.php`, found no more hits.

Maniphest Tasks: T13382

Differential Revision: https://secure.phabricator.com/D20725
2019-08-20 17:58:20 -07:00
epriestley
fc34554892 Replace "bin/people profileimage" with "bin/user enable|empower"
Summary:
Ref T13382.

  - Remove "bin/people profileimage" which previously generated profile image caches but now feels obsolete.
  - Replace it with "bin/user", with "enable" and "empower" flows. This command is now focused on regaining access to an install after you lock your keys inside.
  - Document the various ways to unlock objects and accounts from the CLI.

Test Plan:
  - Ran `bin/user enable` and `bin/user empower` with various flags.
  - Grepped for `people profileimage` and found no references.
  - Grepped for `bin/people` and found no references.
  - Read documentation.

Maniphest Tasks: T13382

Differential Revision: https://secure.phabricator.com/D20724
2019-08-20 17:51:14 -07:00
epriestley
87b0ef8839 Remove "iconv" PHP extension dependency
Summary: Depends on D20069. Ref T13232. This is a very, very weak dependency and we can reasonably polyfill it.

Test Plan: Grepped for `iconv` in libphutil, arcanist, and Phabricator.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13232

Differential Revision: https://secure.phabricator.com/D20070
2019-01-30 19:46:58 -08:00
Austin McKinley
c756bf3476 Fix bin/accountadmin when not making changes
Summary: If you go through the `accountadmin` flow and change nothing, you get an exception about the transaction not having any effect. Instead, let the `applyTransactions` call continue even on no effect.

Test Plan: Ran `accountadmin` without changing anything for an existing user. No longer got an exception about no-effect transactions.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D20009
2019-01-21 12:40:59 -08:00
epriestley
5537e29ee8 Move "Welcome" mail generation out of PhabricatorUser
Summary:
Ref PHI1027. Currently, `PhabricatorUser` has a couple of mail-related methods which shouldn't really be there in the long term. Immediately, I want to make some adjusments to the welcome email.

Move "Welcome" mail generation to a separate class and consolidate all the error handling. (Eventually, "invite" and "verify address" email should move to similar subclasses, too.) Previously, a bunch of errors/conditions got checked in multiple places.

The only functional change is that we no longer allow you to send welcome mail to disabled users.

Test Plan:
  - Used "Send Welcome Mail" from profile pages to send mail.
  - Hit "not admin", "disabled user", "bot/mailing list" errors.
  - Used `scripts/user/add_user.php` to send welcome mail.
  - Used "Create New User" to send welcome mail.
  - Verified mail with `bin/mail show-outbound`. (Cleaned up a couple of minor display issues here.)

Reviewers: amckinley

Reviewed By: amckinley

Differential Revision: https://secure.phabricator.com/D19989
2019-01-18 19:50:35 -08:00
Marc Lindenberg
360ebce00f call qsprintf() without warnings
Summary:
After T13217 import_repository_symbols.php was showing a lot of warnings, using %LQ fixes that.
I'm aware, that there are changes planned to the whole managing the symbols complex but until then less warnings are nice.

Test Plan: No more warnings when updating symbols

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D19962
2019-01-14 09:41:31 -08:00
epriestley
cfcd35d8a3 Remove standalone SMS support in favor of a "Mail, SMS, and other media are mostly the same thing" approach
Summary:
Ref T920. Over time, mail has become much more complex and I think considering "mail", "sms", "postcards", "whatsapp", etc., to be mostly-the-same is now a more promising avenue than building separate stacks for each one.

Throw away all the standalone SMS code, including the Twilio config options. I have a separate diff that adds Twilio as a mail adapter and functions correctly, but it needs some more work to bring upstream.

This permanently destroys the `sms` table, which no real reachable code ever wrote to. I'll call this out in the changelog.

Test Plan:
  - Grepped for `SMS` and `Twilio`.
  - Ran storage upgrade.

Reviewers: amckinley

Reviewed By: amckinley

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T920

Differential Revision: https://secure.phabricator.com/D19939
2019-01-03 04:05:20 -08:00
Austin McKinley
979187132d Update accountadmin to use new admin empowerment code
Summary: Fixes https://discourse.phabricator-community.org/t/admin-account-creation-fails-call-to-undefined-method-phabricatorusereditor-makeadminuser/2227. This callsite got skipped when updating the EmpowerController to use the new transactional admin approval code.

Test Plan: Invoked `accountadmin` to promote a user, no longer got an exception.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D19915
2018-12-19 12:00:53 -08:00
epriestley
533e4e13b3 Add a bin/herald test ... for doing test runs via the CLI
Summary: Ref T13216. See D19666. It's currently tricky to profile Herald test runs since you have to submit a form and repeating them is a bit of a mess. Provide a simple CLI wrapper so we can use `--xprofile`. This is also maybe nice-to-have if we're ever debugging anything here.

Test Plan: Ran `bin/herald test --object ... --type ...` and got a sensible looking transcript in the UI.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13216

Differential Revision: https://secure.phabricator.com/D19806
2018-11-15 15:48:52 -08:00
epriestley
c32fa06266 Use phutil_microseconds_since(...) to simplify some timing arithmetic
Summary: Depends on D19796. Simplify some timing code by using phutil_microseconds_since() instead of duplicate casting and arithmetic.

Test Plan: Grepped for `1000000` to find these. Pulled, pushed, made a conduit call. This isn't exhaustive but it should be hard for these to break in a bad way since they're all just diagnostic.

Reviewers: amckinley

Reviewed By: amckinley

Differential Revision: https://secure.phabricator.com/D19797
2018-11-08 16:46:32 -08:00
epriestley
b12e92e6e2 Add timing information for commit hooks to push logs
Summary:
Depends on D19779. Ref T13216. The push logs currently record the "hostWait", which is roughly "locking + subprocess cost". We also record locking separately, so we can figure out "subprocess cost" alone by subtracting the lock costs.

However, the subprocess (normally `git receive-pack`) runs hooks, and we don't have an easy way to figure out how much time was spent doing actual `git` stuff vs spent doing commit hook processing. This would have been useful in diagnosing at least one recent issue.

Track at least a rough hook cost and record it in the push logs.

Test Plan: Pushed to a repository, saw a reasonable hook cost appear in the database table.

Reviewers: amckinley

Reviewed By: amckinley

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13216

Differential Revision: https://secure.phabricator.com/D19780
2018-11-08 06:00:26 -08:00
epriestley
bbfc860c63 Improve aesthetics of commit hook rejection message
Summary: See PHI939. Ref T13216. Make the dragon's companion animal more clearly cow-like.

Test Plan:
Before:

```
\     \__/
 \____(Oo)
 (    (--)
 //__\\
//    \\
```

After:

```
*     \__/
 \____(Oo)
 (    (..)
 //___\\
//     \\
```

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13216

Differential Revision: https://secure.phabricator.com/D19775
2018-11-06 09:39:48 -08:00
epriestley
7967fab5cc Remove outdated "rebuild_summaries.php" script
Summary:
Ref T13195. See PHI842. Alternative to D19638.

Instead of doing all the stuff in D19638, //just// remove the `rebuild_summaries.php` script. This script is outdated, copy/pastes the rebuild logic, and doesn't understand unreachable commits.

If we had some use for it it should move to `bin/repository rebuild-summary ...` or similar, but it's not clear there's any use for it. The incremental summary rebuilds seem to work fine as-is.

Test Plan: Grepped for callers or documentation referencing this script, found nothing.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13195

Differential Revision: https://secure.phabricator.com/D19643
2018-09-06 08:18:47 -07:00
Joshua Spence
ba25586016 Improve symbol generation scripts
Summary: Currently the symbol generation scripts fail if passed a list containing no files because `explode("\n", $input)` returns `array("")` rather than `array()`. This means that a generic Harbormaster Build Plan with a step which executes `find . -type f -name '*.php' | ./scripts/generate_php_symbols.php` won't work because it fails in repositories that don't contain any PHP code.

Test Plan: Ran `echo | generate_php_symbols` and saw no output instead of an exception.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D19588
2018-08-16 06:40:13 +10:00
epriestley
6df278bea8 In "bin/ssh-auth", cache a structure instead of a flat file because paths may change at runtime
Summary:
Fixes T12397. Ref T13164. See PHI801.

Several installs have hit various use cases where the path on disk where Phabricator lives changes at runtime. Currently, `bin/ssh-auth` caches a flat file which includes the path to `bin/ssh-exec`, so this may fall out of date if `phabricator/` moves.

These use cases have varying strengths of legitimacy, but "we're migrating to a new set of hosts and the pool is half old machines and half new machines" seems reasonably compelling and not a problem entirely of one's own making.

Test Plan:
  - Compared output on `master` to output after change, found them byte-for-byte identical.
  - Moved `phabricator/` to `phabricator2/`, ran `bin/ssh-auth`, got updated output.
  - Added a new SSH key, saw it appear in the output.
  - Grepped for `AUTHFILE_CACHEKEY` (no hits).
  - Dropped the cache, verified that the file regenerates cleanly.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13164, T12397

Differential Revision: https://secure.phabricator.com/D19568
2018-08-09 13:33:23 -07:00
Arturas Moskvinas
4c09e88c95 Add parsing for ssh options (-o) which are passed when using GIT v2 wire protocol by git command (SSH transport)
Summary:
Makes `ssh-connect` compatible with Git v2 wire protocol over SSH

More details about git V2 wire: https://opensource.googleblog.com/2018/05/introducing-git-protocol-version-2.html

`git` command (2.18+) passes extra options (`-o "SendEnv GIT_PROTOCOL"`) to underlying `ssh` command to enable v2 wire protocol (environment variable enabling new protocol).

Phabricator `ssh-connect` command doesn't understand `-o` options and interprets it as host parts hence when you enable git v2 all clones/ls-remotes crash with:
```
#0 ExecFuture::resolvex() called at [<phabricator>/src/applications/repository/storage/PhabricatorRepository.php:525]
#1 PhabricatorRepository::execxRemoteCommand(string, PhutilOpaqueEnvelope) called at [<phabricator>/src/applications/repository/engine/PhabricatorRepositoryPullEngine.php:400]
#2 PhabricatorRepositoryPullEngine::loadGitRemoteRefs(PhabricatorRepository) called at [<phabricator>/src/applications/repository/engine/PhabricatorRepositoryPullEngine.php:343]
#3 PhabricatorRepositoryPullEngine::executeGitUpdate() called at [<phabricator>/src/applications/repository/engine/PhabricatorRepositoryPullEngine.php:126]
#4 PhabricatorRepositoryPullEngine::pullRepositoryWithLock() called at [<phabricator>/src/applications/repository/engine/PhabricatorRepositoryPullEngine.php:40]
#5 PhabricatorRepositoryPullEngine::pullRepository() called at [<phabricator>/src/applications/repository/management/PhabricatorRepositoryManagementUpdateWorkflow.php:59]
#6 PhabricatorRepositoryManagementUpdateWorkflow::execute(PhutilArgumentParser) called at [<phutil>/src/parser/argument/PhutilArgumentParser.php:441]
#7 PhutilArgumentParser::parseWorkflowsFull(array) called at [<phutil>/src/parser/argument/PhutilArgumentParser.php:333]
#8 PhutilArgumentParser::parseWorkflows(array) called at [<phabricator>/scripts/repository/manage_repositories.php:22]
COMMAND
git ls-remote '********'
STDOUT
(empty)
STDERR
ssh: Could not resolve hostname -o: Name or service not known
fatal: Could not read from remote repository.
Please make sure you have the correct access rights
and the repository exists.
at [<phutil>/src/future/exec/ExecFuture.php:369]
```

Test Plan:
How to reproduce:
1. add repository to Phabricator which is accessed via `ssh`
2. Use git 2.18+
3. Enable wire protocol in `/etc/gitconfig`:
```
[protocol]
    version = 2
```
4. Try refreshing repository: `phabricator/bin/repository update somecallsing`
5. Repository update fails with `ssh: Could not resolve hostname -o: Name or service not known`

after this changes - updates will succeed

Reviewers: epriestley, Pawka, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D19542
2018-08-02 16:42:59 +03:00
Aviv Eyal
15e6e2adea Update install_ubuntu.sh to the new age
Summary:
Ref T4200. Since the last time this script was written, Ubuntu has made lots of changes.
Try to keep up with those.

Test Plan:
Ran this on frash installs of Ubuntu 16.04 LTS and 18.04 LTS (Pre-release).
Got to see Phabricator running.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: amckinley, Korvin

Maniphest Tasks: T4200

Differential Revision: https://secure.phabricator.com/D19394
2018-04-21 09:57:33 -07:00
epriestley
69bff489d4 Generate a random unique "Request ID" for SSH requests so processes can coordinate better
Summary:
Depends on D19247. Ref T13109. When we receive an SSH request, generate a random unique ID for the request. Then thread it down through the process tree.

The immediate goal is to let the `ssh-exec` process coordinate with `commit-hook` process and log information about read and write lock wait times. Today, there's no way for `ssh-exec` to interact with the `PushEvent`, but this is the most helpful place to store this data for users.

Test Plan: Made pushes, saw the `PushEvent` table populate with a random request ID. Exported data and saw the ID preserved in the export.

Maniphest Tasks: T13109

Differential Revision: https://secure.phabricator.com/D19249
2018-03-22 13:44:30 -07:00
epriestley
0bf8e33bb6 Issue setup guidance recommending MySQLi and MySQL Native Driver
Summary:
Fixes T12994. We need `MYSQLI_ASYNC` to implement client-side query timeouts, and we need MySQLi + MySQL Native Driver to get `MYSQLI_ASYNC`.

Recommend users install MySQLi and MySQL Native Driver if they don't have them. These are generally the defaults and best practice anyway, but Ubuntu makes it easy to use the older stuff.

All the cases we're currently aware of stem from `apt-get install php5-mysql` (which explicitly selects the non-native driver) so issue particular guidance about `php5-mysqlnd`.

Test Plan:
  - Faked both issues locally, reviewed the text.
  - Will deploy to `secure`, which currently has the non-native driver.

Maniphest Tasks: T12994

Differential Revision: https://secure.phabricator.com/D19216
2018-03-13 12:38:09 -07:00
epriestley
44f0664d2c Add a "lock log" for debugging where locks are being held
Summary: Depends on D19173. Ref T13096. Adds an optional, disabled-by-default lock log to make it easier to figure out what is acquiring and holding locks.

Test Plan: Ran `bin/lock log --enable`, `--disable`, `--name`, etc. Saw sensible-looking output with log enabled and daemons restarted. Saw no additional output with log disabled and daemons restarted.

Maniphest Tasks: T13096

Differential Revision: https://secure.phabricator.com/D19174
2018-03-05 17:55:34 -08:00
epriestley
07028cfc30 When bin/drydock lease is interrupted, release leases
Summary:
Depends on D19072. Ref T13073. Currently, you can leave leases stranded by using `^C` to interrupt the script. Handle signals and release leases on destruction if they haven't activated yet.

Also, print out more useful information before and after activation.

Test Plan: Mashed ^C while runnning `bin/drydock lease ... --trace`, saw the lease release.

Subscribers: yelirekim, PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13073

Differential Revision: https://secure.phabricator.com/D19073
2018-02-13 13:14:21 -08:00
epriestley
0470125d9e Add skeleton code for webhooks
Summary: Ref T11330. Adds general support for webhooks. This is still rough and missing a lot of pieces -- and not yet useful for anything -- but can make HTTP requests.

Test Plan: Used `bin/webhook call ...` to complete requests to a test endpoint.

Maniphest Tasks: T11330

Differential Revision: https://secure.phabricator.com/D19045
2018-02-09 13:55:04 -08:00
epriestley
956c4058e6 Add a bin/conduit call support binary
Summary:
Ref T13060. See PHI343. Triaging this bug required figuring out where in the pipeline UTF8 was being dropped, and bisecting the pipeline required making calls to Conduit.

Currently, there's no easy way to debug/inspect arbitrary Conduit calls, especially when they are `diffusion.*` calls which route to a different host (even if you have a real session and use the web console for these, you just see an HTTP service call to the target host in DarkConsole).

Add a `bin/conduit` utility to make this kind of debugging easier, with an eye toward the Phacility production cluster (or other similar clusters) specifically.

Test Plan:
  - Ran `echo '{}' | bin/conduit call --method conduit.ping --input -` and similar.
  - Used a similar approach to successfully diagnose the UTF8 issue in T13060.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13060

Differential Revision: https://secure.phabricator.com/D18987
2018-02-05 12:20:49 -08:00
epriestley
2914613444 Fix failure to record pullerPHID in repository pull logs
Summary:
See PHI305. Ref T13046.

The SSH workflows currently extend `PhabricatorManagementWorkflow` to benefit from sharing all the standard argument parsing code. Sharing the parsing code is good, but it also means they inherit a `getViewer()` method which returns the ommnipotent viewer.

This is appropriate for everything else which extends `ManagementWorkflow` (like `bin/storage`, `bin/auth`, etc.) but not appropriate for SSH workflows, which have a real user.

This caused a bug with the pull logs where `pullerPHID` was not recorded properly. We used `$this->getViewer()->getPHID()` but the correct code was `$this->getUser()->getPHID()`.

To harden this against future mistakes:

  - Don't extend `ManagementWorkflow`. Extend `PhutilArgumentWorkflow` instead. We **only** want the argument parsing code.
  - Rename `get/setUser()` to `get/setSSHUser()` to make them explicit.

Then, fix the pull log bug by calling `getSSHUser()` instead of `getViewer()`.

Test Plan:
  - Pulled and pushed to a repository over SSH.
  - Grepped all the SSH stuff for the altered symbols.
  -  Saw pulls record a valid `pullerPHID` in the pull log.
  - Used `echo {} | ssh ... conduit conduit.ping` to test conduit over SSH.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13046

Differential Revision: https://secure.phabricator.com/D18912
2018-01-23 14:09:42 -08:00
epriestley
aa3b582c7b Remove "set password" from bin/accountadmin and let bin/auth recover recover anyone
Summary:
Ref T13043. This cleans some things up to prepare for moving account passwords to shared infrastructure.

Currently, the (very old, fairly unusual) `bin/accountadmin` tool can set account passwords. This is a bit weird, generally not great, and makes upgrading to shared infrastructure more difficult. Just get rid of this to simplify things. Many installs don't have passwords and this is pointless and unhelpful in those cases.

Instead, let `bin/auth recover` recover any account, not just administrator accounts. This was a guardrail against administrative abuse, but it has always seemed especially flimsy (since anyone who can run the tool can easily comment out the checks) and I use this tool in cluster support with some frequency, occasionally just commenting out the checks. This is generally a better solution than actually setting a password on accounts anyway. Just get rid of the check and give users enough rope to shoot themselves in the foot with if they truly desire.

Test Plan:
  - Ran `bin/accountadmin`, didn't get prompted to swap passwords anymore.
  - Ran `bin/auth recover` to recover a non-admin account.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13043

Differential Revision: https://secure.phabricator.com/D18901
2018-01-23 10:58:11 -08:00
epriestley
3038d564a6 Allow bulk edits to be made silently if you have CLI access
Summary:
Fixes T13042. This hooks up the new "silent" mode from D18882 and makes it actually work.

The UI (where we tell you to go run some command and then reload the page) is pretty clumsy, but should solve some problems for now and can be cleaned up eventually. The actual mechanics (timeline aggregation, Herald interaction,  etc.) are on firmer ground.

Test Plan:
  - Made a normal bulk edit, got mail and feed stories.
  - Made a silent bulk edit, no mail and no feed.
  - Saw "Silent Edit" marker in timeline for silent edits:

{F5386245}

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13042

Differential Revision: https://secure.phabricator.com/D18883
2018-01-19 13:24:54 -08:00