1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-10 08:52:39 +01:00
Commit graph

14655 commits

Author SHA1 Message Date
Chad Little
8e680f17a9 Add Home menu default Dashboard documentation
Summary: From Z1336, we don't currently document anywhere how the default dashboard works. I should also update the copy in the UI. Ref T12969

Test Plan: regenerate docs, read carefully

Reviewers: epriestley

Reviewed By: epriestley

Spies: Korvin

Maniphest Tasks: T12969

Differential Revision: https://secure.phabricator.com/D18454
2017-08-23 09:28:20 -07:00
epriestley
df21391b8e When "apcu_clear_cache()" exists, prefer it as a cache clear callback over "apc_clear_cache()"
Summary:
See PHI36. APCu originally had `apc_` methods, but at some point dropped these and only provides `apcu_` methods.

When the `apcu_` method is present, use it. It may not be present for older versions of APCu, so keep the fallback.

Test Plan:
  - With modern APCu, clicked "Purge Caches" in Config > Caches.
  - Before: fatal on bad `apc_clear_caches` call.
  - After: Valid cache clear.

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D18449
2017-08-21 15:32:44 -07:00
Chad Little
e40c002a6d Add a basic view count to Phame
Summary: This adds a very very basic view count to Phame, so bloggers can get some idea which posts are more popular than others. Anything more than this I think should be Facts or Google Analytics.

Test Plan: Write a new post, see post count. Reload page, post count goes up. Archive post, post count stays the same.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D18446
2017-08-21 14:03:21 -07:00
Chad Little
39d19c33ec Redirect users back to where they added an SSH Key
Summary: Ref T12964. This feels like a cheat, but works well. Just redirect the user back to the form they came from instead of to the key page.

Test Plan: Add a key to a user profile, add a key to an Alamanac device. Grep for PhabricatorAuthSSHKeyTableView and check all locations.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12964

Differential Revision: https://secure.phabricator.com/D18445
2017-08-21 14:02:27 -07:00
Chad Little
a145d00be6 Update Diffusion File UI for single column
Summary: Moves browseFile to single column, implements Owners as a list under the file (and now directory as well), improved information listed in Owners, and moves actions into the Diffusion action bar instead of the header.

Test Plan:
Test browsing directories, files, text, images, binaries, enabling blame. Mobile and desktop.

{F5111045}

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D18448
2017-08-21 13:35:25 -07:00
Chad Little
d2a3f2da73 Add indication of hg branch open/closed in branch list
Summary: Adds some basic UI for open / closed state when viewing a list of branches in Mercurial. Fixes T12838

Test Plan: Close and open branches, view list.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12838

Differential Revision: https://secure.phabricator.com/D18447
2017-08-21 09:09:16 -07:00
Chad Little
295c806219 Hide branch status if repository is not hg
Summary: Better table layouts here for branches view

Test Plan: Test git, hg repositories. See column go away.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D18444
2017-08-17 14:44:55 -07:00
Chad Little
60df67af53 Give mobile page tabs more space
Summary: These can wrap on iPhone 5 screens, no need for so much padding.

Test Plan: iPhone 5 simulator, tablet and mobile.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D18437
2017-08-17 12:16:31 -07:00
Chad Little
864dd9a196 List branch on main repository view
Summary: This is in the crumbs, but a little hidden. Puts branch name at the top of the browse table header.

Test Plan: Review a few branchs, change branch, see new name.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D18441
2017-08-17 12:15:53 -07:00
Chad Little
281fc19f3f Add more information to Branch status page in Manage Repository
Summary: Adds an icon for default branch, status for branch status

Test Plan: Review `hg` and `git` repositories, change default branch, etc.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D18443
2017-08-17 12:09:20 -07:00
epriestley
68008dce60 Fix a possible database ref fatal during MySQL setup checks if a host is unreachable
Summary:
Ref T12966. See that task for a description and reproduction steps.

