https://phabricator.wikimedia.org/D1203 is my fix for this issue at Wikimedia's install.

@avivey what would be required to merge @Dylsss 's patch into phorge stable?
(This thing has been annoying our team for the past few weeks)

Auto file attachments are also not working when editing a comment.

@kwisatz I'll try to review the diff this week, and that will probably get to master/stable soon...

For completeness: The security breaches mentioned in the security guidance task are all about exposing an existing file that the attacker has new view access to; This flow is only relevant to explicitly uploading a new file, so it doesn't have the same security implications.

Another use case: Drag & Drop on a new Object description, even if the Object is not already created, should attach by default to it.

I had communicated these upstream (almost exactly a year ago~) and some helpful information was provided

@speck wrote:

Ref: https://secure.phabricator.com/T13683

Per that description filing these notices here.

A user ran into a case where pasting an image from system clipboard into a text field (specifically an inline comment) would not attach the file to the object as expected. I have not yet tested this (did not realize images in clipboard could be pasted into fields).

Additionally the phorge project has had a few related issues reported

1. Deleting files leaves leftover "unknown object" references (on diff revisions)
   • https://we.phorge.it/T15110
2. Deleting a file object via ./bin/remove destroy results in an exception due to a missing edge
   • https://we.phorge.it/T15125
   • https://phabricator.wikimedia.org/T318547 (more details)

If you prefer I can post this on https://secure.phabricator.com/T13682 instead

epriestley wrote:

Thanks -- I think these are known elsewhere (the inline comment thing is just "inline comment text never attaches files", and probably not specific to pasting images), I just haven't had a chance to collect them in https://secure.phabricator.com/T13683 and fix them yet since the amount of software development I'm doing these days rounds down to zero. My plan is:

1. Deleting a file should delete all corresponding PhabricatorFileAttachment objects.
2. When an object is destroyed, attached edges should be destroyed even if their edge type is unknown, perhaps with a console warning.
3. Inline comments should attach files.

...but no clue when I'll get there.

---

(1) is probably simple and straightforward -- a patch using PhabricatorFileAttachmentQuery, following the PhabricatorQueryIterator pattern in DifferentialRevision->destroyObjectPermanently(), seems likely reasonable to me.

(2) may be tricky since I think the code raising the error is far-removed from the knowledge that a deletion is occurring. Alternate but less-pure remedies are "restore stub edge type definitions for the removed edges" or "migrate to destroy all those edges", but I think those are a fairly big step down from "don't fail when destroying unknown edge types".

(3) is probably moderately complicated, I think the patch has several pieces and probably doesn't closely resemble other file attachment patches.

epriestley wrote:

...perhaps with a console warning.

While reviewing this, one note mostly for my own sake: although the current console behavior says "exception", it does continue and actually destroy the object (but not the outdated edge).

---

I noted the rest of this in T13682.

This feature is not complete also for Conpherence AFAIK

Minor clarification.

I've seen this phrase in a unit test:

// Reference the file in a comment. This should not affect the file
// policy.

https://we.phorge.it/source/phorge/browse/master/src/applications/files/storage/__tests__/PhabricatorFileTestCase.php;f75c7ce7664b1baa23463a3f3fa62cbb671687d2$196-197

I'm unsure about the purpose of this.