Error type [Breaking]: reveal details less liberally

Previously, we only checked `isJSONAPIReady` before reading in the
error’s `message`—and even then only for the `title`, not for
`details`—but we didn’t require `isJSONAPIReady` to migrate over the
`code` and other properties. This was a security risk, since APIErrors
are displayed to the client directly, so they should only expose info
that the user has explicitly signaled is safe.

This commit also stops referencing err.message for both the JSON API
Error’s `title` and `details` property; instead, `message` can only go
into `details`. This is more consistent with JSON API semantics and
prevents the odd case in which `title` and `details` are the same

src/types/APIError.js

@@ -43,22 +43,30 @@ export default class APIError extends Error {
    *
    */
   static fromError(err) {
+    const fallbackTitle = "An unknown error occurred while trying to process this request.";
+
     if(err instanceof APIError) {
       return err;
     }
 
-    const title = err.title
-      || (err.isJSONAPIDisplayReady && err.message)
-      || "An unknown error occurred while trying to process this request.";
+    // If the error is marked as ready for JSON API display, it's secure
+    // to read values off it and show them to the user. (Note: most of
+    // the args below will probably be null/undefined, but that's fine.)
+    else if(err.isJSONAPIDisplayReady) {
+      return new APIError(
+        err.status || err.statusCode || 500,
+        err.code,
+        err.title || fallbackTitle,
+        err.details || err.message,
+        err.links,
+        err.paths
+      );
+    }
+
+    // Otherwise, we just show a generic error message.
+    else {
+      return new APIError(500, undefined, fallbackTitle)
+    }
 
-    // most of the args below will probably be null/undefined, but that's fine.
-    return new APIError(
-      err.status || err.statusCode || 500,
-      err.code,
-      title,
-      err.message || err.details,
-      err.links,
-      err.paths
-    );
   }
 }

test/unit/types/APIError.js

@@ -15,11 +15,18 @@ describe("Error Objects", () => {
   });
 
   describe("the fromError helper", () => {
-    it("should use the error's statusCode val as status if status not defined", () => {
-      let er = APIError.fromError({"statusCode": 300});
+    it("should use the error's statusCode val as status iff status not defined", () => {
+      let er = APIError.fromError({
+        "statusCode": 300,
+        "isJSONAPIDisplayReady": true
+      });
       expect(er.status === "300").to.be.true;
 
-      er = APIError.fromError({"status": 200, "statusCode": 300});
+      er = APIError.fromError({
+        "status": 200,
+        "statusCode": 300,
+        "isJSONAPIDisplayReady": true
+      });
       expect(er.status === "200").to.be.true;
     });