Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 249761 - CVS should validate package.mask atoms in pre-commit.
Summary: CVS should validate package.mask atoms in pre-commit.
Status: RESOLVED INVALID
Alias: None
Product: Gentoo Linux
Classification: Unclassified
Component: [OLD] Development (show other bugs)
Hardware: All Linux
: High normal (vote)
Assignee: Alec Warner
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-12-04 03:55 UTC by Alec Warner
Modified: 2012-02-10 07:38 UTC (History)
5 users (show)

See Also:
Package list:
Runtime testing required: ---


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Alec Warner (RETIRED) archtester gentoo-dev Security 2008-12-04 03:55:44 UTC
Technical difficulty: trivial

When commiting package.mask files to CVS, a pre-commit script should validate all atoms.  This is trivial to implement via a script; I will write it if we are interested in doing this.

-Alec
Comment 1 Robin Johnson archtester Gentoo Infrastructure gentoo-dev Security 2008-12-04 04:22:30 UTC
Give me a script and I'll put it in place.
Comment 2 Robin Johnson archtester Gentoo Infrastructure gentoo-dev Security 2008-12-04 04:24:13 UTC
And it's checking STRICTLY syntax, not existence.
Comment 3 Brian Harring (RETIRED) gentoo-dev 2008-12-04 09:50:23 UTC
Do you want it to blow up if it's trying to revert something not in a parent profile?
Comment 4 Rémi Cardona (RETIRED) gentoo-dev 2008-12-04 10:40:17 UTC
What about a repoman script?
Comment 5 Ryan Hill (RETIRED) gentoo-dev 2008-12-04 19:12:06 UTC
repoman doesn't run in /profiles
Comment 6 Alec Warner (RETIRED) archtester gentoo-dev Security 2008-12-05 00:43:18 UTC
(In reply to comment #3)
> Do you want it to blow up if it's trying to revert something not in a parent
> profile?
> 

This is not an exhaustive check and it is not meant to ensure that what the developer wanted is actually occurring.  It is to prevent obvious cases of human error involving atoms (our most common p.mask screwup is typos).

The check is meant to be quick (not CPU intensive), easy to write, easy to deploy, and easy to maintain.

at a minimum it is :

from portage.SOMETHING import isvalidatom

f = open(pmask)
for line in f:
  line = line.strip()
  if line.startswith('#'):
    continue
  if not isvalidatom(line):
    return False

return True

Thats all I plan to do.

-Alec
Comment 7 Brian Harring (RETIRED) gentoo-dev 2008-12-05 01:33:33 UTC
(In reply to comment #6)
> This is not an exhaustive check and it is not meant to ensure that what the
> developer wanted is actually occurring.  It is to prevent obvious cases of
> human error involving atoms (our most common p.mask screwup is typos).
> 
> The check is meant to be quick (not CPU intensive), easy to write, easy to
> deploy, and easy to maintain.

That's not what I asked, although my questioning is re: crafting an exhaustive check.  The pkgcore implementation of profiles is fairly strict (meaning it could be used for checking any profile file instead of just package.mask) w/ a minor tweak, and weighing in around 10 lines I'd figure.

So.. should the check try and enforce that a node's attempted reversal of a parents masking is an error, or...?
Comment 8 Robin Johnson archtester Gentoo Infrastructure gentoo-dev Security 2008-12-05 02:17:40 UTC
ferringb: for these checks running on the server side, the CVS hooks run them in a temporary directory, with ONLY the new files in directory. They need to depend on no other files, because there is no specific guarantee that they will exist.

Eg, for the GDP guys, we have post-commit hooks that update the seperate copies of the DTDs on the CVS servers, so that they can use a pre-commit hook on the guidexml files.
Comment 9 Alec Warner (RETIRED) archtester gentoo-dev Security 2008-12-05 21:42:59 UTC
(In reply to comment #7)
> (In reply to comment #6)
> > This is not an exhaustive check and it is not meant to ensure that what the
> > developer wanted is actually occurring.  It is to prevent obvious cases of
> > human error involving atoms (our most common p.mask screwup is typos).
> > 
> > The check is meant to be quick (not CPU intensive), easy to write, easy to
> > deploy, and easy to maintain.
> 
> That's not what I asked, although my questioning is re: crafting an exhaustive
> check.  The pkgcore implementation of profiles is fairly strict (meaning it
> could be used for checking any profile file instead of just package.mask) w/ a
> minor tweak, and weighing in around 10 lines I'd figure.
> 
> So.. should the check try and enforce that a node's attempted reversal of a
> parents masking is an error, or...?

An explicit no to that question (in my planned implmentation).

>