fix: each browser-suite run builds and removes an image of its own (closes #547)

script/test-e2e and script/test-e2e-firefox tagged their image with one
fixed name. When two clones ran a suite at once, the second build moved
that tag, the image store deleted the first run's now-untagged image, and
that suite exited 125 without running a test. Each run now tags its image
with the suite's name and twelve random hex digits and runs it by that
tag. The cleanup that runs on exit, Ctrl-C, kill and a closed terminal
removes the run's container if docker's client left it running, then the
image. The image-ID file that guarded against a moving tag is gone.
README.md's End-to-End Tests section says which endings clean up.

Model: opus-5-5

README.md

@@ -362,6 +362,14 @@ are outside `make check`. Neither opens the popup from the toolbar button: both
 load its page in an ordinary tab, so what the toolbar popup itself adds, its
 size and its closing when it loses focus, is covered by neither.
 
+Each run builds its image under a tag of its own, `autistmask-e2e-chrome:` or
+`autistmask-e2e-firefox:` followed by twelve random hex digits, so two clones
+can run the same suite at once without one build taking the other's image away.
+The run removes that image, and its container if one is still running, after the
+suite passes or fails, on Ctrl-C, on `kill`, and when the terminal or SSH
+session it runs in closes. A run killed outright (`kill -9`) cannot clean up:
+its image stays under its tag until removed by hand with `docker image rm`.
+
 ### Chrome (`make test-e2e`)
 
 `make test-e2e` builds `dist/chrome/` and drives the **real popup in a real
@@ -2080,12 +2088,9 @@ view would leave a wallet one click from deletion.
   `wallet_requestPermissions` and is on neither the allowed nor the denied list.
   A site is its full origin, `scheme://host[:port]`, on both lists and for a
   connection allowed without "Remember": a choice for `https://dapp.example`
-  says nothing about `http://dapp.example` or another port of that host. A page
-  with an opaque origin, such as one its server sandboxes with a
-  Content-Security-Policy, is reported with the origin `null`, which is no site:
-  every request from it is refused with EIP-1193 code `4100` and opens no
-  prompt. The background script prefers the toolbar popup (`action.openPopup()`)
-  and falls back to a separate popup window (`src/background/index.js`,
+  says nothing about `http://dapp.example` or another port of that host. The
+  background script prefers the toolbar popup (`action.openPopup()`) and falls
+  back to a separate popup window (`src/background/index.js`,
   `requestApproval()`). Only one exists per site at a time: a further connection
   request from a site whose prompt is still unanswered is refused with EIP-1193
   code `-32002` and opens no new prompt. If that prompt was in a toolbar popup

script/test-e2e

@@ -22,13 +22,29 @@ set -eu
 SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)"
 ROOT="$(cd "$SCRIPT_DIR/.." && pwd -P)"
 
-IMAGE="$("$SCRIPT_DIR/projectname")-e2e-chrome"
-
-IIDFILE=""
+# Each run builds, runs and then removes an image under a tag of its own.
+# Under one tag shared by every run, a second clone's build moves the tag
+# off this run's image, and the image store can then delete that untagged
+# image before or while this run uses it. The tag is random rather than
+# $$ because CI jobs share one docker daemon, but each job's container
+# numbers its processes from scratch.
+SUFFIX="$(od -An -N6 -tx1 /dev/urandom | tr -d ' \n')"
+NAME="$("$SCRIPT_DIR/projectname")-e2e-chrome"
+IMAGE="$NAME:$SUFFIX"
+# Both docker runs below use this container name: docker run --rm returns
+# only once its container is removed, so the second never meets the first.
+CONTAINER="$NAME-$SUFFIX"
 
 cleanup() {
-    if [ -n "$IIDFILE" ]; then
-        rm -f "$IIDFILE"
+    # After a third Ctrl-C docker's client exits whether or not the
+    # container has stopped, and a container still running keeps the image
+    # in use.
+    if docker container inspect "$CONTAINER" >/dev/null 2>&1; then
+        docker container rm -f "$CONTAINER" >/dev/null
+    fi
+    # No image to remove when the build failed or was interrupted.
+    if docker image inspect "$IMAGE" >/dev/null 2>&1; then
+        docker image rm "$IMAGE" >/dev/null
     fi
 }
 
@@ -40,22 +56,16 @@ main() {
         exit 1
     fi
 
-    IIDFILE="$(mktemp)"
     trap cleanup EXIT
-    trap 'cleanup; exit 130' INT TERM
+    # HUP: the terminal or SSH session running the suite closed.
+    trap 'cleanup; exit 130' HUP INT TERM
 
     echo "Building the Chrome e2e image (extension included)..."
     # --no-cache: the image build runs make build and its checks, and a
     # cached layer is a check that did not run.
-    docker build --no-cache --iidfile "$IIDFILE" -t "$IMAGE" \
-        -f tests/e2e/Dockerfile .
+    docker build --no-cache -t "$IMAGE" -f tests/e2e/Dockerfile .
 
     echo "Running e2e suite in the pinned Playwright container..."
-    # The image is run by ID, not by tag: where two clones of this repo run
-    # the suite at once, the other build can move the tag between this
-    # build and this run, and the suite would then silently test the other
-    # checkout.
-    #
     # --ipc=host: Chromium's shared-memory needs more than the default
     #   64MB /dev/shm or renderers crash.
     # HOME=/tmp: the image's root home is not a reliable place for the
@@ -75,11 +85,12 @@ main() {
     #   probe. The image is pinned by digest, so this can only ever bite
     #   on a deliberate bump.
     docker run --rm \
+        --name "$CONTAINER" \
         --ipc=host \
         -e HOME=/tmp \
         -e PW_EXPERIMENTAL_SERVICE_WORKER_NETWORK_EVENTS=1 \
         -e "E2E_TRACE_NETWORK=${E2E_TRACE_NETWORK:-0}" \
-        "$(cat "$IIDFILE")" \
+        "$IMAGE" \
         node tests/e2e/run.js
 
     # Where chrome.storage.local lives, and what moves it: two unpacked loads
@@ -93,9 +104,10 @@ main() {
     # --host-resolver-rules=MAP * ~NOTFOUND so nothing it does can leave.
     echo "Running the extension-id and storage-partition observations..."
     docker run --rm \
+        --name "$CONTAINER" \
         --ipc=host \
         -e HOME=/tmp \
-        "$(cat "$IIDFILE")" \
+        "$IMAGE" \
         node tests/e2e/storagePartition.js
 }
 

script/test-e2e-firefox

@@ -21,13 +21,23 @@ set -eu
 SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)"
 ROOT="$(cd "$SCRIPT_DIR/.." && pwd -P)"
 
-IMAGE="$("$SCRIPT_DIR/projectname")-e2e-firefox"
-
-IIDFILE=""
+# A tag and a container name of this run's own, as in script/test-e2e,
+# which says why.
+SUFFIX="$(od -An -N6 -tx1 /dev/urandom | tr -d ' \n')"
+NAME="$("$SCRIPT_DIR/projectname")-e2e-firefox"
+IMAGE="$NAME:$SUFFIX"
+CONTAINER="$NAME-$SUFFIX"
 
 cleanup() {
-    if [ -n "$IIDFILE" ]; then
-        rm -f "$IIDFILE"
+    # After a third Ctrl-C docker's client exits whether or not the
+    # container has stopped, and a container still running keeps the image
+    # in use.
+    if docker container inspect "$CONTAINER" >/dev/null 2>&1; then
+        docker container rm -f "$CONTAINER" >/dev/null
+    fi
+    # No image to remove when the build failed or was interrupted.
+    if docker image inspect "$IMAGE" >/dev/null 2>&1; then
+        docker image rm "$IMAGE" >/dev/null
     fi
 }
 
@@ -39,22 +49,16 @@ main() {
         exit 1
     fi
 
-    IIDFILE="$(mktemp)"
     trap cleanup EXIT
-    trap 'cleanup; exit 130' INT TERM
+    # HUP: the terminal or SSH session running the suite closed.
+    trap 'cleanup; exit 130' HUP INT TERM
 
     echo "Building the pinned Firefox e2e image (extension included)..."
     # --no-cache: the image build runs make build and its checks, and a
     # cached layer is a check that did not run.
-    docker build --no-cache --iidfile "$IIDFILE" -t "$IMAGE" \
-        -f tests/e2e/firefox/Dockerfile .
+    docker build --no-cache -t "$IMAGE" -f tests/e2e/firefox/Dockerfile .
 
     echo "Running the Firefox e2e suite..."
-    # The image is run by ID, not by tag: where two clones of this repo run
-    # the suite at once, the other build can move the tag between this
-    # build and this run, and the suite would then silently test the other
-    # checkout.
-    #
     # --shm-size=1g: Firefox needs more than the default 64MB /dev/shm.
     # --network none: the suite stubs nothing, so this is what keeps the
     #   run offline and deterministic. The extension swallows its own
@@ -69,10 +73,11 @@ main() {
     # "CanCreateUserNamespace() clone() failure: EPERM" on startup here;
     # it is cosmetic and headless Firefox runs fine without it.
     docker run --rm \
+        --name "$CONTAINER" \
         --shm-size=1g \
         --network none \
         -e HOME=/tmp \
-        "$(cat "$IIDFILE")" \
+        "$IMAGE" \
         node tests/e2e/firefox/run.js dist/firefox
 
     # The install/uninstall/re-install property, against the packaged XPI
@@ -82,10 +87,11 @@ main() {
     # and the suite above shares one session throughout.
     echo "Running the Firefox re-install suite against the packaged XPI..."
     docker run --rm \
+        --name "$CONTAINER" \
         --shm-size=1g \
         --network none \
         -e HOME=/tmp \
-        "$(cat "$IIDFILE")" \
+        "$IMAGE" \
         node tests/e2e/firefox/reinstall.js
 }
 

src/background/index.js

@@ -1428,11 +1428,6 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
         // the origin of sender.url is used: the frame that sent the message,
         // not the tab's page, which may be another site embedding that
         // frame. With neither, the request is refused.
-        //
-        // A page with an opaque origin, such as one its server sandboxes with
-        // a Content-Security-Policy, is reported with the origin "null". That
-        // is no site, so the request is refused too, and not credited to the
-        // site its URL names.
         let trustedOrigin = sender.origin;
         if (!trustedOrigin && sender.url) {
             try {
@@ -1441,7 +1436,7 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
                 // an unparseable URL leaves the origin unknown
             }
         }
-        if (!trustedOrigin || trustedOrigin === "null") {
+        if (!trustedOrigin) {
             sendResponse({
                 error: {
                     code: 4100,

tests/rpcOrigin.test.js

@@ -8,14 +8,8 @@
 // and a request the browser said nothing about was credited to whatever the
 // page wrote (https://git.eeqj.de/sneak/AutistMask/issues/407).
 //
-// A page with an opaque origin, such as one its server sandboxes with a
-// Content-Security-Policy, is reported with the origin "null". That names no
-// site, so its request is refused
-// (https://git.eeqj.de/sneak/AutistMask/issues/527).
-//
-// Every other sender here lacks sender.origin, as on old Firefox. The
-// connection check on eth_accounts is what shows which site a request was
-// credited to.
+// Every sender here lacks sender.origin, as on old Firefox. The connection
+// check on eth_accounts is what shows which site a request was credited to.
 
 const { makeStorageStub } = require("./support/storageStub");
 
@@ -150,23 +144,3 @@ describe("a request the browser does not say the sender of", () => {
         });
     });
 });
-
-describe("a request from a page with an opaque origin", () => {
-    test("is refused, whatever the page's URL says", async () => {
-        const accounts = loadBackground();
-
-        const result = await accounts({
-            origin: "null",
-            url: CONNECTED_ORIGIN + "/sandboxed.html",
-            tab: { url: CONNECTED_ORIGIN + "/sandboxed.html" },
-        });
-
-        expect(result).toEqual({
-            error: {
-                code: 4100,
-                message:
-                    "The wallet could not tell which site sent this request.",
-            },
-        });
-    });
-});