From: Omar Polo <op@omarpolo.com>
Subject: Re: make gotd reject sha2 repos earlier
To: Stefan Sperling <stsp@stsp.name>
Cc: gameoftrees@openbsd.org
Date: Wed, 07 Aug 2024 15:01:03 +0200

On 2024/08/07 14:28:00 +0200, Stefan Sperling <stsp@stsp.name> wrote:
> Gotd currently errors when a sha2 repository is found while
> handling a client request.
> 
> It would be better to error out at startup if a sha2 repostiory is
> present in the configuration file so that the admin becomes aware
> of the problem when the service fails to start up.
> 
> ok?
> 
> -----------------------------------------------
>  make gotd(8) reject sha256 repositories at startup for now
>  
>  M  gotd/gotd.c                        |  21+   0-
>  M  lib/read_gitconfig.c               |  15+  22-
>  M  regress/gotd/prepare_test_repo.sh  |   1+   1-
> 
> 3 files changed, 37 insertions(+), 23 deletions(-)
> 
> diff 11f34534de34dc3b24c47b54c86b1fd8aaf8699d ac686c0b234848c2daf18708ae84dd3c7c81eee1
> commit - 11f34534de34dc3b24c47b54c86b1fd8aaf8699d
> commit + ac686c0b234848c2daf18708ae84dd3c7c81eee1
> blob - e087575f78236937985aeb124917d2ff9962a9de
> blob + 5187b871340d649b17625e640cde2a36647502a4
> --- gotd/gotd.c
> +++ gotd/gotd.c
> @@ -2093,6 +2093,27 @@ main(int argc, char **argv)
>  	if (pw->pw_uid == 0)
>  		fatalx("cannot run %s as the superuser", getprogname());
>  
> +	/*
> +	 * SHA2 repositories cannot be used with gotd until Git protocol v2
> +	 * support is added. Reject them at startup for now.
> +	 */
> +	TAILQ_FOREACH(repo, &gotd.repos, entry) {
> +		struct got_repository *r;
> +
> +		error = got_repo_open(&r, repo->path, NULL, NULL);
> +		if (error)
> +			fatalx("%s: %s", repo->path, error->msg);
> +
> +		if (got_repo_get_object_format(r) != GOT_HASH_SHA1) {
> +			error = got_error_msg(GOT_ERR_NOT_IMPL,
> +			    "sha256 object IDs unsupported in network "
> +			    "protocol");
> +			fatalx("%s: %s", repo->path, error->msg);
> +		}
> +
> +		got_repo_close(r);
> +	}

didn't we also allowed repositories to be listed but not yet created?
or maybe i'm remembering wrong.  if it's not, then ok for me.

>  	if (noaction) {
>  		fprintf(stderr, "configuration OK\n");
>  		return 0;
> blob - 10b2b156100b2de5d6fa3f09483b5ac4658b3074
> blob + 9f370db70d268838b93dbe6d96b498def97df472
> --- lib/read_gitconfig.c
> +++ lib/read_gitconfig.c

This is ok op@ in any case.

> --- regress/gotd/prepare_test_repo.sh
> +++ regress/gotd/prepare_test_repo.sh

and this too.