Add edges to Phabricator task data dump transaction history
Closed, ResolvedPublic8 Story Points
Actions

Description

Steps to reproduce

Add a project to a task
Remove a project from a task
view the project within the Phabricator json data dump.

example data: T49344, which includes this bit of history:

Jdforrester-WMF edited projects, added VisualEditor 2015/16 Q1 blockers; removed VisualEditor 2014/15 Q4 blockers. Via Web Tue, Jun 16, 8:00 PM

Actual results
The 'edge' portion of the dump contains one item in 'edge' with a matching timestamp, showing only the added project, not the removed project.

['PHID-TASK-h27s7yvr62xzheogrrv7', 41, 'PHID-PROJ-ly2ydkopj6mc3byztenf', 1434484858, 0, None]

PHID-PROJ-ly2ydkopj6mc3byztenf = VisualEditor 2015/16 Q1 blockers.

Python code to examine the dump:

import json
with open('phabricator_public.dump') as dump_file:
    data = json.load(dump_file)
for x in data['task']['49344']['edge']:
    print(x)

Expected results
A record in edge shows VisualEditor 2014/15 Q4 blockers being removed.

Details

Commits: rPHTO22de08268491: phab: public dump include core:edge relationships

Related Objects
Search...

Status	Assigned	Task
Resolved	JAufrecht	T101207 Make improvements to the VisualEditor team's processes
Resolved	JAufrecht	T102272 Create VE Product Burnup chart
Resolved	chasemp	T104920 Add edges to Phabricator task data dump transaction history

Event Timeline

JAufrecht created this task.Jul 6 2015, 9:43 PM

JAufrecht assigned this task to chasemp.

JAufrecht raised the priority of this task from to High.

JAufrecht lowered the priority of this task from High to Normal.

JAufrecht updated the task description. (Show Details)

JAufrecht added a project: VisualEditor.

JAufrecht set Security to None.

JAufrecht added subscribers: Aklapper, JAufrecht, csteipp.

csteipp removed a subscriber: csteipp.Jul 6 2015, 9:46 PM

Krenair added a subscriber: Krenair.Jul 6 2015, 9:47 PM

I don't see any unexpected behavior in Phabricator described here so reading "Expected results" in the task description, this task is invalid IMHO.

phabricator_maniphest.edge stores current edges between objects. When you remove a project↔task edge the corresponding record is removed from the edge table.
You can find information about edges removed between projects and tasks in phabricator_maniphest.maniphest_transaction:

The field objectPHID is the task ID
The field transactionType has the value core:edge
Example JSON blob values of the fields oldValue and newValue when removing one of two projects associated to a task:
- oldValue: {"PHID-PROJ-siajv64zsvjhgcwj2fip":{"src":"PHID-TASK-debddmt4kz3xglbdib77","type":"41","dst":"PHID-PROJ-siajv64zsvjhgcwj2fip","dateCreated":"1436269873","seq":"0","dataID":null,"data":[]},"PHID-PROJ-k2u427l6iwrbgs4pcwdw":{"src":"PHID-TASK-debddmt4kz3xglbdib77","type":"41","dst":"PHID-PROJ-k2u427l6iwrbgs4pcwdw","dateCreated":"1425291780","seq":"0","dataID":null,"data":[]}}
- newValue: {"PHID-PROJ-k2u427l6iwrbgs4pcwdw":{"dst":"PHID-PROJ-k2u427l6iwrbgs4pcwdw","type":41,"data":[]}}

I have no idea if the maniphest_transaction table is in the dump though, if that is the request here.

Revised Expected results
A record in transaction shows VisualEditor 2014/15 Q4 blockers being removed.

Actual Results
maniphest_transaction is in the dump, for only for certain transaction types. It looks like edge transactions are not included.

>>> data['task']['102272']['transactions'].keys()                                                                            
dict_keys(['projectcolumn', 'priority', 'reassign', 'status'])

Chase, can you add edges to the transaction dump? Would that introduce any security concerns? EDIT FOR CLARITY: Can you add 'edge' or 'edges' to the list of transaction types that are dumped from maniphest_transaction, in addition to projectcolumn, priority, reassign, and status?

JAufrecht renamed this task from Removing a Project from a task may not be captured in the dump to Add edges to Phabricator task data dump transaction history.Jul 16 2015, 6:19 PM

Jdforrester-WMF moved this task from To Triage to Freezer on the VisualEditor board.Jul 19 2015, 8:48 PM

Need to get @csteipp's feeling on this but I think it's OK....

Examples...

Adding project foo

             id: 602
           phid: PHID-XACT-TASK-e2iui2xdvdmm55o
     authorPHID: PHID-USER-7t36l5d3llsm5abqfx3u
     objectPHID: PHID-TASK-nh3iurpcgbvedsl2bh7i
     viewPolicy: public
     editPolicy: PHID-USER-7t36l5d3llsm5abqfx3u
    commentPHID: NULL
 commentVersion: 0
transactionType: core:edge
       oldValue: []
       newValue: {"PHID-PROJ-3cegrtti4klrd3guxrsd":{"dst":"PHID-PROJ-3cegrtti4klrd3guxrsd","type":41,"data":[]}}
  contentSource: {"source":"web","params":{"ip":"10.68.16.65"}}
       metadata: {"edge:type":41}
    dateCreated: 1437495683
   dateModified: 1437495683

Removing project foo

************************** 603. row ***************************
             id: 603
           phid: PHID-XACT-TASK-qsgncgnhizpzzbq
     authorPHID: PHID-USER-7t36l5d3llsm5abqfx3u
     objectPHID: PHID-TASK-nh3iurpcgbvedsl2bh7i
     viewPolicy: public
     editPolicy: PHID-USER-7t36l5d3llsm5abqfx3u
    commentPHID: NULL
 commentVersion: 0
