[namcap] Treat all non-tar files as PKGBUILD files, regardless of filename
diff mbox

Message ID 20180125183145.9296-1-lukeshu@lukeshu.com
State New
Headers show

Commit Message

Luke Shumaker Jan. 25, 2018, 6:31 p.m. UTC
From: Luke Shumaker <lukeshu@parabola.nu>

https://bugs.archlinux.org/task/57193

If the user mistakenly passes in some other (non-package, non-PKGBUILD)
file, instead of refusing to process it (saying
"Error: Cannot process ${filename}"), it will now try to parse it as a
PKGBUILD, probably encounter and print at least one error, but finally say
"Error: ${filename} is not a valid PKGBUILD".  This is actually probably
*more* helpful to a confused user who passed in the wrong file.
---
 namcap.py | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

Comments

Dave Reisner Jan. 25, 2018, 6:50 p.m. UTC | #1
On Thu, Jan 25, 2018 at 01:31:45PM -0500, Luke Shumaker wrote:
> From: Luke Shumaker <lukeshu@parabola.nu>
> 
> https://bugs.archlinux.org/task/57193
> 
> If the user mistakenly passes in some other (non-package, non-PKGBUILD)
> file, instead of refusing to process it (saying
> "Error: Cannot process ${filename}"), it will now try to parse it as a
> PKGBUILD, probably encounter and print at least one error, but finally say
> "Error: ${filename} is not a valid PKGBUILD".  This is actually probably
> *more* helpful to a confused user who passed in the wrong file.
> ---
>  namcap.py | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/namcap.py b/namcap.py
> index b62a2fa..fd62d3c 100755
> --- a/namcap.py
> +++ b/namcap.py
> @@ -248,9 +248,7 @@ for package in packages:
>  
>  	if os.path.isfile(package) and tarfile.is_tarfile(package):
>  		process_realpackage(package, active_modules)
> -	elif package.endswith('PKGBUILD'):
> +	else:

Install files? Other assets? Doesn't this mean we assume they're
PKGBUILDs?

>  		process_pkgbuild(package, active_modules)
> -	else:
> -		print("Error: Cannot process %s" % package)
>  
>  # vim: set ts=4 sw=4 noet:
> -- 
> 2.15.1
Luke Shumaker Jan. 25, 2018, 9:41 p.m. UTC | #2
On Thu, 25 Jan 2018 13:50:19 -0500,
Dave Reisner wrote:
> On Thu, Jan 25, 2018 at 01:31:45PM -0500, Luke Shumaker wrote:
> > From: Luke Shumaker <lukeshu@parabola.nu>
> > 
> > https://bugs.archlinux.org/task/57193
> > 
> > If the user mistakenly passes in some other (non-package, non-PKGBUILD)
> > file, instead of refusing to process it (saying
> > "Error: Cannot process ${filename}"), it will now try to parse it as a
> > PKGBUILD, probably encounter and print at least one error, but finally say
> > "Error: ${filename} is not a valid PKGBUILD".  This is actually probably
> > *more* helpful to a confused user who passed in the wrong file.
> > ---
> >  namcap.py | 4 +---
> >  1 file changed, 1 insertion(+), 3 deletions(-)
> > 
> > diff --git a/namcap.py b/namcap.py
> > index b62a2fa..fd62d3c 100755
> > --- a/namcap.py
> > +++ b/namcap.py
> > @@ -248,9 +248,7 @@ for package in packages:
> >  
> >  	if os.path.isfile(package) and tarfile.is_tarfile(package):
> >  		process_realpackage(package, active_modules)
> > -	elif package.endswith('PKGBUILD'):
> > +	else:
> 
> Install files? Other assets? Doesn't this mean we assume they're
> PKGBUILDs?

Yes, it does.  It's already the case that running namcap on those
install files and other assets is an error.

Though, this will make those errors much more... verbose.

For example, running `namcap *` in pacman's package directory
currently looks like:

	$ namcap *
	PKGBUILD (pacman) W: Reference to i686 should be changed to $CARCH
	PKGBUILD (pacman) W: Reference to x86_64 should be changed to $CARCH
	PKGBUILD (pacman) W: Reference to i686 should be changed to $CARCH
	PKGBUILD (pacman) W: Reference to i686 should be changed to $CARCH
	PKGBUILD (pacman) W: Reference to i686 should be changed to $CARCH
	PKGBUILD (pacman) W: Reference to i686 should be changed to $CARCH
	PKGBUILD (pacman) W: Reference to x86_64 should be changed to $CARCH
	PKGBUILD (pacman) W: Reference to x86_64 should be changed to $CARCH
	PKGBUILD (pacman) W: Reference to x86_64 should be changed to $CARCH
	Error: Cannot process makepkg.conf
	Error: Cannot process pacman.conf.i686
	Error: Cannot process pacman.conf.x86_64

