fix: a page with an opaque origin is refused, not treated as a site (closes #527)

A page with an opaque origin, such as one its server sandboxes with a
Content-Security-Policy, is reported to the background with the origin
"null". The background took that as a site, so every such page on every
website was one site: a connection remembered for one covered them all,
and the phishing check never matched. A request whose origin is "null" is
now refused with 4100, like one with no origin, before any method runs,
and is not credited to the site its URL names. tests/rpcOrigin.test.js
sends one, and README's SiteApproval says so.

Model: opus-5-5

README.md

@@ -362,14 +362,6 @@ 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
@@ -2088,9 +2080,12 @@ 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. 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. 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`,
   `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,29 +22,13 @@ set -eu
 SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)"
 ROOT="$(cd "$SCRIPT_DIR/.." && pwd -P)"
 
-# 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"
+IMAGE="$("$SCRIPT_DIR/projectname")-e2e-chrome"
+
+IIDFILE=""
 
 cleanup() {
-    # 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
+    if [ -n "$IIDFILE" ]; then
+        rm -f "$IIDFILE"
     fi
 }
 
@@ -56,16 +40,22 @@ main() {
         exit 1
     fi
 
+    IIDFILE="$(mktemp)"
     trap cleanup EXIT
-    # HUP: the terminal or SSH session running the suite closed.
-    trap 'cleanup; exit 130' HUP INT TERM
+    trap 'cleanup; exit 130' 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 -t "$IMAGE" -f tests/e2e/Dockerfile .
+    docker build --no-cache --iidfile "$IIDFILE" -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
@@ -85,12 +75,11 @@ 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}" \
-        "$IMAGE" \
+        "$(cat "$IIDFILE")" \
         node tests/e2e/run.js
 
     # Where chrome.storage.local lives, and what moves it: two unpacked loads
@@ -104,10 +93,9 @@ 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 \
-        "$IMAGE" \
+        "$(cat "$IIDFILE")" \
         node tests/e2e/storagePartition.js
 }
 

script/test-e2e-firefox

@@ -21,23 +21,13 @@ set -eu
 SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)"
 ROOT="$(cd "$SCRIPT_DIR/.." && pwd -P)"
 
-# 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"
+IMAGE="$("$SCRIPT_DIR/projectname")-e2e-firefox"
+
+IIDFILE=""
 
 cleanup() {
-    # 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
+    if [ -n "$IIDFILE" ]; then
+        rm -f "$IIDFILE"
     fi
 }
 
@@ -49,16 +39,22 @@ main() {
         exit 1
     fi
 
+    IIDFILE="$(mktemp)"
     trap cleanup EXIT
-    # HUP: the terminal or SSH session running the suite closed.
-    trap 'cleanup; exit 130' HUP INT TERM
+    trap 'cleanup; exit 130' 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 -t "$IMAGE" -f tests/e2e/firefox/Dockerfile .
+    docker build --no-cache --iidfile "$IIDFILE" -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
@@ -73,11 +69,10 @@ 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 \
-        "$IMAGE" \
+        "$(cat "$IIDFILE")" \
         node tests/e2e/firefox/run.js dist/firefox
 
     # The install/uninstall/re-install property, against the packaged XPI
@@ -87,11 +82,10 @@ 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 \
-        "$IMAGE" \
+        "$(cat "$IIDFILE")" \
         node tests/e2e/firefox/reinstall.js
 }
 

src/background/index.js

@@ -1428,6 +1428,11 @@ 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 {
@@ -1436,7 +1441,7 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
                 // an unparseable URL leaves the origin unknown
             }
         }
-        if (!trustedOrigin) {
+        if (!trustedOrigin || trustedOrigin === "null") {
             sendResponse({
                 error: {
                     code: 4100,

tests/rpcOrigin.test.js

@@ -8,8 +8,14 @@
 // and a request the browser said nothing about was credited to whatever the
 // page wrote (https://git.eeqj.de/sneak/AutistMask/issues/407).
 //
-// 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.
+// 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.
 
 const { makeStorageStub } = require("./support/storageStub");
 
@@ -144,3 +150,23 @@ 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.",
+            },
+        });
+    });
+});