info-cvs
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] importinfo/admininfo


From: Ralf S. Engelschall
Subject: Re: [PATCH] importinfo/admininfo
Date: Sat, 4 Oct 2003 22:24:23 +0200
User-agent: Mutt/1.4.1i

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 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
documented.

> [...]
> [I believe that Larry has suggested that a separate importinfo may not
> be needed and that the existing commitinfo and taginfo scripts should be
> extended. I will let Larry make his own comments concerning your patch.]
>
> My own opinion is that some administrators may wish to impose controls
> that are orthogonal to normal commits and having a different script
> allows for more flexibility to the cvs administrator.

Exactly, that's also my point of view.

> 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.

> 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.

> 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.

> 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...

> 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
view...

> 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.
                                       Ralf S. Engelschall
                                       address@hidden
                                       www.engelschall.com





reply via email to

[Prev in Thread] Current Thread [Next in Thread]