But this patch changes that to:

	$ namcap *
	PKGBUILD (pacman) W: Reference to i686 should be changed to $CARCH
	PKGBUILD (pacman) W: Reference to x86_64 should be changed to $CARCH
	PKGBUILD (pacman) W: Reference to i686 should be changed to $CARCH
	PKGBUILD (pacman) W: Reference to i686 should be changed to $CARCH
	PKGBUILD (pacman) W: Reference to i686 should be changed to $CARCH
	PKGBUILD (pacman) W: Reference to i686 should be changed to $CARCH
	PKGBUILD (pacman) W: Reference to x86_64 should be changed to $CARCH
	PKGBUILD (pacman) W: Reference to x86_64 should be changed to $CARCH
	PKGBUILD (pacman) W: Reference to x86_64 should be changed to $CARCH
	Error: error: invalid package file
	
	Error: makepkg.conf is not a valid PKGBUILD
	Error: error: invalid package file
	
	Error: pacman.conf.i686: line 9: [options]: command not found
	pacman.conf.i686: line 18: HoldPkg: command not found
	pacman.conf.i686: line 23: Architecture: command not found
	pacman.conf.i686: line 36: CheckSpace: command not found
	pacman.conf.i686: line 41: SigLevel: command not found
	pacman.conf.i686: line 42: LocalFileSigLevel: command not found
	pacman.conf.i686: line 75: [core]: command not found
	pacman.conf.i686: line 76: Include: command not found
	pacman.conf.i686: line 78: [extra]: command not found
	pacman.conf.i686: line 79: Include: command not found
	pacman.conf.i686: line 84: [community]: command not found
	pacman.conf.i686: line 85: Include: command not found
	
	Error: pacman.conf.i686 is not a valid PKGBUILD
	Error: error: invalid package file
	
	Error: pacman.conf.x86_64: line 9: [options]: command not found
	pacman.conf.x86_64: line 18: HoldPkg: command not found
	pacman.conf.x86_64: line 23: Architecture: command not found
	pacman.conf.x86_64: line 36: CheckSpace: command not found
	pacman.conf.x86_64: line 41: SigLevel: command not found
	pacman.conf.x86_64: line 42: LocalFileSigLevel: command not found
	pacman.conf.x86_64: line 75: [core]: command not found
	pacman.conf.x86_64: line 76: Include: command not found
	pacman.conf.x86_64: line 78: [extra]: command not found
	pacman.conf.x86_64: line 79: Include: command not found
	pacman.conf.x86_64: line 84: [community]: command not found
	pacman.conf.x86_64: line 85: Include: command not found
	
	Error: pacman.conf.x86_64 is not a valid PKGBUILD

