diff --git a/.github/workflows/qupath-gui.yml b/.github/workflows/qupath-gui.yml index d71d3e4..3481ffe 100644 --- a/.github/workflows/qupath-gui.yml +++ b/.github/workflows/qupath-gui.yml @@ -51,9 +51,12 @@ jobs: QUPATH="$PWD/qupath-dist/QuPath" ./build.sh - name: Register the apps the tests use (tests/ci_setup_apps.sh) run: tests/ci_setup_apps.sh "${{ runner.temp }}/lc" "$TOOLS_REF" + - name: Add a second user for the registry owner checks (gui_test_parity_trust_body.groovy chowns entries to it with sudo) + run: sudo useradd --no-create-home --shell /usr/sbin/nologin tester - name: Run every GUI test body env: LC_HOME: ${{ runner.temp }}/lc/home + LC_REQUIRE_OWNER_CHECKS: "1" # the owner checks of the trust body must run here, not be skipped (issue #20) LOGS: ${{ github.workspace }}/gui-logs run: tests/run_gui_ci.sh - name: Which apps QuPath lists (the app matrix, the sandbox driver tests/gui_test_listing_body.groovy) diff --git a/README.md b/README.md index 04fbe8c..c37c698 100644 --- a/README.md +++ b/README.md @@ -155,7 +155,7 @@ Users get the jar from the Releases page (see Install). Developers: `QUPATH=/pat The `gui_test_parity_*` bodies need the registered `interactions` app (its Python also writes the test images and runs `labconstrictor-tools`; the Playground app for the cancel body). The bodies that write their own registry and results (trust, form, host, lifecycle, pixel, support) empty their `LC_HOME` first, so that one must be a scratch folder with the name given below, and `LC_SHARED_HOME` must name the registry that holds `interactions`; `tests/run_gui_ci.sh` sets both for every body. -`BODY=gui_test_parity_trust_body.groovy LC_HOME= tests/run_gui_test.sh` writes registry entries that break each trust check and checks that each one is refused with its reason and that a good entry loads (26 checks; needs root to test the owner check). +`BODY=gui_test_parity_trust_body.groovy LC_HOME= tests/run_gui_test.sh` writes registry entries that break each trust check and checks that each one is refused with its reason and that a good entry loads (56 checks in CI). The owner checks (an entry or schema file owned by another user, a root-owned entry in your own folder) need a second user `tester` and root or passwordless sudo to `chown`; without them they are skipped with a NOTE, and `LC_REQUIRE_OWNER_CHECKS=1` turns that skip into a failure. CI creates the user and sets the variable, so there the owner checks always run. `BODY=gui_test_parity_messages_body.groovy tests/run_gui_test.sh` opens a 3-plane TIFF (written with tifffile from `/home/tester/v-napari`) and checks that a refusal is the sentence alone, that a z-stack is refused for a `YX` tool with the Tools sentence and the way out, that a series, z, t and channel counts are named, and when nothing is refused (18 checks). diff --git a/tests/gui_test_parity_trust_body.groovy b/tests/gui_test_parity_trust_body.groovy index 52eb143..a71814c 100644 --- a/tests/gui_test_parity_trust_body.groovy +++ b/tests/gui_test_parity_trust_body.groovy @@ -1,6 +1,7 @@ // ================================================================== GUI test body: registry trust checks (QP-1; appended to the tool script by tests/run_gui_test.sh) // Needs no image and no app: it writes its own registry entries into $LC_HOME/apps (a scratch folder, emptied first) and calls LcRegistry.load(). -// Owner checks need a second user ("tester") and root to chown; they are skipped (and say so) when chown is not allowed. +// Owner checks need a second user ("tester") and root (or passwordless sudo) to chown; they are skipped (and say so) when chown is not allowed, +// unless LC_REQUIRE_OWNER_CHECKS=1 (CI sets it, .github/workflows/qupath-gui.yml): then a check that cannot be made is a failure, never a skip. def results = [:] def expect = { String name, boolean ok, Object detail = "" -> results[name] = ok @@ -34,7 +35,14 @@ def mk = { String name, Closure entryMods = null, Closure schemaMods = null -> return [entryFile, schemaFile] } def probe = new File(home, "probe"); probe.text = "x" -def canChown = ["chown", "tester", probe.path].execute().waitFor() == 0 +// chown as this user (root), else through sudo without a password prompt (-n: a CI runner); a missing sudo is "not allowed" +def chownTo = { String user, File f -> + if (["chown", user, f.path].execute().waitFor() == 0) return true + try { return ["sudo", "-n", "chown", user, f.path].execute().waitFor() == 0 } catch (IOException noSudo) { return false } +} +def canChown = chownTo("tester", probe) +if (System.getenv("LC_REQUIRE_OWNER_CHECKS") == "1") + expect("trust_owner_checks_can_run", canChown, "LC_REQUIRE_OWNER_CHECKS=1 but this process cannot chown a file to the user 'tester' (create the user; run as root or with passwordless sudo)") mk("trust_ok") mk("trust_group_writable_entry") { e, p -> }.with { chmod("664", it[0]) } @@ -84,8 +92,9 @@ mk("trust_tools_no_fields") { e, p -> } { s -> s.remove("tools_version"); s.remo mk("trust_tools_no_version") { e, p -> } { s -> s.remove("tools_version") } mk("trust_tools_banana") { e, p -> } { s -> s.tools_version = "banana" } mk("trust_tools_newer") { e, p -> } { s -> s.tools_version = "0.10.0"; s.features = ["app_hosts_v1", "reserved_inputs_v1", "retention_prune", "later"] } -mk("trust_owner_other") { e, p -> }.with { if (canChown) assert ["chown", "tester", it[0].path].execute().waitFor() == 0 } -mk("trust_schema_owner_other").with { if (canChown) assert ["chown", "tester", it[1].path].execute().waitFor() == 0 } +mk("trust_owner_other") { e, p -> }.with { if (canChown) assert chownTo("tester", it[0]) } +mk("trust_schema_owner_other").with { if (canChown) assert chownTo("tester", it[1]) } +mk("trust_user_entry_owned_by_root").with { if (canChown) assert chownTo("root", it[0]) } // root may own entries in shared folders only, not in your own def registry = LcRegistry.load() def loaded = registry.apps.keySet(), problems = registry.problems as List @@ -147,6 +156,8 @@ expect("trust_a_good_app_has_no_warning", !(registry.warnings as List).a if (canChown) { refused("trust_owner_other", "entry file is not owned by the current user") refused("trust_schema_owner_other", "schema file is not owned by the current user") + refused("trust_user_entry_owned_by_root", "entry file is not owned by the current user") + expect("trust_root_is_not_accepted_in_the_user_folder", problemOf("trust_user_entry_owned_by_root")?.endsWith("entry file is not owned by the current user") == true, problemOf("trust_user_entry_owned_by_root")) // the sentence for a shared folder ends "or root" } else println "NOTE owner checks skipped: chown is not allowed here" expect("trust_no_null_pointer_text_anywhere", !problems.any { it.contains("null") && !it.contains("not a JSON") && !it.contains("must be") }, problems) // a hostile parameter name must never reach a file name: nothing was written outside the scratch folder