`arc download` broken for non-CDN Phabricator

Observed Behavior:
arc download downloads Download File? confirmation page.

Expected Behavior:
arc download downloads the actual file.

Phabricator Version:

phabricator 2e7686262637072e72fd713054102575ec601356 (Fri, Apr 27) (branched from 5f774f7008b48d22839ced7040fbe5b1d48c2d34 on origin)
arcanist 8794ce1eaca0b3ef020e2cd5d2d5479fd3626b9f (Fri, Apr 27) (branched from a604548101025875de20a9c263df3790fea425b3 on origin)
phutil f3e10579f640ebad648c56f677164647ab7251a4 (Fri, Apr 13) (branched from 20eff1c8d14f08f05ef72828fa379e871d29662c on origin)
libcore d1ff2e44911bf2ead6a3d50832f8cdcdde1d97d5 (Thu, Apr 26)
services ed898eab124917b0f3556b4e3d6bad578da3a686 (Fri, Apr 27) (branched from 0b2b879298eae259aa13ea6ceb56840fac90ecef on origin)
diff 3.3 at /usr/bin/diff
git 2.17.0 at /usr/bin/git
hg 2.8.2 at /usr/bin/hg
pygmentize 2.2.0 at /usr/local/bin/pygmentize
svn 1.8.8 at /usr/bin/svn

Reproduction Steps:
On an instance without a CDN configured via alternate-file-domain (i.e. not a Phacility test instance as that uses cloudfront.net):

  1. Upload a non browser-viewable file (e.g. binary file, ZIP file)
  2. Run arc download ... to download that file
  3. Inspect file, notice it contains an HTML confirmation page instead of the binary

Notes:
This seems to be a regression in ab579f2511102775d27bda92c6f3d3c8fc302d3c which made CSP stricter.

-      $is_safe = ($is_alternate_domain || $is_lfs || $is_post || $is_public);
+      $is_safe = ($is_alternate_domain || $is_lfs || $is_post);

Since arc download (any version, even latest) doesn’t use POST it will download the page instead.

Changing ArcanistDownloadWorkflow.php to use $future->setMethod("POST"); acts as a workaround, but of course that requires all clients to be updated.

From reading ab579f2511102775d27bda92c6f3d3c8fc302d3c notes, I don’t think this was intended to be a breaking change.

1 Like

Thanks, I can reproduce with the instructions provided (Once I get through my broken system).

Upstreamed as https://secure.phabricator.com/T13132

1 Like

Thanks, see https://secure.phabricator.com/D19421.

This configuration is potentially unsafe and strongly discouraged outside of development/testing, but the behavior will be consistent with a safe configuration once that change lands.

Hi Evan,

thanks for looking into this and also explaining the attack vector! We just upgraded our instance (thanks to the team for all the great stuff in there), didn’t re-read the setup instructions and stumbled over this - we’ll definitely follow the recommendation and configure that value.