[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] importinfo/admininfo
Mark D. Baushke
Re: [PATCH] importinfo/admininfo
Sat, 04 Oct 2003 15:20:01 -0700
-----BEGIN PGP SIGNED MESSAGE-----
Ralf S. Engelschall <address@hidden> writes:
> On Sat, Oct 04, 2003, Mark D. Baushke wrote:
> > Ralf S. Engelschall <address@hidden> writes:
> > > The patch adds two still missing and very important auditing hook
> > > facilities to CVS: "CVSROOT/importinfo" for auditing "cvs import"
> > > operations and "CVSROOT/admininfo" for auditing "cvs admin" operations.
> > > I was prompted to implement these some years ago when I wrote OSSP
> > > Shiela (see http://www.ossp.org/pkg/tool/shiela/ for details) and had
> > > to recognize that it still could be circumvented by the two remaining
> > > repository-destructive operations "cvs import" and "cvs admin".
> > I believe it is still possible for a 'cvs import' to destroy important
> > cvs branch and version tags. If the intent is to help lock down the
> > 'cvs import' command, then more work is probably needed...
> Can you give more details, please? Why can "cvs import" still
> destroy any tags if an "importinfo" filter would stop processing? My
> "importinfo" hook is very early in the "cvs import" processing and AFAIK
> there is no repository write access before it.
I was under the impression that the importinfo_runproc was running the
filter and passing the importinfo_vtag, the repository and the files
being imported. Is the vendor branch getting to the script thru some
I suppose this is not a big deal as it should only really matter for
tags that are for real RCS branches as opposed to CVS magic branches,
but I wasn't 100% sure about failure modes...
Another fairly minor matter is that the -k options is not known at
import time and might be fairly critical to folks that might want
the -kb default for 'binary' files to be checked... I find it less
important to worry about the -k option on a cvs commit than on an
import. However, I suppose that both of them really should be treated
the same, so I am just noting in passing that a server on a Windows box
may be at more of a disadvantage in understanding how the imported files
are to be opened for verification than their non-windows counterparts.
> > [...]
> > I will note that the man pages are not the primary reference
> > documentation for cvs, so it would be desirable for you to extend the
> > docuemntation in the doc/cvs.texinfo file as well.
> I've looked at the doc/cvs.texinfo file and have not found any chapter
> which really deals with the xxxxinfo hooks. There are just a few
> references and small examples at various places for some hooks, but
> no general documentation on them. So I decided to document them in
> cvs(5), because there at least mostly all xxxxinfo hooks _are_ already
I understand the documentation is light, but just as there is an '@node
commitinfo' in doc/cvs.textinfo, there should be one for each of the two
new files you are adding.
> > It may not be argued that it is more complex to use multiple scripts and
> > possibly accidentally introduce different standards for entry into the
> > source base. I have a slight bias toward moving to a new importinfo
> > script to augment what already exists rather than overloading the
> > existing commitinfo with the added state caused by knowing that this is
> > an import over multiple directories and files with two new tags being
> > added to the repository... so, I have a slight bias in favor of your new
> > feature in principle over reworking the commitinfo script.
> Especially, if you change the existing commitinfo script you too
> easily could break lots of existing tools.
Yes, I find this to be a good argument. It remains to be seen if the
consensus of the others matches it.
> > One thing that does concern me is that the tags for a cvsimport do
> > not appear to get passed to either importinfo nor taginfo for checking.
> > If the importinfo feature is to really check what the user is doing, is
> > it not possible that the user is about to destroy a very important
> > release tag that already exists as a side-effect of their import?
> Why are the tags not passed? At least for my "importinfo", the
> vendor-tag is passed as an argument. Without this my OSSP Shiela
> facility would not be able to control imports to different branches.
Hmmm... I didn't notice this and it did not appear in the documentation
for how the file is used...
> > With regard to the admininfo trigger, I presume that users will either
> > need to be a part of the CVS_ADMIN_GROUP or there will need to be no
> > CVS_ADMIN_GROUP at all in order for the admininfo script to be run. In
> > addition, only those options permitted by UserAdminOptions will get
> > thru. Is that correct? If so, it probably deserves a mention in the
> > doc/cvs.texinfo file.
> Correct. My current "admininfo" is processed _after_ the CVS_ADMIN_GROUP
> stuff is processed.
Good. I thought this was the case, but it should be documented for
folks in the man page and cvs.texinfo node on the file.
> > Also, there are many discrete operations possible with 'cvs admin'
> > and they are very different. It is not clear to me how the script
> > would know that a user might be deleting a revision in the file
> > versus changing from -kkv to -kk keyword expansion mode or trying
> > to unlock a revision that had somehow been locked.
> That's a good point. I think we should pass the arguments
> to "cvs admin" through to the script. I'll see...
Excellent. That would make a lot of sense to me.
> > For example, might it be desirable to ensure that a replacement
> > log message (-mrev:msg) conformed to the same standards as one
> > that was given during a 'cvs commit' operation?
> Hmmm... my personal opinion here is that "cvs admin" is a low-level
> operation and hence (once passed the "admininfo" filters) should be
> able to do whatever is wished and does not have to conform to the same
> standards as higher-level operations. But that's just my point of
Sure, it is one point of view.
Right now the administrator is able to allow various of the switches to
be used by anyone regardless of membership in the CVS_ADMIN_GROUP. However,
if the administrator wanted to make some of the more powerful features
available, they still might want to verify basic uses of the features.
> > It might be that the cvs administrator would need/want the list
> > of switches being passed to the admininfo script. A good design
> > now might really give a cvs administrator more flexibility in
> > what they allow their users to do for themselves.
> Yes, we should pass the options on the "cvs admin" command line to the
> filter script.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.3 (FreeBSD)
-----END PGP SIGNATURE-----