From 09b3893571434ae4b44c94e96ad37a84963aee4f Mon Sep 17 00:00:00 2001 From: Seb M'Caw Date: Mon, 13 Jan 2025 16:33:10 +0000 Subject: [PATCH] feat: setting to override trusted sites list (#1819) * Make trusted sites list configurable * Fix Windows test failures * Update user-changes.md * Review --- .gitmodules | 2 +- alire.toml | 7 +- deps/ada-toml | 2 +- doc/publishing.md | 4 +- doc/user-changes.md | 10 +++ src/alire/alire-index.adb | 4 +- src/alire/alire-publish.adb | 72 ++++++++++++----- src/alire/alire-publish.ads | 16 ++-- src/alire/alire-settings-builtins.ads | 12 +++ src/alire/alire.ads | 11 +++ src/alr/alr-commands-publish.adb | 3 +- .../my_index/index/cr/crate/crate-1.0.0.toml | 10 +++ .../untrusted-host/my_index/index/index.toml | 1 + testsuite/tests/index/untrusted-host/test.py | 77 +++++++++++++++++++ .../tests/index/untrusted-host/test.yaml | 4 + .../tests/publish/private-indexes/test.py | 63 ++++++++++++++- testsuite/tests/publish/trusted-sites/test.py | 36 +++++++++ .../tests/publish/trusted-sites/test.yaml | 1 + 18 files changed, 302 insertions(+), 33 deletions(-) create mode 100644 testsuite/tests/index/untrusted-host/my_index/index/cr/crate/crate-1.0.0.toml create mode 100644 testsuite/tests/index/untrusted-host/my_index/index/index.toml create mode 100644 testsuite/tests/index/untrusted-host/test.py create mode 100644 testsuite/tests/index/untrusted-host/test.yaml create mode 100644 testsuite/tests/publish/trusted-sites/test.py create mode 100644 testsuite/tests/publish/trusted-sites/test.yaml diff --git a/.gitmodules b/.gitmodules index f150d0ee9..dfde5214d 100644 --- a/.gitmodules +++ b/.gitmodules @@ -15,7 +15,7 @@ url = https://github.com/alire-project/simple_logging.git [submodule "deps/ada-toml"] path = deps/ada-toml - url = https://github.com/mosteo/ada-toml + url = https://github.com/pmderodat/ada-toml [submodule "deps/gnatcoll-slim"] path = deps/gnatcoll-slim url = https://github.com/alire-project/gnatcoll-core.git diff --git a/alire.toml b/alire.toml index 7ace0fa76..e9e5cdfee 100644 --- a/alire.toml +++ b/alire.toml @@ -16,7 +16,7 @@ executables = ["alr"] [[depends-on]] aaa = "~0.3.0" -ada_toml = "~0.3" +ada_toml = "~0.5" ajunitgen = "^1.0.1" ansiada = "^1.1" c_strings = "^1.0" @@ -56,8 +56,9 @@ url = "https://github.com/mosteo/aaa" commit = "ddfeffe2d6c8f9d19161df7b31d16d37bef4ba71" [pins.ada_toml] -url = "https://github.com/mosteo/ada-toml" -commit = "da4e59c382ceb0de6733d571ecbab7ea4919b33d" +url = "https://github.com/pmderodat/ada-toml" +commit = "e760110ad2b5b776a44dace31b8421532e429fbb" + [pins.ansiada] url = "https://github.com/mosteo/ansi-ada" commit = "0772e48d3e1f640829d142745a36b37edcd5470b" diff --git a/deps/ada-toml b/deps/ada-toml index da4e59c38..e760110ad 160000 --- a/deps/ada-toml +++ b/deps/ada-toml @@ -1 +1 @@ -Subproject commit da4e59c382ceb0de6733d571ecbab7ea4919b33d +Subproject commit e760110ad2b5b776a44dace31b8421532e429fbb diff --git a/doc/publishing.md b/doc/publishing.md index fbaaeae19..9367d4365 100644 --- a/doc/publishing.md +++ b/doc/publishing.md @@ -326,7 +326,9 @@ where the `--for-private-index` switch disables the submission step and certain checks which are only applicable to the community index, and the remaining arguments function as described above. This will generate a manifest file which you can place at the indicated path (relative to the location of `index.toml`) -in your private index. +in your private index. If you are using a remote Git repository which is not on +one of the community index's trusted hosts, you will need to configure it with +the `origins.git.trusted_sites` [setting](settings). One important thing to note is that publishing from a local repository will detect the URL configured as the Git remote (as displayed by diff --git a/doc/user-changes.md b/doc/user-changes.md index 8fbb0c257..0b37a415a 100644 --- a/doc/user-changes.md +++ b/doc/user-changes.md @@ -6,6 +6,16 @@ stay on top of `alr` new features. ## Release `2.1` +### Configurable trusted sites list for Git repositories + +PR [#1819](https://github.com/alire-project/alire/pull/1819) + +The list of hosts which the `alr publish --for-private-index` and +`alr index --check` commands consider to be trusted for Git repository origins +can now be configured with the `origins.git.trusted_sites` settings key. The +existing hard-coded list still applies when using `alr publish` to submit to the +community index. + ### Custom download command for archive crates PR [#1815](https://github.com/alire-project/alire/pull/1815) diff --git a/src/alire/alire-index.adb b/src/alire/alire-index.adb index 7ffdeb100..3037cf626 100644 --- a/src/alire/alire-index.adb +++ b/src/alire/alire-index.adb @@ -129,7 +129,9 @@ package body Alire.Index is for Crate of All_Crates.all loop for Rel of Crate.Releases loop if Rel.Origin.Kind in Origins.VCS_Kinds then - if not Publish.Is_Trusted (Rel.Origin.URL) then + if not Publish.Is_Trusted + (Rel.Origin.URL, For_Community => False) + then OK := False; Put_Warning ("Release " & Rel.Milestone.TTY_Image & " has URL not in known hosts: " diff --git a/src/alire/alire-publish.adb b/src/alire/alire-publish.adb index ba287bb1d..c78561b7f 100644 --- a/src/alire/alire-publish.adb +++ b/src/alire/alire-publish.adb @@ -49,15 +49,6 @@ package body Alire.Publish is use Directories.Operators; use AAA.Strings; - Trusted_Sites : constant AAA.Strings.Vector := - AAA.Strings.Empty_Vector - .Append ("bitbucket.org") - .Append ("github.com") - .Append ("gitlab.com") - .Append ("savannah.gnu.org") - .Append ("savannah.nongnu.org") - .Append ("sf.net"); - Early_Stop : exception; -- Raise this exception from a step to terminate prematurely but without -- generating an error. E.g., if the user doesn't want to submit online @@ -939,13 +930,14 @@ package body Alire.Publish is if Context.Origin.Kind in Origins.VCS_Kinds then - -- Check an VCS origin is from a trusted site, unless we are forcing - -- a local repository. + -- Check a VCS origin is from a trusted site, unless we are forcing + -- a local repository or '--for-private-index' is specified. if (Force and then URI.URI_Kind (URL) in URI.Local_URIs) or else - Is_Trusted (URL) + Is_Trusted + (URL, For_Community => not Context.Options.For_Private_Index) then Put_Success ("Origin is hosted on trusted site: " & URI.Host (URL)); @@ -1084,13 +1076,50 @@ package body Alire.Publish is else No_Steps)); end Directory_Tar; + ------------------- + -- Trusted_Sites -- + ------------------- + + function Trusted_Sites (For_Community : Boolean) return Vector is + Space_Separated : constant String := + (if For_Community then Community_Trusted_Sites + else Settings.Builtins.Origins_Git_Trusted_Sites.Get); + Split_Vector : constant Vector := Split (Space_Separated, ' '); + Sites : Vector := Empty_Vector; + begin + for Site of Split_Vector loop + if Site /= "" then + Sites.Append (Site); + end if; + end loop; + return Sites; + end Trusted_Sites; + + --------------------------- + -- All_Sites_Are_Trusted -- + --------------------------- + + function All_Sites_Are_Trusted (For_Community : Boolean) return Boolean is + Sites : constant Vector := Trusted_Sites (For_Community); + begin + return Sites.Length in 1 and then Sites (1) = "..."; + end All_Sites_Are_Trusted; + ---------------- -- Is_Trusted -- ---------------- - function Is_Trusted (URL : Alire.URL) return Boolean - is (for some Site of Trusted_Sites => URI.Host (URL) = Site - or else Has_Suffix (URI.Host (URL), "." & Site)); + function Is_Trusted (URL : Alire.URL; For_Community : Boolean) + return Boolean + is + Sites : constant Vector := Trusted_Sites (For_Community); + begin + return + All_Sites_Are_Trusted (For_Community) + or else (for some Site of Sites + => URI.Host (URL) = Site + or else Has_Suffix (URI.Host (URL), "." & Site)); + end Is_Trusted; ---------------------- -- Local_Repository -- @@ -1452,11 +1481,16 @@ package body Alire.Publish is -- Print_Trusted_Sites -- ------------------------- - procedure Print_Trusted_Sites is + procedure Print_Trusted_Sites (For_Community : Boolean) is + Sites : constant Vector := Trusted_Sites (For_Community); begin - for Site of Trusted_Sites loop - Trace.Always (Site); - end loop; + if All_Sites_Are_Trusted (For_Community) then + Trace.Always ("All sites are currently trusted for private indexes."); + else + for Site of Sites loop + Trace.Always (Site); + end loop; + end if; end Print_Trusted_Sites; end Alire.Publish; diff --git a/src/alire/alire-publish.ads b/src/alire/alire-publish.ads index 92bc78be7..99cce9e03 100644 --- a/src/alire/alire-publish.ads +++ b/src/alire/alire-publish.ads @@ -47,11 +47,17 @@ package Alire.Publish is & M.Crate.As_String & "-" & M.Version.Image); - procedure Print_Trusted_Sites; - -- Print our list of allowed sites to host git releases - - function Is_Trusted (URL : Alire.URL) return Boolean; - -- According to our whitelist + procedure Print_Trusted_Sites (For_Community : Boolean); + -- Print our list of allowed sites to host git releases. + -- + -- If For_Community is True, the list is the hardcoded + -- Community_Trusted_Sites list, otherwise it is that configured with the + -- 'origins.git.trusted_sites' setting. + + function Is_Trusted (URL : Alire.URL; For_Community : Boolean) + return Boolean; + -- According to the 'origins.git.trusted_sites' setting, or the hardcoded + -- Community_Trusted_Sites if For_Community is True. type Data is tagged limited private; diff --git a/src/alire/alire-settings-builtins.ads b/src/alire/alire-settings-builtins.ads index 1444f0826..d468e0dac 100644 --- a/src/alire/alire-settings-builtins.ads +++ b/src/alire/alire-settings-builtins.ads @@ -126,6 +126,18 @@ package Alire.Settings.Builtins is & " character. The token ${DEST} is replaced by the destination path," & " and ${URL} by the URL to download."); + Origins_Git_Trusted_Sites : constant Builtin := New_Builtin + (Key => "origins.git.trusted_sites", + Kind => Stn_String, + Def => Community_Trusted_Sites, + Global_Only => True, + Help => + "Space-separated list of trusted sites for Git origins, used by" + & " 'alr index --check' and 'alr publish --for-private-index'. If set to" + & " '...', all origins are trusted. Note that this does not have any" + & " effect when using 'alr publish' for submissions to the community" + & " index (which only permits the default list)."); + -- SOLVER Solver_Autonarrow : constant Builtin := New_Builtin diff --git a/src/alire/alire.ads b/src/alire/alire.ads index e9f7e124f..f18ebeca7 100644 --- a/src/alire/alire.ads +++ b/src/alire/alire.ads @@ -52,6 +52,17 @@ package Alire with Preelaborate is Extension_Separator : constant Character := '.'; -- Refers to extension releases! Nothing to do with files + Community_Trusted_Sites : constant String := + "bitbucket.org" + & " github.com" + & " gitlab.com" + & " savannah.gnu.org" + & " savannah.nongnu.org" + & " sf.net"; + -- Space separated list of hosts that are known to not be vulnerable to + -- SHA-1 collision attacks, and therefore trusted for use on the community + -- index. Also used as the default value for 'origins.git.trusted_sites'. + -- Strings that are used quite generally package UStrings renames Ada.Strings.Unbounded; diff --git a/src/alr/alr-commands-publish.adb b/src/alr/alr-commands-publish.adb index 9b1d27e65..f3e7357dc 100644 --- a/src/alr/alr-commands-publish.adb +++ b/src/alr/alr-commands-publish.adb @@ -53,7 +53,8 @@ package body Alr.Commands.Publish is Cmd.Auto_Update_Index; if Cmd.Print_Trusted then - Alire.Publish.Print_Trusted_Sites; + Alire.Publish.Print_Trusted_Sites + (For_Community => not Cmd.For_Private_Index); elsif Cmd.Tar then diff --git a/testsuite/tests/index/untrusted-host/my_index/index/cr/crate/crate-1.0.0.toml b/testsuite/tests/index/untrusted-host/my_index/index/cr/crate/crate-1.0.0.toml new file mode 100644 index 000000000..16a1272d1 --- /dev/null +++ b/testsuite/tests/index/untrusted-host/my_index/index/cr/crate/crate-1.0.0.toml @@ -0,0 +1,10 @@ +description = "Sample crate" +name = "crate" +version = "1.0.0" +licenses = "GPL-3.0-only" +maintainers = ["any@bo.dy"] +maintainers-logins = ["someone"] + +[origin] +url = "git+https://some.host/path/to/repo" +commit = "0000000000000000000000000000000000000000" diff --git a/testsuite/tests/index/untrusted-host/my_index/index/index.toml b/testsuite/tests/index/untrusted-host/my_index/index/index.toml new file mode 100644 index 000000000..bad265e4f --- /dev/null +++ b/testsuite/tests/index/untrusted-host/my_index/index/index.toml @@ -0,0 +1 @@ +version = "1.1" diff --git a/testsuite/tests/index/untrusted-host/test.py b/testsuite/tests/index/untrusted-host/test.py new file mode 100644 index 000000000..ea1ff29b2 --- /dev/null +++ b/testsuite/tests/index/untrusted-host/test.py @@ -0,0 +1,77 @@ +""" +Check detection of untrusted hosts when running `alr index --check` +""" + + +import os + +from drivers.alr import run_alr, alr_settings_set +from drivers.asserts import assert_eq, assert_match +from drivers.helpers import replace_in_file + + +# `alr index --check` should fail due to untrusted host. +p = run_alr("index", "--check", quiet=False, complain_on_error=False) +assert_match( + ( + r"Warning: Release crate=1\.0\.0 has URL not in known hosts: " + r"https://some\.host/path/to/repo" + r".*ERROR: Issues were found in index contents" + ), + p.out +) + +# Configure the untrusted host as trusted. +alr_settings_set("origins.git.trusted_sites", "github.com some.host gitlab.com") + +# `alr index --check` should now succeed. +p = run_alr("index", "--check", quiet=False) +assert_eq("Success: No issues found in index contents.\n", p.out) + +# Change the index manifest to use a new untrusted host, and check that +# `alr index --check` fails again. +manifest_path = os.path.join( + "my_index", "index", "cr", "crate", "crate-1.0.0.toml" +) +replace_in_file( + manifest_path, + "git+https://some.host/path/to/repo", + "git+https://untrusted.host/path/to/repo" +) +p = run_alr("index", "--check", quiet=False, complain_on_error=False) +assert_match( + ( + r"Warning: Release crate=1\.0\.0 has URL not in known hosts: " + r"https://untrusted\.host/path/to/repo" + r".*ERROR: Issues were found in index contents" + ), + p.out +) + +# Verify that it still fails when the host has a trailing dot (note the double +# space in the setting value, which was causing the empty string to be +# considered a trusted host, and hence `untrusted.host.` to be considered a +# trusted subdomain thereof). +replace_in_file( + manifest_path, + "git+https://untrusted.host/path/to/repo", + "git+https://untrusted.host./path/to/repo" +) +p = run_alr("index", "--check", quiet=False, complain_on_error=False) +assert_match( + ( + r"Warning: Release crate=1\.0\.0 has URL not in known hosts: " + r"https://untrusted\.host\./path/to/repo" + r".*ERROR: Issues were found in index contents" + ), + p.out +) + +# Set 'origins.git.trusted_sites' to '...' and verify that all hosts are now +# permitted. +alr_settings_set("origins.git.trusted_sites", "...") +p = run_alr("index", "--check", quiet=False) +assert_eq("Success: No issues found in index contents.\n", p.out) + + +print("SUCCESS") diff --git a/testsuite/tests/index/untrusted-host/test.yaml b/testsuite/tests/index/untrusted-host/test.yaml new file mode 100644 index 000000000..0a859639c --- /dev/null +++ b/testsuite/tests/index/untrusted-host/test.yaml @@ -0,0 +1,4 @@ +driver: python-script +indexes: + my_index: + in_fixtures: false diff --git a/testsuite/tests/publish/private-indexes/test.py b/testsuite/tests/publish/private-indexes/test.py index 736bafed2..7cf914956 100644 --- a/testsuite/tests/publish/private-indexes/test.py +++ b/testsuite/tests/publish/private-indexes/test.py @@ -8,7 +8,7 @@ import shutil import subprocess -from drivers.alr import run_alr, run_alr_interactive +from drivers.alr import run_alr, run_alr_interactive, alr_settings_set from drivers.helpers import init_git_repo, WrapCommand from drivers.asserts import assert_match, assert_file_exists @@ -140,6 +140,12 @@ def test( shutil.rmtree(idx_manifest_dir, ignore_errors=True) # may not exist +# Configure a non-default list of trusted sites +alr_settings_set( + "origins.git.trusted_sites", + "bitbucket.org trusted.host github.com" +) + # All tests should behave the same with and without "--force" for force_arg in ([], ["--force"]): # A crate suitable for the community index: @@ -263,6 +269,61 @@ def test( expect_success=True ) + # A crate unsuitable for the community index because it uses an origin with + # an untrusted host, but suitable for a private index because the user has + # configured the host as trusted: + # + # "alr publish" and "alr publish --skip-submit" should fail. + test( + args=force_arg + ["publish"], + url="https://trusted.host/some_user/repo-name", + maint_logins='["github-username"]', + num_confirms=1, + output=[r".*Origin is hosted on unknown site: trusted\.host.*"], + gen_manifest=None, + expect_success=False + ) + test( + args=force_arg + ["publish", "--skip-submit"], + url="https://trusted.host/some_user/repo-name", + maint_logins='["github-username"]', + num_confirms=1, + output=[r".*Origin is hosted on unknown site: trusted\.host.*"], + gen_manifest=None, + expect_success=False + ) + # "alr publish --for-private-index" will succeed. + test( + args=force_arg + ["publish", "--for-private-index"], + url="https://trusted.host/some_user/repo-name", + maint_logins='["github-username"]', + num_confirms=2, + output=[ + r".*Success: Your index manifest file has been generated.*", + r".*Please upload this file to the index in the xx/xxx/ subdirectory", + ], + gen_manifest=[ + r'.*url = "git\+https://trusted\.host/some_user/repo-name".*', + ], + expect_success=True + ) + + # A crate unsuitable for both the community index and a private index + # because it uses an origin with an untrusted host, which the user has not + # configured as trusted: + # + # "alr publish [--skip-submit | --for-private-index]" should all fail. + for switch in ([], ["--skip-submit"], ["--for-private-index"]): + test( + args=force_arg + ["publish"] + switch, + url="https://untrusted.host/some_user/repo-name", + maint_logins='["github-username"]', + num_confirms=1, + output=[r".*Origin is hosted on unknown site: untrusted\.host.*"], + gen_manifest=None, + expect_success=False + ) + # A crate unsuitable for the community index because it has a # "maintainers-logins" value which is invalid for GitHub: # diff --git a/testsuite/tests/publish/trusted-sites/test.py b/testsuite/tests/publish/trusted-sites/test.py new file mode 100644 index 000000000..054a504dc --- /dev/null +++ b/testsuite/tests/publish/trusted-sites/test.py @@ -0,0 +1,36 @@ +""" +Check `alr publish --trusted-sites` +""" + + +from drivers.alr import run_alr, alr_settings_set +from drivers.asserts import assert_eq, assert_substring + + +# Configure a non-default trusted sites list +alr_settings_set("origins.git.trusted_sites", "some.host other.host third.host") + +# Verify that `alr publish --trusted-sites` prints the hardcoded list of hosts +# trusted by the community index +p = run_alr("publish", "--trusted-sites") +assert_substring("\ngithub.com\n", p.out) + +# Verify that `alr publish --for-private-index --trusted-sites` prints the +# configured list +p = run_alr("publish", "--for-private-index", "--trusted-sites") +assert_eq("some.host\nother.host\nthird.host\n", p.out) + +# Set `origins.git.trusted_sites` to '...' (which trusts all hosts) +alr_settings_set("origins.git.trusted_sites", "...") + +# Verify that the output of `alr publish --trusted-sites` is unchanged +p = run_alr("publish", "--trusted-sites") +assert_substring("\ngithub.com\n", p.out) + +# Verify that `alr publish --for-private-index --trusted-sites` prints a +# suitable message +p = run_alr("publish", "--for-private-index", "--trusted-sites") +assert_eq("All sites are currently trusted for private indexes.\n", p.out) + + +print('SUCCESS') diff --git a/testsuite/tests/publish/trusted-sites/test.yaml b/testsuite/tests/publish/trusted-sites/test.yaml new file mode 100644 index 000000000..32c747b3f --- /dev/null +++ b/testsuite/tests/publish/trusted-sites/test.yaml @@ -0,0 +1 @@ +driver: python-script