From: Omar Polo <op@omarpolo.com>
Subject: Re: [gotwebd] add respect_exportok flag to hide/dont expose some repositories
To: Landry Breuil <landry@openbsd.org>
Cc: gameoftrees@openbsd.org
Date: Mon, 31 Oct 2022 17:05:12 +0100

On 2022/10/31 16:34:26 +0100, Landry Breuil <landry@openbsd.org> wrote:
> Le Mon, Oct 31, 2022 at 04:04:19PM +0100, Omar Polo a écrit :
> > On 2022/10/31 14:54:16 +0100, Landry Breuil <landry@openbsd.org> wrote:
> > > +	if (srv->respect_exportok) {
> > > +		if (asprintf(&dir_test, "%s/%s/git-daemon-export-ok",
> > 
> > This should be fine, however I'd prefer if we can avoid allocating
> > yet another string and just peek into the directory with faccessat
> > since we've just opened it.
> 
> great idea, wouldnt have thought about it. Attached diff (on top of
> previous, will need to check how to squash both) seems to achieve that,
> works in my limited testing, and reads simpler.

`got up -c main && got he -f' is how you'd spell squash :)

> > > +		    srv->repos_path, repo_dir->name) == -1)
> > > +				return got_error_from_errno("asprintf");
> > 
> > one extra indent level for the return.
> 
> dropped since the asprintf disappeared.
> 
> > Also, what about enabling respect_exportok by default? :)
> 
> i didnt enable it by default to avoid disturbing all existing instances,
> i'm not the one to decide :)

Ah, duh, I haven't thought about the consequences.  probably safer to
keep as it is then.

I'd wait for Tracey to comment, ok op@ if it's fine for him too :)

Thanks!

> Landry
> diff /home/othersrc/got
> commit - 70c522a63bb0def0601951963f7dfd546c08437a
> path + /home/othersrc/got
> blob - 0e5695cd1f8bf1ddc42ba2137e7544ebb541d645
> file + gotwebd/gotweb.c
> --- gotwebd/gotweb.c
> +++ gotwebd/gotweb.c
> @@ -2439,14 +2439,10 @@ done:
>  	}
>  
>  done:
> -	if (srv->respect_exportok) {
> -		if (asprintf(&dir_test, "%s/%s/git-daemon-export-ok",
> -		    srv->repos_path, repo_dir->name) == -1)
> -				return got_error_from_errno("asprintf");
> -		if (access(dir_test, F_OK) == -1) {
> -			error = got_error_path(repo_dir->name, GOT_ERR_NOT_GIT_REPO);
> -			goto err;
> -		}
> +	if (srv->respect_exportok &&
> +	    faccessat(dirfd(dt), "git-daemon-export-ok", F_OK, 0) == -1) {
> +		error = got_error_path(repo_dir->name, GOT_ERR_NOT_GIT_REPO);
> +		goto err;
>  	}
>  
>