transactionType: core:edge
       oldValue: {"PHID-PROJ-3cegrtti4klrd3guxrsd":{"src":"PHID-TASK-nh3iurpcgbvedsl2bh7i","type":"41","dst":"PHID-PROJ-3cegrtti4klrd3guxrsd","dateCreated":"1437495683","seq":"0","dataID":null,"data":[]}}
       newValue: []
  contentSource: {"source":"web","params":{"ip":"10.68.16.65"}}
       metadata: {"edge:type":41}
    dateCreated: 1437495756
   dateModified: 1437495756

From https://phab-01.wmflabs.org/T19

Edge relationship transactions will affect more than just project relationships, but in general I think is OK per already public task...we could go the route of selecting only metadata: {"edge:type":41} but I'm not sure how protective to be here.

@csteipp?

Is viewPolicy related to the task, so we can make sure to only export public ones?

And it looks like that IP address is the labs proxy. Can that ever have user ip's in production?

In T104920#1497089, @csteipp wrote:

Is viewPolicy related to the task

No, it's not as it's the general transaction table (though I have no idea what that "transaction view policy" would actually express here).
The viewPolicy of a task is defined in the DB phabricator_maniphest in maniphest_task.viewPolicy instead.
To connect in SQL, WHERE maniphest_task.phid = maniphest_transaction.objectPHID

And it looks like that IP address is the labs proxy. Can that ever have user ip's in production?

I don't think so on production, as long as we don't fix T840 (but that's only an uneducated guess)

In T104920#1497089, @csteipp wrote:

Is viewPolicy related to the task, so we can make sure to only export public ones?
And it looks like that IP address is the labs proxy. Can that ever have user ip's in production?

The way our dump works we get a list of public tasks and then a list of defined safe transactions for them. The per transaction view policy we don't honor as it's not really indicative yet. We only pull the transactions mentioned as:

dict_keys(['projectcolumn', 'priority', 'reassign', 'status'])

We also only pull fixed fields:

_ = p.sql_x("SELECT id, \
                    phid, \
                    authorPHID, \
                    objectPHID, \
                    commentPHID, \
                    commentVersion, \
                    transactionType, \
                    oldValue, \
                    newValue, \
                    metadata, \
                    dateCreated, \
                    dateModified \
            FROM maniphest_transaction WHERE objectPHID=%s AND transactionType=%s",
            (objectPHID, type), limit=None)

i.e. I never included contentSource as I don't want T840 to be a silent time bomb and I haven't seen a compelling use case for it that outways the risk.

I'm running this dump w/ core:edges now to see how things go and I can provide a real deal end state dump for you @csteipp but I think this should be ok.

The only issue I see is if there is a private project (of which we have none that I know of in that even the existence of the project name itself is hidden the PHID may be revealed if it is added or removed from a public task -- possibly similar disclosure for other hidden items but only the internal alphanumeric reference would ever be included). We have pseudo-officially said we have no use case for absolutely hidden projects and so I'm not too worried about this atm.

In T104920#1503261, @chasemp wrote:

The only issue I see is if there is a private project (of which we have none that I know of in that even the existence of the project name itself is hidden the PHID may be revealed if it is added or removed from a public task -- possibly similar disclosure for other hidden items but only the internal alphanumeric reference would ever be included). We have pseudo-officially said we have no use case for absolutely hidden projects and so I'm not too worried about this atm.

Note that privacy of projects isn't well supported in Phabricator, so I agree we should be relaxed. For example, adding a private project (name is locked down) to a public task doesn't show the name in the Web view, but does in the e-mail it sends to all subscribers. Fun.

In T104920#1503280, @Jdforrester-WMF wrote:

Note that privacy of projects isn't well supported in Phabricator, so I agree we should be relaxed. For example, adding a private project (name is locked down) to a public task doesn't show the name in the Web view, but does in the e-mail it sends to all subscribers. Fun.

I thought they fixed the issues surrounding the rights of the person who triggered a notification vs. email receiver's rights? Is this really separate?

Either way we should not allow any restricted-visibility projects in our Phabricator instance.

[offtopic]

In T104920#1503280, @Jdforrester-WMF wrote:

adding a private project (name is locked down) to a public task doesn't show the name in the Web view, but does in the e-mail it sends to all subscribers.

Should be fixed, https://secure.phabricator.com/T6367

I'm running this dump w/ core:edges now to see how things go and I can provide a real deal end state dump for you @csteipp but I think this should be ok.

terbium: /home/csteipp/phabricator_public.dump_WITH_EDGES

as per discussion: *ping*

csteipp added a project: Security-Team-Reviews.Aug 24 2015, 7:06 PM

csteipp moved this task from Backlog to In Progress on the Security-Team-Reviews board.

Sorry for the slow response. This looks ok.

JAufrecht reassigned this task from csteipp to chasemp.Aug 25 2015, 8:29 PM

should be in the next days copy

chasemp added a commit: rPHTO22de08268491: phab: public dump include core:edge relationships.Aug 26 2015, 3:03 AM

Jdforrester-WMF moved this task from Freezer to TR0: Interrupt on the VisualEditor board.Aug 27 2015, 4:53 PM

Jdforrester-WMF edited a custom field.Sep 15 2015, 2:39 AM

sbassett moved this task from In Progress to Archive on the Security-Team-Reviews board.Jul 9 2019, 8:19 PM

Restricted Application added a project: User-Ryasmeen. · View Herald TranscriptJul 9 2019, 8:19 PM