bin/local-pull: Clarify docs, add more tests for corrupted local pulls

I was reading the pull-local command docs and realized it was somewhat unclear
that `--untrusted` *only* applied to local repo pulls; in other words that we
always treat non-local pulls as untrusted.

Tweak the docstring, and add tests that verify this explicitly.

Closes: #1130
Approved by: jlebon

diff --git a/src/ostree/ot-builtin-pull-local.c b/src/ostree/ot-builtin-pull-local.c
index a45957680b68a8fedac2d78f469292d0061a9626..34115a01f6f4a78f201b9927f2edff226c46b8ed 100644 (file)
--- a/src/ostree/ot-builtin-pull-local.c
+++ b/src/ostree/ot-builtin-pull-local.c
@@ -47,7 +47,7 @@ static int opt_depth = 0;
 static GOptionEntry options[] = {
   { "remote", 0, 0, G_OPTION_ARG_STRING, &opt_remote, "Add REMOTE to refspec", "REMOTE" },
   { "disable-fsync", 0, 0, G_OPTION_ARG_NONE, &opt_disable_fsync, "Do not invoke fsync()", NULL },
-  { "untrusted", 0, 0, G_OPTION_ARG_NONE, &opt_untrusted, "Do not trust source", NULL },
+  { "untrusted", 0, 0, G_OPTION_ARG_NONE, &opt_untrusted, "Do not verify checksums of local sources (always enabled for HTTP pulls)", NULL },
   { "bareuseronly-files", 0, 0, G_OPTION_ARG_NONE, &opt_bareuseronly_files, "Reject regular files with mode outside of 0775 (world writable, suid, etc.)", NULL },
   { "require-static-deltas", 0, 0, G_OPTION_ARG_NONE, &opt_require_static_deltas, "Require static deltas", NULL },
   { "gpg-verify", 0, 0, G_OPTION_ARG_NONE, &opt_gpg_verify, "GPG verify commits (must specify --remote)", NULL },

diff --git a/src/ostree/ot-builtin-pull.c b/src/ostree/ot-builtin-pull.c
index ebcde49dbbc6e33f171666436240b665dcc00781..119e66562914ba98dfbafecdcb1fc5fb348003d1 100644 (file)
--- a/src/ostree/ot-builtin-pull.c
+++ b/src/ostree/ot-builtin-pull.c
@@ -57,7 +57,7 @@ static GOptionEntry options[] = {
    { "require-static-deltas", 0, 0, G_OPTION_ARG_NONE, &opt_require_static_deltas, "Require static deltas", NULL },
    { "mirror", 0, 0, G_OPTION_ARG_NONE, &opt_mirror, "Write refs suitable for a mirror and fetches all refs if none provided", NULL },
    { "subpath", 0, 0, G_OPTION_ARG_FILENAME_ARRAY, &opt_subpaths, "Only pull the provided subpath(s)", NULL },
-   { "untrusted", 0, 0, G_OPTION_ARG_NONE, &opt_untrusted, "Do not trust (local) sources", NULL },
+   { "untrusted", 0, 0, G_OPTION_ARG_NONE, &opt_untrusted, "Do not verify checksums of local sources (always enabled for HTTP pulls)", NULL },
    { "bareuseronly-files", 0, 0, G_OPTION_ARG_NONE, &opt_bareuseronly_files, "Reject regular files with mode outside of 0775 (world writable, suid, etc.)", NULL },
    { "dry-run", 0, 0, G_OPTION_ARG_NONE, &opt_dry_run, "Only print information on what will be downloaded (requires static deltas)", NULL },
    { "depth", 0, 0, G_OPTION_ARG_INT, &opt_depth, "Traverse DEPTH parents (-1=infinite) (default: 0)", "DEPTH" },

diff --git a/tests/test-pull-corruption.sh b/tests/test-pull-corruption.sh
index 9dc7d62bb320c3116eb0877fd70719642ab37137..3696acc4c9f51d45619fc8343ff003fc22bc2294 100755 (executable)
--- a/tests/test-pull-corruption.sh
+++ b/tests/test-pull-corruption.sh
@@ -29,7 +29,7 @@ fi
 
 setup_fake_remote_repo1 "archive"
 
-echo '1..2'
+echo '1..3'
 
 repopath=${test_tmpdir}/ostree-srv/gnomerepo
 cp -a ${repopath} ${repopath}.orig
@@ -59,3 +59,44 @@ gjs $(dirname $0)/corrupt-repo-ref.js ${repopath} main || true
 assert_file_has_content corrupted-status.txt 'Changed byte'
 do_corrupt_pull_test
 echo "ok corruption"
+
+if ! skip_one_without_user_xattrs; then
+    # Set up a corrupted commit object
+    rm ostree-srv httpd repo -rf
+    setup_fake_remote_repo1 "archive"
+    rev=$(ostree --repo=ostree-srv/gnomerepo rev-parse main)
+    corruptrev=$(echo ${rev} hello | sha256sum | cut -f 1 -d ' ')
+    assert_not_streq ${rev} ${corruptrev}
+    rev_path=ostree-srv/gnomerepo/objects/${rev:0:2}/${rev:2}.commit
+    corruptrev_path=ostree-srv/gnomerepo/objects/${corruptrev:0:2}/${corruptrev:2}.commit
+    mkdir -p $(dirname ${corruptrev_path})
+    mv ${rev_path} ${corruptrev_path}
+    echo ${corruptrev} > ostree-srv/gnomerepo/refs/heads/main
+    ${CMD_PREFIX} ostree --repo=ostree-srv/gnomerepo summary -u
+    if ${CMD_PREFIX} ostree --repo=ostree-srv/gnomerepo fsck 2>err.txt; then
+        assert_not_reached "fsck with corrupted commit worked?"
+    fi
+    assert_file_has_content err.txt "corrupted object ${corruptrev}\.commit"
+
+    # Do a pull-local; this should succeed since we don't verify checksums
+    # for local repos by default.
+    rm repo err.txt -rf
+    ostree_repo_init repo --mode=bare-user
+    ${CMD_PREFIX} ostree --repo=repo pull-local ostree-srv/gnomerepo main
+
+    rm repo err.txt -rf
+    ostree_repo_init repo --mode=bare-user
+    if ${CMD_PREFIX} ostree --repo=repo pull-local --untrusted ostree-srv/gnomerepo main 2>err.txt; then
+        assert_not_reached "pull-local --untrusted worked?"
+    fi
+    assert_file_has_content err.txt "Corrupted commit object ${corruptrev}.*actual checksum is ${rev}"
+
+    rm repo err.txt -rf
+    ostree_repo_init repo --mode=bare-user
+    ${CMD_PREFIX} ostree --repo=repo remote add --set=gpg-verify=false origin $(cat httpd-address)/ostree/gnomerepo
+    if ${CMD_PREFIX} ostree --repo=repo pull origin main 2>err.txt; then
+        assert_not_reached "pull unexpectedly succeeded!"
+    fi
+    assert_file_has_content err.txt "Corrupted commit object ${corruptrev}.*actual checksum is ${rev}"
+    echo "ok pull commit corruption"
+fi