There is a possible safety concern.  namcap will call `parsepkgbuild`
on the file, which does evaluate the file as a bash script; it is
conceivable that it this could accidentally do something bad,
depending on what the file is.  However, I don't believe this to be a
legitimate concern because of the rbash restricted environment that it
is executed in.
ashark@linuxcomp.ru April 16, 2018, 1:41 p.m. UTC | #3
<div>Why not just use "-p" for explicitly specifying that we want to parse file as a PKGBUILD?</div><div><br /></div><div><br /></div><div>26.01.2018, 00:41, "Luke Shumaker" &lt;lukeshu@lukeshu.com&gt;:</div><blockquote type="cite"><p>On Thu, 25 Jan 2018 13:50:19 -0500,<br />Dave Reisner wrote:<br /></p><blockquote> On Thu, Jan 25, 2018 at 01:31:45PM -0500, Luke Shumaker wrote:<br /> &gt; From: Luke Shumaker &lt;<a href="mailto:lukeshu@parabola.nu">lukeshu@parabola.nu</a>&gt;<br /> &gt; <br /> &gt; <a href="https://bugs.archlinux.org/task/57193">https://bugs.archlinux.org/task/57193</a><br /> &gt; <br /> &gt; If the user mistakenly passes in some other (non-package, non-PKGBUILD)<br /> &gt; file, instead of refusing to process it (saying<br /> &gt; "Error: Cannot process ${filename}"), it will now try to parse it as a<br /> &gt; PKGBUILD, probably encounter and print at least one error, but finally say<br /> &gt; "Error: ${filename} is not a valid PKGBUILD".  This is actually probably<br /> &gt; *more* helpful to a confused user who passed in the wrong file.<br /> &gt; ---<br /> &gt;  namcap.py | 4 +---<br /> &gt;  1 file changed, 1 insertion(+), 3 deletions(-)<br /> &gt; <br /> &gt; diff --git a/namcap.py b/namcap.py<br /> &gt; index b62a2fa..fd62d3c 100755<br /> &gt; --- a/namcap.py<br /> &gt; +++ b/namcap.py<br /> &gt; @@ -248,9 +248,7 @@ for package in packages:<br /> &gt;  <br /> &gt;  	if os.path.isfile(package) and tarfile.is_tarfile(package):<br /> &gt;  		process_realpackage(package, active_modules)<br /> &gt; -	elif package.endswith('PKGBUILD'):<br /> &gt; +	else:<br /><br /> Install files? Other assets? Doesn't this mean we assume they're<br /> PKGBUILDs?<br /></blockquote><p><br />Yes, it does.  It's already the case that running namcap on those<br />install files and other assets is an error.<br /><br />Though, this will make those errors much more... verbose.<br /><br />For example, running `namcap *` in pacman's package directory<br />currently looks like:<br /><br />        $ namcap *<br />        PKGBUILD (pacman) W: Reference to i686 should be changed to $CARCH<br />        PKGBUILD (pacman) W: Reference to x86_64 should be changed to $CARCH<br />        PKGBUILD (pacman) W: Reference to i686 should be changed to $CARCH<br />        PKGBUILD (pacman) W: Reference to i686 should be changed to $CARCH<br />        PKGBUILD (pacman) W: Reference to i686 should be changed to $CARCH<br />        PKGBUILD (pacman) W: Reference to i686 should be changed to $CARCH<br />        PKGBUILD (pacman) W: Reference to x86_64 should be changed to $CARCH<br />        PKGBUILD (pacman) W: Reference to x86_64 should be changed to $CARCH<br />        PKGBUILD (pacman) W: Reference to x86_64 should be changed to $CARCH<br />        Error: Cannot process makepkg.conf<br />        Error: Cannot process pacman.conf.i686<br />        Error: Cannot process pacman.conf.x86_64<br /><br />But this patch changes that to:<br /><br />        $ namcap *<br />        PKGBUILD (pacman) W: Reference to i686 should be changed to $CARCH<br />        PKGBUILD (pacman) W: Reference to x86_64 should be changed to $CARCH<br />        PKGBUILD (pacman) W: Reference to i686 should be changed to $CARCH<br />        PKGBUILD (pacman) W: Reference to i686 should be changed to $CARCH<br />        PKGBUILD (pacman) W: Reference to i686 should be changed to $CARCH<br />        PKGBUILD (pacman) W: Reference to i686 should be changed to $CARCH<br />        PKGBUILD (pacman) W: Reference to x86_64 should be changed to $CARCH<br />        PKGBUILD (pacman) W: Reference to x86_64 should be changed to $CARCH<br />        PKGBUILD (pacman) W: Reference to x86_64 should be changed to $CARCH<br />        Error: error: invalid package file<br /><br />        Error: makepkg.conf is not a valid PKGBUILD<br />        Error: error: invalid package file<br /><br />        Error: pacman.conf.i686: line 9: [options]: command not found<br />        pacman.conf.i686: line 18: HoldPkg: command not found<br />        pacman.conf.i686: line 23: Architecture: command not found<br />        pacman.conf.i686: line 36: CheckSpace: command not found<br />        pacman.conf.i686: line 41: SigLevel: command not found<br />        pacman.conf.i686: line 42: LocalFileSigLevel: command not found<br />        pacman.conf.i686: line 75: [core]: command not found<br />        pacman.conf.i686: line 76: Include: command not found<br />        pacman.conf.i686: line 78: [extra]: command not found<br />        pacman.conf.i686: line 79: Include: command not found<br />        pacman.conf.i686: line 84: [community]: command not found<br />        pacman.conf.i686: line 85: Include: command not found<br /><br />        Error: pacman.conf.i686 is not a valid PKGBUILD<br />        Error: error: invalid package file<br /><br />        Error: pacman.conf.x86_64: line 9: [options]: command not found<br />        pacman.conf.x86_64: line 18: HoldPkg: command not found<br />        pacman.conf.x86_64: line 23: Architecture: command not found<br />        pacman.conf.x86_64: line 36: CheckSpace: command not found<br />        pacman.conf.x86_64: line 41: SigLevel: command not found<br />        pacman.conf.x86_64: line 42: LocalFileSigLevel: command not found<br />        pacman.conf.x86_64: line 75: [core]: command not found<br />        pacman.conf.x86_64: line 76: Include: command not found<br />        pacman.conf.x86_64: line 78: [extra]: command not found<br />        pacman.conf.x86_64: line 79: Include: command not found<br />        pacman.conf.x86_64: line 84: [community]: command not found<br />        pacman.conf.x86_64: line 85: Include: command not found<br /><br />        Error: pacman.conf.x86_64 is not a valid PKGBUILD<br /><br />There is a possible safety concern.  namcap will call `parsepkgbuild`<br />on the file, which does evaluate the file as a bash script; it is<br />conceivable that it this could accidentally do something bad,<br />depending on what the file is.  However, I don't believe this to be a<br />legitimate concern because of the rbash restricted environment that it<br />is executed in.<br /><br /></p><span>-- <br />Happy hacking,<br />~ Luke Shumaker<br /></span></blockquote>

Patch
diff mbox

diff --git a/namcap.py b/namcap.py
index b62a2fa..fd62d3c 100755
--- a/namcap.py
+++ b/namcap.py
@@ -248,9 +248,7 @@  for package in packages:
 
 	if os.path.isfile(package) and tarfile.is_tarfile(package):
 		process_realpackage(package, active_modules)
-	elif package.endswith('PKGBUILD'):
+	else:
 		process_pkgbuild(package, active_modules)
-	else:
-		print("Error: Cannot process %s" % package)
 
 # vim: set ts=4 sw=4 noet: