From dc837bd6b8189ffe8838b6a7291a0c78871d3a3d Mon Sep 17 00:00:00 2001 From: Veiko Aasa Date: Tue, 23 Sep 2025 12:25:43 +0300 Subject: [PATCH] gitweb: Use Git credential helper when cloning URLs with credentials This prevents logging usernames and passwords to the journal logs and to the Git repo configuration. Also, avoids usernames and passwords appear in the process list when cloning a repository. Tests performed: - Create a new repository by cloning an existing repository URL with basic auth credentials. Check that: - Cloning succeeds. - Journal logs don't contain URLs with credential info. - The configuration of the cloned repository doesn't contain credential info. - Try to clone a non-existing repository URL that contains credential info. Cloning fails and there are no credential info in the journal logs. - Cloning a public git repository without credential info succeeds. - All the gitweb module tests pass. Signed-off-by: Veiko Aasa [sunil: Add/fix some more type hints] [sunil: Add tests for URL parsing] Signed-off-by: Sunil Mohan Adapa Reviewed-by: Sunil Mohan Adapa --- plinth/modules/gitweb/__init__.py | 2 +- plinth/modules/gitweb/privileged.py | 78 +++++++++++++------ .../modules/gitweb/tests/test_privileged.py | 25 ++++++ 3 files changed, 81 insertions(+), 24 deletions(-) diff --git a/plinth/modules/gitweb/__init__.py b/plinth/modules/gitweb/__init__.py index 3dcd79829..00c751e7a 100644 --- a/plinth/modules/gitweb/__init__.py +++ b/plinth/modules/gitweb/__init__.py @@ -34,7 +34,7 @@ class GitwebApp(app_module.App): app_id = 'gitweb' - _version = 3 + _version = 4 def __init__(self) -> None: """Create components for the app.""" diff --git a/plinth/modules/gitweb/privileged.py b/plinth/modules/gitweb/privileged.py index bf69f0cb4..332d42af5 100644 --- a/plinth/modules/gitweb/privileged.py +++ b/plinth/modules/gitweb/privileged.py @@ -4,14 +4,16 @@ import configparser import logging import os +import pathlib import re import shutil import subprocess import time from typing import Any +from urllib import parse from plinth import action_utils -from plinth.actions import privileged +from plinth.actions import privileged, secret_str from plinth.modules.gitweb.forms import RepositoryValidator, get_name_from_url from plinth.modules.gitweb.manifest import GIT_REPO_PATH, REPO_DIR_OWNER @@ -27,7 +29,7 @@ def validate_repo_name(name: str) -> str: return name -def validate_repo_url(url: str) -> str: +def validate_repo_url(url: secret_str) -> secret_str: """Validate a repository URL.""" RepositoryValidator(input_should_be='url')(url) return url @@ -35,33 +37,58 @@ def validate_repo_url(url: str) -> str: @privileged def setup(): - """Disable default Apache2 Gitweb configuration.""" + """Configure Gitweb module.""" + # Disable default Apache2 Gitweb configuration. action_utils.webserver_disable('gitweb') - if not _get_global_default_branch(): - _set_global_default_branch('main') + + # Configure Git client. + if not _get_git_global_config('init.defaultBranch'): + _set_git_global_config('init.defaultBranch', 'main') + if not _get_git_global_config('credential.helper'): + _set_git_global_config('credential.helper', 'cache') -def _get_global_default_branch(): - """Get globally configured default branch name.""" +def _get_git_global_config(key: str) -> str | None: + """Return a value from Git global configuration.""" try: - default_branch = action_utils.run( - ['git', 'config', '--global', '--get', 'init.defaultBranch'], - check=True).stdout.decode().strip() + value = action_utils.run(['git', 'config', '--global', '--get', key], + check=True).stdout.decode().strip() except subprocess.CalledProcessError as exception: - if exception.returncode == 1: # Default branch not configured + if exception.returncode == 1: # Configuration option doesn't exist return None raise - return default_branch + return value -def _set_global_default_branch(name): - """Configure default branch name globally.""" - action_utils.run(['git', 'config', '--global', 'init.defaultBranch', name], - check=True) +def _set_git_global_config(key: str, value: str) -> None: + """Set a Git global configuration value.""" + action_utils.run(['git', 'config', '--global', key, value], check=True) -def _clone_with_progress_report(url, repo_dir): +def _setup_git_credentials(url: secret_str) -> str: + """Set up git credential helper and return URL without credentials.""" + url_parts = parse.urlsplit(url) + safe_netloc = url_parts.netloc.split('@')[-1] + safe_url = url_parts._replace(netloc=safe_netloc).geturl() + username = url_parts.username or '' + password = url_parts.password or '' + + if username or password: + # Feed credentials to Git credential helper + input = (f'protocol={url_parts.scheme}\n' + f'host={safe_netloc}\n' + f'username={username}\n' + f'password={password}\n\n') + env = dict(os.environ, GIT_TERMINAL_PROMPT='0') + action_utils.run(['git', 'credential', 'approve'], + input=input.encode(), stdout=subprocess.DEVNULL, + check=True, env=env) + + return safe_url + + +def _clone_with_progress_report(url: secret_str, repo_dir: pathlib.Path): """Clone a repository and write progress info to the file.""" starttime = time.time() status_file = repo_dir / 'clone_progress' @@ -70,9 +97,12 @@ def _clone_with_progress_report(url, repo_dir): env = dict(os.environ, GIT_TERMINAL_PROMPT='0', LC_ALL='C', GIT_HTTP_LOW_SPEED_LIMIT='100', GIT_HTTP_LOW_SPEED_TIME='60') + safe_url = _setup_git_credentials(url) + logger.info(f'Cloning Git repository {safe_url} ...') proc = subprocess.Popen( - ['git', 'clone', '--bare', '--progress', url, + ['git', 'clone', '--bare', '--progress', safe_url, str(repo_temp_dir)], stderr=subprocess.PIPE, text=True, env=env) + assert proc.stderr is not None # write clone progress to the file errors = [] @@ -108,7 +138,7 @@ def _clone_with_progress_report(url, repo_dir): raise RuntimeError('Git repository cloning failed.', errors) -def _prepare_clone_repo(url: str, is_private: bool): +def _prepare_clone_repo(url: secret_str, is_private: bool): """Prepare cloning a repository.""" repo_name = get_name_from_url(url) if not repo_name.endswith('.git'): @@ -150,7 +180,8 @@ def _clone_status_line_to_percent(line): return None -def _clone_repo(url: str, description: str, owner: str, keep_ownership: bool): +def _clone_repo(url: secret_str, description: str, owner: str, + keep_ownership: bool): """Clone a repository.""" repo = get_name_from_url(url) if not repo.endswith('.git'): @@ -343,7 +374,7 @@ def repo_info(name: str) -> dict[str, str]: @privileged -def create_repo(url: str | None = None, name: str | None = None, +def create_repo(url: secret_str | None = None, name: str | None = None, description: str = '', owner: str = '', keep_ownership: bool = False, is_private: bool = False, skip_prepare: bool = False, prepare_only: bool = False): @@ -365,12 +396,13 @@ def create_repo(url: str | None = None, name: str | None = None, @privileged -def repo_exists(url: str) -> bool: +def repo_exists(url: secret_str) -> bool: """Return whether remote repository exists.""" url = validate_repo_url(url) + safe_url = _setup_git_credentials(url) env = dict(os.environ, GIT_TERMINAL_PROMPT='0') try: - action_utils.run(['git', 'ls-remote', url, 'HEAD'], timeout=10, + action_utils.run(['git', 'ls-remote', safe_url, 'HEAD'], timeout=10, env=env, check=True) return True except subprocess.CalledProcessError: diff --git a/plinth/modules/gitweb/tests/test_privileged.py b/plinth/modules/gitweb/tests/test_privileged.py index f9ddf6d12..cc884c501 100644 --- a/plinth/modules/gitweb/tests/test_privileged.py +++ b/plinth/modules/gitweb/tests/test_privileged.py @@ -2,6 +2,8 @@ """Test module for gitweb module operations.""" import pathlib +import subprocess +from unittest.mock import call, patch import pytest from django.forms import ValidationError @@ -121,3 +123,26 @@ def test_action_create_repo_with_invalid_urls(url): with pytest.raises(ValidationError): privileged.create_repo(url=url, description='', owner='', keep_ownership=True) + + +@patch('plinth.action_utils.run') +def test_setup_git_creentials(run): + """Test that setting up git credentials works.""" + url = 'https://user:pass@host.example/path?key=value' + safe_url = privileged._setup_git_credentials(url) + assert safe_url == 'https://host.example/path?key=value' + + input_ = b'protocol=https\nhost=host.example\nusername=user\n' \ + b'password=pass\n\n' + env = run.mock_calls[0].kwargs.pop('env') + assert env['GIT_TERMINAL_PROMPT'] == '0' + assert run.mock_calls == [ + call(['git', 'credential', 'approve'], input=input_, + stdout=subprocess.DEVNULL, check=True) + ] + + run.reset_mock() + url = 'https://host2.example/path?key=value' + safe_url = privileged._setup_git_credentials(url) + assert safe_url == 'https://host2.example/path?key=value' + run.assert_not_called()