Make write safer (#1416)

1. write will no longer allow "." or "" as a write location
2. write will only unlink files, and not do a deep unlink. This means that two different build options might conflict in wake but I think that's bad style anyway
3. write will not write outside of the root workspace. This will break some rare use cases but they can be replaced with a bespoke job instead
4. write will not overwrite a source file

share/wake/lib/system/io.wake

@@ -39,6 +39,35 @@ target writeImp inputs mode path content =
 
         makeRunner "write" (\_ Pass 0.0) pre post virtualRunner
 
+    # There are a couple likely bad paths that we don't want the user writing to
+    # so we give good error messages for these cases
+    require False = path ==* ""
+    else
+        failWithError
+        "Attempt to write to the path of the empty string. This was likely a mixup between the path and the content."
+
+    require False = path ==* "."
+    else failWithError "Attempt to write to write to the root workspace"
+
+    # We don't want `write` to write to anything outside of the workspace via
+    # a relative path
+    require False = matches `\.\..*` path
+    else failWithError "Attempt to write outside of the workspace"
+
+    # We don't want `write` to write to anything outside of the workspace via
+    # an absolute path
+    require False = matches `/.*` path
+    else failWithError "Attempt to write to an absolute path"
+
+    # Source files should never be deleted so we check for this case
+    def scan dir regexp = prim "sources"
+    def isSource = exists (_ ==~ path) (scan "." path.quote)
+
+    require False = isSource
+    else failWithError "Attempt to write over a source file"
+
+    # If all those checks pass we go ahead and perform the write. The write will
+    # overwrite single files but it will not overwrite a whole directory with a file.
     makeExecPlan ("<write>", str mode, path, Nil) inputs
     | setPlanLabel "write: {path} {str mode}"
     | setPlanOnce False

share/wake/lib/system/job.wake

@@ -979,10 +979,11 @@ export def makeJSONRunner (plan: JSONRunnerPlan): Runner =
 
             def specFilePath = "{build}/spec-{prefix}.json"
 
-            require Pass inFile =
+            require Pair (Pass inFile) _ =
                 write specFilePath (prettyJSON json)
                 | rmap getPathName
-            else Pair (Fail (makeError "Failed to 'write {specFilePath}'.")) ""
+                | addErrorContext "Failed to 'write {specFilePath}: '"
+                | (Pair _ "")
 
             def outFile = resultPath inFile
             def cmd = script, "-I", "-p", inFile, "-o", outFile, extraArgs

src/runtime/string.cpp

@@ -251,7 +251,26 @@ static PRIMFN(prim_write) {
   REQUIRE(mpz_cmp_si(mode, 0x1ff) <= 0);
   long mask = mpz_get_si(mode);
 
-  deep_unlink(AT_FDCWD, path->c_str());
+  // We want `write` to overwrite existing files so that
+  // each time the build runs it isn't blocked by previous files.
+  // However we want it to fail if it attempts to delete a directory.
+  // We need to give the user a good error message in that case.
+  if (unlink(path->c_str()) < 0 && errno != ENOENT) {
+    if (errno == EISDIR) {
+      std::string error = path->c_str();
+      error +=
+          " is a directory and cannot be overwritten. If this is intentional please manually "
+          "delete this directory";
+      size_t len = std::min(error.size(), max_error);
+      String *out = String::claim(runtime.heap, error.c_str(), len);
+      RETURN(claim_result(runtime.heap, false, out));
+    }
+    std::string error = strerror(errno);
+    size_t len = std::min(error.size(), max_error);
+    String *out = String::claim(runtime.heap, error.c_str(), len);
+    RETURN(claim_result(runtime.heap, false, out));
+  }
+
   std::ofstream t(path->c_str(), std::ios_base::trunc);
   if (!t.fail()) {
     t.write(body->c_str(), body->size());