From e425e06d26b30fb67200d92611d53af935f38742 Mon Sep 17 00:00:00 2001 From: Abhijeet Kasurde Date: Fri, 19 Sep 2025 14:35:07 -0700 Subject: [PATCH 1/5] apt_repository: validate line in sources.list * According to `sources.list(5)` man pages, only four fields are mandatory Types, URIs, Suites, Component. Validate as per this requirement while adding new source in sources.list. Fixes: #85715 Signed-off-by: Abhijeet Kasurde --- changelogs/fragments/apt_repository.yml | 3 + lib/ansible/modules/apt_repository.py | 73 ++++++++++++++++--- .../targets/apt_repository/tasks/apt.yml | 13 ++++ test/units/modules/test_apt_repository.py | 25 +++++++ 4 files changed, 105 insertions(+), 9 deletions(-) create mode 100644 changelogs/fragments/apt_repository.yml create mode 100644 test/units/modules/test_apt_repository.py diff --git a/changelogs/fragments/apt_repository.yml b/changelogs/fragments/apt_repository.yml new file mode 100644 index 00000000000..2b4dfaefc63 --- /dev/null +++ b/changelogs/fragments/apt_repository.yml @@ -0,0 +1,3 @@ +--- +bugfixes: + - apt_repository - validate the line in sources.list (https://github.com/ansible/ansible/issues/85715). diff --git a/lib/ansible/modules/apt_repository.py b/lib/ansible/modules/apt_repository.py index 5be21c2b0c5..21466f0ad7c 100644 --- a/lib/ansible/modules/apt_repository.py +++ b/lib/ansible/modules/apt_repository.py @@ -282,6 +282,62 @@ class SourcesList(object): return '%s.list' % _cleanup_filename(' '.join(parts[:1])) + @staticmethod + def _validate_source(source: str) -> bool: + """ + Validate a source string according to the SOURCES.LIST(5). + See: https://manpages.debian.org/trixie/apt/sources.list.5.en.html#ONE-LINE-STYLE_FORMAT + + Args: + source (str): The source string to validate. + + Returns: + bool: True if the source string is valid, False otherwise. + """ + parts = source.split() + + if not parts: + return False + + # 2. Extract the type and handle options + entry_type = parts[0] + if entry_type not in VALID_SOURCE_TYPES: + return False + + # Check for options enclosed in square brackets + # The first element after the type might be the start of options + if len(parts) > 1 and parts[1].startswith('['): + if parts[1].endswith(']'): + # For single-word options + remaining_parts = parts[2:] + else: + # For multi-word options + end_bracket_index = -1 + for i, part in enumerate(parts[2:], start=2): + if part.endswith(']'): + end_bracket_index = i + break + + if end_bracket_index != -1: + remaining_parts = parts[end_bracket_index + 1:] + else: + # Malformed options, treat the whole thing as a single part for now. + remaining_parts = parts[1:] + return False + else: + remaining_parts = parts[1:] + + # According to `sources.list(5)` man pages, only four fields are mandatory: + # * `Types` either `deb` or/and `deb-src` + # * `URIs` to repositories holding valid APT structure (unclear if multiple are allowed) + # * `Suites` usually being distribution codenames + # * `Component` most of the time `main`, but it's a section of the repository + if len(remaining_parts) < 3: + # Invalid line format + return False + + return True + def _parse(self, line, raise_if_invalid_or_disabled=False): valid = False enabled = True @@ -303,10 +359,9 @@ class SourcesList(object): # Duplicated whitespaces in a valid source spec will be removed. source = line.strip() if source: - chunks = source.split() - if chunks[0] in VALID_SOURCE_TYPES: - valid = True - source = ' '.join(chunks) + valid = self._validate_source(source) + if raise_if_invalid_or_disabled and not valid: + raise InvalidSource(source) if raise_if_invalid_or_disabled and (not valid or not enabled): raise InvalidSource(line) @@ -315,11 +370,11 @@ class SourcesList(object): def load(self, file): group = [] - f = open(file, 'r') - for n, line in enumerate(f): - valid, enabled, source, comment = self._parse(line) - group.append((n, valid, enabled, source, comment)) - self.files[file] = group + with open(file, 'r') as f: + for n, line in enumerate(f): + valid, enabled, source, comment = self._parse(line) + group.append((n, valid, enabled, source, comment)) + self.files[file] = group def save(self): for filename, sources in list(self.files.items()): diff --git a/test/integration/targets/apt_repository/tasks/apt.yml b/test/integration/targets/apt_repository/tasks/apt.yml index f1706ea0302..53fdfc6e175 100644 --- a/test/integration/targets/apt_repository/tasks/apt.yml +++ b/test/integration/targets/apt_repository/tasks/apt.yml @@ -354,6 +354,19 @@ - name: uninstall local-apt-repository with apt apt: pkg=local-apt-repository state=absent purge=yes +# Invalid repo with no component +- name: Add repo with empty component + ansible.builtin.apt_repository: + repo: "deb http://archive.canonical.com/ubuntu jammy" + state: present + register: emptycomp_result + +- name: Assert that the repo was not added + assert: + that: + - emptycomp_result is failed + - emptycomp_result.msg == 'Invalid repository string: deb http://archive.canonical.com/ubuntu jammy' + # # TEST: PPA HTTPS URL # diff --git a/test/units/modules/test_apt_repository.py b/test/units/modules/test_apt_repository.py new file mode 100644 index 00000000000..ed44250aa94 --- /dev/null +++ b/test/units/modules/test_apt_repository.py @@ -0,0 +1,25 @@ +# Copyright: Contributors to the Ansible project +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) +from __future__ import annotations + +import pytest + +from ansible.modules.apt_repository import SourcesList + + +@pytest.mark.parametrize( + "line, expected", [ + pytest.param("deb http://deb.debian.org/debian stable main contrib non-free", True, id="valid_line"), + pytest.param("# This is a commented line that should be ignored", False, id="commented_line"), + pytest.param("deb http://ftp.us.debian.org/debian sid main", True, id="no_options_line"), + pytest.param("deb [arch=amd64,i386] http://ftp.us.debian.org/debian sid main", True, id="multi_arch_option_line"), + pytest.param("deb [trusted=yes arch=amd64] https://example.com/debian focal", False, id="invalid_line"), + pytest.param("deb [trusted=yes arch=amd64] https://example.com/debian focal main", True, id="trusted_option_line"), + pytest.param("deb [trusted=yes signed-by=/etc/apt/key.gpg] http://my.repo.com/ubuntu focal-updates main", True, id="signed_by_option_line"), + pytest.param("deb-src [arch=amd64 trusted=yes] http://my.repo.com/ubuntu focal main universe", True, id="multiple_components_line"), + pytest.param("deb [arch=amd64,i386 trusted=yes] http://my.repo.com/ubuntu focal main", True, id="multiple_arch_trusted_option_line"), + pytest.param("deb [arch=amd64,i386] http://archive.ubuntu.com/ubuntu/ xenial-updates main restricted # a comment at the end", True, id="comment_at_end_line"), + ] +) +def test_validate(line, expected): + assert SourcesList._validate_source(line) == expected From 5b322d0208965fe9c493d1f6e55acb332a02556c Mon Sep 17 00:00:00 2001 From: Abhijeet Kasurde Date: Fri, 19 Sep 2025 14:55:31 -0700 Subject: [PATCH 2/5] Make CI green Signed-off-by: Abhijeet Kasurde --- test/integration/targets/apt_repository/tasks/apt.yml | 1 + test/units/modules/test_apt_repository.py | 6 +++++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/test/integration/targets/apt_repository/tasks/apt.yml b/test/integration/targets/apt_repository/tasks/apt.yml index 53fdfc6e175..380a1f751b1 100644 --- a/test/integration/targets/apt_repository/tasks/apt.yml +++ b/test/integration/targets/apt_repository/tasks/apt.yml @@ -360,6 +360,7 @@ repo: "deb http://archive.canonical.com/ubuntu jammy" state: present register: emptycomp_result + ignore_errors: true - name: Assert that the repo was not added assert: diff --git a/test/units/modules/test_apt_repository.py b/test/units/modules/test_apt_repository.py index ed44250aa94..bd43c5f3d72 100644 --- a/test/units/modules/test_apt_repository.py +++ b/test/units/modules/test_apt_repository.py @@ -18,7 +18,11 @@ from ansible.modules.apt_repository import SourcesList pytest.param("deb [trusted=yes signed-by=/etc/apt/key.gpg] http://my.repo.com/ubuntu focal-updates main", True, id="signed_by_option_line"), pytest.param("deb-src [arch=amd64 trusted=yes] http://my.repo.com/ubuntu focal main universe", True, id="multiple_components_line"), pytest.param("deb [arch=amd64,i386 trusted=yes] http://my.repo.com/ubuntu focal main", True, id="multiple_arch_trusted_option_line"), - pytest.param("deb [arch=amd64,i386] http://archive.ubuntu.com/ubuntu/ xenial-updates main restricted # a comment at the end", True, id="comment_at_end_line"), + pytest.param( + "deb [arch=amd64,i386] http://archive.ubuntu.com/ubuntu/ xenial-updates main restricted # a comment at the end", + True, + id="comment_at_end_line" + ), ] ) def test_validate(line, expected): From ec99a45be3fc41c9bdd7ae07e7c5eb3a3c979e15 Mon Sep 17 00:00:00 2001 From: Abhijeet Kasurde Date: Fri, 19 Sep 2025 15:49:03 -0700 Subject: [PATCH 3/5] Make CI green II Signed-off-by: Abhijeet Kasurde --- test/integration/targets/apt_repository/tasks/apt.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/targets/apt_repository/tasks/apt.yml b/test/integration/targets/apt_repository/tasks/apt.yml index 380a1f751b1..3cd08404644 100644 --- a/test/integration/targets/apt_repository/tasks/apt.yml +++ b/test/integration/targets/apt_repository/tasks/apt.yml @@ -366,7 +366,7 @@ assert: that: - emptycomp_result is failed - - emptycomp_result.msg == 'Invalid repository string: deb http://archive.canonical.com/ubuntu jammy' + - "'Invalid repository string' in emptycomp_result.msg" # # TEST: PPA HTTPS URL From a425438672bd5e16221d83b61906c53bdcc8a673 Mon Sep 17 00:00:00 2001 From: Abhijeet Kasurde Date: Wed, 1 Oct 2025 09:18:00 -0700 Subject: [PATCH 4/5] Review request Signed-off-by: Abhijeet Kasurde --- lib/ansible/modules/apt_repository.py | 7 ++++--- test/units/modules/test_apt_repository.py | 2 ++ 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/lib/ansible/modules/apt_repository.py b/lib/ansible/modules/apt_repository.py index 21466f0ad7c..605bac5ea58 100644 --- a/lib/ansible/modules/apt_repository.py +++ b/lib/ansible/modules/apt_repository.py @@ -332,7 +332,10 @@ class SourcesList(object): # * `URIs` to repositories holding valid APT structure (unclear if multiple are allowed) # * `Suites` usually being distribution codenames # * `Component` most of the time `main`, but it's a section of the repository - if len(remaining_parts) < 3: + if remaining_parts[1].endswith('/') and len(remaining_parts) > 2: + # Suites with trailing slash makes component optional + return False + elif not remaining_parts[1].endswith('/') and len(remaining_parts) < 3: # Invalid line format return False @@ -360,8 +363,6 @@ class SourcesList(object): source = line.strip() if source: valid = self._validate_source(source) - if raise_if_invalid_or_disabled and not valid: - raise InvalidSource(source) if raise_if_invalid_or_disabled and (not valid or not enabled): raise InvalidSource(line) diff --git a/test/units/modules/test_apt_repository.py b/test/units/modules/test_apt_repository.py index bd43c5f3d72..1deea9a1dc2 100644 --- a/test/units/modules/test_apt_repository.py +++ b/test/units/modules/test_apt_repository.py @@ -12,6 +12,8 @@ from ansible.modules.apt_repository import SourcesList pytest.param("deb http://deb.debian.org/debian stable main contrib non-free", True, id="valid_line"), pytest.param("# This is a commented line that should be ignored", False, id="commented_line"), pytest.param("deb http://ftp.us.debian.org/debian sid main", True, id="no_options_line"), + pytest.param("deb-src http://ftp.debian.org/debian/ experimental/", True, id="suite_with_slash"), + pytest.param("deb-src http://ftp.debian.org/debian/ experimental/ main", False, id="suite_with_slash_and_component"), pytest.param("deb [arch=amd64,i386] http://ftp.us.debian.org/debian sid main", True, id="multi_arch_option_line"), pytest.param("deb [trusted=yes arch=amd64] https://example.com/debian focal", False, id="invalid_line"), pytest.param("deb [trusted=yes arch=amd64] https://example.com/debian focal main", True, id="trusted_option_line"), From 254bc81260cae389eaad3709410e3ab9b3f821c3 Mon Sep 17 00:00:00 2001 From: Abhijeet Kasurde Date: Wed, 1 Oct 2025 12:41:28 -0700 Subject: [PATCH 5/5] Review request Signed-off-by: Abhijeet Kasurde --- lib/ansible/modules/apt_repository.py | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/lib/ansible/modules/apt_repository.py b/lib/ansible/modules/apt_repository.py index 605bac5ea58..6bef25d0858 100644 --- a/lib/ansible/modules/apt_repository.py +++ b/lib/ansible/modules/apt_repository.py @@ -287,19 +287,13 @@ class SourcesList(object): """ Validate a source string according to the SOURCES.LIST(5). See: https://manpages.debian.org/trixie/apt/sources.list.5.en.html#ONE-LINE-STYLE_FORMAT - - Args: - source (str): The source string to validate. - - Returns: - bool: True if the source string is valid, False otherwise. """ parts = source.split() if not parts: return False - # 2. Extract the type and handle options + # Extract the type and handle options entry_type = parts[0] if entry_type not in VALID_SOURCE_TYPES: return False @@ -335,7 +329,7 @@ class SourcesList(object): if remaining_parts[1].endswith('/') and len(remaining_parts) > 2: # Suites with trailing slash makes component optional return False - elif not remaining_parts[1].endswith('/') and len(remaining_parts) < 3: + if not remaining_parts[1].endswith('/') and len(remaining_parts) < 3: # Invalid line format return False