Page MenuHomePhabricator

Make `setCWD('/does/not/exist')` safer on ExecFuture
Closed, ResolvedPublic

Description

When you run a command with setCWD(...) (which ultimately resolves to a cwd parameter to proc_open()) and the process is unable to switch to the specified cwd, the command runs in the current CWD without raising any errors.

This script lists the current directory:

epriestley@orbital ~ $ cat test.php 
<?php

$spec = array();
$pipes = array();

$result = proc_open(
  'ls',
  $spec,
  $pipes,
  '/does/not/exist');

var_dump($result);
epriestley@orbital ~ $ php -f test.php 
resource(4) of type (process)
ABBYY			Desktop			Dropbox			Music			dbdump.db		out.2			test.php
Applications		Documents		Library			Pictures		dev			scans
Creative Cloud Files	Downloads		Movies			Public			out.1			src

Normally this is okay-ish because most commands just fail when run in the wrong directory, but some commands (like git fetch +refs/*:refs/* or rm ...) may be very dangerous.

There doesn't seem to be any way to detect or prevent this that I can see. Ideally, proc_open would fail abruptly if changing the CWD failed. Two things we could do:

  • Test if the CWD exists before running the command. It's still possible that the directory is removed between our check and the proc_open().
  • Explicitly chdir() somewhere "safe" before running any CWD commands, so if they mess up they're less likely to destroy anything? This gets complicated because it's a side effect that the caller may not really expect or anticipate. We (probably?) don't rely on on CWD anywhere in web Phabricator, but the various CLI scripts do.

Revisions and Commits

Restricted Differential Revision
Restricted Differential Revision

Event Timeline

epriestley added a revision: Restricted Differential Revision.Apr 22 2016, 12:26 PM
epriestley added a commit: Restricted Diffusion Commit.Apr 22 2016, 3:11 PM
epriestley added a revision: Restricted Differential Revision.Feb 22 2019, 11:40 PM
epriestley closed this task as Resolved by committing Restricted Diffusion Commit.Feb 25 2019, 8:14 PM
epriestley added a commit: Restricted Diffusion Commit.