From: Tracey Emery <tracey@traceyemery.net>
Subject: Re: [gotwebd] add respect_exportok flag to hide/dont expose some repositories
To: Omar Polo <op@omarpolo.com>
Cc: Landry Breuil <landry@openbsd.org>, gameoftrees@openbsd.org
Date: Mon, 31 Oct 2022 10:07:38 -0600

On Mon, Oct 31, 2022 at 05:05:12PM +0100, Omar Polo wrote:
> 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 :)

I'm good with it with this change. And you don't need to wait for me! :)

> 
> 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;
> >  	}
> >  
> >  
> 
> 

-- 

Tracey Emery