If you put Phabricator in a master/replica configuration and then restart it, we may fatal here if the master is unreachable. Instead, we should survive setup checks.

Test Plan: Put Phabricator in a master/replica configuration, explicitly disabled the master by misconfiguring the port, restarted Phabricator. Before: fatal; after: login screen in read-only mode.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12966

Differential Revision: https://secure.phabricator.com/D18442
2017-08-17 11:43:13 -07:00
epriestley
c9986fd5de Don't fatal in ElasticSearch setup check if no "master" database is configured
Summary:
Ref T12965. See that task for discussion, and PHI36 for context.

This sweeps the fatal under the rug by skipping it, letting things move forward for now.

Test Plan: Followed instructions in T12965, got a read-only recovery after restart instead of a fatal.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12965

Differential Revision: https://secure.phabricator.com/D18440
2017-08-17 10:39:00 -07:00
Chad Little
053cab4d59 Update VCS Password settings page
Summary: Use proper background.

Test Plan: Visit page, see correct background.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D18435
2017-08-17 08:51:38 -07:00
Chad Little
dc10bb1f49 Update Settings to use TwoColumn fixed layout
Summary: Simplifies the page, adds base support for PHUITwoColumn fixed from Instances (which I'll delete css there).

Test Plan:
click on every settings page, UI seems in tact, check mobile, desktop, mobile menus.

{F5102572}

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D18436
2017-08-17 08:51:17 -07:00
Chad Little
5019960b61 Set border on crumbs on Lint page
Summary: Minor, sets the border, corrects a page header.

Test Plan: View lint pages

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D18433
2017-08-16 12:19:35 -07:00
Chad Little
19dae88728 Add branch, tag info to Diffusion Headers
Summary: Improves overall UX of browsing Diffusion. Clarifies branch and tag when possible, changes 'home' to 'code', uses tabs in more locations. Fixes T12837

Test Plan: Review branchs, tags, git, hg, search, browse, history.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12837

Differential Revision: https://secure.phabricator.com/D18434
2017-08-16 12:16:15 -07:00
Chad Little
7bbd26427f Add pattern search to diffusion home
Summary: Moves the method up to DiffusionController, so it can be more universally used. Also now center aligns tabs on mobile. Still todo, get search nicely toggled on mobile

Test Plan: Test mobile, desktop. Test search from home, from browse, and browsing a specific path.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D18432
2017-08-15 14:16:33 -07:00
Chad Little
f4fdb92e13 Move Diffusion Actions into action bar on home
Summary: Moving this down the the "bar" to allow pattern search on home. Rebuilds the mobile layout a little.

Test Plan:
Test actions on mobile, desktop, tablet.

{F5100460}

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D18431
2017-08-15 12:19:49 -07:00
Chad Little
3a50ea4f47 Simplify Create Repository page
Summary: Also adds images, nice images.

Test Plan: Create a repository, test mobile, tablet, desktop layouts.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D18430
2017-08-15 11:05:50 -07:00
Chad Little
4d335b7bef Build a basic DiffusionPatternSearchView
Summary: Roughs this in a little, kinda basic. Allows for grouping results by page. A bit better on mobile. Would like more content return from conduit though.

Test Plan:
Test `CMS`, `cms`, and `OMGLOLWTFBBQ`, desktop and mobile

{F5099081}

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D18429
2017-08-15 06:38:47 -07:00
Chad Little
0a9ad6d5e7 Move pattern search into Diffusion header
Summary: This is only on browse pages, but I think could be global (home) also. Moves it from a button, field, to just a field.

Test Plan:
Review search on desktop, mobile.

{F5098886}

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D18428
2017-08-14 19:03:56 -07:00
Chad Little
37489865d4 Remove "File Name" search tool
Summary: Removing this cleanly in event we want to put it back later. 99% of these cases are likely workable either by command line or the typeahead. Will gauge feedback if users notice.

Test Plan: Reload page, perform file grep search.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D18425
2017-08-14 18:58:25 +00:00
epriestley
766ecf8d6b Update "Reporting Bugs" documentation for removal of free instances
Summary: I wrote "free" since "test" only give us 7 days to confirm the issue, but "free" no longer exists and "test" should be good enough.

Test Plan: o_O

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D18423
2017-08-14 11:28:30 -07:00
Chad Little
bd1d22baa2 Touch up typeahead behavior in locate-file
Summary: Uses `fire`, `underline`. Sets text that overflows to ellipsis.

Test Plan:
Test searching for CMS in Phabricator. Check other typeaheads, tokenizers.

{F5098496}

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D18424
2017-08-14 11:28:13 -07:00
Chad Little
07c0032491 Add a link directly to Browse in Diffusion
Summary: Getting to the straight browse view went away, this adds a link back. I'll look at more long term solution for getting to grep search.

Test Plan: Click on header, get take to browse view.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D18421
2017-08-14 11:14:14 -07:00
epriestley
8c3243ef68 Lightly modernize NamedQueryQuery
Summary: Ref T12956. No real behavioral changes here, just slightly more modern code.

Test Plan: Reviewed named queries in Maniphest and "Edit Queries...".

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12956

Differential Revision: https://secure.phabricator.com/D18420
2017-08-14 09:07:11 -07:00
epriestley
48a74de0b6 Move all revision status transactions to modern values and mechanics
Summary:
Ref T2543. This updates and migrates the status change transactions:

  - All storage now records the modern modular transaction ("differential.revision.status"), not the obsolete non-modular transaction ("differential:status").
  - All storage now records the modern constants ("accepted"), not the obsolete numeric values ("2").

Test Plan:
  - Selected all the relevant rows before/after migration, data looked sane.
  - Browsed around, reviewed timelines, no changes after migration.
  - Changed revision states, saw appropriate new transactions in the database and timeline rendering.
  - Grepped for `differential:status`.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T2543

Differential Revision: https://secure.phabricator.com/D18419
2017-08-12 04:05:57 -07:00
epriestley
7b695aa43b Migrate revision storage to modern status constants ("accepted") instead of legacy numeric values ("2")
Summary:
Ref T2543. Rewrites all the storage to use constants.

Note that transactions still use legacy values, I'll migrate and update them separately.

Test Plan:
  - Ran migration.
  - Browsed around, changed revision states, viewed dashboard, etc.
  - Selected `DISTINCT()` and `GROUP_CONCAT()` of the `status` field in the database, saw sane/expected before and after values.
  - Verified that old Conduit methods still return numeric constants for compatibility.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T2543

Differential Revision: https://secure.phabricator.com/D18418
2017-08-12 04:02:10 -07:00
epriestley
5348f34c9e Make all revision status readers explicitly read modern or legacy status
Summary: Ref T2543. All writers now write modern statuses. Make all readers explicit about whether they are reading modern or legacy statuses, so I can swap the storage format.

Test Plan:
  - Grepped for `getStatus()`, scanned the list. Other applications have methods with this name so it's possible I missed something.
  - Browed around, changed revision statuses.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T2543

Differential Revision: https://secure.phabricator.com/D18417
2017-08-11 17:22:22 -07:00
epriestley
0b1d6a3f6e Convert straggling Herald rules to modern revision status constants
Summary:
Ref T2543. These are the last `ArcanistDifferentialRevisionStatus` callsites.

This removes the very old legacy `precommitRevisionStatus` field, which has no other readers. This was obsoleted by the `CLOSED_FROM_ACCEPTED` stuff, but retained for compatibility.

Test Plan:
  - Poked these with the test console, although they're a little tricky to be sure about.
  - Grepped for `ArcanistDifferentialRevisionStatus`, no more hits.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T2543

Differential Revision: https://secure.phabricator.com/D18416
2017-08-11 17:22:05 -07:00
epriestley
cd15c2d545 Swap transactions and initialization over to modern status constants
Summary: Ref T2543. Update these for the modern stuff.

Test Plan: Created a new revision, got a revision in the right state ("Needs Review"). Accepted, planned, requested, abandoned revision; state transitions looked good.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T2543

Differential Revision: https://secure.phabricator.com/D18415
2017-08-11 17:21:51 -07:00
epriestley
895f0cde1f Use modern revision statuses when bucketing revisions on the Differential dashboard
Summary: Ref T2543. Swaps these over to modern constants.

Test Plan: Viewed dashboard, no chagnes to bucketing.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T2543

Differential Revision: https://secure.phabricator.com/D18414
2017-08-11 17:21:27 -07:00
epriestley
7f743c14d5 Remove remaining ArcanistDifferentialRevisionStatus references in revision state logic
Summary: Ref T2543. This cleans up all the "when no one is rejecting/blocking and someone accepted, mark the revision overall as accepted" logic to use more modern status stuff instead of `ArcanistDifferentialRevisionStatus`.

Test Plan:
  - Updated revisions, saw them go to "Needs Review".
  - Accepted, requested changes to revisions.
  - Updated one with changes requested, saw it go to "needs review" again.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T2543

Differential Revision: https://secure.phabricator.com/D18413
2017-08-11 17:21:09 -07:00
epriestley
2b9838b482 Modularize remaining TYPE_ACTION transactions in Differential, reducing calls to ArcanistDifferentialRevisionStatus
Summary:
Ref T2543. This cleans up a couple of remaining rough edges:

  - We could do an older TYPE_ACTION "close" via the daemons.
  - We could do an older TYPE_ACTION "close" via `arc close-revision`, explicitly or implicitly in `arc land`, via API (`differential.close`).
  - We could do an older TYPE_ACTION "rethink" ("Plan Changes") via the API, via `arc diff --plan-changes` (`differential.createcomment`).

Move these to modern modular transactions, then get rid of all the validation and application logic for them. This nukes a bunch of `ArcanistDifferentialRevision::...` junk.

Test Plan:
  - Used `bin/repository reparse --message rXYZ...` to reparse a commit, closing a corresponding revision.
  - Used `differential.close` to close a revision.
  - Used `differential.createcomment` to plan changes to a revision.
  - Reviewed transaction log for full "closed by commit" message (linking to commit and mentioning author).
  - Grepped for `::TYPE_ACTION` to look for remaining callsites, didn't find any.
  - Grepped for `differential.close` and `differential.createcomment` in `arcanist/` to look for anything suspicious, seemed clean.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T2543

Differential Revision: https://secure.phabricator.com/D18412
2017-08-11 17:20:55 -07:00
epriestley
19bc91fd20 Modularize the Differential "status" transaction and move away from ArcanistDifferentialRevisionStatus
Summary:
Ref T2543. Converts the TYPE_STATUS transaction (used to render "This revision now requires changes to proceed.", "This revision is accepted and ready to land.", etc) to ModularTransactions.

Also, continue consolidating all the status-related information (here, more colors and icons) into a single place. By the end of this, we may learn that NEEDS_REVIEW uses //every// color.

Test Plan:
Reviewed old status transactions (unchanged) and created new ones (looked the same as the old ones).

(I plan to migrate all of these a few diffs from now, around when I change the storage format.)

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T2543

Differential Revision: https://secure.phabricator.com/D18410
2017-08-11 17:20:40 -07:00
Chad Little
3b14c1fdd1 Update AphlictClientServer to support ws2 or ws3
Summary: This lets us support either ws2 or ws3. Fixes T12755

Test Plan: Update server to version 3, send message, watch debug log. Downgrade to 2.x, send messages, watch debug log. Everything seems OK

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12755

Differential Revision: https://secure.phabricator.com/D18411
2017-08-11 15:17:07 -07:00
epriestley
77bf245637 Continue reducing callsites to ArcanistDifferentialRevisionStatus in transactions
Summary: Ref T2543. Cleans up some more references to ArcanistDifferentialRevisionStatus, moving toward getting rid of it completely.

Test Plan: Planned changes, requested review, inspected the "close" one since it isn't trivial to trigger.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T2543

Differential Revision: https://secure.phabricator.com/D18408
2017-08-11 13:43:21 -07:00
epriestley
36197bf783 Provide revision status information via API all "differential.revision.search"
Summary: Ref T2543. Now that the integer status constants are banished to the internals, we can expose status information from "differential.revision.search".

Test Plan:
Searched for revisions.

{F5093873}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T2543

Differential Revision: https://secure.phabricator.com/D18400
2017-08-11 13:42:45 -07:00
epriestley
ef8d4e2126 Fix an inverted condition for the "Reopen Revision" action
Summary: Ref T2543. I converted this condition the wrong way, missing a `!`. I'll cherry-pick this to `stable`.

Test Plan: No more "Reopen Revision" action available on open revisions.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T2543

Differential Revision: https://secure.phabricator.com/D18399
2017-08-11 13:41:54 -07:00
epriestley
153e4d8a38 Remove old reviewer double writes to legacy edge table in Differential
Summary:
Ref T2543. Ref T10967. This isn't precisely related to "draft" status, but while I'm churning this stuff anyway, get rid of the old double writes to clean the code up a bit.

These were added in T10967 to make sure the migration was reversible/recoverable, but we haven't seen any issues with it in several months so I believe they can now be removed safely. Nothing has read this table since ~April.

Test Plan: Took various review actions on revisions (accept, reject, resign, comment, etc). If this change is correct, there should be no visible effect.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10967, T2543

Differential Revision: https://secure.phabricator.com/D18398
2017-08-11 13:38:52 -07:00
epriestley
42020e1357 Completely remove "differential.find" Conduit API method
Summary:
Ref T2543. I believe there have been no upstream callsites of this method since D1646, in February 2012.

The method works, and we can revert this if needbe, but this seems like a good time to remove support.

Test Plan: Grepped for `differential.find`.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T2543

Differential Revision: https://secure.phabricator.com/D18397
2017-08-11 13:38:28 -07:00
epriestley
13ddb15bbc Remove legacy withStatus() method from RevisionQuery
Summary: Ref T2543. All callsites are now in terms of `withStatuses()`.

Test Plan:
  - Called `differential.query` and `differential.find` from Conduit API.
  - Grepped through all `withStatus()` callsites.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T2543

Differential Revision: https://secure.phabricator.com/D18396
2017-08-11 13:37:47 -07:00
Chad Little
f25acf2dee Uh, update Phabricator login image
Summary: The things you find testing production.

Test Plan: Run sprite, celerity

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D18409
2017-08-11 13:37:26 -07:00
epriestley
50dfdb8d03 Replace legacy Differential queries for "open" revisions with a modern mechanism
Summary: Ref T2543. Several queries want only open revisions. Provide a tailored, non-legacy way to issue that query.

Test Plan: Viewed some of these callsites (e.g., "Similar open revisions affecting these files"), saw only open revisions.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T2543

Differential Revision: https://secure.phabricator.com/D18395
2017-08-11 13:37:11 -07:00
epriestley
212d4d0dc7 Migrate Differential Revision SavedQueries to the new "statuses" tokenizer
Summary: Ref T2543. This migrates existing saved queries so they use the right modern values for the new tokenizer control, introduced in D18393.

Test Plan:
  - Saved a query with "Abandoned" selected as the status in the old "<select />", prior to D18393.
  - Upgraded to D18393, which broke the query (it no longer selected any status filter).
  - Ran the migration to fix things.
  - Saw the query now execute with "Abandoned" selected in the tokenizer, preseving the original behavior accurately.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T2543

Differential Revision: https://secure.phabricator.com/D18394
2017-08-11 13:36:37 -07:00
epriestley
53516093ae Replace Differential hard-coded status "<select />" with tokenizer
Summary:
Ref T2543. This updates the UI control in the web UI. Also:

  - This implicitly makes this queryable with the API (`differential.revision.search`); it previously was not.
  - This does NOT migrate existing saved queries. I'll do those in the next change, and hold this until it happens.
  - This will break some existing `/differential/?status=XYZ` links. For example, `status=open` now needs to be `status=open()`. I couldn't find any of these in the upstream, and I suspect these are rare in the wild (users would normally link directly to saved queries, not use URI query construction).

Test Plan: {F5093611}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T2543

Differential Revision: https://secure.phabricator.com/D18393
2017-08-11 13:36:00 -07:00
epriestley
8160baec2a Add a Differential revision status tokenizer datasource
Summary:
Ref T2543. This adds a tokenizer, similar to the Maniphest tokenizer, so the hard-coded `<select />` control in Differential ApplicationSearch can be replaced with a more flexible control that handles the addition of new statuses with more grace.

This only adds the new datasource.

Test Plan: Used `/typeahead/class/` to preview the behavior of the new datasource.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T2543

Differential Revision: https://secure.phabricator.com/D18392
2017-08-11 13:35:15 -07:00
Alex Vandiver
45b0fd8f9b Remove a debugging "echo" that crept in in dccd799b
Summary: This echo was accidentally added in dccd799b

Test Plan: Inspection.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D18391
2017-08-11 05:50:43 -07:00
epriestley
2c150076b0 Stop populating or updating working copies in observed Mercurial repositories
Summary:
Ref T12961. Fixes T4416. Currently, for observed Mercurial repositories, we build a working copy with `pull -u` (for "update").

This should be unnecessary, and we don't do it for hosted Mercurial repositories. We also stopped doing it years ago for Git repositories. We also don't clone Mercurial repositories with a working copy.

It's possible something has slipped through the cracks here so I'll hold this until after the release cut, but I believe there are no actual technical blockers here.

Test Plan:
  - Observed a public Mercurial repository on Bitbucket.
  - Let it import.
  - Browsed commits, branches, file content, etc., without any apparent issues.

Reviewers: chad

Reviewed By: chad

Subscribers: cspeckmim

Maniphest Tasks: T12961, T4416

Differential Revision: https://secure.phabricator.com/D18390
2017-08-10 19:14:56 -07:00
epriestley
794e185bf9 Pass SSH wrappers to VCS commands unconditonally, not just if there's an SSH remote
Summary:
Ref T12961. In Mercurial, it's possible to have "subrepos" which may use a different protocol than the main repository.

By putting an SSH repository inside an HTTP repository, an attacker can theoretically get us to execute `hg` without overriding `ui.ssh`, then execute code via the SSH hostname attack.

As an immediate mitigation to this attack, specify `ui.ssh` unconditionally. Normally, this will have no effect (it will just be ignored). In the specific case of an SSH repo inside an HTTP repo, it will defuse the `ssh` protocol.

For good measure and consistency, do the same for Subversion and Git. However, we don't normally maintain working copies for either Subversion or Git so it's unlikely that similar attacks exist there.

Test Plan:
  - Put an SSH subrepo with an attack URI inside an HTTP outer repo in Mercurial.
  - Ran `hg up` with and without `ui.ssh` specified.
  - Got dangerous badness without `ui.ssh` and safe `ssh` subprocesses with `ui.ssh`.

I'm not yet able to confirm that `hg pull -u -- <uri>` can actually trigger this, but this can't hurt and our SSH wrapper is safer than the native behavior for all Subversion, Git and Mercurial versions released prior to today.

Reviewers: chad

Reviewed By: chad

Subscribers: cspeckmim

Maniphest Tasks: T12961

Differential Revision: https://secure.phabricator.com/D18389
2017-08-10 17:49:55 -07:00