more work on untrusted committers

Wired up check_canedit and check_canremove, still need to deal with
check_canattach, and test.

8 files changed, 168 insertions, 47 deletions

diff --git a/IkiWiki.pm b/IkiWiki.pm
index 245eaafba..698244187 100644
--- a/IkiWiki.pm
+++ b/IkiWiki.pm
@@ -1582,8 +1582,8 @@ sub rcs_getctime ($) { #{{{
 	$hooks{rcs}{rcs_getctime}{call}->(@_);
 } #}}}
 
-sub rcs_test_receive ($) { #{{{
-	$hooks{rcs}{rcs_test_receive}{call}->(@_);
+sub rcs_receive ($) { #{{{
+	$hooks{rcs}{rcs_receive}{call}->(@_);
 } #}}}
 
 sub globlist_to_pagespec ($) { #{{{
diff --git a/IkiWiki/Plugin/editpage.pm b/IkiWiki/Plugin/editpage.pm
index 30c93df20..fe2864bac 100644
--- a/IkiWiki/Plugin/editpage.pm
+++ b/IkiWiki/Plugin/editpage.pm
@@ -122,7 +122,7 @@ sub cgi_editpage ($$) { #{{{
 	my $absolute=($page =~ s#^/+##);
 	if (! defined $page || ! length $page ||
 	    file_pruned($page, $config{srcdir})) {
-		error("bad page name");
+		error(gettext("bad page name"));
 	}
 
 	my $baseurl = urlto($page, undef, 1);
diff --git a/IkiWiki/Plugin/git.pm b/IkiWiki/Plugin/git.pm
index 1facb14c0..234e7af2e 100644
--- a/IkiWiki/Plugin/git.pm
+++ b/IkiWiki/Plugin/git.pm
@@ -23,7 +23,7 @@ sub import { #{{{
 	hook(type => "rcs", id => "rcs_recentchanges", call => \&rcs_recentchanges);
 	hook(type => "rcs", id => "rcs_diff", call => \&rcs_diff);
 	hook(type => "rcs", id => "rcs_getctime", call => \&rcs_getctime);
-	hook(type => "rcs", id => "rcs_test_receive", call => \&rcs_test_receive);
+	hook(type => "rcs", id => "rcs_receive", call => \&rcs_receive);
 } #}}}
 
 sub checkconfig () { #{{{
@@ -77,7 +77,7 @@ sub getsetup () { #{{{
 			safe => 0, # file
 			rebuild => 0,
 		},
-		git_untrusted_committers => {
+		untrusted_committers => {
 			type => "string",
 			example => [],
 			description => "unix users whose commits should be checked by the pre-receive hook",
@@ -588,15 +588,7 @@ sub rcs_getctime ($) { #{{{
 	return $ctime;
 } #}}}
 
-sub rcs_test_receive () { #{{{
-	# quick success if the user is trusted
-	my $committer=(getpwuid($<))[0];
-	if (! defined $committer) {
-		error("cannot determine username for $<");
-	}
-	exit 0 if ! ref $config{git_untrusted_committers} ||
-	          ! grep { $_ eq $committer } @{$config{git_untrusted_committers}};
-
+sub rcs_receive () { #{{{
 	# The wiki may not be the only thing in the git repo.
 	# Determine if it is in a subdirectory by examining the srcdir,
 	# and its parents, looking for the .git directory.
@@ -610,54 +602,64 @@ sub rcs_test_receive () { #{{{
 		}
 	}
 
-	my @errors;
+	my @rets;
 	while (<>) {
 		chomp;
 		my ($oldrev, $newrev, $refname) = split(' ', $_, 3);
 
 		# only allow changes to gitmaster_branch
 		if ($refname !~ /^refs\/heads\/\Q$config{gitmaster_branch}\E$/) {
-			push @errors, sprintf(gettext("you are not allowed to change %s"), $refname);
+			error sprintf(gettext("you are not allowed to change %s"), $refname);
 		}
 
 		foreach my $ci (git_commit_info($oldrev."..".$newrev)) {
 			foreach my $detail (@{ $ci->{'details'} }) {
 				my $file = $detail->{'file'};
 
-				# check that all changed files are in the subdir
+				# check that all changed files are in the
+				# subdir
 				if (length $subdir &&
 				    ! ($file =~ s/^\Q$subdir\E//)) {
-					push @errors, sprintf(gettext("you are not allowed to change %s"), $file);
-					next;
+					error sprintf(gettext("you are not allowed to change %s"), $file);
 				}
 
-				if ($detail->{'mode_from'} ne $detail->{'mode_to'}) {
-					push @errors, gettext("you are not allowed to change file modes");
+				my $action;
+				my $mode;			
+				if ($detail->{'status'} =~ /^[M]+\d*$/) {
+					$action="change";
+					$mode=$detail->{'mode_to'};
 				}
-
-				if ($detail->{'status'} =~ /^D+\d*/) {
-					# TODO check_canremove
+				elsif ($detail->{'status'} =~ /^[AM]+\d*$/) {
+					$action="add";
+					$mode=$detail->{'mode_to'};
 				}
-				elsif ($detail->{'status'} !~ /^[MA]+\d*$/) {
-					push @errors, "unknown status ".$detail->{'status'};
+				elsif ($detail->{'status'} =~ /^[DAM]+\d*/) {
+					$action="remove";
+					$mode=$detail->{'mode_from'};
 				}
 				else {
-					# TODO check_canedit
-					# TODO check_canattach
+					error "unknown status ".$detail->{'status'};
 				}
+				
+				# test that the file mode is ok
+				if ($mode !~ /^100[64][64][64]$/) {
+					error sprintf(gettext("you cannot act on a file with mode %s"), $mode);
+				}
+				if ($action eq "change") {
+					if ($detail->{'mode_from'} ne $detail->{'mode_to'}) {
+						error gettext("you are not allowed to change file modes");
+					}
+				}
+
+				push @rets, {
+					file => $file,
+					action => $action,
+				};
 			}
 		}
 	}
 
-	if (@errors) {
-		# TODO clean up objects from failed push
-
-		print STDERR "$_\n" foreach @errors;
-		exit 1;
-	}
-	else {
-		exit 0;
-	}
+	return @rets;
 } #}}}
 
 1
diff --git a/IkiWiki/Plugin/remove.pm b/IkiWiki/Plugin/remove.pm
index 68bf9d1ee..c512b3b97 100644
--- a/IkiWiki/Plugin/remove.pm
+++ b/IkiWiki/Plugin/remove.pm
@@ -41,7 +41,7 @@ sub check_canremove ($$$) { #{{{
 		error(sprintf(gettext("%s is not a file"), $file));
 	}
 	
-	# Must be editiable.
+	# Must be editable.
 	IkiWiki::check_canedit($page, $q, $session);
 
 	# If a user can't upload an attachment, don't let them delete it.
diff --git a/IkiWiki/Receive.pm b/IkiWiki/Receive.pm
new file mode 100644
index 000000000..63944bb81
--- /dev/null
+++ b/IkiWiki/Receive.pm
@@ -0,0 +1,101 @@
+#!/usr/bin/perl
+
+package IkiWiki::Receive;
+
+use warnings;
+use strict;
+use IkiWiki;
+
+sub getuser () { #{{{
+	my $user=(getpwuid($<))[0];
+	if (! defined $user) {
+		error("cannot determine username for $<");
+	}
+	return $user;
+} #}}}
+
+sub trusted () { #{{{
+	my $user=getuser();
+	return ! ref $config{untrusted_committers} ||
+		! grep { $_ eq $user } @{$config{untrusted_committers}};
+} #}}}
+
+sub test () { #{{{
+	exit 0 if trusted();
+	IkiWiki::rcs_test_receive();
+	
+	# Dummy up a cgi environment to use when calling check_canedit
+	# and friends.
+	eval q{use CGI};
+	error($@) if $@;
+	my $cgi=CGI->new;
+	require IkiWiki::CGI;
+	my $session=IkiWiki::cgi_getsession($cgi);
+	my $user=getuser();
+	$session->param("name", $user);
+	$ENV{REMOTE_ADDR}='unknown' unless exists $ENV{REMOTE_ADDR};
+
+	lockwiki();
+	loadindex();
+
+	my %newfiles;
+
+	foreach my $change (IkiWiki::rcs_receive()) {
+		# This untaint is safe because we check file_pruned and
+		# wiki_file_regexp.
+		my $file=$change->{file}=~/$config{wiki_file_regexp}/;
+		$file=possibly_foolish_untaint($file);
+		if (! defined $file || ! length $file ||
+		    IkiWiki::file_pruned($file, $config{srcdir})) {
+			error(gettext("bad file name"));
+		}
+
+		my $type=pagetype($file);
+		my $page=pagename($file) if defined $type;
+		
+		if ($change->{action} eq 'add') {
+			$newfiles{$file}=1;
+		}
+
+		if ($change->{action} eq 'change' ||
+		    $change->{action} eq 'add') {
+			if (defined $page) {
+				if (IkiWiki->can("check_canedit") &&
+				    IkiWiki::check_canedit($page, $cgi, $session)) {
+				    	next;
+				}
+			}
+			else {
+				# TODO
+				#if (IkiWiki::Plugin::attachment->can("check_canattach") &&
+				#    IkiWiki::Plugin::attachment::check_canattach($session, $file, $path)) {
+				#    	next;
+				#}
+			}
+		}
+		elsif ($change->{action} eq 'remove') {
+			# check_canremove tests to see if the file is present
+			# on disk. This will fail is a single commit adds a
+			# file and then removes it again. Avoid the problem
+			# by not testing the removal in such pairs of changes.
+			# (The add is still tested, just to make sure that
+			# no data is added to the repo that a web edit
+			# could add.)
+			next if $newfiles{$file};
+
+			if (IkiWiki::Plugin::remove->can("check_canremove") &&
+			    IkiWiki::Plugin::remove::check_canremove(defined $page ? $page : $file, $cgi, $session)) {
+				next;
+			}
+		}
+		else {
+			error "unknown action ".$change->{action};
+		}
+				
+		error sprintf(gettext("you are not allowed to change %s"), $file);
+	}
+
+	exit 0;
+} #}}}
+
+1
diff --git a/doc/plugins/write.mdwn b/doc/plugins/write.mdwn
index 5a5db6be0..9f096e4f7 100644
--- a/doc/plugins/write.mdwn
+++ b/doc/plugins/write.mdwn
@@ -820,14 +820,26 @@ it up in the history.
 
 It's ok if this is not implemented, and throws an error.
 
-#### `rcs_test_receive()`
+#### `rcs_receive()`
 
-This is used to test if changes pushed into the RCS should be accepted.
-Ikiwiki will be running as a pre-receive hook (or equivilant) and should
-examine the incoming changes, decide if they are allowed, and communicate
-that to the RCS.
+This is called when ikiwiki is running as a pre-receive hook (or
+equivilant), and is testing if changes pushed into the RCS from an
+untrusted user should be accepted. This is optional, and doesn't make
+sense to implement for all RCSs.
 
-This is optional, and doesn't make sense for all RCSs.
+It should examine the incoming changes, and do any sanity 
+checks that are appropriate for the RCS to limit changes to safe file adds,
+removes, and renames. If something bad is found, it should exit
+nonzero, to abort the push. Otherwise, it should return a list of
+files that were changed, in the form:
+
+	{
+		file => # name of file that was changed
+		action => # either "add", "change", or "remove"
+	}
+
+The list will then be checked to make sure that each change is one that
+is allowed to be made via the web interface.
 
 ### PageSpec plugins
 
diff --git a/doc/rcs/git.mdwn b/doc/rcs/git.mdwn
index 2a6feecf5..6ba0da894 100644
--- a/doc/rcs/git.mdwn
+++ b/doc/rcs/git.mdwn
@@ -116,7 +116,7 @@ committers. Trusted committers, including the user that ikiwiki runs as,
 will not have their commits checked by the `pre-receive` hook. Untrusted
 committers will have their commits checked. The configuration settings to
 enable are `git_test_receive_wrapper`, which enables generation of a
-`pre-receive` hook, and `git_untrusted_committers`, which is a list of
+`pre-receive` hook, and `untrusted_committers`, which is a list of
 usernames of the untrusted committers.
 
 Note that when the `pre-receive` hook is checking incoming changes, it
diff --git a/ikiwiki.in b/ikiwiki.in
index 22addb463..60663bc89 100755
--- a/ikiwiki.in
+++ b/ikiwiki.in
@@ -119,10 +119,15 @@ sub getconfig () { #{{{
 		}
 		delete $ENV{WRAPPED_OPTIONS};
 
-		# optimisation for no-op post_commit 
 		if ($config{post_commit} && ! commit_hook_enabled()) {
+			# optimisation for no-op post_commit 
 			exit 0;
 		}
+		elsif ($config{test_receive}) {
+			# quick success if the user is trusted
+			require IkiWiki::Receive;
+			exit 0 if IkiWiki::Receive::trusted();
+		}
 
 		loadplugins();
 		checkconfig();
@@ -190,7 +195,8 @@ sub main () { #{{{
 		# do nothing
 	}
 	elsif ($config{test_receive}) {
-		rcs_test_receive();
+		require IkiWiki::Receive;
+		IkiWiki::Receive::test();
 	}
 	else {
 		if ($config{rebuild}) {